AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amd/display: Fix stale replay_events after mod_power stream removal
@ 2026-09-17 10:42 Simon Polack
  2026-09-24 17:25 ` Leo Li
  0 siblings, 1 reply; 4+ messages in thread
From: Simon Polack @ 2026-09-17 10:42 UTC (permalink / raw)
  To: Harry Wentland, Leo Li
  Cc: Alex Deucher, Christian König, Rodrigo Siqueira, Ray Wu,
	amd-gfx, dri-devel

[Why]
mod_power_remove_stream() shifts the remaining power_entity slots down
but does not move replay_events, and mod_power_add_stream() does not
initialize it. replay_events therefore stay bound to the map slot
instead of the stream.

When several streams are disabled in one atomic commit,
amdgpu_dm_mod_power_update_streams() removes them one after another.
The eDP stream can then be looked up in a slot whose stale
replay_events already have replay_event_hw_programming set, so
amdgpu_dm_replay_set_event() returns early ("already in desired state")
without calling mod_power_set_replay_event(). Replay is not disabled
before the eDP panel is powered off. After DPMS on, the sink reports
neither replay state nor frame lock (DPCD 0x378 = 0x00, no error bits),
so the HPD IRQ recovery does not trigger and the panel stays black
until a full modeset.

Seen with an eDP panel using FreeSync Replay plus two DP-MST displays:
DPMS off/on of all outputs leaves eDP black, while DPMS of eDP alone
works. Doing an eDP-only DPMS first makes the next all-output DPMS
fail reliably.

[How]
Shift replay_events together with the PSR cached fields in
mod_power_remove_stream() and initialize it to replay_event_vsync in
mod_power_add_stream(), matching the psr_event_vsync initial value used
for PSR (both vsync events are driven together by
amdgpu_dm_crtc_set_static_screen_optimze()).

Tested on 7.3.0-rc3 (238650ef6c7c): the reproducer above now recovers
reliably, and Replay still engages when the screen is idle.

The issue was debugged with help from an AI assistant (Claude), which
analysed ftrace/kprobe traces and the driver source, pointed to the
missing replay_events handling and suggested this change. I collected
the traces and built and tested the fix on the affected hardware.

Fixes: 4cef2ac4c795 ("drm/amd/display: Introduce power module on Linux")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Simon Polack <spolack+git@mailbox.org>
---
Based on v7.3-rc3 (238650ef6c7c), where it was tested; also applies to
amd-staging-drm-next.

This does not address why stale replay_events can reach the eDP slot
with replay_event_hw_programming already set in the first place; with
per-stream bookkeeping that state is consistent again. Traces
(kprobes/fprobes on the Replay path, eDP-only vs. all-output DPMS) and
DPCD dumps are available on request.

 drivers/gpu/drm/amd/display/modules/power/power.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/modules/power/power.c b/drivers/gpu/drm/amd/display/modules/power/power.c
index 2f9690e65ca9..d900be8cb5fc 100644
--- a/drivers/gpu/drm/amd/display/modules/power/power.c
+++ b/drivers/gpu/drm/amd/display/modules/power/power.c
@@ -328,6 +328,7 @@ bool mod_power_add_stream(struct mod_power *mod_power,
 		core_power->map[core_power->num_entities].psr_enabled = 0;
 		core_power->map[core_power->num_entities].psr_events = psr_event_vsync;
 		core_power->map[core_power->num_entities].psr_power_opt = 0;
+		core_power->map[core_power->num_entities].replay_events = replay_event_vsync;
 		core_power->num_entities++;
 		return true;
 	}
@@ -387,6 +388,7 @@ bool mod_power_remove_stream(struct mod_power *mod_power,
 		core_power->map[i].psr_enabled = core_power->map[i + 1].psr_enabled;
 		core_power->map[i].psr_events = core_power->map[i + 1].psr_events;
 		core_power->map[i].psr_power_opt = core_power->map[i + 1].psr_power_opt;
+		core_power->map[i].replay_events = core_power->map[i + 1].replay_events;
 
 		memcpy(core_power->map[i].psr_context, core_power->map[i + 1].psr_context, sizeof(struct mod_power_psr_context));
 		memset(core_power->map[i + 1].psr_context, 0, sizeof(struct mod_power_psr_context));

base-commit: 238650ef6c7c7cca08e032527329424c9fbd70e5
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/amd/display: Fix stale replay_events after mod_power stream removal
  2026-09-17 10:42 [PATCH] drm/amd/display: Fix stale replay_events after mod_power stream removal Simon Polack
@ 2026-09-24 17:25 ` Leo Li
  2026-10-01 11:12   ` Simon Polack
  0 siblings, 1 reply; 4+ messages in thread
From: Leo Li @ 2026-09-24 17:25 UTC (permalink / raw)
  To: Simon Polack, Harry Wentland
  Cc: Alex Deucher, Christian König, Rodrigo Siqueira, Ray Wu,
	amd-gfx, dri-devel



On 2026-09-17 06:42, Simon Polack wrote:
> [Some people who received this message don't often get email from spolack+git@mailbox.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> [Why]
> mod_power_remove_stream() shifts the remaining power_entity slots down
> but does not move replay_events, and mod_power_add_stream() does not
> initialize it. replay_events therefore stay bound to the map slot
> instead of the stream.
> 
> When several streams are disabled in one atomic commit,
> amdgpu_dm_mod_power_update_streams() removes them one after another.
> The eDP stream can then be looked up in a slot whose stale
> replay_events already have replay_event_hw_programming set, so
> amdgpu_dm_replay_set_event() returns early ("already in desired state")
> without calling mod_power_set_replay_event(). Replay is not disabled
> before the eDP panel is powered off. After DPMS on, the sink reports
> neither replay state nor frame lock (DPCD 0x378 = 0x00, no error bits),
> so the HPD IRQ recovery does not trigger and the panel stays black
> until a full modeset.
> 
> Seen with an eDP panel using FreeSync Replay plus two DP-MST displays:
> DPMS off/on of all outputs leaves eDP black, while DPMS of eDP alone
> works. Doing an eDP-only DPMS first makes the next all-output DPMS
> fail reliably.
> 
> [How]
> Shift replay_events together with the PSR cached fields in
> mod_power_remove_stream() and initialize it to replay_event_vsync in
> mod_power_add_stream(), matching the psr_event_vsync initial value used
> for PSR (both vsync events are driven together by
> amdgpu_dm_crtc_set_static_screen_optimze()).
> 
> Tested on 7.3.0-rc3 (238650ef6c7c): the reproducer above now recovers
> reliably, and Replay still engages when the screen is idle.
> 
> The issue was debugged with help from an AI assistant (Claude), which
> analysed ftrace/kprobe traces and the driver source, pointed to the
> missing replay_events handling and suggested this change. I collected
> the traces and built and tested the fix on the affected hardware.
> 
> Fixes: 4cef2ac4c795 ("drm/amd/display: Introduce power module on Linux")
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Simon Polack <spolack+git@mailbox.org>

Reviewed-by: Leo Li <sunpeng.li@amd.com>

Thanks!
> ---
> Based on v7.3-rc3 (238650ef6c7c), where it was tested; also applies to
> amd-staging-drm-next.
> 
> This does not address why stale replay_events can reach the eDP slot
> with replay_event_hw_programming already set in the first place; with
> per-stream bookkeeping that state is consistent again. Traces
> (kprobes/fprobes on the Replay path, eDP-only vs. all-output DPMS) and
> DPCD dumps are available on request.
> 
>  drivers/gpu/drm/amd/display/modules/power/power.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/modules/power/power.c b/drivers/gpu/drm/amd/display/modules/power/power.c
> index 2f9690e65ca9..d900be8cb5fc 100644
> --- a/drivers/gpu/drm/amd/display/modules/power/power.c
> +++ b/drivers/gpu/drm/amd/display/modules/power/power.c
> @@ -328,6 +328,7 @@ bool mod_power_add_stream(struct mod_power *mod_power,
>                 core_power->map[core_power->num_entities].psr_enabled = 0;
>                 core_power->map[core_power->num_entities].psr_events = psr_event_vsync;
>                 core_power->map[core_power->num_entities].psr_power_opt = 0;
> +               core_power->map[core_power->num_entities].replay_events = replay_event_vsync;
>                 core_power->num_entities++;
>                 return true;
>         }
> @@ -387,6 +388,7 @@ bool mod_power_remove_stream(struct mod_power *mod_power,
>                 core_power->map[i].psr_enabled = core_power->map[i + 1].psr_enabled;
>                 core_power->map[i].psr_events = core_power->map[i + 1].psr_events;
>                 core_power->map[i].psr_power_opt = core_power->map[i + 1].psr_power_opt;
> +               core_power->map[i].replay_events = core_power->map[i + 1].replay_events;
> 
>                 memcpy(core_power->map[i].psr_context, core_power->map[i + 1].psr_context, sizeof(struct mod_power_psr_context));
>                 memset(core_power->map[i + 1].psr_context, 0, sizeof(struct mod_power_psr_context));
> 
> base-commit: 238650ef6c7c7cca08e032527329424c9fbd70e5
> --
> 2.55.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/amd/display: Fix stale replay_events after mod_power stream removal
  2026-09-24 17:25 ` Leo Li
@ 2026-10-01 11:12   ` Simon Polack
  2026-10-01 21:25     ` Alex Deucher
  0 siblings, 1 reply; 4+ messages in thread
From: Simon Polack @ 2026-10-01 11:12 UTC (permalink / raw)
  To: Leo Li, Harry Wentland
  Cc: Alex Deucher, Christian König, Rodrigo Siqueira, Ray Wu,
	amd-gfx, dri-devel

Thanks for the review.

Gentle ping on this one - I don't see it in any of the amdgpu trees yet.

Since the bug was introduced in 7.2 and leaves the internal panel black
after a DPMS cycle on multi-display setups, would it make sense to take
this through the fixes branch for 7.3 rather than waiting for the next
merge window?

Thanks,
Simon

Am 24.09.26 um 19:25 schrieb Leo Li:
> Reviewed-by: Leo Li<sunpeng.li@amd.com>
> 
> Thanks!


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/amd/display: Fix stale replay_events after mod_power stream removal
  2026-10-01 11:12   ` Simon Polack
@ 2026-10-01 21:25     ` Alex Deucher
  0 siblings, 0 replies; 4+ messages in thread
From: Alex Deucher @ 2026-10-01 21:25 UTC (permalink / raw)
  To: Simon Polack
  Cc: Leo Li, Harry Wentland, Alex Deucher, Christian König,
	Rodrigo Siqueira, Ray Wu, amd-gfx, dri-devel

On Thu, Oct 1, 2026 at 3:36 PM Simon Polack <spolack+git@mailbox.org> wrote:
>
> Thanks for the review.
>
> Gentle ping on this one - I don't see it in any of the amdgpu trees yet.
>
> Since the bug was introduced in 7.2 and leaves the internal panel black
> after a DPMS cycle on multi-display setups, would it make sense to take
> this through the fixes branch for 7.3 rather than waiting for the next
> merge window?

It's going to -fixes and -next this week.

Alex

>
> Thanks,
> Simon
>
> Am 24.09.26 um 19:25 schrieb Leo Li:
> > Reviewed-by: Leo Li<sunpeng.li@amd.com>
> >
> > Thanks!
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-01 21:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-17 10:42 [PATCH] drm/amd/display: Fix stale replay_events after mod_power stream removal Simon Polack
2026-09-24 17:25 ` Leo Li
2026-10-01 11:12   ` Simon Polack
2026-10-01 21:25     ` Alex Deucher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox