diff mbox

[v2] drm/i915/skl: Retrieve the Rpe value from Pcode

Message ID 1434101468-14889-1-git-send-email-akash.goel@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

akash.goel@intel.com June 12, 2015, 9:31 a.m. UTC
From: Akash Goel <akash.goel@intel.com>

Read the efficient frequency (aka RPe) value through the the mailbox
command (0x1A) from the pcode, as done on Haswell and Broadwell.
The turbo minimum frequency softlimit is not revised as per the
efficient frequency value.

v2: Replaced the conditional expression operator with 'if' statement (Tom)

Issue: VIZ-5143
Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Ville Syrjala June 12, 2015, 10:32 a.m. UTC | #1
On Fri, Jun 12, 2015 at 03:01:08PM +0530, akash.goel@intel.com wrote:
> From: Akash Goel <akash.goel@intel.com>
> 
> Read the efficient frequency (aka RPe) value through the the mailbox
> command (0x1A) from the pcode, as done on Haswell and Broadwell.
> The turbo minimum frequency softlimit is not revised as per the
> efficient frequency value.
> 
> v2: Replaced the conditional expression operator with 'if' statement (Tom)
> 
> Issue: VIZ-5143
> Signed-off-by: Akash Goel <akash.goel@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d091fec..21b22a7 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4314,16 +4314,20 @@ static void gen6_init_rps_frequencies(struct drm_device *dev)
>  	dev_priv->rps.max_freq		= dev_priv->rps.rp0_freq;
>  
>  	dev_priv->rps.efficient_freq = dev_priv->rps.rp1_freq;
> -	if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> +	if (IS_HASWELL(dev) || IS_BROADWELL(dev) || IS_SKYLAKE(dev)) {
>  		ret = sandybridge_pcode_read(dev_priv,
>  					HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
>  					&ddcc_status);
> -		if (0 == ret)
> +		if (0 == ret) {
>  			dev_priv->rps.efficient_freq =
>  				clamp_t(u8,
>  					((ddcc_status >> 8) & 0xff),
>  					dev_priv->rps.min_freq,
>  					dev_priv->rps.max_freq);

That's wrong now since min/max_freq were already multiplied by
GEN9_FREQ_SCALER.

> +
> +			if (IS_SKYLAKE(dev))
> +			      dev_priv->rps.efficient_freq *= GEN9_FREQ_SCALER;
> +		}
>  	}

I would suggest moving all the GEN9_FREQ_SCALER multiplications here.

>  
>  	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
> -- 
> 1.9.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
akash.goel@intel.com June 12, 2015, 11:48 a.m. UTC | #2
On Fri, 2015-06-12 at 13:32 +0300, Ville Syrjälä wrote:
> On Fri, Jun 12, 2015 at 03:01:08PM +0530, akash.goel@intel.com wrote:
> > From: Akash Goel <akash.goel@intel.com>
> > 
> > Read the efficient frequency (aka RPe) value through the the mailbox
> > command (0x1A) from the pcode, as done on Haswell and Broadwell.
> > The turbo minimum frequency softlimit is not revised as per the
> > efficient frequency value.
> > 
> > v2: Replaced the conditional expression operator with 'if' statement (Tom)
> > 
> > Issue: VIZ-5143
> > Signed-off-by: Akash Goel <akash.goel@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index d091fec..21b22a7 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4314,16 +4314,20 @@ static void gen6_init_rps_frequencies(struct drm_device *dev)
> >  	dev_priv->rps.max_freq		= dev_priv->rps.rp0_freq;
> >  
> >  	dev_priv->rps.efficient_freq = dev_priv->rps.rp1_freq;
> > -	if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> > +	if (IS_HASWELL(dev) || IS_BROADWELL(dev) || IS_SKYLAKE(dev)) {
> >  		ret = sandybridge_pcode_read(dev_priv,
> >  					HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
> >  					&ddcc_status);
> > -		if (0 == ret)
> > +		if (0 == ret) {
> >  			dev_priv->rps.efficient_freq =
> >  				clamp_t(u8,
> >  					((ddcc_status >> 8) & 0xff),
> >  					dev_priv->rps.min_freq,
> >  					dev_priv->rps.max_freq);
> 
> That's wrong now since min/max_freq were already multiplied by
> GEN9_FREQ_SCALER.

Thanks for catching this issue. Sorry for the lapse.

> > +
> > +			if (IS_SKYLAKE(dev))
> > +			      dev_priv->rps.efficient_freq *= GEN9_FREQ_SCALER;
> > +		}
> >  	}
> 
> I would suggest moving all the GEN9_FREQ_SCALER multiplications here.
> 
Fine this will be cleaner.
Can I do this movement as a part of this patch only ?

Best regards
Akash

> >  
> >  	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
> > -- 
> > 1.9.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Shuang He June 28, 2015, 1:25 p.m. UTC | #3
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6562
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                                  303/303              303/303
SNB                                  312/312              312/312
IVB                                  343/343              343/343
BYT                                  284/284              284/284
HSW                                  380/380              380/380
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
Note: You need to pay more attention to line start with '*'
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d091fec..21b22a7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4314,16 +4314,20 @@  static void gen6_init_rps_frequencies(struct drm_device *dev)
 	dev_priv->rps.max_freq		= dev_priv->rps.rp0_freq;
 
 	dev_priv->rps.efficient_freq = dev_priv->rps.rp1_freq;
-	if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
+	if (IS_HASWELL(dev) || IS_BROADWELL(dev) || IS_SKYLAKE(dev)) {
 		ret = sandybridge_pcode_read(dev_priv,
 					HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
 					&ddcc_status);
-		if (0 == ret)
+		if (0 == ret) {
 			dev_priv->rps.efficient_freq =
 				clamp_t(u8,
 					((ddcc_status >> 8) & 0xff),
 					dev_priv->rps.min_freq,
 					dev_priv->rps.max_freq);
+
+			if (IS_SKYLAKE(dev))
+			      dev_priv->rps.efficient_freq *= GEN9_FREQ_SCALER;
+		}
 	}
 
 	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;