[libcamera-devel] [PATCH 14/15] ipa: ipu3: Configure IPA with libcamera controls

[Jacopo Mondi]

Pass to the IPA configure() function the list of libcamera
controls::internal controls in place of sending to the IPA the
raw V4L2 control values.

As the controls::internal::ExposureTime is already expressed in
micro-seconds, there's not need to multiply by the line duration.

AnalogueGain still requires translation using the CameraSensorHelper
class.

While at it, rationalize the sequence of operations IPA::configure() to:
- check the validity of the configuration
- update the session configuration
- configure algorithm
- assign class member variables

Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
---
 src/ipa/ipu3/ipu3.cpp                | 67 ++++++++++++----------------
 src/libcamera/pipeline/ipu3/ipu3.cpp |  2 +-
 2 files changed, 30 insertions(+), 39 deletions(-)

diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index 1f80a10bc48d..b67ff1948bc2 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -150,7 +150,7 @@ public:
 private:
 	void updateSessionConfiguration(const IPAConfigInfo &info);
 
-	bool validateSensorControls();
+	bool validateConfiguration(const IPAConfigInfo &config);
 
 	void setControls(unsigned int frame);
 	void calculateBdsGrid(const Size &bdsOutputSize);
@@ -183,16 +183,6 @@ void IPAIPU3::updateSessionConfiguration(const IPAConfigInfo &info)
 	context_.configuration.sensor.lineDuration = sensorInfo.lineLength * 1.0s
 						   / sensorInfo.pixelRate;
 
-	const ControlInfoMap &sensorControls = info.sensorControls;
-
-	const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;
-	int32_t minExposure = v4l2Exposure.min().get<int32_t>();
-	int32_t maxExposure = v4l2Exposure.max().get<int32_t>();
-
-	const ControlInfo &v4l2Gain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;
-	int32_t minGain = v4l2Gain.min().get<int32_t>();
-	int32_t maxGain = v4l2Gain.max().get<int32_t>();
-
 	/*
 	 * When the AGC computes the new exposure values for a frame, it needs
 	 * to know the limits for shutter speed and analogue gain.
@@ -200,8 +190,16 @@ void IPAIPU3::updateSessionConfiguration(const IPAConfigInfo &info)
 	 *
 	 * \todo take VBLANK into account for maximum shutter speed
 	 */
-	context_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;
-	context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;
+	const ControlInfoMap &sensorControls = info.sensorControls;
+
+	const auto &[expId, exposure] = *sensorControls.find(&controls::internal::ExposureTime);
+	context_.configuration.agc.minShutterSpeed = exposure.min().get<int32_t>() * 1.0us;
+	context_.configuration.agc.maxShutterSpeed = exposure.max().get<int32_t>() * 1.0us;
+
+	const auto &[gainId, gain] = *sensorControls.find(&controls::internal::draft::SensorAnalogueGain);
+	int32_t minGain = gain.min().get<int32_t>();
+	int32_t maxGain = gain.max().get<int32_t>();
+
 	context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
 	context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
 }
@@ -209,18 +207,18 @@ void IPAIPU3::updateSessionConfiguration(const IPAConfigInfo &info)
 /**
  * \brief Validate that the sensor controls mandatory for the IPA exists
  */
-bool IPAIPU3::validateSensorControls()
+bool IPAIPU3::validateConfiguration(const IPAConfigInfo &config)
 {
-	static const uint32_t ctrls[] = {
-		V4L2_CID_ANALOGUE_GAIN,
-		V4L2_CID_EXPOSURE,
-		V4L2_CID_VBLANK,
+	static constexpr std::array<const ControlId *, 3> ctrls = {
+		&controls::internal::ExposureTime,
+		&controls::internal::FrameDuration,
+		&controls::internal::draft::SensorAnalogueGain,
 	};
 
-	for (auto c : ctrls) {
-		if (sensorCtrls_.find(c) == sensorCtrls_.end()) {
+	for (const ControlId *c : ctrls) {
+		if (config.sensorControls.find(c) == config.sensorControls.end()) {
 			LOG(IPAIPU3, Error) << "Unable to find sensor control "
-					    << utils::hex(c);
+					    << c->name();
 			return false;
 		}
 	}
@@ -369,30 +367,19 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)
 		return -ENODATA;
 	}
 
-	sensorInfo_ = configInfo.sensorInfo;
-
-	lensCtrls_ = configInfo.lensControls;
-
-	/*
-	 * Compute the sensor V4L2 controls to be used by the algorithms and
-	 * to be set on the sensor.
-	 */
-	sensorCtrls_ = configInfo.sensorControls;
-
-	calculateBdsGrid(configInfo.bdsOutputSize);
+	if (!validateConfiguration(configInfo)) {
+		LOG(IPAIPU3, Error) << "Sensor control validation failed.";
+		return -EINVAL;
+	}
 
 	/* Clean IPAActiveState at each reconfiguration. */
 	context_.activeState = {};
 	IPAFrameContext initFrameContext;
 	context_.frameContexts.fill(initFrameContext);
 
-	if (!validateSensorControls()) {
-		LOG(IPAIPU3, Error) << "Sensor control validation failed.";
-		return -EINVAL;
-	}
-
-	/* Update the IPASessionConfiguration using the sensor settings. */
+	/* Update the IPA configuration using the new settings. */
 	updateSessionConfiguration(configInfo);
+	calculateBdsGrid(configInfo.bdsOutputSize);
 
 	for (auto const &algo : algorithms_) {
 		int ret = algo->configure(context_, configInfo);
@@ -400,6 +387,10 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)
 			return ret;
 	}
 
+	sensorInfo_ = configInfo.sensorInfo;
+	lensCtrls_ = configInfo.lensControls;
+	sensorCtrls_ = configInfo.sensorControls;
+
 	return 0;
 }
 
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 44e65d9f2d46..8bf51411581a 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -661,7 +661,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
 	}
 
 	ipa::ipu3::IPAConfigInfo configInfo;
-	configInfo.sensorControls = data->cio2_.sensor()->v4l2Controls();
+	configInfo.sensorControls = data->cio2_.sensor()->controls();
 
 	CameraLens *lens = data->cio2_.sensor()->focusLens();
 	if (lens)
-- 
2.36.1