Skip to contentSkip to conversation

drm/amd/display: add module param to disable immediate vblank off

PATCH In review Michele Palazzi 5 participants 6 messages Feb 11 – Feb 12

Subsystems (4)
Stable branch DRM drivers Radeon and amdgpu DRM drivers AMD display core
lore
Share & tools
lore

Michele Palazzi

author
#0 lore raw

PATCH 1/1 drm/amd/display: add module param to disable immediate vblank off

Add amdgpu.no_vblank_immediate parameter to optionally disable the immediate vblank disable path on DCN35+ non-PSR CRTCs. When set to 1, a 2-frame offdelay is used instead, matching the behavior used for older hardware and DGPUs.

This works around flip_done timeouts and GPU hangs that some users experience with the immediate vblank disable path, particularly with DisplayPort connections. The default behavior is unchanged.

This is a temporary workaround, to be removed once the underlying vblank/DM locking issue is properly resolved.

Link https://gitlab.freedesktop.org/drm/amd/-/issues/3787
Signed-off-by Michele Palazzi <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h             |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c         | 16 ++++++++++++++++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   | 17 ++++++++++++++---
 3 files changed, 31 insertions(+), 3 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu.h +1 -0
index b20a06abb65d..5de60af8c5f5 100644
@@ -213,6 +213,7 @@ extern uint amdgpu_dc_visual_confirm;
 extern int amdgpu_dm_abm_level;
 extern int amdgpu_backlight;
 extern int amdgpu_damage_clips;
+extern int amdgpu_no_vblank_immediate;
 extern struct amdgpu_mgpu_info mgpu_info;
 extern int amdgpu_ras_enable;
 extern uint amdgpu_ras_mask;
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +16 -0
index 39387da8586b..94cab76805ed 100644
@@ -924,6 +924,22 @@ MODULE_PARM_DESC(damageclips,
 		 "Damage clips support (0 = disable, 1 = enable, -1 auto (default))");
 module_param_named(damageclips, amdgpu_damage_clips, int, 0444);
 
+/**
+ * DOC: no_vblank_immediate (int)
+ * Disable immediate vblank disable on DCN35+ non-PSR CRTCs. Use a 2-frame
+ * offdelay instead, matching the behavior used for older hardware and DGPUs.
+ * This works around flip_done timeouts that cause GPU hangs on some hardware
+ * with DisplayPort connections.
+ * (0 = use default immediate disable (default), 1 = use 2-frame offdelay)
+ *
+ * This is a temporary workaround and should be removed once the underlying
+ * vblank/DM locking issue is resolved.
+ */
+int amdgpu_no_vblank_immediate;
+MODULE_PARM_DESC(no_vblank_immediate,
+	"Disable immediate vblank off on DCN35+ (0 = default, 1 = use 2-frame offdelay)");
+module_param_named(no_vblank_immediate, amdgpu_no_vblank_immediate, int, 0444);
+
 /**
  * DOC: tmz (int)
  * Trusted Memory Zone (TMZ) is a method to protect data being written
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +14 -3
index a8a59126b2d2..e18ad3949826 100644
@@ -9335,9 +9335,20 @@ static void manage_dm_interrupts(struct amdgpu_device *adev,
 
 			config.offdelay_ms = offdelay ?: 30;
 		} else {
-			/* offdelay_ms = 0 will never disable vblank */
-			config.offdelay_ms = 1;
-			config.disable_immediate = true;
+			if (amdgpu_no_vblank_immediate) {
+				/*
+				 * Use 2-frame offdelay instead of immediate
+				 * disable to work around flip_done timeouts.
+				 */
+				offdelay = DIV64_U64_ROUND_UP((u64)20 *
+							      timing->v_total *
+							      timing->h_total,
+							      timing->pix_clk_100hz);
+				config.offdelay_ms = offdelay ?: 30;
+			} else {
+				config.offdelay_ms = 1;
+				config.disable_immediate = true;
+			}
 		}
 
 		drm_crtc_vblank_on_config(&acrtc->base,
Signature · 1 line
2.53.0
maintainer
#1 lore raw
Quoted 4 lines · Michele Palazzi

Add amdgpu.no_vblank_immediate parameter to optionally disable the immediate vblank disable path on DCN35+ non-PSR CRTCs. When set to 1, a 2-frame offdelay is used instead, matching the behavior used for older hardware and DGPUs.

Please no more module parameters, this is not the 1990's with only one one device in the system. Please fix this the proper way.

Also, this isn't the correct way to submit patches to stable.

thanks,

greg k-h

maintainer
#2 lore raw

Dropping stable, putting Grep on BCC.

Quoted 8 lines · Greg KH
Quoted 4 lines · Michele Palazzi

Add amdgpu.no_vblank_immediate parameter to optionally disable the immediate vblank disable path on DCN35+ non-PSR CRTCs. When set to 1, a 2-frame offdelay is used instead, matching the behavior used for older hardware and DGPUs.

Please no more module parameters, this is not the 1990's with only one one device in the system. Please fix this the proper way.

I just wanted to write the same.

We can of course implement something in DAL/DC, but clearly not behind a module parameter.

Or is there any other negative consequence except for that the display blanking is delayed by ~40ms?

Regards,
Christian.

Quoted 5 lines

Also, this isn't the correct way to submit patches to stable.

thanks,

greg k-h

#3 lore raw
Quoted 9 lines · Christian König

Dropping stable, putting Grep on BCC.

Quoted 6 lines · Greg KH
Quoted 4 lines · Michele Palazzi

Add amdgpu.no_vblank_immediate parameter to optionally disable the immediate vblank disable path on DCN35+ non-PSR CRTCs. When set to 1, a 2-frame offdelay is used instead, matching the behavior used for older hardware and DGPUs.

Quoted 9 lines
Quoted 2 lines

Please no more module parameters, this is not the 1990's with only one one device in the system. Please fix this the proper way.

I just wanted to write the same.

We can of course implement something in DAL/DC, but clearly not behind a module parameter.

Or is there any other negative consequence except for that the display blanking is delayed by ~40ms?

If there is a strong enough interest in keeping something like this as a debugging mechanism, there is already an amdgpu.dcdebugmask parameter. This accepts a bitmask of debug bits. Assign this functionality to a new bit.

#4 lore raw
In reply to Christian König
Quoted 9 lines · Christian König

Dropping stable, putting Grep on BCC.

Quoted 6 lines · Greg KH
Quoted 4 lines · Michele Palazzi

Add amdgpu.no_vblank_immediate parameter to optionally disable the immediate vblank disable path on DCN35+ non-PSR CRTCs. When set to 1, a 2-frame offdelay is used instead, matching the behavior used for older hardware and DGPUs.

Quoted 8 lines
Quoted 2 lines

Please no more module parameters, this is not the 1990's with only one one device in the system. Please fix this the proper way.

I just wanted to write the same.

We can of course implement something in DAL/DC, but clearly not behind a module parameter.

Or is there any other negative consequence except for that the display blanking is delayed by ~40ms?

It doesn't affect display blanking, only how long the vblank interrupt stays enabled. (If it stays enabled unnecessarily, that might waste some energy)

Signature · 2 lines
Earthling Michel Dänzer       \        GNOME / Xwayland / Mesa developer
https://redhat.com             \               Libre software enthusiast
author
#5 lore raw
In reply to Greg KH

Thanks for the feedback. The AMD display subsystem developer had agreed to this approach as a temporary workaround while the underlying locking fix is being developed, but I understand the policy against new module parameters. I'll defer to the AMD team to submit the proper fix.

Apologies for the incorrect stable process - first time submitting upstream

Quoted 13 lines · Greg KH
Quoted 4 lines · Michele Palazzi

Add amdgpu.no_vblank_immediate parameter to optionally disable the immediate vblank disable path on DCN35+ non-PSR CRTCs. When set to 1, a 2-frame offdelay is used instead, matching the behavior used for older hardware and DGPUs.

Please no more module parameters, this is not the 1990's with only one one device in the system. Please fix this the proper way.

Also, this isn't the correct way to submit patches to stable.

thanks,

greg k-h