diff mbox series

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

Message ID 20220324142621.347452-4-matthew.auld@intel.com (mailing list archive)
State New, archived
Headers show
Series [i-g-t,1/4] test/gem_lmem_swapping: account for object rounding | expand

Commit Message

Matthew Auld March 24, 2022, 2:26 p.m. UTC
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(-)

Comments

Thomas Hellstrom March 24, 2022, 2:33 p.m. UTC | #1
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();
Dixit, Ashutosh March 24, 2022, 5:47 p.m. UTC | #2
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?
Matthew Auld March 25, 2022, 8:40 a.m. UTC | #3
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.
Petri Latvala March 25, 2022, 10:02 a.m. UTC | #4
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.
Nirmoy Das March 25, 2022, 10:13 a.m. UTC | #5
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();
Petri Latvala March 25, 2022, 10:18 a.m. UTC | #6
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?
Matthew Auld March 25, 2022, 10:20 a.m. UTC | #7
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...

> 
>
diff mbox series

Patch

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();