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 <[EMAIL PROTECTED]>
> ---
>  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)


-------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to