Re: [igt-dev] [PATCH i-g-t 17/24] i915: Add gem_ctx_clone

Chris Wilson
2019-03-26T15:54:59Z

Quoting Tvrtko Ursulin (2019-03-26 15:44:35)
>
> On 22/03/2019 09:21, Chris Wilson wrote:
> > +static void clone_vm(int i915)
> > +{
> > + struct drm_i915_gem_context_create_ext_setparam set = {
> > + { .name = I915_CONTEXT_CREATE_EXT_SETPARAM },
> > + { .param = I915_CONTEXT_PARAM_VM },
> > + };
> > + struct drm_i915_gem_context_create_ext_clone ext = {
> > + { .name = I915_CONTEXT_CREATE_EXT_CLONE },
> > + .flags = I915_CONTEXT_CLONE_VM,
> > + };
> > + struct drm_i915_gem_context_create_ext create = {
> > + .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> > + .extensions = to_user_pointer(&ext),
> > + };
> > + uint32_t vm_id[2];
> > + int expected;
> > +
> > + igt_require(__gem_context_set_param(i915, &set.param) == -ENOENT);
> > +
> > + set.param.ctx_id = gem_context_create(i915);
> > + gem_context_get_param(i915, &set.param);
> > + vm_id[0] = set.param.value;
> > + gem_context_destroy(i915, set.param.ctx_id);
>
> Do you have in the VM ioctl IGT, or the ctx get/set VM, a subtest that
> verifies VM survives context destroy, if it was exported before that? It
> would need one I think.
>
> But in this particular one, what is the reason you create one VM
> indirectly via context create which is immediately destroyed? Exactly to
> test the above mentioned?

It was precisely that to check that we could keep a VM from an earlier
context.

But honestly it was because I had forgotten about gem_vm_create() at
that point and had to go and write another bunch of tests.

> > + set.param.ctx_id = 0;
> > +
> > + vm_id[1] = gem_vm_create(i915);

And then I was supposed to check we got similar results using explict
allocation.

> > + for (int pass = 0; pass < 2; pass++) { /* cloning default, then child */
> > + igt_debug("Cloning %d\n", ext.clone_id);
> > + igt_assert_eq(__create_ext(i915, &create), 0);
> > +
> > + set.param.ctx_id = ext.clone_id;
> > + gem_context_get_param(i915, &set.param);
> > + expected = set.param.value;
> > +
> > + set.param.ctx_id = create.ctx_id;
> > + gem_context_get_param(i915, &set.param);
> > +
> > + igt_assert_eq_u64(set.param.param, I915_CONTEXT_PARAM_VM);
> > + igt_assert_eq((int)set.param.value, expected);
> > +
> > + gem_context_destroy(i915, create.ctx_id);
> > +
> > + expected = set.param.value = vm_id[0];
> > + set.param.ctx_id = ext.clone_id;
> > + gem_context_set_param(i915, &set.param);
> > +
> > + igt_assert_eq(__create_ext(i915, &create), 0);
> > +
> > + set.param.ctx_id = create.ctx_id;
> > + gem_context_get_param(i915, &set.param);
> > +
> > + igt_assert_eq_u64(set.param.param, I915_CONTEXT_PARAM_VM);
> > + igt_assert_eq((int)set.param.value, expected);
> > +
> > + gem_context_destroy(i915, create.ctx_id);
> > + ext.clone_id = gem_context_create(i915);
> > + }
>
> vm_id[1] is unused.

Yeah...
-Chris