drm/i915/vlv: Remove wait for for punit to updates freq.

From: Deepak S <[email protected]>

It is obsered on BSW that requesting a new frequency from Punit does nothing when the GPU is in rc6, and if we let it enter rc6 with a high frequency Vnn also remains high.

Extending vlv_set_rps_idle()
workaround on CHV/BSW.

suggested-by Ville Syrjälä <[email protected]>

Signed-off-by Deepak S <[email protected]>

---
 drivers/gpu/drm/i915/intel_pm.c | 7 -------
 1 file changed, 7 deletions(-)

drivers/gpu/drm/i915/intel_pm.c +0 -7

index 3df929a..852f756 100644
@@ -4052,15 +4052,8 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
 */
 static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 {
-	struct drm_device *dev = dev_priv->dev;
 	u32 val = dev_priv->rps.idle_freq;
 
-	/* CHV don't need to force the gfx clock */
-	if (IS_CHERRYVIEW(dev)) {
-		valleyview_set_rps(dev_priv->dev, val);
-		return;
-	}
-
 	if (dev_priv->rps.cur_freq <= val)
 		return;
 

Signature · 1 line

1.9.1

From: Deepak S <[email protected]>

After feedback from the hardware team, now we set the GPU min/idel freq to RPe. Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the frequency to RPn, punit is failing to change the input voltage to minimum :(

Since Punit validates the rps range [RPe, RP0]. This patch removes unused cherryview_rps_min_freq function.

v2: Change commit message

v3: set min_freq before idle_freq (chris)

v4: Squash 'Remove unused rps min function' patch

Signed-off-by Deepak S <[email protected]>

Acked-by Chris Wilson <[email protected]>

---
 drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 deletions(-)

drivers/gpu/drm/i915/intel_pm.c +2 -19

index 852f756..b6b14a4 100644
@@ -4689,24 +4689,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
 	return rp1;
 }
 
-static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
-{
-	struct drm_device *dev = dev_priv->dev;
-	u32 val, rpn;
-
-	if (dev->pdev->revision >= 0x20) {
-		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
-		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
-		       FB_GFX_FREQ_FUSE_MASK);
-	} else { /* For pre-production hardware */
-		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
-		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
-		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
-	}
-
-	return rpn;
-}
-
 static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
 {
 	u32 val, rp1;

@@ -4958,7 +4940,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
 			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
 			 dev_priv->rps.rp1_freq);
 
-	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
+	/* PUnit validated range is only [RPe, RP0] */
+	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
 	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
 			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
 			 dev_priv->rps.min_freq);

Signature · 1 line

1.9.1

Quoted 6 lines · On Fri, May 08, 2015 at 08:43:12PM +0530, [email protected] wrote:

From: Deepak S <[email protected]>

After feedback from the hardware team, now we set the GPU min/idel freq to RPe. Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the frequency to RPn, punit is failing to change the input voltage to minimum :(

As I mentioned I've been unable to reproduce that particular problem on my BSW. Perhaps add a note about that in the commit message.

Quoted 2 lines

Since Punit validates the rps range [RPe, RP0]. This patch removes unused cherryview_rps_min_freq function.

But I can accept that we should stick to the validated range, so I can slap an r-b on the patch anyway:

Reviewed-by Ville Syrjälä <[email protected]>

Quoted 45 lines

v2: Change commit message

v3: set min_freq before idle_freq (chris)

v4: Squash 'Remove unused rps min function' patch

Signed-off-by Deepak S <[email protected]>

Acked-by Chris Wilson <[email protected]>

---
  drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
  1 file changed, 2 insertions(+), 19 deletions(-)

drivers/gpu/drm/i915/intel_pm.c +0 -18

index 852f756..b6b14a4 100644
@@ -4689,24 +4689,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
  	return rp1;
  }
 
-static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
-{
-	struct drm_device *dev = dev_priv->dev;
-	u32 val, rpn;
-
-	if (dev->pdev->revision >= 0x20) {
-		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
-		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
-		       FB_GFX_FREQ_FUSE_MASK);
-	} else { /* For pre-production hardware */
-		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
-		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
-		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
-	}
-
-	return rpn;
-}
-
  static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
  {
  	u32 val, rp1;

@@ -4958,7 +4940,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
  			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
  			 dev_priv->rps.rp1_freq);
 

Quoted 8 lines

drivers/gpu/drm/i915/intel_pm.c +2 -1

-	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
+	/* PUnit validated range is only [RPe, RP0] */
+	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
  	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
  			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
  			 dev_priv->rps.min_freq);

Signature · 1 line

1.9.1

From: Deepak S <[email protected]>

After feedback from the hardware team, now we set the GPU min/idel freq to RPe. Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the frequency to RPn, punit is failing to change the vgg input voltage to minimum :(

Since Punit validates the rps range [RPe, RP0]. This patch removes unused cherryview_rps_min_freq function.

v2: Change commit message

v3: set min_freq before idle_freq (chris)

v4: Squash 'Remove unused rps min function' patch

Signed-off-by Deepak S <[email protected]>

Acked-by Chris Wilson <[email protected]>

Reviewed-by Ville Syrjälä <[email protected]>

---
 drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 deletions(-)

drivers/gpu/drm/i915/intel_pm.c +2 -19

index 064f11a..c229d7e 100644
@@ -4689,24 +4689,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
 	return rp1;
 }
 
-static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
-{
-	struct drm_device *dev = dev_priv->dev;
-	u32 val, rpn;
-
-	if (dev->pdev->revision >= 0x20) {
-		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
-		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
-		       FB_GFX_FREQ_FUSE_MASK);
-	} else { /* For pre-production hardware */
-		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
-		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
-		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
-	}
-
-	return rpn;
-}
-
 static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
 {
 	u32 val, rp1;

@@ -4958,7 +4940,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
 			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
 			 dev_priv->rps.rp1_freq);
 
-	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
+	/* PUnit validated range is only [RPe, RP0] */
+	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
 	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
 			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
 			 dev_priv->rps.min_freq);

Signature · 6 lines

1.9.1

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Quoted 3 lines · Ville Syrjälä

Quoted 2 lines · On Fri, May 08, 2015 at 08:43:12PM +0530, [email protected] wrote:

From: Deepak S <[email protected]>

Quoted 7 lines

Quoted 4 lines

After feedback from the hardware team, now we set the GPU min/idel freq to RPe. Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the frequency to RPn, punit is failing to change the input voltage to minimum :(

As I mentioned I've been unable to reproduce that particular problem on my BSW. Perhaps add a note about that in the commit message.

Issue is Vgg_in voltage not getting dropped :(. Vnn observation is same as what your seeing.

We saw this issue on CHV platform and confirmed by punit team. Let me update the commit msg.

Quoted 59 lines

Quoted 2 lines

Since Punit validates the rps range [RPe, RP0]. This patch removes unused cherryview_rps_min_freq function.

But I can accept that we should stick to the validated range, so I can slap an r-b on the patch anyway:

Reviewed-by Ville Syrjälä <[email protected]>

Quoted 53 lines

v2: Change commit message

v3: set min_freq before idle_freq (chris)

v4: Squash 'Remove unused rps min function' patch

Signed-off-by Deepak S <[email protected]>

Acked-by Chris Wilson <[email protected]>

---
  drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
  1 file changed, 2 insertions(+), 19 deletions(-)

drivers/gpu/drm/i915/intel_pm.c +2 -19

index 852f756..b6b14a4 100644
@@ -4689,24 +4689,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
  	return rp1;
  }
 
-static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
-{
-	struct drm_device *dev = dev_priv->dev;
-	u32 val, rpn;
-
-	if (dev->pdev->revision >= 0x20) {
-		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
-		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
-		       FB_GFX_FREQ_FUSE_MASK);
-	} else { /* For pre-production hardware */
-		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
-		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
-		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
-	}
-
-	return rpn;
-}
-
  static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
  {
  	u32 val, rp1;

@@ -4958,7 +4940,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
  			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
  			 dev_priv->rps.rp1_freq);
 
-	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
+	/* PUnit validated range is only [RPe, RP0] */
+	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
  	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
  			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
  			 dev_priv->rps.min_freq);

Signature · 1 line

1.9.1

From: Deepak S <[email protected]>

When GPU is idle on VLV, Request freq to punit should be good enough to get the voltage back to VNN. Also, make sure gfx clock force applies before requesting the freq fot vlv.

v2: Do forcewake before setting idle frequency (ville)
Update function comments to match the code (Deepak)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75244

suggested-by Jesse Barnes <[email protected]>

Signed-off-by Deepak S <[email protected]>

---
 drivers/gpu/drm/i915/intel_pm.c | 41 +++++++++++------------------------------
 1 file changed, 11 insertions(+), 30 deletions(-)

drivers/gpu/drm/i915/intel_pm.c +6 -25

index a7516ed..3df929a 100644
@@ -4043,51 +4043,32 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
 	trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val));
 }
 
-/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
+/* vlv_set_rps_idle: Set the frequency to idle, if Gfx clocks are down
  *
  * * If Gfx is Idle, then
- * 1. Mask Turbo interrupts
- * 2. Bring up Gfx clock
- * 3. Change the freq to Rpn and wait till P-Unit updates freq
- * 4. Clear the Force GFX CLK ON bit so that Gfx can down
- * 5. Unmask Turbo interrupts
+ * 1. Forcewake Media well.
+ * 2. Request idle freq.
+ * 3. Release Forcewake of Media well.
 */
 static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 	u32 val = dev_priv->rps.idle_freq;
 
-	/* CHV and latest VLV don't need to force the gfx clock */
-	if (IS_CHERRYVIEW(dev) || dev->pdev->revision >= 0xd) {
+	/* CHV don't need to force the gfx clock */
+	if (IS_CHERRYVIEW(dev)) {
 		valleyview_set_rps(dev_priv->dev, val);
 		return;
 	}
 
-	/*
-	 * When we are idle.  Drop to min voltage state.
-	 */
-
 	if (dev_priv->rps.cur_freq <= val)
 		return;
 
-	/* Mask turbo interrupt so that they will not come in between */
-	I915_WRITE(GEN6_PMINTRMSK,
-		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
-
-	vlv_force_gfx_clock(dev_priv, true);
-
-	dev_priv->rps.cur_freq = 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, 100))
-		DRM_ERROR("timed out waiting for Punit\n");

drivers/gpu/drm/i915/intel_pm.c +0 -0

 }
 
 void gen6_rps_busy(struct drm_i915_private *dev_priv)

Signature · 1 line

1.9.1

Quoted 70 lines · On Fri, May 08, 2015 at 08:43:10PM +0530, [email protected] wrote:

From: Deepak S <[email protected]>

When GPU is idle on VLV, Request freq to punit should be good enough to get the voltage back to VNN. Also, make sure gfx clock force applies before requesting the freq fot vlv.

v2: Do forcewake before setting idle frequency (ville)
Update function comments to match the code (Deepak)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75244

suggested-by Jesse Barnes <[email protected]>

Signed-off-by Deepak S <[email protected]>

---
  drivers/gpu/drm/i915/intel_pm.c | 41 +++++++++++------------------------------
  1 file changed, 11 insertions(+), 30 deletions(-)

drivers/gpu/drm/i915/intel_pm.c +6 -25

index a7516ed..3df929a 100644
@@ -4043,51 +4043,32 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
  	trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val));
  }
 
-/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
+/* vlv_set_rps_idle: Set the frequency to idle, if Gfx clocks are down
   *
   * * If Gfx is Idle, then
- * 1. Mask Turbo interrupts
- * 2. Bring up Gfx clock
- * 3. Change the freq to Rpn and wait till P-Unit updates freq
- * 4. Clear the Force GFX CLK ON bit so that Gfx can down
- * 5. Unmask Turbo interrupts
+ * 1. Forcewake Media well.
+ * 2. Request idle freq.
+ * 3. Release Forcewake of Media well.
  */
  static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
  {
  	struct drm_device *dev = dev_priv->dev;
  	u32 val = dev_priv->rps.idle_freq;
 
-	/* CHV and latest VLV don't need to force the gfx clock */
-	if (IS_CHERRYVIEW(dev) || dev->pdev->revision >= 0xd) {
+	/* CHV don't need to force the gfx clock */
+	if (IS_CHERRYVIEW(dev)) {
  		valleyview_set_rps(dev_priv->dev, val);
  		return;
  	}
 
-	/*
-	 * When we are idle.  Drop to min voltage state.
-	 */
-
  	if (dev_priv->rps.cur_freq <= val)
  		return;
 
-	/* Mask turbo interrupt so that they will not come in between */
-	I915_WRITE(GEN6_PMINTRMSK,
-		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
-
-	vlv_force_gfx_clock(dev_priv, true);
-
-	dev_priv->rps.cur_freq = 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, 100))
-		DRM_ERROR("timed out waiting for Punit\n");

drivers/gpu/drm/i915/intel_pm.c static void valleyview_set_rps(struct drm_device *dev, u8 val) · line 4071

Quoted 10 lines

drivers/gpu/drm/i915/intel_pm.c +5 -5

-
-	gen6_set_rps_thresholds(dev_priv, val);
-	vlv_force_gfx_clock(dev_priv, false);
-
-	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
+	/* Wake up the media well, as that takes a lot less
+	 * power than the Render well. */
+	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);
+	valleyview_set_rps(dev_priv->dev, val);
+	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);

_put

With that fixed this is:

Reviewed-by Ville Syrjälä <[email protected]>

Quoted 5 lines

drivers/gpu/drm/i915/intel_pm.c +0 -0

  }
 
  void gen6_rps_busy(struct drm_i915_private *dev_priv)

Signature · 1 line

1.9.1