[3/4] drm/i915: Make eb do more

Commit Message

Ben Widawsky March 14, 2013, 12:21 a.m. UTC

I've written a couple of versions of this patch, and it's always in prep
for some scheduler work I am doing. Only difference this time is a new
maintainer to never merge them.

Signed-off-by: Ben Widawsky <[e-mail]>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 38 +++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 14 deletions(-)

Comments

Jesse Barnes March 19, 2013, 7:41 p.m. UTC | #1

On Wed, 13 Mar 2013 17:21:07 -0700
Ben Widawsky <[e-mail]> wrote:

> I've written a couple of versions of this patch, and it's always in prep
> for some scheduler work I am doing. Only difference this time is a new
> maintainer to never merge them.
> 
> Signed-off-by: Ben Widawsky <[e-mail]>
> ---

I think the commit message should reflect what's going on here,
"consolidate execbuf args into a struct" or somesuch.

Ben Widawsky March 19, 2013, 7:49 p.m. UTC | #2

On Tue, Mar 19, 2013 at 12:41:51PM -0700, Jesse Barnes wrote:
> On Wed, 13 Mar 2013 17:21:07 -0700
> Ben Widawsky <[e-mail]> wrote:
> 
> > I've written a couple of versions of this patch, and it's always in prep
> > for some scheduler work I am doing. Only difference this time is a new
> > maintainer to never merge them.
> > 
> > Signed-off-by: Ben Widawsky <[e-mail]>
> > ---
> 
> I think the commit message should reflect what's going on here,
> "consolidate execbuf args into a struct" or somesuch.

It was a pretty useless commit message, and I had already updated it
locally:
http://cgit.freedesktop.org/~bwidawsk/drm-intel/commit/?h=execbuf-clean&id=7d0dbbeed1334c49279910c3b6e7d792d28dce3d

As some backup to my gripe, here is the 17 month old version of the
patch:
http://cgit.freedesktop.org/~bwidawsk/drm-intel/commit/?h=eb_clean&id=04cf2b166f3e31f271c2dedf1791ccd8e0f922a0

> 
> -- 
> Jesse Barnes, Intel Open Source Technology Center

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 354cecf..99ebb36 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -36,10 +36,17 @@ 
 struct eb_objects {
 	struct list_head objects;
 	int and;
+
+	struct drm_file *file;
+	struct intel_ring_buffer *ring;
+	struct drm_i915_gem_execbuffer2 *args;
+	struct drm_i915_gem_exec_object2 *exec;
+
 	union {
 		struct drm_i915_gem_object *lut[0];
 		struct hlist_head buckets[0];
 	};
+	/* NB: don't put anything here */
 };
 
 static struct eb_objects *
@@ -82,11 +89,11 @@  eb_reset(struct eb_objects *eb)
 }
 
 static int
-eb_lookup_objects(struct eb_objects *eb,
-		  struct drm_i915_gem_exec_object2 *exec,
-		  const struct drm_i915_gem_execbuffer2 *args,
-		  struct drm_file *file)
+eb_lookup_objects(struct eb_objects *eb)
 {
+	struct drm_i915_gem_exec_object2 *exec = eb->exec;
+	struct drm_i915_gem_execbuffer2 *args = eb->args;
+	struct drm_file *file = eb->file;
 	int i;
 
 	spin_lock(&file->table_lock);
@@ -572,18 +579,17 @@  err:		/* Decrement pin count for bound objects */
 }
 
 static int
-i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
-				  struct drm_i915_gem_execbuffer2 *args,
-				  struct drm_file *file,
-				  struct intel_ring_buffer *ring,
-				  struct eb_objects *eb,
-				  struct drm_i915_gem_exec_object2 *exec)
+i915_gem_execbuffer_relocate_slow(struct eb_objects *eb)
 {
 	struct drm_i915_gem_relocation_entry *reloc;
 	struct drm_i915_gem_object *obj;
 	bool need_relocs;
 	int *reloc_offset;
 	int i, total, ret;
+	struct drm_device *dev = eb->ring->dev;
+	struct drm_i915_gem_execbuffer2 *args = eb->args;
+	struct intel_ring_buffer *ring = eb->ring;
+	struct drm_i915_gem_exec_object2 *exec = eb->exec;
 	int count = args->buffer_count;
 
 	/* We may process another execbuffer during the unlock... */
@@ -656,7 +662,7 @@  i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
 
 	/* reacquire the objects */
 	eb_reset(eb);
-	ret = eb_lookup_objects(eb, exec, args, file);
+	ret = eb_lookup_objects(eb);
 	if (ret)
 		goto err;
 
@@ -966,8 +972,13 @@  i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		goto pre_mutex_err;
 	}
 
+	eb->args = args;
+	eb->exec = exec;
+	eb->file = file;
+	eb->ring = ring;
+
 	/* Look up object handles */
-	ret = eb_lookup_objects(eb, exec, args, file);
+	ret = eb_lookup_objects(eb);
 	if (ret)
 		goto err;
 
@@ -987,8 +998,7 @@  i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		ret = i915_gem_execbuffer_relocate(eb);
 	if (ret) {
 		if (ret == -EFAULT) {
-			ret = i915_gem_execbuffer_relocate_slow(dev, args, file, ring,
-								eb, exec);
+			ret = i915_gem_execbuffer_relocate_slow(eb);
 			BUG_ON(!mutex_is_locked(&dev->struct_mutex));
 		}
 		if (ret)