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

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH i-g-t 13/24] i915/gem_ctx_param: Test set/get (copy) VM
Date: Tue, 26 Mar 2019 10:22:18 +0000	[thread overview]
Message-ID: <d0f0ef4f-716f-ad65-3cc4-269e26cc1291@linux.intel.com> (raw)
In-Reply-To: <20190322092155.1656-13-chris@chris-wilson.co.uk>

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx