[Intel-gfx] [PATCH] Manage PIPESTAT pending interrupt values to unblock vblank interrupts
Eric Anholt
eric at anholt.net
Fri Nov 7 20:11:35 CET 2008
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?
> 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.
--
Eric Anholt
eric at anholt.net eric.anholt at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20081107/0b01d046/attachment.sig>
More information about the Intel-gfx
mailing list