[igt-dev] [PATCH v16 8/8] test: perf_pmu: use the gem_engine_topology library

[Andi Shyti]

> > -static igt_spin_t * __spin_poll(int fd, uint32_t ctx, unsigned long flags)
> > +static igt_spin_t * __spin_poll(int fd, uint32_t ctx, const struct intel_execution_engine2 *e)
> >   {
> >   	struct igt_spin_factory opts = {
> >   		.ctx = ctx,
> > -		.engine = flags,
> > +		.engine = e->flags,
> >   	};
> > -	if (gem_can_store_dword(fd, flags))
> > +	if (gem_engine_can_store_dword(fd, e))
> >   		opts.flags |= IGT_SPIN_POLL_RUN;
> >   	return __igt_spin_batch_factory(fd, &opts);
> > @@ -211,7 +211,16 @@ static unsigned long __spin_wait(int fd, igt_spin_t *spin)
> >   static igt_spin_t * __spin_sync(int fd, uint32_t ctx, unsigned long flags)
> 
> __spin_sync should also take engine instead of flags, like __spin_poll.

But if __spin_sync is called inside __for_each_physical_engine
(which in this code never happens), engine doesn't have any flags
set. right?

> >   {
> > -	igt_spin_t *spin = __spin_poll(fd, ctx, flags);
> > +	struct igt_spin_factory opts = {
> > +		.ctx = ctx,
> > +		.engine = flags,
> > +	};
> > +	igt_spin_t *spin;
> > +
> > +	if (gem_can_store_dword(fd, flags))
> > +		opts.flags |= IGT_SPIN_POLL_RUN;
> > +
> > +	spin = __igt_spin_batch_factory(fd, &opts);
> 
> And then you don't need this hunk at all, making the whole thing completely
> ready for more engines.
> 
> And e2ring will end up with no callers so can be removed.

Same reason as above, which is this one:

> > -		__for_each_engine_class_instance(e) {
> > +		__for_each_static_engine(e) {
> >   			igt_subtest_group {
> >   				igt_fixture {
> >   					gem_require_engine(render_fd,
> > 
> 
> Yep, modulo __spin_sync change, this is what I had in mind.

at this point I don't have any flag set, any check whether the
engine really exists, no context mapping (still flags) :/

Am I missing it?

Andi