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

Ramalingam C
2019-06-06T15:46:07Z

On 2019-06-06 at 16:34:03 +0100, Tvrtko Ursulin wrote:
>
> On 06/06/2019 16:31, Ramalingam C wrote:
> > On 2019-06-06 at 15:38:36 +0100, Tvrtko Ursulin wrote:
> > >
> > > On 06/06/2019 15:17, Andi Shyti wrote:
> > > > 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;

-Ram
>
> Regards,
>
> Tvrtko
>
>
> > -Ram
> > >
> > > Regards,
> > >
> > > Tvrtko
> > >
> > > > The rest looks good.
> > > >
> > > > Reviewed-by: Andi Shyti <andi.shyti@intel.com>
> > > >
> > > > Thanks,
> > > > Andi
> > > >
> >