[libcamera-devel,v3,02/14] ipa: ipu3: set frameContext before controls
diff mbox series

Message ID 20211021164401.110033-3-jeanmichel.hautbois@ideasonboard.com
State Superseded
Headers show
Series
  • ipa: ipu3: Fix AGC bugs
Related show

Commit Message

Jean-Michel Hautbois Oct. 21, 2021, 4:43 p.m. UTC 
The AGC frame context needs to be initialised correctly for the first
iteration. Until now, the IPA uses the minimum exposure and gain values
and caches those in local variables.

In order to give the sensor limits to AGC, create a new structure in
IPASessionConfiguration. Store the exposure in time (and not line
duration) and the analogue gain after CameraSensorHelper conversion.

Set the gain and exposure appropriately to the current values known to
the IPA and remove the setting of exposure and gain in IPAIPU3 as those
are now fully controlled by IPU3Agc.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/ipa/ipu3/algorithms/agc.cpp |  8 ++++-
 src/ipa/ipu3/ipa_context.h      |  9 +++++
 src/ipa/ipu3/ipu3.cpp           | 64 +++++++++++++++++++++++++++++----
 3 files changed, 73 insertions(+), 8 deletions(-)

Patch
diff mbox series 

diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
index 970ec424..828e822c 100644
--- a/src/ipa/ipu3/algorithms/agc.cpp
+++ b/src/ipa/ipu3/algorithms/agc.cpp
@@ -60,7 +60,13 @@  int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
 
 	lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s
 		      / configInfo.sensorInfo.pixelRate;
-	maxExposureTime_ = kMaxExposure * lineDuration_;
+	maxExposureTime_ = context.configuration.agc.maxShutterSpeed;
+
+	/* Configure the default exposure and gain. */
+	context.frameContext.agc.gain =
+		context.configuration.agc.minAnalogueGain;
+	context.frameContext.agc.exposure =
+		context.configuration.agc.minShutterSpeed / lineDuration_;
 
 	return 0;
 }
diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
index 5bab684c..847c03fe 100644
--- a/src/ipa/ipu3/ipa_context.h
+++ b/src/ipa/ipu3/ipa_context.h
@@ -10,6 +10,8 @@ 
 
 #include <linux/intel-ipu3.h>
 
+#include <libcamera/base/utils.h>
+
 #include <libcamera/geometry.h>
 
 namespace libcamera {
@@ -22,6 +24,13 @@  struct IPASessionConfiguration {
 		Size bdsOutputSize;
 		uint32_t stride;
 	} grid;
+
+	struct {
+		utils::Duration minShutterSpeed;
+		utils::Duration maxShutterSpeed;
+		double minAnalogueGain;
+		double maxAnalogueGain;
+	} agc;
 };
 
 struct IPAFrameContext {
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index 388f1902..65d3fd2c 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -97,6 +97,23 @@ 
  * \brief Number of cells on one line including the ImgU padding
  */
 
+/**
+ * \struct IPASessionConfiguration::agc
+ * \brief AGC parameters configuration of the IPA
+ *
+ * \var IPASessionConfiguration::agc::minShutterSpeed
+ * \brief Minimum shutter speed supported with the configured sensor
+ *
+ * \var IPASessionConfiguration::grid::maxShutterSpeed
+ * \brief Maximum shutter speed supported with the configured sensor
+ *
+ * \var IPASessionConfiguration::grid::minAnalogueGain
+ * \brief Minimum analogue gain supported with the configured sensor
+ *
+ * \var IPASessionConfiguration::grid::maxAnalogueGain
+ * \brief Maximum analogue gain supported with the configured sensor
+ */
+
 /**
  * \struct IPAFrameContext::agc
  * \brief Context for the Automatic Gain Control algorithm
@@ -158,6 +175,8 @@  namespace libcamera {
 
 LOG_DEFINE_CATEGORY(IPAIPU3)
 
+using namespace std::literals::chrono_literals;
+
 namespace ipa::ipu3 {
 
 class IPAIPU3 : public IPAIPU3Interface
@@ -182,6 +201,8 @@  private:
 	void updateControls(const IPACameraSensorInfo &sensorInfo,
 			    const ControlInfoMap &sensorControls,
 			    ControlInfoMap *ipaControls);
+	void updateSessionConfiguration(const IPACameraSensorInfo &sensorInfo,
+					const ControlInfoMap &sensorControls);
 	void processControls(unsigned int frame, const ControlList &controls);
 	void fillParams(unsigned int frame, ipu3_uapi_params *params);
 	void parseStatistics(unsigned int frame,
@@ -216,6 +237,36 @@  private:
 	struct IPAContext context_;
 };
 
+/*
+ * Compute IPASessionConfiguration using the sensor information and the sensor
+ * v4l2 controls.
+ */
+void IPAIPU3::updateSessionConfiguration(const IPACameraSensorInfo &sensorInfo,
+					 const ControlInfoMap &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>();
+
+	utils::Duration lineDuration = sensorInfo.lineLength * 1.0s
+				     / sensorInfo.pixelRate;
+
+	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.
+	 * As it depends on the sensor, update it with the controls.
+	 *
+	 * \todo take VBLANK into account for maximum shutter speed
+	 */
+	context_.configuration.agc.minShutterSpeed = minExposure * lineDuration;
+	context_.configuration.agc.maxShutterSpeed = maxExposure * lineDuration;
+	context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
+	context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
+}
 
 /*
  * Compute camera controls using the sensor information and the sensor
@@ -436,15 +487,18 @@  int IPAIPU3::configure(const IPAConfigInfo &configInfo,
 
 	calculateBdsGrid(configInfo.bdsOutputSize);
 
+	/* Update the camera controls using the new sensor settings. */
+	updateControls(sensorInfo_, ctrls_, ipaControls);
+
+	/* Update the IPASessionConfiguration using the sensor settings. */
+	updateSessionConfiguration(sensorInfo_, ctrls_);
+
 	for (auto const &algo : algorithms_) {
 		int ret = algo->configure(context_, configInfo);
 		if (ret)
 			return ret;
 	}
 
-	/* Update the camera controls using the new sensor settings. */
-	updateControls(sensorInfo_, ctrls_, ipaControls);
-
 	return 0;
 }
 
@@ -543,10 +597,6 @@  void IPAIPU3::parseStatistics(unsigned int frame,
 {
 	ControlList ctrls(controls::controls);
 
-	/* \todo These fields should not be written by the IPAIPU3 layer */
-	context_.frameContext.agc.gain = camHelper_->gain(gain_);
-	context_.frameContext.agc.exposure = exposure_;
-
 	for (auto const &algo : algorithms_)
 		algo->process(context_, stats);