libcamera: rpi: Draw sensor delays from CameraSensorProperties

Now that we have camera sensor control application delay values in
the CameraSensorProperties class, remove the duplicated definitions
in the RPi IPA's CameraSensorHelpers and update the pipeline handler
to use the values from CameraSensorProperties.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

15 files changed, 15 insertions, 154 deletions

diff --git a/src/ipa/rpi/cam_helper/cam_helper.cpp b/src/ipa/rpi/cam_helper/cam_helper.cpp
index 6493e882..8c720652 100644
--- a/src/ipa/rpi/cam_helper/cam_helper.cpp
+++ b/src/ipa/rpi/cam_helper/cam_helper.cpp
@@ -156,19 +156,6 @@ void CamHelper::setCameraMode(const CameraMode &mode)
 	}
 }
 
-void CamHelper::getDelays(int &exposureDelay, int &gainDelay,
-			  int &vblankDelay, int &hblankDelay) const
-{
-	/*
-	 * These values are correct for many sensors. Other sensors will
-	 * need to over-ride this function.
-	 */
-	exposureDelay = 2;
-	gainDelay = 1;
-	vblankDelay = 2;
-	hblankDelay = 2;
-}
-
 bool CamHelper::sensorEmbeddedDataPresent() const
 {
 	return false;
diff --git a/src/ipa/rpi/cam_helper/cam_helper.h b/src/ipa/rpi/cam_helper/cam_helper.h
index 4a4ab5e6..29371bdb 100644
--- a/src/ipa/rpi/cam_helper/cam_helper.h
+++ b/src/ipa/rpi/cam_helper/cam_helper.h
@@ -36,11 +36,6 @@ namespace RPiController {
  * exposure time, and to convert between the sensor's gain codes and actual
  * gains.
  *
- * A function to return the number of frames of delay between updating exposure,
- * analogue gain and vblanking, and for the changes to take effect. For many
- * sensors these take the values 2, 1 and 2 respectively, but sensors that are
- * different will need to over-ride the default function provided.
- *
  * A function to query if the sensor outputs embedded data that can be parsed.
  *
  * A function to return the sensitivity of a given camera mode.
@@ -91,8 +86,6 @@ public:
 	libcamera::utils::Duration lineLengthPckToDuration(uint32_t lineLengthPck) const;
 	virtual uint32_t gainCode(double gain) const = 0;
 	virtual double gain(uint32_t gainCode) const = 0;
-	virtual void getDelays(int &exposureDelay, int &gainDelay,
-			       int &vblankDelay, int &hblankDelay) const;
 	virtual bool sensorEmbeddedDataPresent() const;
 	virtual double getModeSensitivity(const CameraMode &mode) const;
 	virtual unsigned int hideFramesStartup() const;
diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx283.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx283.cpp
index cb0be72a..efc03193 100644
--- a/src/ipa/rpi/cam_helper/cam_helper_imx283.cpp
+++ b/src/ipa/rpi/cam_helper/cam_helper_imx283.cpp
@@ -17,8 +17,6 @@ public:
 	CamHelperImx283();
 	uint32_t gainCode(double gain) const override;
 	double gain(uint32_t gainCode) const override;
-	void getDelays(int &exposureDelay, int &gainDelay,
-		       int &vblankDelay, int &hblankDelay) const override;
 	unsigned int hideFramesModeSwitch() const override;
 
 private:
@@ -49,16 +47,6 @@ double CamHelperImx283::gain(uint32_t gainCode) const
 	return static_cast<double>(2048.0 / (2048 - gainCode));
 }
 
-void CamHelperImx283::getDelays(int &exposureDelay, int &gainDelay,
-				int &vblankDelay, int &hblankDelay) const
-{
-	/* The driver appears to behave as follows: */
-	exposureDelay = 2;
-	gainDelay = 2;
-	vblankDelay = 2;
-	hblankDelay = 2;
-}
-
 unsigned int CamHelperImx283::hideFramesModeSwitch() const
 {
 	/* After a mode switch, we seem to get 1 bad frame. */
diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp
index 3b87751e..c1aa8528 100644
--- a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp
+++ b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp
@@ -17,8 +17,6 @@ public:
 	CamHelperImx290();
 	uint32_t gainCode(double gain) const override;
 	double gain(uint32_t gainCode) const override;
-	void getDelays(int &exposureDelay, int &gainDelay,
-		       int &vblankDelay, int &hblankDelay) const override;
 	unsigned int hideFramesStartup() const override;
 	unsigned int hideFramesModeSwitch() const override;
 
@@ -46,15 +44,6 @@ double CamHelperImx290::gain(uint32_t gainCode) const
 	return std::pow(10, 0.015 * gainCode);
 }
 
-void CamHelperImx290::getDelays(int &exposureDelay, int &gainDelay,
-				int &vblankDelay, int &hblankDelay) const
-{
-	exposureDelay = 2;
-	gainDelay = 2;
-	vblankDelay = 2;
-	hblankDelay = 2;
-}
-
 unsigned int CamHelperImx290::hideFramesStartup() const
 {
 	/* On startup, we seem to get 1 bad frame. */
diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx296.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx296.cpp
index d4a4fa79..ac7ee2ea 100644
--- a/src/ipa/rpi/cam_helper/cam_helper_imx296.cpp
+++ b/src/ipa/rpi/cam_helper/cam_helper_imx296.cpp
@@ -23,8 +23,6 @@ public:
 	double gain(uint32_t gainCode) const override;
 	uint32_t exposureLines(const Duration exposure, const Duration lineLength) const override;
 	Duration exposure(uint32_t exposureLines, const Duration lineLength) const override;
-	void getDelays(int &exposureDelay, int &gainDelay,
-		       int &vblankDelay, int &hblankDelay) const override;
 
 private:
 	static constexpr uint32_t minExposureLines = 1;
@@ -66,15 +64,6 @@ Duration CamHelperImx296::exposure(uint32_t exposureLines,
 	return std::max<uint32_t>(minExposureLines, exposureLines) * timePerLine + 14.26us;
 }
 
-void CamHelperImx296::getDelays(int &exposureDelay, int &gainDelay,
-				int &vblankDelay, int &hblankDelay) const
-{
-	exposureDelay = 2;
-	gainDelay = 2;
-	vblankDelay = 2;
-	hblankDelay = 2;
-}
-
 static CamHelper *create()
 {
 	return new CamHelperImx296();
diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx477.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx477.cpp
index a53c40cd..a72ac67d 100644
--- a/src/ipa/rpi/cam_helper/cam_helper_imx477.cpp
+++ b/src/ipa/rpi/cam_helper/cam_helper_imx477.cpp
@@ -51,8 +51,6 @@ public:
 	void prepare(libcamera::Span<const uint8_t> buffer, Metadata &metadata) override;
 	std::pair<uint32_t, uint32_t> getBlanking(Duration &exposure, Duration minFrameDuration,
 						  Duration maxFrameDuration) const override;
-	void getDelays(int &exposureDelay, int &gainDelay,
-		       int &vblankDelay, int &hblankDelay) const override;
 	bool sensorEmbeddedDataPresent() const override;
 
 private:
@@ -159,15 +157,6 @@ std::pair<uint32_t, uint32_t> CamHelperImx477::getBlanking(Duration &exposure,
 	return { frameLength - mode_.height, hblank };
 }
 
-void CamHelperImx477::getDelays(int &exposureDelay, int &gainDelay,
-				int &vblankDelay, int &hblankDelay) const
-{
-	exposureDelay = 2;
-	gainDelay = 2;
-	vblankDelay = 3;
-	hblankDelay = 3;
-}
-
 bool CamHelperImx477::sensorEmbeddedDataPresent() const
 {
 	return true;
diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx519.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx519.cpp
index 2ff08653..10cbea48 100644
--- a/src/ipa/rpi/cam_helper/cam_helper_imx519.cpp
+++ b/src/ipa/rpi/cam_helper/cam_helper_imx519.cpp
@@ -51,8 +51,6 @@ public:
 	void prepare(libcamera::Span<const uint8_t> buffer, Metadata &metadata) override;
 	std::pair<uint32_t, uint32_t> getBlanking(Duration &exposure, Duration minFrameDuration,
 						  Duration maxFrameDuration) const override;
-	void getDelays(int &exposureDelay, int &gainDelay,
-		       int &vblankDelay, int &hblankDelay) const override;
 	bool sensorEmbeddedDataPresent() const override;
 
 private:
@@ -159,15 +157,6 @@ std::pair<uint32_t, uint32_t> CamHelperImx519::getBlanking(Duration &exposure,
 	return { frameLength - mode_.height, hblank };
 }
 
-void CamHelperImx519::getDelays(int &exposureDelay, int &gainDelay,
-				int &vblankDelay, int &hblankDelay) const
-{
-	exposureDelay = 2;
-	gainDelay = 2;
-	vblankDelay = 3;
-	hblankDelay = 3;
-}
-
 bool CamHelperImx519::sensorEmbeddedDataPresent() const
 {
 	return true;
diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp
index 16a017e9..6150909c 100644
--- a/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp
+++ b/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp
@@ -54,8 +54,6 @@ public:
 	void process(StatisticsPtr &stats, Metadata &metadata) override;
 	std::pair<uint32_t, uint32_t> getBlanking(Duration &exposure, Duration minFrameDuration,
 						  Duration maxFrameDuration) const override;
-	void getDelays(int &exposureDelay, int &gainDelay,
-		       int &vblankDelay, int &hblankDelay) const override;
 	bool sensorEmbeddedDataPresent() const override;
 	double getModeSensitivity(const CameraMode &mode) const override;
 	unsigned int hideFramesModeSwitch() const override;
@@ -208,15 +206,6 @@ std::pair<uint32_t, uint32_t> CamHelperImx708::getBlanking(Duration &exposure,
 	return { frameLength - mode_.height, hblank };
 }
 
-void CamHelperImx708::getDelays(int &exposureDelay, int &gainDelay,
-				int &vblankDelay, int &hblankDelay) const
-{
-	exposureDelay = 2;
-	gainDelay = 2;
-	vblankDelay = 3;
-	hblankDelay = 3;
-}
-
 bool CamHelperImx708::sensorEmbeddedDataPresent() const
 {
 	return true;
diff --git a/src/ipa/rpi/cam_helper/cam_helper_ov5647.cpp b/src/ipa/rpi/cam_helper/cam_helper_ov5647.cpp
index c30b017c..40d6b6d7 100644
--- a/src/ipa/rpi/cam_helper/cam_helper_ov5647.cpp
+++ b/src/ipa/rpi/cam_helper/cam_helper_ov5647.cpp
@@ -17,8 +17,6 @@ public:
 	CamHelperOv5647();
 	uint32_t gainCode(double gain) const override;
 	double gain(uint32_t gainCode) const override;
-	void getDelays(int &exposureDelay, int &gainDelay,
-		       int &vblankDelay, int &hblankDelay) const override;
 	unsigned int hideFramesStartup() const override;
 	unsigned int hideFramesModeSwitch() const override;
 	unsigned int mistrustFramesStartup() const override;
@@ -52,19 +50,6 @@ double CamHelperOv5647::gain(uint32_t gainCode) const
 	return static_cast<double>(gainCode) / 16.0;
 }
 
-void CamHelperOv5647::getDelays(int &exposureDelay, int &gainDelay,
-				int &vblankDelay, int &hblankDelay) const
-{
-	/*
-	 * We run this sensor in a mode where the gain delay is bumped up to
-	 * 2. It seems to be the only way to make the delays "predictable".
-	 */
-	exposureDelay = 2;
-	gainDelay = 2;
-	vblankDelay = 2;
-	hblankDelay = 2;
-}
-
 unsigned int CamHelperOv5647::hideFramesStartup() const
 {
 	/*
diff --git a/src/ipa/rpi/cam_helper/cam_helper_ov64a40.cpp b/src/ipa/rpi/cam_helper/cam_helper_ov64a40.cpp
index a8efd389..980495a8 100644
--- a/src/ipa/rpi/cam_helper/cam_helper_ov64a40.cpp
+++ b/src/ipa/rpi/cam_helper/cam_helper_ov64a40.cpp
@@ -18,8 +18,6 @@ public:
 	CamHelperOv64a40();
 	uint32_t gainCode(double gain) const override;
 	double gain(uint32_t gainCode) const override;
-	void getDelays(int &exposureDelay, int &gainDelay,
-		       int &vblankDelay, int &hblankDelay) const override;
 	double getModeSensitivity(const CameraMode &mode) const override;
 
 private:
@@ -45,16 +43,6 @@ double CamHelperOv64a40::gain(uint32_t gainCode) const
 	return static_cast<double>(gainCode) / 128.0;
 }
 
-void CamHelperOv64a40::getDelays(int &exposureDelay, int &gainDelay,
-				 int &vblankDelay, int &hblankDelay) const
-{
-	/* The driver appears to behave as follows: */
-	exposureDelay = 2;
-	gainDelay = 2;
-	vblankDelay = 2;
-	hblankDelay = 2;
-}
-
 double CamHelperOv64a40::getModeSensitivity(const CameraMode &mode) const
 {
 	if (mode.binX >= 2 && mode.scaleX >= 4) {
diff --git a/src/ipa/rpi/cam_helper/cam_helper_ov7251.cpp b/src/ipa/rpi/cam_helper/cam_helper_ov7251.cpp
index 7b12c445..fc7b999f 100644
--- a/src/ipa/rpi/cam_helper/cam_helper_ov7251.cpp
+++ b/src/ipa/rpi/cam_helper/cam_helper_ov7251.cpp
@@ -17,8 +17,6 @@ public:
 	CamHelperOv7251();
 	uint32_t gainCode(double gain) const override;
 	double gain(uint32_t gainCode) const override;
-	void getDelays(int &exposureDelay, int &gainDelay,
-		       int &vblankDelay, int &hblankDelay) const override;
 
 private:
 	/*
@@ -48,16 +46,6 @@ double CamHelperOv7251::gain(uint32_t gainCode) const
 	return static_cast<double>(gainCode) / 16.0;
 }
 
-void CamHelperOv7251::getDelays(int &exposureDelay, int &gainDelay,
-				int &vblankDelay, int &hblankDelay) const
-{
-	/* The driver appears to behave as follows: */
-	exposureDelay = 2;
-	gainDelay = 2;
-	vblankDelay = 2;
-	hblankDelay = 2;
-}
-
 static CamHelper *create()
 {
 	return new CamHelperOv7251();
diff --git a/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp b/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp
index 96f7fff4..e93a4691 100644
--- a/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp
+++ b/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp
@@ -17,8 +17,6 @@ public:
 	CamHelperOv9281();
 	uint32_t gainCode(double gain) const override;
 	double gain(uint32_t gainCode) const override;
-	void getDelays(int &exposureDelay, int &gainDelay,
-		       int &vblankDelay, int &hblankDelay) const override;
 
 private:
 	/*
@@ -48,16 +46,6 @@ double CamHelperOv9281::gain(uint32_t gainCode) const
 	return static_cast<double>(gainCode) / 16.0;
 }
 
-void CamHelperOv9281::getDelays(int &exposureDelay, int &gainDelay,
-				int &vblankDelay, int &hblankDelay) const
-{
-	/* The driver appears to behave as follows: */
-	exposureDelay = 2;
-	gainDelay = 2;
-	vblankDelay = 2;
-	hblankDelay = 2;
-}
-
 static CamHelper *create()
 {
 	return new CamHelperOv9281();
diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
index 5fce17e6..45e2a1d7 100644
--- a/src/ipa/rpi/common/ipa_base.cpp
+++ b/src/ipa/rpi/common/ipa_base.cpp
@@ -134,18 +134,8 @@ int32_t IpaBase::init(const IPASettings &settings, const InitParams &params, Ini
 		return -EINVAL;
 	}
 
-	/*
-	 * Pass out the sensor config to the pipeline handler in order
-	 * to setup the staggered writer class.
-	 */
-	int gainDelay, exposureDelay, vblankDelay, hblankDelay, sensorMetadata;
-	helper_->getDelays(exposureDelay, gainDelay, vblankDelay, hblankDelay);
-	sensorMetadata = helper_->sensorEmbeddedDataPresent();
-
-	result->sensorConfig.gainDelay = gainDelay;
-	result->sensorConfig.exposureDelay = exposureDelay;
-	result->sensorConfig.vblankDelay = vblankDelay;
-	result->sensorConfig.hblankDelay = hblankDelay;
+	/* Pass out the sensor metadata to the pipeline handler */
+	int sensorMetadata = helper_->sensorEmbeddedDataPresent();
 	result->sensorConfig.sensorMetadata = sensorMetadata;
 
 	/* Load the tuning file for this sensor. */
diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
index 6f278b29..4b147fdb 100644
--- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
@@ -816,11 +816,12 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr<RPi::CameraData> &camera
 	 * Setup our delayed control writer with the sensor default
 	 * gain and exposure delays. Mark VBLANK for priority write.
 	 */
+	const CameraSensorProperties::SensorDelays &delays = data->sensor_->sensorDelays();
 	std::unordered_map<uint32_t, RPi::DelayedControls::ControlParams> params = {
-		{ V4L2_CID_ANALOGUE_GAIN, { result.sensorConfig.gainDelay, false } },
-		{ V4L2_CID_EXPOSURE, { result.sensorConfig.exposureDelay, false } },
-		{ V4L2_CID_HBLANK, { result.sensorConfig.hblankDelay, false } },
-		{ V4L2_CID_VBLANK, { result.sensorConfig.vblankDelay, true } }
+		{ V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },
+		{ V4L2_CID_EXPOSURE, { delays.exposureDelay, false } },
+		{ V4L2_CID_HBLANK, { delays.hblankDelay, false } },
+		{ V4L2_CID_VBLANK, { delays.vblankDelay, true } }
 	};
 	data->delayedCtrls_ = std::make_unique<RPi::DelayedControls>(data->sensor_->device(), params);
 	data->sensorMetadata_ = result.sensorConfig.sensorMetadata;
diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
index c56f5777..bd1fc869 100644
--- a/src/libcamera/sensor/camera_sensor_properties.cpp
+++ b/src/libcamera/sensor/camera_sensor_properties.cpp
@@ -331,6 +331,14 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
 		{ "ov5647", {
 			.unitCellSize = { 1400, 1400 },
 			.testPatternModes = {},
+			/*
+			 * We run this sensor in a mode where the gain delay is
+			 * bumped up to 2. It seems to be the only way to make
+			 * the delays "predictable".
+			 *
+			 * \todo Verify these delays properly, as the upstream
+			 * driver appears to configure _no_ delay.
+			 */
 			.sensorDelays = {
 				.exposureDelay = 2,
 				.gainDelay = 2,