From patchwork Thu Aug 18 09:44:03 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 17165 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 30F81C3275 for ; Thu, 18 Aug 2022 09:44:38 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BA92861FDD; Thu, 18 Aug 2022 11:44:37 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1660815877; bh=ydmiNplBrbkufWHt45m/xxp4OZlEQL6jepSEzxK6QHo=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=MKABOTMNULyK1jYbbcxTQT3OMpY0SIGi2/E4wBnDLH5cx+53ehiN1l3Km1gH8Kvs9 isbga3am6VVoMFiI9+dGYbVq3rtcnaHSzk71NQuLKot6YYUogAGDaH98HIct1m765P X2zSnvcel4at2Jb2N+FSABUBy2aH/YWCytTQ7tdfVfFnqgRxbTS3h4zFnvq/TskHys zlvSeZNIPgjW2huv6yPDoWjfXkPrsH3ecjrkUyDcODo52wBrYZPbgF6nMwJ+UqUN0f SeNfEHOO3QaaeTS0vaZx14gZs4d2LO25O80PnNoOeKNkpleMDh6/hOvki2vIuuSWE7 j0uy8CqQaWQ2w== Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A88AF61FD9 for ; Thu, 18 Aug 2022 11:44:32 +0200 (CEST) Received: (Authenticated sender: jacopo@jmondi.org) by mail.gandi.net (Postfix) with ESMTPSA id B68CAFF80C; Thu, 18 Aug 2022 09:44:31 +0000 (UTC) To: libcamera-devel@lists.libcamera.org Date: Thu, 18 Aug 2022 11:44:03 +0200 Message-Id: <20220818094410.1671-11-jacopo@jmondi.org> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20220818094410.1671-1-jacopo@jmondi.org> References: <20220818094410.1671-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 10/17] ipa: rkisp1: Align configure() with IPU3 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 Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The RkISP1 IPA::configure() function signature requires a map of entities controls and a map of stream configuration to be provided by the pipeline handler to the IPA. This design comes from the early days when the first IPA module was implemented. With the introduction of mojom-defined IPA interfaces it's now easier to define custom structures to group parameters together, as the IPU3 IPA does. Align the IPU3 and RkISP1 IPA interfaces to use the same function signature and introduce rkisp1::IPAConfigInfo for the purpose of grouping configuration parameters together. Update the implementation of the RkISP1 IPA to validate the supplied list of controls and update the session configuration in separate functions. Signed-off-by: Jacopo Mondi --- include/libcamera/ipa/rkisp1.mojom | 10 ++- src/ipa/rkisp1/rkisp1.cpp | 98 ++++++++++++++---------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 25 +++--- 3 files changed, 74 insertions(+), 59 deletions(-) diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom index eaf3955e4096..7efe17746804 100644 --- a/include/libcamera/ipa/rkisp1.mojom +++ b/include/libcamera/ipa/rkisp1.mojom @@ -8,6 +8,12 @@ module ipa.rkisp1; import "include/libcamera/ipa/core.mojom"; +struct IPAConfigInfo { + libcamera.IPACameraSensorInfo sensorInfo; + libcamera.ControlInfoMap sensorControls; + libcamera.ControlInfoMap lensControls; +}; + interface IPARkISP1Interface { init(libcamera.IPASettings settings, uint32 hwRevision) @@ -15,9 +21,7 @@ interface IPARkISP1Interface { start() => (int32 ret); stop(); - configure(libcamera.IPACameraSensorInfo sensorInfo, - map streamConfig, - map entityControls) + configure(IPAConfigInfo configInfo) => (int32 ret); mapBuffers(array buffers); diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 3a98aaf75d98..f2075c893d29 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -49,9 +49,7 @@ 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 &configInfo) override; void mapBuffers(const std::vector &buffers) override; void unmapBuffers(const std::vector &ids) override; @@ -64,6 +62,9 @@ protected: std::string logPrefix() const override; private: + void updateSessionConfiguration(const IPAConfigInfo &configInfo); + bool validateSensorControls(const ControlInfoMap &sensorControls); + void setControls(unsigned int frame); void prepareMetadata(unsigned int frame, unsigned int aeState); @@ -193,44 +194,18 @@ 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([[maybe_unused]] const IPACameraSensorInfo &info, - [[maybe_unused]] const std::map &streamConfig, - const std::map &entityControls) +void IPARkISP1::updateSessionConfiguration(const IPAConfigInfo &configInfo) { - if (entityControls.empty()) - return -EINVAL; - - sensorCtrls_ = entityControls.at(0); - - auto lensControls = entityControls.find(1); - if (lensControls != entityControls.end()) - lensCtrls_ = lensControls->second; + const IPACameraSensorInfo &sensorInfo = configInfo.sensorInfo; + const ControlInfoMap &sensorControls = configInfo.sensorControls; - const auto itExp = sensorCtrls_.find(V4L2_CID_EXPOSURE); - if (itExp == sensorCtrls_.end()) { - LOG(IPARkISP1, Error) << "Can't find exposure control"; - return -EINVAL; - } - - const auto itGain = sensorCtrls_.find(V4L2_CID_ANALOGUE_GAIN); - if (itGain == sensorCtrls_.end()) { - LOG(IPARkISP1, Error) << "Can't find gain control"; - return -EINVAL; - } + const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second; + const ControlInfo &v4l2Gain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second; + int32_t minExposure = v4l2Exposure.min().get(); + int32_t maxExposure = v4l2Exposure.max().get(); - autoExposure_ = true; - - int32_t minExposure = itExp->second.min().get(); - int32_t maxExposure = itExp->second.max().get(); - - int32_t minGain = itGain->second.min().get(); - int32_t maxGain = itGain->second.max().get(); + int32_t minGain = v4l2Gain.min().get(); + int32_t maxGain = v4l2Gain.max().get(); LOG(IPARkISP1, Info) << "Exposure: " << minExposure << "-" << maxExposure @@ -243,8 +218,9 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info, /* Set the hardware revision for the algorithms. */ context_.configuration.hw.revision = hwRevision_; - context_.configuration.sensor.size = info.outputSize; - context_.configuration.sensor.lineDuration = info.lineLength * 1.0s / info.pixelRate; + context_.configuration.sensor.size = sensorInfo.outputSize; + context_.configuration.sensor.lineDuration = sensorInfo.lineLength * 1.0s + / sensorInfo.pixelRate; /* * When the AGC computes the new exposure values for a frame, it needs @@ -259,9 +235,49 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info, context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain); context_.activeState.frameCount = 0; +} + +bool IPARkISP1::validateSensorControls(const ControlInfoMap &sensorControls) +{ + static const uint32_t ctrls[] = { + V4L2_CID_ANALOGUE_GAIN, + V4L2_CID_EXPOSURE, + }; + + for (auto c : ctrls) { + if (sensorControls.find(c) == sensorControls.end()) { + LOG(IPARkISP1, Error) << "Unable to find sensor control " + << utils::hex(c); + return false; + } + } + + return true; + +} + +/** + * \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 &configInfo) +{ + if (!validateSensorControls(configInfo.sensorControls)) { + LOG(IPARkISP1, Error) << "Sensor control validation failed."; + return -EINVAL; + } + + sensorCtrls_ = configInfo.sensorControls; + lensCtrls_ = configInfo.lensControls; + + autoExposure_ = true; + + updateSessionConfiguration(configInfo); for (auto const &algo : algorithms()) { - int ret = algo->configure(context_, info); + int ret = algo->configure(context_, configInfo.sensorInfo); if (ret) return ret; } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 5f10c26bcb4d..3b250b0ae346 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -651,20 +651,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) << "ISP output pad configured with " << format << " crop " << rect; - std::map streamConfig; - for (const StreamConfiguration &cfg : *config) { - if (cfg.stream() == &data->mainPathStream_) { + if (cfg.stream() == &data->mainPathStream_) ret = mainPath_.configure(cfg, format); - streamConfig[0] = IPAStream(cfg.pixelFormat, - cfg.size); - } else if (hasSelfPath_) { + else if (hasSelfPath_) ret = selfPath_.configure(cfg, format); - streamConfig[1] = IPAStream(cfg.pixelFormat, - cfg.size); - } else { + else return -ENODEV; - } if (ret) return ret; @@ -682,7 +675,9 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) if (ret) return ret; - /* Inform IPA of stream configuration and sensor controls. */ + /* Configure the IPA module. */ + ipa::rkisp1::IPAConfigInfo configInfo; + IPACameraSensorInfo sensorInfo = {}; ret = data->sensor_->sensorInfo(&sensorInfo); if (ret) { @@ -692,14 +687,14 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) ret = 0; } - std::map entityControls; - entityControls.emplace(0, data->sensor_->controls()); + configInfo.sensorInfo = sensorInfo; + configInfo.sensorControls = data->sensor_->controls(); CameraLens *lens = data->sensor_->focusLens(); if (lens) - entityControls.emplace(1, lens->controls()); + configInfo.lensControls = lens->controls(); - ret = data->ipa_->configure(sensorInfo, streamConfig, entityControls); + ret = data->ipa_->configure(configInfo); if (ret) { LOG(RkISP1, Error) << "failed configuring IPA (" << ret << ")"; return ret;