On 10/31/2026 9:00 PM, Hugo Sales wrote:
> Writing panel_power_savings while the eDP is in PSR hangs DMUB on a
> Cezanne laptop (DCN 2.1)

Hi Hugo,

Thanks for the patch. We've seen the same DMCUB hang on Renoir
(DCN 2.1) when an ABM level is sent with PSR still active, so taking
the panel out of self-refresh before the update is needed.

> 
> An abm_level change on its own is a fast update, so the commit skips
> amdgpu_dm_commit_streams() and the self-refresh pause in it. The ABM
> level then reaches DMUB through amdgpu_dm_commit_planes(), or through the
> connector update loop in amdgpu_dm_atomic_commit_tail(), with the panel
> still in self-refresh
> 
> Raise the PSR and Replay hw_programming events before both updates. As
> with a backlight change, the events stay set until the next plane commit
> clears them in amdgpu_dm_enable_self_refresh()

The problem is leaving them set. amdgpu_dm_enable_self_refresh() only
clears hw_programming on a later fast update. A panel_power_savings
write on an idle desktop isn't followed by any commit, so PSR stays off
until the compositor commits for an unrelated reason. That costs PSR
residency on every ASIC that uses these events, not only DCN 2.1.

I reproduced this on Strix (DCN 3.5) with this patch and 177461: after
a write on an idle desktop, PSR didn't come back until an unrelated
commit. With the events cleared right after the DC update, PSR came
back by itself.
Could you clear the events right after the update, still under
dc_lock? Something like:
        update_planes_and_stream_adapter(dm->dc, ...);
        if (bundle->stream_update.abm_level) {
                amdgpu_dm_psr_set_event(dm, acrtc_state->stream, false,
                        psr_event_hw_programming, false);
                amdgpu_dm_replay_set_event(dm, acrtc_state->stream, false,
                        replay_event_hw_programming, false);
        }
and the same after dc_update_planes_and_stream() in the connector loop
of amdgpu_dm_atomic_commit_tail() when abm_changed.

Since only DCN 2.1 shows the hang, could you rerun your steps on the
GV301QH with that change? The case I'd most like confirmed is PSR
re-entering while the ABM ramp is still running, for example 0 to 4
and then leaving the screen idle.

Also, 177461 2/5 stops forcing the modeset for property changes, which
sends those down the same fast path. This patch needs to land before
2/5, so it would help to say so in 2/5, or to carry both in one series.

Regards,
Chenyu

> 
> Fixes: 8c322309e48e ("drm/amd/display: Enable PSR")
> Signed-off-by: Hugo Sales <[email protected]>
> ---
> Tested on an ASUS ROG Flow X13 (GV301QH, Cezanne, DCN 2.1, PSR1 eDP)
> with PSR turned on through amdgpu.dcfeaturemask=0xa and "drm/amd/display:
> Don't require DMUB PSR caps before DCN 3.0" [1], which DCN 2.1 needs for
> PSR to come up at all, but, as far as I can tell, the same bug would
> manifest on other DMUB versions that use PSR and ABM
> 
> On 7.3 without this patch, writing 3 to panel_power_savings while
> psr_state reads 6 makes DMUB fail within a second. The log fills with
> "DMCUB error - collecting diagnostic data" and the panel shows vertical
> lines. amdgpu doesn't attempt a GPU reset, and the errors go on until a
> reboot
> 
> With the fix, I let psr_state reach 6 before each of these steps:
> 
> - turn ABM on, from level 0 to 3
> - change the level, from 4 to 1
> - turn ABM off, from 1 to 0, which takes the immediate disable path
> - switch power-profiles-daemon to balanced and back to power-saver,
>   which sets a new level each time
> 
> None of them logged a DMCUB error. Both kernels had [2] applied as well
> 
> Commit 8c322309e48e ("drm/amd/display: Enable PSR") only paused PSR for
> full updates. The power module events used here came with commit
> 3c108046e1d6 ("drm/amd/display: Add power module on Linux"), so a
> backport before that needs another way to leave PSR
> 
> An HDR static metadata change while HDR stays on is a fast update too,
> and the connector loop sends the new infopacket with the panel in
> self-refresh. I have no panel with HDR and PSR or Replay to try it on,
> and the VRR and AVI infopackets already go out the same way. Does an
> infoframe update in self-refresh need the same pause?
> 
> [1] https://lore.kernel.org/r/[email protected]
> [2] https://lore.kernel.org/r/[email protected]
> 
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> 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 dfbc384fbbdb4..0b40e860734a0 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4224,6 +4224,19 @@ static void amdgpu_dm_commit_planes(struct 
> drm_atomic_commit *state,
>                       amdgpu_dm_replay_set_event(dm, acrtc_state->stream, 
> true,
>                               replay_event_immediate_flip, true);
>               }
> +             /*
> +              * An ABM change alone is a fast update, which skips the
> +              * self-refresh pause in amdgpu_dm_commit_streams(). As with a
> +              * backlight change, amdgpu_dm_enable_self_refresh() clears the
> +              * event
> +              */
> +             if (bundle->stream_update.abm_level) {
> +                     dc_exit_ips_for_hw_access(dm->dc);
> +                     amdgpu_dm_psr_set_event(dm, acrtc_state->stream, true,
> +                             psr_event_hw_programming, true);
> +                     amdgpu_dm_replay_set_event(dm, acrtc_state->stream, 
> true,
> +                             replay_event_hw_programming, true);
> +             }
>               update_planes_and_stream_adapter(dm->dc,
>                                        planes_count,
>                                        acrtc_state->stream,
> @@ -5280,6 +5293,12 @@ static void amdgpu_dm_atomic_commit_tail(struct 
> drm_atomic_commit *state)
>  
>               mutex_lock(&dm->dc_lock);
>               dc_exit_ips_for_hw_access(dm->dc);
> +             if (abm_changed) {
> +                     amdgpu_dm_psr_set_event(dm, dm_new_crtc_state->stream, 
> true,
> +                             psr_event_hw_programming, true);
> +                     amdgpu_dm_replay_set_event(dm, 
> dm_new_crtc_state->stream, true,
> +                             replay_event_hw_programming, true);
> +             }
>               dc_update_planes_and_stream(dm->dc,
>                                           dummy_updates,
>                                           status->plane_count,

Reply via email to