Re: [PATCH 05/20] drm/i915/gem: Break apart the early i915_vma_pin from execbuf object lookup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 06/07/2020 07:19, Chris Wilson wrote:
As a prelude to the next step where we want to perform all the object
allocations together under the same lock, we first must delay the
i915_vma_pin() as that implicitly does the allocations for us, one by
one. As it only does the allocations one by one, it is not allowed to
wait/evict, whereas pulling all the allocations together the entire set
can be scheduled as one.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
  .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 70 +++++++++++--------
  1 file changed, 39 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index bf8193d9e279..35a57c1fc9c3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -33,6 +33,8 @@ struct eb_vma {
/** This vma's place in the execbuf reservation list */
  	struct drm_i915_gem_exec_object2 *exec;
+
+	struct list_head bind_link;
  	struct list_head unbound_link;
  	struct list_head reloc_link;
@@ -240,8 +242,8 @@ struct i915_execbuffer {
  	/** actual size of execobj[] as we may extend it for the cmdparser */
  	unsigned int buffer_count;
- /** list of vma not yet bound during reservation phase */
-	struct list_head unbound;
+	/** list of all vma required to bound for this execbuf */
+	struct list_head bind_list;
/** list of vma that have execobj.relocation_count */
  	struct list_head relocs;
@@ -565,6 +567,8 @@ eb_add_vma(struct i915_execbuffer *eb,
  						    eb->lut_size)]);
  	}
+ list_add_tail(&ev->bind_link, &eb->bind_list);
+
  	if (entry->relocation_count)
  		list_add_tail(&ev->reloc_link, &eb->relocs);
@@ -586,16 +590,6 @@ eb_add_vma(struct i915_execbuffer *eb, eb->batch = ev;
  	}
-
-	if (eb_pin_vma(eb, entry, ev)) {
-		if (entry->offset != vma->node.start) {
-			entry->offset = vma->node.start | UPDATE;
-			eb->args->flags |= __EXEC_HAS_RELOC;
-		}
-	} else {
-		eb_unreserve_vma(ev);
-		list_add_tail(&ev->unbound_link, &eb->unbound);
-	}
  }
static int eb_reserve_vma(const struct i915_execbuffer *eb,
@@ -670,13 +664,31 @@ static int wait_for_timeline(struct intel_timeline *tl)
  	} while (1);
  }
-static int eb_reserve(struct i915_execbuffer *eb)
+static int eb_reserve_vm(struct i915_execbuffer *eb)
  {
-	const unsigned int count = eb->buffer_count;
  	unsigned int pin_flags = PIN_USER | PIN_NONBLOCK;
-	struct list_head last;
+	struct list_head last, unbound;
  	struct eb_vma *ev;
-	unsigned int i, pass;
+	unsigned int pass;
+
+	INIT_LIST_HEAD(&unbound);
+	list_for_each_entry(ev, &eb->bind_list, bind_link) {
+		struct drm_i915_gem_exec_object2 *entry = ev->exec;
+		struct i915_vma *vma = ev->vma;
+
+		if (eb_pin_vma(eb, entry, ev)) {
+			if (entry->offset != vma->node.start) {
+				entry->offset = vma->node.start | UPDATE;
+				eb->args->flags |= __EXEC_HAS_RELOC;
+			}
+		} else {
+			eb_unreserve_vma(ev);
+			list_add_tail(&ev->unbound_link, &unbound);
+		}
+	}
+
+	if (list_empty(&unbound))
+		return 0;
/*
  	 * Attempt to pin all of the buffers into the GTT.
@@ -699,7 +711,7 @@ static int eb_reserve(struct i915_execbuffer *eb)
  		if (mutex_lock_interruptible(&eb->i915->drm.struct_mutex))
  			return -EINTR;
- list_for_each_entry(ev, &eb->unbound, unbound_link) {
+		list_for_each_entry(ev, &unbound, unbound_link) {
  			err = eb_reserve_vma(eb, ev, pin_flags);
  			if (err)
  				break;
@@ -710,13 +722,11 @@ static int eb_reserve(struct i915_execbuffer *eb)
  		}
/* Resort *all* the objects into priority order */
-		INIT_LIST_HEAD(&eb->unbound);
+		INIT_LIST_HEAD(&unbound);
  		INIT_LIST_HEAD(&last);
-		for (i = 0; i < count; i++) {
-			unsigned int flags;
+		list_for_each_entry(ev, &eb->bind_list, bind_link) {
+			unsigned int flags = ev->flags;
- ev = &eb->vma[i];
-			flags = ev->flags;
  			if (flags & EXEC_OBJECT_PINNED &&
  			    flags & __EXEC_OBJECT_HAS_PIN)
  				continue;
@@ -725,17 +735,17 @@ static int eb_reserve(struct i915_execbuffer *eb)
if (flags & EXEC_OBJECT_PINNED)
  				/* Pinned must have their slot */
-				list_add(&ev->unbound_link, &eb->unbound);
+				list_add(&ev->unbound_link, &unbound);
  			else if (flags & __EXEC_OBJECT_NEEDS_MAP)
  				/* Map require the lowest 256MiB (aperture) */
-				list_add_tail(&ev->unbound_link, &eb->unbound);
+				list_add_tail(&ev->unbound_link, &unbound);
  			else if (!(flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS))
  				/* Prioritise 4GiB region for restricted bo */
  				list_add(&ev->unbound_link, &last);
  			else
  				list_add_tail(&ev->unbound_link, &last);
  		}
-		list_splice_tail(&last, &eb->unbound);
+		list_splice_tail(&last, &unbound);
  		mutex_unlock(&eb->i915->drm.struct_mutex);
if (err == -EAGAIN) {
@@ -891,8 +901,8 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
  	unsigned int i;
  	int err = 0;
+ INIT_LIST_HEAD(&eb->bind_list);
  	INIT_LIST_HEAD(&eb->relocs);
-	INIT_LIST_HEAD(&eb->unbound);
for (i = 0; i < eb->buffer_count; i++) {
  		struct i915_vma *vma;
@@ -1539,11 +1549,9 @@ static int eb_relocate(struct i915_execbuffer *eb)
  	if (err)
  		return err;
- if (!list_empty(&eb->unbound)) {
-		err = eb_reserve(eb);
-		if (err)
-			return err;
-	}
+	err = eb_reserve_vm(eb);
+	if (err)
+		return err;
/* The objects are in their final locations, apply the relocations. */
  	if (eb->args->flags & __EXEC_HAS_RELOC) {


Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux