[igt-dev] [PATCH v14 4/5] lib/i915: add gem_engine_topology library and for_each loop definition

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-03-21 11:39:35)
> I was just discussing of our desired end game in therms of number and 
> signature for for_each_.. iterators.
> 
> For me for_each_physical_engine doesn't need the context since it is 
> about physical engines - not engine from the engine map. Maybe that one 
> should even have some asserts then to make sure someone hasn't 
> re-configured the default context.

Ok, I'd buy that we don't need to restrict for_each_physical to the
ctx->engines[]. But that implies to me that for_each_physical should set
its engines on the context, and that be a documented side-effect. Which
should be perfectly fine with users of that iface.

> And we have __for_each_physical_engine which uses the static table, for 
> subtest enumeration.

Still sadly required.
 
> Then for_each_context_engine is the fully featured one, which has the 
> context id in parameters.
> 
> Can we solve all use cases with those three or we need mode?

__for_each_static_engine

for_each_legacy_ring
for_each_physical_engine
for_each_[context]_engine

You could convince me that we never need to iterate over the legacy ABI,
as all such legacy testing can be confined to __for_each_static_engine
subtests. And I could accept that for_each_context_engine is merely a
fiction of the ctx->engines[] ABI tests.

So imo the most important iterators are:
__for_each_static_engine # for subtest definition
for_each_physical_engine # for actual workload construction

Aside from that,
gem_context_set_engine(ctx, class, instance)
/* maps ctx->engines[0] = { class, instance } */
is about all we need to specific execution outside of tests that know
they have a ctx->engines[] to work with (e.g. load balancing)

igt_require(gem_context_set_engines(ctx, ci, count)) would be fine for
new tests that need to define a workload across multiple engines.

I do like the direction the physical iterators are taking; construct the
table and return it as an iterator, that should see us through the ABI
transition smoothly. And in 10 years, you can kill off the legacy :-p
-Chris