Re: [igt-dev] [PATCH i-g-t v2 5/5] test/prime_vgem: use the gem_engine_topology library

Andi Shyti
2019-06-07T10:38:24Z

Hi Ram,

> > > > > just a nitpick
> > > > >
> > > > > > - execbuf.flags = ring | flags;
> > > > > > + execbuf.flags = e ? e->flags : I915_EXEC_DEFAULT;
> > > > >
> > > > > Because we are moving to the new engine naming, the
> > > > > "I915_EXEC_DEFAULT" engine, conceptually, should not be used.
> > > > >
> > > > > Politically, with Tvrtko, we agreed that he default engine now
> > > > > is "I915_ENGINE_CLASS_RENDER", it's not the best as solution, but
> > > > > works.
> > > >
> > > > I totally don't remember I ever suggested to use class or even it was
> > > > discussed. :) Didn't we say to just use zero?
> > > Can't use I915_EXEC_RENDER instead of DEFAULT!? because that will be
> > > exec_flag for render class right?
> >
> > No, I915_EXEC_RENDER is 1 so if the context has engine map configured that
> > will map to 2nd engine in the map - which is not rcs0.
> >
> > We are relying here on rcs0 being first (index 0). But that is also not
> > reliable. We need a lookup helper to be more robust going forward.
> something like below?
>
> get_engine_exec_flag_for_class(fd, e, class) __for_each_physical_engine(fd, e) \
> if (e->class == class) { \
> e->flags;
> break; \
> } \
> 0;

I guess we would need to query the gpu, fetch the engine list and
search the needed engine, somthing similar to
gem_context_lookup_engine() in gem_engine_topology.c

if you want you can give it a shot, otherwise I can do it over
the weekend and send it on Monday.

Andi