[libcamera-devel] [PATCH] ipa: ipu3: Correct context during configure()

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Fri, Sep 30, 2022 at 08:08:21PM +0100, Kieran Bingham via libcamera-devel wrote:
> The introduction of the FCQueue in the IPU3 inadvertently introduced a
> bug which cleared the initialisation of the session configuration
> immediately after some parameters had been set.
> 
> Furthermore, it cleared and never re-initialised the sensor line
> duration property, which was previously only set during the call to
> init().
> 
> Move the clearing of the contexts from the updateSessionConfiguration()
> call to the earliest opportunity in configure(), and immediately
> re-initialise the sensor parameters.
> 
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=151
> Fixes: 85c5c47325ab ("ipa: ipu3: Use the FCQueue")
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/ipa/ipu3/ipu3.cpp | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index d1ea081d595d..7fb62c749fde 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -217,11 +217,6 @@ void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)
>  	int32_t minGain = v4l2Gain.min().get<int32_t>();
>  	int32_t maxGain = v4l2Gain.max().get<int32_t>();
>  
> -	/* Clear the IPA context before the streaming session. */
> -	context_.configuration = {};
> -	context_.activeState = {};
> -	context_.frameContexts.clear();
> -
>  	/*
>  	 * When the AGC computes the new exposure values for a frame, it needs
>  	 * to know the limits for shutter speed and analogue gain.
> @@ -498,6 +493,14 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
>  
>  	lensCtrls_ = configInfo.lensControls;
>  
> +	/* Clear the IPA context for the new streaming session. */
> +	context_.activeState = {};
> +	context_.configuration = {};
> +	context_.frameContexts.clear();
> +
> +	/* Intialise the sensor configuration */
> +	context_.configuration.sensor.lineDuration = sensorInfo_.lineLength * 1.0s / sensorInfo_.pixelRate;

This could be dropped from init(), but that can be done in a different
patch, we should revisit the init()/configure() split in the IPU3 IPA
module implementation.

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

> +
>  	/*
>  	 * Compute the sensor V4L2 controls to be used by the algorithms and
>  	 * to be set on the sensor.

-- 
Regards,

Laurent Pinchart