[igt-dev] [RFC 2/2] lib: implement new engine discovery interface

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-11-19 19:59:19)
> 
> On 19/11/2018 15:55, Andi Shyti wrote:
> > Kernel commits:
> > 
> > [1] ae8f4544dd8f ("drm/i915: Engine discovery query")
> > [2] 31e7d35667a0 ("drm/i915: Allow a context to define its set of engines")
> > 
> > from [*] repository, implement a new way uapi for engine
> > discovery that consist in first coupling context with engine [2]
> > and then query the engines through a specific structure [1].
> > 
> > In igt the classic way for discovering engines is done trhough
> > the for_each_physical_engine() macro, that is replaced by the
> > new for_each_engine_ctx().
> 
> My idea was that we migrate for_each_physical_engine to this new scheme.
> 
> As an easy starting point I was thinking to keep the current 
> for_each_physical_engine as is, and just add new 
> for_each_physical_engine_new and migrate one test to it as a demo.
> 
> Then when this part is reviewed, as a second step we convert the rest 
> and rename the macro stripping the _new suffix and nuking the old one.
> 
> With regards to implementation details I was thinking along these lines:
> 
> On first invocation for_each_physical_engine_new initializes some hidden 
> data stored in one or more globals (which will live in igt_gt.c).
> 
> This would query the engines and create a context with all engines mapped.
> 
> We also add a helper to get this internal ctx id to use within 
> for_each_physical_engine_new loops.
> 
> That should make it easy to convert simple tests over like:
> 
>    igt_subtest {
>         for_each_physical_engine_new(fd, engine) {
>                 ...
>                 execbuf.rsvd1 = gem_physical_engine_ctx();
>                 execbuf.flags = gem_physical_engine_idx(engine);
>                 gem_execbuf(fd, execbuf);
>         }
>    }

Granted that we have a lot of tests that just use the default ctx, I
don't think the iterator interface we create should enforce that.

for_each_physical_engine_new(fd, ctx, engine) {
	execbuf.rsvd1 = ctx;
	execbuf.flags = engine;
	gem_execbuf(fd, execbuf);
}

where ctx is provided, and engine the iterator. Off the top of my head,
I have a silly idea like

for (int __max_engine__; (__max_engine__ = igt_physical_engine_iter_init(fd, ctx)); )
	for (engine = 1; engine <= __max_engine__; engine++)

where igt_physical_engine_iter_init(int fd, uint32_t ctx) {
	if (ctx_getparam(fd, ctx, ENGINES).count > 0) {
		ctx_setparam(fd, ctx, ENGINES, NULL);
		return 0;
	}

	// query and set engine array
	return count;
}
-Chris