* [RFC PATCH 0/5] drm/amd/display: recover from PSR-related display stalls
@ 2026-08-05 11:52 David Weber
2026-08-05 11:52 ` [RFC PATCH 1/5] drm/amd/display: avoid nested retries when polling PSR state David Weber
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: David Weber @ 2026-08-05 11:52 UTC (permalink / raw)
To: amd-gfx
Cc: alexander.deucher, christian.koenig, harry.wentland, sunpeng.li,
siqueira, someguy, David Weber
Hi,
I am still trying to find every PSR problem on my system by running
different stress tests.
One thing that has always bothered me about PSR failures is that I cannot
recover from them. Automatic recovery does not kick in, and manually
requesting GPU recovery has never brought back a working graphical
session for me. I always have to restart the machine. I would like to
change that.
This RFC comes from a reproducible eDP failure on a Lenovo system with an
AMD Ryzen 5 PRO 7540U and Radeon 740M Graphics (DCN 3.1.4). On this
machine, the observed failure starts with DMCUB errors while PSR is enabled
and leaves a page flip permanently pending. Atomic commits then block,
KWin repeatedly reports page-flip timeouts, and the display freezes for
good.
The first three patches fix problems I found while tracing this failure.
In particular, PSR state polling currently nests two
1001-iteration retry loops. Since each GPINT transaction can busy-wait
for 30 us, an intended roughly 500 ms transition wait can occupy a CPU
for roughly 30 seconds. The changes remove the nested retry, propagate
an unconfirmed transition back to the caller, track whether the cached
request is valid, and invalidate that cache when DMUB is reset or resumed.
This bounds the wait and lets a later policy request retry, but it does
not recover a display pipeline which is already stuck. Patch 4 tries to
do that. It handles a failed bounded PSR exit and a flip timeout on a
PSR-capable eDP link, disables PSR for the rest of the driver instance,
queues a full GPU reset through the AMDGPU reset domain, deals with the
stuck flip/commit state, and forces a modeset afterwards. Patch 5 adds
two debugfs controls for fault injection so the recovery path can be
tested without waiting for a natural failure. These controls are only
for debugging.
Patch 4 puts failure detection, the persistent PSR fallback, and GPU
recovery into one patch so the whole idea can be discussed together.
I kept the PSR recovery separate from the generic
dm_helpers_dmu_timeout() callback. There is already another proposal
which uses that callback to reinitialize DMUB locally:
https://www.mail-archive.com/amd-gfx@lists.freedesktop.org/msg142429.html
There is also a proposal which tries a full modeset after a generic page
flip timeout:
https://www.mail-archive.com/amd-gfx@lists.freedesktop.org/msg142493.html
I would especially like feedback on these questions.
1. Could this be upstreamed?
Are the bounded polling and cache fixes in patches 1-3 useful on their
own? Is a PSR-specific recovery path like patch 4 a reasonable way to
deal with failures which have already wedged scanout?
Is testing on one DCN 3.1.4 system enough for this RFC, or do PSR,
DMUB, and reset handling differ enough between DCN generations that
testing on more hardware is necessary?
2. Is the approximately 530 ms PSR transition bound appropriate?
The old roughly 30-second wait looks like an accidental result of the
nested retries, not an intentional timeout. However, PSR policy
transitions can also happen through the ABM/backlight path, and valid
DMUB operations have been reported to take longer than shorter timeout
limits. Should this use a wall-clock deadline, a different limit for
each path, or simply a larger limit?
3. Which failures should trigger a full recovery?
The direct trigger only fires when a bounded synchronous PSR disable
fails after software had previously confirmed PSR as enabled. The
flip-timeout trigger is less certain: it fires on an eDP CRTC whose
link still has PSR enabled as a feature. The cached allow-active
request is not proof of firmware state because it may describe an exit
which firmware never completed. Is eDP plus PSR-feature-enabled a
reasonable trigger, or is that too broad because the flip could have
stalled for another reason?
4. What should the recovery escalation and DRM-state handling be?
Restarting DMUB may make commands work again without clearing the
pending HUBP flip or DRM commit state. A manually requested full GPU
reset completed on this system, including DMUB initialization and ring
tests, but still did not bring the display back. This RFC therefore
adds handling around the existing full-reset path and performs another
modeset afterwards. The complete sequence recovered one natural
failure, but I do not yet know which extra step made the difference.
Would a smaller step-by-step recovery be better, for example:
a. a forced modeset,
b. DMUB-local reinitialization,
c. full GPU recovery, and
d. a post-reset forced modeset?
If restarting DMUB works, how can the kernel reliably tell whether
scanout and atomic commits are moving again?
The current code keeps a reference to the exact timed-out commit. It
resets first so scanout is stopped and rebuilt, then retires only that
flip if its original event is still pending. It waits for a post-reset
vblank before falling back to a synthetic event. Is this an acceptable
way to release the atomic commit, or does DRM need a separate way to
abort one?
The successful recovery included the forced modeset, but that does not
prove it was needed after DM's normal reset restore. I kept it for now
and verify that its flip really completes. Should it stay, only run
after a failed progress check, or be removed until an A/B test proves
it is needed? Should some of this instead be shared through AMDGPU
display pre-reset/post-reset hooks or with the generic DMUB-timeout and
page-flip-timeout work linked above?
The worker currently holds all modeset locks across the full GPU reset
to stop another atomic commit from racing recovery. This works because
DM's reset-specific suspend path does not take those locks again, but it
makes recovery depend on that behavior. Is that safe enough, or should
reset and commit serialization be handled differently?
5. Is halting DMUB useful fault injection for this failure?
Patch 5 can either call the recovery worker directly or enter PSR,
halt DMUB, and request a synchronous PSR exit. On my system the latter
looks similar to a natural stall: the PSR exit receives no valid state
replies and needs the same recovery path. Recovery from the injected
failure also works reliably.
Halting all of DMUB is still more severe than the natural failure and
can affect unrelated features. It also does not reproduce the pending
flip seen during a natural stall. Is it nevertheless close enough to
be useful for testing recovery from a stalled PSR exit? Is there a
better way to inject that failure without stopping all of DMUB, and is
this a reasonable debug-only interface to include upstream?
The two controls can be triggered with:
echo 1 > /sys/kernel/debug/dri/0/amdgpu_dm_trigger_psr_recovery
echo 0xDEADDEAD > \
/sys/kernel/debug/dri/0/amdgpu_dm_halt_dmub_for_psr_recovery
The second control is deliberately destructive. The magic value is a
safety cookie, and the control refuses to halt DMUB if GPU recovery
is unavailable, there is no reset domain, or PSR recovery is already
running or stopping.
Current test status
-------------------
Test system:
Lenovo 21F8CTO1WW
AMD Ryzen 5 PRO 7540U / Radeon 740M Graphics
DCN 3.1.4 / amdgpu
internal eDP panel
Plasma/KWin Wayland
Linux v7.2-rc4
Tested on this system:
- Patches 1-3 expose failed PSR transitions promptly instead of
spending tens of seconds in the nested busy-wait.
- Patch 4 recovered a natural stall with a submitted flip.
MODE2 recovery reinitialized DMUB and the GPU rings. No post-reset
vblank arrived within one second, so recovery retired the exact stuck
flip, its commit cleanup completed, and the forced modeset and flip
validation succeeded. The existing Plasma session remained usable
afterwards without any observed problems. Once the flip timeout was
detected, recovery took 2.1 seconds and produced an AMDGPU device
coredump.
An abridged log was:
[ 94.193436] [drm] *ERROR* [CRTC:376:crtc-0] flip_done timed out
[ 94.193454] [drm] *ERROR* [CRTC:376:crtc-0] eDP flip timeout: link=0 psr=1 request_valid=1 request=1 submitted=1
[ 94.193504] [drm] PSR recovery: recorded 1 commits and 1 submitted flips
[ 95.212736] amdgpu: GPU reset(1) succeeded!
[ 96.219900] [drm] [CRTC:376:crtc-0] no vblank after PSR recovery; retiring flip without one
[ 96.219922] [drm] PSR recovery: retired 1 recovered flips
[ 96.219926] [drm] PSR recovery: recorded commits quiesced
[ 96.220247] [drm] PSR recovery: PSR disabled after reset (sink writes=1)
[ 96.259603] [drm] PSR recovery: forced modeset and flip validation returned 0
[ 96.259928] [drm] PSR display recovery completed
- I ran the injected DMUB-halt recovery from patches 4 and 5 many times.
Recovery completed successfully each time, and the existing Plasma
session remained usable afterwards without any observed problems.
One run also exercised the retained-flip refresh: the initial snapshot
recorded one commit and no submitted flip. After reset, recovery found
the newly submitted flip belonging to that retained commit, retired it,
and its cleanup completed. The forced modeset and flip validation
succeeded. Once the PSR exit timeout was detected, this recovery took
5.3 seconds and produced an AMDGPU device coredump.
- The post-modeset PSR fallback remained effective after that recovery;
debugfs reported "Driver support: no" for the eDP link.
Not tested:
- The IRQ-sensitive failure path with CONFIG_PROVE_LOCKING and lockdep.
- Merging a later exact flip timeout into recovery which was first
requested by a synchronous PSR transition timeout.
- Hardware other than this DCN 3.1.4 system.
The series is based on v7.2-rc4 because it regularly produces natural PSR
stalls on my system without fault injection. If this looks suitable for
a non-RFC revision, I will rebase it onto the latest
amd-staging-drm-next.
AI assistance
-------------
I am learning more about the AMD display code while working on these
problems, but this series is still way over my head. All patch code in
this RFC was generated with AI assistance. I did my best to understand
and review the changes, connect them to failures I can reproduce, and
test them on the affected hardware so this is not just generated slop.
Still, it is difficult for me to judge many of the locking, lifetime,
DRM-state, and recovery decisions. That is also why I am sending this
as an RFC and asking for help with the overall direction.
David Weber (5):
drm/amd/display: avoid nested retries when polling PSR state
drm/amd/display: retry unconfirmed PSR transitions
drm/amd/display: invalidate PSR request cache after DMUB reset
drm/amd/display: recover from fatal PSR-related display timeouts
drm/amd/display: add PSR recovery fault injection
drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c | 3 +
drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h | 1 +
.../gpu/drm/amd/display/amdgpu_dm/Makefile | 1 +
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 135 +++-
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 19 +
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 177 ++++
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 10 +
.../drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c | 14 +-
.../amdgpu_dm/amdgpu_dm_psr_recovery.c | 754 ++++++++++++++++++
.../gpu/drm/amd/display/dc/clk_mgr/clk_mgr.c | 9 +-
drivers/gpu/drm/amd/display/dc/core/dc.c | 6 +-
drivers/gpu/drm/amd/display/dc/dc_types.h | 3 +-
drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c | 115 +--
drivers/gpu/drm/amd/display/dc/dce/dmub_psr.h | 8 +-
drivers/gpu/drm/amd/display/dc/dm_helpers.h | 1 +
.../gpu/drm/amd/display/dc/inc/hw/clk_mgr.h | 1 +
.../dc/link/protocols/link_dp_irq_handler.c | 21 +-
.../link/protocols/link_edp_panel_control.c | 42 +-
.../drm/amd/display/modules/inc/mod_power.h | 5 +-
.../drm/amd/display/modules/power/power_psr.c | 190 +++--
20 files changed, 1372 insertions(+), 143 deletions(-)
create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr_recovery.c
base-commit: 1590cf0329716306e948a8fc29f1d3ee87d3989f
--
2.54.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH 1/5] drm/amd/display: avoid nested retries when polling PSR state
2026-08-05 11:52 [RFC PATCH 0/5] drm/amd/display: recover from PSR-related display stalls David Weber
@ 2026-08-05 11:52 ` David Weber
2026-08-05 11:52 ` [RFC PATCH 2/5] drm/amd/display: retry unconfirmed PSR transitions David Weber
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: David Weber @ 2026-08-05 11:52 UTC (permalink / raw)
To: amd-gfx
Cc: alexander.deucher, christian.koenig, harry.wentland, sunpeng.li,
siqueira, someguy, David Weber
dmub_psr_get_state() retries a PSR state query up to 1001 times when a
GPINT command times out. Each GPINT transaction busy-waits for up to
30 us.
The retry is nested under transition polling in two live paths.
dmub_psr_enable(wait=true) polls the state directly, including from the
HPD RX recovery path. The Linux power module has a separate transition
loop after issuing the enable command without waiting for the final PSR
state.
In the worst case, each of the 1001 outer iterations performs 1001
state-query attempts. This turns a nominal 500 ms transition timeout
into roughly 30 seconds of CPU busy-wait. PSR error recovery can perform
the wait twice, first while disabling and then while enabling PSR.
Make dmub_psr_get_state() perform one GPINT transaction and return
whether a valid state was obtained. Leave retry policy to the callers,
including a local retry in dmub_psr_set_level(). The explicit 500 us
delays and up-to-30 us GPINT reply waits total about 530 ms; DMUB wake
and command-processing overhead is additional.
Return the DMUB command status for callers that do not wait for the
final PSR state, and continue to use the observed firmware state as the
final authority for synchronous transitions. Use a
'< PSR_STATE_MAX_RETRIES' loop and a separate success flag so a
successful final attempt cannot still be reported as a timeout. Log
command completion, valid state-query counts, and the last observed PSR
state when bounded transition polling expires.
Make edp_get_psr_state() return false when neither the DMUB nor DMCU
backend handles the query. This prevents callers from treating an
untouched output state as a valid firmware reply.
On a Phoenix (DCN 3.1.4) system, I observed the inner retry exhaust its
limit after DMCUB errors, followed by flip_done and commit-wait timeouts.
Fixes: 04f3c88f0955 ("drm/amd/display: Retry getting PSR state if command times out")
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: David Weber <weber.aulendorf@gmail.com>
---
drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c | 115 ++++++++++--------
drivers/gpu/drm/amd/display/dc/dce/dmub_psr.h | 8 +-
.../link/protocols/link_edp_panel_control.c | 4 +-
3 files changed, 74 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c b/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c
index 45630c3effe1..d16bfd328cbd 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c
@@ -29,9 +29,12 @@
#include "dmub/dmub_srv.h"
#include "core_types.h"
+#define DC_LOGGER dmub->ctx->logger
#define DC_TRACE_LEVEL_MESSAGE(...) do {} while (0) /* do nothing */
#define MAX_PIPES 6
+#define PSR_STATE_MAX_RETRIES 1000
+#define PSR_STATE_RETRY_DELAY_US 500
static const uint8_t DP_SINK_DEVICE_STR_ID_1[] = {7, 1, 8, 7, 3};
static const uint8_t DP_SINK_DEVICE_STR_ID_2[] = {7, 1, 8, 7, 5};
@@ -105,33 +108,26 @@ static enum dc_psr_state convert_psr_state(uint32_t raw_state)
/*
* Get PSR state from firmware.
*/
-static void dmub_psr_get_state(struct dmub_psr *dmub, enum dc_psr_state *state, uint8_t panel_inst)
+static bool dmub_psr_get_state(struct dmub_psr *dmub, enum dc_psr_state *state, uint8_t panel_inst)
{
uint32_t raw_state = 0;
- uint32_t retry_count = 0;
-
- do {
- // Send gpint command and wait for ack
- if (dc_wake_and_execute_gpint(dmub->ctx, DMUB_GPINT__GET_PSR_STATE, panel_inst, &raw_state,
- DM_DMUB_WAIT_TYPE_WAIT_WITH_REPLY)) {
- *state = convert_psr_state(raw_state);
- } else {
- // Return invalid state when GPINT times out
- *state = PSR_STATE_INVALID;
- }
- } while (++retry_count <= 1000 && *state == PSR_STATE_INVALID);
- // Assert if max retry hit
- if (retry_count >= 1000 && *state == PSR_STATE_INVALID) {
- ASSERT(0);
- DC_TRACE_LEVEL_MESSAGE(DAL_TRACE_LEVEL_ERROR,
- WPP_BIT_FLAG_Firmware_PsrState,
- "Unable to get PSR state from FW.");
- } else
- DC_TRACE_LEVEL_MESSAGE(DAL_TRACE_LEVEL_VERBOSE,
- WPP_BIT_FLAG_Firmware_PsrState,
- "Got PSR state from FW. PSR state: %d, Retry count: %d",
- *state, retry_count);
+ /* The caller owns any retry policy and its total timeout. */
+ if (!dc_wake_and_execute_gpint(dmub->ctx,
+ DMUB_GPINT__GET_PSR_STATE, panel_inst,
+ &raw_state, DM_DMUB_WAIT_TYPE_WAIT_WITH_REPLY)) {
+ *state = PSR_STATE_INVALID;
+ return false;
+ }
+
+ *state = convert_psr_state(raw_state);
+
+ DC_TRACE_LEVEL_MESSAGE(DAL_TRACE_LEVEL_VERBOSE,
+ WPP_BIT_FLAG_Firmware_PsrState,
+ "Got PSR state from FW. PSR state: %d",
+ *state);
+
+ return *state != PSR_STATE_INVALID;
}
/*
@@ -176,12 +172,15 @@ static bool dmub_psr_set_version(struct dmub_psr *dmub, struct dc_stream_state *
/*
* Enable/Disable PSR.
*/
-static void dmub_psr_enable(struct dmub_psr *dmub, bool enable, bool wait, uint8_t panel_inst)
+static bool dmub_psr_enable(struct dmub_psr *dmub, bool enable, bool wait, uint8_t panel_inst)
{
union dmub_rb_cmd cmd;
struct dc_context *dc = dmub->ctx;
uint32_t retry_count;
- enum dc_psr_state state = PSR_STATE0;
+ u32 valid_query_count = 0;
+ enum dc_psr_state state = PSR_STATE_INVALID;
+ bool command_ok;
+ bool state_reached = false;
memset(&cmd, 0, sizeof(cmd));
cmd.psr_enable.header.type = DMUB_CMD__PSR;
@@ -196,32 +195,41 @@ static void dmub_psr_enable(struct dmub_psr *dmub, bool enable, bool wait, uint8
cmd.psr_enable.header.payload_bytes = 0; // Send header only
- dc_wake_and_execute_dmub_cmd(dc->dmub_srv->ctx, &cmd, DM_DMUB_WAIT_TYPE_WAIT);
+ command_ok = dc_wake_and_execute_dmub_cmd(dc->dmub_srv->ctx, &cmd, DM_DMUB_WAIT_TYPE_WAIT);
+
+ if (!wait)
+ return command_ok;
- /* Below loops 1000 x 500us = 500 ms.
- * Exit PSR may need to wait 1-2 frames to power up. Timeout after at
- * least a few frames. Should never hit the max retry assert below.
+ /*
+ * Must not use fsleep() because this can be called from high IRQ levels.
+ * Each query may take 30 us, followed by the existing 500 us delay.
+ * Exit PSR may need 1-2 frames to power up.
*/
- if (wait) {
- for (retry_count = 0; retry_count <= 1000; retry_count++) {
- dmub_psr_get_state(dmub, &state, panel_inst);
-
- if (enable) {
- if (state != PSR_STATE0)
- break;
- } else {
- if (state == PSR_STATE0)
- break;
+ for (retry_count = 0; retry_count < PSR_STATE_MAX_RETRIES; retry_count++) {
+ if (!dmub_psr_get_state(dmub, &state, panel_inst)) {
+ udelay(PSR_STATE_RETRY_DELAY_US);
+ continue;
+ }
+ valid_query_count++;
+ if (enable) {
+ if (state != PSR_STATE0) {
+ state_reached = true;
+ break;
}
-
- /* must *not* be fsleep - this can be called from high irq levels */
- udelay(500);
+ } else if (state == PSR_STATE0) {
+ state_reached = true;
+ break;
}
- /* assert if max retry hit */
- if (retry_count >= 1000)
- ASSERT(0);
+ udelay(PSR_STATE_RETRY_DELAY_US);
}
+
+ if (!state_reached)
+ DC_LOG_ERROR("PSR %s timeout: panel=%u cmd=%d queries=%u state=%d\n",
+ enable ? "enable" : "disable", panel_inst,
+ command_ok, valid_query_count, state);
+
+ return state_reached;
}
/*
@@ -230,10 +238,21 @@ static void dmub_psr_enable(struct dmub_psr *dmub, bool enable, bool wait, uint8
static void dmub_psr_set_level(struct dmub_psr *dmub, uint16_t psr_level, uint8_t panel_inst)
{
union dmub_rb_cmd cmd;
- enum dc_psr_state state = PSR_STATE0;
+ enum dc_psr_state state = PSR_STATE_INVALID;
struct dc_context *dc = dmub->ctx;
+ unsigned int retry_count;
+
+ /* Keep this operation's retry policy local so it cannot be nested by
+ * transition callers of dmub_psr_get_state().
+ */
+ for (retry_count = 0; retry_count < PSR_STATE_MAX_RETRIES; retry_count++)
+ if (dmub_psr_get_state(dmub, &state, panel_inst))
+ break;
- dmub_psr_get_state(dmub, &state, panel_inst);
+ if (retry_count == PSR_STATE_MAX_RETRIES) {
+ ASSERT(0);
+ return;
+ }
if (state == PSR_STATE0)
return;
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.h b/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.h
index a6e282d950c3..e534822fb4d2 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.h
+++ b/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.h
@@ -39,10 +39,10 @@ struct dmub_psr {
struct dmub_psr_funcs {
bool (*psr_copy_settings)(struct dmub_psr *dmub, struct dc_link *link,
struct psr_context *psr_context, uint8_t panel_inst);
- void (*psr_enable)(struct dmub_psr *dmub, bool enable, bool wait,
- uint8_t panel_inst);
- void (*psr_get_state)(struct dmub_psr *dmub, enum dc_psr_state *dc_psr_state,
- uint8_t panel_inst);
+ bool (*psr_enable)(struct dmub_psr *dmub, bool enable, bool wait,
+ uint8_t panel_inst);
+ bool (*psr_get_state)(struct dmub_psr *dmub, enum dc_psr_state *dc_psr_state,
+ uint8_t panel_inst);
void (*psr_set_level)(struct dmub_psr *dmub, uint16_t psr_level,
uint8_t panel_inst);
void (*psr_force_static)(struct dmub_psr *dmub, uint8_t panel_inst);
diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
index 80a372ceaa51..4e19ccd836ae 100644
--- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
+++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
@@ -637,9 +637,11 @@ bool edp_get_psr_state(const struct dc_link *link, enum dc_psr_state *state)
return false;
if (psr != NULL && link->psr_settings.psr_feature_enabled)
- psr->funcs->psr_get_state(psr, state, (uint8_t)panel_inst);
+ return psr->funcs->psr_get_state(psr, state, (uint8_t)panel_inst);
else if (dmcu != NULL && link->psr_settings.psr_feature_enabled)
dmcu->funcs->get_psr_state(dmcu, state);
+ else
+ return false;
return true;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC PATCH 2/5] drm/amd/display: retry unconfirmed PSR transitions
2026-08-05 11:52 [RFC PATCH 0/5] drm/amd/display: recover from PSR-related display stalls David Weber
2026-08-05 11:52 ` [RFC PATCH 1/5] drm/amd/display: avoid nested retries when polling PSR state David Weber
@ 2026-08-05 11:52 ` David Weber
2026-08-05 11:52 ` [RFC PATCH 3/5] drm/amd/display: invalidate PSR request cache after DMUB reset David Weber
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: David Weber @ 2026-08-05 11:52 UTC (permalink / raw)
To: amd-gfx
Cc: alexander.deucher, christian.koenig, harry.wentland, sunpeng.li,
siqueira, someguy, David Weber
The DMUB PSR backend can fail to complete a command or exhaust a
synchronous state transition wait. Do not treat either outcome as a
successfully programmed allow-active request.
Return transition status through the eDP control layer and update the
cached request only after backend handling succeeds. Track whether that
cache is usable for request deduplication; a command timeout may still
mean firmware consumed the command, but software cannot safely suppress
the next request.
Move request deduplication into the power module, where both the desired
event policy and the link cache are available. This also covers direct
power-module callers such as the ABM backlight path. Skip an unchanged
request only when the power-module state and the valid link request cache
agree. Otherwise retry the transition and update the power-module cache
only after success. Remove the now-unused mod_power_get_psr_event()
accessor after moving that decision into the module.
When a subordinate event is blocked by a forced-PSR policy, preserve the
existing result for actual event changes and return success for unchanged
events. Reissue the dominant policy if the link cache is invalid or the
link and power-module caches disagree. Dynamic-display-switch and
dynamic-link-rate-control reapply force-static mode, while OS override
hold preserves the currently applied enabled state.
Make the power-module polling loop honor failed state queries and fix its
final-iteration test. Initialize the request cache conservatively after
setup.
Require a valid allow-active cache when reporting that PSR is allowed.
The idle-detection loop therefore stops instead of re-enabling idle
optimizations based on unknown firmware state.
The HPD RX recovery path now stops if PSR could not be disabled instead
of immediately issuing a re-enable. If the previous request is unknown,
leave PSR disabled rather than restoring stale cached state.
Fixes: 3c108046e1d6 ("drm/amd/display: Add power module on Linux")
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: David Weber <weber.aulendorf@gmail.com>
---
.../drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c | 14 +-
drivers/gpu/drm/amd/display/dc/dc_types.h | 3 +-
.../dc/link/protocols/link_dp_irq_handler.c | 21 ++-
.../link/protocols/link_edp_panel_control.c | 23 ++-
.../drm/amd/display/modules/inc/mod_power.h | 4 -
.../drm/amd/display/modules/power/power_psr.c | 131 ++++++++++--------
6 files changed, 114 insertions(+), 82 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
index 0dadc0bb214f..f045612e8b10 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
@@ -152,7 +152,9 @@ bool amdgpu_dm_psr_is_active_allowed(struct amdgpu_display_manager *dm)
if (!link)
continue;
- if (link->psr_settings.psr_feature_enabled && link->psr_settings.psr_allow_active)
+ if (link->psr_settings.psr_feature_enabled &&
+ link->psr_settings.psr_allow_active_valid &&
+ link->psr_settings.psr_allow_active)
return true;
}
return false;
@@ -171,21 +173,11 @@ bool amdgpu_dm_psr_is_active_allowed(struct amdgpu_display_manager *dm)
bool amdgpu_dm_psr_set_event(struct amdgpu_display_manager *dm, struct dc_stream_state *stream,
bool set_event, enum psr_event event, bool wait_for_disable)
{
- unsigned int psr_events;
-
/* Validate all required parameters */
if (!stream || !stream->link ||
!stream->link->psr_settings.psr_feature_enabled)
return false;
- /* Get current psr events */
- if (!mod_power_get_psr_event(dm->power_module, stream, &psr_events))
- return false;
-
- /* If all events already in desired state, return true. */
- if ((psr_events & event) == (set_event ? event : 0))
- return true;
-
return mod_power_set_psr_event(dm->power_module, stream,
set_event, event, wait_for_disable);
}
diff --git a/drivers/gpu/drm/amd/display/dc/dc_types.h b/drivers/gpu/drm/amd/display/dc/dc_types.h
index 4ed1efa17270..cd61ff160b93 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_types.h
@@ -1114,7 +1114,8 @@ struct link_mst_stream_allocation_table {
/* PSR feature flags */
struct psr_settings {
bool psr_feature_enabled; // PSR is supported by sink
- bool psr_allow_active; // PSR is currently active
+ bool psr_allow_active; // Cached allow-active request
+ bool psr_allow_active_valid; // Cache is usable for request deduplication
enum dc_psr_version psr_version; // Internal PSR version, determined based on DPCD
bool psr_vtotal_control_support; // Vtotal control is supported by sink
unsigned long long psr_dirty_rects_change_timestamp_ns; // for delay of enabling PSR-SU
diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_irq_handler.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_irq_handler.c
index 54ce768ae6ad..bcf9e0ded92c 100644
--- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_irq_handler.c
+++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_irq_handler.c
@@ -156,6 +156,7 @@ static bool handle_hpd_irq_psr_sink(struct dc_link *link)
psr_error_status.bits.RFB_STORAGE_ERROR ||
psr_error_status.bits.VSC_SDP_ERROR) {
bool allow_active;
+ bool restore_psr;
/* Acknowledge and clear error bits */
dm_helpers_dp_write_dpcd(
@@ -165,12 +166,24 @@ static bool handle_hpd_irq_psr_sink(struct dc_link *link)
&psr_error_status.raw,
sizeof(psr_error_status.raw));
- /* PSR error, disable and re-enable PSR */
- if (link->psr_settings.psr_allow_active) {
+ /* Restore PSR only when the enabled request is known. */
+ restore_psr = link->psr_settings.psr_allow_active_valid &&
+ link->psr_settings.psr_allow_active;
+ if (!link->psr_settings.psr_allow_active_valid ||
+ link->psr_settings.psr_allow_active) {
allow_active = false;
- edp_set_psr_allow_active(link, &allow_active, true, false, NULL);
+ if (!edp_set_psr_allow_active(link, &allow_active,
+ true, false, NULL)) {
+ DC_LOG_ERROR("Failed to disable PSR after sink error\n");
+ return true;
+ }
+ if (!restore_psr)
+ return true;
+
allow_active = true;
- edp_set_psr_allow_active(link, &allow_active, true, false, NULL);
+ if (!edp_set_psr_allow_active(link, &allow_active,
+ true, false, NULL))
+ DC_LOG_ERROR("Failed to restore PSR after sink error\n");
}
return true;
diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
index 4e19ccd836ae..f7001f01f960 100644
--- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
+++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
@@ -609,19 +609,31 @@ bool edp_set_psr_allow_active(struct dc_link *link, const bool *allow_active,
psr->funcs->psr_force_static(psr, (uint8_t)panel_inst);
/* Enable or Disable PSR */
- if (allow_active && link->psr_settings.psr_allow_active != *allow_active) {
- link->psr_settings.psr_allow_active = *allow_active;
+ if (allow_active &&
+ (!link->psr_settings.psr_allow_active_valid ||
+ link->psr_settings.psr_allow_active != *allow_active)) {
+ bool programmed = true;
- if (!link->psr_settings.psr_allow_active)
+ if (!*allow_active)
dc_z10_restore(dc);
if (psr != NULL && link->psr_settings.psr_feature_enabled)
- psr->funcs->psr_enable(psr, link->psr_settings.psr_allow_active, wait, (uint8_t)panel_inst);
+ programmed = psr->funcs->psr_enable(psr, *allow_active,
+ wait, (uint8_t)panel_inst);
else if ((dmcu != NULL && dmcu->funcs->is_dmcu_initialized(dmcu)) &&
link->psr_settings.psr_feature_enabled)
- dmcu->funcs->set_psr_enable(dmcu, link->psr_settings.psr_allow_active, wait);
+ dmcu->funcs->set_psr_enable(dmcu, *allow_active, wait);
else
return false;
+
+ if (!programmed) {
+ /* The command may have succeeded, but its state is unknown. */
+ link->psr_settings.psr_allow_active_valid = false;
+ return false;
+ }
+
+ link->psr_settings.psr_allow_active = *allow_active;
+ link->psr_settings.psr_allow_active_valid = true;
}
return true;
}
@@ -896,6 +908,7 @@ bool edp_setup_psr(struct dc_link *link,
} else {
link->psr_settings.psr_feature_enabled = dmcu->funcs->setup_psr(dmcu, link, psr_context);
}
+ link->psr_settings.psr_allow_active_valid = false;
/* psr_enabled == 0 indicates setup_psr did not succeed, but this
* should not happen since firmware should be running at this point
diff --git a/drivers/gpu/drm/amd/display/modules/inc/mod_power.h b/drivers/gpu/drm/amd/display/modules/inc/mod_power.h
index f9814cf7bbdb..893fcd04a197 100644
--- a/drivers/gpu/drm/amd/display/modules/inc/mod_power.h
+++ b/drivers/gpu/drm/amd/display/modules/inc/mod_power.h
@@ -333,10 +333,6 @@ bool mod_power_set_psr_event(struct mod_power *mod_power,
struct dc_stream_state *stream, bool set_event,
enum psr_event event, bool wait);
-bool mod_power_get_psr_event(struct mod_power *mod_power,
- struct dc_stream_state *stream,
- unsigned int *active_psr_events);
-
bool mod_power_get_psr_state(struct mod_power *mod_power,
const struct dc_stream_state *stream,
enum dc_psr_state *state);
diff --git a/drivers/gpu/drm/amd/display/modules/power/power_psr.c b/drivers/gpu/drm/amd/display/modules/power/power_psr.c
index 5ecb570c204e..7ef53966c9e4 100644
--- a/drivers/gpu/drm/amd/display/modules/power/power_psr.c
+++ b/drivers/gpu/drm/amd/display/modules/power/power_psr.c
@@ -151,6 +151,7 @@ static bool set_psr_enable(struct mod_power *mod_power,
unsigned int retry_count;
const unsigned int max_retry = 1000;
struct dc_link *link = NULL;
+ bool state_reached = false;
if (mod_power == NULL)
return false;
@@ -213,15 +214,18 @@ static bool set_psr_enable(struct mod_power *mod_power,
"set psr enable: BEGIN WAIT: psr_enable=%d",
(int)psr_enable);
- for (retry_count = 0; retry_count <= max_retry; retry_count++) {
- dc_link_get_psr_state(link, &state);
- if (psr_enable) {
- if (state != PSR_STATE0 &&
- (!force_static || state == PSR_STATE3))
- break;
- } else {
- if (state == PSR_STATE0)
+ for (retry_count = 0; retry_count < max_retry; retry_count++) {
+ if (dc_link_get_psr_state(link, &state)) {
+ if (psr_enable) {
+ if (state != PSR_STATE0 &&
+ (!force_static || state == PSR_STATE3)) {
+ state_reached = true;
+ break;
+ }
+ } else if (state == PSR_STATE0) {
+ state_reached = true;
break;
+ }
}
udelay(500);
}
@@ -231,13 +235,11 @@ static bool set_psr_enable(struct mod_power *mod_power,
"set psr enable: END WAIT: psr_enable=%d",
(int)psr_enable);
- /* assert if max retry hit */
- if (retry_count >= max_retry) {
+ if (!state_reached) {
ASSERT(0);
- DC_TRACE_LEVEL_MESSAGE(DAL_TRACE_LEVEL_ERROR,
- WPP_BIT_FLAG_Firmware_PsrState,
- "set psr enable: ERROR: retry_count=%u: Unexpectedly long wait for PSR state change.",
- retry_count);
+ /* The command may have succeeded even though its state is unknown. */
+ link->psr_settings.psr_allow_active_valid = false;
+ return false;
}
} else {
DC_TRACE_LEVEL_MESSAGE(DAL_TRACE_LEVEL_INFORMATION,
@@ -249,38 +251,16 @@ static bool set_psr_enable(struct mod_power *mod_power,
return true;
}
-bool mod_power_get_psr_event(struct mod_power *mod_power,
- struct dc_stream_state *stream,
- unsigned int *active_psr_events)
-{
- struct core_power *core_power = NULL;
- unsigned int stream_index = 0;
-
- if (mod_power == NULL)
- return false;
-
- core_power = MOD_POWER_TO_CORE(mod_power);
-
- if (core_power->num_entities == 0)
- return false;
-
- stream_index = map_index_from_stream(core_power, stream);
-
- if (!core_power->map[stream_index].caps->psr_version)
- return false;
-
- *active_psr_events = core_power->map[stream_index].psr_events;
-
- return true;
-}
-
bool mod_power_set_psr_event(struct mod_power *mod_power,
struct dc_stream_state *stream, bool set_event,
enum psr_event event, bool wait)
{
struct core_power *core_power = NULL;
+ struct dc_link *link = NULL;
unsigned int stream_index = 0;
unsigned int active_psr_events = 0;
+ bool event_changed;
+ bool forced_subordinate_event;
bool psr_enable_request = false;
bool force_static = false;
@@ -303,6 +283,12 @@ bool mod_power_set_psr_event(struct mod_power *mod_power,
if (!core_power->map[stream_index].caps->psr_version)
return false;
+ link = dc_stream_get_link(stream);
+ if (!link)
+ return false;
+
+ event_changed = (core_power->map[stream_index].psr_events & event) !=
+ (set_event ? event : 0);
if (set_event)
core_power->map[stream_index].psr_events |= event;
else
@@ -310,23 +296,44 @@ bool mod_power_set_psr_event(struct mod_power *mod_power,
active_psr_events = core_power->map[stream_index].psr_events;
- // ignore other events when we're in forced psr enabled state
- if (active_psr_events & psr_event_dynamic_display_switch &&
- event != psr_event_dynamic_display_switch)
- return false;
+ forced_subordinate_event = false;
- // ignore other events when we're in forced psr enabled state
- if (active_psr_events & psr_event_os_override_hold &&
- event != psr_event_os_override_hold)
- return false;
+ /* OS override hold preserves the currently applied enabled state. */
+ if ((active_psr_events & psr_event_os_override_hold) &&
+ (event != psr_event_os_override_hold || !event_changed)) {
+ forced_subordinate_event = true;
+ psr_enable_request = core_power->map[stream_index].psr_enabled;
+ } else if ((active_psr_events & psr_event_dynamic_display_switch) &&
+ event != psr_event_dynamic_display_switch) {
+ forced_subordinate_event = true;
+ psr_enable_request = true;
+ force_static = true;
+ } else if ((active_psr_events & psr_event_dynamic_link_rate_control) &&
+ event != psr_event_dynamic_link_rate_control &&
+ event != psr_event_dds_defer_stream_enable &&
+ event != psr_event_dynamic_display_switch) {
+ forced_subordinate_event = true;
+ psr_enable_request = true;
+ force_static = true;
+ }
- // ignore other events when we're in forced psr enabled state
- // dds events need to be processed while in dynamic_link_rate_control
- if (active_psr_events & psr_event_dynamic_link_rate_control &&
- event != psr_event_dynamic_link_rate_control &&
- event != psr_event_dds_defer_stream_enable &&
- event != psr_event_dynamic_display_switch)
- return false;
+ if (forced_subordinate_event) {
+ if (!link->psr_settings.psr_allow_active_valid ||
+ core_power->map[stream_index].psr_enabled != psr_enable_request ||
+ link->psr_settings.psr_allow_active != psr_enable_request) {
+ mod_power_psr_set_power_opt(mod_power, stream,
+ active_psr_events,
+ psr_enable_request);
+ if (!set_psr_enable(mod_power, stream, psr_enable_request,
+ wait, force_static))
+ return false;
+
+ core_power->map[stream_index].psr_enabled =
+ psr_enable_request;
+ }
+
+ return !event_changed;
+ }
if (active_psr_events & (psr_event_test_harness_disable_psr | psr_event_os_request_disable))
psr_enable_request = false;
@@ -361,6 +368,11 @@ bool mod_power_set_psr_event(struct mod_power *mod_power,
else
psr_enable_request = true;
+ if (!event_changed && link->psr_settings.psr_allow_active_valid &&
+ core_power->map[stream_index].psr_enabled == psr_enable_request &&
+ link->psr_settings.psr_allow_active == psr_enable_request)
+ return true;
+
DC_TRACE_LEVEL_MESSAGE(DAL_TRACE_LEVEL_VERBOSE,
WPP_BIT_FLAG_Firmware_PsrState,
"mod_power set_psr_event: before: psr_enabled=%d -> request: set_event=%d event=0x%04x -> result: psr_events=0x%04x psr_enable_request=%d",
@@ -371,9 +383,14 @@ bool mod_power_set_psr_event(struct mod_power *mod_power,
(int)psr_enable_request);
mod_power_psr_set_power_opt(mod_power, stream, active_psr_events, psr_enable_request);
- if (core_power->map[stream_index].psr_enabled != psr_enable_request || force_static) {
- if (set_psr_enable(mod_power, stream, psr_enable_request, wait, force_static))
- core_power->map[stream_index].psr_enabled = psr_enable_request;
+ if (!link->psr_settings.psr_allow_active_valid ||
+ core_power->map[stream_index].psr_enabled != psr_enable_request ||
+ link->psr_settings.psr_allow_active != psr_enable_request ||
+ force_static) {
+ if (!set_psr_enable(mod_power, stream, psr_enable_request, wait, force_static))
+ return false;
+
+ core_power->map[stream_index].psr_enabled = psr_enable_request;
}
return true;
--
2.54.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC PATCH 3/5] drm/amd/display: invalidate PSR request cache after DMUB reset
2026-08-05 11:52 [RFC PATCH 0/5] drm/amd/display: recover from PSR-related display stalls David Weber
2026-08-05 11:52 ` [RFC PATCH 1/5] drm/amd/display: avoid nested retries when polling PSR state David Weber
2026-08-05 11:52 ` [RFC PATCH 2/5] drm/amd/display: retry unconfirmed PSR transitions David Weber
@ 2026-08-05 11:52 ` David Weber
2026-08-05 11:52 ` [RFC PATCH 4/5] drm/amd/display: recover from fatal PSR-related display timeouts David Weber
2026-08-05 11:52 ` [RFC PATCH 5/5] drm/amd/display: add PSR recovery fault injection David Weber
4 siblings, 0 replies; 6+ messages in thread
From: David Weber @ 2026-08-05 11:52 UTC (permalink / raw)
To: amd-gfx
Cc: alexander.deucher, christian.koenig, harry.wentland, sunpeng.li,
siqueira, someguy, David Weber
The cached PSR allow-active request is used to suppress redundant
firmware programming. A DMUB reset or resume can discard firmware state
without destroying the corresponding dc_link, leaving that request cache
stale.
Invalidate the allow-active cache whenever DMUB is reset or resumed.
Require a valid cache in direct PSR control so the next request can
reestablish the firmware state.
Preserve cache validity alongside the clock manager's saved PSR request
and do not restore an invalid snapshot. Replay restore remains unchanged;
the PSR validity flag does not describe Replay state.
Fixes: 3c108046e1d6 ("drm/amd/display: Add power module on Linux")
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: David Weber <weber.aulendorf@gmail.com>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 16 ++++++++++++++++
drivers/gpu/drm/amd/display/dc/clk_mgr/clk_mgr.c | 9 +++++++--
drivers/gpu/drm/amd/display/dc/core/dc.c | 6 ++++--
drivers/gpu/drm/amd/display/dc/inc/hw/clk_mgr.h | 1 +
4 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 9c564cd5edee..ee3d65226a69 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1304,6 +1304,19 @@ static void amdgpu_dm_audio_eld_notify(struct amdgpu_device *adev, int pin)
}
}
+static void dm_invalidate_psr_request_caches(struct amdgpu_device *adev)
+{
+ struct dc *dc = adev->dm.dc;
+ int i;
+
+ if (!dc)
+ return;
+
+ for (i = 0; i < dc->link_count; i++)
+ if (dc->links[i])
+ dc->links[i]->psr_settings.psr_allow_active_valid = false;
+}
+
static int dm_dmub_hw_init(struct amdgpu_device *adev)
{
const struct dmcub_firmware_header_v1_0 *hdr;
@@ -1354,6 +1367,7 @@ static int dm_dmub_hw_init(struct amdgpu_device *adev)
status = dmub_srv_hw_reset(dmub_srv);
if (status != DMUB_STATUS_OK)
drm_warn(adev_to_drm(adev), "Error resetting DMUB HW: %d\n", status);
+ dm_invalidate_psr_request_caches(adev);
hdr = (const struct dmcub_firmware_header_v1_0 *)dmub_fw->data;
@@ -1505,6 +1519,8 @@ static void dm_dmub_hw_resume(struct amdgpu_device *adev)
return;
}
+ dm_invalidate_psr_request_caches(adev);
+
status = dmub_srv_is_hw_init(dmub_srv, &init);
if (status != DMUB_STATUS_OK)
drm_warn(adev_to_drm(adev), "DMUB hardware init check failed: %d\n", status);
diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/clk_mgr.c b/drivers/gpu/drm/amd/display/dc/clk_mgr/clk_mgr.c
index 2a1353cb7e7d..2c587cb0f5e1 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/clk_mgr.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/clk_mgr.c
@@ -114,6 +114,8 @@ void clk_mgr_exit_optimized_pwr_state(const struct dc *dc, struct clk_mgr *clk_m
if (!edp_link->psr_settings.psr_feature_enabled)
continue;
clk_mgr->psr_allow_active_cache = edp_link->psr_settings.psr_allow_active;
+ clk_mgr->psr_allow_active_cache_valid =
+ edp_link->psr_settings.psr_allow_active_valid;
dc->link_srv->edp_set_psr_allow_active(edp_link, &allow_active, false, false, NULL);
dc->link_srv->edp_set_replay_allow_active(edp_link, &allow_active, false, false, NULL);
}
@@ -134,8 +136,11 @@ void clk_mgr_optimize_pwr_state(const struct dc *dc, struct clk_mgr *clk_mgr)
edp_link = edp_links[panel_inst];
if (!edp_link->psr_settings.psr_feature_enabled)
continue;
- dc->link_srv->edp_set_psr_allow_active(edp_link,
- &clk_mgr->psr_allow_active_cache, false, false, NULL);
+ if (clk_mgr->psr_allow_active_cache_valid)
+ dc->link_srv->edp_set_psr_allow_active(edp_link,
+ &clk_mgr->psr_allow_active_cache,
+ false, false, NULL);
+ /* PSR cache validity does not describe Replay state. */
dc->link_srv->edp_set_replay_allow_active(edp_link,
&clk_mgr->psr_allow_active_cache, false, false, NULL);
}
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index e25b94b65dac..8d76101a17a3 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -6308,11 +6308,13 @@ bool dc_set_psr_allow_active(struct dc *dc, bool enable)
continue;
if (link->psr_settings.psr_feature_enabled) {
- if (enable && !link->psr_settings.psr_allow_active) {
+ if (enable && (!link->psr_settings.psr_allow_active_valid ||
+ !link->psr_settings.psr_allow_active)) {
allow_active = true;
if (!dc_link_set_psr_allow_active(link, &allow_active, false, false, NULL))
return false;
- } else if (!enable && link->psr_settings.psr_allow_active) {
+ } else if (!enable && (!link->psr_settings.psr_allow_active_valid ||
+ link->psr_settings.psr_allow_active)) {
allow_active = false;
if (!dc_link_set_psr_allow_active(link, &allow_active, true, false, NULL))
return false;
diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/clk_mgr.h b/drivers/gpu/drm/amd/display/dc/inc/hw/clk_mgr.h
index f829ce3f70e5..e351f3dee8ff 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/hw/clk_mgr.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/hw/clk_mgr.h
@@ -412,6 +412,7 @@ struct clk_mgr {
struct clk_mgr_funcs *funcs;
struct dc_clocks clks;
bool psr_allow_active_cache;
+ bool psr_allow_active_cache_valid;
bool force_smu_not_present;
bool dc_mode_softmax_enabled;
int dprefclk_khz; // Used by program pixel clock in clock source funcs, need to figureout where this goes
--
2.54.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC PATCH 4/5] drm/amd/display: recover from fatal PSR-related display timeouts
2026-08-05 11:52 [RFC PATCH 0/5] drm/amd/display: recover from PSR-related display stalls David Weber
` (2 preceding siblings ...)
2026-08-05 11:52 ` [RFC PATCH 3/5] drm/amd/display: invalidate PSR request cache after DMUB reset David Weber
@ 2026-08-05 11:52 ` David Weber
2026-08-05 11:52 ` [RFC PATCH 5/5] drm/amd/display: add PSR recovery fault injection David Weber
4 siblings, 0 replies; 6+ messages in thread
From: David Weber @ 2026-08-05 11:52 UTC (permalink / raw)
To: amd-gfx
Cc: alexander.deucher, christian.koenig, harry.wentland, sunpeng.li,
siqueira, someguy, David Weber
A PSR-related display stall can leave a page flip permanently submitted.
Later atomic commits then block on that flip, while the normal GPU hang
recovery does not start because no graphics ring timeout occurred. On a
Phoenix (DCN 3.1.4) system, manual GPU recovery completed but did not
restore the graphical session.
Recover from two indications of this failure: a bounded synchronous PSR
disable which cannot be confirmed after PSR was previously confirmed
enabled, and an eDP flip timeout while PSR remains enabled as a feature
and the exact AMDGPU flip is still submitted. The latter is a fail-safe
attribution heuristic because a flip timeout alone does not prove PSR was
the cause.
Keep PSR disabled for the rest of the driver instance and synchronize the
power-module cache. Queue recovery through the AMDGPU reset domain,
honor the configured GPU recovery policy, and retain references to the
commits which were outstanding when recovery was requested. Merge exact
flip evidence which arrives while recovery is already pending.
Acquire all modeset locks before irreversible recovery, wait for recorded
hardware programming with one total deadline, and perform a full GPU
reset. After reset has quiesced and rebuilt scanout, retire only a
recorded stuck flip whose event still belongs to its retained commit.
Refresh the submitted-flip status of retained commits after reset because
a commit can arm its flip after the initial request snapshot but before
recovery acquires all modeset locks. Bound late-evidence reconciliation
and fail recovery if recorded commit cleanup remains incomplete.
Reapply the persistent PSR disable to rebuilt link and power-module state
and to active eDP sinks. Force a modeset from the live DRM state and
verify its flip completion. Add AMDGPU_RESET_SRC_PSR in AMDGPU core so
reset logs and coredumps identify the recovery source.
The complete path recovered a natural submitted-flip stall on the test
system in 2.1 seconds. The post-reset vblank wait expired, the exact flip
was retired, commit cleanup completed, and the forced modeset flip
finished. The existing Plasma session remained usable. This does not
yet prove that the forced modeset is required beyond recovered-flip
retirement and DM's normal reset restoration.
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: David Weber <weber.aulendorf@gmail.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c | 3 +
drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h | 1 +
.../gpu/drm/amd/display/amdgpu_dm/Makefile | 1 +
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 119 ++-
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 19 +
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 10 +
.../amdgpu_dm/amdgpu_dm_psr_recovery.c | 754 ++++++++++++++++++
drivers/gpu/drm/amd/display/dc/dm_helpers.h | 1 +
.../link/protocols/link_edp_panel_control.c | 15 +
.../drm/amd/display/modules/inc/mod_power.h | 3 +
.../drm/amd/display/modules/power/power_psr.c | 61 +-
11 files changed, 981 insertions(+), 6 deletions(-)
create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr_recovery.c
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
index 428c3cbc4a40..6342f16882ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
@@ -344,6 +344,9 @@ void amdgpu_reset_get_desc(struct amdgpu_reset_context *rst_ctxt, char *buf,
case AMDGPU_RESET_SRC_USERQ:
strscpy(buf, "user queue trigger", len);
break;
+ case AMDGPU_RESET_SRC_PSR:
+ strscpy(buf, "PSR-related display failure", len);
+ break;
default:
strscpy(buf, "unknown", len);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
index c9f23a8e8db8..4aebac5bb045 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
@@ -44,6 +44,7 @@ enum AMDGPU_RESET_SRCS {
AMDGPU_RESET_SRC_HWS,
AMDGPU_RESET_SRC_USER,
AMDGPU_RESET_SRC_USERQ,
+ AMDGPU_RESET_SRC_PSR,
};
/**
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile b/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile
index 54a93e4255b3..66c3edaa8657 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/Makefile
@@ -37,6 +37,7 @@ AMDGPUDM = \
amdgpu_dm_helpers.o \
amdgpu_dm_pp_smu.o \
amdgpu_dm_psr.o \
+ amdgpu_dm_psr_recovery.o \
amdgpu_dm_replay.o \
amdgpu_dm_quirks.o \
amdgpu_dm_wb.o \
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index ee3d65226a69..dc312291b4d3 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2074,6 +2074,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
mutex_init(&adev->dm.audio_lock);
spin_lock_init(&adev->dm.dmub_lock);
+ amdgpu_dm_psr_recovery_init(&adev->dm);
if (amdgpu_dm_irq_init(adev)) {
drm_err(adev_to_drm(adev), "failed to initialize DM IRQ support.\n");
@@ -2436,6 +2437,8 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
{
int i;
+ amdgpu_dm_psr_recovery_fini(&adev->dm);
+
if (adev->dm.vblank_control_workqueue) {
destroy_workqueue(adev->dm.vblank_control_workqueue);
adev->dm.vblank_control_workqueue = NULL;
@@ -11458,6 +11461,115 @@ static int amdgpu_dm_atomic_setup_commit(struct drm_atomic_commit *state)
return 0;
}
+struct amdgpu_dm_psr_commit_info {
+ bool edp;
+ bool psr_feature_enabled;
+ bool request_valid;
+ bool request;
+ u8 link_index;
+};
+
+static void amdgpu_dm_snapshot_psr_commit_info(
+ struct drm_atomic_commit *state,
+ struct amdgpu_dm_psr_commit_info *info)
+{
+ struct amdgpu_display_manager *dm = &drm_to_adev(state->dev)->dm;
+ struct drm_crtc_state *new_crtc_state;
+ struct drm_crtc *crtc;
+ int i;
+
+ for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+ struct dm_crtc_state *dm_state;
+ struct dc_link *link;
+ unsigned int index = drm_crtc_index(crtc);
+
+ if (index >= AMDGPU_DM_MAX_CRTC)
+ continue;
+
+ dm_state = to_dm_crtc_state(new_crtc_state);
+ if (!dm_state->stream || !dm_state->stream->link)
+ continue;
+
+ link = dm_state->stream->link;
+ if (link->connector_signal != SIGNAL_TYPE_EDP)
+ continue;
+
+ info[index].edp = true;
+ info[index].link_index = link->link_index;
+ info[index].psr_feature_enabled =
+ !READ_ONCE(dm->psr_disabled_by_recovery) &&
+ READ_ONCE(link->psr_settings.psr_feature_enabled);
+ info[index].request_valid =
+ READ_ONCE(link->psr_settings.psr_allow_active_valid);
+ info[index].request =
+ READ_ONCE(link->psr_settings.psr_allow_active);
+ }
+}
+
+static void amdgpu_dm_wait_for_flip_done(
+ struct drm_device *dev,
+ struct drm_atomic_commit *state,
+ const struct amdgpu_dm_psr_commit_info *psr_info)
+{
+ struct amdgpu_display_manager *dm = &drm_to_adev(dev)->dm;
+ bool recovery_requested = false;
+ struct drm_crtc *crtc;
+ int i;
+
+ for (i = 0; i < dev->mode_config.num_crtc; i++) {
+ struct drm_crtc_commit *commit = state->crtcs[i].commit;
+ struct amdgpu_crtc *acrtc;
+ const struct amdgpu_dm_psr_commit_info *info;
+ unsigned int index;
+ unsigned long flags;
+ bool flip_submitted;
+ unsigned long ret;
+
+ crtc = state->crtcs[i].ptr;
+ if (!crtc || !commit)
+ continue;
+ index = drm_crtc_index(crtc);
+ if (index >= AMDGPU_DM_MAX_CRTC)
+ continue;
+ info = &psr_info[index];
+
+ ret = wait_for_completion_timeout(&commit->flip_done, 10 * HZ);
+ if (ret)
+ continue;
+
+ drm_err(dev, "[CRTC:%d:%s] flip_done timed out\n",
+ crtc->base.id, crtc->name);
+
+ acrtc = to_amdgpu_crtc(crtc);
+ spin_lock_irqsave(&dev->event_lock, flags);
+ flip_submitted =
+ acrtc->pflip_status == AMDGPU_FLIP_SUBMITTED &&
+ acrtc->event &&
+ acrtc->event->base.completion == &commit->flip_done;
+ spin_unlock_irqrestore(&dev->event_lock, flags);
+
+ if (info->edp)
+ drm_err(dev,
+ "[CRTC:%d:%s] eDP flip timeout: link=%u psr=%d request_valid=%d request=%d submitted=%d\n",
+ crtc->base.id, crtc->name, info->link_index,
+ info->psr_feature_enabled, info->request_valid,
+ info->request, flip_submitted);
+
+ if (recovery_requested || !info->edp ||
+ !info->psr_feature_enabled || !flip_submitted)
+ continue;
+
+ drm_err(dev,
+ "[CRTC:%d:%s] flip timeout on PSR link %u; scheduling recovery\n",
+ crtc->base.id, crtc->name, info->link_index);
+ if (amdgpu_dm_schedule_psr_recovery(dm, crtc, commit))
+ recovery_requested = true;
+ }
+
+ if (state->fake_commit)
+ complete_all(&state->fake_commit->flip_done);
+}
+
/**
* amdgpu_dm_atomic_commit_tail() - AMDgpu DM's commit tail implementation.
* @state: The atomic state to commit
@@ -11471,6 +11583,8 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_commit *state)
struct drm_device *dev = state->dev;
struct amdgpu_device *adev = drm_to_adev(dev);
struct amdgpu_display_manager *dm = &adev->dm;
+ struct amdgpu_dm_psr_commit_info
+ psr_commit_info[AMDGPU_DM_MAX_CRTC] = {};
struct dm_atomic_state *dm_state;
struct dc_state *dc_state = NULL;
u32 i, j;
@@ -11737,11 +11851,14 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_commit *state)
}
spin_unlock_irqrestore(&adev_to_drm(adev)->event_lock, flags);
+ /* Snapshot commit-specific link state before commit_hw_done(). */
+ amdgpu_dm_snapshot_psr_commit_info(state, psr_commit_info);
+
/* Signal HW programming completion */
drm_atomic_helper_commit_hw_done(state);
if (wait_for_vblank)
- drm_atomic_helper_wait_for_flip_done(dev, state);
+ amdgpu_dm_wait_for_flip_done(dev, state, psr_commit_info);
drm_atomic_helper_cleanup_planes(dev, state);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index dd199e0b7922..db114398ca81 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -583,6 +583,21 @@ struct amdgpu_display_manager {
*/
struct workqueue_struct *vblank_control_workqueue;
+ /** @psr_recovery_work: Deferred fatal PSR-related recovery. */
+ struct work_struct psr_recovery_work;
+ /** @psr_recovery_lock: Serializes scheduling with teardown. */
+ spinlock_t psr_recovery_lock;
+ /** @psr_recovery_pending: Prevents duplicate recovery requests. */
+ bool psr_recovery_pending;
+ /** @psr_recovery_stopping: Prevents new work during teardown. */
+ bool psr_recovery_stopping;
+ /** @psr_disabled_by_recovery: Keeps PSR disabled after recovery. */
+ bool psr_disabled_by_recovery;
+ /** @psr_recovery_commits: Exact commits retained by the request. */
+ struct drm_crtc_commit *psr_recovery_commits[AMDGPU_DM_MAX_CRTC];
+ /** @psr_recovery_flip_mask: CRTCs with a recorded stuck page flip. */
+ u32 psr_recovery_flip_mask;
+
/**
* @idle_workqueue:
*
@@ -1168,4 +1183,8 @@ int amdgpu_dm_initialize_hdmi_connector(struct amdgpu_dm_connector *aconnector);
void retrieve_dmi_info(struct amdgpu_display_manager *dm);
void amdgpu_dm_update_backlight_caps(struct amdgpu_display_manager *dm, int bl_idx);
+bool amdgpu_dm_schedule_psr_recovery(struct amdgpu_display_manager *dm,
+ struct drm_crtc *crtc, struct drm_crtc_commit *commit);
+void amdgpu_dm_psr_recovery_init(struct amdgpu_display_manager *dm);
+void amdgpu_dm_psr_recovery_fini(struct amdgpu_display_manager *dm);
#endif /* __AMDGPU_DM_H__ */
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 6be7f6edd0b2..eaa813b4bf15 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -1302,6 +1302,13 @@ void dm_helpers_dmu_timeout(struct dc_context *ctx)
//amdgpu_device_gpu_recover(dc_context->driver-context, NULL);
}
+void dm_helpers_psr_failure(struct dc_context *ctx)
+{
+ struct amdgpu_device *adev = ctx->driver_context;
+
+ amdgpu_dm_schedule_psr_recovery(&adev->dm, NULL, NULL);
+}
+
void dm_helpers_smu_timeout(struct dc_context *ctx, unsigned int msg_id, unsigned int param, unsigned int timeout_us)
{
// TODO:
@@ -1330,11 +1337,14 @@ void dm_helpers_override_panel_settings(
struct dc_context *ctx,
struct dc_link *link)
{
+ struct amdgpu_device *adev = ctx->driver_context;
unsigned int panel_inst = 0;
// Feature DSC
if (amdgpu_dc_debug_mask & DC_DISABLE_DSC)
link->panel_config.dsc.disable_dsc_edp = true;
+ if (READ_ONCE(adev->dm.psr_disabled_by_recovery))
+ link->panel_config.psr.disable_psr = true;
if (dc_get_edp_link_panel_inst(ctx->dc, link, &panel_inst) && panel_inst == 1) {
link->panel_config.psr.disable_psr = true;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr_recovery.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr_recovery.c
new file mode 100644
index 000000000000..72edbaac2403
--- /dev/null
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr_recovery.c
@@ -0,0 +1,754 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright 2026 David Weber
+ */
+
+#include <linux/jiffies.h>
+#include <linux/string.h>
+#include <linux/wait.h>
+
+#include <drm/drm_atomic.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_modeset_lock.h>
+#include <drm/drm_vblank.h>
+
+#include "amdgpu.h"
+#include "amdgpu_reset.h"
+#include "amdgpu_dm.h"
+#include "core_types.h"
+#include "dc.h"
+#include "dm_helpers.h"
+#include "dpcd_defs.h"
+#include "mod_power.h"
+
+#define PSR_RECOVERY_COMMIT_TIMEOUT_MS 2000
+#define PSR_RECOVERY_VBLANK_TIMEOUT_MS 1000
+#define PSR_RECOVERY_MAX_RECONCILE_PASSES 3
+
+static void amdgpu_dm_snapshot_current_commits(
+ struct amdgpu_display_manager *dm,
+ struct drm_crtc_commit **commits,
+ u32 *flip_mask)
+{
+ struct drm_crtc_commit *commit;
+ struct drm_crtc *crtc;
+ unsigned long flags;
+
+ drm_for_each_crtc(crtc, dm->ddev) {
+ unsigned int index = drm_crtc_index(crtc);
+
+ if (index >= AMDGPU_DM_MAX_CRTC)
+ continue;
+
+ spin_lock_irqsave(&crtc->commit_lock, flags);
+ commit = list_first_entry_or_null(&crtc->commit_list,
+ struct drm_crtc_commit, commit_entry);
+ if (commit)
+ commits[index] = drm_crtc_commit_get(commit);
+ spin_unlock_irqrestore(&crtc->commit_lock, flags);
+
+ if (!commit)
+ continue;
+
+ /* Retire only a flip tied to the exact retained commit. */
+ spin_lock_irqsave(&dm->ddev->event_lock, flags);
+ if (to_amdgpu_crtc(crtc)->pflip_status == AMDGPU_FLIP_SUBMITTED &&
+ to_amdgpu_crtc(crtc)->event &&
+ to_amdgpu_crtc(crtc)->event->base.completion ==
+ &commit->flip_done)
+ *flip_mask |= BIT(index);
+ spin_unlock_irqrestore(&dm->ddev->event_lock, flags);
+ }
+}
+
+static void amdgpu_dm_put_recovery_commits(
+ struct drm_crtc_commit **commits)
+{
+ int i;
+
+ for (i = 0; i < AMDGPU_DM_MAX_CRTC; i++) {
+ if (!commits[i])
+ continue;
+ drm_crtc_commit_put(commits[i]);
+ commits[i] = NULL;
+ }
+}
+
+bool amdgpu_dm_schedule_psr_recovery(struct amdgpu_display_manager *dm,
+ struct drm_crtc *crtc, struct drm_crtc_commit *commit)
+{
+ struct drm_crtc_commit *commits[AMDGPU_DM_MAX_CRTC] = {0};
+ unsigned long flags;
+ u32 flip_mask = 0;
+ bool queue_failed = false;
+ bool scheduled = false;
+ int i;
+
+ /*
+ * A fatal PSR-related failure makes the cached and firmware state
+ * untrustworthy. Keep the fail-safe disable even if teardown rejects
+ * this request or queueing recovery fails.
+ */
+ WRITE_ONCE(dm->psr_disabled_by_recovery, true);
+
+ if (crtc && commit) {
+ unsigned int index = drm_crtc_index(crtc);
+
+ if (index >= AMDGPU_DM_MAX_CRTC)
+ return false;
+ commits[index] = drm_crtc_commit_get(commit);
+ flip_mask = BIT(index);
+ } else {
+ /*
+ * A direct DC failure has no atomic state. Retain the newest
+ * outstanding commit on each CRTC so reset cannot race hardware
+ * programming which was already in flight.
+ */
+ amdgpu_dm_snapshot_current_commits(dm, commits, &flip_mask);
+ }
+
+ spin_lock_irqsave(&dm->psr_recovery_lock, flags);
+ if (dm->psr_recovery_stopping)
+ goto unlock;
+ if (dm->psr_recovery_pending) {
+ /* Merge exact flip evidence without scheduling another reset. */
+ if (crtc && commit) {
+ unsigned int index = drm_crtc_index(crtc);
+ struct drm_crtc_commit *old_commit;
+
+ old_commit = dm->psr_recovery_commits[index];
+ dm->psr_recovery_commits[index] = commits[index];
+ commits[index] = old_commit;
+ dm->psr_recovery_flip_mask |= BIT(index);
+ }
+ scheduled = true;
+ goto unlock;
+ }
+
+ for (i = 0; i < AMDGPU_DM_MAX_CRTC; i++) {
+ dm->psr_recovery_commits[i] = commits[i];
+ commits[i] = NULL;
+ }
+ dm->psr_recovery_flip_mask = flip_mask;
+ dm->psr_recovery_pending = true;
+
+ if (!dm->adev->reset_domain ||
+ !amdgpu_reset_domain_schedule(dm->adev->reset_domain,
+ &dm->psr_recovery_work)) {
+ for (i = 0; i < AMDGPU_DM_MAX_CRTC; i++) {
+ commits[i] = dm->psr_recovery_commits[i];
+ dm->psr_recovery_commits[i] = NULL;
+ }
+ dm->psr_recovery_flip_mask = 0;
+ dm->psr_recovery_pending = false;
+ queue_failed = true;
+ } else {
+ scheduled = true;
+ }
+
+unlock:
+ spin_unlock_irqrestore(&dm->psr_recovery_lock, flags);
+ amdgpu_dm_put_recovery_commits(commits);
+
+ if (queue_failed)
+ drm_err(dm->ddev, "failed to queue fatal PSR recovery\n");
+
+ return scheduled;
+}
+
+static int amdgpu_dm_validate_modeset_flips(struct drm_device *dev,
+ struct drm_atomic_commit *state)
+{
+ struct drm_crtc_commit *commit;
+ struct drm_crtc *crtc;
+ int i;
+
+ for (i = 0; i < dev->mode_config.num_crtc; i++) {
+ crtc = state->crtcs[i].ptr;
+ commit = state->crtcs[i].commit;
+ if (!crtc || !commit || completion_done(&commit->flip_done))
+ continue;
+
+ drm_err(dev,
+ "[CRTC:%d:%s] PSR recovery modeset flip timed out\n",
+ crtc->base.id, crtc->name);
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static int amdgpu_dm_force_full_modeset_locked(
+ struct drm_device *dev,
+ struct drm_modeset_acquire_ctx *ctx)
+{
+ struct drm_atomic_commit *state;
+ struct drm_connector_list_iter conn_iter;
+ struct drm_connector_state *connector_state;
+ struct drm_plane_state *plane_state;
+ struct drm_crtc_state *crtc_state;
+ struct drm_connector *connector;
+ struct drm_plane *plane;
+ struct drm_crtc *crtc;
+ unsigned int active_crtcs = 0;
+ int ret;
+
+ state = drm_atomic_commit_alloc(dev);
+ if (!state)
+ return -ENOMEM;
+ state->acquire_ctx = ctx;
+
+ drm_for_each_crtc(crtc, dev) {
+ if (!crtc->state || !crtc->state->active)
+ continue;
+
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state)) {
+ ret = PTR_ERR(crtc_state);
+ goto out;
+ }
+ crtc_state->mode_changed = true;
+ active_crtcs++;
+ }
+
+ ret = 0;
+ drm_connector_list_iter_begin(dev, &conn_iter);
+ drm_for_each_connector_iter(connector, &conn_iter) {
+ if (!connector->state || !connector->state->crtc)
+ continue;
+
+ connector_state = drm_atomic_get_connector_state(state, connector);
+ if (IS_ERR(connector_state)) {
+ ret = PTR_ERR(connector_state);
+ break;
+ }
+ }
+ drm_connector_list_iter_end(&conn_iter);
+ if (ret)
+ goto out;
+
+ drm_for_each_plane(plane, dev) {
+ if (!plane->state || !plane->state->crtc)
+ continue;
+
+ plane_state = drm_atomic_get_plane_state(state, plane);
+ if (IS_ERR(plane_state)) {
+ ret = PTR_ERR(plane_state);
+ goto out;
+ }
+ }
+
+ drm_info(dev, "PSR recovery: forcing modeset on %u active CRTCs\n",
+ active_crtcs);
+ ret = drm_atomic_commit(state);
+ if (!ret)
+ ret = amdgpu_dm_validate_modeset_flips(dev, state);
+
+out:
+ drm_atomic_commit_put(state);
+ return ret;
+}
+
+static int amdgpu_dm_wait_for_recovery_commits(
+ struct drm_device *dev,
+ struct drm_crtc_commit **commits,
+ bool cleanup)
+{
+ unsigned long deadline = jiffies +
+ msecs_to_jiffies(PSR_RECOVERY_COMMIT_TIMEOUT_MS);
+ struct completion *completion;
+ unsigned long timeout;
+ bool timed_out = false;
+ int i;
+
+ for (i = 0; i < AMDGPU_DM_MAX_CRTC; i++) {
+ if (!commits[i])
+ continue;
+
+ completion = cleanup ? &commits[i]->cleanup_done :
+ &commits[i]->hw_done;
+ if (completion_done(completion))
+ continue;
+
+ timeout = time_before(jiffies, deadline) ?
+ deadline - jiffies : 0;
+ if (!timeout || !wait_for_completion_timeout(completion, timeout)) {
+ drm_warn(dev,
+ "[CRTC:%d:%s] PSR recovery %s wait timed out\n",
+ commits[i]->crtc->base.id,
+ commits[i]->crtc->name,
+ cleanup ? "cleanup_done" : "hw_done");
+ timed_out = true;
+ }
+ }
+
+ return timed_out ? -ETIMEDOUT : 0;
+}
+
+static u32 amdgpu_dm_find_submitted_recovery_flips(
+ struct amdgpu_display_manager *dm,
+ struct drm_crtc_commit **commits)
+{
+ struct drm_device *dev = dm->ddev;
+ u32 flip_mask = 0;
+ int i;
+
+ for (i = 0; i < AMDGPU_DM_MAX_CRTC; i++) {
+ struct drm_crtc_commit *commit = commits[i];
+ struct amdgpu_crtc *acrtc;
+ unsigned long flags;
+
+ if (!commit)
+ continue;
+
+ acrtc = to_amdgpu_crtc(commit->crtc);
+ spin_lock_irqsave(&dev->event_lock, flags);
+ if (acrtc->pflip_status == AMDGPU_FLIP_SUBMITTED &&
+ acrtc->event &&
+ acrtc->event->base.completion == &commit->flip_done)
+ flip_mask |= BIT(i);
+ spin_unlock_irqrestore(&dev->event_lock, flags);
+ }
+
+ return flip_mask;
+}
+
+static unsigned int amdgpu_dm_retire_recovered_flips(
+ struct amdgpu_display_manager *dm,
+ struct drm_crtc_commit **commits,
+ u32 flip_mask)
+{
+ struct drm_device *dev = dm->ddev;
+ unsigned int completed = 0;
+ int i;
+
+ for (i = 0; i < AMDGPU_DM_MAX_CRTC; i++) {
+ struct drm_crtc_commit *commit = commits[i];
+ struct amdgpu_crtc *acrtc;
+ struct drm_crtc *crtc;
+ unsigned long flags;
+ u64 last_vblank;
+ long waited;
+ int ret;
+
+ if (!(flip_mask & BIT(i)) || !commit)
+ continue;
+
+ crtc = commit->crtc;
+ if (!crtc->state || !crtc->state->active) {
+ drm_warn(dev,
+ "[CRTC:%d:%s] cannot retire recovered flip on inactive CRTC\n",
+ crtc->base.id, crtc->name);
+ continue;
+ }
+
+ ret = drm_crtc_vblank_get(crtc);
+ if (ret) {
+ drm_warn(dev,
+ "[CRTC:%d:%s] no vblank available after PSR recovery: %d\n",
+ crtc->base.id, crtc->name, ret);
+ } else {
+ last_vblank = drm_crtc_vblank_count(crtc);
+ waited = wait_event_timeout(
+ *drm_crtc_vblank_waitqueue(crtc),
+ last_vblank !=
+ drm_crtc_vblank_count(crtc),
+ msecs_to_jiffies(
+ PSR_RECOVERY_VBLANK_TIMEOUT_MS));
+ drm_crtc_vblank_put(crtc);
+ if (!waited)
+ drm_warn(dev,
+ "[CRTC:%d:%s] no vblank after PSR recovery; retiring flip without one\n",
+ crtc->base.id, crtc->name);
+ }
+
+ acrtc = to_amdgpu_crtc(crtc);
+ spin_lock_irqsave(&dev->event_lock, flags);
+ if (acrtc->pflip_status != AMDGPU_FLIP_SUBMITTED ||
+ !acrtc->event) {
+ spin_unlock_irqrestore(&dev->event_lock, flags);
+ continue;
+ }
+
+ if (acrtc->event->base.completion != &commit->flip_done) {
+ drm_warn(dev,
+ "[CRTC:%d:%s] pending flip changed during PSR recovery\n",
+ crtc->base.id, crtc->name);
+ spin_unlock_irqrestore(&dev->event_lock, flags);
+ continue;
+ }
+
+ drm_info(dev,
+ "[CRTC:%d:%s] retiring flip after successful PSR recovery\n",
+ crtc->base.id, crtc->name);
+ drm_crtc_send_vblank_event(crtc, acrtc->event);
+ acrtc->event = NULL;
+ drm_crtc_vblank_put(crtc);
+ acrtc->pflip_status = AMDGPU_FLIP_NONE;
+ completed++;
+ spin_unlock_irqrestore(&dev->event_lock, flags);
+ }
+
+ return completed;
+}
+
+static void amdgpu_dm_mark_psr_disabled(struct amdgpu_display_manager *dm)
+{
+ struct dc *dc = dm->dc;
+ struct dc_link *link;
+ int i;
+
+ if (!dc)
+ return;
+
+ for (i = 0; i < dc->link_count; i++) {
+ link = dc->links[i];
+ if (!link)
+ continue;
+
+ if (dm->power_module)
+ mod_power_disable_psr_for_link(dm->power_module, link);
+ link->panel_config.psr.disable_psr = true;
+ link->psr_settings.psr_feature_enabled = false;
+ link->psr_settings.psr_allow_active = false;
+ link->psr_settings.psr_allow_active_valid = false;
+ }
+}
+
+static unsigned int amdgpu_dm_disable_psr_at_active_sinks(
+ struct amdgpu_display_manager *dm)
+{
+ union dpcd_psr_configuration psr_configuration = {0};
+ struct dc *dc = dm->dc;
+ struct dc_link *link;
+ unsigned int writes = 0;
+ int i;
+ int j;
+
+ if (!dc || !dc->current_state)
+ return 0;
+
+ for (i = 0; i < dc->current_state->stream_count; i++) {
+ struct dc_stream_state *stream = dc->current_state->streams[i];
+
+ if (!stream || !stream->link ||
+ stream->link->connector_signal != SIGNAL_TYPE_EDP)
+ continue;
+
+ link = stream->link;
+ for (j = 0; j < i; j++)
+ if (dc->current_state->streams[j] &&
+ dc->current_state->streams[j]->link == link)
+ break;
+ if (j != i)
+ continue;
+
+ if (!dm_helpers_dp_write_dpcd(link->ctx, link, DP_PSR_EN_CFG,
+ &psr_configuration.raw,
+ sizeof(psr_configuration.raw))) {
+ drm_warn(dm->ddev,
+ "failed to disable PSR at sink on link %d\n",
+ link->link_index);
+ } else {
+ writes++;
+ }
+ }
+
+ return writes;
+}
+
+static int amdgpu_dm_lock_all_modeset(struct drm_device *dev,
+ struct drm_modeset_acquire_ctx *ctx)
+{
+ int ret;
+
+ drm_modeset_acquire_init(ctx, 0);
+
+retry:
+ ret = drm_modeset_lock_all_ctx(dev, ctx);
+ if (ret == -EDEADLK) {
+ ret = drm_modeset_backoff(ctx);
+ if (!ret)
+ goto retry;
+ }
+
+ return ret;
+}
+
+static unsigned int amdgpu_dm_collect_recovery_evidence(
+ struct amdgpu_display_manager *dm,
+ struct drm_crtc_commit **commits,
+ u32 *flip_mask)
+{
+ struct drm_crtc_commit *old_commits[AMDGPU_DM_MAX_CRTC] = {0};
+ unsigned long flags;
+ unsigned int collected = 0;
+ int i;
+
+ spin_lock_irqsave(&dm->psr_recovery_lock, flags);
+ for (i = 0; i < AMDGPU_DM_MAX_CRTC; i++) {
+ if (!dm->psr_recovery_commits[i])
+ continue;
+
+ old_commits[i] = commits[i];
+ commits[i] = dm->psr_recovery_commits[i];
+ dm->psr_recovery_commits[i] = NULL;
+ collected++;
+ }
+ *flip_mask |= dm->psr_recovery_flip_mask;
+ dm->psr_recovery_flip_mask = 0;
+ spin_unlock_irqrestore(&dm->psr_recovery_lock, flags);
+
+ amdgpu_dm_put_recovery_commits(old_commits);
+
+ return collected;
+}
+
+static bool amdgpu_dm_finish_recovery_request(
+ struct amdgpu_display_manager *dm,
+ struct drm_crtc_commit **commits)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&dm->psr_recovery_lock, flags);
+ if (dm->psr_recovery_flip_mask) {
+ spin_unlock_irqrestore(&dm->psr_recovery_lock, flags);
+ return false;
+ }
+ dm->psr_recovery_pending = false;
+ spin_unlock_irqrestore(&dm->psr_recovery_lock, flags);
+
+ amdgpu_dm_put_recovery_commits(commits);
+
+ return true;
+}
+
+static void amdgpu_dm_abort_recovery_request(
+ struct amdgpu_display_manager *dm,
+ struct drm_crtc_commit **commits)
+{
+ struct drm_crtc_commit *late_commits[AMDGPU_DM_MAX_CRTC] = {0};
+ unsigned long flags;
+ int i;
+
+ spin_lock_irqsave(&dm->psr_recovery_lock, flags);
+ for (i = 0; i < AMDGPU_DM_MAX_CRTC; i++) {
+ late_commits[i] = dm->psr_recovery_commits[i];
+ dm->psr_recovery_commits[i] = NULL;
+ }
+ dm->psr_recovery_flip_mask = 0;
+ dm->psr_recovery_pending = false;
+ spin_unlock_irqrestore(&dm->psr_recovery_lock, flags);
+
+ amdgpu_dm_put_recovery_commits(commits);
+ amdgpu_dm_put_recovery_commits(late_commits);
+}
+
+static void amdgpu_dm_psr_recovery_work(struct work_struct *work)
+{
+ struct amdgpu_display_manager *dm =
+ container_of(work, struct amdgpu_display_manager,
+ psr_recovery_work);
+ struct amdgpu_device *adev = dm->adev;
+ struct drm_crtc_commit *commits[AMDGPU_DM_MAX_CRTC] = {0};
+ struct amdgpu_reset_context reset_context = {
+ .method = AMD_RESET_METHOD_NONE,
+ .reset_req_dev = adev,
+ .src = AMDGPU_RESET_SRC_PSR,
+ };
+ struct drm_modeset_acquire_ctx ctx;
+ unsigned int completed_flips;
+ unsigned int reconcile_passes = 0;
+ unsigned int recorded_commits = 0;
+ unsigned int sink_writes;
+ u32 flip_mask = 0;
+ u32 submitted_flip_mask;
+ bool request_finished = false;
+ int lock_ret;
+ int ret = 0;
+
+ recorded_commits = amdgpu_dm_collect_recovery_evidence(dm, commits,
+ &flip_mask);
+
+ drm_err(dm->ddev,
+ "fatal PSR-related display failure; disabling PSR and recovering GPU\n");
+ drm_info(dm->ddev,
+ "PSR recovery: recorded %u commits and %u submitted flips\n",
+ recorded_commits, hweight32(flip_mask));
+
+ /*
+ * Stop atomic commits while reset and recovered-flip reconciliation run.
+ * This relies on DM's amdgpu_in_reset() suspend path avoiding the normal
+ * DRM atomic suspend, which would try to acquire these locks again.
+ */
+ drm_info(dm->ddev, "PSR recovery: acquiring modeset locks\n");
+ lock_ret = amdgpu_dm_lock_all_modeset(dm->ddev, &ctx);
+ if (lock_ret) {
+ ret = lock_ret;
+ goto out_locks;
+ }
+ drm_info(dm->ddev, "PSR recovery: modeset locks acquired\n");
+
+ drm_info(dm->ddev,
+ "PSR recovery: waiting for recorded hardware programming\n");
+ ret = amdgpu_dm_wait_for_recovery_commits(dm->ddev, commits, false);
+ if (ret) {
+ drm_warn(dm->ddev,
+ "PSR recovery: proceeding with reset after hw_done timeout\n");
+ ret = 0;
+ }
+
+ mutex_lock(&dm->dc_lock);
+ amdgpu_dm_mark_psr_disabled(dm);
+ mutex_unlock(&dm->dc_lock);
+ drm_info(dm->ddev, "PSR recovery: PSR disabled in software\n");
+
+ if (!amdgpu_device_should_recover_gpu(adev)) {
+ drm_err(dm->ddev, "PSR recovery: GPU recovery policy is disabled\n");
+ ret = -EOPNOTSUPP;
+ goto out_locks;
+ }
+
+ set_bit(AMDGPU_NEED_FULL_RESET, &reset_context.flags);
+ drm_info(dm->ddev, "PSR recovery: starting full GPU reset\n");
+ ret = amdgpu_device_gpu_recover(adev, NULL, &reset_context);
+ drm_info(dm->ddev, "PSR recovery: full GPU reset returned %d\n", ret);
+ if (ret)
+ goto out_locks;
+
+reconcile_after_reset:
+ if (++reconcile_passes > PSR_RECOVERY_MAX_RECONCILE_PASSES) {
+ drm_err(dm->ddev,
+ "PSR recovery: too many late evidence reconciliation passes\n");
+ ret = -EBUSY;
+ goto out_locks;
+ }
+
+ recorded_commits = amdgpu_dm_collect_recovery_evidence(dm, commits,
+ &flip_mask);
+ if (recorded_commits)
+ drm_info(dm->ddev,
+ "PSR recovery: merged %u late commit records and %u submitted flips\n",
+ recorded_commits, hweight32(flip_mask));
+
+ /*
+ * A retained commit can arm its flip after the initial request snapshot
+ * but before recovery acquires all modeset locks. Refresh the exact
+ * event-to-commit match after reset so that flip does not strand
+ * cleanup_done.
+ */
+ submitted_flip_mask =
+ amdgpu_dm_find_submitted_recovery_flips(dm, commits);
+ if (submitted_flip_mask & ~flip_mask)
+ drm_info(dm->ddev,
+ "PSR recovery: found %u newly submitted retained flips after reset\n",
+ hweight32(submitted_flip_mask & ~flip_mask));
+ flip_mask |= submitted_flip_mask;
+
+ completed_flips = amdgpu_dm_retire_recovered_flips(dm, commits,
+ flip_mask);
+ flip_mask = 0;
+ drm_info(dm->ddev, "PSR recovery: retired %u recovered flips\n",
+ completed_flips);
+ drm_info(dm->ddev, "PSR recovery: waiting for recorded commit cleanup\n");
+ ret = amdgpu_dm_wait_for_recovery_commits(dm->ddev, commits, true);
+ if (ret && amdgpu_dm_collect_recovery_evidence(dm, commits,
+ &flip_mask)) {
+ drm_info(dm->ddev,
+ "PSR recovery: retrying cleanup with late exact flip evidence\n");
+ ret = 0;
+ goto reconcile_after_reset;
+ }
+ if (ret)
+ goto out_locks;
+ drm_info(dm->ddev, "PSR recovery: recorded commits quiesced\n");
+
+ mutex_lock(&dm->dc_lock);
+ amdgpu_dm_mark_psr_disabled(dm);
+ sink_writes = amdgpu_dm_disable_psr_at_active_sinks(dm);
+ mutex_unlock(&dm->dc_lock);
+ drm_info(dm->ddev,
+ "PSR recovery: PSR disabled after reset (sink writes=%u)\n",
+ sink_writes);
+
+ drm_info(dm->ddev, "PSR recovery: starting forced modeset\n");
+ ret = amdgpu_dm_force_full_modeset_locked(dm->ddev, &ctx);
+ drm_info(dm->ddev,
+ "PSR recovery: forced modeset and flip validation returned %d\n",
+ ret);
+
+ /* The modeset PSR setup path must not undo the fatal fallback. */
+ mutex_lock(&dm->dc_lock);
+ amdgpu_dm_mark_psr_disabled(dm);
+ sink_writes = amdgpu_dm_disable_psr_at_active_sinks(dm);
+ mutex_unlock(&dm->dc_lock);
+ drm_info(dm->ddev,
+ "PSR recovery: PSR disable reasserted after modeset (sink writes=%u)\n",
+ sink_writes);
+ if (ret)
+ goto out_locks;
+
+ if (!amdgpu_dm_finish_recovery_request(dm, commits)) {
+ drm_info(dm->ddev,
+ "PSR recovery: processing late exact flip evidence\n");
+ goto reconcile_after_reset;
+ }
+ request_finished = true;
+
+out_locks:
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+
+ if (ret) {
+ drm_err(dm->ddev, "PSR display recovery failed: %d\n", ret);
+ drm_dev_wedged_event(dm->ddev,
+ DRM_WEDGE_RECOVERY_REBIND |
+ DRM_WEDGE_RECOVERY_BUS_RESET, NULL);
+ } else {
+ drm_info(dm->ddev, "PSR display recovery completed\n");
+ }
+
+ if (!request_finished)
+ amdgpu_dm_abort_recovery_request(dm, commits);
+}
+
+void amdgpu_dm_psr_recovery_init(struct amdgpu_display_manager *dm)
+{
+ spin_lock_init(&dm->psr_recovery_lock);
+ dm->psr_recovery_pending = false;
+ dm->psr_recovery_stopping = false;
+ dm->psr_disabled_by_recovery = false;
+ dm->psr_recovery_flip_mask = 0;
+ memset(dm->psr_recovery_commits, 0,
+ sizeof(dm->psr_recovery_commits));
+ INIT_WORK(&dm->psr_recovery_work, amdgpu_dm_psr_recovery_work);
+}
+
+void amdgpu_dm_psr_recovery_fini(struct amdgpu_display_manager *dm)
+{
+ struct drm_crtc_commit *commits[AMDGPU_DM_MAX_CRTC] = {0};
+ unsigned long flags;
+ int i;
+
+ spin_lock_irqsave(&dm->psr_recovery_lock, flags);
+ dm->psr_recovery_stopping = true;
+ spin_unlock_irqrestore(&dm->psr_recovery_lock, flags);
+
+ /*
+ * Recovery runs on the reset-domain workqueue. Device teardown must
+ * not invoke this finalizer from that workqueue, or this synchronous
+ * cancellation would wait for the current worker itself.
+ */
+ cancel_work_sync(&dm->psr_recovery_work);
+
+ spin_lock_irqsave(&dm->psr_recovery_lock, flags);
+ for (i = 0; i < AMDGPU_DM_MAX_CRTC; i++) {
+ commits[i] = dm->psr_recovery_commits[i];
+ dm->psr_recovery_commits[i] = NULL;
+ }
+ dm->psr_recovery_flip_mask = 0;
+ dm->psr_recovery_pending = false;
+ spin_unlock_irqrestore(&dm->psr_recovery_lock, flags);
+
+ amdgpu_dm_put_recovery_commits(commits);
+}
diff --git a/drivers/gpu/drm/amd/display/dc/dm_helpers.h b/drivers/gpu/drm/amd/display/dc/dm_helpers.h
index 63704d21a0b5..122c21d684fe 100644
--- a/drivers/gpu/drm/amd/display/dc/dm_helpers.h
+++ b/drivers/gpu/drm/amd/display/dc/dm_helpers.h
@@ -209,6 +209,7 @@ void dm_set_phyd32clk(struct dc_context *ctx, int freq_khz);
bool dm_helpers_dmub_outbox_interrupt_control(struct dc_context *ctx, bool enable);
void dm_helpers_dmu_timeout(struct dc_context *ctx);
+void dm_helpers_psr_failure(struct dc_context *ctx);
void dm_helpers_smu_timeout(struct dc_context *ctx, unsigned int msg_id, unsigned int param, unsigned int timeout_us);
// 0x1 = Result_OK, 0xFE = Result_UnkmownCmd, 0x0 = Status_Busy
diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
index f7001f01f960..160a658de727 100644
--- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
+++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
@@ -584,6 +584,7 @@ bool edp_set_psr_allow_active(struct dc_link *link, const bool *allow_active,
struct dmcu *dmcu = dc->res_pool->dmcu;
struct dmub_psr *psr = dc->res_pool->psr;
unsigned int panel_inst;
+ bool recover_failed_exit = false;
if (psr == NULL && force_static)
return false;
@@ -614,6 +615,10 @@ bool edp_set_psr_allow_active(struct dc_link *link, const bool *allow_active,
link->psr_settings.psr_allow_active != *allow_active)) {
bool programmed = true;
+ recover_failed_exit = wait && !*allow_active &&
+ link->psr_settings.psr_allow_active_valid &&
+ link->psr_settings.psr_allow_active;
+
if (!*allow_active)
dc_z10_restore(dc);
@@ -629,6 +634,8 @@ bool edp_set_psr_allow_active(struct dc_link *link, const bool *allow_active,
if (!programmed) {
/* The command may have succeeded, but its state is unknown. */
link->psr_settings.psr_allow_active_valid = false;
+ if (recover_failed_exit)
+ dm_helpers_psr_failure(link->ctx);
return false;
}
@@ -713,6 +720,14 @@ bool edp_setup_psr(struct dc_link *link,
if (!link)
return false;
+ /* PSR may have been disabled after a fatal runtime failure. */
+ if (link->panel_config.psr.disable_psr) {
+ link->psr_settings.psr_feature_enabled = false;
+ link->psr_settings.psr_allow_active = false;
+ link->psr_settings.psr_allow_active_valid = false;
+ return false;
+ }
+
/* This is a workaround: some vendors require the source to
* read the PSR cap; otherwise, the vendor's PSR feature will
* fall back to its default behavior, causing a misconfiguration
diff --git a/drivers/gpu/drm/amd/display/modules/inc/mod_power.h b/drivers/gpu/drm/amd/display/modules/inc/mod_power.h
index 893fcd04a197..fdbf32ed9213 100644
--- a/drivers/gpu/drm/amd/display/modules/inc/mod_power.h
+++ b/drivers/gpu/drm/amd/display/modules/inc/mod_power.h
@@ -333,6 +333,9 @@ bool mod_power_set_psr_event(struct mod_power *mod_power,
struct dc_stream_state *stream, bool set_event,
enum psr_event event, bool wait);
+void mod_power_disable_psr_for_link(struct mod_power *mod_power,
+ const struct dc_link *link);
+
bool mod_power_get_psr_state(struct mod_power *mod_power,
const struct dc_stream_state *stream,
enum dc_psr_state *state);
diff --git a/drivers/gpu/drm/amd/display/modules/power/power_psr.c b/drivers/gpu/drm/amd/display/modules/power/power_psr.c
index 7ef53966c9e4..ab742597d744 100644
--- a/drivers/gpu/drm/amd/display/modules/power/power_psr.c
+++ b/drivers/gpu/drm/amd/display/modules/power/power_psr.c
@@ -3,6 +3,7 @@
// Copyright 2026 Advanced Micro Devices, Inc.
#include "dm_services.h"
+#include "dm_helpers.h"
#include "dc.h"
#include "mod_power.h"
#include "core_types.h"
@@ -16,6 +17,7 @@
#define DC_TRACE_LEVEL_MESSAGE(...) /* do nothing */
#define DC_TRACE_LEVEL_MESSAGEP(...) /* do nothing */
+#define DC_LOGGER core_power->dc->ctx->logger
#include "dc/inc/hw/dmcu.h"
#include "dc/inc/hw/abm.h"
#include "dmub_cmd.h"
@@ -54,6 +56,15 @@ bool mod_power_psr_notify_mode_change(struct mod_power *mod_power,
core_power = MOD_POWER_TO_CORE(mod_power);
dc = core_power->dc;
+ /* A fatal recovery can disable PSR after initial link setup. */
+ if (link->panel_config.psr.disable_psr) {
+ mod_power_disable_psr_for_link(mod_power, link);
+ link->psr_settings.psr_feature_enabled = false;
+ link->psr_settings.psr_allow_active = false;
+ link->psr_settings.psr_allow_active_valid = false;
+ return true;
+ }
+
// NO num_entities check here - already validated by caller
// stream_index is passed as validated parameter
active_psr_events = core_power->map[stream_index].psr_events;
@@ -147,10 +158,13 @@ static bool set_psr_enable(struct mod_power *mod_power,
bool force_static)
{
struct core_power *core_power = NULL;
- enum dc_psr_state state = PSR_STATE0;
+ enum dc_psr_state state = PSR_STATE_INVALID;
unsigned int retry_count;
+ unsigned int valid_query_count = 0;
const unsigned int max_retry = 1000;
struct dc_link *link = NULL;
+ bool programmed;
+ bool recover_failed_exit;
bool state_reached = false;
if (mod_power == NULL)
@@ -196,15 +210,21 @@ static bool set_psr_enable(struct mod_power *mod_power,
}
link = dc_stream_get_link(stream);
+ recover_failed_exit = wait && !psr_enable &&
+ link->psr_settings.psr_allow_active_valid &&
+ link->psr_settings.psr_allow_active;
- if (!dc_link_set_psr_allow_active(link, &psr_enable, false, force_static, NULL)) {
+ programmed = dc_link_set_psr_allow_active(link, &psr_enable, false,
+ force_static, NULL);
+ if (!programmed) {
DC_TRACE_LEVEL_MESSAGE(DAL_TRACE_LEVEL_ERROR,
WPP_BIT_FLAG_Firmware_PsrState,
"set psr enable: ERROR: stream=%p link=%p psr_enable=%d",
stream,
link,
psr_enable);
- return false;
+ if (!wait)
+ return false;
}
if (wait == true) {
@@ -216,6 +236,7 @@ static bool set_psr_enable(struct mod_power *mod_power,
for (retry_count = 0; retry_count < max_retry; retry_count++) {
if (dc_link_get_psr_state(link, &state)) {
+ valid_query_count++;
if (psr_enable) {
if (state != PSR_STATE0 &&
(!force_static || state == PSR_STATE3)) {
@@ -236,11 +257,21 @@ static bool set_psr_enable(struct mod_power *mod_power,
(int)psr_enable);
if (!state_reached) {
- ASSERT(0);
+ DC_LOG_ERROR("PSR policy %s timeout: link=%u queries=%u state=%d\n",
+ psr_enable ? "enable" : "disable",
+ link->link_index, valid_query_count, state);
/* The command may have succeeded even though its state is unknown. */
link->psr_settings.psr_allow_active_valid = false;
+ if (recover_failed_exit)
+ dm_helpers_psr_failure(core_power->dc->ctx);
return false;
}
+
+ /* A state response confirms a command whose acknowledgment was lost. */
+ if (!programmed) {
+ link->psr_settings.psr_allow_active = psr_enable;
+ link->psr_settings.psr_allow_active_valid = true;
+ }
} else {
DC_TRACE_LEVEL_MESSAGE(DAL_TRACE_LEVEL_INFORMATION,
WPP_BIT_FLAG_Firmware_PsrState,
@@ -284,7 +315,7 @@ bool mod_power_set_psr_event(struct mod_power *mod_power,
return false;
link = dc_stream_get_link(stream);
- if (!link)
+ if (!link || !link->psr_settings.psr_feature_enabled)
return false;
event_changed = (core_power->map[stream_index].psr_events & event) !=
@@ -396,6 +427,26 @@ bool mod_power_set_psr_event(struct mod_power *mod_power,
return true;
}
+void mod_power_disable_psr_for_link(struct mod_power *mod_power,
+ const struct dc_link *link)
+{
+ struct core_power *core_power;
+ unsigned int i;
+
+ if (!mod_power || !link)
+ return;
+
+ core_power = MOD_POWER_TO_CORE(mod_power);
+ for (i = 0; i < core_power->num_entities; i++) {
+ if (!core_power->map[i].stream ||
+ dc_stream_get_link(core_power->map[i].stream) != link)
+ continue;
+
+ core_power->map[i].psr_enabled = false;
+ core_power->map[i].psr_events |= psr_event_os_request_disable;
+ }
+}
+
bool mod_power_get_psr_state(struct mod_power *mod_power,
const struct dc_stream_state *stream,
enum dc_psr_state *state)
--
2.54.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC PATCH 5/5] drm/amd/display: add PSR recovery fault injection
2026-08-05 11:52 [RFC PATCH 0/5] drm/amd/display: recover from PSR-related display stalls David Weber
` (3 preceding siblings ...)
2026-08-05 11:52 ` [RFC PATCH 4/5] drm/amd/display: recover from fatal PSR-related display timeouts David Weber
@ 2026-08-05 11:52 ` David Weber
4 siblings, 0 replies; 6+ messages in thread
From: David Weber @ 2026-08-05 11:52 UTC (permalink / raw)
To: amd-gfx
Cc: alexander.deucher, christian.koenig, harry.wentland, sunpeng.li,
siqueira, someguy, David Weber
Introduce two write-only debugfs controls for testing PSR recovery.
amdgpu_dm_trigger_psr_recovery directly queues the PSR recovery worker
without first creating a display or firmware failure. Trigger it with:
echo 1 > /sys/kernel/debug/dri/0/amdgpu_dm_trigger_psr_recovery
amdgpu_dm_halt_dmub_for_psr_recovery selects an active PSR-enabled eDP
stream, enters PSR, halts DMUB firmware, and requests a synchronous PSR
exit. The failed exit should schedule the normal PSR recovery path.
Trigger it with:
echo 0xDEADDEAD > \
/sys/kernel/debug/dri/0/amdgpu_dm_halt_dmub_for_psr_recovery
Halting DMUB is destructive until reset. The second control therefore
requires the existing STOP_FW response value 0xDEADDEAD as a safety
cookie. Both controls refuse to run when GPU recovery is disabled, no
reset domain is available, teardown has started, or another PSR recovery
is pending.
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: David Weber <weber.aulendorf@gmail.com>
---
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 177 ++++++++++++++++++
1 file changed, 177 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 7db38ad3f848..a80e13b98ad7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -29,6 +29,7 @@
#include <media/cec-notifier.h>
#include "dc.h"
+#include "core_types.h"
#include "amdgpu.h"
#include "amdgpu_dm.h"
#include "amdgpu_dm_debugfs.h"
@@ -4483,6 +4484,176 @@ DEFINE_DEBUGFS_ATTRIBUTE(skip_detection_link_training_fops,
skip_detection_link_training_get,
skip_detection_link_training_set, "%llu\n");
+static int psr_recovery_test_available(struct amdgpu_device *adev)
+{
+ unsigned long flags;
+ bool pending;
+ bool stopping;
+
+ if (!amdgpu_device_should_recover_gpu(adev) || !adev->reset_domain)
+ return -EOPNOTSUPP;
+
+ spin_lock_irqsave(&adev->dm.psr_recovery_lock, flags);
+ pending = adev->dm.psr_recovery_pending;
+ stopping = adev->dm.psr_recovery_stopping;
+ spin_unlock_irqrestore(&adev->dm.psr_recovery_lock, flags);
+
+ if (stopping)
+ return -ESHUTDOWN;
+
+ return pending ? -EBUSY : 0;
+}
+
+static int trigger_psr_recovery_set(void *data, u64 val)
+{
+ struct amdgpu_device *adev = data;
+ int ret;
+
+ if (val != 1)
+ return -EINVAL;
+ if (!adev->dm.dc || !adev->dm.dc->ctx)
+ return -ENODEV;
+ ret = psr_recovery_test_available(adev);
+ if (ret)
+ return ret;
+
+ drm_warn(adev_to_drm(adev),
+ "injecting a fatal PSR failure to test display recovery\n");
+ if (!amdgpu_dm_schedule_psr_recovery(&adev->dm, NULL, NULL))
+ return -EIO;
+
+ return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(trigger_psr_recovery_fops, NULL,
+ trigger_psr_recovery_set, "%llu\n");
+
+static int halt_dmub_for_psr_recovery_set(void *data, u64 val)
+{
+ struct amdgpu_device *adev = data;
+ struct dc *dc = adev->dm.dc;
+ struct dc_dmub_srv *dc_dmub_srv;
+ struct dmub_srv *dmub;
+ struct dc_link *link = NULL;
+ enum dmub_status status;
+ u32 response = 0;
+ bool allow_active;
+ bool pending;
+ bool programmed;
+ unsigned long flags;
+ int ret = 0;
+ int i;
+
+ /*
+ * Halting DMUB is destructive until reset. Require the STOP_FW ABI
+ * response cookie to make an accidental write less likely.
+ */
+ if (val != DMUB_GPINT__STOP_FW_RESPONSE)
+ return -EINVAL;
+ ret = psr_recovery_test_available(adev);
+ if (ret)
+ return ret;
+ if (!dc || !dc->ctx || !dc->ctx->dmub_srv ||
+ !dc->res_pool || !dc->res_pool->psr)
+ return -ENODEV;
+
+ dc_dmub_srv = dc->ctx->dmub_srv;
+ dmub = dc_dmub_srv->dmub;
+ if (!dmub)
+ return -ENODEV;
+
+ mutex_lock(&adev->dm.dc_lock);
+ if (dc->current_state) {
+ for (i = 0; i < dc->current_state->stream_count; i++) {
+ struct dc_stream_state *stream =
+ dc->current_state->streams[i];
+
+ if (stream && stream->link &&
+ stream->link->connector_signal == SIGNAL_TYPE_EDP &&
+ stream->link->psr_settings.psr_feature_enabled) {
+ link = stream->link;
+ break;
+ }
+ }
+ }
+
+ if (!link) {
+ mutex_unlock(&adev->dm.dc_lock);
+ return -ENODEV;
+ }
+ ret = psr_recovery_test_available(adev);
+ if (ret)
+ goto unlock;
+
+ if (dc_dmub_srv->idle_allowed)
+ dc_dmub_srv_apply_idle_power_optimizations(dc, false);
+
+ /* Enter PSR before halting firmware so the test exercises failed exit. */
+ link->psr_settings.psr_allow_active_valid = false;
+ allow_active = true;
+ programmed = dc_link_set_psr_allow_active(link, &allow_active, true,
+ false, NULL);
+ if (!programmed) {
+ drm_err(adev_to_drm(adev),
+ "failed to enter PSR before halting DMUB\n");
+ ret = -EIO;
+ goto unlock;
+ }
+
+ drm_warn(adev_to_drm(adev),
+ "halting DMUB firmware to test PSR exit recovery\n");
+ status = dmub_srv_send_gpint_command(dmub, DMUB_GPINT__STOP_FW, 0,
+ 100000);
+ if (status != DMUB_STATUS_OK) {
+ drm_err(adev_to_drm(adev),
+ "failed to halt DMUB firmware: %d; recovering\n", status);
+ if (!amdgpu_dm_schedule_psr_recovery(&adev->dm, NULL, NULL))
+ drm_err(adev_to_drm(adev),
+ "failed to schedule recovery after DMUB halt error\n");
+ ret = -EIO;
+ goto unlock;
+ }
+
+ for (i = 0; i < 100000; i++) {
+ status = dmub_srv_get_gpint_response(dmub, &response);
+ if (status == DMUB_STATUS_OK &&
+ response == DMUB_GPINT__STOP_FW_RESPONSE)
+ break;
+ udelay(1);
+ }
+
+ if (response != DMUB_GPINT__STOP_FW_RESPONSE)
+ drm_warn(adev_to_drm(adev),
+ "DMUB halt response timed out; testing PSR exit anyway\n");
+
+ allow_active = false;
+ programmed = dc_link_set_psr_allow_active(link, &allow_active, true,
+ false, NULL);
+ spin_lock_irqsave(&adev->dm.psr_recovery_lock, flags);
+ pending = adev->dm.psr_recovery_pending;
+ spin_unlock_irqrestore(&adev->dm.psr_recovery_lock, flags);
+ if (programmed) {
+ drm_err(adev_to_drm(adev),
+ "PSR exit unexpectedly succeeded after DMUB halt\n");
+ ret = -EIO;
+ } else if (!pending) {
+ drm_err(adev_to_drm(adev),
+ "PSR exit timeout did not schedule recovery\n");
+ ret = -EIO;
+ } else {
+ drm_info(adev_to_drm(adev),
+ "PSR exit timeout scheduled recovery\n");
+ }
+
+unlock:
+ mutex_unlock(&adev->dm.dc_lock);
+
+ return ret;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(halt_dmub_for_psr_recovery_fops, NULL,
+ halt_dmub_for_psr_recovery_set, "%llu\n");
+
/*
* Dumps the DCC_EN bit for each pipe.
* Example usage: cat /sys/kernel/debug/dri/0/amdgpu_dm_dcc_en
@@ -4579,6 +4750,12 @@ void dtn_debugfs_init(struct amdgpu_device *adev)
debugfs_create_file_unsafe("amdgpu_dm_skip_detection_link_training", 0644, root, adev,
&skip_detection_link_training_fops);
+ debugfs_create_file_unsafe("amdgpu_dm_trigger_psr_recovery", 0200,
+ root, adev, &trigger_psr_recovery_fops);
+ debugfs_create_file_unsafe("amdgpu_dm_halt_dmub_for_psr_recovery", 0200,
+ root, adev,
+ &halt_dmub_for_psr_recovery_fops);
+
debugfs_create_file_unsafe("amdgpu_dm_dmub_tracebuffer", 0644, root,
adev, &dmub_tracebuffer_fops);
--
2.54.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-08-06 6:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05 11:52 [RFC PATCH 0/5] drm/amd/display: recover from PSR-related display stalls David Weber
2026-08-05 11:52 ` [RFC PATCH 1/5] drm/amd/display: avoid nested retries when polling PSR state David Weber
2026-08-05 11:52 ` [RFC PATCH 2/5] drm/amd/display: retry unconfirmed PSR transitions David Weber
2026-08-05 11:52 ` [RFC PATCH 3/5] drm/amd/display: invalidate PSR request cache after DMUB reset David Weber
2026-08-05 11:52 ` [RFC PATCH 4/5] drm/amd/display: recover from fatal PSR-related display timeouts David Weber
2026-08-05 11:52 ` [RFC PATCH 5/5] drm/amd/display: add PSR recovery fault injection David Weber
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.