[PATCH] drm/i915: fix RPS on runtime suspend

[PATCH] drm/i915: fix RPS on runtime suspend

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 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/

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 0c7cf48..2404b2b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -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);
-- 
2.1.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: fix RPS on runtime suspend

[Imre Deak]

On Thu, 2014-10-30 at 15:59 -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> 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
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

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
Reviewed-by: Imre Deak <imre.deak@intel.com>

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.

> ---
>  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/
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 0c7cf48..2404b2b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -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);


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: fix RPS on runtime suspend

[Daniel Vetter]

On Thu, Oct 30, 2014 at 10:36:23PM +0200, Imre Deak wrote:
> On Thu, 2014-10-30 at 15:59 -0200, Paulo Zanoni wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > 
> > 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
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> 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
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Queued for -next, thanks for the patch.

> 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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx