[igt-dev] [PATCH] [PATCH i-g-t][V6]tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available

* [igt-dev] [PATCH] [PATCH i-g-t][V6]tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available
@ 2020-04-14 16:40 Arjun Melkaveri
  2020-04-14 16:46 ` Chris Wilson
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Arjun Melkaveri @ 2020-04-14 16:40 UTC (permalink / raw)
  To: arjun.melkaveri, igt-dev

Replaced the legacy for_each_engine* defines with the ones
implemented in the gem_engine_topology library.

Used  gem_context_clone_with_engines
to make sure that engine index was potentially created
based on a  default context with engine map configured.

Added Legacy engine coverage for sync_ring and sync_all.

Divided tests into static for ALL_Engine test cases
and dynamic for multiple engine .

Cc: Dec Katarzyna <katarzyna.dec@intel.com>
Cc: Ursulin Tvrtko <tvrtko.ursulin@intel.com>
Signed-off-by: sai gowtham <sai.gowtham.ch@intel.com>
Signed-off-by: Arjun Melkaveri <arjun.melkaveri@intel.com>
---
 tests/i915/gem_sync.c | 278 +++++++++++++++++++++++++++---------------
 1 file changed, 179 insertions(+), 99 deletions(-)

diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c
index 2ef55ecc..9c9a4319 100644
--- a/tests/i915/gem_sync.c
+++ b/tests/i915/gem_sync.c
@@ -24,6 +24,9 @@
 #include <time.h>
 #include <pthread.h>
 
+#include "igt_debugfs.h"
+#include "igt_dummyload.h"
+#include "igt_gt.h"
 #include "igt.h"
 #include "igt_sysfs.h"
 
@@ -37,6 +40,9 @@
 #define MIN_PRIO LOCAL_I915_CONTEXT_MIN_USER_PRIORITY
 
 #define ENGINE_MASK  (I915_EXEC_RING_MASK | LOCAL_I915_EXEC_BSD_MASK)
+#define RESET_TIMEOUT_MS (2 * MSEC_PER_SEC)
+static unsigned long reset_timeout_ms = RESET_TIMEOUT_MS;
+#define NSEC_PER_MSEC (1000 * 1000ull)
 
 IGT_TEST_DESCRIPTION("Basic check of ring<->ring write synchronisation.");
 
@@ -78,24 +84,36 @@ out:
 	return ts.tv_sec + 1e-9*ts.tv_nsec;
 }
 
+static void cleanup(int i915)
+{
+	igt_drop_caches_set(i915,
+			    /* cancel everything */
+			    DROP_RESET_ACTIVE | DROP_RESET_SEQNO |
+			    /* cleanup */
+			    DROP_ACTIVE | DROP_RETIRE | DROP_IDLE | DROP_FREED);
+	igt_require_gem(i915);
+}
+
+
 static void
 sync_ring(int fd, unsigned ring, int num_children, int timeout)
 {
+	const struct intel_execution_engine2 *e2;
 	unsigned engines[16];
 	const char *names[16];
 	int num_engines = 0;
 
 	if (ring == ALL_ENGINES) {
-		for_each_physical_engine(e, fd) {
-			names[num_engines] = e->name;
-			engines[num_engines++] = eb_ring(e);
+		__for_each_physical_engine(fd, e2) {
+			names[num_engines] = e2->name;
+			engines[num_engines++] = e2->flags;
 			if (num_engines == ARRAY_SIZE(engines))
 				break;
 		}
 
 		num_children *= num_engines;
 	} else {
-		gem_require_ring(fd, ring);
+		igt_require(gem_can_store_dword(fd, ring));
 		names[num_engines] = NULL;
 		engines[num_engines++] = ring;
 	}
@@ -139,7 +157,7 @@ sync_ring(int fd, unsigned ring, int num_children, int timeout)
 }
 
 static void
-idle_ring(int fd, unsigned ring, int timeout)
+idle_ring(int fd, unsigned int ring, int num_children, int timeout)
 {
 	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	struct drm_i915_gem_exec_object2 object;
@@ -180,23 +198,23 @@ idle_ring(int fd, unsigned ring, int timeout)
 static void
 wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
 {
+	const struct intel_execution_engine2 *e2;
 	unsigned engines[16];
 	const char *names[16];
 	int num_engines = 0;
 
 	if (ring == ALL_ENGINES) {
-		for_each_physical_engine(e, fd) {
-			if (!gem_can_store_dword(fd, eb_ring(e)))
+		__for_each_physical_engine(fd, e2) {
+			if (!gem_class_can_store_dword(fd, e2->class))
 				continue;
 
-			names[num_engines] = e->name;
-			engines[num_engines++] = eb_ring(e);
+			names[num_engines] = e2->name;
+			engines[num_engines++] = e2->flags;
 			if (num_engines == ARRAY_SIZE(engines))
 				break;
 		}
 		igt_require(num_engines);
 	} else {
-		gem_require_ring(fd, ring);
 		igt_require(gem_can_store_dword(fd, ring));
 		names[num_engines] = NULL;
 		engines[num_engines++] = ring;
@@ -290,25 +308,26 @@ wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
 	igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
 }
 
-static void active_ring(int fd, unsigned ring, int timeout)
+static void active_ring(int fd, unsigned int ring,
+			int num_children, int timeout)
 {
+	const struct intel_execution_engine2 *e2;
 	unsigned engines[16];
 	const char *names[16];
 	int num_engines = 0;
 
 	if (ring == ALL_ENGINES) {
-		for_each_physical_engine(e, fd) {
-			if (!gem_can_store_dword(fd, eb_ring(e)))
+		__for_each_physical_engine(fd, e2) {
+			if (!gem_class_can_store_dword(fd, e2->class))
 				continue;
 
-			names[num_engines] = e->name;
-			engines[num_engines++] = eb_ring(e);
+			names[num_engines] = e2->name;
+			engines[num_engines++] = e2->flags;
 			if (num_engines == ARRAY_SIZE(engines))
 				break;
 		}
 		igt_require(num_engines);
 	} else {
-		gem_require_ring(fd, ring);
 		igt_require(gem_can_store_dword(fd, ring));
 		names[num_engines] = NULL;
 		engines[num_engines++] = ring;
@@ -359,23 +378,23 @@ static void active_ring(int fd, unsigned ring, int timeout)
 static void
 active_wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
 {
+	const struct intel_execution_engine2 *e2;
 	unsigned engines[16];
 	const char *names[16];
 	int num_engines = 0;
 
 	if (ring == ALL_ENGINES) {
-		for_each_physical_engine(e, fd) {
-			if (!gem_can_store_dword(fd, eb_ring(e)))
+		__for_each_physical_engine(fd, e2) {
+			if (!gem_class_can_store_dword(fd, e2->class))
 				continue;
 
-			names[num_engines] = e->name;
-			engines[num_engines++] = eb_ring(e);
+			names[num_engines] = e2->name;
+			engines[num_engines++] = e2->flags;
 			if (num_engines == ARRAY_SIZE(engines))
 				break;
 		}
 		igt_require(num_engines);
 	} else {
-		gem_require_ring(fd, ring);
 		igt_require(gem_can_store_dword(fd, ring));
 		names[num_engines] = NULL;
 		engines[num_engines++] = ring;
@@ -493,25 +512,25 @@ active_wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
 static void
 store_ring(int fd, unsigned ring, int num_children, int timeout)
 {
+	const struct intel_execution_engine2 *e2;
 	const int gen = intel_gen(intel_get_drm_devid(fd));
 	unsigned engines[16];
 	const char *names[16];
 	int num_engines = 0;
 
 	if (ring == ALL_ENGINES) {
-		for_each_physical_engine(e, fd) {
-			if (!gem_can_store_dword(fd, eb_ring(e)))
+		__for_each_physical_engine(fd, e2) {
+			if (!gem_class_can_store_dword(fd, e2->class))
 				continue;
 
-			names[num_engines] = e->name;
-			engines[num_engines++] = eb_ring(e);
+			names[num_engines] = e2->name;
+			engines[num_engines++] = e2->flags;
 			if (num_engines == ARRAY_SIZE(engines))
 				break;
 		}
 
 		num_children *= num_engines;
 	} else {
-		gem_require_ring(fd, ring);
 		igt_require(gem_can_store_dword(fd, ring));
 		names[num_engines] = NULL;
 		engines[num_engines++] = ring;
@@ -608,6 +627,7 @@ store_ring(int fd, unsigned ring, int num_children, int timeout)
 static void
 switch_ring(int fd, unsigned ring, int num_children, int timeout)
 {
+	const struct intel_execution_engine2 *e2;
 	const int gen = intel_gen(intel_get_drm_devid(fd));
 	unsigned engines[16];
 	const char *names[16];
@@ -616,19 +636,18 @@ switch_ring(int fd, unsigned ring, int num_children, int timeout)
 	gem_require_contexts(fd);
 
 	if (ring == ALL_ENGINES) {
-		for_each_physical_engine(e, fd) {
-			if (!gem_can_store_dword(fd, eb_ring(e)))
+		__for_each_physical_engine(fd, e2) {
+			if (!gem_class_can_store_dword(fd, e2->class))
 				continue;
 
-			names[num_engines] = e->name;
-			engines[num_engines++] = eb_ring(e);
+			names[num_engines] = e2->name;
+			engines[num_engines++] = e2->flags;
 			if (num_engines == ARRAY_SIZE(engines))
 				break;
 		}
 
 		num_children *= num_engines;
 	} else {
-		gem_require_ring(fd, ring);
 		igt_require(gem_can_store_dword(fd, ring));
 		names[num_engines] = NULL;
 		engines[num_engines++] = ring;
@@ -931,8 +950,9 @@ __store_many(int fd, unsigned ring, int timeout, unsigned long *cycles)
 }
 
 static void
-store_many(int fd, unsigned ring, int timeout)
+store_many(int fd, unsigned int ring, int num_children, int timeout)
 {
+	const struct intel_execution_engine2 *e2;
 	unsigned long *shared;
 	const char *names[16];
 	int n = 0;
@@ -943,21 +963,20 @@ store_many(int fd, unsigned ring, int timeout)
 	intel_detect_and_clear_missed_interrupts(fd);
 
 	if (ring == ALL_ENGINES) {
-		for_each_physical_engine(e, fd) {
-			if (!gem_can_store_dword(fd, eb_ring(e)))
+		__for_each_physical_engine(fd, e2) {
+			if (!gem_class_can_store_dword(fd, e2->class))
 				continue;
 
 			igt_fork(child, 1)
 				__store_many(fd,
-					     eb_ring(e),
+					     e2->flags,
 					     timeout,
 					     &shared[n]);
 
-			names[n++] = e->name;
+			names[n++] = e2->name;
 		}
 		igt_waitchildren();
 	} else {
-		gem_require_ring(fd, ring);
 		igt_require(gem_can_store_dword(fd, ring));
 		__store_many(fd, ring, timeout, &shared[n]);
 		names[n++] = NULL;
@@ -1025,15 +1044,16 @@ sync_all(int fd, int num_children, int timeout)
 static void
 store_all(int fd, int num_children, int timeout)
 {
+	const struct intel_execution_engine2 *e;
 	const int gen = intel_gen(intel_get_drm_devid(fd));
 	unsigned engines[16];
 	int num_engines = 0;
 
-	for_each_physical_engine(e, fd) {
-		if (!gem_can_store_dword(fd, eb_ring(e)))
+	__for_each_physical_engine(fd, e) {
+		if (!gem_class_can_store_dword(fd, e->class))
 			continue;
 
-		engines[num_engines++] = eb_ring(e);
+		engines[num_engines++] = e->flags;
 		if (num_engines == ARRAY_SIZE(engines))
 			break;
 	}
@@ -1132,22 +1152,22 @@ store_all(int fd, int num_children, int timeout)
 static void
 preempt(int fd, unsigned ring, int num_children, int timeout)
 {
+	const struct intel_execution_engine2 *e2;
 	unsigned engines[16];
 	const char *names[16];
 	int num_engines = 0;
 	uint32_t ctx[2];
 
 	if (ring == ALL_ENGINES) {
-		for_each_physical_engine(e, fd) {
-			names[num_engines] = e->name;
-			engines[num_engines++] = eb_ring(e);
+		__for_each_physical_engine(fd, e2) {
+			names[num_engines] = e2->name;
+			engines[num_engines++] = e2->flags;
 			if (num_engines == ARRAY_SIZE(engines))
 				break;
 		}
 
 		num_children *= num_engines;
 	} else {
-		gem_require_ring(fd, ring);
 		names[num_engines] = NULL;
 		engines[num_engines++] = ring;
 	}
@@ -1207,76 +1227,99 @@ preempt(int fd, unsigned ring, int num_children, int timeout)
 	gem_context_destroy(fd, ctx[0]);
 }
 
+static void do_test(void (*test)(int i915, unsigned int engine,
+		    int num_children, int test_timeout),
+		    int i915, unsigned int engine, int num_children,
+		    int test_timeout, const char *name)
+{
+#define ATTR "preempt_timeout_ms"
+	int timeout = -1;
+
+	cleanup(i915);
+
+	gem_engine_property_scanf(i915, name, ATTR, "%d", &timeout);
+	if (timeout != -1) {
+		igt_require(gem_engine_property_printf(i915, name,
+						       ATTR, "%d", 50) > 0);
+		reset_timeout_ms = 200;
+	}
+
+	test(i915, engine, num_children, test_timeout);
+	gem_quiescent_gpu(i915);
+}
+
 igt_main
 {
-	const struct intel_execution_engine *e;
 	const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
 	int fd = -1;
 
+	struct {
+		const char *name;
+		void (*func)(int fd, unsigned int engine,
+			     int num_children, int timeout);
+		int num_children;
+		int timeout;
+	} *test, allengine[] = {
+		{ "basic-each", sync_ring, 1, 2},
+		{ "basic-store-each", store_ring, 1, 2},
+		{ "basic-many-each", store_many, 0, 2},
+		{ "switch-each", switch_ring, 1, 20},
+		{ "forked-switch-each", switch_ring, ncpus, 20},
+		{ "forked-each", sync_ring, ncpus, 20},
+		{ "forked-store-each", store_ring, ncpus, 20},
+		{ "active-each", active_ring, 0, 20},
+		{ "wakeup-each", wakeup_ring, 20, 1},
+		{ "active-wakeup-each", active_wakeup_ring, 20, 1},
+		{ "double-wakeup-each", wakeup_ring, 20, 2},
+		{ NULL, NULL },
+	}, tests[] = {
+		{ "default", sync_ring, 1, 20},
+		{ "idle", idle_ring, 0, 20},
+		{ "active", active_ring, 0, 20},
+		{ "wakeup", wakeup_ring, 20, 1},
+		{ "active-wakeup", active_wakeup_ring, 20, 1},
+		{ "double-wakeup", wakeup_ring, 20, 2},
+		{ "store", store_ring, 1, 20},
+		{ "switch", switch_ring, 1, 20},
+		{ "forked-switch", switch_ring, ncpus, 20},
+		{ "many", store_many, 0, 20},
+		{ "forked", sync_ring, ncpus, 20},
+		{ "forked-store", store_ring, ncpus, 20},
+		{ NULL, NULL },
+	};
+
+
 	igt_fixture {
 		fd = drm_open_driver(DRIVER_INTEL);
 		igt_require_gem(fd);
 		gem_submission_print_method(fd);
 		gem_scheduler_print_capability(fd);
-
 		igt_fork_hang_detector(fd);
 	}
 
-	for (e = intel_execution_engines; e->name; e++) {
-		igt_subtest_f("%s", e->name)
-			sync_ring(fd, eb_ring(e), 1, 20);
-		igt_subtest_f("idle-%s", e->name)
-			idle_ring(fd, eb_ring(e), 20);
-		igt_subtest_f("active-%s", e->name)
-			active_ring(fd, eb_ring(e), 20);
-		igt_subtest_f("wakeup-%s", e->name)
-			wakeup_ring(fd, eb_ring(e), 20, 1);
-		igt_subtest_f("active-wakeup-%s", e->name)
-			active_wakeup_ring(fd, eb_ring(e), 20, 1);
-		igt_subtest_f("double-wakeup-%s", e->name)
-			wakeup_ring(fd, eb_ring(e), 20, 2);
-		igt_subtest_f("store-%s", e->name)
-			store_ring(fd, eb_ring(e), 1, 20);
-		igt_subtest_f("switch-%s", e->name)
-			switch_ring(fd, eb_ring(e), 1, 20);
-		igt_subtest_f("forked-switch-%s", e->name)
-			switch_ring(fd, eb_ring(e), ncpus, 20);
-		igt_subtest_f("many-%s", e->name)
-			store_many(fd, eb_ring(e), 20);
-		igt_subtest_f("forked-%s", e->name)
-			sync_ring(fd, eb_ring(e), ncpus, 20);
-		igt_subtest_f("forked-store-%s", e->name)
-			store_ring(fd, eb_ring(e), ncpus, 20);
-	}
+	/* legacy of selecting engines. */
 
-	igt_subtest("basic-each")
-		sync_ring(fd, ALL_ENGINES, 1, 2);
-	igt_subtest("basic-store-each")
-		store_ring(fd, ALL_ENGINES, 1, 2);
-	igt_subtest("basic-many-each")
-		store_many(fd, ALL_ENGINES, 2);
-	igt_subtest("switch-each")
-		switch_ring(fd, ALL_ENGINES, 1, 20);
-	igt_subtest("forked-switch-each")
-		switch_ring(fd, ALL_ENGINES, ncpus, 20);
-	igt_subtest("forked-each")
-		sync_ring(fd, ALL_ENGINES, ncpus, 20);
-	igt_subtest("forked-store-each")
-		store_ring(fd, ALL_ENGINES, ncpus, 20);
-	igt_subtest("active-each")
-		active_ring(fd, ALL_ENGINES, 20);
-	igt_subtest("wakeup-each")
-		wakeup_ring(fd, ALL_ENGINES, 20, 1);
-	igt_subtest("active-wakeup-each")
-		active_wakeup_ring(fd, ALL_ENGINES, 20, 1);
-	igt_subtest("double-wakeup-each")
-		wakeup_ring(fd, ALL_ENGINES, 20, 2);
+	igt_subtest_group {
+		for (test = tests; test->name; test++) {
+			igt_subtest_with_dynamic_f("legacy-engines-%s",
+						   test->name) {
+				for_each_physical_engine(e, fd) {
+					igt_dynamic_f("%s", e->name) {
+						do_test(test->func,
+							fd, eb_ring(e),
+							test->num_children,
+							test->timeout,
+							e->full_name);
+					}
+				}
+			}
+		}
+	}
 
 	igt_subtest("basic-all")
 		sync_all(fd, 1, 2);
 	igt_subtest("basic-store-all")
 		store_all(fd, 1, 2);
-
 	igt_subtest("all")
 		sync_all(fd, 1, 20);
 	igt_subtest("store-all")
@@ -1286,7 +1329,42 @@ igt_main
 	igt_subtest("forked-store-all")
 		store_all(fd, ncpus, 20);
 
+	/* All Engines */
+	igt_subtest_group {
+		for (test = allengine; test->name; test++) {
+			igt_subtest_f("%s", test->name) {
+				do_test(test->func,
+					fd, ALL_ENGINES,
+					test->num_children,
+					test->timeout,
+					test->name);
+			}
+		}
+	}
+
+	/* New way of selecting engines. */
 	igt_subtest_group {
+		const struct intel_execution_engine2 *e;
+
+		for (test = tests; test->name; test++) {
+			igt_subtest_with_dynamic_f("%s", test->name) {
+				__for_each_physical_engine(fd, e) {
+					igt_dynamic_f("%s", e->name) {
+						do_test(test->func,
+							fd, e->flags,
+							test->num_children,
+							test->timeout,
+							e->name);
+					}
+				}
+			}
+		}
+	}
+
+
+	igt_subtest_group {
+		const struct intel_execution_engine2 *e;
+
 		igt_fixture {
 			gem_require_contexts(fd);
 			igt_require(gem_scheduler_has_ctx_priority(fd));
@@ -1295,10 +1373,12 @@ igt_main
 
 		igt_subtest("preempt-all")
 			preempt(fd, ALL_ENGINES, 1, 20);
-
-		for (e = intel_execution_engines; e->name; e++) {
-			igt_subtest_f("preempt-%s", e->name)
-				preempt(fd, eb_ring(e), ncpus, 20);
+		igt_subtest_with_dynamic("preempt") {
+			__for_each_physical_engine(fd, e) {
+				/* Requires master for STORE_DWORD on gen4/5 */
+				igt_dynamic_f("%s", e->name)
+					preempt(fd, e->flags, ncpus, 20);
+			}
 		}
 	}
 
-- 
2.25.1

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

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