Re: [PATCH i-g-t v14 3/5] lib/igt_multigpu: Introduce library for multi-GPU scenarios

[Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>]

Hi Kamil,

On Wednesday, 20 March 2024 16:56:57 CET Kamil Konieczny wrote:
> From: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
> 
> Implemented igt_require_multigpu in order to replace
> igt_require(gpu_count > 1), as well as printing available
> PCI devices if requirement fails.
> 
> Introduced igt_multigpu_count_class function that returns an actual
> number of GPUs present in system, which allows for writing multi-GPU
> test scenarios that does not require filter
> --device pci:vendor=intel,device=discrete,card=all
> to run as intended. Based on patch by Chris Wilson.
> 
> Introduced igt_multi_fork_foreach_gpu macro that helps with
> writing multi-GPU test scenarios in idiomatic form:
> 
> igt_multi_fork_foreach_gpu(i915, DRIVER_INTEL)
> 	test_function(i915);
> igt_waitchildren();
> 
> This is multi-GPU ready as it will run also on single GPU
> board. For running tests on two or more GPUs there is new
> igt_multi_fork_foreach_multigpu macro.
> 
> v10: squashed two commits which introduce multigpu functions (Zbigniew)
>   renamed __id and id__ into __chipset/chipset__ (Zbigniew)
>   used __drm_close_driver() instead of close() in first macro (Kamil)
> v11: added functions descriptions (Janusz)
> v12: moved to igt_multigpu.c/h (Janusz)
>  changed names from gem_ to igt_, removed '\' from end of macro (Janusz)
> v13: added new macro igt_multi_fork_foreach_multigpu which will require
>  two or more GPUs, changing old igt_multi_fork_foreach_gpu to work with
>  one or more GPUs, so tests using _gpu will be multi-GPU ready (Janusz)
> v14: dropped multigpu_open_another and replace it with
>  drm_open_driver_another, extending description (Janusz)
> 
> Cc: "Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>
> Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> Signed-off-by: "Dominik Karol Piątkowski" <dominik.karol.piatkowski@intel.com>
> [Kamil: fixed whitespace and tabs, moved to lib/igt_multigpu.*]
> Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> ---
>  lib/igt_multigpu.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_multigpu.h | 36 +++++++++++++++++++
>  lib/meson.build    |  1 +
>  3 files changed, 125 insertions(+)
>  create mode 100644 lib/igt_multigpu.c
>  create mode 100644 lib/igt_multigpu.h
> 
> diff --git a/lib/igt_multigpu.c b/lib/igt_multigpu.c
> new file mode 100644
> index 000000000..be0c11332
> --- /dev/null
> +++ b/lib/igt_multigpu.c
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#include "drmtest.h"
> +#include "i915/gem.h"
> +#include "igt_core.h"
> +#include "igt_device_scan.h"
> +#include "igt_multigpu.h"
> +
> +/**
> + * igt_multigpu_count_class:
> + * @class: chipset, e.g. DRIVER_XE or DRIVER_INTEL
> + *
> + * Function counts number of GPU cards with the help of opening all of them.
> + *
> + * Returns: number of GPUs cards found
> + */
> +int igt_multigpu_count_class(int class)
> +{
> +	int count = 0;
> +
> +	igt_foreach_gpu(fd, class)
> +		count++;
> +
> +	return count;
> +}
> +
> +static int print_gpus(int count, int gpu_num)
> +{
> +	struct igt_devices_print_format fmt = {
> +		.type = IGT_PRINT_SIMPLE,
> +		.option = IGT_PRINT_PCI,
> +	};
> +	int devices;
> +
> +	igt_info("PCI devices available in the system:\n");
> +
> +	igt_devices_scan(true);
> +	devices = igt_device_filter_pci();
> +	igt_devices_print(&fmt);
> +
> +	return devices;
> +}
> +
> +/**
> + * igt_require_filtered_multigpu:
> + * @count: minimum number of GPUs required found with filters
> + *
> + * Function checks number of filtered GPU cards.
> + * On error prints available GPUs found on PCI bus and skips.
> + */
> +int igt_require_filtered_multigpu(int gpus_wanted)

NIT: Not used by any other function withing this series, then probably belongs 
to another set of patches.

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

> +{
> +	int gpu_count = igt_device_filter_count();
> +	int num;
> +
> +	if (gpu_count >= gpus_wanted)
> +		return gpu_count;
> +
> +	num = print_gpus(gpus_wanted, gpu_count);
> +	igt_skip_on_f(gpu_count < gpus_wanted, "Test requires at least %d GPUs, got %d, available: %d\n", gpus_wanted, gpu_count, num);
> +
> +	return 0; /* unreachable */
> +}
> +
> +/**
> + * igt_require_multigpu:
> + * @count: minimum number of GPUs required
> + * @chipset: for example DRIVER_XE or DRIVER_INTEL
> + *
> + * Function checks number of GPU cards with __drm_open_driver_another()
> + * On error prints available GPUs found on PCI bus and skips.
> + */
> +int igt_require_multigpu(int gpus_wanted, unsigned int chipset)
> +{
> +	int gpu_filters = igt_multigpu_count_class(chipset);
> +	int num;
> +
> +	if (gpu_filters >= gpus_wanted)
> +		return gpu_filters;
> +
> +	num = print_gpus(gpus_wanted, gpu_filters);
> +	igt_skip_on_f(gpu_filters < gpus_wanted, "Test requires at least %d GPUs, got %d, available: %d\n", gpus_wanted, gpu_filters, num);
> +
> +	return 0; /* unreachable */
> +}
> diff --git a/lib/igt_multigpu.h b/lib/igt_multigpu.h
> new file mode 100644
> index 000000000..b002f98e0
> --- /dev/null
> +++ b/lib/igt_multigpu.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#ifndef __INTEL_MULTIGPU_H
> +#define __INTEL_MULTIGPU_H
> +
> +#include "drmtest.h"
> +#include "igt_core.h"
> +
> +int igt_multigpu_count_class(int chipset);
> +int igt_require_filtered_multigpu(int count);
> +int igt_require_multigpu(int count, unsigned int chipset);
> +
> +#define igt_foreach_gpu(fd__, chipset__) \
> +	for (int igt_unique(i) = 0, fd__; \
> +		(fd__ = __drm_open_driver_another(igt_unique(i)++, (chipset__))) >= 0; \
> +		__drm_close_driver(fd__))
> +
> +#define igt_multi_fork_foreach_gpu_num(__fd, __gpu_idx, __chipset, __wanted) \
> +	for (int igt_unique(__j) = igt_require_multigpu((__wanted), (__chipset)); \
> +	     igt_unique(__j) != -1; \
> +	     igt_unique(__j) = -1) \
> +		igt_multi_fork(__gpu_idx, igt_unique(__j)) \
> +			for (int __fd = drm_open_driver_another(__gpu_idx, (__chipset)); \
> +			     __fd >= 0; \
> +			     drm_close_driver(__fd), __fd = -1)
> +
> +#define igt_multi_fork_foreach_gpu(__fd, __gpu_idx, __chipset) \
> +		igt_multi_fork_foreach_gpu_num(__fd, __gpu_idx, (__chipset), 1)
> +
> +#define igt_multi_fork_foreach_multigpu(__fd, __gpu_idx, __chipset) \
> +		igt_multi_fork_foreach_gpu_num(__fd, __gpu_idx, (__chipset), 2)
> +
> +#endif /* __INTEL_MULTIGPU_H */
> diff --git a/lib/meson.build b/lib/meson.build
> index 934bac5c6..a5651571b 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -66,6 +66,7 @@ lib_sources = [
>  	'intel_device_info.c',
>  	'intel_mmio.c',
>  	'intel_mocs.c',
> +	'igt_multigpu.c',
>  	'intel_pat.c',
>  	'ioctl_wrappers.c',
>  	'media_spin.c',
>