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

From: Andi Shyti <andi.shyti@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: igt-dev@lists.freedesktop.org, Intel-gfx@lists.freedesktop.org
Subject: Re: [igt-dev] [RFT i-g-t 6/6] test: perf_pmu: use the gem_engine_topology library
Date: Thu, 11 Apr 2019 17:55:13 +0300	[thread overview]
Message-ID: <20190411145513.GB2333@intel.intel> (raw)
In-Reply-To: <155499004582.22467.3989945936737043582@skylake-alporthouse-com>

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx