From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/i915: flush delayed_resume_work when suspending Date: Tue, 01 Jul 2014 17:49:05 +0300 Message-ID: <87y4wdf82m.fsf@intel.com> References: <1403905911-3859-1-git-send-email-przanoni@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 94FA16E569 for ; Tue, 1 Jul 2014 07:50:07 -0700 (PDT) In-Reply-To: <1403905911-3859-1-git-send-email-przanoni@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Paulo Zanoni , intel-gfx@lists.freedesktop.org Cc: Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Sat, 28 Jun 2014, Paulo Zanoni wrote: > From: Paulo Zanoni > > It is possible that, by the time we run i915_drm_freeze(), > delayed_resume_work was already queued but did not run yet. If it > still didn't run after intel_runtime_pm_disable_interrupts(), by the > time it runs it will try to change the interrupt registers with the > interrupts already disabled, which will trigger a WARN. We can > reliably reproduce this with the pm_rpm system-suspend test case. > > In order to avoid the problem, we have to flush the work before > disabling the interrupts. We could also cancel the work instead of > flushing it, but that would require us to put a runtime PM reference - > and any other resource we may need in the future - in case the work > was already queued, so I believe flushing the work is more > future-proof, although less efficient. But I can also change this part > if someone requests. > > Another thing I tried was to move the intel_suspend_gt_powersave() > call to before intel_runtime_pm_disable_interrupts(), but since that > function needs to be called after the interrupts are already disabled, > due to dev_priv->rps.work, this strategy didn't work. > > Testcase: igt/pm_rpm/system-suspend > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80517 > Signed-off-by: Paulo Zanoni Pushed to dinq, thanks for the patch and review. BR, Jani. > --- > drivers/gpu/drm/i915/i915_drv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index e64547e..672694b 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -524,6 +524,8 @@ static int i915_drm_freeze(struct drm_device *dev) > return error; > } > > + flush_delayed_work(&dev_priv->rps.delayed_resume_work); > + > intel_runtime_pm_disable_interrupts(dev); > dev_priv->enable_hotplug_processing = false; > > -- > 2.0.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center