Re: [PATCH] drm/amd/display: Take every stream out of self-refresh before a modeset

[Ray Wu <ray.wu@amd.com>]

Reviewed-by: Ray Wu <ray.wu@amd.com>

On 10/30/2026 12:54 AM, Hugo Sales wrote:
> Commit 3c108046e1d6 ("drm/amd/display: Add power module on Linux")
> dropped amdgpu_dm_psr_disable_all() and amdgpu_dm_replay_disable_all()
> from amdgpu_dm_commit_streams(). Since then, a modeset only disables
> self-refresh on the CRTCs in the commit, so an eDP on another CRTC stays
> in PSR through dc_commit_streams()
>
> Commit 6ee90e88b976 ("drm/amd/display: PSR eDP p-state warning occurs
> intermittently after unplug DP") added that disable-all for this very
> case. With it gone, turning off or unplugging an HDMI monitor while the
> eDP is in PSR hangs DMUB on a Cezanne laptop (DCN 2.1), on the first
> try:
>
>    dc_dmub_srv_log_diagnostic_data: DMCUB error
>    Error queueing DMUB command: status=2
>
> The eDP then stays frozen until a GPU reset
>
> Do the same through the power module's events. When a CRTC that is or
> was active needs a modeset, raise the PSR and Replay pause events on
> every stream mod_power tracks and wait for self-refresh to exit, before
> amdgpu_dm_mod_power_update_streams() changes the stream set. Clear them
> on the commit's streams after amdgpu_dm_mod_power_setup_streams(), since
> enabling a stream clears psr_feature_enabled until then. Use
> psr_event_pause and replay_event_pause rather than the hw_programming
> events, which a fast update on the eDP can clear in
> amdgpu_dm_enable_self_refresh() while the modeset is still running
>
> Fixes: 3c108046e1d6 ("drm/amd/display: Add power module on Linux")
> Signed-off-by: Hugo Sales <hugo@hsal.es>
> ---
> Tested on an ASUS ROG Flow X13 (Cezanne, DMUB 0x0101002B) with PSR 1 on
> the eDP. That's the only hardware I have, and PSR there needs
> DC_PSR_MASK and "drm/amd/display: Don't require DMUB PSR caps before
> DCN 3.0", which I'm sending next. With this patch, 50 HDMI unplugs in a
> row with the eDP in PSR went through without a DMUB error. The same
> kernel without it hung on the first
>
> Ray, Leo: 3c108046e1d6 doesn't say whether leaving the other CRTCs in
> self-refresh was intended. If newer DMUB firmware copes with an external
> link going away during PSR, I can limit this to DCN 2.1
>
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 101 ++++++++++++++++++
>   1 file changed, 101 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..54eee9e2393fa 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4443,6 +4443,105 @@ STATIC_IFN_KUNIT void dm_clear_writeback(struct amdgpu_display_manager *dm,
>   }
>   EXPORT_IF_KUNIT(dm_clear_writeback);
>   
> +static bool amdgpu_dm_self_refresh_pause_needed(struct drm_atomic_commit *state)
> +{
> +	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> +	struct drm_crtc *crtc;
> +	int i;
> +
> +	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i)
> +		if (drm_atomic_crtc_needs_modeset(new_crtc_state) &&
> +		    (old_crtc_state->active || new_crtc_state->active))
> +			return true;
> +
> +	return false;
> +}
> +
> +static bool amdgpu_dm_is_new_modeset_stream(struct drm_atomic_commit *state,
> +					    const struct dc_stream_state *stream)
> +{
> +	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)
> +		if (drm_atomic_crtc_needs_modeset(new_crtc_state) &&
> +		    to_dm_crtc_state(new_crtc_state)->stream == stream)
> +			return true;
> +
> +	return false;
> +}
> +
> +/*
> + * A modeset CRTC may have no PSR or Replay, such as an HDMI output being
> + * turned off, while an eDP on another CRTC is still in self-refresh. Pause
> + * self-refresh on every stream mod_power tracks before
> + * amdgpu_dm_mod_power_update_streams() changes the set: the old streams of
> + * the CRTCs in the commit and the unchanged streams of the new state.
> + * Unlike hw_programming, which a fast update on another CRTC clears, the
> + * pause holds until amdgpu_dm_resume_self_refresh()
> + */
> +static void amdgpu_dm_pause_self_refresh(struct drm_atomic_commit *state,
> +					 struct amdgpu_display_manager *dm,
> +					 struct dc_state *dc_state)
> +{
> +	struct drm_crtc_state *old_crtc_state;
> +	struct dc_stream_state *stream;
> +	struct drm_crtc *crtc;
> +	int i;
> +
> +	if (!amdgpu_dm_self_refresh_pause_needed(state))
> +		return;
> +
> +	guard(mutex)(&dm->dc_lock);
> +	dc_exit_ips_for_hw_access(dm->dc);
> +
> +	for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
> +		stream = to_dm_crtc_state(old_crtc_state)->stream;
> +		if (!old_crtc_state->active || !stream)
> +			continue;
> +
> +		amdgpu_dm_psr_set_event(dm, stream, true, psr_event_pause, true);
> +		amdgpu_dm_replay_set_event(dm, stream, true, replay_event_pause, true);
> +	}
> +
> +	for (i = 0; i < dc_state->stream_count; i++) {
> +		stream = dc_state->streams[i];
> +		if (amdgpu_dm_is_new_modeset_stream(state, stream))
> +			continue;
> +
> +		amdgpu_dm_psr_set_event(dm, stream, true, psr_event_pause, true);
> +		amdgpu_dm_replay_set_event(dm, stream, true, replay_event_pause, true);
> +	}
> +}
> +
> +/*
> + * Ends amdgpu_dm_pause_self_refresh() on the streams of the commit, which
> + * mod_power tracks from amdgpu_dm_mod_power_update_streams() on, even if
> + * dc_commit_streams() failed. mod_power keeps events across
> + * mod_power_replace_stream(), so the replaced streams carry the pause too.
> + * Runs after amdgpu_dm_mod_power_setup_streams(), since enabling a stream
> + * clears psr_feature_enabled and replay_feature_enabled on its link until
> + * then, and the set_event helpers skip a link without them
> + */
> +static void amdgpu_dm_resume_self_refresh(struct drm_atomic_commit *state,
> +					  struct amdgpu_display_manager *dm,
> +					  struct dc_state *dc_state)
> +{
> +	int i;
> +
> +	if (!dc_state || !amdgpu_dm_self_refresh_pause_needed(state))
> +		return;
> +
> +	guard(mutex)(&dm->dc_lock);
> +	for (i = 0; i < dc_state->stream_count; i++) {
> +		amdgpu_dm_psr_set_event(dm, dc_state->streams[i], false,
> +					psr_event_pause, false);
> +		amdgpu_dm_replay_set_event(dm, dc_state->streams[i], false,
> +					   replay_event_pause, false);
> +	}
> +}
> +
>   /**
>    * amdgpu_dm_mod_power_update_streams - update mod_power stream state
>    * @state: the drm atomic state
> @@ -4627,6 +4726,7 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_commit *state,
>   		acrtc->wb_enabled = false;
>   	}
>   
> +	amdgpu_dm_pause_self_refresh(state, dm, dc_state);
>   	amdgpu_dm_mod_power_update_streams(state, dm);
>   
>   	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
> @@ -5370,6 +5470,7 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_commit *state)
>   	}
>   
>   	amdgpu_dm_mod_power_setup_streams(state, dm);
> +	amdgpu_dm_resume_self_refresh(state, dm, dc_state);
>   
>   	for_each_new_crtc_in_state(state, crtc, new_crtc_state, j)
>   		if (new_crtc_state->async_flip)
>
> base-commit: ba496bb837882eb1af58f41572fc289e46c77b8c