[igt-dev] [PATCH i-g-t] igt: Replace 'all-engines' magic numbers with macro

* [igt-dev] [PATCH i-g-t] igt: Replace 'all-engines' magic numbers with macro
@ 2018-03-19 22:30 Antonio Argenziano
  2018-03-19 23:08 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Antonio Argenziano @ 2018-03-19 22:30 UTC (permalink / raw)
  To: igt-dev

In interfaces where a parameter allow to select an engine, we usually
use '-1' or '~0u' to select all engines. This patch replaces magic
numbers with a named constant.

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>

Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/i915/gem_submission.c |  6 +++---
 lib/igt_dummyload.c       |  2 +-
 lib/igt_gt.h              |  2 ++
 tests/gem_ctx_create.c    |  6 +++---
 tests/gem_eio.c           |  2 +-
 tests/gem_exec_nop.c      |  4 ++--
 tests/gem_exec_parallel.c |  4 ++--
 tests/gem_exec_reloc.c    |  4 ++--
 tests/gem_exec_schedule.c |  4 ++--
 tests/gem_exec_suspend.c  | 12 ++++++------
 tests/gem_exec_whisper.c  |  6 +++---
 tests/gem_sync.c          | 20 ++++++++++----------
 tests/gem_wait.c          | 18 +++++++++---------
 13 files changed, 46 insertions(+), 44 deletions(-)

diff --git a/lib/i915/gem_submission.c b/lib/i915/gem_submission.c
index e27ea9ec..c5e96969 100644
--- a/lib/i915/gem_submission.c
+++ b/lib/i915/gem_submission.c
@@ -177,8 +177,8 @@ static bool is_wedged(int i915)
  * @i915: open i915 drm file descriptor
  * @engine: the engine (I915_EXEC_RING id) to exercise
  *
- * Execute a nop batch on the specified, or -1 for all, and check it
- * executes.
+ * Execute a nop batch on the engine specified, or ALL_ENGINES for all,
+ * and check it executes.
  */
 void gem_test_engine(int i915, unsigned int engine)
 {
@@ -194,7 +194,7 @@ void gem_test_engine(int i915, unsigned int engine)
 	igt_assert(!is_wedged(i915));
 	gem_write(i915, obj.handle, 0, &bbe, sizeof(bbe));
 
-	if (engine == -1u) {
+	if (engine == ALL_ENGINES) {
 		for_each_engine(i915, engine) {
 			execbuf.flags = engine;
 			gem_execbuf(i915, &execbuf);
diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 4b20f23d..dbc92e8f 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -91,7 +91,7 @@ static int emit_recursive_batch(igt_spin_t *spin,
 	int i;
 
 	nengine = 0;
-	if (engine == -1) {
+	if (engine == ALL_ENGINES) {
 		for_each_engine(fd, engine)
 			if (engine)
 				engines[nengine++] = engine;
diff --git a/lib/igt_gt.h b/lib/igt_gt.h
index 4d9d1aa0..a517ed7b 100644
--- a/lib/igt_gt.h
+++ b/lib/igt_gt.h
@@ -66,6 +66,8 @@ void igt_clflush_range(void *addr, int size);
 
 unsigned intel_detect_and_clear_missed_interrupts(int fd);
 
+#define ALL_ENGINES ~0u /* Use in interfaces to iterate all engines */
+
 extern const struct intel_execution_engine {
 	const char *name;
 	const char *full_name;
diff --git a/tests/gem_ctx_create.c b/tests/gem_ctx_create.c
index 1b32d6c3..a664070d 100644
--- a/tests/gem_ctx_create.c
+++ b/tests/gem_ctx_create.c
@@ -110,7 +110,7 @@ static void active(int fd, unsigned engine, int timeout, int ncpus)
 	unsigned int nengine, engines[16];
 	unsigned *shared;
 
-	if (engine == -1) {
+	if (engine == ALL_ENGINES) {
 		igt_require(all_nengine);
 		nengine = all_nengine;
 		memcpy(engines, all_engines, sizeof(engines[0])*nengine);
@@ -365,9 +365,9 @@ igt_main
 		files(fd, 150, ncpus);
 
 	igt_subtest("active-all")
-		active(fd, -1, 120, 1);
+		active(fd, ALL_ENGINES, 120, 1);
 	igt_subtest("forked-active-all")
-		active(fd, -1, 120, ncpus);
+		active(fd, ALL_ENGINES, 120, ncpus);
 
 	for (const struct intel_execution_engine *e = intel_execution_engines;
 	     e->name; e++) {
diff --git a/tests/gem_eio.c b/tests/gem_eio.c
index 249f5bff..4bcc5937 100644
--- a/tests/gem_eio.c
+++ b/tests/gem_eio.c
@@ -66,7 +66,7 @@ static void trigger_reset(int fd)
 
 	/* And just check the gpu is indeed running again */
 	igt_debug("Checking that the GPU recovered\n");
-	gem_test_engine(fd, -1);
+	gem_test_engine(fd, ALL_ENGINES);
 
 	gem_quiescent_gpu(fd);
 }
diff --git a/tests/gem_exec_nop.c b/tests/gem_exec_nop.c
index d971ffcb..50f0a3aa 100644
--- a/tests/gem_exec_nop.c
+++ b/tests/gem_exec_nop.c
@@ -488,7 +488,7 @@ static void fence_signal(int fd, uint32_t handle,
 	igt_require(gem_has_exec_fence(fd));
 
 	nengine = 0;
-	if (ring_id == -1) {
+	if (ring_id == ALL_ENGINES) {
 		for_each_physical_engine(fd, n)
 			engines[nengine++] = n;
 	} else {
@@ -642,7 +642,7 @@ igt_main
 	}
 
 	igt_subtest("signal-all")
-		fence_signal(device, handle, -1, "all", 150);
+		fence_signal(device, handle, ALL_ENGINES, "all", 150);
 
 	igt_subtest("series")
 		series(device, handle, 150);
diff --git a/tests/gem_exec_parallel.c b/tests/gem_exec_parallel.c
index fe5ffe8f..a6fa698e 100644
--- a/tests/gem_exec_parallel.c
+++ b/tests/gem_exec_parallel.c
@@ -168,7 +168,7 @@ static void all(int fd, unsigned engine, unsigned flags)
 		igt_require(gen > 5);
 
 	nengine = 0;
-	if (engine == -1) {
+	if (engine == ALL_ENGINES) {
 		for_each_physical_engine(fd, engine) {
 			if (gem_can_store_dword(fd, engine))
 				engines[nengine++] = engine;
@@ -247,7 +247,7 @@ igt_main
 
 	for (const struct mode *m = modes; m->name; m++)
 		igt_subtest_f("%s", *m->name ? m->name : "basic")
-			all(fd, -1, m->flags);
+			all(fd, ALL_ENGINES, m->flags);
 
 	for (const struct intel_execution_engine *e = intel_execution_engines;
 	     e->name; e++) {
diff --git a/tests/gem_exec_reloc.c b/tests/gem_exec_reloc.c
index 213de1d7..91c6691a 100644
--- a/tests/gem_exec_reloc.c
+++ b/tests/gem_exec_reloc.c
@@ -257,7 +257,7 @@ static void active(int fd, unsigned engine)
 	int pass;
 
 	nengine = 0;
-	if (engine == -1) {
+	if (engine == ALL_ENGINES) {
 		for_each_physical_engine(fd, engine) {
 			if (gem_can_store_dword(fd, engine))
 				engines[nengine++] = engine;
@@ -746,7 +746,7 @@ igt_main
 		from_gpu(fd);
 
 	igt_subtest("active")
-		active(fd, -1);
+		active(fd, ALL_ENGINES);
 	for (const struct intel_execution_engine *e = intel_execution_engines;
 	     e->name; e++) {
 		igt_subtest_f("active-%s", e->name)
diff --git a/tests/gem_exec_schedule.c b/tests/gem_exec_schedule.c
index 89cf4285..d2f040ab 100644
--- a/tests/gem_exec_schedule.c
+++ b/tests/gem_exec_schedule.c
@@ -171,7 +171,7 @@ static void smoketest(int fd, unsigned ring, unsigned timeout)
 	uint32_t *ptr;
 
 	nengine = 0;
-	if (ring == -1) {
+	if (ring == ALL_ENGINES) {
 		for_each_physical_engine(fd, engine)
 			engines[nengine++] = engine;
 	} else {
@@ -949,7 +949,7 @@ igt_main
 		}
 
 		igt_subtest("smoketest-all")
-			smoketest(fd, -1, 30);
+			smoketest(fd, ALL_ENGINES, 30);
 
 		for (e = intel_execution_engines; e->name; e++) {
 			if (e->exec_id == 0)
diff --git a/tests/gem_exec_suspend.c b/tests/gem_exec_suspend.c
index 351347cb..db2bca26 100644
--- a/tests/gem_exec_suspend.c
+++ b/tests/gem_exec_suspend.c
@@ -98,7 +98,7 @@ static void run_test(int fd, unsigned engine, unsigned flags)
 	igt_spin_t *spin = NULL;
 
 	nengine = 0;
-	if (engine == -1) {
+	if (engine == ALL_ENGINES) {
 		/* If we don't have semaphores, then every ring switch
 		 * will result in a CPU stall until the previous write
 		 * has finished. This is likely to hide any issue with
@@ -254,15 +254,15 @@ igt_main
 	}
 
 	igt_subtest("basic")
-		run_test(fd, -1, NOSLEEP);
+		run_test(fd, ALL_ENGINES, NOSLEEP);
 	igt_subtest("basic-S3-devices")
-		run_test(fd, -1, SUSPEND_DEVICES);
+		run_test(fd, ALL_ENGINES, SUSPEND_DEVICES);
 	igt_subtest("basic-S3")
-		run_test(fd, -1, SUSPEND);
+		run_test(fd, ALL_ENGINES, SUSPEND);
 	igt_subtest("basic-S4-devices")
-		run_test(fd, -1, HIBERNATE_DEVICES);
+		run_test(fd, ALL_ENGINES, HIBERNATE_DEVICES);
 	igt_subtest("basic-S4")
-		run_test(fd, -1, HIBERNATE);
+		run_test(fd, ALL_ENGINES, HIBERNATE);
 
 	for (e = intel_execution_engines; e->name; e++) {
 		for (m = modes; m->suffix; m++) {
diff --git a/tests/gem_exec_whisper.c b/tests/gem_exec_whisper.c
index 1f4dad63..81303f84 100644
--- a/tests/gem_exec_whisper.c
+++ b/tests/gem_exec_whisper.c
@@ -205,7 +205,7 @@ static void whisper(int fd, unsigned engine, unsigned flags)
 	debugfs = igt_debugfs_dir(fd);
 
 	nengine = 0;
-	if (engine == -1) {
+	if (engine == ALL_ENGINES) {
 		for_each_physical_engine(fd, engine) {
 			if (gem_can_store_dword(fd, engine))
 				engines[nengine++] = engine;
@@ -534,7 +534,7 @@ igt_main
 
 	for (const struct mode *m = modes; m->name; m++)
 		igt_subtest_f("%s", m->name)
-			whisper(fd, -1, m->flags);
+			whisper(fd, ALL_ENGINES, m->flags);
 
 	for (const struct intel_execution_engine *e = intel_execution_engines;
 	     e->name; e++) {
@@ -556,7 +556,7 @@ igt_main
 			if (m->flags & INTERRUPTIBLE)
 				continue;
 			igt_subtest_f("hang-%s", m->name)
-				whisper(fd, -1, m->flags | HANG);
+				whisper(fd, ALL_ENGINES, m->flags | HANG);
 		}
 	}
 
diff --git a/tests/gem_sync.c b/tests/gem_sync.c
index f451287a..1e2e089a 100644
--- a/tests/gem_sync.c
+++ b/tests/gem_sync.c
@@ -85,7 +85,7 @@ sync_ring(int fd, unsigned ring, int num_children, int timeout)
 	const char *names[16];
 	int num_engines = 0;
 
-	if (ring == ~0u) {
+	if (ring == ALL_ENGINES) {
 		for_each_physical_engine(fd, ring) {
 			names[num_engines] = e__->name;
 			engines[num_engines++] = ring;
@@ -185,7 +185,7 @@ store_ring(int fd, unsigned ring, int num_children, int timeout)
 	const char *names[16];
 	int num_engines = 0;
 
-	if (ring == ~0u) {
+	if (ring == ALL_ENGINES) {
 		for_each_physical_engine(fd, ring) {
 			if (!gem_can_store_dword(fd, ring))
 				continue;
@@ -473,7 +473,7 @@ store_many(int fd, unsigned ring, int timeout)
 
 	intel_detect_and_clear_missed_interrupts(fd);
 
-	if (ring == ~0u) {
+	if (ring == ALL_ENGINES) {
 		for_each_physical_engine(fd, ring) {
 			if (!gem_can_store_dword(fd, ring))
 				continue;
@@ -669,7 +669,7 @@ preempt(int fd, unsigned ring, int num_children, int timeout)
 	int num_engines = 0;
 	uint32_t ctx[2];
 
-	if (ring == ~0u) {
+	if (ring == ALL_ENGINES) {
 		for_each_physical_engine(fd, ring) {
 			names[num_engines] = e__->name;
 			engines[num_engines++] = ring;
@@ -773,15 +773,15 @@ igt_main
 	}
 
 	igt_subtest("basic-each")
-		sync_ring(fd, ~0u, 1, 5);
+		sync_ring(fd, ALL_ENGINES, 1, 5);
 	igt_subtest("basic-store-each")
-		store_ring(fd, ~0u, 1, 5);
+		store_ring(fd, ALL_ENGINES, 1, 5);
 	igt_subtest("basic-many-each")
-		store_many(fd, ~0u, 5);
+		store_many(fd, ALL_ENGINES, 5);
 	igt_subtest("forked-each")
-		sync_ring(fd, ~0u, ncpus, 150);
+		sync_ring(fd, ALL_ENGINES, ncpus, 150);
 	igt_subtest("forked-store-each")
-		store_ring(fd, ~0u, ncpus, 150);
+		store_ring(fd, ALL_ENGINES, ncpus, 150);
 
 	igt_subtest("basic-all")
 		sync_all(fd, 1, 5);
@@ -805,7 +805,7 @@ igt_main
 		}
 
 		igt_subtest("preempt-all")
-			preempt(fd, -1, 1, 20);
+			preempt(fd, ALL_ENGINES, 1, 20);
 
 		for (e = intel_execution_engines; e->name; e++) {
 			igt_subtest_f("preempt-%s", e->name)
diff --git a/tests/gem_wait.c b/tests/gem_wait.c
index 34388d26..1d10055b 100644
--- a/tests/gem_wait.c
+++ b/tests/gem_wait.c
@@ -156,23 +156,23 @@ igt_main
 
 		igt_subtest("basic-busy-all") {
 			gem_quiescent_gpu(fd);
-			basic(fd, -1, BUSY);
+			basic(fd, ALL_ENGINES, BUSY);
 		}
 		igt_subtest("basic-wait-all") {
 			gem_quiescent_gpu(fd);
-			basic(fd, -1, 0);
+			basic(fd, ALL_ENGINES, 0);
 		}
 		igt_subtest("basic-await-all") {
 			gem_quiescent_gpu(fd);
-			basic(fd, -1, AWAIT);
+			basic(fd, ALL_ENGINES, AWAIT);
 		}
 		igt_subtest("basic-busy-write-all") {
 			gem_quiescent_gpu(fd);
-			basic(fd, -1, BUSY | WRITE);
+			basic(fd, ALL_ENGINES, BUSY | WRITE);
 		}
 		igt_subtest("basic-wait-write-all") {
 			gem_quiescent_gpu(fd);
-			basic(fd, -1, WRITE);
+			basic(fd, ALL_ENGINES, WRITE);
 		}
 
 		for (e = intel_execution_engines; e->name; e++) {
@@ -216,20 +216,20 @@ igt_main
 
 		igt_subtest("hang-busy-all") {
 			gem_quiescent_gpu(fd);
-			basic(fd, -1, BUSY | HANG);
+			basic(fd, ALL_ENGINES, BUSY | HANG);
 		}
 		igt_subtest("hang-wait-all") {
 			gem_quiescent_gpu(fd);
-			basic(fd, -1, HANG);
+			basic(fd, ALL_ENGINES, HANG);
 		}
 
 		igt_subtest("hang-busy-write-all") {
 			gem_quiescent_gpu(fd);
-			basic(fd, -1, BUSY | WRITE | HANG);
+			basic(fd, ALL_ENGINES, BUSY | WRITE | HANG);
 		}
 		igt_subtest("hang-wait-write-all") {
 			gem_quiescent_gpu(fd);
-			basic(fd, -1, WRITE | HANG);
+			basic(fd, ALL_ENGINES, WRITE | HANG);
 		}
 
 		for (e = intel_execution_engines; e->name; e++) {
-- 
2.16.2

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 5+ messages in thread