[igt-dev] [PATCH i-g-t 0/2] Integrate igt_spin_new with Xe.

[igt-dev] [PATCH i-g-t 0/2] Integrate igt_spin_new with Xe.

[sai.gowtham.ch]

From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Extending the spin_create implementation and allocator handle support in xe,
where it submits dummy work loads to engine. This Implementation is wrapped
around vm_bind and unbind as we are supposed to do it manually for xe.

Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Sai Gowtham Ch (2):
  lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  tests/xe/xe_spin_batch: Add new test to exercise igt_spin_new for xe.

 lib/igt_dummyload.c      |  24 ++++++---
 lib/igt_dummyload.h      |  11 ++++
 lib/xe/xe_spin.c         |  68 +++++++++++++++++++++++
 lib/xe/xe_spin.h         |   7 +++
 tests/meson.build        |   1 +
 tests/xe/xe_spin_batch.c | 113 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 218 insertions(+), 6 deletions(-)
 create mode 100644 tests/xe/xe_spin_batch.c

-- 
2.39.1

[igt-dev] [PATCH i-g-t 0/2] Integrate igt_spin_new with Xe.

[sai.gowtham.ch]

From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Extending the spin_create implementation and allocator handle support in xe,
where it submits dummy work loads to engine. This Implementation is wrapped
around vm_bind and unbind as we are supposed to do it manually for xe.

Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Sai Gowtham Ch (2):
  lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  tests/xe/xe_spin_batch: Add new test to exercise igt_spin_new for xe.

 lib/igt_dummyload.c      |  24 +++++--
 lib/igt_dummyload.h      |  10 +++
 lib/xe/xe_spin.c         |  89 +++++++++++++++++++++++++
 lib/xe/xe_spin.h         |   7 ++
 tests/meson.build        |   1 +
 tests/xe/xe_spin_batch.c | 138 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 263 insertions(+), 6 deletions(-)
 create mode 100644 tests/xe/xe_spin_batch.c

-- 
2.39.1

[igt-dev] [PATCH i-g-t 1/2] lib/xe/xe_spin: Integrate igt_spin_new with Xe.

[sai.gowtham.ch]

From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Extending the spin_create implementation and allocator handle support in xe,
where it submits dummy work loads to engine. This Implementation is wrapped
around vm_bind and unbind as we are supposed to do it manually for xe.

Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
---
 lib/igt_dummyload.c | 24 +++++++++---
 lib/igt_dummyload.h | 10 +++++
 lib/xe/xe_spin.c    | 89 +++++++++++++++++++++++++++++++++++++++++++++
 lib/xe/xe_spin.h    |  7 ++++
 4 files changed, 124 insertions(+), 6 deletions(-)

diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 740a58f3..6e89b72d 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -46,6 +46,7 @@
 #include "intel_reg.h"
 #include "ioctl_wrappers.h"
 #include "sw_sync.h"
+#include "xe/xe_spin.h"
 
 /**
  * SECTION:igt_dummyload
@@ -447,7 +448,10 @@ spin_create(int fd, const struct igt_spin_factory *opts)
 igt_spin_t *
 __igt_spin_factory(int fd, const struct igt_spin_factory *opts)
 {
-	return spin_create(fd, opts);
+	if (is_xe_device(fd))
+		return xe_spin_create(fd, opts);
+	else
+		return spin_create(fd, opts);
 }
 
 /**
@@ -467,6 +471,11 @@ igt_spin_factory(int fd, const struct igt_spin_factory *opts)
 {
 	igt_spin_t *spin;
 
+	if (is_xe_device(fd)) {
+		spin = xe_spin_create(fd, opts);
+		return spin;
+	}
+
 	if ((opts->flags & IGT_SPIN_POLL_RUN) && opts->engine != ALL_ENGINES) {
 		unsigned int class;
 
@@ -647,11 +656,14 @@ void igt_spin_free(int fd, igt_spin_t *spin)
 	if (!spin)
 		return;
 
-	pthread_mutex_lock(&list_lock);
-	igt_list_del(&spin->link);
-	pthread_mutex_unlock(&list_lock);
-
-	__igt_spin_free(fd, spin);
+	if (is_xe_device(fd)) {
+		xe_spin_free(fd, spin);
+	} else {
+		pthread_mutex_lock(&list_lock);
+		igt_list_del(&spin->link);
+		pthread_mutex_unlock(&list_lock);
+		__igt_spin_free(fd, spin);
+	}
 }
 
 void igt_terminate_spins(void)
diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
index b247ab02..7bcc7b56 100644
--- a/lib/igt_dummyload.h
+++ b/lib/igt_dummyload.h
@@ -54,6 +54,8 @@ typedef struct igt_spin_factory {
 	unsigned int flags;
 	int fence;
 	uint64_t ahnd;
+	struct drm_xe_engine_class_instance *hwe;
+	uint32_t vm;
 } igt_spin_factory_t;
 
 typedef struct igt_spin {
@@ -83,6 +85,14 @@ typedef struct igt_spin {
 #define SPIN_CLFLUSH (1 << 0)
 
 	struct igt_spin_factory opts;
+
+	struct xe_spin *xe_spin;
+	size_t bo_size;
+	uint64_t address;
+	unsigned int engine;
+	uint32_t vm;
+	uint32_t syncobj;
+
 } igt_spin_t;
 
 
diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
index 856d0ba2..3a8c7bb3 100644
--- a/lib/xe/xe_spin.c
+++ b/lib/xe/xe_spin.c
@@ -15,6 +15,7 @@
 #include "intel_reg.h"
 #include "xe_ioctl.h"
 #include "xe_spin.h"
+#include "lib/igt_dummyload.h"
 
 /**
  * xe_spin_init:
@@ -82,6 +83,94 @@ void xe_spin_end(struct xe_spin *spin)
 	spin->end = 0;
 }
 
+igt_spin_t *
+xe_spin_create(int fd, const struct igt_spin_factory *opt)
+{
+	size_t bo_size = xe_get_default_alignment(fd);
+	uint32_t bo;
+	uint64_t ahnd = opt->ahnd, addr;
+	struct igt_spin *spin;
+	struct xe_spin *xe_spin;
+	struct drm_xe_sync sync = {
+		.flags = DRM_XE_SYNC_SYNCOBJ | DRM_XE_SYNC_SIGNAL,
+	};
+	struct drm_xe_exec exec = {
+		.num_batch_buffer = 1,
+		.num_syncs = 1,
+		.syncs = to_user_pointer(&sync),
+	};
+
+	igt_assert(ahnd);
+	spin = calloc(1, sizeof(struct igt_spin));
+	igt_assert(spin);
+
+	spin->syncobj = syncobj_create(fd, 0);
+	if (opt->engine) {
+		spin->opts.engine = opt->engine;
+		spin->opts.vm = opt->vm;
+
+		spin->handle = xe_bo_create(fd, 0, spin->opts.vm, bo_size);
+		xe_spin = xe_bo_map(fd, spin->handle, bo_size);
+		addr = intel_allocator_alloc_with_strategy(ahnd, spin->handle, bo_size, 0, ALLOC_STRATEGY_LOW_TO_HIGH);
+		xe_vm_bind_sync(fd, spin->opts.vm, spin->handle, 0, addr, bo_size);
+
+		xe_spin_init(xe_spin, addr, true);
+		exec.engine_id = spin->opts.engine;
+		exec.address = addr;
+	} else {
+		spin->vm = xe_vm_create(fd, 0, 0);
+		spin->engine = xe_engine_create(fd, spin->vm, opt->hwe, 0);
+
+		bo = xe_bo_create(fd, 0, spin->vm, bo_size);
+		spin->handle = bo;
+		xe_spin = xe_bo_map(fd, spin->handle, bo_size);
+		addr = intel_allocator_alloc_with_strategy(ahnd, spin->handle, bo_size, 0, ALLOC_STRATEGY_LOW_TO_HIGH);
+		xe_vm_bind_sync(fd, spin->vm, spin->handle, 0, addr, bo_size);
+
+		xe_spin_init(xe_spin, addr, true);
+		exec.engine_id = spin->engine;
+		exec.address = addr;
+	}
+	sync.handle = spin->syncobj;
+	igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_EXEC, &exec), 0);
+	xe_spin_wait_started(xe_spin);
+	igt_info("Spinner started\n");
+
+	spin->bo_size = bo_size;
+	spin->address = addr;
+	spin->xe_spin = xe_spin;
+
+	return spin;
+}
+
+void xe_spin_sync_wait(int fd, struct igt_spin *spin)
+{
+	igt_assert(syncobj_wait(fd, &spin->syncobj, 1, INT64_MAX, 0,
+				NULL));
+}
+
+void xe_spin_free(int fd, struct igt_spin *spin)
+{
+	xe_spin_end(spin->xe_spin);
+	xe_spin_sync_wait(fd, spin);
+
+	if (!spin->opts.engine) {
+		xe_vm_unbind_sync(fd, spin->vm, 0, spin->address, spin->bo_size);
+	} else {
+		xe_vm_unbind_sync(fd, spin->opts.vm, 0, spin->address, spin->bo_size);
+	}
+
+	syncobj_destroy(fd, spin->syncobj);
+	gem_munmap(spin->xe_spin, spin->bo_size);
+	gem_close(fd, spin->handle);
+
+	if (!spin->opts.engine) {
+		xe_engine_destroy(fd, spin->engine);
+		xe_vm_destroy(fd, spin->vm);
+	}
+	free(spin);
+}
+
 void xe_cork_init(int fd, struct drm_xe_engine_class_instance *hwe,
 		  struct xe_cork *cork)
 {
diff --git a/lib/xe/xe_spin.h b/lib/xe/xe_spin.h
index 73f9a026..48867eb8 100644
--- a/lib/xe/xe_spin.h
+++ b/lib/xe/xe_spin.h
@@ -13,19 +13,26 @@
 #include <stdbool.h>
 
 #include "xe_query.h"
+#include "lib/igt_dummyload.h"
 
 /* Mapped GPU object */
+
 struct xe_spin {
 	uint32_t batch[16];
 	uint64_t pad;
 	uint32_t start;
 	uint32_t end;
+
 };
 
+igt_spin_t *
+xe_spin_create(int fd, const struct igt_spin_factory *opt);
 void xe_spin_init(struct xe_spin *spin, uint64_t addr, bool preempt);
 bool xe_spin_started(struct xe_spin *spin);
+void xe_spin_sync_wait(int fd, struct igt_spin *spin);
 void xe_spin_wait_started(struct xe_spin *spin);
 void xe_spin_end(struct xe_spin *spin);
+void xe_spin_free(int fd, struct igt_spin *spin);
 
 struct xe_cork {
 	struct xe_spin *spin;
-- 
2.39.1

[igt-dev] [PATCH i-g-t 2/2] tests/xe/xe_spin_batch: Add new test to exercise igt_spin_new for xe.

[sai.gowtham.ch]

From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

xe_spin_batch test exercises basic igt_spin_new submissions and
and with all engines.

Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
---
 tests/meson.build        |   1 +
 tests/xe/xe_spin_batch.c | 138 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 139 insertions(+)
 create mode 100644 tests/xe/xe_spin_batch.c

diff --git a/tests/meson.build b/tests/meson.build
index f71be1db..e794b75a 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -268,6 +268,7 @@ xe_progs = [
 	'xe_query',
 	'xe_vm',
 	'xe_waitfence',
+	'xe_spin_batch',
 ]
 
 msm_progs = [
diff --git a/tests/xe/xe_spin_batch.c b/tests/xe/xe_spin_batch.c
new file mode 100644
index 00000000..1b216199
--- /dev/null
+++ b/tests/xe/xe_spin_batch.c
@@ -0,0 +1,138 @@
+#include "igt.h"
+#include "lib/intel_reg.h"
+#include "xe_drm.h"
+#include "xe/xe_ioctl.h"
+#include "xe/xe_query.h"
+
+#define MAX_INSTANCE 9
+/**
+ * TEST:Test for spin batch submissons.
+ *
+ * SUBTEST: spin-batch
+ * Description: Test to submit spin batch with engines and vm.
+ * Run type: FULL
+ * TODO: change ``'Run type' == FULL`` to a better category
+ *
+ */
+
+static void spin(int fd, struct drm_xe_engine_class_instance *hwe)
+{
+	uint64_t ahnd;
+	unsigned int engine;
+	uint32_t vm;
+	igt_spin_t *spin;
+
+	vm = xe_vm_create(fd, 0, 0);
+	engine = xe_engine_create(fd, vm, hwe, 0);
+	ahnd = intel_allocator_open(fd, 0, INTEL_ALLOCATOR_RELOC);
+
+	spin = __igt_spin_new(fd, .ahnd = ahnd, .engine = engine, .vm = vm);
+	igt_assert(spin);
+
+	igt_spin_free(fd, spin);
+	put_ahnd(ahnd);
+}
+
+/**
+ * TEST: Basic test for spin batch submission.
+ *
+ * SUBTEST: spin-basic
+ * Description: Basic test which validates the functionality of spinner.
+ * Run type: FULL
+ * TODO: change ``'Run type' == FULL`` to a better category
+ *
+ */
+static void spin_basic(int fd, struct drm_xe_engine_class_instance *hwe)
+{
+	uint64_t ahnd;
+	igt_spin_t *spin;
+
+	ahnd = intel_allocator_open(fd, 0, INTEL_ALLOCATOR_RELOC);
+	spin = __igt_spin_new(fd, .ahnd = ahnd, .hwe = hwe);
+	igt_assert(spin);
+
+	igt_spin_free(fd, spin);
+	put_ahnd(ahnd);
+}
+
+/**
+ * TEST: Test for spin batch submissions.
+ * SUBTEST: spin-all
+ * Description: Spinner test to run on all the engines!
+ * Run type: FULL
+ * TODO: change ``'Run type' == FULL`` to a better category
+ *
+ */
+
+static void spin_all (int fd, int gt, int class)
+{
+	uint64_t ahnd;
+	uint32_t engines[MAX_INSTANCE];
+	uint32_t vm[MAX_INSTANCE];
+	int i, num_placements = 0;
+	struct drm_xe_engine_class_instance eci[MAX_INSTANCE];
+	igt_spin_t *spin[MAX_INSTANCE];
+	struct drm_xe_engine_class_instance *hwe;
+
+	ahnd = intel_allocator_open(fd, 0, INTEL_ALLOCATOR_RELOC);
+
+	xe_for_each_hw_engine(fd, hwe) {
+		if (hwe->engine_class != class || hwe->gt_id != gt)
+			continue;
+		eci[num_placements++] = *hwe;
+	}
+	if (num_placements < 2)
+		return;
+
+	for (i = 0; i < num_placements; i++) {
+		struct drm_xe_engine_create create;
+		vm[i] = xe_vm_create(fd, 0, 0);
+
+		create.vm_id = vm[i];
+		create.width = 1;
+		create.num_placements = num_placements;
+		create.instances = to_user_pointer(eci);
+
+		igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_ENGINE_CREATE,
+					&create), 0);
+		engines[i] = create.engine_id;
+		spin[i] = __igt_spin_new(fd, .ahnd = ahnd, .engine = engines[i], .vm = vm[i]);
+	}
+
+	for (i = 0; i < num_placements; i++) {
+		igt_assert(spin[i]);
+		igt_spin_free(fd, spin[i]);
+	}
+	put_ahnd(ahnd);
+}
+
+igt_main
+{
+	struct drm_xe_engine_class_instance *hwe;
+	int fd;
+	int gt, class;
+
+	igt_fixture {
+		fd = drm_open_driver(DRIVER_XE);
+		xe_device_get(fd);
+	}
+
+	igt_subtest("spin-batch")
+		xe_for_each_hw_engine(fd, hwe)
+			spin(fd, hwe);
+
+	igt_subtest("spin-basic")
+		xe_for_each_hw_engine(fd, hwe)
+			spin_basic(fd, hwe);
+
+	igt_subtest("spin-all") {
+		xe_for_each_gt(fd, gt)
+			xe_for_each_hw_engine_class(class)
+				spin_all(fd, gt, class);
+	}
+
+	igt_fixture {
+		xe_device_put(fd);
+		close(fd);
+	}
+}
-- 
2.39.1

[igt-dev] ✗ GitLab.Pipeline: warning for Integrate igt_spin_new with Xe. (rev2)

[Patchwork]

== Series Details ==

Series: Integrate igt_spin_new with Xe. (rev2)
URL   : https://patchwork.freedesktop.org/series/118121/
State : warning

== Summary ==

Pipeline status: FAILED.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/890115 for the overview.

build-containers:build-debian has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/42415062):
  time="2023-05-25T06:02:24Z" level=fatal msg="Invalid status code returned when fetching blob 500 (Internal Server Error)" 
  Building!
  STEP 1: FROM debian:buster
  Getting image source signatures
  Copying blob sha256:c722db24a050621ee87ea07acd5d066d3d6a94737c32012f27d73a1ad5cc645c
  Copying config sha256:8b5601a5a7f855241ac7f372ec0042e793b0b3eb3f3a601014845f22bd371c90
  Writing manifest to image destination
  Storing signatures
  STEP 2: RUN apt-get update
  error running container: error creating container for [/bin/sh -c apt-get update]: time="2023-05-25T06:02:29Z" level=warning msg="signal: killed"
  time="2023-05-25T06:02:29Z" level=error msg="container_linux.go:346: starting container process caused \"process_linux.go:297: applying cgroup configuration for process caused \\\"mountpoint for cgroup not found\\\"\"\n"
  container_linux.go:346: starting container process caused "process_linux.go:297: applying cgroup configuration for process caused \"mountpoint for cgroup not found\""
  : exit status 1
  Error: error building at STEP "RUN apt-get update": error while running runtime: exit status 1
  section_end:1684994551:step_script
  section_start:1684994551:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1684994552:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build-containers:build-debian-arm64 has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/42415064):
  time="2023-05-25T06:02:23Z" level=fatal msg="Invalid status code returned when fetching blob 500 (Internal Server Error)" 
  Building!
  STEP 1: FROM debian:buster
  Getting image source signatures
  Copying blob sha256:c722db24a050621ee87ea07acd5d066d3d6a94737c32012f27d73a1ad5cc645c
  Copying config sha256:8b5601a5a7f855241ac7f372ec0042e793b0b3eb3f3a601014845f22bd371c90
  Writing manifest to image destination
  Storing signatures
  STEP 2: RUN apt-get update
  error running container: error creating container for [/bin/sh -c apt-get update]: time="2023-05-25T06:02:26Z" level=warning msg="signal: killed"
  time="2023-05-25T06:02:26Z" level=error msg="container_linux.go:346: starting container process caused \"process_linux.go:297: applying cgroup configuration for process caused \\\"mountpoint for cgroup not found\\\"\"\n"
  container_linux.go:346: starting container process caused "process_linux.go:297: applying cgroup configuration for process caused \"mountpoint for cgroup not found\""
  : exit status 1
  Error: error building at STEP "RUN apt-get update": error while running runtime: exit status 1
  section_end:1684994546:step_script
  section_start:1684994546:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1684994548:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build-containers:build-debian-armhf has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/42415063):
  time="2023-05-25T06:02:28Z" level=fatal msg="Invalid status code returned when fetching blob 500 (Internal Server Error)" 
  Building!
  STEP 1: FROM debian:buster
  Getting image source signatures
  Copying blob sha256:c722db24a050621ee87ea07acd5d066d3d6a94737c32012f27d73a1ad5cc645c
  Copying config sha256:8b5601a5a7f855241ac7f372ec0042e793b0b3eb3f3a601014845f22bd371c90
  Writing manifest to image destination
  Storing signatures
  STEP 2: RUN apt-get update
  error running container: error creating container for [/bin/sh -c apt-get update]: time="2023-05-25T06:02:32Z" level=warning msg="signal: killed"
  time="2023-05-25T06:02:32Z" level=error msg="container_linux.go:346: starting container process caused \"process_linux.go:297: applying cgroup configuration for process caused \\\"mountpoint for cgroup not found\\\"\"\n"
  container_linux.go:346: starting container process caused "process_linux.go:297: applying cgroup configuration for process caused \"mountpoint for cgroup not found\""
  : exit status 1
  Error: error building at STEP "RUN apt-get update": error while running runtime: exit status 1
  section_end:1684994554:step_script
  section_start:1684994554:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1684994559:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build-containers:build-debian-mips has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/42415065):
  time="2023-05-25T06:02:24Z" level=fatal msg="Invalid status code returned when fetching blob 500 (Internal Server Error)" 
  Building!
  STEP 1: FROM debian:buster
  Getting image source signatures
  Copying blob sha256:c722db24a050621ee87ea07acd5d066d3d6a94737c32012f27d73a1ad5cc645c
  Copying config sha256:8b5601a5a7f855241ac7f372ec0042e793b0b3eb3f3a601014845f22bd371c90
  Writing manifest to image destination
  Storing signatures
  STEP 2: RUN apt-get update
  error running container: error creating container for [/bin/sh -c apt-get update]: time="2023-05-25T06:02:28Z" level=warning msg="signal: killed"
  time="2023-05-25T06:02:28Z" level=error msg="container_linux.go:346: starting container process caused \"process_linux.go:297: applying cgroup configuration for process caused \\\"mountpoint for cgroup not found\\\"\"\n"
  container_linux.go:346: starting container process caused "process_linux.go:297: applying cgroup configuration for process caused \"mountpoint for cgroup not found\""
  : exit status 1
  Error: error building at STEP "RUN apt-get update": error while running runtime: exit status 1
  section_end:1684994549:step_script
  section_start:1684994549:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1684994550:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build-containers:build-fedora has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/42415066):
  time="2023-05-25T06:02:27Z" level=fatal msg="Invalid status code returned when fetching blob 500 (Internal Server Error)" 
  Building!
  STEP 1: FROM fedora:31
  Getting image source signatures
  Copying blob sha256:854946d575a439a894349addd141568875d7c1e673d3286b08250f3dde002e6a
  Copying config sha256:7e94ed77b448a8d2ff08b92d3ca743e4e862c744892d6886c73487581eb5863a
  Writing manifest to image destination
  Storing signatures
  STEP 2: RUN dnf install -y 	gcc flex bison libatomic meson ninja-build xdotool 	'pkgconfig(libdrm)' 	'pkgconfig(pciaccess)' 	'pkgconfig(libkmod)' 	'pkgconfig(libprocps)' 	'pkgconfig(libunwind)' 	'pkgconfig(libdw)' 	'pkgconfig(pixman-1)' 	'pkgconfig(valgrind)' 	'pkgconfig(cairo)' 	'pkgconfig(libudev)' 	'pkgconfig(glib-2.0)' 	'pkgconfig(gsl)' 	'pkgconfig(alsa)' 	'pkgconfig(xmlrpc)' 	'pkgconfig(xmlrpc_util)' 	'pkgconfig(xmlrpc_client)' 	'pkgconfig(json-c)' 	'pkgconfig(gtk-doc)' 	'pkgconfig(xv)' 	'pkgconfig(xrandr)' 	python3-docutils
  error running container: error creating container for [/bin/sh -c dnf install -y 	gcc flex bison libatomic meson ninja-build xdotool 	'pkgconfig(libdrm)' 	'pkgconfig(pciaccess)' 	'pkgconfig(libkmod)' 	'pkgconfig(libprocps)' 	'pkgconfig(libunwind)' 	'pkgconfig(libdw)' 	'pkgconfig(pixman-1)' 	'pkgconfig(valgrind)' 	'pkgconfig(cairo)' 	'pkgconfig(libudev)' 	'pkgconfig(glib-2.0)' 	'pkgconfig(gsl)' 	'pkgconfig(alsa)' 	'pkgconfig(xmlrpc)' 	'pkgconfig(xmlrpc_util)' 	'pkgconfig(xmlrpc_client)' 	'pkgconfig(json-c)' 	'pkgconfig(gtk-doc)' 	'pkgconfig(xv)' 	'pkgconfig(xrandr)' 	python3-docutils]: time="2023-05-25T06:02:31Z" level=warning msg="signal: killed"
  time="2023-05-25T06:02:31Z" level=error msg="container_linux.go:346: starting container process caused \"process_linux.go:297: applying cgroup configuration for process caused \\\"mountpoint for cgroup not found\\\"\"\n"
  container_linux.go:346: starting container process caused "process_linux.go:297: applying cgroup configuration for process caused \"mountpoint for cgroup not found\""
  : exit status 1
  Error: error building at STEP "RUN dnf install -y 	gcc flex bison libatomic meson ninja-build xdotool 	'pkgconfig(libdrm)' 	'pkgconfig(pciaccess)' 	'pkgconfig(libkmod)' 	'pkgconfig(libprocps)' 	'pkgconfig(libunwind)' 	'pkgconfig(libdw)' 	'pkgconfig(pixman-1)' 	'pkgconfig(valgrind)' 	'pkgconfig(cairo)' 	'pkgconfig(libudev)' 	'pkgconfig(glib-2.0)' 	'pkgconfig(gsl)' 	'pkgconfig(alsa)' 	'pkgconfig(xmlrpc)' 	'pkgconfig(xmlrpc_util)' 	'pkgconfig(xmlrpc_client)' 	'pkgconfig(json-c)' 	'pkgconfig(gtk-doc)' 	'pkgconfig(xv)' 	'pkgconfig(xrandr)' 	python3-docutils": error while running runtime: exit status 1
  section_end:1684994552:step_script
  section_start:1684994552:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1684994554:cleanup_file_variables
  ERROR: Job failed: exit code 1

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/890115

Re: [igt-dev] [PATCH i-g-t 1/2] lib/xe/xe_spin: Integrate igt_spin_new with Xe.

[Zbigniew Kempczyński]

On Thu, May 25, 2023 at 11:25:18AM +0530, sai.gowtham.ch@intel.com wrote:
> From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> 
> Extending the spin_create implementation and allocator handle support in xe,
> where it submits dummy work loads to engine. This Implementation is wrapped
> around vm_bind and unbind as we are supposed to do it manually for xe.
> 
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> ---
>  lib/igt_dummyload.c | 24 +++++++++---
>  lib/igt_dummyload.h | 10 +++++
>  lib/xe/xe_spin.c    | 89 +++++++++++++++++++++++++++++++++++++++++++++
>  lib/xe/xe_spin.h    |  7 ++++
>  4 files changed, 124 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index 740a58f3..6e89b72d 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -46,6 +46,7 @@
>  #include "intel_reg.h"
>  #include "ioctl_wrappers.h"
>  #include "sw_sync.h"
> +#include "xe/xe_spin.h"
>  
>  /**
>   * SECTION:igt_dummyload
> @@ -447,7 +448,10 @@ spin_create(int fd, const struct igt_spin_factory *opts)
>  igt_spin_t *
>  __igt_spin_factory(int fd, const struct igt_spin_factory *opts)
>  {
> -	return spin_create(fd, opts);
> +	if (is_xe_device(fd))
> +		return xe_spin_create(fd, opts);
> +	else
> +		return spin_create(fd, opts);
>  }
>  
>  /**
> @@ -467,6 +471,11 @@ igt_spin_factory(int fd, const struct igt_spin_factory *opts)
>  {
>  	igt_spin_t *spin;
>  
> +	if (is_xe_device(fd)) {
> +		spin = xe_spin_create(fd, opts);
> +		return spin;
> +	}
> +
>  	if ((opts->flags & IGT_SPIN_POLL_RUN) && opts->engine != ALL_ENGINES) {
>  		unsigned int class;
>  
> @@ -647,11 +656,14 @@ void igt_spin_free(int fd, igt_spin_t *spin)
>  	if (!spin)
>  		return;
>  
> -	pthread_mutex_lock(&list_lock);
> -	igt_list_del(&spin->link);
> -	pthread_mutex_unlock(&list_lock);
> -
> -	__igt_spin_free(fd, spin);
> +	if (is_xe_device(fd)) {
> +		xe_spin_free(fd, spin);
> +	} else {
> +		pthread_mutex_lock(&list_lock);
> +		igt_list_del(&spin->link);
> +		pthread_mutex_unlock(&list_lock);
> +		__igt_spin_free(fd, spin);
> +	}
>  }
>  
>  void igt_terminate_spins(void)
> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
> index b247ab02..7bcc7b56 100644
> --- a/lib/igt_dummyload.h
> +++ b/lib/igt_dummyload.h
> @@ -54,6 +54,8 @@ typedef struct igt_spin_factory {
>  	unsigned int flags;
>  	int fence;
>  	uint64_t ahnd;
> +	struct drm_xe_engine_class_instance *hwe;
> +	uint32_t vm;
>  } igt_spin_factory_t;
>  
>  typedef struct igt_spin {
> @@ -83,6 +85,14 @@ typedef struct igt_spin {
>  #define SPIN_CLFLUSH (1 << 0)
>  
>  	struct igt_spin_factory opts;
> +
> +	struct xe_spin *xe_spin;
> +	size_t bo_size;
> +	uint64_t address;
> +	unsigned int engine;
> +	uint32_t vm;
> +	uint32_t syncobj;
> +
>  } igt_spin_t;
>  
>  
> diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
> index 856d0ba2..3a8c7bb3 100644
> --- a/lib/xe/xe_spin.c
> +++ b/lib/xe/xe_spin.c
> @@ -15,6 +15,7 @@
>  #include "intel_reg.h"
>  #include "xe_ioctl.h"
>  #include "xe_spin.h"
> +#include "lib/igt_dummyload.h"
>  
>  /**
>   * xe_spin_init:
> @@ -82,6 +83,94 @@ void xe_spin_end(struct xe_spin *spin)
>  	spin->end = 0;
>  }
>  
> +igt_spin_t *
> +xe_spin_create(int fd, const struct igt_spin_factory *opt)
> +{
> +	size_t bo_size = xe_get_default_alignment(fd);
> +	uint32_t bo;
> +	uint64_t ahnd = opt->ahnd, addr;
> +	struct igt_spin *spin;
> +	struct xe_spin *xe_spin;
> +	struct drm_xe_sync sync = {
> +		.flags = DRM_XE_SYNC_SYNCOBJ | DRM_XE_SYNC_SIGNAL,
> +	};
> +	struct drm_xe_exec exec = {
> +		.num_batch_buffer = 1,
> +		.num_syncs = 1,
> +		.syncs = to_user_pointer(&sync),
> +	};
> +
> +	igt_assert(ahnd);
> +	spin = calloc(1, sizeof(struct igt_spin));
> +	igt_assert(spin);
> +
> +	spin->syncobj = syncobj_create(fd, 0);
> +	if (opt->engine) {
> +		spin->opts.engine = opt->engine;
> +		spin->opts.vm = opt->vm;
> +
> +		spin->handle = xe_bo_create(fd, 0, spin->opts.vm, bo_size);
> +		xe_spin = xe_bo_map(fd, spin->handle, bo_size);
> +		addr = intel_allocator_alloc_with_strategy(ahnd, spin->handle, bo_size, 0, ALLOC_STRATEGY_LOW_TO_HIGH);
> +		xe_vm_bind_sync(fd, spin->opts.vm, spin->handle, 0, addr, bo_size);
> +
> +		xe_spin_init(xe_spin, addr, true);
> +		exec.engine_id = spin->opts.engine;
> +		exec.address = addr;
> +	} else {
> +		spin->vm = xe_vm_create(fd, 0, 0);
> +		spin->engine = xe_engine_create(fd, spin->vm, opt->hwe, 0);
> +
> +		bo = xe_bo_create(fd, 0, spin->vm, bo_size);
> +		spin->handle = bo;
> +		xe_spin = xe_bo_map(fd, spin->handle, bo_size);
> +		addr = intel_allocator_alloc_with_strategy(ahnd, spin->handle, bo_size, 0, ALLOC_STRATEGY_LOW_TO_HIGH);
> +		xe_vm_bind_sync(fd, spin->vm, spin->handle, 0, addr, bo_size);
> +
> +		xe_spin_init(xe_spin, addr, true);
> +		exec.engine_id = spin->engine;
> +		exec.address = addr;
> +	}

I think you should support two variants of executing spinners:

1. hwe != NULL (in this case engine must be 0):
   if (!vm) -> create vm;
   create engine for vm according to hwe

2. engine != 0, vm must be vm on top of which engine was created
   hwe must be NULL in this case
   see Dominik answer about passing engine+vm in:
   https://patchwork.freedesktop.org/patch/539173/?series=118227&rev=2

And add support ALL_ENGINES flag (then iterate over all hw engines
like Bhanu suggested). And track engine ownership - if caller
is the engine/vm owner use it but don't destroy, if spinner code
is creating engine you may freely destroy it in spinner free path().

--
Zbigniew


> +	sync.handle = spin->syncobj;
> +	igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_EXEC, &exec), 0);
> +	xe_spin_wait_started(xe_spin);
> +	igt_info("Spinner started\n");
> +
> +	spin->bo_size = bo_size;
> +	spin->address = addr;
> +	spin->xe_spin = xe_spin;
> +
> +	return spin;
> +}
> +
> +void xe_spin_sync_wait(int fd, struct igt_spin *spin)
> +{
> +	igt_assert(syncobj_wait(fd, &spin->syncobj, 1, INT64_MAX, 0,
> +				NULL));
> +}
> +
> +void xe_spin_free(int fd, struct igt_spin *spin)
> +{
> +	xe_spin_end(spin->xe_spin);
> +	xe_spin_sync_wait(fd, spin);
> +
> +	if (!spin->opts.engine) {
> +		xe_vm_unbind_sync(fd, spin->vm, 0, spin->address, spin->bo_size);
> +	} else {
> +		xe_vm_unbind_sync(fd, spin->opts.vm, 0, spin->address, spin->bo_size);
> +	}
> +
> +	syncobj_destroy(fd, spin->syncobj);
> +	gem_munmap(spin->xe_spin, spin->bo_size);
> +	gem_close(fd, spin->handle);
> +
> +	if (!spin->opts.engine) {
> +		xe_engine_destroy(fd, spin->engine);
> +		xe_vm_destroy(fd, spin->vm);
> +	}
> +	free(spin);
> +}
> +
>  void xe_cork_init(int fd, struct drm_xe_engine_class_instance *hwe,
>  		  struct xe_cork *cork)
>  {
> diff --git a/lib/xe/xe_spin.h b/lib/xe/xe_spin.h
> index 73f9a026..48867eb8 100644
> --- a/lib/xe/xe_spin.h
> +++ b/lib/xe/xe_spin.h
> @@ -13,19 +13,26 @@
>  #include <stdbool.h>
>  
>  #include "xe_query.h"
> +#include "lib/igt_dummyload.h"
>  
>  /* Mapped GPU object */
> +
>  struct xe_spin {
>  	uint32_t batch[16];
>  	uint64_t pad;
>  	uint32_t start;
>  	uint32_t end;
> +
>  };
>  
> +igt_spin_t *
> +xe_spin_create(int fd, const struct igt_spin_factory *opt);
>  void xe_spin_init(struct xe_spin *spin, uint64_t addr, bool preempt);
>  bool xe_spin_started(struct xe_spin *spin);
> +void xe_spin_sync_wait(int fd, struct igt_spin *spin);
>  void xe_spin_wait_started(struct xe_spin *spin);
>  void xe_spin_end(struct xe_spin *spin);
> +void xe_spin_free(int fd, struct igt_spin *spin);
>  
>  struct xe_cork {
>  	struct xe_spin *spin;
> -- 
> 2.39.1
> 

Re: [igt-dev] [PATCH i-g-t 2/2] tests/xe/xe_spin_batch: Add new test to exercise igt_spin_new for xe.

[Zbigniew Kempczyński]

On Thu, May 25, 2023 at 11:25:19AM +0530, sai.gowtham.ch@intel.com wrote:
> From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> 
> xe_spin_batch test exercises basic igt_spin_new submissions and
> and with all engines.
> 
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> ---
>  tests/meson.build        |   1 +
>  tests/xe/xe_spin_batch.c | 138 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 139 insertions(+)
>  create mode 100644 tests/xe/xe_spin_batch.c
> 
> diff --git a/tests/meson.build b/tests/meson.build
> index f71be1db..e794b75a 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -268,6 +268,7 @@ xe_progs = [
>  	'xe_query',
>  	'xe_vm',
>  	'xe_waitfence',
> +	'xe_spin_batch',
>  ]
>  
>  msm_progs = [
> diff --git a/tests/xe/xe_spin_batch.c b/tests/xe/xe_spin_batch.c
> new file mode 100644
> index 00000000..1b216199
> --- /dev/null
> +++ b/tests/xe/xe_spin_batch.c
> @@ -0,0 +1,138 @@
> +#include "igt.h"
> +#include "lib/intel_reg.h"
> +#include "xe_drm.h"
> +#include "xe/xe_ioctl.h"
> +#include "xe/xe_query.h"
> +
> +#define MAX_INSTANCE 9
> +/**
> + * TEST:Test for spin batch submissons.
> + *
> + * SUBTEST: spin-batch
> + * Description: Test to submit spin batch with engines and vm.
> + * Run type: FULL
> + * TODO: change ``'Run type' == FULL`` to a better category
> + *
> + */
> +
> +static void spin(int fd, struct drm_xe_engine_class_instance *hwe)
> +{
> +	uint64_t ahnd;
> +	unsigned int engine;
> +	uint32_t vm;
> +	igt_spin_t *spin;
> +
> +	vm = xe_vm_create(fd, 0, 0);
> +	engine = xe_engine_create(fd, vm, hwe, 0);
> +	ahnd = intel_allocator_open(fd, 0, INTEL_ALLOCATOR_RELOC);
> +
> +	spin = __igt_spin_new(fd, .ahnd = ahnd, .engine = engine, .vm = vm);
> +	igt_assert(spin);
> +
> +	igt_spin_free(fd, spin);

You shouldn't imo destroy engine/vm inside igt_spin_free(). Caller
is the owner and they should stay intact.

If instead .engine you'll pass .hwe you may create engine on top
of vm (if passed, if not create new one) and destroy in igt_spin_free().

Resume: track the ownershipping.

> +	put_ahnd(ahnd);
> +}
> +
> +/**
> + * TEST: Basic test for spin batch submission.
> + *
> + * SUBTEST: spin-basic
> + * Description: Basic test which validates the functionality of spinner.
> + * Run type: FULL
> + * TODO: change ``'Run type' == FULL`` to a better category
> + *
> + */
> +static void spin_basic(int fd, struct drm_xe_engine_class_instance *hwe)
> +{
> +	uint64_t ahnd;
> +	igt_spin_t *spin;
> +
> +	ahnd = intel_allocator_open(fd, 0, INTEL_ALLOCATOR_RELOC);
> +	spin = __igt_spin_new(fd, .ahnd = ahnd, .hwe = hwe);
> +	igt_assert(spin);
> +
> +	igt_spin_free(fd, spin);
> +	put_ahnd(ahnd);
> +}
> +
> +/**
> + * TEST: Test for spin batch submissions.
> + * SUBTEST: spin-all
> + * Description: Spinner test to run on all the engines!
> + * Run type: FULL
> + * TODO: change ``'Run type' == FULL`` to a better category
> + *
> + */
> +
> +static void spin_all (int fd, int gt, int class)
> +{
> +	uint64_t ahnd;
> +	uint32_t engines[MAX_INSTANCE];
> +	uint32_t vm[MAX_INSTANCE];
> +	int i, num_placements = 0;
> +	struct drm_xe_engine_class_instance eci[MAX_INSTANCE];
> +	igt_spin_t *spin[MAX_INSTANCE];
> +	struct drm_xe_engine_class_instance *hwe;
> +
> +	ahnd = intel_allocator_open(fd, 0, INTEL_ALLOCATOR_RELOC);
> +
> +	xe_for_each_hw_engine(fd, hwe) {
> +		if (hwe->engine_class != class || hwe->gt_id != gt)
> +			continue;
> +		eci[num_placements++] = *hwe;
> +	}
> +	if (num_placements < 2)
> +		return;
> +
> +	for (i = 0; i < num_placements; i++) {
> +		struct drm_xe_engine_create create;
> +		vm[i] = xe_vm_create(fd, 0, 0);
> +
> +		create.vm_id = vm[i];
> +		create.width = 1;
> +		create.num_placements = num_placements;
> +		create.instances = to_user_pointer(eci);
> +
> +		igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_ENGINE_CREATE,
> +					&create), 0);
> +		engines[i] = create.engine_id;
> +		spin[i] = __igt_spin_new(fd, .ahnd = ahnd, .engine = engines[i], .vm = vm[i]);
> +	}
> +
> +	for (i = 0; i < num_placements; i++) {
> +		igt_assert(spin[i]);
> +		igt_spin_free(fd, spin[i]);
> +	}
> +	put_ahnd(ahnd);
> +}
> +
> +igt_main
> +{
> +	struct drm_xe_engine_class_instance *hwe;
> +	int fd;
> +	int gt, class;
> +
> +	igt_fixture {
> +		fd = drm_open_driver(DRIVER_XE);
> +		xe_device_get(fd);
> +	}
> +
> +	igt_subtest("spin-batch")
> +		xe_for_each_hw_engine(fd, hwe)
> +			spin(fd, hwe);

Test both variants: with engine/vm created in the test and passed
via .engine + .vm field and with passing .hwe only. Add also
support for ALL_ENGINES flag.

--
Zbigniew

> +
> +	igt_subtest("spin-basic")
> +		xe_for_each_hw_engine(fd, hwe)
> +			spin_basic(fd, hwe);
> +
> +	igt_subtest("spin-all") {
> +		xe_for_each_gt(fd, gt)
> +			xe_for_each_hw_engine_class(class)
> +				spin_all(fd, gt, class);
> +	}
> +
> +	igt_fixture {
> +		xe_device_put(fd);
> +		close(fd);
> +	}
> +}
> -- 
> 2.39.1
> 

Re: [igt-dev] [PATCH i-g-t 2/2] tests/xe/xe_spin_batch: Add new test to exercise igt_spin_new for xe.

[Ch, Sai Gowtham]

> -----Original Message-----
> From: Kempczynski, Zbigniew <zbigniew.kempczynski@intel.com>
> Sent: Monday, May 29, 2023 11:27 AM
> To: Ch, Sai Gowtham <sai.gowtham.ch@intel.com>
> Cc: igt-dev@lists.freedesktop.org
> Subject: Re: [PATCH i-g-t 2/2] tests/xe/xe_spin_batch: Add new test to exercise
> igt_spin_new for xe.
> 
> On Thu, May 25, 2023 at 11:25:19AM +0530, sai.gowtham.ch@intel.com wrote:
> > From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> >
> > xe_spin_batch test exercises basic igt_spin_new submissions and and
> > with all engines.
> >
> > Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> > ---
> >  tests/meson.build        |   1 +
> >  tests/xe/xe_spin_batch.c | 138
> > +++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 139 insertions(+)
> >  create mode 100644 tests/xe/xe_spin_batch.c
> >
> > diff --git a/tests/meson.build b/tests/meson.build index
> > f71be1db..e794b75a 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -268,6 +268,7 @@ xe_progs = [
> >  	'xe_query',
> >  	'xe_vm',
> >  	'xe_waitfence',
> > +	'xe_spin_batch',
> >  ]
> >
> >  msm_progs = [
> > diff --git a/tests/xe/xe_spin_batch.c b/tests/xe/xe_spin_batch.c new
> > file mode 100644 index 00000000..1b216199
> > --- /dev/null
> > +++ b/tests/xe/xe_spin_batch.c
> > @@ -0,0 +1,138 @@
> > +#include "igt.h"
> > +#include "lib/intel_reg.h"
> > +#include "xe_drm.h"
> > +#include "xe/xe_ioctl.h"
> > +#include "xe/xe_query.h"
> > +
> > +#define MAX_INSTANCE 9
> > +/**
> > + * TEST:Test for spin batch submissons.
> > + *
> > + * SUBTEST: spin-batch
> > + * Description: Test to submit spin batch with engines and vm.
> > + * Run type: FULL
> > + * TODO: change ``'Run type' == FULL`` to a better category
> > + *
> > + */
> > +
> > +static void spin(int fd, struct drm_xe_engine_class_instance *hwe) {
> > +	uint64_t ahnd;
> > +	unsigned int engine;
> > +	uint32_t vm;
> > +	igt_spin_t *spin;
> > +
> > +	vm = xe_vm_create(fd, 0, 0);
> > +	engine = xe_engine_create(fd, vm, hwe, 0);
> > +	ahnd = intel_allocator_open(fd, 0, INTEL_ALLOCATOR_RELOC);
> > +
> > +	spin = __igt_spin_new(fd, .ahnd = ahnd, .engine = engine, .vm = vm);
> > +	igt_assert(spin);
> > +
> > +	igt_spin_free(fd, spin);
> 
> You shouldn't imo destroy engine/vm inside igt_spin_free(). Caller is the owner
> and they should stay intact.
> 
> If instead .engine you'll pass .hwe you may create engine on top of vm (if
> passed, if not create new one) and destroy in igt_spin_free().
> 
> Resume: track the ownershipping.
I'm handling this in igt_spin_free itself, if you look at the igt_spin_free code that I've sent for review I'm checking if it's passed from the igt test or 
Is it created inside the xe_spin_create, I'm not destroying engine and vm when it's passed from igt test.
> 
> > +	put_ahnd(ahnd);
> > +}
> > +
> > +/**
> > + * TEST: Basic test for spin batch submission.
> > + *
> > + * SUBTEST: spin-basic
> > + * Description: Basic test which validates the functionality of spinner.
> > + * Run type: FULL
> > + * TODO: change ``'Run type' == FULL`` to a better category
> > + *
> > + */
> > +static void spin_basic(int fd, struct drm_xe_engine_class_instance
> > +*hwe) {
> > +	uint64_t ahnd;
> > +	igt_spin_t *spin;
> > +
> > +	ahnd = intel_allocator_open(fd, 0, INTEL_ALLOCATOR_RELOC);
> > +	spin = __igt_spin_new(fd, .ahnd = ahnd, .hwe = hwe);
> > +	igt_assert(spin);
> > +
> > +	igt_spin_free(fd, spin);
> > +	put_ahnd(ahnd);
> > +}
> > +
> > +/**
> > + * TEST: Test for spin batch submissions.
> > + * SUBTEST: spin-all
> > + * Description: Spinner test to run on all the engines!
> > + * Run type: FULL
> > + * TODO: change ``'Run type' == FULL`` to a better category
> > + *
> > + */
> > +
> > +static void spin_all (int fd, int gt, int class) {
> > +	uint64_t ahnd;
> > +	uint32_t engines[MAX_INSTANCE];
> > +	uint32_t vm[MAX_INSTANCE];
> > +	int i, num_placements = 0;
> > +	struct drm_xe_engine_class_instance eci[MAX_INSTANCE];
> > +	igt_spin_t *spin[MAX_INSTANCE];
> > +	struct drm_xe_engine_class_instance *hwe;
> > +
> > +	ahnd = intel_allocator_open(fd, 0, INTEL_ALLOCATOR_RELOC);
> > +
> > +	xe_for_each_hw_engine(fd, hwe) {
> > +		if (hwe->engine_class != class || hwe->gt_id != gt)
> > +			continue;
> > +		eci[num_placements++] = *hwe;
> > +	}
> > +	if (num_placements < 2)
> > +		return;
> > +
> > +	for (i = 0; i < num_placements; i++) {
> > +		struct drm_xe_engine_create create;
> > +		vm[i] = xe_vm_create(fd, 0, 0);
> > +
> > +		create.vm_id = vm[i];
> > +		create.width = 1;
> > +		create.num_placements = num_placements;
> > +		create.instances = to_user_pointer(eci);
> > +
> > +		igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_ENGINE_CREATE,
> > +					&create), 0);
> > +		engines[i] = create.engine_id;
> > +		spin[i] = __igt_spin_new(fd, .ahnd = ahnd, .engine = engines[i],
> .vm = vm[i]);
> > +	}
> > +
> > +	for (i = 0; i < num_placements; i++) {
> > +		igt_assert(spin[i]);
> > +		igt_spin_free(fd, spin[i]);
> > +	}
> > +	put_ahnd(ahnd);
> > +}
> > +
> > +igt_main
> > +{
> > +	struct drm_xe_engine_class_instance *hwe;
> > +	int fd;
> > +	int gt, class;
> > +
> > +	igt_fixture {
> > +		fd = drm_open_driver(DRIVER_XE);
> > +		xe_device_get(fd);
> > +	}
> > +
> > +	igt_subtest("spin-batch")
> > +		xe_for_each_hw_engine(fd, hwe)
> > +			spin(fd, hwe);
> 
> Test both variants: with engine/vm created in the test and passed via .engine +
> .vm field and with passing .hwe only. Add also support for ALL_ENGINES flag.
> 
I think spin-batch already covers that, it passes both engine and vm to igt_spin_new which are created in the test.
And spin-basic passes only .hwe to igt_spin_new.

Thanks,
Gowtham 
> --
> Zbigniew
> 
> > +
> > +	igt_subtest("spin-basic")
> > +		xe_for_each_hw_engine(fd, hwe)
> > +			spin_basic(fd, hwe);
> > +
> > +	igt_subtest("spin-all") {
> > +		xe_for_each_gt(fd, gt)
> > +			xe_for_each_hw_engine_class(class)
> > +				spin_all(fd, gt, class);
> > +	}
> > +
> > +	igt_fixture {
> > +		xe_device_put(fd);
> > +		close(fd);
> > +	}
> > +}
> > --
> > 2.39.1
> >

Re: [igt-dev] [PATCH i-g-t 1/2] lib/xe/xe_spin: Integrate igt_spin_new with Xe.

[Ch, Sai Gowtham]

> -----Original Message-----
> From: Kempczynski, Zbigniew <zbigniew.kempczynski@intel.com>
> Sent: Monday, May 29, 2023 11:21 AM
> To: Ch, Sai Gowtham <sai.gowtham.ch@intel.com>
> Cc: igt-dev@lists.freedesktop.org
> Subject: Re: [PATCH i-g-t 1/2] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
> 
> On Thu, May 25, 2023 at 11:25:18AM +0530, sai.gowtham.ch@intel.com wrote:
> > From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> >
> > Extending the spin_create implementation and allocator handle support
> > in xe, where it submits dummy work loads to engine. This
> > Implementation is wrapped around vm_bind and unbind as we are supposed to
> do it manually for xe.
> >
> > Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> > ---
> >  lib/igt_dummyload.c | 24 +++++++++---  lib/igt_dummyload.h | 10 +++++
> >  lib/xe/xe_spin.c    | 89 +++++++++++++++++++++++++++++++++++++++++++++
> >  lib/xe/xe_spin.h    |  7 ++++
> >  4 files changed, 124 insertions(+), 6 deletions(-)
> >
> > diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c index
> > 740a58f3..6e89b72d 100644
> > --- a/lib/igt_dummyload.c
> > +++ b/lib/igt_dummyload.c
> > @@ -46,6 +46,7 @@
> >  #include "intel_reg.h"
> >  #include "ioctl_wrappers.h"
> >  #include "sw_sync.h"
> > +#include "xe/xe_spin.h"
> >
> >  /**
> >   * SECTION:igt_dummyload
> > @@ -447,7 +448,10 @@ spin_create(int fd, const struct igt_spin_factory
> > *opts)  igt_spin_t *  __igt_spin_factory(int fd, const struct
> > igt_spin_factory *opts)  {
> > -	return spin_create(fd, opts);
> > +	if (is_xe_device(fd))
> > +		return xe_spin_create(fd, opts);
> > +	else
> > +		return spin_create(fd, opts);
> >  }
> >
> >  /**
> > @@ -467,6 +471,11 @@ igt_spin_factory(int fd, const struct
> > igt_spin_factory *opts)  {
> >  	igt_spin_t *spin;
> >
> > +	if (is_xe_device(fd)) {
> > +		spin = xe_spin_create(fd, opts);
> > +		return spin;
> > +	}
> > +
> >  	if ((opts->flags & IGT_SPIN_POLL_RUN) && opts->engine !=
> ALL_ENGINES) {
> >  		unsigned int class;
> >
> > @@ -647,11 +656,14 @@ void igt_spin_free(int fd, igt_spin_t *spin)
> >  	if (!spin)
> >  		return;
> >
> > -	pthread_mutex_lock(&list_lock);
> > -	igt_list_del(&spin->link);
> > -	pthread_mutex_unlock(&list_lock);
> > -
> > -	__igt_spin_free(fd, spin);
> > +	if (is_xe_device(fd)) {
> > +		xe_spin_free(fd, spin);
> > +	} else {
> > +		pthread_mutex_lock(&list_lock);
> > +		igt_list_del(&spin->link);
> > +		pthread_mutex_unlock(&list_lock);
> > +		__igt_spin_free(fd, spin);
> > +	}
> >  }
> >
> >  void igt_terminate_spins(void)
> > diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h index
> > b247ab02..7bcc7b56 100644
> > --- a/lib/igt_dummyload.h
> > +++ b/lib/igt_dummyload.h
> > @@ -54,6 +54,8 @@ typedef struct igt_spin_factory {
> >  	unsigned int flags;
> >  	int fence;
> >  	uint64_t ahnd;
> > +	struct drm_xe_engine_class_instance *hwe;
> > +	uint32_t vm;
> >  } igt_spin_factory_t;
> >
> >  typedef struct igt_spin {
> > @@ -83,6 +85,14 @@ typedef struct igt_spin {  #define SPIN_CLFLUSH (1
> > << 0)
> >
> >  	struct igt_spin_factory opts;
> > +
> > +	struct xe_spin *xe_spin;
> > +	size_t bo_size;
> > +	uint64_t address;
> > +	unsigned int engine;
> > +	uint32_t vm;
> > +	uint32_t syncobj;
> > +
> >  } igt_spin_t;
> >
> >
> > diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c index
> > 856d0ba2..3a8c7bb3 100644
> > --- a/lib/xe/xe_spin.c
> > +++ b/lib/xe/xe_spin.c
> > @@ -15,6 +15,7 @@
> >  #include "intel_reg.h"
> >  #include "xe_ioctl.h"
> >  #include "xe_spin.h"
> > +#include "lib/igt_dummyload.h"
> >
> >  /**
> >   * xe_spin_init:
> > @@ -82,6 +83,94 @@ void xe_spin_end(struct xe_spin *spin)
> >  	spin->end = 0;
> >  }
> >
> > +igt_spin_t *
> > +xe_spin_create(int fd, const struct igt_spin_factory *opt) {
> > +	size_t bo_size = xe_get_default_alignment(fd);
> > +	uint32_t bo;
> > +	uint64_t ahnd = opt->ahnd, addr;
> > +	struct igt_spin *spin;
> > +	struct xe_spin *xe_spin;
> > +	struct drm_xe_sync sync = {
> > +		.flags = DRM_XE_SYNC_SYNCOBJ | DRM_XE_SYNC_SIGNAL,
> > +	};
> > +	struct drm_xe_exec exec = {
> > +		.num_batch_buffer = 1,
> > +		.num_syncs = 1,
> > +		.syncs = to_user_pointer(&sync),
> > +	};
> > +
> > +	igt_assert(ahnd);
> > +	spin = calloc(1, sizeof(struct igt_spin));
> > +	igt_assert(spin);
> > +
> > +	spin->syncobj = syncobj_create(fd, 0);
> > +	if (opt->engine) {
> > +		spin->opts.engine = opt->engine;
> > +		spin->opts.vm = opt->vm;
> > +
> > +		spin->handle = xe_bo_create(fd, 0, spin->opts.vm, bo_size);
> > +		xe_spin = xe_bo_map(fd, spin->handle, bo_size);
> > +		addr = intel_allocator_alloc_with_strategy(ahnd, spin->handle,
> bo_size, 0, ALLOC_STRATEGY_LOW_TO_HIGH);
> > +		xe_vm_bind_sync(fd, spin->opts.vm, spin->handle, 0, addr,
> bo_size);
> > +
> > +		xe_spin_init(xe_spin, addr, true);
> > +		exec.engine_id = spin->opts.engine;
> > +		exec.address = addr;
> > +	} else {
> > +		spin->vm = xe_vm_create(fd, 0, 0);
> > +		spin->engine = xe_engine_create(fd, spin->vm, opt->hwe, 0);
> > +
> > +		bo = xe_bo_create(fd, 0, spin->vm, bo_size);
> > +		spin->handle = bo;
> > +		xe_spin = xe_bo_map(fd, spin->handle, bo_size);
> > +		addr = intel_allocator_alloc_with_strategy(ahnd, spin->handle,
> bo_size, 0, ALLOC_STRATEGY_LOW_TO_HIGH);
> > +		xe_vm_bind_sync(fd, spin->vm, spin->handle, 0, addr, bo_size);
> > +
> > +		xe_spin_init(xe_spin, addr, true);
> > +		exec.engine_id = spin->engine;
> > +		exec.address = addr;
> > +	}
> 
> I think you should support two variants of executing spinners:
> 
> 1. hwe != NULL (in this case engine must be 0):
>    if (!vm) -> create vm;
>    create engine for vm according to hwe
> 
> 2. engine != 0, vm must be vm on top of which engine was created
>    hwe must be NULL in this case
>    see Dominik answer about passing engine+vm in:
>    https://patchwork.freedesktop.org/patch/539173/?series=118227&rev=2
> 
> And add support ALL_ENGINES flag (then iterate over all hw engines like Bhanu
> suggested). And track engine ownership - if caller is the engine/vm owner use it
> but don't destroy, if spinner code is creating engine you may freely destroy it in
> spinner free path().
> 
Do we have Driver support for ALL_ENGINES flag ?? 

I think what Bhanu asking for is as kms tests  doesn’t need hwe, We have to pass hwe from the xe_spin_create it self. 
In that case we can add a support in xe_spin_create where hwe = 0 and engine = 0 ( Note  this is only for kms tests) .

I feel ALL_ENGINES support can be added later, Once kms tests are unblocked.

Regarding igt_spin_free, I've already added support not to destroy vm and engines if they are passed from the igt test. Can you have a look at igt_spin_free (Verified it's working fine for me).
-----
Gowtham
> --
> Zbigniew
> 
> 
> > +	sync.handle = spin->syncobj;
> > +	igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_EXEC, &exec), 0);
> > +	xe_spin_wait_started(xe_spin);
> > +	igt_info("Spinner started\n");
> > +
> > +	spin->bo_size = bo_size;
> > +	spin->address = addr;
> > +	spin->xe_spin = xe_spin;
> > +
> > +	return spin;
> > +}
> > +
> > +void xe_spin_sync_wait(int fd, struct igt_spin *spin) {
> > +	igt_assert(syncobj_wait(fd, &spin->syncobj, 1, INT64_MAX, 0,
> > +				NULL));
> > +}
> > +
> > +void xe_spin_free(int fd, struct igt_spin *spin) {
> > +	xe_spin_end(spin->xe_spin);
> > +	xe_spin_sync_wait(fd, spin);
> > +
> > +	if (!spin->opts.engine) {
> > +		xe_vm_unbind_sync(fd, spin->vm, 0, spin->address, spin-
> >bo_size);
> > +	} else {
> > +		xe_vm_unbind_sync(fd, spin->opts.vm, 0, spin->address, spin-
> >bo_size);
> > +	}
> > +
> > +	syncobj_destroy(fd, spin->syncobj);
> > +	gem_munmap(spin->xe_spin, spin->bo_size);
> > +	gem_close(fd, spin->handle);
> > +
> > +	if (!spin->opts.engine) {
> > +		xe_engine_destroy(fd, spin->engine);
> > +		xe_vm_destroy(fd, spin->vm);
> > +	}
> > +	free(spin);
> > +}
> > +
> >  void xe_cork_init(int fd, struct drm_xe_engine_class_instance *hwe,
> >  		  struct xe_cork *cork)
> >  {
> > diff --git a/lib/xe/xe_spin.h b/lib/xe/xe_spin.h index
> > 73f9a026..48867eb8 100644
> > --- a/lib/xe/xe_spin.h
> > +++ b/lib/xe/xe_spin.h
> > @@ -13,19 +13,26 @@
> >  #include <stdbool.h>
> >
> >  #include "xe_query.h"
> > +#include "lib/igt_dummyload.h"
> >
> >  /* Mapped GPU object */
> > +
> >  struct xe_spin {
> >  	uint32_t batch[16];
> >  	uint64_t pad;
> >  	uint32_t start;
> >  	uint32_t end;
> > +
> >  };
> >
> > +igt_spin_t *
> > +xe_spin_create(int fd, const struct igt_spin_factory *opt);
> >  void xe_spin_init(struct xe_spin *spin, uint64_t addr, bool preempt);
> > bool xe_spin_started(struct xe_spin *spin);
> > +void xe_spin_sync_wait(int fd, struct igt_spin *spin);
> >  void xe_spin_wait_started(struct xe_spin *spin);  void
> > xe_spin_end(struct xe_spin *spin);
> > +void xe_spin_free(int fd, struct igt_spin *spin);
> >
> >  struct xe_cork {
> >  	struct xe_spin *spin;
> > --
> > 2.39.1
> >

Re: [igt-dev] [PATCH i-g-t 1/2] lib/xe/xe_spin: Integrate igt_spin_new with Xe.

[Modem, Bhanuprakash]

Hi Sai,

On Tue-30-05-2023 10:09 am, Ch, Sai Gowtham wrote:
> 
> 
>> -----Original Message-----
>> From: Kempczynski, Zbigniew <zbigniew.kempczynski@intel.com>
>> Sent: Monday, May 29, 2023 11:21 AM
>> To: Ch, Sai Gowtham <sai.gowtham.ch@intel.com>
>> Cc: igt-dev@lists.freedesktop.org
>> Subject: Re: [PATCH i-g-t 1/2] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
>>
>> On Thu, May 25, 2023 at 11:25:18AM +0530, sai.gowtham.ch@intel.com wrote:
>>> From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
>>>
>>> Extending the spin_create implementation and allocator handle support
>>> in xe, where it submits dummy work loads to engine. This
>>> Implementation is wrapped around vm_bind and unbind as we are supposed to
>> do it manually for xe.
>>>
>>> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
>>> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
>>> ---
>>>   lib/igt_dummyload.c | 24 +++++++++---  lib/igt_dummyload.h | 10 +++++
>>>   lib/xe/xe_spin.c    | 89 +++++++++++++++++++++++++++++++++++++++++++++
>>>   lib/xe/xe_spin.h    |  7 ++++
>>>   4 files changed, 124 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c index
>>> 740a58f3..6e89b72d 100644
>>> --- a/lib/igt_dummyload.c
>>> +++ b/lib/igt_dummyload.c
>>> @@ -46,6 +46,7 @@
>>>   #include "intel_reg.h"
>>>   #include "ioctl_wrappers.h"
>>>   #include "sw_sync.h"
>>> +#include "xe/xe_spin.h"
>>>
>>>   /**
>>>    * SECTION:igt_dummyload
>>> @@ -447,7 +448,10 @@ spin_create(int fd, const struct igt_spin_factory
>>> *opts)  igt_spin_t *  __igt_spin_factory(int fd, const struct
>>> igt_spin_factory *opts)  {
>>> -	return spin_create(fd, opts);
>>> +	if (is_xe_device(fd))
>>> +		return xe_spin_create(fd, opts);
>>> +	else
>>> +		return spin_create(fd, opts);
>>>   }
>>>
>>>   /**
>>> @@ -467,6 +471,11 @@ igt_spin_factory(int fd, const struct
>>> igt_spin_factory *opts)  {
>>>   	igt_spin_t *spin;
>>>
>>> +	if (is_xe_device(fd)) {
>>> +		spin = xe_spin_create(fd, opts);
>>> +		return spin;
>>> +	}
>>> +
>>>   	if ((opts->flags & IGT_SPIN_POLL_RUN) && opts->engine !=
>> ALL_ENGINES) {
>>>   		unsigned int class;
>>>
>>> @@ -647,11 +656,14 @@ void igt_spin_free(int fd, igt_spin_t *spin)
>>>   	if (!spin)
>>>   		return;
>>>
>>> -	pthread_mutex_lock(&list_lock);
>>> -	igt_list_del(&spin->link);
>>> -	pthread_mutex_unlock(&list_lock);
>>> -
>>> -	__igt_spin_free(fd, spin);
>>> +	if (is_xe_device(fd)) {
>>> +		xe_spin_free(fd, spin);
>>> +	} else {
>>> +		pthread_mutex_lock(&list_lock);
>>> +		igt_list_del(&spin->link);
>>> +		pthread_mutex_unlock(&list_lock);
>>> +		__igt_spin_free(fd, spin);
>>> +	}
>>>   }
>>>
>>>   void igt_terminate_spins(void)
>>> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h index
>>> b247ab02..7bcc7b56 100644
>>> --- a/lib/igt_dummyload.h
>>> +++ b/lib/igt_dummyload.h
>>> @@ -54,6 +54,8 @@ typedef struct igt_spin_factory {
>>>   	unsigned int flags;
>>>   	int fence;
>>>   	uint64_t ahnd;
>>> +	struct drm_xe_engine_class_instance *hwe;
>>> +	uint32_t vm;
>>>   } igt_spin_factory_t;
>>>
>>>   typedef struct igt_spin {
>>> @@ -83,6 +85,14 @@ typedef struct igt_spin {  #define SPIN_CLFLUSH (1
>>> << 0)
>>>
>>>   	struct igt_spin_factory opts;
>>> +
>>> +	struct xe_spin *xe_spin;
>>> +	size_t bo_size;
>>> +	uint64_t address;
>>> +	unsigned int engine;
>>> +	uint32_t vm;
>>> +	uint32_t syncobj;
>>> +
>>>   } igt_spin_t;
>>>
>>>
>>> diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c index
>>> 856d0ba2..3a8c7bb3 100644
>>> --- a/lib/xe/xe_spin.c
>>> +++ b/lib/xe/xe_spin.c
>>> @@ -15,6 +15,7 @@
>>>   #include "intel_reg.h"
>>>   #include "xe_ioctl.h"
>>>   #include "xe_spin.h"
>>> +#include "lib/igt_dummyload.h"
>>>
>>>   /**
>>>    * xe_spin_init:
>>> @@ -82,6 +83,94 @@ void xe_spin_end(struct xe_spin *spin)
>>>   	spin->end = 0;
>>>   }
>>>
>>> +igt_spin_t *
>>> +xe_spin_create(int fd, const struct igt_spin_factory *opt) {
>>> +	size_t bo_size = xe_get_default_alignment(fd);
>>> +	uint32_t bo;
>>> +	uint64_t ahnd = opt->ahnd, addr;
>>> +	struct igt_spin *spin;
>>> +	struct xe_spin *xe_spin;
>>> +	struct drm_xe_sync sync = {
>>> +		.flags = DRM_XE_SYNC_SYNCOBJ | DRM_XE_SYNC_SIGNAL,
>>> +	};
>>> +	struct drm_xe_exec exec = {
>>> +		.num_batch_buffer = 1,
>>> +		.num_syncs = 1,
>>> +		.syncs = to_user_pointer(&sync),
>>> +	};
>>> +
>>> +	igt_assert(ahnd);
>>> +	spin = calloc(1, sizeof(struct igt_spin));
>>> +	igt_assert(spin);
>>> +
>>> +	spin->syncobj = syncobj_create(fd, 0);
>>> +	if (opt->engine) {
>>> +		spin->opts.engine = opt->engine;
>>> +		spin->opts.vm = opt->vm;
>>> +
>>> +		spin->handle = xe_bo_create(fd, 0, spin->opts.vm, bo_size);
>>> +		xe_spin = xe_bo_map(fd, spin->handle, bo_size);
>>> +		addr = intel_allocator_alloc_with_strategy(ahnd, spin->handle,
>> bo_size, 0, ALLOC_STRATEGY_LOW_TO_HIGH);
>>> +		xe_vm_bind_sync(fd, spin->opts.vm, spin->handle, 0, addr,
>> bo_size);
>>> +
>>> +		xe_spin_init(xe_spin, addr, true);
>>> +		exec.engine_id = spin->opts.engine;
>>> +		exec.address = addr;
>>> +	} else {
>>> +		spin->vm = xe_vm_create(fd, 0, 0);
>>> +		spin->engine = xe_engine_create(fd, spin->vm, opt->hwe, 0);
>>> +
>>> +		bo = xe_bo_create(fd, 0, spin->vm, bo_size);
>>> +		spin->handle = bo;
>>> +		xe_spin = xe_bo_map(fd, spin->handle, bo_size);
>>> +		addr = intel_allocator_alloc_with_strategy(ahnd, spin->handle,
>> bo_size, 0, ALLOC_STRATEGY_LOW_TO_HIGH);
>>> +		xe_vm_bind_sync(fd, spin->vm, spin->handle, 0, addr, bo_size);
>>> +
>>> +		xe_spin_init(xe_spin, addr, true);
>>> +		exec.engine_id = spin->engine;
>>> +		exec.address = addr;
>>> +	}
>>
>> I think you should support two variants of executing spinners:
>>
>> 1. hwe != NULL (in this case engine must be 0):
>>     if (!vm) -> create vm;
>>     create engine for vm according to hwe
>>
>> 2. engine != 0, vm must be vm on top of which engine was created
>>     hwe must be NULL in this case
>>     see Dominik answer about passing engine+vm in:
>>     https://patchwork.freedesktop.org/patch/539173/?series=118227&rev=2
>>
>> And add support ALL_ENGINES flag (then iterate over all hw engines like Bhanu
>> suggested). And track engine ownership - if caller is the engine/vm owner use it
>> but don't destroy, if spinner code is creating engine you may freely destroy it in
>> spinner free path().
>>
> Do we have Driver support for ALL_ENGINES flag ??
> 
> I think what Bhanu asking for is as kms tests  doesn’t need hwe, We have to pass hwe from the xe_spin_create it self.
> In that case we can add a support in xe_spin_create where hwe = 0 and engine = 0 ( Note  this is only for kms tests) .

Yes, KMS tests are not supposed to pass hwe or engines.

- Bhanu

> 
> I feel ALL_ENGINES support can be added later, Once kms tests are unblocked.
> 
> Regarding igt_spin_free, I've already added support not to destroy vm and engines if they are passed from the igt test. Can you have a look at igt_spin_free (Verified it's working fine for me).
> -----
> Gowtham
>> --
>> Zbigniew
>>
>>
>>> +	sync.handle = spin->syncobj;
>>> +	igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_EXEC, &exec), 0);
>>> +	xe_spin_wait_started(xe_spin);
>>> +	igt_info("Spinner started\n");
>>> +
>>> +	spin->bo_size = bo_size;
>>> +	spin->address = addr;
>>> +	spin->xe_spin = xe_spin;
>>> +
>>> +	return spin;
>>> +}
>>> +
>>> +void xe_spin_sync_wait(int fd, struct igt_spin *spin) {
>>> +	igt_assert(syncobj_wait(fd, &spin->syncobj, 1, INT64_MAX, 0,
>>> +				NULL));
>>> +}
>>> +
>>> +void xe_spin_free(int fd, struct igt_spin *spin) {
>>> +	xe_spin_end(spin->xe_spin);
>>> +	xe_spin_sync_wait(fd, spin);
>>> +
>>> +	if (!spin->opts.engine) {
>>> +		xe_vm_unbind_sync(fd, spin->vm, 0, spin->address, spin-
>>> bo_size);
>>> +	} else {
>>> +		xe_vm_unbind_sync(fd, spin->opts.vm, 0, spin->address, spin-
>>> bo_size);
>>> +	}
>>> +
>>> +	syncobj_destroy(fd, spin->syncobj);
>>> +	gem_munmap(spin->xe_spin, spin->bo_size);
>>> +	gem_close(fd, spin->handle);
>>> +
>>> +	if (!spin->opts.engine) {
>>> +		xe_engine_destroy(fd, spin->engine);
>>> +		xe_vm_destroy(fd, spin->vm);
>>> +	}
>>> +	free(spin);
>>> +}
>>> +
>>>   void xe_cork_init(int fd, struct drm_xe_engine_class_instance *hwe,
>>>   		  struct xe_cork *cork)
>>>   {
>>> diff --git a/lib/xe/xe_spin.h b/lib/xe/xe_spin.h index
>>> 73f9a026..48867eb8 100644
>>> --- a/lib/xe/xe_spin.h
>>> +++ b/lib/xe/xe_spin.h
>>> @@ -13,19 +13,26 @@
>>>   #include <stdbool.h>
>>>
>>>   #include "xe_query.h"
>>> +#include "lib/igt_dummyload.h"
>>>
>>>   /* Mapped GPU object */
>>> +
>>>   struct xe_spin {
>>>   	uint32_t batch[16];
>>>   	uint64_t pad;
>>>   	uint32_t start;
>>>   	uint32_t end;
>>> +
>>>   };
>>>
>>> +igt_spin_t *
>>> +xe_spin_create(int fd, const struct igt_spin_factory *opt);
>>>   void xe_spin_init(struct xe_spin *spin, uint64_t addr, bool preempt);
>>> bool xe_spin_started(struct xe_spin *spin);
>>> +void xe_spin_sync_wait(int fd, struct igt_spin *spin);
>>>   void xe_spin_wait_started(struct xe_spin *spin);  void
>>> xe_spin_end(struct xe_spin *spin);
>>> +void xe_spin_free(int fd, struct igt_spin *spin);
>>>
>>>   struct xe_cork {
>>>   	struct xe_spin *spin;
>>> --
>>> 2.39.1
>>>

Re: [igt-dev] [PATCH i-g-t 1/2] lib/xe/xe_spin: Integrate igt_spin_new with Xe.

[Ch, Sai Gowtham]

> -----Original Message-----
> From: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>
> Sent: Tuesday, May 30, 2023 10:14 AM
> To: Ch, Sai Gowtham <sai.gowtham.ch@intel.com>; Kempczynski, Zbigniew
> <zbigniew.kempczynski@intel.com>; Gandi, Ramadevi
> <ramadevi.gandi@intel.com>
> Cc: igt-dev@lists.freedesktop.org
> Subject: Re: [PATCH i-g-t 1/2] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
> 
> Hi Sai,
> 
> On Tue-30-05-2023 10:09 am, Ch, Sai Gowtham wrote:
> >
> >
> >> -----Original Message-----
> >> From: Kempczynski, Zbigniew <zbigniew.kempczynski@intel.com>
> >> Sent: Monday, May 29, 2023 11:21 AM
> >> To: Ch, Sai Gowtham <sai.gowtham.ch@intel.com>
> >> Cc: igt-dev@lists.freedesktop.org
> >> Subject: Re: [PATCH i-g-t 1/2] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
> >>
> >> On Thu, May 25, 2023 at 11:25:18AM +0530, sai.gowtham.ch@intel.com
> wrote:
> >>> From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> >>>
> >>> Extending the spin_create implementation and allocator handle
> >>> support in xe, where it submits dummy work loads to engine. This
> >>> Implementation is wrapped around vm_bind and unbind as we are
> >>> supposed to
> >> do it manually for xe.
> >>>
> >>> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> >>> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> >>> ---
> >>>   lib/igt_dummyload.c | 24 +++++++++---  lib/igt_dummyload.h | 10 +++++
> >>>   lib/xe/xe_spin.c    | 89
> +++++++++++++++++++++++++++++++++++++++++++++
> >>>   lib/xe/xe_spin.h    |  7 ++++
> >>>   4 files changed, 124 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c index
> >>> 740a58f3..6e89b72d 100644
> >>> --- a/lib/igt_dummyload.c
> >>> +++ b/lib/igt_dummyload.c
> >>> @@ -46,6 +46,7 @@
> >>>   #include "intel_reg.h"
> >>>   #include "ioctl_wrappers.h"
> >>>   #include "sw_sync.h"
> >>> +#include "xe/xe_spin.h"
> >>>
> >>>   /**
> >>>    * SECTION:igt_dummyload
> >>> @@ -447,7 +448,10 @@ spin_create(int fd, const struct
> >>> igt_spin_factory
> >>> *opts)  igt_spin_t *  __igt_spin_factory(int fd, const struct
> >>> igt_spin_factory *opts)  {
> >>> -	return spin_create(fd, opts);
> >>> +	if (is_xe_device(fd))
> >>> +		return xe_spin_create(fd, opts);
> >>> +	else
> >>> +		return spin_create(fd, opts);
> >>>   }
> >>>
> >>>   /**
> >>> @@ -467,6 +471,11 @@ igt_spin_factory(int fd, const struct
> >>> igt_spin_factory *opts)  {
> >>>   	igt_spin_t *spin;
> >>>
> >>> +	if (is_xe_device(fd)) {
> >>> +		spin = xe_spin_create(fd, opts);
> >>> +		return spin;
> >>> +	}
> >>> +
> >>>   	if ((opts->flags & IGT_SPIN_POLL_RUN) && opts->engine !=
> >> ALL_ENGINES) {
> >>>   		unsigned int class;
> >>>
> >>> @@ -647,11 +656,14 @@ void igt_spin_free(int fd, igt_spin_t *spin)
> >>>   	if (!spin)
> >>>   		return;
> >>>
> >>> -	pthread_mutex_lock(&list_lock);
> >>> -	igt_list_del(&spin->link);
> >>> -	pthread_mutex_unlock(&list_lock);
> >>> -
> >>> -	__igt_spin_free(fd, spin);
> >>> +	if (is_xe_device(fd)) {
> >>> +		xe_spin_free(fd, spin);
> >>> +	} else {
> >>> +		pthread_mutex_lock(&list_lock);
> >>> +		igt_list_del(&spin->link);
> >>> +		pthread_mutex_unlock(&list_lock);
> >>> +		__igt_spin_free(fd, spin);
> >>> +	}
> >>>   }
> >>>
> >>>   void igt_terminate_spins(void)
> >>> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h index
> >>> b247ab02..7bcc7b56 100644
> >>> --- a/lib/igt_dummyload.h
> >>> +++ b/lib/igt_dummyload.h
> >>> @@ -54,6 +54,8 @@ typedef struct igt_spin_factory {
> >>>   	unsigned int flags;
> >>>   	int fence;
> >>>   	uint64_t ahnd;
> >>> +	struct drm_xe_engine_class_instance *hwe;
> >>> +	uint32_t vm;
> >>>   } igt_spin_factory_t;
> >>>
> >>>   typedef struct igt_spin {
> >>> @@ -83,6 +85,14 @@ typedef struct igt_spin {  #define SPIN_CLFLUSH
> >>> (1 << 0)
> >>>
> >>>   	struct igt_spin_factory opts;
> >>> +
> >>> +	struct xe_spin *xe_spin;
> >>> +	size_t bo_size;
> >>> +	uint64_t address;
> >>> +	unsigned int engine;
> >>> +	uint32_t vm;
> >>> +	uint32_t syncobj;
> >>> +
> >>>   } igt_spin_t;
> >>>
> >>>
> >>> diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c index
> >>> 856d0ba2..3a8c7bb3 100644
> >>> --- a/lib/xe/xe_spin.c
> >>> +++ b/lib/xe/xe_spin.c
> >>> @@ -15,6 +15,7 @@
> >>>   #include "intel_reg.h"
> >>>   #include "xe_ioctl.h"
> >>>   #include "xe_spin.h"
> >>> +#include "lib/igt_dummyload.h"
> >>>
> >>>   /**
> >>>    * xe_spin_init:
> >>> @@ -82,6 +83,94 @@ void xe_spin_end(struct xe_spin *spin)
> >>>   	spin->end = 0;
> >>>   }
> >>>
> >>> +igt_spin_t *
> >>> +xe_spin_create(int fd, const struct igt_spin_factory *opt) {
> >>> +	size_t bo_size = xe_get_default_alignment(fd);
> >>> +	uint32_t bo;
> >>> +	uint64_t ahnd = opt->ahnd, addr;
> >>> +	struct igt_spin *spin;
> >>> +	struct xe_spin *xe_spin;
> >>> +	struct drm_xe_sync sync = {
> >>> +		.flags = DRM_XE_SYNC_SYNCOBJ | DRM_XE_SYNC_SIGNAL,
> >>> +	};
> >>> +	struct drm_xe_exec exec = {
> >>> +		.num_batch_buffer = 1,
> >>> +		.num_syncs = 1,
> >>> +		.syncs = to_user_pointer(&sync),
> >>> +	};
> >>> +
> >>> +	igt_assert(ahnd);
> >>> +	spin = calloc(1, sizeof(struct igt_spin));
> >>> +	igt_assert(spin);
> >>> +
> >>> +	spin->syncobj = syncobj_create(fd, 0);
> >>> +	if (opt->engine) {
> >>> +		spin->opts.engine = opt->engine;
> >>> +		spin->opts.vm = opt->vm;
> >>> +
> >>> +		spin->handle = xe_bo_create(fd, 0, spin->opts.vm, bo_size);
> >>> +		xe_spin = xe_bo_map(fd, spin->handle, bo_size);
> >>> +		addr = intel_allocator_alloc_with_strategy(ahnd, spin->handle,
> >> bo_size, 0, ALLOC_STRATEGY_LOW_TO_HIGH);
> >>> +		xe_vm_bind_sync(fd, spin->opts.vm, spin->handle, 0, addr,
> >> bo_size);
> >>> +
> >>> +		xe_spin_init(xe_spin, addr, true);
> >>> +		exec.engine_id = spin->opts.engine;
> >>> +		exec.address = addr;
> >>> +	} else {
> >>> +		spin->vm = xe_vm_create(fd, 0, 0);
> >>> +		spin->engine = xe_engine_create(fd, spin->vm, opt->hwe, 0);
> >>> +
> >>> +		bo = xe_bo_create(fd, 0, spin->vm, bo_size);
> >>> +		spin->handle = bo;
> >>> +		xe_spin = xe_bo_map(fd, spin->handle, bo_size);
> >>> +		addr = intel_allocator_alloc_with_strategy(ahnd, spin->handle,
> >> bo_size, 0, ALLOC_STRATEGY_LOW_TO_HIGH);
> >>> +		xe_vm_bind_sync(fd, spin->vm, spin->handle, 0, addr, bo_size);
> >>> +
> >>> +		xe_spin_init(xe_spin, addr, true);
> >>> +		exec.engine_id = spin->engine;
> >>> +		exec.address = addr;
> >>> +	}
> >>
> >> I think you should support two variants of executing spinners:
> >>
> >> 1. hwe != NULL (in this case engine must be 0):
> >>     if (!vm) -> create vm;
> >>     create engine for vm according to hwe
> >>
> >> 2. engine != 0, vm must be vm on top of which engine was created
> >>     hwe must be NULL in this case
> >>     see Dominik answer about passing engine+vm in:
> >>
> >> https://patchwork.freedesktop.org/patch/539173/?series=118227&rev=2
> >>
> >> And add support ALL_ENGINES flag (then iterate over all hw engines
> >> like Bhanu suggested). And track engine ownership - if caller is the
> >> engine/vm owner use it but don't destroy, if spinner code is creating
> >> engine you may freely destroy it in spinner free path().
> >>
> > Do we have Driver support for ALL_ENGINES flag ??
> >
> > I think what Bhanu asking for is as kms tests  doesn’t need hwe, We have to
> pass hwe from the xe_spin_create it self.
> > In that case we can add a support in xe_spin_create where hwe = 0 and engine
> = 0 ( Note  this is only for kms tests) .


I just realised xe_spin_create with engine_id = 0 will not work, However is there a way to create engine without hwe ? 
- Gowtham

> 
> Yes, KMS tests are not supposed to pass hwe or engines.
> 
> - Bhanu
> 
> >
> > I feel ALL_ENGINES support can be added later, Once kms tests are unblocked.
> >
> > Regarding igt_spin_free, I've already added support not to destroy vm and
> engines if they are passed from the igt test. Can you have a look at igt_spin_free
> (Verified it's working fine for me).
> > -----
> > Gowtham
> >> --
> >> Zbigniew
> >>
> >>
> >>> +	sync.handle = spin->syncobj;
> >>> +	igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_EXEC, &exec), 0);
> >>> +	xe_spin_wait_started(xe_spin);
> >>> +	igt_info("Spinner started\n");
> >>> +
> >>> +	spin->bo_size = bo_size;
> >>> +	spin->address = addr;
> >>> +	spin->xe_spin = xe_spin;
> >>> +
> >>> +	return spin;
> >>> +}
> >>> +
> >>> +void xe_spin_sync_wait(int fd, struct igt_spin *spin) {
> >>> +	igt_assert(syncobj_wait(fd, &spin->syncobj, 1, INT64_MAX, 0,
> >>> +				NULL));
> >>> +}
> >>> +
> >>> +void xe_spin_free(int fd, struct igt_spin *spin) {
> >>> +	xe_spin_end(spin->xe_spin);
> >>> +	xe_spin_sync_wait(fd, spin);
> >>> +
> >>> +	if (!spin->opts.engine) {
> >>> +		xe_vm_unbind_sync(fd, spin->vm, 0, spin->address, spin-
> >>> bo_size);
> >>> +	} else {
> >>> +		xe_vm_unbind_sync(fd, spin->opts.vm, 0, spin->address, spin-
> >>> bo_size);
> >>> +	}
> >>> +
> >>> +	syncobj_destroy(fd, spin->syncobj);
> >>> +	gem_munmap(spin->xe_spin, spin->bo_size);
> >>> +	gem_close(fd, spin->handle);
> >>> +
> >>> +	if (!spin->opts.engine) {
> >>> +		xe_engine_destroy(fd, spin->engine);
> >>> +		xe_vm_destroy(fd, spin->vm);
> >>> +	}
> >>> +	free(spin);
> >>> +}
> >>> +
> >>>   void xe_cork_init(int fd, struct drm_xe_engine_class_instance *hwe,
> >>>   		  struct xe_cork *cork)
> >>>   {
> >>> diff --git a/lib/xe/xe_spin.h b/lib/xe/xe_spin.h index
> >>> 73f9a026..48867eb8 100644
> >>> --- a/lib/xe/xe_spin.h
> >>> +++ b/lib/xe/xe_spin.h
> >>> @@ -13,19 +13,26 @@
> >>>   #include <stdbool.h>
> >>>
> >>>   #include "xe_query.h"
> >>> +#include "lib/igt_dummyload.h"
> >>>
> >>>   /* Mapped GPU object */
> >>> +
> >>>   struct xe_spin {
> >>>   	uint32_t batch[16];
> >>>   	uint64_t pad;
> >>>   	uint32_t start;
> >>>   	uint32_t end;
> >>> +
> >>>   };
> >>>
> >>> +igt_spin_t *
> >>> +xe_spin_create(int fd, const struct igt_spin_factory *opt);
> >>>   void xe_spin_init(struct xe_spin *spin, uint64_t addr, bool
> >>> preempt); bool xe_spin_started(struct xe_spin *spin);
> >>> +void xe_spin_sync_wait(int fd, struct igt_spin *spin);
> >>>   void xe_spin_wait_started(struct xe_spin *spin);  void
> >>> xe_spin_end(struct xe_spin *spin);
> >>> +void xe_spin_free(int fd, struct igt_spin *spin);
> >>>
> >>>   struct xe_cork {
> >>>   	struct xe_spin *spin;
> >>> --
> >>> 2.39.1
> >>>

[igt-dev] [PATCH i-g-t 0/2] Integrate igt_spin_new with Xe.

[sai.gowtham.ch]

From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Extending the spin_create implementation and allocator handle support in xe,
where it submits dummy work loads to engine. This Implementation is wrapped
around vm_bind and unbind as we are supposed to do it manually for xe.

Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Sai Gowtham Ch (2):
  lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  tests/xe/xe_spin_batch: Add new test to exercise igt_spin_new for xe.

 lib/igt_dummyload.c      |  24 ++++--
 lib/igt_dummyload.h      |  10 +++
 lib/xe/xe_spin.c         |  91 +++++++++++++++++++++
 lib/xe/xe_spin.h         |   7 ++
 tests/meson.build        |   1 +
 tests/xe/xe_spin_batch.c | 168 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 295 insertions(+), 6 deletions(-)
 create mode 100644 tests/xe/xe_spin_batch.c

-- 
2.39.1

Re: [igt-dev] [PATCH i-g-t 2/2] tests/xe/xe_spin_batch: Add new test to exercise igt_spin_new for xe.

[Zbigniew Kempczyński]

On Mon, May 29, 2023 at 01:02:53PM +0200, Ch, Sai Gowtham wrote:
> 
> 
> > -----Original Message-----
> > From: Kempczynski, Zbigniew <zbigniew.kempczynski@intel.com>
> > Sent: Monday, May 29, 2023 11:27 AM
> > To: Ch, Sai Gowtham <sai.gowtham.ch@intel.com>
> > Cc: igt-dev@lists.freedesktop.org
> > Subject: Re: [PATCH i-g-t 2/2] tests/xe/xe_spin_batch: Add new test to exercise
> > igt_spin_new for xe.
> > 
> > On Thu, May 25, 2023 at 11:25:19AM +0530, sai.gowtham.ch@intel.com wrote:
> > > From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> > >
> > > xe_spin_batch test exercises basic igt_spin_new submissions and and
> > > with all engines.
> > >
> > > Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > > Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> > > ---
> > >  tests/meson.build        |   1 +
> > >  tests/xe/xe_spin_batch.c | 138
> > > +++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 139 insertions(+)
> > >  create mode 100644 tests/xe/xe_spin_batch.c
> > >
> > > diff --git a/tests/meson.build b/tests/meson.build index
> > > f71be1db..e794b75a 100644
> > > --- a/tests/meson.build
> > > +++ b/tests/meson.build
> > > @@ -268,6 +268,7 @@ xe_progs = [
> > >  	'xe_query',
> > >  	'xe_vm',
> > >  	'xe_waitfence',
> > > +	'xe_spin_batch',
> > >  ]
> > >
> > >  msm_progs = [
> > > diff --git a/tests/xe/xe_spin_batch.c b/tests/xe/xe_spin_batch.c new
> > > file mode 100644 index 00000000..1b216199
> > > --- /dev/null
> > > +++ b/tests/xe/xe_spin_batch.c
> > > @@ -0,0 +1,138 @@
> > > +#include "igt.h"
> > > +#include "lib/intel_reg.h"
> > > +#include "xe_drm.h"
> > > +#include "xe/xe_ioctl.h"
> > > +#include "xe/xe_query.h"
> > > +
> > > +#define MAX_INSTANCE 9
> > > +/**
> > > + * TEST:Test for spin batch submissons.
> > > + *
> > > + * SUBTEST: spin-batch
> > > + * Description: Test to submit spin batch with engines and vm.
> > > + * Run type: FULL
> > > + * TODO: change ``'Run type' == FULL`` to a better category
> > > + *
> > > + */
> > > +
> > > +static void spin(int fd, struct drm_xe_engine_class_instance *hwe) {
> > > +	uint64_t ahnd;
> > > +	unsigned int engine;
> > > +	uint32_t vm;
> > > +	igt_spin_t *spin;
> > > +
> > > +	vm = xe_vm_create(fd, 0, 0);
> > > +	engine = xe_engine_create(fd, vm, hwe, 0);
> > > +	ahnd = intel_allocator_open(fd, 0, INTEL_ALLOCATOR_RELOC);
> > > +
> > > +	spin = __igt_spin_new(fd, .ahnd = ahnd, .engine = engine, .vm = vm);
> > > +	igt_assert(spin);
> > > +
> > > +	igt_spin_free(fd, spin);
> > 
> > You shouldn't imo destroy engine/vm inside igt_spin_free(). Caller is the owner
> > and they should stay intact.
> > 
> > If instead .engine you'll pass .hwe you may create engine on top of vm (if
> > passed, if not create new one) and destroy in igt_spin_free().
> > 
> > Resume: track the ownershipping.
> I'm handling this in igt_spin_free itself, if you look at the igt_spin_free code that I've sent for review I'm checking if it's passed from the igt test or 
> Is it created inside the xe_spin_create, I'm not destroying engine and vm when it's passed from igt test.

So in this case you should destroy it in the test, otherwise it will
dangle when test is done.

--
Zbigniew


> > 
> > > +	put_ahnd(ahnd);
> > > +}
> > > +
> > > +/**
> > > + * TEST: Basic test for spin batch submission.
> > > + *
> > > + * SUBTEST: spin-basic
> > > + * Description: Basic test which validates the functionality of spinner.
> > > + * Run type: FULL
> > > + * TODO: change ``'Run type' == FULL`` to a better category
> > > + *
> > > + */
> > > +static void spin_basic(int fd, struct drm_xe_engine_class_instance
> > > +*hwe) {
> > > +	uint64_t ahnd;
> > > +	igt_spin_t *spin;
> > > +
> > > +	ahnd = intel_allocator_open(fd, 0, INTEL_ALLOCATOR_RELOC);
> > > +	spin = __igt_spin_new(fd, .ahnd = ahnd, .hwe = hwe);
> > > +	igt_assert(spin);
> > > +
> > > +	igt_spin_free(fd, spin);
> > > +	put_ahnd(ahnd);
> > > +}
> > > +
> > > +/**
> > > + * TEST: Test for spin batch submissions.
> > > + * SUBTEST: spin-all
> > > + * Description: Spinner test to run on all the engines!
> > > + * Run type: FULL
> > > + * TODO: change ``'Run type' == FULL`` to a better category
> > > + *
> > > + */
> > > +
> > > +static void spin_all (int fd, int gt, int class) {
> > > +	uint64_t ahnd;
> > > +	uint32_t engines[MAX_INSTANCE];
> > > +	uint32_t vm[MAX_INSTANCE];
> > > +	int i, num_placements = 0;
> > > +	struct drm_xe_engine_class_instance eci[MAX_INSTANCE];
> > > +	igt_spin_t *spin[MAX_INSTANCE];
> > > +	struct drm_xe_engine_class_instance *hwe;
> > > +
> > > +	ahnd = intel_allocator_open(fd, 0, INTEL_ALLOCATOR_RELOC);
> > > +
> > > +	xe_for_each_hw_engine(fd, hwe) {
> > > +		if (hwe->engine_class != class || hwe->gt_id != gt)
> > > +			continue;
> > > +		eci[num_placements++] = *hwe;
> > > +	}
> > > +	if (num_placements < 2)
> > > +		return;
> > > +
> > > +	for (i = 0; i < num_placements; i++) {
> > > +		struct drm_xe_engine_create create;
> > > +		vm[i] = xe_vm_create(fd, 0, 0);
> > > +
> > > +		create.vm_id = vm[i];
> > > +		create.width = 1;
> > > +		create.num_placements = num_placements;
> > > +		create.instances = to_user_pointer(eci);
> > > +
> > > +		igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_ENGINE_CREATE,
> > > +					&create), 0);
> > > +		engines[i] = create.engine_id;
> > > +		spin[i] = __igt_spin_new(fd, .ahnd = ahnd, .engine = engines[i],
> > .vm = vm[i]);
> > > +	}
> > > +
> > > +	for (i = 0; i < num_placements; i++) {
> > > +		igt_assert(spin[i]);
> > > +		igt_spin_free(fd, spin[i]);
> > > +	}
> > > +	put_ahnd(ahnd);
> > > +}
> > > +
> > > +igt_main
> > > +{
> > > +	struct drm_xe_engine_class_instance *hwe;
> > > +	int fd;
> > > +	int gt, class;
> > > +
> > > +	igt_fixture {
> > > +		fd = drm_open_driver(DRIVER_XE);
> > > +		xe_device_get(fd);
> > > +	}
> > > +
> > > +	igt_subtest("spin-batch")
> > > +		xe_for_each_hw_engine(fd, hwe)
> > > +			spin(fd, hwe);
> > 
> > Test both variants: with engine/vm created in the test and passed via .engine +
> > .vm field and with passing .hwe only. Add also support for ALL_ENGINES flag.
> > 
> I think spin-batch already covers that, it passes both engine and vm to igt_spin_new which are created in the test.
> And spin-basic passes only .hwe to igt_spin_new.
> 
> Thanks,
> Gowtham 
> > --
> > Zbigniew
> > 
> > > +
> > > +	igt_subtest("spin-basic")
> > > +		xe_for_each_hw_engine(fd, hwe)
> > > +			spin_basic(fd, hwe);
> > > +
> > > +	igt_subtest("spin-all") {
> > > +		xe_for_each_gt(fd, gt)
> > > +			xe_for_each_hw_engine_class(class)
> > > +				spin_all(fd, gt, class);
> > > +	}
> > > +
> > > +	igt_fixture {
> > > +		xe_device_put(fd);
> > > +		close(fd);
> > > +	}
> > > +}
> > > --
> > > 2.39.1
> > >

[igt-dev] [PATCH i-g-t 0/2] Integrate igt_spin_new with Xe

[sai.gowtham.ch]

From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Extending the spin_create implementation and allocator handle support in xe,
where it submits dummy work loads to engine. This Implementation is wrapped
around vm_bind and unbind as we are supposed to do it manually for xe.

Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Sai Gowtham Ch (2):
  lib/xe/xe_spin: Integrate igt_spin_new with Xe
  tests/xe/xe_spin_batch: Add new test to exercise igt_spin_new for xe

 lib/igt_dummyload.c      |  24 ++++--
 lib/igt_dummyload.h      |  12 +++
 lib/xe/xe_spin.c         | 121 ++++++++++++++++++++++++++++
 lib/xe/xe_spin.h         |   6 +-
 tests/meson.build        |   1 +
 tests/xe/xe_spin_batch.c | 168 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 325 insertions(+), 7 deletions(-)
 create mode 100644 tests/xe/xe_spin_batch.c

-- 
2.39.1

[igt-dev] [PATCH i-g-t 0/2] Integrate igt_spin_new with Xe

[sai.gowtham.ch]

From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Extending the spin_create implementation and allocator handle support in xe,
where it submits dummy work loads to engine. This Implementation is wrapped
around vm_bind and unbind as we are supposed to do it manually for xe.

Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Sai Gowtham Ch (2):
  lib/xe/xe_spin: Integrate igt_spin_new with Xe
  tests/xe/xe_spin_batch: Add new test to exercise igt_spin_new for xe

 lib/igt_dummyload.c      |  24 ++++--
 lib/igt_dummyload.h      |  12 +++
 lib/xe/xe_spin.c         | 121 ++++++++++++++++++++++++++++
 lib/xe/xe_spin.h         |   6 +-
 tests/meson.build        |   1 +
 tests/xe/xe_spin_batch.c | 168 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 325 insertions(+), 7 deletions(-)
 create mode 100644 tests/xe/xe_spin_batch.c

-- 
2.39.1

[igt-dev] [PATCH i-g-t 0/2] Integrate igt_spin_new with Xe

[sai.gowtham.ch]

From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Extending the spin_create implementation and allocator handle support in xe,
where it submits dummy work loads to engine. This Implementation is wrapped
around vm_bind and unbind as we are supposed to do it manually for xe.

Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Sai Gowtham Ch (2):
  lib/xe/xe_spin: Integrate igt_spin_new with Xe
  tests/xe/xe_spin_batch: Add new test to exercise igt_spin_new for xe

 lib/igt_dummyload.c      |  38 +++++++--
 lib/igt_dummyload.h      |  11 +++
 lib/xe/xe_spin.c         |  97 ++++++++++++++++++++++
 lib/xe/xe_spin.h         |   6 +-
 tests/meson.build        |   1 +
 tests/xe/xe_spin_batch.c | 168 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 315 insertions(+), 6 deletions(-)
 create mode 100644 tests/xe/xe_spin_batch.c

-- 
2.39.1

[igt-dev] [PATCH i-g-t 0/2] Integrate igt_spin_new with Xe

[sai.gowtham.ch]

From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Extending the spin_create implementation and allocator handle support in xe,
where it submits dummy work loads to engine. This Implementation is wrapped
around vm_bind and unbind as we are supposed to do it manually for xe.

Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Sai Gowtham Ch (2):
  lib/xe/xe_spin: Integrate igt_spin_new with Xe
  tests/xe/xe_spin_batch: Add new test to exercise igt_spin_new for xe

 lib/igt_dummyload.c      |  38 +++++++--
 lib/igt_dummyload.h      |  11 +++
 lib/xe/xe_spin.c         |  96 +++++++++++++++++++++
 lib/xe/xe_spin.h         |   6 +-
 tests/meson.build        |   1 +
 tests/xe/xe_spin_batch.c | 180 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 326 insertions(+), 6 deletions(-)
 create mode 100644 tests/xe/xe_spin_batch.c

-- 
2.39.1

[igt-dev] [PATCH i-g-t 0/2] Integrate igt_spin_new with Xe

[sai.gowtham.ch]

From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Extending the spin_create implementation and allocator handle support in xe,
where it submits dummy work loads to engine. This Implementation is wrapped
around vm_bind and unbind as we are supposed to do it manually for xe.

Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Sai Gowtham Ch (2):
  lib/xe/xe_spin: Integrate igt_spin_new with Xe
  tests/xe/xe_spin_batch: Add new test to exercise igt_spin_new for xe

 lib/igt_dummyload.c      |  38 ++++++--
 lib/igt_dummyload.h      |  11 +++
 lib/xe/xe_spin.c         |  96 +++++++++++++++++++++
 lib/xe/xe_spin.h         |   6 +-
 tests/meson.build        |   1 +
 tests/xe/xe_spin_batch.c | 182 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 328 insertions(+), 6 deletions(-)
 create mode 100644 tests/xe/xe_spin_batch.c

-- 
2.39.1

[igt-dev] [PATCH i-g-t 0/2] Integrate igt_spin_new with Xe

[sai.gowtham.ch]

From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Extending the spin_create implementation and allocator handle support in xe,
where it submits dummy work loads to engine. This Implementation is wrapped
around vm_bind and unbind as we are supposed to do it manually for xe.

Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Sai Gowtham Ch (2):
  lib/xe/xe_spin: Integrate igt_spin_new with Xe
  tests/xe/xe_spin_batch: Add new test to exercise igt_spin_new for xe

 lib/igt_dummyload.c      |  40 +++++++--
 lib/igt_dummyload.h      |  12 +++
 lib/xe/xe_spin.c         |  97 +++++++++++++++++++++
 lib/xe/xe_spin.h         |   5 +-
 tests/meson.build        |   1 +
 tests/xe/xe_spin_batch.c | 179 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 328 insertions(+), 6 deletions(-)
 create mode 100644 tests/xe/xe_spin_batch.c

-- 
2.39.1