[Intel-gfx] [PATCH] drm/i915: intel hw has only one gpu write domain

Daniel Vetter daniel.vetter at ffwll.ch
Sun Mar 14 12:18:15 CET 2010


It therefore holds that all objects on the gpu_write_domain are
in this single write domain. Drop an if checking for the write
domain in process_flushing_list.

For some odd reason retire_commands thinks that the sampler cache
can be written to. Remove this and the assorted logic in do_execbuf.

Also check that userspace doesn't try to screw us over by claiming
to write to some strange cache.

The idea for this patch emerged from a discussion with Chris Wilson.

Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h |    4 ++
 drivers/gpu/drm/i915/i915_gem.c |   69 +++++++++++++++++++--------------------
 2 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 979439c..a11d76a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -527,6 +527,10 @@ typedef struct drm_i915_private {
 		 * All elements on this list will belong to either the
 		 * active_list or flushing_list, last_rendering_seqno can
 		 * be used to differentiate between the two elements.
+		 *
+		 * Because intel hw has only one write domain, it holds that
+		 * write_domain == I915_GEM_DOMAIN_RENDER for all elements
+		 * on this list.
 		 */
 		struct list_head gpu_write_list;
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fba37e9..dda36ad 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1560,7 +1560,7 @@ i915_gem_object_move_to_inactive(struct drm_gem_object *obj)
 
 static void
 i915_gem_process_flushing_list(struct drm_device *dev,
-			       uint32_t flush_domains, uint32_t seqno)
+			       uint32_t seqno)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *obj_priv, *next;
@@ -1569,24 +1569,20 @@ i915_gem_process_flushing_list(struct drm_device *dev,
 				 &dev_priv->mm.gpu_write_list,
 				 gpu_write_list) {
 		struct drm_gem_object *obj = obj_priv->obj;
+		uint32_t old_write_domain = obj->write_domain;
 
-		if ((obj->write_domain & flush_domains) ==
-		    obj->write_domain) {
-			uint32_t old_write_domain = obj->write_domain;
-
-			obj->write_domain = 0;
-			list_del_init(&obj_priv->gpu_write_list);
-			i915_gem_object_move_to_active(obj, seqno);
+		obj->write_domain = 0;
+		list_del_init(&obj_priv->gpu_write_list);
+		i915_gem_object_move_to_active(obj, seqno);
 
-			/* update the fence lru list */
-			if (obj_priv->fence_reg != I915_FENCE_REG_NONE)
-				list_move_tail(&obj_priv->fence_list,
-						&dev_priv->mm.fence_list);
+		/* update the fence lru list */
+		if (obj_priv->fence_reg != I915_FENCE_REG_NONE)
+			list_move_tail(&obj_priv->fence_list,
+					&dev_priv->mm.fence_list);
 
-			trace_i915_gem_object_change_domain(obj,
-							    obj->read_domains,
-							    old_write_domain);
-		}
+		trace_i915_gem_object_change_domain(obj,
+						    obj->read_domains,
+						    old_write_domain);
 	}
 }
 
@@ -1648,8 +1644,12 @@ i915_add_request(struct drm_device *dev, struct drm_file *file_priv,
 	/* Associate any objects on the flushing list matching the write
 	 * domain we're flushing with our flush.
 	 */
-	if (flush_domains != 0) 
-		i915_gem_process_flushing_list(dev, flush_domains, seqno);
+	if (flush_domains != 0) {
+		BUG_ON((flush_domains & I915_GEM_DOMAIN_RENDER)
+				!= I915_GEM_DOMAIN_RENDER);
+
+		i915_gem_process_flushing_list(dev, seqno);
+	}
 
 	if (!dev_priv->mm.suspended) {
 		mod_timer(&dev_priv->hangcheck_timer, jiffies + DRM_I915_HANGCHECK_PERIOD);
@@ -1665,22 +1665,18 @@ i915_add_request(struct drm_device *dev, struct drm_file *file_priv,
  * Ensures that all commands in the ring are finished
  * before signalling the CPU
  */
-static uint32_t
+static void
 i915_retire_commands(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	uint32_t cmd = MI_FLUSH | MI_NO_WRITE_FLUSH;
-	uint32_t flush_domains = 0;
 	RING_LOCALS;
 
-	/* The sampler always gets flushed on i965 (sigh) */
-	if (IS_I965G(dev))
-		flush_domains |= I915_GEM_DOMAIN_SAMPLER;
 	BEGIN_LP_RING(2);
 	OUT_RING(cmd);
 	OUT_RING(0); /* noop */
 	ADVANCE_LP_RING();
-	return flush_domains;
+	return;
 }
 
 /**
@@ -3367,25 +3363,28 @@ i915_gem_object_pin_and_relocate(struct drm_gem_object *obj,
 		}
 
 		/* Validate that the target is in a valid r/w GPU domain */
-		if (reloc->write_domain & (reloc->write_domain - 1)) {
-			DRM_ERROR("reloc with multiple write domains: "
+		if (reloc->write_domain & I915_GEM_DOMAIN_CPU ||
+		    reloc->read_domains & I915_GEM_DOMAIN_CPU) {
+			DRM_ERROR("reloc with read/write CPU domains: "
 				  "obj %p target %d offset %d "
 				  "read %08x write %08x",
 				  obj, reloc->target_handle,
 				  (int) reloc->offset,
 				  reloc->read_domains,
 				  reloc->write_domain);
+			drm_gem_object_unreference(target_obj);
+			i915_gem_object_unpin(obj);
 			return -EINVAL;
 		}
-		if (reloc->write_domain & I915_GEM_DOMAIN_CPU ||
-		    reloc->read_domains & I915_GEM_DOMAIN_CPU) {
-			DRM_ERROR("reloc with read/write CPU domains: "
+		if (reloc->write_domain
+		    && reloc->write_domain != I915_GEM_DOMAIN_RENDER) {
+			DRM_ERROR("Nonexistant write domain: "
 				  "obj %p target %d offset %d "
-				  "read %08x write %08x",
+				  "new %08x old %08x\n",
 				  obj, reloc->target_handle,
 				  (int) reloc->offset,
-				  reloc->read_domains,
-				  reloc->write_domain);
+				  reloc->write_domain,
+				  target_obj->pending_write_domain);
 			drm_gem_object_unreference(target_obj);
 			i915_gem_object_unpin(obj);
 			return -EINVAL;
@@ -3725,7 +3724,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	struct drm_i915_gem_relocation_entry *relocs = NULL;
 	int ret = 0, ret2, i, pinned = 0;
 	uint64_t exec_offset;
-	uint32_t seqno, flush_domains, reloc_index;
+	uint32_t seqno, reloc_index;
 	int pin_tries, flips;
 
 #if WATCH_EXEC
@@ -3967,7 +3966,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	 * Ensure that the commands in the batch buffer are
 	 * finished before the interrupt fires
 	 */
-	flush_domains = i915_retire_commands(dev);
+	i915_retire_commands(dev);
 
 	i915_verify_inactive(dev, __FILE__, __LINE__);
 
@@ -3978,7 +3977,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	 * *some* interrupts representing completion of buffers that we can
 	 * wait on when trying to clear up gtt space).
 	 */
-	seqno = i915_add_request(dev, file_priv, flush_domains);
+	seqno = i915_add_request(dev, file_priv, 0);
 	BUG_ON(seqno == 0);
 	for (i = 0; i < args->buffer_count; i++) {
 		struct drm_gem_object *obj = object_list[i];
-- 
1.6.6.1




More information about the Intel-gfx mailing list