| Message ID | 1302040846-18447-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 5 Apr 2011 23:00:46 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote: > This looks like it fixes two bugs: > > 1) What if there is an error recorded before we start and so we > immediately service an IIR/EIR upon intalling the IRQ. Did we generate > that error during initialisation or was that SEP? Now we do stuff before > we even install the IRQ so there is a possibility that we zap one of our > own. How will processing a pre-existing error condition cause trouble? > Does EIR even persist between phases to cause these issues in the > first place? Yeah, seems like a bit of testing might be able to check this fairly quickly.
On Tue, 05 Apr 2011 18:27:45 -0700, Keith Packard <keithp@keithp.com> wrote: > On Tue, 5 Apr 2011 23:00:46 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > This looks like it fixes two bugs: > > > > 1) What if there is an error recorded before we start and so we > > immediately service an IIR/EIR upon intalling the IRQ. Did we generate > > that error during initialisation or was that SEP? Now we do stuff before > > we even install the IRQ so there is a possibility that we zap one of our > > own. > > How will processing a pre-existing error condition cause trouble? So that I can separate out bug reports that are caused by us during initialisation from those that are caused by whatever was running before us. Considering the difficulty in working out what caused a PGTBL_ER during initialisation, any reduction in the volume of noise is a good thing. -Chris
On Wed, 06 Apr 2011 07:52:27 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote: > So that I can separate out bug reports that are caused by us during > initialisation from those that are caused by whatever was running before > us. Ok, sounds like good -next material to me then. > Considering the difficulty in working out what caused a PGTBL_ER during > initialisation, any reduction in the volume of noise is a good thing. Yeah, accurate error reports can go a long ways towards finding the real trouble.
On Tue, 5 Apr 2011 23:00:46 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Something to think about...
Missing SoB here...
On Wed, 04 May 2011 12:21:27 -0700, Keith Packard <keithp@keithp.com> wrote: > On Tue, 5 Apr 2011 23:00:46 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > Something to think about... > > Missing SoB here... Indeed, I'm still thinking about this one. I don't think it's ready yet. -Chris
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 46ccfc8..bb1a624 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1650,11 +1650,24 @@ static int ironlake_irq_postinstall(struct drm_device *dev) return 0; } +static void i915_irq_clear_errors(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + u32 eir = I915_READ(EIR); + if (eir) { + I915_WRITE(EIR, eir); + POSTING_READ(EIR); + } +} + void i915_driver_irq_preinstall(struct drm_device * dev) { drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; int pipe; + /* Clear any errors recorded before we begin. */ + i915_irq_clear_errors(dev); + atomic_set(&dev_priv->irq_received, 0); INIT_WORK(&dev_priv->hotplug_work, i915_hotplug_work_func); @@ -1719,7 +1732,8 @@ int i915_driver_irq_postinstall(struct drm_device *dev) error_mask = ~(I915_ERROR_PAGE_TABLE | I915_ERROR_MEMORY_REFRESH); } - I915_WRITE(EMR, error_mask); + /* Some pre-existing errors might have become stuck, mask them. */ + I915_WRITE(EMR, error_mask | I915_READ(EIR)); I915_WRITE(IMR, dev_priv->irq_mask); I915_WRITE(IER, enable_mask);