drm/amd/display: Fix vblank refcount in vrr transition
authorYunxiang Li <Yunxiang.Li@amd.com>
Wed, 21 Sep 2022 21:20:19 +0000 (17:20 -0400)
committerAlex Deucher <alexander.deucher@amd.com>
Thu, 6 Oct 2022 16:06:25 +0000 (12:06 -0400)
manage_dm_interrupts disable/enable vblank using drm_crtc_vblank_off/on
which causes drm_crtc_vblank_get in vrr_transition to fail, and later
when drm_crtc_vblank_put is called the refcount on vblank will be messed
up. Therefore move the call to after manage_dm_interrupts.

Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1247
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1380

Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Signed-off-by: Yunxiang Li <Yunxiang.Li@amd.com>
Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c

index d5222d5..b84aedb 100644 (file)
@@ -7479,15 +7479,15 @@ static void amdgpu_dm_handle_vrr_transition(struct dm_crtc_state *old_state,
                 * We also need vupdate irq for the actual core vblank handling
                 * at end of vblank.
                 */
-               dm_set_vupdate_irq(new_state->base.crtc, true);
-               drm_crtc_vblank_get(new_state->base.crtc);
+               WARN_ON(dm_set_vupdate_irq(new_state->base.crtc, true) != 0);
+               WARN_ON(drm_crtc_vblank_get(new_state->base.crtc) != 0);
                DRM_DEBUG_DRIVER("%s: crtc=%u VRR off->on: Get vblank ref\n",
                                 __func__, new_state->base.crtc->base.id);
        } else if (old_vrr_active && !new_vrr_active) {
                /* Transition VRR active -> inactive:
                 * Allow vblank irq disable again for fixed refresh rate.
                 */
-               dm_set_vupdate_irq(new_state->base.crtc, false);
+               WARN_ON(dm_set_vupdate_irq(new_state->base.crtc, false) != 0);
                drm_crtc_vblank_put(new_state->base.crtc);
                DRM_DEBUG_DRIVER("%s: crtc=%u VRR on->off: Drop vblank ref\n",
                                 __func__, new_state->base.crtc->base.id);
@@ -8243,23 +8243,6 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
                mutex_unlock(&dm->dc_lock);
        }
 
-       /* Count number of newly disabled CRTCs for dropping PM refs later. */
-       for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
-                                     new_crtc_state, i) {
-               if (old_crtc_state->active && !new_crtc_state->active)
-                       crtc_disable_count++;
-
-               dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
-               dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
-
-               /* For freesync config update on crtc state and params for irq */
-               update_stream_irq_parameters(dm, dm_new_crtc_state);
-
-               /* Handle vrr on->off / off->on transitions */
-               amdgpu_dm_handle_vrr_transition(dm_old_crtc_state,
-                                               dm_new_crtc_state);
-       }
-
        /**
         * Enable interrupts for CRTCs that are newly enabled or went through
         * a modeset. It was intentionally deferred until after the front end
@@ -8269,16 +8252,29 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
        for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
                struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
 #ifdef CONFIG_DEBUG_FS
-               bool configure_crc = false;
                enum amdgpu_dm_pipe_crc_source cur_crc_src;
 #if defined(CONFIG_DRM_AMD_SECURE_DISPLAY)
-               struct crc_rd_work *crc_rd_wrk = dm->crc_rd_wrk;
+               struct crc_rd_work *crc_rd_wrk;
+#endif
+#endif
+               /* Count number of newly disabled CRTCs for dropping PM refs later. */
+               if (old_crtc_state->active && !new_crtc_state->active)
+                       crtc_disable_count++;
+
+               dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
+               dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
+
+               /* For freesync config update on crtc state and params for irq */
+               update_stream_irq_parameters(dm, dm_new_crtc_state);
+
+#ifdef CONFIG_DEBUG_FS
+#if defined(CONFIG_DRM_AMD_SECURE_DISPLAY)
+               crc_rd_wrk = dm->crc_rd_wrk;
 #endif
                spin_lock_irqsave(&adev_to_drm(adev)->event_lock, flags);
                cur_crc_src = acrtc->dm_irq_params.crc_src;
                spin_unlock_irqrestore(&adev_to_drm(adev)->event_lock, flags);
 #endif
-               dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
 
                if (new_crtc_state->active &&
                    (!old_crtc_state->active ||
@@ -8286,16 +8282,19 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
                        dc_stream_retain(dm_new_crtc_state->stream);
                        acrtc->dm_irq_params.stream = dm_new_crtc_state->stream;
                        manage_dm_interrupts(adev, acrtc, true);
+               }
+               /* Handle vrr on->off / off->on transitions */
+               amdgpu_dm_handle_vrr_transition(dm_old_crtc_state, dm_new_crtc_state);
 
 #ifdef CONFIG_DEBUG_FS
+               if (new_crtc_state->active &&
+                   (!old_crtc_state->active ||
+                    drm_atomic_crtc_needs_modeset(new_crtc_state))) {
                        /**
                         * Frontend may have changed so reapply the CRC capture
                         * settings for the stream.
                         */
-                       dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
-
                        if (amdgpu_dm_is_valid_crc_source(cur_crc_src)) {
-                               configure_crc = true;
 #if defined(CONFIG_DRM_AMD_SECURE_DISPLAY)
                                if (amdgpu_dm_crc_window_is_activated(crtc)) {
                                        spin_lock_irqsave(&adev_to_drm(adev)->event_lock, flags);
@@ -8307,12 +8306,10 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
                                        spin_unlock_irqrestore(&adev_to_drm(adev)->event_lock, flags);
                                }
 #endif
-                       }
-
-                       if (configure_crc)
                                if (amdgpu_dm_crtc_configure_crc_source(
                                        crtc, dm_new_crtc_state, cur_crc_src))
                                        DRM_DEBUG_DRIVER("Failed to configure crc source");
+                       }
 #endif
                }
        }