Re: [igt-dev] [PATCH i-g-t 13/24] i915/gem_ctx_param: Test set/get (copy) VM

Tvrtko Ursulin
2019-03-26T10:22:18Z

On 22/03/2019 09:21, Chris Wilson wrote:
> Exercise reusing the GTT of one ctx in another.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> tests/i915/gem_ctx_param.c | 83 ++++++++++++++++++++++++++++++++------
> 1 file changed, 71 insertions(+), 12 deletions(-)
>
> diff --git a/tests/i915/gem_ctx_param.c b/tests/i915/gem_ctx_param.c
> index b3f8637df..54ade8b4b 100644
> --- a/tests/i915/gem_ctx_param.c
> +++ b/tests/i915/gem_ctx_param.c
> @@ -36,17 +36,6 @@ IGT_TEST_DESCRIPTION("Basic test for context set/get param input validation.");
> #define NEW_CTX BIT(0)
> #define USER BIT(1)
>
> -static int reopen_driver(int fd)
> -{
> - char path[256];
> -
> - snprintf(path, sizeof(path), "/proc/self/fd/%d", fd);
> - fd = open(path, O_RDWR);
> - igt_assert_lte(0, fd);
> -
> - return fd;
> -}
> -
> static void set_priority(int i915)
> {
> static const int64_t test_values[] = {
> @@ -91,7 +80,7 @@ static void set_priority(int i915)
> igt_permute_array(values, size, igt_exchange_int64);
>
> igt_fork(flags, NEW_CTX | USER) {
> - int fd = reopen_driver(i915);
> + int fd = gem_reopen_driver(i915);
> struct drm_i915_gem_context_param arg = {
> .param = I915_CONTEXT_PARAM_PRIORITY,
> .ctx_id = flags & NEW_CTX ? gem_context_create(fd) : 0,
> @@ -143,6 +132,73 @@ static void set_priority(int i915)
> free(values);
> }
>
> +static uint32_t __batch_create(int i915, uint32_t offset)
> +{
> + const uint32_t bbe = MI_BATCH_BUFFER_END;
> + uint32_t handle;
> +
> + handle = gem_create(i915, ALIGN(offset + 4, 4096));
> + gem_write(i915, handle, offset, &bbe, sizeof(bbe));
> +
> + return handle;
> +}
> +
> +static uint32_t batch_create(int i915)
> +{
> + return __batch_create(i915, 0);
> +}

Looks familiar. :)

> +
> +static void test_vm(int i915)
> +{
> + struct drm_i915_gem_exec_object2 batch = {
> + .handle = batch_create(i915),
> + };
> + struct drm_i915_gem_execbuffer2 eb = {
> + .buffers_ptr = to_user_pointer(&batch),
> + .buffer_count = 1,
> + };
> + struct drm_i915_gem_context_param arg = {
> + .param = I915_CONTEXT_PARAM_VM,
> + };
> + uint32_t parent, child;
> +
> + arg.value = -1ull;
> + igt_require(__gem_context_set_param(i915, &arg) == -ENOENT);
> +
> + parent = gem_context_create(i915);
> + child = gem_context_create(i915);
> +
> + eb.rsvd1 = parent;
> + batch.offset = 48 << 20;
> + gem_execbuf(i915, &eb);
> + igt_assert_eq_u64(batch.offset, 48 << 20);
> +
> + eb.rsvd1 = child;
> + batch.offset = 0;
> + gem_execbuf(i915, &eb);
> + igt_assert_eq_u64(batch.offset, 0);
> +
> + eb.rsvd1 = parent;
> + gem_execbuf(i915, &eb);
> + igt_assert_eq_u64(batch.offset, 48 << 20);

Please drop a comment at the start of these execbuf operations to
explain what and why. We don't need softpin to guarantee they will get
pinned to where we want them to?

> +
> + arg.ctx_id = parent;
> + gem_context_get_param(i915, &arg);
> +
> + arg.ctx_id = child;
> + gem_context_set_param(i915, &arg);

Another get param to assert child vm id is the same as the parent?

Also, try self-assign? I mean set the same vm id as already have?

> +
> + eb.rsvd1 = child;
> + gem_execbuf(i915, &eb);
> + igt_assert_eq_u64(batch.offset, 48 << 20);

Interesting, for me at least. Please put a comment here.

> +
> + gem_context_destroy(i915, child);
> + gem_context_destroy(i915, parent);
> +
> + gem_sync(i915, batch.handle);
> + gem_close(i915, batch.handle);
> +}
> +
> igt_main
> {
> struct drm_i915_gem_context_param arg;
> @@ -253,6 +309,9 @@ igt_main
> gem_context_set_param(fd, &arg);
> }
>
> + igt_subtest("vm")
> + test_vm(fd);
> +
> arg.param = I915_CONTEXT_PARAM_PRIORITY;
>
> igt_subtest("set-priority-not-supported") {
>

Add to basic test list? Or call basic-vm? Honestly don't remember how we
do it these days..

Regards,

Tvrtko