[PATCH] tests/intel/xe_create: Isolate performance tests

[Kamil Konieczny]

Hi Balasubramani,
On 2024-06-07 at 15:19:36 +0530, Balasubramani Vivekanandan wrote:

please subscribe to igt-dev mailinglist.

If you will ever send cc to kmd list, please add i-g-t after PATCH.

> create_execqueues subtest is testing the time required for creating
> execqueues along with testing the functionality.
> Performance measurements are mostly not relevant when testing on PreSi
> environments. Isolate the performance checks into new tests allowing it
> to be skipped for PreSi testing.
> 
> Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan at intel.com>
> ---
>  tests/intel/xe_create.c | 28 +++++++++++++++++++---------
>  1 file changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/intel/xe_create.c b/tests/intel/xe_create.c
> index d41557c85..346c807c0 100644
> --- a/tests/intel/xe_create.c
> +++ b/tests/intel/xe_create.c
> @@ -171,9 +171,11 @@ enum vm_count {
>   * @leak:				destroy exec_queues in close() path
>   * @noleak-shared:			same as noleak, but with a shared vm
>   * @leak-shared:			same as leak, but with a shared vm
> + * @performance:			Measure time required for creating execqueues
> + * @performance-shared:			same as performance, but with a shared vm

You didn't created any new test, the only thing changed is skipping
assert.

>   */
>  static void create_execqueues(int fd, enum exec_queue_destroy ed,
> -			      enum vm_count vc)
> +			      enum vm_count vc, bool perf)

Instead of bool parameter imho it is better to check for simulation.

>  {
>  	struct timespec tv = { };
>  	uint32_t num_engines, exec_queues_per_process, vm;
> @@ -237,10 +239,12 @@ static void create_execqueues(int fd, enum exec_queue_destroy ed,
>  		drm_close_driver(fd);
>  	}
>  
> -	seconds = igt_seconds_elapsed(&tv);
> -	igt_assert_f(seconds < real_timeout,
> -		     "Creating %d exec_queues tooks too long: %d [limit: %d]\n",
> -		     MAXEXECQUEUES, seconds, real_timeout);
> +	if (perf) {
> +		seconds = igt_seconds_elapsed(&tv);
> +		igt_assert_f(seconds < real_timeout,
> +			     "Creating %d exec_queues tooks too long: %d [limit: %d]\n",
> +			     MAXEXECQUEUES, seconds, real_timeout);
> +	}

imho you could still print this info and only assert if not in simulation,
so your change will be only here like:

	seconds = igt_seconds_elapsed(&tv);
    if (igt_run_in_simulation())
		     igt_info("Creating %d exec_queues tooks: %d [limit: %d]\n",
		     MAXEXECQUEUES, seconds, real_timeout);
    else
	        igt_assert_f(seconds < real_timeout,
		                "Creating %d exec_queues tooks too long: %d [limit: %d]\n",
                        MAXEXECQUEUES, seconds, real_timeout);

Regards,
Kamil

>  }
>  
>  /**
> @@ -410,16 +414,22 @@ igt_main_args("Q:p:", NULL, help_str, opt_handler, NULL)
>  	}
>  
>  	igt_subtest("create-execqueues-noleak")
> -		create_execqueues(xe, NOLEAK, MULTI);
> +		create_execqueues(xe, NOLEAK, MULTI, false);
>  
>  	igt_subtest("create-execqueues-leak")
> -		create_execqueues(xe, LEAK, MULTI);
> +		create_execqueues(xe, LEAK, MULTI, false);
>  
>  	igt_subtest("create-execqueues-noleak-shared")
> -		create_execqueues(xe, NOLEAK, SHARED);
> +		create_execqueues(xe, NOLEAK, SHARED, false);
>  
>  	igt_subtest("create-execqueues-leak-shared")
> -		create_execqueues(xe, LEAK, SHARED);
> +		create_execqueues(xe, LEAK, SHARED, false);
> +
> +	igt_subtest("create-execqueues-performance")
> +		create_execqueues(xe, NOLEAK, MULTI, true);
> +
> +	igt_subtest("create-execqueues-performance-shared")
> +		create_execqueues(xe, NOLEAK, SHARED, true);
>  
>  	igt_subtest("create-massive-size") {
>  		create_massive_size(xe);
> -- 
> 2.25.1
>