[PATCH 2/5] ipa: rkisp1: agc: Add digital gain

[Umang Jain]

Hi Paul,

On 05/04/24 8:17 pm, Paul Elder wrote:
> Add support to the rkisp1 AGC to set digital gain.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>   src/ipa/rkisp1/algorithms/agc.cpp | 5 +++++
>   src/ipa/rkisp1/ipa_context.h      | 3 +++
>   src/ipa/rkisp1/rkisp1.cpp         | 2 ++
>   3 files changed, 10 insertions(+)
>
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index fd47ba4e..7220f00a 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -10,6 +10,7 @@
>   #include <algorithm>
>   #include <chrono>
>   #include <cmath>
> +#include <tuple>

Is this required? Not sure by reading this particular patch

Looks good to me otherwise. With this addressed,

Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

>   
>   #include <libcamera/base/log.h>
>   #include <libcamera/base/utils.h>
> @@ -152,8 +153,10 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
>   	context.activeState.agc.automatic.gain = context.configuration.sensor.minAnalogueGain;
>   	context.activeState.agc.automatic.exposure =
>   		10ms / context.configuration.sensor.lineDuration;
> +	context.activeState.agc.automatic.dgain = 1;
>   	context.activeState.agc.manual.gain = context.activeState.agc.automatic.gain;
>   	context.activeState.agc.manual.exposure = context.activeState.agc.automatic.exposure;
> +	context.activeState.agc.manual.dgain = 1;
>   	context.activeState.agc.autoEnabled = !context.configuration.raw;
>   
>   	/*
> @@ -237,6 +240,7 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,
>   	if (frameContext.agc.autoEnabled) {
>   		frameContext.agc.exposure = context.activeState.agc.automatic.exposure;
>   		frameContext.agc.gain = context.activeState.agc.automatic.gain;
> +		frameContext.agc.dgain = context.activeState.agc.automatic.dgain;
>   	}
>   
>   	/* \todo Remove this when we can set the below with controls */
> @@ -380,6 +384,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>   	/* Update the estimated exposure and gain. */
>   	activeState.agc.automatic.exposure = shutterTime / context.configuration.sensor.lineDuration;
>   	activeState.agc.automatic.gain = aGain;
> +	activeState.agc.automatic.dgain = dGain;
>   
>   	fillMetadata(context, frameContext, metadata);
>   }
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index 256b75eb..a70c7eb3 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -61,10 +61,12 @@ struct IPAActiveState {
>   		struct {
>   			uint32_t exposure;
>   			double gain;
> +			double dgain;
>   		} manual;
>   		struct {
>   			uint32_t exposure;
>   			double gain;
> +			double dgain;
>   		} automatic;
>   
>   		bool autoEnabled;
> @@ -110,6 +112,7 @@ struct IPAFrameContext : public FrameContext {
>   	struct {
>   		uint32_t exposure;
>   		double gain;
> +		double dgain;
>   		bool autoEnabled;
>   	} agc;
>   
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index b0bbcd8c..d66dfdd7 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -446,10 +446,12 @@ void IPARkISP1::setControls(unsigned int frame)
>   	IPAFrameContext &frameContext = context_.frameContexts.get(frame);
>   	uint32_t exposure = frameContext.agc.exposure;
>   	uint32_t gain = camHelper_->gainCode(frameContext.agc.gain);
> +	uint32_t dgain = camHelper_->gainCode(frameContext.agc.dgain);
>   
>   	ControlList ctrls(sensorControls_);
>   	ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));
>   	ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain));
> +	ctrls.set(V4L2_CID_DIGITAL_GAIN, static_cast<int32_t>(dgain));
>   
>   	setSensorControls.emit(frame, ctrls);
>   }