Patch Detail
Show a patch.
GET /api/1.1/patches/16716/?format=api
{ "id": 16716, "url": "https://patchwork.libcamera.org/api/1.1/patches/16716/?format=api", "web_url": "https://patchwork.libcamera.org/patch/16716/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20220721121310.1286862-5-kieran.bingham@ideasonboard.com>", "date": "2022-07-21T12:13:02", "name": "[libcamera-devel,RFC,04/12] ipa: ipu3: Move the Frame Context queue to libipa", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "7fce69180b0c05da23b0dd888e1271e9ec8ee81e", "submitter": { "id": 4, "url": "https://patchwork.libcamera.org/api/1.1/people/4/?format=api", "name": "Kieran Bingham", "email": "kieran.bingham@ideasonboard.com" }, "delegate": { "id": 11, "url": "https://patchwork.libcamera.org/api/1.1/users/11/?format=api", "username": "kbingham", "first_name": "Kieran", "last_name": "Bingham", "email": "kieran.bingham@ideasonboard.com" }, "mbox": "https://patchwork.libcamera.org/patch/16716/mbox/", "series": [ { "id": 3313, "url": "https://patchwork.libcamera.org/api/1.1/series/3313/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3313", "date": "2022-07-21T12:12:58", "name": "libcamera: Align IPU3 and RKISP1 interfaces", "version": 1, "mbox": "https://patchwork.libcamera.org/series/3313/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/16716/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/16716/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 919E7C3274\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 21 Jul 2022 12:13:23 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6A9B26330F;\n\tThu, 21 Jul 2022 14:13:21 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A2FF363310\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 21 Jul 2022 14:13:14 +0200 (CEST)", "from Monstersaurus.local\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 45AE512F3;\n\tThu, 21 Jul 2022 14:13:14 +0200 (CEST)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658405601;\n\tbh=8pIoV3OSe56tFqhGsij1IN86VZn7OmRaT4osmx2JqsE=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=Seu9IJQtFtaBNFLs3ZHM3MGHDyE69qPaSWqpkzLQUaHWQvZRpRN1GcYIWhkY/r+LK\n\tFXSI2l5jWKijcQKIIrkZc8r0Rd2NskfWiIYsAE4fR4/DiqgBrFbrVNW3s0yVMaRcDO\n\tjXEpB2AhJGRzunt/DM8e5EP+ZhuEuSikkOEUhWOSUKl4m8KzD856lvI9JRPe8ozwWq\n\te2oSYn6yDheb1fLYPpDtDriKQrYnwMIhwcdh91SeVpbXEIiJ5S5V12liqI3adC62ZI\n\tAputkYmscsRmq3kdpbq+0pcz2mwIdSDoalZ43X1nsXX88/7OxV6qOd6sGSf3lbvMB4\n\t5PrVBJXD66cJg==", "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658405594;\n\tbh=8pIoV3OSe56tFqhGsij1IN86VZn7OmRaT4osmx2JqsE=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=BmHTd2AvCgPi1FBCQkckIKUBAaDDCwsUUj2XIivcmV8gmM6o9KeVYoiaL3sYKZoKT\n\ty5yKwDjqkaOzNBkaSfxvHkf6Mn19kAU2BXQkFM4OWdbZwgch4meF/Z2WXR7PsYQJtD\n\tDbmSlYQvHOMOGdjAdOG7ip/SCkxFTRQFuVKkX4UU=" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"BmHTd2Av\"; dkim-atps=neutral", "To": "libcamera devel <libcamera-devel@lists.libcamera.org>", "Date": "Thu, 21 Jul 2022 13:13:02 +0100", "Message-Id": "<20220721121310.1286862-5-kieran.bingham@ideasonboard.com>", "X-Mailer": "git-send-email 2.34.1", "In-Reply-To": "<20220721121310.1286862-1-kieran.bingham@ideasonboard.com>", "References": "<20220721121310.1286862-1-kieran.bingham@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [RFC PATCH 04/12] ipa: ipu3: Move the Frame\n\tContext queue to libipa", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "From": "Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>", "Reply-To": "Kieran Bingham <kieran.bingham@ideasonboard.com>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "From: Umang Jain <umang.jain@ideasonboard.com>\n\nProvide a direct FCQueue abstraction to facilitate creating a queue of\nFrame Context structures.\n\nThe IPAFrameContext is moved back to the ipu3/ipa_context.h and a queue\nadded to the IPU3, but the Algorithms are not yet moved from the single\nFrameContext exposed by the IPA context to use the correct one from the\nqueue until the Algorithm interfaces are fully updated.\n\nSigned-off-by: Umang Jain <umang.jain@ideasonboard.com>\nSigned-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n---\n src/ipa/ipu3/ipa_context.cpp | 17 -------\n src/ipa/ipu3/ipa_context.h | 15 ++----\n src/ipa/ipu3/ipu3.cpp | 13 +++--\n src/ipa/libipa/fc_queue.cpp | 96 ++++++++++++++++++++++++++++++++++++\n src/ipa/libipa/fc_queue.h | 93 ++++++++++++++++++++++++++++++++++\n src/ipa/libipa/meson.build | 2 +\n 6 files changed, 204 insertions(+), 32 deletions(-)\n create mode 100644 src/ipa/libipa/fc_queue.cpp\n create mode 100644 src/ipa/libipa/fc_queue.h", "diff": "diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\nindex 13cdb835ef7f..dd139cb4c868 100644\n--- a/src/ipa/ipu3/ipa_context.cpp\n+++ b/src/ipa/ipu3/ipa_context.cpp\n@@ -180,27 +180,10 @@ namespace libcamera::ipa::ipu3 {\n * <linux/intel-ipu3.h> struct ipu3_uapi_gamma_corr_lut for further details.\n */\n \n-/**\n- * \\brief Default constructor for IPAFrameContext\n- */\n-IPAFrameContext::IPAFrameContext() = default;\n-\n-/**\n- * \\brief Construct a IPAFrameContext instance\n- */\n-IPAFrameContext::IPAFrameContext(uint32_t id, const ControlList &reqControls)\n-\t: frame(id), frameControls(reqControls)\n-{\n-\tsensor = {};\n-}\n-\n /**\n * \\var IPAFrameContext::frame\n * \\brief The frame number\n *\n- * \\var IPAFrameContext::frameControls\n- * \\brief Controls sent in by the application while queuing the request\n- *\n * \\var IPAFrameContext::sensor\n * \\brief Effective sensor values that were applied for the frame\n *\ndiff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\nindex 42e11141d3a1..193fc44a821a 100644\n--- a/src/ipa/ipu3/ipa_context.h\n+++ b/src/ipa/ipu3/ipa_context.h\n@@ -8,8 +8,6 @@\n \n #pragma once\n \n-#include <array>\n-\n #include <linux/intel-ipu3.h>\n \n #include <libcamera/base/utils.h>\n@@ -17,13 +15,12 @@\n #include <libcamera/controls.h>\n #include <libcamera/geometry.h>\n \n+#include <libipa/fc_queue.h>\n+\n namespace libcamera {\n \n namespace ipa::ipu3 {\n \n-/* Maximum number of frame contexts to be held */\n-static constexpr uint32_t kMaxFrameContexts = 16;\n-\n struct IPASessionConfiguration {\n \tstruct {\n \t\tipu3_uapi_grid_config bdsGrid;\n@@ -77,23 +74,19 @@ struct IPAActiveState {\n };\n \n struct IPAFrameContext {\n-\tIPAFrameContext();\n-\tIPAFrameContext(uint32_t id, const ControlList &reqControls);\n+\tuint32_t frame;\n \n \tstruct {\n \t\tuint32_t exposure;\n \t\tdouble gain;\n \t} sensor;\n-\n-\tuint32_t frame;\n-\tControlList frameControls;\n };\n \n struct IPAContext {\n \tIPASessionConfiguration configuration;\n \tIPAActiveState activeState;\n \n-\tstd::array<IPAFrameContext, kMaxFrameContexts> frameContexts;\n+\tFCQueue<IPAFrameContext> frameContexts;\n };\n \n } /* namespace ipa::ipu3 */\ndiff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\nindex 2f6bb672f7bb..55e82cd404f4 100644\n--- a/src/ipa/ipu3/ipu3.cpp\n+++ b/src/ipa/ipu3/ipu3.cpp\n@@ -38,6 +38,8 @@\n #include \"algorithms/tone_mapping.h\"\n #include \"libipa/camera_sensor_helper.h\"\n \n+#include \"ipa_context.h\"\n+\n /* Minimum grid width, expressed as a number of cells */\n static constexpr uint32_t kMinGridWidth = 16;\n /* Maximum grid width, expressed as a number of cells */\n@@ -456,8 +458,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n \n \t/* Clean IPAActiveState at each reconfiguration. */\n \tcontext_.activeState = {};\n-\tIPAFrameContext initFrameContext;\n-\tcontext_.frameContexts.fill(initFrameContext);\n+\tcontext_.frameContexts.clear();\n \n \tif (!validateSensorControls()) {\n \t\tLOG(IPAIPU3, Error) << \"Sensor control validation failed.\";\n@@ -569,7 +570,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n \tconst ipu3_uapi_stats_3a *stats =\n \t\treinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());\n \n-\tIPAFrameContext &frameContext = context_.frameContexts[frame % kMaxFrameContexts];\n+\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n \n \tif (frameContext.frame != frame)\n \t\tLOG(IPAIPU3, Warning) << \"Frame \" << frame << \" does not match its frame context\";\n@@ -618,7 +619,11 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)\n {\n \t/* \\todo Start processing for 'frame' based on 'controls'. */\n-\tcontext_.frameContexts[frame % kMaxFrameContexts] = { frame, controls };\n+\tIPAFrameContext &frameContext = context_.frameContexts.initialise(frame);\n+\n+\t/* \\todo Implement queueRequest to each algorithm. */\n+\t(void)frameContext;\n+\t(void)controls;\n }\n \n /**\ndiff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp\nnew file mode 100644\nindex 000000000000..ecb47f287350\n--- /dev/null\n+++ b/src/ipa/libipa/fc_queue.cpp\n@@ -0,0 +1,96 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2022, Google Inc.\n+ *\n+ * fc_queue.cpp - IPA Frame context queue\n+ */\n+\n+#include \"fc_queue.h\"\n+\n+#include <libcamera/base/log.h>\n+\n+namespace libcamera {\n+\n+LOG_DEFINE_CATEGORY(FCQueue)\n+\n+namespace ipa {\n+\n+/**\n+ * \\file fc_queue.h\n+ * \\brief Queue to access per-frame Context\n+ */\n+\n+/**\n+ * \\class FCQueue\n+ * \\brief A support class for queueing Frame Context instances through the IPA\n+ * \\tparam FrameContext The IPA specific Frame Context type for this queue\n+ *\n+ * The Frame Context Queue provides a simple circular buffer implementation to\n+ * store IPA specific context for each frame through its lifetime within the\n+ * IPA.\n+ *\n+ * FrameContexts are expected to be referenced by a monotonically increasing\n+ * sequence count referring to a Frame sequence.\n+ *\n+ * A FrameContext is initialised for a given frame when the corresponding\n+ * Request is first queued into the IPA. From that point on the FrameContext can\n+ * be obtained by the IPA and its algorithms by referencing it from the frame\n+ * sequence number.\n+ *\n+ * A frame sequence number from the image source must correspond to the request\n+ * sequence number for this implementation to be supported in an IPA. It is\n+ * expected that the same sequence number will be used to reference the context\n+ * of the Frame from the point of queueing the request, specifying controls for\n+ * a given frame, and processing of any ISP related controls and statistics for\n+ * the same corresponding image.\n+ *\n+ * IPA specific FrameContexts should inherit from the IPAFrameContext to support\n+ * the minimum required features for a FrameContext, including the frame number\n+ * and error flags that relate to the frame.\n+ *\n+ * FrameContexts are overwritten when they are recycled and re-initialised by\n+ * the first access made on them by either initialise(frame) or get(frame). If a\n+ * FrameContext is first accessed through get(frame) before calling initialise()\n+ * a PFCError is automatically raised on the FrameContext to be transferred to\n+ * the Request when it completes.\n+ */\n+\n+/**\n+ * \\fn FCQueue::clear()\n+ * \\brief Reinitialise all data on the queue\n+ *\n+ * Reset the queue and ensure all FrameContext slots are re-initialised.\n+ */\n+\n+/**\n+ * \\fn FCQueue::initialise(uint32_t frame)\n+ * \\brief Initialize and return the Frame Context at slot specified by \\a frame\n+ * \\param[in] frame The frame context sequence number\n+ *\n+ * The first call to obtain a FrameContext from the FCQueue should be handled\n+ * through this call. The FrameContext will be initialised for the frame and\n+ * returned to the caller if it was not already initialised.\n+ *\n+ * If the FrameContext was already initialized for this sequence, a warning will\n+ * be reported and the previously initialized FrameContext is returned.\n+ *\n+ * \\return A reference to the FrameContext for sequence \\a frame\n+ */\n+\n+/**\n+ * \\fn FCQueue::get()\n+ * \\brief Obtain the Frame Context at slot specified by \\a frame\n+ * \\param[in] frame The frame context sequence number\n+ *\n+ * Obtains an existing FrameContext from the queue and returns it to the caller.\n+ *\n+ * If the FrameContext is not correctly intiialised for the \\a frame, it will be\n+ * initialised and a PFCError will be flagged on the context to be transferred\n+ * to the Request when it completes.\n+ *\n+ * \\return A reference to the FrameContext for sequence \\a frame\n+ */\n+\n+} /* namespace ipa */\n+\n+} /* namespace libcamera */\ndiff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h\nnew file mode 100644\nindex 000000000000..82ce36811926\n--- /dev/null\n+++ b/src/ipa/libipa/fc_queue.h\n@@ -0,0 +1,93 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2022, Google Inc.\n+ *\n+ * fc_queue.h - IPA Frame context queue\n+ *\n+ */\n+\n+#pragma once\n+\n+#include <array>\n+\n+#include <libcamera/base/log.h>\n+\n+#include <libcamera/controls.h>\n+\n+namespace libcamera {\n+\n+LOG_DECLARE_CATEGORY(FCQueue)\n+\n+namespace ipa {\n+\n+/*\n+ * Maximum number of frame contexts to be held onto\n+ *\n+ * \\todo Should be larger than ISP delay + sensor delay\n+ */\n+static constexpr uint32_t kMaxFrameContexts = 16;\n+\n+template<typename FrameContext>\n+class FCQueue : public std::array<FrameContext, kMaxFrameContexts>\n+{\n+private:\n+\tvoid initialise(FrameContext &frameContext, const uint32_t frame)\n+\t{\n+\t\tframeContext = {};\n+\t\tframeContext.frame = frame;\n+\t}\n+\n+public:\n+\tvoid clear()\n+\t{\n+\t\tthis->fill({});\n+\t}\n+\n+\tFrameContext &initialise(const uint32_t frame)\n+\t{\n+\t\tFrameContext &frameContext = this->at(frame & kMaxFrameContexts);\n+\n+\t\t/*\n+\t\t * Do not re-initialise if a get() call has already fetched this\n+\t\t * frame context to preseve the error flags already raised.\n+\t\t */\n+\t\tif (frame == frameContext.frame && frame != 0) {\n+\t\t\tLOG(FCQueue, Warning)\n+\t\t\t\t<< \"Frame \" << frame << \" already initialised\";\n+\t\t} else {\n+\t\t\tinitialise(frameContext, frame);\n+\t\t}\n+\n+\t\treturn frameContext;\n+\t}\n+\n+\tFrameContext &get(uint32_t frame)\n+\t{\n+\t\tFrameContext &frameContext = this->at(frame & kMaxFrameContexts);\n+\n+\t\tif (frame != frameContext.frame) {\n+\t\t\tLOG(FCQueue, Warning)\n+\t\t\t\t<< \"Obtained an uninitialised FrameContext for \"\n+\t\t\t\t<< frame;\n+\n+\t\t\tinitialise(frameContext, frame);\n+\n+\t\t\t/*\n+\t\t\t * The frame context has been retrieved before it was\n+\t\t\t * initialised through the initialise() call. This\n+\t\t\t * indicates an algorithm attempted to access a Frame\n+\t\t\t * context before it was queued to the IPA.\n+\t\t\t *\n+\t\t\t * Controls applied for this request may be left\n+\t\t\t * unhandled.\n+\t\t\t */\n+\t\t\tframeContext.error |= Request::PFCError;\n+\t\t}\n+\n+\t\treturn frameContext;\n+\t}\n+};\n+\n+} /* namespace ipa */\n+\n+} /* namespace libcamera */\ndiff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\nindex fb894bc614af..016b8e0ec9be 100644\n--- a/src/ipa/libipa/meson.build\n+++ b/src/ipa/libipa/meson.build\n@@ -3,6 +3,7 @@\n libipa_headers = files([\n 'algorithm.h',\n 'camera_sensor_helper.h',\n+ 'fc_queue.h',\n 'histogram.h',\n 'module.h',\n ])\n@@ -10,6 +11,7 @@ libipa_headers = files([\n libipa_sources = files([\n 'algorithm.cpp',\n 'camera_sensor_helper.cpp',\n+ 'fc_queue.cpp',\n 'histogram.cpp',\n 'module.cpp',\n ])\n", "prefixes": [ "libcamera-devel", "RFC", "04/12" ] }