Re: [igt-dev] [Intel-gfx] [RFT i-g-t 6/6] test: perf_pmu: use the gem_engine_topology library

Andi Shyti
2019-04-11T14:55:13Z

On Thu, Apr 11, 2019 at 02:40:45PM +0100, Chris Wilson wrote:
> Quoting Andi Shyti (2019-04-11 14:01:01)
> > On Thu, Apr 11, 2019 at 01:32:03PM +0100, Chris Wilson wrote:
> > > Quoting Tvrtko Ursulin (2019-04-11 13:26:54)
> > > > From: Andi Shyti <andi.shyti@intel.com>
> > > >
> > > > Replace the legacy for_each_engine* defines with the ones
> > > > implemented in the gem_engine_topology library.
> > > >
> > > > Use whenever possible gem_engine_can_store_dword() that checks
> > > > class instead of flags.
> > > >
> > > > Now the __for_each_engine_class_instance and
> > > > for_each_engine_class_instance are unused, remove them.
> > > >
> > > > Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > Signed-off-by: Andi Shyti <andi.shyti@intel.com>
> > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> > > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > >
> > > I see a lot of new gem_context_create(), but not gem_require_contexts().
> >
> > gem_require_contexts() should go on top of engine_topology.
>
> engine_topology shouldn't require contexts per se, as we should be able
> to quite happily modify the engine map for Pineview. To what purpose,
> we do not ask!
>
> Just be careful you don't legacy tests.

I created a helper to avoid initializing the whole "think".
Tvrtko, indeed at some point calls "init_engine_list".

The helper looks like this:

+int64_t gem_map_all_engines(int fd)
+{
+ DEFINE_CONTEXT_PARAM(engines, param, 0, GEM_MAX_ENGINES);
+ struct intel_engine_data engine_data = { };

/* here is where I would add gem_require_contexts() */

+ param.ctx_id = gem_context_create(fd);
+
+ if (gem_topology_get_param(fd, &param) > 0 && !param.size) {
+ query_engine_list(fd, &engine_data);
+ ctx_map_engines(fd, &engine_data, &param);
+ }
+
+ return param.ctx_id;
+}

and of course there is an unmap_engines that just deletes the
context.

Maybe such a helper is not required in the new world, but it can
be useful for legacy tests.

Andi