Re: [PATCH] drm/amd/display: Prevent OTG shutdown during PSR SU

[Leo Li]

On 2022-09-28 09:52, Harry Wentland wrote:


On 2022-09-27 19:13, sunpeng...@amd.com wrote:
From: Leo Li <sunpeng...@amd.com>

[Why]

Enabling Z10 optimizations allows DMUB to disable the OTG during PSR
link-off. This theoretically saves power by putting more of the display
hardware to sleep. However, we observe that with PSR SU, it causes
visual artifacts, higher power usage, and potential system hang.

This is partly due to an odd behavior with the VStartup interrupt used
to signal DRM vblank events. If the OTG is toggled on/off during a PSR
link on/off cycle, the vstartup interrupt fires twice in quick
succession. This generates incorrectly timed vblank events.
Additionally, it can cause cursor updates to generate visual artifacts.

Note that this is not observed with PSR1 since PSR is fully disabled
when there are vblank event requestors. Cursor updates are also
artifact-free, likely because there are no selectively-updated (SU)
frames that can generate artifacts.

[How]

A potential solution is to disable z10 idle optimizations only when fast
updates (flips & cursor updates) are committed. A mechanism to do so
would require some thoughtful design. Let's just disable idle
optimizations for PSR2 for now.


Great writeup. Wish every bugfix came with details like this.

Fixes: 7cc191ee7621 ("drm/amd/display: Implement MPO PSR SU")
Signed-off-by: Leo Li <sunpeng...@amd.com>

With Thorsten's comments addressed this is
Reviewed-by: Harry Wentland <harry.wentl...@amd.com>

Harry


Thanks all for the bug report, comments, and review. Will send out v2 
and merge shortly.

Leo

---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c | 8 +++++++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
index c8da18e45b0e..8ca10ab3dfc1 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
@@ -170,7 +170,13 @@ bool amdgpu_dm_psr_enable(struct dc_stream_state *stream)
                                           &stream, 1,
                                           &params);
  
-	power_opt |= psr_power_opt_z10_static_screen;
+       /*
+        * Only enable static-screen optimizations for PSR1. For PSR SU, this
+        * causes vstartup interrupt issues, used by amdgpu_dm to send vblank
+        * events.
+        */
+       if (link->psr_settings.psr_version < DC_PSR_VERSION_SU_1)
+               power_opt |= psr_power_opt_z10_static_screen;
  
  	return dc_link_set_psr_allow_active(link, &psr_enable, false, false, &power_opt);
  }