From: "Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>
To: sai.gowtham.ch@intel.com
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory
Date: Mon, 3 Jan 2022 12:27:40 +0100 [thread overview]
Message-ID: <YdLdrGVVsSySTuee@zkempczy-mobl2> (raw)
In-Reply-To: <20211224200310.10340-1-sai.gowtham.ch@intel.com>
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
>
next prev parent reply other threads:[~2022-01-03 11:27 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-24 20:03 [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory sai.gowtham.ch
2021-12-24 20:50 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/i915/gem_sync: Add support for local memory (rev2) Patchwork
2021-12-24 22:00 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
2022-01-03 11:27 ` Zbigniew Kempczyński [this message]
-- strict thread matches above, loose matches on Subject: below --
2022-02-03 5:25 [igt-dev] [PATCH i-g-t] tests/i915/gem_sync: Add support for local memory sai.gowtham.ch
2022-02-15 12:07 ` Zbigniew Kempczyński
2022-01-11 16:56 sai.gowtham.ch
2022-01-12 17:47 ` 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YdLdrGVVsSySTuee@zkempczy-mobl2 \
--to=zbigniew.kempczynski@intel.com \
--cc=igt-dev@lists.freedesktop.org \
--cc=sai.gowtham.ch@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.