2022-02-08 14:47:06

by Lyude Paul

[permalink] [raw]
Subject: [PATCH] drm/i915/psr: Disable PSR2 selective fetch for all TGL steps

As we've unfortunately started to come to expect from PSR on Intel
platforms, PSR2 selective fetch is not at all ready to be enabled on
Tigerlake as it results in severe flickering issues - at least on this
ThinkPad X1 Carbon 9th generation. The easiest way I've found of
reproducing these issues is to just move the cursor around the left border
of the screen (suspicious…).

So, fix people's displays again and turn PSR2 selective fetch off for all
steppings of Tigerlake. This can be re-enabled again if someone from Intel
finds the time to fix this functionality on OEM machines.

Signed-off-by: Lyude Paul <[email protected]>
Fixes: 7f6002e58025 ("drm/i915/display: Enable PSR2 selective fetch by default")
Cc: Gwan-gyeong Mun <[email protected]>
Cc: Ville Syrjälä <[email protected]>
Cc: José Roberto de Souza <[email protected]>
Cc: Jani Nikula <[email protected]>
Cc: Rodrigo Vivi <[email protected]>
Cc: [email protected]
Cc: <[email protected]> # v5.16+
---
drivers/gpu/drm/i915/display/intel_psr.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index a1a663f362e7..25c16abcd9cd 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -737,10 +737,14 @@ static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
return false;
}

- /* Wa_14010254185 Wa_14010103792 */
- if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_C0)) {
+ /*
+ * There's two things stopping this from being enabled on TGL:
+ * For steps A0-C0: workarounds Wa_14010254185 Wa_14010103792 are missing
+ * For all steps: PSR2 selective fetch causes screen flickering
+ */
+ if (IS_TIGERLAKE(dev_priv)) {
drm_dbg_kms(&dev_priv->drm,
- "PSR2 sel fetch not enabled, missing the implementation of WAs\n");
+ "PSR2 sel fetch not enabled, currently broken on TGL\n");
return false;
}

--
2.34.1



2022-02-09 08:47:49

by Lyude Paul

[permalink] [raw]
Subject: Re: [PATCH] drm/i915/psr: Disable PSR2 selective fetch for all TGL steps

On Tue, 2022-02-08 at 13:06 +0000, Souza, Jose wrote:
> On Mon, 2022-02-07 at 16:38 -0500, Lyude Paul wrote:
> > As we've unfortunately started to come to expect from PSR on Intel
> > platforms, PSR2 selective fetch is not at all ready to be enabled on
> > Tigerlake as it results in severe flickering issues - at least on this
> > ThinkPad X1 Carbon 9th generation. The easiest way I've found of
> > reproducing these issues is to just move the cursor around the left border
> > of the screen (suspicious…).
>
> Where is the bug for that? Where is the logs?

I'm happy to open up a bug and include some logs, will do it in just a moment

> We can't go from enabled to disabled without any debug and because of a
> single device.
> In the mean time you have the option to set the i915 parameter to disable
> it.

I mean - I totally understand the hesistance with the lack of debug info, I'll
go open up an issue with said info in a bit. FWIW is a machine currently being
sold with Linux pre-installs which is expected to work out of the box, so it's
not exactly an uncommon laptop to be running Linux. Also I don't have any
problem with us trying to fix the issue before flat out disabling things - I
sent the revert hoping that would happen, and also because I needed to write
the revert anyway since I had to disable this in Fedora's kernel.

>
> >
> > So, fix people's displays again and turn PSR2 selective fetch off for all
> > steppings of Tigerlake. This can be re-enabled again if someone from Intel
> > finds the time to fix this functionality on OEM machines.
> >
> > Signed-off-by: Lyude Paul <[email protected]>
> > Fixes: 7f6002e58025 ("drm/i915/display: Enable PSR2 selective fetch by
> > default")
> > Cc: Gwan-gyeong Mun <[email protected]>
> > Cc: Ville Syrjälä <[email protected]>
> > Cc: José Roberto de Souza <[email protected]>
> > Cc: Jani Nikula <[email protected]>
> > Cc: Rodrigo Vivi <[email protected]>
> > Cc: [email protected]
> > Cc: <[email protected]> # v5.16+
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index a1a663f362e7..25c16abcd9cd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -737,10 +737,14 @@ static bool intel_psr2_sel_fetch_config_valid(struct
> > intel_dp *intel_dp,
> >                 return false;
> >         }
> >  
> > -       /* Wa_14010254185 Wa_14010103792 */
> > -       if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_C0)) {
> > +       /*
> > +        * There's two things stopping this from being enabled on TGL:
> > +        * For steps A0-C0: workarounds Wa_14010254185 Wa_14010103792 are
> > missing
> > +        * For all steps: PSR2 selective fetch causes screen flickering
> > +        */
> > +       if (IS_TIGERLAKE(dev_priv)) {
> >                 drm_dbg_kms(&dev_priv->drm,
> > -                           "PSR2 sel fetch not enabled, missing the
> > implementation of WAs\n");
> > +                           "PSR2 sel fetch not enabled, currently broken
> > on TGL\n");
> >                 return false;
> >         }
> >  
>

--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat


2022-02-09 10:07:50

by José Roberto de Souza

[permalink] [raw]
Subject: Re: [PATCH] drm/i915/psr: Disable PSR2 selective fetch for all TGL steps

On Mon, 2022-02-07 at 16:38 -0500, Lyude Paul wrote:
> As we've unfortunately started to come to expect from PSR on Intel
> platforms, PSR2 selective fetch is not at all ready to be enabled on
> Tigerlake as it results in severe flickering issues - at least on this
> ThinkPad X1 Carbon 9th generation. The easiest way I've found of
> reproducing these issues is to just move the cursor around the left border
> of the screen (suspicious…).

Where is the bug for that? Where is the logs?
We can't go from enabled to disabled without any debug and because of a single device.
In the mean time you have the option to set the i915 parameter to disable it.

>
> So, fix people's displays again and turn PSR2 selective fetch off for all
> steppings of Tigerlake. This can be re-enabled again if someone from Intel
> finds the time to fix this functionality on OEM machines.
>
> Signed-off-by: Lyude Paul <[email protected]>
> Fixes: 7f6002e58025 ("drm/i915/display: Enable PSR2 selective fetch by default")
> Cc: Gwan-gyeong Mun <[email protected]>
> Cc: Ville Syrjälä <[email protected]>
> Cc: José Roberto de Souza <[email protected]>
> Cc: Jani Nikula <[email protected]>
> Cc: Rodrigo Vivi <[email protected]>
> Cc: [email protected]
> Cc: <[email protected]> # v5.16+
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index a1a663f362e7..25c16abcd9cd 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -737,10 +737,14 @@ static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
> return false;
> }
>
> - /* Wa_14010254185 Wa_14010103792 */
> - if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_C0)) {
> + /*
> + * There's two things stopping this from being enabled on TGL:
> + * For steps A0-C0: workarounds Wa_14010254185 Wa_14010103792 are missing
> + * For all steps: PSR2 selective fetch causes screen flickering
> + */
> + if (IS_TIGERLAKE(dev_priv)) {
> drm_dbg_kms(&dev_priv->drm,
> - "PSR2 sel fetch not enabled, missing the implementation of WAs\n");
> + "PSR2 sel fetch not enabled, currently broken on TGL\n");
> return false;
> }
>

2022-02-09 10:13:48

by Hogander, Jouni

[permalink] [raw]
Subject: Re: [Intel-gfx] [PATCH] drm/i915/psr: Disable PSR2 selective fetch for all TGL steps

Hello Paul,

On Mon, 2022-02-07 at 16:38 -0500, Lyude Paul wrote:
> As we've unfortunately started to come to expect from PSR on Intel
> platforms, PSR2 selective fetch is not at all ready to be enabled on
> Tigerlake as it results in severe flickering issues - at least on
> this
> ThinkPad X1 Carbon 9th generation. The easiest way I've found of
> reproducing these issues is to just move the cursor around the left
> border
> of the screen (suspicious…).

Sorry to hear you have this bad experience with i915 PSR2 selective
fetch support. There is already some discussion about these problems
and possible root causes here:

https://gitlab.freedesktop.org/drm/intel/-/issues/4951

I have Dell XPS 13 7390 which has panel supporting PSR2. Luckily I'm
not facing these problems with it.

>
> So, fix people's displays again and turn PSR2 selective fetch off for
> all
> steppings of Tigerlake. This can be re-enabled again if someone from
> Intel
> finds the time to fix this functionality on OEM machines.
>
> Signed-off-by: Lyude Paul <[email protected]>
> Fixes: 7f6002e58025 ("drm/i915/display: Enable PSR2 selective fetch
> by default")
> Cc: Gwan-gyeong Mun <[email protected]>
> Cc: Ville Syrjälä <[email protected]>
> Cc: José Roberto de Souza <[email protected]>
> Cc: Jani Nikula <[email protected]>
> Cc: Rodrigo Vivi <[email protected]>
> Cc: [email protected]
> Cc: <[email protected]> # v5.16+
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index a1a663f362e7..25c16abcd9cd 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -737,10 +737,14 @@ static bool
> intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
> return false;
> }
>
> - /* Wa_14010254185 Wa_14010103792 */
> - if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_C0)) {
> + /*
> + * There's two things stopping this from being enabled on TGL:
> + * For steps A0-C0: workarounds Wa_14010254185 Wa_14010103792
> are missing
> + * For all steps: PSR2 selective fetch causes screen flickering
> + */
> + if (IS_TIGERLAKE(dev_priv)) {
> drm_dbg_kms(&dev_priv->drm,
> - "PSR2 sel fetch not enabled, missing the
> implementation of WAs\n");
> + "PSR2 sel fetch not enabled, currently
> broken on TGL\n");
> return false;
> }
>

BR,

Jouni Högander

2022-02-09 11:33:58

by Lyude Paul

[permalink] [raw]
Subject: Re: [PATCH] drm/i915/psr: Disable PSR2 selective fetch for all TGL steps

Opened the issue at https://gitlab.freedesktop.org/drm/intel/-/issues/5077 ,
included dmesg + video. Feel free to let me know if you need any more info, or
need me to try any patches

On Tue, 2022-02-08 at 13:06 +0000, Souza, Jose wrote:
> On Mon, 2022-02-07 at 16:38 -0500, Lyude Paul wrote:
> > As we've unfortunately started to come to expect from PSR on Intel
> > platforms, PSR2 selective fetch is not at all ready to be enabled on
> > Tigerlake as it results in severe flickering issues - at least on this
> > ThinkPad X1 Carbon 9th generation. The easiest way I've found of
> > reproducing these issues is to just move the cursor around the left border
> > of the screen (suspicious…).
>
> Where is the bug for that? Where is the logs?
> We can't go from enabled to disabled without any debug and because of a
> single device.
> In the mean time you have the option to set the i915 parameter to disable
> it.
>
> >
> > So, fix people's displays again and turn PSR2 selective fetch off for all
> > steppings of Tigerlake. This can be re-enabled again if someone from Intel
> > finds the time to fix this functionality on OEM machines.
> >
> > Signed-off-by: Lyude Paul <[email protected]>
> > Fixes: 7f6002e58025 ("drm/i915/display: Enable PSR2 selective fetch by
> > default")
> > Cc: Gwan-gyeong Mun <[email protected]>
> > Cc: Ville Syrjälä <[email protected]>
> > Cc: José Roberto de Souza <[email protected]>
> > Cc: Jani Nikula <[email protected]>
> > Cc: Rodrigo Vivi <[email protected]>
> > Cc: [email protected]
> > Cc: <[email protected]> # v5.16+
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index a1a663f362e7..25c16abcd9cd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -737,10 +737,14 @@ static bool intel_psr2_sel_fetch_config_valid(struct
> > intel_dp *intel_dp,
> >                 return false;
> >         }
> >  
> > -       /* Wa_14010254185 Wa_14010103792 */
> > -       if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_C0)) {
> > +       /*
> > +        * There's two things stopping this from being enabled on TGL:
> > +        * For steps A0-C0: workarounds Wa_14010254185 Wa_14010103792 are
> > missing
> > +        * For all steps: PSR2 selective fetch causes screen flickering
> > +        */
> > +       if (IS_TIGERLAKE(dev_priv)) {
> >                 drm_dbg_kms(&dev_priv->drm,
> > -                           "PSR2 sel fetch not enabled, missing the
> > implementation of WAs\n");
> > +                           "PSR2 sel fetch not enabled, currently broken
> > on TGL\n");
> >                 return false;
> >         }
> >  
>

--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat