[Intel-gfx] [PATCH 1/5] drm/i915/display: Limit disabling PSR around cdclk changes to ADL-P

[Souza, Jose]

On Thu, 2021-06-17 at 14:26 -0700, Srivatsa, Anusha wrote:
> 
> > -----Original Message-----
> > From: Souza, Jose <jose.souza at intel.com>
> > Sent: Thursday, June 17, 2021 2:18 PM
> > To: Mun, Gwan-gyeong <gwan-gyeong.mun at intel.com>; Srivatsa, Anusha
> > <anusha.srivatsa at intel.com>; intel-gfx at lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH 1/5] drm/i915/display: Limit disabling PSR
> > around cdclk changes to ADL-P
> > 
> > On Thu, 2021-06-17 at 14:12 -0700, Anusha Srivatsa wrote:
> > > From: Gwan-gyeong Mun <gwan-gyeong.mun at intel.com>
> > > 
> > > Only ADL-P platform requires a temporal disabling of PSR for changing
> > > the CDCLK PLL frequency with crawling. Changing the CDCLK PLL
> > > frequency on prior platforms of ADL-P or changing the CDCLK PLL
> > > frequency without crawling on ADL-P don't need to disable of PSR.
> > 
> > This is only hiding a possible bug found in ICL under the IS_ALDERLAKE_P()
> > check.
> > There is no reason to not pause PSR in older platforms during cdclck changes.
> 
> According to 15729, pausing PSR during cdclk changes is not valid. 

The bspec tag is not valid, it do not means the sequence is not valid for GEN9 and GEN11 platforms.

> 
> Anusha 
> > > 
> > > Bspec: 49207
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > Cc: Mika Kahola <mika.kahola at intel.com>
> > > Cc: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
> > > Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
> > > Fixes: 17c1a4b7ac6f ("drm/i915: Disable PSR around cdclk change")
> > > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_cdclk.c | 22
> > > ++++++++++++++++------
> > >  1 file changed, 16 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > index 613ffcc68eba..6da426d26aac 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > @@ -1962,10 +1962,18 @@ static void intel_set_cdclk(struct
> > > drm_i915_private *dev_priv,
> > > 
> > >  	intel_dump_cdclk_config(cdclk_config, "Changing CDCLK to");
> > > 
> > > -	for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) {
> > > -		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > > +	/*
> > > +	 * Only ADL-P platform requires a temporal disabling of PSR for
> > changing
> > > +	 * the CDCLK PLL frequency with crawling.
> > > +	 * Changing the CDCLK PLL frequency on prior platforms of ADL-P or
> > changing
> > > +	 * the CDCLK PLL frequency without crawling on ADL-P don't need to
> > disable of PSR.
> > > +	 */
> > > +	if (IS_ALDERLAKE_P(dev_priv)) {
> > > +		for_each_intel_encoder_with_psr(&dev_priv->drm,
> > encoder) {
> > > +			struct intel_dp *intel_dp =
> > enc_to_intel_dp(encoder);
> > > 
> > > -		intel_psr_pause(intel_dp);
> > > +			intel_psr_pause(intel_dp);
> > > +		}
> > >  	}
> > > 
> > >  	/*
> > > @@ -1990,10 +1998,12 @@ static void intel_set_cdclk(struct
> > drm_i915_private *dev_priv,
> > >  	}
> > >  	mutex_unlock(&dev_priv->gmbus_mutex);
> > > 
> > > -	for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) {
> > > -		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > > +	if (IS_ALDERLAKE_P(dev_priv)) {
> > > +		for_each_intel_encoder_with_psr(&dev_priv->drm,
> > encoder) {
> > > +			struct intel_dp *intel_dp =
> > enc_to_intel_dp(encoder);
> > > 
> > > -		intel_psr_resume(intel_dp);
> > > +			intel_psr_resume(intel_dp);
> > > +		}
> > >  	}
> > > 
> > >  	if (drm_WARN(&dev_priv->drm,
>