Skip to contentSkip to conversation

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

PATCH Superseded [email protected] 6 participants 14 messages Mar 5, 2015 – May 4, 2015

Subsystems (2)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

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

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.

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

---
 drivers/gpu/drm/i915/intel_pm.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +4 -16
index e710b43..2e1ed07 100644
@@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
  * * 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
+ * 3. Request the freq to Rpn.
  * 4. Clear the Force GFX CLK ON bit so that Gfx can down
  * 5. Unmask Turbo interrupts
 */
@@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 
-	/* 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, dev_priv->rps.min_freq_softlimit);
 		return;
 	}
@@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
 
 	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);
-
-	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
-				& GENFREQSTATUS) == 0, 100))
-		DRM_ERROR("timed out waiting for Punit\n");
-
+	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
 	vlv_force_gfx_clock(dev_priv, false);
-
-	I915_WRITE(GEN6_PMINTRMSK,
-		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
 }
 
 void gen6_rps_idle(struct drm_i915_private *dev_priv)
Signature · 6 lines
1.9.1

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

PATCH 1/1 drm/i915: Update PM interrupts before updating the freq

From: Deepak S <[email protected]>

We update the GT PM interrupts mask at the end of set rps. We observed even though we are requesting a RPn or RP0, there is a chance to get a DOWN or UP interrupts before interrupts mask. These extra interrupts are simply wasting cpu cycles. In this patch we mask the interrupts for given freq before requesting new frequency.

---
 drivers/gpu/drm/i915/intel_pm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +2 -2
index 2e1ed07..bbfe4f0 100644
@@ -3879,12 +3879,12 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
 	if (WARN_ONCE(IS_CHERRYVIEW(dev) && (val & 1),
 		      "Odd GPU freq value\n"))
 		val &= ~1;
+	
+	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
 
 	if (val != dev_priv->rps.cur_freq)
 		vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
 
-	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
-
 	dev_priv->rps.cur_freq = val;
 	trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val));
 }
Signature · 6 lines
1.9.1

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Tested-by on v1 1/1
#3 lore raw
In reply to [email protected]

Task id: 5889 -------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV -1 280/280 279/280

ILK                                  308/308              308/308
SNB                                  328/328              328/328
IVB                                  379/379              379/379
BYT                                  294/294              294/294
HSW                 -1              387/387              386/387
BDW                 -1              316/316              315/316
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt_gem_userptr_blits_minor-unsync-interruptible      PASS(2)      DMESG_WARN(2)
*HSW  igt_gem_storedw_loop_bsd      PASS(2)      DMESG_WARN(1)PASS(1)
*BDW  igt_gem_gtt_hog      PASS(4)      DMESG_WARN(1)PASS(1)

Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list [email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Tested-by on v1 1/1
#2 lore raw

Re: [PATCH] drm/i915: Update PM interrupts before updating the freq

In reply to [email protected]

Task id: 5890 -------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied

PNV                 -1              280/280              279/280
ILK                 -1              308/308              307/308
SNB                 -18              328/328              310/328
IVB                                  379/379              379/379
BYT                                  294/294              294/294
HSW                                  387/387              387/387
BDW                 -1              316/316              315/316
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt_gem_userptr_blits_minor-unsync-normal      DMESG_WARN(1)PASS(5)      DMESG_WARN(1)PASS(1)
*ILK  igt_gem_unfence_active_buffers      PASS(2)      DMESG_WARN(1)PASS(1)
*SNB  igt_kms_rotation_crc_primary-rotation      NSPT(2)DMESG_WARN(1)PASS(3)      DMESG_FAIL(1)NSPT(1)
 SNB  igt_kms_rotation_crc_sprite-rotation      NSPT(2)DMESG_WARN(1)PASS(4)      NSPT(2)
 SNB  igt_pm_rpm_cursor      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_cursor-dpms      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_dpms-mode-unset-non-lpsp      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_dpms-non-lpsp      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_drm-resources-equal      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_fences      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_fences-dpms      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_gem-execbuf      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_gem-mmap-cpu      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_gem-mmap-gtt      NSPT(2)DMESG_WARN(1)PASS(3)      NSPT(2)
 SNB  igt_pm_rpm_gem-pread      NSPT(2)DMESG_WARN(1)PASS(1)      NSPT(2)
 SNB  igt_pm_rpm_i2c      NSPT(2)DMESG_WARN(1)PASS(1)      NSPT(2)
 SNB  igt_pm_rpm_modeset-non-lpsp      NSPT(2)DMESG_WARN(1)PASS(1)      NSPT(2)
 SNB  igt_pm_rpm_modeset-non-lpsp-stress-no-wait      NSPT(2)DMESG_WARN(1)PASS(1)      NSPT(2)
 SNB  igt_pm_rpm_pci-d3-state      NSPT(2)DMESG_WARN(1)PASS(1)      NSPT(2)
 SNB  igt_pm_rpm_rte      NSPT(2)DMESG_WARN(1)PASS(1)      NSPT(2)
*BDW  igt_gem_gtt_hog      PASS(6)      DMESG_WARN(1)PASS(1)

Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list [email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

maintainer
#4 lore raw
In reply to [email protected]
Quoted 9 lines · On Thu, 05 Mar 2015, [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.

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

Deepak, these patches seem to have fallen through the cracks. Are they still valid? Please rebase and repost if they are.

Ville, your opinion also appreciated.

BR,
Jani.

Quoted 57 lines
---
 drivers/gpu/drm/i915/intel_pm.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +4 -16
index e710b43..2e1ed07 100644
@@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
  * * 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
+ * 3. Request the freq to Rpn.
  * 4. Clear the Force GFX CLK ON bit so that Gfx can down
  * 5. Unmask Turbo interrupts
 */
@@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 
-	/* 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, dev_priv->rps.min_freq_softlimit);
 		return;
 	}
@@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
 
 	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);
-
-	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
-				& GENFREQSTATUS) == 0, 100))
-		DRM_ERROR("timed out waiting for Punit\n");
-
+	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
 	vlv_force_gfx_clock(dev_priv, false);
-
-	I915_WRITE(GEN6_PMINTRMSK,
-		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
 }
 
 void gen6_rps_idle(struct drm_i915_private *dev_priv)
Signature · 6 lines
1.9.1

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#5 lore raw
Quoted 7 lines · Jani Nikula
Quoted 6 lines · On Thu, 05 Mar 2015, [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.

Quoted 8 lines
Quoted 3 lines

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

Deepak, these patches seem to have fallen through the cracks. Are they still valid? Please rebase and repost if they are.

Ville, your opinion also appreciated.

I don't I have any VLVs old enough to hit this, so can't really confirm one way or the other.

Quoted 27 lines

BR,
Jani.

Quoted 21 lines
---
 drivers/gpu/drm/i915/intel_pm.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +1 -1
index e710b43..2e1ed07 100644
@@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
  * * 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
+ * 3. Request the freq to Rpn.
  * 4. Clear the Force GFX CLK ON bit so that Gfx can down
  * 5. Unmask Turbo interrupts
 */
@@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 
Quoted 4 lines

drivers/gpu/drm/i915/intel_pm.c static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) · line 3906

Quoted 4 lines
drivers/gpu/drm/i915/intel_pm.c +2 -2
-	/* 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)) {

Why was the stepping check removed?

Quoted 39 lines
Quoted 32 lines
drivers/gpu/drm/i915/intel_pm.c +1 -13
 		valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
 		return;
 	}
@@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
 
 	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);
-
-	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
-				& GENFREQSTATUS) == 0, 100))
-		DRM_ERROR("timed out waiting for Punit\n");
-
+	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
 	vlv_force_gfx_clock(dev_priv, false);
-
-	I915_WRITE(GEN6_PMINTRMSK,
-		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
 }
 
 void gen6_rps_idle(struct drm_i915_private *dev_priv)
Signature · 6 lines
1.9.1

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 6 lines
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#6 lore raw
Quoted 11 lines · Ville Syrjälä
Quoted 10 lines · Jani Nikula
Quoted 9 lines · On Thu, 05 Mar 2015, [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.

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

Quoted 6 lines
Quoted 4 lines

Deepak, these patches seem to have fallen through the cracks. Are they still valid? Please rebase and repost if they are.

Ville, your opinion also appreciated.

I don't I have any VLVs old enough to hit this, so can't really confirm one way or the other.

Hi Jesse, Can you please review the patch?

Thanks
Deepak

Quoted 26 lines
Quoted 26 lines

BR,
Jani.

Quoted 21 lines
---
  drivers/gpu/drm/i915/intel_pm.c | 20 ++++----------------
  1 file changed, 4 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +1 -1
index e710b43..2e1ed07 100644
@@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
   * * 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
+ * 3. Request the freq to Rpn.
   * 4. Clear the Force GFX CLK ON bit so that Gfx can down
   * 5. Unmask Turbo interrupts
  */
@@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
  {
  	struct drm_device *dev = dev_priv->dev;
 
Quoted 44 lines
Quoted 4 lines

drivers/gpu/drm/i915/intel_pm.c static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) · line 3906

Quoted 4 lines
drivers/gpu/drm/i915/intel_pm.c +2 -2
-	/* 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)) {

Why was the stepping check removed?

Quoted 38 lines
Quoted 32 lines
drivers/gpu/drm/i915/intel_pm.c +1 -13
  		valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
  		return;
  	}
@@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
  		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
 
  	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);
-
-	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
-				& GENFREQSTATUS) == 0, 100))
-		DRM_ERROR("timed out waiting for Punit\n");
-
+	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
  	vlv_force_gfx_clock(dev_priv, false);
-
-	I915_WRITE(GEN6_PMINTRMSK,
-		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
  }
 
  void gen6_rps_idle(struct drm_i915_private *dev_priv)
Signature · 6 lines
1.9.1

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Reviewed-by on v1 1/1
#7 lore raw
In reply to [email protected]
Quoted 50 lines · On 03/04/2015 08:08 PM, [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.

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

---
 drivers/gpu/drm/i915/intel_pm.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +3 -11
index e710b43..2e1ed07 100644
@@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
  * * 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
+ * 3. Request the freq to Rpn.
  * 4. Clear the Force GFX CLK ON bit so that Gfx can down
  * 5. Unmask Turbo interrupts
 */
@@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 
-	/* 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, dev_priv->rps.min_freq_softlimit);
 		return;
 	}
@@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
 
 	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);
-
-	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
-				& GENFREQSTATUS) == 0, 100))

drivers/gpu/drm/i915/intel_pm.c static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) · line 3927

Quoted 10 lines
drivers/gpu/drm/i915/intel_pm.c +1 -5
-		DRM_ERROR("timed out waiting for Punit\n");
-
+	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
 	vlv_force_gfx_clock(dev_priv, false);
-
-	I915_WRITE(GEN6_PMINTRMSK,
-		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
 }
 
 void gen6_rps_idle(struct drm_i915_private *dev_priv)

Yeah I think this is fine (may need a rebase though, you can keep my r-b if you do that in case Jani doesn't want to deal with the merge conflicts).

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

#8 lore raw
Quoted 59 lines · Jesse Barnes
Quoted 58 lines · On 03/04/2015 08:08 PM, [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.

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

---
 drivers/gpu/drm/i915/intel_pm.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +4 -16
index e710b43..2e1ed07 100644
@@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
  * * 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
+ * 3. Request the freq to Rpn.
  * 4. Clear the Force GFX CLK ON bit so that Gfx can down
  * 5. Unmask Turbo interrupts
 */
@@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 
-	/* 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, dev_priv->rps.min_freq_softlimit);
 		return;
 	}
@@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
 
 	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);
-
-	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
-				& GENFREQSTATUS) == 0, 100))
-		DRM_ERROR("timed out waiting for Punit\n");
-
+	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
 	vlv_force_gfx_clock(dev_priv, false);
-
-	I915_WRITE(GEN6_PMINTRMSK,
-		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
 }
 
Quoted 7 lines

drivers/gpu/drm/i915/intel_pm.c static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) · line 3927

Quoted 2 lines
drivers/gpu/drm/i915/intel_pm.c +0 -0
 void gen6_rps_idle(struct drm_i915_private *dev_priv)

Yeah I think this is fine (may need a rebase though, you can keep my r-b if you do that in case Jani doesn't want to deal with the merge conflicts).

The removal of the stepping check is still confusing me even if the rest would be OK.

Signature · 6 lines
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#9 lore raw
In reply to Ville Syrjälä
Quoted 60 lines · Ville Syrjälä
Quoted 59 lines · Jesse Barnes
Quoted 58 lines · On 03/04/2015 08:08 PM, [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.

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

---
  drivers/gpu/drm/i915/intel_pm.c | 20 ++++----------------
  1 file changed, 4 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +4 -16
index e710b43..2e1ed07 100644
@@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
   * * 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
+ * 3. Request the freq to Rpn.
   * 4. Clear the Force GFX CLK ON bit so that Gfx can down
   * 5. Unmask Turbo interrupts
  */
@@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
  {
  	struct drm_device *dev = dev_priv->dev;
 
-	/* 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, dev_priv->rps.min_freq_softlimit);
  		return;
  	}
@@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
  		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
 
  	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);
-
-	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
-				& GENFREQSTATUS) == 0, 100))
-		DRM_ERROR("timed out waiting for Punit\n");
-
+	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
  	vlv_force_gfx_clock(dev_priv, false);
-
-	I915_WRITE(GEN6_PMINTRMSK,
-		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
  }
 
Quoted 9 lines
Quoted 6 lines

drivers/gpu/drm/i915/intel_pm.c static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) · line 3927

Quoted 2 lines
drivers/gpu/drm/i915/intel_pm.c +0 -0
  void gen6_rps_idle(struct drm_i915_private *dev_priv)

Yeah I think this is fine (may need a rebase though, you can keep my r-b if you do that in case Jani doesn't want to deal with the merge conflicts).

The removal of the stepping check is still confusing me even if the rest would be OK.

Stepping check was added latest BYT release. On older BYT stepping, We used to wait for punit to grant the freq in GT Idle case, (most of the cases punit is timing out :( ) We now make the gfx clock force apply to all VLV and then request the freq to RPn this should be good enough to get voltage to Vnn.

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

#10 lore raw
Quoted 68 lines · Deepak S
Quoted 65 lines · Ville Syrjälä
Quoted 64 lines · Jesse Barnes
Quoted 50 lines · On 03/04/2015 08:08 PM, [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.

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

---
  drivers/gpu/drm/i915/intel_pm.c | 20 ++++----------------
  1 file changed, 4 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +3 -11
index e710b43..2e1ed07 100644
@@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
   * * 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
+ * 3. Request the freq to Rpn.
   * 4. Clear the Force GFX CLK ON bit so that Gfx can down
   * 5. Unmask Turbo interrupts
  */
@@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
  {
  	struct drm_device *dev = dev_priv->dev;
 
-	/* 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, dev_priv->rps.min_freq_softlimit);
  		return;
  	}
@@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
  		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
 
  	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);
-
-	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
-				& GENFREQSTATUS) == 0, 100))

drivers/gpu/drm/i915/intel_pm.c static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) · line 3927

Quoted 10 lines
drivers/gpu/drm/i915/intel_pm.c +1 -5
-		DRM_ERROR("timed out waiting for Punit\n");
-
+	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
  	vlv_force_gfx_clock(dev_priv, false);
-
-	I915_WRITE(GEN6_PMINTRMSK,
-		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
  }
 
  void gen6_rps_idle(struct drm_i915_private *dev_priv)

Yeah I think this is fine (may need a rebase though, you can keep my r-b if you do that in case Jani doesn't want to deal with the merge conflicts).

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

The removal of the stepping check is still confusing me even if the rest would be OK.

Stepping check was added latest BYT release. On older BYT stepping, We used to wait for punit to grant the freq in GT Idle case, (most of the cases punit is timing out :( ) We now make the gfx clock force apply to all VLV and then request the freq to RPn this should be good enough to get voltage to Vnn.

But we shouldn't need the gfx clock force for the latest VLV stepping(s), and we certainly didn't do it before. So why do it now?

Signature · 6 lines
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#11 lore raw
Quoted 69 lines · Ville Syrjälä
Quoted 68 lines · Deepak S
Quoted 66 lines · Ville Syrjälä
Quoted 65 lines · Jesse Barnes
Quoted 50 lines · On 03/04/2015 08:08 PM, [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.

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

---
   drivers/gpu/drm/i915/intel_pm.c | 20 ++++----------------
   1 file changed, 4 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +3 -11
index e710b43..2e1ed07 100644
@@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
    * * 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
+ * 3. Request the freq to Rpn.
    * 4. Clear the Force GFX CLK ON bit so that Gfx can down
    * 5. Unmask Turbo interrupts
   */
@@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
   {
   	struct drm_device *dev = dev_priv->dev;
 
-	/* 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, dev_priv->rps.min_freq_softlimit);
   		return;
   	}
@@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
   		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
 
   	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);
-
-	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
-				& GENFREQSTATUS) == 0, 100))

drivers/gpu/drm/i915/intel_pm.c static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) · line 3927

Quoted 10 lines
drivers/gpu/drm/i915/intel_pm.c +1 -5
-		DRM_ERROR("timed out waiting for Punit\n");
-
+	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
   	vlv_force_gfx_clock(dev_priv, false);
-
-	I915_WRITE(GEN6_PMINTRMSK,
-		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
   }
 
   void gen6_rps_idle(struct drm_i915_private *dev_priv)

Yeah I think this is fine (may need a rebase though, you can keep my r-b if you do that in case Jani doesn't want to deal with the merge conflicts).

Quoted 9 lines
Quoted 5 lines
Quoted 3 lines

The removal of the stepping check is still confusing me even if the rest would be OK.

Stepping check was added latest BYT release. On older BYT stepping, We used to wait for punit to grant the freq in GT Idle case, (most of the cases punit is timing out :( ) We now make the gfx clock force apply to all VLV and then request the freq to RPn this should be good enough to get voltage to Vnn.

But we shouldn't need the gfx clock force for the latest VLV stepping(s), and we certainly didn't do it before. So why do it now?

Hi Ville, This is keep code common across all the VLV stepping. :)

Thanks
Deepak

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

#12 lore raw
Quoted 72 lines · Deepak S
Quoted 69 lines · Ville Syrjälä
Quoted 68 lines · Deepak S
Quoted 66 lines · Ville Syrjälä
Quoted 61 lines · Jesse Barnes
Quoted 47 lines · On 03/04/2015 08:08 PM, [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.

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

---
  drivers/gpu/drm/i915/intel_pm.c | 20 ++++----------------
  1 file changed, 4 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +3 -11
index e710b43..2e1ed07 100644
@@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
   * * 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
+ * 3. Request the freq to Rpn.
   * 4. Clear the Force GFX CLK ON bit so that Gfx can down
   * 5. Unmask Turbo interrupts
  */
@@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
  {
  	struct drm_device *dev = dev_priv->dev;
-	/* 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, dev_priv->rps.min_freq_softlimit);
  		return;
  	}
@@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
  		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
  	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);
-
-	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
-				& GENFREQSTATUS) == 0, 100))

drivers/gpu/drm/i915/intel_pm.c static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) · line 3926

Quoted 10 lines
drivers/gpu/drm/i915/intel_pm.c +1 -5
-		DRM_ERROR("timed out waiting for Punit\n");
-
+	valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
  	vlv_force_gfx_clock(dev_priv, false);
-
-	I915_WRITE(GEN6_PMINTRMSK,
-		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
  }
  void gen6_rps_idle(struct drm_i915_private *dev_priv)
 

Yeah I think this is fine (may need a rebase though, you can keep my r-b if you do that in case Jani doesn't want to deal with the merge conflicts).

Quoted 1 line

The removal of the stepping check is still confusing me even if the rest would be OK.

Quoted 7 lines
Quoted 6 lines
Quoted 2 lines

Stepping check was added latest BYT release. On older BYT stepping, We used to wait for punit to grant the freq in GT Idle case, (most of the cases punit is timing out :( ) We now make the gfx clock force apply to all VLV and then request the freq to RPn this should be good enough to get voltage to Vnn.

But we shouldn't need the gfx clock force for the latest VLV stepping(s), and we certainly didn't do it before. So why do it now?

Hi Ville, This is keep code common across all the VLV stepping. :)

Makes sense (at least to me) but please add this explanation to the commit message when resending so it won't get lost.
-Daniel

Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx