[PATCH i-g-t v14 5/5] tests/intel/gem_exec_gttfill: simplify multiGPU subtest

[Janusz Krzysztofik]

Hi Kamil,

On Wednesday, 20 March 2024 16:56:59 CET Kamil Konieczny wrote:
> Simplify multi-GPU subtest with the help of new multigpu library.
> 
> v10: remove igt_require_multigpu() as it checks filters but we
>   want to use legacy opens (Kamil), correct Cc name (Zbigniew)
> v14: add missing igt_require_gem() in multigpu-basic (Janusz)
> 
> Cc: "Zbigniew Kempczyński" <zbigniew.kempczynski at intel.com>
> Cc: "Dominik Karol Piątkowski" <dominik.karol.piatkowski at intel.com>
> Cc: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
> Signed-off-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> ---
>  tests/intel/gem_exec_gttfill.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/intel/gem_exec_gttfill.c b/tests/intel/gem_exec_gttfill.c
> index b1063437b..d79a6ffd5 100644
> --- a/tests/intel/gem_exec_gttfill.c
> +++ b/tests/intel/gem_exec_gttfill.c
> @@ -24,8 +24,8 @@
>  #include "i915/gem.h"
>  #include "i915/gem_create.h"
>  #include "igt.h"
> -#include "igt_device_scan.h"
>  #include "igt_rand.h"
> +#include "igt_multigpu.h"

NIT: Don't we want to follow alphabetic ordering of headers?

>  /**
>   * TEST: gem exec gttfill
>   * Description: Fill the GTT with batches.
> @@ -251,7 +251,7 @@ igt_main
>  {
>  	const struct intel_execution_engine2 *e;
>  	const intel_ctx_t *ctx;
> -	int i915 = -1, gpu_count;
> +	int i915 = -1;
>  
>  	igt_fixture {
>  		i915 = drm_open_driver(DRIVER_INTEL);
> @@ -285,17 +285,12 @@ igt_main
>  	igt_fixture {
>  		igt_stop_hang_detector();
>  		intel_ctx_destroy(i915, ctx);
> -		// prepare multigpu tests
> -		gpu_count = igt_device_filter_count();
>  	}
>  
>  	igt_subtest("multigpu-basic") { /* run on two or more discrete cards */
> -		igt_require(gpu_count > 1);
> -		igt_multi_fork(child, gpu_count) {
> -			int g_fd;
> +		igt_multi_fork_foreach_gpu(g_fd, gpu_idx, DRIVER_INTEL) {
>  			// prepare
> -			g_fd = __drm_open_driver_another(child, DRIVER_INTEL);
> -			igt_assert(g_fd >= 0);
> +			igt_require_gem(g_fd);

NIT: Not needed -- as you stated, it is already called, for any subtest, from 
initial igt_fixture section.

Anyway,
Reviewed-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>

>  			ctx = intel_ctx_create_all_physical(g_fd);
>  			igt_fork_hang_detector(g_fd);
>  			// subtest
> @@ -303,7 +298,6 @@ igt_main
>  			// release resources
>  			igt_stop_hang_detector();
>  			intel_ctx_destroy(g_fd, ctx);
> -			drm_close_driver(g_fd);
>  		}
>  
>  		igt_waitchildren();
>