diff mbox

drm/i915: Disable DRRS when PSR is enabled

Message ID 20170831003252.7529-1-radhakrishna.sripada@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sripada, Radhakrishna Aug. 31, 2017, 12:32 a.m. UTC
Some platforms donot support PSR and DRRS simultaneously. Deferring
to PSR when both PSR and DRRS are supported by the panel.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=101111
Cc: Nicholas Stommel <nicholas.stommel@gmail.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Clinton Taylor <clinton.a.taylor@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Rodrigo Vivi Aug. 31, 2017, 12:58 a.m. UTC | #1
On Wed, 2017-08-30 at 17:32 -0700, Radhakrishna Sripada wrote:
> Some platforms donot support PSR and DRRS simultaneously. Deferring

> to PSR when both PSR and DRRS are supported by the panel.

> 

> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=101111


Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101111

"Fixes: <commit>" is only used to -fixes cherry-picks. Not a case for
this patch.

> Cc: Nicholas Stommel <nicholas.stommel@gmail.com>

> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> Cc: Jani Nikula <jani.nikula@intel.com>

> Cc: Clinton Taylor <clinton.a.taylor@intel.com>

> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>

> ---

>  drivers/gpu/drm/i915/intel_dp.c | 10 +++++-----

>  1 file changed, 5 insertions(+), 5 deletions(-)

> 

> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c

> index d3e5fdf0d2fa..dc7a6721e0dd 100644

> --- a/drivers/gpu/drm/i915/intel_dp.c

> +++ b/drivers/gpu/drm/i915/intel_dp.c

> @@ -5469,11 +5469,6 @@ static void intel_dp_set_drrs_state(struct drm_i915_private *dev_priv,

>  		return;

>  	}

>  

> -	/*

> -	 * FIXME: This needs proper synchronization with psr state for some

> -	 * platforms that cannot have PSR and DRRS enabled at the same time.

> -	 */

> -

>  	dig_port = dp_to_dig_port(intel_dp);

>  	encoder = &dig_port->base;

>  	intel_crtc = to_intel_crtc(encoder->base.crtc);

> @@ -5557,6 +5552,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp,

>  		return;

>  	}

>  

> +	if (dev_priv->psr.enabled != NULL) {


	if (dev_priv->psr.enabled) {
?

> +		DRM_DEBUG_KMS("PSR active. Disabling DRRS.\n");

> +		return;

> +	}

> +

>  	mutex_lock(&dev_priv->drrs.mutex);

>  	if (WARN_ON(dev_priv->drrs.dp)) {

>  		DRM_ERROR("DRRS already enabled\n");
Sripada, Radhakrishna Aug. 31, 2017, 5:37 p.m. UTC | #2
> -----Original Message-----

> From: Vivi, Rodrigo

> Sent: Wednesday, August 30, 2017 5:59 PM

> To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>

> Cc: intel-gfx@lists.freedesktop.org; Pandiyan, Dhinakaran

> <dhinakaran.pandiyan@intel.com>; Nikula, Jani <jani.nikula@intel.com>;

> Taylor, Clinton A <clinton.a.taylor@intel.com>; nicholas.stommel@gmail.com

> Subject: Re: [PATCH] drm/i915: Disable DRRS when PSR is enabled

> 

> On Wed, 2017-08-30 at 17:32 -0700, Radhakrishna Sripada wrote:

> > Some platforms donot support PSR and DRRS simultaneously. Deferring to

> > PSR when both PSR and DRRS are supported by the panel.

> >

> > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=101111

> 

> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101111

> 

> "Fixes: <commit>" is only used to -fixes cherry-picks. Not a case for this

> patch.

Got it. Will update in the next revision of the patch.
> 

> > Cc: Nicholas Stommel <nicholas.stommel@gmail.com>

> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> > Cc: Jani Nikula <jani.nikula@intel.com>

> > Cc: Clinton Taylor <clinton.a.taylor@intel.com>

> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>

> > ---

> >  drivers/gpu/drm/i915/intel_dp.c | 10 +++++-----

> >  1 file changed, 5 insertions(+), 5 deletions(-)

> >

> > diff --git a/drivers/gpu/drm/i915/intel_dp.c

> > b/drivers/gpu/drm/i915/intel_dp.c index d3e5fdf0d2fa..dc7a6721e0dd

> > 100644

> > --- a/drivers/gpu/drm/i915/intel_dp.c

> > +++ b/drivers/gpu/drm/i915/intel_dp.c

> > @@ -5469,11 +5469,6 @@ static void intel_dp_set_drrs_state(struct

> drm_i915_private *dev_priv,

> >  		return;

> >  	}

> >

> > -	/*

> > -	 * FIXME: This needs proper synchronization with psr state for some

> > -	 * platforms that cannot have PSR and DRRS enabled at the same

> time.

> > -	 */

> > -

> >  	dig_port = dp_to_dig_port(intel_dp);

> >  	encoder = &dig_port->base;

> >  	intel_crtc = to_intel_crtc(encoder->base.crtc);

> > @@ -5557,6 +5552,11 @@ void intel_edp_drrs_enable(struct intel_dp

> *intel_dp,

> >  		return;

> >  	}

> >

> > +	if (dev_priv->psr.enabled != NULL) {

> 

> 	if (dev_priv->psr.enabled) {

> ?

This looks cleaner will use this in the follow up patch.
> 

> > +		DRM_DEBUG_KMS("PSR active. Disabling DRRS.\n");

> > +		return;

> > +	}

> > +

> >  	mutex_lock(&dev_priv->drrs.mutex);

> >  	if (WARN_ON(dev_priv->drrs.dp)) {

> >  		DRM_ERROR("DRRS already enabled\n");
Daniel Vetter Sept. 4, 2017, 8:13 a.m. UTC | #3
On Thu, Aug 31, 2017 at 05:37:31PM +0000, Sripada, Radhakrishna wrote:
> 
> 
> > -----Original Message-----
> > From: Vivi, Rodrigo
> > Sent: Wednesday, August 30, 2017 5:59 PM
> > To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; Pandiyan, Dhinakaran
> > <dhinakaran.pandiyan@intel.com>; Nikula, Jani <jani.nikula@intel.com>;
> > Taylor, Clinton A <clinton.a.taylor@intel.com>; nicholas.stommel@gmail.com
> > Subject: Re: [PATCH] drm/i915: Disable DRRS when PSR is enabled
> > 
> > On Wed, 2017-08-30 at 17:32 -0700, Radhakrishna Sripada wrote:
> > > Some platforms donot support PSR and DRRS simultaneously. Deferring to
> > > PSR when both PSR and DRRS are supported by the panel.
> > >
> > > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=101111
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101111
> > 
> > "Fixes: <commit>" is only used to -fixes cherry-picks. Not a case for this
> > patch.
> Got it. Will update in the next revision of the patch.

Did you check igt coverage for this and make sure those platforms do blow
up somewhere?

We currently don't yet run the full panel tests (psr, drrs) in CI, but
we're slowly working on that problem too. Would be good to have the
testsuite ready already.
-Daniel

> > 
> > > Cc: Nicholas Stommel <nicholas.stommel@gmail.com>
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Cc: Clinton Taylor <clinton.a.taylor@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c index d3e5fdf0d2fa..dc7a6721e0dd
> > > 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -5469,11 +5469,6 @@ static void intel_dp_set_drrs_state(struct
> > drm_i915_private *dev_priv,
> > >  		return;
> > >  	}
> > >
> > > -	/*
> > > -	 * FIXME: This needs proper synchronization with psr state for some
> > > -	 * platforms that cannot have PSR and DRRS enabled at the same
> > time.
> > > -	 */
> > > -
> > >  	dig_port = dp_to_dig_port(intel_dp);
> > >  	encoder = &dig_port->base;
> > >  	intel_crtc = to_intel_crtc(encoder->base.crtc);
> > > @@ -5557,6 +5552,11 @@ void intel_edp_drrs_enable(struct intel_dp
> > *intel_dp,
> > >  		return;
> > >  	}
> > >
> > > +	if (dev_priv->psr.enabled != NULL) {
> > 
> > 	if (dev_priv->psr.enabled) {
> > ?
> This looks cleaner will use this in the follow up patch.
> > 
> > > +		DRM_DEBUG_KMS("PSR active. Disabling DRRS.\n");
> > > +		return;
> > > +	}
> > > +
> > >  	mutex_lock(&dev_priv->drrs.mutex);
> > >  	if (WARN_ON(dev_priv->drrs.dp)) {
> > >  		DRM_ERROR("DRRS already enabled\n");
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Sripada, Radhakrishna Sept. 11, 2017, 5:33 p.m. UTC | #4
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Monday, September 4, 2017 1:14 AM
> To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>
> Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Nikula, Jani
> <jani.nikula@intel.com>; intel-gfx@lists.freedesktop.org; Pandiyan,
> Dhinakaran <dhinakaran.pandiyan@intel.com>;
> nicholas.stommel@gmail.com
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Disable DRRS when PSR is enabled
> 
> On Thu, Aug 31, 2017 at 05:37:31PM +0000, Sripada, Radhakrishna wrote:
> >
> >
> > > -----Original Message-----
> > > From: Vivi, Rodrigo
> > > Sent: Wednesday, August 30, 2017 5:59 PM
> > > To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>
> > > Cc: intel-gfx@lists.freedesktop.org; Pandiyan, Dhinakaran
> > > <dhinakaran.pandiyan@intel.com>; Nikula, Jani
> > > <jani.nikula@intel.com>; Taylor, Clinton A
> > > <clinton.a.taylor@intel.com>; nicholas.stommel@gmail.com
> > > Subject: Re: [PATCH] drm/i915: Disable DRRS when PSR is enabled
> > >
> > > On Wed, 2017-08-30 at 17:32 -0700, Radhakrishna Sripada wrote:
> > > > Some platforms donot support PSR and DRRS simultaneously.
> > > > Deferring to PSR when both PSR and DRRS are supported by the panel.
> > > >
> > > > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=101111
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101111
> > >
> > > "Fixes: <commit>" is only used to -fixes cherry-picks. Not a case
> > > for this patch.
> > Got it. Will update in the next revision of the patch.
> 
> Did you check igt coverage for this and make sure those platforms do blow
> up somewhere?
Currently there is no igt test coverage to check if both psr and drrs are enabled at
the same time. I will add it as a subtest in tests/kms_psr_sink_crc if you prefer.
-Radhakrishna
> 
> We currently don't yet run the full panel tests (psr, drrs) in CI, but we're
> slowly working on that problem too. Would be good to have the testsuite
> ready already.
> -Daniel
> 
> > >
> > > > Cc: Nicholas Stommel <nicholas.stommel@gmail.com>
> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > > Cc: Clinton Taylor <clinton.a.taylor@intel.com>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Signed-off-by: Radhakrishna Sripada
> > > > <radhakrishna.sripada@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dp.c | 10 +++++-----
> > > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > b/drivers/gpu/drm/i915/intel_dp.c index d3e5fdf0d2fa..dc7a6721e0dd
> > > > 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -5469,11 +5469,6 @@ static void intel_dp_set_drrs_state(struct
> > > drm_i915_private *dev_priv,
> > > >  		return;
> > > >  	}
> > > >
> > > > -	/*
> > > > -	 * FIXME: This needs proper synchronization with psr state for some
> > > > -	 * platforms that cannot have PSR and DRRS enabled at the same
> > > time.
> > > > -	 */
> > > > -
> > > >  	dig_port = dp_to_dig_port(intel_dp);
> > > >  	encoder = &dig_port->base;
> > > >  	intel_crtc = to_intel_crtc(encoder->base.crtc);
> > > > @@ -5557,6 +5552,11 @@ void intel_edp_drrs_enable(struct intel_dp
> > > *intel_dp,
> > > >  		return;
> > > >  	}
> > > >
> > > > +	if (dev_priv->psr.enabled != NULL) {
> > >
> > > 	if (dev_priv->psr.enabled) {
> > > ?
> > This looks cleaner will use this in the follow up patch.
> > >
> > > > +		DRM_DEBUG_KMS("PSR active. Disabling DRRS.\n");
> > > > +		return;
> > > > +	}
> > > > +
> > > >  	mutex_lock(&dev_priv->drrs.mutex);
> > > >  	if (WARN_ON(dev_priv->drrs.dp)) {
> > > >  		DRM_ERROR("DRRS already enabled\n");
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d3e5fdf0d2fa..dc7a6721e0dd 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5469,11 +5469,6 @@  static void intel_dp_set_drrs_state(struct drm_i915_private *dev_priv,
 		return;
 	}
 
-	/*
-	 * FIXME: This needs proper synchronization with psr state for some
-	 * platforms that cannot have PSR and DRRS enabled at the same time.
-	 */
-
 	dig_port = dp_to_dig_port(intel_dp);
 	encoder = &dig_port->base;
 	intel_crtc = to_intel_crtc(encoder->base.crtc);
@@ -5557,6 +5552,11 @@  void intel_edp_drrs_enable(struct intel_dp *intel_dp,
 		return;
 	}
 
+	if (dev_priv->psr.enabled != NULL) {
+		DRM_DEBUG_KMS("PSR active. Disabling DRRS.\n");
+		return;
+	}
+
 	mutex_lock(&dev_priv->drrs.mutex);
 	if (WARN_ON(dev_priv->drrs.dp)) {
 		DRM_ERROR("DRRS already enabled\n");