* [PATCH] drm/amd/display: Fix flip-done timeouts on mode1 reset
@ 2026-07-23 18:01 sunpeng.li
2026-07-23 19:18 ` Mario Limonciello
0 siblings, 1 reply; 4+ messages in thread
From: sunpeng.li @ 2026-07-23 18:01 UTC (permalink / raw)
To: amd-gfx
Cc: Harry.Wentland, mario.limonciello, alexander.deucher, Leo Li,
stable
From: Leo Li <sunpeng.li@amd.com>
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: stable@vger.kernel.org
Fixes: 8382cd234981 ("drm/amd/display: consolidate DCN vblank/flip handling onto vupdate_no_lock")
Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
.../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);
+ }
} 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 {
--
2.54.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] drm/amd/display: Fix flip-done timeouts on mode1 reset
2026-07-23 18:01 [PATCH] drm/amd/display: Fix flip-done timeouts on mode1 reset sunpeng.li
@ 2026-07-23 19:18 ` Mario Limonciello
2026-07-23 19:47 ` Leo Li
0 siblings, 1 reply; 4+ messages in thread
From: Mario Limonciello @ 2026-07-23 19:18 UTC (permalink / raw)
To: sunpeng.li, amd-gfx; +Cc: Harry.Wentland, alexander.deucher, stable
On 7/23/26 13:01, sunpeng.li@amd.com wrote:
> From: Leo Li <sunpeng.li@amd.com>
>
> 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: stable@vger.kernel.org
> Fixes: 8382cd234981 ("drm/amd/display: consolidate DCN vblank/flip handling onto vupdate_no_lock")
> Signed-off-by: Leo Li <sunpeng.li@amd.com>
> ---
> .../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 {
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] drm/amd/display: Fix flip-done timeouts on mode1 reset
2026-07-23 19:18 ` Mario Limonciello
@ 2026-07-23 19:47 ` Leo Li
2026-07-23 19:55 ` Mario Limonciello
0 siblings, 1 reply; 4+ messages in thread
From: Leo Li @ 2026-07-23 19:47 UTC (permalink / raw)
To: Mario Limonciello, amd-gfx; +Cc: Harry.Wentland, alexander.deucher, stable
On 2026-07-23 15:18, Mario Limonciello 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 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.
It's not shown in the context here, but the rc is passed up later:
https://elixir.bootlin.com/linux/v7.2-rc4/source/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c#L295
- Leo
>
>> } 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 {
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] drm/amd/display: Fix flip-done timeouts on mode1 reset
2026-07-23 19:47 ` Leo Li
@ 2026-07-23 19:55 ` Mario Limonciello
0 siblings, 0 replies; 4+ messages in thread
From: Mario Limonciello @ 2026-07-23 19:55 UTC (permalink / raw)
To: Leo Li, amd-gfx; +Cc: Harry.Wentland, alexander.deucher, stable
On 7/23/26 14:47, Leo Li wrote:
>
>
> On 2026-07-23 15:18, Mario Limonciello 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 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.
>
> It's not shown in the context here, but the rc is passed up later:
> https://elixir.bootlin.com/linux/v7.2-rc4/source/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c#L295
>
> - Leo
>
Ah got it, thanks. No concerns.
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
>>
>>> } 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 {
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-23 19:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-23 18:01 [PATCH] drm/amd/display: Fix flip-done timeouts on mode1 reset sunpeng.li
2026-07-23 19:18 ` Mario Limonciello
2026-07-23 19:47 ` Leo Li
2026-07-23 19:55 ` Mario Limonciello
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.