<div dir="ltr">I have the feeling the safest side would be disable rc6 on resume instead of force its enabling... or am I missing something?<div>why don't you just cancel the work? and put another after resume?<br></div>
<div><br></div><div>but if the patch really solves the problem and this is what you meant feel free to use:</div><div>Reviewed-by: Rodrigo Vivi <<a href="mailto:rodrigo.vivi@intel.com">rodrigo.vivi@intel.com</a>><br>
</div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jun 27, 2014 at 2:51 PM, Paulo Zanoni <span dir="ltr"><<a href="mailto:przanoni@gmail.com" target="_blank">przanoni@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">From: Paulo Zanoni <<a href="mailto:paulo.r.zanoni@intel.com">paulo.r.zanoni@intel.com</a>><br>
<br>
It is possible that, by the time we run i915_drm_freeze(),<br>
delayed_resume_work was already queued but did not run yet. If it<br>
still didn't run after intel_runtime_pm_disable_interrupts(), by the<br>
time it runs it will try to change the interrupt registers with the<br>
interrupts already disabled, which will trigger a WARN. We can<br>
reliably reproduce this with the pm_rpm system-suspend test case.<br>
<br>
In order to avoid the problem, we have to flush the work before<br>
disabling the interrupts. We could also cancel the work instead of<br>
flushing it, but that would require us to put a runtime PM reference -<br>
and any other resource we may need in the future - in case the work<br>
was already queued, so I believe flushing the work is more<br>
future-proof, although less efficient. But I can also change this part<br>
if someone requests.<br>
<br>
Another thing I tried was to move the intel_suspend_gt_powersave()<br>
call to before intel_runtime_pm_disable_interrupts(), but since that<br>
function needs to be called after the interrupts are already disabled,<br>
due to dev_priv->rps.work, this strategy didn't work.<br>
<br>
Testcase: igt/pm_rpm/system-suspend<br>
Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=80517" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=80517</a><br>
Signed-off-by: Paulo Zanoni <<a href="mailto:paulo.r.zanoni@intel.com">paulo.r.zanoni@intel.com</a>><br>
---<br>
 drivers/gpu/drm/i915/i915_drv.c | 2 ++<br>
 1 file changed, 2 insertions(+)<br>
<br>
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c<br>
index e64547e..672694b 100644<br>
--- a/drivers/gpu/drm/i915/i915_drv.c<br>
+++ b/drivers/gpu/drm/i915/i915_drv.c<br>
@@ -524,6 +524,8 @@ static int i915_drm_freeze(struct drm_device *dev)<br>
                        return error;<br>
                }<br>
<br>
+               flush_delayed_work(&dev_priv->rps.delayed_resume_work);<br>
+<br>
                intel_runtime_pm_disable_interrupts(dev);<br>
                dev_priv->enable_hotplug_processing = false;<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
2.0.0<br>
<br>
_______________________________________________<br>
Intel-gfx mailing list<br>
<a href="mailto:Intel-gfx@lists.freedesktop.org">Intel-gfx@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/intel-gfx" target="_blank">http://lists.freedesktop.org/mailman/listinfo/intel-gfx</a><br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Rodrigo Vivi</div><div>Blog: <a href="http://blog.vivi.eng.br" target="_blank">http://blog.vivi.eng.br</a></div><div> </div>
</div>