[Intel-gfx] [PATCH] i915: add page flipping ioctl

Chris Wilson chris at chris-wilson.co.uk
Thu Feb 19 20:37:01 CET 2009


With a few additional suggestions by Jesse, I've managed to get
tear-free compositing working on i915. Here's the diff on top of the
original patch (though obviously this is just a suggestion, still need
to prevent multiple pending flips to the same plane and ensure that the
old buffer is eventually unpinned, and you might choose to drop the
mutex around the wait_for_vblank ;-):

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index bc440ff..b4d347c 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -835,19 +835,18 @@ static int i915_pipe_to_plane(struct drm_device *dev, int pipe)
 	u32 reg, pipe_mask;
 
 	if (pipe == 0)
-		pipe_mask = DISPPLANE_SEL_PIPE_A;
+		pipe_mask = DISPPLANE_SEL_PIPE_A | DISPLAY_PLANE_ENABLE;
 	else
-		pipe_mask = DISPPLANE_SEL_PIPE_B;
+		pipe_mask = DISPPLANE_SEL_PIPE_B | DISPLAY_PLANE_ENABLE;
 
+#define DISPPLANE_ENABLED_PIPE_MASK (DISPLAY_PLANE_ENABLE | DISPPLANE_SEL_PIPE_MASK)
 	reg = I915_READ(DSPACNTR);
-	if ((reg & DISPLAY_PLANE_ENABLE) &&
-	    ((reg & DISPPLANE_SEL_PIPE_MASK) == pipe_mask))
-			return 0;
+	if ((reg & DISPPLANE_ENABLED_PIPE_MASK) == pipe_mask)
+		return 0;
 
 	reg = I915_READ(DSPBCNTR);
-	if ((reg & DISPLAY_PLANE_ENABLE) &&
-	    ((reg & DISPPLANE_SEL_PIPE_MASK) == pipe_mask))
-			return 1;
+	if ((reg & DISPPLANE_ENABLED_PIPE_MASK) == pipe_mask)
+		return 1;
 
 	return -1;
 }
@@ -893,6 +892,8 @@ static int i915_gem_page_flip(struct drm_device *dev, void *data,
 		return -EBADF;
 
 	obj_priv = obj->driver_private;
+
+	mutex_lock(&dev->struct_mutex);
 	if (obj_priv->tiling_mode != I915_TILING_NONE &&
 	    obj_priv->tiling_mode != I915_TILING_X) {
 		DRM_ERROR("can only flip non-tiled or X tiled pages\n");
@@ -903,13 +904,18 @@ static int i915_gem_page_flip(struct drm_device *dev, void *data,
 	ret = i915_gem_object_pin(obj, 0);
 	if (ret) {
 		DRM_ERROR("failed to pin object for flip\n");
-		ret = -EBUSY;
+		goto out_unref;
+	}
+
+	ret = i915_gem_object_set_to_gtt_domain(obj, 0);
+	if (ret) {
+		DRM_ERROR("failed to set object to gtt domain for flip\n");
 		goto out_unref;
 	}
 
 	offset = obj_priv->gtt_offset;
 	pitch = obj_priv->stride;
-	tiled = !!(obj_priv->tiling_mode == I915_TILING_X);
+	tiled = obj_priv->tiling_mode == I915_TILING_X;
 
 	/*
 	 * Queue the flip with the lock held in case the flip happens
@@ -919,21 +925,27 @@ static int i915_gem_page_flip(struct drm_device *dev, void *data,
 
 	BEGIN_LP_RING(4);
 	OUT_RING(CMD_OP_DISPLAYBUFFER_INFO | (plane << 20));
-	OUT_RING((pitch / 8) << 3); /* flip queue and/or pitch */
-	OUT_RING(offset | tiled);
-	OUT_RING(((flip_data->x - 1) << 16) | (flip_data->y - 1));
+	OUT_RING(pitch);
+	if (IS_I965G(dev)) {
+	    OUT_RING(offset | tiled);
+	    OUT_RING(((flip_data->x - 1) << 16) | (flip_data->y - 1));
+	} else {
+	    OUT_RING(offset);
+	    OUT_RING(0);
+	}
 	ADVANCE_LP_RING();
 
-	list_add_tail(&obj_priv->vblank_head, &dev_priv->mm.vblank_list[pipe]);
-	drm_vblank_get(dev, pipe);
+	ret = drm_vblank_get(dev, pipe);
+	if (ret == 0)
+		list_add_tail(&obj_priv->vblank_head,
+			      &dev_priv->mm.vblank_list[pipe]);
 
 	spin_unlock_irqrestore(&dev_priv->vblank_lock, flags);
 
-	if (flip_data->flags & I915_PAGE_FLIP_WAIT)
+	if (ret == 0 && flip_data->flags & I915_PAGE_FLIP_WAIT)
 		wait_for_completion(&obj_priv->vblank);
 
 out_unref:
-	mutex_lock(&dev->struct_mutex);
 	drm_gem_object_unreference(obj);
 	mutex_unlock(&dev->struct_mutex);
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8f2dc72..28e8177 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2467,6 +2467,25 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file_priv)
 	return ret;
 }
 
+static void
+i915_gem_object_wait_for_vblank(struct drm_device *dev,
+				struct drm_gem_object *obj)
+{
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *obj_priv;
+	unsigned long irqflags;
+	int wait_for_vblank;
+
+	obj_priv = obj->driver_private;
+
+	spin_lock_irqsave(&dev_priv->vblank_lock, irqflags);
+	wait_for_vblank = !list_empty(&obj_priv->vblank_head);
+	spin_unlock_irqrestore(&dev_priv->vblank_lock, irqflags);
+
+	if (wait_for_vblank)
+		wait_for_completion(&obj_priv->vblank);
+}
+
 int
 i915_gem_execbuffer(struct drm_device *dev, void *data,
 		    struct drm_file *file_priv)
@@ -2477,7 +2496,6 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
 	struct drm_i915_gem_exec_object *exec_list = NULL;
 	struct drm_gem_object **object_list = NULL;
 	struct drm_gem_object *batch_obj;
-	unsigned long irqflags;
 	int ret, i, pinned = 0;
 	uint64_t exec_offset;
 	uint32_t seqno, flush_domains;
@@ -2542,23 +2560,14 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
 			ret = -EBADF;
 			goto err;
 		}
+
+		i915_gem_object_wait_for_vblank(dev, object_list[i]);
 	}
 
 	/* Pin and relocate */
 	for (pin_tries = 0; ; pin_tries++) {
 		ret = 0;
 		for (i = 0; i < args->buffer_count; i++) {
-			struct drm_i915_gem_object *obj_priv;
-
-			obj_priv = object_list[i]->driver_private;
-			spin_lock_irqsave(&dev_priv->vblank_lock, irqflags);
-			if (!list_empty(&obj_priv->vblank_head)) {
-				spin_unlock_irqrestore(&dev_priv->vblank_lock,
-						       irqflags);
-				wait_for_completion(&obj_priv->vblank);
-			} else
-				spin_unlock_irqrestore(&dev_priv->vblank_lock,
-						       irqflags);
 			object_list[i]->pending_read_domains = 0;
 			object_list[i]->pending_write_domain = 0;
 			ret = i915_gem_object_pin_and_relocate(object_list[i],
@@ -3379,6 +3388,8 @@ i915_gem_load(struct drm_device *dev)
 			  i915_gem_retire_work_handler);
 	dev_priv->mm.next_gem_seqno = 1;
 
+	spin_lock_init(&dev_priv->vblank_lock);
+
 	/* Old X drivers will take 0-2 for front, back, depth buffers */
 	dev_priv->fence_reg_start = 3;
 
 

> The other piece, of course, is to come up with a KMS API; the current mode set 
> interface *should* be sufficient for flipping, but afaik it's only been 
> tested with panning flips, so may need some work.  An alternative would be to 
> make this API also work w/KMS, which might mean re-defining the "pipe" field 
> to a CRTC id if KMS is active...  Any thoughts?

The part I'm struggling with is working out how to fill out the pipe
parameter based on mode/connector/encoder chosen from
drmModeGetResources() - which presumably is where you would take the
crtc id instead of the pipe id.
-ickle




More information about the Intel-gfx mailing list