[PATCH 1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline

[PATCH 1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline

[Chris Wilson]

2 clflushes on two different objects are not ordered, and so do not
belong to the same timeline (context). Either we use a unique context
for each, or we reserve a special global context to mean unordered.
Ideally, we would reserve 0 to mean unordered (DMA_FENCE_NO_CONTEXT) to
have the same semantics everywhere.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         | 2 ++
 drivers/gpu/drm/i915/i915_gem.c         | 2 +-
 drivers/gpu/drm/i915/i915_gem_clflush.c | 8 +-------
 drivers/gpu/drm/i915/i915_gem_clflush.h | 1 -
 4 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bb6fc1e08a52..b47aae031eef 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1511,6 +1511,8 @@ struct i915_gem_mm {
 	/** LRU list of objects with fence regs on them. */
 	struct list_head fence_list;
 
+	u64 unordered_timeline;
+
 	/* the indicator for dispatch video commands on two BSD rings */
 	atomic_t bsd_engine_dispatch_index;
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b210acc3d0b4..79b5eab5ae83 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4681,7 +4681,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 
 	mutex_lock(&dev_priv->drm.struct_mutex);
 
-	i915_gem_clflush_init(dev_priv);
+	dev_priv->mm.unordered_timeline = dma_fence_context_alloc(1);
 
 	if (!i915.enable_execlists) {
 		dev_priv->gt.resume = intel_legacy_submission_resume;
diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c b/drivers/gpu/drm/i915/i915_gem_clflush.c
index ffd01e02fe94..ffac7a1f0caf 100644
--- a/drivers/gpu/drm/i915/i915_gem_clflush.c
+++ b/drivers/gpu/drm/i915/i915_gem_clflush.c
@@ -27,7 +27,6 @@
 #include "i915_gem_clflush.h"
 
 static DEFINE_SPINLOCK(clflush_lock);
-static u64 clflush_context;
 
 struct clflush {
 	struct dma_fence dma; /* Must be first for dma_fence_free() */
@@ -157,7 +156,7 @@ void i915_gem_clflush_object(struct drm_i915_gem_object *obj,
 		dma_fence_init(&clflush->dma,
 			       &i915_clflush_ops,
 			       &clflush_lock,
-			       clflush_context,
+			       to_i915(obj->base.dev)->mm.unordered_timeline,
 			       0);
 		i915_sw_fence_init(&clflush->wait, i915_clflush_notify);
 
@@ -182,8 +181,3 @@ void i915_gem_clflush_object(struct drm_i915_gem_object *obj,
 		GEM_BUG_ON(obj->base.write_domain != I915_GEM_DOMAIN_CPU);
 	}
 }
-
-void i915_gem_clflush_init(struct drm_i915_private *i915)
-{
-	clflush_context = dma_fence_context_alloc(1);
-}
diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.h b/drivers/gpu/drm/i915/i915_gem_clflush.h
index b62d61a2d15f..2455a7820937 100644
--- a/drivers/gpu/drm/i915/i915_gem_clflush.h
+++ b/drivers/gpu/drm/i915/i915_gem_clflush.h
@@ -28,7 +28,6 @@
 struct drm_i915_private;
 struct drm_i915_gem_object;
 
-void i915_gem_clflush_init(struct drm_i915_private *i915);
 void i915_gem_clflush_object(struct drm_i915_gem_object *obj,
 			     unsigned int flags);
 #define I915_CLFLUSH_FORCE BIT(0)
-- 
2.11.0

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

[PATCH 2/5] drm/i915: Lift timeline ordering to await_dma_fence

[Chris Wilson]

Currently we filter out repeated use of the same timeline in the low
level i915_gem_request_await_request(), after having added the
dependency on the old request. However, we can lift this to
i915_gem_request_await_dma_fence() (before the dependency is added)
using the observation that requests along the same timeline are
explicitly ordered via i915_add_request (along with the dependencies).

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

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 313cdff7c6dd..31874a38752e 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -663,6 +663,7 @@ i915_gem_request_await_request(struct drm_i915_gem_request *to,
 	int ret;
 
 	GEM_BUG_ON(to == from);
+	GEM_BUG_ON(to->timeline == from->timeline);
 
 	if (to->engine->schedule) {
 		ret = i915_priotree_add_dependency(to->i915,
@@ -672,9 +673,6 @@ i915_gem_request_await_request(struct drm_i915_gem_request *to,
 			return ret;
 	}
 
-	if (to->timeline == from->timeline)
-		return 0;
-
 	if (to->engine == from->engine) {
 		ret = i915_sw_fence_await_sw_fence_gfp(&to->submit,
 						       &from->submit,
@@ -723,6 +721,13 @@ i915_gem_request_await_dma_fence(struct drm_i915_gem_request *req,
 	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
 		return 0;
 
+	/* Requests on the same timeline are explicitly ordered, along with
+	 * their dependencies, by i915_add_request() which ensures that requests
+	 * are submitted in-order through each ring.
+	 */
+	if (fence->context == req->fence.context)
+		return 0;
+
 	if (dma_fence_is_i915(fence))
 		return i915_gem_request_await_request(req, to_request(fence));
 
-- 
2.11.0

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

[PATCH 3/5] drm/i915: Make ptr_unpack_bits() more function-like

[Chris Wilson]

ptr_unpack_bits() is a function-like macro, as such it is meant to be
replaceable by a function. In this case, we should be passing in the
out-param as a pointer.

Bizarrely this does affect code generation:

function                                     old     new   delta
i915_gem_object_pin_map                      409     389     -20

An improvement(?) in this case, but one can't help wonder what
strict-aliasing optimisations we are preventing.

The generated code looks identical in using ptr_unpack_bits (no extra
motions to stack, the pointer and bits appear to be kept in registers),
the difference appears to be code ordering and with a reorder it is able
to use smaller forward jumps.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c   | 2 +-
 drivers/gpu/drm/i915/i915_utils.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 79b5eab5ae83..f39b36a8622a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2560,7 +2560,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj,
 	}
 	GEM_BUG_ON(!obj->mm.pages);
 
-	ptr = ptr_unpack_bits(obj->mm.mapping, has_type);
+	ptr = ptr_unpack_bits(obj->mm.mapping, &has_type);
 	if (ptr && has_type != type) {
 		if (pinned) {
 			ret = -EBUSY;
diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index c5455d36b617..aca11aad5da7 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -77,7 +77,7 @@
 
 #define ptr_unpack_bits(ptr, bits) ({					\
 	unsigned long __v = (unsigned long)(ptr);			\
-	(bits) = __v & ~PAGE_MASK;					\
+	*(bits) = __v & ~PAGE_MASK;					\
 	(typeof(ptr))(__v & PAGE_MASK);					\
 })
 
-- 
2.11.0

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

[PATCH 4/5] drm/i915: Redefine ptr_pack_bits() and friends

[Chris Wilson]

Rebrand the current (pointer | bits) pack/unpack utility macros as
explicit bit twiddling for PAGE_SIZE so that we can use the more
flexible underlying macros for different bits.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_cmd_parser.c |  2 +-
 drivers/gpu/drm/i915/i915_gem.c        |  6 +++---
 drivers/gpu/drm/i915/i915_utils.h      | 19 +++++++++++++------
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 7af100f84410..1b69a1f9a1ea 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1284,7 +1284,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 
 		if (*cmd == MI_BATCH_BUFFER_END) {
 			if (needs_clflush_after) {
-				void *ptr = ptr_mask_bits(shadow_batch_obj->mm.mapping);
+				void *ptr = page_mask_bits(shadow_batch_obj->mm.mapping);
 				drm_clflush_virt_range(ptr,
 						       (void *)(cmd + 1) - ptr);
 			}
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f39b36a8622a..3c0e63234021 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2228,7 +2228,7 @@ void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
 	if (obj->mm.mapping) {
 		void *ptr;
 
-		ptr = ptr_mask_bits(obj->mm.mapping);
+		ptr = page_mask_bits(obj->mm.mapping);
 		if (is_vmalloc_addr(ptr))
 			vunmap(ptr);
 		else
@@ -2560,7 +2560,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj,
 	}
 	GEM_BUG_ON(!obj->mm.pages);
 
-	ptr = ptr_unpack_bits(obj->mm.mapping, &has_type);
+	ptr = page_unpack_bits(obj->mm.mapping, &has_type);
 	if (ptr && has_type != type) {
 		if (pinned) {
 			ret = -EBUSY;
@@ -2582,7 +2582,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj,
 			goto err_unpin;
 		}
 
-		obj->mm.mapping = ptr_pack_bits(ptr, type);
+		obj->mm.mapping = page_pack_bits(ptr, type);
 	}
 
 out_unlock:
diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index aca11aad5da7..f0500c65726d 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -70,20 +70,27 @@
 #define overflows_type(x, T) \
 	(sizeof(x) > sizeof(T) && (x) >> (sizeof(T) * BITS_PER_BYTE))
 
-#define ptr_mask_bits(ptr) ({						\
+#define ptr_mask_bits(ptr, n) ({					\
 	unsigned long __v = (unsigned long)(ptr);			\
-	(typeof(ptr))(__v & PAGE_MASK);					\
+	(typeof(ptr))(__v & -BIT(n));					\
 })
 
-#define ptr_unpack_bits(ptr, bits) ({					\
+#define ptr_unmask_bits(ptr, n) ((unsigned long)(ptr) & (BIT(n) - 1))
+
+#define ptr_unpack_bits(ptr, bits, n) ({				\
 	unsigned long __v = (unsigned long)(ptr);			\
-	*(bits) = __v & ~PAGE_MASK;					\
-	(typeof(ptr))(__v & PAGE_MASK);					\
+	*(bits) = __v & (BIT(n) - 1);					\
+	(typeof(ptr))(__v & -BIT(n));					\
 })
 
-#define ptr_pack_bits(ptr, bits)					\
+#define ptr_pack_bits(ptr, bits, n)					\
 	((typeof(ptr))((unsigned long)(ptr) | (bits)))
 
+#define page_mask_bits(ptr) ptr_mask_bits(ptr, PAGE_SHIFT)
+#define page_unmask_bits(ptr) ptr_unmask_bits(ptr, PAGE_SHIFT)
+#define page_pack_bits(ptr, bits) ptr_pack_bits(ptr, bits, PAGE_SHIFT)
+#define page_unpack_bits(ptr, bits) ptr_unpack_bits(ptr, bits, PAGE_SHIFT)
+
 #define ptr_offset(ptr, member) offsetof(typeof(*(ptr)), member)
 
 #define fetch_and_zero(ptr) ({						\
-- 
2.11.0

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

[PATCH 5/5] drm/i915: Squash repeated awaits on the same fence

[Chris Wilson]

Track the latest fence waited upon on each context, and only add a new
asynchronous wait if the new fence is more recent than the recorded
fence for that context. This requires us to filter out unordered
timelines, which are noted by DMA_FENCE_NO_CONTEXT. However, in the
absence of a universal identifier, we have to use our own
i915->mm.unordered_timeline token.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_request.c            |  72 +++---
 drivers/gpu/drm/i915/i915_gem_timeline.c           | 255 +++++++++++++++++++++
 drivers/gpu/drm/i915/i915_gem_timeline.h           |  23 ++
 drivers/gpu/drm/i915/selftests/i915_gem_timeline.c | 127 ++++++++++
 .../gpu/drm/i915/selftests/i915_mock_selftests.h   |   1 +
 lib/radix-tree.c                                   |   1 +
 6 files changed, 452 insertions(+), 27 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/selftests/i915_gem_timeline.c

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 31874a38752e..6e68387aa097 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -714,9 +714,7 @@ int
 i915_gem_request_await_dma_fence(struct drm_i915_gem_request *req,
 				 struct dma_fence *fence)
 {
-	struct dma_fence_array *array;
 	int ret;
-	int i;
 
 	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
 		return 0;
@@ -728,39 +726,59 @@ i915_gem_request_await_dma_fence(struct drm_i915_gem_request *req,
 	if (fence->context == req->fence.context)
 		return 0;
 
-	if (dma_fence_is_i915(fence))
-		return i915_gem_request_await_request(req, to_request(fence));
+	/* Squash repeated waits to the same timelines, picking the latest */
+	if (fence->context != req->i915->mm.unordered_timeline) {
+		if (intel_timeline_sync_get(req->timeline,
+					    fence->context,
+					    fence->seqno))
+			return 0;
 
-	if (!dma_fence_is_array(fence)) {
+		ret = intel_timeline_sync_reserve(req->timeline);
+		if (unlikely(ret))
+			return ret;
+	}
+
+	if (dma_fence_is_i915(fence)) {
+		ret = i915_gem_request_await_request(req, to_request(fence));
+		if (ret < 0)
+			return ret;
+	} else if (!dma_fence_is_array(fence)) {
 		ret = i915_sw_fence_await_dma_fence(&req->submit,
 						    fence, I915_FENCE_TIMEOUT,
 						    GFP_KERNEL);
-		return ret < 0 ? ret : 0;
-	}
-
-	/* Note that if the fence-array was created in signal-on-any mode,
-	 * we should *not* decompose it into its individual fences. However,
-	 * we don't currently store which mode the fence-array is operating
-	 * in. Fortunately, the only user of signal-on-any is private to
-	 * amdgpu and we should not see any incoming fence-array from
-	 * sync-file being in signal-on-any mode.
-	 */
-
-	array = to_dma_fence_array(fence);
-	for (i = 0; i < array->num_fences; i++) {
-		struct dma_fence *child = array->fences[i];
-
-		if (dma_fence_is_i915(child))
-			ret = i915_gem_request_await_request(req,
-							     to_request(child));
-		else
-			ret = i915_sw_fence_await_dma_fence(&req->submit,
-							    child, I915_FENCE_TIMEOUT,
-							    GFP_KERNEL);
 		if (ret < 0)
 			return ret;
+	} else {
+		struct dma_fence_array *array = to_dma_fence_array(fence);
+		int i;
+
+		/* Note that if the fence-array was created in signal-on-any mode,
+		 * we should *not* decompose it into its individual fences. However,
+		 * we don't currently store which mode the fence-array is operating
+		 * in. Fortunately, the only user of signal-on-any is private to
+		 * amdgpu and we should not see any incoming fence-array from
+		 * sync-file being in signal-on-any mode.
+		 */
+
+		for (i = 0; i < array->num_fences; i++) {
+			struct dma_fence *child = array->fences[i];
+
+			if (dma_fence_is_i915(child))
+				ret = i915_gem_request_await_request(req,
+								     to_request(child));
+			else
+				ret = i915_sw_fence_await_dma_fence(&req->submit,
+								    child, I915_FENCE_TIMEOUT,
+								    GFP_KERNEL);
+			if (ret < 0)
+				return ret;
+		}
 	}
 
+	if (fence->context != req->i915->mm.unordered_timeline)
+		intel_timeline_sync_set(req->timeline,
+					fence->context, fence->seqno);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_timeline.c b/drivers/gpu/drm/i915/i915_gem_timeline.c
index b596ca7ee058..30b4d07a5444 100644
--- a/drivers/gpu/drm/i915/i915_gem_timeline.c
+++ b/drivers/gpu/drm/i915/i915_gem_timeline.c
@@ -24,6 +24,255 @@
 
 #include "i915_drv.h"
 
+#if 0
+#define assert(x) BUG_ON(!(x))
+#define dbg(x) pr_err x
+#else
+#define assert(x) do { } while (0)
+#define dbg(x) do { } while (0)
+#endif
+
+#define SHIFT ilog2(NSEQMAP)
+#define MASK (NSEQMAP - 1)
+
+static void seqmap_free_layers(struct seqmap_layer *p)
+{
+	unsigned int i;
+
+	if (p->height) {
+		for (; (i = ffs(p->bitmap)); p->bitmap &= ~0u << i)
+			seqmap_free_layers(p->slot[i - 1]);
+	}
+
+	kfree(p);
+}
+
+static void seqmap_free(struct seqmap *seqmap)
+{
+	if (seqmap->top)
+		seqmap_free_layers(seqmap->top);
+
+	while (seqmap->freed) {
+		struct seqmap_layer *p;
+
+		p = ptr_mask_bits(seqmap->freed, 2);
+		seqmap->freed = p->parent;
+		kfree(p);
+	}
+}
+
+__malloc static struct seqmap_layer *
+seqmap_alloc_layer(struct seqmap *shared)
+{
+	struct seqmap_layer *p;
+
+	assert(shared->freed);
+
+	p = ptr_mask_bits(shared->freed, 2);
+	shared->freed = p->parent;
+
+	return p;
+}
+
+static int layer_idx(const struct seqmap_layer *p, u64 id)
+{
+	return (id >> p->height) & MASK;
+}
+
+bool intel_timeline_sync_get(struct intel_timeline *tl, u64 id, u32 seqno)
+{
+	struct seqmap *shared = &tl->sync;
+	struct seqmap_layer *p;
+	unsigned int idx;
+
+	dbg(("%s(id=%llx, top? %d)\n", __func__, id, !!shared->top));
+
+	if (!shared->top)
+		return false;
+
+	/* First see if this fence is in the same layer as the previous fence */
+	p = shared->hint;
+	if ((id >> SHIFT) == p->prefix)
+		goto found;
+
+	p = shared->top;
+	do {
+		dbg(("id=%llx, p->(prefix=%llx, height=%d), delta=%llx, idx=%x\n",
+		     id, p->prefix, p->height,
+		     id >> p->height >> SHIFT,
+		     layer_idx(p, id)));
+
+		if ((id >> p->height >> SHIFT) != p->prefix)
+			return false;
+
+		if (!p->height) /* matching base layer */
+			break;
+
+		/* descend into the next layer */
+		p = p->slot[layer_idx(p, id)];
+		if (unlikely(!p))
+			return false;
+	} while (1);
+
+	shared->hint = p;
+found:
+	idx = id & MASK;
+	dbg(("id=%llx, found layer [idx=%u], present? %lu\n",
+	     id, idx, p->bitmap & BIT(idx)));
+	if (!(p->bitmap & BIT(idx)))
+		return false;
+
+	return i915_seqno_passed((uintptr_t)p->slot[idx], seqno);
+}
+
+void intel_timeline_sync_set(struct intel_timeline *tl, u64 id, u32 seqno)
+{
+	struct seqmap *shared = &tl->sync;
+	struct seqmap_layer *p, *cur;
+	unsigned int idx;
+
+	dbg(("%s(id=%llx)\n", __func__, id));
+
+	/* First see if this fence is in the same layer as the previous fence */
+	p = shared->hint;
+	if (p && (id >> SHIFT) == p->prefix) {
+		assert(p->height == 0);
+		goto found_layer;
+	}
+
+	p = shared->top;
+	if (!p) {
+		cur = seqmap_alloc_layer(shared);
+		cur->parent = NULL;
+		shared->top = cur;
+		goto new_layer;
+	}
+
+	/* No shortcut, we have to descend the tree to find the right layer
+	 * containing this fence.
+	 *
+	 * Each layer in the tree holds 16 (NSHARED) pointers, either fences
+	 * or lower layers. Leaf nodes (height = 0) contain the fences, all
+	 * other nodes (height > 0) are internal layers that point to a lower
+	 * node. Each internal layer has at least 2 descendents.
+	 *
+	 * Starting at the top, we check whether the current prefix matches. If
+	 * it doesn't, we have gone passed our layer and need to insert a join
+	 * into the tree, and a new leaf node as a descendent as well as the
+	 * original layer.
+	 *
+	 * The matching prefix means we are still following the right branch
+	 * of the tree. If it has height 0, we have found our leaf and just
+	 * need to replace the fence slot with ourselves. If the height is
+	 * not zero, our slot contains the next layer in the tree (unless
+	 * it is empty, in which case we can add ourselves as a new leaf).
+	 * As descend the tree the prefix grows (and height decreases).
+	 */
+	do {
+		dbg(("id=%llx, p->(prefix=%llx, height=%d), delta=%llx, idx=%x\n",
+		     id, p->prefix, p->height,
+		     id >> p->height >> SHIFT,
+		     layer_idx(p, id)));
+
+		if ((id >> p->height >> SHIFT) != p->prefix) {
+			/* insert a join above the current layer */
+			cur = seqmap_alloc_layer(shared);
+			cur->height = ALIGN(fls64((id >> p->height >> SHIFT) ^ p->prefix),
+					    SHIFT) + p->height;
+			cur->prefix = id >> cur->height >> SHIFT;
+
+			dbg(("id=%llx, join prefix=%llu, height=%d\n",
+			     id, cur->prefix, cur->height));
+
+			assert((id >> cur->height >> SHIFT) == cur->prefix);
+			assert(cur->height > p->height);
+
+			if (p->parent) {
+				assert(p->parent->slot[layer_idx(p->parent, id)] == p);
+				assert(cur->height < p->parent->height);
+				p->parent->slot[layer_idx(p->parent, id)] = cur;
+			} else {
+				shared->top = cur;
+			}
+			cur->parent = p->parent;
+
+			idx = p->prefix >> (cur->height - p->height - SHIFT) & MASK;
+			cur->slot[idx] = p;
+			cur->bitmap |= BIT(idx);
+			p->parent = cur;
+		} else if (!p->height) {
+			/* matching base layer */
+			shared->hint = p;
+			goto found_layer;
+		} else {
+			/* descend into the next layer */
+			idx = layer_idx(p, id);
+			cur = p->slot[idx];
+			if (unlikely(!cur)) {
+				cur = seqmap_alloc_layer(shared);
+				p->slot[idx] = cur;
+				p->bitmap |= BIT(idx);
+				cur->parent = p;
+				goto new_layer;
+			}
+		}
+
+		p = cur;
+	} while (1);
+
+found_layer:
+	assert(p->height == 0);
+	idx = id & MASK;
+	p->slot[idx] = (void *)(uintptr_t)seqno;
+	p->bitmap |= BIT(idx);
+	dbg(("id=%llx, found existing layer prefix=%llx, idx=%x [bitmap=%x]\n",
+	     id, p->prefix, idx, p->bitmap));
+	return;
+
+new_layer:
+	assert(cur->height == 0);
+	assert(cur->bitmap == 0);
+	idx = id & MASK;
+	cur->prefix = id >> SHIFT;
+	cur->slot[idx] = (void *)(uintptr_t)seqno;
+	cur->bitmap = BIT(idx);
+	shared->hint = cur;
+	dbg(("id=%llx, new layer prefix=%llx, idx=%x [bitmap=%x]\n",
+	     id, cur->prefix, (int)(id & MASK), cur->bitmap));
+	return;
+}
+
+int intel_timeline_sync_reserve(struct intel_timeline *tl)
+{
+	struct seqmap *shared = &tl->sync;
+	int count;
+
+	might_sleep();
+
+	/* To guarantee being able to replace a fence in the radixtree,
+	 * we need at most 2 layers: one to create a join in the tree,
+	 * and one to contain the fence. Typically we expect to reuse
+	 * a layer and so avoid any insertions.
+	 *
+	 * We use the low bits of the freed list to track its length
+	 * since we only need a couple of bits.
+	 */
+	count = ptr_unmask_bits(shared->freed, 2);
+	while (count++ < 2) {
+		struct seqmap_layer *p;
+
+		p = kzalloc(sizeof(*p), GFP_KERNEL);
+		if (unlikely(!p))
+			return -ENOMEM;
+
+		p->parent = shared->freed;
+		shared->freed = ptr_pack_bits(p, count, 2);
+	}
+
+	return 0;
+}
+
+
 static int __i915_gem_timeline_init(struct drm_i915_private *i915,
 				    struct i915_gem_timeline *timeline,
 				    const char *name,
@@ -91,8 +340,14 @@ void i915_gem_timeline_fini(struct i915_gem_timeline *timeline)
 		struct intel_timeline *tl = &timeline->engine[i];
 
 		GEM_BUG_ON(!list_empty(&tl->requests));
+
+		seqmap_free(&tl->sync);
 	}
 
 	list_del(&timeline->link);
 	kfree(timeline->name);
 }
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftests/i915_gem_timeline.c"
+#endif
diff --git a/drivers/gpu/drm/i915/i915_gem_timeline.h b/drivers/gpu/drm/i915/i915_gem_timeline.h
index 6c53e14cab2a..5d0e37f212ac 100644
--- a/drivers/gpu/drm/i915/i915_gem_timeline.h
+++ b/drivers/gpu/drm/i915/i915_gem_timeline.h
@@ -26,11 +26,28 @@
 #define I915_GEM_TIMELINE_H
 
 #include <linux/list.h>
+#include <linux/radix-tree.h>
 
 #include "i915_gem_request.h"
 
 struct i915_gem_timeline;
 
+#define NSEQMAP 16
+
+struct seqmap_layer {
+	u64 prefix;
+	unsigned int height;
+	unsigned int bitmap;
+	struct seqmap_layer *parent;
+	void *slot[NSEQMAP];
+};
+
+struct seqmap {
+	struct seqmap_layer *hint;
+	struct seqmap_layer *top;
+	struct seqmap_layer *freed;
+};
+
 struct intel_timeline {
 	u64 fence_context;
 	u32 seqno;
@@ -55,6 +72,8 @@ struct intel_timeline {
 	 * struct_mutex.
 	 */
 	struct i915_gem_active last_request;
+
+	struct seqmap sync;
 	u32 sync_seqno[I915_NUM_ENGINES];
 
 	struct i915_gem_timeline *common;
@@ -75,4 +94,8 @@ int i915_gem_timeline_init(struct drm_i915_private *i915,
 int i915_gem_timeline_init__global(struct drm_i915_private *i915);
 void i915_gem_timeline_fini(struct i915_gem_timeline *tl);
 
+bool intel_timeline_sync_get(struct intel_timeline *tl, u64 id, u32 seqno);
+int intel_timeline_sync_reserve(struct intel_timeline *tl);
+void intel_timeline_sync_set(struct intel_timeline *tl, u64 id, u32 seqno);
+
 #endif
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_timeline.c b/drivers/gpu/drm/i915/selftests/i915_gem_timeline.c
new file mode 100644
index 000000000000..15e2b2e63081
--- /dev/null
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_timeline.c
@@ -0,0 +1,127 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "../i915_selftest.h"
+#include "mock_gem_device.h"
+
+static int igt_seqmap(void *arg)
+{
+	struct drm_i915_private *i915 = arg;
+	const struct {
+		const char *name;
+		u32 seqno;
+		bool expected;
+		bool set;
+	} pass[] = {
+		{ "unset", 0, false, false },
+		{ "new", 0, false, true },
+		{ "0a", 0, true, true },
+		{ "1a", 1, false, true },
+		{ "1b", 1, true, true },
+		{ "0b", 0, true, false },
+		{ "2a", 2, false, true },
+		{ "4", 4, false, true },
+		{ "INT_MAX", INT_MAX, false, true },
+		{ "INT_MAX-1", INT_MAX-1, true, false },
+		{ "INT_MAX+1", (u32)INT_MAX+1, false, true },
+		{ "INT_MAX", INT_MAX, true, false },
+		{ "UINT_MAX", UINT_MAX, false, true },
+		{ "wrap", 0, false, true },
+		{ "unwrap", UINT_MAX, true, false },
+		{},
+	}, *p;
+	struct intel_timeline *tl;
+	int order, offset;
+	int ret;
+
+	tl = &i915->gt.global_timeline.engine[RCS];
+	for (p = pass; p->name; p++) {
+		for (order = 1; order < 64; order++) {
+			for (offset = -1; offset <= (order > 1); offset++) {
+				u64 ctx = BIT_ULL(order) + offset;
+
+				if (intel_timeline_sync_get(tl,
+							    ctx,
+							    p->seqno) != p->expected) {
+					pr_err("1: %s(ctx=%llu, seqno=%u) expected passed %s but failed\n",
+					       p->name, ctx, p->seqno, yesno(p->expected));
+					return -EINVAL;
+				}
+
+				if (p->set) {
+					ret = intel_timeline_sync_reserve(tl);
+					if (ret)
+						return ret;
+
+					intel_timeline_sync_set(tl, ctx, p->seqno);
+				}
+			}
+		}
+	}
+
+	tl = &i915->gt.global_timeline.engine[BCS];
+	for (order = 1; order < 64; order++) {
+		for (offset = -1; offset <= (order > 1); offset++) {
+			u64 ctx = BIT_ULL(order) + offset;
+
+			for (p = pass; p->name; p++) {
+				if (intel_timeline_sync_get(tl,
+							    ctx,
+							    p->seqno) != p->expected) {
+					pr_err("2: %s(ctx=%llu, seqno=%u) expected passed %s but failed\n",
+					       p->name, ctx, p->seqno, yesno(p->expected));
+					return -EINVAL;
+				}
+
+				if (p->set) {
+					ret = intel_timeline_sync_reserve(tl);
+					if (ret)
+						return ret;
+
+					intel_timeline_sync_set(tl, ctx, p->seqno);
+				}
+			}
+		}
+	}
+
+	return 0;
+}
+
+int i915_gem_timeline_mock_selftests(void)
+{
+	static const struct i915_subtest tests[] = {
+		SUBTEST(igt_seqmap),
+	};
+	struct drm_i915_private *i915;
+	int err;
+
+	i915 = mock_gem_device();
+	if (!i915)
+		return -ENOMEM;
+
+	err = i915_subtests(tests, i915);
+	drm_dev_unref(&i915->drm);
+
+	return err;
+}
diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
index be9a9ebf5692..8d0f50c25df8 100644
--- a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
@@ -12,6 +12,7 @@ selftest(sanitycheck, i915_mock_sanitycheck) /* keep first (igt selfcheck) */
 selftest(scatterlist, scatterlist_mock_selftests)
 selftest(uncore, intel_uncore_mock_selftests)
 selftest(breadcrumbs, intel_breadcrumbs_mock_selftests)
+selftest(timelines, i915_gem_timeline_mock_selftests)
 selftest(requests, i915_gem_request_mock_selftests)
 selftest(objects, i915_gem_object_mock_selftests)
 selftest(dmabuf, i915_gem_dmabuf_mock_selftests)
diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 691a9ad48497..84cccf7138c4 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -2022,6 +2022,7 @@ void radix_tree_iter_delete(struct radix_tree_root *root,
 	if (__radix_tree_delete(root, iter->node, slot))
 		iter->index = iter->next_index;
 }
+EXPORT_SYMBOL(radix_tree_iter_delete);
 
 /**
  * radix_tree_delete_item - delete an item from a radix tree
-- 
2.11.0

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

✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline

[Patchwork]

== Series Details ==

Series: series starting with [1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline
URL   : https://patchwork.freedesktop.org/series/22759/
State : success

== Summary ==

Series 22759v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/22759/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-snb-2600) fdo#100125

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:427s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:429s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:585s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:500s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:539s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:487s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:484s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:407s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:405s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:420s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:490s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:463s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:457s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:567s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:449s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:572s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:457s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:496s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:433s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:530s
fi-snb-2600      total:278  pass:248  dwarn:1   dfail:0   fail:0   skip:29  time:417s

d86f5fa557fdf089bdb5b51f625a7f5967f11015 drm-tip: 2017y-04m-10d-06h-38m-49s UTC integration manifest
2788baf drm/i915: Squash repeated awaits on the same fence
926bcad drm/i915: Redefine ptr_pack_bits() and friends
5a4ed59 drm/i915: Make ptr_unpack_bits() more function-like
71fdbdc drm/i915: Lift timeline ordering to await_dma_fence
a64db54 drm/i915: Mark up clflushes as belonging to an unordered timeline

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4454/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/5] drm/i915: Mark up clflushes as belonging to an unordered timeline

[Joonas Lahtinen]

On ma, 2017-04-10 at 09:55 +0100, Chris Wilson wrote:
> 2 clflushes on two different objects are not ordered, and so do not
> belong to the same timeline (context). Either we use a unique context
> for each, or we reserve a special global context to mean unordered.
> Ideally, we would reserve 0 to mean unordered (DMA_FENCE_NO_CONTEXT) to
> have the same semantics everywhere.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

<SNIP>

> @@ -157,7 +156,7 @@ void i915_gem_clflush_object(struct drm_i915_gem_object *obj,
>  		dma_fence_init(&clflush->dma,
>  			       &i915_clflush_ops,
>  			       &clflush_lock,
> -			       clflush_context,
> +			       to_i915(obj->base.dev)->mm.unordered_timeline,

It seems you have high confidence on being able to replace this line
with DMA_FENCE_NO_CONTEXT ;)

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/5] drm/i915: Lift timeline ordering to await_dma_fence

[Joonas Lahtinen]

On ma, 2017-04-10 at 09:55 +0100, Chris Wilson wrote:
> Currently we filter out repeated use of the same timeline in the low
> level i915_gem_request_await_request(), after having added the
> dependency on the old request. However, we can lift this to
> i915_gem_request_await_dma_fence() (before the dependency is added)
> using the observation that requests along the same timeline are
> explicitly ordered via i915_add_request (along with the dependencies).
> 
> 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>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx