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

[Leo Li <sunpeng.li@xxxxxxx>]

On 2022-09-28 09:52, Harry Wentland wrote:
> 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

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);
> >   }