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

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

[Francis Marlou Pacaro]

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;
 
 				/* 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

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

[sashiko-bot]

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

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

[Francis Marlou Pacaro]

On Thu, 03 Sep 2026 15:41:51 +0800, sashiko-bot@kernel.org wrote:
> [Severity: High]
> This is a pre-existing issue, but could this calculation trigger a division
> by zero kernel panic if denom is 0?
[...]
> Would it make sense to add an early return to check if refresh_hz evaluates
> to 0 before proceeding with these calculations?

The arithmetic is right: refresh_hz == 0 gives denom == 0, which divides
by zero both in the div_u64() and in the 100000/refresh_hz check a few
lines below.

As noted, it is pre-existing.  The original code divided by the same
denom, so this patch does not change that exposure in either direction.

For it to trigger, refresh_hz = pix_clk_100hz * 100 / (v_total * h_total)
would have to truncate to zero, i.e. a mode below 1 Hz.  This block only
runs for a single active stream that has already passed the MALL
eligibility checks above it, and with VRR the v_total used here is
adjust.v_total_max, which corresponds to the panel's minimum refresh
(48 Hz on the display I tested).  I could not construct a case where a
live stream reaches this point with refresh_hz == 0.

I would rather not fold a guard into this patch: it carries a Fixes: tag
for the underflow, and the two problems are independent.  If the
hardening is wanted I am happy to send it separately, something like

	if (!refresh_hz)
		return false;

before denom is computed.

Thanks,
Francis

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

[Francis Marlou Pacaro]

This is now runtime tested, so the "compile tested only" note under the
--- in the original posting is out of date.

I rebuilt my distro kernel (CachyOS 7.2.2, same source and config, clang
ThinLTO, generic x86_64) with this patch applied.  Hardware is Navi 23
(RX 6600, DCN 3.0.2) driving an ASUS XG279CNS over DisplayPort at
1920x1080, VRR range 48-360.

Unpatched:

  - the WARN fired once per boot, at the compositor's first modeset
  - after resetting /sys/kernel/debug/clear_warn_once it reproduced on
    demand at 280 Hz and 360 Hz, and never at or below 240 Hz, which
    matches the ~243 Hz threshold in the commit message

Patched:

  - no WARN at boot
  - no WARN at 280 Hz or 360 Hz, with clear_warn_once reset before each
    mode, so those are independent negatives rather than one suppressed
    warning
  - the function no longer bails out of the hysteresis calculation at
    those refresh rates

No display problems observed on the patched kernel.

Thanks,
Francis

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

[Francis Marlou Pacaro]

On Thu, 03 Sep 2026 15:41:51 +0800, sashiko-bot@kernel.org wrote:
> [Severity: High]
> This is a pre-existing issue, but could this calculation trigger a division
> by zero kernel panic if denom is 0?
[...]
> Would it make sense to add an early return to check if refresh_hz evaluates
> to 0 before proceeding with these calculations?

The arithmetic is right: refresh_hz == 0 gives denom == 0, which divides
by zero both in the div_u64() and in the 100000/refresh_hz check a few
lines below.

As noted, it is pre-existing.  The original code divided by the same
denom, so this patch does not change that exposure in either direction.

For it to trigger, refresh_hz = pix_clk_100hz * 100 / (v_total * h_total)
would have to truncate to zero, i.e. a mode below 1 Hz.  This block only
runs for a single active stream that has already passed the MALL
eligibility checks above it, and with VRR the v_total used here is
adjust.v_total_max, which corresponds to the panel's minimum refresh
(48 Hz on the display I tested).  I could not construct a case where a
live stream reaches this point with refresh_hz == 0.

I would rather not fold a guard into this patch: it carries a Fixes: tag
for the underflow, and the two problems are independent.  If the
hardening is wanted I am happy to send it separately, something like

	if (!refresh_hz)
		return false;

before denom is computed.

Thanks,
Francis

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

[Leo Li]

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
>