[PATCH RFC 3/3] ipa: rkisp1: Pass sensor delays from IPA to pipeline handler

Mikhail Rudenko mike.rudenko at gmail.com
Mon Oct 28 18:36:59 CET 2024


At present, rkisp1 uses hardcoded sensor control delays. In the case
when they do not match real sensor delays, the AGC algorithm operates
in a suboptimal mode, leading to gain/exposure oscillations on capture
start and slower convergence. So, pass the correct delays from IPA at
the init() time.

Signed-off-by: Mikhail Rudenko <mike.rudenko at gmail.com>
---
 include/libcamera/ipa/rkisp1.mojom       |  3 ++-
 src/ipa/rkisp1/rkisp1.cpp                |  8 +++++--
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 27 ++++++++++--------------
 3 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
index 80d54a03..aedd9461 100644
--- a/include/libcamera/ipa/rkisp1.mojom
+++ b/include/libcamera/ipa/rkisp1.mojom
@@ -19,7 +19,8 @@ interface IPARkISP1Interface {
 	     uint32 hwRevision,
 	     libcamera.IPACameraSensorInfo sensorInfo,
 	     libcamera.ControlInfoMap sensorControls)
-		=> (int32 ret, libcamera.ControlInfoMap ipaControls);
+		=> (int32 ret, libcamera.ControlInfoMap ipaControls,
+		    libcamera.IPASensorDelays sensorDelays);
 	start() => (int32 ret);
 	stop();
 
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 9e161cab..b42ec976 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -54,7 +54,8 @@ public:
 	int init(const IPASettings &settings, unsigned int hwRevision,
 		 const IPACameraSensorInfo &sensorInfo,
 		 const ControlInfoMap &sensorControls,
-		 ControlInfoMap *ipaControls) override;
+		 ControlInfoMap *ipaControls,
+		 IPASensorDelays *sensorDelays) override;
 	int start() override;
 	void stop() override;
 
@@ -136,7 +137,8 @@ std::string IPARkISP1::logPrefix() const
 int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
 		    const IPACameraSensorInfo &sensorInfo,
 		    const ControlInfoMap &sensorControls,
-		    ControlInfoMap *ipaControls)
+		    ControlInfoMap *ipaControls,
+		    IPASensorDelays *sensorDelays)
 {
 	/* \todo Add support for other revisions */
 	switch (hwRevision) {
@@ -168,6 +170,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
 		return -ENODEV;
 	}
 
+	*sensorDelays = context_.camHelper->sensorDelays();
+
 	context_.configuration.sensor.lineDuration =
 		sensorInfo.minLineLength * 1.0s / sensorInfo.pixelRate;
 
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index c7b0b392..0ece3c55 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -96,7 +96,7 @@ public:
 	}
 
 	PipelineHandlerRkISP1 *pipe();
-	int loadIPA(unsigned int hwRevision);
+	int loadIPA(unsigned int hwRevision, IPASensorDelays *sensorDelays);
 
 	Stream mainPathStream_;
 	Stream selfPathStream_;
@@ -360,7 +360,7 @@ PipelineHandlerRkISP1 *RkISP1CameraData::pipe()
 	return static_cast<PipelineHandlerRkISP1 *>(Camera::Private::pipe());
 }
 
-int RkISP1CameraData::loadIPA(unsigned int hwRevision)
+int RkISP1CameraData::loadIPA(unsigned int hwRevision, IPASensorDelays *sensorDelays)
 {
 	ipa_ = IPAManager::createIPA<ipa::rkisp1::IPAProxyRkISP1>(pipe(), 1, 1);
 	if (!ipa_)
@@ -391,7 +391,8 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)
 	}
 
 	ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,
-			 sensorInfo, sensor_->controls(), &ipaControls_);
+			 sensorInfo, sensor_->controls(), &controlInfo_,
+			 sensorDelays);
 	if (ret < 0) {
 		LOG(RkISP1, Error) << "IPA initialization failure";
 		return ret;
@@ -1240,14 +1241,14 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
 	/* Initialize the camera properties. */
 	data->properties_ = data->sensor_->properties();
 
-	/*
-	 * \todo Read delay values from the sensor itself or from a
-	 * a sensor database. For now use generic values taken from
-	 * the Raspberry Pi and listed as generic values.
-	 */
+	IPASensorDelays sensorDelays;
+	ret = data->loadIPA(media_->hwRevision(), &sensorDelays);
+	if (ret)
+		return ret;
+
 	std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
-		{ V4L2_CID_ANALOGUE_GAIN, { 1, false } },
-		{ V4L2_CID_EXPOSURE, { 2, false } },
+		{ V4L2_CID_ANALOGUE_GAIN, { sensorDelays.gainDelay, false } },
+		{ V4L2_CID_EXPOSURE, { sensorDelays.exposureDelay, false } },
 	};
 
 	data->delayedCtrls_ =
@@ -1256,12 +1257,6 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
 	isp_->frameStart.connect(data->delayedCtrls_.get(),
 				 &DelayedControls::applyControls);
 
-	ret = data->loadIPA(media_->hwRevision());
-	if (ret)
-		return ret;
-
-	updateControls(data.get());
-
 	std::set<Stream *> streams{
 		&data->mainPathStream_,
 		&data->selfPathStream_,
-- 
2.46.0



More information about the libcamera-devel mailing list