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

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 07/03/2019 12:07, Tvrtko Ursulin wrote:
> 
> On 05/03/2019 13:16, Andi Shyti wrote:
>> 'for_each_engine2()' defines a loop through the gpu engines.
>>
>> It can work with both active and pre-defined engines.
>>
>> In case we are looping through active engines (i.e. the running
>> kernel has the query and get/setparam ioctls), the
>> intel_active_engines2 points to an array that contains only the
>> list engines dynamically allocated after having interrogated the
>> driver.
>>
>> While, if we are looping through pre-defined engines,
>> intel_active_engines2 points to the 'intel_execution_engines2'
>> array and works exactly as for_each_engine() but using the
>> new 'struct intel_execution_engine2' data type.
>>
>> Signed-off-by: Andi Shyti <andi.shyti@intel.com>
>> ---
>>   lib/igt_gt.h | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/lib/igt_gt.h b/lib/igt_gt.h
>> index f4bd6c22a81a..f8bdaa62899f 100644
>> --- a/lib/igt_gt.h
>> +++ b/lib/igt_gt.h
>> @@ -30,6 +30,8 @@
>>   #include "i915_drm.h"
>> +#include "i915/gem_engine_topology.h"
>> +
>>   void igt_require_hang_ring(int fd, int ring);
>>   typedef struct igt_hang {
>> @@ -86,6 +88,12 @@ extern const struct intel_execution_engine {
>>            e__++) \
>>           for_if (gem_ring_has_physical_engine(fd__, flags__ = 
>> e__->exec_id | e__->flags))
>> +#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..

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

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

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;
}

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

Regards,

Tvrtko

> 
>> +
>>   bool gem_ring_is_physical_engine(int fd, unsigned int ring);
>>   bool gem_ring_has_physical_engine(int fd, unsigned int ring);
>>
> 
> Regards,
> 
> Tvrtko
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev