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

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 08/05/2019 11:09, Chris Wilson wrote:
> Exercise reusing the GTT of one ctx in another.
> 
> v2: Test setting back to the same VM
> v3: Check the VM still exists after the parent ctx are dead.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   tests/i915/gem_ctx_param.c | 107 ++++++++++++++++++++++++++++++++-----
>   1 file changed, 95 insertions(+), 12 deletions(-)
> 
> diff --git a/tests/i915/gem_ctx_param.c b/tests/i915/gem_ctx_param.c
> index b6f57236c..d949cef32 100644
> --- a/tests/i915/gem_ctx_param.c
> +++ b/tests/i915/gem_ctx_param.c
> @@ -28,6 +28,7 @@
>   #include <limits.h>
>   
>   #include "igt.h"
> +#include "i915/gem_vm.h"
>   
>   IGT_TEST_DESCRIPTION("Basic test for context set/get param input validation.");
>   
> @@ -36,17 +37,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 +81,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 +133,96 @@ 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);
> +}
> +
> +static void test_vm(int i915)
> +{
> +	const uint64_t nonzero_offset = 48 << 20;
> +	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);
> +
> +	/* Using implicit soft-pinning */
> +	eb.rsvd1 = parent;
> +	batch.offset = nonzero_offset;
> +	gem_execbuf(i915, &eb);
> +	igt_assert_eq_u64(batch.offset, nonzero_offset);
> +
> +	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, nonzero_offset);
> +
> +	arg.ctx_id = parent;
> +	gem_context_get_param(i915, &arg);
> +	gem_context_set_param(i915, &arg);
> +
> +	/* Still the same VM, so expect the old VMA again */
> +	batch.offset = 0;
> +	gem_execbuf(i915, &eb);
> +	igt_assert_eq_u64(batch.offset, nonzero_offset);
> +
> +	arg.ctx_id = child;
> +	gem_context_set_param(i915, &arg);
> +
> +	eb.rsvd1 = child;
> +	batch.offset = 0;
> +	gem_execbuf(i915, &eb);
> +	igt_assert_eq_u64(batch.offset, nonzero_offset);
> +
> +	gem_context_destroy(i915, child);
> +	gem_context_destroy(i915, parent);

High level commentary is lacking so my condolences to future readers.

I'd also add a get_vm after set_vm just to check it reads back the same.

Otherwise:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

> +
> +	/* both contexts destroyed, but we still keep hold of the vm */
> +	child = gem_context_create(i915);
> +
> +	arg.ctx_id = child;
> +	gem_context_set_param(i915, &arg);
> +
> +	eb.rsvd1 = child;
> +	batch.offset = 0;
> +	gem_execbuf(i915, &eb);
> +	igt_assert_eq_u64(batch.offset, nonzero_offset);
> +
> +	gem_context_destroy(i915, child);
> +	gem_vm_destroy(i915, arg.value);
> +
> +	gem_sync(i915, batch.handle);
> +	gem_close(i915, batch.handle);
> +}
> +
>   igt_main
>   {
>   	struct drm_i915_gem_context_param arg;
> @@ -253,6 +333,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") {
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev