[libcamera-devel,v2,2/4] ipa: rkisp1: Use IPAConfig in IPA::configure()
diff mbox series

Message ID 20221123134346.129807-3-jacopo@jmondi.org
State Accepted
Commit 855228f7d5243278459f32fc372cbbecbde9cdfc
Headers show
Series
  • ipa: Validate controls in CameraSensor
Related show

Commit Message

Jacopo Mondi Nov. 23, 2022, 1:43 p.m. UTC 
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 <jacopo@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 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(-)

Patch
diff mbox series 

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<uint32, libcamera.IPAStream> streamConfig,
-		  map<uint32, libcamera.ControlInfoMap> entityControls)
+	configure(IPAConfigInfo configInfo,
+		  map<uint32, libcamera.IPAStream> streamConfig)
 		=> (int32 ret);
 
 	mapBuffers(array<libcamera.IPABuffer> 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<uint32_t, IPAStream> &streamConfig,
-		      const std::map<uint32_t, ControlInfoMap> &entityControls) override;
+	int configure(const IPAConfigInfo &ipaConfig,
+		      const std::map<uint32_t, IPAStream> &streamConfig) override;
 	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
 	void unmapBuffers(const std::vector<unsigned int> &ids) override;
 
@@ -73,7 +72,7 @@  private:
 	std::map<unsigned int, FrameBuffer> buffers_;
 	std::map<unsigned int, MappedFrameBuffer> 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<uint32_t, IPAStream> &streamConfig,
-			 const std::map<uint32_t, ControlInfoMap> &entityControls)
+int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,
+			 [[maybe_unused]] const std::map<uint32_t, IPAStream> &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>();
 	int32_t maxExposure = itExp->second.max().get<int32_t>();
 
-	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>();
 	int32_t maxGain = itGain->second.max().get<int32_t>();
 
@@ -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<int32_t>();
 	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<int32_t>(exposure));
 	ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(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<uint32_t, ControlInfoMap> 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;