Skip to contentSkip to conversation

drm/i915: fix RPS on runtime suspend

PATCH Applied Paulo Zanoni 3 participants 3 messages Oct 30, 2014 – Nov 3, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drm/i915: fix RPS on runtime suspend

From: Paulo Zanoni <[email protected]>

With this patch, the RPS sequence for runtime suspend/resume is exactly like the sequence for S3 suspend/resume:
- flush_delayed_work(&dev_priv->rps.delayed_resume_work)
- intel_runtime_pm_disable_interrupts()
- intel_suspend_gt_powersave()
(suspended)
- intel_runtime_pm_enable_interrupts()
- intel_enable_gt_powersave()

With this, we get rid of WARNs that are currently intermittently triggered by the system-suspend-execbuf subtest of runtime PM. Notice that these WARNs could also be triggered in other ways that involved doing lots of RPM suspend/resume cycles just after a system S3 resume.

Testcase: igt/pm_rpm/system-suspend-execbuf

---
 drivers/gpu/drm/i915/i915_drv.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

Also, with this patch + the sprite planes regression fix, this will be the first time ever that my BDW machine passes every single test of pm_rpm without any WARNs! \o/

drivers/gpu/drm/i915/i915_drv.c +3 -7
index 0c7cf48..2404b2b 100644
@@ -1395,13 +1395,9 @@ static int intel_runtime_suspend(struct device *device)
 	i915_gem_release_all_mmaps(dev_priv);
 	mutex_unlock(&dev->struct_mutex);
 
-	/*
-	 * rps.work can't be rearmed here, since we get here only after making
-	 * sure the GPU is idle and the RPS freq is set to the minimum. See
-	 * intel_mark_idle().
-	 */
-	cancel_work_sync(&dev_priv->rps.work);
+	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
 	intel_runtime_pm_disable_interrupts(dev_priv);
+	intel_suspend_gt_powersave(dev);
 
 	ret = intel_suspend_complete(dev_priv);
 	if (ret) {
@@ -1473,7 +1469,7 @@ static int intel_runtime_resume(struct device *device)
 	gen6_update_ring_freq(dev);
 
 	intel_runtime_pm_enable_interrupts(dev_priv);
-	intel_reset_gt_powersave(dev);
+	intel_enable_gt_powersave(dev);
 
 	if (ret)
 		DRM_ERROR("Runtime resume failed, disabling it (%d)\n", ret);
Signature · 6 lines
2.1.1

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by on v1 1/1
#1 lore raw
Quoted 11 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

With this patch, the RPS sequence for runtime suspend/resume is exactly like the sequence for S3 suspend/resume:
- flush_delayed_work(&dev_priv->rps.delayed_resume_work)
- intel_runtime_pm_disable_interrupts()
- intel_suspend_gt_powersave()
(suspended)
- intel_runtime_pm_enable_interrupts()
- intel_enable_gt_powersave()

Quoted 7 lines

With this, we get rid of WARNs that are currently intermittently triggered by the system-suspend-execbuf subtest of runtime PM. Notice that these WARNs could also be triggered in other ways that involved doing lots of RPM suspend/resume cycles just after a system S3 resume.

Testcase: igt/pm_rpm/system-suspend-execbuf

This also fixes some parts of pm_rpm subtest failures on VLV so you could add:
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=82939
and

While fixing the same WARN in the above bug I also cleaned up the rps code a bit and made the disabling part more robust against rearming the rps work. But that's a separate issue which can't normally happen here, so I can send a version rebased on top of your changes.

Quoted 29 lines
---
 drivers/gpu/drm/i915/i915_drv.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

Also, with this patch + the sprite planes regression fix, this will be the first time ever that my BDW machine passes every single test of pm_rpm without any WARNs! \o/

drivers/gpu/drm/i915/i915_drv.c +2 -6
index 0c7cf48..2404b2b 100644
@@ -1395,13 +1395,9 @@ static int intel_runtime_suspend(struct device *device)
 	i915_gem_release_all_mmaps(dev_priv);
 	mutex_unlock(&dev->struct_mutex);
 
-	/*
-	 * rps.work can't be rearmed here, since we get here only after making
-	 * sure the GPU is idle and the RPS freq is set to the minimum. See
-	 * intel_mark_idle().
-	 */
-	cancel_work_sync(&dev_priv->rps.work);
+	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
 	intel_runtime_pm_disable_interrupts(dev_priv);
+	intel_suspend_gt_powersave(dev);
 
 	ret = intel_suspend_complete(dev_priv);
 	if (ret) {

drivers/gpu/drm/i915/i915_drv.c static int intel_runtime_resume(struct device *device) · line 1475

Quoted 8 lines
drivers/gpu/drm/i915/i915_drv.c +1 -1
@@ -1473,7 +1469,7 @@ static int intel_runtime_resume(struct device *device)
 	gen6_update_ring_freq(dev);
 
 	intel_runtime_pm_enable_interrupts(dev_priv);
-	intel_reset_gt_powersave(dev);
+	intel_enable_gt_powersave(dev);
 
 	if (ret)
 		DRM_ERROR("Runtime resume failed, disabling it (%d)\n", ret);

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

#2 lore raw
Quoted 17 lines · Imre Deak
Quoted 16 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

With this patch, the RPS sequence for runtime suspend/resume is exactly like the sequence for S3 suspend/resume:
- flush_delayed_work(&dev_priv->rps.delayed_resume_work)
- intel_runtime_pm_disable_interrupts()
- intel_suspend_gt_powersave()
(suspended)
- intel_runtime_pm_enable_interrupts()
- intel_enable_gt_powersave()

With this, we get rid of WARNs that are currently intermittently triggered by the system-suspend-execbuf subtest of runtime PM. Notice that these WARNs could also be triggered in other ways that involved doing lots of RPM suspend/resume cycles just after a system S3 resume.

Quoted 8 lines
Quoted 2 lines

Testcase: igt/pm_rpm/system-suspend-execbuf

This also fixes some parts of pm_rpm subtest failures on VLV so you could add:
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=82939
and

Queued for -next, thanks for the patch.

Quoted 4 lines

While fixing the same WARN in the above bug I also cleaned up the rps code a bit and made the disabling part more robust against rearming the rps work. But that's a separate issue which can't normally happen here, so I can send a version rebased on top of your changes.

Yes, please!

Thanks, Daniel

Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx