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]>
Reviewed-by: Leo Li <[email protected]>
---
v2: s/u64/uint64_t/ to follow DC convention (Leo Li).
Leo - I also restored the two (uint32_t) casts while I was in there.
v1 had drifted those from the (uint32_t) the replaced line used, and
leaving uint64_t declarations beside (u32) casts looked worse than
fixing both; dcn30_hwseq.c is back to zero u64/u32 uses. Say the
word if you would rather that were its own patch and I will drop it
from this one.
No functional change. checkpatch --strict reports "Prefer kernel
type 'u64' over 'uint64_t'" on the two declarations; that is the
known DC false positive. 0 errors otherwise.
Boot tested since v1, on the hardware named above: Navi 23 (RX 6600),
DCN 3.0.2, 1920x1080 over DisplayPort, VRR 48-360 Hz. Same v7.2.2 tree
and .config either way, clang 22.1.8 / LLD 22.1.8:
7.2.2 build boot at greeter 360 Hz
unpatched WARN WARN
patched none none
Three boots of the patched build produced no WARN at all; the unpatched
build warned once per boot at the login greeter, where v_total is
briefly inconsistent across the handover and the computed refresh lands
just above the cliff. ASSERT() is WARN_ON_ONCE, so each 360 Hz test
reset /sys/kernel/debug/clear_warn_once immediately beforehand, which
keeps the negatives independent rather than one suppressed warning.
No regression below the cliff: 240 Hz still idles at 96 MHz memory clock
and ~5 W package power, unchanged from the unpatched build. MALL entry
itself is not instrumented, so this shows the ASSERT is gone and that
nothing regressed at or below the cliff, not that the frame cache now
engages above it.
.../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
index 2d3587a..2ecd3a4 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
@@ -1058,10 +1058,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;
+ uint64_t num = (1000000LL + 2 * stutter_period
* refresh_hz) *
+ (100LL +
dc->debug.mall_additional_timer_percent);
+ uint64_t 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 ?
(uint32_t)(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()
@@ -1083,9 +1085,8 @@ bool dcn30_apply_idle_power_optimizations(struct dc *dc,
bool enable)
}
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 ?
(uint32_t)(tmr_ticks - 64) : 0;
}
/* Copy HW cursor */
