[libcamera-devel] [PATCH] ipa: ipu3: af: enforce grid size restrictions

[Umang Jain]

Hello Kieran,

Thank you for the patch,

On 3/31/22 17:40, Kieran Bingham via libcamera-devel wrote:
> Provide enforcement of the selection of the block_{width,height}_log2
> parameters to the capabilities of the hardware.
>
> While this selection is currently hardcoded to the minimum, providing
> the restriction now allows for further dynamic sizing in the future and
> documents the restrictions directly in code, making use of the already
> existing constants.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


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

> ---
>
> Note, this is kept separate from my other AF improvement series, as it
> can be applied independantly - but due to a compilation failure if
> kAfMinGridBlockWidth, kAfMaxGridBlockWidth kAfMinGridBlockHeight, and
> kAfMaxGridBlockHeight are not used, this patch must be applied before
> that series can be integrated.
>
>   src/ipa/ipu3/algorithms/af.cpp | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
>
> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp
> index 6c25d02ec7f0..abfaad6164f1 100644
> --- a/src/ipa/ipu3/algorithms/af.cpp
> +++ b/src/ipa/ipu3/algorithms/af.cpp
> @@ -139,6 +139,22 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>   	grid.height = kAfMinGridHeight;
>   	grid.block_width_log2 = kAfMinGridBlockWidth;
>   	grid.block_height_log2 = kAfMinGridBlockHeight;
> +
> +	/*
> +	 * \todo - while this clamping code is effectively a no-op, it satisfies
> +	 * the compiler that the definitions regarding the hardware restrictions
> +	 * are used, and paves the way to support dynamic grid sizing in the
> +	 * future. While the block_{width,height}_log2 remain assigned to the
> +	 * minimum, this code should be optimized out by the compiler.
> +	 */
> +	grid.block_width_log2 = std::clamp(grid.block_width_log2,
> +					   kAfMinGridBlockWidth,
> +					   kAfMaxGridBlockWidth);
> +
> +	grid.block_height_log2 = std::clamp(grid.block_height_log2,
> +					    kAfMinGridBlockHeight,
> +					    kAfMaxGridBlockHeight);
> +
>   	grid.height_per_slice = kAfDefaultHeightPerSlice;
>   
>   	/* Position the AF grid in the center of the BDS output. */