[Why]
Commit b96150a70696 ("drm/amd/display: Should support p-state under
dcn21") calls dcn21_fast_validate_bw() with allow_self_refresh_only
false. The second dml_get_voltage_level() pass, which retries with
dm_allow_self_refresh when no p-state capable voltage level exists,
is therefore never taken.
That fallback is the only way some short-vblank eDP timings can
validate. The AUO 1920x1080@360 panel on the ASUS ROG Strix G733QSA
(Cezanne, DCN 2.1) runs at 800.01 MHz with vtotal 1111: 31 lines of
vblank, about 77 us, too short for a DRAM p-state. It now fails
dml_get_voltage_level(), dcn21_validate_bandwidth() returns
DC_NOT_SUPPORTED, and the driver keeps 1920x1080@60 only:
[drm] Mode Validation Warning: Unknown Status failed validation.
create_validate_stream_for_sink: Unhandled validation failure 24
Rejected mode: "1920x1080": 360 800010 ... 1095 1111 (ERROR)
This is a regression versus v6.18, where 360 Hz was enumerated and
usable.
That commit dropped the fallback to work around a flip_done timeout
under MPO. atomic_check (DC_VALIDATE_MODE_ONLY) and commit_tail
(DC_VALIDATE_MODE_AND_PROGRAMMING) were observing different
soc.sr_exit_time_us / soc.sr_enter_plus_exit_time_us, so a configuration
could pass check and fail in commit_tail. Passing true again without
fixing that mismatch would bring the timeout back.
[How]
calculate_wm_set_for_vlevel() overlays three DML soc latencies with
each watermark range entry:
dml->soc.dram_clock_change_latency_us = table_entry->pstate_latency_us;
dml->soc.sr_exit_time_us = table_entry->sr_exit_time_us;
dml->soc.sr_enter_plus_exit_time_us =
table_entry->sr_enter_plus_exit_time_us;
and restores only dram_clock_change_latency_us. The stutter latencies
keep the last entry (WM set A) in context->bw_ctx.dml. Watermarks run
only for DC_VALIDATE_MODE_AND_PROGRAMMING, so the leak is written into
dc->current_state at commit_tail. atomic_check validates
dm_state->context with MODE_ONLY and never computes watermarks, so the
two copies of soc can disagree on the next full update, which copies
current_state and re-validates.
Snapshot both stutter latencies in dcn21_validate_bandwidth_fp() and
restore them after dcn21_calculate_wm() and
dcn20_calculate_dlg_params(). The DCN21 DLG path
(dml21_rq_dlg_get_dlg_reg()) still reads soc.sr_enter_plus_exit_time_us
while computing Lwait, so restoring afterwards keeps DLG reading the
same latencies as today; only the leak into dc->current_state goes
away.
With check and commit_tail now using the bounding-box latencies, pass
allow_self_refresh_only true again. Modes that can p-state still
succeed on the first pass; the fallback is taken only when no p-state
capable voltage level exists.
dcn301_fpu.c has the same helper and the same restore gap. Left alone;
I cannot test DCN 3.01.
Tested on ASUS ROG Strix G733QSA (Cezanne, DCN 2.1): 1920x1080@360 is
enumerated and in daily use on a kernel built with this change. The
original MPO flip_done timeout is not reproduced here; that path is
reasoned from the code.
Fixes: b96150a70696 ("drm/amd/display: Should support p-state under dcn21")
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5628
Cc: Wayne Lin <[email protected]>
Cc: Nicholas Kazlauskas <[email protected]>
Cc: [email protected]
Signed-off-by: Julien Le Bourg <[email protected]>
---
v2:
- Fix the soc stutter-latency leak that b96150a70696 worked around,
then restore the fallback. v1 only flipped the flag and would have
reintroduced the MPO flip_done timeout. Reported by the Sashiko
review bot on v1.
v1:
https://lore.kernel.org/amd-gfx/[email protected]/
.../drm/amd/display/dc/dml/dcn20/dcn20_fpu.c | 25 ++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
index e82f2d531211..3697a3139552 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
@@ -2321,6 +2321,8 @@ bool dcn21_validate_bandwidth_fp(struct dc *dc, struct
dc_state *context,
enum dc_validate_mode validate_mode,
display_e2e_pipe_params_st *pipes)
{
bool out = false;
+ double sr_exit_time_us;
+ double sr_enter_plus_exit_time_us;
BW_VAL_TRACE_SETUP();
@@ -2336,7 +2338,8 @@ bool dcn21_validate_bandwidth_fp(struct dc *dc, struct
dc_state *context,
/*Unsafe due to current pipe merge and split logic*/
ASSERT(context != dc->current_state);
- out = dcn21_fast_validate_bw(dc, context, pipes, &pipe_cnt,
pipe_split_from, &vlevel, validate_mode, false);
+ out = dcn21_fast_validate_bw(dc, context, pipes, &pipe_cnt,
pipe_split_from,
+ &vlevel, validate_mode, true);
if (pipe_cnt == 0)
goto validate_out;
@@ -2351,9 +2354,29 @@ bool dcn21_validate_bandwidth_fp(struct dc *dc, struct
dc_state *context,
goto validate_out;
}
+ /*
+ * calculate_wm_set_for_vlevel() overrides the DML soc stutter
+ * latencies with the values of each watermark range entry, but only
+ * restores dram_clock_change_latency_us. Snapshot the two stutter
+ * latencies and put them back once the watermark and DLG parameters
+ * have been computed, so the override cannot leak into
+ * dc->current_state. A later atomic_check() duplicates that state,
+ * and validating against mutated latencies is what makes
+ * atomic_check() and atomic_commit_tail() disagree.
+ *
+ * Restoring after dcn20_calculate_dlg_params() keeps the DLG
+ * computation reading the same latencies as before, so this only
+ * removes the leak.
+ */
+ sr_exit_time_us = context->bw_ctx.dml.soc.sr_exit_time_us;
+ sr_enter_plus_exit_time_us =
context->bw_ctx.dml.soc.sr_enter_plus_exit_time_us;
+
dcn21_calculate_wm(dc, context, pipes, &pipe_cnt, pipe_split_from,
vlevel, validate_mode);
dcn20_calculate_dlg_params(dc, context, pipes, pipe_cnt, vlevel);
+ context->bw_ctx.dml.soc.sr_exit_time_us = sr_exit_time_us;
+ context->bw_ctx.dml.soc.sr_enter_plus_exit_time_us =
sr_enter_plus_exit_time_us;
+
BW_VAL_TRACE_END_WATERMARKS();
goto validate_out;
--
2.55.0
