From patchwork Wed Nov 23 13:43:43 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 17847 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id B2EF1BE08B for ; Wed, 23 Nov 2022 13:43:57 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8302B63313; Wed, 23 Nov 2022 14:43:57 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1669211037; bh=IaihdJ0pG/sEBwaULAyeE/Uy4s2OUK4t0JrSZI6am5o=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=eLn1VHpfFVs+El2srEAuwW4lEKYwLiXykC+6DRqjmX7NIqADA6zS7etkUNcEJxDaA jSv4f1ceoh52oW83Kny3CXRPQ05qI8s2C0Jm+flobGDkwgicv9SjE8O5IeCSghY4wK agvayzDgBpQVNUc9F8jNG3EUDCQTvI7RY+BKYzdJ342MkoBxHTSNUIRnU9YFP9VX2S sX+b6uj7va6ieACB+ZW4JgW/qu8qmJVuILpfUmg0WpwgVWKhEFit2N4Vq8GjKt4GPJ tu8+d7efWNvff+H4u7RxNk+QduSHp2HhnLIFyNYLjS2nw/RljjETkDWdMPZqx3vH1J TBYPL5e+rQ0TA== Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 16A9D63311 for ; Wed, 23 Nov 2022 14:43:56 +0100 (CET) Received: (Authenticated sender: jacopo@jmondi.org) by mail.gandi.net (Postfix) with ESMTPSA id 23AF8E0003; Wed, 23 Nov 2022 13:43:53 +0000 (UTC) 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 Subject: [libcamera-devel] [PATCH v2 1/4] libcamera: Move IPA sensor controls validation to CameraSensor X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jacopo Mondi via libcamera-devel From: Jacopo Mondi Reply-To: Jacopo Mondi Cc: Nicholas Roth Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The CameraSensor class validates that the sensor driver in use supports the controls required for IPA modules to work correctly. For in-tree IPA modules, whose pipeline handlers already use CameraSensor there's no need to validate such controls again. Remove controls validation from the IPU3 and RkISP1 IPA modules and rely on CameraSensor doing that at initialization time. The list of mandatory controls is expanded to add V4L2_CID_ANALOGUE_GAIN without which IPA modules cannot function. The new requirement only applies to RAW sensors, platforms like UVC and Simple are not impacted by this change. While at it, expand the sensor driver requirements documentation to include V4L2_ANALOGUE_GAIN in the list of mandatory controls a sensor driver has to support. Signed-off-by: Jacopo Mondi Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- Documentation/sensor_driver_requirements.rst | 7 +++++ src/ipa/ipu3/ipu3.cpp | 29 -------------------- src/ipa/rkisp1/rkisp1.cpp | 12 +------- src/libcamera/camera_sensor.cpp | 1 + 4 files changed, 9 insertions(+), 40 deletions(-) diff --git a/Documentation/sensor_driver_requirements.rst b/Documentation/sensor_driver_requirements.rst index b0854be3328a..3abc8f35924a 100644 --- a/Documentation/sensor_driver_requirements.rst +++ b/Documentation/sensor_driver_requirements.rst @@ -24,16 +24,23 @@ The sensor driver is assumed to be fully compliant with the V4L2 specification. For RAW sensors, the sensor driver shall support the following V4L2 controls: +* `V4L2_CID_ANALOGUE_GAIN`_ * `V4L2_CID_EXPOSURE`_ * `V4L2_CID_HBLANK`_ * `V4L2_CID_PIXEL_RATE`_ * `V4L2_CID_VBLANK`_ +.. _V4L2_CID_ANALOGUE_GAIN: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-image-source.html .. _V4L2_CID_EXPOSURE: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/control.html .. _V4L2_CID_HBLANK: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-image-source.html .. _V4L2_CID_PIXEL_RATE: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-image-process.html .. _V4L2_CID_VBLANK: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-image-source.html +The ``ANALOGUE_GAIN`` control units are sensor-specific. libcamera requires +a sensor-specific CameraSensorHelper implementation to translate between the +sensor specific ``gain code`` and the analogue ``gain value`` expressed as an +absolute number as defined by ``controls::AnalogueGain``. + While V4L2 doesn't specify a unit for the ``EXPOSURE`` control, libcamera requires it to be expressed as a number of image lines. Camera sensor drivers that do not comply with this requirement will need to be adapted or will produce diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index a9a2b49ca95b..08ee6eb30cc5 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -171,8 +171,6 @@ private: ControlInfoMap *ipaControls); void updateSessionConfiguration(const ControlInfoMap &sensorControls); - bool validateSensorControls(); - void setControls(unsigned int frame); void calculateBdsGrid(const Size &bdsOutputSize); @@ -292,28 +290,6 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo, *ipaControls = ControlInfoMap(std::move(controls), controls::controls); } -/** - * \brief Validate that the sensor controls mandatory for the IPA exists - */ -bool IPAIPU3::validateSensorControls() -{ - static const uint32_t ctrls[] = { - V4L2_CID_ANALOGUE_GAIN, - V4L2_CID_EXPOSURE, - V4L2_CID_VBLANK, - }; - - for (auto c : ctrls) { - if (sensorCtrls_.find(c) == sensorCtrls_.end()) { - LOG(IPAIPU3, Error) << "Unable to find sensor control " - << utils::hex(c); - return false; - } - } - - return true; -} - /** * \brief Initialize the IPA module and its controls * @@ -512,11 +488,6 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo, calculateBdsGrid(configInfo.bdsOutputSize); - if (!validateSensorControls()) { - LOG(IPAIPU3, Error) << "Sensor control validation failed."; - return -EINVAL; - } - /* Update the camera controls using the new sensor settings. */ updateControls(sensorInfo_, sensorCtrls_, ipaControls); diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index fcb9dacccc3c..d237758f7bf0 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -216,20 +216,10 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info, ctrls_ = entityControls.at(0); const auto itExp = ctrls_.find(V4L2_CID_EXPOSURE); - if (itExp == ctrls_.end()) { - LOG(IPARkISP1, Error) << "Can't find exposure control"; - return -EINVAL; - } - - const auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN); - if (itGain == ctrls_.end()) { - LOG(IPARkISP1, Error) << "Can't find gain control"; - return -EINVAL; - } - int32_t minExposure = itExp->second.min().get(); int32_t maxExposure = itExp->second.max().get(); + const auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN); int32_t minGain = itGain->second.min().get(); int32_t maxGain = itGain->second.max().get(); diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index 572a313a8f99..ea373458a164 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -301,6 +301,7 @@ int CameraSensor::validateSensorDriver() * required by the CameraSensor class. */ static constexpr uint32_t mandatoryControls[] = { + V4L2_CID_ANALOGUE_GAIN, V4L2_CID_EXPOSURE, V4L2_CID_HBLANK, V4L2_CID_PIXEL_RATE,