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

[Dhinakaran Pandiyan]

On Tue, 2018-09-11 at 17:04 +0300, Ville Syrjälä wrote:
> 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.
> 
I had to check with Art about this; we do need AUX handshake to wake up
the sink like the spec says. The current code is right.

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

>