On Tue, 2008-11-04 at 16:04 -0800, Eric Anholt wrote:
> On Tue, 2008-11-04 at 02:03 -0800, Keith Packard wrote:
> > The pipestat fields affect reporting of all vblank-related interrupts, so we
> > have to reset them during the irq_handler, and while enabling vblank
> > interrupts.
> >
> > This patch adds i915_enable_pipestat and i915_disable_pipestat to abstract
> > out the steps needed to change the reported interrupts.
>
> This looks pretty good, and covers a lot of my concerns with interrupt
> handling. See comments, and my additional MSI patch that I required for
> stability against wiggling a window over vblank-synced glxgears
> (reproducible in <30s before, and I've now spent a couple of minutes
> wiggling windows successfully).
>
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index b2ba2b6..f7ac581 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -34,28 +34,69 @@
> > #define MAX_NOPID ((u32)~0)
> >
> > /** These are the interrupts used by the driver */
> > -#define I915_INTERRUPT_ENABLE_MASK (I915_USER_INTERRUPT | \
> > - I915_ASLE_INTERRUPT | \
> > - I915_DISPLAY_PIPE_A_EVENT_INTERRUPT | \
> > - I915_DISPLAY_PIPE_B_EVENT_INTERRUPT)
> > +#define I915_INTERRUPT_ENABLE_FIX (I915_ASLE_INTERRUPT | \
> > + I915_DISPLAY_PIPE_A_EVENT_INTERRUPT | \
> > + I915_DISPLAY_PIPE_B_EVENT_INTERRUPT)
> > +
> > +#define I915_INTERRUPT_ENABLE_VAR (I915_USER_INTERRUPT)
> > +
> > +#define I915_INTERRUPT_ENABLE_MASK (I915_INTERRUPT_ENABLE_FIX | \
> > + I915_INTERRUPT_ENABLE_VAR)
> >
> > void
> > i915_enable_irq(drm_i915_private_t *dev_priv, u32 mask)
> > {
> > - if ((dev_priv->irq_mask_reg & mask) != 0) {
> > - dev_priv->irq_mask_reg &= ~mask;
> > - I915_WRITE(IMR, dev_priv->irq_mask_reg);
> > - (void) I915_READ(IMR);
> > + mask &= I915_INTERRUPT_ENABLE_VAR;
> > + if ((dev_priv->irq_enable_reg & mask) != mask) {
> > + dev_priv->irq_enable_reg |= mask;
> > + I915_WRITE(IER, dev_priv->irq_enable_reg);
> > + (void) I915_READ(IER);
> > }
> > }
>
> IER versus IMR would be worthwhile to mention in the commit message.
>
> > spin_lock_irqsave(&dev_priv->user_irq_lock, irqflags);
> > - /* Enabling vblank events in IMR comes before PIPESTAT write, or
> > - * there's a race where the PIPESTAT vblank bit gets set to 1, so
> > - * the OR of enabled PIPESTAT bits goes to 1, so the PIPExEVENT in
> > - * ISR flashes to 1, but the IIR bit doesn't get set to 1 because
> > - * IMR masks it. It doesn't ever get set after we clear the masking
> > - * in IMR because the ISR bit is edge, not level-triggered, on the
> > - * OR of PIPESTAT bits.
> > - */
> > - i915_enable_irq(dev_priv, interrupt);
> > - pipestat = I915_READ(pipestat_reg);
> > - if (IS_I965G(dev))
> > - pipestat |= PIPE_START_VBLANK_INTERRUPT_ENABLE;
> > - else
> > - pipestat |= PIPE_VBLANK_INTERRUPT_ENABLE;
> > - /* Clear any stale interrupt status */
> > - pipestat |= (PIPE_START_VBLANK_INTERRUPT_STATUS |
> > - PIPE_VBLANK_INTERRUPT_STATUS);
> > - I915_WRITE(pipestat_reg, pipestat);
> > - (void) I915_READ(pipestat_reg); /* Posting read */
> > + i915_enable_pipestat(dev_priv, pipe, I915_VBLANK_INTERRUPT_ENABLE);
>
> I'm uncomfortable with the dropping of the START_VBLANK versus VBLANK,
> as I'm pretty sure people had figured out that the other bit was bad
> news on 965+.
>
I'm on 965GM and I'm having a serious interrupt problem since this patch
went into for-review:
Nov 7 04:20:22 infinity irq 16: nobody cared (try booting with the
"irqpoll" option)
Nov 7 04:20:22 infinity Pid: 0, comm: swapper Not tainted
2.6.28-rc3-00236-g1d7eff8 #23
Nov 7 04:20:22 infinity Call Trace:
Nov 7 04:20:22 infinity <IRQ> [<ffffffff80491a25>] ?
i915_driver_irq_handler+0x53/0x186
Nov 7 04:20:22 infinity [<ffffffff80270b55>] __report_bad_irq+0x3d/0x8c
Nov 7 04:20:22 infinity [<ffffffff80270cb7>] note_interrupt+0x113/0x178
Nov 7 04:20:22 infinity [<ffffffff802713db>] handle_fasteoi_irq
+0x99/0xc3
Nov 7 04:20:22 infinity [<ffffffff8020ee5f>] do_IRQ+0x9c/0x11d
Nov 7 04:20:22 infinity [<ffffffff8020c826>] ret_from_intr+0x0/0xa
Nov 7 04:20:22 infinity <EOI> [<ffffffff804572c0>] ?
acpi_idle_enter_simple+0x175/0x1a8
Nov 7 04:20:22 infinity [<ffffffff804572b6>] ? acpi_idle_enter_simple
+0x16b/0x1a8
Nov 7 04:20:22 infinity [<ffffffff8052af56>] ? cpuidle_idle_call
+0xa6/0xe0
Nov 7 04:20:22 infinity [<ffffffff8020b47a>] ? cpu_idle+0x4c/0xb0
Nov 7 04:20:22 infinity [<ffffffff80614551>] ? rest_init+0x75/0x77
Nov 7 04:20:22 infinity handlers:
Nov 7 04:20:22 infinity [<ffffffff804919d2>] (i915_driver_irq_handler
+0x0/0x186)
Nov 7 04:20:22 infinity Disabling IRQ #16
This happens after a random amount of time in X, athough never very
long. From this point on there are no interrupts generated unless I
switch vts away from X and back again. This gets interrupts working
again for a short while.
This is possibly also related to the massive slowdown I get X uses 20%+
CPU constantly and continually probes DDC, when I switch to battery,
this I had expected to be fixed by the recent patch removing ACPI event
handling, but strangely it still occurs.
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
