diff mbox series

[i-g-t,3/7] lib/i915: Add query to detect if engine accepts only ro batches

Message ID 20191113154913.8787-3-mika.kuoppala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [i-g-t,1/7] lib/igt_dummyload: Send batch as first | expand

Commit Message

Mika Kuoppala Nov. 13, 2019, 3:49 p.m. UTC 
From: "Kuoppala, Mika" <mika.kuoppala@intel.com>

If cmd parser is mandatory, batch can't be modified post execbuf.
Some tests rely on modifying batch post execbuf. Give those
tests a method to query if those modifications ever reach
the actual engine command stream.

v2: pull in the test changes, doh
v3: class based query
v4: rebase

Signed-off-by: Kuoppala, Mika <mika.kuoppala@intel.com>
---
 lib/i915/gem_submission.c      | 62 ++++++++++++++++++++++++++++++++++
 lib/i915/gem_submission.h      |  2 ++
 tests/i915/gem_busy.c          |  7 +++-
 tests/i915/gem_exec_async.c    |  3 ++
 tests/i915/gem_exec_await.c    |  7 +++-
 tests/i915/gem_exec_fence.c    |  8 +++++
 tests/i915/gem_exec_latency.c  |  7 ++++
 tests/i915/gem_exec_nop.c      |  4 ++-
 tests/i915/gem_exec_schedule.c |  6 +++-
 tests/i915/gem_exec_whisper.c  |  4 ++-
 tests/prime_busy.c             |  3 ++
 tests/prime_vgem.c             |  6 ++++
 12 files changed, 114 insertions(+), 5 deletions(-)

Comments

Chris Wilson Nov. 13, 2019, 11:44 p.m. UTC | #1 
Quoting Mika Kuoppala (2019-11-13 15:49:09)
> From: "Kuoppala, Mika" <mika.kuoppala@intel.com>
> 
> If cmd parser is mandatory, batch can't be modified post execbuf.
> Some tests rely on modifying batch post execbuf. Give those
> tests a method to query if those modifications ever reach
> the actual engine command stream.
> 
> v2: pull in the test changes, doh
> v3: class based query
> v4: rebase
> 
> Signed-off-by: Kuoppala, Mika <mika.kuoppala@intel.com>
> ---
>  lib/i915/gem_submission.c      | 62 ++++++++++++++++++++++++++++++++++
>  lib/i915/gem_submission.h      |  2 ++
>  tests/i915/gem_busy.c          |  7 +++-
>  tests/i915/gem_exec_async.c    |  3 ++
>  tests/i915/gem_exec_await.c    |  7 +++-
>  tests/i915/gem_exec_fence.c    |  8 +++++
>  tests/i915/gem_exec_latency.c  |  7 ++++
>  tests/i915/gem_exec_nop.c      |  4 ++-
>  tests/i915/gem_exec_schedule.c |  6 +++-
>  tests/i915/gem_exec_whisper.c  |  4 ++-
>  tests/prime_busy.c             |  3 ++
>  tests/prime_vgem.c             |  6 ++++
>  12 files changed, 114 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/i915/gem_submission.c b/lib/i915/gem_submission.c
> index 4f946493..9bdf28bc 100644
> --- a/lib/i915/gem_submission.c
> +++ b/lib/i915/gem_submission.c
> @@ -64,6 +64,22 @@ static bool has_semaphores(int fd, int dir)
>         return val;
>  }
>  
> +static int cmd_parser_version(int fd)

gem_cmdparser_version() ?

Drop the engine parameter as it turns out you didn't need it.
-Chris
Chris Wilson Nov. 13, 2019, 11:55 p.m. UTC | #2 
Quoting Mika Kuoppala (2019-11-13 15:49:09)
> diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
> index 5c15f177..ca6bef6a 100644
> --- a/tests/i915/gem_exec_schedule.c
> +++ b/tests/i915/gem_exec_schedule.c
> @@ -652,7 +652,8 @@ static void semaphore_noskip(int i915)
>                 igt_spin_t *chain, *spin;
>  
>                 if (eb_ring(inner) == eb_ring(outer) ||
> -                   !gem_can_store_dword(i915, eb_ring(inner)))
> +                   !gem_can_store_dword(i915, eb_ring(inner)) ||
> +                   !gem_engine_has_mutable_submission(i915, eb_ring(inner)))
>                         continue;
>  
>                 chain = __igt_spin_new(i915, .engine = eb_ring(outer));
> @@ -1157,6 +1158,9 @@ static void deep(int fd, unsigned ring)
>         int dep_nreq;
>         int n;
>  
> +       igt_require(gem_can_store_dword(fd, ring));
> +       igt_require(gem_engine_has_mutable_submission(fd, ring));
> +
>         ctx = malloc(sizeof(*ctx) * MAX_CONTEXTS);
>         for (n = 0; n < MAX_CONTEXTS; n++) {
>                 ctx[n] = gem_context_create(fd);

Works, no need to skip.

> diff --git a/tests/i915/gem_exec_whisper.c b/tests/i915/gem_exec_whisper.c
> index a3384760..45cc1437 100644
> --- a/tests/i915/gem_exec_whisper.c
> +++ b/tests/i915/gem_exec_whisper.c
> @@ -204,13 +204,15 @@ static void whisper(int fd, unsigned engine, unsigned flags)
>         nengine = 0;
>         if (engine == ALL_ENGINES) {
>                 for_each_physical_engine(e, fd) {
> -                       if (gem_can_store_dword(fd, eb_ring(e)))
> +                       if (gem_can_store_dword(fd, eb_ring(e)) &&
> +                           gem_engine_has_mutable_submission(fd, eb_ring(e)))
>                                 engines[nengine++] = eb_ring(e);
>                 }
>         } else {
>                 igt_assert(!(flags & ALL));
>                 igt_require(gem_has_ring(fd, engine));
>                 igt_require(gem_can_store_dword(fd, engine));
> +               igt_require(gem_engine_has_mutable_submission(fd, engine));
>                 engines[nengine++] = engine;
>         }
>         igt_require(nengine);

You should be able to handle the whisper; it's one of the basic does the
kernel work at all. All it is testing is relocation and scheduling...

Worksforme.
-Chris
diff mbox series

Patch

diff --git a/lib/i915/gem_submission.c b/lib/i915/gem_submission.c
index 4f946493..9bdf28bc 100644
--- a/lib/i915/gem_submission.c
+++ b/lib/i915/gem_submission.c
@@ -64,6 +64,22 @@  static bool has_semaphores(int fd, int dir)
 	return val;
 }
 
+static int cmd_parser_version(int fd)
+{
+	int val = 0;
+	struct drm_i915_getparam gp = {
+		gp.param = I915_PARAM_CMD_PARSER_VERSION,
+		gp.value = &val,
+	};
+
+	if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp) < 0) {
+		val = -errno;
+		igt_assert(val < 0);
+	}
+
+	return val;
+}
+
 /**
  * gem_submission_method:
  * @fd: open i915 drm file descriptor
@@ -253,3 +269,49 @@  void gem_require_blitter(int i915)
 {
 	igt_require(gem_has_blitter(i915));
 }
+
+static bool gem_engine_has_immutable_submission(int i915, int class)
+{
+	const int gen = intel_gen(intel_get_drm_devid(i915));
+        int parser_version;
+
+	parser_version = cmd_parser_version(i915);
+
+	if (parser_version < 0)
+		return false;
+
+	if (gen == 9 && class == I915_ENGINE_CLASS_COPY && parser_version > 9)
+		return true;
+
+	return false;
+}
+
+
+/**
+ * gem_class_has_mutable_submission:
+ * @i915: open i915 drm file descriptor
+ * @class: engine class
+ *
+ * Returns boolean value if the engine class allows batch modifications
+ * post execbuf.
+ */
+
+bool
+gem_class_has_mutable_submission(int fd, int class)
+{
+	return !gem_engine_has_immutable_submission(fd, class);
+}
+
+/**
+ * gem_engine_has_mutable_submission:
+ * @i915: open i915 drm file descriptor
+ * @engine: the engine (I915_EXEC_RING id) of target
+ *
+ * Returns boolean value if the engine allows batch modifications
+ * post execbuf.
+ */
+bool gem_engine_has_mutable_submission(int i915, unsigned int engine)
+{
+	return gem_class_has_mutable_submission(i915,
+						gem_execbuf_flags_to_engine_class(engine));
+}
diff --git a/lib/i915/gem_submission.h b/lib/i915/gem_submission.h
index 6deb7e2d..acd24bcb 100644
--- a/lib/i915/gem_submission.h
+++ b/lib/i915/gem_submission.h
@@ -34,6 +34,8 @@  void gem_submission_print_method(int fd);
 bool gem_has_semaphores(int fd);
 bool gem_has_execlists(int fd);
 bool gem_has_guc_submission(int fd);
+bool gem_engine_has_mutable_submission(int fd, unsigned int engine);
+bool gem_class_has_mutable_submission(int fd, int class);
 
 int gem_cmdparser_version(int i915, uint32_t engine);
 static inline bool gem_has_cmdparser(int i915, uint32_t engine)
diff --git a/tests/i915/gem_busy.c b/tests/i915/gem_busy.c
index a8388149..b84e2bd1 100644
--- a/tests/i915/gem_busy.c
+++ b/tests/i915/gem_busy.c
@@ -243,6 +243,10 @@  static void one(int fd, const struct intel_execution_engine2 *e, unsigned test_f
 	i++;
 
 	igt_assert(i < size/sizeof(*batch));
+
+	if ((test_flags & HANG) == 0)
+		igt_require(gem_class_has_mutable_submission(fd, e->class));
+
 	igt_require(__gem_execbuf(fd, &execbuf) == 0);
 
 	__gem_busy(fd, obj[SCRATCH].handle, &read[SCRATCH], &write[SCRATCH]);
@@ -256,7 +260,8 @@  static void one(int fd, const struct intel_execution_engine2 *e, unsigned test_f
 			    e2->instance == e->instance)
 				continue;
 
-			if (!gem_class_can_store_dword(fd, e2->class))
+			if (!gem_class_can_store_dword(fd, e2->class) ||
+			    !gem_class_has_mutable_submission(fd, e2->class))
 				continue;
 
 			igt_debug("Testing %s in parallel\n", e2->name);
diff --git a/tests/i915/gem_exec_async.c b/tests/i915/gem_exec_async.c
index d83e9f6d..b9859ffa 100644
--- a/tests/i915/gem_exec_async.c
+++ b/tests/i915/gem_exec_async.c
@@ -138,6 +138,9 @@  static void one(int fd, unsigned ring, uint32_t flags)
 	execbuf.buffers_ptr = to_user_pointer(obj);
 	execbuf.buffer_count = 2;
 	execbuf.flags = ring | flags;
+
+	igt_require(gem_engine_has_mutable_submission(fd, ring));
+
 	igt_require(__gem_execbuf(fd, &execbuf) == 0);
 	gem_close(fd, obj[BATCH].handle);
 
diff --git a/tests/i915/gem_exec_await.c b/tests/i915/gem_exec_await.c
index 33e2ee45..7817b483 100644
--- a/tests/i915/gem_exec_await.c
+++ b/tests/i915/gem_exec_await.c
@@ -78,8 +78,13 @@  static void wide(int fd, int ring_size, int timeout, unsigned int flags)
 	double time;
 
 	nengine = 0;
-	for_each_physical_engine(e, fd)
+	for_each_physical_engine(e, fd) {
+		if (!gem_engine_has_mutable_submission(fd, eb_ring(e)))
+			continue;
+
 		engines[nengine++] = eb_ring(e);
+	}
+
 	igt_require(nengine);
 
 	exec = calloc(nengine, sizeof(*exec));
diff --git a/tests/i915/gem_exec_fence.c b/tests/i915/gem_exec_fence.c
index 002120bf..fbb11ab2 100644
--- a/tests/i915/gem_exec_fence.c
+++ b/tests/i915/gem_exec_fence.c
@@ -129,6 +129,9 @@  static void test_fence_busy(int fd, unsigned ring, unsigned flags)
 	uint32_t *batch;
 	int fence, i, timeout;
 
+	if ((flags & HANG) == 0)
+		igt_require(gem_engine_has_mutable_submission(fd, ring));
+
 	gem_quiescent_gpu(fd);
 
 	memset(&execbuf, 0, sizeof(execbuf));
@@ -265,6 +268,8 @@  static void test_fence_busy_all(int fd, unsigned flags)
 	for_each_engine(e, fd) {
 		int fence, new;
 
+		if ((flags & HANG) == 0)
+			igt_require(gem_engine_has_mutable_submission(fd, eb_ring(e)));
 		execbuf.flags = eb_ring(e) | LOCAL_EXEC_FENCE_OUT;
 		execbuf.rsvd2 = -1;
 		gem_execbuf_wr(fd, &execbuf);
@@ -321,6 +326,9 @@  static void test_fence_await(int fd, unsigned ring, unsigned flags)
 	uint32_t *out;
 	int i;
 
+	if ((flags & HANG) == 0)
+		igt_require(gem_engine_has_mutable_submission(fd, ring));
+
 	igt_require(gem_can_store_dword(fd, 0));
 
 	out = gem_mmap__wc(fd, scratch, 0, 4096, PROT_WRITE);
diff --git a/tests/i915/gem_exec_latency.c b/tests/i915/gem_exec_latency.c
index 3d99182a..e4c0c862 100644
--- a/tests/i915/gem_exec_latency.c
+++ b/tests/i915/gem_exec_latency.c
@@ -128,6 +128,8 @@  static void latency_on_ring(int fd,
 	double gpu_latency;
 	int i, j;
 
+	igt_require(gem_engine_has_mutable_submission(fd, ring));
+
 	reg = (volatile uint32_t *)((volatile char *)igt_global_mmio + RCS_TIMESTAMP);
 
 	memset(&execbuf, 0, sizeof(execbuf));
@@ -271,6 +273,8 @@  static void latency_from_ring(int fd,
 	uint32_t ctx[2] = {};
 	int i, j;
 
+	igt_require(gem_engine_has_mutable_submission(fd, ring));
+
 	if (flags & PREEMPT) {
 		ctx[0] = gem_context_create(fd);
 		gem_context_set_priority(fd, ctx[0], -1023);
@@ -316,6 +320,9 @@  static void latency_from_ring(int fd,
 		igt_spin_t *spin = NULL;
 		IGT_CORK_HANDLE(c);
 
+		if (!gem_engine_has_mutable_submission(fd, eb_ring(e)))
+			continue;
+
 		gem_set_domain(fd, obj[2].handle,
 			       I915_GEM_DOMAIN_GTT,
 			       I915_GEM_DOMAIN_GTT);
diff --git a/tests/i915/gem_exec_nop.c b/tests/i915/gem_exec_nop.c
index 2ad8aeea..dbedb356 100644
--- a/tests/i915/gem_exec_nop.c
+++ b/tests/i915/gem_exec_nop.c
@@ -123,6 +123,7 @@  static void poll_ring(int fd, unsigned engine, const char *name, int timeout)
 
 	gem_require_ring(fd, engine);
 	igt_require(gem_can_store_dword(fd, engine));
+	igt_require(gem_engine_has_mutable_submission(fd, engine));
 
 	memset(&obj, 0, sizeof(obj));
 	obj.handle = gem_create(fd, 4096);
@@ -234,7 +235,8 @@  static void poll_sequential(int fd, const char *name, int timeout)
 
 	nengine = 0;
 	for_each_physical_engine(e, fd) {
-		if (!gem_can_store_dword(fd, eb_ring(e)))
+		if (!gem_can_store_dword(fd, eb_ring(e)) ||
+		    !gem_engine_has_mutable_submission(fd, eb_ring(e)))
 			continue;
 
 		engines[nengine++] = eb_ring(e);
diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
index 5c15f177..ca6bef6a 100644
--- a/tests/i915/gem_exec_schedule.c
+++ b/tests/i915/gem_exec_schedule.c
@@ -652,7 +652,8 @@  static void semaphore_noskip(int i915)
 		igt_spin_t *chain, *spin;
 
 		if (eb_ring(inner) == eb_ring(outer) ||
-		    !gem_can_store_dword(i915, eb_ring(inner)))
+		    !gem_can_store_dword(i915, eb_ring(inner)) ||
+		    !gem_engine_has_mutable_submission(i915, eb_ring(inner)))
 			continue;
 
 		chain = __igt_spin_new(i915, .engine = eb_ring(outer));
@@ -1157,6 +1158,9 @@  static void deep(int fd, unsigned ring)
 	int dep_nreq;
 	int n;
 
+	igt_require(gem_can_store_dword(fd, ring));
+	igt_require(gem_engine_has_mutable_submission(fd, ring));
+
 	ctx = malloc(sizeof(*ctx) * MAX_CONTEXTS);
 	for (n = 0; n < MAX_CONTEXTS; n++) {
 		ctx[n] = gem_context_create(fd);
diff --git a/tests/i915/gem_exec_whisper.c b/tests/i915/gem_exec_whisper.c
index a3384760..45cc1437 100644
--- a/tests/i915/gem_exec_whisper.c
+++ b/tests/i915/gem_exec_whisper.c
@@ -204,13 +204,15 @@  static void whisper(int fd, unsigned engine, unsigned flags)
 	nengine = 0;
 	if (engine == ALL_ENGINES) {
 		for_each_physical_engine(e, fd) {
-			if (gem_can_store_dword(fd, eb_ring(e)))
+			if (gem_can_store_dword(fd, eb_ring(e)) &&
+			    gem_engine_has_mutable_submission(fd, eb_ring(e)))
 				engines[nengine++] = eb_ring(e);
 		}
 	} else {
 		igt_assert(!(flags & ALL));
 		igt_require(gem_has_ring(fd, engine));
 		igt_require(gem_can_store_dword(fd, engine));
+		igt_require(gem_engine_has_mutable_submission(fd, engine));
 		engines[nengine++] = engine;
 	}
 	igt_require(nengine);
diff --git a/tests/prime_busy.c b/tests/prime_busy.c
index 8ee23bb3..bb29216e 100644
--- a/tests/prime_busy.c
+++ b/tests/prime_busy.c
@@ -53,6 +53,9 @@  static void busy(int fd, unsigned ring, unsigned flags)
 	uint32_t *batch, *bbe;
 	int i, count, timeout;
 
+	if ((flags & HANG) == 0)
+		igt_require(gem_engine_has_mutable_submission(fd, ring));
+
 	gem_quiescent_gpu(fd);
 
 	memset(&execbuf, 0, sizeof(execbuf));
diff --git a/tests/prime_vgem.c b/tests/prime_vgem.c
index 2b21ff41..04cc913d 100644
--- a/tests/prime_vgem.c
+++ b/tests/prime_vgem.c
@@ -342,6 +342,8 @@  static void work(int i915, int dmabuf, unsigned ring, uint32_t flags)
 	uint32_t *batch, *bbe;
 	int i, count;
 
+	igt_require(gem_engine_has_mutable_submission(i915, ring));
+
 	memset(&execbuf, 0, sizeof(execbuf));
 	execbuf.buffers_ptr = (uintptr_t)obj;
 	execbuf.buffer_count = 2;
@@ -850,6 +852,7 @@  igt_main
 			      e->name) {
 			gem_require_ring(i915, eb_ring(e));
 			igt_require(gem_can_store_dword(i915, eb_ring(e)));
+			igt_require(gem_engine_has_mutable_submission(i915, eb_ring(e)));
 
 			gem_quiescent_gpu(i915);
 			test_sync(i915, vgem, e->exec_id, e->flags);
@@ -862,6 +865,7 @@  igt_main
 			      e->name) {
 			gem_require_ring(i915, eb_ring(e));
 			igt_require(gem_can_store_dword(i915, eb_ring(e)));
+			igt_require(gem_engine_has_mutable_submission(i915, eb_ring(e)));
 
 			gem_quiescent_gpu(i915);
 			test_busy(i915, vgem, e->exec_id, e->flags);
@@ -874,6 +878,7 @@  igt_main
 			      e->name) {
 			gem_require_ring(i915, eb_ring(e));
 			igt_require(gem_can_store_dword(i915, eb_ring(e)));
+			igt_require(gem_engine_has_mutable_submission(i915, eb_ring(e)));
 
 			gem_quiescent_gpu(i915);
 			test_wait(i915, vgem, e->exec_id, e->flags);
@@ -897,6 +902,7 @@  igt_main
 					e->name) {
 				gem_require_ring(i915, eb_ring(e));
 				igt_require(gem_can_store_dword(i915, eb_ring(e)));
+				igt_require(gem_engine_has_mutable_submission(i915, eb_ring(e)));
 
 				gem_quiescent_gpu(i915);
 				test_fence_wait(i915, vgem, e->exec_id, e->flags);