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

[Leo Li <sunpeng.li@amd.com>]

On 2026-09-03 01:59, Francis Marlou Pacaro wrote:
> [You don't often get email from pacaro.francis.marlou.n@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> dcn30_apply_idle_power_optimizations() derives the MALL frame cache
> hysteresis timer with
> 
>         tmr_delay = (uint32_t)(div_u64(..., denom) - 64LL);
> 
> div_u64() returns a u64, so when the quotient is smaller than 64 the
> subtraction wraps instead of going negative and tmr_delay ends up huge.
> The loop that follows tries to squeeze it into the 6 bit register field
> by doubling denom, but that only makes the quotient smaller, so tmr_delay
> can never converge.  tmr_scale is bumped past 3 and the function gives up
> with
> 
>         /* Delay exceeds range of hysteresis timer */
>         ASSERT(false);
> 
> even though the requested delay is too *short* to encode, not too long.
> 
> With mall_additional_timer_percent left at its default of 0, the quotient
> drops below 64 once the refresh rate used for the calculation goes above
> ~243 Hz.  Every DCN 3.0 display above that loses MALL static screen
> entirely and splats a WARN once per boot.  Reproduced on Navi 23
> (RX 6600) driving 1920x1080, resetting /sys/kernel/debug/clear_warn_once
> between modes:
> 
>         refresh   MALL       ASSERT
>         144 Hz    enabled    no
>         240 Hz    enabled    no
>         280 Hz    skipped    yes
>         360 Hz    skipped    yes
> 
> Commit 3bb68cec4db8 ("drm/amd/display: Add Overflow check to skip MALL")
> already covered the other end of the range, where a large stutter period
> makes the delay too long to encode.  Cover the short end by clamping to
> 0, which selects the shortest hysteresis the register can express,
> 65.28us * 64 = ~4.18ms.  That is marginally longer than what the formula
> asks for at these refresh rates, and erring long is the safe direction:
> it only delays MALL entry, it can never enter early.
> 
> The numerator does not change between iterations, only denom does, so
> compute it once and keep both call sites inside 100 columns.
> 
> The genuinely out of range case at very low refresh rates still reaches
> the ASSERT, which is where it belongs.
> 
> Fixes: 52f2e83e2fe5 ("drm/amdgpu/display: add MALL support (v2)")
> Signed-off-by: Francis Marlou Pacaro <pacaro.francis.marlou.n@gmail.com>
> ---
> Compile tested only: vanilla v7.2.2 built with clang 22.1.8 on x86_64
> using a CachyOS .config, full amdgpu module, no new warnings.  The WARN
> reproduction described above was observed on unpatched 6.18.48 and
> 7.2.2; I have not yet booted a kernel with this patch applied.
> 
>  .../gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> 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
> --- a/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c
> +++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c
> @@ -1064,10 +1064,12 @@
>                                  */
>                                 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);
> +                               tmr_delay = tmr_ticks > 64 ? (u32)(tmr_ticks - 64) : 0;

Thanks for the patch, the 0 clamp makes sense to me.
Only change: s/u64/uint64_t/ to follow DC convention.

With that,
Reviewed-by: Leo Li <sunpeng.li@amd.com>

- Leo

> 
>                                 /* In some cases the stutter period is really big (tiny modes) in these
>                                  * cases MALL cant be enabled, So skip these cases to avoid a ASSERT()
> @@ -1089,9 +1091,8 @@
>                                         }
> 
>                                         denom *= 2;
> -                                       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);
> +                                       tmr_delay = tmr_ticks > 64 ? (u32)(tmr_ticks - 64) : 0;
>                                 }
> 
>                                 /* Copy HW cursor */
> --
> 2.51.0
>