[PATCH i-g-t v2 0/8] introduce Xe multigpu and other multi-GPU helpers

[PATCH i-g-t v2 0/8] introduce Xe multigpu and other multi-GPU helpers

[Kamil Konieczny]

Introduce some multi-gpu function helpers and macros. Change in drmtest
base opening function allows it to use __drm_open_driver_another(N, ...)
out of order, for example on board with three discrete GPUs:
__drm_open_driver_another(2, DRIVER_INTEL);
__drm_open_driver_another(0, DRIVER_INTEL);

Second solution for Xe is based on filtered views. Both allows to
quickly write tests. There is still drawback of not printing in
children logs <g:gpu-number> for first opened device. I renamed
lib from lib/i915/igt_multigpu.* to lib/intel_multigu.* as I do not
know of any non Intel developer using igt for such tests.

v2: corrected two patches which introduced multigpu lib (Dominik)
  rebased patch intoducing Xe multigpu macro, corrected description

Dominik Karol Piątkowski (4):
  lib/igt_device_scan: Introduce filtering out non-PCI devices
  lib/drmtest: Introduced drm_open_driver_another
  lib/intel_multigpu: Introduce library for multi-GPU scenarios
  lib/intel_multigpu: Introduced gem_multigpu_count_class and
    igt_multi_fork_foreach_gpu

Kamil Konieczny (4):
  lib/drmtest: allow opening cards out of order
  lib/intel_multigpu: Add xe_multi_fork_foreach_gpu
  tests/intel/xe_exec_basic: add multigpu subtests
  tests/intel/gem_mmap: add basic multi-GPU subtest

 lib/drmtest.c               | 119 +++++++++++++++++++++++++++++-----
 lib/drmtest.h               |   1 +
 lib/igt_device_scan.c       |  21 ++++++
 lib/igt_device_scan.h       |   2 +
 lib/intel_multigpu.c        | 125 ++++++++++++++++++++++++++++++++++++
 lib/intel_multigpu.h        |  36 +++++++++++
 lib/meson.build             |   1 +
 tests/intel/gem_mmap.c      |  77 ++++++++++++++--------
 tests/intel/xe_exec_basic.c |  36 +++++++++++
 9 files changed, 374 insertions(+), 44 deletions(-)
 create mode 100644 lib/intel_multigpu.c
 create mode 100644 lib/intel_multigpu.h

-- 
2.42.0

[PATCH i-g-t v2 1/8] lib/igt_device_scan: Introduce filtering out non-PCI devices

[Kamil Konieczny]

From: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>

Introduced igt_device_filter_pci function that filters out non-PCI
devices from IGT devices.

Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
---
 lib/igt_device_scan.c | 21 +++++++++++++++++++++
 lib/igt_device_scan.h |  2 ++
 2 files changed, 23 insertions(+)

diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
index fbf3fa482..37ebe1964 100644
--- a/lib/igt_device_scan.c
+++ b/lib/igt_device_scan.c
@@ -1908,6 +1908,27 @@ const char *igt_device_filter_get(int num)
 	return NULL;
 }
 
+/**
+ * igt_device_filter_pci
+ *
+ * Filter devices to PCI only.
+ *
+ * Returns PCI devices count.
+ */
+int igt_device_filter_pci(void)
+{
+	int count = 0;
+	struct igt_device *dev, *tmp;
+
+	igt_list_for_each_entry_safe(dev, tmp, &igt_devs.filtered, link)
+		if (strcmp(dev->subsystem, "pci") != 0)
+			igt_list_del(&dev->link);
+		else
+			count++;
+
+	return count;
+}
+
 static bool igt_device_filter_apply(const char *fstr)
 {
 	struct igt_device *dev, *tmp;
diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
index 48690e236..908733745 100644
--- a/lib/igt_device_scan.h
+++ b/lib/igt_device_scan.h
@@ -81,6 +81,8 @@ int igt_device_filter_add(const char *filter);
 void igt_device_filter_free_all(void);
 const char *igt_device_filter_get(int num);
 
+int igt_device_filter_pci(void);
+
 /* Use filter to match the device and fill card structure */
 bool igt_device_card_match(const char *filter, struct igt_device_card *card);
 bool igt_device_card_match_pci(const char *filter,
-- 
2.42.0

[PATCH i-g-t v2 2/8] lib/drmtest: Introduced drm_open_driver_another

[Kamil Konieczny]

From: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>

Introduced drm_open_driver_another as a wrapper for
__drm_open_driver_another with successful open assert.

v2: rebased (Kamil)

Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
---
 lib/drmtest.c | 19 +++++++++++++++++++
 lib/drmtest.h |  1 +
 2 files changed, 20 insertions(+)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index 52b5a2020..73d9159af 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -563,6 +563,25 @@ int __drm_open_driver_another(int idx, int chipset)
 	return fd;
 }
 
+/*
+ * drm_open_driver_another
+ * @idx: index of the device you are opening
+ * @chipset: OR'd flags for each chipset to search, eg. #DRIVER_INTEL
+ *
+ * A wrapper for __drm_open_driver with successful open assert.
+ *
+ * Returns:
+ * An open DRM fd
+ */
+int drm_open_driver_another(int idx, int chipset)
+{
+	int fd = __drm_open_driver_another(idx, chipset);
+
+	igt_assert_fd(fd);
+
+	return fd;
+}
+
 /**
  * __drm_open_driver:
  * @chipset: OR'd flags for each chipset to search, eg. #DRIVER_INTEL
diff --git a/lib/drmtest.h b/lib/drmtest.h
index 6bc819734..856208c48 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -101,6 +101,7 @@ void __set_forced_driver(const char *name);
 
 int __drm_open_device(const char *name, unsigned int chipset);
 void drm_load_module(unsigned int chipset);
+int drm_open_driver_another(int idx, int chipset);
 int drm_open_driver(int chipset);
 int drm_open_driver_master(int chipset);
 int drm_open_driver_render(int chipset);
-- 
2.42.0

[PATCH i-g-t v2 3/8] lib/drmtest: allow opening cards out of order

[Kamil Konieczny]

Current __drm_open_driver_another() implementation prevents
opening cards out of order, for example card 0 and card 3. Relax
that condition. While at this, add some more error checks and
prints and also allow user to open already opened card if the
index is the same.

While at this add also caching for names of opened cards.

Changes in __search_and_open() was suggested by Zbigniew.

Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
---
 lib/drmtest.c | 100 +++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 83 insertions(+), 17 deletions(-)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index 73d9159af..80809571b 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -315,42 +315,81 @@ err:
 static struct {
 	int fd;
 	struct stat stat;
+	char *fname;
 }_opened_fds[64];
 
 static int _opened_fds_count;
 
+static void _set_opened_fname(int idx, char *name)
+{
+	assert(idx < ARRAY_SIZE(_opened_fds));
+	assert(idx >= 0);
+
+	if (!name) {
+		if (_opened_fds[idx].fname)
+			free(_opened_fds[idx].fname);
+
+		_opened_fds[idx].fname = name;
+		return;
+	}
+
+	if (_opened_fds[idx].fname && !strcmp(name, _opened_fds[idx].fname))
+		return;
+
+	free(_opened_fds[idx].fname);
+	_opened_fds[idx].fname = strdup(name);
+}
+
 static void _set_opened_fd(int idx, int fd)
 {
 	assert(idx < ARRAY_SIZE(_opened_fds));
-	assert(idx <= _opened_fds_count);
+	assert(idx >= 0);
 
 	_opened_fds[idx].fd = fd;
 
 	assert(fstat(fd, &_opened_fds[idx].stat) == 0);
 
-	_opened_fds_count = idx+1;
+	for (int n = _opened_fds_count; n < idx; n++)
+		_opened_fds[n].fd = -1;
+
+	if (idx >= _opened_fds_count)
+		_opened_fds_count = idx + 1;
 }
 
-static bool _is_already_opened(const char *path, int as_idx)
+/* in *err returns -errno or index to _opened_fds[] */
+static bool _is_already_opened(const char *path, int as_idx, int *err)
 {
 	struct stat new;
 
 	assert(as_idx < ARRAY_SIZE(_opened_fds));
-	assert(as_idx <= _opened_fds_count);
+	assert(as_idx >= 0);
 
 	/*
 	 * we cannot even stat the device, so it's of no use - let's claim it's
 	 * already opened
 	 */
-	if (igt_debug_on(stat(path, &new) != 0))
+	if (igt_debug_on(stat(path, &new) != 0)) {
+		if (err)
+			*err = -errno;
+
 		return true;
+	}
+
+	if (err)
+		*err = 0;
+
+	for (int i = 0; i < _opened_fds_count && i < as_idx; ++i) {
+		if (_opened_fds[i].fd == -1)
+			continue;
 
-	for (int i = 0; i < as_idx; ++i) {
 		/* did we cross filesystem boundary? */
 		assert(_opened_fds[i].stat.st_dev == new.st_dev);
 
-		if (_opened_fds[i].stat.st_ino == new.st_ino)
+		if (_opened_fds[i].stat.st_ino == new.st_ino) {
+			*err = i;
+
 			return true;
+		}
 	}
 
 	return false;
@@ -359,23 +398,40 @@ static bool _is_already_opened(const char *path, int as_idx)
 static int __search_and_open(const char *base, int offset, unsigned int chipset, int as_idx)
 {
 	const char *forced;
+	int err;
 
 	forced = forced_driver();
 	if (forced)
 		igt_debug("Force option used: Using driver %s\n", forced);
 
-	for (int i = 0; i < 16; i++) {
+	for (int i = 0, idx = -1; i < 16 && idx < as_idx; i++) {
 		char name[80];
-		int fd;
+		int fd = -1;
 
 		sprintf(name, "%s%u", base, i + offset);
 
-		if (_is_already_opened(name, as_idx))
-			continue;
+		if (_is_already_opened(name, as_idx, &err)) {
+			if (err < 0)
+				continue;
+
+			if (idx + 1 < as_idx) {
+				++idx;
+				continue;
+			}
+		}
 
-		fd = __drm_open_device(name, chipset);
-		if (fd != -1)
+		if (err >= 0)
+			fd = __drm_open_device(name, chipset);
+		if (fd != -1) {
+			++idx;
+			if (idx < as_idx) {
+				close(fd);
+				continue;
+			}
+
+			_set_opened_fname(as_idx, name);
 			return fd;
+		}
 	}
 
 	return -1;
@@ -542,11 +598,21 @@ int __drm_open_driver_another(int idx, int chipset)
 		if (!found || !strlen(card.card))
 			igt_warn("No card matches the filter! [%s]\n",
 				 igt_device_filter_get(idx));
-		else if (_is_already_opened(card.card, idx))
-			igt_warn("card maching filter %d is already opened\n", idx);
-		else
-			fd = __open_driver_exact(card.card, chipset);
+		else {
+			int ret;
 
+			if (_is_already_opened(card.card, idx, &ret))
+				if (ret >= 0 && idx != ret)
+					igt_warn("card maching filter %d is already opened at idx %d\nfilenames: %s::%s",
+						 idx, ret, card.card, _opened_fds[ret].fname);
+				else
+					fd = __open_driver_exact(card.card, chipset);
+			else
+				fd = __open_driver_exact(card.card, chipset);
+
+			if (fd != -1)
+				_set_opened_fname(idx, card.card);
+		}
 	} else {
 		/* no filter for device idx, let's open whatever is available */
 		fd = __open_driver("/dev/dri/card", 0, chipset, idx);
-- 
2.42.0

[PATCH i-g-t v2 4/8] lib/intel_multigpu: Introduce library for multi-GPU scenarios

[Kamil Konieczny]

From: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>

Implemented gem_require_multigpu in order to replace
igt_require(gpu_count > 1), as well as printing available
PCI devices if requirement fails.

Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
[Kamil: fixed whitespace and tabs, moved to lib/intel_multigpu.*]
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
---
 lib/intel_multigpu.c | 28 ++++++++++++++++++++++++++++
 lib/intel_multigpu.h | 11 +++++++++++
 lib/meson.build      |  1 +
 3 files changed, 40 insertions(+)
 create mode 100644 lib/intel_multigpu.c
 create mode 100644 lib/intel_multigpu.h

diff --git a/lib/intel_multigpu.c b/lib/intel_multigpu.c
new file mode 100644
index 000000000..988c20033
--- /dev/null
+++ b/lib/intel_multigpu.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#include "intel_multigpu.h"
+#include "igt_core.h"
+#include "igt_device_scan.h"
+
+void gem_require_multigpu(int count)
+{
+	struct igt_devices_print_format fmt = {
+		.type = IGT_PRINT_SIMPLE,
+		.option = IGT_PRINT_PCI,
+	};
+	int devices;
+
+	if (igt_device_filter_count() >= count)
+		return;
+
+	igt_info("PCI devices available in the system:\n");
+
+	igt_devices_scan(true);
+	devices = igt_device_filter_pci();
+	igt_devices_print(&fmt);
+
+	igt_skip("Test requires at least %d GPUs, %d available.\n", count, devices);
+}
diff --git a/lib/intel_multigpu.h b/lib/intel_multigpu.h
new file mode 100644
index 000000000..98dc5a4ce
--- /dev/null
+++ b/lib/intel_multigpu.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#ifndef __INTEL_MULTIGPU_H
+#define __INTEL_MULTIGPU_H
+
+void gem_require_multigpu(int count);
+
+#endif /* __INTEL_MULTIGPU_H */
diff --git a/lib/meson.build b/lib/meson.build
index 0fc11b26c..189017f05 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -65,6 +65,7 @@ lib_sources = [
 	'intel_device_info.c',
 	'intel_mmio.c',
 	'intel_mocs.c',
+	'intel_multigpu.c',
 	'intel_pat.c',
 	'ioctl_wrappers.c',
 	'media_spin.c',
-- 
2.42.0

[PATCH i-g-t v2 5/8] lib/intel_multigpu: Introduced gem_multigpu_count_class and igt_multi_fork_foreach_gpu

[Kamil Konieczny]

From: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>

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

Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
Signed-off-by: Chris Wilson <chris.p.wilson@linux.intel.com>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
---
 lib/intel_multigpu.c | 13 ++++++++++++-
 lib/intel_multigpu.h | 15 +++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/lib/intel_multigpu.c b/lib/intel_multigpu.c
index 988c20033..0e76d8aa3 100644
--- a/lib/intel_multigpu.c
+++ b/lib/intel_multigpu.c
@@ -3,9 +3,20 @@
  * Copyright © 2023 Intel Corporation
  */
 
-#include "intel_multigpu.h"
+#include "drmtest.h"
 #include "igt_core.h"
 #include "igt_device_scan.h"
+#include "intel_multigpu.h"
+
+int gem_multigpu_count_class(int class)
+{
+	int count = 0;
+
+	igt_foreach_gpu(fd, class)
+		count++;
+
+	return count;
+}
 
 void gem_require_multigpu(int count)
 {
diff --git a/lib/intel_multigpu.h b/lib/intel_multigpu.h
index 98dc5a4ce..0ebc73e4a 100644
--- a/lib/intel_multigpu.h
+++ b/lib/intel_multigpu.h
@@ -6,6 +6,21 @@
 #ifndef __INTEL_MULTIGPU_H
 #define __INTEL_MULTIGPU_H
 
+#include "drmtest.h"
+#include "igt_core.h"
+
 void gem_require_multigpu(int count);
+int gem_multigpu_count_class(int class);
+
+#define igt_foreach_gpu(fd__, id__) \
+	for (int igt_unique(i) = 0, fd__; \
+		(fd__ = __drm_open_driver_another(igt_unique(i)++, id__)) >= 0; \
+		close(fd__))
+
+#define igt_multi_fork_foreach_gpu(__fd, __id) \
+	igt_multi_fork(igt_unique(__i), gem_multigpu_count_class(__id)) \
+		for (int __fd = drm_open_driver_another(igt_unique(__i), __id); \
+			__fd >= 0; \
+			close(__fd), __fd = -1) \
 
 #endif /* __INTEL_MULTIGPU_H */
-- 
2.42.0

[PATCH i-g-t v2 6/8] lib/intel_multigpu: Add xe_multi_fork_foreach_gpu

[Kamil Konieczny]

Create macro for testing Xe driver in multigpu scenarios with
the help of few new multigpu functions. Also while at this,
add documentation for public functions and use new helper in
existing multi-gpu macro.

v2: corrected description (Kamil), rebased on corrected patches
after Dominik review

Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
---
 lib/intel_multigpu.c | 94 ++++++++++++++++++++++++++++++++++++++++++--
 lib/intel_multigpu.h | 12 +++++-
 2 files changed, 101 insertions(+), 5 deletions(-)

diff --git a/lib/intel_multigpu.c b/lib/intel_multigpu.c
index 0e76d8aa3..e631ab0ed 100644
--- a/lib/intel_multigpu.c
+++ b/lib/intel_multigpu.c
@@ -4,10 +4,18 @@
  */
 
 #include "drmtest.h"
+#include "i915/gem.h"
 #include "igt_core.h"
 #include "igt_device_scan.h"
 #include "intel_multigpu.h"
 
+/**
+ * gem_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 gem_multigpu_count_class(int class)
 {
 	int count = 0;
@@ -18,7 +26,7 @@ int gem_multigpu_count_class(int class)
 	return count;
 }
 
-void gem_require_multigpu(int count)
+static void skip_on_sigle_gpu(int count)
 {
 	struct igt_devices_print_format fmt = {
 		.type = IGT_PRINT_SIMPLE,
@@ -26,9 +34,6 @@ void gem_require_multigpu(int count)
 	};
 	int devices;
 
-	if (igt_device_filter_count() >= count)
-		return;
-
 	igt_info("PCI devices available in the system:\n");
 
 	igt_devices_scan(true);
@@ -37,3 +42,84 @@ void gem_require_multigpu(int count)
 
 	igt_skip("Test requires at least %d GPUs, %d available.\n", count, devices);
 }
+
+/**
+ * gem_require_multigpu:
+ * @count: minimum number of GPUs required
+ *
+ * Function checks number of filtered GPU cards.
+ * On error skips and prints available GPUs found on PCI bus.
+ */
+void gem_require_multigpu(int count)
+{
+	if (igt_device_filter_count() >= count)
+		return;
+
+	skip_on_sigle_gpu(count);
+}
+
+/**
+ * multigpu_aquire_view:
+ * @gpus_wanted: minimum number of GPUs required
+ * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
+ *
+ * Function prepares filtered view for GPU cards with given chipset.
+ * On error skips and prints available GPUs found on PCI bus.
+ * Returns: number of GPUs found
+ */
+int multigpu_aquire_view(int gpus_wanted, unsigned int chipset)
+{
+	int gpu_count = drm_prepare_filtered_multigpu(chipset);
+
+	igt_assert(gpus_wanted > 1);
+	if (gpu_count < gpus_wanted)
+		skip_on_sigle_gpu(gpus_wanted);
+
+	return gpu_count;
+}
+
+/**
+ * multigpu_open_another:
+ * @idx: index of GPU card, starting from 0
+ * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
+ *
+ * Function opens GPU card with drm_open_driver_another(). For i915 it checks
+ * if opened card is functional. Skips on errors.
+ * Returns: opened fd for @idx card
+ */
+int multigpu_open_another(int idx, unsigned int chipset)
+{
+	int fd;
+
+	igt_assert(idx >= 0);
+	fd = drm_open_driver_another(idx, chipset);
+
+	igt_require(fd != -1);
+	if (chipset == DRIVER_INTEL)
+		igt_require_gem(fd);
+
+	return fd;
+}
+
+/**
+ * multigpu_open_filtered_card:
+ * @idx: index of GPU card, starting from 0
+ * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
+ *
+ * Function opens GPU card prepared with filtered view. It also checks if
+ * opened card agrees with desired chipset or checks if opened card is
+ * functional. Skips on errors.
+ * Returns: opened fd for @idx card
+ */
+int multigpu_open_filtered_card(int idx, unsigned int chipset)
+{
+	int fd = drm_open_filtered_card(idx);
+
+	igt_require(fd != -1);
+	if (chipset == DRIVER_XE)
+		igt_require_xe(fd);
+	else if (chipset == DRIVER_INTEL)
+		igt_require_gem(fd);
+
+	return fd;
+}
diff --git a/lib/intel_multigpu.h b/lib/intel_multigpu.h
index 0ebc73e4a..0032a619e 100644
--- a/lib/intel_multigpu.h
+++ b/lib/intel_multigpu.h
@@ -12,6 +12,10 @@
 void gem_require_multigpu(int count);
 int gem_multigpu_count_class(int class);
 
+int multigpu_aquire_view(int min_gpus_wanted, unsigned int chipset);
+int multigpu_open_another(int idx, unsigned int chipset);
+int multigpu_open_filtered_card(int idx, unsigned int chipset);
+
 #define igt_foreach_gpu(fd__, id__) \
 	for (int igt_unique(i) = 0, fd__; \
 		(fd__ = __drm_open_driver_another(igt_unique(i)++, id__)) >= 0; \
@@ -19,8 +23,14 @@ int gem_multigpu_count_class(int class);
 
 #define igt_multi_fork_foreach_gpu(__fd, __id) \
 	igt_multi_fork(igt_unique(__i), gem_multigpu_count_class(__id)) \
-		for (int __fd = drm_open_driver_another(igt_unique(__i), __id); \
+		for (int __fd = multigpu_open_another(igt_unique(__i), __id); \
 			__fd >= 0; \
 			close(__fd), __fd = -1) \
 
+#define xe_multi_fork_foreach_gpu(__fd, __gpu_index) \
+	igt_multi_fork(__gpu_index, multigpu_aquire_view(2, DRIVER_XE)) \
+		for (int __fd = multigpu_open_filtered_card(__gpu_index, DRIVER_XE); \
+			__fd >= 0; \
+			drm_close_driver(__fd), __fd = -1) \
+
 #endif /* __INTEL_MULTIGPU_H */
-- 
2.42.0

[PATCH i-g-t v2 7/8] tests/intel/xe_exec_basic: add multigpu subtests

[Kamil Konieczny]

Add a few multi-gpu subtests:

multigpu-once-*
multigpu-many-execqueues-many-vm-*
multigpu-no-exec-*

run on two or more GPUs. Many variant was limited to take at most
few seconds.

Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
---
 tests/intel/xe_exec_basic.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/tests/intel/xe_exec_basic.c b/tests/intel/xe_exec_basic.c
index 8994859fa..debffaaf3 100644
--- a/tests/intel/xe_exec_basic.c
+++ b/tests/intel/xe_exec_basic.c
@@ -11,6 +11,7 @@
  */
 
 #include "igt.h"
+#include "intel_multigpu.h"
 #include "lib/igt_syncobj.h"
 #include "lib/intel_reg.h"
 #include "xe_drm.h"
@@ -54,6 +55,18 @@
  * Description: Run no-exec %arg[1] test
  * Test category: functionality test
  *
+ * SUBTEST: multigpu-once-%s
+ * Description: Run %arg[1] test only once on multiGPU
+ * Test category: functionality test
+ *
+ * SUBTEST: multigpu-many-execqueues-many-vm-%s
+ * Description: Run %arg[1] test on many exec_queues and many VMs on multiGPU
+ * Test category: stress test
+ *
+ * SUBTEST: multigpu-no-exec-%s
+ * Description: Run no-exec %arg[1] test on multiGPU
+ * Test category: functionality test
+ *
  * arg[1]:
  *
  * @basic:				basic
@@ -369,4 +382,27 @@ igt_main
 
 	igt_fixture
 		drm_close_driver(fd);
+
+	for (const struct section *s = sections; s->name; s++) {
+		igt_subtest_f("multigpu-once-%s", s->name) {
+			xe_multi_fork_foreach_gpu(xe, gpu_idx)
+				xe_for_each_engine(xe, hwe)
+					test_exec(xe, hwe, 1, 1, 1, s->flags);
+			igt_waitchildren();
+		}
+
+		igt_subtest_f("multigpu-many-execqueues-many-vm-%s", s->name) {
+			xe_multi_fork_foreach_gpu(xe, gpu_idx)
+				xe_for_each_engine(fd, hwe)
+					test_exec(fd, hwe, 16, 32, 16, s->flags);
+			igt_waitchildren();
+		}
+
+		igt_subtest_f("multigpu-no-exec-%s", s->name) {
+			xe_multi_fork_foreach_gpu(xe, gpu_idx)
+				xe_for_each_engine(fd, hwe)
+					test_exec(fd, hwe, 1, 0, 1, s->flags);
+			igt_waitchildren();
+		}
+	}
 }
-- 
2.42.0

[PATCH i-g-t v2 8/8] tests/intel/gem_mmap: add basic multi-GPU subtest

[Kamil Konieczny]

Test basic mmap for two or more GPU cards.

Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
---
 tests/intel/gem_mmap.c | 77 +++++++++++++++++++++++++++---------------
 1 file changed, 50 insertions(+), 27 deletions(-)

diff --git a/tests/intel/gem_mmap.c b/tests/intel/gem_mmap.c
index d4ca1eda7..90bcec7b0 100644
--- a/tests/intel/gem_mmap.c
+++ b/tests/intel/gem_mmap.c
@@ -38,6 +38,8 @@
 
 #include "drm.h"
 #include "i915/gem_create.h"
+#include "intel_multigpu.h"
+
 /**
  * TEST: gem mmap
  * Description: Basic MMAP IOCTL tests for memory regions.
@@ -61,6 +63,11 @@
  *   mapping existence after gem_close and unmapping.
  * Run type: BAT
  *
+ * SUBTEST: basic-multigpu
+ * Description:
+ *   Test basics of mmap on two or more GPUs.
+ * Run type: BAT
+ *
  * SUBTEST: basic-small-bo
  * Description:
  *   Test the write read coherency and simultaneous access of different pages of a small buffer
@@ -209,12 +216,40 @@ static int mmap_ioctl(int i915, struct drm_i915_gem_mmap *arg)
 	return err;
 }
 
-igt_main
+static void test_basic(int i915, int pat)
 {
+	struct drm_i915_gem_mmap arg = {
+		.handle = gem_create(fd, OBJECT_SIZE),
+		.size = OBJECT_SIZE,
+	};
 	uint8_t expected[OBJECT_SIZE];
 	uint8_t buf[OBJECT_SIZE];
 	uint8_t *addr;
 
+	igt_assert_eq(mmap_ioctl(fd, &arg), 0);
+	addr = from_user_pointer(arg.addr_ptr);
+
+	igt_info("Testing contents of newly created object.\n");
+	memset(expected, 0, sizeof(expected));
+	igt_assert(memcmp(addr, expected, sizeof(expected)) == 0);
+
+	igt_info("Testing coherency of writes and mmap reads.\n");
+	memset(buf, 0, sizeof(buf));
+	memset(buf + 1024, pat, 1024);
+	memset(expected + 1024, pat, 1024);
+	gem_write(fd, arg.handle, 0, buf, OBJECT_SIZE);
+	igt_assert(memcmp(buf, addr, sizeof(buf)) == 0);
+
+	igt_info("Testing that mapping stays after close\n");
+	gem_close(fd, arg.handle);
+	igt_assert(memcmp(buf, addr, sizeof(buf)) == 0);
+
+	igt_info("Testing unmapping\n");
+	munmap(addr, OBJECT_SIZE);
+}
+
+igt_main
+{
 	igt_fixture {
 		fd = drm_open_driver(DRIVER_INTEL);
 		igt_require(gem_has_legacy_mmap(fd));
@@ -306,36 +341,13 @@ igt_main
 
 	igt_describe("Test basics of newly mapped gem object like default content, write and read "
 		     "coherency, mapping existence after gem_close and unmapping.");
-	igt_subtest("basic") {
-		struct drm_i915_gem_mmap arg = {
-			.handle = gem_create(fd, OBJECT_SIZE),
-			.size = OBJECT_SIZE,
-		};
-		igt_assert_eq(mmap_ioctl(fd, &arg), 0);
-		addr = from_user_pointer(arg.addr_ptr);
-
-		igt_info("Testing contents of newly created object.\n");
-		memset(expected, 0, sizeof(expected));
-		igt_assert(memcmp(addr, expected, sizeof(expected)) == 0);
-
-		igt_info("Testing coherency of writes and mmap reads.\n");
-		memset(buf, 0, sizeof(buf));
-		memset(buf + 1024, 0x01, 1024);
-		memset(expected + 1024, 0x01, 1024);
-		gem_write(fd, arg.handle, 0, buf, OBJECT_SIZE);
-		igt_assert(memcmp(buf, addr, sizeof(buf)) == 0);
-
-		igt_info("Testing that mapping stays after close\n");
-		gem_close(fd, arg.handle);
-		igt_assert(memcmp(buf, addr, sizeof(buf)) == 0);
-
-		igt_info("Testing unmapping\n");
-		munmap(addr, OBJECT_SIZE);
-	}
+	igt_subtest("basic")
+		test_basic(fd, 1);
 
 	igt_describe("Map small buffer object though direct CPU access, bypassing GPU.");
 	igt_subtest("short-mmap") {
 		uint32_t handle = gem_create(fd, OBJECT_SIZE);
+		uint8_t *addr;
 
 		igt_assert(OBJECT_SIZE > 4096);
 
@@ -373,4 +385,15 @@ igt_main
 
 	igt_fixture
 		drm_close_driver(fd);
+
+	igt_describe("Test basics of mapped on two or more GPUs.");
+	igt_subtest("basic-multigpu") {
+		igt_multi_fork_foreach_gpu(gpu, DRIVER_INTEL) {
+			int pat = 1 + rand() % 128;
+
+			igt_info("Basic mmap test with pattern x%x\n", pat);
+			test_basic(gpu, pat);
+		}
+		igt_waitchildren();
+	}
 }
-- 
2.42.0

✗ Fi.CI.BAT: failure for introduce Xe multigpu and other multi-GPU helpers (rev2)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 10611 bytes --]

== Series Details ==

Series: introduce Xe multigpu and other multi-GPU helpers (rev2)
URL   : https://patchwork.freedesktop.org/series/129101/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_14171 -> IGTPW_10585
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_10585 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_10585, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/index.html

Participating hosts (37 -> 36)
------------------------------

  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_10585:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@hangcheck:
    - bat-adln-1:         [PASS][1] -> [ABORT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14171/bat-adln-1/igt@i915_selftest@live@hangcheck.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-adln-1/igt@i915_selftest@live@hangcheck.html

  
Known issues
------------

  Here are the changes found in IGTPW_10585 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - bat-rpls-2:         NOTRUN -> [SKIP][3] ([i915#9318])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-rpls-2/igt@debugfs_test@basic-hwmon.html

  * igt@gem_lmem_swapping@parallel-random-engines:
    - bat-adlm-1:         NOTRUN -> [SKIP][4] ([i915#4613]) +3 other tests skip
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-adlm-1/igt@gem_lmem_swapping@parallel-random-engines.html

  * igt@gem_lmem_swapping@verify-random:
    - bat-rpls-2:         NOTRUN -> [SKIP][5] ([i915#4613]) +3 other tests skip
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-rpls-2/igt@gem_lmem_swapping@verify-random.html

  * igt@gem_tiled_pread_basic:
    - bat-rpls-2:         NOTRUN -> [SKIP][6] ([i915#3282])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-rpls-2/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_rps@basic-api:
    - bat-rpls-2:         NOTRUN -> [SKIP][7] ([i915#6621])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-rpls-2/igt@i915_pm_rps@basic-api.html
    - bat-adlm-1:         NOTRUN -> [SKIP][8] ([i915#6621])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-adlm-1/igt@i915_pm_rps@basic-api.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - bat-rpls-2:         NOTRUN -> [SKIP][9] ([i915#4103]) +1 other test skip
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-rpls-2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_dsc@dsc-basic:
    - bat-rpls-2:         NOTRUN -> [SKIP][10] ([i915#3555] / [i915#3840] / [i915#9886])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-rpls-2/igt@kms_dsc@dsc-basic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-adlm-1:         NOTRUN -> [SKIP][11] ([fdo#109285])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-adlm-1/igt@kms_force_connector_basic@force-load-detect.html
    - bat-rpls-2:         NOTRUN -> [SKIP][12] ([fdo#109285])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-rpls-2/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_frontbuffer_tracking@basic:
    - bat-adlm-1:         NOTRUN -> [SKIP][13] ([i915#1849] / [i915#4342])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-adlm-1/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_pipe_crc_basic@hang-read-crc:
    - bat-adlm-1:         NOTRUN -> [SKIP][14] ([i915#9875] / [i915#9900]) +6 other tests skip
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-adlm-1/igt@kms_pipe_crc_basic@hang-read-crc.html

  * igt@kms_pm_backlight@basic-brightness:
    - bat-adlm-1:         NOTRUN -> [SKIP][15] ([i915#5354])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-adlm-1/igt@kms_pm_backlight@basic-brightness.html
    - bat-rpls-2:         NOTRUN -> [SKIP][16] ([i915#5354])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-rpls-2/igt@kms_pm_backlight@basic-brightness.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-adlm-1:         NOTRUN -> [SKIP][17] ([i915#3555])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-adlm-1/igt@kms_setmode@basic-clone-single-crtc.html
    - bat-rpls-2:         NOTRUN -> [SKIP][18] ([i915#3555])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-rpls-2/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-flip:
    - bat-adlm-1:         NOTRUN -> [SKIP][19] ([i915#3708] / [i915#9900])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-adlm-1/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-fence-read:
    - bat-rpls-2:         NOTRUN -> [SKIP][20] ([fdo#109295] / [i915#3708]) +2 other tests skip
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-rpls-2/igt@prime_vgem@basic-fence-read.html

  * igt@prime_vgem@basic-write:
    - bat-adlm-1:         NOTRUN -> [SKIP][21] ([i915#3708]) +2 other tests skip
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/bat-adlm-1/igt@prime_vgem@basic-write.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4342]: https://gitlab.freedesktop.org/drm/intel/issues/4342
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#9318]: https://gitlab.freedesktop.org/drm/intel/issues/9318
  [i915#9673]: https://gitlab.freedesktop.org/drm/intel/issues/9673
  [i915#9732]: https://gitlab.freedesktop.org/drm/intel/issues/9732
  [i915#9875]: https://gitlab.freedesktop.org/drm/intel/issues/9875
  [i915#9886]: https://gitlab.freedesktop.org/drm/intel/issues/9886
  [i915#9900]: https://gitlab.freedesktop.org/drm/intel/issues/9900


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_7691 -> IGTPW_10585

  CI-20190529: 20190529
  CI_DRM_14171: 7ec08146b3938f825a1cbe49e36a5362b10204be @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_10585: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/index.html
  IGT_7691: 7691


Testlist changes
----------------

+igt@gem_mmap@basic-multigpu
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-basic
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-basic-defer-bind
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-basic-defer-mmap
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-bindexecqueue
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-bindexecqueue-rebind
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-bindexecqueue-userptr
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-bindexecqueue-userptr-invalidate
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-bindexecqueue-userptr-invalidate-race
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-bindexecqueue-userptr-rebind
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-null
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-null-defer-bind
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-null-defer-mmap
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-null-rebind
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-rebind
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-userptr
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-userptr-invalidate
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-userptr-invalidate-race
+igt@xe_exec_basic@multigpu-many-execqueues-many-vm-userptr-rebind
+igt@xe_exec_basic@multigpu-no-exec-basic
+igt@xe_exec_basic@multigpu-no-exec-basic-defer-bind
+igt@xe_exec_basic@multigpu-no-exec-basic-defer-mmap
+igt@xe_exec_basic@multigpu-no-exec-bindexecqueue
+igt@xe_exec_basic@multigpu-no-exec-bindexecqueue-rebind
+igt@xe_exec_basic@multigpu-no-exec-bindexecqueue-userptr
+igt@xe_exec_basic@multigpu-no-exec-bindexecqueue-userptr-invalidate
+igt@xe_exec_basic@multigpu-no-exec-bindexecqueue-userptr-invalidate-race
+igt@xe_exec_basic@multigpu-no-exec-bindexecqueue-userptr-rebind
+igt@xe_exec_basic@multigpu-no-exec-null
+igt@xe_exec_basic@multigpu-no-exec-null-defer-bind
+igt@xe_exec_basic@multigpu-no-exec-null-defer-mmap
+igt@xe_exec_basic@multigpu-no-exec-null-rebind
+igt@xe_exec_basic@multigpu-no-exec-rebind
+igt@xe_exec_basic@multigpu-no-exec-userptr
+igt@xe_exec_basic@multigpu-no-exec-userptr-invalidate
+igt@xe_exec_basic@multigpu-no-exec-userptr-invalidate-race
+igt@xe_exec_basic@multigpu-no-exec-userptr-rebind
+igt@xe_exec_basic@multigpu-once-basic
+igt@xe_exec_basic@multigpu-once-basic-defer-bind
+igt@xe_exec_basic@multigpu-once-basic-defer-mmap
+igt@xe_exec_basic@multigpu-once-bindexecqueue
+igt@xe_exec_basic@multigpu-once-bindexecqueue-rebind
+igt@xe_exec_basic@multigpu-once-bindexecqueue-userptr
+igt@xe_exec_basic@multigpu-once-bindexecqueue-userptr-invalidate
+igt@xe_exec_basic@multigpu-once-bindexecqueue-userptr-invalidate-race
+igt@xe_exec_basic@multigpu-once-bindexecqueue-userptr-rebind
+igt@xe_exec_basic@multigpu-once-null
+igt@xe_exec_basic@multigpu-once-null-defer-bind
+igt@xe_exec_basic@multigpu-once-null-defer-mmap
+igt@xe_exec_basic@multigpu-once-null-rebind
+igt@xe_exec_basic@multigpu-once-rebind
+igt@xe_exec_basic@multigpu-once-userptr
+igt@xe_exec_basic@multigpu-once-userptr-invalidate
+igt@xe_exec_basic@multigpu-once-userptr-invalidate-race
+igt@xe_exec_basic@multigpu-once-userptr-rebind

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/index.html

[-- Attachment #2: Type: text/html, Size: 12416 bytes --]

✓ CI.xeBAT: success for introduce Xe multigpu and other multi-GPU helpers (rev2)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 1572 bytes --]

== Series Details ==

Series: introduce Xe multigpu and other multi-GPU helpers (rev2)
URL   : https://patchwork.freedesktop.org/series/129101/
State : success

== Summary ==

CI Bug Log - changes from XEIGT_7691_BAT -> XEIGTPW_10585_BAT
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (4 -> 4)
------------------------------

  No changes in participating hosts

Known issues
------------

  Here are the changes found in XEIGTPW_10585_BAT that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_frontbuffer_tracking@basic:
    - bat-adlp-7:         [PASS][1] -> [DMESG-FAIL][2] ([Intel XE#1033])
   [1]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7691/bat-adlp-7/igt@kms_frontbuffer_tracking@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10585/bat-adlp-7/igt@kms_frontbuffer_tracking@basic.html

  
  [Intel XE#1033]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/1033


Build changes
-------------

  * IGT: IGT_7691 -> IGTPW_10585
  * Linux: xe-674-0496fe20d8ed5e73ed186d293ef67e7bae4658eb -> xe-677-7ec08146b3938f825a1cbe49e36a5362b10204be

  IGTPW_10585: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10585/index.html
  IGT_7691: 7691
  xe-674-0496fe20d8ed5e73ed186d293ef67e7bae4658eb: 0496fe20d8ed5e73ed186d293ef67e7bae4658eb
  xe-677-7ec08146b3938f825a1cbe49e36a5362b10204be: 7ec08146b3938f825a1cbe49e36a5362b10204be

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10585/index.html

[-- Attachment #2: Type: text/html, Size: 2148 bytes --]

RE: [PATCH i-g-t v2 5/8] lib/intel_multigpu: Introduced gem_multigpu_count_class and igt_multi_fork_foreach_gpu

[Piatkowski, Dominik Karol]

Hi Kamil,

> -----Original Message-----
> From: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> Sent: Wednesday, January 24, 2024 9:42 PM
> To: igt-dev@lists.freedesktop.org
> Cc: Piatkowski, Dominik Karol <dominik.karol.piatkowski@intel.com>; Chris
> Wilson <chris.p.wilson@linux.intel.com>; Kamil Konieczny
> <kamil.konieczny@linux.intel.com>
> Subject: [PATCH i-g-t v2 5/8] lib/intel_multigpu: Introduced
> gem_multigpu_count_class and igt_multi_fork_foreach_gpu
> 
> From: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
> 
> Introduced gem_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 --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();
> 
> Signed-off-by: Dominik Karol Piątkowski
> <dominik.karol.piatkowski@intel.com>
> Signed-off-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> ---
>  lib/intel_multigpu.c | 13 ++++++++++++-  lib/intel_multigpu.h | 15
> +++++++++++++++
>  2 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/intel_multigpu.c b/lib/intel_multigpu.c index
> 988c20033..0e76d8aa3 100644
> --- a/lib/intel_multigpu.c
> +++ b/lib/intel_multigpu.c
> @@ -3,9 +3,20 @@
>   * Copyright © 2023 Intel Corporation
>   */
> 
> -#include "intel_multigpu.h"
> +#include "drmtest.h"
>  #include "igt_core.h"
>  #include "igt_device_scan.h"
> +#include "intel_multigpu.h"

A very small nitpick: in 4/8 #include "intel_multigpu.h" is added a line before igt_core.h. In here (5/8), it is moved down to maintain alphabetical order. Maybe that's a good idea to put it there in 4/8 and don't "fix" it in 5/8?
Otherwise, 1/8, 2/8, 4/8 and 5/8 LGTM, but I think I can't review them, as they are originally my patches. Please correct me if I'm wrong.

Dominik Karol

> +
> +int gem_multigpu_count_class(int class) {
> +	int count = 0;
> +
> +	igt_foreach_gpu(fd, class)
> +		count++;
> +
> +	return count;
> +}
> 
>  void gem_require_multigpu(int count)
>  {
> diff --git a/lib/intel_multigpu.h b/lib/intel_multigpu.h index
> 98dc5a4ce..0ebc73e4a 100644
> --- a/lib/intel_multigpu.h
> +++ b/lib/intel_multigpu.h
> @@ -6,6 +6,21 @@
>  #ifndef __INTEL_MULTIGPU_H
>  #define __INTEL_MULTIGPU_H
> 
> +#include "drmtest.h"
> +#include "igt_core.h"
> +
>  void gem_require_multigpu(int count);
> +int gem_multigpu_count_class(int class);
> +
> +#define igt_foreach_gpu(fd__, id__) \
> +	for (int igt_unique(i) = 0, fd__; \
> +		(fd__ = __drm_open_driver_another(igt_unique(i)++, id__)) >=
> 0; \
> +		close(fd__))
> +
> +#define igt_multi_fork_foreach_gpu(__fd, __id) \
> +	igt_multi_fork(igt_unique(__i), gem_multigpu_count_class(__id)) \
> +		for (int __fd = drm_open_driver_another(igt_unique(__i),
> __id); \
> +			__fd >= 0; \
> +			close(__fd), __fd = -1) \
> 
>  #endif /* __INTEL_MULTIGPU_H */
> --
> 2.42.0

RE: [PATCH i-g-t v2 6/8] lib/intel_multigpu: Add xe_multi_fork_foreach_gpu

[Piatkowski, Dominik Karol]

Hi Kamil,

> -----Original Message-----
> From: igt-dev <igt-dev-bounces@lists.freedesktop.org> On Behalf Of Kamil
> Konieczny
> Sent: Wednesday, January 24, 2024 9:42 PM
> To: igt-dev@lists.freedesktop.org
> Subject: [PATCH i-g-t v2 6/8] lib/intel_multigpu: Add
> xe_multi_fork_foreach_gpu
> 
> Create macro for testing Xe driver in multigpu scenarios with the help of few
> new multigpu functions. Also while at this, add documentation for public
> functions and use new helper in existing multi-gpu macro.
> 
> v2: corrected description (Kamil), rebased on corrected patches after Dominik
> review
> 
> Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> ---
>  lib/intel_multigpu.c | 94
> ++++++++++++++++++++++++++++++++++++++++++--
>  lib/intel_multigpu.h | 12 +++++-
>  2 files changed, 101 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/intel_multigpu.c b/lib/intel_multigpu.c index
> 0e76d8aa3..e631ab0ed 100644
> --- a/lib/intel_multigpu.c
> +++ b/lib/intel_multigpu.c
> @@ -4,10 +4,18 @@
>   */
> 
>  #include "drmtest.h"
> +#include "i915/gem.h"
>  #include "igt_core.h"
>  #include "igt_device_scan.h"
>  #include "intel_multigpu.h"
> 
> +/**
> + * gem_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 gem_multigpu_count_class(int class)  {
>  	int count = 0;
> @@ -18,7 +26,7 @@ int gem_multigpu_count_class(int class)
>  	return count;
>  }
> 
> -void gem_require_multigpu(int count)
> +static void skip_on_sigle_gpu(int count)

Nitpick: typo
I guess it should be named "skip_on_single_gpu".

>  {
>  	struct igt_devices_print_format fmt = {
>  		.type = IGT_PRINT_SIMPLE,
> @@ -26,9 +34,6 @@ void gem_require_multigpu(int count)
>  	};
>  	int devices;
> 
> -	if (igt_device_filter_count() >= count)
> -		return;
> -

As this conditional return was moved to gem_require_multigpu(), that function will work correctly. Although, calling skip_on_single_gpu() directly will always cause a skip due to always reachable igt_skip. I suggest changing igt_skip to igt_skip_on to avoid unexpected behavior in the future.

Additionally, skip_on_single_gpu can be refactored to not take any arguments, as we already get the number of gpus present in the system, and taking required count of gpus is not needed (it is always >1). The idea is to use igt_skip_on when devices < 2.

>  	igt_info("PCI devices available in the system:\n");
> 
>  	igt_devices_scan(true);
> @@ -37,3 +42,84 @@ void gem_require_multigpu(int count)
> 
>  	igt_skip("Test requires at least %d GPUs, %d available.\n", count,
> devices);  }
> +
> +/**
> + * gem_require_multigpu:
> + * @count: minimum number of GPUs required
> + *
> + * Function checks number of filtered GPU cards.
> + * On error skips and prints available GPUs found on PCI bus.
> + */
> +void gem_require_multigpu(int count)
> +{
> +	if (igt_device_filter_count() >= count)
> +		return;
> +
> +	skip_on_sigle_gpu(count);
> +}
> +
> +/**
> + * multigpu_aquire_view:
> + * @gpus_wanted: minimum number of GPUs required
> + * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
> + *
> + * Function prepares filtered view for GPU cards with given chipset.
> + * On error skips and prints available GPUs found on PCI bus.
> + * Returns: number of GPUs found
> + */
> +int multigpu_aquire_view(int gpus_wanted, unsigned int chipset) {

Nitpick: typo
I guess it should be named "multigpu_acquire_view" (in function name and documentation).

Dominik Karol

> +	int gpu_count = drm_prepare_filtered_multigpu(chipset);
> +
> +	igt_assert(gpus_wanted > 1);
> +	if (gpu_count < gpus_wanted)
> +		skip_on_sigle_gpu(gpus_wanted);
> +
> +	return gpu_count;
> +}
> +
> +/**
> + * multigpu_open_another:
> + * @idx: index of GPU card, starting from 0
> + * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
> + *
> + * Function opens GPU card with drm_open_driver_another(). For i915 it
> +checks
> + * if opened card is functional. Skips on errors.
> + * Returns: opened fd for @idx card
> + */
> +int multigpu_open_another(int idx, unsigned int chipset) {
> +	int fd;
> +
> +	igt_assert(idx >= 0);
> +	fd = drm_open_driver_another(idx, chipset);
> +
> +	igt_require(fd != -1);
> +	if (chipset == DRIVER_INTEL)
> +		igt_require_gem(fd);
> +
> +	return fd;
> +}
> +
> +/**
> + * multigpu_open_filtered_card:
> + * @idx: index of GPU card, starting from 0
> + * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
> + *
> + * Function opens GPU card prepared with filtered view. It also checks
> +if
> + * opened card agrees with desired chipset or checks if opened card is
> + * functional. Skips on errors.
> + * Returns: opened fd for @idx card
> + */
> +int multigpu_open_filtered_card(int idx, unsigned int chipset) {
> +	int fd = drm_open_filtered_card(idx);
> +
> +	igt_require(fd != -1);
> +	if (chipset == DRIVER_XE)
> +		igt_require_xe(fd);
> +	else if (chipset == DRIVER_INTEL)
> +		igt_require_gem(fd);
> +
> +	return fd;
> +}
> diff --git a/lib/intel_multigpu.h b/lib/intel_multigpu.h index
> 0ebc73e4a..0032a619e 100644
> --- a/lib/intel_multigpu.h
> +++ b/lib/intel_multigpu.h
> @@ -12,6 +12,10 @@
>  void gem_require_multigpu(int count);
>  int gem_multigpu_count_class(int class);
> 
> +int multigpu_aquire_view(int min_gpus_wanted, unsigned int chipset);
> +int multigpu_open_another(int idx, unsigned int chipset); int
> +multigpu_open_filtered_card(int idx, unsigned int chipset);
> +
>  #define igt_foreach_gpu(fd__, id__) \
>  	for (int igt_unique(i) = 0, fd__; \
>  		(fd__ = __drm_open_driver_another(igt_unique(i)++, id__)) >=
> 0; \ @@ -19,8 +23,14 @@ int gem_multigpu_count_class(int class);
> 
>  #define igt_multi_fork_foreach_gpu(__fd, __id) \
>  	igt_multi_fork(igt_unique(__i), gem_multigpu_count_class(__id)) \
> -		for (int __fd = drm_open_driver_another(igt_unique(__i),
> __id); \
> +		for (int __fd = multigpu_open_another(igt_unique(__i), __id);
> \
>  			__fd >= 0; \
>  			close(__fd), __fd = -1) \
> 
> +#define xe_multi_fork_foreach_gpu(__fd, __gpu_index) \
> +	igt_multi_fork(__gpu_index, multigpu_aquire_view(2, DRIVER_XE)) \
> +		for (int __fd = multigpu_open_filtered_card(__gpu_index,
> DRIVER_XE); \
> +			__fd >= 0; \
> +			drm_close_driver(__fd), __fd = -1) \
> +
>  #endif /* __INTEL_MULTIGPU_H */
> --
> 2.42.0

RE: [PATCH i-g-t v2 7/8] tests/intel/xe_exec_basic: add multigpu subtests

[Piatkowski, Dominik Karol]

Hi Kamil,

> -----Original Message-----
> From: igt-dev <igt-dev-bounces@lists.freedesktop.org> On Behalf Of Kamil
> Konieczny
> Sent: Wednesday, January 24, 2024 9:42 PM
> To: igt-dev@lists.freedesktop.org
> Subject: [PATCH i-g-t v2 7/8] tests/intel/xe_exec_basic: add multigpu subtests
> 
> Add a few multi-gpu subtests:
> 
> multigpu-once-*
> multigpu-many-execqueues-many-vm-*
> multigpu-no-exec-*
> 
> run on two or more GPUs. Many variant was limited to take at most few
> seconds.
> 
> Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> ---
>  tests/intel/xe_exec_basic.c | 36
> ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/tests/intel/xe_exec_basic.c b/tests/intel/xe_exec_basic.c index
> 8994859fa..debffaaf3 100644
> --- a/tests/intel/xe_exec_basic.c
> +++ b/tests/intel/xe_exec_basic.c
> @@ -11,6 +11,7 @@
>   */
> 
>  #include "igt.h"
> +#include "intel_multigpu.h"
>  #include "lib/igt_syncobj.h"
>  #include "lib/intel_reg.h"
>  #include "xe_drm.h"
> @@ -54,6 +55,18 @@
>   * Description: Run no-exec %arg[1] test
>   * Test category: functionality test
>   *
> + * SUBTEST: multigpu-once-%s
> + * Description: Run %arg[1] test only once on multiGPU
> + * Test category: functionality test
> + *
> + * SUBTEST: multigpu-many-execqueues-many-vm-%s
> + * Description: Run %arg[1] test on many exec_queues and many VMs on
> + multiGPU
> + * Test category: stress test
> + *
> + * SUBTEST: multigpu-no-exec-%s
> + * Description: Run no-exec %arg[1] test on multiGPU
> + * Test category: functionality test
> + *
>   * arg[1]:
>   *
>   * @basic:				basic
> @@ -369,4 +382,27 @@ igt_main
> 
>  	igt_fixture
>  		drm_close_driver(fd);
> +
> +	for (const struct section *s = sections; s->name; s++) {
> +		igt_subtest_f("multigpu-once-%s", s->name) {
> +			xe_multi_fork_foreach_gpu(xe, gpu_idx)
> +				xe_for_each_engine(xe, hwe)
> +					test_exec(xe, hwe, 1, 1, 1, s->flags);

Nitpick: consider using fd instead of xe for consistency
Otherwise, LGTM
Reviewed-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>

> +			igt_waitchildren();
> +		}
> +
> +		igt_subtest_f("multigpu-many-execqueues-many-vm-%s", s-
> >name) {
> +			xe_multi_fork_foreach_gpu(xe, gpu_idx)
> +				xe_for_each_engine(fd, hwe)
> +					test_exec(fd, hwe, 16, 32, 16, s-
> >flags);
> +			igt_waitchildren();
> +		}
> +
> +		igt_subtest_f("multigpu-no-exec-%s", s->name) {
> +			xe_multi_fork_foreach_gpu(xe, gpu_idx)
> +				xe_for_each_engine(fd, hwe)
> +					test_exec(fd, hwe, 1, 0, 1, s->flags);
> +			igt_waitchildren();
> +		}
> +	}
>  }
> --
> 2.42.0

RE: [PATCH i-g-t v2 8/8] tests/intel/gem_mmap: add basic multi-GPU subtest

[Piatkowski, Dominik Karol]

Hi Kamil,

LGTM.
Reviewed-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>

> -----Original Message-----
> From: igt-dev <igt-dev-bounces@lists.freedesktop.org> On Behalf Of Kamil
> Konieczny
> Sent: Wednesday, January 24, 2024 9:42 PM
> To: igt-dev@lists.freedesktop.org
> Subject: [PATCH i-g-t v2 8/8] tests/intel/gem_mmap: add basic multi-GPU
> subtest
> 
> Test basic mmap for two or more GPU cards.
> 
> Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> ---
>  tests/intel/gem_mmap.c | 77 +++++++++++++++++++++++++++--------------
> -
>  1 file changed, 50 insertions(+), 27 deletions(-)
> 
> diff --git a/tests/intel/gem_mmap.c b/tests/intel/gem_mmap.c index
> d4ca1eda7..90bcec7b0 100644
> --- a/tests/intel/gem_mmap.c
> +++ b/tests/intel/gem_mmap.c
> @@ -38,6 +38,8 @@
> 
>  #include "drm.h"
>  #include "i915/gem_create.h"
> +#include "intel_multigpu.h"
> +
>  /**
>   * TEST: gem mmap
>   * Description: Basic MMAP IOCTL tests for memory regions.
> @@ -61,6 +63,11 @@
>   *   mapping existence after gem_close and unmapping.
>   * Run type: BAT
>   *
> + * SUBTEST: basic-multigpu
> + * Description:
> + *   Test basics of mmap on two or more GPUs.
> + * Run type: BAT
> + *
>   * SUBTEST: basic-small-bo
>   * Description:
>   *   Test the write read coherency and simultaneous access of different pages
> of a small buffer
> @@ -209,12 +216,40 @@ static int mmap_ioctl(int i915, struct
> drm_i915_gem_mmap *arg)
>  	return err;
>  }
> 
> -igt_main
> +static void test_basic(int i915, int pat)
>  {
> +	struct drm_i915_gem_mmap arg = {
> +		.handle = gem_create(fd, OBJECT_SIZE),
> +		.size = OBJECT_SIZE,
> +	};
>  	uint8_t expected[OBJECT_SIZE];
>  	uint8_t buf[OBJECT_SIZE];
>  	uint8_t *addr;
> 
> +	igt_assert_eq(mmap_ioctl(fd, &arg), 0);
> +	addr = from_user_pointer(arg.addr_ptr);
> +
> +	igt_info("Testing contents of newly created object.\n");
> +	memset(expected, 0, sizeof(expected));
> +	igt_assert(memcmp(addr, expected, sizeof(expected)) == 0);
> +
> +	igt_info("Testing coherency of writes and mmap reads.\n");
> +	memset(buf, 0, sizeof(buf));
> +	memset(buf + 1024, pat, 1024);
> +	memset(expected + 1024, pat, 1024);
> +	gem_write(fd, arg.handle, 0, buf, OBJECT_SIZE);
> +	igt_assert(memcmp(buf, addr, sizeof(buf)) == 0);
> +
> +	igt_info("Testing that mapping stays after close\n");
> +	gem_close(fd, arg.handle);
> +	igt_assert(memcmp(buf, addr, sizeof(buf)) == 0);
> +
> +	igt_info("Testing unmapping\n");
> +	munmap(addr, OBJECT_SIZE);
> +}
> +
> +igt_main
> +{
>  	igt_fixture {
>  		fd = drm_open_driver(DRIVER_INTEL);
>  		igt_require(gem_has_legacy_mmap(fd));
> @@ -306,36 +341,13 @@ igt_main
> 
>  	igt_describe("Test basics of newly mapped gem object like default
> content, write and read "
>  		     "coherency, mapping existence after gem_close and
> unmapping.");
> -	igt_subtest("basic") {
> -		struct drm_i915_gem_mmap arg = {
> -			.handle = gem_create(fd, OBJECT_SIZE),
> -			.size = OBJECT_SIZE,
> -		};
> -		igt_assert_eq(mmap_ioctl(fd, &arg), 0);
> -		addr = from_user_pointer(arg.addr_ptr);
> -
> -		igt_info("Testing contents of newly created object.\n");
> -		memset(expected, 0, sizeof(expected));
> -		igt_assert(memcmp(addr, expected, sizeof(expected)) == 0);
> -
> -		igt_info("Testing coherency of writes and mmap reads.\n");
> -		memset(buf, 0, sizeof(buf));
> -		memset(buf + 1024, 0x01, 1024);
> -		memset(expected + 1024, 0x01, 1024);
> -		gem_write(fd, arg.handle, 0, buf, OBJECT_SIZE);
> -		igt_assert(memcmp(buf, addr, sizeof(buf)) == 0);
> -
> -		igt_info("Testing that mapping stays after close\n");
> -		gem_close(fd, arg.handle);
> -		igt_assert(memcmp(buf, addr, sizeof(buf)) == 0);
> -
> -		igt_info("Testing unmapping\n");
> -		munmap(addr, OBJECT_SIZE);
> -	}
> +	igt_subtest("basic")
> +		test_basic(fd, 1);
> 
>  	igt_describe("Map small buffer object though direct CPU access,
> bypassing GPU.");
>  	igt_subtest("short-mmap") {
>  		uint32_t handle = gem_create(fd, OBJECT_SIZE);
> +		uint8_t *addr;
> 
>  		igt_assert(OBJECT_SIZE > 4096);
> 
> @@ -373,4 +385,15 @@ igt_main
> 
>  	igt_fixture
>  		drm_close_driver(fd);
> +
> +	igt_describe("Test basics of mapped on two or more GPUs.");
> +	igt_subtest("basic-multigpu") {
> +		igt_multi_fork_foreach_gpu(gpu, DRIVER_INTEL) {
> +			int pat = 1 + rand() % 128;
> +
> +			igt_info("Basic mmap test with pattern x%x\n", pat);
> +			test_basic(gpu, pat);
> +		}
> +		igt_waitchildren();
> +	}
>  }
> --
> 2.42.0

Re: [PATCH i-g-t v2 7/8] tests/intel/xe_exec_basic: add multigpu subtests

[Kamil Konieczny]

Hi Dominik,

On 2024-01-25 at 07:51:56 +0000, Piatkowski, Dominik Karol wrote:
> Hi Kamil,
> 
> > -----Original Message-----
> > From: igt-dev <igt-dev-bounces@lists.freedesktop.org> On Behalf Of Kamil
> > Konieczny
> > Sent: Wednesday, January 24, 2024 9:42 PM
> > To: igt-dev@lists.freedesktop.org
> > Subject: [PATCH i-g-t v2 7/8] tests/intel/xe_exec_basic: add multigpu subtests
> > 
> > Add a few multi-gpu subtests:
> > 
> > multigpu-once-*
> > multigpu-many-execqueues-many-vm-*
> > multigpu-no-exec-*
> > 
> > run on two or more GPUs. Many variant was limited to take at most few
> > seconds.
> > 
> > Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > ---
> >  tests/intel/xe_exec_basic.c | 36
> > ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git a/tests/intel/xe_exec_basic.c b/tests/intel/xe_exec_basic.c index
> > 8994859fa..debffaaf3 100644
> > --- a/tests/intel/xe_exec_basic.c
> > +++ b/tests/intel/xe_exec_basic.c
> > @@ -11,6 +11,7 @@
> >   */
> > 
> >  #include "igt.h"
> > +#include "intel_multigpu.h"
> >  #include "lib/igt_syncobj.h"
> >  #include "lib/intel_reg.h"
> >  #include "xe_drm.h"
> > @@ -54,6 +55,18 @@
> >   * Description: Run no-exec %arg[1] test
> >   * Test category: functionality test
> >   *
> > + * SUBTEST: multigpu-once-%s
> > + * Description: Run %arg[1] test only once on multiGPU
> > + * Test category: functionality test
> > + *
> > + * SUBTEST: multigpu-many-execqueues-many-vm-%s
> > + * Description: Run %arg[1] test on many exec_queues and many VMs on
> > + multiGPU
> > + * Test category: stress test
> > + *
> > + * SUBTEST: multigpu-no-exec-%s
> > + * Description: Run no-exec %arg[1] test on multiGPU
> > + * Test category: functionality test
> > + *
> >   * arg[1]:
> >   *
> >   * @basic:				basic
> > @@ -369,4 +382,27 @@ igt_main
> > 
> >  	igt_fixture
> >  		drm_close_driver(fd);
> > +
> > +	for (const struct section *s = sections; s->name; s++) {
> > +		igt_subtest_f("multigpu-once-%s", s->name) {
> > +			xe_multi_fork_foreach_gpu(xe, gpu_idx)
> > +				xe_for_each_engine(xe, hwe)
> > +					test_exec(xe, hwe, 1, 1, 1, s->flags);
> 
> Nitpick: consider using fd instead of xe for consistency

Macro defines a var with that name so it rather should be different,
for example could be gpu_fd.

Regards,
Kamil

> Otherwise, LGTM
> Reviewed-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
> 
> > +			igt_waitchildren();
> > +		}
> > +
> > +		igt_subtest_f("multigpu-many-execqueues-many-vm-%s", s-
> > >name) {
> > +			xe_multi_fork_foreach_gpu(xe, gpu_idx)
> > +				xe_for_each_engine(fd, hwe)
> > +					test_exec(fd, hwe, 16, 32, 16, s-
> > >flags);
> > +			igt_waitchildren();
> > +		}
> > +
> > +		igt_subtest_f("multigpu-no-exec-%s", s->name) {
> > +			xe_multi_fork_foreach_gpu(xe, gpu_idx)
> > +				xe_for_each_engine(fd, hwe)
> > +					test_exec(fd, hwe, 1, 0, 1, s->flags);
> > +			igt_waitchildren();
> > +		}
> > +	}
> >  }
> > --
> > 2.42.0
> 

Re: [PATCH i-g-t v2 6/8] lib/intel_multigpu: Add xe_multi_fork_foreach_gpu

[Kamil Konieczny]

Hi Dominik,

On 2024-01-25 at 07:43:35 +0000, Piatkowski, Dominik Karol wrote:
> Hi Kamil,
> 
> > -----Original Message-----
> > From: igt-dev <igt-dev-bounces@lists.freedesktop.org> On Behalf Of Kamil
> > Konieczny
> > Sent: Wednesday, January 24, 2024 9:42 PM
> > To: igt-dev@lists.freedesktop.org
> > Subject: [PATCH i-g-t v2 6/8] lib/intel_multigpu: Add
> > xe_multi_fork_foreach_gpu
> > 
> > Create macro for testing Xe driver in multigpu scenarios with the help of few
> > new multigpu functions. Also while at this, add documentation for public
> > functions and use new helper in existing multi-gpu macro.
> > 
> > v2: corrected description (Kamil), rebased on corrected patches after Dominik
> > review
> > 
> > Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > ---
> >  lib/intel_multigpu.c | 94
> > ++++++++++++++++++++++++++++++++++++++++++--
> >  lib/intel_multigpu.h | 12 +++++-
> >  2 files changed, 101 insertions(+), 5 deletions(-)
> > 
> > diff --git a/lib/intel_multigpu.c b/lib/intel_multigpu.c index
> > 0e76d8aa3..e631ab0ed 100644
> > --- a/lib/intel_multigpu.c
> > +++ b/lib/intel_multigpu.c
> > @@ -4,10 +4,18 @@
> >   */
> > 
> >  #include "drmtest.h"
> > +#include "i915/gem.h"
> >  #include "igt_core.h"
> >  #include "igt_device_scan.h"
> >  #include "intel_multigpu.h"
> > 
> > +/**
> > + * gem_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 gem_multigpu_count_class(int class)  {
> >  	int count = 0;
> > @@ -18,7 +26,7 @@ int gem_multigpu_count_class(int class)
> >  	return count;
> >  }
> > 
> > -void gem_require_multigpu(int count)
> > +static void skip_on_sigle_gpu(int count)
> 
> Nitpick: typo
> I guess it should be named "skip_on_single_gpu".
> 

Good point, I missed that, will fix.

> >  {
> >  	struct igt_devices_print_format fmt = {
> >  		.type = IGT_PRINT_SIMPLE,
> > @@ -26,9 +34,6 @@ void gem_require_multigpu(int count)
> >  	};
> >  	int devices;
> > 
> > -	if (igt_device_filter_count() >= count)
> > -		return;
> > -
> 
> As this conditional return was moved to gem_require_multigpu(),
> that function will work correctly. Although, calling skip_on_single_gpu()
> directly will always cause a skip due to always reachable igt_skip.
> I suggest changing igt_skip to igt_skip_on to avoid unexpected behavior
> in the future.

What about changing this into print-only function and moving
skips directly in both functions?

> 
> Additionally, skip_on_single_gpu can be refactored to not take any
> arguments, as we already get the number of gpus present in the system, 
> and taking required count of gpus is not needed (it is always >1).
> The idea is to use igt_skip_on when devices < 2.
> 

It was an optimization, I was thinking about other direction, adding
one more parameter to it and printing:

number of GPUs wanted
what number of GPUs program got
number of GPUs after PCI bus scan

What do you think?

> >  	igt_info("PCI devices available in the system:\n");
> > 
> >  	igt_devices_scan(true);
> > @@ -37,3 +42,84 @@ void gem_require_multigpu(int count)
> > 
> >  	igt_skip("Test requires at least %d GPUs, %d available.\n", count,
> > devices);  }
> > +
> > +/**
> > + * gem_require_multigpu:
> > + * @count: minimum number of GPUs required
> > + *
> > + * Function checks number of filtered GPU cards.
> > + * On error skips and prints available GPUs found on PCI bus.
> > + */
> > +void gem_require_multigpu(int count)
> > +{
> > +	if (igt_device_filter_count() >= count)
> > +		return;
> > +
> > +	skip_on_sigle_gpu(count);
> > +}
> > +
> > +/**
> > + * multigpu_aquire_view:
> > + * @gpus_wanted: minimum number of GPUs required
> > + * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
> > + *
> > + * Function prepares filtered view for GPU cards with given chipset.
> > + * On error skips and prints available GPUs found on PCI bus.
> > + * Returns: number of GPUs found
> > + */
> > +int multigpu_aquire_view(int gpus_wanted, unsigned int chipset) {
> 
> Nitpick: typo
> I guess it should be named "multigpu_acquire_view" (in function name and documentation).
> 

I will fix this,

Regards,
Kamil

> Dominik Karol
> 
> > +	int gpu_count = drm_prepare_filtered_multigpu(chipset);
> > +
> > +	igt_assert(gpus_wanted > 1);
> > +	if (gpu_count < gpus_wanted)
> > +		skip_on_sigle_gpu(gpus_wanted);
> > +
> > +	return gpu_count;
> > +}
> > +
> > +/**
> > + * multigpu_open_another:
> > + * @idx: index of GPU card, starting from 0
> > + * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
> > + *
> > + * Function opens GPU card with drm_open_driver_another(). For i915 it
> > +checks
> > + * if opened card is functional. Skips on errors.
> > + * Returns: opened fd for @idx card
> > + */
> > +int multigpu_open_another(int idx, unsigned int chipset) {
> > +	int fd;
> > +
> > +	igt_assert(idx >= 0);
> > +	fd = drm_open_driver_another(idx, chipset);
> > +
> > +	igt_require(fd != -1);
> > +	if (chipset == DRIVER_INTEL)
> > +		igt_require_gem(fd);
> > +
> > +	return fd;
> > +}
> > +
> > +/**
> > + * multigpu_open_filtered_card:
> > + * @idx: index of GPU card, starting from 0
> > + * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
> > + *
> > + * Function opens GPU card prepared with filtered view. It also checks
> > +if
> > + * opened card agrees with desired chipset or checks if opened card is
> > + * functional. Skips on errors.
> > + * Returns: opened fd for @idx card
> > + */
> > +int multigpu_open_filtered_card(int idx, unsigned int chipset) {
> > +	int fd = drm_open_filtered_card(idx);
> > +
> > +	igt_require(fd != -1);
> > +	if (chipset == DRIVER_XE)
> > +		igt_require_xe(fd);
> > +	else if (chipset == DRIVER_INTEL)
> > +		igt_require_gem(fd);
> > +
> > +	return fd;
> > +}
> > diff --git a/lib/intel_multigpu.h b/lib/intel_multigpu.h index
> > 0ebc73e4a..0032a619e 100644
> > --- a/lib/intel_multigpu.h
> > +++ b/lib/intel_multigpu.h
> > @@ -12,6 +12,10 @@
> >  void gem_require_multigpu(int count);
> >  int gem_multigpu_count_class(int class);
> > 
> > +int multigpu_aquire_view(int min_gpus_wanted, unsigned int chipset);
> > +int multigpu_open_another(int idx, unsigned int chipset); int
> > +multigpu_open_filtered_card(int idx, unsigned int chipset);
> > +
> >  #define igt_foreach_gpu(fd__, id__) \
> >  	for (int igt_unique(i) = 0, fd__; \
> >  		(fd__ = __drm_open_driver_another(igt_unique(i)++, id__)) >=
> > 0; \ @@ -19,8 +23,14 @@ int gem_multigpu_count_class(int class);
> > 
> >  #define igt_multi_fork_foreach_gpu(__fd, __id) \
> >  	igt_multi_fork(igt_unique(__i), gem_multigpu_count_class(__id)) \
> > -		for (int __fd = drm_open_driver_another(igt_unique(__i),
> > __id); \
> > +		for (int __fd = multigpu_open_another(igt_unique(__i), __id);
> > \
> >  			__fd >= 0; \
> >  			close(__fd), __fd = -1) \
> > 
> > +#define xe_multi_fork_foreach_gpu(__fd, __gpu_index) \
> > +	igt_multi_fork(__gpu_index, multigpu_aquire_view(2, DRIVER_XE)) \
> > +		for (int __fd = multigpu_open_filtered_card(__gpu_index,
> > DRIVER_XE); \
> > +			__fd >= 0; \
> > +			drm_close_driver(__fd), __fd = -1) \
> > +
> >  #endif /* __INTEL_MULTIGPU_H */
> > --
> > 2.42.0
> 

Re: [PATCH i-g-t v2 5/8] lib/intel_multigpu: Introduced gem_multigpu_count_class and igt_multi_fork_foreach_gpu

[Kamil Konieczny]

Hi Dominik,
On 2024-01-25 at 07:14:47 +0000, Piatkowski, Dominik Karol wrote:
> Hi Kamil,
> 
> > -----Original Message-----
> > From: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > Sent: Wednesday, January 24, 2024 9:42 PM
> > To: igt-dev@lists.freedesktop.org
> > Cc: Piatkowski, Dominik Karol <dominik.karol.piatkowski@intel.com>; Chris
> > Wilson <chris.p.wilson@linux.intel.com>; Kamil Konieczny
> > <kamil.konieczny@linux.intel.com>
> > Subject: [PATCH i-g-t v2 5/8] lib/intel_multigpu: Introduced
> > gem_multigpu_count_class and igt_multi_fork_foreach_gpu
> > 
> > From: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
> > 
> > Introduced gem_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 --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();
> > 
> > Signed-off-by: Dominik Karol Piątkowski
> > <dominik.karol.piatkowski@intel.com>
> > Signed-off-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> > Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > ---
> >  lib/intel_multigpu.c | 13 ++++++++++++-  lib/intel_multigpu.h | 15
> > +++++++++++++++
> >  2 files changed, 27 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/intel_multigpu.c b/lib/intel_multigpu.c index
> > 988c20033..0e76d8aa3 100644
> > --- a/lib/intel_multigpu.c
> > +++ b/lib/intel_multigpu.c
> > @@ -3,9 +3,20 @@
> >   * Copyright © 2023 Intel Corporation
> >   */
> > 
> > -#include "intel_multigpu.h"
> > +#include "drmtest.h"
> >  #include "igt_core.h"
> >  #include "igt_device_scan.h"
> > +#include "intel_multigpu.h"
> 
> A very small nitpick: in 4/8 #include "intel_multigpu.h" is added
> a line before igt_core.h. In here (5/8), it is moved down to
> maintain alphabetical order. Maybe that's a good idea to put it
> there in 4/8 and don't "fix" it in 5/8?
> Otherwise, 1/8, 2/8, 4/8 and 5/8 LGTM, but I think I can't review them,
> as they are originally my patches. Please correct me if I'm wrong.
> 
> Dominik Karol
> 

Thank you for spotting this, I will fix it. You are right,
someone else should do a review,

Regards,
Kamil

> > +
> > +int gem_multigpu_count_class(int class) {
> > +	int count = 0;
> > +
> > +	igt_foreach_gpu(fd, class)
> > +		count++;
> > +
> > +	return count;
> > +}
> > 
> >  void gem_require_multigpu(int count)
> >  {
> > diff --git a/lib/intel_multigpu.h b/lib/intel_multigpu.h index
> > 98dc5a4ce..0ebc73e4a 100644
> > --- a/lib/intel_multigpu.h
> > +++ b/lib/intel_multigpu.h
> > @@ -6,6 +6,21 @@
> >  #ifndef __INTEL_MULTIGPU_H
> >  #define __INTEL_MULTIGPU_H
> > 
> > +#include "drmtest.h"
> > +#include "igt_core.h"
> > +
> >  void gem_require_multigpu(int count);
> > +int gem_multigpu_count_class(int class);
> > +
> > +#define igt_foreach_gpu(fd__, id__) \
> > +	for (int igt_unique(i) = 0, fd__; \
> > +		(fd__ = __drm_open_driver_another(igt_unique(i)++, id__)) >=
> > 0; \
> > +		close(fd__))
> > +
> > +#define igt_multi_fork_foreach_gpu(__fd, __id) \
> > +	igt_multi_fork(igt_unique(__i), gem_multigpu_count_class(__id)) \
> > +		for (int __fd = drm_open_driver_another(igt_unique(__i),
> > __id); \
> > +			__fd >= 0; \
> > +			close(__fd), __fd = -1) \
> > 
> >  #endif /* __INTEL_MULTIGPU_H */
> > --
> > 2.42.0
> 

RE: [PATCH i-g-t v2 7/8] tests/intel/xe_exec_basic: add multigpu subtests

[Piatkowski, Dominik Karol]

Hi Kamil,

> -----Original Message-----
> From: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> Sent: Thursday, January 25, 2024 2:44 PM
> To: igt-dev@lists.freedesktop.org
> Cc: Piatkowski, Dominik Karol <dominik.karol.piatkowski@intel.com>
> Subject: Re: [PATCH i-g-t v2 7/8] tests/intel/xe_exec_basic: add multigpu
> subtests
> 
> Hi Dominik,
> 
> On 2024-01-25 at 07:51:56 +0000, Piatkowski, Dominik Karol wrote:
> > Hi Kamil,
> >
> > > -----Original Message-----
> > > From: igt-dev <igt-dev-bounces@lists.freedesktop.org> On Behalf Of
> > > Kamil Konieczny
> > > Sent: Wednesday, January 24, 2024 9:42 PM
> > > To: igt-dev@lists.freedesktop.org
> > > Subject: [PATCH i-g-t v2 7/8] tests/intel/xe_exec_basic: add
> > > multigpu subtests
> > >
> > > Add a few multi-gpu subtests:
> > >
> > > multigpu-once-*
> > > multigpu-many-execqueues-many-vm-*
> > > multigpu-no-exec-*
> > >
> > > run on two or more GPUs. Many variant was limited to take at most
> > > few seconds.
> > >
> > > Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > > ---
> > >  tests/intel/xe_exec_basic.c | 36
> > > ++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 36 insertions(+)
> > >
> > > diff --git a/tests/intel/xe_exec_basic.c
> > > b/tests/intel/xe_exec_basic.c index
> > > 8994859fa..debffaaf3 100644
> > > --- a/tests/intel/xe_exec_basic.c
> > > +++ b/tests/intel/xe_exec_basic.c
> > > @@ -11,6 +11,7 @@
> > >   */
> > >
> > >  #include "igt.h"
> > > +#include "intel_multigpu.h"
> > >  #include "lib/igt_syncobj.h"
> > >  #include "lib/intel_reg.h"
> > >  #include "xe_drm.h"
> > > @@ -54,6 +55,18 @@
> > >   * Description: Run no-exec %arg[1] test
> > >   * Test category: functionality test
> > >   *
> > > + * SUBTEST: multigpu-once-%s
> > > + * Description: Run %arg[1] test only once on multiGPU
> > > + * Test category: functionality test
> > > + *
> > > + * SUBTEST: multigpu-many-execqueues-many-vm-%s
> > > + * Description: Run %arg[1] test on many exec_queues and many VMs
> > > + on multiGPU
> > > + * Test category: stress test
> > > + *
> > > + * SUBTEST: multigpu-no-exec-%s
> > > + * Description: Run no-exec %arg[1] test on multiGPU
> > > + * Test category: functionality test
> > > + *
> > >   * arg[1]:
> > >   *
> > >   * @basic:				basic
> > > @@ -369,4 +382,27 @@ igt_main
> > >
> > >  	igt_fixture
> > >  		drm_close_driver(fd);
> > > +
> > > +	for (const struct section *s = sections; s->name; s++) {
> > > +		igt_subtest_f("multigpu-once-%s", s->name) {
> > > +			xe_multi_fork_foreach_gpu(xe, gpu_idx)
> > > +				xe_for_each_engine(xe, hwe)
> > > +					test_exec(xe, hwe, 1, 1, 1, s->flags);
> >
> > Nitpick: consider using fd instead of xe for consistency
> 
> Macro defines a var with that name so it rather should be different, for
> example could be gpu_fd.

Okay. "fd" is used in other added testcases, please double check if it should be "fd" and not "xe" in multigpu-many-execqueues-many-vm-* and multigpu-no-exec-*. R-b carries on. 

Dominik Karol

> 
> Regards,
> Kamil
> 
> > Otherwise, LGTM
> > Reviewed-by: Dominik Karol Piątkowski
> > <dominik.karol.piatkowski@intel.com>
> >
> > > +			igt_waitchildren();
> > > +		}
> > > +
> > > +		igt_subtest_f("multigpu-many-execqueues-many-vm-%s", s-
> > > >name) {
> > > +			xe_multi_fork_foreach_gpu(xe, gpu_idx)
> > > +				xe_for_each_engine(fd, hwe)
> > > +					test_exec(fd, hwe, 16, 32, 16, s-
> > > >flags);
> > > +			igt_waitchildren();
> > > +		}
> > > +
> > > +		igt_subtest_f("multigpu-no-exec-%s", s->name) {
> > > +			xe_multi_fork_foreach_gpu(xe, gpu_idx)
> > > +				xe_for_each_engine(fd, hwe)
> > > +					test_exec(fd, hwe, 1, 0, 1, s->flags);
> > > +			igt_waitchildren();
> > > +		}
> > > +	}
> > >  }
> > > --
> > > 2.42.0
> >

RE: [PATCH i-g-t v2 6/8] lib/intel_multigpu: Add xe_multi_fork_foreach_gpu

[Piatkowski, Dominik Karol]

Hi Kamil,

> -----Original Message-----
> From: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> Sent: Thursday, January 25, 2024 2:55 PM
> To: igt-dev@lists.freedesktop.org
> Cc: Piatkowski, Dominik Karol <dominik.karol.piatkowski@intel.com>
> Subject: Re: [PATCH i-g-t v2 6/8] lib/intel_multigpu: Add
> xe_multi_fork_foreach_gpu
> 
> Hi Dominik,
> 
> On 2024-01-25 at 07:43:35 +0000, Piatkowski, Dominik Karol wrote:
> > Hi Kamil,
> >
> > > -----Original Message-----
> > > From: igt-dev <igt-dev-bounces@lists.freedesktop.org> On Behalf Of
> > > Kamil Konieczny
> > > Sent: Wednesday, January 24, 2024 9:42 PM
> > > To: igt-dev@lists.freedesktop.org
> > > Subject: [PATCH i-g-t v2 6/8] lib/intel_multigpu: Add
> > > xe_multi_fork_foreach_gpu
> > >
> > > Create macro for testing Xe driver in multigpu scenarios with the
> > > help of few new multigpu functions. Also while at this, add
> > > documentation for public functions and use new helper in existing multi-
> gpu macro.
> > >
> > > v2: corrected description (Kamil), rebased on corrected patches
> > > after Dominik review
> > >
> > > Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > > ---
> > >  lib/intel_multigpu.c | 94
> > > ++++++++++++++++++++++++++++++++++++++++++--
> > >  lib/intel_multigpu.h | 12 +++++-
> > >  2 files changed, 101 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/lib/intel_multigpu.c b/lib/intel_multigpu.c index
> > > 0e76d8aa3..e631ab0ed 100644
> > > --- a/lib/intel_multigpu.c
> > > +++ b/lib/intel_multigpu.c
> > > @@ -4,10 +4,18 @@
> > >   */
> > >
> > >  #include "drmtest.h"
> > > +#include "i915/gem.h"
> > >  #include "igt_core.h"
> > >  #include "igt_device_scan.h"
> > >  #include "intel_multigpu.h"
> > >
> > > +/**
> > > + * gem_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 gem_multigpu_count_class(int class)  {
> > >  	int count = 0;
> > > @@ -18,7 +26,7 @@ int gem_multigpu_count_class(int class)
> > >  	return count;
> > >  }
> > >
> > > -void gem_require_multigpu(int count)
> > > +static void skip_on_sigle_gpu(int count)
> >
> > Nitpick: typo
> > I guess it should be named "skip_on_single_gpu".
> >
> 
> Good point, I missed that, will fix.
> 
> > >  {
> > >  	struct igt_devices_print_format fmt = {
> > >  		.type = IGT_PRINT_SIMPLE,
> > > @@ -26,9 +34,6 @@ void gem_require_multigpu(int count)
> > >  	};
> > >  	int devices;
> > >
> > > -	if (igt_device_filter_count() >= count)
> > > -		return;
> > > -
> >
> > As this conditional return was moved to gem_require_multigpu(), that
> > function will work correctly. Although, calling skip_on_single_gpu()
> > directly will always cause a skip due to always reachable igt_skip.
> > I suggest changing igt_skip to igt_skip_on to avoid unexpected
> > behavior in the future.
> 
> What about changing this into print-only function and moving skips directly in
> both functions?

Can you describe it further?

> 
> >
> > Additionally, skip_on_single_gpu can be refactored to not take any
> > arguments, as we already get the number of gpus present in the system,
> > and taking required count of gpus is not needed (it is always >1).
> > The idea is to use igt_skip_on when devices < 2.
> >
> 
> It was an optimization, I was thinking about other direction, adding one more
> parameter to it and printing:
> 
> number of GPUs wanted
> what number of GPUs program got
> number of GPUs after PCI bus scan
> 
> What do you think?

Ok, but then skip_on_single_gpu won't hold to its name, unless you mean the change in gem_require_multigpu. Please further describe your idea.

Dominik Karol

> 
> > >  	igt_info("PCI devices available in the system:\n");
> > >
> > >  	igt_devices_scan(true);
> > > @@ -37,3 +42,84 @@ void gem_require_multigpu(int count)
> > >
> > >  	igt_skip("Test requires at least %d GPUs, %d available.\n", count,
> > > devices);  }
> > > +
> > > +/**
> > > + * gem_require_multigpu:
> > > + * @count: minimum number of GPUs required
> > > + *
> > > + * Function checks number of filtered GPU cards.
> > > + * On error skips and prints available GPUs found on PCI bus.
> > > + */
> > > +void gem_require_multigpu(int count) {
> > > +	if (igt_device_filter_count() >= count)
> > > +		return;
> > > +
> > > +	skip_on_sigle_gpu(count);
> > > +}
> > > +
> > > +/**
> > > + * multigpu_aquire_view:
> > > + * @gpus_wanted: minimum number of GPUs required
> > > + * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
> > > + *
> > > + * Function prepares filtered view for GPU cards with given chipset.
> > > + * On error skips and prints available GPUs found on PCI bus.
> > > + * Returns: number of GPUs found
> > > + */
> > > +int multigpu_aquire_view(int gpus_wanted, unsigned int chipset) {
> >
> > Nitpick: typo
> > I guess it should be named "multigpu_acquire_view" (in function name and
> documentation).
> >
> 
> I will fix this,
> 
> Regards,
> Kamil
> 
> > Dominik Karol
> >
> > > +	int gpu_count = drm_prepare_filtered_multigpu(chipset);
> > > +
> > > +	igt_assert(gpus_wanted > 1);
> > > +	if (gpu_count < gpus_wanted)
> > > +		skip_on_sigle_gpu(gpus_wanted);
> > > +
> > > +	return gpu_count;
> > > +}
> > > +
> > > +/**
> > > + * multigpu_open_another:
> > > + * @idx: index of GPU card, starting from 0
> > > + * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
> > > + *
> > > + * Function opens GPU card with drm_open_driver_another(). For i915
> > > +it checks
> > > + * if opened card is functional. Skips on errors.
> > > + * Returns: opened fd for @idx card  */ int
> > > +multigpu_open_another(int idx, unsigned int chipset) {
> > > +	int fd;
> > > +
> > > +	igt_assert(idx >= 0);
> > > +	fd = drm_open_driver_another(idx, chipset);
> > > +
> > > +	igt_require(fd != -1);
> > > +	if (chipset == DRIVER_INTEL)
> > > +		igt_require_gem(fd);
> > > +
> > > +	return fd;
> > > +}
> > > +
> > > +/**
> > > + * multigpu_open_filtered_card:
> > > + * @idx: index of GPU card, starting from 0
> > > + * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
> > > + *
> > > + * Function opens GPU card prepared with filtered view. It also
> > > +checks if
> > > + * opened card agrees with desired chipset or checks if opened card
> > > +is
> > > + * functional. Skips on errors.
> > > + * Returns: opened fd for @idx card  */ int
> > > +multigpu_open_filtered_card(int idx, unsigned int chipset) {
> > > +	int fd = drm_open_filtered_card(idx);
> > > +
> > > +	igt_require(fd != -1);
> > > +	if (chipset == DRIVER_XE)
> > > +		igt_require_xe(fd);
> > > +	else if (chipset == DRIVER_INTEL)
> > > +		igt_require_gem(fd);
> > > +
> > > +	return fd;
> > > +}
> > > diff --git a/lib/intel_multigpu.h b/lib/intel_multigpu.h index
> > > 0ebc73e4a..0032a619e 100644
> > > --- a/lib/intel_multigpu.h
> > > +++ b/lib/intel_multigpu.h
> > > @@ -12,6 +12,10 @@
> > >  void gem_require_multigpu(int count);  int
> > > gem_multigpu_count_class(int class);
> > >
> > > +int multigpu_aquire_view(int min_gpus_wanted, unsigned int
> > > +chipset); int multigpu_open_another(int idx, unsigned int chipset);
> > > +int multigpu_open_filtered_card(int idx, unsigned int chipset);
> > > +
> > >  #define igt_foreach_gpu(fd__, id__) \
> > >  	for (int igt_unique(i) = 0, fd__; \
> > >  		(fd__ = __drm_open_driver_another(igt_unique(i)++, id__)) >=
> 0; \
> > > @@ -19,8 +23,14 @@ int gem_multigpu_count_class(int class);
> > >
> > >  #define igt_multi_fork_foreach_gpu(__fd, __id) \
> > >  	igt_multi_fork(igt_unique(__i), gem_multigpu_count_class(__id)) \
> > > -		for (int __fd = drm_open_driver_another(igt_unique(__i),
> > > __id); \
> > > +		for (int __fd = multigpu_open_another(igt_unique(__i), __id);
> > > \
> > >  			__fd >= 0; \
> > >  			close(__fd), __fd = -1) \
> > >
> > > +#define xe_multi_fork_foreach_gpu(__fd, __gpu_index) \
> > > +	igt_multi_fork(__gpu_index, multigpu_aquire_view(2, DRIVER_XE)) \
> > > +		for (int __fd = multigpu_open_filtered_card(__gpu_index,
> > > DRIVER_XE); \
> > > +			__fd >= 0; \
> > > +			drm_close_driver(__fd), __fd = -1) \
> > > +
> > >  #endif /* __INTEL_MULTIGPU_H */
> > > --
> > > 2.42.0
> >

Re: [PATCH i-g-t v2 6/8] lib/intel_multigpu: Add xe_multi_fork_foreach_gpu

[Kamil Konieczny]

Hi Piatkowski,,
On 2024-01-25 at 14:11:30 +0000, Piatkowski, Dominik Karol wrote:
> Hi Kamil,
> 
> > -----Original Message-----
> > From: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > Sent: Thursday, January 25, 2024 2:55 PM
> > To: igt-dev@lists.freedesktop.org
> > Cc: Piatkowski, Dominik Karol <dominik.karol.piatkowski@intel.com>
> > Subject: Re: [PATCH i-g-t v2 6/8] lib/intel_multigpu: Add
> > xe_multi_fork_foreach_gpu
> > 
> > Hi Dominik,
> > 
> > On 2024-01-25 at 07:43:35 +0000, Piatkowski, Dominik Karol wrote:
> > > Hi Kamil,
> > >
> > > > -----Original Message-----
> > > > From: igt-dev <igt-dev-bounces@lists.freedesktop.org> On Behalf Of
> > > > Kamil Konieczny
> > > > Sent: Wednesday, January 24, 2024 9:42 PM
> > > > To: igt-dev@lists.freedesktop.org
> > > > Subject: [PATCH i-g-t v2 6/8] lib/intel_multigpu: Add
> > > > xe_multi_fork_foreach_gpu
> > > >
> > > > Create macro for testing Xe driver in multigpu scenarios with the
> > > > help of few new multigpu functions. Also while at this, add
> > > > documentation for public functions and use new helper in existing multi-
> > gpu macro.
> > > >
> > > > v2: corrected description (Kamil), rebased on corrected patches
> > > > after Dominik review
> > > >
> > > > Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > > > ---
> > > >  lib/intel_multigpu.c | 94
> > > > ++++++++++++++++++++++++++++++++++++++++++--
> > > >  lib/intel_multigpu.h | 12 +++++-
> > > >  2 files changed, 101 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/lib/intel_multigpu.c b/lib/intel_multigpu.c index
> > > > 0e76d8aa3..e631ab0ed 100644
> > > > --- a/lib/intel_multigpu.c
> > > > +++ b/lib/intel_multigpu.c
> > > > @@ -4,10 +4,18 @@
> > > >   */
> > > >
> > > >  #include "drmtest.h"
> > > > +#include "i915/gem.h"
> > > >  #include "igt_core.h"
> > > >  #include "igt_device_scan.h"
> > > >  #include "intel_multigpu.h"
> > > >
> > > > +/**
> > > > + * gem_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 gem_multigpu_count_class(int class)  {
> > > >  	int count = 0;
> > > > @@ -18,7 +26,7 @@ int gem_multigpu_count_class(int class)
> > > >  	return count;
> > > >  }
> > > >
> > > > -void gem_require_multigpu(int count)
> > > > +static void skip_on_sigle_gpu(int count)
> > >
> > > Nitpick: typo
> > > I guess it should be named "skip_on_single_gpu".
> > >
> > 
> > Good point, I missed that, will fix.
> > 
> > > >  {
> > > >  	struct igt_devices_print_format fmt = {
> > > >  		.type = IGT_PRINT_SIMPLE,
> > > > @@ -26,9 +34,6 @@ void gem_require_multigpu(int count)
> > > >  	};
> > > >  	int devices;
> > > >
> > > > -	if (igt_device_filter_count() >= count)
> > > > -		return;
> > > > -
> > >
> > > As this conditional return was moved to gem_require_multigpu(), that
> > > function will work correctly. Although, calling skip_on_single_gpu()
> > > directly will always cause a skip due to always reachable igt_skip.
> > > I suggest changing igt_skip to igt_skip_on to avoid unexpected
> > > behavior in the future.
> > 
> > What about changing this into print-only function and moving skips directly in
> > both functions?
> 
> Can you describe it further?

I will send new version soon, I hope it will be clear then.

Thanks for review,
Kamil

> 
> > 
> > >
> > > Additionally, skip_on_single_gpu can be refactored to not take any
> > > arguments, as we already get the number of gpus present in the system,
> > > and taking required count of gpus is not needed (it is always >1).
> > > The idea is to use igt_skip_on when devices < 2.
> > >
> > 
> > It was an optimization, I was thinking about other direction, adding one more
> > parameter to it and printing:
> > 
> > number of GPUs wanted
> > what number of GPUs program got
> > number of GPUs after PCI bus scan
> > 
> > What do you think?
> 
> Ok, but then skip_on_single_gpu won't hold to its name, unless you mean the change in gem_require_multigpu. Please further describe your idea.
> 
> Dominik Karol
> 
> > 
> > > >  	igt_info("PCI devices available in the system:\n");
> > > >
> > > >  	igt_devices_scan(true);
> > > > @@ -37,3 +42,84 @@ void gem_require_multigpu(int count)
> > > >
> > > >  	igt_skip("Test requires at least %d GPUs, %d available.\n", count,
> > > > devices);  }
> > > > +
> > > > +/**
> > > > + * gem_require_multigpu:
> > > > + * @count: minimum number of GPUs required
> > > > + *
> > > > + * Function checks number of filtered GPU cards.
> > > > + * On error skips and prints available GPUs found on PCI bus.
> > > > + */
> > > > +void gem_require_multigpu(int count) {
> > > > +	if (igt_device_filter_count() >= count)
> > > > +		return;
> > > > +
> > > > +	skip_on_sigle_gpu(count);
> > > > +}
> > > > +
> > > > +/**
> > > > + * multigpu_aquire_view:
> > > > + * @gpus_wanted: minimum number of GPUs required
> > > > + * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
> > > > + *
> > > > + * Function prepares filtered view for GPU cards with given chipset.
> > > > + * On error skips and prints available GPUs found on PCI bus.
> > > > + * Returns: number of GPUs found
> > > > + */
> > > > +int multigpu_aquire_view(int gpus_wanted, unsigned int chipset) {
> > >
> > > Nitpick: typo
> > > I guess it should be named "multigpu_acquire_view" (in function name and
> > documentation).
> > >
> > 
> > I will fix this,
> > 
> > Regards,
> > Kamil
> > 
> > > Dominik Karol
> > >
> > > > +	int gpu_count = drm_prepare_filtered_multigpu(chipset);
> > > > +
> > > > +	igt_assert(gpus_wanted > 1);
> > > > +	if (gpu_count < gpus_wanted)
> > > > +		skip_on_sigle_gpu(gpus_wanted);
> > > > +
> > > > +	return gpu_count;
> > > > +}
> > > > +
> > > > +/**
> > > > + * multigpu_open_another:
> > > > + * @idx: index of GPU card, starting from 0
> > > > + * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
> > > > + *
> > > > + * Function opens GPU card with drm_open_driver_another(). For i915
> > > > +it checks
> > > > + * if opened card is functional. Skips on errors.
> > > > + * Returns: opened fd for @idx card  */ int
> > > > +multigpu_open_another(int idx, unsigned int chipset) {
> > > > +	int fd;
> > > > +
> > > > +	igt_assert(idx >= 0);
> > > > +	fd = drm_open_driver_another(idx, chipset);
> > > > +
> > > > +	igt_require(fd != -1);
> > > > +	if (chipset == DRIVER_INTEL)
> > > > +		igt_require_gem(fd);
> > > > +
> > > > +	return fd;
> > > > +}
> > > > +
> > > > +/**
> > > > + * multigpu_open_filtered_card:
> > > > + * @idx: index of GPU card, starting from 0
> > > > + * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
> > > > + *
> > > > + * Function opens GPU card prepared with filtered view. It also
> > > > +checks if
> > > > + * opened card agrees with desired chipset or checks if opened card
> > > > +is
> > > > + * functional. Skips on errors.
> > > > + * Returns: opened fd for @idx card  */ int
> > > > +multigpu_open_filtered_card(int idx, unsigned int chipset) {
> > > > +	int fd = drm_open_filtered_card(idx);
> > > > +
> > > > +	igt_require(fd != -1);
> > > > +	if (chipset == DRIVER_XE)
> > > > +		igt_require_xe(fd);
> > > > +	else if (chipset == DRIVER_INTEL)
> > > > +		igt_require_gem(fd);
> > > > +
> > > > +	return fd;
> > > > +}
> > > > diff --git a/lib/intel_multigpu.h b/lib/intel_multigpu.h index
> > > > 0ebc73e4a..0032a619e 100644
> > > > --- a/lib/intel_multigpu.h
> > > > +++ b/lib/intel_multigpu.h
> > > > @@ -12,6 +12,10 @@
> > > >  void gem_require_multigpu(int count);  int
> > > > gem_multigpu_count_class(int class);
> > > >
> > > > +int multigpu_aquire_view(int min_gpus_wanted, unsigned int
> > > > +chipset); int multigpu_open_another(int idx, unsigned int chipset);
> > > > +int multigpu_open_filtered_card(int idx, unsigned int chipset);
> > > > +
> > > >  #define igt_foreach_gpu(fd__, id__) \
> > > >  	for (int igt_unique(i) = 0, fd__; \
> > > >  		(fd__ = __drm_open_driver_another(igt_unique(i)++, id__)) >=
> > 0; \
> > > > @@ -19,8 +23,14 @@ int gem_multigpu_count_class(int class);
> > > >
> > > >  #define igt_multi_fork_foreach_gpu(__fd, __id) \
> > > >  	igt_multi_fork(igt_unique(__i), gem_multigpu_count_class(__id)) \
> > > > -		for (int __fd = drm_open_driver_another(igt_unique(__i),
> > > > __id); \
> > > > +		for (int __fd = multigpu_open_another(igt_unique(__i), __id);
> > > > \
> > > >  			__fd >= 0; \
> > > >  			close(__fd), __fd = -1) \
> > > >
> > > > +#define xe_multi_fork_foreach_gpu(__fd, __gpu_index) \
> > > > +	igt_multi_fork(__gpu_index, multigpu_aquire_view(2, DRIVER_XE)) \
> > > > +		for (int __fd = multigpu_open_filtered_card(__gpu_index,
> > > > DRIVER_XE); \
> > > > +			__fd >= 0; \
> > > > +			drm_close_driver(__fd), __fd = -1) \
> > > > +
> > > >  #endif /* __INTEL_MULTIGPU_H */
> > > > --
> > > > 2.42.0
> > >

Re: [PATCH i-g-t v2 7/8] tests/intel/xe_exec_basic: add multigpu subtests

[Kamil Konieczny]

Hi,

On 2024-01-25 at 14:05:50 +0000, Piatkowski, Dominik Karol wrote:
> Hi Kamil,
> 
> > -----Original Message-----
> > From: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > Sent: Thursday, January 25, 2024 2:44 PM
> > To: igt-dev@lists.freedesktop.org
> > Cc: Piatkowski, Dominik Karol <dominik.karol.piatkowski@intel.com>
> > Subject: Re: [PATCH i-g-t v2 7/8] tests/intel/xe_exec_basic: add multigpu
> > subtests
> > 
> > Hi Dominik,
> > 
> > On 2024-01-25 at 07:51:56 +0000, Piatkowski, Dominik Karol wrote:
> > > Hi Kamil,
> > >
> > > > -----Original Message-----
> > > > From: igt-dev <igt-dev-bounces@lists.freedesktop.org> On Behalf Of
> > > > Kamil Konieczny
> > > > Sent: Wednesday, January 24, 2024 9:42 PM
> > > > To: igt-dev@lists.freedesktop.org
> > > > Subject: [PATCH i-g-t v2 7/8] tests/intel/xe_exec_basic: add
> > > > multigpu subtests
> > > >
> > > > Add a few multi-gpu subtests:
> > > >
> > > > multigpu-once-*
> > > > multigpu-many-execqueues-many-vm-*
> > > > multigpu-no-exec-*
> > > >
> > > > run on two or more GPUs. Many variant was limited to take at most
> > > > few seconds.
> > > >
> > > > Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > > > ---
> > > >  tests/intel/xe_exec_basic.c | 36
> > > > ++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 36 insertions(+)
> > > >
> > > > diff --git a/tests/intel/xe_exec_basic.c
> > > > b/tests/intel/xe_exec_basic.c index
> > > > 8994859fa..debffaaf3 100644
> > > > --- a/tests/intel/xe_exec_basic.c
> > > > +++ b/tests/intel/xe_exec_basic.c
> > > > @@ -11,6 +11,7 @@
> > > >   */
> > > >
> > > >  #include "igt.h"
> > > > +#include "intel_multigpu.h"
> > > >  #include "lib/igt_syncobj.h"
> > > >  #include "lib/intel_reg.h"
> > > >  #include "xe_drm.h"
> > > > @@ -54,6 +55,18 @@
> > > >   * Description: Run no-exec %arg[1] test
> > > >   * Test category: functionality test
> > > >   *
> > > > + * SUBTEST: multigpu-once-%s
> > > > + * Description: Run %arg[1] test only once on multiGPU
> > > > + * Test category: functionality test
> > > > + *
> > > > + * SUBTEST: multigpu-many-execqueues-many-vm-%s
> > > > + * Description: Run %arg[1] test on many exec_queues and many VMs
> > > > + on multiGPU
> > > > + * Test category: stress test
> > > > + *
> > > > + * SUBTEST: multigpu-no-exec-%s
> > > > + * Description: Run no-exec %arg[1] test on multiGPU
> > > > + * Test category: functionality test
> > > > + *
> > > >   * arg[1]:
> > > >   *
> > > >   * @basic:				basic
> > > > @@ -369,4 +382,27 @@ igt_main
> > > >
> > > >  	igt_fixture
> > > >  		drm_close_driver(fd);
> > > > +
> > > > +	for (const struct section *s = sections; s->name; s++) {
> > > > +		igt_subtest_f("multigpu-once-%s", s->name) {
> > > > +			xe_multi_fork_foreach_gpu(xe, gpu_idx)
> > > > +				xe_for_each_engine(xe, hwe)
> > > > +					test_exec(xe, hwe, 1, 1, 1, s->flags);
> > >
> > > Nitpick: consider using fd instead of xe for consistency
> > 
> > Macro defines a var with that name so it rather should be different, for
> > example could be gpu_fd.
> 
> Okay. "fd" is used in other added testcases, please double check if
> it should be "fd" and not "xe" in multigpu-many-execqueues-many-vm-* 
> and multigpu-no-exec-*. R-b carries on. 

This is local var passed as param, there should be no problems.

Regards,
Kamil

> 
> Dominik Karol
> 
> > 
> > Regards,
> > Kamil
> > 
> > > Otherwise, LGTM
> > > Reviewed-by: Dominik Karol Piątkowski
> > > <dominik.karol.piatkowski@intel.com>
> > >
> > > > +			igt_waitchildren();
> > > > +		}
> > > > +
> > > > +		igt_subtest_f("multigpu-many-execqueues-many-vm-%s", s-
> > > > >name) {
> > > > +			xe_multi_fork_foreach_gpu(xe, gpu_idx)
> > > > +				xe_for_each_engine(fd, hwe)
> > > > +					test_exec(fd, hwe, 16, 32, 16, s-
> > > > >flags);
> > > > +			igt_waitchildren();
> > > > +		}
> > > > +
> > > > +		igt_subtest_f("multigpu-no-exec-%s", s->name) {
> > > > +			xe_multi_fork_foreach_gpu(xe, gpu_idx)
> > > > +				xe_for_each_engine(fd, hwe)
> > > > +					test_exec(fd, hwe, 1, 0, 1, s->flags);
> > > > +			igt_waitchildren();
> > > > +		}
> > > > +	}
> > > >  }
> > > > --
> > > > 2.42.0
> > >

Re: [PATCH i-g-t v2 6/8] lib/intel_multigpu: Add xe_multi_fork_foreach_gpu

[Kamil Konieczny]

Hi,

On 2024-01-25 at 14:11:30 +0000, Piatkowski, Dominik Karol wrote:
> Hi Kamil,
> 
> > -----Original Message-----
> > From: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > Sent: Thursday, January 25, 2024 2:55 PM
> > To: igt-dev@lists.freedesktop.org
> > Cc: Piatkowski, Dominik Karol <dominik.karol.piatkowski@intel.com>
> > Subject: Re: [PATCH i-g-t v2 6/8] lib/intel_multigpu: Add
> > xe_multi_fork_foreach_gpu
> > 
> > Hi Dominik,
> > 
> > On 2024-01-25 at 07:43:35 +0000, Piatkowski, Dominik Karol wrote:
> > > Hi Kamil,
> > >
> > > > -----Original Message-----
> > > > From: igt-dev <igt-dev-bounces@lists.freedesktop.org> On Behalf Of
> > > > Kamil Konieczny
> > > > Sent: Wednesday, January 24, 2024 9:42 PM
> > > > To: igt-dev@lists.freedesktop.org
> > > > Subject: [PATCH i-g-t v2 6/8] lib/intel_multigpu: Add
> > > > xe_multi_fork_foreach_gpu
> > > >
> > > > Create macro for testing Xe driver in multigpu scenarios with the
> > > > help of few new multigpu functions. Also while at this, add
> > > > documentation for public functions and use new helper in existing multi-
> > gpu macro.
> > > >
> > > > v2: corrected description (Kamil), rebased on corrected patches
> > > > after Dominik review
> > > >
> > > > Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > > > ---
> > > >  lib/intel_multigpu.c | 94
> > > > ++++++++++++++++++++++++++++++++++++++++++--
> > > >  lib/intel_multigpu.h | 12 +++++-
> > > >  2 files changed, 101 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/lib/intel_multigpu.c b/lib/intel_multigpu.c index
> > > > 0e76d8aa3..e631ab0ed 100644
> > > > --- a/lib/intel_multigpu.c
> > > > +++ b/lib/intel_multigpu.c
> > > > @@ -4,10 +4,18 @@
> > > >   */
> > > >
> > > >  #include "drmtest.h"
> > > > +#include "i915/gem.h"
> > > >  #include "igt_core.h"
> > > >  #include "igt_device_scan.h"
> > > >  #include "intel_multigpu.h"
> > > >
> > > > +/**
> > > > + * gem_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 gem_multigpu_count_class(int class)  {
> > > >  	int count = 0;
> > > > @@ -18,7 +26,7 @@ int gem_multigpu_count_class(int class)
> > > >  	return count;
> > > >  }
> > > >
> > > > -void gem_require_multigpu(int count)
> > > > +static void skip_on_sigle_gpu(int count)
> > >
> > > Nitpick: typo
> > > I guess it should be named "skip_on_single_gpu".
> > >
> > 
> > Good point, I missed that, will fix.
> > 
> > > >  {
> > > >  	struct igt_devices_print_format fmt = {
> > > >  		.type = IGT_PRINT_SIMPLE,
> > > > @@ -26,9 +34,6 @@ void gem_require_multigpu(int count)
> > > >  	};
> > > >  	int devices;
> > > >
> > > > -	if (igt_device_filter_count() >= count)
> > > > -		return;
> > > > -
> > >
> > > As this conditional return was moved to gem_require_multigpu(), that
> > > function will work correctly. [...]

This is what git did, this wasn't actually moved anywhere outside of this
function, see below where it is added...

Regards,
Kamil

> > > Although, calling skip_on_single_gpu()
> > > directly will always cause a skip due to always reachable igt_skip.
> > > I suggest changing igt_skip to igt_skip_on to avoid unexpected
> > > behavior in the future.
> > 
> > What about changing this into print-only function and moving skips directly in
> > both functions?
> 
> Can you describe it further?
> 
> > 
> > >
> > > Additionally, skip_on_single_gpu can be refactored to not take any
> > > arguments, as we already get the number of gpus present in the system,
> > > and taking required count of gpus is not needed (it is always >1).
> > > The idea is to use igt_skip_on when devices < 2.
> > >
> > 
> > It was an optimization, I was thinking about other direction, adding one more
> > parameter to it and printing:
> > 
> > number of GPUs wanted
> > what number of GPUs program got
> > number of GPUs after PCI bus scan
> > 
> > What do you think?
> 
> Ok, but then skip_on_single_gpu won't hold to its name, unless you mean the change in gem_require_multigpu. Please further describe your idea.
> 
> Dominik Karol
> 
> > 
> > > >  	igt_info("PCI devices available in the system:\n");
> > > >
> > > >  	igt_devices_scan(true);
> > > > @@ -37,3 +42,84 @@ void gem_require_multigpu(int count)
> > > >
> > > >  	igt_skip("Test requires at least %d GPUs, %d available.\n", count,
> > > > devices);  }
> > > > +
> > > > +/**
> > > > + * gem_require_multigpu:
> > > > + * @count: minimum number of GPUs required
> > > > + *
> > > > + * Function checks number of filtered GPU cards.
> > > > + * On error skips and prints available GPUs found on PCI bus.
> > > > + */
> > > > +void gem_require_multigpu(int count) {
> > > > +	if (igt_device_filter_count() >= count)
> > > > +		return;
> > > > +
> > > > +	skip_on_sigle_gpu(count);
> > > > +}
> > > > +
> > > > +/**
> > > > + * multigpu_aquire_view:
> > > > + * @gpus_wanted: minimum number of GPUs required
> > > > + * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
> > > > + *
> > > > + * Function prepares filtered view for GPU cards with given chipset.
> > > > + * On error skips and prints available GPUs found on PCI bus.
> > > > + * Returns: number of GPUs found
> > > > + */
> > > > +int multigpu_aquire_view(int gpus_wanted, unsigned int chipset) {
> > >
> > > Nitpick: typo
> > > I guess it should be named "multigpu_acquire_view" (in function name and
> > documentation).
> > >
> > 
> > I will fix this,
> > 
> > Regards,
> > Kamil
> > 
> > > Dominik Karol
> > >
> > > > +	int gpu_count = drm_prepare_filtered_multigpu(chipset);
> > > > +
> > > > +	igt_assert(gpus_wanted > 1);
> > > > +	if (gpu_count < gpus_wanted)
> > > > +		skip_on_sigle_gpu(gpus_wanted);
> > > > +
> > > > +	return gpu_count;
> > > > +}
> > > > +
> > > > +/**
> > > > + * multigpu_open_another:
> > > > + * @idx: index of GPU card, starting from 0
> > > > + * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
> > > > + *
> > > > + * Function opens GPU card with drm_open_driver_another(). For i915
> > > > +it checks
> > > > + * if opened card is functional. Skips on errors.
> > > > + * Returns: opened fd for @idx card  */ int
> > > > +multigpu_open_another(int idx, unsigned int chipset) {
> > > > +	int fd;
> > > > +
> > > > +	igt_assert(idx >= 0);
> > > > +	fd = drm_open_driver_another(idx, chipset);
> > > > +
> > > > +	igt_require(fd != -1);
> > > > +	if (chipset == DRIVER_INTEL)
> > > > +		igt_require_gem(fd);
> > > > +
> > > > +	return fd;
> > > > +}
> > > > +
> > > > +/**
> > > > + * multigpu_open_filtered_card:
> > > > + * @idx: index of GPU card, starting from 0
> > > > + * @chipset: chipset, e.g. DRIVER_XE or DRIVER_INTEL
> > > > + *
> > > > + * Function opens GPU card prepared with filtered view. It also
> > > > +checks if
> > > > + * opened card agrees with desired chipset or checks if opened card
> > > > +is
> > > > + * functional. Skips on errors.
> > > > + * Returns: opened fd for @idx card  */ int
> > > > +multigpu_open_filtered_card(int idx, unsigned int chipset) {
> > > > +	int fd = drm_open_filtered_card(idx);
> > > > +
> > > > +	igt_require(fd != -1);
> > > > +	if (chipset == DRIVER_XE)
> > > > +		igt_require_xe(fd);
> > > > +	else if (chipset == DRIVER_INTEL)
> > > > +		igt_require_gem(fd);
> > > > +
> > > > +	return fd;
> > > > +}
> > > > diff --git a/lib/intel_multigpu.h b/lib/intel_multigpu.h index
> > > > 0ebc73e4a..0032a619e 100644
> > > > --- a/lib/intel_multigpu.h
> > > > +++ b/lib/intel_multigpu.h
> > > > @@ -12,6 +12,10 @@
> > > >  void gem_require_multigpu(int count);  int
> > > > gem_multigpu_count_class(int class);
> > > >
> > > > +int multigpu_aquire_view(int min_gpus_wanted, unsigned int
> > > > +chipset); int multigpu_open_another(int idx, unsigned int chipset);
> > > > +int multigpu_open_filtered_card(int idx, unsigned int chipset);
> > > > +
> > > >  #define igt_foreach_gpu(fd__, id__) \
> > > >  	for (int igt_unique(i) = 0, fd__; \
> > > >  		(fd__ = __drm_open_driver_another(igt_unique(i)++, id__)) >=
> > 0; \
> > > > @@ -19,8 +23,14 @@ int gem_multigpu_count_class(int class);
> > > >
> > > >  #define igt_multi_fork_foreach_gpu(__fd, __id) \
> > > >  	igt_multi_fork(igt_unique(__i), gem_multigpu_count_class(__id)) \
> > > > -		for (int __fd = drm_open_driver_another(igt_unique(__i),
> > > > __id); \
> > > > +		for (int __fd = multigpu_open_another(igt_unique(__i), __id);
> > > > \
> > > >  			__fd >= 0; \
> > > >  			close(__fd), __fd = -1) \
> > > >
> > > > +#define xe_multi_fork_foreach_gpu(__fd, __gpu_index) \
> > > > +	igt_multi_fork(__gpu_index, multigpu_aquire_view(2, DRIVER_XE)) \
> > > > +		for (int __fd = multigpu_open_filtered_card(__gpu_index,
> > > > DRIVER_XE); \
> > > > +			__fd >= 0; \
> > > > +			drm_close_driver(__fd), __fd = -1) \
> > > > +
> > > >  #endif /* __INTEL_MULTIGPU_H */
> > > > --
> > > > 2.42.0
> > >