Re: [igt-dev] [PATCH i-g-t 5/7] tests/i915/gem_basic: add multigpu-create-close subtest

[Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>]

On Fri,  2 Dec 2022 21:57:03 +0100
Kamil Konieczny <kamil.konieczny@linux.intel.com> wrote:

> Add one subtest to run on more than one gpu simultanosly. For
> this to work test should be run with --device option on machine
> with two or more dgfx cards, for example with:
> 
> --device=pci:vendor=Intel,device=discrete,card=all
> 
> Test will skip if only one card is present.
> 
> Cc: Anna Karas <anna.karas@intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Cc: Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> ---
>  tests/i915/gem_basic.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/i915/gem_basic.c b/tests/i915/gem_basic.c
> index 2aa5d850..9ee09782 100644
> --- a/tests/i915/gem_basic.c
> +++ b/tests/i915/gem_basic.c
> @@ -40,6 +40,7 @@
>  #include "i915/gem_create.h"
>  #include "igt.h"
>  #include "igt_types.h"
> +#include "lib/igt_device_scan.h"
>  
>  IGT_TEST_DESCRIPTION("Tests basic gem_create and gem_close IOCTLs");
>  
> @@ -83,10 +84,16 @@ test_create_fd_close(int fd)
>  igt_main
>  {
>  	igt_fd_t(fd);
> +	int gpu_count;
>  
> -	igt_fixture
> +	igt_fixture {
> +		int count;
>  		fd = drm_open_driver(DRIVER_INTEL);
>  
> +		count = igt_device_filter_count();
> +		gpu_count = count >= 2 ? count : 1;

Hmm... I would just do:

		gpu_count = igt_device_filter_count();

As it seems that you're just wanting to ensure that gpu_count
is >= 1 here...

> +	}
> +
>  	igt_describe("Verify that gem_close fails with bad params.");
>  	igt_subtest("bad-close")
>  		test_bad_close(fd);
> @@ -95,6 +102,21 @@ igt_main
>  	igt_subtest("create-close")
>  		test_create_close(fd);
>  
> +	igt_describe("Verify basic functionality of gem_create and gem_close on multi-GPU.");
> +	igt_subtest("multigpu-create-close") {
> +		igt_require(gpu_count > 1);

... Yet, this is the only place where you use gpu_count. So, count <= 0
won't cause any troubles.

> +		igt_multi_fork(child, gpu_count) {
> +			int gpu_fd;
> +
> +			gpu_fd = __drm_open_driver_another(child, DRIVER_INTEL);
> +			igt_assert_f(gpu_fd > 0, "cannot open gpu-%d, errno=%d\n", child, errno);
> +			test_create_close(gpu_fd);
> +			close(gpu_fd);
> +		}
> +
> +		igt_waitchildren();
> +	}
> +
>  	igt_describe("Verify that closing drm driver is possible with opened gem object.");
>  	igt_subtest("create-fd-close")
>  		test_create_fd_close(fd);

Once applying this cleanup, feel free to add by RB:

Reviewed-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Regards,
Mauro