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

Chris Wilson
2019-03-21T11:57:54Z

Quoting Chris Wilson (2019-03-21 11:54:49)
> 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

I will also say that it will become quickly apparent if we've missed
anything as the changes ripple through the tests, so don't worry too
much about it. :)
-Chris