[Intel-gfx] [PATCH] Manage PIPESTAT pending interrupt values to unblock vblank interrupts
Steven J Newbury
steve at snewbury.org.uk
Fri Nov 7 21:45:31 CET 2008
On Fri, 2008-11-07 at 11:11 -0800, Eric Anholt wrote:
> On Fri, 2008-11-07 at 14:01 +0000, Steven J Newbury wrote:
> > 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.
>
> Can you get /proc/dri/0/i915_gem_interrupt from before and just after
> the problem occurs?
>
I'll fire up a for-review kernel and see what it says.
> > 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.
>
> You're the only person I've heard of with this problem. You'll need to
> figure out what's causing it. We still handle ACPI events, it was just
> an internal timer potentially firing off DDC that was removed.
>
I wonder if it's the VBIOS triggering continuous events? It may have
started happening when I updated to revision A13 (the latest) of the
Dell D830 BIOS. Perhaps I'm the only tester with a D830?
Any idea how I could track this down?
More information about the Intel-gfx
mailing list