Re: [PATCH v5 11/12] media: microchip-isc: smooth AWB gains with EMA filter

[Eugen Hristev]

On 5/27/26 14:07, Balakrishnan Sambath wrote:
> Apply exponential moving average (alpha=0.25) to reduce per-frame
> flicker from sensor noise.
> 
> Signed-off-by: Balakrishnan Sambath <balakrishnan.s@xxxxxxxxxxxxx>
> ---
>  drivers/media/platform/microchip/microchip-isc-base.c | 19 ++++++++++++++++---
>  drivers/media/platform/microchip/microchip-isc.h      |  1 +
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
> index a2719830d39b..d07ea2fa33c6 100644
> --- a/drivers/media/platform/microchip/microchip-isc-base.c
> +++ b/drivers/media/platform/microchip/microchip-isc-base.c
> @@ -94,6 +94,7 @@ static inline void isc_reset_awb_ctrls(struct isc_device *isc)
>  	for (c = ISC_HIS_CFG_MODE_GR; c <= ISC_HIS_CFG_MODE_B; c++) {
>  		/* gains have a fixed point at 9 decimals */
>  		ctrls->gain[c] = 1 << 9;
> +		ctrls->gain_smooth[c] = 1 << 9;
>  		/* offsets are in 2's complements */
>  		ctrls->offset[c] = 0;
>  	}
> @@ -1477,11 +1478,23 @@ static void isc_wb_update(struct isc_ctrls *ctrls)
>  		/* Combine stretch and grey-world gains; result stays in Q9. */
>  		gain = (s_gain * gw_gain) >> 9;
>  
> -		ctrls->gain[c] = clamp_val(gain, 0, GENMASK(12, 0));
> +		/*
> +		 * Smooth gain updates with an exponential weighted average
> +		 * to suppress per-frame flicker:
> +		 *   smooth[n] = (3 * smooth[n-1] + gain) / 4
> +		 * Clamp to the hardware register width to prevent unbounded
> +		 * accumulation under degenerate (near-empty histogram) inputs.
> +		 */
> +		ctrls->gain_smooth[c] = (3 * ctrls->gain_smooth[c] + gain) / 4;
> +		ctrls->gain_smooth[c] = min_t(u32, ctrls->gain_smooth[c],
> +					      GENMASK(12, 0));
> +
> +		ctrls->gain[c] = ctrls->gain_smooth[c];

If now 'gain' becomes 'gain_smooth' , what is the purpose of still
having 'gain' at all ?
Does it make sense to just recompute gain in the new way ?


>  
>  		dev_dbg(isc->dev,
> -			"isc wb: c=%u black=%u avg=%u s_gain=%u gw_gain=%u gain=%u",
> -			c, hist_min, channel_avg, s_gain, gw_gain, gain);
> +			"isc wb: c=%u black=%u avg=%u s_gain=%u gw_gain=%u gain=%u smooth=%u\n",
> +			c, hist_min, channel_avg, s_gain, gw_gain, gain,
> +			ctrls->gain_smooth[c]);
>  	}
>  }
>  
> diff --git a/drivers/media/platform/microchip/microchip-isc.h b/drivers/media/platform/microchip/microchip-isc.h
> index 45168c62e3bc..0ae9b4e8f32d 100644
> --- a/drivers/media/platform/microchip/microchip-isc.h
> +++ b/drivers/media/platform/microchip/microchip-isc.h
> @@ -149,6 +149,7 @@ struct isc_ctrls {
>  
>  	/* one for each component : GR, R, GB, B */
>  	u32 gain[HIST_BAYER];
> +	u32 gain_smooth[HIST_BAYER];
>  	s32 offset[HIST_BAYER];
>  
>  	u32 hist_entry[HIST_ENTRIES];
>