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

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-03-22 09:56:49)
> 
> On 22/03/2019 07:59, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-03-22 07:47:02)
> >>> +void intel_next_engine(struct intel_engine_data *ed);
> >>> +
> >>> +#define IS_PHYSICAL_ENGINE(e2) ((e2->class >= 0) && (e2->instance >= 0))
> >>
> >> Chris, do you think this will be future proof enough?
> > 
> > At the moment, we've reserved just the one identifier for placeholders
> > (class == I915_ENGINE_CLASS_INVALID). And I feel confident that should
> > be enough.
> > 
> > The problem is if something else gave us multiple instances of a logical
> > engine for which we have no means to determine the physical mapping,
> > which is vvv
> > 
> >> I remembered how at one point I had "IS_PHYSICAL" as a flag in engine query.
> >>
> >> Or we make this here more explicit by being "IS_VIRTUAL" and invert the
> >> test in the caller?
> > 
> > Aye. I think you are right here, and we need to put a caps field into
> > the engine_data (filled in by i915_query for valid classes and default
> > to !phys for invalid slots). A lot of the for_each_physical_engine()
> > tests do not make sense if there is automagic engine mapping going on
> > behind the scenes.
> 
> You are simply saying to move the "IS_PHYISICAL" test to init_engine 
> here and store it in a flag per engine?

Yes, with a view to supplying that information from the kernel if the
future landscape changes for the actual engines.
-Chris