Quoting Tvrtko Ursulin (2020-01-30 20:41:24) > From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > Converts all per-engine tests into dynamic subtests and in the process: > > * Put back I915_EXEC_BSD legacy coverage. > * Remove one added static engine list usage. > * Compact code by driving two groups of the name/func table. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > Cc: Bommu Krishnaiah <krishnaiah.bommu@xxxxxxxxx> > --- > tests/i915/gem_ctx_persistence.c | 97 +++++++++++++------------------- > 1 file changed, 39 insertions(+), 58 deletions(-) > > diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c > index 8b9633b214ff..0d5b22d2b162 100644 > --- a/tests/i915/gem_ctx_persistence.c > +++ b/tests/i915/gem_ctx_persistence.c > @@ -759,7 +759,20 @@ static void smoketest(int i915) > > igt_main > { > - const struct intel_execution_engine2 *e; > + struct { > + const char *name; > + void (*func)(int fd, unsigned int engine); > + } *test, tests[] = { > + { "persistence", test_persistence }, > + { "cleanup", test_nonpersistent_cleanup }, > + { "queued", test_nonpersistent_queued }, > + { "mixed", test_nonpersistent_mixed }, > + { "mixed-process", test_process_mixed }, > + { "hostile", test_nonpersistent_hostile }, > + { "hostile-preempt", test_nonpersistent_hostile_preempt }, > + { "hang", test_nonpersistent_hang }, > + { NULL, NULL }, Heh, that's what I was tempted to suggest but did want to seem like I was requesting too much :) > + }; > int i915; > > igt_fixture { > @@ -792,72 +805,40 @@ igt_main > igt_subtest("hang") > test_nohangcheck_hang(i915); > > - __for_each_static_engine(e) { > - igt_subtest_group { > - igt_fixture { > - gem_require_ring(i915, e->flags); > - gem_require_contexts(i915); > - } > - > - igt_subtest_f("legacy-%s-persistence", e->name) > - test_persistence(i915, e->flags); > - > - igt_subtest_f("legacy-%s-cleanup", e->name) > - test_nonpersistent_cleanup(i915, e->flags); > - > - igt_subtest_f("legacy-%s-queued", e->name) > - test_nonpersistent_queued(i915, e->flags); > - > - igt_subtest_f("legacy-%s-mixed", e->name) > - test_nonpersistent_mixed(i915, e->flags); > - > - igt_subtest_f("legacy-%s-mixed-process", e->name) > - test_process_mixed(i915, e->flags); > - > - igt_subtest_f("legacy-%s-hostile", e->name) > - test_nonpersistent_hostile(i915, e->flags); > + igt_subtest("smoketest") > + smoketest(i915); > > - igt_subtest_f("legacy-%s-hostile-preempt", e->name) > - test_nonpersistent_hostile_preempt(i915, > - e->flags); > + igt_subtest_group { > + igt_fixture > + gem_require_contexts(i915); > + > + for (test = tests; test->name; test++) { > + igt_subtest_with_dynamic_f("legacy-engines-%s", > + test->name) { > + for_each_engine(e, i915) { Don't we need a if (!gem_has_ring(i915, eb_ring(e)) continue; here? > + igt_dynamic_f("%s", e->name) > + test->func(i915, eb_ring(e)); > + } > + } > } > } > > - __for_each_physical_engine(i915, e) { > - igt_subtest_group { > - igt_fixture > - gem_require_contexts(i915); > - > - igt_subtest_f("%s-persistence", e->name) > - test_persistence(i915, e->flags); > - > - igt_subtest_f("%s-cleanup", e->name) > - test_nonpersistent_cleanup(i915, e->flags); > - > - igt_subtest_f("%s-queued", e->name) > - test_nonpersistent_queued(i915, e->flags); > - > - igt_subtest_f("%s-mixed", e->name) > - test_nonpersistent_mixed(i915, e->flags); > - > - igt_subtest_f("%s-mixed-process", e->name) > - test_process_mixed(i915, e->flags); > + igt_subtest_group { > + const struct intel_execution_engine2 *e; > > - igt_subtest_f("%s-hostile", e->name) > - test_nonpersistent_hostile(i915, e->flags); > + igt_fixture > + gem_require_contexts(i915); > > - igt_subtest_f("%s-hostile-preempt", e->name) > - test_nonpersistent_hostile_preempt(i915, > - e->flags); > - > - igt_subtest_f("%s-hang", e->name) > - test_nonpersistent_hang(i915, e->flags); > + for (test = tests; test->name; test++) { > + igt_subtest_with_dynamic_f("engines-%s", test->name) { > + __for_each_physical_engine(i915, e) { > + igt_dynamic_f("%s", e->name) > + test->func(i915, e->flags); > + } > + } > } Then I had a plan to add the pathological engines[] here. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx