Re: [PATCH 1/3] drm/amd/display: consolidate DCN vblank/flip handling onto vupdate_no_lock

[Matthew Schwartz]

On 6/16/26 1:18 PM, sunpeng...@amd.com wrote:

[...]

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> index c9916ed0ddc14..8a6b732cf80c8 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> @@ -281,7 +281,14 @@ static inline int amdgpu_dm_crtc_set_vblank(struct 
> drm_crtc *crtc, bool enable)
>                       drm_crtc_vblank_restore(crtc);
>       }
>  
> -     if (dc_supports_vrr(dm->dc->ctx->dce_version)) {
> +     /*
> +      * On DCN, VUPDATE_NO_LOCK is the single OTG interrupt used to deliver
> +      * vblank and pageflip completion events, so enable it whenever vblank
> +      * is enabled. On DCE, vupdate is only needed in VRR mode.
> +      */
> +     if (amdgpu_ip_version(adev, DCE_HWIP, 0) != 0) {
> +             rc = amdgpu_dm_crtc_set_vupdate_irq(crtc, enable);
> +     } else if (dc_supports_vrr(dm->dc->ctx->dce_version)) {

Hi Leo (dropping Stable),

This arms VUPDATE_NO_LOCK on all of DCN, but amdgpu_dm_handle_vrr_transition() 
still disables it when switching from VRR active -> inactive. This causes a new 
flip_done timed out event when a compositor like gamescope disables VRR while 
it's already active.

I had to fix it up with something like:

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 e09c0eb9f865..ee337ca816cf 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -10085,8 +10085,16 @@ static void amdgpu_dm_handle_vrr_transition(struct 
amdgpu_display_manager *dm,
                                            struct dm_crtc_state *old_state,
                                            struct dm_crtc_state *new_state)
 {
+       struct amdgpu_device *adev = dm->adev;
        bool old_vrr_active = amdgpu_dm_crtc_vrr_active(old_state);
        bool new_vrr_active = amdgpu_dm_crtc_vrr_active(new_state);
+       /*
+        * On DCN, VUPDATE_NO_LOCK is the sole vblank and pageflip completion
+        * source and amdgpu_dm_crtc_set_vblank() keeps it armed whenever
+        * vblank is enabled, so it must not be toggled with the VRR state.
+        * Only DCE gates vupdate on VRR.
+        */
+       bool vrr_gates_vupdate = amdgpu_ip_version(adev, DCE_HWIP, 0) == 0;
 
        if (!old_vrr_active && new_vrr_active) {
                /* Transition VRR inactive -> active:
@@ -10097,7 +10105,8 @@ static void amdgpu_dm_handle_vrr_transition(struct 
amdgpu_display_manager *dm,
                 * We also need vupdate irq for the actual core vblank handling
                 * at end of vblank.
                 */
-               WARN_ON(amdgpu_dm_crtc_set_vupdate_irq(new_state->base.crtc, 
true) != 0);
+               if (vrr_gates_vupdate)
+                       
WARN_ON(amdgpu_dm_crtc_set_vupdate_irq(new_state->base.crtc, true) != 0);
                WARN_ON(drm_crtc_vblank_get(new_state->base.crtc) != 0);
                drm_dbg_driver(new_state->base.crtc->dev, "%s: crtc=%u VRR 
off->on: Get vblank ref\n",
                                 __func__, new_state->base.crtc->base.id);
@@ -10113,7 +10122,8 @@ static void amdgpu_dm_handle_vrr_transition(struct 
amdgpu_display_manager *dm,
                /* Transition VRR active -> inactive:
                 * Allow vblank irq disable again for fixed refresh rate.
                 */
-               WARN_ON(amdgpu_dm_crtc_set_vupdate_irq(new_state->base.crtc, 
false) != 0);
+               if (vrr_gates_vupdate)
+                       
WARN_ON(amdgpu_dm_crtc_set_vupdate_irq(new_state->base.crtc, false) != 0);
                drm_crtc_vblank_put(new_state->base.crtc);
                drm_dbg_driver(new_state->base.crtc->dev, "%s: crtc=%u VRR 
on->off: Drop vblank ref\n",
                                 __func__, new_state->base.crtc->base.id);

Feel free to fold some version of this into the next version if it makes sense, 
or pick it up with my s-o-b.

Thanks,
Matthew