diff mbox series

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

Message ID 20210617211225.13549-2-anusha.srivatsa@intel.com (mailing list archive)
State New, archived
Headers show
Series Pipe DMC bits + PSR fix | expand

Commit Message

Srivatsa, Anusha June 17, 2021, 9:12 p.m. UTC 
From: Gwan-gyeong Mun <gwan-gyeong.mun@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.

Bspec: 49207

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Fixes: 17c1a4b7ac6f ("drm/i915: Disable PSR around cdclk change")
Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

Comments

Souza, Jose June 17, 2021, 9:18 p.m. UTC | #1 
On Thu, 2021-06-17 at 14:12 -0700, Anusha Srivatsa wrote:
> From: Gwan-gyeong Mun <gwan-gyeong.mun@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.

> 
> Bspec: 49207
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Fixes: 17c1a4b7ac6f ("drm/i915: Disable PSR around cdclk change")
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@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,
Srivatsa, Anusha June 17, 2021, 9:26 p.m. UTC | #2 
> -----Original Message-----
> From: Souza, Jose <jose.souza@intel.com>
> Sent: Thursday, June 17, 2021 2:18 PM
> To: Mun, Gwan-gyeong <gwan-gyeong.mun@intel.com>; Srivatsa, Anusha
> <anusha.srivatsa@intel.com>; intel-gfx@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@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. 

Anusha 
> >
> > Bspec: 49207
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Fixes: 17c1a4b7ac6f ("drm/i915: Disable PSR around cdclk change")
> > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@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,
Souza, Jose June 17, 2021, 9:30 p.m. UTC | #3 
On Thu, 2021-06-17 at 14:26 -0700, Srivatsa, Anusha wrote:
> 
> > -----Original Message-----
> > From: Souza, Jose <jose.souza@intel.com>
> > Sent: Thursday, June 17, 2021 2:18 PM
> > To: Mun, Gwan-gyeong <gwan-gyeong.mun@intel.com>; Srivatsa, Anusha
> > <anusha.srivatsa@intel.com>; intel-gfx@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@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@linux.intel.com>
> > > Cc: Mika Kahola <mika.kahola@intel.com>
> > > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > Fixes: 17c1a4b7ac6f ("drm/i915: Disable PSR around cdclk change")
> > > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@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,
>
diff mbox series

Patch

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,