[PATCH] drm/i915: Aggressively downclock Baytrail

[PATCH] drm/i915: Aggressively downclock Baytrail

[Chris Wilson]

Baytrail uses the RPS wait-boosting mechanism of Sandybridge+ but also has
a very lax downclocking strategy (upclock if more than 90% busy over 76ms,
downclock if less than 70% busy over 450ms). This causes Baytrail to use
maximum clocks, and for them to stay high, when doing simple tasks such as
scrolling through webpages. However, we can take a leaf out of the same
wait-boost mechansim and apply the aggressive downclocking strategy from
Sandybridge+ as well.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_pm.c | 50 +++++++++++++----------------------------
 1 file changed, 16 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7005ea3595e2..009f81a922e4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3090,14 +3090,6 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
 		/* Downclock if less than 85% busy over 32ms */
 		I915_WRITE(GEN6_RP_DOWN_EI, 25000);
 		I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 21250);
-
-		I915_WRITE(GEN6_RP_CONTROL,
-			   GEN6_RP_MEDIA_TURBO |
-			   GEN6_RP_MEDIA_HW_NORMAL_MODE |
-			   GEN6_RP_MEDIA_IS_GFX |
-			   GEN6_RP_ENABLE |
-			   GEN6_RP_UP_BUSY_AVG |
-			   GEN6_RP_DOWN_IDLE_AVG);
 		break;
 
 	case BETWEEN:
@@ -3108,14 +3100,6 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
 		/* Downclock if less than 75% busy over 32ms */
 		I915_WRITE(GEN6_RP_DOWN_EI, 25000);
 		I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 18750);
-
-		I915_WRITE(GEN6_RP_CONTROL,
-			   GEN6_RP_MEDIA_TURBO |
-			   GEN6_RP_MEDIA_HW_NORMAL_MODE |
-			   GEN6_RP_MEDIA_IS_GFX |
-			   GEN6_RP_ENABLE |
-			   GEN6_RP_UP_BUSY_AVG |
-			   GEN6_RP_DOWN_IDLE_AVG);
 		break;
 
 	case HIGH_POWER:
@@ -3126,17 +3110,17 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
 		/* Downclock if less than 60% busy over 32ms */
 		I915_WRITE(GEN6_RP_DOWN_EI, 25000);
 		I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 15000);
-
-		I915_WRITE(GEN6_RP_CONTROL,
-			   GEN6_RP_MEDIA_TURBO |
-			   GEN6_RP_MEDIA_HW_NORMAL_MODE |
-			   GEN6_RP_MEDIA_IS_GFX |
-			   GEN6_RP_ENABLE |
-			   GEN6_RP_UP_BUSY_AVG |
-			   GEN6_RP_DOWN_IDLE_AVG);
 		break;
 	}
 
+	I915_WRITE(GEN6_RP_CONTROL,
+		   GEN6_RP_MEDIA_TURBO |
+		   GEN6_RP_MEDIA_HW_NORMAL_MODE |
+		   GEN6_RP_MEDIA_IS_GFX |
+		   GEN6_RP_ENABLE |
+		   GEN6_RP_UP_BUSY_AVG |
+		   GEN6_RP_DOWN_IDLE_AVG);
+
 	dev_priv->rps.power = new_power;
 	dev_priv->rps.last_adj = 0;
 }
@@ -3210,29 +3194,25 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
 */
 static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 {
+	u32 val = dev_priv->rps.min_freq_softlimit;
+
 	/*
 	 * When we are idle.  Drop to min voltage state.
 	 */
-
-	if (dev_priv->rps.cur_freq <= dev_priv->rps.min_freq_softlimit)
+	if (dev_priv->rps.cur_freq <= val)
 		return;
 
 	/* Mask turbo interrupt so that they will not come in between */
 	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
-
 	vlv_force_gfx_clock(dev_priv, true);
 
-	dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit;
-
-	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
-					dev_priv->rps.min_freq_softlimit);
-
+	gen6_set_rps_thresholds(dev_priv, val);
+	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
 	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
 				& GENFREQSTATUS) == 0, 5))
 		DRM_ERROR("timed out waiting for Punit\n");
 
 	vlv_force_gfx_clock(dev_priv, false);
-
 	I915_WRITE(GEN6_PMINTRMSK,
 		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
 }
@@ -3280,8 +3260,10 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
 			 dev_priv->rps.cur_freq,
 			 vlv_gpu_freq(dev_priv, val), val);
 
-	if (val != dev_priv->rps.cur_freq)
+	if (val != dev_priv->rps.cur_freq) {
+		gen6_set_rps_thresholds(dev_priv, val);
 		vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
+	}
 
 	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
 
-- 
2.0.1

Re: [PATCH] drm/i915: Aggressively downclock Baytrail

[Jesse Barnes]

On Thu,  3 Jul 2014 15:29:26 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> Baytrail uses the RPS wait-boosting mechanism of Sandybridge+ but also has
> a very lax downclocking strategy (upclock if more than 90% busy over 76ms,
> downclock if less than 70% busy over 450ms). This causes Baytrail to use
> maximum clocks, and for them to stay high, when doing simple tasks such as
> scrolling through webpages. However, we can take a leaf out of the same
> wait-boost mechansim and apply the aggressive downclocking strategy from
> Sandybridge+ as well.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---

We really need a thorough test suite to cover stuff like this, mapping
frequency, power, and total energy over a big set of workloads to make
sure we're not adding big regressions.

I know you have the cairo traces, but did you also try this with a GL
benchmark suite?

I'm like the change (well you did mix in a cleanup to
set_rps_thresholds), I just want us to get better at collecting numbers
for this stuff...

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] drm/i915: Aggressively downclock Baytrail

[Chris Wilson]

On Thu, Jul 03, 2014 at 08:49:22AM -0700, Jesse Barnes wrote:
> On Thu,  3 Jul 2014 15:29:26 +0100
> Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> > Baytrail uses the RPS wait-boosting mechanism of Sandybridge+ but also has
> > a very lax downclocking strategy (upclock if more than 90% busy over 76ms,
> > downclock if less than 70% busy over 450ms). This causes Baytrail to use
> > maximum clocks, and for them to stay high, when doing simple tasks such as
> > scrolling through webpages. However, we can take a leaf out of the same
> > wait-boost mechansim and apply the aggressive downclocking strategy from
> > Sandybridge+ as well.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> 
> We really need a thorough test suite to cover stuff like this, mapping
> frequency, power, and total energy over a big set of workloads to make
> sure we're not adding big regressions.
> 
> I know you have the cairo traces, but did you also try this with a GL
> benchmark suite?

glxgears went from max clocks to min clocks whilst hitting 60fps.

Note that you first have to disable the cmdparser to make the machine
pleasant to use.

> I'm like the change (well you did mix in a cleanup to
> set_rps_thresholds),

Actually, I left it replicated originally because they used different
strategies at one point and keeping it separate eased experimentation.

The only thing that is missing is a comment to explain that I found I
needed to rewrite the control register every time for the change in
thresholds to take effect.

> I just want us to get better at collecting numbers
> for this stuff...

It's not like we have pretty tools to overlay realtime GPU usage and
bottlenecks...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Aggressively downclock Baytrail

[Jesse Barnes]

On Thu, 3 Jul 2014 16:59:17 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Thu, Jul 03, 2014 at 08:49:22AM -0700, Jesse Barnes wrote:
> > On Thu,  3 Jul 2014 15:29:26 +0100
> > Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > 
> > > Baytrail uses the RPS wait-boosting mechanism of Sandybridge+ but also has
> > > a very lax downclocking strategy (upclock if more than 90% busy over 76ms,
> > > downclock if less than 70% busy over 450ms). This causes Baytrail to use
> > > maximum clocks, and for them to stay high, when doing simple tasks such as
> > > scrolling through webpages. However, we can take a leaf out of the same
> > > wait-boost mechansim and apply the aggressive downclocking strategy from
> > > Sandybridge+ as well.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > ---
> > 
> > We really need a thorough test suite to cover stuff like this, mapping
> > frequency, power, and total energy over a big set of workloads to make
> > sure we're not adding big regressions.
> > 
> > I know you have the cairo traces, but did you also try this with a GL
> > benchmark suite?
> 
> glxgears went from max clocks to min clocks whilst hitting 60fps.
> 
> Note that you first have to disable the cmdparser to make the machine
> pleasant to use.
> 
> > I'm like the change (well you did mix in a cleanup to
> > set_rps_thresholds),
> 
> Actually, I left it replicated originally because they used different
> strategies at one point and keeping it separate eased experimentation.
> 
> The only thing that is missing is a comment to explain that I found I
> needed to rewrite the control register every time for the change in
> thresholds to take effect.
> 
> > I just want us to get better at collecting numbers
> > for this stuff...
> 
> It's not like we have pretty tools to overlay realtime GPU usage and
> bottlenecks...

Yeah I know we have nice tools, I'm just thinking of data over time so
we can see regressions easily etc.  The power stuff QA collects ought
to catch it, but I don't think we've done specific runs with turbo
changes recently; would be good to get Wendy to do that for this patch
and see what happens.

-- 
Jesse Barnes, Intel Open Source Technology Center