Patch Detail
Show a patch.
GET /api/1.1/patches/13288/?format=api
{ "id": 13288, "url": "https://patchwork.libcamera.org/api/1.1/patches/13288/?format=api", "web_url": "https://patchwork.libcamera.org/patch/13288/", "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": "<20210810151211.56702-5-jacopo@jmondi.org>", "date": "2021-08-10T15:12:10", "name": "[libcamera-devel,v4,4/5] libcamera: ipu3: Initialize controls in the IPA", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "6ee4778ddfadbac23fe65b1d18253dc6bff32dde", "submitter": { "id": 3, "url": "https://patchwork.libcamera.org/api/1.1/people/3/?format=api", "name": "Jacopo Mondi", "email": "jacopo@jmondi.org" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/13288/mbox/", "series": [ { "id": 2334, "url": "https://patchwork.libcamera.org/api/1.1/series/2334/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2334", "date": "2021-08-10T15:12:06", "name": "libcamera: Initialize controls in the IPA", "version": 4, "mbox": "https://patchwork.libcamera.org/series/2334/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/13288/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/13288/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 3C95BBD87D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 10 Aug 2021 15:11:32 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F12C668891;\n\tTue, 10 Aug 2021 17:11:31 +0200 (CEST)", "from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 546856888C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 10 Aug 2021 17:11:29 +0200 (CEST)", "(Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id 93C50200004;\n\tTue, 10 Aug 2021 15:11:28 +0000 (UTC)" ], "From": "Jacopo Mondi <jacopo@jmondi.org>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Tue, 10 Aug 2021 17:12:10 +0200", "Message-Id": "<20210810151211.56702-5-jacopo@jmondi.org>", "X-Mailer": "git-send-email 2.32.0", "In-Reply-To": "<20210810151211.56702-1-jacopo@jmondi.org>", "References": "<20210810151211.56702-1-jacopo@jmondi.org>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v4 4/5] libcamera: ipu3: Initialize\n\tcontrols in the IPA", "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>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "All the IPU3 Camera controls are currently initialized by the pipeline\nhandler which initializes them using the camera sensor configuration and\nplatform specific requirements.\n\nHowever, some controls are better initialized by the IPA, which might,\nin example, cap the exposure times and frame duration to the constraints\nof its algorithms implementation.\n\nAlso, moving forward, the IPA should register controls to report its\ncapabilities, in example the ability to enable/disable 3A algorithms on\nrequest.\n\nMove the existing controls initialization to the IPA, by providing\nthe sensor configuration and its controls to the IPU3IPA::init()\nfunction, which initializes controls and returns them to the pipeline\nthrough an output parameter.\n\nThe existing controls initialization has been copied verbatim from the\npipeline handler to the IPA, if not a for few line breaks adjustments\nand the resulting Camera controls values are not changed.\n\nSigned-off-by: Jacopo Mondi <jacopo@jmondi.org>\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/ipa/ipu3.mojom | 5 +-\n src/ipa/ipu3/ipu3.cpp | 72 ++++++++++++++++++++-\n src/libcamera/pipeline/ipu3/ipu3.cpp | 95 ++++++++++++----------------\n 3 files changed, 114 insertions(+), 58 deletions(-)", "diff": "diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom\nindex 911a3a072464..d561c2244907 100644\n--- a/include/libcamera/ipa/ipu3.mojom\n+++ b/include/libcamera/ipa/ipu3.mojom\n@@ -38,7 +38,10 @@ struct IPAConfigInfo {\n };\n \n interface IPAIPU3Interface {\n-\tinit(libcamera.IPASettings settings) => (int32 ret);\n+\tinit(libcamera.IPASettings settings,\n+\t libcamera.IPACameraSensorInfo sensorInfo,\n+\t libcamera.ControlInfoMap sensorControls)\n+\t\t=> (int32 ret, libcamera.ControlInfoMap ipaControls);\n \tstart() => (int32 ret);\n \tstop();\n \ndiff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\nindex c903f7518983..36a069661702 100644\n--- a/src/ipa/ipu3/ipu3.cpp\n+++ b/src/ipa/ipu3/ipu3.cpp\n@@ -5,7 +5,9 @@\n * ipu3.cpp - IPU3 Image Processing Algorithms\n */\n \n+#include <array>\n #include <stdint.h>\n+#include <utility>\n \n #include <linux/intel-ipu3.h>\n #include <linux/v4l2-controls.h>\n@@ -37,7 +39,11 @@ namespace ipa::ipu3 {\n class IPAIPU3 : public IPAIPU3Interface\n {\n public:\n-\tint init(const IPASettings &settings) override;\n+\tint init(const IPASettings &settings,\n+\t\t const IPACameraSensorInfo &sensorInfo,\n+\t\t const ControlInfoMap &sensorControls,\n+\t\t ControlInfoMap *ipaControls) override;\n+\n \tint start() override;\n \tvoid stop() override {}\n \n@@ -85,14 +91,74 @@ private:\n \tstruct ipu3_uapi_grid_config bdsGrid_;\n };\n \n-int IPAIPU3::init(const IPASettings &settings)\n+/**\n+ * Initialize the IPA module and its controls.\n+ *\n+ * This function receives the camera sensor information from the pipeline\n+ * handler, computes the limits of the controls it handles and returns\n+ * them in the \\a ipaControls output parameter.\n+ */\n+int IPAIPU3::init(const IPASettings &settings,\n+\t\t const IPACameraSensorInfo &sensorInfo,\n+\t\t const ControlInfoMap &sensorControls,\n+\t\t ControlInfoMap *ipaControls)\n {\n \tcamHelper_ = CameraSensorHelperFactory::create(settings.sensorModel);\n \tif (camHelper_ == nullptr) {\n-\t\tLOG(IPAIPU3, Error) << \"Failed to create camera sensor helper for \" << settings.sensorModel;\n+\t\tLOG(IPAIPU3, Error)\n+\t\t\t<< \"Failed to create camera sensor helper for \"\n+\t\t\t<< settings.sensorModel;\n \t\treturn -ENODEV;\n \t}\n \n+\t/* Initialize Controls. */\n+\tControlInfoMap::Map controls{};\n+\n+\t/*\n+\t * Compute exposure time limits.\n+\t *\n+\t * Initialize the control using the line length and pixel rate of the\n+\t * current configuration converted to microseconds. Use the\n+\t * V4L2_CID_EXPOSURE control to get exposure min, max and default and\n+\t * convert it from lines to microseconds.\n+\t */\n+\tdouble lineDuration = sensorInfo.lineLength / (sensorInfo.pixelRate / 1e6);\n+\tconst ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;\n+\tint32_t minExposure = v4l2Exposure.min().get<int32_t>() * lineDuration;\n+\tint32_t maxExposure = v4l2Exposure.max().get<int32_t>() * lineDuration;\n+\tint32_t defExposure = v4l2Exposure.def().get<int32_t>() * lineDuration;\n+\tcontrols[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure,\n+\t\t\t\t\t\t\tdefExposure);\n+\n+\t/*\n+\t * Compute the frame duration limits.\n+\t *\n+\t * The frame length is computed assuming a fixed line length combined\n+\t * with the vertical frame sizes.\n+\t */\n+\tconst ControlInfo &v4l2HBlank = sensorControls.find(V4L2_CID_HBLANK)->second;\n+\tuint32_t hblank = v4l2HBlank.def().get<int32_t>();\n+\tuint32_t lineLength = sensorInfo.outputSize.width + hblank;\n+\n+\tconst ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second;\n+\tstd::array<uint32_t, 3> frameHeights{\n+\t\tv4l2VBlank.min().get<int32_t>() + sensorInfo.outputSize.height,\n+\t\tv4l2VBlank.max().get<int32_t>() + sensorInfo.outputSize.height,\n+\t\tv4l2VBlank.def().get<int32_t>() + sensorInfo.outputSize.height,\n+\t};\n+\n+\tstd::array<int64_t, 3> frameDurations;\n+\tfor (unsigned int i = 0; i < frameHeights.size(); ++i) {\n+\t\tuint64_t frameSize = lineLength * frameHeights[i];\n+\t\tframeDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);\n+\t}\n+\n+\tcontrols[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],\n+\t\t\t\t\t\t\t frameDurations[1],\n+\t\t\t\t\t\t\t frameDurations[2]);\n+\n+\t*ipaControls = ControlInfoMap(std::move(controls), controls::controls);\n+\n \treturn 0;\n }\n \ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex 9c23788e5231..6e26a7b7f640 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -88,6 +88,8 @@ public:\n \n \tstd::queue<Request *> pendingRequests_;\n \n+\tControlInfoMap ipaControls_;\n+\n private:\n \tvoid queueFrameAction(unsigned int id,\n \t\t\t const ipa::ipu3::IPU3Action &action);\n@@ -954,7 +956,6 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data)\n \t\treturn ret;\n \n \tControlInfoMap::Map controls = IPU3Controls;\n-\tconst ControlInfoMap &sensorControls = sensor->controls();\n \tconst std::vector<int32_t> &testPatternModes = sensor->testPatternModes();\n \tif (!testPatternModes.empty()) {\n \t\tstd::vector<ControlValue> values;\n@@ -966,58 +967,6 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data)\n \t\tcontrols[&controls::draft::TestPatternMode] = ControlInfo(values);\n \t}\n \n-\t/*\n-\t * Compute exposure time limits.\n-\t *\n-\t * Initialize the control using the line length and pixel rate of the\n-\t * current configuration converted to microseconds. Use the\n-\t * V4L2_CID_EXPOSURE control to get exposure min, max and default and\n-\t * convert it from lines to microseconds.\n-\t */\n-\tdouble lineDuration = sensorInfo.lineLength\n-\t\t\t / (sensorInfo.pixelRate / 1e6);\n-\tconst ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;\n-\tint32_t minExposure = v4l2Exposure.min().get<int32_t>() * lineDuration;\n-\tint32_t maxExposure = v4l2Exposure.max().get<int32_t>() * lineDuration;\n-\tint32_t defExposure = v4l2Exposure.def().get<int32_t>() * lineDuration;\n-\n-\t/*\n-\t * \\todo Report the actual exposure time, use the default for the\n-\t * moment.\n-\t */\n-\tdata->exposureTime_ = defExposure;\n-\n-\tcontrols[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure,\n-\t\t\t\t\t\t\tdefExposure);\n-\n-\t/*\n-\t * Compute the frame duration limits.\n-\t *\n-\t * The frame length is computed assuming a fixed line length combined\n-\t * with the vertical frame sizes.\n-\t */\n-\tconst ControlInfo &v4l2HBlank = sensorControls.find(V4L2_CID_HBLANK)->second;\n-\tuint32_t hblank = v4l2HBlank.def().get<int32_t>();\n-\tuint32_t lineLength = sensorInfo.outputSize.width + hblank;\n-\n-\tconst ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second;\n-\tstd::array<uint32_t, 3> frameHeights{\n-\t\tv4l2VBlank.min().get<int32_t>() + sensorInfo.outputSize.height,\n-\t\tv4l2VBlank.max().get<int32_t>() + sensorInfo.outputSize.height,\n-\t\tv4l2VBlank.def().get<int32_t>() + sensorInfo.outputSize.height,\n-\t};\n-\n-\tstd::array<int64_t, 3> frameDurations;\n-\tfor (unsigned int i = 0; i < frameHeights.size(); ++i) {\n-\t\tuint64_t frameSize = lineLength * frameHeights[i];\n-\t\tframeDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);\n-\t}\n-\n-\tcontrols[&controls::FrameDurationLimits] =\n-\t\tControlInfo(frameDurations[0],\n-\t\t\t frameDurations[1],\n-\t\t\t frameDurations[2]);\n-\n \t/*\n \t * Compute the scaler crop limits.\n \t *\n@@ -1071,6 +1020,21 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data)\n \n \tcontrols[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop);\n \n+\t/*\n+\t * \\todo Report the actual exposure time, use the default for the\n+\t * moment.\n+\t */\n+\tconst auto exposureInfo = data->ipaControls_.find(&controls::ExposureTime);\n+\tif (exposureInfo == data->ipaControls_.end()) {\n+\t\tLOG(IPU3, Error) << \"Exposure control not initialized by the IPA\";\n+\t\treturn -EINVAL;\n+\t}\n+\tdata->exposureTime_ = exposureInfo->second.def().get<int32_t>();\n+\n+\t/* Add the IPA registered controls to list of camera controls. */\n+\tfor (const auto &ipaControl : data->ipaControls_)\n+\t\tcontrols[ipaControl.first] = ipaControl.second;\n+\n \tdata->controlInfo_ = ControlInfoMap(std::move(controls),\n \t\t\t\t\t controls::controls);\n \n@@ -1223,8 +1187,31 @@ int IPU3CameraData::loadIPA()\n \n \tipa_->queueFrameAction.connect(this, &IPU3CameraData::queueFrameAction);\n \n+\t/*\n+\t * Pass the sensor info to the IPA to initialize controls.\n+\t *\n+\t * \\todo Find a way to initialize IPA controls without basing their\n+\t * limits on a particular sensor mode. We currently pass sensor\n+\t * information corresponding to the largest sensor resolution, and the\n+\t * IPA uses this to compute limits for supported controls. There's a\n+\t * discrepancy between the need to compute IPA control limits at init\n+\t * time, and the fact that those limits may depend on the sensor mode.\n+\t * Research is required to find out to handle this issue.\n+\t */\n \tCameraSensor *sensor = cio2_.sensor();\n-\tint ret = ipa_->init(IPASettings{ \"\", sensor->model() });\n+\tV4L2SubdeviceFormat sensorFormat = {};\n+\tsensorFormat.size = sensor->resolution();\n+\tint ret = sensor->setFormat(&sensorFormat);\n+\tif (ret)\n+\t\treturn ret;\n+\n+\tIPACameraSensorInfo sensorInfo{};\n+\tret = sensor->sensorInfo(&sensorInfo);\n+\tif (ret)\n+\t\treturn ret;\n+\n+\tret = ipa_->init(IPASettings{ \"\", sensor->model() }, sensorInfo,\n+\t\t\t sensor->controls(), &ipaControls_);\n \tif (ret) {\n \t\tLOG(IPU3, Error) << \"Failed to initialise the IPU3 IPA\";\n \t\treturn ret;\n", "prefixes": [ "libcamera-devel", "v4", "4/5" ] }