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

[Ramalingam C]

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 at intel.com>
> > > > 
> > > > Thanks,
> > > > Andi
> > > > 
> >