From e9d21d7f5ae1e625f3687d88bb50b00478c533ad Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Thu, 16 Oct 2008 11:31:38 -0700 Subject: i915: Protect vblank IRQ reg access with spinlock This uses the same spinlock as the user_irq code as it shares the same register, ensuring that interrupt registers are updated atomically. Signed-off-by: Keith Packard Signed-off-by: Eric Anholt Signed-off-by: Dave Airlie --- drivers/gpu/drm/i915/i915_irq.c | 69 ++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index baae511c785..671dd79d0aa 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -481,22 +481,24 @@ static int i915_emit_irq(struct drm_device * dev) void i915_user_irq_get(struct drm_device *dev) { drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; + unsigned long irqflags; - spin_lock(&dev_priv->user_irq_lock); + spin_lock_irqsave(&dev_priv->user_irq_lock, irqflags); if (dev->irq_enabled && (++dev_priv->user_irq_refcount == 1)) i915_enable_irq(dev_priv, I915_USER_INTERRUPT); - spin_unlock(&dev_priv->user_irq_lock); + spin_unlock_irqrestore(&dev_priv->user_irq_lock, irqflags); } void i915_user_irq_put(struct drm_device *dev) { drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; + unsigned long irqflags; - spin_lock(&dev_priv->user_irq_lock); + spin_lock_irqsave(&dev_priv->user_irq_lock, irqflags); BUG_ON(dev->irq_enabled && dev_priv->user_irq_refcount <= 0); if (dev->irq_enabled && (--dev_priv->user_irq_refcount == 0)) i915_disable_irq(dev_priv, I915_USER_INTERRUPT); - spin_unlock(&dev_priv->user_irq_lock); + spin_unlock_irqrestore(&dev_priv->user_irq_lock, irqflags); } static int i915_wait_irq(struct drm_device * dev, int irq_nr) @@ -584,33 +586,37 @@ int i915_enable_vblank(struct drm_device *dev, int plane) int pipe = i915_get_pipe(dev, plane); u32 pipestat_reg = 0; u32 pipestat; + u32 interrupt = 0; + unsigned long irqflags; switch (pipe) { case 0: pipestat_reg = PIPEASTAT; - i915_enable_irq(dev_priv, I915_DISPLAY_PIPE_A_EVENT_INTERRUPT); + interrupt = I915_DISPLAY_PIPE_A_EVENT_INTERRUPT; break; case 1: pipestat_reg = PIPEBSTAT; - i915_enable_irq(dev_priv, I915_DISPLAY_PIPE_B_EVENT_INTERRUPT); + interrupt = I915_DISPLAY_PIPE_B_EVENT_INTERRUPT; break; default: DRM_ERROR("tried to enable vblank on non-existent pipe %d\n", pipe); - break; + return 0; } - if (pipestat_reg) { - 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); - } + spin_lock_irqsave(&dev_priv->user_irq_lock, irqflags); + 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_irq(dev_priv, interrupt); + spin_unlock_irqrestore(&dev_priv->user_irq_lock, irqflags); return 0; } @@ -621,31 +627,36 @@ void i915_disable_vblank(struct drm_device *dev, int plane) int pipe = i915_get_pipe(dev, plane); u32 pipestat_reg = 0; u32 pipestat; + u32 interrupt = 0; + unsigned long irqflags; switch (pipe) { case 0: pipestat_reg = PIPEASTAT; - i915_disable_irq(dev_priv, I915_DISPLAY_PIPE_A_EVENT_INTERRUPT); + interrupt = I915_DISPLAY_PIPE_A_EVENT_INTERRUPT; break; case 1: pipestat_reg = PIPEBSTAT; - i915_disable_irq(dev_priv, I915_DISPLAY_PIPE_B_EVENT_INTERRUPT); + interrupt = I915_DISPLAY_PIPE_B_EVENT_INTERRUPT; break; default: DRM_ERROR("tried to disable vblank on non-existent pipe %d\n", pipe); + return; break; } - if (pipestat_reg) { - pipestat = I915_READ(pipestat_reg); - pipestat &= ~(PIPE_START_VBLANK_INTERRUPT_ENABLE | - PIPE_VBLANK_INTERRUPT_ENABLE); - /* Clear any stale interrupt status */ - pipestat |= (PIPE_START_VBLANK_INTERRUPT_STATUS | - PIPE_VBLANK_INTERRUPT_STATUS); - I915_WRITE(pipestat_reg, pipestat); - } + spin_lock_irqsave(&dev_priv->user_irq_lock, irqflags); + i915_disable_irq(dev_priv, interrupt); + pipestat = I915_READ(pipestat_reg); + pipestat &= ~(PIPE_START_VBLANK_INTERRUPT_ENABLE | + 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 */ + spin_unlock_irqrestore(&dev_priv->user_irq_lock, irqflags); } /* Set the vblank monitor pipe -- cgit v1.2.3-70-g09d2 From 863842736fb4571b22e0f7f4696bf13eeec57166 Mon Sep 17 00:00:00 2001 From: Zhenyu Wang Date: Fri, 17 Oct 2008 13:15:48 +0800 Subject: drm: Set cliprects to NULL when changing drawable to having 0 cliprects. This avoids setting the cliprects pointer to a zero-sized allocation. Signed-off-by: Zhenyu Wang Signed-off-by: Eric Anholt Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_drawable.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_drawable.c b/drivers/gpu/drm/drm_drawable.c index 1839c57663c..70b15d1b8f5 100644 --- a/drivers/gpu/drm/drm_drawable.c +++ b/drivers/gpu/drm/drm_drawable.c @@ -111,7 +111,9 @@ int drm_update_drawable_info(struct drm_device *dev, void *data, struct drm_file switch (update->type) { case DRM_DRAWABLE_CLIPRECTS: - if (update->num != info->num_rects) { + if (update->num == 0) + rects = NULL; + else if (update->num != info->num_rects) { rects = drm_alloc(update->num * sizeof(struct drm_clip_rect), DRM_MEM_BUFS); } else -- cgit v1.2.3-70-g09d2 From d1ed629f44b3a4108d5c445971535f05f441fce7 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 17 Oct 2008 00:44:42 -0700 Subject: i915: Disable MSI on GM965 (errata says it doesn't work) Current Intel errata for the GM965 says that using MSI may cause interrupts to be delayed or lost. The only workaround offered is to not use it. Signed-off-by: Keith Packard Signed-off-by: Eric Anholt Signed-off-by: Dave Airlie --- drivers/gpu/drm/i915/i915_dma.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index db34780edbb..01de536e021 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -844,8 +844,11 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) * correctly in testing on 945G. * This may be a side effect of MSI having been made available for PEG * and the registers being closely associated. + * + * According to chipset errata, on the 965GM, MSI interrupts may + * be lost or delayed */ - if (!IS_I945G(dev) && !IS_I945GM(dev)) + if (!IS_I945G(dev) && !IS_I945GM(dev) && !IS_I965GM(dev)) if (pci_enable_msi(dev->pdev)) DRM_ERROR("failed to enable MSI\n"); -- cgit v1.2.3-70-g09d2 From 786225eb2f4e55b5dda3cf8c62a145e824aae199 Mon Sep 17 00:00:00 2001 From: Zhenyu Wang Date: Fri, 17 Oct 2008 15:48:44 +0800 Subject: drm: fix leak of cliprects in drm_rmdraw() Signed-off-by: Zhenyu Wang Signed-off-by: Eric Anholt Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_drawable.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_drawable.c b/drivers/gpu/drm/drm_drawable.c index 70b15d1b8f5..4a794d89942 100644 --- a/drivers/gpu/drm/drm_drawable.c +++ b/drivers/gpu/drm/drm_drawable.c @@ -76,11 +76,14 @@ int drm_rmdraw(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_draw *draw = data; unsigned long irqflags; + struct drm_drawable_info *info; spin_lock_irqsave(&dev->drw_lock, irqflags); - drm_free(drm_get_drawable_info(dev, draw->handle), - sizeof(struct drm_drawable_info), DRM_MEM_BUFS); + info = drm_get_drawable_info(dev, draw->handle); + drm_free(info->rects, info->num_rects * sizeof(struct drm_clip_rect), + DRM_MEM_BUFS); + drm_free(info, sizeof(struct drm_drawable_info), DRM_MEM_BUFS); idr_remove(&dev->drw_idr, draw->handle); -- cgit v1.2.3-70-g09d2 From 35ad68c18148a18938ff4f40e945c9734e7d2265 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 17 Oct 2008 11:03:53 -0700 Subject: drm: Remove two leaks of vblank reference count in error paths. If the failing paths were hit, the vblank IRQ would never get turned off again. Signed-off-by: Eric Anholt Acked-by: Keith Packard Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_irq.c | 5 ++++- drivers/gpu/drm/i915/i915_irq.c | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 4091b9e291f..212a94f715b 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -594,11 +594,14 @@ int drm_wait_vblank(struct drm_device *dev, void *data, goto done; } + /* Get a refcount on the vblank, which will be released by + * drm_vbl_send_signals(). + */ ret = drm_vblank_get(dev, crtc); if (ret) { drm_free(vbl_sig, sizeof(struct drm_vbl_sig), DRM_MEM_DRIVER); - return ret; + goto done; } atomic_inc(&dev->vbl_signal_pending); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 671dd79d0aa..2cd853b2c56 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -771,6 +771,7 @@ int i915_vblank_swap(struct drm_device *dev, void *data, vbl_swap->plane == plane && vbl_swap->sequence == swap->sequence) { spin_unlock_irqrestore(&dev_priv->swaps_lock, irqflags); + drm_vblank_put(dev, pipe); DRM_DEBUG("Already scheduled\n"); return 0; } -- cgit v1.2.3-70-g09d2 From 053d7f244a8739d96d316c77a97cd063804c8e35 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 17 Oct 2008 15:41:26 -0700 Subject: i915: Enable IMR passthrough of vblank events before enabling it in pipestat. Otherwise, if we lost the race, the pipestat bit would be set without being reflected in IIR, and we would never clear the pipestat bit so the pipe event would never be generated again, and all vblank waits would time out. Signed-off-by: Eric Anholt Acked-by: Keith Packard Signed-off-by: Dave Airlie --- drivers/gpu/drm/i915/i915_irq.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 2cd853b2c56..ca55c40353a 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -605,6 +605,15 @@ int i915_enable_vblank(struct drm_device *dev, int plane) } 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; @@ -615,7 +624,6 @@ int i915_enable_vblank(struct drm_device *dev, int plane) PIPE_VBLANK_INTERRUPT_STATUS); I915_WRITE(pipestat_reg, pipestat); (void) I915_READ(pipestat_reg); /* Posting read */ - i915_enable_irq(dev_priv, interrupt); spin_unlock_irqrestore(&dev_priv->user_irq_lock, irqflags); return 0; -- cgit v1.2.3-70-g09d2 From a2d44cca4fc9ae04ed2c9fc2ef16b6989eeae218 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 17 Oct 2008 14:41:03 -0700 Subject: i915: Don't dereference HWS in /proc debug files when it isn't initialized. Signed-off-by: Eric Anholt Acked-by: Keith Packard Signed-off-by: Dave Airlie --- drivers/gpu/drm/i915/i915_gem_proc.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_proc.c b/drivers/gpu/drm/i915/i915_gem_proc.c index 15d4160415b..93de15b4c9a 100644 --- a/drivers/gpu/drm/i915/i915_gem_proc.c +++ b/drivers/gpu/drm/i915/i915_gem_proc.c @@ -192,7 +192,12 @@ static int i915_gem_seqno_info(char *buf, char **start, off_t offset, *start = &buf[offset]; *eof = 0; - DRM_PROC_PRINT("Current sequence: %d\n", i915_get_gem_seqno(dev)); + if (dev_priv->hw_status_page != NULL) { + DRM_PROC_PRINT("Current sequence: %d\n", + i915_get_gem_seqno(dev)); + } else { + DRM_PROC_PRINT("Current sequence: hws uninitialized\n"); + } DRM_PROC_PRINT("Waiter sequence: %d\n", dev_priv->mm.waiting_gem_seqno); DRM_PROC_PRINT("IRQ sequence: %d\n", dev_priv->mm.irq_gem_seqno); @@ -230,8 +235,12 @@ static int i915_interrupt_info(char *buf, char **start, off_t offset, I915_READ(PIPEBSTAT)); DRM_PROC_PRINT("Interrupts received: %d\n", atomic_read(&dev_priv->irq_received)); - DRM_PROC_PRINT("Current sequence: %d\n", - i915_get_gem_seqno(dev)); + if (dev_priv->hw_status_page != NULL) { + DRM_PROC_PRINT("Current sequence: %d\n", + i915_get_gem_seqno(dev)); + } else { + DRM_PROC_PRINT("Current sequence: hws uninitialized\n"); + } DRM_PROC_PRINT("Waiter sequence: %d\n", dev_priv->mm.waiting_gem_seqno); DRM_PROC_PRINT("IRQ sequence: %d\n", -- cgit v1.2.3-70-g09d2 From fe8133dc07e613587f8e667bce769edce8490f2a Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Fri, 17 Oct 2008 15:43:02 -0700 Subject: i915: Fix format string warnings on x86-64. Signed-off-by: Eric Anholt Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_proc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_proc.c b/drivers/gpu/drm/drm_proc.c index d490db4c0de..ae73b7f7249 100644 --- a/drivers/gpu/drm/drm_proc.c +++ b/drivers/gpu/drm/drm_proc.c @@ -522,12 +522,12 @@ static int drm_gem_one_name_info(int id, void *ptr, void *data) struct drm_gem_object *obj = ptr; struct drm_gem_name_info_data *nid = data; - DRM_INFO("name %d size %d\n", obj->name, obj->size); + DRM_INFO("name %d size %zd\n", obj->name, obj->size); if (nid->eof) return 0; nid->len += sprintf(&nid->buf[nid->len], - "%6d%9d%8d%9d\n", + "%6d %8zd %7d %8d\n", obj->name, obj->size, atomic_read(&obj->handlecount.refcount), atomic_read(&obj->refcount.refcount)); -- cgit v1.2.3-70-g09d2 From 9e44af790f8bf8c3aa8a3101fd4f9bca2e932baa Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Thu, 16 Oct 2008 21:18:27 -0700 Subject: drm/i915: hold dev->struct_mutex and DRM lock during vblank ring operations To synchronize clip lists with the X server, the DRM lock must be held while looking at drawable clip lists. To synchronize with other ring access, the ring mutex must be held while inserting commands into the ring. Failure to do the first resulted in easy visual corruption when moving windows, and the second could have corrupted the ring with DRI2. Grabbing the DRM lock involves using the DRM tasklet mechanism, grabbing the ring mutex means potentially sleeping. Deal with both of these by always running the tasklet from a work handler. Also, protect from clip list changes since the vblank request was queued by making sure the window has at least one rectangle while looking inside, preventing oopses . Signed-off-by: Keith Packard Signed-off-by: Eric Anholt Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_lock.c | 2 + drivers/gpu/drm/i915/i915_drv.h | 10 +-- drivers/gpu/drm/i915/i915_gem.c | 2 - drivers/gpu/drm/i915/i915_irq.c | 138 +++++++++++++++++++++++++--------------- 4 files changed, 92 insertions(+), 60 deletions(-) diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c index a4caf95485d..888159e03d2 100644 --- a/drivers/gpu/drm/drm_lock.c +++ b/drivers/gpu/drm/drm_lock.c @@ -232,6 +232,7 @@ int drm_lock_take(struct drm_lock_data *lock_data, } return 0; } +EXPORT_SYMBOL(drm_lock_take); /** * This takes a lock forcibly and hands it to context. Should ONLY be used @@ -299,6 +300,7 @@ int drm_lock_free(struct drm_lock_data *lock_data, unsigned int context) wake_up_interruptible(&lock_data->lock_queue); return 0; } +EXPORT_SYMBOL(drm_lock_free); /** * If we get here, it means that the process has called DRM_IOCTL_LOCK diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index eae4ed3956e..f20ffe17df7 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -90,7 +90,7 @@ struct mem_block { typedef struct _drm_i915_vbl_swap { struct list_head head; drm_drawable_t drw_id; - unsigned int plane; + unsigned int pipe; unsigned int sequence; } drm_i915_vbl_swap_t; @@ -240,6 +240,9 @@ typedef struct drm_i915_private { u8 saveDACDATA[256*3]; /* 256 3-byte colors */ u8 saveCR[37]; + /** Work task for vblank-related ring access */ + struct work_struct vblank_work; + struct { struct drm_mm gtt_space; @@ -285,9 +288,6 @@ typedef struct drm_i915_private { */ struct delayed_work retire_work; - /** Work task for vblank-related ring access */ - struct work_struct vblank_work; - uint32_t next_gem_seqno; /** @@ -441,7 +441,7 @@ extern int i915_irq_wait(struct drm_device *dev, void *data, void i915_user_irq_get(struct drm_device *dev); void i915_user_irq_put(struct drm_device *dev); -extern void i915_gem_vblank_work_handler(struct work_struct *work); +extern void i915_vblank_work_handler(struct work_struct *work); extern irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS); extern void i915_driver_irq_preinstall(struct drm_device * dev); extern int i915_driver_irq_postinstall(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 9ac73dd1b42..92550883d19 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2550,8 +2550,6 @@ i915_gem_load(struct drm_device *dev) INIT_LIST_HEAD(&dev_priv->mm.request_list); INIT_DELAYED_WORK(&dev_priv->mm.retire_work, i915_gem_retire_work_handler); - INIT_WORK(&dev_priv->mm.vblank_work, - i915_gem_vblank_work_handler); dev_priv->mm.next_gem_seqno = 1; i915_gem_detect_bit_6_swizzle(dev); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index ca55c40353a..86237569443 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -121,6 +121,9 @@ i915_pipe_enabled(struct drm_device *dev, int pipe) * Emit blits for scheduled buffer swaps. * * This function will be called with the HW lock held. + * Because this function must grab the ring mutex (dev->struct_mutex), + * it can no longer run at soft irq time. We'll fix this when we do + * the DRI2 swap buffer work. */ static void i915_vblank_tasklet(struct drm_device *dev) { @@ -141,6 +144,8 @@ static void i915_vblank_tasklet(struct drm_device *dev) u32 ropcpp = (0xcc << 16) | ((cpp - 1) << 24); RING_LOCALS; + mutex_lock(&dev->struct_mutex); + if (IS_I965G(dev) && sarea_priv->front_tiled) { cmd |= XY_SRC_COPY_BLT_DST_TILED; dst_pitch >>= 2; @@ -150,8 +155,8 @@ static void i915_vblank_tasklet(struct drm_device *dev) src_pitch >>= 2; } - counter[0] = drm_vblank_count(dev, 0); - counter[1] = drm_vblank_count(dev, 1); + counter[0] = drm_vblank_count(dev, i915_get_plane(dev, 0)); + counter[1] = drm_vblank_count(dev, i915_get_plane(dev, 1)); DRM_DEBUG("\n"); @@ -165,7 +170,7 @@ static void i915_vblank_tasklet(struct drm_device *dev) list_for_each_safe(list, tmp, &dev_priv->vbl_swaps.head) { drm_i915_vbl_swap_t *vbl_swap = list_entry(list, drm_i915_vbl_swap_t, head); - int pipe = i915_get_pipe(dev, vbl_swap->plane); + int pipe = vbl_swap->pipe; if ((counter[pipe] - vbl_swap->sequence) > (1<<23)) continue; @@ -179,20 +184,19 @@ static void i915_vblank_tasklet(struct drm_device *dev) drw = drm_get_drawable_info(dev, vbl_swap->drw_id); - if (!drw) { - spin_unlock(&dev->drw_lock); - drm_free(vbl_swap, sizeof(*vbl_swap), DRM_MEM_DRIVER); - spin_lock(&dev_priv->swaps_lock); - continue; - } - list_for_each(hit, &hits) { drm_i915_vbl_swap_t *swap_cmp = list_entry(hit, drm_i915_vbl_swap_t, head); struct drm_drawable_info *drw_cmp = drm_get_drawable_info(dev, swap_cmp->drw_id); - if (drw_cmp && + /* Make sure both drawables are still + * around and have some rectangles before + * we look inside to order them for the + * blts below. + */ + if (drw_cmp && drw_cmp->num_rects > 0 && + drw && drw->num_rects > 0 && drw_cmp->rects[0].y1 > drw->rects[0].y1) { list_add_tail(list, hit); break; @@ -212,6 +216,7 @@ static void i915_vblank_tasklet(struct drm_device *dev) if (nhits == 0) { spin_unlock_irqrestore(&dev_priv->swaps_lock, irqflags); + mutex_unlock(&dev->struct_mutex); return; } @@ -265,18 +270,21 @@ static void i915_vblank_tasklet(struct drm_device *dev) drm_i915_vbl_swap_t *swap_hit = list_entry(hit, drm_i915_vbl_swap_t, head); struct drm_clip_rect *rect; - int num_rects, plane; + int num_rects, pipe; unsigned short top, bottom; drw = drm_get_drawable_info(dev, swap_hit->drw_id); + /* The drawable may have been destroyed since + * the vblank swap was queued + */ if (!drw) continue; rect = drw->rects; - plane = swap_hit->plane; - top = upper[plane]; - bottom = lower[plane]; + pipe = swap_hit->pipe; + top = upper[pipe]; + bottom = lower[pipe]; for (num_rects = drw->num_rects; num_rects--; rect++) { int y1 = max(rect->y1, top); @@ -302,6 +310,7 @@ static void i915_vblank_tasklet(struct drm_device *dev) } spin_unlock_irqrestore(&dev->drw_lock, irqflags); + mutex_unlock(&dev->struct_mutex); list_for_each_safe(hit, tmp, &hits) { drm_i915_vbl_swap_t *swap_hit = @@ -350,18 +359,37 @@ u32 i915_get_vblank_counter(struct drm_device *dev, int plane) } void -i915_gem_vblank_work_handler(struct work_struct *work) +i915_vblank_work_handler(struct work_struct *work) { - drm_i915_private_t *dev_priv; - struct drm_device *dev; + drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t, + vblank_work); + struct drm_device *dev = dev_priv->dev; + unsigned long irqflags; + + if (dev->lock.hw_lock == NULL) { + i915_vblank_tasklet(dev); + return; + } - dev_priv = container_of(work, drm_i915_private_t, - mm.vblank_work); - dev = dev_priv->dev; + spin_lock_irqsave(&dev->tasklet_lock, irqflags); + dev->locked_tasklet_func = i915_vblank_tasklet; + spin_unlock_irqrestore(&dev->tasklet_lock, irqflags); + + /* Try to get the lock now, if this fails, the lock + * holder will execute the tasklet during unlock + */ + if (!drm_lock_take(&dev->lock, DRM_KERNEL_CONTEXT)) + return; + + dev->lock.lock_time = jiffies; + atomic_inc(&dev->counts[_DRM_STAT_LOCKS]); + + spin_lock_irqsave(&dev->tasklet_lock, irqflags); + dev->locked_tasklet_func = NULL; + spin_unlock_irqrestore(&dev->tasklet_lock, irqflags); - mutex_lock(&dev->struct_mutex); i915_vblank_tasklet(dev); - mutex_unlock(&dev->struct_mutex); + drm_lock_free(&dev->lock, DRM_KERNEL_CONTEXT); } irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS) @@ -441,12 +469,8 @@ irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS) if (iir & I915_ASLE_INTERRUPT) opregion_asle_intr(dev); - if (vblank && dev_priv->swaps_pending > 0) { - if (dev_priv->ring.ring_obj == NULL) - drm_locked_tasklet(dev, i915_vblank_tasklet); - else - schedule_work(&dev_priv->mm.vblank_work); - } + if (vblank && dev_priv->swaps_pending > 0) + schedule_work(&dev_priv->vblank_work); return IRQ_HANDLED; } @@ -706,7 +730,7 @@ int i915_vblank_swap(struct drm_device *dev, void *data, { drm_i915_private_t *dev_priv = dev->dev_private; drm_i915_vblank_swap_t *swap = data; - drm_i915_vbl_swap_t *vbl_swap; + drm_i915_vbl_swap_t *vbl_swap, *vbl_old; unsigned int pipe, seqtype, curseq, plane; unsigned long irqflags; struct list_head *list; @@ -770,45 +794,52 @@ int i915_vblank_swap(struct drm_device *dev, void *data, } } + vbl_swap = drm_calloc(1, sizeof(*vbl_swap), DRM_MEM_DRIVER); + + if (!vbl_swap) { + DRM_ERROR("Failed to allocate memory to queue swap\n"); + drm_vblank_put(dev, pipe); + return -ENOMEM; + } + + vbl_swap->drw_id = swap->drawable; + vbl_swap->pipe = pipe; + vbl_swap->sequence = swap->sequence; + spin_lock_irqsave(&dev_priv->swaps_lock, irqflags); list_for_each(list, &dev_priv->vbl_swaps.head) { - vbl_swap = list_entry(list, drm_i915_vbl_swap_t, head); + vbl_old = list_entry(list, drm_i915_vbl_swap_t, head); - if (vbl_swap->drw_id == swap->drawable && - vbl_swap->plane == plane && - vbl_swap->sequence == swap->sequence) { + if (vbl_old->drw_id == swap->drawable && + vbl_old->pipe == pipe && + vbl_old->sequence == swap->sequence) { spin_unlock_irqrestore(&dev_priv->swaps_lock, irqflags); drm_vblank_put(dev, pipe); + drm_free(vbl_swap, sizeof(*vbl_swap), DRM_MEM_DRIVER); DRM_DEBUG("Already scheduled\n"); return 0; } } - spin_unlock_irqrestore(&dev_priv->swaps_lock, irqflags); - - if (dev_priv->swaps_pending >= 100) { + if (dev_priv->swaps_pending >= 10) { DRM_DEBUG("Too many swaps queued\n"); + DRM_DEBUG(" pipe 0: %d pipe 1: %d\n", + drm_vblank_count(dev, i915_get_plane(dev, 0)), + drm_vblank_count(dev, i915_get_plane(dev, 1))); + + list_for_each(list, &dev_priv->vbl_swaps.head) { + vbl_old = list_entry(list, drm_i915_vbl_swap_t, head); + DRM_DEBUG("\tdrw %x pipe %d seq %x\n", + vbl_old->drw_id, vbl_old->pipe, + vbl_old->sequence); + } + spin_unlock_irqrestore(&dev_priv->swaps_lock, irqflags); drm_vblank_put(dev, pipe); + drm_free(vbl_swap, sizeof(*vbl_swap), DRM_MEM_DRIVER); return -EBUSY; } - vbl_swap = drm_calloc(1, sizeof(*vbl_swap), DRM_MEM_DRIVER); - - if (!vbl_swap) { - DRM_ERROR("Failed to allocate memory to queue swap\n"); - drm_vblank_put(dev, pipe); - return -ENOMEM; - } - - DRM_DEBUG("\n"); - - vbl_swap->drw_id = swap->drawable; - vbl_swap->plane = plane; - vbl_swap->sequence = swap->sequence; - - spin_lock_irqsave(&dev_priv->swaps_lock, irqflags); - list_add_tail(&vbl_swap->head, &dev_priv->vbl_swaps.head); dev_priv->swaps_pending++; @@ -835,6 +866,7 @@ int i915_driver_irq_postinstall(struct drm_device *dev) spin_lock_init(&dev_priv->swaps_lock); INIT_LIST_HEAD(&dev_priv->vbl_swaps.head); + INIT_WORK(&dev_priv->vblank_work, i915_vblank_work_handler); dev_priv->swaps_pending = 0; /* Set initial unmasked IRQs to just the selected vblank pipes. */ -- cgit v1.2.3-70-g09d2 From 42f52ef8d96b1434f12ad9f895b5412fda392847 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Sat, 18 Oct 2008 19:39:29 -0700 Subject: drm/i915: use pipes, not planes to label vblank data vblank in the kernel is far simpler if it deals with pipes instead of planes, so we're changing both user and kernel side. Signed-off-by: Keith Packard Signed-off-by: Eric Anholt Signed-off-by: Dave Airlie --- drivers/gpu/drm/i915/i915_irq.c | 73 +++++++++++------------------------------ 1 file changed, 20 insertions(+), 53 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 86237569443..26f48932a51 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -59,43 +59,6 @@ i915_disable_irq(drm_i915_private_t *dev_priv, u32 mask) } } -/** - * i915_get_pipe - return the the pipe associated with a given plane - * @dev: DRM device - * @plane: plane to look for - * - * The Intel Mesa & 2D drivers call the vblank routines with a plane number - * rather than a pipe number, since they may not always be equal. This routine - * maps the given @plane back to a pipe number. - */ -static int -i915_get_pipe(struct drm_device *dev, int plane) -{ - drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; - u32 dspcntr; - - dspcntr = plane ? I915_READ(DSPBCNTR) : I915_READ(DSPACNTR); - - return dspcntr & DISPPLANE_SEL_PIPE_MASK ? 1 : 0; -} - -/** - * i915_get_plane - return the the plane associated with a given pipe - * @dev: DRM device - * @pipe: pipe to look for - * - * The Intel Mesa & 2D drivers call the vblank routines with a plane number - * rather than a plane number, since they may not always be equal. This routine - * maps the given @pipe back to a plane number. - */ -static int -i915_get_plane(struct drm_device *dev, int pipe) -{ - if (i915_get_pipe(dev, 0) == pipe) - return 0; - return 1; -} - /** * i915_pipe_enabled - check if a pipe is enabled * @dev: DRM device @@ -155,8 +118,8 @@ static void i915_vblank_tasklet(struct drm_device *dev) src_pitch >>= 2; } - counter[0] = drm_vblank_count(dev, i915_get_plane(dev, 0)); - counter[1] = drm_vblank_count(dev, i915_get_plane(dev, 1)); + counter[0] = drm_vblank_count(dev, 0); + counter[1] = drm_vblank_count(dev, 1); DRM_DEBUG("\n"); @@ -322,15 +285,16 @@ static void i915_vblank_tasklet(struct drm_device *dev) } } -u32 i915_get_vblank_counter(struct drm_device *dev, int plane) +/* Called from drm generic code, passed a 'crtc', which + * we use as a pipe index + */ +u32 i915_get_vblank_counter(struct drm_device *dev, int pipe) { drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; unsigned long high_frame; unsigned long low_frame; u32 high1, high2, low, count; - int pipe; - pipe = i915_get_pipe(dev, plane); high_frame = pipe ? PIPEBFRAMEHIGH : PIPEAFRAMEHIGH; low_frame = pipe ? PIPEBFRAMEPIXEL : PIPEAFRAMEPIXEL; @@ -426,7 +390,7 @@ irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS) else if (pipea_stats & (PIPE_START_VBLANK_INTERRUPT_STATUS| PIPE_VBLANK_INTERRUPT_STATUS)) { vblank++; - drm_handle_vblank(dev, i915_get_plane(dev, 0)); + drm_handle_vblank(dev, 0); } I915_WRITE(PIPEASTAT, pipea_stats); @@ -444,7 +408,7 @@ irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS) else if (pipeb_stats & (PIPE_START_VBLANK_INTERRUPT_STATUS| PIPE_VBLANK_INTERRUPT_STATUS)) { vblank++; - drm_handle_vblank(dev, i915_get_plane(dev, 1)); + drm_handle_vblank(dev, 1); } if (pipeb_stats & I915_LEGACY_BLC_EVENT_STATUS) @@ -604,10 +568,12 @@ int i915_irq_wait(struct drm_device *dev, void *data, return i915_wait_irq(dev, irqwait->irq_seq); } -int i915_enable_vblank(struct drm_device *dev, int plane) +/* Called from drm generic code, passed 'crtc' which + * we use as a pipe index + */ +int i915_enable_vblank(struct drm_device *dev, int pipe) { drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; - int pipe = i915_get_pipe(dev, plane); u32 pipestat_reg = 0; u32 pipestat; u32 interrupt = 0; @@ -653,10 +619,12 @@ int i915_enable_vblank(struct drm_device *dev, int plane) return 0; } -void i915_disable_vblank(struct drm_device *dev, int plane) +/* Called from drm generic code, passed 'crtc' which + * we use as a pipe index + */ +void i915_disable_vblank(struct drm_device *dev, int pipe) { drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; - int pipe = i915_get_pipe(dev, plane); u32 pipestat_reg = 0; u32 pipestat; u32 interrupt = 0; @@ -731,7 +699,7 @@ int i915_vblank_swap(struct drm_device *dev, void *data, drm_i915_private_t *dev_priv = dev->dev_private; drm_i915_vblank_swap_t *swap = data; drm_i915_vbl_swap_t *vbl_swap, *vbl_old; - unsigned int pipe, seqtype, curseq, plane; + unsigned int pipe, seqtype, curseq; unsigned long irqflags; struct list_head *list; int ret; @@ -752,8 +720,7 @@ int i915_vblank_swap(struct drm_device *dev, void *data, return -EINVAL; } - plane = (swap->seqtype & _DRM_VBLANK_SECONDARY) ? 1 : 0; - pipe = i915_get_pipe(dev, plane); + pipe = (swap->seqtype & _DRM_VBLANK_SECONDARY) ? 1 : 0; seqtype = swap->seqtype & (_DRM_VBLANK_RELATIVE | _DRM_VBLANK_ABSOLUTE); @@ -825,8 +792,8 @@ int i915_vblank_swap(struct drm_device *dev, void *data, if (dev_priv->swaps_pending >= 10) { DRM_DEBUG("Too many swaps queued\n"); DRM_DEBUG(" pipe 0: %d pipe 1: %d\n", - drm_vblank_count(dev, i915_get_plane(dev, 0)), - drm_vblank_count(dev, i915_get_plane(dev, 1))); + drm_vblank_count(dev, 0), + drm_vblank_count(dev, 1)); list_for_each(list, &dev_priv->vbl_swaps.head) { vbl_old = list_entry(list, drm_i915_vbl_swap_t, head); -- cgit v1.2.3-70-g09d2 From 49568873705e45a0de77b7824a9a46d3201019a7 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Tue, 21 Oct 2008 11:38:50 -0700 Subject: drm: Add 32-bit compatibility for DRM_IOCTL_UPDATE_DRAW. This fixes vblank support for a 32-bit X Server on a 64-bit kernel. Signed-off-by: Eric Anholt Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_ioc32.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c index 90f5a8d9bdc..920b72fbc95 100644 --- a/drivers/gpu/drm/drm_ioc32.c +++ b/drivers/gpu/drm/drm_ioc32.c @@ -64,6 +64,8 @@ #define DRM_IOCTL_SG_ALLOC32 DRM_IOW( 0x38, drm_scatter_gather32_t) #define DRM_IOCTL_SG_FREE32 DRM_IOW( 0x39, drm_scatter_gather32_t) +#define DRM_IOCTL_UPDATE_DRAW32 DRM_IOW( 0x3f, drm_update_draw32_t) + #define DRM_IOCTL_WAIT_VBLANK32 DRM_IOWR(0x3a, drm_wait_vblank32_t) typedef struct drm_version_32 { @@ -952,6 +954,37 @@ static int compat_drm_sg_free(struct file *file, unsigned int cmd, DRM_IOCTL_SG_FREE, (unsigned long)request); } +typedef struct drm_update_draw32 { + drm_drawable_t handle; + unsigned int type; + unsigned int num; + /* 64-bit version has a 32-bit pad here */ + u64 data; /**< Pointer */ +} __attribute__((packed)) drm_update_draw32_t; + +static int compat_drm_update_draw(struct file *file, unsigned int cmd, + unsigned long arg) +{ + drm_update_draw32_t update32; + struct drm_update_draw __user *request; + int err; + + if (copy_from_user(&update32, (void __user *)arg, sizeof(update32))) + return -EFAULT; + + request = compat_alloc_user_space(sizeof(*request)); + if (!access_ok(VERIFY_WRITE, request, sizeof(*request)) || + __put_user(update32.handle, &request->handle) || + __put_user(update32.type, &request->type) || + __put_user(update32.num, &request->num) || + __put_user(update32.data, &request->data)) + return -EFAULT; + + err = drm_ioctl(file->f_path.dentry->d_inode, file, + DRM_IOCTL_UPDATE_DRAW, (unsigned long)request); + return err; +} + struct drm_wait_vblank_request32 { enum drm_vblank_seq_type type; unsigned int sequence; @@ -1033,6 +1066,7 @@ drm_ioctl_compat_t *drm_compat_ioctls[] = { #endif [DRM_IOCTL_NR(DRM_IOCTL_SG_ALLOC32)] = compat_drm_sg_alloc, [DRM_IOCTL_NR(DRM_IOCTL_SG_FREE32)] = compat_drm_sg_free, + [DRM_IOCTL_NR(DRM_IOCTL_UPDATE_DRAW32)] = compat_drm_update_draw, [DRM_IOCTL_NR(DRM_IOCTL_WAIT_VBLANK32)] = compat_drm_wait_vblank, }; -- cgit v1.2.3-70-g09d2 From 7e78f72524b794fa8d73dc59aeeacc12a2e937fe Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Tue, 21 Oct 2008 11:53:01 -0700 Subject: drm: Avoid oops in DRM_IOCTL_RM_DRAW if a bad handle is supplied. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Eric Anholt Acked-by: Michel Dänzer Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_drawable.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/drm_drawable.c b/drivers/gpu/drm/drm_drawable.c index 4a794d89942..80be1cab62a 100644 --- a/drivers/gpu/drm/drm_drawable.c +++ b/drivers/gpu/drm/drm_drawable.c @@ -81,6 +81,10 @@ int drm_rmdraw(struct drm_device *dev, void *data, struct drm_file *file_priv) spin_lock_irqsave(&dev->drw_lock, irqflags); info = drm_get_drawable_info(dev, draw->handle); + if (info == NULL) { + spin_unlock_irqrestore(&dev->drw_lock, irqflags); + return -EINVAL; + } drm_free(info->rects, info->num_rects * sizeof(struct drm_clip_rect), DRM_MEM_BUFS); drm_free(info, sizeof(struct drm_drawable_info), DRM_MEM_BUFS); -- cgit v1.2.3-70-g09d2