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

Chris Wilson
2019-04-11T13:50:13Z

Quoting Tvrtko Ursulin (2019-04-11 13:53:24)
>
> On 11/04/2019 13:32, 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().
>
> Problem is __for_each_physical_engine modifies the default context so
> tests under that iterator can't keep using I915_EXEC_RENDER.
>
> It was perhaps a knee-jerk reaction to make them create their own
> context as a workaround.

From the looks of the tests affected, they all should support contexts. I
would be more concerned if the basic busyness checking tests grew a new
dependency.

Operating on the default context, should be fine though. May require an
operation to reset in case a previous subtest failed, which is a
downside.

> Should we have a helper after all to get eb flags for given class:instance?

I think resist/delay as much as possible. In 3 months time we can sweep
away any evidence of the old ways, and only the explicit legacy ABI
tests need concern themselves with BSD | BSD2 ;)
-Chris