From: Steven J N. <st...@sn...> - 2008-11-07 20:46:20
|
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? |