Re: [igt-dev] [PATCH] [PATCH i-g-t][V7]tests/i915/gem_sync.c :Added __for_each_physical_engine to utilize all available

["Melkaveri, Arjun" <arjun.melkaveri@intel.com>]

On Wed, Apr 22, 2020 at 02:13:44PM +0100, Tvrtko Ursulin wrote:
> 
> On 14/04/2020 18:51, Arjun Melkaveri wrote:
> > Replaced the legacy for_each_engine* defines with the ones
> > implemented in the gem_engine_topology library.
> > 
> > Used  gem_context_clone_with_engines
> > to make sure that engine index was potentially created
> > based on a  default context with engine map configured.
> > 
> > Added Legacy engine coverage for sync_ring and sync_all.
> > 
> > Divided tests into static for ALL_Engine test cases
> > and dynamic for multiple engine .
> > 
> > V7:
> > Initializing engine and engine name with
> > maximum supported engines value.
> 
> You lost the rest of the change log, not terribly important though.
> 
> > 
> > Cc: Dec Katarzyna <katarzyna.dec@intel.com>
> > Cc: Ursulin Tvrtko <tvrtko.ursulin@intel.com>
> > Signed-off-by: sai gowtham <sai.gowtham.ch@intel.com>
> > Signed-off-by: Arjun Melkaveri <arjun.melkaveri@intel.com>
> > ---
> >   tests/i915/gem_sync.c | 310 ++++++++++++++++++++++++++----------------
> >   1 file changed, 195 insertions(+), 115 deletions(-)
> > 
> > diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c
> > index 2ef55ecc..f1a87f59 100644
> > --- a/tests/i915/gem_sync.c
> > +++ b/tests/i915/gem_sync.c
> > @@ -24,6 +24,9 @@
> >   #include <time.h>
> >   #include <pthread.h>
> > +#include "igt_debugfs.h"
> > +#include "igt_dummyload.h"
> > +#include "igt_gt.h"
> >   #include "igt.h"
> >   #include "igt_sysfs.h"
> > @@ -37,6 +40,10 @@
> >   #define MIN_PRIO LOCAL_I915_CONTEXT_MIN_USER_PRIORITY
> >   #define ENGINE_MASK  (I915_EXEC_RING_MASK | LOCAL_I915_EXEC_BSD_MASK)
> > +#define RESET_TIMEOUT_MS (2 * MSEC_PER_SEC)
> > +static unsigned long reset_timeout_ms = RESET_TIMEOUT_MS;
> 
> I've asked in one of the previous rounds what's this for? It's only set in
> do_test and never acted upon.
> 
Will modify this in next patch
> > +#define NSEC_PER_MSEC (1000 * 1000ull)
> 
> Unused.
>
Will remove this 
> > +#define EXECBUF_MAX_ENGINES (I915_EXEC_RING_MASK + 1)
> >   IGT_TEST_DESCRIPTION("Basic check of ring<->ring write synchronisation.");
> > @@ -78,24 +85,35 @@ out:
> >   	return ts.tv_sec + 1e-9*ts.tv_nsec;
> >   }
> > +static void cleanup(int i915)
> > +{
> > +	igt_drop_caches_set(i915,
> > +			    /* cancel everything */
> > +			    DROP_RESET_ACTIVE | DROP_RESET_SEQNO |
> > +			    /* cleanup */
> > +			    DROP_ACTIVE | DROP_RETIRE | DROP_IDLE | DROP_FREED);
> > +	igt_require_gem(i915);
> > +}
> > +
> > +
> >   static void
> >   sync_ring(int fd, unsigned ring, int num_children, int timeout)
> >   {
> > -	unsigned engines[16];
> > -	const char *names[16];
> > +	const struct intel_execution_engine2 *e2;
> > +	unsigned int engines[EXECBUF_MAX_ENGINES];
> > +	const char *names[EXECBUF_MAX_ENGINES];
> >   	int num_engines = 0;
> >   	if (ring == ALL_ENGINES) {
> > -		for_each_physical_engine(e, fd) {
> > -			names[num_engines] = e->name;
> > -			engines[num_engines++] = eb_ring(e);
> > +		__for_each_physical_engine(fd, e2) {
> > +			names[num_engines] = strdup(e2->name);
> > +			engines[num_engines++] = e2->flags;
> >   			if (num_engines == ARRAY_SIZE(engines))
> >   				break;
> >   		}
> >   		num_children *= num_engines;
> >   	} else {
> > -		gem_require_ring(fd, ring);
> >   		names[num_engines] = NULL;
> >   		engines[num_engines++] = ring;
> >   	}
> > @@ -139,7 +157,7 @@ sync_ring(int fd, unsigned ring, int num_children, int timeout)
> >   }
> >   static void
> > -idle_ring(int fd, unsigned ring, int timeout)
> > +idle_ring(int fd, unsigned int ring, int num_children, int timeout)
> >   {
> >   	const uint32_t bbe = MI_BATCH_BUFFER_END;
> >   	struct drm_i915_gem_exec_object2 object;
> > @@ -180,23 +198,23 @@ idle_ring(int fd, unsigned ring, int timeout)
> >   static void
> >   wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
> >   {
> > -	unsigned engines[16];
> > -	const char *names[16];
> > +	const struct intel_execution_engine2 *e2;
> > +	unsigned int engines[EXECBUF_MAX_ENGINES];
> > +	const char *names[EXECBUF_MAX_ENGINES];
> >   	int num_engines = 0;
> >   	if (ring == ALL_ENGINES) {
> > -		for_each_physical_engine(e, fd) {
> > -			if (!gem_can_store_dword(fd, eb_ring(e)))
> > +		__for_each_physical_engine(fd, e2) {
> > +			if (!gem_class_can_store_dword(fd, e2->class))
> >   				continue;
> > -			names[num_engines] = e->name;
> > -			engines[num_engines++] = eb_ring(e);
> > +			names[num_engines] = e2->name;
> 
> Looks like you need strdup here as well.
> 
I am changing this to use intel_engine_data, to remove usage of local
variable. 
> > +			engines[num_engines++] = e2->flags;
> >   			if (num_engines == ARRAY_SIZE(engines))
> >   				break;
> >   		}
> >   		igt_require(num_engines);
> >   	} else {
> > -		gem_require_ring(fd, ring);
> >   		igt_require(gem_can_store_dword(fd, ring));
> >   		names[num_engines] = NULL;
> >   		engines[num_engines++] = ring;
> > @@ -290,25 +308,26 @@ wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
> >   	igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
> >   }
> > -static void active_ring(int fd, unsigned ring, int timeout)
> > +static void active_ring(int fd, unsigned int ring,
> > +			int num_children, int timeout)
> >   {
> > -	unsigned engines[16];
> > -	const char *names[16];
> > +	const struct intel_execution_engine2 *e2;
> > +	unsigned int engines[EXECBUF_MAX_ENGINES];
> > +	const char *names[EXECBUF_MAX_ENGINES];
> >   	int num_engines = 0;
> >   	if (ring == ALL_ENGINES) {
> > -		for_each_physical_engine(e, fd) {
> > -			if (!gem_can_store_dword(fd, eb_ring(e)))
> > +		__for_each_physical_engine(fd, e2) {
> > +			if (!gem_class_can_store_dword(fd, e2->class))
> >   				continue;
> > -			names[num_engines] = e->name;
> > -			engines[num_engines++] = eb_ring(e);
> > +			names[num_engines] = e2->name;
> 
> And here.
> 
> > +			engines[num_engines++] = e2->flags;
> >   			if (num_engines == ARRAY_SIZE(engines))
> >   				break;
> >   		}
> >   		igt_require(num_engines);
> >   	} else {
> > -		gem_require_ring(fd, ring);
> >   		igt_require(gem_can_store_dword(fd, ring));
> >   		names[num_engines] = NULL;
> >   		engines[num_engines++] = ring;
> > @@ -359,23 +378,23 @@ static void active_ring(int fd, unsigned ring, int timeout)
> >   static void
> >   active_wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
> >   {
> > -	unsigned engines[16];
> > -	const char *names[16];
> > +	const struct intel_execution_engine2 *e2;
> > +	unsigned int engines[EXECBUF_MAX_ENGINES];
> > +	const char *names[EXECBUF_MAX_ENGINES];
> >   	int num_engines = 0;
> >   	if (ring == ALL_ENGINES) {
> > -		for_each_physical_engine(e, fd) {
> > -			if (!gem_can_store_dword(fd, eb_ring(e)))
> > +		__for_each_physical_engine(fd, e2) {
> > +			if (!gem_class_can_store_dword(fd, e2->class))
> >   				continue;
> > -			names[num_engines] = e->name;
> > -			engines[num_engines++] = eb_ring(e);
> > +			names[num_engines] = e2->name;
> 
> And here.
> 
> > +			engines[num_engines++] = e2->flags;
> >   			if (num_engines == ARRAY_SIZE(engines))
> >   				break;
> >   		}
> >   		igt_require(num_engines);
> >   	} else {
> > -		gem_require_ring(fd, ring);
> >   		igt_require(gem_can_store_dword(fd, ring));
> >   		names[num_engines] = NULL;
> >   		engines[num_engines++] = ring;
> > @@ -493,25 +512,25 @@ active_wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
> >   static void
> >   store_ring(int fd, unsigned ring, int num_children, int timeout)
> >   {
> > +	const struct intel_execution_engine2 *e2;
> >   	const int gen = intel_gen(intel_get_drm_devid(fd));
> > -	unsigned engines[16];
> > -	const char *names[16];
> > +	unsigned int engines[EXECBUF_MAX_ENGINES];
> > +	const char *names[EXECBUF_MAX_ENGINES];
> >   	int num_engines = 0;
> >   	if (ring == ALL_ENGINES) {
> > -		for_each_physical_engine(e, fd) {
> > -			if (!gem_can_store_dword(fd, eb_ring(e)))
> > +		__for_each_physical_engine(fd, e2) {
> > +			if (!gem_class_can_store_dword(fd, e2->class))
> >   				continue;
> > -			names[num_engines] = e->name;
> > -			engines[num_engines++] = eb_ring(e);
> > +			names[num_engines] = e2->name;
> 
> And here.
> 
> > +			engines[num_engines++] = e2->flags;
> >   			if (num_engines == ARRAY_SIZE(engines))
> >   				break;
> >   		}
> >   		num_children *= num_engines;
> >   	} else {
> > -		gem_require_ring(fd, ring);
> >   		igt_require(gem_can_store_dword(fd, ring));
> >   		names[num_engines] = NULL;
> >   		engines[num_engines++] = ring;
> > @@ -608,27 +627,27 @@ store_ring(int fd, unsigned ring, int num_children, int timeout)
> >   static void
> >   switch_ring(int fd, unsigned ring, int num_children, int timeout)
> >   {
> > +	const struct intel_execution_engine2 *e2;
> >   	const int gen = intel_gen(intel_get_drm_devid(fd));
> > -	unsigned engines[16];
> > -	const char *names[16];
> > +	unsigned int engines[EXECBUF_MAX_ENGINES];
> > +	const char *names[EXECBUF_MAX_ENGINES];
> >   	int num_engines = 0;
> >   	gem_require_contexts(fd);
> >   	if (ring == ALL_ENGINES) {
> > -		for_each_physical_engine(e, fd) {
> > -			if (!gem_can_store_dword(fd, eb_ring(e)))
> > +		__for_each_physical_engine(fd, e2) {
> > +			if (!gem_class_can_store_dword(fd, e2->class))
> >   				continue;
> > -			names[num_engines] = e->name;
> > -			engines[num_engines++] = eb_ring(e);
> > +			names[num_engines] = e2->name;
> 
> Ditto.
> 
> > +			engines[num_engines++] = e2->flags;
> >   			if (num_engines == ARRAY_SIZE(engines))
> >   				break;
> >   		}
> >   		num_children *= num_engines;
> >   	} else {
> > -		gem_require_ring(fd, ring);
> >   		igt_require(gem_can_store_dword(fd, ring));
> >   		names[num_engines] = NULL;
> >   		engines[num_engines++] = ring;
> > @@ -931,10 +950,11 @@ __store_many(int fd, unsigned ring, int timeout, unsigned long *cycles)
> >   }
> >   static void
> > -store_many(int fd, unsigned ring, int timeout)
> > +store_many(int fd, unsigned int ring, int num_children, int timeout)
> >   {
> > +	const struct intel_execution_engine2 *e2;
> >   	unsigned long *shared;
> > -	const char *names[16];
> > +	const char *names[EXECBUF_MAX_ENGINES];
> >   	int n = 0;
> >   	shared = mmap(NULL, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0);
> > @@ -943,21 +963,20 @@ store_many(int fd, unsigned ring, int timeout)
> >   	intel_detect_and_clear_missed_interrupts(fd);
> >   	if (ring == ALL_ENGINES) {
> > -		for_each_physical_engine(e, fd) {
> > -			if (!gem_can_store_dword(fd, eb_ring(e)))
> > +		__for_each_physical_engine(fd, e2) {
> > +			if (!gem_class_can_store_dword(fd, e2->class))
> >   				continue;
> >   			igt_fork(child, 1)
> >   				__store_many(fd,
> > -					     eb_ring(e),
> > +					     e2->flags,
> >   					     timeout,
> >   					     &shared[n]);
> > -			names[n++] = e->name;
> > +			names[n++] = e2->name;
> 
> Ditto.
> 
> >   		}
> >   		igt_waitchildren();
> >   	} else {
> > -		gem_require_ring(fd, ring);
> >   		igt_require(gem_can_store_dword(fd, ring));
> >   		__store_many(fd, ring, timeout, &shared[n]);
> >   		names[n++] = NULL;
> > @@ -1025,15 +1044,16 @@ sync_all(int fd, int num_children, int timeout)
> >   static void
> >   store_all(int fd, int num_children, int timeout)
> >   {
> > +	const struct intel_execution_engine2 *e;
> >   	const int gen = intel_gen(intel_get_drm_devid(fd));
> > -	unsigned engines[16];
> > +	unsigned int engines[EXECBUF_MAX_ENGINES];
> >   	int num_engines = 0;
> > -	for_each_physical_engine(e, fd) {
> > -		if (!gem_can_store_dword(fd, eb_ring(e)))
> > +	__for_each_physical_engine(fd, e) {
> > +		if (!gem_class_can_store_dword(fd, e->class))
> >   			continue;
> > -		engines[num_engines++] = eb_ring(e);
> > +		engines[num_engines++] = e->flags;
> >   		if (num_engines == ARRAY_SIZE(engines))
> >   			break;
> >   	}
> > @@ -1132,22 +1152,22 @@ store_all(int fd, int num_children, int timeout)
> >   static void
> >   preempt(int fd, unsigned ring, int num_children, int timeout)
> >   {
> > -	unsigned engines[16];
> > -	const char *names[16];
> > +	const struct intel_execution_engine2 *e2;
> > +	unsigned int engines[EXECBUF_MAX_ENGINES];
> > +	const char *names[EXECBUF_MAX_ENGINES];
> >   	int num_engines = 0;
> >   	uint32_t ctx[2];
> >   	if (ring == ALL_ENGINES) {
> > -		for_each_physical_engine(e, fd) {
> > -			names[num_engines] = e->name;
> > -			engines[num_engines++] = eb_ring(e);
> > +		__for_each_physical_engine(fd, e2) {
> > +			names[num_engines] = e2->name;
> 
> Last one.
> 
> > +			engines[num_engines++] = e2->flags;
> >   			if (num_engines == ARRAY_SIZE(engines))
> >   				break;
> >   		}
> >   		num_children *= num_engines;
> >   	} else {
> > -		gem_require_ring(fd, ring);
> >   		names[num_engines] = NULL;
> >   		engines[num_engines++] = ring;
> >   	}
> > @@ -1207,76 +1227,99 @@ preempt(int fd, unsigned ring, int num_children, int timeout)
> >   	gem_context_destroy(fd, ctx[0]);
> >   }
> > +static void do_test(void (*test)(int i915, unsigned int engine,
> > +		    int num_children, int test_timeout),
> > +		    int i915, unsigned int engine, int num_children,
> > +		    int test_timeout, const char *name)
> > +{
> > +#define ATTR "preempt_timeout_ms"
> > +	int timeout = -1;
> > +
> > +	cleanup(i915);
> > +
> > +	gem_engine_property_scanf(i915, name, ATTR, "%d", &timeout);
> > +	if (timeout != -1) {
> > +		igt_require(gem_engine_property_printf(i915, name,
> > +						       ATTR, "%d", 50) > 0);
> > +		reset_timeout_ms = 200;
> > +	}
> > +
> > +	test(i915, engine, num_children, test_timeout);
> > +	gem_quiescent_gpu(i915);
> 
> For stuff done in this wrapper - I don't really like stuffing in changes
> unrelated to patch title. If it is converting engine iteration it should
> stick to that. If you want to make some other improvements it should be a
> separate patch.
> 
I was acting on the reviews comments . by following 28e25ad1f1f987450f017d7f99548d2d7727d388
commit . Will clean up do_test ,as needed for this test.
> > +}
> > +
> >   igt_main
> >   {
> > -	const struct intel_execution_engine *e;
> >   	const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> >   	int fd = -1;
> > +	struct {
> > +		const char *name;
> > +		void (*func)(int fd, unsigned int engine,
> > +			     int num_children, int timeout);
> > +		int num_children;
> > +		int timeout;
> > +	} *test, allengine[] = {
> > +		{ "basic-each", sync_ring, 1, 2},
> > +		{ "basic-store-each", store_ring, 1, 2},
> > +		{ "basic-many-each", store_many, 0, 2},
> > +		{ "switch-each", switch_ring, 1, 20},
> > +		{ "forked-switch-each", switch_ring, ncpus, 20},
> > +		{ "forked-each", sync_ring, ncpus, 20},
> > +		{ "forked-store-each", store_ring, ncpus, 20},
> > +		{ "active-each", active_ring, 0, 20},
> > +		{ "wakeup-each", wakeup_ring, 20, 1},
> > +		{ "active-wakeup-each", active_wakeup_ring, 20, 1},
> > +		{ "double-wakeup-each", wakeup_ring, 20, 2},
> > +		{ NULL, NULL },
> > +	}, tests[] = {
> > +		{ "default", sync_ring, 1, 20},
> > +		{ "idle", idle_ring, 0, 20},
> > +		{ "active", active_ring, 0, 20},
> > +		{ "wakeup", wakeup_ring, 20, 1},
> > +		{ "active-wakeup", active_wakeup_ring, 20, 1},
> > +		{ "double-wakeup", wakeup_ring, 20, 2},
> > +		{ "store", store_ring, 1, 20},
> > +		{ "switch", switch_ring, 1, 20},
> > +		{ "forked-switch", switch_ring, ncpus, 20},
> > +		{ "many", store_many, 0, 20},
> > +		{ "forked", sync_ring, ncpus, 20},
> > +		{ "forked-store", store_ring, ncpus, 20},
> > +		{ NULL, NULL },
> > +	};
> > +
> > +
> >   	igt_fixture {
> >   		fd = drm_open_driver(DRIVER_INTEL);
> >   		igt_require_gem(fd);
> >   		gem_submission_print_method(fd);
> >   		gem_scheduler_print_capability(fd);
> > -
> 
> Best to avoid noise.
> 
> >   		igt_fork_hang_detector(fd);
> >   	}
> > -	for (e = intel_execution_engines; e->name; e++) {
> > -		igt_subtest_f("%s", e->name)
> > -			sync_ring(fd, eb_ring(e), 1, 20);
> > -		igt_subtest_f("idle-%s", e->name)
> > -			idle_ring(fd, eb_ring(e), 20);
> > -		igt_subtest_f("active-%s", e->name)
> > -			active_ring(fd, eb_ring(e), 20);
> > -		igt_subtest_f("wakeup-%s", e->name)
> > -			wakeup_ring(fd, eb_ring(e), 20, 1);
> > -		igt_subtest_f("active-wakeup-%s", e->name)
> > -			active_wakeup_ring(fd, eb_ring(e), 20, 1);
> > -		igt_subtest_f("double-wakeup-%s", e->name)
> > -			wakeup_ring(fd, eb_ring(e), 20, 2);
> > -		igt_subtest_f("store-%s", e->name)
> > -			store_ring(fd, eb_ring(e), 1, 20);
> > -		igt_subtest_f("switch-%s", e->name)
> > -			switch_ring(fd, eb_ring(e), 1, 20);
> > -		igt_subtest_f("forked-switch-%s", e->name)
> > -			switch_ring(fd, eb_ring(e), ncpus, 20);
> > -		igt_subtest_f("many-%s", e->name)
> > -			store_many(fd, eb_ring(e), 20);
> > -		igt_subtest_f("forked-%s", e->name)
> > -			sync_ring(fd, eb_ring(e), ncpus, 20);
> > -		igt_subtest_f("forked-store-%s", e->name)
> > -			store_ring(fd, eb_ring(e), ncpus, 20);
> > -	}
> > +	/* legacy of selecting engines. */
> > -	igt_subtest("basic-each")
> > -		sync_ring(fd, ALL_ENGINES, 1, 2);
> > -	igt_subtest("basic-store-each")
> > -		store_ring(fd, ALL_ENGINES, 1, 2);
> > -	igt_subtest("basic-many-each")
> > -		store_many(fd, ALL_ENGINES, 2);
> > -	igt_subtest("switch-each")
> > -		switch_ring(fd, ALL_ENGINES, 1, 20);
> > -	igt_subtest("forked-switch-each")
> > -		switch_ring(fd, ALL_ENGINES, ncpus, 20);
> > -	igt_subtest("forked-each")
> > -		sync_ring(fd, ALL_ENGINES, ncpus, 20);
> > -	igt_subtest("forked-store-each")
> > -		store_ring(fd, ALL_ENGINES, ncpus, 20);
> > -	igt_subtest("active-each")
> > -		active_ring(fd, ALL_ENGINES, 20);
> > -	igt_subtest("wakeup-each")
> > -		wakeup_ring(fd, ALL_ENGINES, 20, 1);
> > -	igt_subtest("active-wakeup-each")
> > -		active_wakeup_ring(fd, ALL_ENGINES, 20, 1);
> > -	igt_subtest("double-wakeup-each")
> > -		wakeup_ring(fd, ALL_ENGINES, 20, 2);
> > +	igt_subtest_group {
> > +		for (test = tests; test->name; test++) {
> > +			igt_subtest_with_dynamic_f("legacy-engines-%s",
> 
> Have we not agreed to not add "engines" to test name?
> 
I removed it for other test group. will modify this too .
> > +						   test->name) {
> > +				for_each_physical_engine(e, fd) {
> > +					igt_dynamic_f("%s", e->name) {
> > +						do_test(test->func,
> > +							fd, eb_ring(e),
> > +							test->num_children,
> > +							test->timeout,
> > +							e->full_name);
> > +					}
> > +				}
> > +			}
> > +		}
> > +	}
> >   	igt_subtest("basic-all")
> >   		sync_all(fd, 1, 2);
> >   	igt_subtest("basic-store-all")
> >   		store_all(fd, 1, 2);
> > -
> 
> Noise as well.
> 
> >   	igt_subtest("all")
> >   		sync_all(fd, 1, 20);
> >   	igt_subtest("store-all")
> > @@ -1286,7 +1329,42 @@ igt_main
> >   	igt_subtest("forked-store-all")
> >   		store_all(fd, ncpus, 20);
> > +	/* All Engines */
> >   	igt_subtest_group {
> > +		for (test = allengine; test->name; test++) {
> > +			igt_subtest_f("%s", test->name) {
> > +				do_test(test->func,
> > +					fd, ALL_ENGINES,
> > +					test->num_children,
> > +					test->timeout,
> > +					test->name);
> > +			}
> > +		}
> > +	}
> > +
> > +	/* New way of selecting engines. */
> > +	igt_subtest_group {
> > +		const struct intel_execution_engine2 *e;
> > +
> > +		for (test = tests; test->name; test++) {
> > +			igt_subtest_with_dynamic_f("%s", test->name) {
> > +				__for_each_physical_engine(fd, e) {
> > +					igt_dynamic_f("%s", e->name) {
> > +						do_test(test->func,
> > +							fd, e->flags,
> > +							test->num_children,
> > +							test->timeout,
> > +							e->name);
> > +					}
> > +				}
> > +			}
> > +		}
> > +	}
> > +
> > +
> > +	igt_subtest_group {
> > +		const struct intel_execution_engine2 *e;
> > +
> >   		igt_fixture {
> 
> Regards,
> 
> Tvrtko
> 
> >   			gem_require_contexts(fd);
> >   			igt_require(gem_scheduler_has_ctx_priority(fd));
> > @@ -1295,10 +1373,12 @@ igt_main
> >   		igt_subtest("preempt-all")
> >   			preempt(fd, ALL_ENGINES, 1, 20);
> > -
> > -		for (e = intel_execution_engines; e->name; e++) {
> > -			igt_subtest_f("preempt-%s", e->name)
> > -				preempt(fd, eb_ring(e), ncpus, 20);
> > +		igt_subtest_with_dynamic("preempt") {
> > +			__for_each_physical_engine(fd, e) {
> > +				/* Requires master for STORE_DWORD on gen4/5 */
> > +				igt_dynamic_f("%s", e->name)
> > +					preempt(fd, e->flags, ncpus, 20);
> > +			}
> >   		}
> >   	}
> > 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev