{"id":17847,"url":"https://patchwork.libcamera.org/api/1.1/patches/17847/?format=json","web_url":"https://patchwork.libcamera.org/patch/17847/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20221123134346.129807-2-jacopo@jmondi.org>","date":"2022-11-23T13:43:43","name":"[libcamera-devel,v2,1/4] libcamera: Move IPA sensor controls validation to CameraSensor","commit_ref":"29d6d0e93b0193b70c6c0564eb424f2b6accf4a8","pull_url":null,"state":"accepted","archived":false,"hash":"8b645d244c365fc49bb51c5d3978d537cf929d6e","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/1.1/people/3/?format=json","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/17847/mbox/","series":[{"id":3633,"url":"https://patchwork.libcamera.org/api/1.1/series/3633/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=3633","date":"2022-11-23T13:43:42","name":"ipa: Validate controls in CameraSensor","version":2,"mbox":"https://patchwork.libcamera.org/series/3633/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/17847/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/17847/checks/","tags":{},"headers":{"Return-Path":"","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 B2EF1BE08B\n\tfor ;\n\tWed, 23 Nov 2022 13:43:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8302B63313;\n\tWed, 23 Nov 2022 14:43:57 +0100 (CET)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 16A9D63311\n\tfor ;\n\tWed, 23 Nov 2022 14:43:56 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 23AF8E0003;\n\tWed, 23 Nov 2022 13:43:53 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669211037;\n\tbh=IaihdJ0pG/sEBwaULAyeE/Uy4s2OUK4t0JrSZI6am5o=;\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:Cc:\n\tFrom;\n\tb=eLn1VHpfFVs+El2srEAuwW4lEKYwLiXykC+6DRqjmX7NIqADA6zS7etkUNcEJxDaA\n\tjSv4f1ceoh52oW83Kny3CXRPQ05qI8s2C0Jm+flobGDkwgicv9SjE8O5IeCSghY4wK\n\tagvayzDgBpQVNUc9F8jNG3EUDCQTvI7RY+BKYzdJ342MkoBxHTSNUIRnU9YFP9VX2S\n\tsX+b6uj7va6ieACB+ZW4JgW/qu8qmJVuILpfUmg0WpwgVWKhEFit2N4Vq8GjKt4GPJ\n\ttu8+d7efWNvff+H4u7RxNk+QduSHp2HhnLIFyNYLjS2nw/RljjETkDWdMPZqx3vH1J\n\tTBYPL5e+rQ0TA==","To":"libcamera-devel@lists.libcamera.org","Date":"Wed, 23 Nov 2022 14:43:43 +0100","Message-Id":"<20221123134346.129807-2-jacopo@jmondi.org>","X-Mailer":"git-send-email 2.38.1","In-Reply-To":"<20221123134346.129807-1-jacopo@jmondi.org>","References":"<20221123134346.129807-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v2 1/4] libcamera: Move IPA sensor\n\tcontrols validation to CameraSensor","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Nicholas Roth ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"The CameraSensor class validates that the sensor driver in use supports\nthe controls required for IPA modules to work correctly.\n\nFor in-tree IPA modules, whose pipeline handlers already use\nCameraSensor there's no need to validate such controls again.\n\nRemove controls validation from the IPU3 and RkISP1 IPA modules and rely\non CameraSensor doing that at initialization time.\n\nThe list of mandatory controls is expanded to add V4L2_CID_ANALOGUE_GAIN\nwithout which IPA modules cannot function.\n\nThe new requirement only applies to RAW sensors, platforms like UVC and\nSimple are not impacted by this change.\n\nWhile at it, expand the sensor driver requirements documentation to\ninclude V4L2_ANALOGUE_GAIN in the list of mandatory controls a sensor\ndriver has to support.\n\nSigned-off-by: Jacopo Mondi \nReviewed-by: Kieran Bingham \n---\n Documentation/sensor_driver_requirements.rst | 7 +++++\n src/ipa/ipu3/ipu3.cpp | 29 --------------------\n src/ipa/rkisp1/rkisp1.cpp | 12 +-------\n src/libcamera/camera_sensor.cpp | 1 +\n 4 files changed, 9 insertions(+), 40 deletions(-)","diff":"diff --git a/Documentation/sensor_driver_requirements.rst b/Documentation/sensor_driver_requirements.rst\nindex b0854be3328a..3abc8f35924a 100644\n--- a/Documentation/sensor_driver_requirements.rst\n+++ b/Documentation/sensor_driver_requirements.rst\n@@ -24,16 +24,23 @@ The sensor driver is assumed to be fully compliant with the V4L2 specification.\n \n For RAW sensors, the sensor driver shall support the following V4L2 controls:\n \n+* `V4L2_CID_ANALOGUE_GAIN`_\n * `V4L2_CID_EXPOSURE`_\n * `V4L2_CID_HBLANK`_\n * `V4L2_CID_PIXEL_RATE`_\n * `V4L2_CID_VBLANK`_\n \n+.. _V4L2_CID_ANALOGUE_GAIN: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-image-source.html\n .. _V4L2_CID_EXPOSURE: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/control.html\n .. _V4L2_CID_HBLANK: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-image-source.html\n .. _V4L2_CID_PIXEL_RATE: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-image-process.html\n .. _V4L2_CID_VBLANK: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-image-source.html\n \n+The ``ANALOGUE_GAIN`` control units are sensor-specific. libcamera requires\n+a sensor-specific CameraSensorHelper implementation to translate between the\n+sensor specific ``gain code`` and the analogue ``gain value`` expressed as an\n+absolute number as defined by ``controls::AnalogueGain``.\n+\n While V4L2 doesn't specify a unit for the ``EXPOSURE`` control, libcamera\n requires it to be expressed as a number of image lines. Camera sensor drivers\n that do not comply with this requirement will need to be adapted or will produce\ndiff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\nindex a9a2b49ca95b..08ee6eb30cc5 100644\n--- a/src/ipa/ipu3/ipu3.cpp\n+++ b/src/ipa/ipu3/ipu3.cpp\n@@ -171,8 +171,6 @@ private:\n \t\t\t ControlInfoMap *ipaControls);\n \tvoid updateSessionConfiguration(const ControlInfoMap &sensorControls);\n \n-\tbool validateSensorControls();\n-\n \tvoid setControls(unsigned int frame);\n \tvoid calculateBdsGrid(const Size &bdsOutputSize);\n \n@@ -292,28 +290,6 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,\n \t*ipaControls = ControlInfoMap(std::move(controls), controls::controls);\n }\n \n-/**\n- * \\brief Validate that the sensor controls mandatory for the IPA exists\n- */\n-bool IPAIPU3::validateSensorControls()\n-{\n-\tstatic const uint32_t ctrls[] = {\n-\t\tV4L2_CID_ANALOGUE_GAIN,\n-\t\tV4L2_CID_EXPOSURE,\n-\t\tV4L2_CID_VBLANK,\n-\t};\n-\n-\tfor (auto c : ctrls) {\n-\t\tif (sensorCtrls_.find(c) == sensorCtrls_.end()) {\n-\t\t\tLOG(IPAIPU3, Error) << \"Unable to find sensor control \"\n-\t\t\t\t\t << utils::hex(c);\n-\t\t\treturn false;\n-\t\t}\n-\t}\n-\n-\treturn true;\n-}\n-\n /**\n * \\brief Initialize the IPA module and its controls\n *\n@@ -512,11 +488,6 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n \n \tcalculateBdsGrid(configInfo.bdsOutputSize);\n \n-\tif (!validateSensorControls()) {\n-\t\tLOG(IPAIPU3, Error) << \"Sensor control validation failed.\";\n-\t\treturn -EINVAL;\n-\t}\n-\n \t/* Update the camera controls using the new sensor settings. */\n \tupdateControls(sensorInfo_, sensorCtrls_, ipaControls);\n \ndiff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\nindex fcb9dacccc3c..d237758f7bf0 100644\n--- a/src/ipa/rkisp1/rkisp1.cpp\n+++ b/src/ipa/rkisp1/rkisp1.cpp\n@@ -216,20 +216,10 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n \tctrls_ = entityControls.at(0);\n \n \tconst auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);\n-\tif (itExp == ctrls_.end()) {\n-\t\tLOG(IPARkISP1, Error) << \"Can't find exposure control\";\n-\t\treturn -EINVAL;\n-\t}\n-\n-\tconst auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);\n-\tif (itGain == ctrls_.end()) {\n-\t\tLOG(IPARkISP1, Error) << \"Can't find gain control\";\n-\t\treturn -EINVAL;\n-\t}\n-\n \tint32_t minExposure = itExp->second.min().get();\n \tint32_t maxExposure = itExp->second.max().get();\n \n+\tconst auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);\n \tint32_t minGain = itGain->second.min().get();\n \tint32_t maxGain = itGain->second.max().get();\n \ndiff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\nindex 572a313a8f99..ea373458a164 100644\n--- a/src/libcamera/camera_sensor.cpp\n+++ b/src/libcamera/camera_sensor.cpp\n@@ -301,6 +301,7 @@ int CameraSensor::validateSensorDriver()\n \t * required by the CameraSensor class.\n \t */\n \tstatic constexpr uint32_t mandatoryControls[] = {\n+\t\tV4L2_CID_ANALOGUE_GAIN,\n \t\tV4L2_CID_EXPOSURE,\n \t\tV4L2_CID_HBLANK,\n \t\tV4L2_CID_PIXEL_RATE,\n","prefixes":["libcamera-devel","v2","1/4"]}