From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id D0B8410E130 for ; Mon, 3 Jan 2022 11:27:44 +0000 (UTC) Date: Mon, 3 Jan 2022 12:27:40 +0100 From: Zbigniew =?utf-8?Q?Kempczy=C5=84ski?= Message-ID: References: <20211224200310.10340-1-sai.gowtham.ch@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20211224200310.10340-1-sai.gowtham.ch@intel.com> Subject: Re: [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: sai.gowtham.ch@intel.com Cc: igt-dev@lists.freedesktop.org List-ID: On Sat, Dec 25, 2021 at 01:33:10AM +0530, sai.gowtham.ch@intel.com wrote: > From: Ch Sai Gowtham > > Add support for local memory region (Device memory) > > Signed-off-by: Ch Sai Gowtham > Cc: Zbigniew Kempczyński > --- > 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 >