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

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 11/04/2019 14:50, Chris Wilson wrote:
> 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.

Basic busyness ones have not, but the ones I fixed up by making them 
create a context have. Which is fixable with below.

Failures are only with gem_wait/basic-*-all, so I suspect something's 
off in igt_dummyload.c. Will see what happens with the debugs added.

> 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 ;)

perf_pmu tests which I added context_create to open PMU for rcs:0 and 
submit batches to I915_EXEC_RENDER.

Well alternative to the helper could be to change them to submit to 
I915_EXEC_DEFAULT and rely on context map to always have rcs:0 at index 
0. Which could even be passable. If the promise is broken the tests will 
fail to let us know.

Okay with this alternative hack?

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev