diff mbox series

[v2,02/17] drm/i915/psr: Disable PSR when bigjoiner is used

Message ID 20240404213441.17637-3-ville.syrjala@linux.intel.com (mailing list archive)
State New
Headers show
Series drm/i915: Bigjoiner modeset sequence redesign and MST support | expand

Commit Message

Ville Syrjala April 4, 2024, 9:34 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Bigjoiner seem to be causing all kinds of grief to the PSR
code currently. I don't believe there is any hardware issue
but the code simply not handling this correctly. For now
just disable PSR when bigjoiner is needed.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Hogander, Jouni April 5, 2024, 6:58 a.m. UTC | #1
On Fri, 2024-04-05 at 00:34 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Bigjoiner seem to be causing all kinds of grief to the PSR
> code currently. I don't believe there is any hardware issue
> but the code simply not handling this correctly. For now
> just disable PSR when bigjoiner is needed.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index eef62983e9db..a3ff916b53f9 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1584,6 +1584,17 @@ void intel_psr_compute_config(struct intel_dp
> *intel_dp,
>                 return;
>         }
>  
> +       /*
> +        * FIXME figure out what is wrong with PSR+bigjoiner and
> +        * fix it. Presumably something related to the fact that
> +        * PSR is a transcoder level feature.
> +        */
> +       if (crtc_state->bigjoiner_pipes) {
> +               drm_dbg_kms(&dev_priv->drm,
> +                           "PSR disabled due to bigjoiner\n");
> +               return;
> +       }
> +

Are these problems with both PSR1 and PSR2?

BR,

Jouni Högander

>         if (CAN_PANEL_REPLAY(intel_dp))
>                 crtc_state->has_panel_replay = true;
>         else
Murthy, Arun R April 5, 2024, 12:58 p.m. UTC | #2
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Hogander, Jouni
> Sent: Friday, April 5, 2024 12:29 PM
> To: ville.syrjala@linux.intel.com; intel-gfx@lists.freedesktop.org
> Subject: Re: [PATCH v2 02/17] drm/i915/psr: Disable PSR when bigjoiner is used
> 
> On Fri, 2024-04-05 at 00:34 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Bigjoiner seem to be causing all kinds of grief to the PSR code
> > currently. I don't believe there is any hardware issue but the code
> > simply not handling this correctly. For now just disable PSR when
> > bigjoiner is needed.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index eef62983e9db..a3ff916b53f9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1584,6 +1584,17 @@ void intel_psr_compute_config(struct intel_dp
> > *intel_dp,
> >                 return;
> >         }
> >
> > +       /*
> > +        * FIXME figure out what is wrong with PSR+bigjoiner and
> > +        * fix it. Presumably something related to the fact that
> > +        * PSR is a transcoder level feature.
> > +        */
> > +       if (crtc_state->bigjoiner_pipes) {
> > +               drm_dbg_kms(&dev_priv->drm,
> > +                           "PSR disabled due to bigjoiner\n");
> > +               return;
> > +       }
> > +
> 
> Are these problems with both PSR1 and PSR2?
> 
> BR,
> 
> Jouni Högander
> 
> >         if (CAN_PANEL_REPLAY(intel_dp))
> >                 crtc_state->has_panel_replay = true;
> >         else
Murthy, Arun R April 5, 2024, 1 p.m. UTC | #3
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Hogander, Jouni
> Sent: Friday, April 5, 2024 12:29 PM
> To: ville.syrjala@linux.intel.com; intel-gfx@lists.freedesktop.org
> Subject: Re: [PATCH v2 02/17] drm/i915/psr: Disable PSR when bigjoiner is used
> 
> On Fri, 2024-04-05 at 00:34 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Bigjoiner seem to be causing all kinds of grief to the PSR code
> > currently. I don't believe there is any hardware issue but the code
> > simply not handling this correctly. For now just disable PSR when
> > bigjoiner is needed.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index eef62983e9db..a3ff916b53f9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1584,6 +1584,17 @@ void intel_psr_compute_config(struct intel_dp
> > *intel_dp,
> >                 return;
> >         }
> >
> > +       /*
> > +        * FIXME figure out what is wrong with PSR+bigjoiner and
> > +        * fix it. Presumably something related to the fact that
> > +        * PSR is a transcoder level feature.
> > +        */
> > +       if (crtc_state->bigjoiner_pipes) {
> > +               drm_dbg_kms(&dev_priv->drm,
> > +                           "PSR disabled due to bigjoiner\n");
> > +               return;
> > +       }
> > +
> 
> Are these problems with both PSR1 and PSR2?

Yes, as per the code this would have impact on both PSR1 and PSR2. So better to disable. 
Looks good to me.

Thanks and Regards,
Arun R Murthy
--------------------
> 
> BR,
> 
> Jouni Högander
> 
> >         if (CAN_PANEL_REPLAY(intel_dp))
> >                 crtc_state->has_panel_replay = true;
> >         else
Murthy, Arun R April 5, 2024, 3:45 p.m. UTC | #4
Forgot to add my Rb

Reviewed-by: Arun R Murthy <arun.r.mruthy@intel.com>

Thanks and Regards,
Arun R Murthy
--------------------

> -----Original Message-----
> From: Murthy, Arun R
> Sent: Friday, April 5, 2024 6:30 PM
> To: Hogander, Jouni <jouni.hogander@intel.com>; ville.syrjala@linux.intel.com;
> intel-gfx@lists.freedesktop.org
> Subject: RE: [PATCH v2 02/17] drm/i915/psr: Disable PSR when bigjoiner is used
> 
> 
> 
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of
> > Hogander, Jouni
> > Sent: Friday, April 5, 2024 12:29 PM
> > To: ville.syrjala@linux.intel.com; intel-gfx@lists.freedesktop.org
> > Subject: Re: [PATCH v2 02/17] drm/i915/psr: Disable PSR when bigjoiner
> > is used
> >
> > On Fri, 2024-04-05 at 00:34 +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Bigjoiner seem to be causing all kinds of grief to the PSR code
> > > currently. I don't believe there is any hardware issue but the code
> > > simply not handling this correctly. For now just disable PSR when
> > > bigjoiner is needed.
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_psr.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index eef62983e9db..a3ff916b53f9 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -1584,6 +1584,17 @@ void intel_psr_compute_config(struct intel_dp
> > > *intel_dp,
> > >                 return;
> > >         }
> > >
> > > +       /*
> > > +        * FIXME figure out what is wrong with PSR+bigjoiner and
> > > +        * fix it. Presumably something related to the fact that
> > > +        * PSR is a transcoder level feature.
> > > +        */
> > > +       if (crtc_state->bigjoiner_pipes) {
> > > +               drm_dbg_kms(&dev_priv->drm,
> > > +                           "PSR disabled due to bigjoiner\n");
> > > +               return;
> > > +       }
> > > +
> >
> > Are these problems with both PSR1 and PSR2?
> 
> Yes, as per the code this would have impact on both PSR1 and PSR2. So better
> to disable.
> Looks good to me.
> 
> Thanks and Regards,
> Arun R Murthy
> --------------------
> >
> > BR,
> >
> > Jouni Högander
> >
> > >         if (CAN_PANEL_REPLAY(intel_dp))
> > >                 crtc_state->has_panel_replay = true;
> > >         else
Ville Syrjala April 5, 2024, 7:34 p.m. UTC | #5
On Fri, Apr 05, 2024 at 06:58:44AM +0000, Hogander, Jouni wrote:
> On Fri, 2024-04-05 at 00:34 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Bigjoiner seem to be causing all kinds of grief to the PSR
> > code currently. I don't believe there is any hardware issue
> > but the code simply not handling this correctly. For now
> > just disable PSR when bigjoiner is needed.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index eef62983e9db..a3ff916b53f9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1584,6 +1584,17 @@ void intel_psr_compute_config(struct intel_dp
> > *intel_dp,
> >                 return;
> >         }
> >  
> > +       /*
> > +        * FIXME figure out what is wrong with PSR+bigjoiner and
> > +        * fix it. Presumably something related to the fact that
> > +        * PSR is a transcoder level feature.
> > +        */
> > +       if (crtc_state->bigjoiner_pipes) {
> > +               drm_dbg_kms(&dev_priv->drm,
> > +                           "PSR disabled due to bigjoiner\n");
> > +               return;
> > +       }
> > +
> 
> Are these problems with both PSR1 and PSR2?

I didn't look at he logs in that much detail.

It's now happening in CI because the bigjoiner force knob is
getting leaked to all kinds of tests. Eg. this might be one:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14519/re-mtlp-1/igt@kms_busy@basic.html
Hogander, Jouni April 8, 2024, 5:09 a.m. UTC | #6
On Fri, 2024-04-05 at 22:34 +0300, Ville Syrjälä wrote:
> On Fri, Apr 05, 2024 at 06:58:44AM +0000, Hogander, Jouni wrote:
> > On Fri, 2024-04-05 at 00:34 +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Bigjoiner seem to be causing all kinds of grief to the PSR
> > > code currently. I don't believe there is any hardware issue
> > > but the code simply not handling this correctly. For now
> > > just disable PSR when bigjoiner is needed.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Acked-by: Jouni Högander <jouni.hogander@intel.com>

> > > ---
> > >  drivers/gpu/drm/i915/display/intel_psr.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index eef62983e9db..a3ff916b53f9 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -1584,6 +1584,17 @@ void intel_psr_compute_config(struct
> > > intel_dp
> > > *intel_dp,
> > >                 return;
> > >         }
> > >  
> > > +       /*
> > > +        * FIXME figure out what is wrong with PSR+bigjoiner and
> > > +        * fix it. Presumably something related to the fact that
> > > +        * PSR is a transcoder level feature.
> > > +        */
> > > +       if (crtc_state->bigjoiner_pipes) {
> > > +               drm_dbg_kms(&dev_priv->drm,
> > > +                           "PSR disabled due to bigjoiner\n");
> > > +               return;
> > > +       }
> > > +
> > 
> > Are these problems with both PSR1 and PSR2?
> 
> I didn't look at he logs in that much detail.
> 
> It's now happening in CI because the bigjoiner force knob is
> getting leaked to all kinds of tests. Eg. this might be one:
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14519/re-mtlp-1/igt@kms_busy@basic.html
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index eef62983e9db..a3ff916b53f9 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1584,6 +1584,17 @@  void intel_psr_compute_config(struct intel_dp *intel_dp,
 		return;
 	}
 
+	/*
+	 * FIXME figure out what is wrong with PSR+bigjoiner and
+	 * fix it. Presumably something related to the fact that
+	 * PSR is a transcoder level feature.
+	 */
+	if (crtc_state->bigjoiner_pipes) {
+		drm_dbg_kms(&dev_priv->drm,
+			    "PSR disabled due to bigjoiner\n");
+		return;
+	}
+
 	if (CAN_PANEL_REPLAY(intel_dp))
 		crtc_state->has_panel_replay = true;
 	else