Re: [igt-dev] [RFC PATCH v10 3/6] lib/igt_gt: use for_each_engine2 to loop through engines

[Andi Shyti <andi.shyti@intel.com>]

Hi Tvrtko,

> > > +#define for_each_engine2(fd, ctx) \
> > > +        for (struct intel_execution_engine2 *e2__ = \
> > > +             gem_set_context_get_engines(fd, ctx); e2__->name; e2__++) \
> > > +            for_if (gem_has_engine_topology() || \
> > > +                gem_has_engine(fd, e2__->class, e2__->instance))
> > 
> > gem_has_engine is a legacy hack which shouldn't be used and can
> > hopefully be eliminated by the end of this work.
> > 
> > I think this iterator should assume ctx engine map has been configured
> > with only available engines so neiher gem_has_engine_topology or
> > gem_has_engine should be needed. Unless I am missing something?
> 
> I forgot about the desire to run on old kernels.. blah..

yes! :)

> But it still doesn't work since whatever is in the loop will use the index
> and the context may not have one.

You think so? Unless I am very likely to be missing something,
this should work in any case, thus the next patch:

'[RFC PATCH v10 4/6] lib: ioctl_wrappers: reach engines by index as well'

Where, I modified the existing 'gem_has_ring()' to work with both
index and not index. Still in the next patch there is:

> Would a helper like gem_get_ctx_engine_flags(fd, ctx, e2, i) solve this?

it looks like the version in patch v2 or v3 something :D

> I think something along those lines was in discussion some time back.
> 
> bool gem_get_ctx_engine_flags(...)
> {
> 	if (ctx.has_map, or maybe, if gem_has_engine_topology)
> 		return i;
> 	else
> 		return e2->eb_flags;

I was asked not to add anything in 'struct
intel_execution_engine2' :/

> }
> 
> It would need a comeback of index param to the iterator though.

About the iterator we had also some discussions and at the end I
did as Chris asked.

Doing what you ask, it's more an architectural decision, do we
like it as it is now or as it was before?

Personally I like the explicit iterator, and as it is now, with
the changes in patch 4, it works fine with new and old api
without many changes. Right? :)

Andi
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev