[PATCH v1 1/5] ipa: rkisp1: awb: Clamp gains to machine limits

[Dan Scally]

Hi Stefan, thanks for the set

On 12/07/2024 15:32, Stefan Klug wrote:
> When the color gains where set manually it was possible to specify a
> gain that wrapped the hardware limits. It would also be possible to
> further tune the floating point limits, but that is an error prone
> approach. So the limits are imposed on the integers, just before writing
> to the hardware. This noticeably reduces some oscillations in the awb
> regulation.
>
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> ---

The code change is fine:

Reviewed-by: Daniel Scally <dan.scally at ideasonboard.com>


But how's it possible that you get values larger than 1023 here? The control value is declared with 
a range of 0 to 3.996; is it that the limits aren't checked when controls are set, or is there some 
floating point magic going on?

>   src/ipa/rkisp1/algorithms/awb.cpp | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> index a01fe5d90973..1a5d4776970a 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -120,10 +120,14 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,
>   		frameContext.awb.gains.blue = context.activeState.awb.gains.automatic.blue;
>   	}
>   
> -	params->others.awb_gain_config.gain_green_b = 256 * frameContext.awb.gains.green;
> -	params->others.awb_gain_config.gain_blue = 256 * frameContext.awb.gains.blue;
> -	params->others.awb_gain_config.gain_red = 256 * frameContext.awb.gains.red;
> -	params->others.awb_gain_config.gain_green_r = 256 * frameContext.awb.gains.green;
> +	params->others.awb_gain_config.gain_green_b =
> +		std::clamp<int>(256 * frameContext.awb.gains.green, 0, 0x3ff);
> +	params->others.awb_gain_config.gain_blue =
> +		std::clamp<int>(256 * frameContext.awb.gains.blue, 0, 0x3ff);
> +	params->others.awb_gain_config.gain_red =
> +		std::clamp<int>(256 * frameContext.awb.gains.red, 0, 0x3ff);
> +	params->others.awb_gain_config.gain_green_r =
> +		std::clamp<int>(256 * frameContext.awb.gains.green, 0, 0x3ff);
>   
>   	/* Update the gains. */
>   	params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN;