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

[Tvrtko Ursulin]

On 06/06/2019 16:48, Ramalingam C wrote:
> 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;

I think it needs a context and struct i915_engine_class_instance as 
input and returns flags, yes.

(fd, ctx, ci) -> flags

Regards,

Tvrtko