svn commit: r189047 - head/sys/dev/drm
Robert Noland
rnoland at FreeBSD.org
Wed Feb 25 10:22:58 PST 2009
Author: rnoland
Date: Wed Feb 25 18:22:57 2009
New Revision: 189047
URL: http://svn.freebsd.org/changeset/base/189047
Log:
The vblank_swap ioctl was fundamentally race prone. Get rid of it.
MFC after: 2 weeks
Modified:
head/sys/dev/drm/i915_dma.c
head/sys/dev/drm/i915_drv.h
head/sys/dev/drm/i915_irq.c
Modified: head/sys/dev/drm/i915_dma.c
==============================================================================
--- head/sys/dev/drm/i915_dma.c Wed Feb 25 18:19:16 2009 (r189046)
+++ head/sys/dev/drm/i915_dma.c Wed Feb 25 18:22:57 2009 (r189047)
@@ -1087,7 +1087,6 @@ int i915_driver_load(struct drm_device *
#ifdef I915_HAVE_GEM
i915_gem_load(dev);
#endif
- DRM_SPININIT(&dev_priv->swaps_lock, "swap");
DRM_SPININIT(&dev_priv->user_irq_lock, "userirq");
#ifdef __linux__
@@ -1117,7 +1116,6 @@ int i915_driver_unload(struct drm_device
drm_rmmap(dev, dev_priv->mmio_map);
- DRM_SPINUNINIT(&dev_priv->swaps_lock);
DRM_SPINUNINIT(&dev_priv->user_irq_lock);
#ifdef __linux__
Modified: head/sys/dev/drm/i915_drv.h
==============================================================================
--- head/sys/dev/drm/i915_drv.h Wed Feb 25 18:19:16 2009 (r189046)
+++ head/sys/dev/drm/i915_drv.h Wed Feb 25 18:22:57 2009 (r189047)
@@ -178,9 +178,6 @@ typedef struct drm_i915_private {
struct drm_i915_validate_buffer *val_bufs;
#endif
- DRM_SPINTYPE swaps_lock;
- drm_i915_vbl_swap_t vbl_swaps;
- unsigned int swaps_pending;
#if defined(I915_HAVE_BUFFER)
/* DRI2 sarea */
struct drm_buffer_object *sarea_bo;
Modified: head/sys/dev/drm/i915_irq.c
==============================================================================
--- head/sys/dev/drm/i915_irq.c Wed Feb 25 18:19:16 2009 (r189046)
+++ head/sys/dev/drm/i915_irq.c Wed Feb 25 18:22:57 2009 (r189047)
@@ -121,277 +121,6 @@ i915_pipe_enabled(struct drm_device *dev
return 0;
}
-/**
- * Emit a synchronous flip.
- *
- * This function must be called with the drawable spinlock held.
- */
-static void
-i915_dispatch_vsync_flip(struct drm_device *dev, struct drm_drawable_info *drw,
- int plane)
-{
- drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
- drm_i915_sarea_t *sarea_priv = dev_priv->sarea_priv;
- u16 x1, y1, x2, y2;
- int pf_planes = 1 << plane;
-
- DRM_SPINLOCK_ASSERT(&dev->drw_lock);
-
- /* If the window is visible on the other plane, we have to flip on that
- * plane as well.
- */
- if (plane == 1) {
- x1 = sarea_priv->planeA_x;
- y1 = sarea_priv->planeA_y;
- x2 = x1 + sarea_priv->planeA_w;
- y2 = y1 + sarea_priv->planeA_h;
- } else {
- x1 = sarea_priv->planeB_x;
- y1 = sarea_priv->planeB_y;
- x2 = x1 + sarea_priv->planeB_w;
- y2 = y1 + sarea_priv->planeB_h;
- }
-
- if (x2 > 0 && y2 > 0) {
- int i, num_rects = drw->num_rects;
- struct drm_clip_rect *rect = drw->rects;
-
- for (i = 0; i < num_rects; i++)
- if (!(rect[i].x1 >= x2 || rect[i].y1 >= y2 ||
- rect[i].x2 <= x1 || rect[i].y2 <= y1)) {
- pf_planes = 0x3;
-
- break;
- }
- }
-
- i915_dispatch_flip(dev, pf_planes, 1);
-}
-
-/**
- * Emit blits for scheduled buffer swaps.
- *
- * This function will be called with the HW lock held.
- */
-static void i915_vblank_tasklet(struct drm_device *dev)
-{
- drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
- struct list_head *list, *tmp, hits, *hit;
- int nhits, nrects, slice[2], upper[2], lower[2], i, num_pages;
- unsigned counter[2];
- struct drm_drawable_info *drw;
- drm_i915_sarea_t *sarea_priv = dev_priv->sarea_priv;
- u32 cpp = dev_priv->cpp, offsets[3];
- u32 cmd = (cpp == 4) ? (XY_SRC_COPY_BLT_CMD |
- XY_SRC_COPY_BLT_WRITE_ALPHA |
- XY_SRC_COPY_BLT_WRITE_RGB)
- : XY_SRC_COPY_BLT_CMD;
- u32 src_pitch = sarea_priv->pitch * cpp;
- u32 dst_pitch = sarea_priv->pitch * cpp;
- /* COPY rop (0xcc), map cpp to magic color depth constants */
- u32 ropcpp = (0xcc << 16) | ((cpp - 1) << 24);
- RING_LOCALS;
-
- if (IS_I965G(dev) && sarea_priv->front_tiled) {
- cmd |= XY_SRC_COPY_BLT_DST_TILED;
- dst_pitch >>= 2;
- }
- if (IS_I965G(dev) && sarea_priv->back_tiled) {
- cmd |= XY_SRC_COPY_BLT_SRC_TILED;
- src_pitch >>= 2;
- }
-
- counter[0] = drm_vblank_count(dev, 0);
- counter[1] = drm_vblank_count(dev, 1);
-
- DRM_DEBUG("\n");
-
- INIT_LIST_HEAD(&hits);
-
- nhits = nrects = 0;
-
- /* No irqsave/restore necessary. This tasklet may be run in an
- * interrupt context or normal context, but we don't have to worry
- * about getting interrupted by something acquiring the lock, because
- * we are the interrupt context thing that acquires the lock.
- */
- DRM_SPINLOCK(&dev_priv->swaps_lock);
-
- /* Find buffer swaps scheduled for this vertical blank */
- 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);
-
- if ((counter[pipe] - vbl_swap->sequence) > (1<<23))
- continue;
-
- list_del(list);
- dev_priv->swaps_pending--;
- drm_vblank_put(dev, pipe);
-
- DRM_SPINUNLOCK(&dev_priv->swaps_lock);
- DRM_SPINLOCK(&dev->drw_lock);
-
- drw = drm_get_drawable_info(dev, vbl_swap->drw_id);
-
- if (!drw) {
- DRM_SPINUNLOCK(&dev->drw_lock);
- drm_free(vbl_swap, sizeof(*vbl_swap), DRM_MEM_DRIVER);
- DRM_SPINLOCK(&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->rects[0].y1 > drw->rects[0].y1) {
- list_add_tail(list, hit);
- break;
- }
- }
-
- DRM_SPINUNLOCK(&dev->drw_lock);
-
- /* List of hits was empty, or we reached the end of it */
- if (hit == &hits)
- list_add_tail(list, hits.prev);
-
- nhits++;
-
- DRM_SPINLOCK(&dev_priv->swaps_lock);
- }
-
- DRM_SPINUNLOCK(&dev_priv->swaps_lock);
-
- if (nhits == 0) {
- return;
- }
-
- i915_kernel_lost_context(dev);
-
- upper[0] = upper[1] = 0;
- slice[0] = max(sarea_priv->planeA_h / nhits, 1);
- slice[1] = max(sarea_priv->planeB_h / nhits, 1);
- lower[0] = sarea_priv->planeA_y + slice[0];
- lower[1] = sarea_priv->planeB_y + slice[0];
-
- offsets[0] = sarea_priv->front_offset;
- offsets[1] = sarea_priv->back_offset;
- offsets[2] = sarea_priv->third_offset;
- num_pages = sarea_priv->third_handle ? 3 : 2;
-
- DRM_SPINLOCK(&dev->drw_lock);
-
- /* Emit blits for buffer swaps, partitioning both outputs into as many
- * slices as there are buffer swaps scheduled in order to avoid tearing
- * (based on the assumption that a single buffer swap would always
- * complete before scanout starts).
- */
- for (i = 0; i++ < nhits;
- upper[0] = lower[0], lower[0] += slice[0],
- upper[1] = lower[1], lower[1] += slice[1]) {
- int init_drawrect = 1;
-
- if (i == nhits)
- lower[0] = lower[1] = sarea_priv->height;
-
- list_for_each(hit, &hits) {
- 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, front, back;
- unsigned short top, bottom;
-
- drw = drm_get_drawable_info(dev, swap_hit->drw_id);
-
- if (!drw)
- continue;
-
- plane = swap_hit->plane;
-
- if (swap_hit->flip) {
- i915_dispatch_vsync_flip(dev, drw, plane);
- continue;
- }
-
- if (init_drawrect) {
- int width = sarea_priv->width;
- int height = sarea_priv->height;
- if (IS_I965G(dev)) {
- BEGIN_LP_RING(4);
-
- OUT_RING(GFX_OP_DRAWRECT_INFO_I965);
- OUT_RING(0);
- OUT_RING(((width - 1) & 0xffff) | ((height - 1) << 16));
- OUT_RING(0);
-
- ADVANCE_LP_RING();
- } else {
- BEGIN_LP_RING(6);
-
- OUT_RING(GFX_OP_DRAWRECT_INFO);
- OUT_RING(0);
- OUT_RING(0);
- OUT_RING(((width - 1) & 0xffff) | ((height - 1) << 16));
- OUT_RING(0);
- OUT_RING(0);
-
- ADVANCE_LP_RING();
- }
-
- sarea_priv->ctxOwner = DRM_KERNEL_CONTEXT;
-
- init_drawrect = 0;
- }
-
- rect = drw->rects;
- top = upper[plane];
- bottom = lower[plane];
-
- front = (dev_priv->sarea_priv->pf_current_page >>
- (2 * plane)) & 0x3;
- back = (front + 1) % num_pages;
-
- for (num_rects = drw->num_rects; num_rects--; rect++) {
- int y1 = max(rect->y1, top);
- int y2 = min(rect->y2, bottom);
-
- if (y1 >= y2)
- continue;
-
- BEGIN_LP_RING(8);
-
- OUT_RING(cmd);
- OUT_RING(ropcpp | dst_pitch);
- OUT_RING((y1 << 16) | rect->x1);
- OUT_RING((y2 << 16) | rect->x2);
- OUT_RING(offsets[front]);
- OUT_RING((y1 << 16) | rect->x1);
- OUT_RING(src_pitch);
- OUT_RING(offsets[back]);
-
- ADVANCE_LP_RING();
- }
- }
- }
-
- DRM_SPINUNLOCK(&dev->drw_lock);
-
- list_for_each_safe(hit, tmp, &hits) {
- drm_i915_vbl_swap_t *swap_hit =
- list_entry(hit, drm_i915_vbl_swap_t, head);
-
- list_del(hit);
-
- drm_free(swap_hit, sizeof(*swap_hit), DRM_MEM_DRIVER);
- }
-}
-
u32 i915_get_vblank_counter(struct drm_device *dev, int plane)
{
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
@@ -526,11 +255,6 @@ irqreturn_t i915_driver_irq_handler(DRM_
#endif
}
- if (vblank) {
- if (dev_priv->swaps_pending > 0)
- drm_locked_tasklet(dev, i915_vblank_tasklet);
- }
-
return IRQ_HANDLED;
}
@@ -796,157 +520,22 @@ int i915_vblank_pipe_get(struct drm_devi
int i915_vblank_swap(struct drm_device *dev, void *data,
struct drm_file *file_priv)
{
- drm_i915_private_t *dev_priv = dev->dev_private;
- drm_i915_vblank_swap_t *swap = data;
- drm_i915_vbl_swap_t *vbl_swap;
- unsigned int pipe, seqtype, curseq, plane;
- unsigned long irqflags;
- struct list_head *list;
- int ret;
-
- if (!dev_priv) {
- DRM_ERROR("%s called with no initialization\n", __func__);
- return -EINVAL;
- }
-
- if (!dev_priv->sarea_priv || dev_priv->sarea_priv->rotation) {
- DRM_DEBUG("Rotation not supported\n");
- return -EINVAL;
- }
-
- if (swap->seqtype & ~(_DRM_VBLANK_RELATIVE | _DRM_VBLANK_ABSOLUTE |
- _DRM_VBLANK_SECONDARY | _DRM_VBLANK_NEXTONMISS |
- _DRM_VBLANK_FLIP)) {
- DRM_ERROR("Invalid sequence type 0x%x\n", swap->seqtype);
- return -EINVAL;
- }
-
- plane = (swap->seqtype & _DRM_VBLANK_SECONDARY) ? 1 : 0;
- pipe = i915_get_pipe(dev, plane);
-
- seqtype = swap->seqtype & (_DRM_VBLANK_RELATIVE | _DRM_VBLANK_ABSOLUTE);
-
- if (!(dev_priv->vblank_pipe & (1 << pipe))) {
- DRM_ERROR("Invalid pipe %d\n", pipe);
- return -EINVAL;
- }
-
- DRM_SPINLOCK_IRQSAVE(&dev->drw_lock, irqflags);
-
- /* It makes no sense to schedule a swap for a drawable that doesn't have
- * valid information at this point. E.g. this could mean that the X
- * server is too old to push drawable information to the DRM, in which
- * case all such swaps would become ineffective.
+ /* The delayed swap mechanism was fundamentally racy, and has been
+ * removed. The model was that the client requested a delayed flip/swap
+ * from the kernel, then waited for vblank before continuing to perform
+ * rendering. The problem was that the kernel might wake the client
+ * up before it dispatched the vblank swap (since the lock has to be
+ * held while touching the ringbuffer), in which case the client would
+ * clear and start the next frame before the swap occurred, and
+ * flicker would occur in addition to likely missing the vblank.
+ *
+ * In the absence of this ioctl, userland falls back to a correct path
+ * of waiting for a vblank, then dispatching the swap on its own.
+ * Context switching to userland and back is plenty fast enough for
+ * meeting the requirements of vblank swapping.
*/
- if (!drm_get_drawable_info(dev, swap->drawable)) {
- DRM_SPINUNLOCK_IRQRESTORE(&dev->drw_lock, irqflags);
- DRM_DEBUG("Invalid drawable ID %d\n", swap->drawable);
- return -EINVAL;
- }
-
- DRM_SPINUNLOCK_IRQRESTORE(&dev->drw_lock, irqflags);
-
- /*
- * We take the ref here and put it when the swap actually completes
- * in the tasklet.
- */
- ret = drm_vblank_get(dev, pipe);
- if (ret)
- return ret;
- curseq = drm_vblank_count(dev, pipe);
-
- if (seqtype == _DRM_VBLANK_RELATIVE)
- swap->sequence += curseq;
-
- if ((curseq - swap->sequence) <= (1<<23)) {
- if (swap->seqtype & _DRM_VBLANK_NEXTONMISS) {
- swap->sequence = curseq + 1;
- } else {
- DRM_DEBUG("Missed target sequence\n");
- drm_vblank_put(dev, pipe);
- return -EINVAL;
- }
- }
-
- if (swap->seqtype & _DRM_VBLANK_FLIP) {
- swap->sequence--;
-
- if ((curseq - swap->sequence) <= (1<<23)) {
- struct drm_drawable_info *drw;
-
- LOCK_TEST_WITH_RETURN(dev, file_priv);
- DRM_SPINLOCK_IRQSAVE(&dev->drw_lock, irqflags);
-
- drw = drm_get_drawable_info(dev, swap->drawable);
-
- if (!drw) {
- DRM_SPINUNLOCK_IRQRESTORE(&dev->drw_lock,
- irqflags);
- DRM_DEBUG("Invalid drawable ID %d\n",
- swap->drawable);
- drm_vblank_put(dev, pipe);
- return -EINVAL;
- }
-
- i915_dispatch_vsync_flip(dev, drw, plane);
-
- DRM_SPINUNLOCK_IRQRESTORE(&dev->drw_lock, irqflags);
-
- drm_vblank_put(dev, pipe);
- return 0;
- }
- }
-
- DRM_SPINLOCK_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);
-
- if (vbl_swap->drw_id == swap->drawable &&
- vbl_swap->plane == plane &&
- vbl_swap->sequence == swap->sequence) {
- vbl_swap->flip = (swap->seqtype & _DRM_VBLANK_FLIP);
- DRM_SPINUNLOCK_IRQRESTORE(&dev_priv->swaps_lock, irqflags);
- DRM_DEBUG("Already scheduled\n");
- return 0;
- }
- }
-
- DRM_SPINUNLOCK_IRQRESTORE(&dev_priv->swaps_lock, irqflags);
-
- if (dev_priv->swaps_pending >= 100) {
- DRM_DEBUG("Too many swaps queued\n");
- drm_vblank_put(dev, pipe);
- 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;
- vbl_swap->flip = (swap->seqtype & _DRM_VBLANK_FLIP);
-
- if (vbl_swap->flip)
- swap->sequence++;
-
- DRM_SPINLOCK_IRQSAVE(&dev_priv->swaps_lock, irqflags);
-
- list_add_tail(&vbl_swap->head, &dev_priv->vbl_swaps.head);
- dev_priv->swaps_pending++;
-
- DRM_SPINUNLOCK_IRQRESTORE(&dev_priv->swaps_lock, irqflags);
-
- return 0;
+ return -EINVAL;
}
/* drm_dma.h hooks
@@ -965,9 +554,6 @@ int i915_driver_irq_postinstall(struct d
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
int ret, num_pipes = 2;
- INIT_LIST_HEAD(&dev_priv->vbl_swaps.head);
- dev_priv->swaps_pending = 0;
-
dev_priv->user_irq_refcount = 0;
dev_priv->irq_mask_reg = ~0;
More information about the svn-src-head
mailing list