[6/7] ipa: rpi: Update digital gain handling in IPA base and derived classes

23589 diff mbox series

diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
index 7af4ea5e..22aadeba 100644
--- a/src/ipa/rpi/common/ipa_base.cpp
+++ b/src/ipa/rpi/common/ipa_base.cpp
@@ -309,20 +309,23 @@  void IpaBase::start(const ControlList &controls, StartResult *result)
 	frameLengths_.clear();
 	frameLengths_.resize(FrameLengthsQueueSize, 0s);
 
-	/* SwitchMode may supply updated exposure/gain values to use. */
-	AgcStatus agcStatus;
-	agcStatus.exposureTime = 0.0s;
-	agcStatus.analogueGain = 0.0;
+	/*
+	 * SwitchMode may supply updated exposure/gain values to use.
+	 * agcStatus_ will store these values for us to use until delayed_status values
+	 * start to appear.
+	 */
+	agcStatus_.exposureTime = 0.0s;
+	agcStatus_.analogueGain = 0.0;
 
-	metadata.get("agc.status", agcStatus);
-	if (agcStatus.exposureTime && agcStatus.analogueGain) {
+	metadata.get("agc.status", agcStatus_);
+	if (agcStatus_.exposureTime && agcStatus_.analogueGain) {
 		ControlList ctrls(sensorCtrls_);
-		applyAGC(&agcStatus, ctrls);
+		applyAGC(&agcStatus_, ctrls);
 		result->controls = std::move(ctrls);
 		setCameraTimeoutValue();
 	}
 	/* Make a note of this as it tells us the HDR status of the first few frames. */
-	hdrStatus_ = agcStatus.hdr;
+	hdrStatus_ = agcStatus_.hdr;
 
 	/*
 	 * Initialise frame counts, and decide how many frames must be hidden or
@@ -476,7 +479,9 @@  void IpaBase::prepareIsp(const PrepareParams &params)
 		controller_.prepare(&rpiMetadata);
 		/* Actually prepare the ISP parameters for the frame. */
 		platformPrepareIsp(params, rpiMetadata);
-	}
+		platformPrepareAgc(rpiMetadata);
+	} else
+		platformPrepareAgc(rpiMetadata);
 
 	frameCount_++;
 
@@ -528,6 +533,7 @@  void IpaBase::processStats(const ProcessParams &params)
 	if (rpiMetadata.get("agc.status", agcStatus) == 0) {
 		ControlList ctrls(sensorCtrls_);
 		applyAGC(&agcStatus, ctrls, offset);
+		rpiMetadata.set("agc.status", agcStatus);
 		setDelayedControls.emit(ctrls, ipaContext);
 		setCameraTimeoutValue();
 	}
@@ -1472,9 +1478,6 @@  void IpaBase::reportMetadata(unsigned int ipaContext)
 	}
 
 	AgcPrepareStatus *agcPrepareStatus = rpiMetadata.getLocked<AgcPrepareStatus>("agc.prepare_status");
-	if (agcPrepareStatus)
-		libcameraMetadata_.set(controls::DigitalGain, agcPrepareStatus->digitalGain);
-
 	RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(
 		controller_.getAlgorithm("agc"));
 	if (agc) {
@@ -1487,6 +1490,13 @@  void IpaBase::reportMetadata(unsigned int ipaContext)
 						       : controls::AeStateSearching);
 	}
 
+	const AgcStatus *agcStatus = rpiMetadata.getLocked<AgcStatus>("agc.delayed_status");
+	if (agcStatus)
+		libcameraMetadata_.set(controls::DigitalGain, agcStatus->digitalGain);
+	else
+		libcameraMetadata_.set(controls::DigitalGain, agcStatus_.digitalGain);
+	/* The HDR metadata reporting will use this agcStatus too. */
+
 	LuxStatus *luxStatus = rpiMetadata.getLocked<LuxStatus>("lux.status");
 	if (luxStatus)
 		libcameraMetadata_.set(controls::Lux, luxStatus->lux);
@@ -1576,7 +1586,6 @@  void IpaBase::reportMetadata(unsigned int ipaContext)
 	 * delayed_status to be available, we use the HDR status that came out of the
 	 * switchMode call.
 	 */
-	const AgcStatus *agcStatus = rpiMetadata.getLocked<AgcStatus>("agc.delayed_status");
 	const HdrStatus &hdrStatus = agcStatus ? agcStatus->hdr : hdrStatus_;
 	if (!hdrStatus.mode.empty() && hdrStatus.mode != "Off") {
 		int32_t hdrMode = controls::HdrModeOff;
@@ -1686,7 +1695,7 @@  void IpaBase::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDu
 	}
 }
 
-void IpaBase::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls, Duration frameDurationOffset)
+void IpaBase::applyAGC(struct AgcStatus *agcStatus, ControlList &ctrls, Duration frameDurationOffset)
 {
 	const int32_t minGainCode = helper_->gainCode(mode_.minAnalogueGain);
 	const int32_t maxGainCode = helper_->gainCode(mode_.maxAnalogueGain);
@@ -1716,6 +1725,19 @@  void IpaBase::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls, Du
 	ctrls.set(V4L2_CID_EXPOSURE, exposureLines);
 	ctrls.set(V4L2_CID_ANALOGUE_GAIN, gainCode);
 
+	/*
+	 * We must update the digital gain to make up for any quantisation that happens, and
+	 * communicate that back into the metadata so that it will appear as the "delayed" status.
+	 * (Note that "exposure" is already the "actual" exposure.)
+	 */
+	double actualGain = helper_->gain(gainCode);
+	double ratio = agcStatus->analogueGain / actualGain;
+	ratio *= agcStatus->exposureTime / exposure;
+	double newDigitalGain = agcStatus->digitalGain * ratio;
+	agcStatus->digitalGain = newDigitalGain;
+	agcStatus->analogueGain = actualGain;
+	agcStatus->exposureTime = exposure;
+
 	/*
 	 * At present, there is no way of knowing if a control is read-only.
 	 * As a workaround, assume that if the minimum and maximum values of
diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h
index ed378f45..943a3c91 100644
--- a/src/ipa/rpi/common/ipa_base.h
+++ b/src/ipa/rpi/common/ipa_base.h
@@ -73,6 +73,9 @@  protected:
 	/* Remember the HDR status after a mode switch. */
 	HdrStatus hdrStatus_;
 
+	/* Remember the AGC status after a mode switch. */
+	AgcStatus agcStatus_;
+
 	/* Whether the stitch block (if available) needs to swap buffers. */
 	bool stitchSwapBuffers_;
 
@@ -86,6 +89,7 @@  private:
 
 	virtual void platformPrepareIsp(const PrepareParams &params,
 					RPiController::Metadata &rpiMetadata) = 0;
+	virtual void platformPrepareAgc(RPiController::Metadata &rpiMetadata) = 0;
 	virtual RPiController::StatisticsPtr platformProcessStats(Span<uint8_t> mem) = 0;
 
 	void setMode(const IPACameraSensorInfo &sensorInfo);
@@ -98,7 +102,7 @@  private:
 	void fillSyncParams(const PrepareParams &params, unsigned int ipaContext);
 	void reportMetadata(unsigned int ipaContext);
 	void applyFrameDurations(utils::Duration minFrameDuration, utils::Duration maxFrameDuration);
-	void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls,
+	void applyAGC(struct AgcStatus *agcStatus, ControlList &ctrls,
 		      utils::Duration frameDurationOffset = utils::Duration(0));
 
 	std::map<unsigned int, MappedFrameBuffer> buffers_;
diff --git a/src/ipa/rpi/pisp/pisp.cpp b/src/ipa/rpi/pisp/pisp.cpp
index e1a804f5..ab70d8f4 100644
--- a/src/ipa/rpi/pisp/pisp.cpp
+++ b/src/ipa/rpi/pisp/pisp.cpp
@@ -218,13 +218,14 @@  private:
 
 	void platformPrepareIsp(const PrepareParams &params,
 				RPiController::Metadata &rpiMetadata) override;
+	void platformPrepareAgc(RPiController::Metadata &rpiMetadata) override;
 	RPiController::StatisticsPtr platformProcessStats(Span<uint8_t> mem) override;
 
 	void handleControls(const ControlList &controls) override;
 
-	void applyWBG(const AwbStatus *awbStatus, const AgcPrepareStatus *agcStatus,
+	void applyWBG(const AwbStatus *awbStatus, double digitalGain,
 		      pisp_be_global_config &global);
-	void applyDgOnly(const AgcPrepareStatus *agcPrepareStatus, pisp_be_global_config &global);
+	void applyDgOnly(double digitalGain, pisp_be_global_config &global);
 	void applyCAC(const CacStatus *cacStatus, pisp_be_global_config &global);
 	void applyContrast(const ContrastStatus *contrastStatus,
 			   pisp_be_global_config &global);
@@ -341,7 +342,6 @@  void IpaPiSP::platformPrepareIsp([[maybe_unused]] const PrepareParams &params,
 				PISP_BE_RGB_ENABLE_SHARPEN + PISP_BE_RGB_ENABLE_SAT_CONTROL);
 
 	NoiseStatus *noiseStatus = rpiMetadata.getLocked<NoiseStatus>("noise.status");
-	AgcPrepareStatus *agcPrepareStatus = rpiMetadata.getLocked<AgcPrepareStatus>("agc.prepare_status");
 
 	{
 		/* All Frontend config goes first, we do not want to hold the FE lock for long! */
@@ -355,14 +355,6 @@  void IpaPiSP::platformPrepareIsp([[maybe_unused]] const PrepareParams &params,
 		if (blackLevelStatus)
 			applyBlackLevel(blackLevelStatus, global);
 
-		AwbStatus *awbStatus = rpiMetadata.getLocked<AwbStatus>("awb.status");
-		if (awbStatus && agcPrepareStatus) {
-			/* Applies digital gain as well. */
-			applyWBG(awbStatus, agcPrepareStatus, global);
-		} else if (agcPrepareStatus) {
-			/* Mono sensor fallback for digital gain. */
-			applyDgOnly(agcPrepareStatus, global);
-		}
 	}
 
 	CacStatus *cacStatus = rpiMetadata.getLocked<CacStatus>("cac.status");
@@ -443,6 +435,34 @@  void IpaPiSP::platformPrepareIsp([[maybe_unused]] const PrepareParams &params,
 	}
 }
 
+void IpaPiSP::platformPrepareAgc(RPiController::Metadata &rpiMetadata)
+{
+	std::scoped_lock<RPiController::Metadata> l(rpiMetadata);
+
+	AgcStatus *delayedAgcStatus = rpiMetadata.getLocked<AgcStatus>("agc.delayed_status");
+	/* If no delayed status, use the gain from the last mode switch. */
+	double digitalGain = delayedAgcStatus ? delayedAgcStatus->digitalGain : agcStatus_.digitalGain;
+	AwbStatus *awbStatus = rpiMetadata.getLocked<AwbStatus>("awb.status");
+
+	pisp_be_global_config global;
+	be_->GetGlobal(global);
+
+	{
+		/* All Frontend config goes first, we do not want to hold the FE lock for long! */
+		std::scoped_lock<FrontEnd> lf(*fe_);
+
+		if (awbStatus) {
+			/* Applies digital gain as well. */
+			applyWBG(awbStatus, digitalGain, global);
+		} else {
+			/* Mono sensor fallback for digital gain. */
+			applyDgOnly(digitalGain, global);
+		}
+	}
+
+	be_->SetGlobal(global);
+}
+
 RPiController::StatisticsPtr IpaPiSP::platformProcessStats(Span<uint8_t> mem)
 {
 	using namespace RPiController;
@@ -515,12 +535,11 @@  void IpaPiSP::handleControls(const ControlList &controls)
 	}
 }
 
-void IpaPiSP::applyWBG(const AwbStatus *awbStatus, const AgcPrepareStatus *agcPrepareStatus,
+void IpaPiSP::applyWBG(const AwbStatus *awbStatus, double digitalGain,
 		       pisp_be_global_config &global)
 {
 	pisp_wbg_config wbg;
 	pisp_fe_rgby_config rgby = {};
-	double dg = agcPrepareStatus ? agcPrepareStatus->digitalGain : 1.0;
 	double minColourGain = std::min({ awbStatus->gainR, awbStatus->gainG, awbStatus->gainB, 1.0 });
 	/* The 0.1 here doesn't mean much, but just stops arithmetic errors and extreme behaviour. */
 	double extraGain = 1.0 / std::max({ minColourGain, 0.1 });
@@ -536,9 +555,9 @@  void IpaPiSP::applyWBG(const AwbStatus *awbStatus, const AgcPrepareStatus *agcPr
 	double gainG = awbStatus->gainG * extraGain;
 	double gainB = awbStatus->gainB * extraGain;
 
-	wbg.gain_r = clampField(dg * gainR, 14, 10);
-	wbg.gain_g = clampField(dg * gainG, 14, 10);
-	wbg.gain_b = clampField(dg * gainB, 14, 10);
+	wbg.gain_r = clampField(digitalGain * gainR, 14, 10);
+	wbg.gain_g = clampField(digitalGain * gainG, 14, 10);
+	wbg.gain_b = clampField(digitalGain * gainB, 14, 10);
 
 	/*
 	 * The YCbCr conversion block should contain the appropriate YCbCr
@@ -561,15 +580,15 @@  void IpaPiSP::applyWBG(const AwbStatus *awbStatus, const AgcPrepareStatus *agcPr
 	global.bayer_enables |= PISP_BE_BAYER_ENABLE_WBG;
 }
 
-void IpaPiSP::applyDgOnly(const AgcPrepareStatus *agcPrepareStatus, pisp_be_global_config &global)
+void IpaPiSP::applyDgOnly(double digitalGain, pisp_be_global_config &global)
 {
 	pisp_wbg_config wbg;
 
-	wbg.gain_r = clampField(agcPrepareStatus->digitalGain, 14, 10);
-	wbg.gain_g = clampField(agcPrepareStatus->digitalGain, 14, 10);
-	wbg.gain_b = clampField(agcPrepareStatus->digitalGain, 14, 10);
+	wbg.gain_r = clampField(digitalGain, 14, 10);
+	wbg.gain_g = clampField(digitalGain, 14, 10);
+	wbg.gain_b = clampField(digitalGain, 14, 10);
 
-	LOG(IPARPI, Debug) << "Applying DG (only) : " << agcPrepareStatus->digitalGain;
+	LOG(IPARPI, Debug) << "Applying DG (only) : " << digitalGain;
 
 	be_->SetWbg(wbg);
 	global.bayer_enables |= PISP_BE_BAYER_ENABLE_WBG;
diff --git a/src/ipa/rpi/vc4/vc4.cpp b/src/ipa/rpi/vc4/vc4.cpp
index 8a7a37c8..b2fec934 100644
--- a/src/ipa/rpi/vc4/vc4.cpp
+++ b/src/ipa/rpi/vc4/vc4.cpp
@@ -57,14 +57,14 @@  private:
 	int32_t platformConfigure(const ConfigParams &params, ConfigResult *result) override;
 
 	void platformPrepareIsp(const PrepareParams &params, RPiController::Metadata &rpiMetadata) override;
+	void platformPrepareAgc([[maybe_unused]] RPiController::Metadata &rpiMetadata) override;
 	RPiController::StatisticsPtr platformProcessStats(Span<uint8_t> mem) override;
 
 	void handleControls(const ControlList &controls) override;
 	bool validateIspControls();
 
 	void applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls);
-	void applyDG(const struct AgcPrepareStatus *dgStatus,
-		     const struct AwbStatus *awbStatus, ControlList &ctrls);
+	void applyDG(double digitalGain, const struct AwbStatus *awbStatus, ControlList &ctrls);
 	void applyCCM(const struct CcmStatus *ccmStatus, ControlList &ctrls);
 	void applyBlackLevel(const struct BlackLevelStatus *blackLevelStatus, ControlList &ctrls);
 	void applyGamma(const struct ContrastStatus *contrastStatus, ControlList &ctrls);
@@ -78,6 +78,7 @@  private:
 
 	/* VC4 ISP controls. */
 	ControlInfoMap ispCtrls_;
+	ControlList ctrls_;
 
 	/* LS table allocation passed in from the pipeline handler. */
 	SharedFD lsTableHandle_;
@@ -107,6 +108,7 @@  int32_t IpaVc4::platformStart([[maybe_unused]] const ControlList &controls,
 int32_t IpaVc4::platformConfigure(const ConfigParams &params, [[maybe_unused]] ConfigResult *result)
 {
 	ispCtrls_ = params.ispControls;
+	ctrls_ = ControlList(ispCtrls_);
 	if (!validateIspControls()) {
 		LOG(IPARPI, Error) << "ISP control validation failed.";
 		return -1;
@@ -139,7 +141,7 @@  int32_t IpaVc4::platformConfigure(const ConfigParams &params, [[maybe_unused]] C
 void IpaVc4::platformPrepareIsp([[maybe_unused]] const PrepareParams &params,
 				RPiController::Metadata &rpiMetadata)
 {
-	ControlList ctrls(ispCtrls_);
+	ControlList &ctrls = ctrls_;
 
 	/* Lock the metadata buffer to avoid constant locks/unlocks. */
 	std::unique_lock<RPiController::Metadata> lock(rpiMetadata);
@@ -152,9 +154,6 @@  void IpaVc4::platformPrepareIsp([[maybe_unused]] const PrepareParams &params,
 	if (ccmStatus)
 		applyCCM(ccmStatus, ctrls);
 
-	AgcPrepareStatus *dgStatus = rpiMetadata.getLocked<AgcPrepareStatus>("agc.prepare_status");
-	applyDG(dgStatus, awbStatus, ctrls);
-
 	AlscStatus *lsStatus = rpiMetadata.getLocked<AlscStatus>("alsc.status");
 	if (lsStatus)
 		applyLS(lsStatus, ctrls);
@@ -190,9 +189,18 @@  void IpaVc4::platformPrepareIsp([[maybe_unused]] const PrepareParams &params,
 		if (!lensctrls.empty())
 			setLensControls.emit(lensctrls);
 	}
+}
 
-	if (!ctrls.empty())
-		setIspControls.emit(ctrls);
+void IpaVc4::platformPrepareAgc(RPiController::Metadata &rpiMetadata)
+{
+	AgcStatus *delayedAgcStatus = rpiMetadata.getLocked<AgcStatus>("agc.delayed_status");
+	double digitalGain = delayedAgcStatus ? delayedAgcStatus->digitalGain : agcStatus_.digitalGain;
+	AwbStatus *awbStatus = rpiMetadata.getLocked<AwbStatus>("awb.status");
+
+	applyDG(digitalGain, awbStatus, ctrls_);
+
+	setIspControls.emit(ctrls_);
+	ctrls_ = ControlList(ispCtrls_);
 }
 
 RPiController::StatisticsPtr IpaVc4::platformProcessStats(Span<uint8_t> mem)
@@ -329,11 +337,9 @@  void IpaVc4::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)
 		  static_cast<int32_t>(awbStatus->gainB * 1000));
 }
 
-void IpaVc4::applyDG(const struct AgcPrepareStatus *dgStatus,
+void IpaVc4::applyDG(double digitalGain,
 		     const struct AwbStatus *awbStatus, ControlList &ctrls)
 {
-	double digitalGain = dgStatus ? dgStatus->digitalGain : 1.0;
-
 	if (awbStatus) {
 		/*
 		 * We must apply sufficient extra digital gain to stop any of the channel gains being