[igt-dev] [PATCH i-g-t 18/93] tests/i915/gem_exec_fence: Move the engine data into inter_engine_context
Jason Ekstrand
jason at jlekstrand.net
Wed Jun 9 15:02:04 UTC 2021
On Wed, Jun 9, 2021 at 2:18 AM Zbigniew Kempczyński
<zbigniew.kempczynski at intel.com> wrote:
>
> On Tue, Jun 08, 2021 at 11:30:04PM -0500, Jason Ekstrand wrote:
> > This will make iteration easier when we switch to intel_ctx_t.
> >
> > Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
> > ---
> > tests/i915/gem_exec_fence.c | 79 +++++++++++++++++++------------------
> > 1 file changed, 40 insertions(+), 39 deletions(-)
> >
> > diff --git a/tests/i915/gem_exec_fence.c b/tests/i915/gem_exec_fence.c
> > index c3a650d8..7e80b0aa 100644
> > --- a/tests/i915/gem_exec_fence.c
> > +++ b/tests/i915/gem_exec_fence.c
> > @@ -2347,7 +2347,7 @@ struct inter_engine_context {
> > uint32_t context;
> > } iterations[9];
> >
> > - struct intel_engine_data *engines;
> > + struct intel_engine_data engines;
> >
> > struct inter_engine_batches {
> > void *increment_bb;
> > @@ -2415,7 +2415,7 @@ static void submit_timeline_execbuf(struct inter_engine_context *context,
> > execbuf->cliprects_ptr = to_user_pointer(&fence_list);
> > }
> >
> > - execbuf->flags |= context->engines->engines[run_engine_idx].flags;
> > + execbuf->flags |= context->engines.engines[run_engine_idx].flags;
> >
> > gem_execbuf(context->fd, execbuf);
> > }
> > @@ -2660,12 +2660,12 @@ get_cs_timestamp_frequency(int fd)
> > igt_skip("Kernel with PARAM_CS_TIMESTAMP_FREQUENCY support required\n");
> > }
> >
> > -static void setup_timeline_chain_engines(struct inter_engine_context *context, int fd, struct intel_engine_data *engines)
> > +static void setup_timeline_chain_engines(struct inter_engine_context *context, int fd)
> > {
> > memset(context, 0, sizeof(*context));
> >
> > context->fd = fd;
> > - context->engines = engines;
> > + context->engines = intel_init_engine_list(fd, 0);
>
> Original code verifies there's nengines > 0. I would keep this igt_require().
I'm not seeing that in the diff or the code. Mind pointing me at it?
In any case, I'm fine with the igt_require(); it should always be true
anyway.
> With that minor nit:
>
> Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
Thanks!
> --
> Zbigniew
>
> >
> > context->wait_context = gem_context_clone_with_engines(fd, 0);
> > context->wait_timeline = syncobj_create(fd, 0);
> > @@ -2685,15 +2685,16 @@ static void setup_timeline_chain_engines(struct inter_engine_context *context, i
> > gem_write(fd, context->wait_bb_handle, 0,
> > context->wait_bb, context->wait_bb_len);
> >
> > - context->batches = calloc(engines->nengines, sizeof(*context->batches));
> > - for (uint32_t e = 0; e < engines->nengines; e++) {
> > + context->batches = calloc(context->engines.nengines,
> > + sizeof(*context->batches));
> > + for (uint32_t e = 0; e < context->engines.nengines; e++) {
> > struct inter_engine_batches *batches = &context->batches[e];
> >
> > batches->timeline = syncobj_create(fd, 0);
> >
> > build_increment_engine_bb(
> > batches,
> > - gem_engine_mmio_base(fd, engines->engines[e].name));
> > + gem_engine_mmio_base(fd, context->engines.engines[e].name));
> > batches->increment_bb_handle = gem_create(fd, 4096);
> > gem_write(fd, batches->increment_bb_handle, 0,
> > batches->increment_bb, batches->increment_bb_len);
> > @@ -2706,7 +2707,7 @@ static void setup_timeline_chain_engines(struct inter_engine_context *context, i
> > {
> > uint64_t dword = 1;
> > gem_write(fd, context->engine_counter_object.handle,
> > - sizeof(dword) * (context->engines->nengines - 1),
> > + sizeof(dword) * (context->engines.nengines - 1),
> > &dword, sizeof(dword));
> > }
> > }
> > @@ -2724,7 +2725,7 @@ static void teardown_timeline_chain_engines(struct inter_engine_context *context
> > gem_close(context->fd, context->wait_bb_handle);
> > free(context->wait_bb);
> >
> > - for (uint32_t e = 0; e < context->engines->nengines; e++) {
> > + for (uint32_t e = 0; e < context->engines.nengines; e++) {
> > struct inter_engine_batches *batches = &context->batches[e];
> >
> > syncobj_destroy(context->fd, batches->timeline);
> > @@ -2734,12 +2735,12 @@ static void teardown_timeline_chain_engines(struct inter_engine_context *context
> > free(context->batches);
> > }
> >
> > -static void test_syncobj_timeline_chain_engines(int fd, struct intel_engine_data *engines)
> > +static void test_syncobj_timeline_chain_engines(int fd)
> > {
> > struct inter_engine_context ctx;
> > uint64_t *counter_output;
> >
> > - setup_timeline_chain_engines(&ctx, fd, engines);
> > + setup_timeline_chain_engines(&ctx, fd);
> >
> > /*
> > * Delay all the other operations by making them depend on an
> > @@ -2748,11 +2749,11 @@ static void test_syncobj_timeline_chain_engines(int fd, struct intel_engine_data
> > wait_engine(&ctx, 0, ctx.wait_timeline, 1);
> >
> > for (uint32_t iter = 0; iter < ARRAY_SIZE(ctx.iterations); iter++) {
> > - for (uint32_t engine = 0; engine < engines->nengines; engine++) {
> > + for (uint32_t engine = 0; engine < ctx.engines.nengines; engine++) {
> > uint32_t prev_prev_engine =
> > - (engines->nengines + engine - 2) % engines->nengines;
> > + (ctx.engines.nengines + engine - 2) % ctx.engines.nengines;
> > uint32_t prev_engine =
> > - (engines->nengines + engine - 1) % engines->nengines;
> > + (ctx.engines.nengines + engine - 1) % ctx.engines.nengines;
> > /*
> > * Pick up the wait engine semaphore for the
> > * first increment, then pick up the previous
> > @@ -2778,28 +2779,28 @@ static void test_syncobj_timeline_chain_engines(int fd, struct intel_engine_data
> >
> > counter_output = gem_mmap__wc(fd, ctx.engine_counter_object.handle, 0, 4096, PROT_READ);
> >
> > - for (uint32_t i = 0; i < ctx.engines->nengines; i++)
> > + for (uint32_t i = 0; i < ctx.engines.nengines; i++)
> > igt_debug("engine %i (%s)\t= %016"PRIx64"\n", i,
> > - ctx.engines->engines[i].name, counter_output[i]);
> > + ctx.engines.engines[i].name, counter_output[i]);
> >
> > /*
> > * Verify that we get the fibonacci number expected (we start
> > * at the sequence on the second number : 1).
> > */
> > - igt_assert_eq(counter_output[engines->nengines - 1],
> > - fib(ARRAY_SIZE(ctx.iterations) * engines->nengines + 1));
> > + igt_assert_eq(counter_output[ctx.engines.nengines - 1],
> > + fib(ARRAY_SIZE(ctx.iterations) * ctx.engines.nengines + 1));
> >
> > munmap(counter_output, 4096);
> >
> > teardown_timeline_chain_engines(&ctx);
> > }
> >
> > -static void test_syncobj_stationary_timeline_chain_engines(int fd, struct intel_engine_data *engines)
> > +static void test_syncobj_stationary_timeline_chain_engines(int fd)
> > {
> > struct inter_engine_context ctx;
> > uint64_t *counter_output;
> >
> > - setup_timeline_chain_engines(&ctx, fd, engines);
> > + setup_timeline_chain_engines(&ctx, fd);
> >
> > /*
> > * Delay all the other operations by making them depend on an
> > @@ -2808,11 +2809,11 @@ static void test_syncobj_stationary_timeline_chain_engines(int fd, struct intel_
> > wait_engine(&ctx, 0, ctx.wait_timeline, 1);
> >
> > for (uint32_t iter = 0; iter < ARRAY_SIZE(ctx.iterations); iter++) {
> > - for (uint32_t engine = 0; engine < engines->nengines; engine++) {
> > + for (uint32_t engine = 0; engine < ctx.engines.nengines; engine++) {
> > uint32_t prev_prev_engine =
> > - (engines->nengines + engine - 2) % engines->nengines;
> > + (ctx.engines.nengines + engine - 2) % ctx.engines.nengines;
> > uint32_t prev_engine =
> > - (engines->nengines + engine - 1) % engines->nengines;
> > + (ctx.engines.nengines + engine - 1) % ctx.engines.nengines;
> > /*
> > * Pick up the wait engine semaphore for the
> > * first increment, then pick up the previous
> > @@ -2844,23 +2845,23 @@ static void test_syncobj_stationary_timeline_chain_engines(int fd, struct intel_
> >
> > counter_output = gem_mmap__wc(fd, ctx.engine_counter_object.handle, 0, 4096, PROT_READ);
> >
> > - for (uint32_t i = 0; i < ctx.engines->nengines; i++)
> > + for (uint32_t i = 0; i < ctx.engines.nengines; i++)
> > igt_debug("engine %i (%s)\t= %016"PRIx64"\n", i,
> > - ctx.engines->engines[i].name, counter_output[i]);
> > - igt_assert_eq(counter_output[engines->nengines - 1],
> > - fib(ARRAY_SIZE(ctx.iterations) * engines->nengines + 1));
> > + ctx.engines.engines[i].name, counter_output[i]);
> > + igt_assert_eq(counter_output[ctx.engines.nengines - 1],
> > + fib(ARRAY_SIZE(ctx.iterations) * ctx.engines.nengines + 1));
> >
> > munmap(counter_output, 4096);
> >
> > teardown_timeline_chain_engines(&ctx);
> > }
> >
> > -static void test_syncobj_backward_timeline_chain_engines(int fd, struct intel_engine_data *engines)
> > +static void test_syncobj_backward_timeline_chain_engines(int fd)
> > {
> > struct inter_engine_context ctx;
> > uint64_t *counter_output;
> >
> > - setup_timeline_chain_engines(&ctx, fd, engines);
> > + setup_timeline_chain_engines(&ctx, fd);
> >
> > /*
> > * Delay all the other operations by making them depend on an
> > @@ -2869,11 +2870,11 @@ static void test_syncobj_backward_timeline_chain_engines(int fd, struct intel_en
> > wait_engine(&ctx, 0, ctx.wait_timeline, 1);
> >
> > for (uint32_t iter = 0; iter < ARRAY_SIZE(ctx.iterations); iter++) {
> > - for (uint32_t engine = 0; engine < engines->nengines; engine++) {
> > + for (uint32_t engine = 0; engine < ctx.engines.nengines; engine++) {
> > uint32_t prev_prev_engine =
> > - (engines->nengines + engine - 2) % engines->nengines;
> > + (ctx.engines.nengines + engine - 2) % ctx.engines.nengines;
> > uint32_t prev_engine =
> > - (engines->nengines + engine - 1) % engines->nengines;
> > + (ctx.engines.nengines + engine - 1) % ctx.engines.nengines;
> > /*
> > * Pick up the wait engine semaphore for the
> > * first increment, then pick up the previous
> > @@ -2905,11 +2906,11 @@ static void test_syncobj_backward_timeline_chain_engines(int fd, struct intel_en
> >
> > counter_output = gem_mmap__wc(fd, ctx.engine_counter_object.handle, 0, 4096, PROT_READ);
> >
> > - for (uint32_t i = 0; i < ctx.engines->nengines; i++)
> > + for (uint32_t i = 0; i < ctx.engines.nengines; i++)
> > igt_debug("engine %i (%s)\t= %016"PRIx64"\n", i,
> > - ctx.engines->engines[i].name, counter_output[i]);
> > - igt_assert_eq(counter_output[engines->nengines - 1],
> > - fib(ARRAY_SIZE(ctx.iterations) * engines->nengines + 1));
> > + ctx.engines.engines[i].name, counter_output[i]);
> > + igt_assert_eq(counter_output[ctx.engines.nengines - 1],
> > + fib(ARRAY_SIZE(ctx.iterations) * ctx.engines.nengines + 1));
> >
> > munmap(counter_output, 4096);
> >
> > @@ -3217,13 +3218,13 @@ igt_main
> > }
> >
> > igt_subtest("syncobj-timeline-chain-engines")
> > - test_syncobj_timeline_chain_engines(i915, &engines);
> > + test_syncobj_timeline_chain_engines(i915);
> >
> > igt_subtest("syncobj-stationary-timeline-chain-engines")
> > - test_syncobj_stationary_timeline_chain_engines(i915, &engines);
> > + test_syncobj_stationary_timeline_chain_engines(i915);
> >
> > igt_subtest("syncobj-backward-timeline-chain-engines")
> > - test_syncobj_backward_timeline_chain_engines(i915, &engines);
> > + test_syncobj_backward_timeline_chain_engines(i915);
> > }
> >
> > igt_fixture {
> > --
> > 2.31.1
> >
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
More information about the igt-dev
mailing list