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, From patchwork Wed Nov 23 13:43:44 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 17848 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 375EFBE08B for ; Wed, 23 Nov 2022 13:44:00 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E98556331E; Wed, 23 Nov 2022 14:43:59 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1669211039; bh=B4yGLK0L1CqI8Ruh1+blBFV5EgL9RagzdtM5FeuENh4=; 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=F/P7aobVjl6kXbMd3VGEGDt+dClWar0SJXAlTQ+mro76tEKXb6w1PLGVltIc8O2rk UB556fB6qH8qwKATFxIH8hsih212xEo55YBdk66jZOwikdepkXinpEacA6YYjzRU5q 8tkIhahZSte4DfrUo7gQrJodC5khqgvZg4wnJL5IJ1Qnj9LLICMJ2aQKf6lj7eSm28 Rrf3gV9DMZS40BU6vWu320LwL8v/RtA4Bspc/d4IQs8hUEqk3kDOgF83gcJwiqFe9I VNtut0s01vcLhFQlhyVfybqh5usjuWHsy/3vbDMqK81rrWqdqIm0SeDab5bGS8D4vl ewe8hHsU878jw== Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::224]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B5CF963311 for ; Wed, 23 Nov 2022 14:43:58 +0100 (CET) Received: (Authenticated sender: jacopo@jmondi.org) by mail.gandi.net (Postfix) with ESMTPSA id 778C2E0014; Wed, 23 Nov 2022 13:43:56 +0000 (UTC) To: libcamera-devel@lists.libcamera.org Date: Wed, 23 Nov 2022 14:43:44 +0100 Message-Id: <20221123134346.129807-3-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 2/4] ipa: rkisp1: Use IPAConfig in IPA::configure() 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 RkISP1 implementation of IPA::configure() still uses the legacy interface where sensor controls (and eventually lens controls) are passed from the pipeline handler to the IPA in a map. Since the introduction of mojom-based IPA interface definition, it is possible to define custom data types and use them in the interface definition between the pipeline handler and the IPA. Align the RkISP1 IPA::configure() implementation with the one in the IPU3 IPA module by using a custom data type instead of relying on a map to pass controls to the IPA. Signed-off-by: Jacopo Mondi Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- include/libcamera/ipa/rkisp1.mojom | 10 ++++++--- src/ipa/rkisp1/rkisp1.cpp | 26 ++++++++++-------------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 +++++------ 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom index eaf3955e4096..36bf291e8a51 100644 --- a/include/libcamera/ipa/rkisp1.mojom +++ b/include/libcamera/ipa/rkisp1.mojom @@ -8,6 +8,11 @@ module ipa.rkisp1; import "include/libcamera/ipa/core.mojom"; +struct IPAConfigInfo { + libcamera.IPACameraSensorInfo sensorInfo; + libcamera.ControlInfoMap sensorControls; +}; + interface IPARkISP1Interface { init(libcamera.IPASettings settings, uint32 hwRevision) @@ -15,9 +20,8 @@ interface IPARkISP1Interface { start() => (int32 ret); stop(); - configure(libcamera.IPACameraSensorInfo sensorInfo, - map streamConfig, - map entityControls) + configure(IPAConfigInfo configInfo, + map streamConfig) => (int32 ret); mapBuffers(array buffers); diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index d237758f7bf0..088d7c74d448 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -53,9 +53,8 @@ public: int start() override; void stop() override; - int configure(const IPACameraSensorInfo &info, - const std::map &streamConfig, - const std::map &entityControls) override; + int configure(const IPAConfigInfo &ipaConfig, + const std::map &streamConfig) override; void mapBuffers(const std::vector &buffers) override; void unmapBuffers(const std::vector &ids) override; @@ -73,7 +72,7 @@ private: std::map buffers_; std::map mappedBuffers_; - ControlInfoMap ctrls_; + ControlInfoMap sensorControls_; /* revision-specific data */ rkisp1_cif_isp_version hwRevision_; @@ -206,20 +205,16 @@ void IPARkISP1::stop() * assemble one. Make sure the reported sensor information are relevant * before accessing them. */ -int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info, - [[maybe_unused]] const std::map &streamConfig, - const std::map &entityControls) +int IPARkISP1::configure(const IPAConfigInfo &ipaConfig, + [[maybe_unused]] const std::map &streamConfig) { - if (entityControls.empty()) - return -EINVAL; - - ctrls_ = entityControls.at(0); + sensorControls_ = ipaConfig.sensorControls; - const auto itExp = ctrls_.find(V4L2_CID_EXPOSURE); + const auto itExp = sensorControls_.find(V4L2_CID_EXPOSURE); int32_t minExposure = itExp->second.min().get(); int32_t maxExposure = itExp->second.max().get(); - const auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN); + const auto itGain = sensorControls_.find(V4L2_CID_ANALOGUE_GAIN); int32_t minGain = itGain->second.min().get(); int32_t maxGain = itGain->second.max().get(); @@ -235,7 +230,8 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info, /* Set the hardware revision for the algorithms. */ context_.configuration.hw.revision = hwRevision_; - const ControlInfo vBlank = ctrls_.find(V4L2_CID_VBLANK)->second; + const IPACameraSensorInfo &info = ipaConfig.sensorInfo; + const ControlInfo vBlank = sensorControls_.find(V4L2_CID_VBLANK)->second; context_.configuration.sensor.defVBlank = vBlank.def().get(); context_.configuration.sensor.size = info.outputSize; context_.configuration.sensor.lineDuration = info.minLineLength * 1.0s / info.pixelRate; @@ -353,7 +349,7 @@ void IPARkISP1::setControls(unsigned int frame) uint32_t exposure = frameContext.agc.exposure; uint32_t gain = camHelper_->gainCode(frameContext.agc.gain); - ControlList ctrls(ctrls_); + ControlList ctrls(sensorControls_); ctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure)); ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast(gain)); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 3d3a7086c03a..4f0e1f8b1a35 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -715,18 +715,18 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) return ret; /* Inform IPA of stream configuration and sensor controls. */ - IPACameraSensorInfo sensorInfo = {}; - ret = data->sensor_->sensorInfo(&sensorInfo); + ipa::rkisp1::IPAConfigInfo ipaConfig{}; + + ret = data->sensor_->sensorInfo(&ipaConfig.sensorInfo); if (ret) { /* \todo Turn this into a hard failure. */ LOG(RkISP1, Warning) << "Camera sensor information not available"; - sensorInfo = {}; + ipaConfig.sensorInfo = {}; } - std::map entityControls; - entityControls.emplace(0, data->sensor_->controls()); + ipaConfig.sensorControls = data->sensor_->controls(); - ret = data->ipa_->configure(sensorInfo, streamConfig, entityControls); + ret = data->ipa_->configure(ipaConfig, streamConfig); if (ret) { LOG(RkISP1, Error) << "failed configuring IPA (" << ret << ")"; return ret; From patchwork Wed Nov 23 13:43:45 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 17849 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 C66ECBE08B for ; Wed, 23 Nov 2022 13:44:02 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9656F63311; Wed, 23 Nov 2022 14:44:02 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1669211042; bh=seA2FH7NwX/dr7IQJDCWP9W/obDqe3Kekvvt2UTeLy4=; 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=YlcKO0KGVzVmb9xtr+2xUuN5KWjCOKU+G3LA1xxNAx76d1O5wf6Sh3MVLyiA++YsU DoXGsMPHhSXx2iUvFsMDuwTPG2x8YF392U1nHU7Ns/VepD9xeL4EFa+oR/B6yedfCR o/YqgNUirJFHbotSaJytri5YibAF2Wl6XS9+l8oYi80SSl+cGzp6o9Fa46wemSyDIx jN/8zQFWv9wcKZ4ZKXmenVOOo9DUH52cL/h9CBLXsSD3GmVqypdjPk3MtX05D8OYeq N3v0poxtW1PEf3mt112TnK1wo/9ZR5PA7kJC0PZD8qlCsW2pGFJcZ78h2D02yucxMl fVYwsXQx2rY0g== Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::224]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 51A6563316 for ; Wed, 23 Nov 2022 14:44:00 +0100 (CET) Received: (Authenticated sender: jacopo@jmondi.org) by mail.gandi.net (Postfix) with ESMTPSA id 17591E000E; Wed, 23 Nov 2022 13:43:58 +0000 (UTC) To: libcamera-devel@lists.libcamera.org Date: Wed, 23 Nov 2022 14:43:45 +0100 Message-Id: <20221123134346.129807-4-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 3/4] ipa: rkisp1: Fail hard on empty CameraSensorInfo 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 RkISP1 pipeline and IPA module allows for the CameraSensorInfo to be empty, probably to accommodate some sensor used in a test platform that does not provide the mandatory libcamera requirements. As the \todo item in the IPA reports, there is a possibility that the received CameraSensorInfo is empty and it should be checked before accessing it, but currently such requirement is not enforced in the code. This allows to assume all the test platforms in use have now successfully moved their sensor driver to comply with the minimum requirements and provide a populated CameraSensorInfo to the IPA. As the safety check is not enforced, and as we don't want to allow faulty sensors to send empty CameraSensorInfo to the IPA, remove the \todo item in the IPA and fail hard in the pipeline handler if the sensor does not comply with libcamera requirements. Signed-off-by: Jacopo Mondi Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/ipa/rkisp1/rkisp1.cpp | 6 ------ src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 ++--- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 088d7c74d448..a9e144303072 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -199,12 +199,6 @@ void IPARkISP1::stop() context_.frameContexts.clear(); } -/** - * \todo The RkISP1 pipeline currently provides an empty IPACameraSensorInfo - * if the connected sensor does not provide enough information to properly - * assemble one. Make sure the reported sensor information are relevant - * before accessing them. - */ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig, [[maybe_unused]] const std::map &streamConfig) { diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 4f0e1f8b1a35..e946ccc4c930 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -719,9 +719,8 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) ret = data->sensor_->sensorInfo(&ipaConfig.sensorInfo); if (ret) { - /* \todo Turn this into a hard failure. */ - LOG(RkISP1, Warning) << "Camera sensor information not available"; - ipaConfig.sensorInfo = {}; + LOG(RkISP1, Error) << "Camera sensor information not available"; + return ret; } ipaConfig.sensorControls = data->sensor_->controls(); From patchwork Wed Nov 23 13:43:46 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 17850 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 72682BE08B for ; Wed, 23 Nov 2022 13:44:03 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1FFA663313; Wed, 23 Nov 2022 14:44:03 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1669211043; bh=Nu8fAcglarQ1J/JjWXeW4NbEtZjXEed4s6EarGgVuVk=; 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=03O2iNd97up/508Md91oNrXKAB8OBK8D9RxGyPU4gk/qStZrP88AGsP8oJk9jYE78 MsbEXq3RE/488GfBgPZqOnxODj6C4LWsLtti3AZhtuUaV2AuEI15aUIh5VVPoy55hm SsPmkNHCU9oFor+eGz05iIBGhYzZzjR0SgyiGupD5k3HYBkjxH1YyqBABjEea68cAL Iu30a5HrkppiaVWW+sYBDxABqTn26Y2jQvAZmfvFljDcUitvepnScBb0YWkDJ0rGv3 AHzGtrjnWxQH8JZJiCPcqfzUOvRXyoYJcHTAKAPSUgfRAppjBMvbCQ1cS/YfbkNAEJ 2mF1yJrDxUAbQ== Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::224]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 38C0E63313 for ; Wed, 23 Nov 2022 14:44:01 +0100 (CET) Received: (Authenticated sender: jacopo@jmondi.org) by mail.gandi.net (Postfix) with ESMTPSA id 7A980E000F; Wed, 23 Nov 2022 13:44:00 +0000 (UTC) To: libcamera-devel@lists.libcamera.org Date: Wed, 23 Nov 2022 14:43:46 +0100 Message-Id: <20221123134346.129807-5-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 4/4] ipa: rkisp1: add FrameDurationLimits control 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" From: Nicholas Roth Currently, the Android HAL does not work on rkisp1-based devices because required FrameDurationLimits metadata is missing from the IPA implementation. This change sets FrameDurationLimits for rkisp1 based on the existing ipu3 implementation, using the sensor's reported range of vertical blanking intervals with the minimum reported horizontal blanking interval. Signed-off-by: Nicholas Roth Signed-off-by: Jacopo Mondi Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- include/libcamera/ipa/rkisp1.mojom | 6 ++- src/ipa/rkisp1/rkisp1.cpp | 57 +++++++++++++++++++++--- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 ++++--- 3 files changed, 67 insertions(+), 13 deletions(-) diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom index 36bf291e8a51..1009e970a1b5 100644 --- a/include/libcamera/ipa/rkisp1.mojom +++ b/include/libcamera/ipa/rkisp1.mojom @@ -15,14 +15,16 @@ struct IPAConfigInfo { interface IPARkISP1Interface { init(libcamera.IPASettings settings, - uint32 hwRevision) + uint32 hwRevision, + libcamera.IPACameraSensorInfo sensorInfo, + libcamera.ControlInfoMap sensorControls) => (int32 ret, libcamera.ControlInfoMap ipaControls); start() => (int32 ret); stop(); configure(IPAConfigInfo configInfo, map streamConfig) - => (int32 ret); + => (int32 ret, libcamera.ControlInfoMap ipaControls); mapBuffers(array buffers); unmapBuffers(array ids); diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index a9e144303072..9265d3c9f53c 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -49,12 +49,15 @@ public: IPARkISP1(); int init(const IPASettings &settings, unsigned int hwRevision, + const IPACameraSensorInfo &sensorInfo, + const ControlInfoMap &sensorControls, ControlInfoMap *ipaControls) override; int start() override; void stop() override; int configure(const IPAConfigInfo &ipaConfig, - const std::map &streamConfig) override; + const std::map &streamConfig, + ControlInfoMap *ipaControls) override; void mapBuffers(const std::vector &buffers) override; void unmapBuffers(const std::vector &ids) override; @@ -67,6 +70,9 @@ protected: std::string logPrefix() const override; private: + void updateControls(const IPACameraSensorInfo &sensorInfo, + const ControlInfoMap &sensorControls, + ControlInfoMap *ipaControls); void setControls(unsigned int frame); std::map buffers_; @@ -114,6 +120,8 @@ std::string IPARkISP1::logPrefix() const } int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision, + const IPACameraSensorInfo &sensorInfo, + const ControlInfoMap &sensorControls, ControlInfoMap *ipaControls) { /* \todo Add support for other revisions */ @@ -180,9 +188,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision, if (ret) return ret; - /* Return the controls handled by the IPA. */ - ControlInfoMap::Map ctrlMap = rkisp1Controls; - *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls); + /* Initialize controls. */ + updateControls(sensorInfo, sensorControls, ipaControls); return 0; } @@ -200,7 +207,8 @@ void IPARkISP1::stop() } int IPARkISP1::configure(const IPAConfigInfo &ipaConfig, - [[maybe_unused]] const std::map &streamConfig) + [[maybe_unused]] const std::map &streamConfig, + ControlInfoMap *ipaControls) { sensorControls_ = ipaConfig.sensorControls; @@ -230,6 +238,9 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig, context_.configuration.sensor.size = info.outputSize; context_.configuration.sensor.lineDuration = info.minLineLength * 1.0s / info.pixelRate; + /* Update the camera controls using the new sensor settings. */ + updateControls(info, sensorControls_, ipaControls); + /* * When the AGC computes the new exposure values for a frame, it needs * to know the limits for shutter speed and analogue gain. @@ -332,6 +343,42 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId metadataReady.emit(frame, metadata); } +void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo, + const ControlInfoMap &sensorControls, + ControlInfoMap *ipaControls) +{ + ControlInfoMap::Map ctrlMap = rkisp1Controls; + + /* + * Compute the frame duration limits. + * + * The frame length is computed assuming a fixed line length combined + * with the vertical frame sizes. + */ + const ControlInfo &v4l2HBlank = sensorControls.find(V4L2_CID_HBLANK)->second; + uint32_t hblank = v4l2HBlank.def().get(); + uint32_t lineLength = sensorInfo.outputSize.width + hblank; + + const ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second; + std::array frameHeights{ + v4l2VBlank.min().get() + sensorInfo.outputSize.height, + v4l2VBlank.max().get() + sensorInfo.outputSize.height, + v4l2VBlank.def().get() + sensorInfo.outputSize.height, + }; + + std::array frameDurations; + for (unsigned int i = 0; i < frameHeights.size(); ++i) { + uint64_t frameSize = lineLength * frameHeights[i]; + frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U); + } + + ctrlMap[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0], + frameDurations[1], + frameDurations[2]); + + *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls); +} + void IPARkISP1::setControls(unsigned int frame) { /* diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index e946ccc4c930..231b16eca110 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -347,8 +347,15 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision) ipaTuningFile = std::string(configFromEnv); } - int ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision, - &controlInfo_); + IPACameraSensorInfo sensorInfo{}; + int ret = sensor_->sensorInfo(&sensorInfo); + if (ret) { + LOG(RkISP1, Error) << "Camera sensor information not available"; + return ret; + } + + ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision, + sensorInfo, sensor_->controls(), &controlInfo_); if (ret < 0) { LOG(RkISP1, Error) << "IPA initialization failure"; return ret; @@ -718,14 +725,12 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) ipa::rkisp1::IPAConfigInfo ipaConfig{}; ret = data->sensor_->sensorInfo(&ipaConfig.sensorInfo); - if (ret) { - LOG(RkISP1, Error) << "Camera sensor information not available"; + if (ret) return ret; - } ipaConfig.sensorControls = data->sensor_->controls(); - ret = data->ipa_->configure(ipaConfig, streamConfig); + ret = data->ipa_->configure(ipaConfig, streamConfig, &data->controlInfo_); if (ret) { LOG(RkISP1, Error) << "failed configuring IPA (" << ret << ")"; return ret;