Re: [PATCH] drm/amd/display: Exit idle optimizations before programming

On 7/28/26 12:02, [email protected] wrote:
> From: Leo Li <[email protected]>
> 
> [Why]
> 
> We need to exit PSR/IPS before programming. Before calling DC for
> programming in amdgpu_dm_commit_planes(), there's a
> vblank_control_workqueue flush. This waits for IPS and PSR exit. (See
> drm_vblank_on/off() > amdgpu_dm_crtc_set_vblank() --queue_work()->
> amdgpu_dm_crtc_vblank_control_worker())
> 
> Prior to the tagged "Fixes:" change, drm_vblank_get() was called before
> the workqueue flush. This ordering ensures that PSR exit occurred before
> programming. After the "Fixes:" change, drm_vblank_get() is called after
> the workqueue flush, leading to programming while idle optimizations are
> still active. This can lead to incorrect flip_pending detection used by
> vblank event delivery.
> 
> [How]
> 
> Split the vblank_get() component of `dm_arm_vblank_event()` into
> `dm_arm_vblank_event_pre_programming()`, which is called before
> programming. Call it before the vblank_control_workqueue flush.
> 
> Includes a drive-by cleanup of prepare_flip_isr(): the only caller is
> dm_arm_vblank_event() and it's simple enough to roll-in.
> 
> Signed-off-by: Leo Li <[email protected]>
> Fixes: 48ab86360af1 ("drm/amd/display: check GRPH_FLIP status before sending event")
> Cc: [email protected]
> Link: https://gitlab.freedesktop.org/drm/amd/-/work_items/4141#note_3583205
> Link: https://gitlab.freedesktop.org/drm/amd/-/work_items/5527
> Assisted-by: Codex:gpt-5.6-sol
> Assisted-by: Claude:opus-5
Reviewed-by: Mario Limonciello (AMD) <[email protected]>
> ---
> Rebased patch for applying on 7.2-rc5:
> https://gitlab.freedesktop.org/leoli/linux-asdn/-/commit/f05db644126540f1d237f846cc0512088551eb73
> 
> Thanks,
> Leo
> 
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 79 +++++++++++--------
>   1 file changed, 47 insertions(+), 32 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 df6b59fb8d0d4..784587697bac8 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -3576,25 +3576,6 @@ static void remove_stream(struct amdgpu_device *adev,
>   	acrtc->enabled = false;
>   }
>   
> -static void prepare_flip_isr(struct amdgpu_crtc *acrtc)
> -{
> -
> -	assert_spin_locked(&acrtc->base.dev->event_lock);
> -	WARN_ON(acrtc->event);
> -
> -	acrtc->event = acrtc->base.state->event;
> -
> -	/* Set the flip status */
> -	acrtc->pflip_status = AMDGPU_FLIP_SUBMITTED;
> -
> -	/* Mark this event as consumed */
> -	acrtc->base.state->event = NULL;
> -
> -	drm_dbg_state(acrtc->base.dev,
> -		      "crtc:%d, pflip_stat:AMDGPU_FLIP_SUBMITTED\n",
> -		      acrtc->crtc_id);
> -}
> -
>   static void amdgpu_dm_commit_cursors(struct drm_atomic_commit *state)
>   {
>   	struct drm_plane *plane;
> @@ -3740,17 +3721,48 @@ static void dm_arm_vblank_event(struct amdgpu_crtc *acrtc,
>   		return;
>   
>   	if (pflip_update) {
> -		drm_crtc_vblank_get(&acrtc->base);
>   		WARN_ON(acrtc->pflip_status != AMDGPU_FLIP_NONE);
> -		/* Arm flip completion handling and event delivery after programming. */
> -		prepare_flip_isr(acrtc);
> +		WARN_ON(acrtc->event);
> +
> +		acrtc->pflip_status = AMDGPU_FLIP_SUBMITTED;
> +		acrtc->event = acrtc->base.state->event;
> +		acrtc->base.state->event = NULL;
> +
> +		drm_dbg_state(acrtc->base.dev,
> +			      "crtc:%d, pflip_stat:AMDGPU_FLIP_SUBMITTED\n",
> +			      acrtc->crtc_id);
>   	} else if (cursor_update) {
> -		drm_crtc_vblank_get(&acrtc->base);
>   		acrtc->event = acrtc->base.state->event;
>   		acrtc->base.state->event = NULL;
>   	}
>   }
>   
> +/**
> + * dm_arm_vblank_event_pre_programming - Prepare for programming
> + * @acrtc: The amdgpu CRTC to prepare
> + * @acrtc_state: The new CRTC state
> + * @pflip_update: Whether a page flip is being programmed
> + * @cursor_update: Whether a cursor update is being programmed
> + *
> + * Grab a reference on the vblank counter if a page flip or cursor update is to
> + * be programmed. Do this before programming so the HW is not in any
> + * idle-optimized state (such as PSR).
> + */
> +static void dm_arm_vblank_event_pre_programming(
> +	struct amdgpu_crtc *acrtc,
> +	struct dm_crtc_state *acrtc_state,
> +	bool pflip_update,
> +	bool cursor_update)
> +{
> +	assert_spin_locked(&acrtc->base.dev->event_lock);
> +
> +	if (!acrtc->base.state->event || acrtc_state->active_planes == 0)
> +		return;
> +
> +	if (pflip_update || cursor_update)
> +		drm_crtc_vblank_get(&acrtc->base);
> +}
> +
>   static void amdgpu_dm_commit_planes(struct drm_atomic_commit *state,
>   				    struct drm_device *dev,
>   				    struct amdgpu_display_manager *dm,
> @@ -4013,16 +4025,19 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_commit *state,
>   		}
>   	}
>   
> -	/*
> -	 * DCE depends on a combination of GRPH_FLIP, VLINE0, and VUPDATE for
> -	 * event delivery. Only GRPH_FLIP handler can send pflip events, and it
> -	 * only fires if HW latched to the flip. Maintain legacy behavior by
> -	 * arming event before programming.
> -	 */
> -	if (amdgpu_ip_version(dm->adev, DCE_HWIP, 0) == 0) {
> -		scoped_guard(spinlock_irqsave, &pcrtc->dev->event_lock) {
> +	scoped_guard(spinlock_irqsave, &pcrtc->dev->event_lock) {
> +		dm_arm_vblank_event_pre_programming(acrtc_attach, acrtc_state,
> +						    pflip_present,
> +						    cursor_update);
> +		/*
> +		 * DCE depends on a combination of GRPH_FLIP, VLINE0, and
> +		 * VUPDATE for event delivery. Only GRPH_FLIP handler can send
> +		 * pflip events, and it only fires if HW latched to the flip.
> +		 * Maintain legacy behavior by arming event before programming.
> +		 */
> +		if (amdgpu_ip_version(dm->adev, DCE_HWIP, 0) == 0) {
>   			dm_arm_vblank_event(acrtc_attach, acrtc_state,
> -					pflip_present, cursor_update);
> +					    pflip_present, cursor_update);
>   		}
>   	}
>