[Intel-gfx] [PATCH] [drm/i915] Handle swap buffers in work proc instead of softirq
Keith Packard
keithp at keithp.com
Fri Oct 17 09:44:43 CEST 2008
Swap buffers must hold both the DRM lock and the struct_mutex to access
both drawable clip lists and the hardware ring. struct_mutex may only be
acquired in a non-interrupt context. This means that vblank will be entirely
executed outside of IRQ context, which is not ideal, but should avoid
lengthy delays while the ring is full along with making it actually work
reliably.
Signed-off-by: Keith Packard <keithp at keithp.com>
---
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 | 153 +++++++++++++++++++++------------------
4 files changed, 90 insertions(+), 77 deletions(-)
diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c
index a4caf95..888159e 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 eae4ed3..f20ffe1 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 9ac73dd..9255088 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 d5a73a6..1bfb39c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -121,8 +121,11 @@ 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_do_swaps(struct drm_device *dev)
+static void i915_vblank_tasklet(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
unsigned long irqflags;
@@ -141,6 +144,8 @@ static void i915_vblank_do_swaps(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;
@@ -165,7 +170,7 @@ static void i915_vblank_do_swaps(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_do_swaps(struct drm_device *dev)
drw = drm_get_drawable_info(dev, vbl_swap->drw_id);
- if (!drw || drw->num_rects == 0) {
- 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 && drw_cmp->num_rects != 0 &&
+ /* 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_do_swaps(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_do_swaps(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_do_swaps(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,38 +359,37 @@ u32 i915_get_vblank_counter(struct drm_device *dev, int plane)
}
void
-i915_vblank_tasklet(struct drm_device *dev)
-{
- if (in_interrupt()) {
- DRM_DEBUG("vblank_tasklet in_interrupt\n");
- if (!mutex_trylock(&dev->struct_mutex)) {
- drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
- schedule_work(&dev_priv->mm.vblank_work);
- return;
- }
- } else {
- DRM_DEBUG("vblank_tasklet in process\n");
- mutex_lock(&dev->struct_mutex);
- }
- i915_vblank_do_swaps(dev);
- mutex_unlock(&dev->struct_mutex);
-}
-
-void
-i915_gem_vblank_work_handler(struct work_struct *work)
+i915_vblank_work_handler(struct work_struct *work)
{
drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t,
- mm.vblank_work);
+ vblank_work);
struct drm_device *dev = dev_priv->dev;
+ unsigned long irqflags;
- if (dev->lock.hw_lock != NULL) {
- drm_locked_tasklet(dev, i915_vblank_tasklet);
+ if (dev->lock.hw_lock == NULL) {
+ i915_vblank_tasklet(dev);
return;
- } else {
- mutex_lock(&dev->struct_mutex);
- i915_vblank_do_swaps(dev);
- mutex_unlock(&dev->struct_mutex);
}
+
+ 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);
+
+ i915_vblank_tasklet(dev);
+ drm_lock_free(&dev->lock, DRM_KERNEL_CONTEXT);
}
irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS)
@@ -461,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->lock.hw_lock != 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;
}
@@ -718,7 +722,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;
@@ -782,44 +786,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++;
@@ -846,6 +858,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. */
--
1.5.6.5
More information about the Intel-gfx
mailing list