[libcamera-devel] [RFC PATCH] ipa: ipu3: Clear incoming parameter use flags

[Jean-Michel Hautbois]

Hi Kieran,

On 10/09/2021 17:49, Kieran Bingham wrote:
> The incoming params buffer may contain uninitialised data, or the
> parameters of previously queued frames. Clearing the entire buffer
> may be an expensive operation, and the kernel will only read from
> structures which have their associated use-flag set.
> 
> It is the responsibility of the algorithms to set the use flags
> accordingly for any data structure they update during prepare().
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Thanks for the patch !
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>

> 
> ---
> 
> Yes, the commit message is the same as the comment before the line.
> I felt the text was worthy of documenting the clearing of the flags, and
> ensuring that it's documented in the code that the algorithms are
> responsible for setting their use flag of any structure they modify.
> 
> Note that this is sent compile tested only, as it's something I noticed,
> while writing documentation and wanted to check.
> 
>  src/ipa/ipu3/ipu3.cpp | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index c925cf642611..30d2a53903ec 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -457,6 +457,17 @@ void IPAIPU3::processControls([[maybe_unused]] unsigned int frame,
>  
>  void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)
>  {
> +	/*
> +	 * The incoming params buffer may contain uninitialised data, or the
> +	 * parameters of previously queued frames. Clearing the entire buffer
> +	 * may be an expensive operation, and the kernel will only read from
> +	 * structures which have their associated use-flag set.
> +	 *
> +	 * It is the responsibility of the algorithms to set the use flags
> +	 * accordingly for any data structure they update during prepare().
> +	 */
> +	params->use = {};
> +
>  	for (auto const &algo : algorithms_)
>  		algo->prepare(context_, params);
>  
>