[Intel-gfx] [PATCH i-g-t v2] i915/gem_engine_topology: Introduce and use gem_context_clone_with_engines

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

In test cases which create new contexts and submit work against them using
the passed in engine index we are sometimes unsure whether this engine
index was potentially created based on a default context with engine map
configured (such as when under the __for_each_physical_engine iterator.

To simplify test code we add gem_context/queue_clone_with_engines which
is to be used in such scenario instead of the current pattern of
gem_context_create followed by gem_context_set_all_engines (which is also
removed by the patch).

v2:
 * Fix swapped arguments to gem_context_clone. (Chris)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/i915/gem_context.c         | 59 ++++++++++++++++++++++++++++++++++
 lib/i915/gem_context.h         |  4 +++
 lib/i915/gem_engine_topology.c | 11 -------
 lib/i915/gem_engine_topology.h |  2 --
 tests/i915/gem_ctx_clone.c     | 15 +--------
 tests/i915/gem_ctx_switch.c    | 19 ++++-------
 tests/i915/gem_exec_parallel.c |  3 +-
 tests/i915/gem_spin_batch.c    | 11 +++----
 tests/perf_pmu.c               |  3 +-
 9 files changed, 76 insertions(+), 51 deletions(-)

diff --git a/lib/i915/gem_context.c b/lib/i915/gem_context.c
index 1fae5191f83f..0b6a554dfe27 100644
--- a/lib/i915/gem_context.c
+++ b/lib/i915/gem_context.c
@@ -372,6 +372,50 @@ uint32_t gem_context_clone(int i915,
 	return ctx;
 }
 
+bool gem_has_context_clone(int i915)
+{
+	struct drm_i915_gem_context_create_ext_clone ext = {
+		{ .name = I915_CONTEXT_CREATE_EXT_CLONE },
+		.clone_id = -1,
+	};
+	struct drm_i915_gem_context_create_ext create = {
+		.flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
+		.extensions = to_user_pointer(&ext),
+	};
+	int err;
+
+	err = 0;
+	if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, &create)) {
+		err = -errno;
+		igt_assume(err);
+	}
+	errno = 0;
+
+	return err == -ENOENT;
+}
+
+/**
+ * gem_context_clone_with_engines:
+ * @i915: open i915 drm file descriptor
+ * @src: i915 context id
+ *
+ * Special purpose wrapper to create a new context by cloning engines from @src.
+ *
+ * In can be called regardless of whether the kernel supports context cloning.
+ *
+ * Intended purpose is to use for creating contexts against which work will be
+ * submitted and the engine index came from external source, derived from a
+ * default context potentially configured with an engine map.
+ */
+uint32_t gem_context_clone_with_engines(int i915, uint32_t src)
+{
+	if (!gem_has_context_clone(i915))
+		return gem_context_create(i915);
+	else
+		return gem_context_clone(i915, src, I915_CONTEXT_CLONE_ENGINES,
+					 0);
+}
+
 uint32_t gem_queue_create(int i915)
 {
 	return gem_context_clone(i915, 0,
@@ -379,6 +423,21 @@ uint32_t gem_queue_create(int i915)
 				 I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE);
 }
 
+/**
+ * gem_queue_clone_with_engines:
+ * @i915: open i915 drm file descriptor
+ * @src: i915 context id
+ *
+ * See gem_context_clone_with_engines.
+ */
+uint32_t gem_queue_clone_with_engines(int i915, uint32_t src)
+{
+	return gem_context_clone(i915, src,
+				 I915_CONTEXT_CLONE_ENGINES |
+				 I915_CONTEXT_CLONE_VM,
+				 I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE);
+}
+
 bool gem_context_has_engine(int fd, uint32_t ctx, uint64_t engine)
 {
 	struct drm_i915_gem_exec_object2 exec = {};
diff --git a/lib/i915/gem_context.h b/lib/i915/gem_context.h
index c0d4c9615677..cf2ba33fee8f 100644
--- a/lib/i915/gem_context.h
+++ b/lib/i915/gem_context.h
@@ -41,8 +41,10 @@ int __gem_context_clone(int i915,
 uint32_t gem_context_clone(int i915,
 			   uint32_t src, unsigned int share,
 			   unsigned int flags);
+uint32_t gem_context_clone_with_engines(int i915, uint32_t src);
 
 uint32_t gem_queue_create(int i915);
+uint32_t gem_queue_clone_with_engines(int i915, uint32_t src);
 
 bool gem_contexts_has_shared_gtt(int i915);
 bool gem_has_queues(int i915);
@@ -52,6 +54,8 @@ void gem_require_contexts(int fd);
 void gem_context_require_bannable(int fd);
 void gem_context_require_param(int fd, uint64_t param);
 
+bool gem_has_context_clone(int i915);
+
 void gem_context_get_param(int fd, struct drm_i915_gem_context_param *p);
 void gem_context_set_param(int fd, struct drm_i915_gem_context_param *p);
 int __gem_context_set_param(int fd, struct drm_i915_gem_context_param *p);
diff --git a/lib/i915/gem_engine_topology.c b/lib/i915/gem_engine_topology.c
index 989a6e26d6ef..43a99e0ff680 100644
--- a/lib/i915/gem_engine_topology.c
+++ b/lib/i915/gem_engine_topology.c
@@ -274,17 +274,6 @@ int gem_context_lookup_engine(int fd, uint64_t engine, uint32_t ctx_id,
 	return 0;
 }
 
-void gem_context_set_all_engines(int fd, uint32_t ctx)
-{
-	DEFINE_CONTEXT_ENGINES_PARAM(engines, param, ctx, GEM_MAX_ENGINES);
-	struct intel_engine_data engine_data = { };
-
-	if (!gem_topology_get_param(fd, &param) && !param.size) {
-		query_engine_list(fd, &engine_data);
-		ctx_map_engines(fd, &engine_data, &param);
-	}
-}
-
 bool gem_has_engine_topology(int fd)
 {
 	struct drm_i915_gem_context_param param = {
diff --git a/lib/i915/gem_engine_topology.h b/lib/i915/gem_engine_topology.h
index 525741cc8a08..e40d7ec8320c 100644
--- a/lib/i915/gem_engine_topology.h
+++ b/lib/i915/gem_engine_topology.h
@@ -51,8 +51,6 @@ void intel_next_engine(struct intel_engine_data *ed);
 int gem_context_lookup_engine(int fd, uint64_t engine, uint32_t ctx_id,
 			      struct intel_execution_engine2 *e);
 
-void gem_context_set_all_engines(int fd, uint32_t ctx);
-
 bool gem_context_has_engine_map(int fd, uint32_t ctx);
 
 bool gem_engine_is_equal(const struct intel_execution_engine2 *e1,
diff --git a/tests/i915/gem_ctx_clone.c b/tests/i915/gem_ctx_clone.c
index 896b24dce39c..471031d4245b 100644
--- a/tests/i915/gem_ctx_clone.c
+++ b/tests/i915/gem_ctx_clone.c
@@ -40,19 +40,6 @@ static int ctx_create_ioctl(int i915, struct drm_i915_gem_context_create_ext *ar
 	return err;
 }
 
-static bool has_ctx_clone(int i915)
-{
-	struct drm_i915_gem_context_create_ext_clone ext = {
-		{ .name = I915_CONTEXT_CREATE_EXT_CLONE },
-		.clone_id = -1,
-	};
-	struct drm_i915_gem_context_create_ext create = {
-		.flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
-		.extensions = to_user_pointer(&ext),
-	};
-	return ctx_create_ioctl(i915, &create) == -ENOENT;
-}
-
 static void invalid_clone(int i915)
 {
 	struct drm_i915_gem_context_create_ext_clone ext = {
@@ -436,7 +423,7 @@ igt_main
 		igt_require_gem(i915);
 		gem_require_contexts(i915);
 
-		igt_require(has_ctx_clone(i915));
+		igt_require(gem_has_context_clone(i915));
 		igt_fork_hang_detector(i915);
 	}
 
diff --git a/tests/i915/gem_ctx_switch.c b/tests/i915/gem_ctx_switch.c
index 6bbd24972676..a65d1b02f5ee 100644
--- a/tests/i915/gem_ctx_switch.c
+++ b/tests/i915/gem_ctx_switch.c
@@ -63,10 +63,8 @@ static int measure_qlen(int fd,
 	uint32_t ctx[64];
 	int min = INT_MAX, max = 0;
 
-	for (int i = 0; i < ARRAY_SIZE(ctx); i++) {
-		ctx[i] = gem_context_create(fd);
-		gem_context_set_all_engines(fd, ctx[i]);
-	}
+	for (int i = 0; i < ARRAY_SIZE(ctx); i++)
+		ctx[i] = gem_context_clone_with_engines(fd, 0);
 
 	for (unsigned int n = 0; n < engines->nengines; n++) {
 		uint64_t saved = execbuf->flags;
@@ -130,12 +128,9 @@ static void single(int fd, uint32_t handle,
 
 	for (n = 0; n < 64; n++) {
 		if (flags & QUEUE)
-			contexts[n] = gem_queue_create(fd);
+			contexts[n] = gem_queue_clone_with_engines(fd, 0);
 		else
-			contexts[n] = gem_context_create(fd);
-
-		if (gem_context_has_engine_map(fd, 0))
-			gem_context_set_all_engines(fd, contexts[n]);
+			contexts[n] = gem_context_clone_with_engines(fd, 0);
 	}
 
 	memset(&obj, 0, sizeof(obj));
@@ -237,11 +232,9 @@ static void all(int fd, uint32_t handle, unsigned flags, int timeout)
 
 	for (n = 0; n < ARRAY_SIZE(contexts); n++) {
 		if (flags & QUEUE)
-			contexts[n] = gem_queue_create(fd);
+			contexts[n] = gem_queue_clone_with_engines(fd, 0);
 		else
-			contexts[n] = gem_context_create(fd);
-
-		gem_context_set_all_engines(fd, contexts[n]);
+			contexts[n] = gem_context_clone_with_engines(fd, 0);
 	}
 
 	memset(obj, 0, sizeof(obj));
diff --git a/tests/i915/gem_exec_parallel.c b/tests/i915/gem_exec_parallel.c
index 56b26cf4dd7b..cfbe78070873 100644
--- a/tests/i915/gem_exec_parallel.c
+++ b/tests/i915/gem_exec_parallel.c
@@ -127,8 +127,7 @@ static void *thread(void *data)
 	if (t->gen < 6)
 		execbuf.flags |= I915_EXEC_SECURE;
 	if (t->flags & CONTEXTS) {
-		execbuf.rsvd1 = gem_context_create(fd);
-		gem_context_set_all_engines(fd, execbuf.rsvd1);
+		execbuf.rsvd1 = gem_context_clone_with_engines(fd, 0);
 	}
 
 	for (i = 0; i < 16; i++) {
diff --git a/tests/i915/gem_spin_batch.c b/tests/i915/gem_spin_batch.c
index c5114ac79b0e..1142a77c7d7f 100644
--- a/tests/i915/gem_spin_batch.c
+++ b/tests/i915/gem_spin_batch.c
@@ -73,9 +73,10 @@ static void spin(int fd, const struct intel_execution_engine2 *e2,
 static void spin_resubmit(int fd, const struct intel_execution_engine2 *e2,
 			  unsigned int flags)
 {
-	const uint32_t ctx0 = gem_context_create(fd);
-	const uint32_t ctx1 = (flags & RESUBMIT_NEW_CTX) ?
-		gem_context_create(fd) : ctx0;
+	const uint32_t ctx0 = gem_context_clone_with_engines(fd, 0);
+	const uint32_t ctx1 =
+		(flags & RESUBMIT_NEW_CTX) ?
+		gem_context_clone_with_engines(fd, 0) : ctx0;
 	igt_spin_t *spin = __igt_spin_new(fd, .ctx = ctx0, .engine = e2->flags);
 	const struct intel_execution_engine2 *other;
 
@@ -89,10 +90,6 @@ static void spin_resubmit(int fd, const struct intel_execution_engine2 *e2,
 		   !(flags & RESUBMIT_ALL_ENGINES));
 
 	if (flags & RESUBMIT_ALL_ENGINES) {
-		gem_context_set_all_engines(fd, ctx0);
-		if (ctx0 != ctx1)
-			gem_context_set_all_engines(fd, ctx1);
-
 		for_each_context_engine(fd, ctx1, other) {
 			if (gem_engine_is_equal(other, e2))
 				continue;
diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index 3e179daef9d5..e17efe293f96 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -361,8 +361,7 @@ busy_double_start(int gem_fd, const struct intel_execution_engine2 *e)
 	uint32_t ctx;
 	int fd;
 
-	ctx = gem_context_create(gem_fd);
-	gem_context_set_all_engines(gem_fd, ctx);
+	ctx = gem_context_clone_with_engines(gem_fd, 0);
 
 	/*
 	 * Defeat the busy stats delayed disable, we need to guarantee we are
-- 
2.20.1

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