[Intel-gfx] [PATCH] drm/i915/psr: Enable PSR1 on gen-9+ HW

[Ville Syrjälä]

On Thu, Sep 06, 2018 at 10:06:09PM -0700, Rodrigo Vivi wrote:
> On Thu, Sep 06, 2018 at 04:52:02PM -0700, Dhinakaran Pandiyan wrote:
> > We have new tests and fixes in place since the feature was last
> > disabled.
> > 
> > Try again for gen-9+ hardware and enable only PSR1 as a first step.
> > 
> > Cc: Jani Nikula <jani.nikula at intel.com>
> > Cc: Jose Roberto de Souza <jose.souza at intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > References: commit 2ee7dc497e34 ("drm/i915: disable PSR by default on HSW/BDW")
> > References: commit dcb2e993f3c0 ("Revert "drm/i915: Enable PSR by default on Valleyview and Cherryview."")
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 17 ++++++++---------
> >  1 file changed, 8 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index b6838b525502..fc823f93a4dc 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -71,6 +71,10 @@ static bool psr_global_enabled(u32 debug)
> >  static bool intel_psr2_enabled(struct drm_i915_private *dev_priv,
> >  			       const struct intel_crtc_state *crtc_state)
> >  {
> > +	/* Disable PSR2 by default for all platforms */
> > +	if (i915_modparams.enable_psr == -1)
> > +		return false;
> > +
> >  	switch (dev_priv->psr.debug & I915_PSR_DEBUG_MODE_MASK) {
> >  	case I915_PSR_DEBUG_FORCE_PSR1:
> >  		return false;
> > @@ -1051,7 +1055,7 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
> >   * intel_psr_init - Init basic PSR work and mutex.
> >   * @dev_priv: i915 device private
> >   *
> > - * This function is  called only once at driver load to initialize basic
> > + * This function is called only once at driver load to initialize basic
> >   * PSR stuff.
> >   */
> >  void intel_psr_init(struct drm_i915_private *dev_priv)
> > @@ -1065,19 +1069,14 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
> >  	if (!dev_priv->psr.sink_support)
> >  		return;
> >  
> > -	if (i915_modparams.enable_psr == -1) {
> > -		i915_modparams.enable_psr = dev_priv->vbt.psr.enable;
> > -
> > -		/* Per platform default: all disabled. */
> > -		i915_modparams.enable_psr = 0;
> > -	}
> > +	if (i915_modparams.enable_psr == -1)
> > +		if (INTEL_GEN(dev_priv) < 9 || !dev_priv->vbt.psr.enable)
> > +			i915_modparams.enable_psr = 0;
> >  
> > -	/* Set link_standby x link_off defaults */
> >  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> >  		/* HSW and BDW require workarounds that we don't implement. */
> >  		dev_priv->psr.link_standby = false;
> >  	else
> > -		/* For new platforms let's respect VBT back again */
> 
> bikeshed: Can we please leave the clean-up for a separated patch?
> In case we need to revert we don't loose the clean-up part! :$
> 
> Also a bikeshed of bikeshed: I think we need to revisit this block entirely
> anyways. I can't remember why we stopped respecting the bspec here.
> And probably this was only masking some issues that got fixed during
> your great journey! ;)

Another vbt related thing was the aux handshake thing. We tried it here
https://patchwork.freedesktop.org/series/8046/ but IIRC it caused some
problems that no one had time to diagnose so we never merged that stuff.
Not sure if anyone wants to try and figure out what went wrong there.

Actually, after a bit more digging I guess the fails were listed here
https://lists.freedesktop.org/archives/intel-gfx/2016-June/097379.html
Some sink crc issues, but as that was deemed unusable anyway maybe
there was nothing wrong after all?

-- 
Ville Syrjälä
Intel