[RFC PATCH 7/7] ipa: rkisp1: Set frameContext.agc in queueRequest for auto mode also

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Fri, Dec 20, 2024 at 05:26:53PM +0100, Stefan Klug wrote:
> If the agc is in auto mode, exposure time and gain used to be set on the
> frame context within prepare(). As exposure time and gain are used by
> getSensorControls(0) from within start() that is too late (prepare()
> hasn't run yet). Also prepare() is documented as the place to initialize
> the params buffer, not the frame context. From the pipeline point of
> view, prepare() gets called immediately after queueRequest(). Therefore
> we can safely move setting the frame context into queueRequest() to fix
> the sensor controls for start().

I don't think that's right. When queueRequest() is called the active
state doesn't contain recent enough agc values.

It may be that computeParams() is called right after queueRequest() in
the pipeline handler, but that's not how it should be.

> 
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/agc.cpp | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 40e5a8f481b2..46be1413a728 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -243,7 +243,10 @@ void Agc::queueRequest(IPAContext &context,
>  
>  	frameContext.agc.autoEnabled = agc.autoEnabled;
>  
> -	if (!frameContext.agc.autoEnabled) {
> +	if (frameContext.agc.autoEnabled) {
> +		frameContext.agc.exposure = context.activeState.agc.automatic.exposure;
> +		frameContext.agc.gain = context.activeState.agc.automatic.gain;
> +	} else {
>  		frameContext.agc.exposure = agc.manual.exposure;
>  		frameContext.agc.gain = agc.manual.gain;
>  	}
> @@ -283,11 +286,6 @@ void Agc::queueRequest(IPAContext &context,
>  void Agc::prepare(IPAContext &context, const uint32_t frame,
>  		  IPAFrameContext &frameContext, RkISP1Params *params)
>  {
> -	if (frameContext.agc.autoEnabled) {
> -		frameContext.agc.exposure = context.activeState.agc.automatic.exposure;
> -		frameContext.agc.gain = context.activeState.agc.automatic.gain;
> -	}
> -
>  	if (frame > 0 && !frameContext.agc.updateMetering)
>  		return;
>  

-- 
Regards,

Laurent Pinchart