[Intel-gfx] [PATCH] [drm/i915] Always read pipestat in irq_handler
Li Peng
peng.li at linux.intel.com
Fri Nov 28 10:13:08 CET 2008
Hi, Keith:
I am seeing a kerneloops due to this commit on 945GME, that in
drm_irq_install(), request_irq() will
try the interrupt handler if it is a shared irq, at this time iir is
zero but pipeb_stat is not zero, so the handler assume
there is an interrupt happened and call drm_handle_vblank() to handle
it, but we don't finish the irq install at all with
data structure like "dev->_vblank_count" allocated. Then cause kernel
oops "unable to handle kernel NULL pointer" as below. Can we keep iir
check code ? That should avoid this problem.
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index c367358..fe7f7aa 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -181,6 +181,9 @@ irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS)
iir = I915_READ(IIR);
+ if (iir == 0)
+ return IRQ_NONE;
+
if (IS_I965G(dev)) {
vblank_status = I915_START_VBLANK_INTERRUPT_STATUS;
vblank_enable = PIPE_START_VBLANK_INTERRUPT_ENABLE;
[ 1241.938452] [drm:drm_ioctl] pid=2089, cmd=0x6459, nr=0x59, dev
0xe200, auth=1
[ 1241.938624] [drm:drm_agp_bind_pages]
[ 1241.938683] [drm:drm_irq_install] irq=16
[ 1241.938704] [drm:i915_driver_irq_handler] *********** pipeb_stats 202
[ 1241.938720] BUG: unable to handle kernel NULL pointer dereference at
00000004
[ 1241.938729] IP: [<c05686a8>] drm_handle_vblank+0x19/0xd6
[ 1241.938744] *pdpt = 00000000362f6001 *pde = 0000000000000000
[ 1241.938753] Oops: 0002 [#1] SMP
[ 1241.938759] last sysfs
file: /sys/devices/pci0000:00/0000:00:1c.3/0000:01:00.0/resource
[ 1241.938767] Dumping ftrace buffer:
[ 1241.938772] (ftrace buffer empty)
[ 1241.938776] Modules linked in: fuse
[ 1241.938781]
[ 1241.938787] Pid: 2089, comm: Xorg Not tainted (2.6.28-rc6 #38) 1000H
[ 1241.938793] EIP: 0060:[<c05686a8>] EFLAGS: 00213002 CPU: 0
[ 1241.938800] EIP is at drm_handle_vblank+0x19/0xd6
[ 1241.938805] EAX: 00000004 EBX: 00000000 ECX: 00007676 EDX: 00000001
[ 1241.938810] ESI: f6cb0800 EDI: 00000202 EBP: f60d1e4c ESP: f60d1e28
[ 1241.938815] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[ 1241.938820] Process Xorg (pid: 2089, ti=f60d1000 task=f6cba6b0
task.ti=f60d1000)
[ 1241.938825] Stack:
[ 1241.938828] 00000001 00000202 f60d1e40 c0703f54 c0811997 f60d1e4c
00000000 f6cae000
[ 1241.938840] 00000202 f60d1e80 c0570f26 f6cae09c f6cae08c f6cb0800
00000000 00000000
[ 1241.938852] 00000002 00203083 00000000 00203246 00000000 00000010
f60d1ea4 c0457c91
[ 1241.938866] Call Trace:
[ 1241.938869] [<c0703f54>] ? printk+0xf/0x11
[ 1241.938878] [<c0570f26>] ? i915_driver_irq_handler+0x1ec/0x216
[ 1241.938888] [<c0457c91>] ? request_irq+0xc9/0xe3
[ 1241.938895] [<c0570d3a>] ? i915_driver_irq_handler+0x0/0x216
[ 1241.938903] [<c0568d56>] ? drm_irq_install+0xdd/0x11f
[ 1241.938909] [<c0573ec5>] ? i915_gem_entervt_ioctl+0x41f/0x42a
[ 1241.938917] [<c0566af8>] ? drm_ioctl+0x1b0/0x225
[ 1241.938923] [<c0573aa6>] ? i915_gem_entervt_ioctl+0x0/0x42a
[ 1241.938930] [<c048d8e9>] ? vfs_ioctl+0x50/0x69
[ 1241.938937] [<c048dcae>] ? do_vfs_ioctl+0x3ac/0x3dd
[ 1241.938944] [<c0441be5>] ? tick_dev_program_event+0x28/0x95
[ 1241.938952] [<c0441c9c>] ? tick_program_event+0x22/0x29
[ 1241.938959] [<c043be3d>] ? hrtimer_interrupt+0x134/0x154
[ 1241.938968] [<c048dd1f>] ? sys_ioctl+0x40/0x5a
[ 1241.938975] [<c04039a1>] ? sysenter_do_call+0x12/0x21
[ 1241.938983] Code: 81 00 02 00 00 81 c2 88 13 00 00 e8 7c 85 ec ff 5d
c3 55 89 e5 57 56 89 c6 53 89 d0 83 ec 18 c1 e0 02 89 55 dc 03 86 e0 01
00 00 <f0> ff 00 6b c2 0c b9 01 00 00 00 03 86 dc 01 00 00 ba 01 00 00
[ 1241.939007] EIP: [<c05686a8>] drm_handle_vblank+0x19/0xd6 SS:ESP
0068:f60d1e28
[ 1241.939007] ---[ end trace 7794c4c7218d2e1a ]---
[ 1241.944733] [drm:drm_vm_shm_close] 0xb7f60000,0x00002000
[ 1241.959056] [drm:drm_fasync] fd = -1, device = 0xe200
[ 1241.959066] [drm:drm_release] open_count = 1
[ 1241.959073] [drm:drm_release] pid = 2089, device = 0xe200, open_count
= 1
[ 1241.959080] [drm:drm_release] File f609ed40 released, freeing lock
for context 1
[ 1241.959132] [drm:drm_release] *ERROR* Device busy: 1 0
On Wed, 2008-11-19 at 14:05 -0800, Keith Packard wrote:
> Because we write pipestat before iir, it's possible that a pipestat
> interrupt will occur between the pipestat write and the iir write. This
> leaves pipestat with an interrupt status not visible in iir. This may cause
> an interrupt flood as we never clear the pipestat event.
>
> Signed-off-by: Keith Packard <keithp at keithp.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 61 ++++++++++++++++++++++++++------------
> 1 files changed, 42 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 7965043..879a696 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -170,37 +170,60 @@ irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS)
> drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> u32 iir, new_iir;
> u32 pipea_stats, pipeb_stats;
> + u32 vblank_status;
> + u32 vblank_enable;
> int vblank = 0;
> unsigned long irqflags;
> + int irq_received;
> + int ret = IRQ_NONE;
>
> atomic_inc(&dev_priv->irq_received);
>
> iir = I915_READ(IIR);
>
> - if (iir == 0)
> - return IRQ_NONE;
> + if (IS_I965G(dev)) {
> + vblank_status = I915_START_VBLANK_INTERRUPT_STATUS;
> + vblank_enable = PIPE_START_VBLANK_INTERRUPT_ENABLE;
> + } else {
> + vblank_status = I915_VBLANK_INTERRUPT_STATUS;
> + vblank_enable = I915_VBLANK_INTERRUPT_ENABLE;
> + }
>
> - do {
> - pipea_stats = 0;
> - pipeb_stats = 0;
> + for (;;) {
> + irq_received = iir != 0;
> +
> + /* Can't rely on pipestat interrupt bit in iir as it might
> + * have been cleared after the pipestat interrupt was received
> + */
> + spin_lock_irqsave(&dev_priv->user_irq_lock, irqflags);
> + pipea_stats = I915_READ(PIPEASTAT);
> + pipeb_stats = I915_READ(PIPEBSTAT);
> /*
> * Clear the PIPE(A|B)STAT regs before the IIR
> */
> - if (iir & I915_DISPLAY_PIPE_A_EVENT_INTERRUPT) {
> - spin_lock_irqsave(&dev_priv->user_irq_lock, irqflags);
> - pipea_stats = I915_READ(PIPEASTAT);
> + if (pipea_stats & 0x8000ffff) {
> I915_WRITE(PIPEASTAT, pipea_stats);
> - spin_unlock_irqrestore(&dev_priv->user_irq_lock,
> - irqflags);
> + WARN((iir & I915_DISPLAY_PIPE_A_EVENT_INTERRUPT) == 0 &&
> + (pipea_stats & vblank_enable) != 0 &&
> + (pipea_stats & vblank_status) != 0,
> + "Pipe A vblank event not in IIR\n");
> + irq_received = 1;
> }
>
> - if (iir & I915_DISPLAY_PIPE_B_EVENT_INTERRUPT) {
> - spin_lock_irqsave(&dev_priv->user_irq_lock, irqflags);
> - pipeb_stats = I915_READ(PIPEBSTAT);
> + if (pipeb_stats & 0x8000ffff) {
> I915_WRITE(PIPEBSTAT, pipeb_stats);
> - spin_unlock_irqrestore(&dev_priv->user_irq_lock,
> - irqflags);
> + WARN((iir & I915_DISPLAY_PIPE_B_EVENT_INTERRUPT) == 0 &&
> + (pipeb_stats & vblank_enable) &&
> + (pipeb_stats & vblank_status) != 0,
> + "Pipe B vblank event not in IIR\n");
> + irq_received = 1;
> }
> + spin_unlock_irqrestore(&dev_priv->user_irq_lock, irqflags);
> +
> + if (!irq_received)
> + break;
> +
> + ret = IRQ_HANDLED;
>
> I915_WRITE(IIR, iir);
> new_iir = I915_READ(IIR); /* Flush posted writes */
> @@ -214,12 +237,12 @@ irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS)
> DRM_WAKEUP(&dev_priv->irq_queue);
> }
>
> - if (pipea_stats & I915_VBLANK_INTERRUPT_STATUS) {
> + if (pipea_stats & vblank_status) {
> vblank++;
> drm_handle_vblank(dev, 0);
> }
>
> - if (pipeb_stats & I915_VBLANK_INTERRUPT_STATUS) {
> + if (pipeb_stats & vblank_status) {
> vblank++;
> drm_handle_vblank(dev, 1);
> }
> @@ -244,9 +267,9 @@ irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS)
> * stray interrupts.
> */
> iir = new_iir;
> - } while (iir != 0 && dev->pdev->msi_enabled);
> + }
>
> - return IRQ_HANDLED;
> + return ret;
> }
>
> static int i915_emit_irq(struct drm_device * dev)
More information about the Intel-gfx
mailing list