[Intel-gfx] [PATCH 4/4] drm/i915: Use eb for more stuff

Ben Widawsky ben at bwidawsk.net
Thu Oct 13 00:56:22 CEST 2011


This refactor is useful for some future work I'll be doing on the
execbuffer path. In addition to being a pretty easy prerequisite, it
also helped me track down the bug uncovered in the first patch.

Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  102 ++++++++++++++--------------
 1 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 182a2b9..b3beaae 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -40,6 +40,16 @@ struct change_domains {
 	uint32_t flips;
 };
 
+struct eb_objects {
+	struct drm_i915_gem_object *batch_obj;
+	struct list_head objects;
+	int buffer_count;
+	int mode;
+	int and;
+	struct hlist_head buckets[0];
+	/* DO NOT PUT ANYTHING HERE */
+};
+
 /*
  * Set the next domain for the specified object. This
  * may not actually perform the necessary flushing/invaliding though,
@@ -207,11 +217,6 @@ i915_gem_object_set_to_gpu_domain(struct drm_i915_gem_object *obj,
 		cd->flush_rings |= ring->id;
 }
 
-struct eb_objects {
-	int and;
-	struct hlist_head buckets[0];
-};
-
 static struct eb_objects *
 eb_create(int size)
 {
@@ -225,6 +230,8 @@ eb_create(int size)
 	if (eb == NULL)
 		return eb;
 
+	INIT_LIST_HEAD(&eb->objects);
+
 	eb->and = count - 1;
 	return eb;
 }
@@ -232,12 +239,22 @@ eb_create(int size)
 static void
 eb_reset(struct eb_objects *eb)
 {
+	while (!list_empty(&eb->objects)) {
+		struct drm_i915_gem_object *obj;
+
+		obj = list_first_entry(&eb->objects,
+				       struct drm_i915_gem_object,
+				       exec_list);
+		list_del_init(&obj->exec_list);
+		drm_gem_object_unreference(&obj->base);
+	}
 	memset(eb->buckets, 0, (eb->and+1)*sizeof(struct hlist_head));
 }
 
 static void
 eb_add_object(struct eb_objects *eb, struct drm_i915_gem_object *obj)
 {
+	list_add_tail(&obj->exec_list, &eb->objects);
 	hlist_add_head(&obj->exec_node,
 		       &eb->buckets[obj->exec_handle & eb->and]);
 }
@@ -262,6 +279,16 @@ eb_get_object(struct eb_objects *eb, unsigned long handle)
 static void
 eb_destroy(struct eb_objects *eb)
 {
+	while (!list_empty(&eb->objects)) {
+		struct drm_i915_gem_object *obj;
+
+		obj = list_first_entry(&eb->objects,
+				       struct drm_i915_gem_object,
+				       exec_list);
+		list_del_init(&obj->exec_list);
+		drm_gem_object_unreference(&obj->base);
+	}
+
 	kfree(eb);
 }
 
@@ -436,8 +463,7 @@ i915_gem_execbuffer_relocate_object_slow(struct drm_i915_gem_object *obj,
 
 static int
 i915_gem_execbuffer_relocate(struct drm_device *dev,
-			     struct eb_objects *eb,
-			     struct list_head *objects)
+			     struct eb_objects *eb)
 {
 	struct drm_i915_gem_object *obj;
 	int ret = 0;
@@ -450,7 +476,7 @@ i915_gem_execbuffer_relocate(struct drm_device *dev,
 	 * lockdep complains vehemently.
 	 */
 	pagefault_disable();
-	list_for_each_entry(obj, objects, exec_list) {
+	list_for_each_entry(obj, &eb->objects, exec_list) {
 		ret = i915_gem_execbuffer_relocate_object(obj, eb);
 		if (ret)
 			break;
@@ -618,24 +644,18 @@ static int
 i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
 				  struct drm_file *file,
 				  struct intel_ring_buffer *ring,
-				  struct list_head *objects,
 				  struct eb_objects *eb,
 				  struct drm_i915_gem_exec_object2 *exec,
 				  int count)
 {
 	struct drm_i915_gem_relocation_entry *reloc;
 	struct drm_i915_gem_object *obj;
+	struct list_head *objects = &eb->objects;
 	int *reloc_offset;
 	int i, total, ret;
 
 	/* We may process another execbuffer during the unlock... */
-	while (!list_empty(objects)) {
-		obj = list_first_entry(objects,
-				       struct drm_i915_gem_object,
-				       exec_list);
-		list_del_init(&obj->exec_list);
-		drm_gem_object_unreference(&obj->base);
-	}
+	eb_reset(eb);
 
 	mutex_unlock(&dev->struct_mutex);
 
@@ -676,7 +696,6 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
 	}
 
 	/* reacquire the objects */
-	eb_reset(eb);
 	for (i = 0; i < count; i++) {
 		obj = to_intel_bo(drm_gem_object_lookup(dev, file,
 							exec[i].handle));
@@ -687,7 +706,6 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
 			goto err;
 		}
 
-		list_add_tail(&obj->exec_list, objects);
 		obj->exec_handle = exec[i].handle;
 		obj->exec_entry = &exec[i];
 		eb_add_object(eb, obj);
@@ -1004,14 +1022,13 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		       struct drm_i915_gem_exec_object2 *exec)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	struct list_head objects;
-	struct eb_objects *eb;
-	struct drm_i915_gem_object *batch_obj;
+	struct eb_objects *eb = NULL;
+	struct drm_i915_gem_object *obj = NULL;
 	struct drm_clip_rect *cliprects = NULL;
 	struct intel_ring_buffer *ring;
 	u32 exec_start, exec_len;
 	u32 seqno;
-	int ret, mode, i;
+	int ret, i;
 
 	if (!i915_gem_check_execbuffer(args)) {
 		DRM_ERROR("execbuf with invalid offset/length\n");
@@ -1091,11 +1108,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		goto pre_mutex_err;
 	}
 
-	/* Look up object handles */
-	INIT_LIST_HEAD(&objects);
 	for (i = 0; i < args->buffer_count; i++) {
-		struct drm_i915_gem_object *obj;
-
 		obj = to_intel_bo(drm_gem_object_lookup(dev, file,
 							exec[i].handle));
 		if (&obj->base == NULL) {
@@ -1113,28 +1126,25 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 			goto err;
 		}
 
-		list_add_tail(&obj->exec_list, &objects);
 		obj->exec_handle = exec[i].handle;
 		obj->exec_entry = &exec[i];
 		eb_add_object(eb, obj);
 	}
 
-	/* take note of the batch buffer before we might reorder the lists */
-	batch_obj = list_entry(objects.prev,
-			       struct drm_i915_gem_object,
-			       exec_list);
+	/* The last object is the batch object */
+	eb->batch_obj = obj;
 
 	/* Move the objects en-masse into the GTT, evicting if necessary. */
-	ret = i915_gem_execbuffer_reserve(ring, file, &objects);
+	ret = i915_gem_execbuffer_reserve(ring, file, &eb->objects);
 	if (ret)
 		goto err;
 
 	/* The objects are in their final locations, apply the relocations. */
-	ret = i915_gem_execbuffer_relocate(dev, eb, &objects);
+	ret = i915_gem_execbuffer_relocate(dev, eb);
 	if (ret) {
 		if (ret == -EFAULT) {
 			ret = i915_gem_execbuffer_relocate_slow(dev, file, ring,
-								&objects, eb,
+								eb,
 								exec,
 								args->buffer_count);
 			BUG_ON(!mutex_is_locked(&dev->struct_mutex));
@@ -1143,20 +1153,20 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 			goto err;
 	}
 
-	mode = args->flags & I915_EXEC_CONSTANTS_MASK;
-	ret = i915_gem_set_constant_offset(ring, mode);
+	eb->mode = args->flags & I915_EXEC_CONSTANTS_MASK;
+	ret = i915_gem_set_constant_offset(ring, eb->mode);
 	if (ret)
 		goto err;
 
 	/* Set the pending read domains for the batch buffer to COMMAND */
-	if (batch_obj->base.pending_write_domain) {
+	if (eb->batch_obj->base.pending_write_domain) {
 		DRM_ERROR("Attempting to use self-modifying batch buffer\n");
 		ret = -EINVAL;
 		goto err;
 	}
-	batch_obj->base.pending_read_domains |= I915_GEM_DOMAIN_COMMAND;
+	eb->batch_obj->base.pending_read_domains |= I915_GEM_DOMAIN_COMMAND;
 
-	ret = i915_gem_execbuffer_move_to_gpu(ring, &objects);
+	ret = i915_gem_execbuffer_move_to_gpu(ring, &eb->objects);
 	if (ret)
 		goto err;
 
@@ -1177,7 +1187,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 
 	trace_i915_gem_ring_dispatch(ring, seqno);
 
-	exec_start = batch_obj->gtt_offset + args->batch_start_offset;
+	exec_start = eb->batch_obj->gtt_offset + args->batch_start_offset;
 	exec_len = args->batch_len;
 	if (cliprects) {
 		for (i = 0; i < args->num_cliprects; i++) {
@@ -1197,21 +1207,11 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 			goto err;
 	}
 
-	i915_gem_execbuffer_move_to_active(&objects, ring, seqno);
+	i915_gem_execbuffer_move_to_active(&eb->objects, ring, seqno);
 	i915_gem_execbuffer_retire_commands(dev, file, ring);
 
 err:
 	eb_destroy(eb);
-	while (!list_empty(&objects)) {
-		struct drm_i915_gem_object *obj;
-
-		obj = list_first_entry(&objects,
-				       struct drm_i915_gem_object,
-				       exec_list);
-		list_del_init(&obj->exec_list);
-		drm_gem_object_unreference(&obj->base);
-	}
-
 	mutex_unlock(&dev->struct_mutex);
 
 pre_mutex_err:
-- 
1.7.7




More information about the Intel-gfx mailing list