diff mbox

drm/i915: respect VBT PSR link standby configuration for HSW/BDW.

Message ID 1461772074-15806-1-git-send-email-rodrigo.vivi@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rodrigo Vivi April 27, 2016, 3:47 p.m. UTC
We have in the history some changes on this behaviour, but
there are many platforms out there and we don't know all panels.

VBT might not be reliable but it knows the platform better than
us usually. Or at least it should.
So, first of all let's respect the VBT. If something bad happens
again with one platform or another it is better to create a
quirk than to bypass the VBT.

Cc: Mihai Dontu <mihai.dontu@gmail.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Comments

Chris Wilson April 27, 2016, 3:53 p.m. UTC | #1
On Wed, Apr 27, 2016 at 08:47:54AM -0700, Rodrigo Vivi wrote:
> We have in the history some changes on this behaviour, but
> there are many platforms out there and we don't know all panels.
> 
> VBT might not be reliable but it knows the platform better than
> us usually. Or at least it should.
> So, first of all let's respect the VBT. If something bad happens
> again with one platform or another it is better to create a
> quirk than to bypass the VBT.
> 
> Cc: Mihai Dontu <mihai.dontu@gmail.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index c3abae4..e65e2c3 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -788,14 +788,11 @@ void intel_psr_init(struct drm_device *dev)
>  	}
>  
>  	/* Set link_standby x link_off defaults */
> -	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> -		/* HSW and BDW require workarounds that we don't implement. */
> -		dev_priv->psr.link_standby = false;

This patch has nothing to do with respecting the VBT or not, it's about
whether the comment that we still require w/a is valid or not.

That's not even mentioned in the changelog.
-Chris
Rodrigo Vivi April 27, 2016, 5:31 p.m. UTC | #2
On Wed, 2016-04-27 at 16:53 +0100, Chris Wilson wrote:
> On Wed, Apr 27, 2016 at 08:47:54AM -0700, Rodrigo Vivi wrote:

> > We have in the history some changes on this behaviour, but

> > there are many platforms out there and we don't know all panels.

> > 

> > VBT might not be reliable but it knows the platform better than

> > us usually. Or at least it should.

> > So, first of all let's respect the VBT. If something bad happens

> > again with one platform or another it is better to create a

> > quirk than to bypass the VBT.

> > 

> > Cc: Mihai Dontu <mihai.dontu@gmail.com>

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

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

> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> > ---

> >  drivers/gpu/drm/i915/intel_psr.c | 5 +----

> >  1 file changed, 1 insertion(+), 4 deletions(-)

> > 

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

> > b/drivers/gpu/drm/i915/intel_psr.c

> > index c3abae4..e65e2c3 100644

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

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

> > @@ -788,14 +788,11 @@ void intel_psr_init(struct drm_device *dev)

> >  	}

> >  

> >  	/* Set link_standby x link_off defaults */

> > -	if (IS_HASWELL(dev) || IS_BROADWELL(dev))

> > -		/* HSW and BDW require workarounds that we don't

> > implement. */

> > -		dev_priv->psr.link_standby = false;

> 

> This patch has nothing to do with respecting the VBT or not, it's

> about

> whether the comment that we still require w/a is valid or not.

> 

> That's not even mentioned in the changelog.


Oh you are right... Looking to the logs and HSD it seems that we still
need the workaround on HSW and BDW, since HSW has no single frame
update and we don't implement on BDW because it requires other 3 W/a...

Maybe we should just disable PSR for the cases VBT tells only link
standby is supported on HSW and BDW.

> -Chris

>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index c3abae4..e65e2c3 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -788,14 +788,11 @@  void intel_psr_init(struct drm_device *dev)
 	}
 
 	/* Set link_standby x link_off defaults */
-	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
-		/* HSW and BDW require workarounds that we don't implement. */
-		dev_priv->psr.link_standby = false;
 	else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
 		/* On VLV and CHV only standby mode is supported. */
 		dev_priv->psr.link_standby = true;
 	else
-		/* For new platforms let's respect VBT back again */
+		/* For other platforms let's respect VBT back again */
 		dev_priv->psr.link_standby = dev_priv->vbt.psr.full_link;
 
 	/* Override link_standby x link_off defaults */