From 855228f7d5243278459f32fc372cbbecbde9cdfc Mon Sep 17 00:00:00 2001 From: Jacopo Mondi Date: Fri, 4 Nov 2022 10:09:08 +0100 Subject: ipa: rkisp1: Use IPAConfig in IPA::configure() 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 eaf3955e..36bf291e 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 9e020df0..d2432982 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_; @@ -205,20 +204,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(); @@ -234,7 +229,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; @@ -352,7 +348,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 3d3a7086..4f0e1f8b 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; -- cgit v1.2.1