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

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

[sunpeng.li]

From: Leo Li <sunpeng.li@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.

Fixes: 7cc191ee7621 ("drm/amd/display: Implement MPO PSR SU")
Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
 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);
 }
-- 
2.37.3

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

[sunpeng.li]

From: Leo Li <sunpeng.li@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.

Fixes: 7cc191ee7621 ("drm/amd/display: Implement MPO PSR SU")
Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
 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);
 }
-- 
2.37.3

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

[Thorsten Leemhuis]

On 28.09.22 01:13, sunpeng.li@amd.com wrote:
> From: Leo Li <sunpeng.li@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.
>
> [...]
>
> Fixes: 7cc191ee7621 ("drm/amd/display: Implement MPO PSR SU")

Many thx for taking care of this. There is one small thing to improve,
please add the following tags here:

Reported-by: August Wikerfors <git@augustwikerfors.se>
Link:
https://lore.kernel.org/r/c1f8886a-5624-8f49-31b1-e42b6d20dcf5@augustwikerfors.se/

To explain: Linus[1] and others considered proper link tags in cases
like important, as they allow anyone to look into the backstory weeks or
years from now. That why they should be placed here, as outlined by the
documentation[2]. I care personally, because these tags make my
regression tracking efforts a whole lot easier, as they allow my
tracking bot 'regzbot' to automatically connect reports with patches
posted or committed to fix tracked regressions.

Apropos regzbot, let me tell regzbot to monitor this thread:

#regzbot ^backmonitor:
https://lore.kernel.org/r/c1f8886a-5624-8f49-31b1-e42b6d20dcf5@augustwikerfors.se/

> [...]

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.

[1] for details, see:
https://lore.kernel.org/all/CAHk-=wjMmSZzMJ3Xnskdg4+GGz=5p5p+GSYyFBTh0f-DgvdBWg@mail.gmail.com/
https://lore.kernel.org/all/CAHk-=wgs38ZrfPvy=nOwVkVzjpM3VFU1zobP37Fwd_h9iAD5JQ@mail.gmail.com/
https://lore.kernel.org/all/CAHk-=wjxzafG-=J8oT30s7upn4RhBs6TX-uVFZ5rME+L5_DoJA@mail.gmail.com/

[2] see Documentation/process/submitting-patches.rst
(http://docs.kernel.org/process/submitting-patches.html) and
Documentation/process/5.Posting.rst
(https://docs.kernel.org/process/5.Posting.html)

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

[Thorsten Leemhuis]

On 28.09.22 01:13, sunpeng.li@amd.com wrote:
> From: Leo Li <sunpeng.li@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.
>
> [...]
>
> Fixes: 7cc191ee7621 ("drm/amd/display: Implement MPO PSR SU")

Many thx for taking care of this. There is one small thing to improve,
please add the following tags here:

Reported-by: August Wikerfors <git@augustwikerfors.se>
Link:
https://lore.kernel.org/r/c1f8886a-5624-8f49-31b1-e42b6d20dcf5@augustwikerfors.se/

To explain: Linus[1] and others considered proper link tags in cases
like important, as they allow anyone to look into the backstory weeks or
years from now. That why they should be placed here, as outlined by the
documentation[2]. I care personally, because these tags make my
regression tracking efforts a whole lot easier, as they allow my
tracking bot 'regzbot' to automatically connect reports with patches
posted or committed to fix tracked regressions.

Apropos regzbot, let me tell regzbot to monitor this thread:

#regzbot ^backmonitor:
https://lore.kernel.org/r/c1f8886a-5624-8f49-31b1-e42b6d20dcf5@augustwikerfors.se/

> [...]

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.

[1] for details, see:
https://lore.kernel.org/all/CAHk-=wjMmSZzMJ3Xnskdg4+GGz=5p5p+GSYyFBTh0f-DgvdBWg@mail.gmail.com/
https://lore.kernel.org/all/CAHk-=wgs38ZrfPvy=nOwVkVzjpM3VFU1zobP37Fwd_h9iAD5JQ@mail.gmail.com/
https://lore.kernel.org/all/CAHk-=wjxzafG-=J8oT30s7upn4RhBs6TX-uVFZ5rME+L5_DoJA@mail.gmail.com/

[2] see Documentation/process/submitting-patches.rst
(http://docs.kernel.org/process/submitting-patches.html) and
Documentation/process/5.Posting.rst
(https://docs.kernel.org/process/5.Posting.html)

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

[August Wikerfors]

Hi Leo,

On 2022-09-28 01:13, sunpeng.li@amd.com wrote:
> From: Leo Li <sunpeng.li@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.
I can confirm that this patch fixes the issues I had. Thanks!

> Fixes: 7cc191ee7621 ("drm/amd/display: Implement MPO PSR SU")
> Signed-off-by: Leo Li <sunpeng.li@amd.com>
You can add:
Tested-by: August Wikerfors <git@augustwikerfors.se>

Regards,
August Wikerfors

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

[August Wikerfors]

Hi Leo,

On 2022-09-28 01:13, sunpeng.li@amd.com wrote:
> From: Leo Li <sunpeng.li@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.
I can confirm that this patch fixes the issues I had. Thanks!

> Fixes: 7cc191ee7621 ("drm/amd/display: Implement MPO PSR SU")
> Signed-off-by: Leo Li <sunpeng.li@amd.com>
You can add:
Tested-by: August Wikerfors <git@augustwikerfors.se>

Regards,
August Wikerfors

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

[Harry Wentland]

On 2022-09-27 19:13, sunpeng.li@amd.com wrote:
> From: Leo Li <sunpeng.li@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.li@amd.com>

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

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

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

[Harry Wentland]

On 2022-09-27 19:13, sunpeng.li@amd.com wrote:
> From: Leo Li <sunpeng.li@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.li@amd.com>

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

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

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.li@amd.com wrote:
>> From: Leo Li <sunpeng.li@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.li@amd.com>
> 
> With Thorsten's comments addressed this is
> Reviewed-by: Harry Wentland <harry.wentland@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);
>>   }
> 

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.li@amd.com wrote:
>> From: Leo Li <sunpeng.li@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.li@amd.com>
> 
> With Thorsten's comments addressed this is
> Reviewed-by: Harry Wentland <harry.wentland@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);
>>   }
> 

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

[sunpeng.li]

From: Leo Li <sunpeng.li@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.

Fixes: 7cc191ee7621 ("drm/amd/display: Implement MPO PSR SU")
Reported-by: August Wikerfors <git@augustwikerfors.se>
Link: https://lore.kernel.org/r/c1f8886a-5624-8f49-31b1-e42b6d20dcf5@augustwikerfors.se/
Tested-by: August Wikerfors <git@augustwikerfors.se>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
 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);
 }
-- 
2.37.3

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

[sunpeng.li]

From: Leo Li <sunpeng.li@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.

Fixes: 7cc191ee7621 ("drm/amd/display: Implement MPO PSR SU")
Reported-by: August Wikerfors <git@augustwikerfors.se>
Link: https://lore.kernel.org/r/c1f8886a-5624-8f49-31b1-e42b6d20dcf5@augustwikerfors.se/
Tested-by: August Wikerfors <git@augustwikerfors.se>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
 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);
 }
-- 
2.37.3