All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/i915: Handle i915_active_fence_set() with the same fence
@ 2019-11-06 15:48 ` Chris Wilson
  0 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2019-11-06 15:48 UTC (permalink / raw)
  To: intel-gfx

If the caller wants to overwrite the currently tracked fence, with
itself, as far as the tracking is concerned it is a no-op, so simply
allow it.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_active.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index 207383dda84d..cde984744f20 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -759,7 +759,9 @@ __i915_active_fence_set(struct i915_active_fence *active,
 
 	prev = rcu_dereference_protected(active->fence, active_is_held(active));
 	if (prev) {
-		GEM_BUG_ON(prev == fence);
+		if (unlikely(prev == fence))
+			goto unlock;
+
 		spin_lock_nested(prev->lock, SINGLE_DEPTH_NESTING);
 		__list_del_entry(&active->cb.node);
 		spin_unlock(prev->lock); /* serialise with prev->cb_list */
@@ -781,6 +783,7 @@ __i915_active_fence_set(struct i915_active_fence *active,
 	rcu_assign_pointer(active->fence, fence);
 	list_add_tail(&active->cb.node, &fence->cb_list);
 
+unlock:
 	spin_unlock_irqrestore(fence->lock, flags);
 
 	return prev;
-- 
2.24.0

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [Intel-gfx] [PATCH 1/3] drm/i915: Handle i915_active_fence_set() with the same fence
@ 2019-11-06 15:48 ` Chris Wilson
  0 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2019-11-06 15:48 UTC (permalink / raw)
  To: intel-gfx

If the caller wants to overwrite the currently tracked fence, with
itself, as far as the tracking is concerned it is a no-op, so simply
allow it.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_active.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index 207383dda84d..cde984744f20 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -759,7 +759,9 @@ __i915_active_fence_set(struct i915_active_fence *active,
 
 	prev = rcu_dereference_protected(active->fence, active_is_held(active));
 	if (prev) {
-		GEM_BUG_ON(prev == fence);
+		if (unlikely(prev == fence))
+			goto unlock;
+
 		spin_lock_nested(prev->lock, SINGLE_DEPTH_NESTING);
 		__list_del_entry(&active->cb.node);
 		spin_unlock(prev->lock); /* serialise with prev->cb_list */
@@ -781,6 +783,7 @@ __i915_active_fence_set(struct i915_active_fence *active,
 	rcu_assign_pointer(active->fence, fence);
 	list_add_tail(&active->cb.node, &fence->cb_list);
 
+unlock:
 	spin_unlock_irqrestore(fence->lock, flags);
 
 	return prev;
-- 
2.24.0

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH 2/3] drm/i915: Drop inspection of execbuf flags during evict
@ 2019-11-06 15:48   ` Chris Wilson
  0 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2019-11-06 15:48 UTC (permalink / raw)
  To: intel-gfx

With the goal of removing the serialisation from around execbuf, we will
no longer have the privilege of there being a single execbuf in flight
at any time and so will only be able to inspect the user's flags within
the carefully controlled execbuf context. i915_gem_evict_for_node() is
the only user outside of execbuf that currently peeks at the flag to
convert an overlapping softpinned request from ENOSPC to EINVAL. Retract
this nicety and only report ENOSPC if the location is in current use,
either due to this execbuf or another.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_evict.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index 7e62c310290f..1395018c657a 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -292,7 +292,8 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
 		GEM_BUG_ON(!drm_mm_node_allocated(node));
 		vma = container_of(node, typeof(*vma), node);
 
-		/* If we are using coloring to insert guard pages between
+		/*
+		 * If we are using coloring to insert guard pages between
 		 * different cache domains within the address space, we have
 		 * to check whether the objects on either side of our range
 		 * abutt and conflict. If they are in conflict, then we evict
@@ -309,22 +310,18 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
 			}
 		}
 
-		if (flags & PIN_NONBLOCK &&
-		    (i915_vma_is_pinned(vma) || i915_vma_is_active(vma))) {
+		if (i915_vma_is_pinned(vma)) {
 			ret = -ENOSPC;
 			break;
 		}
 
-		/* Overlap of objects in the same batch? */
-		if (i915_vma_is_pinned(vma)) {
+		if (flags & PIN_NONBLOCK && i915_vma_is_active(vma)) {
 			ret = -ENOSPC;
-			if (vma->exec_flags &&
-			    *vma->exec_flags & EXEC_OBJECT_PINNED)
-				ret = -EINVAL;
 			break;
 		}
 
-		/* Never show fear in the face of dragons!
+		/*
+		 * Never show fear in the face of dragons!
 		 *
 		 * We cannot directly remove this node from within this
 		 * iterator and as with i915_gem_evict_something() we employ
-- 
2.24.0

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [Intel-gfx] [PATCH 2/3] drm/i915: Drop inspection of execbuf flags during evict
@ 2019-11-06 15:48   ` Chris Wilson
  0 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2019-11-06 15:48 UTC (permalink / raw)
  To: intel-gfx

With the goal of removing the serialisation from around execbuf, we will
no longer have the privilege of there being a single execbuf in flight
at any time and so will only be able to inspect the user's flags within
the carefully controlled execbuf context. i915_gem_evict_for_node() is
the only user outside of execbuf that currently peeks at the flag to
convert an overlapping softpinned request from ENOSPC to EINVAL. Retract
this nicety and only report ENOSPC if the location is in current use,
either due to this execbuf or another.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_evict.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index 7e62c310290f..1395018c657a 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -292,7 +292,8 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
 		GEM_BUG_ON(!drm_mm_node_allocated(node));
 		vma = container_of(node, typeof(*vma), node);
 
-		/* If we are using coloring to insert guard pages between
+		/*
+		 * If we are using coloring to insert guard pages between
 		 * different cache domains within the address space, we have
 		 * to check whether the objects on either side of our range
 		 * abutt and conflict. If they are in conflict, then we evict
@@ -309,22 +310,18 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
 			}
 		}
 
-		if (flags & PIN_NONBLOCK &&
-		    (i915_vma_is_pinned(vma) || i915_vma_is_active(vma))) {
+		if (i915_vma_is_pinned(vma)) {
 			ret = -ENOSPC;
 			break;
 		}
 
-		/* Overlap of objects in the same batch? */
-		if (i915_vma_is_pinned(vma)) {
+		if (flags & PIN_NONBLOCK && i915_vma_is_active(vma)) {
 			ret = -ENOSPC;
-			if (vma->exec_flags &&
-			    *vma->exec_flags & EXEC_OBJECT_PINNED)
-				ret = -EINVAL;
 			break;
 		}
 
-		/* Never show fear in the face of dragons!
+		/*
+		 * Never show fear in the face of dragons!
 		 *
 		 * We cannot directly remove this node from within this
 		 * iterator and as with i915_gem_evict_something() we employ
-- 
2.24.0

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH 3/3] drm/i915/gem: Extract transient execbuf flags from i915_vma
@ 2019-11-06 15:48   ` Chris Wilson
  0 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2019-11-06 15:48 UTC (permalink / raw)
  To: intel-gfx

For our convenience, and to avoid frequent allocations, we placed some
lists we use for execbuf inside the common i915_vma struct. As we look
to parallelise execbuf, such fields guarded by the struct_mutex BKL must
be pulled under local control. Instead of using the i915_vma as our
primary means of tracking the user's list of objects and their virtual
mappings, we use a local eb_vma with the same lists as before (just now
local not global).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 271 +++++++++---------
 drivers/gpu/drm/i915/i915_vma.h               |  11 -
 2 files changed, 130 insertions(+), 152 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index e4f5c269150a..8eb9c4e17514 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -27,6 +27,19 @@
 #include "i915_gem_ioctls.h"
 #include "i915_trace.h"
 
+struct eb_vma {
+	struct i915_vma *vma;
+	unsigned int flags;
+
+	/** This vma's place in the execbuf reservation list */
+	struct drm_i915_gem_exec_object2 *exec;
+	struct list_head bind_link;
+	struct list_head reloc_link;
+
+	struct hlist_node node;
+	u32 handle;
+};
+
 enum {
 	FORCE_CPU_RELOC = 1,
 	FORCE_GTT_RELOC,
@@ -219,15 +232,14 @@ struct i915_execbuffer {
 	struct drm_file *file; /** per-file lookup tables and limits */
 	struct drm_i915_gem_execbuffer2 *args; /** ioctl parameters */
 	struct drm_i915_gem_exec_object2 *exec; /** ioctl execobj[] */
-	struct i915_vma **vma;
-	unsigned int *flags;
+	struct eb_vma *vma;
 
 	struct intel_engine_cs *engine; /** engine to queue the request to */
 	struct intel_context *context; /* logical state for the request */
 	struct i915_gem_context *gem_context; /** caller's context */
 
 	struct i915_request *request; /** our request to build */
-	struct i915_vma *batch; /** identity of the batch obj/vma */
+	struct eb_vma *batch; /** identity of the batch obj/vma */
 
 	/** actual size of execobj[] as we may extend it for the cmdparser */
 	unsigned int buffer_count;
@@ -275,8 +287,6 @@ struct i915_execbuffer {
 	struct hlist_head *buckets; /** ht for relocation handles */
 };
 
-#define exec_entry(EB, VMA) (&(EB)->exec[(VMA)->exec_flags - (EB)->flags])
-
 /*
  * Used to convert any address to canonical form.
  * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
@@ -380,9 +390,9 @@ eb_vma_misplaced(const struct drm_i915_gem_exec_object2 *entry,
 static inline bool
 eb_pin_vma(struct i915_execbuffer *eb,
 	   const struct drm_i915_gem_exec_object2 *entry,
-	   struct i915_vma *vma)
+	   struct eb_vma *ev)
 {
-	unsigned int exec_flags = *vma->exec_flags;
+	struct i915_vma *vma = ev->vma;
 	u64 pin_flags;
 
 	if (vma->node.size)
@@ -391,24 +401,24 @@ eb_pin_vma(struct i915_execbuffer *eb,
 		pin_flags = entry->offset & PIN_OFFSET_MASK;
 
 	pin_flags |= PIN_USER | PIN_NOEVICT | PIN_OFFSET_FIXED;
-	if (unlikely(exec_flags & EXEC_OBJECT_NEEDS_GTT))
+	if (unlikely(ev->flags & EXEC_OBJECT_NEEDS_GTT))
 		pin_flags |= PIN_GLOBAL;
 
 	if (unlikely(i915_vma_pin(vma, 0, 0, pin_flags)))
 		return false;
 
-	if (unlikely(exec_flags & EXEC_OBJECT_NEEDS_FENCE)) {
+	if (unlikely(ev->flags & EXEC_OBJECT_NEEDS_FENCE)) {
 		if (unlikely(i915_vma_pin_fence(vma))) {
 			i915_vma_unpin(vma);
 			return false;
 		}
 
 		if (vma->fence)
-			exec_flags |= __EXEC_OBJECT_HAS_FENCE;
+			ev->flags |= __EXEC_OBJECT_HAS_FENCE;
 	}
 
-	*vma->exec_flags = exec_flags | __EXEC_OBJECT_HAS_PIN;
-	return !eb_vma_misplaced(entry, vma, exec_flags);
+	ev->flags |= __EXEC_OBJECT_HAS_PIN;
+	return !eb_vma_misplaced(entry, vma, ev->flags);
 }
 
 static inline void __eb_unreserve_vma(struct i915_vma *vma, unsigned int flags)
@@ -422,13 +432,13 @@ static inline void __eb_unreserve_vma(struct i915_vma *vma, unsigned int flags)
 }
 
 static inline void
-eb_unreserve_vma(struct i915_vma *vma, unsigned int *flags)
+eb_unreserve_vma(struct eb_vma *ev)
 {
-	if (!(*flags & __EXEC_OBJECT_HAS_PIN))
+	if (!(ev->flags & __EXEC_OBJECT_HAS_PIN))
 		return;
 
-	__eb_unreserve_vma(vma, *flags);
-	*flags &= ~__EXEC_OBJECT_RESERVED;
+	__eb_unreserve_vma(ev->vma, ev->flags);
+	ev->flags &= ~__EXEC_OBJECT_RESERVED;
 }
 
 static int
@@ -458,12 +468,6 @@ eb_validate_vma(struct i915_execbuffer *eb,
 		entry->pad_to_size = 0;
 	}
 
-	if (unlikely(vma->exec_flags)) {
-		DRM_DEBUG("Object [handle %d, index %d] appears more than once in object list\n",
-			  entry->handle, (int)(entry - eb->exec));
-		return -EINVAL;
-	}
-
 	/*
 	 * From drm_mm perspective address space is continuous,
 	 * so from this point we're always using non-canonical
@@ -492,6 +496,7 @@ eb_add_vma(struct i915_execbuffer *eb,
 	   struct i915_vma *vma)
 {
 	struct drm_i915_gem_exec_object2 *entry = &eb->exec[i];
+	struct eb_vma *ev = &eb->vma[i];
 	int err;
 
 	GEM_BUG_ON(i915_vma_is_closed(vma));
@@ -502,25 +507,19 @@ eb_add_vma(struct i915_execbuffer *eb,
 			return err;
 	}
 
+	ev->vma = vma;
+	ev->exec = entry;
+	ev->flags = entry->flags;
+
 	if (eb->lut_size > 0) {
-		vma->exec_handle = entry->handle;
-		hlist_add_head(&vma->exec_node,
+		ev->handle = entry->handle;
+		hlist_add_head(&ev->node,
 			       &eb->buckets[hash_32(entry->handle,
 						    eb->lut_size)]);
 	}
 
 	if (entry->relocation_count)
-		list_add_tail(&vma->reloc_link, &eb->relocs);
-
-	/*
-	 * Stash a pointer from the vma to execobj, so we can query its flags,
-	 * size, alignment etc as provided by the user. Also we stash a pointer
-	 * to the vma inside the execobj so that we can use a direct lookup
-	 * to find the right target VMA when doing relocations.
-	 */
-	eb->vma[i] = vma;
-	eb->flags[i] = entry->flags;
-	vma->exec_flags = &eb->flags[i];
+		list_add_tail(&ev->reloc_link, &eb->relocs);
 
 	/*
 	 * SNA is doing fancy tricks with compressing batch buffers, which leads
@@ -533,28 +532,26 @@ eb_add_vma(struct i915_execbuffer *eb,
 	 */
 	if (i == batch_idx) {
 		if (entry->relocation_count &&
-		    !(eb->flags[i] & EXEC_OBJECT_PINNED))
-			eb->flags[i] |= __EXEC_OBJECT_NEEDS_BIAS;
+		    !(ev->flags & EXEC_OBJECT_PINNED))
+			ev->flags |= __EXEC_OBJECT_NEEDS_BIAS;
 		if (eb->reloc_cache.has_fence)
-			eb->flags[i] |= EXEC_OBJECT_NEEDS_FENCE;
+			ev->flags |= EXEC_OBJECT_NEEDS_FENCE;
 
-		eb->batch = vma;
+		eb->batch = ev;
 	}
 
 	err = 0;
-	if (eb_pin_vma(eb, entry, 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(vma, vma->exec_flags);
+		eb_unreserve_vma(ev);
 
-		list_add_tail(&vma->exec_link, &eb->unbound);
+		list_add_tail(&ev->bind_link, &eb->unbound);
 		if (drm_mm_node_allocated(&vma->node))
 			err = i915_vma_unbind(vma);
-		if (unlikely(err))
-			vma->exec_flags = NULL;
 	}
 	return err;
 }
@@ -576,11 +573,11 @@ static inline int use_cpu_reloc(const struct reloc_cache *cache,
 		obj->cache_level != I915_CACHE_NONE);
 }
 
-static int eb_reserve_vma(const struct i915_execbuffer *eb,
-			  struct i915_vma *vma)
+static int eb_reserve_vma(const struct i915_execbuffer *eb, struct eb_vma *ev)
 {
-	struct drm_i915_gem_exec_object2 *entry = exec_entry(eb, vma);
-	unsigned int exec_flags = *vma->exec_flags;
+	struct drm_i915_gem_exec_object2 *entry = ev->exec;
+	unsigned int exec_flags = ev->flags;
+	struct i915_vma *vma = ev->vma;
 	u64 pin_flags;
 	int err;
 
@@ -627,8 +624,8 @@ static int eb_reserve_vma(const struct i915_execbuffer *eb,
 			exec_flags |= __EXEC_OBJECT_HAS_FENCE;
 	}
 
-	*vma->exec_flags = exec_flags | __EXEC_OBJECT_HAS_PIN;
-	GEM_BUG_ON(eb_vma_misplaced(entry, vma, exec_flags));
+	ev->flags = exec_flags | __EXEC_OBJECT_HAS_PIN;
+	GEM_BUG_ON(eb_vma_misplaced(entry, vma, ev->flags));
 
 	return 0;
 }
@@ -637,7 +634,7 @@ static int eb_reserve(struct i915_execbuffer *eb)
 {
 	const unsigned int count = eb->buffer_count;
 	struct list_head last;
-	struct i915_vma *vma;
+	struct eb_vma *ev;
 	unsigned int i, pass;
 	int err;
 
@@ -658,8 +655,8 @@ static int eb_reserve(struct i915_execbuffer *eb)
 	pass = 0;
 	err = 0;
 	do {
-		list_for_each_entry(vma, &eb->unbound, exec_link) {
-			err = eb_reserve_vma(eb, vma);
+		list_for_each_entry(ev, &eb->unbound, bind_link) {
+			err = eb_reserve_vma(eb, ev);
 			if (err)
 				break;
 		}
@@ -670,26 +667,26 @@ static int eb_reserve(struct i915_execbuffer *eb)
 		INIT_LIST_HEAD(&eb->unbound);
 		INIT_LIST_HEAD(&last);
 		for (i = 0; i < count; i++) {
-			unsigned int flags = eb->flags[i];
-			struct i915_vma *vma = eb->vma[i];
+			struct eb_vma *ev = &eb->vma[i];
+			unsigned int flags = ev->flags;
 
 			if (flags & EXEC_OBJECT_PINNED &&
 			    flags & __EXEC_OBJECT_HAS_PIN)
 				continue;
 
-			eb_unreserve_vma(vma, &eb->flags[i]);
+			eb_unreserve_vma(ev);
 
 			if (flags & EXEC_OBJECT_PINNED)
 				/* Pinned must have their slot */
-				list_add(&vma->exec_link, &eb->unbound);
+				list_add(&ev->bind_link, &eb->unbound);
 			else if (flags & __EXEC_OBJECT_NEEDS_MAP)
 				/* Map require the lowest 256MiB (aperture) */
-				list_add_tail(&vma->exec_link, &eb->unbound);
+				list_add_tail(&ev->bind_link, &eb->unbound);
 			else if (!(flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS))
 				/* Prioritise 4GiB region for restricted bo */
-				list_add(&vma->exec_link, &last);
+				list_add(&ev->bind_link, &last);
 			else
-				list_add_tail(&vma->exec_link, &last);
+				list_add_tail(&ev->bind_link, &last);
 		}
 		list_splice_tail(&last, &eb->unbound);
 
@@ -808,10 +805,8 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
 		if (unlikely(err))
 			goto err_vma;
 
-		GEM_BUG_ON(vma != eb->vma[i]);
-		GEM_BUG_ON(vma->exec_flags != &eb->flags[i]);
 		GEM_BUG_ON(drm_mm_node_allocated(&vma->node) &&
-			   eb_vma_misplaced(&eb->exec[i], vma, eb->flags[i]));
+			   eb_vma_misplaced(&eb->exec[i], vma, eb->vma[i].flags));
 	}
 
 	mutex_unlock(&eb->gem_context->mutex);
@@ -822,27 +817,27 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
 err_obj:
 	i915_gem_object_put(obj);
 err_vma:
-	eb->vma[i] = NULL;
+	eb->vma[i].vma = NULL;
 err_ctx:
 	mutex_unlock(&eb->gem_context->mutex);
 	return err;
 }
 
-static struct i915_vma *
+static struct eb_vma *
 eb_get_vma(const struct i915_execbuffer *eb, unsigned long handle)
 {
 	if (eb->lut_size < 0) {
 		if (handle >= -eb->lut_size)
 			return NULL;
-		return eb->vma[handle];
+		return &eb->vma[handle];
 	} else {
 		struct hlist_head *head;
-		struct i915_vma *vma;
+		struct eb_vma *ev;
 
 		head = &eb->buckets[hash_32(handle, eb->lut_size)];
-		hlist_for_each_entry(vma, head, exec_node) {
-			if (vma->exec_handle == handle)
-				return vma;
+		hlist_for_each_entry(ev, head, node) {
+			if (ev->handle == handle)
+				return ev;
 		}
 		return NULL;
 	}
@@ -854,20 +849,18 @@ static void eb_release_vmas(const struct i915_execbuffer *eb)
 	unsigned int i;
 
 	for (i = 0; i < count; i++) {
-		struct i915_vma *vma = eb->vma[i];
-		unsigned int flags = eb->flags[i];
+		struct eb_vma *ev = &eb->vma[i];
+		struct i915_vma *vma = ev->vma;
 
 		if (!vma)
 			break;
 
-		GEM_BUG_ON(vma->exec_flags != &eb->flags[i]);
-		vma->exec_flags = NULL;
-		eb->vma[i] = NULL;
+		eb->vma[i].vma = NULL;
 
-		if (flags & __EXEC_OBJECT_HAS_PIN)
-			__eb_unreserve_vma(vma, flags);
+		if (ev->flags & __EXEC_OBJECT_HAS_PIN)
+			__eb_unreserve_vma(vma, ev->flags);
 
-		if (flags & __EXEC_OBJECT_HAS_REF)
+		if (ev->flags & __EXEC_OBJECT_HAS_REF)
 			i915_vma_put(vma);
 	}
 }
@@ -1377,10 +1370,10 @@ relocate_entry(struct i915_vma *vma,
 
 static u64
 eb_relocate_entry(struct i915_execbuffer *eb,
-		  struct i915_vma *vma,
+		  struct eb_vma *ev,
 		  const struct drm_i915_gem_relocation_entry *reloc)
 {
-	struct i915_vma *target;
+	struct eb_vma *target;
 	int err;
 
 	/* we've already hold a reference to all valid objects */
@@ -1412,7 +1405,7 @@ eb_relocate_entry(struct i915_execbuffer *eb,
 	}
 
 	if (reloc->write_domain) {
-		*target->exec_flags |= EXEC_OBJECT_WRITE;
+		target->flags |= EXEC_OBJECT_WRITE;
 
 		/*
 		 * Sandybridge PPGTT errata: We need a global gtt mapping
@@ -1422,7 +1415,8 @@ eb_relocate_entry(struct i915_execbuffer *eb,
 		 */
 		if (reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION &&
 		    IS_GEN(eb->i915, 6)) {
-			err = i915_vma_bind(target, target->obj->cache_level,
+			err = i915_vma_bind(target->vma,
+					    target->vma->obj->cache_level,
 					    PIN_GLOBAL, NULL);
 			if (WARN_ONCE(err,
 				      "Unexpected failure to bind target VMA!"))
@@ -1435,17 +1429,17 @@ eb_relocate_entry(struct i915_execbuffer *eb,
 	 * more work needs to be done.
 	 */
 	if (!DBG_FORCE_RELOC &&
-	    gen8_canonical_addr(target->node.start) == reloc->presumed_offset)
+	    gen8_canonical_addr(target->vma->node.start) == reloc->presumed_offset)
 		return 0;
 
 	/* Check that the relocation address is valid... */
 	if (unlikely(reloc->offset >
-		     vma->size - (eb->reloc_cache.use_64bit_reloc ? 8 : 4))) {
+		     ev->vma->size - (eb->reloc_cache.use_64bit_reloc ? 8 : 4))) {
 		DRM_DEBUG("Relocation beyond object bounds: "
 			  "target %d offset %d size %d.\n",
 			  reloc->target_handle,
 			  (int)reloc->offset,
-			  (int)vma->size);
+			  (int)ev->vma->size);
 		return -EINVAL;
 	}
 	if (unlikely(reloc->offset & 3)) {
@@ -1464,18 +1458,18 @@ eb_relocate_entry(struct i915_execbuffer *eb,
 	 * do relocations we are already stalling, disable the user's opt
 	 * out of our synchronisation.
 	 */
-	*vma->exec_flags &= ~EXEC_OBJECT_ASYNC;
+	ev->flags &= ~EXEC_OBJECT_ASYNC;
 
 	/* and update the user's relocation entry */
-	return relocate_entry(vma, reloc, eb, target);
+	return relocate_entry(ev->vma, reloc, eb, target->vma);
 }
 
-static int eb_relocate_vma(struct i915_execbuffer *eb, struct i915_vma *vma)
+static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev)
 {
 #define N_RELOC(x) ((x) / sizeof(struct drm_i915_gem_relocation_entry))
 	struct drm_i915_gem_relocation_entry stack[N_RELOC(512)];
 	struct drm_i915_gem_relocation_entry __user *urelocs;
-	const struct drm_i915_gem_exec_object2 *entry = exec_entry(eb, vma);
+	const struct drm_i915_gem_exec_object2 *entry = ev->exec;
 	unsigned int remain;
 
 	urelocs = u64_to_user_ptr(entry->relocs_ptr);
@@ -1515,7 +1509,7 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct i915_vma *vma)
 
 		remain -= count;
 		do {
-			u64 offset = eb_relocate_entry(eb, vma, r);
+			u64 offset = eb_relocate_entry(eb, ev, r);
 
 			if (likely(offset == 0)) {
 			} else if ((s64)offset < 0) {
@@ -1558,16 +1552,16 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct i915_vma *vma)
 }
 
 static int
-eb_relocate_vma_slow(struct i915_execbuffer *eb, struct i915_vma *vma)
+eb_relocate_vma_slow(struct i915_execbuffer *eb, struct eb_vma *ev)
 {
-	const struct drm_i915_gem_exec_object2 *entry = exec_entry(eb, vma);
+	const struct drm_i915_gem_exec_object2 *entry = ev->exec;
 	struct drm_i915_gem_relocation_entry *relocs =
 		u64_to_ptr(typeof(*relocs), entry->relocs_ptr);
 	unsigned int i;
 	int err;
 
 	for (i = 0; i < entry->relocation_count; i++) {
-		u64 offset = eb_relocate_entry(eb, vma, &relocs[i]);
+		u64 offset = eb_relocate_entry(eb, ev, &relocs[i]);
 
 		if ((s64)offset < 0) {
 			err = (int)offset;
@@ -1711,7 +1705,7 @@ static noinline int eb_relocate_slow(struct i915_execbuffer *eb)
 {
 	struct drm_device *dev = &eb->i915->drm;
 	bool have_copy = false;
-	struct i915_vma *vma;
+	struct eb_vma *ev;
 	int err = 0;
 
 repeat:
@@ -1767,15 +1761,15 @@ static noinline int eb_relocate_slow(struct i915_execbuffer *eb)
 
 	GEM_BUG_ON(!eb->batch);
 
-	list_for_each_entry(vma, &eb->relocs, reloc_link) {
+	list_for_each_entry(ev, &eb->relocs, reloc_link) {
 		if (!have_copy) {
 			pagefault_disable();
-			err = eb_relocate_vma(eb, vma);
+			err = eb_relocate_vma(eb, ev);
 			pagefault_enable();
 			if (err)
 				goto repeat;
 		} else {
-			err = eb_relocate_vma_slow(eb, vma);
+			err = eb_relocate_vma_slow(eb, ev);
 			if (err)
 				goto err;
 		}
@@ -1820,10 +1814,10 @@ static int eb_relocate(struct i915_execbuffer *eb)
 
 	/* The objects are in their final locations, apply the relocations. */
 	if (eb->args->flags & __EXEC_HAS_RELOC) {
-		struct i915_vma *vma;
+		struct eb_vma *ev;
 
-		list_for_each_entry(vma, &eb->relocs, reloc_link) {
-			if (eb_relocate_vma(eb, vma))
+		list_for_each_entry(ev, &eb->relocs, reloc_link) {
+			if (eb_relocate_vma(eb, ev))
 				goto slow;
 		}
 	}
@@ -1844,39 +1838,34 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
 	ww_acquire_init(&acquire, &reservation_ww_class);
 
 	for (i = 0; i < count; i++) {
-		struct i915_vma *vma = eb->vma[i];
+		struct eb_vma *ev = &eb->vma[i];
+		struct i915_vma *vma = ev->vma;
 
 		err = ww_mutex_lock_interruptible(&vma->resv->lock, &acquire);
-		if (!err)
-			continue;
-
-		GEM_BUG_ON(err == -EALREADY); /* No duplicate vma */
-
 		if (err == -EDEADLK) {
 			GEM_BUG_ON(i == 0);
 			do {
 				int j = i - 1;
 
-				ww_mutex_unlock(&eb->vma[j]->resv->lock);
+				ww_mutex_unlock(&eb->vma[j].vma->resv->lock);
 
-				swap(eb->flags[i], eb->flags[j]);
 				swap(eb->vma[i],  eb->vma[j]);
-				eb->vma[i]->exec_flags = &eb->flags[i];
 			} while (--i);
-			GEM_BUG_ON(vma != eb->vma[0]);
-			vma->exec_flags = &eb->flags[0];
 
 			err = ww_mutex_lock_slow_interruptible(&vma->resv->lock,
 							       &acquire);
 		}
+		if (err == -EALREADY)
+			err = 0;
 		if (err)
 			break;
 	}
 	ww_acquire_done(&acquire);
 
 	while (i--) {
-		unsigned int flags = eb->flags[i];
-		struct i915_vma *vma = eb->vma[i];
+		struct eb_vma *ev = &eb->vma[i];
+		struct i915_vma *vma = ev->vma;
+		unsigned int flags = ev->flags;
 		struct drm_i915_gem_object *obj = vma->obj;
 
 		assert_vma_held(vma);
@@ -1920,10 +1909,10 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
 		i915_vma_unlock(vma);
 
 		__eb_unreserve_vma(vma, flags);
-		vma->exec_flags = NULL;
-
 		if (unlikely(flags & __EXEC_OBJECT_HAS_REF))
 			i915_vma_put(vma);
+
+		ev->vma = NULL;
 	}
 	ww_acquire_fini(&acquire);
 
@@ -2001,7 +1990,7 @@ static struct i915_vma *eb_parse(struct i915_execbuffer *eb, bool is_master)
 		return ERR_CAST(pool);
 
 	err = intel_engine_cmd_parser(eb->engine,
-				      eb->batch->obj,
+				      eb->batch->vma->obj,
 				      pool->obj,
 				      eb->batch_start_offset,
 				      eb->batch_len,
@@ -2018,10 +2007,9 @@ static struct i915_vma *eb_parse(struct i915_execbuffer *eb, bool is_master)
 	if (IS_ERR(vma))
 		goto err;
 
-	eb->vma[eb->buffer_count] = i915_vma_get(vma);
-	eb->flags[eb->buffer_count] =
+	eb->vma[eb->buffer_count].vma = i915_vma_get(vma);
+	eb->vma[eb->buffer_count].flags =
 		__EXEC_OBJECT_HAS_PIN | __EXEC_OBJECT_HAS_REF;
-	vma->exec_flags = &eb->flags[eb->buffer_count];
 	eb->buffer_count++;
 
 	vma->private = pool;
@@ -2044,7 +2032,7 @@ add_to_client(struct i915_request *rq, struct drm_file *file)
 	spin_unlock(&file_priv->mm.lock);
 }
 
-static int eb_submit(struct i915_execbuffer *eb)
+static int eb_submit(struct i915_execbuffer *eb, struct i915_vma *batch)
 {
 	int err;
 
@@ -2071,7 +2059,7 @@ static int eb_submit(struct i915_execbuffer *eb)
 	}
 
 	err = eb->engine->emit_bb_start(eb->request,
-					eb->batch->node.start +
+					batch->node.start +
 					eb->batch_start_offset,
 					eb->batch_len,
 					eb->batch_flags);
@@ -2434,6 +2422,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 	struct dma_fence *in_fence = NULL;
 	struct dma_fence *exec_fence = NULL;
 	struct sync_file *out_fence = NULL;
+	struct i915_vma *batch;
 	int out_fence_fd = -1;
 	int err;
 
@@ -2448,9 +2437,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 		args->flags |= __EXEC_HAS_RELOC;
 
 	eb.exec = exec;
-	eb.vma = (struct i915_vma **)(exec + args->buffer_count + 1);
-	eb.vma[0] = NULL;
-	eb.flags = (unsigned int *)(eb.vma + args->buffer_count + 1);
+	eb.vma = (struct eb_vma *)(exec + args->buffer_count + 1);
+	eb.vma[0].vma = NULL;
 
 	eb.invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS;
 	reloc_cache_init(&eb.reloc_cache, eb.i915);
@@ -2527,13 +2515,16 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 		goto err_vma;
 	}
 
-	if (unlikely(*eb.batch->exec_flags & EXEC_OBJECT_WRITE)) {
+	if (unlikely(eb.batch->flags & EXEC_OBJECT_WRITE)) {
 		DRM_DEBUG("Attempting to use self-modifying batch buffer\n");
 		err = -EINVAL;
 		goto err_vma;
 	}
-	if (eb.batch_start_offset > eb.batch->size ||
-	    eb.batch_len > eb.batch->size - eb.batch_start_offset) {
+
+	batch = eb.batch->vma;
+	if (range_overflows_t(u64,
+			      eb.batch_start_offset, eb.batch_len,
+			      batch->size)) {
 		DRM_DEBUG("Attempting to use out-of-bounds batch\n");
 		err = -EINVAL;
 		goto err_vma;
@@ -2560,12 +2551,12 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 			 */
 			eb.batch_flags |= I915_DISPATCH_SECURE;
 			eb.batch_start_offset = 0;
-			eb.batch = vma;
+			batch = vma;
 		}
 	}
 
 	if (eb.batch_len == 0)
-		eb.batch_len = eb.batch->size - eb.batch_start_offset;
+		eb.batch_len = batch->size - eb.batch_start_offset;
 
 	/*
 	 * snb/ivb/vlv conflate the "batch in ppgtt" bit with the "non-secure
@@ -2584,13 +2575,13 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 		 *   fitting due to fragmentation.
 		 * So this is actually safe.
 		 */
-		vma = i915_gem_object_ggtt_pin(eb.batch->obj, NULL, 0, 0, 0);
+		vma = i915_gem_object_ggtt_pin(batch->obj, NULL, 0, 0, 0);
 		if (IS_ERR(vma)) {
 			err = PTR_ERR(vma);
 			goto err_vma;
 		}
 
-		eb.batch = vma;
+		batch = vma;
 	}
 
 	/* All GPU relocation batches must be submitted prior to the user rq */
@@ -2637,12 +2628,12 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 	 * inactive_list and lose its active reference. Hence we do not need
 	 * to explicitly hold another reference here.
 	 */
-	eb.request->batch = eb.batch;
-	if (eb.batch->private)
-		intel_engine_pool_mark_active(eb.batch->private, eb.request);
+	eb.request->batch = batch;
+	if (batch->private)
+		intel_engine_pool_mark_active(batch->private, eb.request);
 
 	trace_i915_request_queue(eb.request, eb.batch_flags);
-	err = eb_submit(&eb);
+	err = eb_submit(&eb, batch);
 err_request:
 	add_to_client(eb.request, file);
 	i915_request_add(eb.request);
@@ -2663,9 +2654,9 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 
 err_batch_unpin:
 	if (eb.batch_flags & I915_DISPATCH_SECURE)
-		i915_vma_unpin(eb.batch);
-	if (eb.batch->private)
-		intel_engine_pool_put(eb.batch->private);
+		i915_vma_unpin(batch);
+	if (batch->private)
+		intel_engine_pool_put(batch->private);
 err_vma:
 	if (eb.exec)
 		eb_release_vmas(&eb);
@@ -2688,9 +2679,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 
 static size_t eb_element_size(void)
 {
-	return (sizeof(struct drm_i915_gem_exec_object2) +
-		sizeof(struct i915_vma *) +
-		sizeof(unsigned int));
+	return sizeof(struct drm_i915_gem_exec_object2) + sizeof(struct eb_vma);
 }
 
 static bool check_buffer_count(size_t count)
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 465932813bc5..71402056d846 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -147,21 +147,10 @@ struct i915_vma {
 	struct rb_node obj_node;
 	struct hlist_node obj_hash;
 
-	/** This vma's place in the execbuf reservation list */
-	struct list_head exec_link;
-	struct list_head reloc_link;
-
 	/** This vma's place in the eviction list */
 	struct list_head evict_link;
 
 	struct list_head closed_link;
-
-	/**
-	 * Used for performing relocations during execbuffer insertion.
-	 */
-	unsigned int *exec_flags;
-	struct hlist_node exec_node;
-	u32 exec_handle;
 };
 
 struct i915_vma *
-- 
2.24.0

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [Intel-gfx] [PATCH 3/3] drm/i915/gem: Extract transient execbuf flags from i915_vma
@ 2019-11-06 15:48   ` Chris Wilson
  0 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2019-11-06 15:48 UTC (permalink / raw)
  To: intel-gfx

For our convenience, and to avoid frequent allocations, we placed some
lists we use for execbuf inside the common i915_vma struct. As we look
to parallelise execbuf, such fields guarded by the struct_mutex BKL must
be pulled under local control. Instead of using the i915_vma as our
primary means of tracking the user's list of objects and their virtual
mappings, we use a local eb_vma with the same lists as before (just now
local not global).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 271 +++++++++---------
 drivers/gpu/drm/i915/i915_vma.h               |  11 -
 2 files changed, 130 insertions(+), 152 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index e4f5c269150a..8eb9c4e17514 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -27,6 +27,19 @@
 #include "i915_gem_ioctls.h"
 #include "i915_trace.h"
 
+struct eb_vma {
+	struct i915_vma *vma;
+	unsigned int flags;
+
+	/** This vma's place in the execbuf reservation list */
+	struct drm_i915_gem_exec_object2 *exec;
+	struct list_head bind_link;
+	struct list_head reloc_link;
+
+	struct hlist_node node;
+	u32 handle;
+};
+
 enum {
 	FORCE_CPU_RELOC = 1,
 	FORCE_GTT_RELOC,
@@ -219,15 +232,14 @@ struct i915_execbuffer {
 	struct drm_file *file; /** per-file lookup tables and limits */
 	struct drm_i915_gem_execbuffer2 *args; /** ioctl parameters */
 	struct drm_i915_gem_exec_object2 *exec; /** ioctl execobj[] */
-	struct i915_vma **vma;
-	unsigned int *flags;
+	struct eb_vma *vma;
 
 	struct intel_engine_cs *engine; /** engine to queue the request to */
 	struct intel_context *context; /* logical state for the request */
 	struct i915_gem_context *gem_context; /** caller's context */
 
 	struct i915_request *request; /** our request to build */
-	struct i915_vma *batch; /** identity of the batch obj/vma */
+	struct eb_vma *batch; /** identity of the batch obj/vma */
 
 	/** actual size of execobj[] as we may extend it for the cmdparser */
 	unsigned int buffer_count;
@@ -275,8 +287,6 @@ struct i915_execbuffer {
 	struct hlist_head *buckets; /** ht for relocation handles */
 };
 
-#define exec_entry(EB, VMA) (&(EB)->exec[(VMA)->exec_flags - (EB)->flags])
-
 /*
  * Used to convert any address to canonical form.
  * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
@@ -380,9 +390,9 @@ eb_vma_misplaced(const struct drm_i915_gem_exec_object2 *entry,
 static inline bool
 eb_pin_vma(struct i915_execbuffer *eb,
 	   const struct drm_i915_gem_exec_object2 *entry,
-	   struct i915_vma *vma)
+	   struct eb_vma *ev)
 {
-	unsigned int exec_flags = *vma->exec_flags;
+	struct i915_vma *vma = ev->vma;
 	u64 pin_flags;
 
 	if (vma->node.size)
@@ -391,24 +401,24 @@ eb_pin_vma(struct i915_execbuffer *eb,
 		pin_flags = entry->offset & PIN_OFFSET_MASK;
 
 	pin_flags |= PIN_USER | PIN_NOEVICT | PIN_OFFSET_FIXED;
-	if (unlikely(exec_flags & EXEC_OBJECT_NEEDS_GTT))
+	if (unlikely(ev->flags & EXEC_OBJECT_NEEDS_GTT))
 		pin_flags |= PIN_GLOBAL;
 
 	if (unlikely(i915_vma_pin(vma, 0, 0, pin_flags)))
 		return false;
 
-	if (unlikely(exec_flags & EXEC_OBJECT_NEEDS_FENCE)) {
+	if (unlikely(ev->flags & EXEC_OBJECT_NEEDS_FENCE)) {
 		if (unlikely(i915_vma_pin_fence(vma))) {
 			i915_vma_unpin(vma);
 			return false;
 		}
 
 		if (vma->fence)
-			exec_flags |= __EXEC_OBJECT_HAS_FENCE;
+			ev->flags |= __EXEC_OBJECT_HAS_FENCE;
 	}
 
-	*vma->exec_flags = exec_flags | __EXEC_OBJECT_HAS_PIN;
-	return !eb_vma_misplaced(entry, vma, exec_flags);
+	ev->flags |= __EXEC_OBJECT_HAS_PIN;
+	return !eb_vma_misplaced(entry, vma, ev->flags);
 }
 
 static inline void __eb_unreserve_vma(struct i915_vma *vma, unsigned int flags)
@@ -422,13 +432,13 @@ static inline void __eb_unreserve_vma(struct i915_vma *vma, unsigned int flags)
 }
 
 static inline void
-eb_unreserve_vma(struct i915_vma *vma, unsigned int *flags)
+eb_unreserve_vma(struct eb_vma *ev)
 {
-	if (!(*flags & __EXEC_OBJECT_HAS_PIN))
+	if (!(ev->flags & __EXEC_OBJECT_HAS_PIN))
 		return;
 
-	__eb_unreserve_vma(vma, *flags);
-	*flags &= ~__EXEC_OBJECT_RESERVED;
+	__eb_unreserve_vma(ev->vma, ev->flags);
+	ev->flags &= ~__EXEC_OBJECT_RESERVED;
 }
 
 static int
@@ -458,12 +468,6 @@ eb_validate_vma(struct i915_execbuffer *eb,
 		entry->pad_to_size = 0;
 	}
 
-	if (unlikely(vma->exec_flags)) {
-		DRM_DEBUG("Object [handle %d, index %d] appears more than once in object list\n",
-			  entry->handle, (int)(entry - eb->exec));
-		return -EINVAL;
-	}
-
 	/*
 	 * From drm_mm perspective address space is continuous,
 	 * so from this point we're always using non-canonical
@@ -492,6 +496,7 @@ eb_add_vma(struct i915_execbuffer *eb,
 	   struct i915_vma *vma)
 {
 	struct drm_i915_gem_exec_object2 *entry = &eb->exec[i];
+	struct eb_vma *ev = &eb->vma[i];
 	int err;
 
 	GEM_BUG_ON(i915_vma_is_closed(vma));
@@ -502,25 +507,19 @@ eb_add_vma(struct i915_execbuffer *eb,
 			return err;
 	}
 
+	ev->vma = vma;
+	ev->exec = entry;
+	ev->flags = entry->flags;
+
 	if (eb->lut_size > 0) {
-		vma->exec_handle = entry->handle;
-		hlist_add_head(&vma->exec_node,
+		ev->handle = entry->handle;
+		hlist_add_head(&ev->node,
 			       &eb->buckets[hash_32(entry->handle,
 						    eb->lut_size)]);
 	}
 
 	if (entry->relocation_count)
-		list_add_tail(&vma->reloc_link, &eb->relocs);
-
-	/*
-	 * Stash a pointer from the vma to execobj, so we can query its flags,
-	 * size, alignment etc as provided by the user. Also we stash a pointer
-	 * to the vma inside the execobj so that we can use a direct lookup
-	 * to find the right target VMA when doing relocations.
-	 */
-	eb->vma[i] = vma;
-	eb->flags[i] = entry->flags;
-	vma->exec_flags = &eb->flags[i];
+		list_add_tail(&ev->reloc_link, &eb->relocs);
 
 	/*
 	 * SNA is doing fancy tricks with compressing batch buffers, which leads
@@ -533,28 +532,26 @@ eb_add_vma(struct i915_execbuffer *eb,
 	 */
 	if (i == batch_idx) {
 		if (entry->relocation_count &&
-		    !(eb->flags[i] & EXEC_OBJECT_PINNED))
-			eb->flags[i] |= __EXEC_OBJECT_NEEDS_BIAS;
+		    !(ev->flags & EXEC_OBJECT_PINNED))
+			ev->flags |= __EXEC_OBJECT_NEEDS_BIAS;
 		if (eb->reloc_cache.has_fence)
-			eb->flags[i] |= EXEC_OBJECT_NEEDS_FENCE;
+			ev->flags |= EXEC_OBJECT_NEEDS_FENCE;
 
-		eb->batch = vma;
+		eb->batch = ev;
 	}
 
 	err = 0;
-	if (eb_pin_vma(eb, entry, 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(vma, vma->exec_flags);
+		eb_unreserve_vma(ev);
 
-		list_add_tail(&vma->exec_link, &eb->unbound);
+		list_add_tail(&ev->bind_link, &eb->unbound);
 		if (drm_mm_node_allocated(&vma->node))
 			err = i915_vma_unbind(vma);
-		if (unlikely(err))
-			vma->exec_flags = NULL;
 	}
 	return err;
 }
@@ -576,11 +573,11 @@ static inline int use_cpu_reloc(const struct reloc_cache *cache,
 		obj->cache_level != I915_CACHE_NONE);
 }
 
-static int eb_reserve_vma(const struct i915_execbuffer *eb,
-			  struct i915_vma *vma)
+static int eb_reserve_vma(const struct i915_execbuffer *eb, struct eb_vma *ev)
 {
-	struct drm_i915_gem_exec_object2 *entry = exec_entry(eb, vma);
-	unsigned int exec_flags = *vma->exec_flags;
+	struct drm_i915_gem_exec_object2 *entry = ev->exec;
+	unsigned int exec_flags = ev->flags;
+	struct i915_vma *vma = ev->vma;
 	u64 pin_flags;
 	int err;
 
@@ -627,8 +624,8 @@ static int eb_reserve_vma(const struct i915_execbuffer *eb,
 			exec_flags |= __EXEC_OBJECT_HAS_FENCE;
 	}
 
-	*vma->exec_flags = exec_flags | __EXEC_OBJECT_HAS_PIN;
-	GEM_BUG_ON(eb_vma_misplaced(entry, vma, exec_flags));
+	ev->flags = exec_flags | __EXEC_OBJECT_HAS_PIN;
+	GEM_BUG_ON(eb_vma_misplaced(entry, vma, ev->flags));
 
 	return 0;
 }
@@ -637,7 +634,7 @@ static int eb_reserve(struct i915_execbuffer *eb)
 {
 	const unsigned int count = eb->buffer_count;
 	struct list_head last;
-	struct i915_vma *vma;
+	struct eb_vma *ev;
 	unsigned int i, pass;
 	int err;
 
@@ -658,8 +655,8 @@ static int eb_reserve(struct i915_execbuffer *eb)
 	pass = 0;
 	err = 0;
 	do {
-		list_for_each_entry(vma, &eb->unbound, exec_link) {
-			err = eb_reserve_vma(eb, vma);
+		list_for_each_entry(ev, &eb->unbound, bind_link) {
+			err = eb_reserve_vma(eb, ev);
 			if (err)
 				break;
 		}
@@ -670,26 +667,26 @@ static int eb_reserve(struct i915_execbuffer *eb)
 		INIT_LIST_HEAD(&eb->unbound);
 		INIT_LIST_HEAD(&last);
 		for (i = 0; i < count; i++) {
-			unsigned int flags = eb->flags[i];
-			struct i915_vma *vma = eb->vma[i];
+			struct eb_vma *ev = &eb->vma[i];
+			unsigned int flags = ev->flags;
 
 			if (flags & EXEC_OBJECT_PINNED &&
 			    flags & __EXEC_OBJECT_HAS_PIN)
 				continue;
 
-			eb_unreserve_vma(vma, &eb->flags[i]);
+			eb_unreserve_vma(ev);
 
 			if (flags & EXEC_OBJECT_PINNED)
 				/* Pinned must have their slot */
-				list_add(&vma->exec_link, &eb->unbound);
+				list_add(&ev->bind_link, &eb->unbound);
 			else if (flags & __EXEC_OBJECT_NEEDS_MAP)
 				/* Map require the lowest 256MiB (aperture) */
-				list_add_tail(&vma->exec_link, &eb->unbound);
+				list_add_tail(&ev->bind_link, &eb->unbound);
 			else if (!(flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS))
 				/* Prioritise 4GiB region for restricted bo */
-				list_add(&vma->exec_link, &last);
+				list_add(&ev->bind_link, &last);
 			else
-				list_add_tail(&vma->exec_link, &last);
+				list_add_tail(&ev->bind_link, &last);
 		}
 		list_splice_tail(&last, &eb->unbound);
 
@@ -808,10 +805,8 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
 		if (unlikely(err))
 			goto err_vma;
 
-		GEM_BUG_ON(vma != eb->vma[i]);
-		GEM_BUG_ON(vma->exec_flags != &eb->flags[i]);
 		GEM_BUG_ON(drm_mm_node_allocated(&vma->node) &&
-			   eb_vma_misplaced(&eb->exec[i], vma, eb->flags[i]));
+			   eb_vma_misplaced(&eb->exec[i], vma, eb->vma[i].flags));
 	}
 
 	mutex_unlock(&eb->gem_context->mutex);
@@ -822,27 +817,27 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
 err_obj:
 	i915_gem_object_put(obj);
 err_vma:
-	eb->vma[i] = NULL;
+	eb->vma[i].vma = NULL;
 err_ctx:
 	mutex_unlock(&eb->gem_context->mutex);
 	return err;
 }
 
-static struct i915_vma *
+static struct eb_vma *
 eb_get_vma(const struct i915_execbuffer *eb, unsigned long handle)
 {
 	if (eb->lut_size < 0) {
 		if (handle >= -eb->lut_size)
 			return NULL;
-		return eb->vma[handle];
+		return &eb->vma[handle];
 	} else {
 		struct hlist_head *head;
-		struct i915_vma *vma;
+		struct eb_vma *ev;
 
 		head = &eb->buckets[hash_32(handle, eb->lut_size)];
-		hlist_for_each_entry(vma, head, exec_node) {
-			if (vma->exec_handle == handle)
-				return vma;
+		hlist_for_each_entry(ev, head, node) {
+			if (ev->handle == handle)
+				return ev;
 		}
 		return NULL;
 	}
@@ -854,20 +849,18 @@ static void eb_release_vmas(const struct i915_execbuffer *eb)
 	unsigned int i;
 
 	for (i = 0; i < count; i++) {
-		struct i915_vma *vma = eb->vma[i];
-		unsigned int flags = eb->flags[i];
+		struct eb_vma *ev = &eb->vma[i];
+		struct i915_vma *vma = ev->vma;
 
 		if (!vma)
 			break;
 
-		GEM_BUG_ON(vma->exec_flags != &eb->flags[i]);
-		vma->exec_flags = NULL;
-		eb->vma[i] = NULL;
+		eb->vma[i].vma = NULL;
 
-		if (flags & __EXEC_OBJECT_HAS_PIN)
-			__eb_unreserve_vma(vma, flags);
+		if (ev->flags & __EXEC_OBJECT_HAS_PIN)
+			__eb_unreserve_vma(vma, ev->flags);
 
-		if (flags & __EXEC_OBJECT_HAS_REF)
+		if (ev->flags & __EXEC_OBJECT_HAS_REF)
 			i915_vma_put(vma);
 	}
 }
@@ -1377,10 +1370,10 @@ relocate_entry(struct i915_vma *vma,
 
 static u64
 eb_relocate_entry(struct i915_execbuffer *eb,
-		  struct i915_vma *vma,
+		  struct eb_vma *ev,
 		  const struct drm_i915_gem_relocation_entry *reloc)
 {
-	struct i915_vma *target;
+	struct eb_vma *target;
 	int err;
 
 	/* we've already hold a reference to all valid objects */
@@ -1412,7 +1405,7 @@ eb_relocate_entry(struct i915_execbuffer *eb,
 	}
 
 	if (reloc->write_domain) {
-		*target->exec_flags |= EXEC_OBJECT_WRITE;
+		target->flags |= EXEC_OBJECT_WRITE;
 
 		/*
 		 * Sandybridge PPGTT errata: We need a global gtt mapping
@@ -1422,7 +1415,8 @@ eb_relocate_entry(struct i915_execbuffer *eb,
 		 */
 		if (reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION &&
 		    IS_GEN(eb->i915, 6)) {
-			err = i915_vma_bind(target, target->obj->cache_level,
+			err = i915_vma_bind(target->vma,
+					    target->vma->obj->cache_level,
 					    PIN_GLOBAL, NULL);
 			if (WARN_ONCE(err,
 				      "Unexpected failure to bind target VMA!"))
@@ -1435,17 +1429,17 @@ eb_relocate_entry(struct i915_execbuffer *eb,
 	 * more work needs to be done.
 	 */
 	if (!DBG_FORCE_RELOC &&
-	    gen8_canonical_addr(target->node.start) == reloc->presumed_offset)
+	    gen8_canonical_addr(target->vma->node.start) == reloc->presumed_offset)
 		return 0;
 
 	/* Check that the relocation address is valid... */
 	if (unlikely(reloc->offset >
-		     vma->size - (eb->reloc_cache.use_64bit_reloc ? 8 : 4))) {
+		     ev->vma->size - (eb->reloc_cache.use_64bit_reloc ? 8 : 4))) {
 		DRM_DEBUG("Relocation beyond object bounds: "
 			  "target %d offset %d size %d.\n",
 			  reloc->target_handle,
 			  (int)reloc->offset,
-			  (int)vma->size);
+			  (int)ev->vma->size);
 		return -EINVAL;
 	}
 	if (unlikely(reloc->offset & 3)) {
@@ -1464,18 +1458,18 @@ eb_relocate_entry(struct i915_execbuffer *eb,
 	 * do relocations we are already stalling, disable the user's opt
 	 * out of our synchronisation.
 	 */
-	*vma->exec_flags &= ~EXEC_OBJECT_ASYNC;
+	ev->flags &= ~EXEC_OBJECT_ASYNC;
 
 	/* and update the user's relocation entry */
-	return relocate_entry(vma, reloc, eb, target);
+	return relocate_entry(ev->vma, reloc, eb, target->vma);
 }
 
-static int eb_relocate_vma(struct i915_execbuffer *eb, struct i915_vma *vma)
+static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev)
 {
 #define N_RELOC(x) ((x) / sizeof(struct drm_i915_gem_relocation_entry))
 	struct drm_i915_gem_relocation_entry stack[N_RELOC(512)];
 	struct drm_i915_gem_relocation_entry __user *urelocs;
-	const struct drm_i915_gem_exec_object2 *entry = exec_entry(eb, vma);
+	const struct drm_i915_gem_exec_object2 *entry = ev->exec;
 	unsigned int remain;
 
 	urelocs = u64_to_user_ptr(entry->relocs_ptr);
@@ -1515,7 +1509,7 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct i915_vma *vma)
 
 		remain -= count;
 		do {
-			u64 offset = eb_relocate_entry(eb, vma, r);
+			u64 offset = eb_relocate_entry(eb, ev, r);
 
 			if (likely(offset == 0)) {
 			} else if ((s64)offset < 0) {
@@ -1558,16 +1552,16 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct i915_vma *vma)
 }
 
 static int
-eb_relocate_vma_slow(struct i915_execbuffer *eb, struct i915_vma *vma)
+eb_relocate_vma_slow(struct i915_execbuffer *eb, struct eb_vma *ev)
 {
-	const struct drm_i915_gem_exec_object2 *entry = exec_entry(eb, vma);
+	const struct drm_i915_gem_exec_object2 *entry = ev->exec;
 	struct drm_i915_gem_relocation_entry *relocs =
 		u64_to_ptr(typeof(*relocs), entry->relocs_ptr);
 	unsigned int i;
 	int err;
 
 	for (i = 0; i < entry->relocation_count; i++) {
-		u64 offset = eb_relocate_entry(eb, vma, &relocs[i]);
+		u64 offset = eb_relocate_entry(eb, ev, &relocs[i]);
 
 		if ((s64)offset < 0) {
 			err = (int)offset;
@@ -1711,7 +1705,7 @@ static noinline int eb_relocate_slow(struct i915_execbuffer *eb)
 {
 	struct drm_device *dev = &eb->i915->drm;
 	bool have_copy = false;
-	struct i915_vma *vma;
+	struct eb_vma *ev;
 	int err = 0;
 
 repeat:
@@ -1767,15 +1761,15 @@ static noinline int eb_relocate_slow(struct i915_execbuffer *eb)
 
 	GEM_BUG_ON(!eb->batch);
 
-	list_for_each_entry(vma, &eb->relocs, reloc_link) {
+	list_for_each_entry(ev, &eb->relocs, reloc_link) {
 		if (!have_copy) {
 			pagefault_disable();
-			err = eb_relocate_vma(eb, vma);
+			err = eb_relocate_vma(eb, ev);
 			pagefault_enable();
 			if (err)
 				goto repeat;
 		} else {
-			err = eb_relocate_vma_slow(eb, vma);
+			err = eb_relocate_vma_slow(eb, ev);
 			if (err)
 				goto err;
 		}
@@ -1820,10 +1814,10 @@ static int eb_relocate(struct i915_execbuffer *eb)
 
 	/* The objects are in their final locations, apply the relocations. */
 	if (eb->args->flags & __EXEC_HAS_RELOC) {
-		struct i915_vma *vma;
+		struct eb_vma *ev;
 
-		list_for_each_entry(vma, &eb->relocs, reloc_link) {
-			if (eb_relocate_vma(eb, vma))
+		list_for_each_entry(ev, &eb->relocs, reloc_link) {
+			if (eb_relocate_vma(eb, ev))
 				goto slow;
 		}
 	}
@@ -1844,39 +1838,34 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
 	ww_acquire_init(&acquire, &reservation_ww_class);
 
 	for (i = 0; i < count; i++) {
-		struct i915_vma *vma = eb->vma[i];
+		struct eb_vma *ev = &eb->vma[i];
+		struct i915_vma *vma = ev->vma;
 
 		err = ww_mutex_lock_interruptible(&vma->resv->lock, &acquire);
-		if (!err)
-			continue;
-
-		GEM_BUG_ON(err == -EALREADY); /* No duplicate vma */
-
 		if (err == -EDEADLK) {
 			GEM_BUG_ON(i == 0);
 			do {
 				int j = i - 1;
 
-				ww_mutex_unlock(&eb->vma[j]->resv->lock);
+				ww_mutex_unlock(&eb->vma[j].vma->resv->lock);
 
-				swap(eb->flags[i], eb->flags[j]);
 				swap(eb->vma[i],  eb->vma[j]);
-				eb->vma[i]->exec_flags = &eb->flags[i];
 			} while (--i);
-			GEM_BUG_ON(vma != eb->vma[0]);
-			vma->exec_flags = &eb->flags[0];
 
 			err = ww_mutex_lock_slow_interruptible(&vma->resv->lock,
 							       &acquire);
 		}
+		if (err == -EALREADY)
+			err = 0;
 		if (err)
 			break;
 	}
 	ww_acquire_done(&acquire);
 
 	while (i--) {
-		unsigned int flags = eb->flags[i];
-		struct i915_vma *vma = eb->vma[i];
+		struct eb_vma *ev = &eb->vma[i];
+		struct i915_vma *vma = ev->vma;
+		unsigned int flags = ev->flags;
 		struct drm_i915_gem_object *obj = vma->obj;
 
 		assert_vma_held(vma);
@@ -1920,10 +1909,10 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
 		i915_vma_unlock(vma);
 
 		__eb_unreserve_vma(vma, flags);
-		vma->exec_flags = NULL;
-
 		if (unlikely(flags & __EXEC_OBJECT_HAS_REF))
 			i915_vma_put(vma);
+
+		ev->vma = NULL;
 	}
 	ww_acquire_fini(&acquire);
 
@@ -2001,7 +1990,7 @@ static struct i915_vma *eb_parse(struct i915_execbuffer *eb, bool is_master)
 		return ERR_CAST(pool);
 
 	err = intel_engine_cmd_parser(eb->engine,
-				      eb->batch->obj,
+				      eb->batch->vma->obj,
 				      pool->obj,
 				      eb->batch_start_offset,
 				      eb->batch_len,
@@ -2018,10 +2007,9 @@ static struct i915_vma *eb_parse(struct i915_execbuffer *eb, bool is_master)
 	if (IS_ERR(vma))
 		goto err;
 
-	eb->vma[eb->buffer_count] = i915_vma_get(vma);
-	eb->flags[eb->buffer_count] =
+	eb->vma[eb->buffer_count].vma = i915_vma_get(vma);
+	eb->vma[eb->buffer_count].flags =
 		__EXEC_OBJECT_HAS_PIN | __EXEC_OBJECT_HAS_REF;
-	vma->exec_flags = &eb->flags[eb->buffer_count];
 	eb->buffer_count++;
 
 	vma->private = pool;
@@ -2044,7 +2032,7 @@ add_to_client(struct i915_request *rq, struct drm_file *file)
 	spin_unlock(&file_priv->mm.lock);
 }
 
-static int eb_submit(struct i915_execbuffer *eb)
+static int eb_submit(struct i915_execbuffer *eb, struct i915_vma *batch)
 {
 	int err;
 
@@ -2071,7 +2059,7 @@ static int eb_submit(struct i915_execbuffer *eb)
 	}
 
 	err = eb->engine->emit_bb_start(eb->request,
-					eb->batch->node.start +
+					batch->node.start +
 					eb->batch_start_offset,
 					eb->batch_len,
 					eb->batch_flags);
@@ -2434,6 +2422,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 	struct dma_fence *in_fence = NULL;
 	struct dma_fence *exec_fence = NULL;
 	struct sync_file *out_fence = NULL;
+	struct i915_vma *batch;
 	int out_fence_fd = -1;
 	int err;
 
@@ -2448,9 +2437,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 		args->flags |= __EXEC_HAS_RELOC;
 
 	eb.exec = exec;
-	eb.vma = (struct i915_vma **)(exec + args->buffer_count + 1);
-	eb.vma[0] = NULL;
-	eb.flags = (unsigned int *)(eb.vma + args->buffer_count + 1);
+	eb.vma = (struct eb_vma *)(exec + args->buffer_count + 1);
+	eb.vma[0].vma = NULL;
 
 	eb.invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS;
 	reloc_cache_init(&eb.reloc_cache, eb.i915);
@@ -2527,13 +2515,16 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 		goto err_vma;
 	}
 
-	if (unlikely(*eb.batch->exec_flags & EXEC_OBJECT_WRITE)) {
+	if (unlikely(eb.batch->flags & EXEC_OBJECT_WRITE)) {
 		DRM_DEBUG("Attempting to use self-modifying batch buffer\n");
 		err = -EINVAL;
 		goto err_vma;
 	}
-	if (eb.batch_start_offset > eb.batch->size ||
-	    eb.batch_len > eb.batch->size - eb.batch_start_offset) {
+
+	batch = eb.batch->vma;
+	if (range_overflows_t(u64,
+			      eb.batch_start_offset, eb.batch_len,
+			      batch->size)) {
 		DRM_DEBUG("Attempting to use out-of-bounds batch\n");
 		err = -EINVAL;
 		goto err_vma;
@@ -2560,12 +2551,12 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 			 */
 			eb.batch_flags |= I915_DISPATCH_SECURE;
 			eb.batch_start_offset = 0;
-			eb.batch = vma;
+			batch = vma;
 		}
 	}
 
 	if (eb.batch_len == 0)
-		eb.batch_len = eb.batch->size - eb.batch_start_offset;
+		eb.batch_len = batch->size - eb.batch_start_offset;
 
 	/*
 	 * snb/ivb/vlv conflate the "batch in ppgtt" bit with the "non-secure
@@ -2584,13 +2575,13 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 		 *   fitting due to fragmentation.
 		 * So this is actually safe.
 		 */
-		vma = i915_gem_object_ggtt_pin(eb.batch->obj, NULL, 0, 0, 0);
+		vma = i915_gem_object_ggtt_pin(batch->obj, NULL, 0, 0, 0);
 		if (IS_ERR(vma)) {
 			err = PTR_ERR(vma);
 			goto err_vma;
 		}
 
-		eb.batch = vma;
+		batch = vma;
 	}
 
 	/* All GPU relocation batches must be submitted prior to the user rq */
@@ -2637,12 +2628,12 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 	 * inactive_list and lose its active reference. Hence we do not need
 	 * to explicitly hold another reference here.
 	 */
-	eb.request->batch = eb.batch;
-	if (eb.batch->private)
-		intel_engine_pool_mark_active(eb.batch->private, eb.request);
+	eb.request->batch = batch;
+	if (batch->private)
+		intel_engine_pool_mark_active(batch->private, eb.request);
 
 	trace_i915_request_queue(eb.request, eb.batch_flags);
-	err = eb_submit(&eb);
+	err = eb_submit(&eb, batch);
 err_request:
 	add_to_client(eb.request, file);
 	i915_request_add(eb.request);
@@ -2663,9 +2654,9 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 
 err_batch_unpin:
 	if (eb.batch_flags & I915_DISPATCH_SECURE)
-		i915_vma_unpin(eb.batch);
-	if (eb.batch->private)
-		intel_engine_pool_put(eb.batch->private);
+		i915_vma_unpin(batch);
+	if (batch->private)
+		intel_engine_pool_put(batch->private);
 err_vma:
 	if (eb.exec)
 		eb_release_vmas(&eb);
@@ -2688,9 +2679,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 
 static size_t eb_element_size(void)
 {
-	return (sizeof(struct drm_i915_gem_exec_object2) +
-		sizeof(struct i915_vma *) +
-		sizeof(unsigned int));
+	return sizeof(struct drm_i915_gem_exec_object2) + sizeof(struct eb_vma);
 }
 
 static bool check_buffer_count(size_t count)
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 465932813bc5..71402056d846 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -147,21 +147,10 @@ struct i915_vma {
 	struct rb_node obj_node;
 	struct hlist_node obj_hash;
 
-	/** This vma's place in the execbuf reservation list */
-	struct list_head exec_link;
-	struct list_head reloc_link;
-
 	/** This vma's place in the eviction list */
 	struct list_head evict_link;
 
 	struct list_head closed_link;
-
-	/**
-	 * Used for performing relocations during execbuffer insertion.
-	 */
-	unsigned int *exec_flags;
-	struct hlist_node exec_node;
-	u32 exec_handle;
 };
 
 struct i915_vma *
-- 
2.24.0

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Handle i915_active_fence_set() with the same fence
@ 2019-11-06 19:22   ` Patchwork
  0 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2019-11-06 19:22 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915: Handle i915_active_fence_set() with the same fence
URL   : https://patchwork.freedesktop.org/series/69074/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7273 -> Patchwork_15157
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/index.html

Known issues
------------

  Here are the changes found in Patchwork_15157 that come from known issues:

### IGT changes ###

#### Possible fixes ####

  * igt@gem_exec_suspend@basic:
    - fi-icl-u3:          [FAIL][1] ([fdo#111699]) -> [PASS][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/fi-icl-u3/igt@gem_exec_suspend@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/fi-icl-u3/igt@gem_exec_suspend@basic.html

  * igt@gem_exec_suspend@basic-s3:
    - {fi-cml-s}:         [DMESG-WARN][3] ([fdo#111764]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/fi-cml-s/igt@gem_exec_suspend@basic-s3.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/fi-cml-s/igt@gem_exec_suspend@basic-s3.html

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-guc:         [FAIL][5] -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_hugepages:
    - fi-icl-u3:          [DMESG-WARN][7] -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/fi-icl-u3/igt@i915_selftest@live_hugepages.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/fi-icl-u3/igt@i915_selftest@live_hugepages.html

  * igt@kms_frontbuffer_tracking@basic:
    - {fi-icl-u4}:        [FAIL][9] ([fdo#103167]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/fi-icl-u4/igt@kms_frontbuffer_tracking@basic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/fi-icl-u4/igt@kms_frontbuffer_tracking@basic.html

  
#### Warnings ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][11] ([fdo#111407]) -> [FAIL][12] ([fdo#111045] / [fdo#111096])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
  [fdo#111699]: https://bugs.freedesktop.org/show_bug.cgi?id=111699
  [fdo#111764]: https://bugs.freedesktop.org/show_bug.cgi?id=111764


Participating hosts (51 -> 44)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7273 -> Patchwork_15157

  CI-20190529: 20190529
  CI_DRM_7273: 41b4771cadb55632f129b94751bf5dee7dcfdcc4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5264: f21213012393bd8041ad93084ce29da088ef8426 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15157: 84128d02d900a3a1a72c8d44c2b09276e1ed407a @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

84128d02d900 drm/i915/gem: Extract transient execbuf flags from i915_vma
080de2dcccc6 drm/i915: Drop inspection of execbuf flags during evict
13993cd01cc9 drm/i915: Handle i915_active_fence_set() with the same fence

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Handle i915_active_fence_set() with the same fence
@ 2019-11-06 19:22   ` Patchwork
  0 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2019-11-06 19:22 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915: Handle i915_active_fence_set() with the same fence
URL   : https://patchwork.freedesktop.org/series/69074/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7273 -> Patchwork_15157
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/index.html

Known issues
------------

  Here are the changes found in Patchwork_15157 that come from known issues:

### IGT changes ###

#### Possible fixes ####

  * igt@gem_exec_suspend@basic:
    - fi-icl-u3:          [FAIL][1] ([fdo#111699]) -> [PASS][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/fi-icl-u3/igt@gem_exec_suspend@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/fi-icl-u3/igt@gem_exec_suspend@basic.html

  * igt@gem_exec_suspend@basic-s3:
    - {fi-cml-s}:         [DMESG-WARN][3] ([fdo#111764]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/fi-cml-s/igt@gem_exec_suspend@basic-s3.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/fi-cml-s/igt@gem_exec_suspend@basic-s3.html

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-guc:         [FAIL][5] -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_hugepages:
    - fi-icl-u3:          [DMESG-WARN][7] -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/fi-icl-u3/igt@i915_selftest@live_hugepages.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/fi-icl-u3/igt@i915_selftest@live_hugepages.html

  * igt@kms_frontbuffer_tracking@basic:
    - {fi-icl-u4}:        [FAIL][9] ([fdo#103167]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/fi-icl-u4/igt@kms_frontbuffer_tracking@basic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/fi-icl-u4/igt@kms_frontbuffer_tracking@basic.html

  
#### Warnings ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][11] ([fdo#111407]) -> [FAIL][12] ([fdo#111045] / [fdo#111096])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
  [fdo#111699]: https://bugs.freedesktop.org/show_bug.cgi?id=111699
  [fdo#111764]: https://bugs.freedesktop.org/show_bug.cgi?id=111764


Participating hosts (51 -> 44)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7273 -> Patchwork_15157

  CI-20190529: 20190529
  CI_DRM_7273: 41b4771cadb55632f129b94751bf5dee7dcfdcc4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5264: f21213012393bd8041ad93084ce29da088ef8426 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15157: 84128d02d900a3a1a72c8d44c2b09276e1ed407a @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

84128d02d900 drm/i915/gem: Extract transient execbuf flags from i915_vma
080de2dcccc6 drm/i915: Drop inspection of execbuf flags during evict
13993cd01cc9 drm/i915: Handle i915_active_fence_set() with the same fence

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH 3/3] drm/i915/gem: Extract transient execbuf flags from i915_vma
@ 2019-11-08  9:53     ` Daniel Vetter
  0 siblings, 0 replies; 30+ messages in thread
From: Daniel Vetter @ 2019-11-08  9:53 UTC (permalink / raw)
  To: Chris Wilson, Maarten Lankhorst; +Cc: intel-gfx

On Wed, Nov 6, 2019 at 4:48 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> For our convenience, and to avoid frequent allocations, we placed some
> lists we use for execbuf inside the common i915_vma struct. As we look
> to parallelise execbuf, such fields guarded by the struct_mutex BKL must
> be pulled under local control. Instead of using the i915_vma as our
> primary means of tracking the user's list of objects and their virtual
> mappings, we use a local eb_vma with the same lists as before (just now
> local not global).
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

(--supress-cc ? Added Maarten for real, didn't seem to be on cc in my
inbox at least)

Why do we need this?

For the lmem stuff we need to hold the dma_resv, which is on the
object. Needs to be on the object, since lmem is physical memory, not
virtual gtt (we might have a different story on old ggtt execbuf
contention lols). vma->obj is N:1 (unless some remapping stuff crept
in that I totally missed), so locking the obj dma_resv ww_mutex during
execbuf prep also gives us the entire vma locked. Which means we can
just leave everything where it is, no need to move stuff. At least no
for the s/struct_mutex/ww_acquire_ctx/ magic trick.

I'm also kinda wondered whether the same applies for some of the other
vma state that got split out already, like the vma->pin_count
tracking. At least naive application of ww_mutex would cover ww_mutex
everywhere we do the new locking for vma pinning, so seems superflous.

But good chances I'm just slightly confused as usual :-)
-Daniel

> ---
>  .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 271 +++++++++---------
>  drivers/gpu/drm/i915/i915_vma.h               |  11 -
>  2 files changed, 130 insertions(+), 152 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index e4f5c269150a..8eb9c4e17514 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -27,6 +27,19 @@
>  #include "i915_gem_ioctls.h"
>  #include "i915_trace.h"
>
> +struct eb_vma {
> +       struct i915_vma *vma;
> +       unsigned int flags;
> +
> +       /** This vma's place in the execbuf reservation list */
> +       struct drm_i915_gem_exec_object2 *exec;
> +       struct list_head bind_link;
> +       struct list_head reloc_link;
> +
> +       struct hlist_node node;
> +       u32 handle;
> +};
> +
>  enum {
>         FORCE_CPU_RELOC = 1,
>         FORCE_GTT_RELOC,
> @@ -219,15 +232,14 @@ struct i915_execbuffer {
>         struct drm_file *file; /** per-file lookup tables and limits */
>         struct drm_i915_gem_execbuffer2 *args; /** ioctl parameters */
>         struct drm_i915_gem_exec_object2 *exec; /** ioctl execobj[] */
> -       struct i915_vma **vma;
> -       unsigned int *flags;
> +       struct eb_vma *vma;
>
>         struct intel_engine_cs *engine; /** engine to queue the request to */
>         struct intel_context *context; /* logical state for the request */
>         struct i915_gem_context *gem_context; /** caller's context */
>
>         struct i915_request *request; /** our request to build */
> -       struct i915_vma *batch; /** identity of the batch obj/vma */
> +       struct eb_vma *batch; /** identity of the batch obj/vma */
>
>         /** actual size of execobj[] as we may extend it for the cmdparser */
>         unsigned int buffer_count;
> @@ -275,8 +287,6 @@ struct i915_execbuffer {
>         struct hlist_head *buckets; /** ht for relocation handles */
>  };
>
> -#define exec_entry(EB, VMA) (&(EB)->exec[(VMA)->exec_flags - (EB)->flags])
> -
>  /*
>   * Used to convert any address to canonical form.
>   * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
> @@ -380,9 +390,9 @@ eb_vma_misplaced(const struct drm_i915_gem_exec_object2 *entry,
>  static inline bool
>  eb_pin_vma(struct i915_execbuffer *eb,
>            const struct drm_i915_gem_exec_object2 *entry,
> -          struct i915_vma *vma)
> +          struct eb_vma *ev)
>  {
> -       unsigned int exec_flags = *vma->exec_flags;
> +       struct i915_vma *vma = ev->vma;
>         u64 pin_flags;
>
>         if (vma->node.size)
> @@ -391,24 +401,24 @@ eb_pin_vma(struct i915_execbuffer *eb,
>                 pin_flags = entry->offset & PIN_OFFSET_MASK;
>
>         pin_flags |= PIN_USER | PIN_NOEVICT | PIN_OFFSET_FIXED;
> -       if (unlikely(exec_flags & EXEC_OBJECT_NEEDS_GTT))
> +       if (unlikely(ev->flags & EXEC_OBJECT_NEEDS_GTT))
>                 pin_flags |= PIN_GLOBAL;
>
>         if (unlikely(i915_vma_pin(vma, 0, 0, pin_flags)))
>                 return false;
>
> -       if (unlikely(exec_flags & EXEC_OBJECT_NEEDS_FENCE)) {
> +       if (unlikely(ev->flags & EXEC_OBJECT_NEEDS_FENCE)) {
>                 if (unlikely(i915_vma_pin_fence(vma))) {
>                         i915_vma_unpin(vma);
>                         return false;
>                 }
>
>                 if (vma->fence)
> -                       exec_flags |= __EXEC_OBJECT_HAS_FENCE;
> +                       ev->flags |= __EXEC_OBJECT_HAS_FENCE;
>         }
>
> -       *vma->exec_flags = exec_flags | __EXEC_OBJECT_HAS_PIN;
> -       return !eb_vma_misplaced(entry, vma, exec_flags);
> +       ev->flags |= __EXEC_OBJECT_HAS_PIN;
> +       return !eb_vma_misplaced(entry, vma, ev->flags);
>  }
>
>  static inline void __eb_unreserve_vma(struct i915_vma *vma, unsigned int flags)
> @@ -422,13 +432,13 @@ static inline void __eb_unreserve_vma(struct i915_vma *vma, unsigned int flags)
>  }
>
>  static inline void
> -eb_unreserve_vma(struct i915_vma *vma, unsigned int *flags)
> +eb_unreserve_vma(struct eb_vma *ev)
>  {
> -       if (!(*flags & __EXEC_OBJECT_HAS_PIN))
> +       if (!(ev->flags & __EXEC_OBJECT_HAS_PIN))
>                 return;
>
> -       __eb_unreserve_vma(vma, *flags);
> -       *flags &= ~__EXEC_OBJECT_RESERVED;
> +       __eb_unreserve_vma(ev->vma, ev->flags);
> +       ev->flags &= ~__EXEC_OBJECT_RESERVED;
>  }
>
>  static int
> @@ -458,12 +468,6 @@ eb_validate_vma(struct i915_execbuffer *eb,
>                 entry->pad_to_size = 0;
>         }
>
> -       if (unlikely(vma->exec_flags)) {
> -               DRM_DEBUG("Object [handle %d, index %d] appears more than once in object list\n",
> -                         entry->handle, (int)(entry - eb->exec));
> -               return -EINVAL;
> -       }
> -
>         /*
>          * From drm_mm perspective address space is continuous,
>          * so from this point we're always using non-canonical
> @@ -492,6 +496,7 @@ eb_add_vma(struct i915_execbuffer *eb,
>            struct i915_vma *vma)
>  {
>         struct drm_i915_gem_exec_object2 *entry = &eb->exec[i];
> +       struct eb_vma *ev = &eb->vma[i];
>         int err;
>
>         GEM_BUG_ON(i915_vma_is_closed(vma));
> @@ -502,25 +507,19 @@ eb_add_vma(struct i915_execbuffer *eb,
>                         return err;
>         }
>
> +       ev->vma = vma;
> +       ev->exec = entry;
> +       ev->flags = entry->flags;
> +
>         if (eb->lut_size > 0) {
> -               vma->exec_handle = entry->handle;
> -               hlist_add_head(&vma->exec_node,
> +               ev->handle = entry->handle;
> +               hlist_add_head(&ev->node,
>                                &eb->buckets[hash_32(entry->handle,
>                                                     eb->lut_size)]);
>         }
>
>         if (entry->relocation_count)
> -               list_add_tail(&vma->reloc_link, &eb->relocs);
> -
> -       /*
> -        * Stash a pointer from the vma to execobj, so we can query its flags,
> -        * size, alignment etc as provided by the user. Also we stash a pointer
> -        * to the vma inside the execobj so that we can use a direct lookup
> -        * to find the right target VMA when doing relocations.
> -        */
> -       eb->vma[i] = vma;
> -       eb->flags[i] = entry->flags;
> -       vma->exec_flags = &eb->flags[i];
> +               list_add_tail(&ev->reloc_link, &eb->relocs);
>
>         /*
>          * SNA is doing fancy tricks with compressing batch buffers, which leads
> @@ -533,28 +532,26 @@ eb_add_vma(struct i915_execbuffer *eb,
>          */
>         if (i == batch_idx) {
>                 if (entry->relocation_count &&
> -                   !(eb->flags[i] & EXEC_OBJECT_PINNED))
> -                       eb->flags[i] |= __EXEC_OBJECT_NEEDS_BIAS;
> +                   !(ev->flags & EXEC_OBJECT_PINNED))
> +                       ev->flags |= __EXEC_OBJECT_NEEDS_BIAS;
>                 if (eb->reloc_cache.has_fence)
> -                       eb->flags[i] |= EXEC_OBJECT_NEEDS_FENCE;
> +                       ev->flags |= EXEC_OBJECT_NEEDS_FENCE;
>
> -               eb->batch = vma;
> +               eb->batch = ev;
>         }
>
>         err = 0;
> -       if (eb_pin_vma(eb, entry, 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(vma, vma->exec_flags);
> +               eb_unreserve_vma(ev);
>
> -               list_add_tail(&vma->exec_link, &eb->unbound);
> +               list_add_tail(&ev->bind_link, &eb->unbound);
>                 if (drm_mm_node_allocated(&vma->node))
>                         err = i915_vma_unbind(vma);
> -               if (unlikely(err))
> -                       vma->exec_flags = NULL;
>         }
>         return err;
>  }
> @@ -576,11 +573,11 @@ static inline int use_cpu_reloc(const struct reloc_cache *cache,
>                 obj->cache_level != I915_CACHE_NONE);
>  }
>
> -static int eb_reserve_vma(const struct i915_execbuffer *eb,
> -                         struct i915_vma *vma)
> +static int eb_reserve_vma(const struct i915_execbuffer *eb, struct eb_vma *ev)
>  {
> -       struct drm_i915_gem_exec_object2 *entry = exec_entry(eb, vma);
> -       unsigned int exec_flags = *vma->exec_flags;
> +       struct drm_i915_gem_exec_object2 *entry = ev->exec;
> +       unsigned int exec_flags = ev->flags;
> +       struct i915_vma *vma = ev->vma;
>         u64 pin_flags;
>         int err;
>
> @@ -627,8 +624,8 @@ static int eb_reserve_vma(const struct i915_execbuffer *eb,
>                         exec_flags |= __EXEC_OBJECT_HAS_FENCE;
>         }
>
> -       *vma->exec_flags = exec_flags | __EXEC_OBJECT_HAS_PIN;
> -       GEM_BUG_ON(eb_vma_misplaced(entry, vma, exec_flags));
> +       ev->flags = exec_flags | __EXEC_OBJECT_HAS_PIN;
> +       GEM_BUG_ON(eb_vma_misplaced(entry, vma, ev->flags));
>
>         return 0;
>  }
> @@ -637,7 +634,7 @@ static int eb_reserve(struct i915_execbuffer *eb)
>  {
>         const unsigned int count = eb->buffer_count;
>         struct list_head last;
> -       struct i915_vma *vma;
> +       struct eb_vma *ev;
>         unsigned int i, pass;
>         int err;
>
> @@ -658,8 +655,8 @@ static int eb_reserve(struct i915_execbuffer *eb)
>         pass = 0;
>         err = 0;
>         do {
> -               list_for_each_entry(vma, &eb->unbound, exec_link) {
> -                       err = eb_reserve_vma(eb, vma);
> +               list_for_each_entry(ev, &eb->unbound, bind_link) {
> +                       err = eb_reserve_vma(eb, ev);
>                         if (err)
>                                 break;
>                 }
> @@ -670,26 +667,26 @@ static int eb_reserve(struct i915_execbuffer *eb)
>                 INIT_LIST_HEAD(&eb->unbound);
>                 INIT_LIST_HEAD(&last);
>                 for (i = 0; i < count; i++) {
> -                       unsigned int flags = eb->flags[i];
> -                       struct i915_vma *vma = eb->vma[i];
> +                       struct eb_vma *ev = &eb->vma[i];
> +                       unsigned int flags = ev->flags;
>
>                         if (flags & EXEC_OBJECT_PINNED &&
>                             flags & __EXEC_OBJECT_HAS_PIN)
>                                 continue;
>
> -                       eb_unreserve_vma(vma, &eb->flags[i]);
> +                       eb_unreserve_vma(ev);
>
>                         if (flags & EXEC_OBJECT_PINNED)
>                                 /* Pinned must have their slot */
> -                               list_add(&vma->exec_link, &eb->unbound);
> +                               list_add(&ev->bind_link, &eb->unbound);
>                         else if (flags & __EXEC_OBJECT_NEEDS_MAP)
>                                 /* Map require the lowest 256MiB (aperture) */
> -                               list_add_tail(&vma->exec_link, &eb->unbound);
> +                               list_add_tail(&ev->bind_link, &eb->unbound);
>                         else if (!(flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS))
>                                 /* Prioritise 4GiB region for restricted bo */
> -                               list_add(&vma->exec_link, &last);
> +                               list_add(&ev->bind_link, &last);
>                         else
> -                               list_add_tail(&vma->exec_link, &last);
> +                               list_add_tail(&ev->bind_link, &last);
>                 }
>                 list_splice_tail(&last, &eb->unbound);
>
> @@ -808,10 +805,8 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
>                 if (unlikely(err))
>                         goto err_vma;
>
> -               GEM_BUG_ON(vma != eb->vma[i]);
> -               GEM_BUG_ON(vma->exec_flags != &eb->flags[i]);
>                 GEM_BUG_ON(drm_mm_node_allocated(&vma->node) &&
> -                          eb_vma_misplaced(&eb->exec[i], vma, eb->flags[i]));
> +                          eb_vma_misplaced(&eb->exec[i], vma, eb->vma[i].flags));
>         }
>
>         mutex_unlock(&eb->gem_context->mutex);
> @@ -822,27 +817,27 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
>  err_obj:
>         i915_gem_object_put(obj);
>  err_vma:
> -       eb->vma[i] = NULL;
> +       eb->vma[i].vma = NULL;
>  err_ctx:
>         mutex_unlock(&eb->gem_context->mutex);
>         return err;
>  }
>
> -static struct i915_vma *
> +static struct eb_vma *
>  eb_get_vma(const struct i915_execbuffer *eb, unsigned long handle)
>  {
>         if (eb->lut_size < 0) {
>                 if (handle >= -eb->lut_size)
>                         return NULL;
> -               return eb->vma[handle];
> +               return &eb->vma[handle];
>         } else {
>                 struct hlist_head *head;
> -               struct i915_vma *vma;
> +               struct eb_vma *ev;
>
>                 head = &eb->buckets[hash_32(handle, eb->lut_size)];
> -               hlist_for_each_entry(vma, head, exec_node) {
> -                       if (vma->exec_handle == handle)
> -                               return vma;
> +               hlist_for_each_entry(ev, head, node) {
> +                       if (ev->handle == handle)
> +                               return ev;
>                 }
>                 return NULL;
>         }
> @@ -854,20 +849,18 @@ static void eb_release_vmas(const struct i915_execbuffer *eb)
>         unsigned int i;
>
>         for (i = 0; i < count; i++) {
> -               struct i915_vma *vma = eb->vma[i];
> -               unsigned int flags = eb->flags[i];
> +               struct eb_vma *ev = &eb->vma[i];
> +               struct i915_vma *vma = ev->vma;
>
>                 if (!vma)
>                         break;
>
> -               GEM_BUG_ON(vma->exec_flags != &eb->flags[i]);
> -               vma->exec_flags = NULL;
> -               eb->vma[i] = NULL;
> +               eb->vma[i].vma = NULL;
>
> -               if (flags & __EXEC_OBJECT_HAS_PIN)
> -                       __eb_unreserve_vma(vma, flags);
> +               if (ev->flags & __EXEC_OBJECT_HAS_PIN)
> +                       __eb_unreserve_vma(vma, ev->flags);
>
> -               if (flags & __EXEC_OBJECT_HAS_REF)
> +               if (ev->flags & __EXEC_OBJECT_HAS_REF)
>                         i915_vma_put(vma);
>         }
>  }
> @@ -1377,10 +1370,10 @@ relocate_entry(struct i915_vma *vma,
>
>  static u64
>  eb_relocate_entry(struct i915_execbuffer *eb,
> -                 struct i915_vma *vma,
> +                 struct eb_vma *ev,
>                   const struct drm_i915_gem_relocation_entry *reloc)
>  {
> -       struct i915_vma *target;
> +       struct eb_vma *target;
>         int err;
>
>         /* we've already hold a reference to all valid objects */
> @@ -1412,7 +1405,7 @@ eb_relocate_entry(struct i915_execbuffer *eb,
>         }
>
>         if (reloc->write_domain) {
> -               *target->exec_flags |= EXEC_OBJECT_WRITE;
> +               target->flags |= EXEC_OBJECT_WRITE;
>
>                 /*
>                  * Sandybridge PPGTT errata: We need a global gtt mapping
> @@ -1422,7 +1415,8 @@ eb_relocate_entry(struct i915_execbuffer *eb,
>                  */
>                 if (reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION &&
>                     IS_GEN(eb->i915, 6)) {
> -                       err = i915_vma_bind(target, target->obj->cache_level,
> +                       err = i915_vma_bind(target->vma,
> +                                           target->vma->obj->cache_level,
>                                             PIN_GLOBAL, NULL);
>                         if (WARN_ONCE(err,
>                                       "Unexpected failure to bind target VMA!"))
> @@ -1435,17 +1429,17 @@ eb_relocate_entry(struct i915_execbuffer *eb,
>          * more work needs to be done.
>          */
>         if (!DBG_FORCE_RELOC &&
> -           gen8_canonical_addr(target->node.start) == reloc->presumed_offset)
> +           gen8_canonical_addr(target->vma->node.start) == reloc->presumed_offset)
>                 return 0;
>
>         /* Check that the relocation address is valid... */
>         if (unlikely(reloc->offset >
> -                    vma->size - (eb->reloc_cache.use_64bit_reloc ? 8 : 4))) {
> +                    ev->vma->size - (eb->reloc_cache.use_64bit_reloc ? 8 : 4))) {
>                 DRM_DEBUG("Relocation beyond object bounds: "
>                           "target %d offset %d size %d.\n",
>                           reloc->target_handle,
>                           (int)reloc->offset,
> -                         (int)vma->size);
> +                         (int)ev->vma->size);
>                 return -EINVAL;
>         }
>         if (unlikely(reloc->offset & 3)) {
> @@ -1464,18 +1458,18 @@ eb_relocate_entry(struct i915_execbuffer *eb,
>          * do relocations we are already stalling, disable the user's opt
>          * out of our synchronisation.
>          */
> -       *vma->exec_flags &= ~EXEC_OBJECT_ASYNC;
> +       ev->flags &= ~EXEC_OBJECT_ASYNC;
>
>         /* and update the user's relocation entry */
> -       return relocate_entry(vma, reloc, eb, target);
> +       return relocate_entry(ev->vma, reloc, eb, target->vma);
>  }
>
> -static int eb_relocate_vma(struct i915_execbuffer *eb, struct i915_vma *vma)
> +static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev)
>  {
>  #define N_RELOC(x) ((x) / sizeof(struct drm_i915_gem_relocation_entry))
>         struct drm_i915_gem_relocation_entry stack[N_RELOC(512)];
>         struct drm_i915_gem_relocation_entry __user *urelocs;
> -       const struct drm_i915_gem_exec_object2 *entry = exec_entry(eb, vma);
> +       const struct drm_i915_gem_exec_object2 *entry = ev->exec;
>         unsigned int remain;
>
>         urelocs = u64_to_user_ptr(entry->relocs_ptr);
> @@ -1515,7 +1509,7 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct i915_vma *vma)
>
>                 remain -= count;
>                 do {
> -                       u64 offset = eb_relocate_entry(eb, vma, r);
> +                       u64 offset = eb_relocate_entry(eb, ev, r);
>
>                         if (likely(offset == 0)) {
>                         } else if ((s64)offset < 0) {
> @@ -1558,16 +1552,16 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct i915_vma *vma)
>  }
>
>  static int
> -eb_relocate_vma_slow(struct i915_execbuffer *eb, struct i915_vma *vma)
> +eb_relocate_vma_slow(struct i915_execbuffer *eb, struct eb_vma *ev)
>  {
> -       const struct drm_i915_gem_exec_object2 *entry = exec_entry(eb, vma);
> +       const struct drm_i915_gem_exec_object2 *entry = ev->exec;
>         struct drm_i915_gem_relocation_entry *relocs =
>                 u64_to_ptr(typeof(*relocs), entry->relocs_ptr);
>         unsigned int i;
>         int err;
>
>         for (i = 0; i < entry->relocation_count; i++) {
> -               u64 offset = eb_relocate_entry(eb, vma, &relocs[i]);
> +               u64 offset = eb_relocate_entry(eb, ev, &relocs[i]);
>
>                 if ((s64)offset < 0) {
>                         err = (int)offset;
> @@ -1711,7 +1705,7 @@ static noinline int eb_relocate_slow(struct i915_execbuffer *eb)
>  {
>         struct drm_device *dev = &eb->i915->drm;
>         bool have_copy = false;
> -       struct i915_vma *vma;
> +       struct eb_vma *ev;
>         int err = 0;
>
>  repeat:
> @@ -1767,15 +1761,15 @@ static noinline int eb_relocate_slow(struct i915_execbuffer *eb)
>
>         GEM_BUG_ON(!eb->batch);
>
> -       list_for_each_entry(vma, &eb->relocs, reloc_link) {
> +       list_for_each_entry(ev, &eb->relocs, reloc_link) {
>                 if (!have_copy) {
>                         pagefault_disable();
> -                       err = eb_relocate_vma(eb, vma);
> +                       err = eb_relocate_vma(eb, ev);
>                         pagefault_enable();
>                         if (err)
>                                 goto repeat;
>                 } else {
> -                       err = eb_relocate_vma_slow(eb, vma);
> +                       err = eb_relocate_vma_slow(eb, ev);
>                         if (err)
>                                 goto err;
>                 }
> @@ -1820,10 +1814,10 @@ static int eb_relocate(struct i915_execbuffer *eb)
>
>         /* The objects are in their final locations, apply the relocations. */
>         if (eb->args->flags & __EXEC_HAS_RELOC) {
> -               struct i915_vma *vma;
> +               struct eb_vma *ev;
>
> -               list_for_each_entry(vma, &eb->relocs, reloc_link) {
> -                       if (eb_relocate_vma(eb, vma))
> +               list_for_each_entry(ev, &eb->relocs, reloc_link) {
> +                       if (eb_relocate_vma(eb, ev))
>                                 goto slow;
>                 }
>         }
> @@ -1844,39 +1838,34 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
>         ww_acquire_init(&acquire, &reservation_ww_class);
>
>         for (i = 0; i < count; i++) {
> -               struct i915_vma *vma = eb->vma[i];
> +               struct eb_vma *ev = &eb->vma[i];
> +               struct i915_vma *vma = ev->vma;
>
>                 err = ww_mutex_lock_interruptible(&vma->resv->lock, &acquire);
> -               if (!err)
> -                       continue;
> -
> -               GEM_BUG_ON(err == -EALREADY); /* No duplicate vma */
> -
>                 if (err == -EDEADLK) {
>                         GEM_BUG_ON(i == 0);
>                         do {
>                                 int j = i - 1;
>
> -                               ww_mutex_unlock(&eb->vma[j]->resv->lock);
> +                               ww_mutex_unlock(&eb->vma[j].vma->resv->lock);
>
> -                               swap(eb->flags[i], eb->flags[j]);
>                                 swap(eb->vma[i],  eb->vma[j]);
> -                               eb->vma[i]->exec_flags = &eb->flags[i];
>                         } while (--i);
> -                       GEM_BUG_ON(vma != eb->vma[0]);
> -                       vma->exec_flags = &eb->flags[0];
>
>                         err = ww_mutex_lock_slow_interruptible(&vma->resv->lock,
>                                                                &acquire);
>                 }
> +               if (err == -EALREADY)
> +                       err = 0;
>                 if (err)
>                         break;
>         }
>         ww_acquire_done(&acquire);
>
>         while (i--) {
> -               unsigned int flags = eb->flags[i];
> -               struct i915_vma *vma = eb->vma[i];
> +               struct eb_vma *ev = &eb->vma[i];
> +               struct i915_vma *vma = ev->vma;
> +               unsigned int flags = ev->flags;
>                 struct drm_i915_gem_object *obj = vma->obj;
>
>                 assert_vma_held(vma);
> @@ -1920,10 +1909,10 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
>                 i915_vma_unlock(vma);
>
>                 __eb_unreserve_vma(vma, flags);
> -               vma->exec_flags = NULL;
> -
>                 if (unlikely(flags & __EXEC_OBJECT_HAS_REF))
>                         i915_vma_put(vma);
> +
> +               ev->vma = NULL;
>         }
>         ww_acquire_fini(&acquire);
>
> @@ -2001,7 +1990,7 @@ static struct i915_vma *eb_parse(struct i915_execbuffer *eb, bool is_master)
>                 return ERR_CAST(pool);
>
>         err = intel_engine_cmd_parser(eb->engine,
> -                                     eb->batch->obj,
> +                                     eb->batch->vma->obj,
>                                       pool->obj,
>                                       eb->batch_start_offset,
>                                       eb->batch_len,
> @@ -2018,10 +2007,9 @@ static struct i915_vma *eb_parse(struct i915_execbuffer *eb, bool is_master)
>         if (IS_ERR(vma))
>                 goto err;
>
> -       eb->vma[eb->buffer_count] = i915_vma_get(vma);
> -       eb->flags[eb->buffer_count] =
> +       eb->vma[eb->buffer_count].vma = i915_vma_get(vma);
> +       eb->vma[eb->buffer_count].flags =
>                 __EXEC_OBJECT_HAS_PIN | __EXEC_OBJECT_HAS_REF;
> -       vma->exec_flags = &eb->flags[eb->buffer_count];
>         eb->buffer_count++;
>
>         vma->private = pool;
> @@ -2044,7 +2032,7 @@ add_to_client(struct i915_request *rq, struct drm_file *file)
>         spin_unlock(&file_priv->mm.lock);
>  }
>
> -static int eb_submit(struct i915_execbuffer *eb)
> +static int eb_submit(struct i915_execbuffer *eb, struct i915_vma *batch)
>  {
>         int err;
>
> @@ -2071,7 +2059,7 @@ static int eb_submit(struct i915_execbuffer *eb)
>         }
>
>         err = eb->engine->emit_bb_start(eb->request,
> -                                       eb->batch->node.start +
> +                                       batch->node.start +
>                                         eb->batch_start_offset,
>                                         eb->batch_len,
>                                         eb->batch_flags);
> @@ -2434,6 +2422,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>         struct dma_fence *in_fence = NULL;
>         struct dma_fence *exec_fence = NULL;
>         struct sync_file *out_fence = NULL;
> +       struct i915_vma *batch;
>         int out_fence_fd = -1;
>         int err;
>
> @@ -2448,9 +2437,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>                 args->flags |= __EXEC_HAS_RELOC;
>
>         eb.exec = exec;
> -       eb.vma = (struct i915_vma **)(exec + args->buffer_count + 1);
> -       eb.vma[0] = NULL;
> -       eb.flags = (unsigned int *)(eb.vma + args->buffer_count + 1);
> +       eb.vma = (struct eb_vma *)(exec + args->buffer_count + 1);
> +       eb.vma[0].vma = NULL;
>
>         eb.invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS;
>         reloc_cache_init(&eb.reloc_cache, eb.i915);
> @@ -2527,13 +2515,16 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>                 goto err_vma;
>         }
>
> -       if (unlikely(*eb.batch->exec_flags & EXEC_OBJECT_WRITE)) {
> +       if (unlikely(eb.batch->flags & EXEC_OBJECT_WRITE)) {
>                 DRM_DEBUG("Attempting to use self-modifying batch buffer\n");
>                 err = -EINVAL;
>                 goto err_vma;
>         }
> -       if (eb.batch_start_offset > eb.batch->size ||
> -           eb.batch_len > eb.batch->size - eb.batch_start_offset) {
> +
> +       batch = eb.batch->vma;
> +       if (range_overflows_t(u64,
> +                             eb.batch_start_offset, eb.batch_len,
> +                             batch->size)) {
>                 DRM_DEBUG("Attempting to use out-of-bounds batch\n");
>                 err = -EINVAL;
>                 goto err_vma;
> @@ -2560,12 +2551,12 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>                          */
>                         eb.batch_flags |= I915_DISPATCH_SECURE;
>                         eb.batch_start_offset = 0;
> -                       eb.batch = vma;
> +                       batch = vma;
>                 }
>         }
>
>         if (eb.batch_len == 0)
> -               eb.batch_len = eb.batch->size - eb.batch_start_offset;
> +               eb.batch_len = batch->size - eb.batch_start_offset;
>
>         /*
>          * snb/ivb/vlv conflate the "batch in ppgtt" bit with the "non-secure
> @@ -2584,13 +2575,13 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>                  *   fitting due to fragmentation.
>                  * So this is actually safe.
>                  */
> -               vma = i915_gem_object_ggtt_pin(eb.batch->obj, NULL, 0, 0, 0);
> +               vma = i915_gem_object_ggtt_pin(batch->obj, NULL, 0, 0, 0);
>                 if (IS_ERR(vma)) {
>                         err = PTR_ERR(vma);
>                         goto err_vma;
>                 }
>
> -               eb.batch = vma;
> +               batch = vma;
>         }
>
>         /* All GPU relocation batches must be submitted prior to the user rq */
> @@ -2637,12 +2628,12 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>          * inactive_list and lose its active reference. Hence we do not need
>          * to explicitly hold another reference here.
>          */
> -       eb.request->batch = eb.batch;
> -       if (eb.batch->private)
> -               intel_engine_pool_mark_active(eb.batch->private, eb.request);
> +       eb.request->batch = batch;
> +       if (batch->private)
> +               intel_engine_pool_mark_active(batch->private, eb.request);
>
>         trace_i915_request_queue(eb.request, eb.batch_flags);
> -       err = eb_submit(&eb);
> +       err = eb_submit(&eb, batch);
>  err_request:
>         add_to_client(eb.request, file);
>         i915_request_add(eb.request);
> @@ -2663,9 +2654,9 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>
>  err_batch_unpin:
>         if (eb.batch_flags & I915_DISPATCH_SECURE)
> -               i915_vma_unpin(eb.batch);
> -       if (eb.batch->private)
> -               intel_engine_pool_put(eb.batch->private);
> +               i915_vma_unpin(batch);
> +       if (batch->private)
> +               intel_engine_pool_put(batch->private);
>  err_vma:
>         if (eb.exec)
>                 eb_release_vmas(&eb);
> @@ -2688,9 +2679,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>
>  static size_t eb_element_size(void)
>  {
> -       return (sizeof(struct drm_i915_gem_exec_object2) +
> -               sizeof(struct i915_vma *) +
> -               sizeof(unsigned int));
> +       return sizeof(struct drm_i915_gem_exec_object2) + sizeof(struct eb_vma);
>  }
>
>  static bool check_buffer_count(size_t count)
> diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
> index 465932813bc5..71402056d846 100644
> --- a/drivers/gpu/drm/i915/i915_vma.h
> +++ b/drivers/gpu/drm/i915/i915_vma.h
> @@ -147,21 +147,10 @@ struct i915_vma {
>         struct rb_node obj_node;
>         struct hlist_node obj_hash;
>
> -       /** This vma's place in the execbuf reservation list */
> -       struct list_head exec_link;
> -       struct list_head reloc_link;
> -
>         /** This vma's place in the eviction list */
>         struct list_head evict_link;
>
>         struct list_head closed_link;
> -
> -       /**
> -        * Used for performing relocations during execbuffer insertion.
> -        */
> -       unsigned int *exec_flags;
> -       struct hlist_node exec_node;
> -       u32 exec_handle;
>  };
>
>  struct i915_vma *
> --
> 2.24.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [Intel-gfx] [PATCH 3/3] drm/i915/gem: Extract transient execbuf flags from i915_vma
@ 2019-11-08  9:53     ` Daniel Vetter
  0 siblings, 0 replies; 30+ messages in thread
From: Daniel Vetter @ 2019-11-08  9:53 UTC (permalink / raw)
  To: Chris Wilson, Maarten Lankhorst; +Cc: intel-gfx

On Wed, Nov 6, 2019 at 4:48 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> For our convenience, and to avoid frequent allocations, we placed some
> lists we use for execbuf inside the common i915_vma struct. As we look
> to parallelise execbuf, such fields guarded by the struct_mutex BKL must
> be pulled under local control. Instead of using the i915_vma as our
> primary means of tracking the user's list of objects and their virtual
> mappings, we use a local eb_vma with the same lists as before (just now
> local not global).
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

(--supress-cc ? Added Maarten for real, didn't seem to be on cc in my
inbox at least)

Why do we need this?

For the lmem stuff we need to hold the dma_resv, which is on the
object. Needs to be on the object, since lmem is physical memory, not
virtual gtt (we might have a different story on old ggtt execbuf
contention lols). vma->obj is N:1 (unless some remapping stuff crept
in that I totally missed), so locking the obj dma_resv ww_mutex during
execbuf prep also gives us the entire vma locked. Which means we can
just leave everything where it is, no need to move stuff. At least no
for the s/struct_mutex/ww_acquire_ctx/ magic trick.

I'm also kinda wondered whether the same applies for some of the other
vma state that got split out already, like the vma->pin_count
tracking. At least naive application of ww_mutex would cover ww_mutex
everywhere we do the new locking for vma pinning, so seems superflous.

But good chances I'm just slightly confused as usual :-)
-Daniel

> ---
>  .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 271 +++++++++---------
>  drivers/gpu/drm/i915/i915_vma.h               |  11 -
>  2 files changed, 130 insertions(+), 152 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index e4f5c269150a..8eb9c4e17514 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -27,6 +27,19 @@
>  #include "i915_gem_ioctls.h"
>  #include "i915_trace.h"
>
> +struct eb_vma {
> +       struct i915_vma *vma;
> +       unsigned int flags;
> +
> +       /** This vma's place in the execbuf reservation list */
> +       struct drm_i915_gem_exec_object2 *exec;
> +       struct list_head bind_link;
> +       struct list_head reloc_link;
> +
> +       struct hlist_node node;
> +       u32 handle;
> +};
> +
>  enum {
>         FORCE_CPU_RELOC = 1,
>         FORCE_GTT_RELOC,
> @@ -219,15 +232,14 @@ struct i915_execbuffer {
>         struct drm_file *file; /** per-file lookup tables and limits */
>         struct drm_i915_gem_execbuffer2 *args; /** ioctl parameters */
>         struct drm_i915_gem_exec_object2 *exec; /** ioctl execobj[] */
> -       struct i915_vma **vma;
> -       unsigned int *flags;
> +       struct eb_vma *vma;
>
>         struct intel_engine_cs *engine; /** engine to queue the request to */
>         struct intel_context *context; /* logical state for the request */
>         struct i915_gem_context *gem_context; /** caller's context */
>
>         struct i915_request *request; /** our request to build */
> -       struct i915_vma *batch; /** identity of the batch obj/vma */
> +       struct eb_vma *batch; /** identity of the batch obj/vma */
>
>         /** actual size of execobj[] as we may extend it for the cmdparser */
>         unsigned int buffer_count;
> @@ -275,8 +287,6 @@ struct i915_execbuffer {
>         struct hlist_head *buckets; /** ht for relocation handles */
>  };
>
> -#define exec_entry(EB, VMA) (&(EB)->exec[(VMA)->exec_flags - (EB)->flags])
> -
>  /*
>   * Used to convert any address to canonical form.
>   * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
> @@ -380,9 +390,9 @@ eb_vma_misplaced(const struct drm_i915_gem_exec_object2 *entry,
>  static inline bool
>  eb_pin_vma(struct i915_execbuffer *eb,
>            const struct drm_i915_gem_exec_object2 *entry,
> -          struct i915_vma *vma)
> +          struct eb_vma *ev)
>  {
> -       unsigned int exec_flags = *vma->exec_flags;
> +       struct i915_vma *vma = ev->vma;
>         u64 pin_flags;
>
>         if (vma->node.size)
> @@ -391,24 +401,24 @@ eb_pin_vma(struct i915_execbuffer *eb,
>                 pin_flags = entry->offset & PIN_OFFSET_MASK;
>
>         pin_flags |= PIN_USER | PIN_NOEVICT | PIN_OFFSET_FIXED;
> -       if (unlikely(exec_flags & EXEC_OBJECT_NEEDS_GTT))
> +       if (unlikely(ev->flags & EXEC_OBJECT_NEEDS_GTT))
>                 pin_flags |= PIN_GLOBAL;
>
>         if (unlikely(i915_vma_pin(vma, 0, 0, pin_flags)))
>                 return false;
>
> -       if (unlikely(exec_flags & EXEC_OBJECT_NEEDS_FENCE)) {
> +       if (unlikely(ev->flags & EXEC_OBJECT_NEEDS_FENCE)) {
>                 if (unlikely(i915_vma_pin_fence(vma))) {
>                         i915_vma_unpin(vma);
>                         return false;
>                 }
>
>                 if (vma->fence)
> -                       exec_flags |= __EXEC_OBJECT_HAS_FENCE;
> +                       ev->flags |= __EXEC_OBJECT_HAS_FENCE;
>         }
>
> -       *vma->exec_flags = exec_flags | __EXEC_OBJECT_HAS_PIN;
> -       return !eb_vma_misplaced(entry, vma, exec_flags);
> +       ev->flags |= __EXEC_OBJECT_HAS_PIN;
> +       return !eb_vma_misplaced(entry, vma, ev->flags);
>  }
>
>  static inline void __eb_unreserve_vma(struct i915_vma *vma, unsigned int flags)
> @@ -422,13 +432,13 @@ static inline void __eb_unreserve_vma(struct i915_vma *vma, unsigned int flags)
>  }
>
>  static inline void
> -eb_unreserve_vma(struct i915_vma *vma, unsigned int *flags)
> +eb_unreserve_vma(struct eb_vma *ev)
>  {
> -       if (!(*flags & __EXEC_OBJECT_HAS_PIN))
> +       if (!(ev->flags & __EXEC_OBJECT_HAS_PIN))
>                 return;
>
> -       __eb_unreserve_vma(vma, *flags);
> -       *flags &= ~__EXEC_OBJECT_RESERVED;
> +       __eb_unreserve_vma(ev->vma, ev->flags);
> +       ev->flags &= ~__EXEC_OBJECT_RESERVED;
>  }
>
>  static int
> @@ -458,12 +468,6 @@ eb_validate_vma(struct i915_execbuffer *eb,
>                 entry->pad_to_size = 0;
>         }
>
> -       if (unlikely(vma->exec_flags)) {
> -               DRM_DEBUG("Object [handle %d, index %d] appears more than once in object list\n",
> -                         entry->handle, (int)(entry - eb->exec));
> -               return -EINVAL;
> -       }
> -
>         /*
>          * From drm_mm perspective address space is continuous,
>          * so from this point we're always using non-canonical
> @@ -492,6 +496,7 @@ eb_add_vma(struct i915_execbuffer *eb,
>            struct i915_vma *vma)
>  {
>         struct drm_i915_gem_exec_object2 *entry = &eb->exec[i];
> +       struct eb_vma *ev = &eb->vma[i];
>         int err;
>
>         GEM_BUG_ON(i915_vma_is_closed(vma));
> @@ -502,25 +507,19 @@ eb_add_vma(struct i915_execbuffer *eb,
>                         return err;
>         }
>
> +       ev->vma = vma;
> +       ev->exec = entry;
> +       ev->flags = entry->flags;
> +
>         if (eb->lut_size > 0) {
> -               vma->exec_handle = entry->handle;
> -               hlist_add_head(&vma->exec_node,
> +               ev->handle = entry->handle;
> +               hlist_add_head(&ev->node,
>                                &eb->buckets[hash_32(entry->handle,
>                                                     eb->lut_size)]);
>         }
>
>         if (entry->relocation_count)
> -               list_add_tail(&vma->reloc_link, &eb->relocs);
> -
> -       /*
> -        * Stash a pointer from the vma to execobj, so we can query its flags,
> -        * size, alignment etc as provided by the user. Also we stash a pointer
> -        * to the vma inside the execobj so that we can use a direct lookup
> -        * to find the right target VMA when doing relocations.
> -        */
> -       eb->vma[i] = vma;
> -       eb->flags[i] = entry->flags;
> -       vma->exec_flags = &eb->flags[i];
> +               list_add_tail(&ev->reloc_link, &eb->relocs);
>
>         /*
>          * SNA is doing fancy tricks with compressing batch buffers, which leads
> @@ -533,28 +532,26 @@ eb_add_vma(struct i915_execbuffer *eb,
>          */
>         if (i == batch_idx) {
>                 if (entry->relocation_count &&
> -                   !(eb->flags[i] & EXEC_OBJECT_PINNED))
> -                       eb->flags[i] |= __EXEC_OBJECT_NEEDS_BIAS;
> +                   !(ev->flags & EXEC_OBJECT_PINNED))
> +                       ev->flags |= __EXEC_OBJECT_NEEDS_BIAS;
>                 if (eb->reloc_cache.has_fence)
> -                       eb->flags[i] |= EXEC_OBJECT_NEEDS_FENCE;
> +                       ev->flags |= EXEC_OBJECT_NEEDS_FENCE;
>
> -               eb->batch = vma;
> +               eb->batch = ev;
>         }
>
>         err = 0;
> -       if (eb_pin_vma(eb, entry, 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(vma, vma->exec_flags);
> +               eb_unreserve_vma(ev);
>
> -               list_add_tail(&vma->exec_link, &eb->unbound);
> +               list_add_tail(&ev->bind_link, &eb->unbound);
>                 if (drm_mm_node_allocated(&vma->node))
>                         err = i915_vma_unbind(vma);
> -               if (unlikely(err))
> -                       vma->exec_flags = NULL;
>         }
>         return err;
>  }
> @@ -576,11 +573,11 @@ static inline int use_cpu_reloc(const struct reloc_cache *cache,
>                 obj->cache_level != I915_CACHE_NONE);
>  }
>
> -static int eb_reserve_vma(const struct i915_execbuffer *eb,
> -                         struct i915_vma *vma)
> +static int eb_reserve_vma(const struct i915_execbuffer *eb, struct eb_vma *ev)
>  {
> -       struct drm_i915_gem_exec_object2 *entry = exec_entry(eb, vma);
> -       unsigned int exec_flags = *vma->exec_flags;
> +       struct drm_i915_gem_exec_object2 *entry = ev->exec;
> +       unsigned int exec_flags = ev->flags;
> +       struct i915_vma *vma = ev->vma;
>         u64 pin_flags;
>         int err;
>
> @@ -627,8 +624,8 @@ static int eb_reserve_vma(const struct i915_execbuffer *eb,
>                         exec_flags |= __EXEC_OBJECT_HAS_FENCE;
>         }
>
> -       *vma->exec_flags = exec_flags | __EXEC_OBJECT_HAS_PIN;
> -       GEM_BUG_ON(eb_vma_misplaced(entry, vma, exec_flags));
> +       ev->flags = exec_flags | __EXEC_OBJECT_HAS_PIN;
> +       GEM_BUG_ON(eb_vma_misplaced(entry, vma, ev->flags));
>
>         return 0;
>  }
> @@ -637,7 +634,7 @@ static int eb_reserve(struct i915_execbuffer *eb)
>  {
>         const unsigned int count = eb->buffer_count;
>         struct list_head last;
> -       struct i915_vma *vma;
> +       struct eb_vma *ev;
>         unsigned int i, pass;
>         int err;
>
> @@ -658,8 +655,8 @@ static int eb_reserve(struct i915_execbuffer *eb)
>         pass = 0;
>         err = 0;
>         do {
> -               list_for_each_entry(vma, &eb->unbound, exec_link) {
> -                       err = eb_reserve_vma(eb, vma);
> +               list_for_each_entry(ev, &eb->unbound, bind_link) {
> +                       err = eb_reserve_vma(eb, ev);
>                         if (err)
>                                 break;
>                 }
> @@ -670,26 +667,26 @@ static int eb_reserve(struct i915_execbuffer *eb)
>                 INIT_LIST_HEAD(&eb->unbound);
>                 INIT_LIST_HEAD(&last);
>                 for (i = 0; i < count; i++) {
> -                       unsigned int flags = eb->flags[i];
> -                       struct i915_vma *vma = eb->vma[i];
> +                       struct eb_vma *ev = &eb->vma[i];
> +                       unsigned int flags = ev->flags;
>
>                         if (flags & EXEC_OBJECT_PINNED &&
>                             flags & __EXEC_OBJECT_HAS_PIN)
>                                 continue;
>
> -                       eb_unreserve_vma(vma, &eb->flags[i]);
> +                       eb_unreserve_vma(ev);
>
>                         if (flags & EXEC_OBJECT_PINNED)
>                                 /* Pinned must have their slot */
> -                               list_add(&vma->exec_link, &eb->unbound);
> +                               list_add(&ev->bind_link, &eb->unbound);
>                         else if (flags & __EXEC_OBJECT_NEEDS_MAP)
>                                 /* Map require the lowest 256MiB (aperture) */
> -                               list_add_tail(&vma->exec_link, &eb->unbound);
> +                               list_add_tail(&ev->bind_link, &eb->unbound);
>                         else if (!(flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS))
>                                 /* Prioritise 4GiB region for restricted bo */
> -                               list_add(&vma->exec_link, &last);
> +                               list_add(&ev->bind_link, &last);
>                         else
> -                               list_add_tail(&vma->exec_link, &last);
> +                               list_add_tail(&ev->bind_link, &last);
>                 }
>                 list_splice_tail(&last, &eb->unbound);
>
> @@ -808,10 +805,8 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
>                 if (unlikely(err))
>                         goto err_vma;
>
> -               GEM_BUG_ON(vma != eb->vma[i]);
> -               GEM_BUG_ON(vma->exec_flags != &eb->flags[i]);
>                 GEM_BUG_ON(drm_mm_node_allocated(&vma->node) &&
> -                          eb_vma_misplaced(&eb->exec[i], vma, eb->flags[i]));
> +                          eb_vma_misplaced(&eb->exec[i], vma, eb->vma[i].flags));
>         }
>
>         mutex_unlock(&eb->gem_context->mutex);
> @@ -822,27 +817,27 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
>  err_obj:
>         i915_gem_object_put(obj);
>  err_vma:
> -       eb->vma[i] = NULL;
> +       eb->vma[i].vma = NULL;
>  err_ctx:
>         mutex_unlock(&eb->gem_context->mutex);
>         return err;
>  }
>
> -static struct i915_vma *
> +static struct eb_vma *
>  eb_get_vma(const struct i915_execbuffer *eb, unsigned long handle)
>  {
>         if (eb->lut_size < 0) {
>                 if (handle >= -eb->lut_size)
>                         return NULL;
> -               return eb->vma[handle];
> +               return &eb->vma[handle];
>         } else {
>                 struct hlist_head *head;
> -               struct i915_vma *vma;
> +               struct eb_vma *ev;
>
>                 head = &eb->buckets[hash_32(handle, eb->lut_size)];
> -               hlist_for_each_entry(vma, head, exec_node) {
> -                       if (vma->exec_handle == handle)
> -                               return vma;
> +               hlist_for_each_entry(ev, head, node) {
> +                       if (ev->handle == handle)
> +                               return ev;
>                 }
>                 return NULL;
>         }
> @@ -854,20 +849,18 @@ static void eb_release_vmas(const struct i915_execbuffer *eb)
>         unsigned int i;
>
>         for (i = 0; i < count; i++) {
> -               struct i915_vma *vma = eb->vma[i];
> -               unsigned int flags = eb->flags[i];
> +               struct eb_vma *ev = &eb->vma[i];
> +               struct i915_vma *vma = ev->vma;
>
>                 if (!vma)
>                         break;
>
> -               GEM_BUG_ON(vma->exec_flags != &eb->flags[i]);
> -               vma->exec_flags = NULL;
> -               eb->vma[i] = NULL;
> +               eb->vma[i].vma = NULL;
>
> -               if (flags & __EXEC_OBJECT_HAS_PIN)
> -                       __eb_unreserve_vma(vma, flags);
> +               if (ev->flags & __EXEC_OBJECT_HAS_PIN)
> +                       __eb_unreserve_vma(vma, ev->flags);
>
> -               if (flags & __EXEC_OBJECT_HAS_REF)
> +               if (ev->flags & __EXEC_OBJECT_HAS_REF)
>                         i915_vma_put(vma);
>         }
>  }
> @@ -1377,10 +1370,10 @@ relocate_entry(struct i915_vma *vma,
>
>  static u64
>  eb_relocate_entry(struct i915_execbuffer *eb,
> -                 struct i915_vma *vma,
> +                 struct eb_vma *ev,
>                   const struct drm_i915_gem_relocation_entry *reloc)
>  {
> -       struct i915_vma *target;
> +       struct eb_vma *target;
>         int err;
>
>         /* we've already hold a reference to all valid objects */
> @@ -1412,7 +1405,7 @@ eb_relocate_entry(struct i915_execbuffer *eb,
>         }
>
>         if (reloc->write_domain) {
> -               *target->exec_flags |= EXEC_OBJECT_WRITE;
> +               target->flags |= EXEC_OBJECT_WRITE;
>
>                 /*
>                  * Sandybridge PPGTT errata: We need a global gtt mapping
> @@ -1422,7 +1415,8 @@ eb_relocate_entry(struct i915_execbuffer *eb,
>                  */
>                 if (reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION &&
>                     IS_GEN(eb->i915, 6)) {
> -                       err = i915_vma_bind(target, target->obj->cache_level,
> +                       err = i915_vma_bind(target->vma,
> +                                           target->vma->obj->cache_level,
>                                             PIN_GLOBAL, NULL);
>                         if (WARN_ONCE(err,
>                                       "Unexpected failure to bind target VMA!"))
> @@ -1435,17 +1429,17 @@ eb_relocate_entry(struct i915_execbuffer *eb,
>          * more work needs to be done.
>          */
>         if (!DBG_FORCE_RELOC &&
> -           gen8_canonical_addr(target->node.start) == reloc->presumed_offset)
> +           gen8_canonical_addr(target->vma->node.start) == reloc->presumed_offset)
>                 return 0;
>
>         /* Check that the relocation address is valid... */
>         if (unlikely(reloc->offset >
> -                    vma->size - (eb->reloc_cache.use_64bit_reloc ? 8 : 4))) {
> +                    ev->vma->size - (eb->reloc_cache.use_64bit_reloc ? 8 : 4))) {
>                 DRM_DEBUG("Relocation beyond object bounds: "
>                           "target %d offset %d size %d.\n",
>                           reloc->target_handle,
>                           (int)reloc->offset,
> -                         (int)vma->size);
> +                         (int)ev->vma->size);
>                 return -EINVAL;
>         }
>         if (unlikely(reloc->offset & 3)) {
> @@ -1464,18 +1458,18 @@ eb_relocate_entry(struct i915_execbuffer *eb,
>          * do relocations we are already stalling, disable the user's opt
>          * out of our synchronisation.
>          */
> -       *vma->exec_flags &= ~EXEC_OBJECT_ASYNC;
> +       ev->flags &= ~EXEC_OBJECT_ASYNC;
>
>         /* and update the user's relocation entry */
> -       return relocate_entry(vma, reloc, eb, target);
> +       return relocate_entry(ev->vma, reloc, eb, target->vma);
>  }
>
> -static int eb_relocate_vma(struct i915_execbuffer *eb, struct i915_vma *vma)
> +static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev)
>  {
>  #define N_RELOC(x) ((x) / sizeof(struct drm_i915_gem_relocation_entry))
>         struct drm_i915_gem_relocation_entry stack[N_RELOC(512)];
>         struct drm_i915_gem_relocation_entry __user *urelocs;
> -       const struct drm_i915_gem_exec_object2 *entry = exec_entry(eb, vma);
> +       const struct drm_i915_gem_exec_object2 *entry = ev->exec;
>         unsigned int remain;
>
>         urelocs = u64_to_user_ptr(entry->relocs_ptr);
> @@ -1515,7 +1509,7 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct i915_vma *vma)
>
>                 remain -= count;
>                 do {
> -                       u64 offset = eb_relocate_entry(eb, vma, r);
> +                       u64 offset = eb_relocate_entry(eb, ev, r);
>
>                         if (likely(offset == 0)) {
>                         } else if ((s64)offset < 0) {
> @@ -1558,16 +1552,16 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct i915_vma *vma)
>  }
>
>  static int
> -eb_relocate_vma_slow(struct i915_execbuffer *eb, struct i915_vma *vma)
> +eb_relocate_vma_slow(struct i915_execbuffer *eb, struct eb_vma *ev)
>  {
> -       const struct drm_i915_gem_exec_object2 *entry = exec_entry(eb, vma);
> +       const struct drm_i915_gem_exec_object2 *entry = ev->exec;
>         struct drm_i915_gem_relocation_entry *relocs =
>                 u64_to_ptr(typeof(*relocs), entry->relocs_ptr);
>         unsigned int i;
>         int err;
>
>         for (i = 0; i < entry->relocation_count; i++) {
> -               u64 offset = eb_relocate_entry(eb, vma, &relocs[i]);
> +               u64 offset = eb_relocate_entry(eb, ev, &relocs[i]);
>
>                 if ((s64)offset < 0) {
>                         err = (int)offset;
> @@ -1711,7 +1705,7 @@ static noinline int eb_relocate_slow(struct i915_execbuffer *eb)
>  {
>         struct drm_device *dev = &eb->i915->drm;
>         bool have_copy = false;
> -       struct i915_vma *vma;
> +       struct eb_vma *ev;
>         int err = 0;
>
>  repeat:
> @@ -1767,15 +1761,15 @@ static noinline int eb_relocate_slow(struct i915_execbuffer *eb)
>
>         GEM_BUG_ON(!eb->batch);
>
> -       list_for_each_entry(vma, &eb->relocs, reloc_link) {
> +       list_for_each_entry(ev, &eb->relocs, reloc_link) {
>                 if (!have_copy) {
>                         pagefault_disable();
> -                       err = eb_relocate_vma(eb, vma);
> +                       err = eb_relocate_vma(eb, ev);
>                         pagefault_enable();
>                         if (err)
>                                 goto repeat;
>                 } else {
> -                       err = eb_relocate_vma_slow(eb, vma);
> +                       err = eb_relocate_vma_slow(eb, ev);
>                         if (err)
>                                 goto err;
>                 }
> @@ -1820,10 +1814,10 @@ static int eb_relocate(struct i915_execbuffer *eb)
>
>         /* The objects are in their final locations, apply the relocations. */
>         if (eb->args->flags & __EXEC_HAS_RELOC) {
> -               struct i915_vma *vma;
> +               struct eb_vma *ev;
>
> -               list_for_each_entry(vma, &eb->relocs, reloc_link) {
> -                       if (eb_relocate_vma(eb, vma))
> +               list_for_each_entry(ev, &eb->relocs, reloc_link) {
> +                       if (eb_relocate_vma(eb, ev))
>                                 goto slow;
>                 }
>         }
> @@ -1844,39 +1838,34 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
>         ww_acquire_init(&acquire, &reservation_ww_class);
>
>         for (i = 0; i < count; i++) {
> -               struct i915_vma *vma = eb->vma[i];
> +               struct eb_vma *ev = &eb->vma[i];
> +               struct i915_vma *vma = ev->vma;
>
>                 err = ww_mutex_lock_interruptible(&vma->resv->lock, &acquire);
> -               if (!err)
> -                       continue;
> -
> -               GEM_BUG_ON(err == -EALREADY); /* No duplicate vma */
> -
>                 if (err == -EDEADLK) {
>                         GEM_BUG_ON(i == 0);
>                         do {
>                                 int j = i - 1;
>
> -                               ww_mutex_unlock(&eb->vma[j]->resv->lock);
> +                               ww_mutex_unlock(&eb->vma[j].vma->resv->lock);
>
> -                               swap(eb->flags[i], eb->flags[j]);
>                                 swap(eb->vma[i],  eb->vma[j]);
> -                               eb->vma[i]->exec_flags = &eb->flags[i];
>                         } while (--i);
> -                       GEM_BUG_ON(vma != eb->vma[0]);
> -                       vma->exec_flags = &eb->flags[0];
>
>                         err = ww_mutex_lock_slow_interruptible(&vma->resv->lock,
>                                                                &acquire);
>                 }
> +               if (err == -EALREADY)
> +                       err = 0;
>                 if (err)
>                         break;
>         }
>         ww_acquire_done(&acquire);
>
>         while (i--) {
> -               unsigned int flags = eb->flags[i];
> -               struct i915_vma *vma = eb->vma[i];
> +               struct eb_vma *ev = &eb->vma[i];
> +               struct i915_vma *vma = ev->vma;
> +               unsigned int flags = ev->flags;
>                 struct drm_i915_gem_object *obj = vma->obj;
>
>                 assert_vma_held(vma);
> @@ -1920,10 +1909,10 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
>                 i915_vma_unlock(vma);
>
>                 __eb_unreserve_vma(vma, flags);
> -               vma->exec_flags = NULL;
> -
>                 if (unlikely(flags & __EXEC_OBJECT_HAS_REF))
>                         i915_vma_put(vma);
> +
> +               ev->vma = NULL;
>         }
>         ww_acquire_fini(&acquire);
>
> @@ -2001,7 +1990,7 @@ static struct i915_vma *eb_parse(struct i915_execbuffer *eb, bool is_master)
>                 return ERR_CAST(pool);
>
>         err = intel_engine_cmd_parser(eb->engine,
> -                                     eb->batch->obj,
> +                                     eb->batch->vma->obj,
>                                       pool->obj,
>                                       eb->batch_start_offset,
>                                       eb->batch_len,
> @@ -2018,10 +2007,9 @@ static struct i915_vma *eb_parse(struct i915_execbuffer *eb, bool is_master)
>         if (IS_ERR(vma))
>                 goto err;
>
> -       eb->vma[eb->buffer_count] = i915_vma_get(vma);
> -       eb->flags[eb->buffer_count] =
> +       eb->vma[eb->buffer_count].vma = i915_vma_get(vma);
> +       eb->vma[eb->buffer_count].flags =
>                 __EXEC_OBJECT_HAS_PIN | __EXEC_OBJECT_HAS_REF;
> -       vma->exec_flags = &eb->flags[eb->buffer_count];
>         eb->buffer_count++;
>
>         vma->private = pool;
> @@ -2044,7 +2032,7 @@ add_to_client(struct i915_request *rq, struct drm_file *file)
>         spin_unlock(&file_priv->mm.lock);
>  }
>
> -static int eb_submit(struct i915_execbuffer *eb)
> +static int eb_submit(struct i915_execbuffer *eb, struct i915_vma *batch)
>  {
>         int err;
>
> @@ -2071,7 +2059,7 @@ static int eb_submit(struct i915_execbuffer *eb)
>         }
>
>         err = eb->engine->emit_bb_start(eb->request,
> -                                       eb->batch->node.start +
> +                                       batch->node.start +
>                                         eb->batch_start_offset,
>                                         eb->batch_len,
>                                         eb->batch_flags);
> @@ -2434,6 +2422,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>         struct dma_fence *in_fence = NULL;
>         struct dma_fence *exec_fence = NULL;
>         struct sync_file *out_fence = NULL;
> +       struct i915_vma *batch;
>         int out_fence_fd = -1;
>         int err;
>
> @@ -2448,9 +2437,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>                 args->flags |= __EXEC_HAS_RELOC;
>
>         eb.exec = exec;
> -       eb.vma = (struct i915_vma **)(exec + args->buffer_count + 1);
> -       eb.vma[0] = NULL;
> -       eb.flags = (unsigned int *)(eb.vma + args->buffer_count + 1);
> +       eb.vma = (struct eb_vma *)(exec + args->buffer_count + 1);
> +       eb.vma[0].vma = NULL;
>
>         eb.invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS;
>         reloc_cache_init(&eb.reloc_cache, eb.i915);
> @@ -2527,13 +2515,16 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>                 goto err_vma;
>         }
>
> -       if (unlikely(*eb.batch->exec_flags & EXEC_OBJECT_WRITE)) {
> +       if (unlikely(eb.batch->flags & EXEC_OBJECT_WRITE)) {
>                 DRM_DEBUG("Attempting to use self-modifying batch buffer\n");
>                 err = -EINVAL;
>                 goto err_vma;
>         }
> -       if (eb.batch_start_offset > eb.batch->size ||
> -           eb.batch_len > eb.batch->size - eb.batch_start_offset) {
> +
> +       batch = eb.batch->vma;
> +       if (range_overflows_t(u64,
> +                             eb.batch_start_offset, eb.batch_len,
> +                             batch->size)) {
>                 DRM_DEBUG("Attempting to use out-of-bounds batch\n");
>                 err = -EINVAL;
>                 goto err_vma;
> @@ -2560,12 +2551,12 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>                          */
>                         eb.batch_flags |= I915_DISPATCH_SECURE;
>                         eb.batch_start_offset = 0;
> -                       eb.batch = vma;
> +                       batch = vma;
>                 }
>         }
>
>         if (eb.batch_len == 0)
> -               eb.batch_len = eb.batch->size - eb.batch_start_offset;
> +               eb.batch_len = batch->size - eb.batch_start_offset;
>
>         /*
>          * snb/ivb/vlv conflate the "batch in ppgtt" bit with the "non-secure
> @@ -2584,13 +2575,13 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>                  *   fitting due to fragmentation.
>                  * So this is actually safe.
>                  */
> -               vma = i915_gem_object_ggtt_pin(eb.batch->obj, NULL, 0, 0, 0);
> +               vma = i915_gem_object_ggtt_pin(batch->obj, NULL, 0, 0, 0);
>                 if (IS_ERR(vma)) {
>                         err = PTR_ERR(vma);
>                         goto err_vma;
>                 }
>
> -               eb.batch = vma;
> +               batch = vma;
>         }
>
>         /* All GPU relocation batches must be submitted prior to the user rq */
> @@ -2637,12 +2628,12 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>          * inactive_list and lose its active reference. Hence we do not need
>          * to explicitly hold another reference here.
>          */
> -       eb.request->batch = eb.batch;
> -       if (eb.batch->private)
> -               intel_engine_pool_mark_active(eb.batch->private, eb.request);
> +       eb.request->batch = batch;
> +       if (batch->private)
> +               intel_engine_pool_mark_active(batch->private, eb.request);
>
>         trace_i915_request_queue(eb.request, eb.batch_flags);
> -       err = eb_submit(&eb);
> +       err = eb_submit(&eb, batch);
>  err_request:
>         add_to_client(eb.request, file);
>         i915_request_add(eb.request);
> @@ -2663,9 +2654,9 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>
>  err_batch_unpin:
>         if (eb.batch_flags & I915_DISPATCH_SECURE)
> -               i915_vma_unpin(eb.batch);
> -       if (eb.batch->private)
> -               intel_engine_pool_put(eb.batch->private);
> +               i915_vma_unpin(batch);
> +       if (batch->private)
> +               intel_engine_pool_put(batch->private);
>  err_vma:
>         if (eb.exec)
>                 eb_release_vmas(&eb);
> @@ -2688,9 +2679,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>
>  static size_t eb_element_size(void)
>  {
> -       return (sizeof(struct drm_i915_gem_exec_object2) +
> -               sizeof(struct i915_vma *) +
> -               sizeof(unsigned int));
> +       return sizeof(struct drm_i915_gem_exec_object2) + sizeof(struct eb_vma);
>  }
>
>  static bool check_buffer_count(size_t count)
> diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
> index 465932813bc5..71402056d846 100644
> --- a/drivers/gpu/drm/i915/i915_vma.h
> +++ b/drivers/gpu/drm/i915/i915_vma.h
> @@ -147,21 +147,10 @@ struct i915_vma {
>         struct rb_node obj_node;
>         struct hlist_node obj_hash;
>
> -       /** This vma's place in the execbuf reservation list */
> -       struct list_head exec_link;
> -       struct list_head reloc_link;
> -
>         /** This vma's place in the eviction list */
>         struct list_head evict_link;
>
>         struct list_head closed_link;
> -
> -       /**
> -        * Used for performing relocations during execbuffer insertion.
> -        */
> -       unsigned int *exec_flags;
> -       struct hlist_node exec_node;
> -       u32 exec_handle;
>  };
>
>  struct i915_vma *
> --
> 2.24.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH 2/3] drm/i915: Drop inspection of execbuf flags during evict
@ 2019-11-08  9:54     ` Daniel Vetter
  0 siblings, 0 replies; 30+ messages in thread
From: Daniel Vetter @ 2019-11-08  9:54 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Wed, Nov 6, 2019 at 4:49 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> With the goal of removing the serialisation from around execbuf, we will
> no longer have the privilege of there being a single execbuf in flight
> at any time and so will only be able to inspect the user's flags within
> the carefully controlled execbuf context. i915_gem_evict_for_node() is
> the only user outside of execbuf that currently peeks at the flag to
> convert an overlapping softpinned request from ENOSPC to EINVAL. Retract
> this nicety and only report ENOSPC if the location is in current use,
> either due to this execbuf or another.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Same reasons as for patch 3, I don't think we have to do this at all.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem_evict.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> index 7e62c310290f..1395018c657a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> @@ -292,7 +292,8 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
>                 GEM_BUG_ON(!drm_mm_node_allocated(node));
>                 vma = container_of(node, typeof(*vma), node);
>
> -               /* If we are using coloring to insert guard pages between
> +               /*
> +                * If we are using coloring to insert guard pages between
>                  * different cache domains within the address space, we have
>                  * to check whether the objects on either side of our range
>                  * abutt and conflict. If they are in conflict, then we evict
> @@ -309,22 +310,18 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
>                         }
>                 }
>
> -               if (flags & PIN_NONBLOCK &&
> -                   (i915_vma_is_pinned(vma) || i915_vma_is_active(vma))) {
> +               if (i915_vma_is_pinned(vma)) {
>                         ret = -ENOSPC;
>                         break;
>                 }
>
> -               /* Overlap of objects in the same batch? */
> -               if (i915_vma_is_pinned(vma)) {
> +               if (flags & PIN_NONBLOCK && i915_vma_is_active(vma)) {
>                         ret = -ENOSPC;
> -                       if (vma->exec_flags &&
> -                           *vma->exec_flags & EXEC_OBJECT_PINNED)
> -                               ret = -EINVAL;
>                         break;
>                 }
>
> -               /* Never show fear in the face of dragons!
> +               /*
> +                * Never show fear in the face of dragons!
>                  *
>                  * We cannot directly remove this node from within this
>                  * iterator and as with i915_gem_evict_something() we employ
> --
> 2.24.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [Intel-gfx] [PATCH 2/3] drm/i915: Drop inspection of execbuf flags during evict
@ 2019-11-08  9:54     ` Daniel Vetter
  0 siblings, 0 replies; 30+ messages in thread
From: Daniel Vetter @ 2019-11-08  9:54 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Wed, Nov 6, 2019 at 4:49 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> With the goal of removing the serialisation from around execbuf, we will
> no longer have the privilege of there being a single execbuf in flight
> at any time and so will only be able to inspect the user's flags within
> the carefully controlled execbuf context. i915_gem_evict_for_node() is
> the only user outside of execbuf that currently peeks at the flag to
> convert an overlapping softpinned request from ENOSPC to EINVAL. Retract
> this nicety and only report ENOSPC if the location is in current use,
> either due to this execbuf or another.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Same reasons as for patch 3, I don't think we have to do this at all.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem_evict.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> index 7e62c310290f..1395018c657a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> @@ -292,7 +292,8 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
>                 GEM_BUG_ON(!drm_mm_node_allocated(node));
>                 vma = container_of(node, typeof(*vma), node);
>
> -               /* If we are using coloring to insert guard pages between
> +               /*
> +                * If we are using coloring to insert guard pages between
>                  * different cache domains within the address space, we have
>                  * to check whether the objects on either side of our range
>                  * abutt and conflict. If they are in conflict, then we evict
> @@ -309,22 +310,18 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
>                         }
>                 }
>
> -               if (flags & PIN_NONBLOCK &&
> -                   (i915_vma_is_pinned(vma) || i915_vma_is_active(vma))) {
> +               if (i915_vma_is_pinned(vma)) {
>                         ret = -ENOSPC;
>                         break;
>                 }
>
> -               /* Overlap of objects in the same batch? */
> -               if (i915_vma_is_pinned(vma)) {
> +               if (flags & PIN_NONBLOCK && i915_vma_is_active(vma)) {
>                         ret = -ENOSPC;
> -                       if (vma->exec_flags &&
> -                           *vma->exec_flags & EXEC_OBJECT_PINNED)
> -                               ret = -EINVAL;
>                         break;
>                 }
>
> -               /* Never show fear in the face of dragons!
> +               /*
> +                * Never show fear in the face of dragons!
>                  *
>                  * We cannot directly remove this node from within this
>                  * iterator and as with i915_gem_evict_something() we employ
> --
> 2.24.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH 3/3] drm/i915/gem: Extract transient execbuf flags from i915_vma
@ 2019-11-08 10:05       ` Chris Wilson
  0 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2019-11-08 10:05 UTC (permalink / raw)
  To: Daniel Vetter, Maarten Lankhorst; +Cc: intel-gfx

Quoting Daniel Vetter (2019-11-08 09:53:51)
> On Wed, Nov 6, 2019 at 4:48 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > For our convenience, and to avoid frequent allocations, we placed some
> > lists we use for execbuf inside the common i915_vma struct. As we look
> > to parallelise execbuf, such fields guarded by the struct_mutex BKL must
> > be pulled under local control. Instead of using the i915_vma as our
> > primary means of tracking the user's list of objects and their virtual
> > mappings, we use a local eb_vma with the same lists as before (just now
> > local not global).
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> 
> (--supress-cc ? Added Maarten for real, didn't seem to be on cc in my
> inbox at least)
> 
> Why do we need this?

Because the execbuf state is not covered by the object locks. We put
them inside the vma for the express purpose of avoiding an allocation; an
allocation we ended up doing anyway, which we can now use for the sole
purpose of tracking the execbuf.

This is _execbuf_ state that we rammed into the i915_vma.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [Intel-gfx] [PATCH 3/3] drm/i915/gem: Extract transient execbuf flags from i915_vma
@ 2019-11-08 10:05       ` Chris Wilson
  0 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2019-11-08 10:05 UTC (permalink / raw)
  To: Daniel Vetter, Maarten Lankhorst; +Cc: intel-gfx

Quoting Daniel Vetter (2019-11-08 09:53:51)
> On Wed, Nov 6, 2019 at 4:48 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > For our convenience, and to avoid frequent allocations, we placed some
> > lists we use for execbuf inside the common i915_vma struct. As we look
> > to parallelise execbuf, such fields guarded by the struct_mutex BKL must
> > be pulled under local control. Instead of using the i915_vma as our
> > primary means of tracking the user's list of objects and their virtual
> > mappings, we use a local eb_vma with the same lists as before (just now
> > local not global).
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> 
> (--supress-cc ? Added Maarten for real, didn't seem to be on cc in my
> inbox at least)
> 
> Why do we need this?

Because the execbuf state is not covered by the object locks. We put
them inside the vma for the express purpose of avoiding an allocation; an
allocation we ended up doing anyway, which we can now use for the sole
purpose of tracking the execbuf.

This is _execbuf_ state that we rammed into the i915_vma.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH 2/3] drm/i915: Drop inspection of execbuf flags during evict
@ 2019-11-08 10:11       ` Chris Wilson
  0 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2019-11-08 10:11 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

Quoting Daniel Vetter (2019-11-08 09:54:42)
> On Wed, Nov 6, 2019 at 4:49 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > With the goal of removing the serialisation from around execbuf, we will
> > no longer have the privilege of there being a single execbuf in flight
> > at any time and so will only be able to inspect the user's flags within
> > the carefully controlled execbuf context. i915_gem_evict_for_node() is
> > the only user outside of execbuf that currently peeks at the flag to
> > convert an overlapping softpinned request from ENOSPC to EINVAL. Retract
> > this nicety and only report ENOSPC if the location is in current use,
> > either due to this execbuf or another.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> Same reasons as for patch 3, I don't think we have to do this at all.

This is already undefined behaviour. That field is protected by
struct_mutex and being evaluated outside of that lock.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [Intel-gfx] [PATCH 2/3] drm/i915: Drop inspection of execbuf flags during evict
@ 2019-11-08 10:11       ` Chris Wilson
  0 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2019-11-08 10:11 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

Quoting Daniel Vetter (2019-11-08 09:54:42)
> On Wed, Nov 6, 2019 at 4:49 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > With the goal of removing the serialisation from around execbuf, we will
> > no longer have the privilege of there being a single execbuf in flight
> > at any time and so will only be able to inspect the user's flags within
> > the carefully controlled execbuf context. i915_gem_evict_for_node() is
> > the only user outside of execbuf that currently peeks at the flag to
> > convert an overlapping softpinned request from ENOSPC to EINVAL. Retract
> > this nicety and only report ENOSPC if the location is in current use,
> > either due to this execbuf or another.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> Same reasons as for patch 3, I don't think we have to do this at all.

This is already undefined behaviour. That field is protected by
struct_mutex and being evaluated outside of that lock.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH 2/3] drm/i915: Drop inspection of execbuf flags during evict
@ 2019-11-08 10:20         ` Daniel Vetter
  0 siblings, 0 replies; 30+ messages in thread
From: Daniel Vetter @ 2019-11-08 10:20 UTC (permalink / raw)
  To: Chris Wilson, Maarten Lankhorst, Joonas Lahtinen; +Cc: intel-gfx

On Fri, Nov 8, 2019 at 11:11 AM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Daniel Vetter (2019-11-08 09:54:42)
> > On Wed, Nov 6, 2019 at 4:49 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > >
> > > With the goal of removing the serialisation from around execbuf, we will
> > > no longer have the privilege of there being a single execbuf in flight
> > > at any time and so will only be able to inspect the user's flags within
> > > the carefully controlled execbuf context. i915_gem_evict_for_node() is
> > > the only user outside of execbuf that currently peeks at the flag to
> > > convert an overlapping softpinned request from ENOSPC to EINVAL. Retract
> > > this nicety and only report ENOSPC if the location is in current use,
> > > either due to this execbuf or another.
> > >
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >
> > Same reasons as for patch 3, I don't think we have to do this at all.
>
> This is already undefined behaviour. That field is protected by
> struct_mutex and being evaluated outside of that lock.

If this can be called on objects involved in execbuf, without
struct_mutex, then we already have a correctness problem of vma space
(which is super tight on old platforms and rather much required to be
well-managed because of that) being lost because concurrent threads
thrash it instead of forming an orderly queue. And if that's not the
case, and they do form an orderly queue, then there's no problem since
even the as-needed-only orderly queue provided by ww_mutex will then
be enough locking to keep this working.

Aside: Yeah I think we need to re-add struct_mutex to the gtt fault
path, the temporary pinning in there could easily starve execbuf on
platforms where batches run in ggtt. Maybe also some other areas where
we lost struct_mutex around temporary vma->pin_count elevations.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [Intel-gfx] [PATCH 2/3] drm/i915: Drop inspection of execbuf flags during evict
@ 2019-11-08 10:20         ` Daniel Vetter
  0 siblings, 0 replies; 30+ messages in thread
From: Daniel Vetter @ 2019-11-08 10:20 UTC (permalink / raw)
  To: Chris Wilson, Maarten Lankhorst, Joonas Lahtinen; +Cc: intel-gfx

On Fri, Nov 8, 2019 at 11:11 AM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Daniel Vetter (2019-11-08 09:54:42)
> > On Wed, Nov 6, 2019 at 4:49 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > >
> > > With the goal of removing the serialisation from around execbuf, we will
> > > no longer have the privilege of there being a single execbuf in flight
> > > at any time and so will only be able to inspect the user's flags within
> > > the carefully controlled execbuf context. i915_gem_evict_for_node() is
> > > the only user outside of execbuf that currently peeks at the flag to
> > > convert an overlapping softpinned request from ENOSPC to EINVAL. Retract
> > > this nicety and only report ENOSPC if the location is in current use,
> > > either due to this execbuf or another.
> > >
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >
> > Same reasons as for patch 3, I don't think we have to do this at all.
>
> This is already undefined behaviour. That field is protected by
> struct_mutex and being evaluated outside of that lock.

If this can be called on objects involved in execbuf, without
struct_mutex, then we already have a correctness problem of vma space
(which is super tight on old platforms and rather much required to be
well-managed because of that) being lost because concurrent threads
thrash it instead of forming an orderly queue. And if that's not the
case, and they do form an orderly queue, then there's no problem since
even the as-needed-only orderly queue provided by ww_mutex will then
be enough locking to keep this working.

Aside: Yeah I think we need to re-add struct_mutex to the gtt fault
path, the temporary pinning in there could easily starve execbuf on
platforms where batches run in ggtt. Maybe also some other areas where
we lost struct_mutex around temporary vma->pin_count elevations.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] drm/i915: Handle i915_active_fence_set() with the same fence
@ 2019-11-08 10:37   ` Tvrtko Ursulin
  0 siblings, 0 replies; 30+ messages in thread
From: Tvrtko Ursulin @ 2019-11-08 10:37 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 06/11/2019 15:48, Chris Wilson wrote:
> If the caller wants to overwrite the currently tracked fence, with
> itself, as far as the tracking is concerned it is a no-op, so simply
> allow it.

This is needed for some of the following patches in this series?

Regards,

Tvrtko

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_active.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
> index 207383dda84d..cde984744f20 100644
> --- a/drivers/gpu/drm/i915/i915_active.c
> +++ b/drivers/gpu/drm/i915/i915_active.c
> @@ -759,7 +759,9 @@ __i915_active_fence_set(struct i915_active_fence *active,
>   
>   	prev = rcu_dereference_protected(active->fence, active_is_held(active));
>   	if (prev) {
> -		GEM_BUG_ON(prev == fence);
> +		if (unlikely(prev == fence))
> +			goto unlock;
> +
>   		spin_lock_nested(prev->lock, SINGLE_DEPTH_NESTING);
>   		__list_del_entry(&active->cb.node);
>   		spin_unlock(prev->lock); /* serialise with prev->cb_list */
> @@ -781,6 +783,7 @@ __i915_active_fence_set(struct i915_active_fence *active,
>   	rcu_assign_pointer(active->fence, fence);
>   	list_add_tail(&active->cb.node, &fence->cb_list);
>   
> +unlock:
>   	spin_unlock_irqrestore(fence->lock, flags);
>   
>   	return prev;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [Intel-gfx] [PATCH 1/3] drm/i915: Handle i915_active_fence_set() with the same fence
@ 2019-11-08 10:37   ` Tvrtko Ursulin
  0 siblings, 0 replies; 30+ messages in thread
From: Tvrtko Ursulin @ 2019-11-08 10:37 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 06/11/2019 15:48, Chris Wilson wrote:
> If the caller wants to overwrite the currently tracked fence, with
> itself, as far as the tracking is concerned it is a no-op, so simply
> allow it.

This is needed for some of the following patches in this series?

Regards,

Tvrtko

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_active.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
> index 207383dda84d..cde984744f20 100644
> --- a/drivers/gpu/drm/i915/i915_active.c
> +++ b/drivers/gpu/drm/i915/i915_active.c
> @@ -759,7 +759,9 @@ __i915_active_fence_set(struct i915_active_fence *active,
>   
>   	prev = rcu_dereference_protected(active->fence, active_is_held(active));
>   	if (prev) {
> -		GEM_BUG_ON(prev == fence);
> +		if (unlikely(prev == fence))
> +			goto unlock;
> +
>   		spin_lock_nested(prev->lock, SINGLE_DEPTH_NESTING);
>   		__list_del_entry(&active->cb.node);
>   		spin_unlock(prev->lock); /* serialise with prev->cb_list */
> @@ -781,6 +783,7 @@ __i915_active_fence_set(struct i915_active_fence *active,
>   	rcu_assign_pointer(active->fence, fence);
>   	list_add_tail(&active->cb.node, &fence->cb_list);
>   
> +unlock:
>   	spin_unlock_irqrestore(fence->lock, flags);
>   
>   	return prev;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH 2/3] drm/i915: Drop inspection of execbuf flags during evict
@ 2019-11-08 10:40           ` Chris Wilson
  0 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2019-11-08 10:40 UTC (permalink / raw)
  To: Daniel Vetter, Joonas Lahtinen, Maarten Lankhorst; +Cc: intel-gfx

Quoting Daniel Vetter (2019-11-08 10:20:23)
> On Fri, Nov 8, 2019 at 11:11 AM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Quoting Daniel Vetter (2019-11-08 09:54:42)
> > > On Wed, Nov 6, 2019 at 4:49 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > >
> > > > With the goal of removing the serialisation from around execbuf, we will
> > > > no longer have the privilege of there being a single execbuf in flight
> > > > at any time and so will only be able to inspect the user's flags within
> > > > the carefully controlled execbuf context. i915_gem_evict_for_node() is
> > > > the only user outside of execbuf that currently peeks at the flag to
> > > > convert an overlapping softpinned request from ENOSPC to EINVAL. Retract
> > > > this nicety and only report ENOSPC if the location is in current use,
> > > > either due to this execbuf or another.
> > > >
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > >
> > > Same reasons as for patch 3, I don't think we have to do this at all.
> >
> > This is already undefined behaviour. That field is protected by
> > struct_mutex and being evaluated outside of that lock.
> 
> If this can be called on objects involved in execbuf, without
> struct_mutex, then we already have a correctness problem of vma space
> (which is super tight on old platforms and rather much required to be
> well-managed because of that) being lost because concurrent threads
> thrash it instead of forming an orderly queue. And if that's not the
> case, and they do form an orderly queue, then there's no problem since
> even the as-needed-only orderly queue provided by ww_mutex will then
> be enough locking to keep this working.

It doesn't get called on those objects, those objects may just be
neighbouring and being inspected for potential eviction candidates. The
lists themselves are protected by their mutex, it's just the contention
over the pin_count.
 
> Aside: Yeah I think we need to re-add struct_mutex to the gtt fault
> path, the temporary pinning in there could easily starve execbuf on
> platforms where batches run in ggtt. Maybe also some other areas where
> we lost struct_mutex around temporary vma->pin_count elevations.

That's where we are going next; not with struct_mutex but fenced access
to reservations to replace the temporary (not HW access) pinning.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [Intel-gfx] [PATCH 2/3] drm/i915: Drop inspection of execbuf flags during evict
@ 2019-11-08 10:40           ` Chris Wilson
  0 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2019-11-08 10:40 UTC (permalink / raw)
  To: Daniel Vetter, Joonas Lahtinen, Maarten Lankhorst; +Cc: intel-gfx

Quoting Daniel Vetter (2019-11-08 10:20:23)
> On Fri, Nov 8, 2019 at 11:11 AM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Quoting Daniel Vetter (2019-11-08 09:54:42)
> > > On Wed, Nov 6, 2019 at 4:49 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > >
> > > > With the goal of removing the serialisation from around execbuf, we will
> > > > no longer have the privilege of there being a single execbuf in flight
> > > > at any time and so will only be able to inspect the user's flags within
> > > > the carefully controlled execbuf context. i915_gem_evict_for_node() is
> > > > the only user outside of execbuf that currently peeks at the flag to
> > > > convert an overlapping softpinned request from ENOSPC to EINVAL. Retract
> > > > this nicety and only report ENOSPC if the location is in current use,
> > > > either due to this execbuf or another.
> > > >
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > >
> > > Same reasons as for patch 3, I don't think we have to do this at all.
> >
> > This is already undefined behaviour. That field is protected by
> > struct_mutex and being evaluated outside of that lock.
> 
> If this can be called on objects involved in execbuf, without
> struct_mutex, then we already have a correctness problem of vma space
> (which is super tight on old platforms and rather much required to be
> well-managed because of that) being lost because concurrent threads
> thrash it instead of forming an orderly queue. And if that's not the
> case, and they do form an orderly queue, then there's no problem since
> even the as-needed-only orderly queue provided by ww_mutex will then
> be enough locking to keep this working.

It doesn't get called on those objects, those objects may just be
neighbouring and being inspected for potential eviction candidates. The
lists themselves are protected by their mutex, it's just the contention
over the pin_count.
 
> Aside: Yeah I think we need to re-add struct_mutex to the gtt fault
> path, the temporary pinning in there could easily starve execbuf on
> platforms where batches run in ggtt. Maybe also some other areas where
> we lost struct_mutex around temporary vma->pin_count elevations.

That's where we are going next; not with struct_mutex but fenced access
to reservations to replace the temporary (not HW access) pinning.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] drm/i915: Handle i915_active_fence_set() with the same fence
@ 2019-11-08 10:42     ` Chris Wilson
  0 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2019-11-08 10:42 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx

Quoting Tvrtko Ursulin (2019-11-08 10:37:37)
> 
> On 06/11/2019 15:48, Chris Wilson wrote:
> > If the caller wants to overwrite the currently tracked fence, with
> > itself, as far as the tracking is concerned it is a no-op, so simply
> > allow it.
> 
> This is needed for some of the following patches in this series?

The implementation is relaxed in patch 3, which means we allow the user
to update the same i915_request.fence on the same timeline multiple times.
We already allow them to use the same fence multiple times, symmetry is
good?...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [Intel-gfx] [PATCH 1/3] drm/i915: Handle i915_active_fence_set() with the same fence
@ 2019-11-08 10:42     ` Chris Wilson
  0 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2019-11-08 10:42 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx

Quoting Tvrtko Ursulin (2019-11-08 10:37:37)
> 
> On 06/11/2019 15:48, Chris Wilson wrote:
> > If the caller wants to overwrite the currently tracked fence, with
> > itself, as far as the tracking is concerned it is a no-op, so simply
> > allow it.
> 
> This is needed for some of the following patches in this series?

The implementation is relaxed in patch 3, which means we allow the user
to update the same i915_request.fence on the same timeline multiple times.
We already allow them to use the same fence multiple times, symmetry is
good?...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* ✗ Fi.CI.IGT: failure for series starting with [1/3] drm/i915: Handle i915_active_fence_set() with the same fence
@ 2019-11-08 12:00   ` Patchwork
  0 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2019-11-08 12:00 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915: Handle i915_active_fence_set() with the same fence
URL   : https://patchwork.freedesktop.org/series/69074/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7273_full -> Patchwork_15157_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_15157_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_15157_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_15157_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_close@many-handles-one-vma:
    - shard-glk:          [PASS][1] -> [DMESG-FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-glk3/igt@gem_close@many-handles-one-vma.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-glk3/igt@gem_close@many-handles-one-vma.html
    - shard-apl:          [PASS][3] -> [DMESG-FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-apl1/igt@gem_close@many-handles-one-vma.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-apl3/igt@gem_close@many-handles-one-vma.html
    - shard-skl:          [PASS][5] -> [DMESG-FAIL][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-skl1/igt@gem_close@many-handles-one-vma.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-skl4/igt@gem_close@many-handles-one-vma.html
    - shard-hsw:          [PASS][7] -> [DMESG-FAIL][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-hsw5/igt@gem_close@many-handles-one-vma.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-hsw4/igt@gem_close@many-handles-one-vma.html
    - shard-iclb:         [PASS][9] -> [DMESG-FAIL][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb3/igt@gem_close@many-handles-one-vma.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb4/igt@gem_close@many-handles-one-vma.html

  * igt@gem_softpin@overlap:
    - shard-skl:          [PASS][11] -> [FAIL][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-skl1/igt@gem_softpin@overlap.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-skl4/igt@gem_softpin@overlap.html
    - shard-apl:          [PASS][13] -> [FAIL][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-apl1/igt@gem_softpin@overlap.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-apl3/igt@gem_softpin@overlap.html
    - shard-tglb:         [PASS][15] -> [FAIL][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb3/igt@gem_softpin@overlap.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb5/igt@gem_softpin@overlap.html
    - shard-glk:          [PASS][17] -> [FAIL][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-glk3/igt@gem_softpin@overlap.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-glk3/igt@gem_softpin@overlap.html
    - shard-hsw:          [PASS][19] -> [FAIL][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-hsw5/igt@gem_softpin@overlap.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-hsw4/igt@gem_softpin@overlap.html
    - shard-kbl:          [PASS][21] -> [FAIL][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-kbl6/igt@gem_softpin@overlap.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-kbl6/igt@gem_softpin@overlap.html
    - shard-snb:          [PASS][23] -> [FAIL][24] +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-snb7/igt@gem_softpin@overlap.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-snb4/igt@gem_softpin@overlap.html
    - shard-iclb:         [PASS][25] -> [FAIL][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb3/igt@gem_softpin@overlap.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb4/igt@gem_softpin@overlap.html

  * igt@runner@aborted:
    - shard-hsw:          NOTRUN -> [FAIL][27]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-hsw4/igt@runner@aborted.html
    - shard-apl:          NOTRUN -> [FAIL][28]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-apl3/igt@runner@aborted.html

  
Known issues
------------

  Here are the changes found in Patchwork_15157_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@vcs1-clean:
    - shard-iclb:         [PASS][29] -> [SKIP][30] ([fdo#109276] / [fdo#112080]) +2 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb1/igt@gem_ctx_isolation@vcs1-clean.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb6/igt@gem_ctx_isolation@vcs1-clean.html

  * igt@gem_ctx_isolation@vcs1-s3:
    - shard-tglb:         [PASS][31] -> [INCOMPLETE][32] ([fdo#111832]) +1 similar issue
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb2/igt@gem_ctx_isolation@vcs1-s3.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb4/igt@gem_ctx_isolation@vcs1-s3.html

  * igt@gem_eio@in-flight-suspend:
    - shard-tglb:         [PASS][33] -> [INCOMPLETE][34] ([fdo#111832] / [fdo#111850] / [fdo#112081])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb6/igt@gem_eio@in-flight-suspend.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb2/igt@gem_eio@in-flight-suspend.html

  * igt@gem_exec_balancer@smoke:
    - shard-tglb:         [PASS][35] -> [INCOMPLETE][36] ([fdo#111593])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb5/igt@gem_exec_balancer@smoke.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb4/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_parallel@vcs1-fds:
    - shard-iclb:         [PASS][37] -> [SKIP][38] ([fdo#112080]) +11 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb4/igt@gem_exec_parallel@vcs1-fds.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb8/igt@gem_exec_parallel@vcs1-fds.html

  * igt@gem_exec_schedule@preempt-queue-bsd:
    - shard-iclb:         [PASS][39] -> [SKIP][40] ([fdo#112146]) +6 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb3/igt@gem_exec_schedule@preempt-queue-bsd.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb2/igt@gem_exec_schedule@preempt-queue-bsd.html

  * igt@gem_persistent_relocs@forked-interruptible-thrashing:
    - shard-snb:          [PASS][41] -> [FAIL][42] ([fdo#112037])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-snb1/igt@gem_persistent_relocs@forked-interruptible-thrashing.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-snb5/igt@gem_persistent_relocs@forked-interruptible-thrashing.html

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          [PASS][43] -> [INCOMPLETE][44] ([fdo#104108]) +1 similar issue
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-skl6/igt@gem_softpin@noreloc-s3.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-skl6/igt@gem_softpin@noreloc-s3.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy-gup:
    - shard-hsw:          [PASS][45] -> [DMESG-WARN][46] ([fdo#111870]) +2 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-hsw1/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-hsw1/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html

  * igt@gem_workarounds@suspend-resume:
    - shard-apl:          [PASS][47] -> [DMESG-WARN][48] ([fdo#108566]) +1 similar issue
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-apl4/igt@gem_workarounds@suspend-resume.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-apl4/igt@gem_workarounds@suspend-resume.html

  * igt@i915_pm_rps@waitboost:
    - shard-apl:          [PASS][49] -> [FAIL][50] ([fdo#102250])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-apl6/igt@i915_pm_rps@waitboost.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-apl2/igt@i915_pm_rps@waitboost.html

  * igt@i915_selftest@mock_requests:
    - shard-skl:          [PASS][51] -> [DMESG-WARN][52] ([fdo#111086])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-skl7/igt@i915_selftest@mock_requests.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-skl5/igt@i915_selftest@mock_requests.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-tglb:         [PASS][53] -> [INCOMPLETE][54] ([fdo#111832] / [fdo#111850]) +5 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb2/igt@i915_suspend@fence-restore-tiled2untiled.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb7/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_color@pipe-b-ctm-red-to-blue:
    - shard-skl:          [PASS][55] -> [DMESG-WARN][56] ([fdo#106107])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-skl10/igt@kms_color@pipe-b-ctm-red-to-blue.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-skl4/igt@kms_color@pipe-b-ctm-red-to-blue.html

  * igt@kms_frontbuffer_tracking@basic:
    - shard-iclb:         [PASS][57] -> [FAIL][58] ([fdo#103167]) +4 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb6/igt@kms_frontbuffer_tracking@basic.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb3/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_frontbuffer_tracking@fbc-stridechange:
    - shard-tglb:         [PASS][59] -> [FAIL][60] ([fdo#103167]) +1 similar issue
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb3/igt@kms_frontbuffer_tracking@fbc-stridechange.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb1/igt@kms_frontbuffer_tracking@fbc-stridechange.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [PASS][61] -> [DMESG-WARN][62] ([fdo#108566]) +3 similar issues
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-suspend:
    - shard-tglb:         [PASS][63] -> [INCOMPLETE][64] ([fdo#111832] / [fdo#111850] / [fdo#111884])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb2/igt@kms_frontbuffer_tracking@fbcpsr-suspend.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb8/igt@kms_frontbuffer_tracking@fbcpsr-suspend.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][65] -> [FAIL][66] ([fdo#108145] / [fdo#110403])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-skl8/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-skl1/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [PASS][67] -> [FAIL][68] ([fdo#108341])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb7/igt@kms_psr@no_drrs.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb1/igt@kms_psr@no_drrs.html

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-iclb:         [PASS][69] -> [SKIP][70] ([fdo#109441])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb1/igt@kms_psr@psr2_cursor_plane_onoff.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][71] -> [FAIL][72] ([fdo#99912])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-apl4/igt@kms_setmode@basic.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-apl4/igt@kms_setmode@basic.html

  * igt@prime_vgem@fence-wait-bsd2:
    - shard-iclb:         [PASS][73] -> [SKIP][74] ([fdo#109276]) +18 similar issues
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb1/igt@prime_vgem@fence-wait-bsd2.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb6/igt@prime_vgem@fence-wait-bsd2.html

  
#### Possible fixes ####

  * igt@gem_ctx_persistence@vcs1-queued:
    - shard-iclb:         [SKIP][75] ([fdo#109276] / [fdo#112080]) -> [PASS][76] +3 similar issues
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb7/igt@gem_ctx_persistence@vcs1-queued.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb4/igt@gem_ctx_persistence@vcs1-queued.html

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [SKIP][77] ([fdo#110841]) -> [PASS][78]
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb4/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb6/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_ctx_shared@q-smoketest-all:
    - shard-tglb:         [INCOMPLETE][79] ([fdo#111735]) -> [PASS][80]
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb4/igt@gem_ctx_shared@q-smoketest-all.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb2/igt@gem_ctx_shared@q-smoketest-all.html

  * igt@gem_ctx_switch@vcs1:
    - shard-iclb:         [SKIP][81] ([fdo#112080]) -> [PASS][82] +10 similar issues
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb7/igt@gem_ctx_switch@vcs1.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb4/igt@gem_ctx_switch@vcs1.html

  * igt@gem_exec_nop@basic-parallel:
    - shard-tglb:         [INCOMPLETE][83] ([fdo#111747]) -> [PASS][84]
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb7/igt@gem_exec_nop@basic-parallel.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb9/igt@gem_exec_nop@basic-parallel.html

  * igt@gem_exec_schedule@deep-bsd:
    - shard-iclb:         [SKIP][85] ([fdo#112146]) -> [PASS][86] +2 similar issues
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb1/igt@gem_exec_schedule@deep-bsd.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb6/igt@gem_exec_schedule@deep-bsd.html

  * igt@gem_userptr_blits@dmabuf-sync:
    - shard-snb:          [DMESG-WARN][87] ([fdo#111870]) -> [PASS][88]
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-snb5/igt@gem_userptr_blits@dmabuf-sync.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-snb6/igt@gem_userptr_blits@dmabuf-sync.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy:
    - shard-hsw:          [DMESG-WARN][89] ([fdo#111870]) -> [PASS][90]
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-hsw6/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-hsw2/igt@gem_userptr_blits@map-fixed-invalidate-busy.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [FAIL][91] ([fdo#110548]) -> [PASS][92]
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb6/igt@i915_pm_dc@dc6-psr.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb7/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          [FAIL][93] ([fdo#105767]) -> [PASS][94]
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-hsw4/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-hsw5/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          [FAIL][95] ([fdo#104873]) -> [PASS][96]
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-glk3/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-glk3/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html

  * igt@kms_flip@2x-flip-vs-expired-vblank:
    - shard-glk:          [FAIL][97] ([fdo#105363]) -> [PASS][98]
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-glk4/igt@kms_flip@2x-flip-vs-expired-vblank.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-glk7/igt@kms_flip@2x-flip-vs-expired-vblank.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [FAIL][99] ([fdo#105363]) -> [PASS][100]
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-skl6/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-skl3/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-hsw:          [INCOMPLETE][101] ([fdo#103540]) -> [PASS][102]
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-hsw2/igt@kms_flip@flip-vs-suspend.html
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-hsw6/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-apl:          [DMESG-WARN][103] ([fdo#108566]) -> [PASS][104] +2 similar issues
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-apl6/igt@kms_flip@flip-vs-suspend-interruptible.html
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-apl2/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - shard-iclb:         [FAIL][105] ([fdo#103167]) -> [PASS][106] +4 similar issues
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-move:
    - shard-tglb:         [FAIL][107] ([fdo#103167]) -> [PASS][108] +1 similar issue
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-move.html
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-move.html

  * igt@kms_frontbuffer_tracking@psr-suspend:
    - shard-tglb:         [INCOMPLETE][109] ([fdo#111832] / [fdo#111850]) -> [PASS][110]
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb3/igt@kms_frontbuffer_tracking@psr-suspend.html
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb9/igt@kms_frontbuffer_tracking@psr-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-kbl:          [DMESG-WARN][111] ([fdo#108566]) -> [PASS][112] +6 similar issues
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-kbl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-kbl2/igt@kms_plane@plane-panning-bottom-right-

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/3] drm/i915: Handle i915_active_fence_set() with the same fence
@ 2019-11-08 12:00   ` Patchwork
  0 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2019-11-08 12:00 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915: Handle i915_active_fence_set() with the same fence
URL   : https://patchwork.freedesktop.org/series/69074/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7273_full -> Patchwork_15157_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_15157_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_15157_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_15157_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_close@many-handles-one-vma:
    - shard-glk:          [PASS][1] -> [DMESG-FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-glk3/igt@gem_close@many-handles-one-vma.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-glk3/igt@gem_close@many-handles-one-vma.html
    - shard-apl:          [PASS][3] -> [DMESG-FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-apl1/igt@gem_close@many-handles-one-vma.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-apl3/igt@gem_close@many-handles-one-vma.html
    - shard-skl:          [PASS][5] -> [DMESG-FAIL][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-skl1/igt@gem_close@many-handles-one-vma.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-skl4/igt@gem_close@many-handles-one-vma.html
    - shard-hsw:          [PASS][7] -> [DMESG-FAIL][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-hsw5/igt@gem_close@many-handles-one-vma.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-hsw4/igt@gem_close@many-handles-one-vma.html
    - shard-iclb:         [PASS][9] -> [DMESG-FAIL][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb3/igt@gem_close@many-handles-one-vma.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb4/igt@gem_close@many-handles-one-vma.html

  * igt@gem_softpin@overlap:
    - shard-skl:          [PASS][11] -> [FAIL][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-skl1/igt@gem_softpin@overlap.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-skl4/igt@gem_softpin@overlap.html
    - shard-apl:          [PASS][13] -> [FAIL][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-apl1/igt@gem_softpin@overlap.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-apl3/igt@gem_softpin@overlap.html
    - shard-tglb:         [PASS][15] -> [FAIL][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb3/igt@gem_softpin@overlap.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb5/igt@gem_softpin@overlap.html
    - shard-glk:          [PASS][17] -> [FAIL][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-glk3/igt@gem_softpin@overlap.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-glk3/igt@gem_softpin@overlap.html
    - shard-hsw:          [PASS][19] -> [FAIL][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-hsw5/igt@gem_softpin@overlap.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-hsw4/igt@gem_softpin@overlap.html
    - shard-kbl:          [PASS][21] -> [FAIL][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-kbl6/igt@gem_softpin@overlap.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-kbl6/igt@gem_softpin@overlap.html
    - shard-snb:          [PASS][23] -> [FAIL][24] +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-snb7/igt@gem_softpin@overlap.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-snb4/igt@gem_softpin@overlap.html
    - shard-iclb:         [PASS][25] -> [FAIL][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb3/igt@gem_softpin@overlap.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb4/igt@gem_softpin@overlap.html

  * igt@runner@aborted:
    - shard-hsw:          NOTRUN -> [FAIL][27]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-hsw4/igt@runner@aborted.html
    - shard-apl:          NOTRUN -> [FAIL][28]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-apl3/igt@runner@aborted.html

  
Known issues
------------

  Here are the changes found in Patchwork_15157_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@vcs1-clean:
    - shard-iclb:         [PASS][29] -> [SKIP][30] ([fdo#109276] / [fdo#112080]) +2 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb1/igt@gem_ctx_isolation@vcs1-clean.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb6/igt@gem_ctx_isolation@vcs1-clean.html

  * igt@gem_ctx_isolation@vcs1-s3:
    - shard-tglb:         [PASS][31] -> [INCOMPLETE][32] ([fdo#111832]) +1 similar issue
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb2/igt@gem_ctx_isolation@vcs1-s3.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb4/igt@gem_ctx_isolation@vcs1-s3.html

  * igt@gem_eio@in-flight-suspend:
    - shard-tglb:         [PASS][33] -> [INCOMPLETE][34] ([fdo#111832] / [fdo#111850] / [fdo#112081])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb6/igt@gem_eio@in-flight-suspend.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb2/igt@gem_eio@in-flight-suspend.html

  * igt@gem_exec_balancer@smoke:
    - shard-tglb:         [PASS][35] -> [INCOMPLETE][36] ([fdo#111593])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb5/igt@gem_exec_balancer@smoke.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb4/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_parallel@vcs1-fds:
    - shard-iclb:         [PASS][37] -> [SKIP][38] ([fdo#112080]) +11 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb4/igt@gem_exec_parallel@vcs1-fds.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb8/igt@gem_exec_parallel@vcs1-fds.html

  * igt@gem_exec_schedule@preempt-queue-bsd:
    - shard-iclb:         [PASS][39] -> [SKIP][40] ([fdo#112146]) +6 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb3/igt@gem_exec_schedule@preempt-queue-bsd.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb2/igt@gem_exec_schedule@preempt-queue-bsd.html

  * igt@gem_persistent_relocs@forked-interruptible-thrashing:
    - shard-snb:          [PASS][41] -> [FAIL][42] ([fdo#112037])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-snb1/igt@gem_persistent_relocs@forked-interruptible-thrashing.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-snb5/igt@gem_persistent_relocs@forked-interruptible-thrashing.html

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          [PASS][43] -> [INCOMPLETE][44] ([fdo#104108]) +1 similar issue
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-skl6/igt@gem_softpin@noreloc-s3.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-skl6/igt@gem_softpin@noreloc-s3.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy-gup:
    - shard-hsw:          [PASS][45] -> [DMESG-WARN][46] ([fdo#111870]) +2 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-hsw1/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-hsw1/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html

  * igt@gem_workarounds@suspend-resume:
    - shard-apl:          [PASS][47] -> [DMESG-WARN][48] ([fdo#108566]) +1 similar issue
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-apl4/igt@gem_workarounds@suspend-resume.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-apl4/igt@gem_workarounds@suspend-resume.html

  * igt@i915_pm_rps@waitboost:
    - shard-apl:          [PASS][49] -> [FAIL][50] ([fdo#102250])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-apl6/igt@i915_pm_rps@waitboost.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-apl2/igt@i915_pm_rps@waitboost.html

  * igt@i915_selftest@mock_requests:
    - shard-skl:          [PASS][51] -> [DMESG-WARN][52] ([fdo#111086])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-skl7/igt@i915_selftest@mock_requests.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-skl5/igt@i915_selftest@mock_requests.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-tglb:         [PASS][53] -> [INCOMPLETE][54] ([fdo#111832] / [fdo#111850]) +5 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb2/igt@i915_suspend@fence-restore-tiled2untiled.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb7/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_color@pipe-b-ctm-red-to-blue:
    - shard-skl:          [PASS][55] -> [DMESG-WARN][56] ([fdo#106107])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-skl10/igt@kms_color@pipe-b-ctm-red-to-blue.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-skl4/igt@kms_color@pipe-b-ctm-red-to-blue.html

  * igt@kms_frontbuffer_tracking@basic:
    - shard-iclb:         [PASS][57] -> [FAIL][58] ([fdo#103167]) +4 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb6/igt@kms_frontbuffer_tracking@basic.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb3/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_frontbuffer_tracking@fbc-stridechange:
    - shard-tglb:         [PASS][59] -> [FAIL][60] ([fdo#103167]) +1 similar issue
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb3/igt@kms_frontbuffer_tracking@fbc-stridechange.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb1/igt@kms_frontbuffer_tracking@fbc-stridechange.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [PASS][61] -> [DMESG-WARN][62] ([fdo#108566]) +3 similar issues
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-suspend:
    - shard-tglb:         [PASS][63] -> [INCOMPLETE][64] ([fdo#111832] / [fdo#111850] / [fdo#111884])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb2/igt@kms_frontbuffer_tracking@fbcpsr-suspend.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb8/igt@kms_frontbuffer_tracking@fbcpsr-suspend.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][65] -> [FAIL][66] ([fdo#108145] / [fdo#110403])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-skl8/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-skl1/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [PASS][67] -> [FAIL][68] ([fdo#108341])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb7/igt@kms_psr@no_drrs.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb1/igt@kms_psr@no_drrs.html

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-iclb:         [PASS][69] -> [SKIP][70] ([fdo#109441])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb1/igt@kms_psr@psr2_cursor_plane_onoff.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][71] -> [FAIL][72] ([fdo#99912])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-apl4/igt@kms_setmode@basic.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-apl4/igt@kms_setmode@basic.html

  * igt@prime_vgem@fence-wait-bsd2:
    - shard-iclb:         [PASS][73] -> [SKIP][74] ([fdo#109276]) +18 similar issues
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb1/igt@prime_vgem@fence-wait-bsd2.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb6/igt@prime_vgem@fence-wait-bsd2.html

  
#### Possible fixes ####

  * igt@gem_ctx_persistence@vcs1-queued:
    - shard-iclb:         [SKIP][75] ([fdo#109276] / [fdo#112080]) -> [PASS][76] +3 similar issues
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb7/igt@gem_ctx_persistence@vcs1-queued.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb4/igt@gem_ctx_persistence@vcs1-queued.html

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [SKIP][77] ([fdo#110841]) -> [PASS][78]
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb4/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb6/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_ctx_shared@q-smoketest-all:
    - shard-tglb:         [INCOMPLETE][79] ([fdo#111735]) -> [PASS][80]
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb4/igt@gem_ctx_shared@q-smoketest-all.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb2/igt@gem_ctx_shared@q-smoketest-all.html

  * igt@gem_ctx_switch@vcs1:
    - shard-iclb:         [SKIP][81] ([fdo#112080]) -> [PASS][82] +10 similar issues
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb7/igt@gem_ctx_switch@vcs1.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb4/igt@gem_ctx_switch@vcs1.html

  * igt@gem_exec_nop@basic-parallel:
    - shard-tglb:         [INCOMPLETE][83] ([fdo#111747]) -> [PASS][84]
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb7/igt@gem_exec_nop@basic-parallel.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb9/igt@gem_exec_nop@basic-parallel.html

  * igt@gem_exec_schedule@deep-bsd:
    - shard-iclb:         [SKIP][85] ([fdo#112146]) -> [PASS][86] +2 similar issues
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb1/igt@gem_exec_schedule@deep-bsd.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb6/igt@gem_exec_schedule@deep-bsd.html

  * igt@gem_userptr_blits@dmabuf-sync:
    - shard-snb:          [DMESG-WARN][87] ([fdo#111870]) -> [PASS][88]
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-snb5/igt@gem_userptr_blits@dmabuf-sync.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-snb6/igt@gem_userptr_blits@dmabuf-sync.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy:
    - shard-hsw:          [DMESG-WARN][89] ([fdo#111870]) -> [PASS][90]
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-hsw6/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-hsw2/igt@gem_userptr_blits@map-fixed-invalidate-busy.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [FAIL][91] ([fdo#110548]) -> [PASS][92]
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb6/igt@i915_pm_dc@dc6-psr.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb7/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          [FAIL][93] ([fdo#105767]) -> [PASS][94]
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-hsw4/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-hsw5/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          [FAIL][95] ([fdo#104873]) -> [PASS][96]
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-glk3/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-glk3/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html

  * igt@kms_flip@2x-flip-vs-expired-vblank:
    - shard-glk:          [FAIL][97] ([fdo#105363]) -> [PASS][98]
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-glk4/igt@kms_flip@2x-flip-vs-expired-vblank.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-glk7/igt@kms_flip@2x-flip-vs-expired-vblank.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [FAIL][99] ([fdo#105363]) -> [PASS][100]
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-skl6/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-skl3/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-hsw:          [INCOMPLETE][101] ([fdo#103540]) -> [PASS][102]
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-hsw2/igt@kms_flip@flip-vs-suspend.html
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-hsw6/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-apl:          [DMESG-WARN][103] ([fdo#108566]) -> [PASS][104] +2 similar issues
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-apl6/igt@kms_flip@flip-vs-suspend-interruptible.html
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-apl2/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - shard-iclb:         [FAIL][105] ([fdo#103167]) -> [PASS][106] +4 similar issues
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-move:
    - shard-tglb:         [FAIL][107] ([fdo#103167]) -> [PASS][108] +1 similar issue
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-move.html
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-move.html

  * igt@kms_frontbuffer_tracking@psr-suspend:
    - shard-tglb:         [INCOMPLETE][109] ([fdo#111832] / [fdo#111850]) -> [PASS][110]
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-tglb3/igt@kms_frontbuffer_tracking@psr-suspend.html
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-tglb9/igt@kms_frontbuffer_tracking@psr-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-kbl:          [DMESG-WARN][111] ([fdo#108566]) -> [PASS][112] +6 similar issues
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7273/shard-kbl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/shard-kbl2/igt@kms_plane@plane-panning-bottom-right-

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15157/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH 2/3] drm/i915: Drop inspection of execbuf flags during evict
@ 2019-11-08 16:06             ` Daniel Vetter
  0 siblings, 0 replies; 30+ messages in thread
From: Daniel Vetter @ 2019-11-08 16:06 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Fri, Nov 8, 2019 at 11:40 AM Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Quoting Daniel Vetter (2019-11-08 10:20:23)
> > On Fri, Nov 8, 2019 at 11:11 AM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > Quoting Daniel Vetter (2019-11-08 09:54:42)
> > > > On Wed, Nov 6, 2019 at 4:49 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > > >
> > > > > With the goal of removing the serialisation from around execbuf, we will
> > > > > no longer have the privilege of there being a single execbuf in flight
> > > > > at any time and so will only be able to inspect the user's flags within
> > > > > the carefully controlled execbuf context. i915_gem_evict_for_node() is
> > > > > the only user outside of execbuf that currently peeks at the flag to
> > > > > convert an overlapping softpinned request from ENOSPC to EINVAL. Retract
> > > > > this nicety and only report ENOSPC if the location is in current use,
> > > > > either due to this execbuf or another.
> > > > >
> > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > >
> > > > Same reasons as for patch 3, I don't think we have to do this at all.
> > >
> > > This is already undefined behaviour. That field is protected by
> > > struct_mutex and being evaluated outside of that lock.
> >
> > If this can be called on objects involved in execbuf, without
> > struct_mutex, then we already have a correctness problem of vma space
> > (which is super tight on old platforms and rather much required to be
> > well-managed because of that) being lost because concurrent threads
> > thrash it instead of forming an orderly queue. And if that's not the
> > case, and they do form an orderly queue, then there's no problem since
> > even the as-needed-only orderly queue provided by ww_mutex will then
> > be enough locking to keep this working.
>
> It doesn't get called on those objects, those objects may just be
> neighbouring and being inspected for potential eviction candidates. The
> lists themselves are protected by their mutex, it's just the contention
> over the pin_count.

Hm yeah in a per-bo locked future world this won't work. But today it
should be either vm->mutex or dev->struct_mutex, not already broken?

Otoh in the per-bo locked future we only care about conflicts with our
own execbuf, which means we could check whether the object belongs to
our batch (very easy by looking at dma_resv->lock.ctx, ttm does that
in a few places), and only do the check in that case. So could retain
full uapi semantics here without additional effort (we need to have
these locks anway, at least in any kind of execbuf slowpath where the
bo aren't all mapped when we start out). So still not understanding
(even with the "it's other bo" overlook rectified) why we have to drop
this?

> > Aside: Yeah I think we need to re-add struct_mutex to the gtt fault
> > path, the temporary pinning in there could easily starve execbuf on
> > platforms where batches run in ggtt. Maybe also some other areas where
> > we lost struct_mutex around temporary vma->pin_count elevations.
>
> That's where we are going next; not with struct_mutex but fenced access
> to reservations to replace the temporary (not HW access) pinning.

fenced as in dma_fence or dma_resv_lock?

Also if we indeed have an issue with lost elevated pin_counts now I
think we shouldn't ship 5.5 with that, and reapply the duct tape until
it's fixed for good.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [Intel-gfx] [PATCH 2/3] drm/i915: Drop inspection of execbuf flags during evict
@ 2019-11-08 16:06             ` Daniel Vetter
  0 siblings, 0 replies; 30+ messages in thread
From: Daniel Vetter @ 2019-11-08 16:06 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Fri, Nov 8, 2019 at 11:40 AM Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Quoting Daniel Vetter (2019-11-08 10:20:23)
> > On Fri, Nov 8, 2019 at 11:11 AM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > Quoting Daniel Vetter (2019-11-08 09:54:42)
> > > > On Wed, Nov 6, 2019 at 4:49 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > > >
> > > > > With the goal of removing the serialisation from around execbuf, we will
> > > > > no longer have the privilege of there being a single execbuf in flight
> > > > > at any time and so will only be able to inspect the user's flags within
> > > > > the carefully controlled execbuf context. i915_gem_evict_for_node() is
> > > > > the only user outside of execbuf that currently peeks at the flag to
> > > > > convert an overlapping softpinned request from ENOSPC to EINVAL. Retract
> > > > > this nicety and only report ENOSPC if the location is in current use,
> > > > > either due to this execbuf or another.
> > > > >
> > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > >
> > > > Same reasons as for patch 3, I don't think we have to do this at all.
> > >
> > > This is already undefined behaviour. That field is protected by
> > > struct_mutex and being evaluated outside of that lock.
> >
> > If this can be called on objects involved in execbuf, without
> > struct_mutex, then we already have a correctness problem of vma space
> > (which is super tight on old platforms and rather much required to be
> > well-managed because of that) being lost because concurrent threads
> > thrash it instead of forming an orderly queue. And if that's not the
> > case, and they do form an orderly queue, then there's no problem since
> > even the as-needed-only orderly queue provided by ww_mutex will then
> > be enough locking to keep this working.
>
> It doesn't get called on those objects, those objects may just be
> neighbouring and being inspected for potential eviction candidates. The
> lists themselves are protected by their mutex, it's just the contention
> over the pin_count.

Hm yeah in a per-bo locked future world this won't work. But today it
should be either vm->mutex or dev->struct_mutex, not already broken?

Otoh in the per-bo locked future we only care about conflicts with our
own execbuf, which means we could check whether the object belongs to
our batch (very easy by looking at dma_resv->lock.ctx, ttm does that
in a few places), and only do the check in that case. So could retain
full uapi semantics here without additional effort (we need to have
these locks anway, at least in any kind of execbuf slowpath where the
bo aren't all mapped when we start out). So still not understanding
(even with the "it's other bo" overlook rectified) why we have to drop
this?

> > Aside: Yeah I think we need to re-add struct_mutex to the gtt fault
> > path, the temporary pinning in there could easily starve execbuf on
> > platforms where batches run in ggtt. Maybe also some other areas where
> > we lost struct_mutex around temporary vma->pin_count elevations.
>
> That's where we are going next; not with struct_mutex but fenced access
> to reservations to replace the temporary (not HW access) pinning.

fenced as in dma_fence or dma_resv_lock?

Also if we indeed have an issue with lost elevated pin_counts now I
think we shouldn't ship 5.5 with that, and reapply the duct tape until
it's fixed for good.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH 3/3] drm/i915/gem: Extract transient execbuf flags from i915_vma
@ 2019-11-08 16:13         ` Daniel Vetter
  0 siblings, 0 replies; 30+ messages in thread
From: Daniel Vetter @ 2019-11-08 16:13 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Fri, Nov 8, 2019 at 11:05 AM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Daniel Vetter (2019-11-08 09:53:51)
> > On Wed, Nov 6, 2019 at 4:48 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > For our convenience, and to avoid frequent allocations, we placed some
> > > lists we use for execbuf inside the common i915_vma struct. As we look
> > > to parallelise execbuf, such fields guarded by the struct_mutex BKL must
> > > be pulled under local control. Instead of using the i915_vma as our
> > > primary means of tracking the user's list of objects and their virtual
> > > mappings, we use a local eb_vma with the same lists as before (just now
> > > local not global).
> > >
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >
> > (--supress-cc ? Added Maarten for real, didn't seem to be on cc in my
> > inbox at least)
> >
> > Why do we need this?
>
> Because the execbuf state is not covered by the object locks. We put
> them inside the vma for the express purpose of avoiding an allocation; an
> allocation we ended up doing anyway, which we can now use for the sole
> purpose of tracking the execbuf.
>
> This is _execbuf_ state that we rammed into the i915_vma.

Ok, that makes sense. The locking story imo doesn't make so much sense
to me really, since no matter where we allocate this we'll pretty much
have to hold the corresponding obj dma_resv, at least for slow-paths
with relocs (which are the ones where we need this stuff). I guess one
downside of this is that the temp allocation is now quite a bit
bigger. Isn't that hurting?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [Intel-gfx] [PATCH 3/3] drm/i915/gem: Extract transient execbuf flags from i915_vma
@ 2019-11-08 16:13         ` Daniel Vetter
  0 siblings, 0 replies; 30+ messages in thread
From: Daniel Vetter @ 2019-11-08 16:13 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Fri, Nov 8, 2019 at 11:05 AM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Daniel Vetter (2019-11-08 09:53:51)
> > On Wed, Nov 6, 2019 at 4:48 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > For our convenience, and to avoid frequent allocations, we placed some
> > > lists we use for execbuf inside the common i915_vma struct. As we look
> > > to parallelise execbuf, such fields guarded by the struct_mutex BKL must
> > > be pulled under local control. Instead of using the i915_vma as our
> > > primary means of tracking the user's list of objects and their virtual
> > > mappings, we use a local eb_vma with the same lists as before (just now
> > > local not global).
> > >
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >
> > (--supress-cc ? Added Maarten for real, didn't seem to be on cc in my
> > inbox at least)
> >
> > Why do we need this?
>
> Because the execbuf state is not covered by the object locks. We put
> them inside the vma for the express purpose of avoiding an allocation; an
> allocation we ended up doing anyway, which we can now use for the sole
> purpose of tracking the execbuf.
>
> This is _execbuf_ state that we rammed into the i915_vma.

Ok, that makes sense. The locking story imo doesn't make so much sense
to me really, since no matter where we allocate this we'll pretty much
have to hold the corresponding obj dma_resv, at least for slow-paths
with relocs (which are the ones where we need this stuff). I guess one
downside of this is that the temp allocation is now quite a bit
bigger. Isn't that hurting?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread
end of thread, other threads:[~2019-11-08 16:14 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-06 15:48 [PATCH 1/3] drm/i915: Handle i915_active_fence_set() with the same fence Chris Wilson
2019-11-06 15:48 ` [Intel-gfx] " Chris Wilson
2019-11-06 15:48 ` [PATCH 2/3] drm/i915: Drop inspection of execbuf flags during evict Chris Wilson
2019-11-06 15:48   ` [Intel-gfx] " Chris Wilson
2019-11-08  9:54   ` Daniel Vetter
2019-11-08  9:54     ` [Intel-gfx] " Daniel Vetter
2019-11-08 10:11     ` Chris Wilson
2019-11-08 10:11       ` [Intel-gfx] " Chris Wilson
2019-11-08 10:20       ` Daniel Vetter
2019-11-08 10:20         ` [Intel-gfx] " Daniel Vetter
2019-11-08 10:40         ` Chris Wilson
2019-11-08 10:40           ` [Intel-gfx] " Chris Wilson
2019-11-08 16:06           ` Daniel Vetter
2019-11-08 16:06             ` [Intel-gfx] " Daniel Vetter
2019-11-06 15:48 ` [PATCH 3/3] drm/i915/gem: Extract transient execbuf flags from i915_vma Chris Wilson
2019-11-06 15:48   ` [Intel-gfx] " Chris Wilson
2019-11-08  9:53   ` Daniel Vetter
2019-11-08  9:53     ` [Intel-gfx] " Daniel Vetter
2019-11-08 10:05     ` Chris Wilson
2019-11-08 10:05       ` [Intel-gfx] " Chris Wilson
2019-11-08 16:13       ` Daniel Vetter
2019-11-08 16:13         ` [Intel-gfx] " Daniel Vetter
2019-11-06 19:22 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Handle i915_active_fence_set() with the same fence Patchwork
2019-11-06 19:22   ` [Intel-gfx] " Patchwork
2019-11-08 10:37 ` [PATCH 1/3] " Tvrtko Ursulin
2019-11-08 10:37   ` [Intel-gfx] " Tvrtko Ursulin
2019-11-08 10:42   ` Chris Wilson
2019-11-08 10:42     ` [Intel-gfx] " Chris Wilson
2019-11-08 12:00 ` ✗ Fi.CI.IGT: failure for series starting with [1/3] " Patchwork
2019-11-08 12:00   ` [Intel-gfx] " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.