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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The raw calculated `gain` is not clamped before being fed into the exponential moving average (EMA) filter, causing severe and prolonged overexposure artifacts under degenerate conditions.
- [High] The unconditionally applied EMA filter breaks the 'Do White Balance' (ISC_WB_ONETIME) feature, requiring users to trigger it multiple times to reach the correct balance.
- [Medium] The exponential moving average (EMA) filter introduces a truncation bias because integer division truncates towards zero.
--

commit 8392fdc05942f24c94ecfeddbdfc0822a7cde3b3
Author: Balakrishnan Sambath <balakrishnan.s@microchip.com>
Date:   Wed Jun 3 06:58:54 2026 +0000

media: microchip-isc: smooth AWB gains with EMA filter

This patch applies an exponential moving average (alpha=0.25) to the automatic
white balance (AWB) gains in the microchip-isc driver. This aims to reduce
per-frame flicker caused by sensor noise.

> diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
> index 6ebbb8adea452..621d90afe730c 100644
> --- a/drivers/media/platform/microchip/microchip-isc-base.c
> +++ b/drivers/media/platform/microchip/microchip-isc-base.c
> @@ -1467,11 +1467,20 @@ 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:
> +		 *   gain = (3 * gain_old + gain_new) / 4   (alpha = 0.25)
> +		 * Clamp to the hardware register width to prevent unbounded
> +		 * accumulation under degenerate (near-empty histogram) inputs.
> +		 */
> +		ctrls->gain[c] = (3 * ctrls->gain[c] + gain) / 4;

[Severity: High]
Does feeding the unbounded raw gain into the EMA filter before clamping cause
prolonged overexposure artifacts under degenerate conditions?

Under degenerate inputs (like a near-empty histogram where hist_max ==
hist_min), the raw calculated gain can reach up to 524,288. Because this
unbounded value is fed into the EMA filter before clamping, (3 * gain_old +
524288) / 4 evaluates to over 131,000, which is then clamped to the maximum
value 8,191.

When normal frames resume, the state is pinned at 8,191 and the EMA makes it
decay slowly, taking around 15 to 20 frames to return to normal values. Before
this patch, the unbounded gain was directly clamped, causing only a 1-frame
flash.

[Severity: High]
Could unconditionally applying the EMA filter here break the 'Do White Balance'
(ISC_WB_ONETIME) feature?

The ISC_WB_ONETIME feature performs exactly one 4-channel Bayer histogram cycle
to compute and apply white balance gains, then sets ctrls->awb = ISC_WB_NONE
and stops requesting histograms:

drivers/media/platform/microchip/microchip-isc-base.c:isc_awb_work() {
    ...
    if (ctrls->awb == ISC_WB_ONETIME) {
        dev_info(isc->dev,
                 "Completed one time white-balance adjustment.\n");
        /* update the v4l2 controls values */
        isc_update_v4l2_ctrls(isc);
        ctrls->awb = ISC_WB_NONE;
    }
    ...
}

By unconditionally applying the EMA filter here, this single adjustment cycle
only updates the gains by 25% of the required delta towards the target.

The one-time AWB fails to converge to the correct values, requiring users to
trigger the V4L2_CID_DO_WHITE_BALANCE ioctl multiple times to asymptotically
approach the correct balance.

[Severity: Medium]
Will this integer division introduce a truncation bias?

Because integer division truncates towards zero, when the target gain is
slightly above the current value (by 1 to 3 units), the fractional addition is
truncated and the value never increases. For example, if the current gain is
512 and the target is 515, (1536 + 515) / 4 = 512, so it never moves.

Conversely, downward trends track effortlessly. This creates a permanent
asymmetrical bias where the smoothed gain settles below the true mathematical
average. Symmetric rounding (3 * ctrls->gain[c] + gain + 2) / 4 might resolve
this.

> +		ctrls->gain[c] = min_t(u32, ctrls->gain[c], GENMASK(12, 0));
>  
>  		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\n",
> +			c, hist_min, channel_avg, s_gain, gw_gain,
> +			ctrls->gain[c]);
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603-microchip-isc-fixes-v6-0-8c3d7474a768@microchip.com?part=11