On 7/23/26 13:01, [email protected] wrote:
From: Leo Li <[email protected]>
The vblank on/off callbacks mixed use of amdgpu_irq_get/put() and
amdgpu_dm_crtc_set_vupdate_irq() to enable and disable IRQs.
With get/put, base driver will callback into DC to disable IRQs when
refcount == 0. With set_vupdate_irq(), DC is called directly to disable
IRQs, bypassing base driver's refcount tracking.
During gpu reset, base driver can restore IRQs via
amdgpu_irq_gpu_reset_resume_helper() > amdgpu_irq_update(). So if
get/put() is not used (i.e. refcount == 0), then vupdate_irq will be
disabled.
This is problematic if DRM requests vblank on before amdgpu_irq_update()
is called: drm_vblank_on() > set_vupdate_irq() enables vupdate_irq, but
the refcount is still 0. gpu_reset_resume_helper() > irq_update() then
immediately disables it, thus leading to flip done timeouts.
This is made worse on DCN since VUPDATE_NO_LOCK is the only IRQ enabled.
Prior to 8382cd234981, a combination of GRPH_FLIP and VSTARTUP IRQs were
used, and they used get/put(). This explains why 8382cd234981 exposed
this issue.
Fix by using get/put() instead of set_vupdate_irq(). DCE is unchanged,
since it relies on unbalanced enable/disable calls based on VRR status,
and hence requires direct set_vupdate_irq(). Plus, it also uses
GRPH_FLIP and VLINE IRQs, which are properly tracked by get/put().
Cc: [email protected]
Fixes: 8382cd234981 ("drm/amd/display: consolidate DCN vblank/flip handling onto
vupdate_no_lock")
Signed-off-by: Leo Li <[email protected]>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
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 05d6915f9a6b0..079d4ccc88da6 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
@@ -287,10 +287,19 @@ static inline int amdgpu_dm_crtc_set_vblank(struct
drm_crtc *crtc, bool enable)
* 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);
+ if (enable) {
+ rc = amdgpu_irq_get(adev, &adev->vupdate_irq, irq_type);
+ drm_dbg_vbl(crtc->dev, "Get vupdate_irq ret=%d\n", rc);
+ } else {
+ rc = amdgpu_irq_put(adev, &adev->vupdate_irq, irq_type);
+ drm_dbg_vbl(crtc->dev, "Put vupdate_irq ret=%d\n", rc);
+ }
For completeness in error handling, shouldn't you pass up the return
code on non-zero? It looks like it could pass up to DRM core then.
} else if (dc_supports_vrr(dm->dc->ctx->dce_version)) {
if (enable) {
- /* vblank irq on -> Only need vupdate irq in vrr mode */
+ /* vblank irq on -> Only need vupdate irq in vrr mode
+ * Not ref-counted since we need explicit enable/disable
+ * for DCE VRR handling
+ */
if (amdgpu_dm_crtc_vrr_active(acrtc_state))
rc = amdgpu_dm_crtc_set_vupdate_irq(crtc, true);
} else {
