[igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory

Zbigniew Kempczyński zbigniew.kempczynski at intel.com
Mon Jan 3 11:27:40 UTC 2022


On Sat, Dec 25, 2021 at 01:33:10AM +0530, sai.gowtham.ch at intel.com wrote:
> From: Ch Sai Gowtham <sai.gowtham.ch at intel.com>
> 
> Add support for local memory region (Device memory)
> 
> Signed-off-by: Ch Sai Gowtham <sai.gowtham.ch at intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski at 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
> 


More information about the igt-dev mailing list