| Message ID | 20190221100107.16789-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t] i915/gem_eio: Not everyone actually has contexts | expand |
On 21/02/19 02:01, Chris Wilson wrote: > Eek, I assumed the 'banned' subtest only applied to context platforms, > ti doesn't. The basic test works for all, checking whether a second ^--- Typo? :). > context works after the first is banned however only applies to > platforms with contexts! > Yeah, I missed that. > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > tests/i915/gem_eio.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c > index c5fd07585..3f941071d 100644 > --- a/tests/i915/gem_eio.c > +++ b/tests/i915/gem_eio.c > @@ -334,13 +334,13 @@ static void __test_banned(int fd) > > /* Only this context, not the file, should be banned */ > igt_assert_neq(__gem_context_create(fd, &ctx), -EIO); > - igt_assert_neq(ctx, 0); Although this assert seems to suggest it didn't apply to context-less platforms as it would fail here. I think it still makes sense to test you get banned on context 0 so, Reviwed-by: Antonio Argenziano <antonio.argenziano@intel.com> > - > - /* And check it actually works! */ > - execbuf.rsvd1 = ctx; > - gem_execbuf(fd, &execbuf); > + if (ctx) { /* remember the contextless! */ > + /* And check it actually works! */ > + execbuf.rsvd1 = ctx; > + gem_execbuf(fd, &execbuf); > > - gem_context_destroy(fd, ctx); > + gem_context_destroy(fd, ctx); > + } > return; > } > >
Quoting Antonio Argenziano (2019-02-21 17:50:12) > > > On 21/02/19 02:01, Chris Wilson wrote: > > Eek, I assumed the 'banned' subtest only applied to context platforms, > > ti doesn't. The basic test works for all, checking whether a second > ^--- Typo? :). > > context works after the first is banned however only applies to > > platforms with contexts! > > > > Yeah, I missed that. > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > tests/i915/gem_eio.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c > > index c5fd07585..3f941071d 100644 > > --- a/tests/i915/gem_eio.c > > +++ b/tests/i915/gem_eio.c > > @@ -334,13 +334,13 @@ static void __test_banned(int fd) > > > > /* Only this context, not the file, should be banned */ > > igt_assert_neq(__gem_context_create(fd, &ctx), -EIO); > > - igt_assert_neq(ctx, 0); > > Although this assert seems to suggest it didn't apply to context-less > platforms as it would fail here. We pass the first with -ENODEV, and fail the second, so yup. > I think it still makes sense to test you get banned on context 0 so, Aye, we still want to ban individual fd using default context. Hmm, we should also go around the loop again to verify that the second context is also banned. Tomorrow! -Chris
diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c index c5fd07585..3f941071d 100644 --- a/tests/i915/gem_eio.c +++ b/tests/i915/gem_eio.c @@ -334,13 +334,13 @@ static void __test_banned(int fd) /* Only this context, not the file, should be banned */ igt_assert_neq(__gem_context_create(fd, &ctx), -EIO); - igt_assert_neq(ctx, 0); - - /* And check it actually works! */ - execbuf.rsvd1 = ctx; - gem_execbuf(fd, &execbuf); + if (ctx) { /* remember the contextless! */ + /* And check it actually works! */ + execbuf.rsvd1 = ctx; + gem_execbuf(fd, &execbuf); - gem_context_destroy(fd, ctx); + gem_context_destroy(fd, ctx); + } return; }
Eek, I assumed the 'banned' subtest only applied to context platforms, ti doesn't. The basic test works for all, checking whether a second context works after the first is banned however only applies to platforms with contexts! Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- tests/i915/gem_eio.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)