| Message ID | 1414691971-13075-1-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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);
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
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);