[PATCH] drm/i915: enable irqs earlier when resuming

[PATCH] drm/i915: enable irqs earlier when resuming

[Daniel Vetter]

We need it to restore the ilk rc6 context, since the gpu wait no
requires interrupts. But in general having interrupts around should
help in code sanity, since more and more stuff is interrupt driven.

This regression has been introduced in

commit 3e9605018ab3e333d51cc90fccfde2031886763b
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue Nov 27 16:22:54 2012 +0000

    drm/i915: Rearrange code to only have a single method for waiting upon the ring

Like in the driver load code we need to make sure that hotplug
interrupts don't cause havoc with our modeset state, hence block them
with the existing infrastructure. Again we ignore races where we might
loose hotplug interrupts ...

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=54691
Cc: stable@vger.kernel.org (for 3.8 only)
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reported-and-Tested-by: Ilya Tumaykin <itumaykin@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b342749..7589a2a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -493,6 +493,7 @@ static int i915_drm_freeze(struct drm_device *dev)
 		intel_modeset_disable(dev);
 
 		drm_irq_uninstall(dev);
+		dev_priv->enable_hotplug_processing = false;
 	}
 
 	i915_save_state(dev);
@@ -566,10 +567,20 @@ static int __i915_drm_thaw(struct drm_device *dev)
 		error = i915_gem_init_hw(dev);
 		mutex_unlock(&dev->struct_mutex);
 
+		/* We need working interrupts for modeset enabling ... */
+		drm_irq_install(dev);
+
 		intel_modeset_init_hw(dev);
 		intel_modeset_setup_hw_state(dev, false);
-		drm_irq_install(dev);
+
+		/*
+		 * ... but also need to make sure that hotplug processing
+		 * doesn't cause havoc. Like in the driver load code we don't
+		 * bother with the tiny race here where we might loose hotplug
+		 * notifications.
+		 * */
 		intel_hpd_init(dev);
+		dev_priv->enable_hotplug_processing = true;
 	}
 
 	intel_opregion_init(dev);
-- 
1.7.10.4

Re: [PATCH] drm/i915: enable irqs earlier when resuming

[Chris Wilson]

On Tue, Mar 05, 2013 at 09:50:58AM +0100, Daniel Vetter wrote:
> We need it to restore the ilk rc6 context, since the gpu wait no
> requires interrupts. But in general having interrupts around should
> help in code sanity, since more and more stuff is interrupt driven.
> 
> This regression has been introduced in
> 
> commit 3e9605018ab3e333d51cc90fccfde2031886763b
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Tue Nov 27 16:22:54 2012 +0000
> 
>     drm/i915: Rearrange code to only have a single method for waiting upon the ring

Not entirely, as we can also regard this as an oversight from fixing up
the irq sequence during initialisation... :-p
> 
> Like in the driver load code we need to make sure that hotplug
> interrupts don't cause havoc with our modeset state, hence block them
> with the existing infrastructure. Again we ignore races where we might
> loose hotplug interrupts ...
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=54691
> Cc: stable@vger.kernel.org (for 3.8 only)
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Reported-and-Tested-by: Ilya Tumaykin <itumaykin@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Chris wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: enable irqs earlier when resuming

[Daniel Vetter]

On Tue, Mar 05, 2013 at 08:53:48AM +0000, Chris Wilson wrote:
> On Tue, Mar 05, 2013 at 09:50:58AM +0100, Daniel Vetter wrote:
> > We need it to restore the ilk rc6 context, since the gpu wait no
> > requires interrupts. But in general having interrupts around should
> > help in code sanity, since more and more stuff is interrupt driven.
> > 
> > This regression has been introduced in
> > 
> > commit 3e9605018ab3e333d51cc90fccfde2031886763b
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date:   Tue Nov 27 16:22:54 2012 +0000
> > 
> >     drm/i915: Rearrange code to only have a single method for waiting upon the ring
> 
> Not entirely, as we can also regard this as an oversight from fixing up
> the irq sequence during initialisation... :-p

Added a citation to the relevant commit ...
> > 
> > Like in the driver load code we need to make sure that hotplug
> > interrupts don't cause havoc with our modeset state, hence block them
> > with the existing infrastructure. Again we ignore races where we might
> > loose hotplug interrupts ...
> > 
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=54691
> > Cc: stable@vger.kernel.org (for 3.8 only)
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > Reported-and-Tested-by: Ilya Tumaykin <itumaykin@gmail.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Reviewed-by: Chris wilson <chris@chris-wilson.co.uk>

... and merged to -fixes, thanks for the review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch