Quoting Tvrtko Ursulin (2019-03-26 10:46:30)
>
> On 22/03/2019 09:21, Chris Wilson wrote:
> > Check that the extended create interface accepts setparam.
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > ---
> > tests/i915/gem_ctx_create.c | 57 +++++++++++++++++++++++++++++++++++++
> > 1 file changed, 57 insertions(+)
> >
> > diff --git a/tests/i915/gem_ctx_create.c b/tests/i915/gem_ctx_create.c
> > index a664070db..e12f41691 100644
> > --- a/tests/i915/gem_ctx_create.c
> > +++ b/tests/i915/gem_ctx_create.c
> > @@ -308,6 +308,60 @@ static void maximum(int fd, int ncpus, unsigned mode)
> > free(contexts);
> > }
> >
> > +static int __create_ext(int i915, struct drm_i915_gem_context_create_ext *arg)
> > +{
> > + int err;
> > +
> > + err = 0;
> > + if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg))
> > + err = -errno;
> > +
> > + errno = 0;
> > + return err;
> > +}
> > +
> > +static void basic_ext_param(int i915)
> > +{
> > + struct drm_i915_gem_context_create_ext_setparam ext = {
> > + { .name = I915_CONTEXT_CREATE_EXT_SETPARAM },
> > + };
> > + struct drm_i915_gem_context_create_ext create = {
> > + .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS
> > + };
> > + struct drm_i915_gem_context_param get;
> > +
> > + igt_require(__create_ext(i915, &create) == 0);
> > + gem_context_destroy(i915, create.ctx_id);
> > +
> > + create.extensions = -1ull;
> > + igt_assert_eq(__create_ext(i915, &create), -EFAULT);
> > +
> > + create.extensions = to_user_pointer(&ext);
> > + igt_assert_eq(__create_ext(i915, &create), -EINVAL);
> > +
> > + ext.param.param = I915_CONTEXT_PARAM_PRIORITY;
> > + if (__create_ext(i915, &create) != -ENODEV) {
>
> Alternatively split into two subtests, one to test invalid and up to
> here, and second one to test the rest and use igt_require(... != ENODEV)
> so can skip where priority is not supported.
>
> Actually could also use with some more basic param which has no ENODEV path.
>
> > + gem_context_destroy(i915, create.ctx_id);
> > +
> > + ext.base.next_extension = -1ull;
> > + igt_assert_eq(__create_ext(i915, &create), -EFAULT);
> > + ext.base.next_extension = to_user_pointer(&ext);
> > + igt_assert_eq(__create_ext(i915, &create), -E2BIG);
> > + ext.base.next_extension = 0;
>
> These two can then also be coupled from CAP_SCHEDULER into the first
> subtest.
>
> > +
> > + ext.param.value = 32;
>
> Why 32? Add comment like /* random non-default priority level */
>
> For complete future proofing should probably query it first to assert it
> is unused.
Our ABI is that it should currently default to zero, and I thought even
under a cgroup masquerade, userspace would still its own namespace
around 0.
> Although again if you used something like bannable it could run on all
> platforms AFAICS.
Sure, the only goal here is to have something we can poke and verify
sets what we expect to make sure it does work before/after the invalid
chains.
> > + igt_assert_eq(__create_ext(i915, &create), 0);
> > +
> > + memset(&get, 0, sizeof(get));
> > + get.ctx_id = create.ctx_id;
> > + get.param = I915_CONTEXT_PARAM_PRIORITY;
> > + gem_context_get_param(i915, &get);
> > + igt_assert_eq(get.value, ext.param.value);
> > +
> > + gem_context_destroy(i915, create.ctx_id);
> > + }
> > +}
I've added second cases to try and cover the likely combinations.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
