On 2026-09-03 01:59, Francis Marlou Pacaro wrote:
> [You don't often get email from [email protected]. 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 <[email protected]>
> ---
> 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 <[email protected]>
- 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
>