Re: [PATCH] drm/amd/display: fix MALL hysteresis timer underflow at high refresh rates

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Potential kernel panic due to division by zero if `refresh_hz` or `denom` evaluates to 0.
--

commit 66e20e6af2a54e1f051a2802b102f884a27f0948
Author: Francis Marlou Pacaro <pacaro.francis.marlou.n@gmail.com>

drm/amd/display: fix MALL hysteresis timer underflow at high refresh rates

This commit fixes an issue where high refresh rates cause the MALL frame
cache hysteresis timer calculation to wrap instead of going negative. It
addresses this by explicitly calculating the timer ticks and clamping the
result to 0 to prevent the underflow.

> diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c b/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c
> index 8b0a667dc59b7..76aa7a82e24e7 100644
> --- a/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c
> +++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c
> @@ -1065,10 +1065,12 @@ bool dcn30_apply_idle_power_optimizations(struct dc *dc, bool enable)
>  				 */
>  				unsigned int denom = refresh_hz * 6528;
>  				unsigned int stutter_period = dc->current_state->perf_params.stutter_period_us;
> +				u64 num = (1000000LL + 2 * stutter_period * refresh_hz) *
> +					(100LL + dc->debug.mall_additional_timer_percent);
> +				u64 tmr_ticks;
>  
> -				tmr_delay = (uint32_t)(div_u64(((1000000LL + 2 * stutter_period * refresh_hz) *
> -						(100LL + dc->debug.mall_additional_timer_percent) + denom - 1),
> -						denom) - 64LL);
> +				tmr_ticks = div_u64(num + denom - 1, denom);

[Severity: High]
This is a pre-existing issue, but could this calculation trigger a division
by zero kernel panic if denom is 0?

Looking at how refresh_hz is calculated earlier in
dcn30_apply_idle_power_optimizations():

    refresh_hz = (unsigned int)div_u64(
            (unsigned long long)stream->timing.pix_clk_100hz * 100LL,
            (v_total * stream->timing.h_total));

If a compositor provides a malformed display mode with very low clocks and
high totals such that pix_clk_100hz * 100 < v_total * h_total, refresh_hz
will evaluate to 0. This makes denom = 0, leading to a division by zero in
the div_u64() call above.

Additionally, this same condition natively panics a few lines down in the
unpatched context where we check the stutter period:

    if (stutter_period > 100000/refresh_hz)
        return false;

Would it make sense to add an early return to check if refresh_hz evaluates
to 0 before proceeding with these calculations?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903055952.62906-1-pacaro.francis.marlou.n@gmail.com?part=1