On 2022-09-27 19:13, sunpeng.li@xxxxxxx wrote:
> From: Leo Li <sunpeng.li@xxxxxxx>
>
> [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.li@xxxxxxx>
With Thorsten's comments addressed this is
Reviewed-by: Harry Wentland <harry.wentland@xxxxxxx>
Harry
> ---
> 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,
> ¶ms);
>
> - 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);
> }
