drm/amdgpu: Fix desktop freezed after gpu-reset
authorAlan Liu <HaoPing.Liu@amd.com>
Fri, 14 Apr 2023 10:39:52 +0000 (18:39 +0800)
committerAlex Deucher <alexander.deucher@amd.com>
Tue, 18 Apr 2023 21:14:13 +0000 (17:14 -0400)
[Why]
After gpu-reset, sometimes the driver fails to enable vblank irq,
causing flip_done timed out and the desktop freezed.

During gpu-reset, we disable and enable vblank irq in dm_suspend() and
dm_resume(). Later on in amdgpu_irq_gpu_reset_resume_helper(), we check
irqs' refcount and decide to enable or disable the irqs again.

However, we have 2 sets of API for controling vblank irq, one is
dm_vblank_get/put() and another is amdgpu_irq_get/put(). Each API has
its own refcount and flag to store the state of vblank irq, and they
are not synchronized.

In drm we use the first API to control vblank irq but in
amdgpu_irq_gpu_reset_resume_helper() we use the second set of API.

The failure happens when vblank irq was enabled by dm_vblank_get()
before gpu-reset, we have vblank->enabled true. However, during
gpu-reset, in amdgpu_irq_gpu_reset_resume_helper() vblank irq's state
checked from amdgpu_irq_update() is DISABLED. So finally it disables
vblank irq again. After gpu-reset, if there is a cursor plane commit,
the driver will try to enable vblank irq by calling drm_vblank_enable(),
but the vblank->enabled is still true, so it fails to turn on vblank
irq and causes flip_done can't be completed in vblank irq handler and
desktop become freezed.

[How]
Combining the 2 vblank control APIs by letting drm's API finally calls
amdgpu_irq's API, so the irq's refcount and state of both APIs can be
synchronized. Also add a check to prevent refcount from being less then
0 in amdgpu_irq_put().

v2:
- Add warning in amdgpu_irq_enable() if the irq is already disabled.
- Call dc_interrupt_set() in dm_set_vblank() to avoid refcount change
  if it is in gpu-reset.

v3:
- Improve commit message and code comments.

Signed-off-by: Alan Liu <HaoPing.Liu@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c

index d0a1cc88832cc4bd3c53ddf6f5f1127d491bcb2a..fafebec5b7b66da8911ffb2f112817a1f1f626ae 100644 (file)
@@ -596,6 +596,9 @@ int amdgpu_irq_put(struct amdgpu_device *adev, struct amdgpu_irq_src *src,
        if (!src->enabled_types || !src->funcs->set)
                return -EINVAL;
 
+       if (WARN_ON(!amdgpu_irq_enabled(adev, src, type)))
+               return -EINVAL;
+
        if (atomic_dec_and_test(&src->enabled_types[type]))
                return amdgpu_irq_update(adev, src, type);
 
index dc4f37240beb4c3a61bfd77386ac548c14be0ebe..8af70feca7205107f03aa1215de09152830210e5 100644 (file)
@@ -169,10 +169,21 @@ static inline int dm_set_vblank(struct drm_crtc *crtc, bool enable)
        if (rc)
                return rc;
 
-       irq_source = IRQ_TYPE_VBLANK + acrtc->otg_inst;
+       if (amdgpu_in_reset(adev)) {
+               irq_source = IRQ_TYPE_VBLANK + acrtc->otg_inst;
+               /* During gpu-reset we disable and then enable vblank irq, so
+                * don't use amdgpu_irq_get/put() to avoid refcount change.
+                */
+               if (!dc_interrupt_set(adev->dm.dc, irq_source, enable))
+                       rc = -EBUSY;
+       } else {
+               rc = (enable)
+                       ? amdgpu_irq_get(adev, &adev->crtc_irq, acrtc->crtc_id)
+                       : amdgpu_irq_put(adev, &adev->crtc_irq, acrtc->crtc_id);
+       }
 
-       if (!dc_interrupt_set(adev->dm.dc, irq_source, enable))
-               return -EBUSY;
+       if (rc)
+               return rc;
 
 skip:
        if (amdgpu_in_reset(adev))