[Intel-gfx] [PATCH i-g-t 0/4] Some gem_lmem_swapping tweaks

[Intel-gfx] [PATCH i-g-t 0/4] Some gem_lmem_swapping tweaks

[Matthew Auld]

-- 
2.34.1

[Intel-gfx] [PATCH i-g-t 1/4] test/gem_lmem_swapping: account for object rounding

[Matthew Auld]

On DG2 the object size might be rounded when allocating lmem. Make sure
we account for any rounding up.

Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
---
 tests/i915/gem_lmem_swapping.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/i915/gem_lmem_swapping.c b/tests/i915/gem_lmem_swapping.c
index 582111dd..193ff370 100644
--- a/tests/i915/gem_lmem_swapping.c
+++ b/tests/i915/gem_lmem_swapping.c
@@ -72,7 +72,7 @@ struct object {
 };
 
 static uint32_t create_bo(int i915,
-			  uint64_t size,
+			  uint64_t *size,
 			  struct drm_i915_gem_memory_class_instance *region,
 			  bool do_oom_test)
 {
@@ -80,7 +80,7 @@ static uint32_t create_bo(int i915,
 	int ret;
 
 retry:
-	ret = __gem_create_in_memory_region_list(i915, &handle, &size, region, 1);
+	ret = __gem_create_in_memory_region_list(i915, &handle, size, region, 1);
 	if (do_oom_test && ret == -ENOMEM)
 		goto retry;
 	igt_assert_eq(ret, 0);
@@ -172,7 +172,7 @@ static void __do_evict(int i915,
 
 	__gem_context_set_persistence(i915, 0, false);
 	size = 4096;
-	batch = create_bo(i915, size, region, params->oom_test);
+	batch = create_bo(i915, &size, region, params->oom_test);
 
 	gem_write(i915, batch, 0, &bbe, sizeof(bbe));
 
@@ -199,7 +199,7 @@ static void __do_evict(int i915,
 			params->count = i;
 			break;
 		}
-		obj->handle = create_bo(i915, obj->size, region, params->oom_test);
+		obj->handle = create_bo(i915, &obj->size, region, params->oom_test);
 
 		move_to_lmem(i915, objects + i, 1, batch, engine,
 			     params->oom_test);
@@ -270,7 +270,7 @@ static void fill_params(int i915, struct params *params,
 
 	if (flags & TEST_RANDOM) {
 		params->size.min = 4096;
-		handle = create_bo(i915, params->size.min, &region->region,
+		handle = create_bo(i915, &params->size.min, &region->region,
 				   do_oom_test);
 		gem_close(i915, handle);
 		params->size.max = 2 * size + params->size.min;
-- 
2.34.1

Re: [Intel-gfx] [PATCH i-g-t 1/4] test/gem_lmem_swapping: account for object rounding

[Dixit, Ashutosh]

On Thu, 24 Mar 2022 07:26:18 -0700, Matthew Auld wrote:
>
> On DG2 the object size might be rounded when allocating lmem. Make sure
> we account for any rounding up.

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

Re: [Intel-gfx] [PATCH i-g-t 1/4] test/gem_lmem_swapping: account for object rounding

[Thomas Hellström]

On Thu, 2022-03-24 at 14:26 +0000, Matthew Auld wrote:
> On DG2 the object size might be rounded when allocating lmem. Make
> sure
> we account for any rounding up.
> 
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux-intel.com>

> ---
>  tests/i915/gem_lmem_swapping.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/i915/gem_lmem_swapping.c
> b/tests/i915/gem_lmem_swapping.c
> index 582111dd..193ff370 100644
> --- a/tests/i915/gem_lmem_swapping.c
> +++ b/tests/i915/gem_lmem_swapping.c
> @@ -72,7 +72,7 @@ struct object {
>  };
>  
>  static uint32_t create_bo(int i915,
> -                         uint64_t size,
> +                         uint64_t *size,
>                           struct drm_i915_gem_memory_class_instance
> *region,
>                           bool do_oom_test)
>  {
> @@ -80,7 +80,7 @@ static uint32_t create_bo(int i915,
>         int ret;
>  
>  retry:
> -       ret = __gem_create_in_memory_region_list(i915, &handle,
> &size, region, 1);
> +       ret = __gem_create_in_memory_region_list(i915, &handle, size,
> region, 1);
>         if (do_oom_test && ret == -ENOMEM)
>                 goto retry;
>         igt_assert_eq(ret, 0);
> @@ -172,7 +172,7 @@ static void __do_evict(int i915,
>  
>         __gem_context_set_persistence(i915, 0, false);
>         size = 4096;
> -       batch = create_bo(i915, size, region, params->oom_test);
> +       batch = create_bo(i915, &size, region, params->oom_test);
>  
>         gem_write(i915, batch, 0, &bbe, sizeof(bbe));
>  
> @@ -199,7 +199,7 @@ static void __do_evict(int i915,
>                         params->count = i;
>                         break;
>                 }
> -               obj->handle = create_bo(i915, obj->size, region,
> params->oom_test);
> +               obj->handle = create_bo(i915, &obj->size, region,
> params->oom_test);
>  
>                 move_to_lmem(i915, objects + i, 1, batch, engine,
>                              params->oom_test);
> @@ -270,7 +270,7 @@ static void fill_params(int i915, struct params
> *params,
>  
>         if (flags & TEST_RANDOM) {
>                 params->size.min = 4096;
> -               handle = create_bo(i915, params->size.min, &region-
> >region,
> +               handle = create_bo(i915, &params->size.min, &region-
> >region,
>                                    do_oom_test);
>                 gem_close(i915, handle);
>                 params->size.max = 2 * size + params->size.min;

[Intel-gfx] [PATCH i-g-t 2/4] test/gem_lmem_swapping: fix physical engine usage

[Matthew Auld]

DG2 seems to have too many physical engines, and during execbuf just hits:

	"execbuf with unknown ring: 5"

Convert the test over to using the non-legacy API where we instead fill
the ctx with all the physical engines and then engine/ring becomes the
index into this.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5192
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
---
 tests/i915/gem_lmem_swapping.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/tests/i915/gem_lmem_swapping.c b/tests/i915/gem_lmem_swapping.c
index 193ff370..995a663f 100644
--- a/tests/i915/gem_lmem_swapping.c
+++ b/tests/i915/gem_lmem_swapping.c
@@ -126,6 +126,7 @@ verify_object(int i915, const struct object *obj,  unsigned int flags)
 }
 
 static void move_to_lmem(int i915,
+			 const intel_ctx_t *ctx,
 			 struct object *list,
 			 unsigned int num,
 			 uint32_t batch,
@@ -137,6 +138,7 @@ static void move_to_lmem(int i915,
 		.buffers_ptr = to_user_pointer(obj),
 		.buffer_count = 1 + num,
 		.flags = I915_EXEC_NO_RELOC | I915_EXEC_HANDLE_LUT | engine,
+		.rsvd1 = ctx->id,
 	};
 	unsigned int i, ret;
 
@@ -156,6 +158,7 @@ retry:
 }
 
 static void __do_evict(int i915,
+		       const intel_ctx_t *ctx,
 		       struct drm_i915_gem_memory_class_instance *region,
 		       struct params *params,
 		       unsigned int seed)
@@ -170,7 +173,6 @@ static void __do_evict(int i915,
 	struct timespec t = {};
 	unsigned int num;
 
-	__gem_context_set_persistence(i915, 0, false);
 	size = 4096;
 	batch = create_bo(i915, &size, region, params->oom_test);
 
@@ -201,7 +203,7 @@ static void __do_evict(int i915,
 		}
 		obj->handle = create_bo(i915, &obj->size, region, params->oom_test);
 
-		move_to_lmem(i915, objects + i, 1, batch, engine,
+		move_to_lmem(i915, ctx, objects + i, 1, batch, engine,
 			     params->oom_test);
 		if (params->flags & TEST_VERIFY)
 			init_object(i915, obj, rand(), params->flags);
@@ -226,7 +228,7 @@ static void __do_evict(int i915,
 			idx = (idx + 1) % params->count;
 		}
 
-		move_to_lmem(i915, list, num, batch, engine, params->oom_test);
+		move_to_lmem(i915, ctx, list, num, batch, engine, params->oom_test);
 
 		if (params->flags & TEST_ENGINES)
 			engine = (engine + 1) % __num_engines__;
@@ -342,6 +344,7 @@ static void fill_params(int i915, struct params *params,
 }
 
 static void test_evict(int i915,
+		       const intel_ctx_t *ctx,
 		       struct drm_i915_memory_region_info *region,
 		       unsigned int flags)
 {
@@ -353,14 +356,17 @@ static void test_evict(int i915,
 	if (flags & TEST_PARALLEL) {
 		int fd = gem_reopen_driver(i915);
 
+		ctx = intel_ctx_create_all_physical(fd);
+		__gem_context_set_persistence(i915, ctx->id, false);
+
 		igt_fork(child, nproc)
-			__do_evict(fd, &region->region, &params,
+			__do_evict(fd, ctx, &region->region, &params,
 				   params.seed + child + 1);
 
 		igt_waitchildren();
 		close(fd);
 	} else {
-		__do_evict(i915, &region->region, &params, params.seed);
+		__do_evict(i915, ctx, &region->region, &params, params.seed);
 	}
 }
 
@@ -399,6 +405,7 @@ static void smem_oom_exit_handler(int sig)
 }
 
 static void test_smem_oom(int i915,
+			  const intel_ctx_t *ctx,
 			  struct drm_i915_memory_region_info *region)
 {
 	const uint64_t smem_size = intel_get_total_ram_mb() +
@@ -421,7 +428,7 @@ static void test_smem_oom(int i915,
 		fill_params(i915, &params, region, 0, 1, true);
 
 		igt_install_exit_handler(smem_oom_exit_handler);
-		__do_evict(fd, &region->region, &params,
+		__do_evict(fd, ctx, &region->region, &params,
 			   params.seed + child + 1);
 
 		close(fd);
@@ -513,6 +520,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
 		{ "parallel-multi", TEST_PARALLEL | TEST_RANDOM | TEST_VERIFY | TEST_ENGINES | TEST_MULTI },
 		{ }
 	};
+	const intel_ctx_t *ctx;
 	int i915 = -1;
 
 	igt_fixture {
@@ -528,17 +536,20 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
 		for_each_physical_engine(i915, e)
 			__num_engines__++;
 		igt_require(__num_engines__);
+		ctx = intel_ctx_create_all_physical(i915);
+		__gem_context_set_persistence(i915, ctx->id, false);
+
 	}
 
 	for (test = tests; test->name; test++) {
 		igt_describe("Exercise local memory swapping to system memory");
 		dynamic_lmem_subtest(region, regions, test->name)
-			test_evict(i915, region, test->flags);
+			test_evict(i915, ctx, region, test->flags);
 	}
 
 	igt_describe("Exercise local memory swapping during exhausting system memory");
 	dynamic_lmem_subtest(region, regions, "smem-oom")
-		test_smem_oom(i915, region);
+		test_smem_oom(i915, ctx, region);
 
 	igt_fixture {
 		free(regions);
-- 
2.34.1

Re: [Intel-gfx] [PATCH i-g-t 2/4] test/gem_lmem_swapping: fix physical engine usage

[Tvrtko Ursulin]

On 24/03/2022 14:26, Matthew Auld wrote:
> DG2 seems to have too many physical engines, and during execbuf just hits:

FWIW it's not that DG2 has too many engines but the test was apparently 
broken (because considering legacy eb ring selector as consecutive index 
namespace within total number of engines is wrong) during porting to 
upstream codebase.

Regards,

Tvrtko

> 
> 	"execbuf with unknown ring: 5"
> 
> Convert the test over to using the non-legacy API where we instead fill
> the ctx with all the physical engines and then engine/ring becomes the
> index into this.
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5192
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
> ---
>   tests/i915/gem_lmem_swapping.c | 27 +++++++++++++++++++--------
>   1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/i915/gem_lmem_swapping.c b/tests/i915/gem_lmem_swapping.c
> index 193ff370..995a663f 100644
> --- a/tests/i915/gem_lmem_swapping.c
> +++ b/tests/i915/gem_lmem_swapping.c
> @@ -126,6 +126,7 @@ verify_object(int i915, const struct object *obj,  unsigned int flags)
>   }
>   
>   static void move_to_lmem(int i915,
> +			 const intel_ctx_t *ctx,
>   			 struct object *list,
>   			 unsigned int num,
>   			 uint32_t batch,
> @@ -137,6 +138,7 @@ static void move_to_lmem(int i915,
>   		.buffers_ptr = to_user_pointer(obj),
>   		.buffer_count = 1 + num,
>   		.flags = I915_EXEC_NO_RELOC | I915_EXEC_HANDLE_LUT | engine,
> +		.rsvd1 = ctx->id,
>   	};
>   	unsigned int i, ret;
>   
> @@ -156,6 +158,7 @@ retry:
>   }
>   
>   static void __do_evict(int i915,
> +		       const intel_ctx_t *ctx,
>   		       struct drm_i915_gem_memory_class_instance *region,
>   		       struct params *params,
>   		       unsigned int seed)
> @@ -170,7 +173,6 @@ static void __do_evict(int i915,
>   	struct timespec t = {};
>   	unsigned int num;
>   
> -	__gem_context_set_persistence(i915, 0, false);
>   	size = 4096;
>   	batch = create_bo(i915, &size, region, params->oom_test);
>   
> @@ -201,7 +203,7 @@ static void __do_evict(int i915,
>   		}
>   		obj->handle = create_bo(i915, &obj->size, region, params->oom_test);
>   
> -		move_to_lmem(i915, objects + i, 1, batch, engine,
> +		move_to_lmem(i915, ctx, objects + i, 1, batch, engine,
>   			     params->oom_test);
>   		if (params->flags & TEST_VERIFY)
>   			init_object(i915, obj, rand(), params->flags);
> @@ -226,7 +228,7 @@ static void __do_evict(int i915,
>   			idx = (idx + 1) % params->count;
>   		}
>   
> -		move_to_lmem(i915, list, num, batch, engine, params->oom_test);
> +		move_to_lmem(i915, ctx, list, num, batch, engine, params->oom_test);
>   
>   		if (params->flags & TEST_ENGINES)
>   			engine = (engine + 1) % __num_engines__;
> @@ -342,6 +344,7 @@ static void fill_params(int i915, struct params *params,
>   }
>   
>   static void test_evict(int i915,
> +		       const intel_ctx_t *ctx,
>   		       struct drm_i915_memory_region_info *region,
>   		       unsigned int flags)
>   {
> @@ -353,14 +356,17 @@ static void test_evict(int i915,
>   	if (flags & TEST_PARALLEL) {
>   		int fd = gem_reopen_driver(i915);
>   
> +		ctx = intel_ctx_create_all_physical(fd);
> +		__gem_context_set_persistence(i915, ctx->id, false);
> +
>   		igt_fork(child, nproc)
> -			__do_evict(fd, &region->region, &params,
> +			__do_evict(fd, ctx, &region->region, &params,
>   				   params.seed + child + 1);
>   
>   		igt_waitchildren();
>   		close(fd);
>   	} else {
> -		__do_evict(i915, &region->region, &params, params.seed);
> +		__do_evict(i915, ctx, &region->region, &params, params.seed);
>   	}
>   }
>   
> @@ -399,6 +405,7 @@ static void smem_oom_exit_handler(int sig)
>   }
>   
>   static void test_smem_oom(int i915,
> +			  const intel_ctx_t *ctx,
>   			  struct drm_i915_memory_region_info *region)
>   {
>   	const uint64_t smem_size = intel_get_total_ram_mb() +
> @@ -421,7 +428,7 @@ static void test_smem_oom(int i915,
>   		fill_params(i915, &params, region, 0, 1, true);
>   
>   		igt_install_exit_handler(smem_oom_exit_handler);
> -		__do_evict(fd, &region->region, &params,
> +		__do_evict(fd, ctx, &region->region, &params,
>   			   params.seed + child + 1);
>   
>   		close(fd);
> @@ -513,6 +520,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>   		{ "parallel-multi", TEST_PARALLEL | TEST_RANDOM | TEST_VERIFY | TEST_ENGINES | TEST_MULTI },
>   		{ }
>   	};
> +	const intel_ctx_t *ctx;
>   	int i915 = -1;
>   
>   	igt_fixture {
> @@ -528,17 +536,20 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>   		for_each_physical_engine(i915, e)
>   			__num_engines__++;
>   		igt_require(__num_engines__);
> +		ctx = intel_ctx_create_all_physical(i915);
> +		__gem_context_set_persistence(i915, ctx->id, false);
> +
>   	}
>   
>   	for (test = tests; test->name; test++) {
>   		igt_describe("Exercise local memory swapping to system memory");
>   		dynamic_lmem_subtest(region, regions, test->name)
> -			test_evict(i915, region, test->flags);
> +			test_evict(i915, ctx, region, test->flags);
>   	}
>   
>   	igt_describe("Exercise local memory swapping during exhausting system memory");
>   	dynamic_lmem_subtest(region, regions, "smem-oom")
> -		test_smem_oom(i915, region);
> +		test_smem_oom(i915, ctx, region);
>   
>   	igt_fixture {
>   		free(regions);

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 2/4] test/gem_lmem_swapping: fix physical engine usage

[Dixit, Ashutosh]

On Thu, 24 Mar 2022 07:26:19 -0700, Matthew Auld wrote:
>
> @@ -353,14 +356,17 @@ static void test_evict(int i915,
>	if (flags & TEST_PARALLEL) {
>		int fd = gem_reopen_driver(i915);
>
> +		ctx = intel_ctx_create_all_physical(fd);
> +		__gem_context_set_persistence(i915, ctx->id, false);
> +
>		igt_fork(child, nproc)
> -			__do_evict(fd, &region->region, &params,
> +			__do_evict(fd, ctx, &region->region, &params,
>				   params.seed + child + 1);
>
>		igt_waitchildren();
>		close(fd);

We might introduce some memory leak tests some time so it would be good to
destroy any contexts we create with intel_ctx_destroy().

> @@ -528,17 +536,20 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>		for_each_physical_engine(i915, e)
>			__num_engines__++;
>		igt_require(__num_engines__);
> +		ctx = intel_ctx_create_all_physical(i915);
> +		__gem_context_set_persistence(i915, ctx->id, false);
> +
>	}
>
>	for (test = tests; test->name; test++) {
>		igt_describe("Exercise local memory swapping to system memory");
>		dynamic_lmem_subtest(region, regions, test->name)
> -			test_evict(i915, region, test->flags);
> +			test_evict(i915, ctx, region, test->flags);
>	}
>
>	igt_describe("Exercise local memory swapping during exhausting system memory");
>	dynamic_lmem_subtest(region, regions, "smem-oom")
> -		test_smem_oom(i915, region);
> +		test_smem_oom(i915, ctx, region);
>
>	igt_fixture {
>		free(regions);

Here too.

[Intel-gfx] [PATCH i-g-t 3/4] tests/gem_lmem_swapping: limit lmem to 4G

[Matthew Auld]

From: CQ Tang <cq.tang@intel.com>

On some systems lmem can be as large as 16G, which seems to trigger
various CI timeouts, and in the best case just takes a long time. For
the purposes of the test we should be able to limit to 4G, without any
big loss in coverage.

Signed-off-by: CQ Tang <cq.tang@intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
---
 tests/i915/gem_lmem_swapping.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tests/i915/gem_lmem_swapping.c b/tests/i915/gem_lmem_swapping.c
index 995a663f..ad1c989c 100644
--- a/tests/i915/gem_lmem_swapping.c
+++ b/tests/i915/gem_lmem_swapping.c
@@ -526,7 +526,13 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
 	igt_fixture {
 		struct intel_execution_engine2 *e;
 
-		i915 = drm_open_driver(DRIVER_INTEL);
+		igt_i915_driver_unload();
+		if (igt_i915_driver_load("lmem_size=4096")) {
+			igt_debug("i915 missing lmem_size modparam support\n");
+			igt_assert_eq(igt_i915_driver_load(NULL), 0);
+		}
+
+		i915 = __drm_open_driver(DRIVER_INTEL);
 		igt_require_gem(i915);
 		igt_require(gem_has_lmem(i915));
 
@@ -554,6 +560,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
 	igt_fixture {
 		free(regions);
 		close(i915);
+		igt_i915_driver_unload();
 	}
 
 	igt_exit();
-- 
2.34.1

Re: [Intel-gfx] [PATCH i-g-t 3/4] tests/gem_lmem_swapping: limit lmem to 4G

[Thomas Hellström]

On 3/24/22 15:26, Matthew Auld wrote:
> From: CQ Tang <cq.tang@intel.com>
>
> On some systems lmem can be as large as 16G, which seems to trigger
> various CI timeouts, and in the best case just takes a long time. For
> the purposes of the test we should be able to limit to 4G, without any
> big loss in coverage.
>
> Signed-off-by: CQ Tang <cq.tang@intel.com>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Nirmoy Das <nirmoy.das@linux.intel.com>

Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>



> ---
>   tests/i915/gem_lmem_swapping.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/tests/i915/gem_lmem_swapping.c b/tests/i915/gem_lmem_swapping.c
> index 995a663f..ad1c989c 100644
> --- a/tests/i915/gem_lmem_swapping.c
> +++ b/tests/i915/gem_lmem_swapping.c
> @@ -526,7 +526,13 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>   	igt_fixture {
>   		struct intel_execution_engine2 *e;
>   
> -		i915 = drm_open_driver(DRIVER_INTEL);
> +		igt_i915_driver_unload();
> +		if (igt_i915_driver_load("lmem_size=4096")) {
> +			igt_debug("i915 missing lmem_size modparam support\n");
> +			igt_assert_eq(igt_i915_driver_load(NULL), 0);
> +		}
> +
> +		i915 = __drm_open_driver(DRIVER_INTEL);
>   		igt_require_gem(i915);
>   		igt_require(gem_has_lmem(i915));
>   
> @@ -554,6 +560,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>   	igt_fixture {
>   		free(regions);
>   		close(i915);
> +		igt_i915_driver_unload();
>   	}
>   
>   	igt_exit();

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/4] tests/gem_lmem_swapping: limit lmem to 4G

[Dixit, Ashutosh]

On Thu, 24 Mar 2022 07:26:20 -0700, Matthew Auld wrote:
>
> @@ -554,6 +560,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>	igt_fixture {
>		free(regions);
>		close(i915);
> +		igt_i915_driver_unload();

I thought we'd reload the module with default params here but when the next
test runs the module gets loaded automatically so maybe this is ok?

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/4] tests/gem_lmem_swapping: limit lmem to 4G

[Matthew Auld]

On 24/03/2022 17:47, Dixit, Ashutosh wrote:
> On Thu, 24 Mar 2022 07:26:20 -0700, Matthew Auld wrote:
>>
>> @@ -554,6 +560,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>> 	igt_fixture {
>> 		free(regions);
>> 		close(i915);
>> +		igt_i915_driver_unload();
> 
> I thought we'd reload the module with default params here but when the next
> test runs the module gets loaded automatically so maybe this is ok?

Yeah, that at least matches my understanding. Adding Petri in case he 
has some comments here.

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/4] tests/gem_lmem_swapping: limit lmem to 4G

[Petri Latvala]

On Fri, Mar 25, 2022 at 08:40:45AM +0000, Matthew Auld wrote:
> On 24/03/2022 17:47, Dixit, Ashutosh wrote:
> > On Thu, 24 Mar 2022 07:26:20 -0700, Matthew Auld wrote:
> > > 
> > > @@ -554,6 +560,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
> > > 	igt_fixture {
> > > 		free(regions);
> > > 		close(i915);
> > > +		igt_i915_driver_unload();
> > 
> > I thought we'd reload the module with default params here but when the next
> > test runs the module gets loaded automatically so maybe this is ok?
> 
> Yeah, that at least matches my understanding. Adding Petri in case he has
> some comments here.

Yes, the convention is to either leave the module loaded with
defaults, or leave it unloaded. If the next test happens to be one
that wants to load the module with different params, we save some
time.

If loading the module again doesn't work we should see some fireworks
in CI results elsewhere anyway. Due to module loading problems we used
to limit them to known places (reloading tests, selftests, ...) so we
might need to revisit this topic later. But no need to FUD it at this
time.



-- 
Petri Latvala

Re: [Intel-gfx] [PATCH i-g-t 3/4] tests/gem_lmem_swapping: limit lmem to 4G

[Das, Nirmoy]

Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>

On 3/24/2022 3:26 PM, Matthew Auld wrote:
> From: CQ Tang <cq.tang@intel.com>
>
> On some systems lmem can be as large as 16G, which seems to trigger
> various CI timeouts, and in the best case just takes a long time. For
> the purposes of the test we should be able to limit to 4G, without any
> big loss in coverage.
>
> Signed-off-by: CQ Tang <cq.tang@intel.com>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
> ---
>   tests/i915/gem_lmem_swapping.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/tests/i915/gem_lmem_swapping.c b/tests/i915/gem_lmem_swapping.c
> index 995a663f..ad1c989c 100644
> --- a/tests/i915/gem_lmem_swapping.c
> +++ b/tests/i915/gem_lmem_swapping.c
> @@ -526,7 +526,13 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>   	igt_fixture {
>   		struct intel_execution_engine2 *e;
>   
> -		i915 = drm_open_driver(DRIVER_INTEL);
> +		igt_i915_driver_unload();
> +		if (igt_i915_driver_load("lmem_size=4096")) {
> +			igt_debug("i915 missing lmem_size modparam support\n");
> +			igt_assert_eq(igt_i915_driver_load(NULL), 0);
> +		}
> +
> +		i915 = __drm_open_driver(DRIVER_INTEL);
>   		igt_require_gem(i915);
>   		igt_require(gem_has_lmem(i915));
>   
> @@ -554,6 +560,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>   	igt_fixture {
>   		free(regions);
>   		close(i915);
> +		igt_i915_driver_unload();
>   	}
>   
>   	igt_exit();

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/4] tests/gem_lmem_swapping: limit lmem to 4G

[Petri Latvala]

On Thu, Mar 24, 2022 at 02:26:20PM +0000, Matthew Auld wrote:
> From: CQ Tang <cq.tang@intel.com>
> 
> On some systems lmem can be as large as 16G, which seems to trigger
> various CI timeouts, and in the best case just takes a long time. For
> the purposes of the test we should be able to limit to 4G, without any
> big loss in coverage.
> 
> Signed-off-by: CQ Tang <cq.tang@intel.com>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
> ---
>  tests/i915/gem_lmem_swapping.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/i915/gem_lmem_swapping.c b/tests/i915/gem_lmem_swapping.c
> index 995a663f..ad1c989c 100644
> --- a/tests/i915/gem_lmem_swapping.c
> +++ b/tests/i915/gem_lmem_swapping.c
> @@ -526,7 +526,13 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>  	igt_fixture {
>  		struct intel_execution_engine2 *e;
>  
> -		i915 = drm_open_driver(DRIVER_INTEL);
> +		igt_i915_driver_unload();
> +		if (igt_i915_driver_load("lmem_size=4096")) {
> +			igt_debug("i915 missing lmem_size modparam support\n");
> +			igt_assert_eq(igt_i915_driver_load(NULL), 0);
> +		}

Does the driver load truly fail with an unknown param?


-- 
Petri Latvala



> +
> +		i915 = __drm_open_driver(DRIVER_INTEL);
>  		igt_require_gem(i915);
>  		igt_require(gem_has_lmem(i915));
>  
> @@ -554,6 +560,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>  	igt_fixture {
>  		free(regions);
>  		close(i915);
> +		igt_i915_driver_unload();
>  	}
>  
>  	igt_exit();
> -- 
> 2.34.1
> 

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/4] tests/gem_lmem_swapping: limit lmem to 4G

[Matthew Auld]

On 25/03/2022 10:18, Petri Latvala wrote:
> On Thu, Mar 24, 2022 at 02:26:20PM +0000, Matthew Auld wrote:
>> From: CQ Tang <cq.tang@intel.com>
>>
>> On some systems lmem can be as large as 16G, which seems to trigger
>> various CI timeouts, and in the best case just takes a long time. For
>> the purposes of the test we should be able to limit to 4G, without any
>> big loss in coverage.
>>
>> Signed-off-by: CQ Tang <cq.tang@intel.com>
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>> Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
>> ---
>>   tests/i915/gem_lmem_swapping.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/i915/gem_lmem_swapping.c b/tests/i915/gem_lmem_swapping.c
>> index 995a663f..ad1c989c 100644
>> --- a/tests/i915/gem_lmem_swapping.c
>> +++ b/tests/i915/gem_lmem_swapping.c
>> @@ -526,7 +526,13 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>>   	igt_fixture {
>>   		struct intel_execution_engine2 *e;
>>   
>> -		i915 = drm_open_driver(DRIVER_INTEL);
>> +		igt_i915_driver_unload();
>> +		if (igt_i915_driver_load("lmem_size=4096")) {
>> +			igt_debug("i915 missing lmem_size modparam support\n");
>> +			igt_assert_eq(igt_i915_driver_load(NULL), 0);
>> +		}
> 
> Does the driver load truly fail with an unknown param?

Yeah, I have since removed that. From the CI logs it just loads anyway...

> 
> 

[Intel-gfx] [PATCH i-g-t 4/4] tests/gem_lmem_swapping: reduce the timeout

[Matthew Auld]

300s is way too much for some BAT test. Drop it down to 45s.

Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
---
 tests/i915/gem_lmem_swapping.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/i915/gem_lmem_swapping.c b/tests/i915/gem_lmem_swapping.c
index ad1c989c..6c47bff2 100644
--- a/tests/i915/gem_lmem_swapping.c
+++ b/tests/i915/gem_lmem_swapping.c
@@ -219,7 +219,7 @@ static void __do_evict(int i915,
 	 * For TEST_MULTI runs, make each object counts a loop to
 	 * avoid excessive run times.
 	 */
-	for (l = 0; l < params->loops && igt_seconds_elapsed(&t) < 300; l += num) {
+	for (l = 0; l < params->loops && igt_seconds_elapsed(&t) < 45; l += num) {
 		unsigned int idx = rand() % params->count;
 
 		num = params->flags & TEST_MULTI ? rand() % max_swap_in + 1 : 1;
-- 
2.34.1

Re: [Intel-gfx] [PATCH i-g-t 4/4] tests/gem_lmem_swapping: reduce the timeout

[Thomas Hellström]

On 3/24/22 15:26, Matthew Auld wrote:
> 300s is way too much for some BAT test. Drop it down to 45s.
>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
> ---
>   tests/i915/gem_lmem_swapping.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/i915/gem_lmem_swapping.c b/tests/i915/gem_lmem_swapping.c
> index ad1c989c..6c47bff2 100644
> --- a/tests/i915/gem_lmem_swapping.c
> +++ b/tests/i915/gem_lmem_swapping.c
> @@ -219,7 +219,7 @@ static void __do_evict(int i915,
>   	 * For TEST_MULTI runs, make each object counts a loop to
>   	 * avoid excessive run times.
>   	 */
> -	for (l = 0; l < params->loops && igt_seconds_elapsed(&t) < 300; l += num) {
> +	for (l = 0; l < params->loops && igt_seconds_elapsed(&t) < 45; l += num) {
>   		unsigned int idx = rand() % params->count;
>   
>   		num = params->flags & TEST_MULTI ? rand() % max_swap_in + 1 : 1;

Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>