All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory
@ 2022-01-11 16:56 sai.gowtham.ch
  2022-01-11 17:38 ` [igt-dev] ✗ Fi.CI.BAT: failure for tests/i915/gem_sync: Add support for local memory (rev3) Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: sai.gowtham.ch @ 2022-01-11 16:56 UTC (permalink / raw)
  To: igt-dev, sai.gowtham.ch, zbigniew.kempczynski

From: Ch Sai Gowtham <sai.gowtham.ch@intel.com>

Add support for local memory region (Device memory)

Signed-off-by: Ch Sai Gowtham <sai.gowtham.ch@intel.com>
Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 lib/igt_dummyload.h   |   1 +
 tests/i915/gem_sync.c | 206 ++++++++++++++++++++++++++----------------
 2 files changed, 129 insertions(+), 78 deletions(-)

diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
index f0205261..649b6ca4 100644
--- a/lib/igt_dummyload.h
+++ b/lib/igt_dummyload.h
@@ -81,6 +81,7 @@ typedef struct igt_spin_factory {
 	unsigned int flags;
 	int fence;
 	uint64_t ahnd;
+	uint32_t region;
 } igt_spin_factory_t;
 
 #define IGT_SPIN_FENCE_IN      (1 << 0)
diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c
index 8c435845..43626a99 100644
--- a/tests/i915/gem_sync.c
+++ b/tests/i915/gem_sync.c
@@ -143,9 +143,20 @@ static void xchg_engine(void *array, unsigned i, unsigned j)
 	igt_swap(E[i], E[j]);
 }
 
+static uint32_t batch_create(int fd, uint32_t batch_size, uint32_t region)
+{
+	const uint32_t bbe = MI_BATCH_BUFFER_END;
+	uint32_t handle;
+
+	handle = gem_create_in_memory_regions(fd, batch_size, region);
+	gem_write(fd, handle, 0, &bbe, sizeof(bbe));
+
+	return handle;
+}
+
 static void
 sync_ring(int fd, const intel_ctx_t *ctx,
-	  unsigned ring, int num_children, int timeout)
+	  unsigned ring, int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 
@@ -155,15 +166,13 @@ sync_ring(int fd, const intel_ctx_t *ctx,
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object;
 		struct drm_i915_gem_execbuffer2 execbuf;
 		double start, elapsed;
 		unsigned long cycles;
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
-		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
+		object.handle = batch_create(fd, 4096, region);
 
 		memset(&execbuf, 0, sizeof(execbuf));
 		execbuf.buffers_ptr = to_user_pointer(&object);
@@ -193,9 +202,8 @@ sync_ring(int fd, const intel_ctx_t *ctx,
 
 static void
 idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
-	  int num_children, int timeout)
+	  int num_children, int timeout, uint32_t region)
 {
-	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	struct drm_i915_gem_exec_object2 object;
 	struct drm_i915_gem_execbuffer2 execbuf;
 	double start, elapsed;
@@ -204,8 +212,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
 	gem_require_ring(fd, ring);
 
 	memset(&object, 0, sizeof(object));
-	object.handle = gem_create(fd, 4096);
-	gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
+	object.handle = batch_create(fd, 4096, region);
 
 	memset(&execbuf, 0, sizeof(execbuf));
 	execbuf.buffers_ptr = to_user_pointer(&object);
@@ -234,7 +241,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
 
 static void
 wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
-	    int timeout, int wlen)
+	    int timeout, int wlen, uint32_t region)
 {
 	struct intel_engine_data ied;
 	uint64_t ahnd = get_reloc_ahnd(fd, ctx->id);
@@ -244,7 +251,6 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, ied.nengines) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object;
 		struct drm_i915_gem_execbuffer2 execbuf;
 		double end, this, elapsed, now, baseline;
@@ -254,11 +260,10 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 		ahnd = get_reloc_ahnd(fd, ctx->id);
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
+		object.handle = batch_create(fd, 4096, region);
 		object.offset = get_offset(ahnd, object.handle, 4096, 0);
 		if (ahnd)
 			object.flags = EXEC_OBJECT_PINNED;
-		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
 
 		memset(&execbuf, 0, sizeof(execbuf));
 		execbuf.buffers_ptr = to_user_pointer(&object);
@@ -339,7 +344,7 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 }
 
 static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
-			int num_children, int timeout)
+			int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 	uint64_t ahnd = get_reloc_ahnd(fd, ctx->id);
@@ -359,13 +364,15 @@ static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
 					 .ahnd = ahnd,
 					 .ctx = ctx,
 					 .engine = ied_flags(&ied, child),
-					 .flags = IGT_SPIN_FAST);
+					 .flags = IGT_SPIN_FAST,
+					 .region = region);
 
 		spin[1] = __igt_spin_new(fd,
 					 .ahnd = ahnd,
 					 .ctx = ctx,
 					 .engine = ied_flags(&ied, child),
-					 .flags = IGT_SPIN_FAST);
+					 .flags = IGT_SPIN_FAST,
+					 .region = region);
 
 		start = gettime();
 		end = start + timeout;
@@ -398,7 +405,7 @@ static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
 
 static void
 active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
-		   int timeout, int wlen)
+		   int timeout, int wlen, uint32_t region)
 {
 	struct intel_engine_data ied;
 	uint64_t ahnd0 = get_reloc_ahnd(fd, 0);
@@ -409,7 +416,6 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, ied.nengines) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object;
 		struct drm_i915_gem_execbuffer2 execbuf;
 		double end, this, elapsed, now, baseline;
@@ -420,11 +426,10 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 		ahnd = get_reloc_ahnd(fd, ctx->id);
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
+		object.handle = batch_create(fd, 4096, region);
 		object.offset = get_offset(ahnd, object.handle, 4096, 0);
 		if (ahnd)
 			object.offset = EXEC_OBJECT_PINNED;
-		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
 
 		memset(&execbuf, 0, sizeof(execbuf));
 		execbuf.buffers_ptr = to_user_pointer(&object);
@@ -529,7 +534,7 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 static void
 store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
-	   int num_children, int timeout)
+	   int num_children, int timeout, uint32_t region)
 {
 	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
 	struct intel_engine_data ied;
@@ -541,7 +546,6 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object[2];
 		struct drm_i915_gem_relocation_entry reloc[1024];
 		struct drm_i915_gem_execbuffer2 execbuf;
@@ -559,14 +563,13 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 		execbuf.rsvd1 = ctx->id;
 
 		memset(object, 0, sizeof(object));
-		object[0].handle = gem_create(fd, 4096);
-		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
+		object[0].handle = batch_create(fd, 4096, region);
 		execbuf.buffer_count = 1;
 		gem_execbuf(fd, &execbuf);
 
 		object[0].flags |= EXEC_OBJECT_WRITE;
 		object[0].flags |= has_relocs ? 0 : EXEC_OBJECT_PINNED;
-		object[1].handle = gem_create(fd, 20*1024);
+		object[1].handle = gem_create_in_memory_regions(fd, 20*1024, region);
 
 		object[1].relocs_ptr = to_user_pointer(reloc);
 		object[1].relocation_count = has_relocs ? 1024 : 0;
@@ -629,7 +632,7 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 static void
 switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
-	    int num_children, int timeout)
+	    int num_children, int timeout, uint32_t region)
 {
 	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
 	struct intel_engine_data ied;
@@ -653,7 +656,6 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 		unsigned long cycles;
 
 		for (int i = 0; i < ARRAY_SIZE(contexts); i++) {
-			const uint32_t bbe = MI_BATCH_BUFFER_END;
 			const uint32_t sz = 32 << 10;
 			struct context *c = &contexts[i];
 			uint32_t *batch, *b;
@@ -670,13 +672,12 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 			c->execbuf.rsvd1 = c->ctx->id;
 
 			memset(c->object, 0, sizeof(c->object));
-			c->object[0].handle = gem_create(fd, 4096);
-			gem_write(fd, c->object[0].handle, 0, &bbe, sizeof(bbe));
+			c->object[0].handle = batch_create(fd, 4096, region);
 			c->execbuf.buffer_count = 1;
 			gem_execbuf(fd, &c->execbuf);
 
 			c->object[0].flags |= EXEC_OBJECT_WRITE;
-			c->object[1].handle = gem_create(fd, sz);
+			c->object[1].handle = gem_create_in_memory_regions(fd, sz, region);
 
 			c->object[1].relocs_ptr = to_user_pointer(c->reloc);
 			c->object[1].relocation_count = has_relocs ? 1024 * i : 0;
@@ -813,10 +814,9 @@ static void *waiter(void *arg)
 
 static void
 __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
-	     int timeout, unsigned long *cycles)
+	     int timeout, unsigned long *cycles, uint32_t region)
 {
 	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
-	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	struct drm_i915_gem_exec_object2 object[2];
 	struct drm_i915_gem_execbuffer2 execbuf;
 	struct drm_i915_gem_relocation_entry reloc[1024];
@@ -836,8 +836,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
 	execbuf.rsvd1 = ctx->id;
 
 	memset(object, 0, sizeof(object));
-	object[0].handle = gem_create(fd, 4096);
-	gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
+	object[0].handle = batch_create(fd, 4096, region);
 	execbuf.buffer_count = 1;
 	gem_execbuf(fd, &execbuf);
 	object[0].flags |= EXEC_OBJECT_WRITE;
@@ -945,7 +944,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 static void
 store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
-	   int num_children, int timeout)
+	   int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 	unsigned long *shared;
@@ -963,7 +962,8 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
 			__store_many(fd, ctx,
 				     ied_flags(&ied, n),
 				     timeout,
-				     &shared[n]);
+				     &shared[n],
+				     region);
 	}
 	igt_waitchildren();
 
@@ -976,7 +976,7 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
 }
 
 static void
-sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
+sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 
@@ -985,15 +985,13 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object;
 		struct drm_i915_gem_execbuffer2 execbuf;
 		double start, elapsed;
 		unsigned long cycles;
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
-		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
+		object.handle = batch_create(fd, 4096, region);
 
 		memset(&execbuf, 0, sizeof(execbuf));
 		execbuf.buffers_ptr = to_user_pointer(&object);
@@ -1023,7 +1021,7 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 }
 
 static void
-store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
+store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
 {
 	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
 	struct intel_engine_data ied;
@@ -1034,7 +1032,6 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object[2];
 		struct drm_i915_gem_relocation_entry reloc[1024];
 		struct drm_i915_gem_execbuffer2 execbuf;
@@ -1051,14 +1048,13 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 		execbuf.rsvd1 = ctx->id;
 
 		memset(object, 0, sizeof(object));
-		object[0].handle = gem_create(fd, 4096);
-		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
+		object[0].handle = batch_create(fd, 4096, region);
 		execbuf.buffer_count = 1;
 		gem_execbuf(fd, &execbuf);
 
 		object[0].flags |= EXEC_OBJECT_WRITE;
 		object[0].flags |= has_relocs ? 0 : EXEC_OBJECT_PINNED;
-		object[1].handle = gem_create(fd, 1024*16 + 4096);
+		object[1].handle = gem_create_in_memory_regions(fd, 1024*16 + 4096, region);
 
 		object[1].relocs_ptr = to_user_pointer(reloc);
 		object[1].relocation_count = has_relocs ? 1024 : 0;
@@ -1126,7 +1122,7 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 
 static void
 preempt(int fd, const intel_ctx_t *ctx, unsigned ring,
-	int num_children, int timeout)
+	int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 	const intel_ctx_t *tmp_ctx[2];
@@ -1144,7 +1140,6 @@ preempt(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object;
 		struct drm_i915_gem_execbuffer2 execbuf;
 		double start, elapsed;
@@ -1153,11 +1148,10 @@ preempt(int fd, const intel_ctx_t *ctx, unsigned ring,
 		ahnd = get_reloc_ahnd(fd, 0);
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
+		object.handle = batch_create(fd, 4096, region);
 		object.offset = get_offset(ahnd, object.handle, 4096, 0);
 		if (ahnd)
 			object.flags = EXEC_OBJECT_PINNED;
-		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
 
 		memset(&execbuf, 0, sizeof(execbuf));
 		execbuf.buffers_ptr = to_user_pointer(&object);
@@ -1205,7 +1199,7 @@ igt_main
 	const struct {
 		const char *name;
 		void (*func)(int fd, const intel_ctx_t *ctx, unsigned int engine,
-			     int num_children, int timeout);
+			     int num_children, int timeout, uint32_t memregion);
 		int num_children;
 		int timeout;
 	} all[] = {
@@ -1236,11 +1230,40 @@ igt_main
 		{ "forked-store", store_ring, ncpus, 20 },
 		{}
 	};
+	const struct test {
+		const char *name;
+		void (*func) (int, const intel_ctx_t *, int, int, uint32_t);
+		int num_children;
+		int timeout;
+	} *test, tests[] = {
+		{ "basic-all", sync_all, 1, 2},
+		{ "basic-store-all", store_all, 1, 2},
+		{ "all", sync_all, 1, 20},
+		{ "store-all", store_all, 1, 20},
+		{ "forked-all", sync_all, ncpus, 20},
+		{ "forked-store-all", store_all, ncpus, 20},
+		{}
+	};
+#define subtest_for_each_combination(__name, __ctx, __num_children, __timeout, __func) \
+	igt_subtest_with_dynamic(__name) { \
+		for_each_combination(regions, 1, set) { \
+			sub_name = memregion_dynamic_subtest_name(regions); \
+			region = igt_collection_get_value(regions, 0); \
+			igt_dynamic_f("%s", sub_name) \
+				(__func)(fd, (__ctx), (__num_children), (__timeout), region); \
+			free(sub_name); \
+		} \
+	}
+
 #define for_each_test(t, T) for(typeof(*T) *t = T; t->name; t++)
 
 	const struct intel_execution_engine2 *e;
 	const intel_ctx_t *ctx;
 	int fd = -1;
+	struct drm_i915_query_memory_regions *query_info;
+	struct igt_collection *regions, *set;
+	char *sub_name;
+	uint32_t region;
 
 	igt_fixture {
 		fd = drm_open_driver(DRIVER_INTEL);
@@ -1250,6 +1273,11 @@ igt_main
 		ctx = intel_ctx_create_all_physical(fd);
 
 		igt_fork_hang_detector(fd);
+		query_info = gem_get_query_memory_regions(fd);
+		igt_assert(query_info);
+		set = get_memory_region_set(query_info,
+					I915_SYSTEM_MEMORY,
+					I915_DEVICE_MEMORY);
 		intel_allocator_multiprocess_start();
 	}
 
@@ -1257,41 +1285,48 @@ igt_main
 	for_each_test(t, individual) {
 		igt_subtest_with_dynamic_f("legacy-%s", t->name) {
 			for (const struct intel_execution_ring *l = intel_execution_rings; l->name; l++) {
-				igt_dynamic_f("%s", l->name) {
-					t->func(fd, intel_ctx_0(fd), eb_ring(l),
-						t->num_children, t->timeout);
+				for_each_combination(regions, 1, set) {
+					sub_name = memregion_dynamic_subtest_name(regions);
+					region = igt_collection_get_value(regions, 0);
+					igt_dynamic_f("%s-%s", l->name, sub_name) {
+						t->func(fd, intel_ctx_0(fd), eb_ring(l),
+								t->num_children,
+								t->timeout,
+								region);
+					}
+					free(sub_name);
 				}
 			}
 		}
 	}
-
-	igt_subtest("basic-all")
-		sync_all(fd, ctx, 1, 2);
-	igt_subtest("basic-store-all")
-		store_all(fd, ctx, 1, 2);
-
-	igt_subtest("all")
-		sync_all(fd, ctx, 1, 20);
-	igt_subtest("store-all")
-		store_all(fd, ctx, 1, 20);
-	igt_subtest("forked-all")
-		sync_all(fd, ctx, ncpus, 20);
-	igt_subtest("forked-store-all")
-		store_all(fd, ctx, ncpus, 20);
+	for (test= tests; test->name; test++)
+		subtest_for_each_combination(test->name, ctx, test->num_children, test->timeout, test->func);
 
 	for_each_test(t, all) {
-		igt_subtest_f("%s", t->name)
-			t->func(fd, ctx, ALL_ENGINES, t->num_children, t->timeout);
+		igt_subtest_with_dynamic_f("%s", t->name) {
+			for_each_combination(regions, 1, set) {
+				sub_name = memregion_dynamic_subtest_name(regions);
+				region = igt_collection_get_value(regions, 0);
+				igt_dynamic_f("%s", sub_name)
+					t->func(fd, ctx, ALL_ENGINES, t->num_children,
+							t->timeout, region);
+				free(sub_name);
+			}
+		}
 	}
 
 	/* New way of selecting engines. */
 	for_each_test(t, individual) {
 		igt_subtest_with_dynamic_f("%s", t->name) {
-			for_each_ctx_engine(fd, ctx, e) {
-				igt_dynamic_f("%s", e->name) {
-					t->func(fd, ctx, e->flags,
-						t->num_children, t->timeout);
-				}
+			for_each_combination(regions, 1, set) {
+				sub_name = memregion_dynamic_subtest_name(regions);
+				region = igt_collection_get_value(regions, 0);
+				for_each_ctx_engine(fd, ctx, e)
+					igt_dynamic_f("%s-%s", e->name, sub_name)
+						t->func(fd, ctx, e->flags,
+								t->num_children,
+								t->timeout, region);
+				free(sub_name);
 			}
 		}
 	}
@@ -1303,17 +1338,32 @@ igt_main
 			igt_require(gem_scheduler_has_preemption(fd));
 		}
 
-		igt_subtest("preempt-all")
-			preempt(fd, ctx, ALL_ENGINES, 1, 20);
+		igt_subtest_with_dynamic("preempt-all") {
+			for_each_combination(regions, 1, set) {
+				sub_name = memregion_dynamic_subtest_name(regions);
+				region = igt_collection_get_value(regions, 0);
+				igt_dynamic_f("%s", sub_name)
+					preempt(fd, ctx, ALL_ENGINES, 1, 20, region);
+				free(sub_name);
+			}
+		}
+
 		igt_subtest_with_dynamic("preempt") {
-			for_each_ctx_engine(fd, ctx, e) {
-				igt_dynamic_f("%s", e->name)
-					preempt(fd, ctx, e->flags, ncpus, 20);
+			for_each_combination(regions, 1, set) {
+				sub_name = memregion_dynamic_subtest_name(regions);
+				region = igt_collection_get_value(regions, 0);
+				for_each_ctx_engine(fd, ctx, e) {
+					igt_dynamic_f("%s-%s", e->name, sub_name)
+						preempt(fd, ctx, e->flags, ncpus, 20, region);
+					free(sub_name);
+				}
 			}
 		}
 	}
 
 	igt_fixture {
+		free(query_info);
+		igt_collection_destroy(set);
 		intel_allocator_multiprocess_stop();
 		igt_stop_hang_detector();
 		intel_ctx_destroy(fd, ctx);
-- 
2.32.0

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

* [igt-dev] ✗ Fi.CI.BAT: failure for tests/i915/gem_sync: Add support for local memory (rev3)
  2022-01-11 16:56 [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory sai.gowtham.ch
@ 2022-01-11 17:38 ` Patchwork
  2022-01-11 17:57 ` [igt-dev] ✗ GitLab.Pipeline: warning " Patchwork
  2022-01-12 17:47 ` [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory Zbigniew Kempczyński
  2 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2022-01-11 17:38 UTC (permalink / raw)
  To: sai.gowtham.ch; +Cc: igt-dev

[-- Attachment #1: Type: text/plain, Size: 5394 bytes --]

== Series Details ==

Series: tests/i915/gem_sync: Add support for local memory (rev3)
URL   : https://patchwork.freedesktop.org/series/95437/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11066 -> IGTPW_6551
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_6551 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_6551, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6551/index.html

Participating hosts (43 -> 41)
------------------------------

  Missing    (2): fi-bsw-cyan fi-bdw-samus 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_6551:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@evict:
    - fi-kbl-soraka:      [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11066/fi-kbl-soraka/igt@i915_selftest@live@evict.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6551/fi-kbl-soraka/igt@i915_selftest@live@evict.html

  
New tests
---------

  New tests have been introduced between CI_DRM_11066 and IGTPW_6551:

### New IGT tests (4) ###

  * igt@gem_sync@basic-all@lmem0:
    - Statuses : 1 pass(s)
    - Exec time: [5.74] s

  * igt@gem_sync@basic-all@smem:
    - Statuses : 38 pass(s)
    - Exec time: [2.04, 5.74] s

  * igt@gem_sync@basic-each@lmem0:
    - Statuses : 1 pass(s)
    - Exec time: [2.58] s

  * igt@gem_sync@basic-each@smem:
    - Statuses : 38 pass(s)
    - Exec time: [2.05, 3.91] s

  

Known issues
------------

  Here are the changes found in IGTPW_6551 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_cs_nop@sync-fork-gfx0:
    - fi-skl-6600u:       NOTRUN -> [SKIP][3] ([fdo#109271]) +21 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6551/fi-skl-6600u/igt@amdgpu/amd_cs_nop@sync-fork-gfx0.html

  * igt@gem_huc_copy@huc-copy:
    - fi-skl-6600u:       NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#2190])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6551/fi-skl-6600u/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@verify-random:
    - fi-skl-6600u:       NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#4613]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6551/fi-skl-6600u/igt@gem_lmem_swapping@verify-random.html

  * igt@kms_chamelium@vga-edid-read:
    - fi-skl-6600u:       NOTRUN -> [SKIP][6] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6551/fi-skl-6600u/igt@kms_chamelium@vga-edid-read.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - fi-skl-6600u:       NOTRUN -> [SKIP][7] ([fdo#109271] / [i915#533])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6551/fi-skl-6600u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3@smem:
    - fi-tgl-1115g4:      [FAIL][8] ([i915#1888]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11066/fi-tgl-1115g4/igt@gem_exec_suspend@basic-s3@smem.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6551/fi-tgl-1115g4/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@a-vga1:
    - fi-bwr-2160:        [FAIL][10] ([i915#2122]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11066/fi-bwr-2160/igt@kms_flip@basic-flip-vs-wf_vblank@a-vga1.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6551/fi-bwr-2160/igt@kms_flip@basic-flip-vs-wf_vblank@a-vga1.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-cml-u2:          [DMESG-WARN][12] ([i915#4269]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11066/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6551/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#4269]: https://gitlab.freedesktop.org/drm/intel/issues/4269
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_6326 -> IGTPW_6551

  CI-20190529: 20190529
  CI_DRM_11066: fc076e8fc52ed40fee33f416a4475a57219011a5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_6551: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6551/index.html
  IGT_6326: ec75f64fcbcf4aac58fbf1bf629e8f59b19db4ce @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6551/index.html

[-- Attachment #2: Type: text/html, Size: 6619 bytes --]

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

* [igt-dev] ✗ GitLab.Pipeline: warning for tests/i915/gem_sync: Add support for local memory (rev3)
  2022-01-11 16:56 [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory sai.gowtham.ch
  2022-01-11 17:38 ` [igt-dev] ✗ Fi.CI.BAT: failure for tests/i915/gem_sync: Add support for local memory (rev3) Patchwork
@ 2022-01-11 17:57 ` Patchwork
  2022-01-12 17:47 ` [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory Zbigniew Kempczyński
  2 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2022-01-11 17:57 UTC (permalink / raw)
  To: sai.gowtham.ch; +Cc: igt-dev

== Series Details ==

Series: tests/i915/gem_sync: Add support for local memory (rev3)
URL   : https://patchwork.freedesktop.org/series/95437/
State : warning

== Summary ==

Pipeline status: FAILED.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/481560 for the overview.

build-containers:build-debian-mips has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/17488613):
  Authenticating with credentials from job payload (GitLab Registry)
  Pulling docker image registry.freedesktop.org/wayland/ci-templates/buildah:2019-08-13.0 ...
  Using docker image sha256:594aa868d31ee3304dee8cae8a3433c89a6fcfcf6c7d420c04cce22f60147176 for registry.freedesktop.org/wayland/ci-templates/buildah:2019-08-13.0 with digest registry.freedesktop.org/wayland/ci-templates/buildah@sha256:7dbcf22cd2c1c7d49db0dc7b4ab207c3d6a4a09bd81cc3b71a688d3727d8749f ...
  section_end:1641921629:prepare_executor
  section_start:1641921629:prepare_script
  Preparing environment
  Running on runner-zbmz6-qr-project-3185-concurrent-0 via fdo-packet-m1xl-1...
  section_end:1641921632:prepare_script
  section_start:1641921632:get_sources
  Getting source from Git repository
  $ eval "$CI_PRE_CLONE_SCRIPT"
  Fetching changes...
  Reinitialized existing Git repository in /builds/gfx-ci/igt-ci-tags/.git/
  fatal: unable to access 'https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags.git/': The requested URL returned error: 504
  section_end:1641923434:get_sources
  section_start:1641923434:cleanup_file_variables
  Cleaning up file based variables
  section_end:1641923435:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build-containers:build-fedora has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/17488614):
  Authenticating with credentials from job payload (GitLab Registry)
  Pulling docker image registry.freedesktop.org/wayland/ci-templates/buildah:2019-08-13.0 ...
  Using docker image sha256:594aa868d31ee3304dee8cae8a3433c89a6fcfcf6c7d420c04cce22f60147176 for registry.freedesktop.org/wayland/ci-templates/buildah:2019-08-13.0 with digest registry.freedesktop.org/wayland/ci-templates/buildah@sha256:7dbcf22cd2c1c7d49db0dc7b4ab207c3d6a4a09bd81cc3b71a688d3727d8749f ...
  section_end:1641921834:prepare_executor
  section_start:1641921834:prepare_script
  Preparing environment
  Running on runner-lb3m2qse-project-3185-concurrent-2 via fdo-packet-m1xl-3...
  section_end:1641921837:prepare_script
  section_start:1641921837:get_sources
  Getting source from Git repository
  $ eval "$CI_PRE_CLONE_SCRIPT"
  Fetching changes...
  Reinitialized existing Git repository in /builds/gfx-ci/igt-ci-tags/.git/
  fatal: unable to access 'https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags.git/': The requested URL returned error: 504
  section_end:1641923640:get_sources
  section_start:1641923640:cleanup_file_variables
  Cleaning up file based variables
  section_end:1641923642:cleanup_file_variables
  ERROR: Job failed: exit code 1

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/481560

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

* Re: [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory
  2022-01-11 16:56 [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory sai.gowtham.ch
  2022-01-11 17:38 ` [igt-dev] ✗ Fi.CI.BAT: failure for tests/i915/gem_sync: Add support for local memory (rev3) Patchwork
  2022-01-11 17:57 ` [igt-dev] ✗ GitLab.Pipeline: warning " Patchwork
@ 2022-01-12 17:47 ` Zbigniew Kempczyński
  2 siblings, 0 replies; 11+ messages in thread
From: Zbigniew Kempczyński @ 2022-01-12 17:47 UTC (permalink / raw)
  To: sai.gowtham.ch; +Cc: igt-dev

On Tue, Jan 11, 2022 at 10:26:00PM +0530, sai.gowtham.ch@intel.com wrote:
> From: Ch Sai Gowtham <sai.gowtham.ch@intel.com>
> 
> Add support for local memory region (Device memory)
> 
> Signed-off-by: Ch Sai Gowtham <sai.gowtham.ch@intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> ---
>  lib/igt_dummyload.h   |   1 +
>  tests/i915/gem_sync.c | 206 ++++++++++++++++++++++++++----------------
>  2 files changed, 129 insertions(+), 78 deletions(-)
> 
> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
> index f0205261..649b6ca4 100644
> --- a/lib/igt_dummyload.h
> +++ b/lib/igt_dummyload.h
> @@ -81,6 +81,7 @@ typedef struct igt_spin_factory {
>  	unsigned int flags;
>  	int fence;
>  	uint64_t ahnd;
> +	uint32_t region;
>  } igt_spin_factory_t;

You've added region as part of structure and spinner code can use it
but spinner object still comes from system memory.

Take a look to handle_create() and emit_recursive_batch() from
igt_dummyload.c. Be aware userptr excludes regioning so add appropriate
assertion there.

Also use gem_detect_safe_alignment() when allocating offsets from
allocator in igt_dummyload.c for spinner.

>  
>  #define IGT_SPIN_FENCE_IN      (1 << 0)
> diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c
> index 8c435845..43626a99 100644
> --- a/tests/i915/gem_sync.c
> +++ b/tests/i915/gem_sync.c
> @@ -143,9 +143,20 @@ static void xchg_engine(void *array, unsigned i, unsigned j)
>  	igt_swap(E[i], E[j]);
>  }
>  
> +static uint32_t batch_create(int fd, uint32_t batch_size, uint32_t region)
> +{
> +	const uint32_t bbe = MI_BATCH_BUFFER_END;
> +	uint32_t handle;
> +
> +	handle = gem_create_in_memory_regions(fd, batch_size, region);
> +	gem_write(fd, handle, 0, &bbe, sizeof(bbe));
> +
> +	return handle;
> +}
> +

Ok, we can have common helper code. It will bite us soon due to requested
object size can be smaller than object size created. So passing requested
size to allocator will create overlapped allocations and -ENOSPC.
But don't worry about this right now.

>  static void
>  sync_ring(int fd, const intel_ctx_t *ctx,
> -	  unsigned ring, int num_children, int timeout)
> +	  unsigned ring, int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  
> @@ -155,15 +166,13 @@ sync_ring(int fd, const intel_ctx_t *ctx,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double start, elapsed;
>  		unsigned long cycles;
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +		object.handle = batch_create(fd, 4096, region);
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -193,9 +202,8 @@ sync_ring(int fd, const intel_ctx_t *ctx,
>  
>  static void
>  idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
> -	  int num_children, int timeout)
> +	  int num_children, int timeout, uint32_t region)
>  {
> -	const uint32_t bbe = MI_BATCH_BUFFER_END;
>  	struct drm_i915_gem_exec_object2 object;
>  	struct drm_i915_gem_execbuffer2 execbuf;
>  	double start, elapsed;
> @@ -204,8 +212,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  	gem_require_ring(fd, ring);
>  
>  	memset(&object, 0, sizeof(object));
> -	object.handle = gem_create(fd, 4096);
> -	gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +	object.handle = batch_create(fd, 4096, region);
>  
>  	memset(&execbuf, 0, sizeof(execbuf));
>  	execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -234,7 +241,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  
>  static void
>  wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	    int timeout, int wlen)
> +	    int timeout, int wlen, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  	uint64_t ahnd = get_reloc_ahnd(fd, ctx->id);
> @@ -244,7 +251,6 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, ied.nengines) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double end, this, elapsed, now, baseline;
> @@ -254,11 +260,10 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		ahnd = get_reloc_ahnd(fd, ctx->id);
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> +		object.handle = batch_create(fd, 4096, region);
>  		object.offset = get_offset(ahnd, object.handle, 4096, 0);
>  		if (ahnd)
>  			object.flags = EXEC_OBJECT_PINNED;
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -339,7 +344,7 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  }
>  
>  static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
> -			int num_children, int timeout)
> +			int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  	uint64_t ahnd = get_reloc_ahnd(fd, ctx->id);
> @@ -359,13 +364,15 @@ static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  					 .ahnd = ahnd,
>  					 .ctx = ctx,
>  					 .engine = ied_flags(&ied, child),
> -					 .flags = IGT_SPIN_FAST);
> +					 .flags = IGT_SPIN_FAST,
> +					 .region = region);
>  
>  		spin[1] = __igt_spin_new(fd,
>  					 .ahnd = ahnd,
>  					 .ctx = ctx,
>  					 .engine = ied_flags(&ied, child),
> -					 .flags = IGT_SPIN_FAST);
> +					 .flags = IGT_SPIN_FAST,
> +					 .region = region);

Passing region here looks correct.

>  
>  		start = gettime();
>  		end = start + timeout;
> @@ -398,7 +405,7 @@ static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  
>  static void
>  active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -		   int timeout, int wlen)
> +		   int timeout, int wlen, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  	uint64_t ahnd0 = get_reloc_ahnd(fd, 0);
> @@ -409,7 +416,6 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, ied.nengines) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double end, this, elapsed, now, baseline;
> @@ -420,11 +426,10 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		ahnd = get_reloc_ahnd(fd, ctx->id);
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> +		object.handle = batch_create(fd, 4096, region);
>  		object.offset = get_offset(ahnd, object.handle, 4096, 0);
>  		if (ahnd)
>  			object.offset = EXEC_OBJECT_PINNED;
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));

These spinners also can be instantiated from region passed as an argument.

>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -529,7 +534,7 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  static void
>  store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	   int num_children, int timeout)
> +	   int num_children, int timeout, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
>  	struct intel_engine_data ied;
> @@ -541,7 +546,6 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object[2];
>  		struct drm_i915_gem_relocation_entry reloc[1024];
>  		struct drm_i915_gem_execbuffer2 execbuf;
> @@ -559,14 +563,13 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		execbuf.rsvd1 = ctx->id;
>  
>  		memset(object, 0, sizeof(object));
> -		object[0].handle = gem_create(fd, 4096);
> -		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
> +		object[0].handle = batch_create(fd, 4096, region);
>  		execbuf.buffer_count = 1;
>  		gem_execbuf(fd, &execbuf);
>  
>  		object[0].flags |= EXEC_OBJECT_WRITE;
>  		object[0].flags |= has_relocs ? 0 : EXEC_OBJECT_PINNED;
> -		object[1].handle = gem_create(fd, 20*1024);
> +		object[1].handle = gem_create_in_memory_regions(fd, 20*1024, region);
>  
>  		object[1].relocs_ptr = to_user_pointer(reloc);
>  		object[1].relocation_count = has_relocs ? 1024 : 0;
> @@ -629,7 +632,7 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  static void
>  switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	    int num_children, int timeout)
> +	    int num_children, int timeout, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
>  	struct intel_engine_data ied;
> @@ -653,7 +656,6 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		unsigned long cycles;
>  
>  		for (int i = 0; i < ARRAY_SIZE(contexts); i++) {
> -			const uint32_t bbe = MI_BATCH_BUFFER_END;
>  			const uint32_t sz = 32 << 10;
>  			struct context *c = &contexts[i];
>  			uint32_t *batch, *b;
> @@ -670,13 +672,12 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  			c->execbuf.rsvd1 = c->ctx->id;
>  
>  			memset(c->object, 0, sizeof(c->object));
> -			c->object[0].handle = gem_create(fd, 4096);
> -			gem_write(fd, c->object[0].handle, 0, &bbe, sizeof(bbe));
> +			c->object[0].handle = batch_create(fd, 4096, region);
>  			c->execbuf.buffer_count = 1;
>  			gem_execbuf(fd, &c->execbuf);
>  
>  			c->object[0].flags |= EXEC_OBJECT_WRITE;
> -			c->object[1].handle = gem_create(fd, sz);
> +			c->object[1].handle = gem_create_in_memory_regions(fd, sz, region);
>  
>  			c->object[1].relocs_ptr = to_user_pointer(c->reloc);
>  			c->object[1].relocation_count = has_relocs ? 1024 * i : 0;
> @@ -813,10 +814,9 @@ static void *waiter(void *arg)
>  
>  static void
>  __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	     int timeout, unsigned long *cycles)
> +	     int timeout, unsigned long *cycles, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
> -	const uint32_t bbe = MI_BATCH_BUFFER_END;
>  	struct drm_i915_gem_exec_object2 object[2];
>  	struct drm_i915_gem_execbuffer2 execbuf;
>  	struct drm_i915_gem_relocation_entry reloc[1024];
> @@ -836,8 +836,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
>  	execbuf.rsvd1 = ctx->id;
>  
>  	memset(object, 0, sizeof(object));
> -	object[0].handle = gem_create(fd, 4096);
> -	gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
> +	object[0].handle = batch_create(fd, 4096, region);
>  	execbuf.buffer_count = 1;
>  	gem_execbuf(fd, &execbuf);
>  	object[0].flags |= EXEC_OBJECT_WRITE;

Interesting code from this function is:

	for (int i = 0; i < ARRAY_SIZE(threads); i++) {
		threads[i].fd = fd;
		threads[i].object = object[1];
		threads[i].object.handle = gem_create(fd, 20*1024);
		gem_write(fd, threads[i].object.handle, 0, batch, 20*1024);

		pthread_cond_init(&threads[i].cond, NULL);
		pthread_mutex_init(&threads[i].mutex, NULL);
		threads[i].done = &done;
		threads[i].ready = 0;

		pthread_create(&threads[i].thread, NULL, waiter, &threads[i]);
		order[i] = i;
	}

Currently all interesting objects are from smem. I would juggle with memory
region used for creating those objects - for i % 2 I would use region, 
otherwise normal gem_create().

> @@ -945,7 +944,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  static void
>  store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
> -	   int num_children, int timeout)
> +	   int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  	unsigned long *shared;
> @@ -963,7 +962,8 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  			__store_many(fd, ctx,
>  				     ied_flags(&ied, n),
>  				     timeout,
> -				     &shared[n]);
> +				     &shared[n],
> +				     region);
>  	}
>  	igt_waitchildren();
>  
> @@ -976,7 +976,7 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  }
>  
>  static void
> -sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
> +sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  
> @@ -985,15 +985,13 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double start, elapsed;
>  		unsigned long cycles;
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +		object.handle = batch_create(fd, 4096, region);
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -1023,7 +1021,7 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  }
>  
>  static void
> -store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
> +store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
>  	struct intel_engine_data ied;
> @@ -1034,7 +1032,6 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object[2];
>  		struct drm_i915_gem_relocation_entry reloc[1024];
>  		struct drm_i915_gem_execbuffer2 execbuf;
> @@ -1051,14 +1048,13 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  		execbuf.rsvd1 = ctx->id;
>  
>  		memset(object, 0, sizeof(object));
> -		object[0].handle = gem_create(fd, 4096);
> -		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
> +		object[0].handle = batch_create(fd, 4096, region);
>  		execbuf.buffer_count = 1;
>  		gem_execbuf(fd, &execbuf);
>  
>  		object[0].flags |= EXEC_OBJECT_WRITE;
>  		object[0].flags |= has_relocs ? 0 : EXEC_OBJECT_PINNED;
> -		object[1].handle = gem_create(fd, 1024*16 + 4096);
> +		object[1].handle = gem_create_in_memory_regions(fd, 1024*16 + 4096, region);
>  
>  		object[1].relocs_ptr = to_user_pointer(reloc);
>  		object[1].relocation_count = has_relocs ? 1024 : 0;
> @@ -1126,7 +1122,7 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  
>  static void
>  preempt(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	int num_children, int timeout)
> +	int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  	const intel_ctx_t *tmp_ctx[2];
> @@ -1144,7 +1140,6 @@ preempt(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double start, elapsed;
> @@ -1153,11 +1148,10 @@ preempt(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		ahnd = get_reloc_ahnd(fd, 0);
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> +		object.handle = batch_create(fd, 4096, region);
>  		object.offset = get_offset(ahnd, object.handle, 4096, 0);
>  		if (ahnd)
>  			object.flags = EXEC_OBJECT_PINNED;
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -1205,7 +1199,7 @@ igt_main
>  	const struct {
>  		const char *name;
>  		void (*func)(int fd, const intel_ctx_t *ctx, unsigned int engine,
> -			     int num_children, int timeout);
> +			     int num_children, int timeout, uint32_t memregion);
>  		int num_children;
>  		int timeout;
>  	} all[] = {
> @@ -1236,11 +1230,40 @@ igt_main
>  		{ "forked-store", store_ring, ncpus, 20 },
>  		{}
>  	};
> +	const struct test {
> +		const char *name;
> +		void (*func) (int, const intel_ctx_t *, int, int, uint32_t);
> +		int num_children;
> +		int timeout;
> +	} *test, tests[] = {
> +		{ "basic-all", sync_all, 1, 2},
> +		{ "basic-store-all", store_all, 1, 2},
> +		{ "all", sync_all, 1, 20},
> +		{ "store-all", store_all, 1, 20},
> +		{ "forked-all", sync_all, ncpus, 20},
> +		{ "forked-store-all", store_all, ncpus, 20},

Adding space before last bracket would be welcome.

Consider to add two macros around for_each_combination (similar to
gem_exec_suspend).  If I good see the pattern one of that macro will
need to for_each_ctx_engine() whereas second will not.

--
Zbigniew

> +		{}
> +	};
> +#define subtest_for_each_combination(__name, __ctx, __num_children, __timeout, __func) \
> +	igt_subtest_with_dynamic(__name) { \
> +		for_each_combination(regions, 1, set) { \
> +			sub_name = memregion_dynamic_subtest_name(regions); \
> +			region = igt_collection_get_value(regions, 0); \
> +			igt_dynamic_f("%s", sub_name) \
> +				(__func)(fd, (__ctx), (__num_children), (__timeout), region); \
> +			free(sub_name); \
> +		} \
> +	}
> +
>  #define for_each_test(t, T) for(typeof(*T) *t = T; t->name; t++)
>  
>  	const struct intel_execution_engine2 *e;
>  	const intel_ctx_t *ctx;
>  	int fd = -1;
> +	struct drm_i915_query_memory_regions *query_info;
> +	struct igt_collection *regions, *set;
> +	char *sub_name;
> +	uint32_t region;
>  
>  	igt_fixture {
>  		fd = drm_open_driver(DRIVER_INTEL);
> @@ -1250,6 +1273,11 @@ igt_main
>  		ctx = intel_ctx_create_all_physical(fd);
>  
>  		igt_fork_hang_detector(fd);
> +		query_info = gem_get_query_memory_regions(fd);
> +		igt_assert(query_info);
> +		set = get_memory_region_set(query_info,
> +					I915_SYSTEM_MEMORY,
> +					I915_DEVICE_MEMORY);
>  		intel_allocator_multiprocess_start();
>  	}
>  
> @@ -1257,41 +1285,48 @@ igt_main
>  	for_each_test(t, individual) {
>  		igt_subtest_with_dynamic_f("legacy-%s", t->name) {
>  			for (const struct intel_execution_ring *l = intel_execution_rings; l->name; l++) {
> -				igt_dynamic_f("%s", l->name) {
> -					t->func(fd, intel_ctx_0(fd), eb_ring(l),
> -						t->num_children, t->timeout);
> +				for_each_combination(regions, 1, set) {
> +					sub_name = memregion_dynamic_subtest_name(regions);
> +					region = igt_collection_get_value(regions, 0);
> +					igt_dynamic_f("%s-%s", l->name, sub_name) {
> +						t->func(fd, intel_ctx_0(fd), eb_ring(l),
> +								t->num_children,
> +								t->timeout,
> +								region);
> +					}
> +					free(sub_name);
>  				}
>  			}
>  		}
>  	}
> -
> -	igt_subtest("basic-all")
> -		sync_all(fd, ctx, 1, 2);
> -	igt_subtest("basic-store-all")
> -		store_all(fd, ctx, 1, 2);
> -
> -	igt_subtest("all")
> -		sync_all(fd, ctx, 1, 20);
> -	igt_subtest("store-all")
> -		store_all(fd, ctx, 1, 20);
> -	igt_subtest("forked-all")
> -		sync_all(fd, ctx, ncpus, 20);
> -	igt_subtest("forked-store-all")
> -		store_all(fd, ctx, ncpus, 20);
> +	for (test= tests; test->name; test++)
> +		subtest_for_each_combination(test->name, ctx, test->num_children, test->timeout, test->func);
>  
>  	for_each_test(t, all) {
> -		igt_subtest_f("%s", t->name)
> -			t->func(fd, ctx, ALL_ENGINES, t->num_children, t->timeout);
> +		igt_subtest_with_dynamic_f("%s", t->name) {
> +			for_each_combination(regions, 1, set) {
> +				sub_name = memregion_dynamic_subtest_name(regions);
> +				region = igt_collection_get_value(regions, 0);
> +				igt_dynamic_f("%s", sub_name)
> +					t->func(fd, ctx, ALL_ENGINES, t->num_children,
> +							t->timeout, region);
> +				free(sub_name);
> +			}
> +		}
>  	}
>  
>  	/* New way of selecting engines. */
>  	for_each_test(t, individual) {
>  		igt_subtest_with_dynamic_f("%s", t->name) {
> -			for_each_ctx_engine(fd, ctx, e) {
> -				igt_dynamic_f("%s", e->name) {
> -					t->func(fd, ctx, e->flags,
> -						t->num_children, t->timeout);
> -				}
> +			for_each_combination(regions, 1, set) {
> +				sub_name = memregion_dynamic_subtest_name(regions);
> +				region = igt_collection_get_value(regions, 0);
> +				for_each_ctx_engine(fd, ctx, e)
> +					igt_dynamic_f("%s-%s", e->name, sub_name)
> +						t->func(fd, ctx, e->flags,
> +								t->num_children,
> +								t->timeout, region);
> +				free(sub_name);
>  			}
>  		}
>  	}
> @@ -1303,17 +1338,32 @@ igt_main
>  			igt_require(gem_scheduler_has_preemption(fd));
>  		}
>  
> -		igt_subtest("preempt-all")
> -			preempt(fd, ctx, ALL_ENGINES, 1, 20);
> +		igt_subtest_with_dynamic("preempt-all") {
> +			for_each_combination(regions, 1, set) {
> +				sub_name = memregion_dynamic_subtest_name(regions);
> +				region = igt_collection_get_value(regions, 0);
> +				igt_dynamic_f("%s", sub_name)
> +					preempt(fd, ctx, ALL_ENGINES, 1, 20, region);
> +				free(sub_name);
> +			}
> +		}
> +
>  		igt_subtest_with_dynamic("preempt") {
> -			for_each_ctx_engine(fd, ctx, e) {
> -				igt_dynamic_f("%s", e->name)
> -					preempt(fd, ctx, e->flags, ncpus, 20);
> +			for_each_combination(regions, 1, set) {
> +				sub_name = memregion_dynamic_subtest_name(regions);
> +				region = igt_collection_get_value(regions, 0);
> +				for_each_ctx_engine(fd, ctx, e) {
> +					igt_dynamic_f("%s-%s", e->name, sub_name)
> +						preempt(fd, ctx, e->flags, ncpus, 20, region);
> +					free(sub_name);
> +				}
>  			}
>  		}
>  	}
>  
>  	igt_fixture {
> +		free(query_info);
> +		igt_collection_destroy(set);
>  		intel_allocator_multiprocess_stop();
>  		igt_stop_hang_detector();
>  		intel_ctx_destroy(fd, ctx);
> -- 
> 2.32.0
> 

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

* Re: [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory
  2022-02-03  5:25 sai.gowtham.ch
@ 2022-02-15 12:07 ` Zbigniew Kempczyński
  0 siblings, 0 replies; 11+ messages in thread
From: Zbigniew Kempczyński @ 2022-02-15 12:07 UTC (permalink / raw)
  To: sai.gowtham.ch; +Cc: igt-dev

On Thu, Feb 03, 2022 at 10:55:43AM +0530, sai.gowtham.ch@intel.com wrote:
> From: Ch Sai Gowtham <sai.gowtham.ch@intel.com>
> 
> Add support for local memory region (Device memory)
> 
> Signed-off-by: Ch Sai Gowtham <sai.gowtham.ch@intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> ---
>  lib/igt_dummyload.c   |   8 +-
>  lib/igt_dummyload.h   |   1 +
>  tests/i915/gem_sync.c | 231 ++++++++++++++++++++++++++----------------
>  3 files changed, 147 insertions(+), 93 deletions(-)
> 
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index 645db922..97dece71 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -71,7 +71,7 @@ static IGT_LIST_HEAD(spin_list);
>  static pthread_mutex_t list_lock = PTHREAD_MUTEX_INITIALIZER;
>  
>  static uint32_t
> -handle_create(int fd, size_t sz, unsigned long flags, uint32_t **mem)
> +handle_create(int fd, size_t sz, unsigned long flags, uint32_t **mem, uint32_t region)
>  {
>  	*mem = NULL;
>  
> @@ -85,7 +85,7 @@ handle_create(int fd, size_t sz, unsigned long flags, uint32_t **mem)
>  		return handle;
>  	}
>  
> -	return gem_create(fd, sz);
> +	return gem_create_in_memory_regions(fd, sz, region);
>  }
>  
>  static int
> @@ -157,7 +157,7 @@ emit_recursive_batch(igt_spin_t *spin,
>  	obj = memset(spin->obj, 0, sizeof(spin->obj));
>  
>  	obj[BATCH].handle =
> -		handle_create(fd, BATCH_SIZE, opts->flags, &spin->batch);
> +		handle_create(fd, BATCH_SIZE, opts->flags, &spin->batch, opts->region);
>  	if (!spin->batch) {
>  		spin->batch = gem_mmap__device_coherent(fd, obj[BATCH].handle,
>  						  0, BATCH_SIZE, PROT_WRITE);
> @@ -222,7 +222,7 @@ emit_recursive_batch(igt_spin_t *spin,
>  		}
>  
>  		spin->poll_handle =
> -			handle_create(fd, 4096, opts->flags, &spin->poll);
> +			handle_create(fd, 4096, opts->flags, &spin->poll, opts->region);
>  		obj[SCRATCH].handle = spin->poll_handle;
>  
>  		if (!spin->poll) {
> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
> index f0205261..649b6ca4 100644
> --- a/lib/igt_dummyload.h
> +++ b/lib/igt_dummyload.h
> @@ -81,6 +81,7 @@ typedef struct igt_spin_factory {
>  	unsigned int flags;
>  	int fence;
>  	uint64_t ahnd;
> +	uint32_t region;
>  } igt_spin_factory_t;
>  
>  #define IGT_SPIN_FENCE_IN      (1 << 0)

Separate the change in dummyload. Looks good for DG1, for DG2 we will got failures
but there rework regarding size and alignment is necessary.


> diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c
> index 8c435845..93d7d992 100644
> --- a/tests/i915/gem_sync.c
> +++ b/tests/i915/gem_sync.c
> @@ -143,9 +143,20 @@ static void xchg_engine(void *array, unsigned i, unsigned j)
>  	igt_swap(E[i], E[j]);
>  }
>  
> +static uint32_t batch_create(int fd, uint32_t batch_size, uint32_t region)
> +{
> +	const uint32_t bbe = MI_BATCH_BUFFER_END;
> +	uint32_t handle;
> +
> +	handle = gem_create_in_memory_regions(fd, batch_size, region);
> +	gem_write(fd, handle, 0, &bbe, sizeof(bbe));
> +
> +	return handle;
> +}
> +
>  static void
>  sync_ring(int fd, const intel_ctx_t *ctx,
> -	  unsigned ring, int num_children, int timeout)
> +	  unsigned ring, int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  
> @@ -155,15 +166,13 @@ sync_ring(int fd, const intel_ctx_t *ctx,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double start, elapsed;
>  		unsigned long cycles;
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +		object.handle = batch_create(fd, 4096, region);
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -193,9 +202,8 @@ sync_ring(int fd, const intel_ctx_t *ctx,
>  
>  static void
>  idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
> -	  int num_children, int timeout)
> +	  int num_children, int timeout, uint32_t region)
>  {
> -	const uint32_t bbe = MI_BATCH_BUFFER_END;
>  	struct drm_i915_gem_exec_object2 object;
>  	struct drm_i915_gem_execbuffer2 execbuf;
>  	double start, elapsed;
> @@ -204,8 +212,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  	gem_require_ring(fd, ring);
>  
>  	memset(&object, 0, sizeof(object));
> -	object.handle = gem_create(fd, 4096);
> -	gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +	object.handle = batch_create(fd, 4096, region);
>  
>  	memset(&execbuf, 0, sizeof(execbuf));
>  	execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -234,7 +241,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  
>  static void
>  wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	    int timeout, int wlen)
> +	    int timeout, int wlen, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  	uint64_t ahnd = get_reloc_ahnd(fd, ctx->id);
> @@ -244,7 +251,6 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, ied.nengines) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double end, this, elapsed, now, baseline;
> @@ -254,11 +260,10 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		ahnd = get_reloc_ahnd(fd, ctx->id);
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> +		object.handle = batch_create(fd, 4096, region);
>  		object.offset = get_offset(ahnd, object.handle, 4096, 0);
>  		if (ahnd)
>  			object.flags = EXEC_OBJECT_PINNED;
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -339,7 +344,7 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  }
>  
>  static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
> -			int num_children, int timeout)
> +			int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  	uint64_t ahnd = get_reloc_ahnd(fd, ctx->id);
> @@ -359,13 +364,15 @@ static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  					 .ahnd = ahnd,
>  					 .ctx = ctx,
>  					 .engine = ied_flags(&ied, child),
> -					 .flags = IGT_SPIN_FAST);
> +					 .flags = IGT_SPIN_FAST,
> +					 .region = region);
>  
>  		spin[1] = __igt_spin_new(fd,
>  					 .ahnd = ahnd,
>  					 .ctx = ctx,
>  					 .engine = ied_flags(&ied, child),
> -					 .flags = IGT_SPIN_FAST);
> +					 .flags = IGT_SPIN_FAST,
> +					 .region = region);
>  
>  		start = gettime();
>  		end = start + timeout;
> @@ -398,7 +405,7 @@ static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  
>  static void
>  active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -		   int timeout, int wlen)
> +		   int timeout, int wlen, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  	uint64_t ahnd0 = get_reloc_ahnd(fd, 0);
> @@ -409,7 +416,6 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, ied.nengines) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double end, this, elapsed, now, baseline;
> @@ -420,11 +426,10 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		ahnd = get_reloc_ahnd(fd, ctx->id);
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> +		object.handle = batch_create(fd, 4096, region);
>  		object.offset = get_offset(ahnd, object.handle, 4096, 0);
>  		if (ahnd)
>  			object.offset = EXEC_OBJECT_PINNED;
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -436,14 +441,16 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  					 .ahnd = ahnd0,
>  					 .engine = execbuf.flags,
>  					 .flags = (IGT_SPIN_POLL_RUN |
> -						   IGT_SPIN_FAST));
> +						   IGT_SPIN_FAST),
> +					 .region = region);
>  		igt_assert(igt_spin_has_poll(spin[0]));
>  
>  		spin[1] = __igt_spin_new(fd,
>  					 .ahnd = ahnd0,
>  					 .engine = execbuf.flags,
>  					 .flags = (IGT_SPIN_POLL_RUN |
> -						   IGT_SPIN_FAST));
> +						   IGT_SPIN_FAST),
> +					 .region = region);
>  
>  		gem_execbuf(fd, &execbuf);
>  
> @@ -529,7 +536,7 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  static void
>  store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	   int num_children, int timeout)
> +	   int num_children, int timeout, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
>  	struct intel_engine_data ied;
> @@ -541,7 +548,6 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object[2];
>  		struct drm_i915_gem_relocation_entry reloc[1024];
>  		struct drm_i915_gem_execbuffer2 execbuf;
> @@ -559,14 +565,13 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		execbuf.rsvd1 = ctx->id;
>  
>  		memset(object, 0, sizeof(object));
> -		object[0].handle = gem_create(fd, 4096);
> -		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
> +		object[0].handle = batch_create(fd, 4096, region);
>  		execbuf.buffer_count = 1;
>  		gem_execbuf(fd, &execbuf);
>  
>  		object[0].flags |= EXEC_OBJECT_WRITE;
>  		object[0].flags |= has_relocs ? 0 : EXEC_OBJECT_PINNED;
> -		object[1].handle = gem_create(fd, 20*1024);
> +		object[1].handle = gem_create_in_memory_regions(fd, 20*1024, region);
>  
>  		object[1].relocs_ptr = to_user_pointer(reloc);
>  		object[1].relocation_count = has_relocs ? 1024 : 0;
> @@ -629,7 +634,7 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  static void
>  switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	    int num_children, int timeout)
> +	    int num_children, int timeout, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
>  	struct intel_engine_data ied;
> @@ -653,7 +658,6 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		unsigned long cycles;
>  
>  		for (int i = 0; i < ARRAY_SIZE(contexts); i++) {
> -			const uint32_t bbe = MI_BATCH_BUFFER_END;
>  			const uint32_t sz = 32 << 10;
>  			struct context *c = &contexts[i];
>  			uint32_t *batch, *b;
> @@ -670,13 +674,12 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  			c->execbuf.rsvd1 = c->ctx->id;
>  
>  			memset(c->object, 0, sizeof(c->object));
> -			c->object[0].handle = gem_create(fd, 4096);
> -			gem_write(fd, c->object[0].handle, 0, &bbe, sizeof(bbe));
> +			c->object[0].handle = batch_create(fd, 4096, region);
>  			c->execbuf.buffer_count = 1;
>  			gem_execbuf(fd, &c->execbuf);
>  
>  			c->object[0].flags |= EXEC_OBJECT_WRITE;
> -			c->object[1].handle = gem_create(fd, sz);
> +			c->object[1].handle = gem_create_in_memory_regions(fd, sz, region);
>  
>  			c->object[1].relocs_ptr = to_user_pointer(c->reloc);
>  			c->object[1].relocation_count = has_relocs ? 1024 * i : 0;
> @@ -813,10 +816,9 @@ static void *waiter(void *arg)
>  
>  static void
>  __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	     int timeout, unsigned long *cycles)
> +	     int timeout, unsigned long *cycles, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
> -	const uint32_t bbe = MI_BATCH_BUFFER_END;
>  	struct drm_i915_gem_exec_object2 object[2];
>  	struct drm_i915_gem_execbuffer2 execbuf;
>  	struct drm_i915_gem_relocation_entry reloc[1024];
> @@ -836,8 +838,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
>  	execbuf.rsvd1 = ctx->id;
>  
>  	memset(object, 0, sizeof(object));
> -	object[0].handle = gem_create(fd, 4096);
> -	gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
> +	object[0].handle = batch_create(fd, 4096, region);
>  	execbuf.buffer_count = 1;
>  	gem_execbuf(fd, &execbuf);
>  	object[0].flags |= EXEC_OBJECT_WRITE;
> @@ -880,8 +881,12 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
>  	for (int i = 0; i < ARRAY_SIZE(threads); i++) {
>  		threads[i].fd = fd;
>  		threads[i].object = object[1];
> -		threads[i].object.handle = gem_create(fd, 20*1024);
> -		gem_write(fd, threads[i].object.handle, 0, batch, 20*1024);
> +		if (i%2)

Code style, spaces and {}.

Use kernel checkpatch.pl and remove obvious code style errors.

> +			threads[i].object.handle = batch_create(fd, 20*1024, region);
> +		else {
> +			threads[i].object.handle = gem_create(fd, 20*1024);
> +			gem_write(fd, threads[i].object.handle, 0, batch, 20*1024);
> +		}
>  
>  		pthread_cond_init(&threads[i].cond, NULL);
>  		pthread_mutex_init(&threads[i].mutex, NULL);
> @@ -945,7 +950,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  static void
>  store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
> -	   int num_children, int timeout)
> +	   int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  	unsigned long *shared;
> @@ -963,7 +968,8 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  			__store_many(fd, ctx,
>  				     ied_flags(&ied, n),
>  				     timeout,
> -				     &shared[n]);
> +				     &shared[n],
> +				     region);
>  	}
>  	igt_waitchildren();
>  
> @@ -976,7 +982,7 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  }
>  
>  static void
> -sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
> +sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  
> @@ -985,15 +991,13 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double start, elapsed;
>  		unsigned long cycles;
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +		object.handle = batch_create(fd, 4096, region);
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -1023,7 +1027,7 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  }
>  
>  static void
> -store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
> +store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
>  	struct intel_engine_data ied;
> @@ -1034,7 +1038,6 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object[2];
>  		struct drm_i915_gem_relocation_entry reloc[1024];
>  		struct drm_i915_gem_execbuffer2 execbuf;
> @@ -1051,14 +1054,13 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  		execbuf.rsvd1 = ctx->id;
>  
>  		memset(object, 0, sizeof(object));
> -		object[0].handle = gem_create(fd, 4096);
> -		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
> +		object[0].handle = batch_create(fd, 4096, region);
>  		execbuf.buffer_count = 1;
>  		gem_execbuf(fd, &execbuf);
>  
>  		object[0].flags |= EXEC_OBJECT_WRITE;
>  		object[0].flags |= has_relocs ? 0 : EXEC_OBJECT_PINNED;
> -		object[1].handle = gem_create(fd, 1024*16 + 4096);
> +		object[1].handle = gem_create_in_memory_regions(fd, 1024*16 + 4096, region);
>  
>  		object[1].relocs_ptr = to_user_pointer(reloc);
>  		object[1].relocation_count = has_relocs ? 1024 : 0;
> @@ -1126,7 +1128,7 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  
>  static void
>  preempt(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	int num_children, int timeout)
> +	int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  	const intel_ctx_t *tmp_ctx[2];
> @@ -1144,7 +1146,6 @@ preempt(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double start, elapsed;
> @@ -1153,11 +1154,10 @@ preempt(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		ahnd = get_reloc_ahnd(fd, 0);
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> +		object.handle = batch_create(fd, 4096, region);
>  		object.offset = get_offset(ahnd, object.handle, 4096, 0);
>  		if (ahnd)
>  			object.flags = EXEC_OBJECT_PINNED;
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -1202,13 +1202,13 @@ preempt(int fd, const intel_ctx_t *ctx, unsigned ring,
>  igt_main
>  {
>  	const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> -	const struct {
> +	const struct test_each_engine{
>  		const char *name;
>  		void (*func)(int fd, const intel_ctx_t *ctx, unsigned int engine,
> -			     int num_children, int timeout);
> +			     int num_children, int timeout, uint32_t memregion);
>  		int num_children;
>  		int timeout;
> -	} all[] = {
> +	} *test_engine, all[] = {
>  		{ "basic-each", sync_ring, 1, 2 },
>  		{ "basic-store-each", store_ring, 1, 2 },
>  		{ "basic-many-each", store_many, 0, 2 },
> @@ -1236,11 +1236,50 @@ igt_main
>  		{ "forked-store", store_ring, ncpus, 20 },
>  		{}
>  	};
> +	const struct test {
> +		const char *name;
> +		void (*func) (int, const intel_ctx_t *, int, int, uint32_t);
> +		int num_children;
> +		int timeout;
> +	} *test, tests[] = {
> +		{ "basic-all", sync_all, 1, 2},
                                             ^ missing space there and below
> +		{ "basic-store-all", store_all, 1, 2},
> +		{ "all", sync_all, 1, 20},
> +		{ "store-all", store_all, 1, 20},
> +		{ "forked-all", sync_all, ncpus, 20},
> +		{ "forked-store-all", store_all, ncpus, 20},
> +		{}
> +	};
> +
> +#define subtest_for_each_combination(__name, __ctx, __num_children, __timeout, __func) \
> +	igt_subtest_with_dynamic(__name) { \
> +		for_each_combination(regions, 1, set) { \
> +			sub_name = memregion_dynamic_subtest_name(regions); \
> +			region = igt_collection_get_value(regions, 0); \
> +			igt_dynamic_f("%s", sub_name) \
> +				(__func)(fd, ctx, (__num_children), (__timeout), region); \
> +			free(sub_name); \
> +		} \
> +	}
> +
> +#define for_each_ctx_engine_combination(__e_name, __e_flags, ctx, __num_children, __timeout, __func) \
> +	for_each_combination(regions, 1, set) { \
> +		sub_name = memregion_dynamic_subtest_name(regions); \
> +		region = igt_collection_get_value(regions, 0); \
> +		igt_dynamic_f("%s-%s", (__e_name), sub_name) \
> +			(__func)(fd, ctx, (__e_flags), (__num_children), (__timeout), region); \
> +		free(sub_name); \
> +	}
> +
>  #define for_each_test(t, T) for(typeof(*T) *t = T; t->name; t++)
>  
>  	const struct intel_execution_engine2 *e;
>  	const intel_ctx_t *ctx;
>  	int fd = -1;
> +	struct drm_i915_query_memory_regions *query_info;
> +	struct igt_collection *regions, *set;
> +	char *sub_name;
> +	uint32_t region;
>  
>  	igt_fixture {
>  		fd = drm_open_driver(DRIVER_INTEL);
> @@ -1250,49 +1289,48 @@ igt_main
>  		ctx = intel_ctx_create_all_physical(fd);
>  
>  		igt_fork_hang_detector(fd);
> +		query_info = gem_get_query_memory_regions(fd);
> +		igt_assert(query_info);
> +		set = get_memory_region_set(query_info,
> +					I915_SYSTEM_MEMORY,
> +					I915_DEVICE_MEMORY);
>  		intel_allocator_multiprocess_start();
>  	}
>  
>  	/* Legacy for selecting rings. */
> -	for_each_test(t, individual) {
> -		igt_subtest_with_dynamic_f("legacy-%s", t->name) {
> +	for (test_engine= individual; test_engine->name; test_engine++) {
                        ^--- same, checkpatch is also complaining here

> +		igt_subtest_with_dynamic_f("legacy-%s", test_engine->name) {
>  			for (const struct intel_execution_ring *l = intel_execution_rings; l->name; l++) {
> -				igt_dynamic_f("%s", l->name) {
> -					t->func(fd, intel_ctx_0(fd), eb_ring(l),
> -						t->num_children, t->timeout);
> -				}
> +				for_each_ctx_engine_combination(l->name, eb_ring(l), intel_ctx_0(fd),
> +						test_engine->num_children,
> +						test_engine->timeout,
> +						test_engine->func);
>  			}
>  		}
>  	}
>  
> -	igt_subtest("basic-all")
> -		sync_all(fd, ctx, 1, 2);
> -	igt_subtest("basic-store-all")
> -		store_all(fd, ctx, 1, 2);
> -
> -	igt_subtest("all")
> -		sync_all(fd, ctx, 1, 20);
> -	igt_subtest("store-all")
> -		store_all(fd, ctx, 1, 20);
> -	igt_subtest("forked-all")
> -		sync_all(fd, ctx, ncpus, 20);
> -	igt_subtest("forked-store-all")
> -		store_all(fd, ctx, ncpus, 20);
> +	for (test= tests; test->name; test++)
> +		subtest_for_each_combination(test->name, ctx, test->num_children, test->timeout, test->func);
>  
>  	for_each_test(t, all) {
> -		igt_subtest_f("%s", t->name)
> -			t->func(fd, ctx, ALL_ENGINES, t->num_children, t->timeout);
> +		igt_subtest_with_dynamic_f("%s", t->name) {
> +			for_each_combination(regions, 1, set) {
> +				sub_name = memregion_dynamic_subtest_name(regions);
> +				region = igt_collection_get_value(regions, 0);
> +				igt_dynamic_f("%s", sub_name)
> +					t->func(fd, ctx, ALL_ENGINES, t->num_children,
> +							t->timeout, region);
> +				free(sub_name);
> +			}
> +		}
>  	}
>  
>  	/* New way of selecting engines. */
> -	for_each_test(t, individual) {
> -		igt_subtest_with_dynamic_f("%s", t->name) {
> -			for_each_ctx_engine(fd, ctx, e) {
> -				igt_dynamic_f("%s", e->name) {
> -					t->func(fd, ctx, e->flags,
> -						t->num_children, t->timeout);
> -				}
> -			}
> +	for (test_engine= individual; test_engine->name; test_engine++) {
> +		igt_subtest_with_dynamic_f("%s", test_engine->name) {
> +			for_each_ctx_engine(fd, ctx, e)
> +				for_each_ctx_engine_combination(e->name, e->flags, ctx,
> +						test_engine->num_children, test_engine->timeout, test_engine->func);
>  		}
>  	}
>  
> @@ -1303,17 +1341,32 @@ igt_main
>  			igt_require(gem_scheduler_has_preemption(fd));
>  		}
>  
> -		igt_subtest("preempt-all")
> -			preempt(fd, ctx, ALL_ENGINES, 1, 20);
> +		igt_subtest_with_dynamic("preempt-all") {
> +			for_each_combination(regions, 1, set) {
> +				sub_name = memregion_dynamic_subtest_name(regions);
> +				region = igt_collection_get_value(regions, 0);
> +				igt_dynamic_f("%s", sub_name)
> +					preempt(fd, ctx, ALL_ENGINES, 1, 20, region);
> +				free(sub_name);
> +			}
> +		}
> +
>  		igt_subtest_with_dynamic("preempt") {
> -			for_each_ctx_engine(fd, ctx, e) {
> -				igt_dynamic_f("%s", e->name)
> -					preempt(fd, ctx, e->flags, ncpus, 20);
> +			for_each_combination(regions, 1, set) {
> +				sub_name = memregion_dynamic_subtest_name(regions);
> +				region = igt_collection_get_value(regions, 0);
> +				for_each_ctx_engine(fd, ctx, e) {
> +					igt_dynamic_f("%s-%s", e->name, sub_name)
> +						preempt(fd, ctx, e->flags, ncpus, 20, region);
> +					free(sub_name);
> +				}

Got memory corruption here (fix is trivial):

(gem_sync:86973) igt_core-CRITICAL: Invalid dynamic subtest name "bcs0-QS^U".
gem_sync: ../lib/igt_core.c:2108: igt_exit: Assertion `!test_with_subtests || skipped_one || succeeded_one || failed_one' failed.
Received signal SIGABRT.
Stack trace: 
 #0 [fatal_sig_handler+0x15c]
 #1 [__sigaction+0x50]
 #2 [pthread_kill+0xf8]
 #3 [raise+0x16]
 #4 [abort+0xd7]
 #5 [<unknown>+0xd7]
 #6 [__assert_fail+0x46]
 #7 [igt_exit+0x1b6]
 #8 [__igt_run_dynamic_subtest+0x156]
 #9 [__igt_unique____real_main1202+0xd31]
 #10 [main+0x2d]
 #11 [__libc_init_first+0x90]
 #12 [__libc_start_main+0x7d]
 #13 [_start+0x25]
Aborted

Anyway - code looks almost done, just fix code-style issues and that unlucky
double free issue.

--
Zbigniew

>  			}
>  		}
>  	}
>  
>  	igt_fixture {
> +		free(query_info);
> +		igt_collection_destroy(set);
>  		intel_allocator_multiprocess_stop();
>  		igt_stop_hang_detector();
>  		intel_ctx_destroy(fd, ctx);
> -- 
> 2.35.0
> 

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

* [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory
@ 2022-02-03  5:25 sai.gowtham.ch
  2022-02-15 12:07 ` Zbigniew Kempczyński
  0 siblings, 1 reply; 11+ messages in thread
From: sai.gowtham.ch @ 2022-02-03  5:25 UTC (permalink / raw)
  To: igt-dev, sai.gowtham.ch, zbigniew.kempczynski

From: Ch Sai Gowtham <sai.gowtham.ch@intel.com>

Add support for local memory region (Device memory)

Signed-off-by: Ch Sai Gowtham <sai.gowtham.ch@intel.com>
Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 lib/igt_dummyload.c   |   8 +-
 lib/igt_dummyload.h   |   1 +
 tests/i915/gem_sync.c | 231 ++++++++++++++++++++++++++----------------
 3 files changed, 147 insertions(+), 93 deletions(-)

diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 645db922..97dece71 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -71,7 +71,7 @@ static IGT_LIST_HEAD(spin_list);
 static pthread_mutex_t list_lock = PTHREAD_MUTEX_INITIALIZER;
 
 static uint32_t
-handle_create(int fd, size_t sz, unsigned long flags, uint32_t **mem)
+handle_create(int fd, size_t sz, unsigned long flags, uint32_t **mem, uint32_t region)
 {
 	*mem = NULL;
 
@@ -85,7 +85,7 @@ handle_create(int fd, size_t sz, unsigned long flags, uint32_t **mem)
 		return handle;
 	}
 
-	return gem_create(fd, sz);
+	return gem_create_in_memory_regions(fd, sz, region);
 }
 
 static int
@@ -157,7 +157,7 @@ emit_recursive_batch(igt_spin_t *spin,
 	obj = memset(spin->obj, 0, sizeof(spin->obj));
 
 	obj[BATCH].handle =
-		handle_create(fd, BATCH_SIZE, opts->flags, &spin->batch);
+		handle_create(fd, BATCH_SIZE, opts->flags, &spin->batch, opts->region);
 	if (!spin->batch) {
 		spin->batch = gem_mmap__device_coherent(fd, obj[BATCH].handle,
 						  0, BATCH_SIZE, PROT_WRITE);
@@ -222,7 +222,7 @@ emit_recursive_batch(igt_spin_t *spin,
 		}
 
 		spin->poll_handle =
-			handle_create(fd, 4096, opts->flags, &spin->poll);
+			handle_create(fd, 4096, opts->flags, &spin->poll, opts->region);
 		obj[SCRATCH].handle = spin->poll_handle;
 
 		if (!spin->poll) {
diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
index f0205261..649b6ca4 100644
--- a/lib/igt_dummyload.h
+++ b/lib/igt_dummyload.h
@@ -81,6 +81,7 @@ typedef struct igt_spin_factory {
 	unsigned int flags;
 	int fence;
 	uint64_t ahnd;
+	uint32_t region;
 } igt_spin_factory_t;
 
 #define IGT_SPIN_FENCE_IN      (1 << 0)
diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c
index 8c435845..93d7d992 100644
--- a/tests/i915/gem_sync.c
+++ b/tests/i915/gem_sync.c
@@ -143,9 +143,20 @@ static void xchg_engine(void *array, unsigned i, unsigned j)
 	igt_swap(E[i], E[j]);
 }
 
+static uint32_t batch_create(int fd, uint32_t batch_size, uint32_t region)
+{
+	const uint32_t bbe = MI_BATCH_BUFFER_END;
+	uint32_t handle;
+
+	handle = gem_create_in_memory_regions(fd, batch_size, region);
+	gem_write(fd, handle, 0, &bbe, sizeof(bbe));
+
+	return handle;
+}
+
 static void
 sync_ring(int fd, const intel_ctx_t *ctx,
-	  unsigned ring, int num_children, int timeout)
+	  unsigned ring, int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 
@@ -155,15 +166,13 @@ sync_ring(int fd, const intel_ctx_t *ctx,
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object;
 		struct drm_i915_gem_execbuffer2 execbuf;
 		double start, elapsed;
 		unsigned long cycles;
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
-		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
+		object.handle = batch_create(fd, 4096, region);
 
 		memset(&execbuf, 0, sizeof(execbuf));
 		execbuf.buffers_ptr = to_user_pointer(&object);
@@ -193,9 +202,8 @@ sync_ring(int fd, const intel_ctx_t *ctx,
 
 static void
 idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
-	  int num_children, int timeout)
+	  int num_children, int timeout, uint32_t region)
 {
-	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	struct drm_i915_gem_exec_object2 object;
 	struct drm_i915_gem_execbuffer2 execbuf;
 	double start, elapsed;
@@ -204,8 +212,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
 	gem_require_ring(fd, ring);
 
 	memset(&object, 0, sizeof(object));
-	object.handle = gem_create(fd, 4096);
-	gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
+	object.handle = batch_create(fd, 4096, region);
 
 	memset(&execbuf, 0, sizeof(execbuf));
 	execbuf.buffers_ptr = to_user_pointer(&object);
@@ -234,7 +241,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
 
 static void
 wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
-	    int timeout, int wlen)
+	    int timeout, int wlen, uint32_t region)
 {
 	struct intel_engine_data ied;
 	uint64_t ahnd = get_reloc_ahnd(fd, ctx->id);
@@ -244,7 +251,6 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, ied.nengines) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object;
 		struct drm_i915_gem_execbuffer2 execbuf;
 		double end, this, elapsed, now, baseline;
@@ -254,11 +260,10 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 		ahnd = get_reloc_ahnd(fd, ctx->id);
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
+		object.handle = batch_create(fd, 4096, region);
 		object.offset = get_offset(ahnd, object.handle, 4096, 0);
 		if (ahnd)
 			object.flags = EXEC_OBJECT_PINNED;
-		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
 
 		memset(&execbuf, 0, sizeof(execbuf));
 		execbuf.buffers_ptr = to_user_pointer(&object);
@@ -339,7 +344,7 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 }
 
 static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
-			int num_children, int timeout)
+			int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 	uint64_t ahnd = get_reloc_ahnd(fd, ctx->id);
@@ -359,13 +364,15 @@ static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
 					 .ahnd = ahnd,
 					 .ctx = ctx,
 					 .engine = ied_flags(&ied, child),
-					 .flags = IGT_SPIN_FAST);
+					 .flags = IGT_SPIN_FAST,
+					 .region = region);
 
 		spin[1] = __igt_spin_new(fd,
 					 .ahnd = ahnd,
 					 .ctx = ctx,
 					 .engine = ied_flags(&ied, child),
-					 .flags = IGT_SPIN_FAST);
+					 .flags = IGT_SPIN_FAST,
+					 .region = region);
 
 		start = gettime();
 		end = start + timeout;
@@ -398,7 +405,7 @@ static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
 
 static void
 active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
-		   int timeout, int wlen)
+		   int timeout, int wlen, uint32_t region)
 {
 	struct intel_engine_data ied;
 	uint64_t ahnd0 = get_reloc_ahnd(fd, 0);
@@ -409,7 +416,6 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, ied.nengines) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object;
 		struct drm_i915_gem_execbuffer2 execbuf;
 		double end, this, elapsed, now, baseline;
@@ -420,11 +426,10 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 		ahnd = get_reloc_ahnd(fd, ctx->id);
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
+		object.handle = batch_create(fd, 4096, region);
 		object.offset = get_offset(ahnd, object.handle, 4096, 0);
 		if (ahnd)
 			object.offset = EXEC_OBJECT_PINNED;
-		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
 
 		memset(&execbuf, 0, sizeof(execbuf));
 		execbuf.buffers_ptr = to_user_pointer(&object);
@@ -436,14 +441,16 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 					 .ahnd = ahnd0,
 					 .engine = execbuf.flags,
 					 .flags = (IGT_SPIN_POLL_RUN |
-						   IGT_SPIN_FAST));
+						   IGT_SPIN_FAST),
+					 .region = region);
 		igt_assert(igt_spin_has_poll(spin[0]));
 
 		spin[1] = __igt_spin_new(fd,
 					 .ahnd = ahnd0,
 					 .engine = execbuf.flags,
 					 .flags = (IGT_SPIN_POLL_RUN |
-						   IGT_SPIN_FAST));
+						   IGT_SPIN_FAST),
+					 .region = region);
 
 		gem_execbuf(fd, &execbuf);
 
@@ -529,7 +536,7 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 static void
 store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
-	   int num_children, int timeout)
+	   int num_children, int timeout, uint32_t region)
 {
 	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
 	struct intel_engine_data ied;
@@ -541,7 +548,6 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object[2];
 		struct drm_i915_gem_relocation_entry reloc[1024];
 		struct drm_i915_gem_execbuffer2 execbuf;
@@ -559,14 +565,13 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 		execbuf.rsvd1 = ctx->id;
 
 		memset(object, 0, sizeof(object));
-		object[0].handle = gem_create(fd, 4096);
-		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
+		object[0].handle = batch_create(fd, 4096, region);
 		execbuf.buffer_count = 1;
 		gem_execbuf(fd, &execbuf);
 
 		object[0].flags |= EXEC_OBJECT_WRITE;
 		object[0].flags |= has_relocs ? 0 : EXEC_OBJECT_PINNED;
-		object[1].handle = gem_create(fd, 20*1024);
+		object[1].handle = gem_create_in_memory_regions(fd, 20*1024, region);
 
 		object[1].relocs_ptr = to_user_pointer(reloc);
 		object[1].relocation_count = has_relocs ? 1024 : 0;
@@ -629,7 +634,7 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 static void
 switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
-	    int num_children, int timeout)
+	    int num_children, int timeout, uint32_t region)
 {
 	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
 	struct intel_engine_data ied;
@@ -653,7 +658,6 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 		unsigned long cycles;
 
 		for (int i = 0; i < ARRAY_SIZE(contexts); i++) {
-			const uint32_t bbe = MI_BATCH_BUFFER_END;
 			const uint32_t sz = 32 << 10;
 			struct context *c = &contexts[i];
 			uint32_t *batch, *b;
@@ -670,13 +674,12 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 			c->execbuf.rsvd1 = c->ctx->id;
 
 			memset(c->object, 0, sizeof(c->object));
-			c->object[0].handle = gem_create(fd, 4096);
-			gem_write(fd, c->object[0].handle, 0, &bbe, sizeof(bbe));
+			c->object[0].handle = batch_create(fd, 4096, region);
 			c->execbuf.buffer_count = 1;
 			gem_execbuf(fd, &c->execbuf);
 
 			c->object[0].flags |= EXEC_OBJECT_WRITE;
-			c->object[1].handle = gem_create(fd, sz);
+			c->object[1].handle = gem_create_in_memory_regions(fd, sz, region);
 
 			c->object[1].relocs_ptr = to_user_pointer(c->reloc);
 			c->object[1].relocation_count = has_relocs ? 1024 * i : 0;
@@ -813,10 +816,9 @@ static void *waiter(void *arg)
 
 static void
 __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
-	     int timeout, unsigned long *cycles)
+	     int timeout, unsigned long *cycles, uint32_t region)
 {
 	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
-	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	struct drm_i915_gem_exec_object2 object[2];
 	struct drm_i915_gem_execbuffer2 execbuf;
 	struct drm_i915_gem_relocation_entry reloc[1024];
@@ -836,8 +838,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
 	execbuf.rsvd1 = ctx->id;
 
 	memset(object, 0, sizeof(object));
-	object[0].handle = gem_create(fd, 4096);
-	gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
+	object[0].handle = batch_create(fd, 4096, region);
 	execbuf.buffer_count = 1;
 	gem_execbuf(fd, &execbuf);
 	object[0].flags |= EXEC_OBJECT_WRITE;
@@ -880,8 +881,12 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
 	for (int i = 0; i < ARRAY_SIZE(threads); i++) {
 		threads[i].fd = fd;
 		threads[i].object = object[1];
-		threads[i].object.handle = gem_create(fd, 20*1024);
-		gem_write(fd, threads[i].object.handle, 0, batch, 20*1024);
+		if (i%2)
+			threads[i].object.handle = batch_create(fd, 20*1024, region);
+		else {
+			threads[i].object.handle = gem_create(fd, 20*1024);
+			gem_write(fd, threads[i].object.handle, 0, batch, 20*1024);
+		}
 
 		pthread_cond_init(&threads[i].cond, NULL);
 		pthread_mutex_init(&threads[i].mutex, NULL);
@@ -945,7 +950,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 static void
 store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
-	   int num_children, int timeout)
+	   int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 	unsigned long *shared;
@@ -963,7 +968,8 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
 			__store_many(fd, ctx,
 				     ied_flags(&ied, n),
 				     timeout,
-				     &shared[n]);
+				     &shared[n],
+				     region);
 	}
 	igt_waitchildren();
 
@@ -976,7 +982,7 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
 }
 
 static void
-sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
+sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 
@@ -985,15 +991,13 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object;
 		struct drm_i915_gem_execbuffer2 execbuf;
 		double start, elapsed;
 		unsigned long cycles;
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
-		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
+		object.handle = batch_create(fd, 4096, region);
 
 		memset(&execbuf, 0, sizeof(execbuf));
 		execbuf.buffers_ptr = to_user_pointer(&object);
@@ -1023,7 +1027,7 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 }
 
 static void
-store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
+store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
 {
 	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
 	struct intel_engine_data ied;
@@ -1034,7 +1038,6 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object[2];
 		struct drm_i915_gem_relocation_entry reloc[1024];
 		struct drm_i915_gem_execbuffer2 execbuf;
@@ -1051,14 +1054,13 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 		execbuf.rsvd1 = ctx->id;
 
 		memset(object, 0, sizeof(object));
-		object[0].handle = gem_create(fd, 4096);
-		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
+		object[0].handle = batch_create(fd, 4096, region);
 		execbuf.buffer_count = 1;
 		gem_execbuf(fd, &execbuf);
 
 		object[0].flags |= EXEC_OBJECT_WRITE;
 		object[0].flags |= has_relocs ? 0 : EXEC_OBJECT_PINNED;
-		object[1].handle = gem_create(fd, 1024*16 + 4096);
+		object[1].handle = gem_create_in_memory_regions(fd, 1024*16 + 4096, region);
 
 		object[1].relocs_ptr = to_user_pointer(reloc);
 		object[1].relocation_count = has_relocs ? 1024 : 0;
@@ -1126,7 +1128,7 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 
 static void
 preempt(int fd, const intel_ctx_t *ctx, unsigned ring,
-	int num_children, int timeout)
+	int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 	const intel_ctx_t *tmp_ctx[2];
@@ -1144,7 +1146,6 @@ preempt(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object;
 		struct drm_i915_gem_execbuffer2 execbuf;
 		double start, elapsed;
@@ -1153,11 +1154,10 @@ preempt(int fd, const intel_ctx_t *ctx, unsigned ring,
 		ahnd = get_reloc_ahnd(fd, 0);
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
+		object.handle = batch_create(fd, 4096, region);
 		object.offset = get_offset(ahnd, object.handle, 4096, 0);
 		if (ahnd)
 			object.flags = EXEC_OBJECT_PINNED;
-		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
 
 		memset(&execbuf, 0, sizeof(execbuf));
 		execbuf.buffers_ptr = to_user_pointer(&object);
@@ -1202,13 +1202,13 @@ preempt(int fd, const intel_ctx_t *ctx, unsigned ring,
 igt_main
 {
 	const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
-	const struct {
+	const struct test_each_engine{
 		const char *name;
 		void (*func)(int fd, const intel_ctx_t *ctx, unsigned int engine,
-			     int num_children, int timeout);
+			     int num_children, int timeout, uint32_t memregion);
 		int num_children;
 		int timeout;
-	} all[] = {
+	} *test_engine, all[] = {
 		{ "basic-each", sync_ring, 1, 2 },
 		{ "basic-store-each", store_ring, 1, 2 },
 		{ "basic-many-each", store_many, 0, 2 },
@@ -1236,11 +1236,50 @@ igt_main
 		{ "forked-store", store_ring, ncpus, 20 },
 		{}
 	};
+	const struct test {
+		const char *name;
+		void (*func) (int, const intel_ctx_t *, int, int, uint32_t);
+		int num_children;
+		int timeout;
+	} *test, tests[] = {
+		{ "basic-all", sync_all, 1, 2},
+		{ "basic-store-all", store_all, 1, 2},
+		{ "all", sync_all, 1, 20},
+		{ "store-all", store_all, 1, 20},
+		{ "forked-all", sync_all, ncpus, 20},
+		{ "forked-store-all", store_all, ncpus, 20},
+		{}
+	};
+
+#define subtest_for_each_combination(__name, __ctx, __num_children, __timeout, __func) \
+	igt_subtest_with_dynamic(__name) { \
+		for_each_combination(regions, 1, set) { \
+			sub_name = memregion_dynamic_subtest_name(regions); \
+			region = igt_collection_get_value(regions, 0); \
+			igt_dynamic_f("%s", sub_name) \
+				(__func)(fd, ctx, (__num_children), (__timeout), region); \
+			free(sub_name); \
+		} \
+	}
+
+#define for_each_ctx_engine_combination(__e_name, __e_flags, ctx, __num_children, __timeout, __func) \
+	for_each_combination(regions, 1, set) { \
+		sub_name = memregion_dynamic_subtest_name(regions); \
+		region = igt_collection_get_value(regions, 0); \
+		igt_dynamic_f("%s-%s", (__e_name), sub_name) \
+			(__func)(fd, ctx, (__e_flags), (__num_children), (__timeout), region); \
+		free(sub_name); \
+	}
+
 #define for_each_test(t, T) for(typeof(*T) *t = T; t->name; t++)
 
 	const struct intel_execution_engine2 *e;
 	const intel_ctx_t *ctx;
 	int fd = -1;
+	struct drm_i915_query_memory_regions *query_info;
+	struct igt_collection *regions, *set;
+	char *sub_name;
+	uint32_t region;
 
 	igt_fixture {
 		fd = drm_open_driver(DRIVER_INTEL);
@@ -1250,49 +1289,48 @@ igt_main
 		ctx = intel_ctx_create_all_physical(fd);
 
 		igt_fork_hang_detector(fd);
+		query_info = gem_get_query_memory_regions(fd);
+		igt_assert(query_info);
+		set = get_memory_region_set(query_info,
+					I915_SYSTEM_MEMORY,
+					I915_DEVICE_MEMORY);
 		intel_allocator_multiprocess_start();
 	}
 
 	/* Legacy for selecting rings. */
-	for_each_test(t, individual) {
-		igt_subtest_with_dynamic_f("legacy-%s", t->name) {
+	for (test_engine= individual; test_engine->name; test_engine++) {
+		igt_subtest_with_dynamic_f("legacy-%s", test_engine->name) {
 			for (const struct intel_execution_ring *l = intel_execution_rings; l->name; l++) {
-				igt_dynamic_f("%s", l->name) {
-					t->func(fd, intel_ctx_0(fd), eb_ring(l),
-						t->num_children, t->timeout);
-				}
+				for_each_ctx_engine_combination(l->name, eb_ring(l), intel_ctx_0(fd),
+						test_engine->num_children,
+						test_engine->timeout,
+						test_engine->func);
 			}
 		}
 	}
 
-	igt_subtest("basic-all")
-		sync_all(fd, ctx, 1, 2);
-	igt_subtest("basic-store-all")
-		store_all(fd, ctx, 1, 2);
-
-	igt_subtest("all")
-		sync_all(fd, ctx, 1, 20);
-	igt_subtest("store-all")
-		store_all(fd, ctx, 1, 20);
-	igt_subtest("forked-all")
-		sync_all(fd, ctx, ncpus, 20);
-	igt_subtest("forked-store-all")
-		store_all(fd, ctx, ncpus, 20);
+	for (test= tests; test->name; test++)
+		subtest_for_each_combination(test->name, ctx, test->num_children, test->timeout, test->func);
 
 	for_each_test(t, all) {
-		igt_subtest_f("%s", t->name)
-			t->func(fd, ctx, ALL_ENGINES, t->num_children, t->timeout);
+		igt_subtest_with_dynamic_f("%s", t->name) {
+			for_each_combination(regions, 1, set) {
+				sub_name = memregion_dynamic_subtest_name(regions);
+				region = igt_collection_get_value(regions, 0);
+				igt_dynamic_f("%s", sub_name)
+					t->func(fd, ctx, ALL_ENGINES, t->num_children,
+							t->timeout, region);
+				free(sub_name);
+			}
+		}
 	}
 
 	/* New way of selecting engines. */
-	for_each_test(t, individual) {
-		igt_subtest_with_dynamic_f("%s", t->name) {
-			for_each_ctx_engine(fd, ctx, e) {
-				igt_dynamic_f("%s", e->name) {
-					t->func(fd, ctx, e->flags,
-						t->num_children, t->timeout);
-				}
-			}
+	for (test_engine= individual; test_engine->name; test_engine++) {
+		igt_subtest_with_dynamic_f("%s", test_engine->name) {
+			for_each_ctx_engine(fd, ctx, e)
+				for_each_ctx_engine_combination(e->name, e->flags, ctx,
+						test_engine->num_children, test_engine->timeout, test_engine->func);
 		}
 	}
 
@@ -1303,17 +1341,32 @@ igt_main
 			igt_require(gem_scheduler_has_preemption(fd));
 		}
 
-		igt_subtest("preempt-all")
-			preempt(fd, ctx, ALL_ENGINES, 1, 20);
+		igt_subtest_with_dynamic("preempt-all") {
+			for_each_combination(regions, 1, set) {
+				sub_name = memregion_dynamic_subtest_name(regions);
+				region = igt_collection_get_value(regions, 0);
+				igt_dynamic_f("%s", sub_name)
+					preempt(fd, ctx, ALL_ENGINES, 1, 20, region);
+				free(sub_name);
+			}
+		}
+
 		igt_subtest_with_dynamic("preempt") {
-			for_each_ctx_engine(fd, ctx, e) {
-				igt_dynamic_f("%s", e->name)
-					preempt(fd, ctx, e->flags, ncpus, 20);
+			for_each_combination(regions, 1, set) {
+				sub_name = memregion_dynamic_subtest_name(regions);
+				region = igt_collection_get_value(regions, 0);
+				for_each_ctx_engine(fd, ctx, e) {
+					igt_dynamic_f("%s-%s", e->name, sub_name)
+						preempt(fd, ctx, e->flags, ncpus, 20, region);
+					free(sub_name);
+				}
 			}
 		}
 	}
 
 	igt_fixture {
+		free(query_info);
+		igt_collection_destroy(set);
 		intel_allocator_multiprocess_stop();
 		igt_stop_hang_detector();
 		intel_ctx_destroy(fd, ctx);
-- 
2.35.0

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

* Re: [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory
  2021-12-24 20:03 sai.gowtham.ch
@ 2022-01-03 11:27 ` Zbigniew Kempczyński
  0 siblings, 0 replies; 11+ messages in thread
From: Zbigniew Kempczyński @ 2022-01-03 11:27 UTC (permalink / raw)
  To: sai.gowtham.ch; +Cc: igt-dev

On Sat, Dec 25, 2021 at 01:33:10AM +0530, sai.gowtham.ch@intel.com wrote:
> From: Ch Sai Gowtham <sai.gowtham.ch@intel.com>
> 
> Add support for local memory region (Device memory)
> 
> Signed-off-by: Ch Sai Gowtham <sai.gowtham.ch@intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> ---
>  tests/i915/gem_sync.c | 193 ++++++++++++++++++++++++++++--------------
>  1 file changed, 131 insertions(+), 62 deletions(-)
> 
> diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c
> index 8c435845..8f8be2b1 100644
> --- a/tests/i915/gem_sync.c
> +++ b/tests/i915/gem_sync.c
> @@ -143,9 +143,20 @@ static void xchg_engine(void *array, unsigned i, unsigned j)
>  	igt_swap(E[i], E[j]);
>  }
>  
> +static uint32_t batch_create(int fd, uint32_t batch_size, uint32_t region)
> +{
> +	const uint32_t bbe = MI_BATCH_BUFFER_END;
> +	uint32_t handle;
> +
> +	handle = gem_create_in_memory_regions(fd, 4096, region);

What batch_size is passed here for?

> +	gem_write(fd, handle, 0, &bbe, sizeof(bbe));
> +
> +	return handle;
> +}
> +
>  static void
>  sync_ring(int fd, const intel_ctx_t *ctx,
> -	  unsigned ring, int num_children, int timeout)
> +	  unsigned ring, int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  
> @@ -155,15 +166,13 @@ sync_ring(int fd, const intel_ctx_t *ctx,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double start, elapsed;
>  		unsigned long cycles;
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +		object.handle = batch_create(fd, 4096, region);
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -193,9 +202,8 @@ sync_ring(int fd, const intel_ctx_t *ctx,
>  
>  static void
>  idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
> -	  int num_children, int timeout)
> +	  int num_children, int timeout, uint32_t region)
>  {
> -	const uint32_t bbe = MI_BATCH_BUFFER_END;
>  	struct drm_i915_gem_exec_object2 object;
>  	struct drm_i915_gem_execbuffer2 execbuf;
>  	double start, elapsed;
> @@ -204,8 +212,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  	gem_require_ring(fd, ring);
>  
>  	memset(&object, 0, sizeof(object));
> -	object.handle = gem_create(fd, 4096);
> -	gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +	object.handle = batch_create(fd, 4096, region);
>  
>  	memset(&execbuf, 0, sizeof(execbuf));
>  	execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -234,7 +241,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  
>  static void
>  wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	    int timeout, int wlen)
> +	    int timeout, int wlen, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  	uint64_t ahnd = get_reloc_ahnd(fd, ctx->id);
> @@ -254,7 +261,7 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		ahnd = get_reloc_ahnd(fd, ctx->id);
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> +		object.handle = gem_create_in_memory_regions(fd, 4096, region);

You've created batch_create() for it, haven't you?

>  		object.offset = get_offset(ahnd, object.handle, 4096, 0);
>  		if (ahnd)
>  			object.flags = EXEC_OBJECT_PINNED;
> @@ -339,7 +346,7 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  }
>  
>  static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
> -			int num_children, int timeout)
> +			int num_children, int timeout, uint32_t region)
>  {

Here we have spinners which should be executed within region but we
got no such possibility right now. We should add .region to spinner
creation flags.

>  	struct intel_engine_data ied;
>  	uint64_t ahnd = get_reloc_ahnd(fd, ctx->id);
> @@ -398,7 +405,7 @@ static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  
>  static void
>  active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -		   int timeout, int wlen)
> +		   int timeout, int wlen, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  	uint64_t ahnd0 = get_reloc_ahnd(fd, 0);
> @@ -420,7 +427,7 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		ahnd = get_reloc_ahnd(fd, ctx->id);
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> +		object.handle = gem_create_in_memory_regions(fd, 4096, region);

Same with create_batch().

>  		object.offset = get_offset(ahnd, object.handle, 4096, 0);
>  		if (ahnd)
>  			object.offset = EXEC_OBJECT_PINNED;
> @@ -529,7 +536,7 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  static void
>  store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	   int num_children, int timeout)
> +	   int num_children, int timeout, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
>  	struct intel_engine_data ied;
> @@ -541,7 +548,6 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object[2];
>  		struct drm_i915_gem_relocation_entry reloc[1024];
>  		struct drm_i915_gem_execbuffer2 execbuf;
> @@ -559,14 +565,13 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		execbuf.rsvd1 = ctx->id;
>  
>  		memset(object, 0, sizeof(object));
> -		object[0].handle = gem_create(fd, 4096);
> -		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
> +		object[0].handle = batch_create(fd, 4096, region);
>  		execbuf.buffer_count = 1;
>  		gem_execbuf(fd, &execbuf);
>  
>  		object[0].flags |= EXEC_OBJECT_WRITE;
>  		object[0].flags |= has_relocs ? 0 : EXEC_OBJECT_PINNED;
> -		object[1].handle = gem_create(fd, 20*1024);
> +		object[1].handle = gem_create_in_memory_regions(fd, 20*1024, region);
>  
>  		object[1].relocs_ptr = to_user_pointer(reloc);
>  		object[1].relocation_count = has_relocs ? 1024 : 0;
> @@ -629,7 +634,7 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  static void
>  switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	    int num_children, int timeout)
> +	    int num_children, int timeout, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
>  	struct intel_engine_data ied;
> @@ -653,7 +658,6 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		unsigned long cycles;
>  
>  		for (int i = 0; i < ARRAY_SIZE(contexts); i++) {
> -			const uint32_t bbe = MI_BATCH_BUFFER_END;
>  			const uint32_t sz = 32 << 10;
>  			struct context *c = &contexts[i];
>  			uint32_t *batch, *b;
> @@ -670,13 +674,12 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  			c->execbuf.rsvd1 = c->ctx->id;
>  
>  			memset(c->object, 0, sizeof(c->object));
> -			c->object[0].handle = gem_create(fd, 4096);
> -			gem_write(fd, c->object[0].handle, 0, &bbe, sizeof(bbe));
> +			c->object[0].handle = batch_create(fd, 4096, region);
>  			c->execbuf.buffer_count = 1;
>  			gem_execbuf(fd, &c->execbuf);
>  
>  			c->object[0].flags |= EXEC_OBJECT_WRITE;
> -			c->object[1].handle = gem_create(fd, sz);
> +			c->object[1].handle = gem_create_in_memory_regions(fd, sz, region);
>  
>  			c->object[1].relocs_ptr = to_user_pointer(c->reloc);
>  			c->object[1].relocation_count = has_relocs ? 1024 * i : 0;
> @@ -813,10 +816,9 @@ static void *waiter(void *arg)
>  
>  static void
>  __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	     int timeout, unsigned long *cycles)
> +	     int timeout, unsigned long *cycles, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
> -	const uint32_t bbe = MI_BATCH_BUFFER_END;
>  	struct drm_i915_gem_exec_object2 object[2];
>  	struct drm_i915_gem_execbuffer2 execbuf;
>  	struct drm_i915_gem_relocation_entry reloc[1024];
> @@ -836,8 +838,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
>  	execbuf.rsvd1 = ctx->id;
>  
>  	memset(object, 0, sizeof(object));
> -	object[0].handle = gem_create(fd, 4096);
> -	gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
> +	object[0].handle = batch_create(fd, 4096, region);
>  	execbuf.buffer_count = 1;
>  	gem_execbuf(fd, &execbuf);
>  	object[0].flags |= EXEC_OBJECT_WRITE;
> @@ -945,7 +946,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  static void
>  store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
> -	   int num_children, int timeout)
> +	   int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  	unsigned long *shared;
> @@ -963,7 +964,8 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  			__store_many(fd, ctx,
>  				     ied_flags(&ied, n),
>  				     timeout,
> -				     &shared[n]);
> +				     &shared[n],
> +				     region);
>  	}
>  	igt_waitchildren();
>  
> @@ -976,7 +978,7 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  }
>  
>  static void
> -sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
> +sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  
> @@ -985,15 +987,13 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double start, elapsed;
>  		unsigned long cycles;
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +		object.handle = batch_create(fd, 4096, region);
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -1023,7 +1023,7 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  }
>  
>  static void
> -store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
> +store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
>  	struct intel_engine_data ied;
> @@ -1034,7 +1034,6 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object[2];
>  		struct drm_i915_gem_relocation_entry reloc[1024];
>  		struct drm_i915_gem_execbuffer2 execbuf;
> @@ -1051,14 +1050,13 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  		execbuf.rsvd1 = ctx->id;
>  
>  		memset(object, 0, sizeof(object));
> -		object[0].handle = gem_create(fd, 4096);
> -		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
> +		object[0].handle = batch_create(fd, 4096, region);
>  		execbuf.buffer_count = 1;
>  		gem_execbuf(fd, &execbuf);
>  
>  		object[0].flags |= EXEC_OBJECT_WRITE;
>  		object[0].flags |= has_relocs ? 0 : EXEC_OBJECT_PINNED;
> -		object[1].handle = gem_create(fd, 1024*16 + 4096);
> +		object[1].handle = gem_create_in_memory_regions(fd, 1024*16 + 4096, region);
>  
>  		object[1].relocs_ptr = to_user_pointer(reloc);
>  		object[1].relocation_count = has_relocs ? 1024 : 0;

Preempt wasn't touched. Is that your intention?

> @@ -1205,7 +1203,7 @@ igt_main
>  	const struct {
>  		const char *name;
>  		void (*func)(int fd, const intel_ctx_t *ctx, unsigned int engine,
> -			     int num_children, int timeout);
> +			     int num_children, int timeout, uint32_t memregion);
>  		int num_children;
>  		int timeout;
>  	} all[] = {
> @@ -1241,6 +1239,10 @@ igt_main
>  	const struct intel_execution_engine2 *e;
>  	const intel_ctx_t *ctx;
>  	int fd = -1;
> +	struct drm_i915_query_memory_regions *query_info;
> +	struct igt_collection *regions, *set;
> +	char *sub_name;
> +	uint32_t region;
>  
>  	igt_fixture {
>  		fd = drm_open_driver(DRIVER_INTEL);
> @@ -1250,6 +1252,11 @@ igt_main
>  		ctx = intel_ctx_create_all_physical(fd);
>  
>  		igt_fork_hang_detector(fd);
> +		query_info = gem_get_query_memory_regions(fd);
> +		igt_assert(query_info);
> +		set = get_memory_region_set(query_info,
> +					I915_SYSTEM_MEMORY,
> +					I915_DEVICE_MEMORY);
>  		intel_allocator_multiprocess_start();
>  	}
>  
> @@ -1257,41 +1264,101 @@ igt_main
>  	for_each_test(t, individual) {
>  		igt_subtest_with_dynamic_f("legacy-%s", t->name) {
>  			for (const struct intel_execution_ring *l = intel_execution_rings; l->name; l++) {
> -				igt_dynamic_f("%s", l->name) {
> -					t->func(fd, intel_ctx_0(fd), eb_ring(l),
> -						t->num_children, t->timeout);
> +				for_each_combination(regions, 1, set) {
> +					sub_name = memregion_dynamic_subtest_name(regions);
> +					region = igt_collection_get_value(regions, 0);
> +					igt_dynamic_f("%s-%s", l->name, sub_name) {
> +						t->func(fd, intel_ctx_0(fd), eb_ring(l),
> +								t->num_children,
> +								t->timeout,
> +								region);
> +					}
> +					free(sub_name);
>  				}
>  			}
>  		}
>  	}
>  
> -	igt_subtest("basic-all")
> -		sync_all(fd, ctx, 1, 2);
> -	igt_subtest("basic-store-all")
> -		store_all(fd, ctx, 1, 2);
> -
> -	igt_subtest("all")
> -		sync_all(fd, ctx, 1, 20);
> -	igt_subtest("store-all")
> -		store_all(fd, ctx, 1, 20);
> -	igt_subtest("forked-all")
> -		sync_all(fd, ctx, ncpus, 20);
> -	igt_subtest("forked-store-all")
> -		store_all(fd, ctx, ncpus, 20);

All below are asking for macro like in gem_exec_suspend.

--
Zbigniew

> +	igt_subtest_with_dynamic("basic-all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("%s", sub_name)
> +				sync_all(fd, ctx, 1, 2, region);
> +			free(sub_name);
> +		}
> +	}
> +	igt_subtest_with_dynamic("basic-store-all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("%s", sub_name)
> +				store_all(fd, ctx, 1, 2, region);
> +			free(sub_name);
> +		}
> +	}
> +	igt_subtest_with_dynamic("all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("%s", sub_name)
> +				sync_all(fd, ctx, 1, 20, region);
> +			free(sub_name);
> +		}
> +	}
> +	igt_subtest_with_dynamic("store-all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("%s", sub_name)
> +				store_all(fd, ctx, 1, 20, region);
> +			free(sub_name);
> +		}
> +	}
> +	igt_subtest_with_dynamic("forked-all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("%s", sub_name)
> +				sync_all(fd, ctx, ncpus, 20, region);
> +			free(sub_name);
> +		}
> +	}
> +	igt_subtest_with_dynamic("forked-store-all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("%s", sub_name)
> +				store_all(fd, ctx, ncpus, 20, region);
> +			free(sub_name);
> +		}
> +	}
>  
>  	for_each_test(t, all) {
> -		igt_subtest_f("%s", t->name)
> -			t->func(fd, ctx, ALL_ENGINES, t->num_children, t->timeout);
> +		igt_subtest_with_dynamic_f("%s", t->name) {
> +			for_each_combination(regions, 1, set) {
> +				sub_name = memregion_dynamic_subtest_name(regions);
> +				region = igt_collection_get_value(regions, 0);
> +				igt_dynamic_f("%s", sub_name)
> +					t->func(fd, ctx, ALL_ENGINES, t->num_children,
> +							t->timeout, region);
> +				free(sub_name);
> +			}
> +		}
>  	}
>  
>  	/* New way of selecting engines. */
>  	for_each_test(t, individual) {
>  		igt_subtest_with_dynamic_f("%s", t->name) {
> -			for_each_ctx_engine(fd, ctx, e) {
> -				igt_dynamic_f("%s", e->name) {
> -					t->func(fd, ctx, e->flags,
> -						t->num_children, t->timeout);
> -				}
> +			for_each_combination(regions, 1, set) {
> +				sub_name = memregion_dynamic_subtest_name(regions);
> +				region = igt_collection_get_value(regions, 0);
> +				for_each_ctx_engine(fd, ctx, e)
> +					igt_dynamic_f("%s-%s", e->name, sub_name)
> +						t->func(fd, ctx, e->flags,
> +								t->num_children,
> +								t->timeout, region);
> +				free(sub_name);
>  			}
>  		}
>  	}
> @@ -1314,6 +1381,8 @@ igt_main
>  	}
>  
>  	igt_fixture {
> +		free(query_info);
> +		igt_collection_destroy(set);
>  		intel_allocator_multiprocess_stop();
>  		igt_stop_hang_detector();
>  		intel_ctx_destroy(fd, ctx);
> -- 
> 2.32.0
> 

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

* [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory
@ 2021-12-24 20:03 sai.gowtham.ch
  2022-01-03 11:27 ` Zbigniew Kempczyński
  0 siblings, 1 reply; 11+ messages in thread
From: sai.gowtham.ch @ 2021-12-24 20:03 UTC (permalink / raw)
  To: igt-dev, sai.gowtham.ch, zbigniew.kempczynski

From: Ch Sai Gowtham <sai.gowtham.ch@intel.com>

Add support for local memory region (Device memory)

Signed-off-by: Ch Sai Gowtham <sai.gowtham.ch@intel.com>
Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 tests/i915/gem_sync.c | 193 ++++++++++++++++++++++++++++--------------
 1 file changed, 131 insertions(+), 62 deletions(-)

diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c
index 8c435845..8f8be2b1 100644
--- a/tests/i915/gem_sync.c
+++ b/tests/i915/gem_sync.c
@@ -143,9 +143,20 @@ static void xchg_engine(void *array, unsigned i, unsigned j)
 	igt_swap(E[i], E[j]);
 }
 
+static uint32_t batch_create(int fd, uint32_t batch_size, uint32_t region)
+{
+	const uint32_t bbe = MI_BATCH_BUFFER_END;
+	uint32_t handle;
+
+	handle = gem_create_in_memory_regions(fd, 4096, region);
+	gem_write(fd, handle, 0, &bbe, sizeof(bbe));
+
+	return handle;
+}
+
 static void
 sync_ring(int fd, const intel_ctx_t *ctx,
-	  unsigned ring, int num_children, int timeout)
+	  unsigned ring, int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 
@@ -155,15 +166,13 @@ sync_ring(int fd, const intel_ctx_t *ctx,
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object;
 		struct drm_i915_gem_execbuffer2 execbuf;
 		double start, elapsed;
 		unsigned long cycles;
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
-		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
+		object.handle = batch_create(fd, 4096, region);
 
 		memset(&execbuf, 0, sizeof(execbuf));
 		execbuf.buffers_ptr = to_user_pointer(&object);
@@ -193,9 +202,8 @@ sync_ring(int fd, const intel_ctx_t *ctx,
 
 static void
 idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
-	  int num_children, int timeout)
+	  int num_children, int timeout, uint32_t region)
 {
-	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	struct drm_i915_gem_exec_object2 object;
 	struct drm_i915_gem_execbuffer2 execbuf;
 	double start, elapsed;
@@ -204,8 +212,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
 	gem_require_ring(fd, ring);
 
 	memset(&object, 0, sizeof(object));
-	object.handle = gem_create(fd, 4096);
-	gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
+	object.handle = batch_create(fd, 4096, region);
 
 	memset(&execbuf, 0, sizeof(execbuf));
 	execbuf.buffers_ptr = to_user_pointer(&object);
@@ -234,7 +241,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
 
 static void
 wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
-	    int timeout, int wlen)
+	    int timeout, int wlen, uint32_t region)
 {
 	struct intel_engine_data ied;
 	uint64_t ahnd = get_reloc_ahnd(fd, ctx->id);
@@ -254,7 +261,7 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 		ahnd = get_reloc_ahnd(fd, ctx->id);
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
+		object.handle = gem_create_in_memory_regions(fd, 4096, region);
 		object.offset = get_offset(ahnd, object.handle, 4096, 0);
 		if (ahnd)
 			object.flags = EXEC_OBJECT_PINNED;
@@ -339,7 +346,7 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 }
 
 static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
-			int num_children, int timeout)
+			int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 	uint64_t ahnd = get_reloc_ahnd(fd, ctx->id);
@@ -398,7 +405,7 @@ static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
 
 static void
 active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
-		   int timeout, int wlen)
+		   int timeout, int wlen, uint32_t region)
 {
 	struct intel_engine_data ied;
 	uint64_t ahnd0 = get_reloc_ahnd(fd, 0);
@@ -420,7 +427,7 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 		ahnd = get_reloc_ahnd(fd, ctx->id);
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
+		object.handle = gem_create_in_memory_regions(fd, 4096, region);
 		object.offset = get_offset(ahnd, object.handle, 4096, 0);
 		if (ahnd)
 			object.offset = EXEC_OBJECT_PINNED;
@@ -529,7 +536,7 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 static void
 store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
-	   int num_children, int timeout)
+	   int num_children, int timeout, uint32_t region)
 {
 	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
 	struct intel_engine_data ied;
@@ -541,7 +548,6 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object[2];
 		struct drm_i915_gem_relocation_entry reloc[1024];
 		struct drm_i915_gem_execbuffer2 execbuf;
@@ -559,14 +565,13 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 		execbuf.rsvd1 = ctx->id;
 
 		memset(object, 0, sizeof(object));
-		object[0].handle = gem_create(fd, 4096);
-		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
+		object[0].handle = batch_create(fd, 4096, region);
 		execbuf.buffer_count = 1;
 		gem_execbuf(fd, &execbuf);
 
 		object[0].flags |= EXEC_OBJECT_WRITE;
 		object[0].flags |= has_relocs ? 0 : EXEC_OBJECT_PINNED;
-		object[1].handle = gem_create(fd, 20*1024);
+		object[1].handle = gem_create_in_memory_regions(fd, 20*1024, region);
 
 		object[1].relocs_ptr = to_user_pointer(reloc);
 		object[1].relocation_count = has_relocs ? 1024 : 0;
@@ -629,7 +634,7 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 static void
 switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
-	    int num_children, int timeout)
+	    int num_children, int timeout, uint32_t region)
 {
 	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
 	struct intel_engine_data ied;
@@ -653,7 +658,6 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 		unsigned long cycles;
 
 		for (int i = 0; i < ARRAY_SIZE(contexts); i++) {
-			const uint32_t bbe = MI_BATCH_BUFFER_END;
 			const uint32_t sz = 32 << 10;
 			struct context *c = &contexts[i];
 			uint32_t *batch, *b;
@@ -670,13 +674,12 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 			c->execbuf.rsvd1 = c->ctx->id;
 
 			memset(c->object, 0, sizeof(c->object));
-			c->object[0].handle = gem_create(fd, 4096);
-			gem_write(fd, c->object[0].handle, 0, &bbe, sizeof(bbe));
+			c->object[0].handle = batch_create(fd, 4096, region);
 			c->execbuf.buffer_count = 1;
 			gem_execbuf(fd, &c->execbuf);
 
 			c->object[0].flags |= EXEC_OBJECT_WRITE;
-			c->object[1].handle = gem_create(fd, sz);
+			c->object[1].handle = gem_create_in_memory_regions(fd, sz, region);
 
 			c->object[1].relocs_ptr = to_user_pointer(c->reloc);
 			c->object[1].relocation_count = has_relocs ? 1024 * i : 0;
@@ -813,10 +816,9 @@ static void *waiter(void *arg)
 
 static void
 __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
-	     int timeout, unsigned long *cycles)
+	     int timeout, unsigned long *cycles, uint32_t region)
 {
 	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
-	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	struct drm_i915_gem_exec_object2 object[2];
 	struct drm_i915_gem_execbuffer2 execbuf;
 	struct drm_i915_gem_relocation_entry reloc[1024];
@@ -836,8 +838,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
 	execbuf.rsvd1 = ctx->id;
 
 	memset(object, 0, sizeof(object));
-	object[0].handle = gem_create(fd, 4096);
-	gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
+	object[0].handle = batch_create(fd, 4096, region);
 	execbuf.buffer_count = 1;
 	gem_execbuf(fd, &execbuf);
 	object[0].flags |= EXEC_OBJECT_WRITE;
@@ -945,7 +946,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 static void
 store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
-	   int num_children, int timeout)
+	   int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 	unsigned long *shared;
@@ -963,7 +964,8 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
 			__store_many(fd, ctx,
 				     ied_flags(&ied, n),
 				     timeout,
-				     &shared[n]);
+				     &shared[n],
+				     region);
 	}
 	igt_waitchildren();
 
@@ -976,7 +978,7 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
 }
 
 static void
-sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
+sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 
@@ -985,15 +987,13 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object;
 		struct drm_i915_gem_execbuffer2 execbuf;
 		double start, elapsed;
 		unsigned long cycles;
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
-		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
+		object.handle = batch_create(fd, 4096, region);
 
 		memset(&execbuf, 0, sizeof(execbuf));
 		execbuf.buffers_ptr = to_user_pointer(&object);
@@ -1023,7 +1023,7 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 }
 
 static void
-store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
+store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
 {
 	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
 	struct intel_engine_data ied;
@@ -1034,7 +1034,6 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object[2];
 		struct drm_i915_gem_relocation_entry reloc[1024];
 		struct drm_i915_gem_execbuffer2 execbuf;
@@ -1051,14 +1050,13 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 		execbuf.rsvd1 = ctx->id;
 
 		memset(object, 0, sizeof(object));
-		object[0].handle = gem_create(fd, 4096);
-		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
+		object[0].handle = batch_create(fd, 4096, region);
 		execbuf.buffer_count = 1;
 		gem_execbuf(fd, &execbuf);
 
 		object[0].flags |= EXEC_OBJECT_WRITE;
 		object[0].flags |= has_relocs ? 0 : EXEC_OBJECT_PINNED;
-		object[1].handle = gem_create(fd, 1024*16 + 4096);
+		object[1].handle = gem_create_in_memory_regions(fd, 1024*16 + 4096, region);
 
 		object[1].relocs_ptr = to_user_pointer(reloc);
 		object[1].relocation_count = has_relocs ? 1024 : 0;
@@ -1205,7 +1203,7 @@ igt_main
 	const struct {
 		const char *name;
 		void (*func)(int fd, const intel_ctx_t *ctx, unsigned int engine,
-			     int num_children, int timeout);
+			     int num_children, int timeout, uint32_t memregion);
 		int num_children;
 		int timeout;
 	} all[] = {
@@ -1241,6 +1239,10 @@ igt_main
 	const struct intel_execution_engine2 *e;
 	const intel_ctx_t *ctx;
 	int fd = -1;
+	struct drm_i915_query_memory_regions *query_info;
+	struct igt_collection *regions, *set;
+	char *sub_name;
+	uint32_t region;
 
 	igt_fixture {
 		fd = drm_open_driver(DRIVER_INTEL);
@@ -1250,6 +1252,11 @@ igt_main
 		ctx = intel_ctx_create_all_physical(fd);
 
 		igt_fork_hang_detector(fd);
+		query_info = gem_get_query_memory_regions(fd);
+		igt_assert(query_info);
+		set = get_memory_region_set(query_info,
+					I915_SYSTEM_MEMORY,
+					I915_DEVICE_MEMORY);
 		intel_allocator_multiprocess_start();
 	}
 
@@ -1257,41 +1264,101 @@ igt_main
 	for_each_test(t, individual) {
 		igt_subtest_with_dynamic_f("legacy-%s", t->name) {
 			for (const struct intel_execution_ring *l = intel_execution_rings; l->name; l++) {
-				igt_dynamic_f("%s", l->name) {
-					t->func(fd, intel_ctx_0(fd), eb_ring(l),
-						t->num_children, t->timeout);
+				for_each_combination(regions, 1, set) {
+					sub_name = memregion_dynamic_subtest_name(regions);
+					region = igt_collection_get_value(regions, 0);
+					igt_dynamic_f("%s-%s", l->name, sub_name) {
+						t->func(fd, intel_ctx_0(fd), eb_ring(l),
+								t->num_children,
+								t->timeout,
+								region);
+					}
+					free(sub_name);
 				}
 			}
 		}
 	}
 
-	igt_subtest("basic-all")
-		sync_all(fd, ctx, 1, 2);
-	igt_subtest("basic-store-all")
-		store_all(fd, ctx, 1, 2);
-
-	igt_subtest("all")
-		sync_all(fd, ctx, 1, 20);
-	igt_subtest("store-all")
-		store_all(fd, ctx, 1, 20);
-	igt_subtest("forked-all")
-		sync_all(fd, ctx, ncpus, 20);
-	igt_subtest("forked-store-all")
-		store_all(fd, ctx, ncpus, 20);
+	igt_subtest_with_dynamic("basic-all") {
+		for_each_combination(regions, 1, set) {
+			sub_name = memregion_dynamic_subtest_name(regions);
+			region = igt_collection_get_value(regions, 0);
+			igt_dynamic_f("%s", sub_name)
+				sync_all(fd, ctx, 1, 2, region);
+			free(sub_name);
+		}
+	}
+	igt_subtest_with_dynamic("basic-store-all") {
+		for_each_combination(regions, 1, set) {
+			sub_name = memregion_dynamic_subtest_name(regions);
+			region = igt_collection_get_value(regions, 0);
+			igt_dynamic_f("%s", sub_name)
+				store_all(fd, ctx, 1, 2, region);
+			free(sub_name);
+		}
+	}
+	igt_subtest_with_dynamic("all") {
+		for_each_combination(regions, 1, set) {
+			sub_name = memregion_dynamic_subtest_name(regions);
+			region = igt_collection_get_value(regions, 0);
+			igt_dynamic_f("%s", sub_name)
+				sync_all(fd, ctx, 1, 20, region);
+			free(sub_name);
+		}
+	}
+	igt_subtest_with_dynamic("store-all") {
+		for_each_combination(regions, 1, set) {
+			sub_name = memregion_dynamic_subtest_name(regions);
+			region = igt_collection_get_value(regions, 0);
+			igt_dynamic_f("%s", sub_name)
+				store_all(fd, ctx, 1, 20, region);
+			free(sub_name);
+		}
+	}
+	igt_subtest_with_dynamic("forked-all") {
+		for_each_combination(regions, 1, set) {
+			sub_name = memregion_dynamic_subtest_name(regions);
+			region = igt_collection_get_value(regions, 0);
+			igt_dynamic_f("%s", sub_name)
+				sync_all(fd, ctx, ncpus, 20, region);
+			free(sub_name);
+		}
+	}
+	igt_subtest_with_dynamic("forked-store-all") {
+		for_each_combination(regions, 1, set) {
+			sub_name = memregion_dynamic_subtest_name(regions);
+			region = igt_collection_get_value(regions, 0);
+			igt_dynamic_f("%s", sub_name)
+				store_all(fd, ctx, ncpus, 20, region);
+			free(sub_name);
+		}
+	}
 
 	for_each_test(t, all) {
-		igt_subtest_f("%s", t->name)
-			t->func(fd, ctx, ALL_ENGINES, t->num_children, t->timeout);
+		igt_subtest_with_dynamic_f("%s", t->name) {
+			for_each_combination(regions, 1, set) {
+				sub_name = memregion_dynamic_subtest_name(regions);
+				region = igt_collection_get_value(regions, 0);
+				igt_dynamic_f("%s", sub_name)
+					t->func(fd, ctx, ALL_ENGINES, t->num_children,
+							t->timeout, region);
+				free(sub_name);
+			}
+		}
 	}
 
 	/* New way of selecting engines. */
 	for_each_test(t, individual) {
 		igt_subtest_with_dynamic_f("%s", t->name) {
-			for_each_ctx_engine(fd, ctx, e) {
-				igt_dynamic_f("%s", e->name) {
-					t->func(fd, ctx, e->flags,
-						t->num_children, t->timeout);
-				}
+			for_each_combination(regions, 1, set) {
+				sub_name = memregion_dynamic_subtest_name(regions);
+				region = igt_collection_get_value(regions, 0);
+				for_each_ctx_engine(fd, ctx, e)
+					igt_dynamic_f("%s-%s", e->name, sub_name)
+						t->func(fd, ctx, e->flags,
+								t->num_children,
+								t->timeout, region);
+				free(sub_name);
 			}
 		}
 	}
@@ -1314,6 +1381,8 @@ igt_main
 	}
 
 	igt_fixture {
+		free(query_info);
+		igt_collection_destroy(set);
 		intel_allocator_multiprocess_stop();
 		igt_stop_hang_detector();
 		intel_ctx_destroy(fd, ctx);
-- 
2.32.0

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

* Re: [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory
  2021-10-05  5:14 sai.gowtham.ch
  2021-10-05 10:11 ` Zbigniew Kempczyński
@ 2021-10-05 10:31 ` Petri Latvala
  1 sibling, 0 replies; 11+ messages in thread
From: Petri Latvala @ 2021-10-05 10:31 UTC (permalink / raw)
  To: sai.gowtham.ch; +Cc: igt-dev, zbigniew.kempczynski

On Tue, Oct 05, 2021 at 10:44:48AM +0530, sai.gowtham.ch@intel.com wrote:
> From: Ch Sai Gowtham <sai.gowtham.ch@intel.com>
> 
> Add support for local memory region (Device memory)
> 
> Signed-off-by: Ch Sai Gowtham <sai.gowtham.ch@intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> ---
>  tests/i915/gem_sync.c | 198 +++++++++++++++++++++++++++++-------------
>  1 file changed, 136 insertions(+), 62 deletions(-)
> 
> diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c
> index 6cb00c40..f80b6310 100644
> --- a/tests/i915/gem_sync.c
> +++ b/tests/i915/gem_sync.c
> @@ -143,9 +143,20 @@ static void xchg_engine(void *array, unsigned i, unsigned j)
>  	igt_swap(E[i], E[j]);
>  }
>  
> +static uint32_t batch_create(int fd, uint32_t batch_size, uint32_t region)
> +{
> +	const uint32_t bbe = MI_BATCH_BUFFER_END;
> +	uint32_t handle;
> +
> +	handle = gem_create_in_memory_regions(fd, 4096, region);
> +	gem_write(fd, handle, 0, &bbe, sizeof(bbe));
> +
> +	return handle;
> +}
> +
>  static void
>  sync_ring(int fd, const intel_ctx_t *ctx,
> -	  unsigned ring, int num_children, int timeout)
> +	  unsigned ring, int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  
> @@ -155,15 +166,13 @@ sync_ring(int fd, const intel_ctx_t *ctx,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double start, elapsed;
>  		unsigned long cycles;
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +		object.handle = batch_create(fd, 4096, region);
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -193,9 +202,8 @@ sync_ring(int fd, const intel_ctx_t *ctx,
>  
>  static void
>  idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
> -	  int num_children, int timeout)
> +	  int num_children, int timeout, uint32_t region)
>  {
> -	const uint32_t bbe = MI_BATCH_BUFFER_END;
>  	struct drm_i915_gem_exec_object2 object;
>  	struct drm_i915_gem_execbuffer2 execbuf;
>  	double start, elapsed;
> @@ -204,8 +212,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  	gem_require_ring(fd, ring);
>  
>  	memset(&object, 0, sizeof(object));
> -	object.handle = gem_create(fd, 4096);
> -	gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +	object.handle = batch_create(fd, 4096, region);
>  
>  	memset(&execbuf, 0, sizeof(execbuf));
>  	execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -234,7 +241,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  
>  static void
>  wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	    int timeout, int wlen)
> +	    int timeout, int wlen, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  
> @@ -243,7 +250,6 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, ied.nengines) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double end, this, elapsed, now, baseline;
> @@ -251,8 +257,7 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		igt_spin_t *spin;
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +		object.handle = batch_create(fd, 4096, region);
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -329,7 +334,7 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  }
>  
>  static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
> -			int num_children, int timeout)
> +			int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  
> @@ -381,7 +386,7 @@ static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  
>  static void
>  active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -		   int timeout, int wlen)
> +		   int timeout, int wlen, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  
> @@ -390,7 +395,6 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, ied.nengines) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double end, this, elapsed, now, baseline;
> @@ -398,8 +402,7 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		igt_spin_t *spin[2];
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +		object.handle = batch_create(fd, 4096, region);
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -497,7 +500,7 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  static void
>  store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	   int num_children, int timeout)
> +	   int num_children, int timeout, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
>  	struct intel_engine_data ied;
> @@ -508,7 +511,6 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object[2];
>  		struct drm_i915_gem_relocation_entry reloc[1024];
>  		struct drm_i915_gem_execbuffer2 execbuf;
> @@ -526,8 +528,7 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		execbuf.rsvd1 = ctx->id;
>  
>  		memset(object, 0, sizeof(object));
> -		object[0].handle = gem_create(fd, 4096);
> -		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
> +		object[0].handle = batch_create(fd, 4096, region);
>  		execbuf.buffer_count = 1;
>  		gem_execbuf(fd, &execbuf);
>  
> @@ -595,7 +596,7 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  static void
>  switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	    int num_children, int timeout)
> +	    int num_children, int timeout, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
>  	struct intel_engine_data ied;
> @@ -618,7 +619,6 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		unsigned long cycles;
>  
>  		for (int i = 0; i < ARRAY_SIZE(contexts); i++) {
> -			const uint32_t bbe = MI_BATCH_BUFFER_END;
>  			const uint32_t sz = 32 << 10;
>  			struct context *c = &contexts[i];
>  			uint32_t *batch, *b;
> @@ -635,8 +635,7 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  			c->execbuf.rsvd1 = c->ctx->id;
>  
>  			memset(c->object, 0, sizeof(c->object));
> -			c->object[0].handle = gem_create(fd, 4096);
> -			gem_write(fd, c->object[0].handle, 0, &bbe, sizeof(bbe));
> +			c->object[0].handle = batch_create(fd, 4096, region);
>  			c->execbuf.buffer_count = 1;
>  			gem_execbuf(fd, &c->execbuf);
>  
> @@ -778,10 +777,9 @@ static void *waiter(void *arg)
>  
>  static void
>  __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	     int timeout, unsigned long *cycles)
> +	     int timeout, unsigned long *cycles, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
> -	const uint32_t bbe = MI_BATCH_BUFFER_END;
>  	struct drm_i915_gem_exec_object2 object[2];
>  	struct drm_i915_gem_execbuffer2 execbuf;
>  	struct drm_i915_gem_relocation_entry reloc[1024];
> @@ -800,8 +798,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
>  	execbuf.rsvd1 = ctx->id;
>  
>  	memset(object, 0, sizeof(object));
> -	object[0].handle = gem_create(fd, 4096);
> -	gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
> +	object[0].handle = batch_create(fd, 4096, region);
>  	execbuf.buffer_count = 1;
>  	gem_execbuf(fd, &execbuf);
>  	object[0].flags |= EXEC_OBJECT_WRITE;
> @@ -908,7 +905,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  static void
>  store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
> -	   int num_children, int timeout)
> +	   int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  	unsigned long *shared;
> @@ -926,7 +923,8 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  			__store_many(fd, ctx,
>  				     ied_flags(&ied, n),
>  				     timeout,
> -				     &shared[n]);
> +				     &shared[n],
> +				     region);
>  	}
>  	igt_waitchildren();
>  
> @@ -939,7 +937,7 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  }
>  
>  static void
> -sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
> +sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  
> @@ -948,15 +946,13 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double start, elapsed;
>  		unsigned long cycles;
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +		object.handle = batch_create(fd, 4096, region);
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -986,7 +982,7 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  }
>  
>  static void
> -store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
> +store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
>  	struct intel_engine_data ied;
> @@ -996,7 +992,6 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object[2];
>  		struct drm_i915_gem_relocation_entry reloc[1024];
>  		struct drm_i915_gem_execbuffer2 execbuf;
> @@ -1013,8 +1008,7 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  		execbuf.rsvd1 = ctx->id;
>  
>  		memset(object, 0, sizeof(object));
> -		object[0].handle = gem_create(fd, 4096);
> -		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
> +		object[0].handle = batch_create(fd, 4096, region);
>  		execbuf.buffer_count = 1;
>  		gem_execbuf(fd, &execbuf);
>  
> @@ -1156,7 +1150,7 @@ igt_main
>  	const struct {
>  		const char *name;
>  		void (*func)(int fd, const intel_ctx_t *ctx, unsigned int engine,
> -			     int num_children, int timeout);
> +			     int num_children, int timeout, uint32_t memregion);
>  		int num_children;
>  		int timeout;
>  	} all[] = {
> @@ -1192,6 +1186,10 @@ igt_main
>  	const struct intel_execution_engine2 *e;
>  	const intel_ctx_t *ctx;
>  	int fd = -1;
> +	struct drm_i915_query_memory_regions *query_info;
> +	struct igt_collection *regions, *set;
> +	char *sub_name;
> +	uint32_t region;
>  
>  	igt_fixture {
>  		fd = drm_open_driver(DRIVER_INTEL);
> @@ -1201,47 +1199,121 @@ igt_main
>  		ctx = intel_ctx_create_all_physical(fd);
>  
>  		igt_fork_hang_detector(fd);
> +		query_info = gem_get_query_memory_regions(fd);
> +		igt_assert(query_info);
> +		set = get_memory_region_set(query_info,
> +					I915_SYSTEM_MEMORY,
> +					I915_DEVICE_MEMORY);
> +
>  	}
>  
>  	/* Legacy for selecting rings. */
>  	for_each_test(t, individual) {
>  		igt_subtest_with_dynamic_f("%s", t->name) {
>  			for (const struct intel_execution_ring *l = intel_execution_rings; l->name; l++) {
> -				igt_dynamic_f("%s", l->name) {
> -					t->func(fd, intel_ctx_0(fd), eb_ring(l),
> -						t->num_children, t->timeout);
> +				for_each_combination(regions, 1, set) {
> +					sub_name = memregion_dynamic_subtest_name(regions);
> +					region = igt_collection_get_value(regions, 0);
> +					igt_dynamic_f("%s", l->name) {

Did you mean to use sub_name here somewhere?

> +						t->func(fd, intel_ctx_0(fd), eb_ring(l),
> +								t->num_children, 
> +								t->timeout,
> +								region);
> +					}
> +					free(sub_name);
>  				}
>  			}
>  		}
>  	}
>  
> -	igt_subtest("basic-all")
> -		sync_all(fd, ctx, 1, 2);
> -	igt_subtest("basic-store-all")
> -		store_all(fd, ctx, 1, 2);
> +	igt_subtest_with_dynamic("basic-all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("basic-all-%s", sub_name)

Don't repeat "basic-all".

> +				sync_all(fd, ctx, 1, 2, region);
> +			free(sub_name);
> +		}
> +	}
> +
> +	igt_subtest_with_dynamic("basic-store-all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("basic-store-all-%s", sub_name)

Same here.

> +				store_all(fd, ctx, 1, 2, region);
> +			free(sub_name);
> +		}
> +	}
> +
> +	igt_subtest_with_dynamic("all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("all-%s", sub_name)

And here.

> +				sync_all(fd, ctx, 1, 20, region);
> +			free(sub_name);
> +		}
> +	}
> +
> +	igt_subtest_with_dynamic("store-all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("store-all-%s", sub_name)

Likewise.

> +				store_all(fd, ctx, 1, 20, region);
> +			free(sub_name);
> +		}
> +	}
>  
> -	igt_subtest("all")
> -		sync_all(fd, ctx, 1, 20);
> -	igt_subtest("store-all")
> -		store_all(fd, ctx, 1, 20);
> -	igt_subtest("forked-all")
> -		sync_all(fd, ctx, ncpus, 20);
> -	igt_subtest("forked-store-all")
> -		store_all(fd, ctx, ncpus, 20);
> +	igt_subtest_with_dynamic("forked-all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("forked-all-%s", sub_name)

Same.

> +				sync_all(fd, ctx, ncpus, 20, region);
> +			free(sub_name);
> +		}
> +	}
> +
> +	igt_subtest_with_dynamic("forked-store-all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("forked-store-all-%s", sub_name)

Same.


-- 
Petri Latvala


> +				store_all(fd, ctx, ncpus, 20, region);
> +			free(sub_name);
> +		}
> +	}
>  
>  	for_each_test(t, all) {
> -		igt_subtest_f("%s", t->name)
> -			t->func(fd, ctx, ALL_ENGINES, t->num_children, t->timeout);
> +		igt_subtest_with_dynamic_f("%s", t->name) {
> +			for_each_combination(regions, 1, set) {
> +				sub_name = memregion_dynamic_subtest_name(regions);
> +				region = igt_collection_get_value(regions, 0);
> +				igt_dynamic_f("%s", sub_name)
> +					t->func(fd, ctx, ALL_ENGINES, t->num_children,
> +							t->timeout,
> +							region);
> +				free(sub_name);
> +			}
> +		}
>  	}
>  
>  	/* New way of selecting engines. */
>  	for_each_test(t, individual) {
>  		igt_subtest_with_dynamic_f("%s", t->name) {
> -			for_each_ctx_engine(fd, ctx, e) {
> -				igt_dynamic_f("%s", e->name) {
> -					t->func(fd, ctx, e->flags,
> -						t->num_children, t->timeout);
> -				}
> +			for_each_combination(regions, 1, set) {
> +				sub_name = memregion_dynamic_subtest_name(regions);
> +				region = igt_collection_get_value(regions, 0);
> +				for_each_ctx_engine(fd, ctx, e)
> +					igt_dynamic_f("%s-%s", e->name, sub_name)
> +						t->func(fd, ctx, e->flags,
> +								t->num_children,
> +								t->timeout,
> +								region);
> +				free(sub_name);
> +
>  			}
>  		}
>  	}
> @@ -1264,6 +1336,8 @@ igt_main
>  	}
>  
>  	igt_fixture {
> +		free(query_info);
> +		igt_collection_destroy(set);
>  		igt_stop_hang_detector();
>  		intel_ctx_destroy(fd, ctx);
>  		close(fd);
> -- 
> 2.32.0
> 

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

* Re: [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory
  2021-10-05  5:14 sai.gowtham.ch
@ 2021-10-05 10:11 ` Zbigniew Kempczyński
  2021-10-05 10:31 ` Petri Latvala
  1 sibling, 0 replies; 11+ messages in thread
From: Zbigniew Kempczyński @ 2021-10-05 10:11 UTC (permalink / raw)
  To: sai.gowtham.ch; +Cc: igt-dev

On Tue, Oct 05, 2021 at 10:44:48AM +0530, sai.gowtham.ch@intel.com wrote:
> From: Ch Sai Gowtham <sai.gowtham.ch@intel.com>
> 
> Add support for local memory region (Device memory)
> 
> Signed-off-by: Ch Sai Gowtham <sai.gowtham.ch@intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> ---
>  tests/i915/gem_sync.c | 198 +++++++++++++++++++++++++++++-------------
>  1 file changed, 136 insertions(+), 62 deletions(-)
> 
> diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c
> index 6cb00c40..f80b6310 100644
> --- a/tests/i915/gem_sync.c
> +++ b/tests/i915/gem_sync.c
> @@ -143,9 +143,20 @@ static void xchg_engine(void *array, unsigned i, unsigned j)
>  	igt_swap(E[i], E[j]);
>  }
>  
> +static uint32_t batch_create(int fd, uint32_t batch_size, uint32_t region)
> +{
> +	const uint32_t bbe = MI_BATCH_BUFFER_END;
> +	uint32_t handle;
> +
> +	handle = gem_create_in_memory_regions(fd, 4096, region);
> +	gem_write(fd, handle, 0, &bbe, sizeof(bbe));
> +
> +	return handle;
> +}
> +
>  static void
>  sync_ring(int fd, const intel_ctx_t *ctx,
> -	  unsigned ring, int num_children, int timeout)
> +	  unsigned ring, int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  
> @@ -155,15 +166,13 @@ sync_ring(int fd, const intel_ctx_t *ctx,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double start, elapsed;
>  		unsigned long cycles;
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +		object.handle = batch_create(fd, 4096, region);
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -193,9 +202,8 @@ sync_ring(int fd, const intel_ctx_t *ctx,
>  
>  static void
>  idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
> -	  int num_children, int timeout)
> +	  int num_children, int timeout, uint32_t region)
>  {
> -	const uint32_t bbe = MI_BATCH_BUFFER_END;
>  	struct drm_i915_gem_exec_object2 object;
>  	struct drm_i915_gem_execbuffer2 execbuf;
>  	double start, elapsed;
> @@ -204,8 +212,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  	gem_require_ring(fd, ring);
>  
>  	memset(&object, 0, sizeof(object));
> -	object.handle = gem_create(fd, 4096);
> -	gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +	object.handle = batch_create(fd, 4096, region);
>  
>  	memset(&execbuf, 0, sizeof(execbuf));
>  	execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -234,7 +241,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  
>  static void
>  wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	    int timeout, int wlen)
> +	    int timeout, int wlen, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  
> @@ -243,7 +250,6 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, ied.nengines) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double end, this, elapsed, now, baseline;
> @@ -251,8 +257,7 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		igt_spin_t *spin;
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +		object.handle = batch_create(fd, 4096, region);
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -329,7 +334,7 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  }
>  
>  static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
> -			int num_children, int timeout)
> +			int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  
> @@ -381,7 +386,7 @@ static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  
>  static void
>  active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -		   int timeout, int wlen)
> +		   int timeout, int wlen, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  
> @@ -390,7 +395,6 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, ied.nengines) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double end, this, elapsed, now, baseline;
> @@ -398,8 +402,7 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		igt_spin_t *spin[2];
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +		object.handle = batch_create(fd, 4096, region);
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -497,7 +500,7 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  static void
>  store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	   int num_children, int timeout)
> +	   int num_children, int timeout, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
>  	struct intel_engine_data ied;
> @@ -508,7 +511,6 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object[2];
>  		struct drm_i915_gem_relocation_entry reloc[1024];
>  		struct drm_i915_gem_execbuffer2 execbuf;
> @@ -526,8 +528,7 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		execbuf.rsvd1 = ctx->id;
>  
>  		memset(object, 0, sizeof(object));
> -		object[0].handle = gem_create(fd, 4096);
> -		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
> +		object[0].handle = batch_create(fd, 4096, region);
>  		execbuf.buffer_count = 1;
>  		gem_execbuf(fd, &execbuf);
>  
> @@ -595,7 +596,7 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  static void
>  switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	    int num_children, int timeout)
> +	    int num_children, int timeout, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
>  	struct intel_engine_data ied;
> @@ -618,7 +619,6 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  		unsigned long cycles;
>  
>  		for (int i = 0; i < ARRAY_SIZE(contexts); i++) {
> -			const uint32_t bbe = MI_BATCH_BUFFER_END;
>  			const uint32_t sz = 32 << 10;
>  			struct context *c = &contexts[i];
>  			uint32_t *batch, *b;
> @@ -635,8 +635,7 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
>  			c->execbuf.rsvd1 = c->ctx->id;
>  
>  			memset(c->object, 0, sizeof(c->object));
> -			c->object[0].handle = gem_create(fd, 4096);
> -			gem_write(fd, c->object[0].handle, 0, &bbe, sizeof(bbe));
> +			c->object[0].handle = batch_create(fd, 4096, region);
>  			c->execbuf.buffer_count = 1;
>  			gem_execbuf(fd, &c->execbuf);
>  
> @@ -778,10 +777,9 @@ static void *waiter(void *arg)
>  
>  static void
>  __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
> -	     int timeout, unsigned long *cycles)
> +	     int timeout, unsigned long *cycles, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
> -	const uint32_t bbe = MI_BATCH_BUFFER_END;
>  	struct drm_i915_gem_exec_object2 object[2];
>  	struct drm_i915_gem_execbuffer2 execbuf;
>  	struct drm_i915_gem_relocation_entry reloc[1024];
> @@ -800,8 +798,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
>  	execbuf.rsvd1 = ctx->id;
>  
>  	memset(object, 0, sizeof(object));
> -	object[0].handle = gem_create(fd, 4096);
> -	gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
> +	object[0].handle = batch_create(fd, 4096, region);
>  	execbuf.buffer_count = 1;
>  	gem_execbuf(fd, &execbuf);
>  	object[0].flags |= EXEC_OBJECT_WRITE;
> @@ -908,7 +905,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
>  
>  static void
>  store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
> -	   int num_children, int timeout)
> +	   int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  	unsigned long *shared;
> @@ -926,7 +923,8 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  			__store_many(fd, ctx,
>  				     ied_flags(&ied, n),
>  				     timeout,
> -				     &shared[n]);
> +				     &shared[n],
> +				     region);
>  	}
>  	igt_waitchildren();
>  
> @@ -939,7 +937,7 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
>  }
>  
>  static void
> -sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
> +sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
>  {
>  	struct intel_engine_data ied;
>  
> @@ -948,15 +946,13 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object;
>  		struct drm_i915_gem_execbuffer2 execbuf;
>  		double start, elapsed;
>  		unsigned long cycles;
>  
>  		memset(&object, 0, sizeof(object));
> -		object.handle = gem_create(fd, 4096);
> -		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
> +		object.handle = batch_create(fd, 4096, region);
>  
>  		memset(&execbuf, 0, sizeof(execbuf));
>  		execbuf.buffers_ptr = to_user_pointer(&object);
> @@ -986,7 +982,7 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  }
>  
>  static void
> -store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
> +store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
>  {
>  	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
>  	struct intel_engine_data ied;
> @@ -996,7 +992,6 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  
>  	intel_detect_and_clear_missed_interrupts(fd);
>  	igt_fork(child, num_children) {
> -		const uint32_t bbe = MI_BATCH_BUFFER_END;
>  		struct drm_i915_gem_exec_object2 object[2];
>  		struct drm_i915_gem_relocation_entry reloc[1024];
>  		struct drm_i915_gem_execbuffer2 execbuf;
> @@ -1013,8 +1008,7 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
>  		execbuf.rsvd1 = ctx->id;
>  
>  		memset(object, 0, sizeof(object));
> -		object[0].handle = gem_create(fd, 4096);
> -		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
> +		object[0].handle = batch_create(fd, 4096, region);
>  		execbuf.buffer_count = 1;
>  		gem_execbuf(fd, &execbuf);
>  
> @@ -1156,7 +1150,7 @@ igt_main
>  	const struct {
>  		const char *name;
>  		void (*func)(int fd, const intel_ctx_t *ctx, unsigned int engine,
> -			     int num_children, int timeout);
> +			     int num_children, int timeout, uint32_t memregion);
>  		int num_children;
>  		int timeout;
>  	} all[] = {
> @@ -1192,6 +1186,10 @@ igt_main
>  	const struct intel_execution_engine2 *e;
>  	const intel_ctx_t *ctx;
>  	int fd = -1;
> +	struct drm_i915_query_memory_regions *query_info;
> +	struct igt_collection *regions, *set;
> +	char *sub_name;
> +	uint32_t region;
>  
>  	igt_fixture {
>  		fd = drm_open_driver(DRIVER_INTEL);
> @@ -1201,47 +1199,121 @@ igt_main
>  		ctx = intel_ctx_create_all_physical(fd);
>  
>  		igt_fork_hang_detector(fd);
> +		query_info = gem_get_query_memory_regions(fd);
> +		igt_assert(query_info);
> +		set = get_memory_region_set(query_info,
> +					I915_SYSTEM_MEMORY,
> +					I915_DEVICE_MEMORY);
> +
>  	}
>  
>  	/* Legacy for selecting rings. */
>  	for_each_test(t, individual) {
>  		igt_subtest_with_dynamic_f("%s", t->name) {
>  			for (const struct intel_execution_ring *l = intel_execution_rings; l->name; l++) {
> -				igt_dynamic_f("%s", l->name) {
> -					t->func(fd, intel_ctx_0(fd), eb_ring(l),
> -						t->num_children, t->timeout);
> +				for_each_combination(regions, 1, set) {
> +					sub_name = memregion_dynamic_subtest_name(regions);
> +					region = igt_collection_get_value(regions, 0);
> +					igt_dynamic_f("%s", l->name) {
> +						t->func(fd, intel_ctx_0(fd), eb_ring(l),
> +								t->num_children, 
> +								t->timeout,
> +								region);
> +					}
> +					free(sub_name);
>  				}
>  			}
>  		}
>  	}
>  
> -	igt_subtest("basic-all")
> -		sync_all(fd, ctx, 1, 2);
> -	igt_subtest("basic-store-all")
> -		store_all(fd, ctx, 1, 2);
> +	igt_subtest_with_dynamic("basic-all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("basic-all-%s", sub_name)
> +				sync_all(fd, ctx, 1, 2, region);
> +			free(sub_name);
> +		}
> +	}
> +
> +	igt_subtest_with_dynamic("basic-store-all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("basic-store-all-%s", sub_name)
> +				store_all(fd, ctx, 1, 2, region);
> +			free(sub_name);
> +		}
> +	}
> +
> +	igt_subtest_with_dynamic("all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("all-%s", sub_name)
> +				sync_all(fd, ctx, 1, 20, region);
> +			free(sub_name);
> +		}
> +	}
> +
> +	igt_subtest_with_dynamic("store-all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("store-all-%s", sub_name)
> +				store_all(fd, ctx, 1, 20, region);
> +			free(sub_name);
> +		}
> +	}
>  
> -	igt_subtest("all")
> -		sync_all(fd, ctx, 1, 20);
> -	igt_subtest("store-all")
> -		store_all(fd, ctx, 1, 20);
> -	igt_subtest("forked-all")
> -		sync_all(fd, ctx, ncpus, 20);
> -	igt_subtest("forked-store-all")
> -		store_all(fd, ctx, ncpus, 20);
> +	igt_subtest_with_dynamic("forked-all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("forked-all-%s", sub_name)
> +				sync_all(fd, ctx, ncpus, 20, region);
> +			free(sub_name);
> +		}
> +	}
> +
> +	igt_subtest_with_dynamic("forked-store-all") {
> +		for_each_combination(regions, 1, set) {
> +			sub_name = memregion_dynamic_subtest_name(regions);
> +			region = igt_collection_get_value(regions, 0);
> +			igt_dynamic_f("forked-store-all-%s", sub_name)
> +				store_all(fd, ctx, ncpus, 20, region);
> +			free(sub_name);
> +		}
> +	}
>  
>  	for_each_test(t, all) {
> -		igt_subtest_f("%s", t->name)
> -			t->func(fd, ctx, ALL_ENGINES, t->num_children, t->timeout);
> +		igt_subtest_with_dynamic_f("%s", t->name) {
> +			for_each_combination(regions, 1, set) {
> +				sub_name = memregion_dynamic_subtest_name(regions);
> +				region = igt_collection_get_value(regions, 0);
> +				igt_dynamic_f("%s", sub_name)
> +					t->func(fd, ctx, ALL_ENGINES, t->num_children,
> +							t->timeout,
> +							region);
> +				free(sub_name);
> +			}
> +		}
>  	}
>  
>  	/* New way of selecting engines. */
>  	for_each_test(t, individual) {
>  		igt_subtest_with_dynamic_f("%s", t->name) {
> -			for_each_ctx_engine(fd, ctx, e) {
> -				igt_dynamic_f("%s", e->name) {
> -					t->func(fd, ctx, e->flags,
> -						t->num_children, t->timeout);
> -				}
> +			for_each_combination(regions, 1, set) {
> +				sub_name = memregion_dynamic_subtest_name(regions);
> +				region = igt_collection_get_value(regions, 0);
> +				for_each_ctx_engine(fd, ctx, e)
> +					igt_dynamic_f("%s-%s", e->name, sub_name)
> +						t->func(fd, ctx, e->flags,
> +								t->num_children,
> +								t->timeout,
> +								region);
> +				free(sub_name);
> +

I got some doubts regarding the idea of the change. Batch is taken
from different regions but in some tests target buffers still are
taken from system memory. It would be nice to mix regions 
(variations with repetitions) when batch + target buffer are used
together in the test.

For gen12+ with integrated cards test will likely run correctly only
on TGL (reloc patch in core-for-CI for RKL/ADL is temporary) so 
you can expect failure when it will be removed on gens > TGL.

Code should also be rewritten to use allocator and no-reloc 
- if you would run this on DG1 you may expect failure here.

Shape of subtest naming change to use igt_collection is ok, just
no-reloc + mixing regions are my main concern.

--
Zbigniew

>  			}
>  		}
>  	}
> @@ -1264,6 +1336,8 @@ igt_main
>  	}
>  
>  	igt_fixture {
> +		free(query_info);
> +		igt_collection_destroy(set);
>  		igt_stop_hang_detector();
>  		intel_ctx_destroy(fd, ctx);
>  		close(fd);
> -- 
> 2.32.0
> 

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

* [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory
@ 2021-10-05  5:14 sai.gowtham.ch
  2021-10-05 10:11 ` Zbigniew Kempczyński
  2021-10-05 10:31 ` Petri Latvala
  0 siblings, 2 replies; 11+ messages in thread
From: sai.gowtham.ch @ 2021-10-05  5:14 UTC (permalink / raw)
  To: igt-dev, sai.gowtham.ch, zbigniew.kempczynski

From: Ch Sai Gowtham <sai.gowtham.ch@intel.com>

Add support for local memory region (Device memory)

Signed-off-by: Ch Sai Gowtham <sai.gowtham.ch@intel.com>
Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 tests/i915/gem_sync.c | 198 +++++++++++++++++++++++++++++-------------
 1 file changed, 136 insertions(+), 62 deletions(-)

diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c
index 6cb00c40..f80b6310 100644
--- a/tests/i915/gem_sync.c
+++ b/tests/i915/gem_sync.c
@@ -143,9 +143,20 @@ static void xchg_engine(void *array, unsigned i, unsigned j)
 	igt_swap(E[i], E[j]);
 }
 
+static uint32_t batch_create(int fd, uint32_t batch_size, uint32_t region)
+{
+	const uint32_t bbe = MI_BATCH_BUFFER_END;
+	uint32_t handle;
+
+	handle = gem_create_in_memory_regions(fd, 4096, region);
+	gem_write(fd, handle, 0, &bbe, sizeof(bbe));
+
+	return handle;
+}
+
 static void
 sync_ring(int fd, const intel_ctx_t *ctx,
-	  unsigned ring, int num_children, int timeout)
+	  unsigned ring, int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 
@@ -155,15 +166,13 @@ sync_ring(int fd, const intel_ctx_t *ctx,
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object;
 		struct drm_i915_gem_execbuffer2 execbuf;
 		double start, elapsed;
 		unsigned long cycles;
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
-		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
+		object.handle = batch_create(fd, 4096, region);
 
 		memset(&execbuf, 0, sizeof(execbuf));
 		execbuf.buffers_ptr = to_user_pointer(&object);
@@ -193,9 +202,8 @@ sync_ring(int fd, const intel_ctx_t *ctx,
 
 static void
 idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
-	  int num_children, int timeout)
+	  int num_children, int timeout, uint32_t region)
 {
-	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	struct drm_i915_gem_exec_object2 object;
 	struct drm_i915_gem_execbuffer2 execbuf;
 	double start, elapsed;
@@ -204,8 +212,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
 	gem_require_ring(fd, ring);
 
 	memset(&object, 0, sizeof(object));
-	object.handle = gem_create(fd, 4096);
-	gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
+	object.handle = batch_create(fd, 4096, region);
 
 	memset(&execbuf, 0, sizeof(execbuf));
 	execbuf.buffers_ptr = to_user_pointer(&object);
@@ -234,7 +241,7 @@ idle_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
 
 static void
 wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
-	    int timeout, int wlen)
+	    int timeout, int wlen, uint32_t region)
 {
 	struct intel_engine_data ied;
 
@@ -243,7 +250,6 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, ied.nengines) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object;
 		struct drm_i915_gem_execbuffer2 execbuf;
 		double end, this, elapsed, now, baseline;
@@ -251,8 +257,7 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 		igt_spin_t *spin;
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
-		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
+		object.handle = batch_create(fd, 4096, region);
 
 		memset(&execbuf, 0, sizeof(execbuf));
 		execbuf.buffers_ptr = to_user_pointer(&object);
@@ -329,7 +334,7 @@ wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 }
 
 static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
-			int num_children, int timeout)
+			int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 
@@ -381,7 +386,7 @@ static void active_ring(int fd, const intel_ctx_t *ctx, unsigned int ring,
 
 static void
 active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
-		   int timeout, int wlen)
+		   int timeout, int wlen, uint32_t region)
 {
 	struct intel_engine_data ied;
 
@@ -390,7 +395,6 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, ied.nengines) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object;
 		struct drm_i915_gem_execbuffer2 execbuf;
 		double end, this, elapsed, now, baseline;
@@ -398,8 +402,7 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 		igt_spin_t *spin[2];
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
-		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
+		object.handle = batch_create(fd, 4096, region);
 
 		memset(&execbuf, 0, sizeof(execbuf));
 		execbuf.buffers_ptr = to_user_pointer(&object);
@@ -497,7 +500,7 @@ active_wakeup_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 static void
 store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
-	   int num_children, int timeout)
+	   int num_children, int timeout, uint32_t region)
 {
 	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
 	struct intel_engine_data ied;
@@ -508,7 +511,6 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object[2];
 		struct drm_i915_gem_relocation_entry reloc[1024];
 		struct drm_i915_gem_execbuffer2 execbuf;
@@ -526,8 +528,7 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 		execbuf.rsvd1 = ctx->id;
 
 		memset(object, 0, sizeof(object));
-		object[0].handle = gem_create(fd, 4096);
-		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
+		object[0].handle = batch_create(fd, 4096, region);
 		execbuf.buffer_count = 1;
 		gem_execbuf(fd, &execbuf);
 
@@ -595,7 +596,7 @@ store_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 static void
 switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
-	    int num_children, int timeout)
+	    int num_children, int timeout, uint32_t region)
 {
 	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
 	struct intel_engine_data ied;
@@ -618,7 +619,6 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 		unsigned long cycles;
 
 		for (int i = 0; i < ARRAY_SIZE(contexts); i++) {
-			const uint32_t bbe = MI_BATCH_BUFFER_END;
 			const uint32_t sz = 32 << 10;
 			struct context *c = &contexts[i];
 			uint32_t *batch, *b;
@@ -635,8 +635,7 @@ switch_ring(int fd, const intel_ctx_t *ctx, unsigned ring,
 			c->execbuf.rsvd1 = c->ctx->id;
 
 			memset(c->object, 0, sizeof(c->object));
-			c->object[0].handle = gem_create(fd, 4096);
-			gem_write(fd, c->object[0].handle, 0, &bbe, sizeof(bbe));
+			c->object[0].handle = batch_create(fd, 4096, region);
 			c->execbuf.buffer_count = 1;
 			gem_execbuf(fd, &c->execbuf);
 
@@ -778,10 +777,9 @@ static void *waiter(void *arg)
 
 static void
 __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
-	     int timeout, unsigned long *cycles)
+	     int timeout, unsigned long *cycles, uint32_t region)
 {
 	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
-	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	struct drm_i915_gem_exec_object2 object[2];
 	struct drm_i915_gem_execbuffer2 execbuf;
 	struct drm_i915_gem_relocation_entry reloc[1024];
@@ -800,8 +798,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
 	execbuf.rsvd1 = ctx->id;
 
 	memset(object, 0, sizeof(object));
-	object[0].handle = gem_create(fd, 4096);
-	gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
+	object[0].handle = batch_create(fd, 4096, region);
 	execbuf.buffer_count = 1;
 	gem_execbuf(fd, &execbuf);
 	object[0].flags |= EXEC_OBJECT_WRITE;
@@ -908,7 +905,7 @@ __store_many(int fd, const intel_ctx_t *ctx, unsigned ring,
 
 static void
 store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
-	   int num_children, int timeout)
+	   int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 	unsigned long *shared;
@@ -926,7 +923,8 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
 			__store_many(fd, ctx,
 				     ied_flags(&ied, n),
 				     timeout,
-				     &shared[n]);
+				     &shared[n],
+				     region);
 	}
 	igt_waitchildren();
 
@@ -939,7 +937,7 @@ store_many(int fd, const intel_ctx_t *ctx, unsigned int ring,
 }
 
 static void
-sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
+sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
 {
 	struct intel_engine_data ied;
 
@@ -948,15 +946,13 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object;
 		struct drm_i915_gem_execbuffer2 execbuf;
 		double start, elapsed;
 		unsigned long cycles;
 
 		memset(&object, 0, sizeof(object));
-		object.handle = gem_create(fd, 4096);
-		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
+		object.handle = batch_create(fd, 4096, region);
 
 		memset(&execbuf, 0, sizeof(execbuf));
 		execbuf.buffers_ptr = to_user_pointer(&object);
@@ -986,7 +982,7 @@ sync_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 }
 
 static void
-store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
+store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout, uint32_t region)
 {
 	const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
 	struct intel_engine_data ied;
@@ -996,7 +992,6 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {
-		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object[2];
 		struct drm_i915_gem_relocation_entry reloc[1024];
 		struct drm_i915_gem_execbuffer2 execbuf;
@@ -1013,8 +1008,7 @@ store_all(int fd, const intel_ctx_t *ctx, int num_children, int timeout)
 		execbuf.rsvd1 = ctx->id;
 
 		memset(object, 0, sizeof(object));
-		object[0].handle = gem_create(fd, 4096);
-		gem_write(fd, object[0].handle, 0, &bbe, sizeof(bbe));
+		object[0].handle = batch_create(fd, 4096, region);
 		execbuf.buffer_count = 1;
 		gem_execbuf(fd, &execbuf);
 
@@ -1156,7 +1150,7 @@ igt_main
 	const struct {
 		const char *name;
 		void (*func)(int fd, const intel_ctx_t *ctx, unsigned int engine,
-			     int num_children, int timeout);
+			     int num_children, int timeout, uint32_t memregion);
 		int num_children;
 		int timeout;
 	} all[] = {
@@ -1192,6 +1186,10 @@ igt_main
 	const struct intel_execution_engine2 *e;
 	const intel_ctx_t *ctx;
 	int fd = -1;
+	struct drm_i915_query_memory_regions *query_info;
+	struct igt_collection *regions, *set;
+	char *sub_name;
+	uint32_t region;
 
 	igt_fixture {
 		fd = drm_open_driver(DRIVER_INTEL);
@@ -1201,47 +1199,121 @@ igt_main
 		ctx = intel_ctx_create_all_physical(fd);
 
 		igt_fork_hang_detector(fd);
+		query_info = gem_get_query_memory_regions(fd);
+		igt_assert(query_info);
+		set = get_memory_region_set(query_info,
+					I915_SYSTEM_MEMORY,
+					I915_DEVICE_MEMORY);
+
 	}
 
 	/* Legacy for selecting rings. */
 	for_each_test(t, individual) {
 		igt_subtest_with_dynamic_f("%s", t->name) {
 			for (const struct intel_execution_ring *l = intel_execution_rings; l->name; l++) {
-				igt_dynamic_f("%s", l->name) {
-					t->func(fd, intel_ctx_0(fd), eb_ring(l),
-						t->num_children, t->timeout);
+				for_each_combination(regions, 1, set) {
+					sub_name = memregion_dynamic_subtest_name(regions);
+					region = igt_collection_get_value(regions, 0);
+					igt_dynamic_f("%s", l->name) {
+						t->func(fd, intel_ctx_0(fd), eb_ring(l),
+								t->num_children, 
+								t->timeout,
+								region);
+					}
+					free(sub_name);
 				}
 			}
 		}
 	}
 
-	igt_subtest("basic-all")
-		sync_all(fd, ctx, 1, 2);
-	igt_subtest("basic-store-all")
-		store_all(fd, ctx, 1, 2);
+	igt_subtest_with_dynamic("basic-all") {
+		for_each_combination(regions, 1, set) {
+			sub_name = memregion_dynamic_subtest_name(regions);
+			region = igt_collection_get_value(regions, 0);
+			igt_dynamic_f("basic-all-%s", sub_name)
+				sync_all(fd, ctx, 1, 2, region);
+			free(sub_name);
+		}
+	}
+
+	igt_subtest_with_dynamic("basic-store-all") {
+		for_each_combination(regions, 1, set) {
+			sub_name = memregion_dynamic_subtest_name(regions);
+			region = igt_collection_get_value(regions, 0);
+			igt_dynamic_f("basic-store-all-%s", sub_name)
+				store_all(fd, ctx, 1, 2, region);
+			free(sub_name);
+		}
+	}
+
+	igt_subtest_with_dynamic("all") {
+		for_each_combination(regions, 1, set) {
+			sub_name = memregion_dynamic_subtest_name(regions);
+			region = igt_collection_get_value(regions, 0);
+			igt_dynamic_f("all-%s", sub_name)
+				sync_all(fd, ctx, 1, 20, region);
+			free(sub_name);
+		}
+	}
+
+	igt_subtest_with_dynamic("store-all") {
+		for_each_combination(regions, 1, set) {
+			sub_name = memregion_dynamic_subtest_name(regions);
+			region = igt_collection_get_value(regions, 0);
+			igt_dynamic_f("store-all-%s", sub_name)
+				store_all(fd, ctx, 1, 20, region);
+			free(sub_name);
+		}
+	}
 
-	igt_subtest("all")
-		sync_all(fd, ctx, 1, 20);
-	igt_subtest("store-all")
-		store_all(fd, ctx, 1, 20);
-	igt_subtest("forked-all")
-		sync_all(fd, ctx, ncpus, 20);
-	igt_subtest("forked-store-all")
-		store_all(fd, ctx, ncpus, 20);
+	igt_subtest_with_dynamic("forked-all") {
+		for_each_combination(regions, 1, set) {
+			sub_name = memregion_dynamic_subtest_name(regions);
+			region = igt_collection_get_value(regions, 0);
+			igt_dynamic_f("forked-all-%s", sub_name)
+				sync_all(fd, ctx, ncpus, 20, region);
+			free(sub_name);
+		}
+	}
+
+	igt_subtest_with_dynamic("forked-store-all") {
+		for_each_combination(regions, 1, set) {
+			sub_name = memregion_dynamic_subtest_name(regions);
+			region = igt_collection_get_value(regions, 0);
+			igt_dynamic_f("forked-store-all-%s", sub_name)
+				store_all(fd, ctx, ncpus, 20, region);
+			free(sub_name);
+		}
+	}
 
 	for_each_test(t, all) {
-		igt_subtest_f("%s", t->name)
-			t->func(fd, ctx, ALL_ENGINES, t->num_children, t->timeout);
+		igt_subtest_with_dynamic_f("%s", t->name) {
+			for_each_combination(regions, 1, set) {
+				sub_name = memregion_dynamic_subtest_name(regions);
+				region = igt_collection_get_value(regions, 0);
+				igt_dynamic_f("%s", sub_name)
+					t->func(fd, ctx, ALL_ENGINES, t->num_children,
+							t->timeout,
+							region);
+				free(sub_name);
+			}
+		}
 	}
 
 	/* New way of selecting engines. */
 	for_each_test(t, individual) {
 		igt_subtest_with_dynamic_f("%s", t->name) {
-			for_each_ctx_engine(fd, ctx, e) {
-				igt_dynamic_f("%s", e->name) {
-					t->func(fd, ctx, e->flags,
-						t->num_children, t->timeout);
-				}
+			for_each_combination(regions, 1, set) {
+				sub_name = memregion_dynamic_subtest_name(regions);
+				region = igt_collection_get_value(regions, 0);
+				for_each_ctx_engine(fd, ctx, e)
+					igt_dynamic_f("%s-%s", e->name, sub_name)
+						t->func(fd, ctx, e->flags,
+								t->num_children,
+								t->timeout,
+								region);
+				free(sub_name);
+
 			}
 		}
 	}
@@ -1264,6 +1336,8 @@ igt_main
 	}
 
 	igt_fixture {
+		free(query_info);
+		igt_collection_destroy(set);
 		igt_stop_hang_detector();
 		intel_ctx_destroy(fd, ctx);
 		close(fd);
-- 
2.32.0

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

end of thread, other threads:[~2022-02-15 12:07 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-11 16:56 [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory sai.gowtham.ch
2022-01-11 17:38 ` [igt-dev] ✗ Fi.CI.BAT: failure for tests/i915/gem_sync: Add support for local memory (rev3) Patchwork
2022-01-11 17:57 ` [igt-dev] ✗ GitLab.Pipeline: warning " Patchwork
2022-01-12 17:47 ` [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory Zbigniew Kempczyński
  -- strict thread matches above, loose matches on Subject: below --
2022-02-03  5:25 sai.gowtham.ch
2022-02-15 12:07 ` Zbigniew Kempczyński
2021-12-24 20:03 sai.gowtham.ch
2022-01-03 11:27 ` Zbigniew Kempczyński
2021-10-05  5:14 sai.gowtham.ch
2021-10-05 10:11 ` Zbigniew Kempczyński
2021-10-05 10:31 ` Petri Latvala

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.