[libcamera-devel] [PATCH 1/2] ipa: ipu3: agc: Drop hard-codec analogue gain max

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Mon, May 29, 2023 at 02:39:25PM +0200, Jacopo Mondi via libcamera-devel wrote:
> As the sensor's analogue gain range is known, drop the arbitrary
> maximum limit for the sensor analogue gain.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

I'm pretty sure that as soon as we'll merge this patch we'll get a
regression report and will need to implement proper AGC tuning. Oh well
:-)

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  src/ipa/ipu3/algorithms/agc.cpp | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index b5309bdbea25..606a237a4a59 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -47,9 +47,8 @@ namespace ipa::ipu3::algorithms {
>  
>  LOG_DEFINE_CATEGORY(IPU3Agc)
>  
> -/* Limits for analogue gain values */
> +/* Minimum limit for analogue gain value */
>  static constexpr double kMinAnalogueGain = 1.0;
> -static constexpr double kMaxAnalogueGain = 8.0;
>  
>  /* \todo Honour the FrameDurationLimits control instead of hardcoding a limit */
>  static constexpr utils::Duration kMaxShutterSpeed = 60ms;
> @@ -97,10 +96,10 @@ int Agc::configure(IPAContext &context,
>  				    kMaxShutterSpeed);
>  
>  	minAnalogueGain_ = std::max(configuration.agc.minAnalogueGain, kMinAnalogueGain);
> -	maxAnalogueGain_ = std::min(configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
> +	maxAnalogueGain_ = configuration.agc.maxAnalogueGain;
>  
>  	/* Configure the default exposure and gain. */
> -	activeState.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain);
> +	activeState.agc.gain = minAnalogueGain_;
>  	activeState.agc.exposure = 10ms / configuration.sensor.lineDuration;
>  
>  	frameCount_ = 0;

-- 
Regards,

Laurent Pinchart