Re: [igt-dev] [PATCH i-g-t] tests/i915: Use engine query interface for gem_ctx_isolation/persistence

Summers, Stuart
2019-12-04T19:05:15Z

[-- Attachment #1.1: Type: text/plain, Size: 2932 bytes --]

On Wed, 2019-12-04 at 16:53 +0200, Petri Latvala wrote:
> On Wed, Dec 04, 2019 at 09:36:11AM +0000, Tvrtko Ursulin wrote:
> >
> > On 03/12/2019 05:11, Stuart Summers wrote:
> > > Align with gem_exec_basic and other tests using the newer
> > > engine query interface into i915 to enumerate active engines.
> > >
> > > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> > > ---
> > > tests/i915/gem_ctx_isolation.c | 4 ++--
> > > tests/i915/gem_ctx_persistence.c | 2 +-
> > > 2 files changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tests/i915/gem_ctx_isolation.c
> > > b/tests/i915/gem_ctx_isolation.c
> > > index 6aa27133..9435209e 100644
> > > --- a/tests/i915/gem_ctx_isolation.c
> > > +++ b/tests/i915/gem_ctx_isolation.c
> > > @@ -856,6 +856,7 @@ static unsigned int
> > > __has_context_isolation(int fd)
> > > igt_main
> > > {
> > > + struct intel_execution_engine2 *e;
> > > unsigned int has_context_isolation = 0;
> > > int fd = -1;
> > > @@ -876,8 +877,7 @@ igt_main
> > > igt_skip_on(gen > LAST_KNOWN_GEN);
> > > }
> > > - for (const struct intel_execution_engine2 *e =
> > > intel_execution_engines2;
> > > - e->name; e++) {
> > > + __for_each_physical_engine(fd, e) {
> > > igt_subtest_group {
> > > igt_fixture {
> > > igt_require(has_context_isolati
> > > on & (1 << e->class));
> > > diff --git a/tests/i915/gem_ctx_persistence.c
> > > b/tests/i915/gem_ctx_persistence.c
> > > index d68431ae..30772159 100644
> > > --- a/tests/i915/gem_ctx_persistence.c
> > > +++ b/tests/i915/gem_ctx_persistence.c
> > > @@ -727,7 +727,7 @@ igt_main
> > > igt_subtest("hangcheck")
> > > test_nohangcheck_hostile(i915);
> > > - __for_each_static_engine(e) {
> > > + __for_each_physical_engine(i915, e) {
> > > igt_subtest_group {
> > > igt_fixture {
> > > gem_require_ring(i915, e-
> > > >flags);
> > >
> >
> > __for_each_static_engine is correct, at least if you don't want CI
> > folks go
> > look for their pitchforks. :) Same for the first hunk, everything
> > that
> > enumerates subtests needs to be static.
> >
> > Option 2, the preferred one - convert the tests to
> > igt_subtest_with_dynamic
> > and then you can use __for_each_physical_engine.
>
> Doesn't __for_each_physical_engine anyway have a hack for being
> called in that context?
>
> Btw, option 2 is
> https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/44
>
> Currently blocking that effort is getting
> https://patchwork.freedesktop.org/series/70286/ into shape.

Wait I'm a little confused, sorry for the naivete here. Does this mean
these kinds of changes are blocked on the above series? Or are you
saying I should go ahead and convert this to the dynamic subtests? Or
that we can move forward with the current approach and convert at a
later time?

Thanks,
Stuart

>
>

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3270 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]