Bug#692607: linux-image-3.2.0-4-686-pae: Kernel crash when coming out of screen saver
Ben Hutchings <ben@decadent.org.uk> writes:
> On Thu, 2013-02-28 at 11:31 +0100, Bjørn Mork wrote:
>
>> Anyone able to spot the missing INIT_WORK()'s? Based on the
>> I915_HAS_HOTPLUG(dev) test, I assume that leaving the first one out was
>> intentional. But the second one cannot be left out, as demonstrated by
>> these bug reports.
>
> Agreed. And this is a faithful backport of the upstream change, i.e.
> the bug was not introduced in backporting. However it seems to have
> been almost immediately fixed upstream by:
>
> commit 8b2e326dc7c5aa6952c88656d04d0d81fd85a6f8
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date: Tue Apr 24 22:59:41 2012 +0100
>
> drm/i915: Unconditionally initialise the interrupt workers
Yup. That's the patch Steinar is currently testing. Backported and
refreshed against the Debian 3.2.39-2 source. Attached for your
convenience.
FWIW, I have no problems understanding how this bug could be missed
during backport. There is nothing in the upstream commit messages
indicating that the second change fixes a serious bug or that there is a
strict dependency between these two changes.
>> I am attaching a proposed fix on top of the 655152 patch, which I have
>> not tested at all on actual Debian kernel sources. Might need context
>> adjustments. I'd appreciate it if anyone with crashing hardware could
>> test it.
>
> It applies and builds.
>
> Instructions for building a patched package are at:
> http://kernel-handbook.alioth.debian.org/ch-common-tasks.html#s-common-official
It was a long time since I last built a Debian kernel, and I was
positively surprised by the "test-patches" script. I was not aware that
testing a patch on top of a Debian kernel would be as simple as
installing the source and running a script. Thanks for making such
tasks so easy for end users.
But if I may request another feature for the script, then I believe a
"fuzz" option would have been useful. Or a more lenient default than 0
at least. That's just annoyingly strict.
Bjørn
>From 3738ac736bf08e589968d8b5af52cef409047472 Mon Sep 17 00:00:00 2001
From: Chris Wilson <chris@chris-wilson.co.uk>
Date: Tue, 24 Apr 2012 22:59:41 +0100
Subject: [PATCH] drm/i915: Unconditionally initialise the interrupt workers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
commit 8b2e326dc7c5aa6952c88656d04d0d81fd85a6f8 upstream.
Rather than duplicate similar code across the IRQ installers, perform
the initialisation of the workers upfront. This will lead to simpler
teardown and quiescent code as we can assume that the workers have
been initialised.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
[bmork: deleted valleyview hunk for 3.2 backport]
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
drivers/gpu/drm/i915/i915_irq.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
Index: linux-3.2.39/drivers/gpu/drm/i915/i915_irq.c
===================================================================
--- linux-3.2.39.orig/drivers/gpu/drm/i915/i915_irq.c 2013-02-28 15:02:55.000000000 +0100
+++ linux-3.2.39/drivers/gpu/drm/i915/i915_irq.c 2013-02-28 15:07:25.882044365 +0100
@@ -1806,10 +1806,6 @@
atomic_set(&dev_priv->irq_received, 0);
- INIT_WORK(&dev_priv->hotplug_work, i915_hotplug_work_func);
- INIT_WORK(&dev_priv->error_work, i915_error_work_func);
- if (IS_GEN6(dev) || IS_IVYBRIDGE(dev))
- INIT_WORK(&dev_priv->rps_work, gen6_pm_rps_work);
I915_WRITE(HWSTAM, 0xeffe);
@@ -1983,9 +1979,6 @@
atomic_set(&dev_priv->irq_received, 0);
- INIT_WORK(&dev_priv->hotplug_work, i915_hotplug_work_func);
- INIT_WORK(&dev_priv->error_work, i915_error_work_func);
-
if (I915_HAS_HOTPLUG(dev)) {
I915_WRITE(PORT_HOTPLUG_EN, 0);
I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
@@ -2290,6 +2283,12 @@
void intel_irq_init(struct drm_device *dev)
{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ INIT_WORK(&dev_priv->hotplug_work, i915_hotplug_work_func);
+ INIT_WORK(&dev_priv->error_work, i915_error_work_func);
+ INIT_WORK(&dev_priv->rps_work, gen6_pm_rps_work);
+
dev->driver->get_vblank_counter = i915_get_vblank_counter;
dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */
if (IS_G4X(dev) || IS_GEN5(dev) || IS_GEN6(dev) || IS_IVYBRIDGE(dev)) {
Reply to: