[igt-dev] [PATCH i-g-t 00/12] Extend intel_blt to work on Xe

[igt-dev] [PATCH i-g-t 00/12] Extend intel_blt to work on Xe

[Zbigniew Kempczyński]

Blitter library currently supports block-copy, ctrl-surf-copy
and fast-copy on i915. Lets extend this to xe as most of the
code is driver independent.

Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Cc: Karolina Stolarek <karolina.stolarek@intel.com>
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>

Zbigniew Kempczyński (12):
  lib/xe_query: Use vramN when returning string region name
  lib/xe_query: Add xe_region_class() helper
  lib/drmtest: Add get_intel_driver() helper
  lib/xe_util: Return dynamic subtest name for Xe
  lib/xe_util: Add vm bind/unbind helper for Xe
  lib/intel_allocator: Add field to distinct underlying driver
  lib/intel_allocator: Add intel_allocator_bind()
  lib/intel_ctx: Add xe context information
  lib/intel_blt: Introduce blt_copy_init() helper to cache driver
  lib/intel_blt: Extend blitter library to support xe driver
  tests/xe_ccs: Check if flatccs is working with block-copy for Xe
  tests/xe_exercise_blt: Check blitter library fast-copy for Xe

 lib/drmtest.c                  |  10 +
 lib/drmtest.h                  |   1 +
 lib/igt_fb.c                   |   2 +-
 lib/intel_allocator.c          | 162 +++++++
 lib/intel_allocator.h          |   6 +
 lib/intel_blt.c                | 293 +++++++++----
 lib/intel_blt.h                |  10 +-
 lib/intel_ctx.c                | 110 ++++-
 lib/intel_ctx.h                |  14 +
 lib/meson.build                |   3 +-
 lib/xe/xe_query.c              |  20 +-
 lib/xe/xe_query.h              |   1 +
 lib/xe/xe_util.c               | 237 ++++++++++
 lib/xe/xe_util.h               |  48 +++
 tests/i915/gem_ccs.c           |  34 +-
 tests/i915/gem_exercise_blt.c  |  22 +-
 tests/i915/gem_lmem_swapping.c |   4 +-
 tests/meson.build              |   2 +
 tests/xe/xe_ccs.c              | 763 +++++++++++++++++++++++++++++++++
 tests/xe/xe_exercise_blt.c     | 372 ++++++++++++++++
 20 files changed, 1992 insertions(+), 122 deletions(-)
 create mode 100644 lib/xe/xe_util.c
 create mode 100644 lib/xe/xe_util.h
 create mode 100644 tests/xe/xe_ccs.c
 create mode 100644 tests/xe/xe_exercise_blt.c

-- 
2.34.1

[igt-dev] [PATCH i-g-t 01/12] lib/xe_query: Use vramN when returning string region name

[Zbigniew Kempczyński]

For tests which are mixing regions (like xe_ccs) name is confusing.
As an example might be example "subtest-name-vram-1-vram-1". It's
more readable when it will be renamed to "subtest-name-vram1-vram1".

Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 lib/xe/xe_query.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/xe/xe_query.c b/lib/xe/xe_query.c
index 48fb5afba7..830b7e401d 100644
--- a/lib/xe/xe_query.c
+++ b/lib/xe/xe_query.c
@@ -445,7 +445,7 @@ struct drm_xe_query_mem_region *xe_mem_region(int fd, uint64_t region)
  * xe_region_name:
  * @region: region mask
  *
- * Returns region string like "system" or "vram-n" where n=0...62.
+ * Returns region string like "system" or "vramN" where N=0...62.
  */
 const char *xe_region_name(uint64_t region)
 {
@@ -457,7 +457,7 @@ const char *xe_region_name(uint64_t region)
 		vrams = calloc(64, sizeof(char *));
 		for (int i = 0; i < 64; i++) {
 			if (i != 0)
-				asprintf(&vrams[i], "vram-%d", i - 1);
+				asprintf(&vrams[i], "vram%d", i - 1);
 			else
 				asprintf(&vrams[i], "system");
 			igt_assert(vrams[i]);
-- 
2.34.1

[igt-dev] [PATCH i-g-t 02/12] lib/xe_query: Add xe_region_class() helper

[Zbigniew Kempczyński]

In the common code we often need to be aware of region class.
Add helper which returns it from region id.

Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 lib/xe/xe_query.c | 16 ++++++++++++++++
 lib/xe/xe_query.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/lib/xe/xe_query.c b/lib/xe/xe_query.c
index 830b7e401d..f535ad8534 100644
--- a/lib/xe/xe_query.c
+++ b/lib/xe/xe_query.c
@@ -467,6 +467,22 @@ const char *xe_region_name(uint64_t region)
 	return vrams[region_idx];
 }
 
+/**
+ * xe_region_class:
+ * @fd: xe device fd
+ * @region: region mask
+ *
+ * Returns class of memory region structure for @region mask.
+ */
+uint16_t xe_region_class(int fd, uint64_t region)
+{
+	struct drm_xe_query_mem_region *memreg;
+
+	memreg = xe_mem_region(fd, region);
+
+	return memreg->mem_class;
+}
+
 /**
  * xe_min_page_size:
  * @fd: xe device fd
diff --git a/lib/xe/xe_query.h b/lib/xe/xe_query.h
index ff328ab942..68ca5a680c 100644
--- a/lib/xe/xe_query.h
+++ b/lib/xe/xe_query.h
@@ -85,6 +85,7 @@ struct drm_xe_engine_class_instance *xe_hw_engines(int fd);
 struct drm_xe_engine_class_instance *xe_hw_engine(int fd, int idx);
 struct drm_xe_query_mem_region *xe_mem_region(int fd, uint64_t region);
 const char *xe_region_name(uint64_t region);
+uint16_t xe_region_class(int fd, uint64_t region);
 uint32_t xe_min_page_size(int fd, uint64_t region);
 struct drm_xe_query_config *xe_config(int fd);
 unsigned int xe_number_hw_engines(int fd);
-- 
2.34.1

[igt-dev] [PATCH i-g-t 03/12] lib/drmtest: Add get_intel_driver() helper

[Zbigniew Kempczyński]

In libraries which diverges i915 and xe code we might use
is_xe_device() or is_i915_device() to distinct code paths.
But to avoid additional open and string compare we can cache
this information in data structures.

Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 lib/drmtest.c | 10 ++++++++++
 lib/drmtest.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index 5cdb0196d3..e1da66c877 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -151,6 +151,16 @@ bool is_intel_device(int fd)
 	return is_i915_device(fd) || is_xe_device(fd);
 }
 
+enum intel_driver get_intel_driver(int fd)
+{
+	if (is_xe_device(fd))
+		return INTEL_DRIVER_XE;
+	else if (is_i915_device(fd))
+		return INTEL_DRIVER_I915;
+
+	igt_assert_f(0, "Device is not handled by Intel driver\n");
+}
+
 static char _forced_driver[16] = "";
 
 /**
diff --git a/lib/drmtest.h b/lib/drmtest.h
index 9c3ea5d14c..97ab6e759e 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -124,6 +124,7 @@ bool is_nouveau_device(int fd);
 bool is_vc4_device(int fd);
 bool is_xe_device(int fd);
 bool is_intel_device(int fd);
+enum intel_driver get_intel_driver(int fd);
 
 /**
  * do_or_die:
-- 
2.34.1

[igt-dev] [PATCH i-g-t 04/12] lib/xe_util: Return dynamic subtest name for Xe

[Zbigniew Kempczyński]

For tests which are working on more than one region using name suffix
like "vram01-system" etc. is common thing. Instead handcrafting this
naming add xe_memregion_dynamic_subtest_name() function which is
similar to memregion_dynamic_subtest_name() for i915.

Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 lib/meson.build  |   3 +-
 lib/xe/xe_util.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/xe/xe_util.h |  33 +++++++++++++++
 3 files changed, 139 insertions(+), 1 deletion(-)
 create mode 100644 lib/xe/xe_util.c
 create mode 100644 lib/xe/xe_util.h

diff --git a/lib/meson.build b/lib/meson.build
index 3e1ecdee2b..1d5b81ac8f 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -105,7 +105,8 @@ lib_sources = [
 	'xe/xe_compute_square_kernels.c',
 	'xe/xe_ioctl.c',
 	'xe/xe_query.c',
-	'xe/xe_spin.c'
+	'xe/xe_spin.c',
+	'xe/xe_util.c',
 ]
 
 lib_deps = [
diff --git a/lib/xe/xe_util.c b/lib/xe/xe_util.c
new file mode 100644
index 0000000000..448b3a3d27
--- /dev/null
+++ b/lib/xe/xe_util.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#include "igt.h"
+#include "igt_syncobj.h"
+#include "xe/xe_ioctl.h"
+#include "xe/xe_query.h"
+#include "xe/xe_util.h"
+
+static bool __region_belongs_to_regions_type(struct drm_xe_query_mem_region *region,
+					     uint32_t *mem_regions_type,
+					     int num_regions)
+{
+	for (int i = 0; i < num_regions; i++)
+		if (mem_regions_type[i] == region->mem_class)
+			return true;
+	return false;
+}
+
+struct igt_collection *
+__xe_get_memory_region_set(int xe, uint32_t *mem_regions_type, int num_regions)
+{
+	struct drm_xe_query_mem_region *memregion;
+	struct igt_collection *set = NULL;
+	uint64_t memreg = all_memory_regions(xe), region;
+	int count = 0, pos = 0;
+
+	xe_for_each_mem_region(xe, memreg, region) {
+		memregion = xe_mem_region(xe, region);
+		if (__region_belongs_to_regions_type(memregion,
+						     mem_regions_type,
+						     num_regions))
+			count++;
+	}
+
+	set = igt_collection_create(count);
+
+	xe_for_each_mem_region(xe, memreg, region) {
+		memregion = xe_mem_region(xe, region);
+		igt_assert(region < (1ull << 31));
+		if (__region_belongs_to_regions_type(memregion,
+						     mem_regions_type,
+						     num_regions)) {
+			igt_collection_set_value(set, pos++, (int)region);
+		}
+	}
+
+	igt_assert(count == pos);
+
+	return set;
+}
+
+/**
+  * xe_memregion_dynamic_subtest_name:
+  * @xe: drm fd of Xe device
+  * @igt_collection: memory region collection
+  *
+  * Function iterates over all memory regions inside the collection (keeped
+  * in the value field) and generates the name which can be used during dynamic
+  * subtest creation.
+  *
+  * Returns: newly allocated string, has to be freed by caller. Asserts if
+  * caller tries to create a name using empty collection.
+  */
+char *xe_memregion_dynamic_subtest_name(int xe, struct igt_collection *set)
+{
+	struct igt_collection_data *data;
+	char *name, *p;
+	uint32_t region, len;
+
+	igt_assert(set && set->size);
+	/* enough for "name%d-" * n */
+	len = set->size * 8;
+	p = name = malloc(len);
+	igt_assert(name);
+
+	for_each_collection_data(data, set) {
+		struct drm_xe_query_mem_region *memreg;
+		int r;
+
+		region = data->value;
+		memreg = xe_mem_region(xe, region);
+
+		if (XE_IS_CLASS_VRAM(memreg))
+			r = snprintf(p, len, "%s%d-",
+				     xe_region_name(region),
+				     memreg->instance);
+		else
+			r = snprintf(p, len, "%s-",
+				     xe_region_name(region));
+
+		igt_assert(r > 0);
+		p += r;
+		len -= r;
+	}
+
+	/* remove last '-' */
+	*(p - 1) = 0;
+
+	return name;
+}
+
diff --git a/lib/xe/xe_util.h b/lib/xe/xe_util.h
new file mode 100644
index 0000000000..46b7e0d46b
--- /dev/null
+++ b/lib/xe/xe_util.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2023 Intel Corporation
+ *
+ */
+
+#ifndef XE_UTIL_H
+#define XE_UTIL_H
+
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <xe_drm.h>
+
+#define XE_REGION_SMEM XE_MEM_REGION_CLASS_SYSMEM
+#define XE_REGION_LMEM XE_MEM_REGION_CLASS_VRAM
+
+#define XE_IS_SYSMEM_MEMORY_REGION(fd, region) \
+	(xe_region_class(fd, region) == XE_MEM_REGION_CLASS_SYSMEM)
+#define XE_IS_VRAM_MEMORY_REGION(fd, region) \
+	(xe_region_class(fd, region) == XE_MEM_REGION_CLASS_VRAM)
+
+struct igt_collection *
+__xe_get_memory_region_set(int xe, uint32_t *mem_regions_type, int num_regions);
+
+#define xe_get_memory_region_set(regions, mem_region_types...) ({ \
+	unsigned int arr__[] = { mem_region_types }; \
+	__xe_get_memory_region_set(regions, arr__, ARRAY_SIZE(arr__)); \
+})
+
+char *xe_memregion_dynamic_subtest_name(int xe, struct igt_collection *set);
+
+#endif /* XE_UTIL_H */
-- 
2.34.1

[igt-dev] [PATCH i-g-t 05/12] lib/xe_util: Add vm bind/unbind helper for Xe

[Zbigniew Kempczyński]

Before calling exec we need to prepare vm to contain valid entries.
Bind/unbind in xe expects single bind_op or vector of bind_ops what
makes preparation of it a little bit inconvinient. Add function
which iterates over list of xe_object (auxiliary structure which
describes bind information for object) and performs the bind/unbind
in one step. It also supports passing syncobj in/out to work in
pipelined executions.

Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 lib/xe/xe_util.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/xe/xe_util.h |  21 ++++++--
 2 files changed, 151 insertions(+), 3 deletions(-)

diff --git a/lib/xe/xe_util.c b/lib/xe/xe_util.c
index 448b3a3d27..8552db47d5 100644
--- a/lib/xe/xe_util.c
+++ b/lib/xe/xe_util.c
@@ -102,3 +102,136 @@ char *xe_memregion_dynamic_subtest_name(int xe, struct igt_collection *set)
 	return name;
 }
 
+#ifdef XEBINDDBG
+#define bind_info igt_info
+#define bind_debug igt_debug
+#else
+#define bind_info(...) {}
+#define bind_debug(...) {}
+#endif
+
+static struct drm_xe_vm_bind_op *xe_alloc_bind_ops(struct igt_list_head *obj_list,
+						   uint32_t *num_ops)
+{
+	struct drm_xe_vm_bind_op *bind_ops, *ops;
+	struct xe_object *obj;
+	uint32_t num_objects = 0, i = 0, op;
+
+	num_objects = 0;
+	igt_list_for_each_entry(obj, obj_list, link)
+		num_objects++;
+
+	*num_ops = num_objects;
+	if (!num_objects) {
+		bind_info(" [nothing to bind]\n");
+		return NULL;
+	}
+
+	bind_ops = calloc(num_objects, sizeof(*bind_ops));
+	igt_assert(bind_ops);
+
+	igt_list_for_each_entry(obj, obj_list, link) {
+		ops = &bind_ops[i];
+
+		if (obj->bind_op == XE_OBJECT_BIND) {
+			op = XE_VM_BIND_OP_MAP | XE_VM_BIND_FLAG_ASYNC;
+			ops->obj = obj->handle;
+		} else {
+			op = XE_VM_BIND_OP_UNMAP | XE_VM_BIND_FLAG_ASYNC;
+		}
+
+		ops->op = op;
+		ops->obj_offset = 0;
+		ops->addr = obj->offset;
+		ops->range = obj->size;
+		ops->region = 0;
+
+		bind_info("  [%d]: [%6s] handle: %u, offset: %llx, size: %llx\n",
+			  i, obj->bind_op == XE_OBJECT_BIND ? "BIND" : "UNBIND",
+			  ops->obj, (long long)ops->addr, (long long)ops->range);
+		i++;
+	}
+
+	return bind_ops;
+}
+
+static void __xe_op_bind_async(int xe, uint32_t vm, uint32_t bind_engine,
+			       struct igt_list_head *obj_list,
+			       uint32_t sync_in, uint32_t sync_out)
+{
+	struct drm_xe_vm_bind_op *bind_ops;
+	struct drm_xe_sync tabsyncs[2] = {
+		{ .flags = DRM_XE_SYNC_SYNCOBJ, .handle = sync_in },
+		{ .flags = DRM_XE_SYNC_SYNCOBJ | DRM_XE_SYNC_SIGNAL, .handle = sync_out },
+	};
+	struct drm_xe_sync *syncs;
+	uint32_t num_binds = 0;
+	int num_syncs;
+
+	bind_info("[Binding to vm: %u]\n", vm);
+	bind_ops = xe_alloc_bind_ops(obj_list, &num_binds);
+
+	if (!num_binds) {
+		if (sync_out)
+			syncobj_signal(xe, &sync_out, 1);
+		return;
+	}
+
+	if (sync_in) {
+		syncs = tabsyncs;
+		num_syncs = 2;
+	} else {
+		syncs = &tabsyncs[1];
+		num_syncs = 1;
+	}
+
+	/* User didn't pass sync out, create it and wait for completion */
+	if (!sync_out)
+		tabsyncs[1].handle = syncobj_create(xe, 0);
+
+	bind_info("[Binding syncobjs: (in: %u, out: %u)]\n",
+		  tabsyncs[0].handle, tabsyncs[1].handle);
+
+	if (num_binds == 1) {
+		if ((bind_ops[0].op & 0xffff) == XE_VM_BIND_OP_MAP)
+			xe_vm_bind_async(xe, vm, bind_engine, bind_ops[0].obj, 0,
+					bind_ops[0].addr, bind_ops[0].range,
+					syncs, num_syncs);
+		else
+			xe_vm_unbind_async(xe, vm, bind_engine, 0,
+					   bind_ops[0].addr, bind_ops[0].range,
+					   syncs, num_syncs);
+	} else {
+		xe_vm_bind_array(xe, vm, bind_engine, bind_ops,
+				 num_binds, syncs, num_syncs);
+	}
+
+	if (!sync_out) {
+		igt_assert_eq(syncobj_wait_err(xe, &tabsyncs[1].handle, 1, INT64_MAX, 0), 0);
+		syncobj_destroy(xe, tabsyncs[1].handle);
+	}
+
+	free(bind_ops);
+}
+
+/**
+  * xe_bind_unbind_async:
+  * @xe: drm fd of Xe device
+  * @vm: vm to bind/unbind objects to/from
+  * @bind_engine: bind engine, 0 if default
+  * @obj_list: list of xe_object
+  * @sync_in: sync object (fence-in), 0 if there's no input dependency
+  * @sync_out: sync object (fence-out) to signal on bind/unbind completion,
+  *            if 0 wait for bind/unbind completion.
+  *
+  * Function iterates over xe_object @obj_list, prepares binding operation
+  * and does bind/unbind in one step. Providing sync_in / sync_out allows
+  * working in pipelined mode. With sync_in and sync_out set to 0 function
+  * waits until binding operation is complete.
+  */
+void xe_bind_unbind_async(int fd, uint32_t vm, uint32_t bind_engine,
+			  struct igt_list_head *obj_list,
+			  uint32_t sync_in, uint32_t sync_out)
+{
+	return __xe_op_bind_async(fd, vm, bind_engine, obj_list, sync_in, sync_out);
+}
diff --git a/lib/xe/xe_util.h b/lib/xe/xe_util.h
index 46b7e0d46b..32f309923e 100644
--- a/lib/xe/xe_util.h
+++ b/lib/xe/xe_util.h
@@ -12,9 +12,6 @@
 #include <stdint.h>
 #include <xe_drm.h>
 
-#define XE_REGION_SMEM XE_MEM_REGION_CLASS_SYSMEM
-#define XE_REGION_LMEM XE_MEM_REGION_CLASS_VRAM
-
 #define XE_IS_SYSMEM_MEMORY_REGION(fd, region) \
 	(xe_region_class(fd, region) == XE_MEM_REGION_CLASS_SYSMEM)
 #define XE_IS_VRAM_MEMORY_REGION(fd, region) \
@@ -30,4 +27,22 @@ __xe_get_memory_region_set(int xe, uint32_t *mem_regions_type, int num_regions);
 
 char *xe_memregion_dynamic_subtest_name(int xe, struct igt_collection *set);
 
+enum xe_bind_op {
+	XE_OBJECT_BIND,
+	XE_OBJECT_UNBIND,
+};
+
+struct xe_object {
+	uint32_t handle;
+	uint64_t offset;
+	uint64_t size;
+	enum xe_bind_op bind_op;
+	void *priv;
+	struct igt_list_head link;
+};
+
+void xe_bind_unbind_async(int fd, uint32_t vm, uint32_t bind_engine,
+			  struct igt_list_head *obj_list,
+			  uint32_t sync_in, uint32_t sync_out);
+
 #endif /* XE_UTIL_H */
-- 
2.34.1

[igt-dev] [PATCH i-g-t 06/12] lib/intel_allocator: Add field to distinct underlying driver

[Zbigniew Kempczyński]

Cache what driver is using on drm fd to avoid calling same code
in allocator functions.

Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 lib/intel_allocator.c | 1 +
 lib/intel_allocator.h | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/lib/intel_allocator.c b/lib/intel_allocator.c
index 8161221dbf..c98c410b3b 100644
--- a/lib/intel_allocator.c
+++ b/lib/intel_allocator.c
@@ -318,6 +318,7 @@ static struct intel_allocator *intel_allocator_create(int fd,
 
 	igt_assert(ial);
 
+	ial->driver = get_intel_driver(fd);
 	ial->type = allocator_type;
 	ial->strategy = allocator_strategy;
 	ial->default_alignment = default_alignment;
diff --git a/lib/intel_allocator.h b/lib/intel_allocator.h
index a6bf573e9d..ed3a78485d 100644
--- a/lib/intel_allocator.h
+++ b/lib/intel_allocator.h
@@ -141,6 +141,9 @@ struct intel_allocator {
 	/* allocator's private structure */
 	void *priv;
 
+	/* driver - i915 or Xe */
+	enum intel_driver driver;
+
 	void (*get_address_range)(struct intel_allocator *ial,
 				  uint64_t *startp, uint64_t *endp);
 	uint64_t (*alloc)(struct intel_allocator *ial, uint32_t handle,
-- 
2.34.1

[igt-dev] [PATCH i-g-t 07/12] lib/intel_allocator: Add intel_allocator_bind()

[Zbigniew Kempczyński]

Synchronize allocator state to vm.

This change allows xe user to execute vm-bind/unbind for allocator
alloc()/free() operations which occurred since last binding/unbinding.
Before doing exec user should call intel_allocator_bind() to ensure
all vma's are in place.

Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 lib/intel_allocator.c | 161 ++++++++++++++++++++++++++++++++++++++++++
 lib/intel_allocator.h |   3 +
 2 files changed, 164 insertions(+)

diff --git a/lib/intel_allocator.c b/lib/intel_allocator.c
index c98c410b3b..20d2c10f50 100644
--- a/lib/intel_allocator.c
+++ b/lib/intel_allocator.c
@@ -17,6 +17,7 @@
 #include "intel_allocator.h"
 #include "intel_allocator_msgchannel.h"
 #include "xe/xe_query.h"
+#include "xe/xe_util.h"
 
 //#define ALLOCDBG
 #ifdef ALLOCDBG
@@ -46,6 +47,14 @@ static inline const char *reqstr(enum reqtype request_type)
 #define alloc_debug(...) {}
 #endif
 
+#ifdef ALLOCBINDDBG
+#define bind_info igt_info
+#define bind_debug igt_debug
+#else
+#define bind_info(...) {}
+#define bind_debug(...) {}
+#endif
+
 /*
  * We limit allocator space to avoid hang when batch would be
  * pinned in the last page.
@@ -58,6 +67,7 @@ struct allocator {
 	uint32_t vm;
 	_Atomic(int32_t) refcount;
 	struct intel_allocator *ial;
+	struct igt_map *bind_map;
 };
 
 struct handle_entry {
@@ -65,6 +75,21 @@ struct handle_entry {
 	struct allocator *al;
 };
 
+/* For tracking alloc()/free() for Xe */
+enum allocator_bind_op {
+	BOUND,
+	TO_BIND,
+	TO_UNBIND,
+};
+
+struct allocator_object {
+	uint32_t handle;
+	uint64_t offset;
+	uint64_t size;
+
+	enum allocator_bind_op bind_op;
+};
+
 struct intel_allocator *
 intel_allocator_reloc_create(int fd, uint64_t start, uint64_t end);
 struct intel_allocator *
@@ -234,6 +259,7 @@ static struct allocator *__allocator_create(int fd, uint32_t ctx, uint32_t vm,
 	al->vm = vm;
 	atomic_init(&al->refcount, 0);
 	al->ial = ial;
+	al->bind_map = igt_map_create(igt_map_hash_32, igt_map_equal_32);
 
 	igt_map_insert(map, al, al);
 
@@ -404,6 +430,7 @@ static bool allocator_close(uint64_t ahnd)
 	released = __allocator_put(al);
 	if (released) {
 		is_empty = al->ial->is_empty(al->ial);
+		igt_map_destroy(al->bind_map, map_entry_free_func);
 		intel_allocator_destroy(al->ial);
 	}
 
@@ -1108,6 +1135,60 @@ void intel_allocator_get_address_range(uint64_t allocator_handle,
 		*endp = resp.address_range.end;
 }
 
+static bool is_same(struct allocator_object *obj,
+		    uint32_t handle, uint64_t offset, uint64_t size,
+		    enum allocator_bind_op bind_op)
+{
+	return obj->handle == handle &&	obj->offset == offset && obj->size == size &&
+	       (obj->bind_op == bind_op || obj->bind_op == BOUND);
+}
+
+static void track_object(uint64_t allocator_handle, uint32_t handle,
+			 uint64_t offset, uint64_t size,
+			 enum allocator_bind_op bind_op)
+{
+	struct allocator_object *obj;
+	struct allocator *al;
+
+	bind_info("track: [%s] ahnd: %lld, handle: %u, offset: %llx, size: %llx\n",
+		  bind_op == TO_BIND ? "BIND" : "UNBIND",
+		  (long long)allocator_handle,
+		  handle, (long long)offset, (long long)size);
+	al = __allocator_find_by_handle(allocator_handle);
+	igt_assert(al);
+
+	if (al->ial->driver == INTEL_DRIVER_I915)
+		return; /* no-op for i915, at least now */
+
+	obj = igt_map_search(al->bind_map, &handle);
+	if (obj) {
+		/*
+		 * User may call alloc() couple of times, check object is the
+		 * same. For free() there's simple case, just remove from
+		 * bind_map.
+		 */
+		if (bind_op == TO_BIND)
+			igt_assert_eq(is_same(obj, handle, offset, size, bind_op), true);
+		else if (bind_op == TO_UNBIND) {
+			if (obj->bind_op == TO_BIND)
+				igt_map_remove(al->bind_map, &obj->handle, map_entry_free_func);
+			else if (obj->bind_op == BOUND)
+				obj->bind_op = bind_op;
+		}
+	} else {
+		/* Ignore to unbind bo which wasn't previously inserted */
+		if (bind_op == TO_UNBIND)
+			return;
+
+		obj = calloc(1, sizeof(*obj));
+		obj->handle = handle;
+		obj->offset = offset;
+		obj->size = size;
+		obj->bind_op = bind_op;
+		igt_map_insert(al->bind_map, &obj->handle, obj);
+	}
+}
+
 /**
  * __intel_allocator_alloc:
  * @allocator_handle: handle to an allocator
@@ -1139,6 +1220,8 @@ uint64_t __intel_allocator_alloc(uint64_t allocator_handle, uint32_t handle,
 	igt_assert(handle_request(&req, &resp) == 0);
 	igt_assert(resp.response_type == RESP_ALLOC);
 
+	track_object(allocator_handle, handle, resp.alloc.offset, size, TO_BIND);
+
 	return resp.alloc.offset;
 }
 
@@ -1216,6 +1299,8 @@ bool intel_allocator_free(uint64_t allocator_handle, uint32_t handle)
 	igt_assert(handle_request(&req, &resp) == 0);
 	igt_assert(resp.response_type == RESP_FREE);
 
+	track_object(allocator_handle, handle, 0, 0, TO_UNBIND);
+
 	return resp.free.freed;
 }
 
@@ -1400,6 +1485,82 @@ void intel_allocator_print(uint64_t allocator_handle)
 	}
 }
 
+static void __xe_op_bind(struct allocator *al, uint32_t sync_in, uint32_t sync_out)
+{
+	struct allocator_object *obj;
+	struct igt_map_entry *pos;
+	struct igt_list_head obj_list;
+	struct xe_object *entry, *tmp;
+
+	IGT_INIT_LIST_HEAD(&obj_list);
+
+	igt_map_foreach(al->bind_map, pos) {
+		obj = pos->data;
+
+		if (obj->bind_op == BOUND)
+			continue;
+
+		bind_info("= [vm: %u] %s => %u %lx %lx\n",
+			  al->ctx,
+			  obj->bind_op == TO_BIND ? "TO BIND" : "TO UNBIND",
+			  obj->handle, obj->offset,
+			  obj->size);
+
+		entry = malloc(sizeof(*entry));
+		entry->handle = obj->handle;
+		entry->offset = obj->offset;
+		entry->size = obj->size;
+		entry->bind_op = obj->bind_op == TO_BIND ? XE_OBJECT_BIND :
+							   XE_OBJECT_UNBIND;
+		entry->priv = obj;
+		igt_list_add(&entry->link, &obj_list);
+	}
+
+	xe_bind_unbind_async(al->fd, al->ctx, 0, &obj_list, sync_in, sync_out);
+
+	igt_list_for_each_entry_safe(entry, tmp, &obj_list, link) {
+		obj = entry->priv;
+		if (obj->bind_op == TO_BIND)
+			obj->bind_op = BOUND;
+		else
+			igt_map_remove(al->bind_map, &obj->handle, map_entry_free_func);
+
+		igt_list_del(&entry->link);
+		free(entry);
+	}
+}
+
+/**
+ * intel_allocator_bind:
+ * @allocator_handle: handle to an allocator
+ * @sync_in: syncobj (fence-in)
+ * @sync_out: syncobj (fence-out)
+ *
+ * Function binds and unbinds all objects added to the allocator which weren't
+ * previously binded/unbinded.
+ *
+ **/
+void intel_allocator_bind(uint64_t allocator_handle,
+			  uint32_t sync_in, uint32_t sync_out)
+{
+	struct allocator *al;
+
+	igt_assert(allocator_handle);
+
+	al = __allocator_find_by_handle(allocator_handle);
+	igt_assert(al);
+
+	if (al->ial->driver == INTEL_DRIVER_I915)
+		return; /* no-op for i915, at least now */
+
+	/*
+	 * We collect bind/unbind operations on alloc()/free() to do group
+	 * operationgetting @sync_in as syncobj handle (fence-in). If user
+	 * passes 0 as @sync_out we bind/unbind synchronously.
+	 */
+	__xe_op_bind(al, sync_in, sync_out);
+}
+
 static int equal_handles(const void *key1, const void *key2)
 {
 	const struct handle_entry *h1 = key1, *h2 = key2;
diff --git a/lib/intel_allocator.h b/lib/intel_allocator.h
index ed3a78485d..ad86dc5524 100644
--- a/lib/intel_allocator.h
+++ b/lib/intel_allocator.h
@@ -214,6 +214,9 @@ bool intel_allocator_reserve_if_not_allocated(uint64_t allocator_handle,
 
 void intel_allocator_print(uint64_t allocator_handle);
 
+void intel_allocator_bind(uint64_t allocator_handle,
+			  uint32_t sync_in, uint32_t sync_out);
+
 #define ALLOC_INVALID_ADDRESS (-1ull)
 #define INTEL_ALLOCATOR_NONE   0
 #define INTEL_ALLOCATOR_RELOC  1
-- 
2.34.1

[igt-dev] [PATCH i-g-t 08/12] lib/intel_ctx: Add xe context information

[Zbigniew Kempczyński]

Most complicated part in adopting i915_blt to intel_blt - which should
handle both drivers - is how to achieve pipelined execution. In term
pipelined execution I mean all gpu workloads are executed without
stalls.

Comparing to i915 relocations and softpinning xe architecture migrates
binding (this means also unbind operation) responsibility from the
kernel to user via vm_bind ioctl(). To avoid stalls user has to
provide in/out fences (syncobjs) between consecutive bindings/execs.
Of course for many igt tests we don't need pipelined execution,
just synchronous bind, then exec. But exercising the driver should
also cover pipelining to verify it is possible to work without stalls.

I decided to extend intel_ctx_t with all necessary for xe objects
(vm, engine, syncobjs) to get flexibility in deciding how to bind,
execute and wait for (synchronize) those operations. Context object
along with i915 engine is already passed to blitter library so adding
xe required fields doesn't break i915 but will allow xe path to get
all necessary data to execute.

Using intel_ctx with xe requires some code patterns caused by usage
of an allocator. For xe the allocator started tracking alloc()/free()
operations to do bind/unbind in one call just before execution.
I've added two helpers in intel_ctx which - intel_ctx_xe_exec()
and intel_ctx_xe_sync(). Depending how intel_ctx was created
(with 0 or real syncobj handles as in/bind/out fences) bind and exec
in intel_ctx_xe_exec() are pipelined but synchronize last operation
(exec). For real syncobjs they are used to join bind + exec calls
but there's no wait for exec (sync-out) completion. This allows
building more cascaded bind + exec operations without stalls.

To wait for a sync-out fence caller may use intel_ctx_xe_sync()
which is synchronous wait on syncobj. It allows user to reset
fences to prepare for next operation.

Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 lib/intel_ctx.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++-
 lib/intel_ctx.h |  14 ++++++
 2 files changed, 123 insertions(+), 1 deletion(-)

diff --git a/lib/intel_ctx.c b/lib/intel_ctx.c
index ded9c0f1e4..f210907fac 100644
--- a/lib/intel_ctx.c
+++ b/lib/intel_ctx.c
@@ -5,9 +5,12 @@
 
 #include <stddef.h>
 
+#include "i915/gem_engine_topology.h"
+#include "igt_syncobj.h"
+#include "intel_allocator.h"
 #include "intel_ctx.h"
 #include "ioctl_wrappers.h"
-#include "i915/gem_engine_topology.h"
+#include "xe/xe_ioctl.h"
 
 /**
  * SECTION:intel_ctx
@@ -390,3 +393,108 @@ unsigned int intel_ctx_engine_class(const intel_ctx_t *ctx, unsigned int engine)
 {
 	return intel_ctx_cfg_engine_class(&ctx->cfg, engine);
 }
+
+/**
+ * intel_ctx_xe:
+ * @fd: open i915 drm file descriptor
+ * @vm: vm
+ * @engine: engine
+ *
+ * Returns an intel_ctx_t representing the xe context.
+ */
+intel_ctx_t *intel_ctx_xe(int fd, uint32_t vm, uint32_t engine,
+			  uint32_t sync_in, uint32_t sync_bind, uint32_t sync_out)
+{
+	intel_ctx_t *ctx;
+
+	ctx = calloc(1, sizeof(*ctx));
+	igt_assert(ctx);
+
+	ctx->fd = fd;
+	ctx->vm = vm;
+	ctx->engine = engine;
+	ctx->sync_in = sync_in;
+	ctx->sync_bind = sync_bind;
+	ctx->sync_out = sync_out;
+
+	return ctx;
+}
+
+static int __xe_exec(int fd, struct drm_xe_exec *exec)
+{
+	int err = 0;
+
+	if (igt_ioctl(fd, DRM_IOCTL_XE_EXEC, exec)) {
+		err = -errno;
+		igt_assume(err != 0);
+	}
+	errno = 0;
+	return err;
+}
+
+int __intel_ctx_xe_exec(const intel_ctx_t *ctx, uint64_t ahnd, uint64_t bb_offset)
+{
+	struct drm_xe_sync syncs[2] = {
+		{ .flags = DRM_XE_SYNC_SYNCOBJ, },
+		{ .flags = DRM_XE_SYNC_SYNCOBJ | DRM_XE_SYNC_SIGNAL, },
+	};
+	struct drm_xe_exec exec = {
+		.engine_id = ctx->engine,
+		.syncs = (uintptr_t)syncs,
+		.num_syncs = 2,
+		.address = bb_offset,
+		.num_batch_buffer = 1,
+	};
+	uint32_t sync_in = ctx->sync_in;
+	uint32_t sync_bind = ctx->sync_bind ?: syncobj_create(ctx->fd, 0);
+	uint32_t sync_out = ctx->sync_out ?: syncobj_create(ctx->fd, 0);
+	int ret;
+
+	/* Synchronize allocator state -> vm */
+	intel_allocator_bind(ahnd, sync_in, sync_bind);
+
+	/* Pipelined exec */
+	syncs[0].handle = sync_bind;
+	syncs[1].handle = sync_out;
+
+	ret = __xe_exec(ctx->fd, &exec);
+	if (ret)
+		goto err;
+
+	if (!ctx->sync_bind || !ctx->sync_out)
+		syncobj_wait_err(ctx->fd, &sync_out, 1, INT64_MAX, 0);
+
+err:
+	if (!ctx->sync_bind)
+		syncobj_destroy(ctx->fd, sync_bind);
+
+	if (!ctx->sync_out)
+		syncobj_destroy(ctx->fd, sync_out);
+
+	return ret;
+}
+
+void intel_ctx_xe_exec(const intel_ctx_t *ctx, uint64_t ahnd, uint64_t bb_offset)
+{
+	igt_assert_eq(__intel_ctx_xe_exec(ctx, ahnd, bb_offset), 0);
+}
+
+#define RESET_SYNCOBJ(__fd, __sync) do { \
+	if (__sync) \
+		syncobj_reset((__fd), &(__sync), 1); \
+} while (0)
+
+int intel_ctx_xe_sync(intel_ctx_t *ctx, bool reset_syncs)
+{
+	int ret;
+
+	ret = syncobj_wait_err(ctx->fd, &ctx->sync_out, 1, INT64_MAX, 0);
+
+	if (reset_syncs) {
+		RESET_SYNCOBJ(ctx->fd, ctx->sync_in);
+		RESET_SYNCOBJ(ctx->fd, ctx->sync_bind);
+		RESET_SYNCOBJ(ctx->fd, ctx->sync_out);
+	}
+
+	return ret;
+}
diff --git a/lib/intel_ctx.h b/lib/intel_ctx.h
index 3cfeaae81e..59d0360ada 100644
--- a/lib/intel_ctx.h
+++ b/lib/intel_ctx.h
@@ -67,6 +67,14 @@ int intel_ctx_cfg_engine_class(const intel_ctx_cfg_t *cfg, unsigned int engine);
 typedef struct intel_ctx {
 	uint32_t id;
 	intel_ctx_cfg_t cfg;
+
+	/* Xe */
+	int fd;
+	uint32_t vm;
+	uint32_t engine;
+	uint32_t sync_in;
+	uint32_t sync_bind;
+	uint32_t sync_out;
 } intel_ctx_t;
 
 int __intel_ctx_create(int fd, const intel_ctx_cfg_t *cfg,
@@ -81,4 +89,10 @@ void intel_ctx_destroy(int fd, const intel_ctx_t *ctx);
 
 unsigned int intel_ctx_engine_class(const intel_ctx_t *ctx, unsigned int engine);
 
+intel_ctx_t *intel_ctx_xe(int fd, uint32_t vm, uint32_t engine,
+			  uint32_t sync_in, uint32_t sync_bind, uint32_t sync_out);
+int __intel_ctx_xe_exec(const intel_ctx_t *ctx, uint64_t ahnd, uint64_t bb_offset);
+void intel_ctx_xe_exec(const intel_ctx_t *ctx, uint64_t ahnd, uint64_t bb_offset);
+int intel_ctx_xe_sync(intel_ctx_t *ctx, bool reset_syncs);
+
 #endif
-- 
2.34.1

[igt-dev] [PATCH i-g-t 09/12] lib/intel_blt: Introduce blt_copy_init() helper to cache driver

[Zbigniew Kempczyński]

Instead of calling is_xe_device() and is_i915_device() multiple
times in code which distincs xe and i915 paths add driver field
to structures used in blitter library.

Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 lib/igt_fb.c                   |  2 +-
 lib/intel_blt.c                | 40 +++++++++++++++++++++++++++++++---
 lib/intel_blt.h                |  8 ++++++-
 tests/i915/gem_ccs.c           | 34 ++++++++++++++++-------------
 tests/i915/gem_exercise_blt.c  | 22 ++++++++++---------
 tests/i915/gem_lmem_swapping.c |  4 ++--
 6 files changed, 78 insertions(+), 32 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index a8988274f2..1814e8db11 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2900,7 +2900,7 @@ static void blitcopy(const struct igt_fb *dst_fb,
 			src = blt_fb_init(src_fb, i, mem_region);
 			dst = blt_fb_init(dst_fb, i, mem_region);
 
-			memset(&blt, 0, sizeof(blt));
+			blt_copy_init(src_fb->fd, &blt);
 			blt.color_depth = blt_get_bpp(src_fb);
 			blt_set_copy_object(&blt.src, src);
 			blt_set_copy_object(&blt.dst, dst);
diff --git a/lib/intel_blt.c b/lib/intel_blt.c
index bc28f15e8d..f2f86e4947 100644
--- a/lib/intel_blt.c
+++ b/lib/intel_blt.c
@@ -692,6 +692,22 @@ static void dump_bb_ext(struct gen12_block_copy_data_ext *data)
 		 data->dw21.src_array_index);
 }
 
+/**
+ * blt_copy_init:
+ * @fd: drm fd
+ * @blt: structure for initialization
+ *
+ * Function is zeroing @blt and sets fd and driver fields (INTEL_DRIVER_I915 or
+ * INTEL_DRIVER_XE).
+ */
+void blt_copy_init(int fd, struct blt_copy_data *blt)
+{
+	memset(blt, 0, sizeof(*blt));
+
+	blt->fd = fd;
+	blt->driver = get_intel_driver(fd);
+}
+
 /**
  * emit_blt_block_copy:
  * @fd: drm fd
@@ -889,6 +905,22 @@ static void dump_bb_surf_ctrl_cmd(const struct gen12_ctrl_surf_copy_data *data)
 		 cmd[4], data->dw04.dst_address_hi, data->dw04.dst_mocs);
 }
 
+/**
+ * blt_ctrl_surf_copy_init:
+ * @fd: drm fd
+ * @surf: structure for initialization
+ *
+ * Function is zeroing @surf and sets fd and driver fields (INTEL_DRIVER_I915 or
+ * INTEL_DRIVER_XE).
+ */
+void blt_ctrl_surf_copy_init(int fd, struct blt_ctrl_surf_copy_data *surf)
+{
+	memset(surf, 0, sizeof(*surf));
+
+	surf->fd = fd;
+	surf->driver = get_intel_driver(fd);
+}
+
 /**
  * emit_blt_ctrl_surf_copy:
  * @fd: drm fd
@@ -1317,7 +1349,7 @@ void blt_set_batch(struct blt_copy_batch *batch,
 }
 
 struct blt_copy_object *
-blt_create_object(int fd, uint32_t region,
+blt_create_object(const struct blt_copy_data *blt, uint32_t region,
 		  uint32_t width, uint32_t height, uint32_t bpp, uint8_t mocs,
 		  enum blt_tiling_type tiling,
 		  enum blt_compression compression,
@@ -1329,10 +1361,12 @@ blt_create_object(int fd, uint32_t region,
 	uint32_t stride = tiling == T_LINEAR ? width * 4 : width;
 	uint32_t handle;
 
+	igt_assert_f(blt->driver, "Driver isn't set, have you called blt_copy_init()?\n");
+
 	obj = calloc(1, sizeof(*obj));
 
 	obj->size = size;
-	igt_assert(__gem_create_in_memory_regions(fd, &handle,
+	igt_assert(__gem_create_in_memory_regions(blt->fd, &handle,
 						  &size, region) == 0);
 
 	blt_set_object(obj, handle, size, region, mocs, tiling,
@@ -1340,7 +1374,7 @@ blt_create_object(int fd, uint32_t region,
 	blt_set_geom(obj, stride, 0, 0, width, height, 0, 0);
 
 	if (create_mapping)
-		obj->ptr = gem_mmap__device_coherent(fd, handle, 0, size,
+		obj->ptr = gem_mmap__device_coherent(blt->fd, handle, 0, size,
 						     PROT_READ | PROT_WRITE);
 
 	return obj;
diff --git a/lib/intel_blt.h b/lib/intel_blt.h
index 9c4ddc7a89..7516ce8ac7 100644
--- a/lib/intel_blt.h
+++ b/lib/intel_blt.h
@@ -102,6 +102,7 @@ struct blt_copy_batch {
 /* Common for block-copy and fast-copy */
 struct blt_copy_data {
 	int fd;
+	enum intel_driver driver;
 	struct blt_copy_object src;
 	struct blt_copy_object dst;
 	struct blt_copy_batch bb;
@@ -155,6 +156,7 @@ struct blt_ctrl_surf_copy_object {
 
 struct blt_ctrl_surf_copy_data {
 	int fd;
+	enum intel_driver driver;
 	struct blt_ctrl_surf_copy_object src;
 	struct blt_ctrl_surf_copy_object dst;
 	struct blt_copy_batch bb;
@@ -185,6 +187,8 @@ bool blt_uses_extended_block_copy(int fd);
 
 const char *blt_tiling_name(enum blt_tiling_type tiling);
 
+void blt_copy_init(int fd, struct blt_copy_data *blt);
+
 uint64_t emit_blt_block_copy(int fd,
 			     uint64_t ahnd,
 			     const struct blt_copy_data *blt,
@@ -205,6 +209,8 @@ uint64_t emit_blt_ctrl_surf_copy(int fd,
 				 uint64_t bb_pos,
 				 bool emit_bbe);
 
+void blt_ctrl_surf_copy_init(int fd, struct blt_ctrl_surf_copy_data *surf);
+
 int blt_ctrl_surf_copy(int fd,
 		       const intel_ctx_t *ctx,
 		       const struct intel_execution_engine2 *e,
@@ -230,7 +236,7 @@ void blt_set_batch(struct blt_copy_batch *batch,
 		   uint32_t handle, uint64_t size, uint32_t region);
 
 struct blt_copy_object *
-blt_create_object(int fd, uint32_t region,
+blt_create_object(const struct blt_copy_data *blt, uint32_t region,
 		  uint32_t width, uint32_t height, uint32_t bpp, uint8_t mocs,
 		  enum blt_tiling_type tiling,
 		  enum blt_compression compression,
diff --git a/tests/i915/gem_ccs.c b/tests/i915/gem_ccs.c
index f9ad9267df..d9d785ed9b 100644
--- a/tests/i915/gem_ccs.c
+++ b/tests/i915/gem_ccs.c
@@ -167,7 +167,7 @@ static void surf_copy(int i915,
 	ccs = gem_create(i915, ccssize);
 	ccs2 = gem_create(i915, ccssize);
 
-	surf.fd = i915;
+	blt_ctrl_surf_copy_init(i915, &surf);
 	surf.print_bb = param.print_bb;
 	set_surf_object(&surf.src, mid->handle, mid->region, mid->size,
 			uc_mocs, BLT_INDIRECT_ACCESS);
@@ -219,7 +219,7 @@ static void surf_copy(int i915,
 			uc_mocs, INDIRECT_ACCESS);
 	blt_ctrl_surf_copy(i915, ctx, e, ahnd, &surf);
 
-	memset(&blt, 0, sizeof(blt));
+	blt_copy_init(i915, &blt);
 	blt.color_depth = CD_32bit;
 	blt.print_bb = param.print_bb;
 	blt_set_copy_object(&blt.src, mid);
@@ -310,7 +310,7 @@ static int blt_block_copy3(int i915,
 	bb_offset = get_offset(ahnd, blt3->bb.handle, blt3->bb.size, alignment);
 
 	/* First blit src -> mid */
-	memset(&blt0, 0, sizeof(blt0));
+	blt_copy_init(i915, &blt0);
 	blt0.src = blt3->src;
 	blt0.dst = blt3->mid;
 	blt0.bb = blt3->bb;
@@ -321,7 +321,7 @@ static int blt_block_copy3(int i915,
 	bb_pos = emit_blt_block_copy(i915, ahnd, &blt0, &ext0, bb_pos, false);
 
 	/* Second blit mid -> dst */
-	memset(&blt0, 0, sizeof(blt0));
+	blt_copy_init(i915, &blt0);
 	blt0.src = blt3->mid;
 	blt0.dst = blt3->dst;
 	blt0.bb = blt3->bb;
@@ -332,7 +332,7 @@ static int blt_block_copy3(int i915,
 	bb_pos = emit_blt_block_copy(i915, ahnd, &blt0, &ext0, bb_pos, false);
 
 	/* Third blit dst -> final */
-	memset(&blt0, 0, sizeof(blt0));
+	blt_copy_init(i915, &blt0);
 	blt0.src = blt3->dst;
 	blt0.dst = blt3->final;
 	blt0.bb = blt3->bb;
@@ -390,11 +390,13 @@ static void block_copy(int i915,
 	if (!blt_uses_extended_block_copy(i915))
 		pext = NULL;
 
-	src = blt_create_object(i915, region1, width, height, bpp, uc_mocs,
+	blt_copy_init(i915, &blt);
+
+	src = blt_create_object(&blt, region1, width, height, bpp, uc_mocs,
 				T_LINEAR, COMPRESSION_DISABLED, comp_type, true);
-	mid = blt_create_object(i915, mid_region, width, height, bpp, uc_mocs,
+	mid = blt_create_object(&blt, mid_region, width, height, bpp, uc_mocs,
 				mid_tiling, mid_compression, comp_type, true);
-	dst = blt_create_object(i915, region1, width, height, bpp, uc_mocs,
+	dst = blt_create_object(&blt, region1, width, height, bpp, uc_mocs,
 				T_LINEAR, COMPRESSION_DISABLED, comp_type, true);
 	igt_assert(src->size == dst->size);
 	PRINT_SURFACE_INFO("src", src);
@@ -404,7 +406,6 @@ static void block_copy(int i915,
 	blt_surface_fill_rect(i915, src, width, height);
 	WRITE_PNG(i915, run_id, "src", src, width, height);
 
-	memset(&blt, 0, sizeof(blt));
 	blt.color_depth = CD_32bit;
 	blt.print_bb = param.print_bb;
 	blt_set_copy_object(&blt.src, src);
@@ -449,7 +450,7 @@ static void block_copy(int i915,
 		}
 	}
 
-	memset(&blt, 0, sizeof(blt));
+	blt_copy_init(i915, &blt);
 	blt.color_depth = CD_32bit;
 	blt.print_bb = param.print_bb;
 	blt_set_copy_object(&blt.src, mid);
@@ -486,6 +487,7 @@ static void block_multicopy(int i915,
 			    const struct test_config *config)
 {
 	struct blt_copy3_data blt3 = {};
+	struct blt_copy_data blt = {};
 	struct blt_block_copy3_data_ext ext3 = {}, *pext3 = &ext3;
 	struct blt_copy_object *src, *mid, *dst, *final;
 	const uint32_t bpp = 32;
@@ -505,13 +507,16 @@ static void block_multicopy(int i915,
 	if (!blt_uses_extended_block_copy(i915))
 		pext3 = NULL;
 
-	src = blt_create_object(i915, region1, width, height, bpp, uc_mocs,
+	/* For object creation */
+	blt_copy_init(i915, &blt);
+
+	src = blt_create_object(&blt, region1, width, height, bpp, uc_mocs,
 				T_LINEAR, COMPRESSION_DISABLED, comp_type, true);
-	mid = blt_create_object(i915, mid_region, width, height, bpp, uc_mocs,
+	mid = blt_create_object(&blt, mid_region, width, height, bpp, uc_mocs,
 				mid_tiling, mid_compression, comp_type, true);
-	dst = blt_create_object(i915, region1, width, height, bpp, uc_mocs,
+	dst = blt_create_object(&blt, region1, width, height, bpp, uc_mocs,
 				mid_tiling, COMPRESSION_DISABLED, comp_type, true);
-	final = blt_create_object(i915, region1, width, height, bpp, uc_mocs,
+	final = blt_create_object(&blt, region1, width, height, bpp, uc_mocs,
 				  T_LINEAR, COMPRESSION_DISABLED, comp_type, true);
 	igt_assert(src->size == dst->size);
 	PRINT_SURFACE_INFO("src", src);
@@ -521,7 +526,6 @@ static void block_multicopy(int i915,
 
 	blt_surface_fill_rect(i915, src, width, height);
 
-	memset(&blt3, 0, sizeof(blt3));
 	blt3.color_depth = CD_32bit;
 	blt3.print_bb = param.print_bb;
 	blt_set_copy_object(&blt3.src, src);
diff --git a/tests/i915/gem_exercise_blt.c b/tests/i915/gem_exercise_blt.c
index 0cd1820430..7355eabbe9 100644
--- a/tests/i915/gem_exercise_blt.c
+++ b/tests/i915/gem_exercise_blt.c
@@ -89,7 +89,7 @@ static int fast_copy_one_bb(int i915,
 	bb_offset = get_offset(ahnd, blt->bb.handle, blt->bb.size, alignment);
 
 	/* First blit */
-	memset(&blt_tmp, 0, sizeof(blt_tmp));
+	blt_copy_init(i915, &blt_tmp);
 	blt_tmp.src = blt->src;
 	blt_tmp.dst = blt->mid;
 	blt_tmp.bb = blt->bb;
@@ -98,7 +98,7 @@ static int fast_copy_one_bb(int i915,
 	bb_pos = emit_blt_fast_copy(i915, ahnd, &blt_tmp, bb_pos, false);
 
 	/* Second blit */
-	memset(&blt_tmp, 0, sizeof(blt_tmp));
+	blt_copy_init(i915, &blt_tmp);
 	blt_tmp.src = blt->mid;
 	blt_tmp.dst = blt->dst;
 	blt_tmp.bb = blt->bb;
@@ -140,6 +140,7 @@ static void fast_copy_emit(int i915, const intel_ctx_t *ctx,
 			   uint32_t region1, uint32_t region2,
 			   enum blt_tiling_type mid_tiling)
 {
+	struct blt_copy_data bltinit = {};
 	struct blt_fast_copy_data blt = {};
 	struct blt_copy_object *src, *mid, *dst;
 	const uint32_t bpp = 32;
@@ -152,11 +153,12 @@ static void fast_copy_emit(int i915, const intel_ctx_t *ctx,
 
 	igt_assert(__gem_create_in_memory_regions(i915, &bb, &bb_size, region1) == 0);
 
-	src = blt_create_object(i915, region1, width, height, bpp, 0,
+	blt_copy_init(i915, &bltinit);
+	src = blt_create_object(&bltinit, region1, width, height, bpp, 0,
 				T_LINEAR, COMPRESSION_DISABLED, 0, true);
-	mid = blt_create_object(i915, region2, width, height, bpp, 0,
+	mid = blt_create_object(&bltinit, region2, width, height, bpp, 0,
 				mid_tiling, COMPRESSION_DISABLED, 0, true);
-	dst = blt_create_object(i915, region1, width, height, bpp, 0,
+	dst = blt_create_object(&bltinit, region1, width, height, bpp, 0,
 				T_LINEAR, COMPRESSION_DISABLED, 0, true);
 	igt_assert(src->size == dst->size);
 
@@ -212,17 +214,17 @@ static void fast_copy(int i915, const intel_ctx_t *ctx,
 
 	igt_assert(__gem_create_in_memory_regions(i915, &bb, &bb_size, region1) == 0);
 
-	src = blt_create_object(i915, region1, width, height, bpp, 0,
+	blt_copy_init(i915, &blt);
+	src = blt_create_object(&blt, region1, width, height, bpp, 0,
 				T_LINEAR, COMPRESSION_DISABLED, 0, true);
-	mid = blt_create_object(i915, region2, width, height, bpp, 0,
+	mid = blt_create_object(&blt, region2, width, height, bpp, 0,
 				mid_tiling, COMPRESSION_DISABLED, 0, true);
-	dst = blt_create_object(i915, region1, width, height, bpp, 0,
+	dst = blt_create_object(&blt, region1, width, height, bpp, 0,
 				T_LINEAR, COMPRESSION_DISABLED, 0, true);
 	igt_assert(src->size == dst->size);
 
 	blt_surface_fill_rect(i915, src, width, height);
 
-	memset(&blt, 0, sizeof(blt));
 	blt.color_depth = CD_32bit;
 	blt.print_bb = param.print_bb;
 	blt_set_copy_object(&blt.src, src);
@@ -235,7 +237,7 @@ static void fast_copy(int i915, const intel_ctx_t *ctx,
 	WRITE_PNG(i915, mid_tiling, "src", &blt.src, width, height);
 	WRITE_PNG(i915, mid_tiling, "mid", &blt.dst, width, height);
 
-	memset(&blt, 0, sizeof(blt));
+	blt_copy_init(i915, &blt);
 	blt.color_depth = CD_32bit;
 	blt.print_bb = param.print_bb;
 	blt_set_copy_object(&blt.src, mid);
diff --git a/tests/i915/gem_lmem_swapping.c b/tests/i915/gem_lmem_swapping.c
index 83dbebec83..2921de8f9f 100644
--- a/tests/i915/gem_lmem_swapping.c
+++ b/tests/i915/gem_lmem_swapping.c
@@ -308,7 +308,7 @@ init_object_ccs(int i915, struct object *obj, struct blt_copy_object *tmp,
 		buf[j] = seed++;
 	munmap(buf, obj->size);
 
-	memset(&blt, 0, sizeof(blt));
+	blt_copy_init(i915, &blt);
 	blt.color_depth = CD_32bit;
 
 	memcpy(&blt.src, tmp, sizeof(blt.src));
@@ -366,7 +366,7 @@ verify_object_ccs(int i915, const struct object *obj,
 	cmd->handle = gem_create_from_pool(i915, &size, region);
 	blt_set_batch(cmd, cmd->handle, size, region);
 
-	memset(&blt, 0, sizeof(blt));
+	blt_copy_init(i915, &blt);
 	blt.color_depth = CD_32bit;
 
 	memcpy(&blt.src, obj->blt_obj, sizeof(blt.src));
-- 
2.34.1

[igt-dev] [PATCH i-g-t 10/12] lib/intel_blt: Extend blitter library to support xe driver

[Zbigniew Kempczyński]

Use already written for i915 blitter library in xe development.
Add appropriate code paths which are unique for those drivers.

Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 lib/intel_blt.c | 257 ++++++++++++++++++++++++++++++++----------------
 lib/intel_blt.h |   2 +-
 2 files changed, 171 insertions(+), 88 deletions(-)

diff --git a/lib/intel_blt.c b/lib/intel_blt.c
index f2f86e4947..43fda14583 100644
--- a/lib/intel_blt.c
+++ b/lib/intel_blt.c
@@ -9,9 +9,13 @@
 #include <malloc.h>
 #include <cairo.h>
 #include "drm.h"
-#include "igt.h"
 #include "i915/gem_create.h"
+#include "igt.h"
+#include "igt_syncobj.h"
 #include "intel_blt.h"
+#include "xe/xe_ioctl.h"
+#include "xe/xe_query.h"
+#include "xe/xe_util.h"
 
 #define BITRANGE(start, end) (end - start + 1)
 #define GET_CMDS_INFO(__fd) intel_get_cmds_info(intel_get_drm_devid(__fd))
@@ -468,24 +472,41 @@ static int __special_mode(const struct blt_copy_data *blt)
 	return SM_NONE;
 }
 
-static int __memory_type(uint32_t region)
+static int __memory_type(int fd, enum intel_driver driver, uint32_t region)
 {
-	igt_assert_f(IS_DEVICE_MEMORY_REGION(region) ||
-		     IS_SYSTEM_MEMORY_REGION(region),
-		     "Invalid region: %x\n", region);
+	if (driver == INTEL_DRIVER_I915) {
+		igt_assert_f(IS_DEVICE_MEMORY_REGION(region) ||
+			     IS_SYSTEM_MEMORY_REGION(region),
+			     "Invalid region: %x\n", region);
+	} else {
+		igt_assert_f(XE_IS_VRAM_MEMORY_REGION(fd, region) ||
+			     XE_IS_SYSMEM_MEMORY_REGION(fd, region),
+			     "Invalid region: %x\n", region);
+	}
 
-	if (IS_DEVICE_MEMORY_REGION(region))
+	if (driver == INTEL_DRIVER_I915 && IS_DEVICE_MEMORY_REGION(region))
 		return TM_LOCAL_MEM;
+	else if (driver == INTEL_DRIVER_XE && XE_IS_VRAM_MEMORY_REGION(fd, region))
+		return TM_LOCAL_MEM;
+
 	return TM_SYSTEM_MEM;
 }
 
-static enum blt_aux_mode __aux_mode(const struct blt_copy_object *obj)
+static enum blt_aux_mode __aux_mode(int fd,
+				    enum intel_driver driver,
+				    const struct blt_copy_object *obj)
 {
-	if (obj->compression == COMPRESSION_ENABLED) {
+
+	if (driver == INTEL_DRIVER_I915 && obj->compression == COMPRESSION_ENABLED) {
 		igt_assert_f(IS_DEVICE_MEMORY_REGION(obj->region),
 			     "XY_BLOCK_COPY_BLT supports compression "
 			     "on device memory only\n");
 		return AM_AUX_CCS_E;
+	} else if (driver == INTEL_DRIVER_XE && obj->compression == COMPRESSION_ENABLED) {
+		igt_assert_f(XE_IS_VRAM_MEMORY_REGION(fd, obj->region),
+			     "XY_BLOCK_COPY_BLT supports compression "
+			     "on device memory only\n");
+		return AM_AUX_CCS_E;
 	}
 
 	return AM_AUX_NONE;
@@ -508,9 +529,9 @@ static void fill_data(struct gen12_block_copy_data *data,
 	data->dw00.length = extended_command ? 20 : 10;
 
 	if (__special_mode(blt) == SM_FULL_RESOLVE)
-		data->dw01.dst_aux_mode = __aux_mode(&blt->src);
+		data->dw01.dst_aux_mode = __aux_mode(blt->fd, blt->driver, &blt->src);
 	else
-		data->dw01.dst_aux_mode = __aux_mode(&blt->dst);
+		data->dw01.dst_aux_mode = __aux_mode(blt->fd, blt->driver, &blt->dst);
 	data->dw01.dst_pitch = blt->dst.pitch - 1;
 
 	data->dw01.dst_mocs = blt->dst.mocs;
@@ -531,13 +552,13 @@ static void fill_data(struct gen12_block_copy_data *data,
 
 	data->dw06.dst_x_offset = blt->dst.x_offset;
 	data->dw06.dst_y_offset = blt->dst.y_offset;
-	data->dw06.dst_target_memory = __memory_type(blt->dst.region);
+	data->dw06.dst_target_memory = __memory_type(blt->fd, blt->driver, blt->dst.region);
 
 	data->dw07.src_x1 = blt->src.x1;
 	data->dw07.src_y1 = blt->src.y1;
 
 	data->dw08.src_pitch = blt->src.pitch - 1;
-	data->dw08.src_aux_mode = __aux_mode(&blt->src);
+	data->dw08.src_aux_mode = __aux_mode(blt->fd, blt->driver, &blt->src);
 	data->dw08.src_mocs = blt->src.mocs;
 	data->dw08.src_compression = blt->src.compression;
 	data->dw08.src_tiling = __block_tiling(blt->src.tiling);
@@ -550,7 +571,7 @@ static void fill_data(struct gen12_block_copy_data *data,
 
 	data->dw11.src_x_offset = blt->src.x_offset;
 	data->dw11.src_y_offset = blt->src.y_offset;
-	data->dw11.src_target_memory = __memory_type(blt->src.region);
+	data->dw11.src_target_memory = __memory_type(blt->fd, blt->driver, blt->src.region);
 }
 
 static void fill_data_ext(struct gen12_block_copy_data_ext *dext,
@@ -739,7 +760,10 @@ uint64_t emit_blt_block_copy(int fd,
 	igt_assert_f(ahnd, "block-copy supports softpin only\n");
 	igt_assert_f(blt, "block-copy requires data to do blit\n");
 
-	alignment = gem_detect_safe_alignment(fd);
+	if (blt->driver == INTEL_DRIVER_XE)
+		alignment = xe_get_default_alignment(fd);
+	else
+		alignment = gem_detect_safe_alignment(fd);
 	src_offset = get_offset(ahnd, blt->src.handle, blt->src.size, alignment)
 		     + blt->src.plane_offset;
 	dst_offset = get_offset(ahnd, blt->dst.handle, blt->dst.size, alignment)
@@ -748,8 +772,11 @@ uint64_t emit_blt_block_copy(int fd,
 
 	fill_data(&data, blt, src_offset, dst_offset, ext);
 
-	bb = gem_mmap__device_coherent(fd, blt->bb.handle, 0, blt->bb.size,
-				       PROT_READ | PROT_WRITE);
+	if (blt->driver == INTEL_DRIVER_XE)
+		bb = xe_bo_map(fd, blt->bb.handle, blt->bb.size);
+	else
+		bb = gem_mmap__device_coherent(fd, blt->bb.handle, 0, blt->bb.size,
+					       PROT_READ | PROT_WRITE);
 
 	igt_assert(bb_pos + sizeof(data) < blt->bb.size);
 	memcpy(bb + bb_pos, &data, sizeof(data));
@@ -812,29 +839,38 @@ int blt_block_copy(int fd,
 
 	igt_assert_f(ahnd, "block-copy supports softpin only\n");
 	igt_assert_f(blt, "block-copy requires data to do blit\n");
+	igt_assert_neq(blt->driver, 0);
 
-	alignment = gem_detect_safe_alignment(fd);
+	if (blt->driver == INTEL_DRIVER_XE)
+		alignment = xe_get_default_alignment(fd);
+	else
+		alignment = gem_detect_safe_alignment(fd);
 	src_offset = get_offset(ahnd, blt->src.handle, blt->src.size, alignment);
 	dst_offset = get_offset(ahnd, blt->dst.handle, blt->dst.size, alignment);
 	bb_offset = get_offset(ahnd, blt->bb.handle, blt->bb.size, alignment);
 
 	emit_blt_block_copy(fd, ahnd, blt, ext, 0, true);
 
-	obj[0].offset = CANONICAL(dst_offset);
-	obj[1].offset = CANONICAL(src_offset);
-	obj[2].offset = CANONICAL(bb_offset);
-	obj[0].handle = blt->dst.handle;
-	obj[1].handle = blt->src.handle;
-	obj[2].handle = blt->bb.handle;
-	obj[0].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_WRITE |
-		       EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
-	obj[1].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
-	obj[2].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
-	execbuf.buffer_count = 3;
-	execbuf.buffers_ptr = to_user_pointer(obj);
-	execbuf.rsvd1 = ctx ? ctx->id : 0;
-	execbuf.flags = e ? e->flags : I915_EXEC_BLT;
-	ret = __gem_execbuf(fd, &execbuf);
+	if (blt->driver == INTEL_DRIVER_XE) {
+		intel_ctx_xe_exec(ctx, ahnd, CANONICAL(bb_offset));
+	} else {
+		obj[0].offset = CANONICAL(dst_offset);
+		obj[1].offset = CANONICAL(src_offset);
+		obj[2].offset = CANONICAL(bb_offset);
+		obj[0].handle = blt->dst.handle;
+		obj[1].handle = blt->src.handle;
+		obj[2].handle = blt->bb.handle;
+		obj[0].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_WRITE |
+				EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
+		obj[1].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
+		obj[2].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
+		execbuf.buffer_count = 3;
+		execbuf.buffers_ptr = to_user_pointer(obj);
+		execbuf.rsvd1 = ctx ? ctx->id : 0;
+		execbuf.flags = e ? e->flags : I915_EXEC_BLT;
+
+		ret = __gem_execbuf(fd, &execbuf);
+	}
 
 	return ret;
 }
@@ -950,7 +986,10 @@ uint64_t emit_blt_ctrl_surf_copy(int fd,
 	igt_assert_f(ahnd, "ctrl-surf-copy supports softpin only\n");
 	igt_assert_f(surf, "ctrl-surf-copy requires data to do ctrl-surf-copy blit\n");
 
-	alignment = max_t(uint64_t, gem_detect_safe_alignment(fd), 1ull << 16);
+	if (surf->driver == INTEL_DRIVER_XE)
+		alignment = max_t(uint64_t, xe_get_default_alignment(fd), 1ull << 16);
+	else
+		alignment = max_t(uint64_t, gem_detect_safe_alignment(fd), 1ull << 16);
 
 	data.dw00.client = 0x2;
 	data.dw00.opcode = 0x48;
@@ -973,8 +1012,11 @@ uint64_t emit_blt_ctrl_surf_copy(int fd,
 	data.dw04.dst_address_hi = dst_offset >> 32;
 	data.dw04.dst_mocs = surf->dst.mocs;
 
-	bb = gem_mmap__device_coherent(fd, surf->bb.handle, 0, surf->bb.size,
-				       PROT_READ | PROT_WRITE);
+	if (surf->driver == INTEL_DRIVER_XE)
+		bb = xe_bo_map(fd, surf->bb.handle, surf->bb.size);
+	else
+		bb = gem_mmap__device_coherent(fd, surf->bb.handle, 0, surf->bb.size,
+					       PROT_READ | PROT_WRITE);
 
 	igt_assert(bb_pos + sizeof(data) < surf->bb.size);
 	memcpy(bb + bb_pos, &data, sizeof(data));
@@ -1002,7 +1044,7 @@ uint64_t emit_blt_ctrl_surf_copy(int fd,
 
 /**
  * blt_ctrl_surf_copy:
- * @fd: drm fd
+ * @blt: bldrm fd
  * @ctx: intel_ctx_t context
  * @e: blitter engine for @ctx
  * @ahnd: allocator handle
@@ -1026,32 +1068,41 @@ int blt_ctrl_surf_copy(int fd,
 
 	igt_assert_f(ahnd, "ctrl-surf-copy supports softpin only\n");
 	igt_assert_f(surf, "ctrl-surf-copy requires data to do ctrl-surf-copy blit\n");
+	igt_assert_neq(surf->driver, 0);
+
+	if (surf->driver == INTEL_DRIVER_XE)
+		alignment = max_t(uint64_t, xe_get_default_alignment(fd), 1ull << 16);
+	else
+		alignment = max_t(uint64_t, gem_detect_safe_alignment(fd), 1ull << 16);
 
-	alignment = max_t(uint64_t, gem_detect_safe_alignment(fd), 1ull << 16);
 	src_offset = get_offset(ahnd, surf->src.handle, surf->src.size, alignment);
 	dst_offset = get_offset(ahnd, surf->dst.handle, surf->dst.size, alignment);
 	bb_offset = get_offset(ahnd, surf->bb.handle, surf->bb.size, alignment);
 
 	emit_blt_ctrl_surf_copy(fd, ahnd, surf, 0, true);
 
-	obj[0].offset = CANONICAL(dst_offset);
-	obj[1].offset = CANONICAL(src_offset);
-	obj[2].offset = CANONICAL(bb_offset);
-	obj[0].handle = surf->dst.handle;
-	obj[1].handle = surf->src.handle;
-	obj[2].handle = surf->bb.handle;
-	obj[0].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_WRITE |
-		       EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
-	obj[1].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
-	obj[2].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
-	execbuf.buffer_count = 3;
-	execbuf.buffers_ptr = to_user_pointer(obj);
-	execbuf.flags = e ? e->flags : I915_EXEC_BLT;
-	execbuf.rsvd1 = ctx ? ctx->id : 0;
-	gem_execbuf(fd, &execbuf);
-	put_offset(ahnd, surf->dst.handle);
-	put_offset(ahnd, surf->src.handle);
-	put_offset(ahnd, surf->bb.handle);
+	if (surf->driver == INTEL_DRIVER_XE) {
+		intel_ctx_xe_exec(ctx, ahnd, CANONICAL(bb_offset));
+	} else {
+		obj[0].offset = CANONICAL(dst_offset);
+		obj[1].offset = CANONICAL(src_offset);
+		obj[2].offset = CANONICAL(bb_offset);
+		obj[0].handle = surf->dst.handle;
+		obj[1].handle = surf->src.handle;
+		obj[2].handle = surf->bb.handle;
+		obj[0].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_WRITE |
+				EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
+		obj[1].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
+		obj[2].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
+		execbuf.buffer_count = 3;
+		execbuf.buffers_ptr = to_user_pointer(obj);
+		execbuf.flags = e ? e->flags : I915_EXEC_BLT;
+		execbuf.rsvd1 = ctx ? ctx->id : 0;
+		gem_execbuf(fd, &execbuf);
+		put_offset(ahnd, surf->dst.handle);
+		put_offset(ahnd, surf->src.handle);
+		put_offset(ahnd, surf->bb.handle);
+	}
 
 	return 0;
 }
@@ -1208,7 +1259,10 @@ uint64_t emit_blt_fast_copy(int fd,
 	uint32_t bbe = MI_BATCH_BUFFER_END;
 	uint32_t *bb;
 
-	alignment = gem_detect_safe_alignment(fd);
+	if (blt->driver == INTEL_DRIVER_XE)
+		alignment = xe_get_default_alignment(fd);
+	else
+		alignment = gem_detect_safe_alignment(fd);
 
 	data.dw00.client = 0x2;
 	data.dw00.opcode = 0x42;
@@ -1218,8 +1272,8 @@ uint64_t emit_blt_fast_copy(int fd,
 
 	data.dw01.dst_pitch = blt->dst.pitch;
 	data.dw01.color_depth = __fast_color_depth(blt->color_depth);
-	data.dw01.dst_memory = __memory_type(blt->dst.region);
-	data.dw01.src_memory = __memory_type(blt->src.region);
+	data.dw01.dst_memory = __memory_type(blt->fd, blt->driver, blt->dst.region);
+	data.dw01.src_memory = __memory_type(blt->fd, blt->driver, blt->src.region);
 	data.dw01.dst_type_y = __new_tile_y_type(blt->dst.tiling) ? 1 : 0;
 	data.dw01.src_type_y = __new_tile_y_type(blt->src.tiling) ? 1 : 0;
 
@@ -1246,8 +1300,11 @@ uint64_t emit_blt_fast_copy(int fd,
 	data.dw08.src_address_lo = src_offset;
 	data.dw09.src_address_hi = src_offset >> 32;
 
-	bb = gem_mmap__device_coherent(fd, blt->bb.handle, 0, blt->bb.size,
-				       PROT_READ | PROT_WRITE);
+	if (blt->driver == INTEL_DRIVER_XE)
+		bb = xe_bo_map(fd, blt->bb.handle, blt->bb.size);
+	else
+		bb = gem_mmap__device_coherent(fd, blt->bb.handle, 0, blt->bb.size,
+					       PROT_READ | PROT_WRITE);
 
 	igt_assert(bb_pos + sizeof(data) < blt->bb.size);
 	memcpy(bb + bb_pos, &data, sizeof(data));
@@ -1297,7 +1354,14 @@ int blt_fast_copy(int fd,
 	uint64_t dst_offset, src_offset, bb_offset, alignment;
 	int ret;
 
-	alignment = gem_detect_safe_alignment(fd);
+	igt_assert_f(ahnd, "fast-copy supports softpin only\n");
+	igt_assert_f(blt, "fast-copy requires data to do fast-copy blit\n");
+	igt_assert_neq(blt->driver, 0);
+
+	if (blt->driver == INTEL_DRIVER_XE)
+		alignment = xe_get_default_alignment(fd);
+	else
+		alignment = gem_detect_safe_alignment(fd);
 
 	src_offset = get_offset(ahnd, blt->src.handle, blt->src.size, alignment);
 	dst_offset = get_offset(ahnd, blt->dst.handle, blt->dst.size, alignment);
@@ -1305,24 +1369,28 @@ int blt_fast_copy(int fd,
 
 	emit_blt_fast_copy(fd, ahnd, blt, 0, true);
 
-	obj[0].offset = CANONICAL(dst_offset);
-	obj[1].offset = CANONICAL(src_offset);
-	obj[2].offset = CANONICAL(bb_offset);
-	obj[0].handle = blt->dst.handle;
-	obj[1].handle = blt->src.handle;
-	obj[2].handle = blt->bb.handle;
-	obj[0].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_WRITE |
-		       EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
-	obj[1].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
-	obj[2].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
-	execbuf.buffer_count = 3;
-	execbuf.buffers_ptr = to_user_pointer(obj);
-	execbuf.rsvd1 = ctx ? ctx->id : 0;
-	execbuf.flags = e ? e->flags : I915_EXEC_BLT;
-	ret = __gem_execbuf(fd, &execbuf);
-	put_offset(ahnd, blt->dst.handle);
-	put_offset(ahnd, blt->src.handle);
-	put_offset(ahnd, blt->bb.handle);
+	if (blt->driver == INTEL_DRIVER_XE) {
+		intel_ctx_xe_exec(ctx, ahnd, CANONICAL(bb_offset));
+	} else {
+		obj[0].offset = CANONICAL(dst_offset);
+		obj[1].offset = CANONICAL(src_offset);
+		obj[2].offset = CANONICAL(bb_offset);
+		obj[0].handle = blt->dst.handle;
+		obj[1].handle = blt->src.handle;
+		obj[2].handle = blt->bb.handle;
+		obj[0].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_WRITE |
+				EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
+		obj[1].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
+		obj[2].flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
+		execbuf.buffer_count = 3;
+		execbuf.buffers_ptr = to_user_pointer(obj);
+		execbuf.rsvd1 = ctx ? ctx->id : 0;
+		execbuf.flags = e ? e->flags : I915_EXEC_BLT;
+		ret = __gem_execbuf(fd, &execbuf);
+		put_offset(ahnd, blt->dst.handle);
+		put_offset(ahnd, blt->src.handle);
+		put_offset(ahnd, blt->bb.handle);
+	}
 
 	return ret;
 }
@@ -1366,16 +1434,26 @@ blt_create_object(const struct blt_copy_data *blt, uint32_t region,
 	obj = calloc(1, sizeof(*obj));
 
 	obj->size = size;
-	igt_assert(__gem_create_in_memory_regions(blt->fd, &handle,
-						  &size, region) == 0);
+
+	if (blt->driver == INTEL_DRIVER_XE) {
+		size = ALIGN(size, xe_get_default_alignment(blt->fd));
+		handle = xe_bo_create_flags(blt->fd, 0, size, region);
+	} else {
+		igt_assert(__gem_create_in_memory_regions(blt->fd, &handle,
+							  &size, region) == 0);
+	}
 
 	blt_set_object(obj, handle, size, region, mocs, tiling,
 		       compression, compression_type);
 	blt_set_geom(obj, stride, 0, 0, width, height, 0, 0);
 
-	if (create_mapping)
-		obj->ptr = gem_mmap__device_coherent(blt->fd, handle, 0, size,
-						     PROT_READ | PROT_WRITE);
+	if (create_mapping) {
+		if (blt->driver == INTEL_DRIVER_XE)
+			obj->ptr = xe_bo_map(blt->fd, handle, size);
+		else
+			obj->ptr = gem_mmap__device_coherent(blt->fd, handle, 0, size,
+							     PROT_READ | PROT_WRITE);
+	}
 
 	return obj;
 }
@@ -1518,14 +1596,19 @@ void blt_surface_to_png(int fd, uint32_t run_id, const char *fileid,
 	int format;
 	int stride = obj->tiling ? obj->pitch * 4 : obj->pitch;
 	char filename[FILENAME_MAX];
+	bool is_xe;
 
 	snprintf(filename, FILENAME_MAX-1, "%d-%s-%s-%ux%u-%s.png",
 		 run_id, fileid, blt_tiling_name(obj->tiling), width, height,
 		 obj->compression ? "compressed" : "uncompressed");
 
-	if (!map)
-		map = gem_mmap__device_coherent(fd, obj->handle, 0,
-						obj->size, PROT_READ);
+	if (!map) {
+		if (is_xe)
+			map = xe_bo_map(fd, obj->handle, obj->size);
+		else
+			map = gem_mmap__device_coherent(fd, obj->handle, 0,
+							obj->size, PROT_READ);
+	}
 	format = CAIRO_FORMAT_RGB24;
 	surface = cairo_image_surface_create_for_data(map,
 						      format, width, height,
diff --git a/lib/intel_blt.h b/lib/intel_blt.h
index 7516ce8ac7..944e2b4ae7 100644
--- a/lib/intel_blt.h
+++ b/lib/intel_blt.h
@@ -8,7 +8,7 @@
 
 /**
  * SECTION:intel_blt
- * @short_description: i915 blitter library
+ * @short_description: i915/xe blitter library
  * @title: Blitter library
  * @include: intel_blt.h
  *
-- 
2.34.1

[igt-dev] [PATCH i-g-t 11/12] tests/xe_ccs: Check if flatccs is working with block-copy for Xe

[Zbigniew Kempczyński]

This is ported to xe copy of i915 gem_ccs test. Ported means all driver
dependent calls - like working on regions, binding and execution were
replaced by xe counterparts. I wondered to add conditionals for xe
in gem_ccs but this would decrease test readability so I dropped
this idea.

Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 tests/meson.build |   1 +
 tests/xe/xe_ccs.c | 763 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 764 insertions(+)
 create mode 100644 tests/xe/xe_ccs.c

diff --git a/tests/meson.build b/tests/meson.build
index ee066b8490..9bca57a5e8 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -244,6 +244,7 @@ i915_progs = [
 ]
 
 xe_progs = [
+	'xe_ccs',
 	'xe_create',
 	'xe_compute',
 	'xe_dma_buf_sync',
diff --git a/tests/xe/xe_ccs.c b/tests/xe/xe_ccs.c
new file mode 100644
index 0000000000..e6bb29a5ed
--- /dev/null
+++ b/tests/xe/xe_ccs.c
@@ -0,0 +1,763 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#include <errno.h>
+#include <glib.h>
+#include <sys/ioctl.h>
+#include <sys/time.h>
+#include <malloc.h>
+#include "drm.h"
+#include "igt.h"
+#include "igt_syncobj.h"
+#include "intel_blt.h"
+#include "intel_mocs.h"
+#include "xe/xe_ioctl.h"
+#include "xe/xe_query.h"
+#include "xe/xe_util.h"
+/**
+ * TEST: xe ccs
+ * Description: Exercise gen12 blitter with and without flatccs compression on Xe
+ * Run type: FULL
+ *
+ * SUBTEST: block-copy-compressed
+ * Description: Check block-copy flatccs compressed blit
+ *
+ * SUBTEST: block-copy-uncompressed
+ * Description: Check block-copy uncompressed blit
+ *
+ * SUBTEST: block-multicopy-compressed
+ * Description: Check block-multicopy flatccs compressed blit
+ *
+ * SUBTEST: block-multicopy-inplace
+ * Description: Check block-multicopy flatccs inplace decompression blit
+ *
+ * SUBTEST: ctrl-surf-copy
+ * Description: Check flatccs data can be copied from/to surface
+ *
+ * SUBTEST: ctrl-surf-copy-new-ctx
+ * Description: Check flatccs data are physically tagged and visible in vm
+ *
+ * SUBTEST: suspend-resume
+ * Description: Check flatccs data persists after suspend / resume (S0)
+ */
+
+IGT_TEST_DESCRIPTION("Exercise gen12 blitter with and without flatccs compression on Xe");
+
+static struct param {
+	int compression_format;
+	int tiling;
+	bool write_png;
+	bool print_bb;
+	bool print_surface_info;
+	int width;
+	int height;
+} param = {
+	.compression_format = 0,
+	.tiling = -1,
+	.write_png = false,
+	.print_bb = false,
+	.print_surface_info = false,
+	.width = 512,
+	.height = 512,
+};
+
+struct test_config {
+	bool compression;
+	bool inplace;
+	bool surfcopy;
+	bool new_ctx;
+	bool suspend_resume;
+};
+
+static void set_surf_object(struct blt_ctrl_surf_copy_object *obj,
+			    uint32_t handle, uint32_t region, uint64_t size,
+			    uint8_t mocs, enum blt_access_type access_type)
+{
+	obj->handle = handle;
+	obj->region = region;
+	obj->size = size;
+	obj->mocs = mocs;
+	obj->access_type = access_type;
+}
+
+#define PRINT_SURFACE_INFO(name, obj) do { \
+	if (param.print_surface_info) \
+		blt_surface_info((name), (obj)); } while (0)
+
+#define WRITE_PNG(fd, id, name, obj, w, h) do { \
+	if (param.write_png) \
+		blt_surface_to_png((fd), (id), (name), (obj), (w), (h)); } while (0)
+
+static int compare_nxn(const struct blt_copy_object *surf1,
+		       const struct blt_copy_object *surf2,
+		       int xsize, int ysize, int bx, int by)
+{
+	int x, y, corrupted;
+	uint32_t pos, px1, px2;
+
+	corrupted = 0;
+	for (y = 0; y < ysize; y++) {
+		for (x = 0; x < xsize; x++) {
+			pos = bx * xsize + by * ysize * surf1->pitch / 4;
+			pos += x + y * surf1->pitch / 4;
+			px1 = surf1->ptr[pos];
+			px2 = surf2->ptr[pos];
+			if (px1 != px2)
+				corrupted++;
+		}
+	}
+
+	return corrupted;
+}
+
+static void dump_corruption_info(const struct blt_copy_object *surf1,
+				 const struct blt_copy_object *surf2)
+{
+	const int xsize = 8, ysize = 8;
+	int w, h, bx, by, corrupted;
+
+	igt_assert(surf1->x1 == surf2->x1 && surf1->x2 == surf2->x2);
+	igt_assert(surf1->y1 == surf2->y1 && surf1->y2 == surf2->y2);
+	w = surf1->x2;
+	h = surf1->y2;
+
+	igt_info("dump corruption - width: %d, height: %d, sizex: %x, sizey: %x\n",
+		 surf1->x2, surf1->y2, xsize, ysize);
+
+	for (by = 0; by < h / ysize; by++) {
+		for (bx = 0; bx < w / xsize; bx++) {
+			corrupted = compare_nxn(surf1, surf2, xsize, ysize, bx, by);
+			if (corrupted == 0)
+				igt_info(".");
+			else
+				igt_info("%c", '0' + corrupted);
+		}
+		igt_info("\n");
+	}
+}
+
+static void surf_copy(int xe,
+		      intel_ctx_t *ctx,
+		      uint64_t ahnd,
+		      const struct blt_copy_object *src,
+		      const struct blt_copy_object *mid,
+		      const struct blt_copy_object *dst,
+		      int run_id, bool suspend_resume)
+{
+	struct blt_copy_data blt = {};
+	struct blt_block_copy_data_ext ext = {};
+	struct blt_ctrl_surf_copy_data surf = {};
+	uint32_t bb1, bb2, ccs, ccs2, *ccsmap, *ccsmap2;
+	uint64_t bb_size, ccssize = mid->size / CCS_RATIO;
+	uint32_t *ccscopy;
+	uint8_t uc_mocs = intel_get_uc_mocs(xe);
+	uint32_t sysmem = system_memory(xe);
+	int result;
+
+	igt_assert(mid->compression);
+	ccscopy = (uint32_t *) malloc(ccssize);
+	ccs = xe_bo_create_flags(xe, 0, ccssize, sysmem);
+	ccs2 = xe_bo_create_flags(xe, 0, ccssize, sysmem);
+
+	blt_ctrl_surf_copy_init(xe, &surf);
+	surf.print_bb = param.print_bb;
+	set_surf_object(&surf.src, mid->handle, mid->region, mid->size,
+			uc_mocs, BLT_INDIRECT_ACCESS);
+	set_surf_object(&surf.dst, ccs, sysmem, ccssize, uc_mocs, DIRECT_ACCESS);
+	bb_size = xe_get_default_alignment(xe);
+	bb1 = xe_bo_create_flags(xe, 0, bb_size, sysmem);
+	blt_set_batch(&surf.bb, bb1, bb_size, sysmem);
+	blt_ctrl_surf_copy(xe, ctx, NULL, ahnd, &surf);
+	intel_ctx_xe_sync(ctx, true);
+
+	ccsmap = xe_bo_map(xe, ccs, surf.dst.size);
+	memcpy(ccscopy, ccsmap, ccssize);
+
+	if (suspend_resume) {
+		char *orig, *orig2, *newsum, *newsum2;
+
+		orig = g_compute_checksum_for_data(G_CHECKSUM_SHA1,
+						   (void *)ccsmap, surf.dst.size);
+		orig2 = g_compute_checksum_for_data(G_CHECKSUM_SHA1,
+						    (void *)mid->ptr, mid->size);
+
+		igt_system_suspend_autoresume(SUSPEND_STATE_FREEZE, SUSPEND_TEST_NONE);
+
+		set_surf_object(&surf.dst, ccs2, REGION_SMEM, ccssize,
+				0, DIRECT_ACCESS);
+		blt_ctrl_surf_copy(xe, ctx, NULL, ahnd, &surf);
+		intel_ctx_xe_sync(ctx, true);
+
+		ccsmap2 = xe_bo_map(xe, ccs2, surf.dst.size);
+		newsum = g_compute_checksum_for_data(G_CHECKSUM_SHA1,
+						     (void *)ccsmap2, surf.dst.size);
+		newsum2 = g_compute_checksum_for_data(G_CHECKSUM_SHA1,
+						      (void *)mid->ptr, mid->size);
+
+		munmap(ccsmap2, ccssize);
+		igt_assert(!strcmp(orig, newsum));
+		igt_assert(!strcmp(orig2, newsum2));
+		g_free(orig);
+		g_free(orig2);
+		g_free(newsum);
+		g_free(newsum2);
+	}
+
+	/* corrupt ccs */
+	for (int i = 0; i < surf.dst.size / sizeof(uint32_t); i++)
+		ccsmap[i] = i;
+	set_surf_object(&surf.src, ccs, sysmem, ccssize,
+			uc_mocs, DIRECT_ACCESS);
+	set_surf_object(&surf.dst, mid->handle, mid->region, mid->size,
+			uc_mocs, INDIRECT_ACCESS);
+	blt_ctrl_surf_copy(xe, ctx, NULL, ahnd, &surf);
+	intel_ctx_xe_sync(ctx, true);
+
+	blt_copy_init(xe, &blt);
+	blt.color_depth = CD_32bit;
+	blt.print_bb = param.print_bb;
+	blt_set_copy_object(&blt.src, mid);
+	blt_set_copy_object(&blt.dst, dst);
+	blt_set_object_ext(&ext.src, mid->compression_type, mid->x2, mid->y2, SURFACE_TYPE_2D);
+	blt_set_object_ext(&ext.dst, 0, dst->x2, dst->y2, SURFACE_TYPE_2D);
+	bb2 = xe_bo_create_flags(xe, 0, bb_size, sysmem);
+	blt_set_batch(&blt.bb, bb2, bb_size, sysmem);
+	blt_block_copy(xe, ctx, NULL, ahnd, &blt, &ext);
+	intel_ctx_xe_sync(ctx, true);
+	WRITE_PNG(xe, run_id, "corrupted", &blt.dst, dst->x2, dst->y2);
+	result = memcmp(src->ptr, dst->ptr, src->size);
+	igt_assert(result != 0);
+
+	/* retrieve back ccs */
+	memcpy(ccsmap, ccscopy, ccssize);
+	blt_ctrl_surf_copy(xe, ctx, NULL, ahnd, &surf);
+
+	blt_block_copy(xe, ctx, NULL, ahnd, &blt, &ext);
+	intel_ctx_xe_sync(ctx, true);
+	WRITE_PNG(xe, run_id, "corrected", &blt.dst, dst->x2, dst->y2);
+	result = memcmp(src->ptr, dst->ptr, src->size);
+	if (result)
+		dump_corruption_info(src, dst);
+
+	munmap(ccsmap, ccssize);
+	gem_close(xe, ccs);
+	gem_close(xe, ccs2);
+	gem_close(xe, bb1);
+	gem_close(xe, bb2);
+
+	igt_assert_f(result == 0,
+		     "Source and destination surfaces are different after "
+		     "restoring source ccs data\n");
+}
+
+struct blt_copy3_data {
+	int xe;
+	struct blt_copy_object src;
+	struct blt_copy_object mid;
+	struct blt_copy_object dst;
+	struct blt_copy_object final;
+	struct blt_copy_batch bb;
+	enum blt_color_depth color_depth;
+
+	/* debug stuff */
+	bool print_bb;
+};
+
+struct blt_block_copy3_data_ext {
+	struct blt_block_copy_object_ext src;
+	struct blt_block_copy_object_ext mid;
+	struct blt_block_copy_object_ext dst;
+	struct blt_block_copy_object_ext final;
+};
+
+#define FILL_OBJ(_idx, _handle, _offset) do { \
+	obj[(_idx)].handle = (_handle); \
+	obj[(_idx)].offset = (_offset); \
+} while (0)
+
+static int blt_block_copy3(int xe,
+			   const intel_ctx_t *ctx,
+			   uint64_t ahnd,
+			   const struct blt_copy3_data *blt3,
+			   const struct blt_block_copy3_data_ext *ext3)
+{
+	struct blt_copy_data blt0;
+	struct blt_block_copy_data_ext ext0;
+	uint64_t bb_offset, alignment;
+	uint64_t bb_pos = 0;
+	int ret;
+
+	igt_assert_f(ahnd, "block-copy3 supports softpin only\n");
+	igt_assert_f(blt3, "block-copy3 requires data to do blit\n");
+
+	alignment = xe_get_default_alignment(xe);
+	get_offset(ahnd, blt3->src.handle, blt3->src.size, alignment);
+	get_offset(ahnd, blt3->mid.handle, blt3->mid.size, alignment);
+	get_offset(ahnd, blt3->dst.handle, blt3->dst.size, alignment);
+	get_offset(ahnd, blt3->final.handle, blt3->final.size, alignment);
+	bb_offset = get_offset(ahnd, blt3->bb.handle, blt3->bb.size, alignment);
+
+	/* First blit src -> mid */
+	blt_copy_init(xe, &blt0);
+	blt0.src = blt3->src;
+	blt0.dst = blt3->mid;
+	blt0.bb = blt3->bb;
+	blt0.color_depth = blt3->color_depth;
+	blt0.print_bb = blt3->print_bb;
+	ext0.src = ext3->src;
+	ext0.dst = ext3->mid;
+	bb_pos = emit_blt_block_copy(xe, ahnd, &blt0, &ext0, bb_pos, false);
+
+	/* Second blit mid -> dst */
+	blt_copy_init(xe, &blt0);
+	blt0.src = blt3->mid;
+	blt0.dst = blt3->dst;
+	blt0.bb = blt3->bb;
+	blt0.color_depth = blt3->color_depth;
+	blt0.print_bb = blt3->print_bb;
+	ext0.src = ext3->mid;
+	ext0.dst = ext3->dst;
+	bb_pos = emit_blt_block_copy(xe, ahnd, &blt0, &ext0, bb_pos, false);
+
+	/* Third blit dst -> final */
+	blt_copy_init(xe, &blt0);
+	blt0.src = blt3->dst;
+	blt0.dst = blt3->final;
+	blt0.bb = blt3->bb;
+	blt0.color_depth = blt3->color_depth;
+	blt0.print_bb = blt3->print_bb;
+	ext0.src = ext3->dst;
+	ext0.dst = ext3->final;
+	bb_pos = emit_blt_block_copy(xe, ahnd, &blt0, &ext0, bb_pos, true);
+
+	intel_ctx_xe_exec(ctx, ahnd, bb_offset);
+
+	return ret;
+}
+
+static void block_copy(int xe,
+		       intel_ctx_t *ctx,
+		       uint32_t region1, uint32_t region2,
+		       enum blt_tiling_type mid_tiling,
+		       const struct test_config *config)
+{
+	struct blt_copy_data blt = {};
+	struct blt_block_copy_data_ext ext = {}, *pext = &ext;
+	struct blt_copy_object *src, *mid, *dst;
+	const uint32_t bpp = 32;
+	uint64_t bb_size = xe_get_default_alignment(xe);
+	uint64_t ahnd = intel_allocator_open(xe, ctx->vm, INTEL_ALLOCATOR_RELOC);
+	uint32_t run_id = mid_tiling;
+	uint32_t mid_region = region2, bb;
+	uint32_t width = param.width, height = param.height;
+	enum blt_compression mid_compression = config->compression;
+	int mid_compression_format = param.compression_format;
+	enum blt_compression_type comp_type = COMPRESSION_TYPE_3D;
+	uint8_t uc_mocs = intel_get_uc_mocs(xe);
+	int result;
+
+	bb = xe_bo_create_flags(xe, 0, bb_size, region1);
+
+	if (!blt_uses_extended_block_copy(xe))
+		pext = NULL;
+
+	blt_copy_init(xe, &blt);
+
+	src = blt_create_object(&blt, region1, width, height, bpp, uc_mocs,
+				T_LINEAR, COMPRESSION_DISABLED, comp_type, true);
+	mid = blt_create_object(&blt, mid_region, width, height, bpp, uc_mocs,
+				mid_tiling, mid_compression, comp_type, true);
+	dst = blt_create_object(&blt, region1, width, height, bpp, uc_mocs,
+				T_LINEAR, COMPRESSION_DISABLED, comp_type, true);
+	igt_assert(src->size == dst->size);
+	PRINT_SURFACE_INFO("src", src);
+	PRINT_SURFACE_INFO("mid", mid);
+	PRINT_SURFACE_INFO("dst", dst);
+
+	blt_surface_fill_rect(xe, src, width, height);
+	WRITE_PNG(xe, run_id, "src", src, width, height);
+
+	blt.color_depth = CD_32bit;
+	blt.print_bb = param.print_bb;
+	blt_set_copy_object(&blt.src, src);
+	blt_set_copy_object(&blt.dst, mid);
+	blt_set_object_ext(&ext.src, 0, width, height, SURFACE_TYPE_2D);
+	blt_set_object_ext(&ext.dst, mid_compression_format, width, height, SURFACE_TYPE_2D);
+	blt_set_batch(&blt.bb, bb, bb_size, region1);
+	blt_block_copy(xe, ctx, NULL, ahnd, &blt, pext);
+	intel_ctx_xe_sync(ctx, true);
+
+	/* We expect mid != src if there's compression */
+	if (mid->compression)
+		igt_assert(memcmp(src->ptr, mid->ptr, src->size) != 0);
+
+	WRITE_PNG(xe, run_id, "src", &blt.src, width, height);
+	WRITE_PNG(xe, run_id, "mid", &blt.dst, width, height);
+
+	if (config->surfcopy && pext) {
+		struct drm_xe_engine_class_instance inst = {
+			.engine_class = DRM_XE_ENGINE_CLASS_COPY,
+		};
+		intel_ctx_t *surf_ctx = ctx;
+		uint64_t surf_ahnd = ahnd;
+		uint32_t vm, engine;
+
+		if (config->new_ctx) {
+			vm = xe_vm_create(xe, DRM_XE_VM_CREATE_ASYNC_BIND_OPS, 0);
+			engine = xe_engine_create(xe, vm, &inst, 0);
+			surf_ctx = intel_ctx_xe(xe, vm, engine, 0, 0, 0);
+			surf_ahnd = intel_allocator_open(xe, surf_ctx->vm,
+							 INTEL_ALLOCATOR_RELOC);
+		}
+		surf_copy(xe, surf_ctx, surf_ahnd, src, mid, dst, run_id,
+			  config->suspend_resume);
+
+		if (surf_ctx != ctx) {
+			xe_engine_destroy(xe, engine);
+			xe_vm_destroy(xe, vm);
+			free(surf_ctx);
+			put_ahnd(surf_ahnd);
+		}
+	}
+
+	blt_copy_init(xe, &blt);
+	blt.color_depth = CD_32bit;
+	blt.print_bb = param.print_bb;
+	blt_set_copy_object(&blt.src, mid);
+	blt_set_copy_object(&blt.dst, dst);
+	blt_set_object_ext(&ext.src, mid_compression_format, width, height, SURFACE_TYPE_2D);
+	blt_set_object_ext(&ext.dst, 0, width, height, SURFACE_TYPE_2D);
+	if (config->inplace) {
+		blt_set_object(&blt.dst, mid->handle, dst->size, mid->region, 0,
+			       T_LINEAR, COMPRESSION_DISABLED, comp_type);
+		blt.dst.ptr = mid->ptr;
+	}
+
+	blt_set_batch(&blt.bb, bb, bb_size, region1);
+	blt_block_copy(xe, ctx, NULL, ahnd, &blt, pext);
+	intel_ctx_xe_sync(ctx, true);
+
+	WRITE_PNG(xe, run_id, "dst", &blt.dst, width, height);
+
+	result = memcmp(src->ptr, blt.dst.ptr, src->size);
+
+	/* Politely clean vm */
+	put_offset(ahnd, src->handle);
+	put_offset(ahnd, mid->handle);
+	put_offset(ahnd, dst->handle);
+	put_offset(ahnd, bb);
+	intel_allocator_bind(ahnd, 0, 0);
+	blt_destroy_object(xe, src);
+	blt_destroy_object(xe, mid);
+	blt_destroy_object(xe, dst);
+	gem_close(xe, bb);
+	put_ahnd(ahnd);
+
+	igt_assert_f(!result, "source and destination surfaces differs!\n");
+}
+
+static void block_multicopy(int xe,
+			    intel_ctx_t *ctx,
+			    uint32_t region1, uint32_t region2,
+			    enum blt_tiling_type mid_tiling,
+			    const struct test_config *config)
+{
+	struct blt_copy3_data blt3 = {};
+	struct blt_copy_data blt = {};
+	struct blt_block_copy3_data_ext ext3 = {}, *pext3 = &ext3;
+	struct blt_copy_object *src, *mid, *dst, *final;
+	const uint32_t bpp = 32;
+	uint64_t bb_size = xe_get_default_alignment(xe);
+	uint64_t ahnd = intel_allocator_open(xe, ctx->vm, INTEL_ALLOCATOR_RELOC);
+	uint32_t run_id = mid_tiling;
+	uint32_t mid_region = region2, bb;
+	uint32_t width = param.width, height = param.height;
+	enum blt_compression mid_compression = config->compression;
+	int mid_compression_format = param.compression_format;
+	enum blt_compression_type comp_type = COMPRESSION_TYPE_3D;
+	uint8_t uc_mocs = intel_get_uc_mocs(xe);
+	int result;
+
+	bb = xe_bo_create_flags(xe, 0, bb_size, region1);
+
+	if (!blt_uses_extended_block_copy(xe))
+		pext3 = NULL;
+
+	blt_copy_init(xe, &blt);
+
+	src = blt_create_object(&blt, region1, width, height, bpp, uc_mocs,
+				T_LINEAR, COMPRESSION_DISABLED, comp_type, true);
+	mid = blt_create_object(&blt, mid_region, width, height, bpp, uc_mocs,
+				mid_tiling, mid_compression, comp_type, true);
+	dst = blt_create_object(&blt, region1, width, height, bpp, uc_mocs,
+				mid_tiling, COMPRESSION_DISABLED, comp_type, true);
+	final = blt_create_object(&blt, region1, width, height, bpp, uc_mocs,
+				  T_LINEAR, COMPRESSION_DISABLED, comp_type, true);
+	igt_assert(src->size == dst->size);
+	PRINT_SURFACE_INFO("src", src);
+	PRINT_SURFACE_INFO("mid", mid);
+	PRINT_SURFACE_INFO("dst", dst);
+	PRINT_SURFACE_INFO("final", final);
+
+	blt_surface_fill_rect(xe, src, width, height);
+
+	blt3.color_depth = CD_32bit;
+	blt3.print_bb = param.print_bb;
+	blt_set_copy_object(&blt3.src, src);
+	blt_set_copy_object(&blt3.mid, mid);
+	blt_set_copy_object(&blt3.dst, dst);
+	blt_set_copy_object(&blt3.final, final);
+
+	if (config->inplace) {
+		blt_set_object(&blt3.dst, mid->handle, dst->size, mid->region,
+			       mid->mocs, mid_tiling, COMPRESSION_DISABLED,
+			       comp_type);
+		blt3.dst.ptr = mid->ptr;
+	}
+
+	blt_set_object_ext(&ext3.src, 0, width, height, SURFACE_TYPE_2D);
+	blt_set_object_ext(&ext3.mid, mid_compression_format, width, height, SURFACE_TYPE_2D);
+	blt_set_object_ext(&ext3.dst, 0, width, height, SURFACE_TYPE_2D);
+	blt_set_object_ext(&ext3.final, 0, width, height, SURFACE_TYPE_2D);
+	blt_set_batch(&blt3.bb, bb, bb_size, region1);
+
+	blt_block_copy3(xe, ctx, ahnd, &blt3, pext3);
+	intel_ctx_xe_sync(ctx, true);
+
+	WRITE_PNG(xe, run_id, "src", &blt3.src, width, height);
+	if (!config->inplace)
+		WRITE_PNG(xe, run_id, "mid", &blt3.mid, width, height);
+	WRITE_PNG(xe, run_id, "dst", &blt3.dst, width, height);
+	WRITE_PNG(xe, run_id, "final", &blt3.final, width, height);
+
+	result = memcmp(src->ptr, blt3.final.ptr, src->size);
+
+	put_offset(ahnd, src->handle);
+	put_offset(ahnd, mid->handle);
+	put_offset(ahnd, dst->handle);
+	put_offset(ahnd, final->handle);
+	put_offset(ahnd, bb);
+	intel_allocator_bind(ahnd, 0, 0);
+	blt_destroy_object(xe, src);
+	blt_destroy_object(xe, mid);
+	blt_destroy_object(xe, dst);
+	blt_destroy_object(xe, final);
+	gem_close(xe, bb);
+	put_ahnd(ahnd);
+
+	igt_assert_f(!result, "source and destination surfaces differs!\n");
+}
+
+enum copy_func {
+	BLOCK_COPY,
+	BLOCK_MULTICOPY,
+};
+
+static const struct {
+	const char *suffix;
+	void (*copyfn)(int fd,
+		       intel_ctx_t *ctx,
+		       uint32_t region1, uint32_t region2,
+		       enum blt_tiling_type btype,
+		       const struct test_config *config);
+} copyfns[] = {
+	[BLOCK_COPY] = { "", block_copy },
+	[BLOCK_MULTICOPY] = { "-multicopy", block_multicopy },
+};
+
+static void block_copy_test(int xe,
+			    const struct test_config *config,
+			    struct igt_collection *set,
+			    enum copy_func copy_function)
+{
+	struct drm_xe_engine_class_instance inst = {
+		.engine_class = DRM_XE_ENGINE_CLASS_COPY,
+	};
+	intel_ctx_t *ctx;
+	struct igt_collection *regions;
+	uint32_t vm, engine;
+	int tiling;
+
+	if (config->compression && !blt_block_copy_supports_compression(xe))
+		return;
+
+	if (config->inplace && !config->compression)
+		return;
+
+	for_each_tiling(tiling) {
+		if (!blt_block_copy_supports_tiling(xe, tiling) ||
+		    (param.tiling >= 0 && param.tiling != tiling))
+			continue;
+
+		for_each_variation_r(regions, 2, set) {
+			uint32_t region1, region2;
+			char *regtxt;
+
+			region1 = igt_collection_get_value(regions, 0);
+			region2 = igt_collection_get_value(regions, 1);
+
+			/* Compressed surface must be in device memory */
+			if (config->compression && !XE_IS_VRAM_MEMORY_REGION(xe, region2))
+				continue;
+
+			regtxt = xe_memregion_dynamic_subtest_name(xe, regions);
+
+			igt_dynamic_f("%s-%s-compfmt%d-%s%s",
+				      blt_tiling_name(tiling),
+				      config->compression ?
+					      "compressed" : "uncompressed",
+				      param.compression_format, regtxt,
+				      copyfns[copy_function].suffix) {
+				uint32_t sync_bind, sync_out;
+
+				vm = xe_vm_create(xe, DRM_XE_VM_CREATE_ASYNC_BIND_OPS, 0);
+				engine = xe_engine_create(xe, vm, &inst, 0);
+				sync_bind = syncobj_create(xe, 0);
+				sync_out = syncobj_create(xe, 0);
+				ctx = intel_ctx_xe(xe, vm, engine,
+						   0, sync_bind, sync_out);
+
+				copyfns[copy_function].copyfn(xe, ctx,
+							      region1, region2,
+							      tiling, config);
+
+				xe_engine_destroy(xe, engine);
+				xe_vm_destroy(xe, vm);
+				syncobj_destroy(xe, sync_bind);
+				syncobj_destroy(xe, sync_out);
+				free(ctx);
+			}
+
+			free(regtxt);
+		}
+	}
+}
+
+static int opt_handler(int opt, int opt_index, void *data)
+{
+	switch (opt) {
+	case 'b':
+		param.print_bb = true;
+		igt_debug("Print bb: %d\n", param.print_bb);
+		break;
+	case 'f':
+		param.compression_format = atoi(optarg);
+		igt_debug("Compression format: %d\n", param.compression_format);
+		igt_assert((param.compression_format & ~0x1f) == 0);
+		break;
+	case 'p':
+		param.write_png = true;
+		igt_debug("Write png: %d\n", param.write_png);
+		break;
+	case 's':
+		param.print_surface_info = true;
+		igt_debug("Print surface info: %d\n", param.print_surface_info);
+		break;
+	case 't':
+		param.tiling = atoi(optarg);
+		igt_debug("Tiling: %d\n", param.tiling);
+		break;
+	case 'W':
+		param.width = atoi(optarg);
+		igt_debug("Width: %d\n", param.width);
+		break;
+	case 'H':
+		param.height = atoi(optarg);
+		igt_debug("Height: %d\n", param.height);
+		break;
+	default:
+		return IGT_OPT_HANDLER_ERROR;
+	}
+
+	return IGT_OPT_HANDLER_SUCCESS;
+}
+
+const char *help_str =
+	"  -b\tPrint bb\n"
+	"  -f\tCompression format (0-31)\n"
+	"  -p\tWrite PNG\n"
+	"  -s\tPrint surface info\n"
+	"  -t\tTiling format (0 - linear, 1 - XMAJOR, 2 - YMAJOR, 3 - TILE4, 4 - TILE64)\n"
+	"  -W\tWidth (default 512)\n"
+	"  -H\tHeight (default 512)"
+	;
+
+igt_main_args("bf:pst:W:H:", NULL, help_str, opt_handler, NULL)
+{
+	struct igt_collection *set;
+	int xe;
+
+	igt_fixture {
+		xe = drm_open_driver(DRIVER_XE);
+		igt_require(blt_has_block_copy(xe));
+
+		xe_device_get(xe);
+
+		set = xe_get_memory_region_set(xe,
+					       XE_MEM_REGION_CLASS_SYSMEM,
+					       XE_MEM_REGION_CLASS_VRAM);
+	}
+
+	igt_describe("Check block-copy uncompressed blit");
+	igt_subtest_with_dynamic("block-copy-uncompressed") {
+		struct test_config config = {};
+
+		block_copy_test(xe, &config, set, BLOCK_COPY);
+	}
+
+	igt_describe("Check block-copy flatccs compressed blit");
+	igt_subtest_with_dynamic("block-copy-compressed") {
+		struct test_config config = { .compression = true };
+
+		block_copy_test(xe, &config, set, BLOCK_COPY);
+	}
+
+	igt_describe("Check block-multicopy flatccs compressed blit");
+	igt_subtest_with_dynamic("block-multicopy-compressed") {
+		struct test_config config = { .compression = true };
+
+		block_copy_test(xe, &config, set, BLOCK_MULTICOPY);
+	}
+
+	igt_describe("Check block-multicopy flatccs inplace decompression blit");
+	igt_subtest_with_dynamic("block-multicopy-inplace") {
+		struct test_config config = { .compression = true,
+					      .inplace = true };
+
+		block_copy_test(xe, &config, set, BLOCK_MULTICOPY);
+	}
+
+	igt_describe("Check flatccs data can be copied from/to surface");
+	igt_subtest_with_dynamic("ctrl-surf-copy") {
+		struct test_config config = { .compression = true,
+					      .surfcopy = true };
+
+		block_copy_test(xe, &config, set, BLOCK_COPY);
+	}
+
+	igt_describe("Check flatccs data are physically tagged and visible"
+		     " in different contexts");
+	igt_subtest_with_dynamic("ctrl-surf-copy-new-ctx") {
+		struct test_config config = { .compression = true,
+					      .surfcopy = true,
+					      .new_ctx = true };
+
+		block_copy_test(xe, &config, set, BLOCK_COPY);
+	}
+
+	igt_describe("Check flatccs data persists after suspend / resume (S0)");
+	igt_subtest_with_dynamic("suspend-resume") {
+		struct test_config config = { .compression = true,
+					      .surfcopy = true,
+					      .suspend_resume = true };
+
+		block_copy_test(xe, &config, set, BLOCK_COPY);
+	}
+
+	igt_fixture {
+		xe_device_put(xe);
+		close(xe);
+	}
+}
-- 
2.34.1

[igt-dev] [PATCH i-g-t 12/12] tests/xe_exercise_blt: Check blitter library fast-copy for Xe

[Zbigniew Kempczyński]

Port this test to work on xe. Instead of adding conditional code for
xe code which would decrease readability this is new test for xe.

Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 tests/meson.build          |   1 +
 tests/xe/xe_exercise_blt.c | 372 +++++++++++++++++++++++++++++++++++++
 2 files changed, 373 insertions(+)
 create mode 100644 tests/xe/xe_exercise_blt.c

diff --git a/tests/meson.build b/tests/meson.build
index 9bca57a5e8..137a5cf01f 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -257,6 +257,7 @@ xe_progs = [
 	'xe_exec_reset',
 	'xe_exec_store',
 	'xe_exec_threads',
+	'xe_exercise_blt',
 	'xe_gpgpu_fill',
 	'xe_guc_pc',
 	'xe_huc_copy',
diff --git a/tests/xe/xe_exercise_blt.c b/tests/xe/xe_exercise_blt.c
new file mode 100644
index 0000000000..8340cf7148
--- /dev/null
+++ b/tests/xe/xe_exercise_blt.c
@@ -0,0 +1,372 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#include "igt.h"
+#include "drm.h"
+#include "lib/intel_chipset.h"
+#include "intel_blt.h"
+#include "intel_mocs.h"
+#include "xe/xe_ioctl.h"
+#include "xe/xe_query.h"
+#include "xe/xe_util.h"
+
+/**
+ * TEST: xe exercise blt
+ * Description: Exercise blitter commands on Xe
+ * Feature: blitter
+ * Run type: FULL
+ * Test category: GEM_Legacy
+ *
+ * SUBTEST: fast-copy
+ * Description:
+ *   Check fast-copy blit
+ *   blitter
+ *
+ * SUBTEST: fast-copy-emit
+ * Description:
+ *   Check multiple fast-copy in one batch
+ *   blitter
+ */
+
+IGT_TEST_DESCRIPTION("Exercise blitter commands on Xe");
+
+static struct param {
+	int tiling;
+	bool write_png;
+	bool print_bb;
+	bool print_surface_info;
+	int width;
+	int height;
+} param = {
+	.tiling = -1,
+	.write_png = false,
+	.print_bb = false,
+	.print_surface_info = false,
+	.width = 512,
+	.height = 512,
+};
+
+#define PRINT_SURFACE_INFO(name, obj) do { \
+	if (param.print_surface_info) \
+		blt_surface_info((name), (obj)); } while (0)
+
+#define WRITE_PNG(fd, id, name, obj, w, h) do { \
+	if (param.write_png) \
+		blt_surface_to_png((fd), (id), (name), (obj), (w), (h)); } while (0)
+
+struct blt_fast_copy_data {
+	int xe;
+	struct blt_copy_object src;
+	struct blt_copy_object mid;
+	struct blt_copy_object dst;
+
+	struct blt_copy_batch bb;
+	enum blt_color_depth color_depth;
+
+	/* debug stuff */
+	bool print_bb;
+};
+
+static int fast_copy_one_bb(int xe,
+			    const intel_ctx_t *ctx,
+			    uint64_t ahnd,
+			    const struct blt_fast_copy_data *blt)
+{
+	struct blt_copy_data blt_tmp;
+	uint64_t bb_offset, alignment;
+	uint64_t bb_pos = 0;
+	int ret = 0;
+
+	alignment = xe_get_default_alignment(xe);
+
+	get_offset(ahnd, blt->src.handle, blt->src.size, alignment);
+	get_offset(ahnd, blt->mid.handle, blt->mid.size, alignment);
+	get_offset(ahnd, blt->dst.handle, blt->dst.size, alignment);
+	bb_offset = get_offset(ahnd, blt->bb.handle, blt->bb.size, alignment);
+
+	/* First blit */
+	blt_copy_init(xe, &blt_tmp);
+	blt_tmp.src = blt->src;
+	blt_tmp.dst = blt->mid;
+	blt_tmp.bb = blt->bb;
+	blt_tmp.color_depth = blt->color_depth;
+	blt_tmp.print_bb = blt->print_bb;
+	bb_pos = emit_blt_fast_copy(xe, ahnd, &blt_tmp, bb_pos, false);
+
+	/* Second blit */
+	blt_copy_init(xe, &blt_tmp);
+	blt_tmp.src = blt->mid;
+	blt_tmp.dst = blt->dst;
+	blt_tmp.bb = blt->bb;
+	blt_tmp.color_depth = blt->color_depth;
+	blt_tmp.print_bb = blt->print_bb;
+	bb_pos = emit_blt_fast_copy(xe, ahnd, &blt_tmp, bb_pos, true);
+
+	intel_ctx_xe_exec(ctx, ahnd, bb_offset);
+
+	return ret;
+}
+
+static void fast_copy_emit(int xe, const intel_ctx_t *ctx,
+			   uint32_t region1, uint32_t region2,
+			   enum blt_tiling_type mid_tiling)
+{
+	struct blt_copy_data bltinit = {};
+	struct blt_fast_copy_data blt = {};
+	struct blt_copy_object *src, *mid, *dst;
+	const uint32_t bpp = 32;
+	uint64_t bb_size = xe_get_default_alignment(xe);
+	uint64_t ahnd = intel_allocator_open_full(xe, ctx->vm, 0, 0,
+						  INTEL_ALLOCATOR_SIMPLE,
+						  ALLOC_STRATEGY_LOW_TO_HIGH, 0);
+	uint32_t bb, width = param.width, height = param.height;
+	int result;
+
+	bb = xe_bo_create_flags(xe, 0, bb_size, region1);
+
+	blt_copy_init(xe, &bltinit);
+	src = blt_create_object(&bltinit, region1, width, height, bpp, 0,
+				T_LINEAR, COMPRESSION_DISABLED, 0, true);
+	mid = blt_create_object(&bltinit, region2, width, height, bpp, 0,
+				mid_tiling, COMPRESSION_DISABLED, 0, true);
+	dst = blt_create_object(&bltinit, region1, width, height, bpp, 0,
+				T_LINEAR, COMPRESSION_DISABLED, 0, true);
+	igt_assert(src->size == dst->size);
+
+	PRINT_SURFACE_INFO("src", src);
+	PRINT_SURFACE_INFO("mid", mid);
+	PRINT_SURFACE_INFO("dst", dst);
+
+	blt_surface_fill_rect(xe, src, width, height);
+	WRITE_PNG(xe, mid_tiling, "src", src, width, height);
+
+	memset(&blt, 0, sizeof(blt));
+	blt.color_depth = CD_32bit;
+	blt.print_bb = param.print_bb;
+	blt_set_copy_object(&blt.src, src);
+	blt_set_copy_object(&blt.mid, mid);
+	blt_set_copy_object(&blt.dst, dst);
+	blt_set_batch(&blt.bb, bb, bb_size, region1);
+
+	fast_copy_one_bb(xe, ctx, ahnd, &blt);
+
+	WRITE_PNG(xe, mid_tiling, "mid", &blt.mid, width, height);
+	WRITE_PNG(xe, mid_tiling, "dst", &blt.dst, width, height);
+
+	result = memcmp(src->ptr, blt.dst.ptr, src->size);
+
+	blt_destroy_object(xe, src);
+	blt_destroy_object(xe, mid);
+	blt_destroy_object(xe, dst);
+	gem_close(xe, bb);
+	put_ahnd(ahnd);
+
+	munmap(&bb, bb_size);
+
+	igt_assert_f(!result, "source and destination surfaces differs!\n");
+}
+
+static void fast_copy(int xe, const intel_ctx_t *ctx,
+		      uint32_t region1, uint32_t region2,
+		      enum blt_tiling_type mid_tiling)
+{
+	struct blt_copy_data blt = {};
+	struct blt_copy_object *src, *mid, *dst;
+	const uint32_t bpp = 32;
+	uint64_t bb_size = xe_get_default_alignment(xe);
+	uint64_t ahnd = intel_allocator_open_full(xe, ctx->vm, 0, 0,
+						  INTEL_ALLOCATOR_SIMPLE,
+						  ALLOC_STRATEGY_LOW_TO_HIGH, 0);
+	uint32_t bb;
+	uint32_t width = param.width, height = param.height;
+	int result;
+
+	bb = xe_bo_create_flags(xe, 0, bb_size, region1);
+
+	blt_copy_init(xe, &blt);
+	src = blt_create_object(&blt, region1, width, height, bpp, 0,
+				T_LINEAR, COMPRESSION_DISABLED, 0, true);
+	mid = blt_create_object(&blt, region2, width, height, bpp, 0,
+				mid_tiling, COMPRESSION_DISABLED, 0, true);
+	dst = blt_create_object(&blt, region1, width, height, bpp, 0,
+				T_LINEAR, COMPRESSION_DISABLED, 0, true);
+	igt_assert(src->size == dst->size);
+
+	blt_surface_fill_rect(xe, src, width, height);
+
+	blt.color_depth = CD_32bit;
+	blt.print_bb = param.print_bb;
+	blt_set_copy_object(&blt.src, src);
+	blt_set_copy_object(&blt.dst, mid);
+	blt_set_batch(&blt.bb, bb, bb_size, region1);
+
+	blt_fast_copy(xe, ctx, NULL, ahnd, &blt);
+
+	WRITE_PNG(xe, mid_tiling, "src", &blt.src, width, height);
+	WRITE_PNG(xe, mid_tiling, "mid", &blt.dst, width, height);
+
+	blt_copy_init(xe, &blt);
+	blt.color_depth = CD_32bit;
+	blt.print_bb = param.print_bb;
+	blt_set_copy_object(&blt.src, mid);
+	blt_set_copy_object(&blt.dst, dst);
+	blt_set_batch(&blt.bb, bb, bb_size, region1);
+
+	blt_fast_copy(xe, ctx, NULL, ahnd, &blt);
+
+	WRITE_PNG(xe, mid_tiling, "dst", &blt.dst, width, height);
+
+	result = memcmp(src->ptr, blt.dst.ptr, src->size);
+
+	blt_destroy_object(xe, src);
+	blt_destroy_object(xe, mid);
+	blt_destroy_object(xe, dst);
+	gem_close(xe, bb);
+	put_ahnd(ahnd);
+
+	igt_assert_f(!result, "source and destination surfaces differs!\n");
+}
+
+enum fast_copy_func {
+	FAST_COPY,
+	FAST_COPY_EMIT
+};
+
+static char
+	*full_subtest_str(char *regtxt, enum blt_tiling_type tiling,
+			  enum fast_copy_func func)
+{
+	char *name;
+	uint32_t len;
+
+	len = asprintf(&name, "%s-%s%s", blt_tiling_name(tiling), regtxt,
+		       func == FAST_COPY_EMIT ? "-emit" : "");
+
+	igt_assert_f(len >= 0, "asprintf failed!\n");
+
+	return name;
+}
+
+static void fast_copy_test(int xe,
+			   struct igt_collection *set,
+			   enum fast_copy_func func)
+{
+	struct drm_xe_engine_class_instance inst = {
+		.engine_class = DRM_XE_ENGINE_CLASS_COPY,
+	};
+	struct igt_collection *regions;
+	void (*copy_func)(int xe, const intel_ctx_t *ctx,
+			  uint32_t r1, uint32_t r2, enum blt_tiling_type tiling);
+	intel_ctx_t *ctx;
+	int tiling;
+
+	for_each_tiling(tiling) {
+		if (!blt_fast_copy_supports_tiling(xe, tiling))
+			continue;
+
+		for_each_variation_r(regions, 2, set) {
+			uint32_t region1, region2;
+			uint32_t vm, engine;
+			char *regtxt, *test_name;
+
+			region1 = igt_collection_get_value(regions, 0);
+			region2 = igt_collection_get_value(regions, 1);
+
+			vm = xe_vm_create(xe, DRM_XE_VM_CREATE_ASYNC_BIND_OPS, 0);
+			engine = xe_engine_create(xe, vm, &inst, 0);
+			ctx = intel_ctx_xe(xe, vm, engine, 0, 0, 0);
+
+			copy_func = (func == FAST_COPY) ? fast_copy : fast_copy_emit;
+			regtxt = xe_memregion_dynamic_subtest_name(xe, regions);
+			test_name = full_subtest_str(regtxt, tiling, func);
+
+			igt_dynamic_f("%s", test_name) {
+				copy_func(xe, ctx,
+					  region1, region2,
+					  tiling);
+			}
+
+			free(regtxt);
+			free(test_name);
+			xe_engine_destroy(xe, engine);
+			xe_vm_destroy(xe, vm);
+			free(ctx);
+		}
+	}
+}
+
+static int opt_handler(int opt, int opt_index, void *data)
+{
+	switch (opt) {
+	case 'b':
+		param.print_bb = true;
+		igt_debug("Print bb: %d\n", param.print_bb);
+		break;
+	case 'p':
+		param.write_png = true;
+		igt_debug("Write png: %d\n", param.write_png);
+		break;
+	case 's':
+		param.print_surface_info = true;
+		igt_debug("Print surface info: %d\n", param.print_surface_info);
+		break;
+	case 't':
+		param.tiling = atoi(optarg);
+		igt_debug("Tiling: %d\n", param.tiling);
+		break;
+	case 'W':
+		param.width = atoi(optarg);
+		igt_debug("Width: %d\n", param.width);
+		break;
+	case 'H':
+		param.height = atoi(optarg);
+		igt_debug("Height: %d\n", param.height);
+		break;
+	default:
+		return IGT_OPT_HANDLER_ERROR;
+	}
+
+	return IGT_OPT_HANDLER_SUCCESS;
+}
+
+const char *help_str =
+	"  -b\tPrint bb\n"
+	"  -p\tWrite PNG\n"
+	"  -s\tPrint surface info\n"
+	"  -t\tTiling format (0 - linear, 1 - XMAJOR, 2 - YMAJOR, 3 - TILE4, 4 - TILE64, 5 - YFMAJOR)\n"
+	"  -W\tWidth (default 512)\n"
+	"  -H\tHeight (default 512)"
+	;
+
+igt_main_args("b:pst:W:H:", NULL, help_str, opt_handler, NULL)
+{
+	struct igt_collection *set;
+	int xe;
+
+	igt_fixture {
+		xe = drm_open_driver(DRIVER_XE);
+		igt_require(blt_has_block_copy(xe));
+
+		xe_device_get(xe);
+
+		set = xe_get_memory_region_set(xe,
+					       XE_MEM_REGION_CLASS_SYSMEM,
+					       XE_MEM_REGION_CLASS_VRAM);
+	}
+
+	igt_describe("Check fast-copy blit");
+	igt_subtest_with_dynamic("fast-copy") {
+		fast_copy_test(xe, set, FAST_COPY);
+	}
+
+	igt_describe("Check multiple fast-copy in one batch");
+	igt_subtest_with_dynamic("fast-copy-emit") {
+		fast_copy_test(xe, set, FAST_COPY_EMIT);
+	}
+
+	igt_fixture {
+		drm_close_driver(xe);
+	}
+}
-- 
2.34.1

[igt-dev] ✗ Fi.CI.BAT: failure for Extend intel_blt to work on Xe

[Patchwork]

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

== Series Details ==

Series: Extend intel_blt to work on Xe
URL   : https://patchwork.freedesktop.org/series/120162/
State : failure

== Summary ==

CI Bug Log - changes from IGT_7369 -> IGTPW_9327
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_9327 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_9327, please notify your bug team 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_9327/index.html

Participating hosts (41 -> 42)
------------------------------

  Additional (2): fi-kbl-soraka fi-pnv-d510 
  Missing    (1): fi-snb-2520m 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_busy@busy@all-engines:
    - bat-mtlp-8:         [PASS][1] -> [FAIL][2] +7 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-mtlp-8/igt@gem_busy@busy@all-engines.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-mtlp-8/igt@gem_busy@busy@all-engines.html

  * igt@gem_exec_fence@basic-await@bcs0:
    - bat-atsm-1:         [PASS][3] -> [FAIL][4] +11 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-atsm-1/igt@gem_exec_fence@basic-await@bcs0.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-atsm-1/igt@gem_exec_fence@basic-await@bcs0.html
    - bat-dg2-8:          [PASS][5] -> [FAIL][6] +11 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-dg2-8/igt@gem_exec_fence@basic-await@bcs0.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-dg2-8/igt@gem_exec_fence@basic-await@bcs0.html
    - bat-adlm-1:         [PASS][7] -> [FAIL][8] +6 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-adlm-1/igt@gem_exec_fence@basic-await@bcs0.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-adlm-1/igt@gem_exec_fence@basic-await@bcs0.html

  * igt@gem_exec_fence@basic-await@ccs0:
    - bat-dg2-11:         [PASS][9] -> [FAIL][10] +8 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-dg2-11/igt@gem_exec_fence@basic-await@ccs0.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-dg2-11/igt@gem_exec_fence@basic-await@ccs0.html

  * igt@gem_exec_fence@basic-await@ccs1:
    - bat-dg2-9:          [PASS][11] -> [FAIL][12] +11 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-dg2-9/igt@gem_exec_fence@basic-await@ccs1.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-dg2-9/igt@gem_exec_fence@basic-await@ccs1.html

  * igt@gem_exec_fence@basic-await@rcs0:
    - bat-mtlp-6:         [PASS][13] -> [FAIL][14] +7 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-mtlp-6/igt@gem_exec_fence@basic-await@rcs0.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-mtlp-6/igt@gem_exec_fence@basic-await@rcs0.html
    - bat-rpls-2:         [PASS][15] -> [FAIL][16] +6 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-rpls-2/igt@gem_exec_fence@basic-await@rcs0.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-rpls-2/igt@gem_exec_fence@basic-await@rcs0.html
    - bat-adlp-6:         [PASS][17] -> [FAIL][18] +6 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-adlp-6/igt@gem_exec_fence@basic-await@rcs0.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-adlp-6/igt@gem_exec_fence@basic-await@rcs0.html

  * igt@gem_exec_fence@basic-await@vcs0:
    - bat-adlp-9:         [PASS][19] -> [FAIL][20] +6 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-adlp-9/igt@gem_exec_fence@basic-await@vcs0.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-adlp-9/igt@gem_exec_fence@basic-await@vcs0.html
    - bat-adln-1:         [PASS][21] -> [FAIL][22] +5 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-adln-1/igt@gem_exec_fence@basic-await@vcs0.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-adln-1/igt@gem_exec_fence@basic-await@vcs0.html
    - bat-rplp-1:         [PASS][23] -> [FAIL][24] +6 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-rplp-1/igt@gem_exec_fence@basic-await@vcs0.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-rplp-1/igt@gem_exec_fence@basic-await@vcs0.html

  * igt@gem_exec_fence@basic-await@vecs0:
    - bat-rpls-1:         [PASS][25] -> [FAIL][26] +6 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-rpls-1/igt@gem_exec_fence@basic-await@vecs0.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-rpls-1/igt@gem_exec_fence@basic-await@vecs0.html
    - fi-rkl-11600:       [PASS][27] -> [FAIL][28] +5 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/fi-rkl-11600/igt@gem_exec_fence@basic-await@vecs0.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/fi-rkl-11600/igt@gem_exec_fence@basic-await@vecs0.html
    - bat-adls-5:         [PASS][29] -> [FAIL][30] +5 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-adls-5/igt@gem_exec_fence@basic-await@vecs0.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-adls-5/igt@gem_exec_fence@basic-await@vecs0.html
    - bat-dg1-5:          [PASS][31] -> [FAIL][32] +6 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-dg1-5/igt@gem_exec_fence@basic-await@vecs0.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-dg1-5/igt@gem_exec_fence@basic-await@vecs0.html
    - bat-dg1-7:          [PASS][33] -> [FAIL][34] +6 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-dg1-7/igt@gem_exec_fence@basic-await@vecs0.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-dg1-7/igt@gem_exec_fence@basic-await@vecs0.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][35] ([fdo#109271] / [i915#2190])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][36] ([fdo#109271] / [i915#4613]) +3 similar issues
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html

  * igt@i915_selftest@live@gt_mocs:
    - bat-mtlp-6:         [PASS][37] -> [DMESG-FAIL][38] ([i915#7059])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-mtlp-6/igt@i915_selftest@live@gt_mocs.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-mtlp-6/igt@i915_selftest@live@gt_mocs.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][39] ([i915#1886] / [i915#7913])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@requests:
    - bat-rpls-2:         [PASS][40] -> [ABORT][41] ([i915#7913] / [i915#7982])
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-rpls-2/igt@i915_selftest@live@requests.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-rpls-2/igt@i915_selftest@live@requests.html

  * igt@i915_selftest@live@workarounds:
    - bat-mtlp-6:         [PASS][42] -> [DMESG-FAIL][43] ([i915#6763])
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-mtlp-6/igt@i915_selftest@live@workarounds.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-mtlp-6/igt@i915_selftest@live@workarounds.html

  * igt@i915_suspend@basic-s3-without-i915:
    - bat-rpls-1:         NOTRUN -> [ABORT][44] ([i915#6687] / [i915#7978] / [i915#8668])
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-rpls-1/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][45] ([fdo#109271]) +15 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/fi-kbl-soraka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
    - bat-rplp-1:         [PASS][46] -> [ABORT][47] ([i915#8442] / [i915#8668])
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html

  * igt@kms_psr@primary_page_flip:
    - fi-pnv-d510:        NOTRUN -> [SKIP][48] ([fdo#109271]) +38 similar issues
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/fi-pnv-d510/igt@kms_psr@primary_page_flip.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@requests:
    - bat-rpls-1:         [ABORT][49] ([i915#7920]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-rpls-1/igt@i915_selftest@live@requests.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-rpls-1/igt@i915_selftest@live@requests.html

  * igt@i915_selftest@live@slpc:
    - bat-mtlp-6:         [DMESG-WARN][51] ([i915#6367]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7369/bat-mtlp-6/igt@i915_selftest@live@slpc.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/bat-mtlp-6/igt@i915_selftest@live@slpc.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
  [i915#6763]: https://gitlab.freedesktop.org/drm/intel/issues/6763
  [i915#7059]: https://gitlab.freedesktop.org/drm/intel/issues/7059
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7920]: https://gitlab.freedesktop.org/drm/intel/issues/7920
  [i915#7978]: https://gitlab.freedesktop.org/drm/intel/issues/7978
  [i915#7982]: https://gitlab.freedesktop.org/drm/intel/issues/7982
  [i915#8442]: https://gitlab.freedesktop.org/drm/intel/issues/8442
  [i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668


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

  * CI: CI-20190529 -> None
  * IGT: IGT_7369 -> IGTPW_9327

  CI-20190529: 20190529
  CI_DRM_13343: f113b6e13583b2068062c3de150051bc59f1cfac @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_9327: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9327/index.html
  IGT_7369: 22009ac9c26ceec8450dd312f5c93fc01d986348 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git


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

+igt@xe_ccs@block-copy-compressed
+igt@xe_ccs@block-copy-uncompressed
+igt@xe_ccs@block-multicopy-compressed
+igt@xe_ccs@block-multicopy-inplace
+igt@xe_ccs@ctrl-surf-copy
+igt@xe_ccs@ctrl-surf-copy-new-ctx
+igt@xe_ccs@suspend-resume
+igt@xe_exercise_blt@fast-copy
+igt@xe_exercise_blt@fast-copy-emit

== Logs ==

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

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

Re: [igt-dev] [PATCH i-g-t 01/12] lib/xe_query: Use vramN when returning string region name

[Karolina Stolarek]

Hi Zbigniew,

On 4.07.2023 11:00, Zbigniew Kempczyński wrote:
> For tests which are mixing regions (like xe_ccs) name is confusing.
> As an example might be example "subtest-name-vram-1-vram-1". It's
> more readable when it will be renamed to "subtest-name-vram1-vram1".

Sounds good, but I wonder how it maps to the convention used in other 
tests. In gem_ccs, iirc, we had "smem" and "lmemN". Could we reuse it here?

All the best,
Karolina

> 
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> ---
>   lib/xe/xe_query.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/xe/xe_query.c b/lib/xe/xe_query.c
> index 48fb5afba7..830b7e401d 100644
> --- a/lib/xe/xe_query.c
> +++ b/lib/xe/xe_query.c
> @@ -445,7 +445,7 @@ struct drm_xe_query_mem_region *xe_mem_region(int fd, uint64_t region)
>    * xe_region_name:
>    * @region: region mask
>    *
> - * Returns region string like "system" or "vram-n" where n=0...62.
> + * Returns region string like "system" or "vramN" where N=0...62.
>    */
>   const char *xe_region_name(uint64_t region)
>   {
> @@ -457,7 +457,7 @@ const char *xe_region_name(uint64_t region)
>   		vrams = calloc(64, sizeof(char *));
>   		for (int i = 0; i < 64; i++) {
>   			if (i != 0)
> -				asprintf(&vrams[i], "vram-%d", i - 1);
> +				asprintf(&vrams[i], "vram%d", i - 1);
>   			else
>   				asprintf(&vrams[i], "system");
>   			igt_assert(vrams[i]);

Re: [igt-dev] [PATCH i-g-t 01/12] lib/xe_query: Use vramN when returning string region name

[Karolina Stolarek]

On 5.07.2023 12:58, Karolina Stolarek wrote:
> Hi Zbigniew,
> 
> On 4.07.2023 11:00, Zbigniew Kempczyński wrote:
>> For tests which are mixing regions (like xe_ccs) name is confusing.
>> As an example might be example "subtest-name-vram-1-vram-1". It's
>> more readable when it will be renamed to "subtest-name-vram1-vram1".
> 
> Sounds good, but I wonder how it maps to the convention used in other 
> tests. In gem_ccs, iirc, we had "smem" and "lmemN". Could we reuse it here?

Ah, OK, now I see these names are derived from drm_xe_memory_class. 
Ignore my comment then:

Reviewed-by: Karolina Stolarek <karolina.stolarek@intel.com>

> 
> All the best,
> Karolina
> 
>>
>> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
>> ---
>>   lib/xe/xe_query.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/xe/xe_query.c b/lib/xe/xe_query.c
>> index 48fb5afba7..830b7e401d 100644
>> --- a/lib/xe/xe_query.c
>> +++ b/lib/xe/xe_query.c
>> @@ -445,7 +445,7 @@ struct drm_xe_query_mem_region *xe_mem_region(int 
>> fd, uint64_t region)
>>    * xe_region_name:
>>    * @region: region mask
>>    *
>> - * Returns region string like "system" or "vram-n" where n=0...62.
>> + * Returns region string like "system" or "vramN" where N=0...62.
>>    */
>>   const char *xe_region_name(uint64_t region)
>>   {
>> @@ -457,7 +457,7 @@ const char *xe_region_name(uint64_t region)
>>           vrams = calloc(64, sizeof(char *));
>>           for (int i = 0; i < 64; i++) {
>>               if (i != 0)
>> -                asprintf(&vrams[i], "vram-%d", i - 1);
>> +                asprintf(&vrams[i], "vram%d", i - 1);
>>               else
>>                   asprintf(&vrams[i], "system");
>>               igt_assert(vrams[i]);

Re: [igt-dev] [PATCH i-g-t 02/12] lib/xe_query: Add xe_region_class() helper

[Karolina Stolarek]

On 4.07.2023 11:00, Zbigniew Kempczyński wrote:
> In the common code we often need to be aware of region class.
> Add helper which returns it from region id.
> 
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

I think this could be merged with 04/12, but I'm fine with the separate 
patch as well:

Reviewed-by: Karolina Stolarek <karolina.stolarek@intel.com>

> ---
>   lib/xe/xe_query.c | 16 ++++++++++++++++
>   lib/xe/xe_query.h |  1 +
>   2 files changed, 17 insertions(+)
> 
> diff --git a/lib/xe/xe_query.c b/lib/xe/xe_query.c
> index 830b7e401d..f535ad8534 100644
> --- a/lib/xe/xe_query.c
> +++ b/lib/xe/xe_query.c
> @@ -467,6 +467,22 @@ const char *xe_region_name(uint64_t region)
>   	return vrams[region_idx];
>   }
>   
> +/**
> + * xe_region_class:
> + * @fd: xe device fd
> + * @region: region mask
> + *
> + * Returns class of memory region structure for @region mask.
> + */
> +uint16_t xe_region_class(int fd, uint64_t region)
> +{
> +	struct drm_xe_query_mem_region *memreg;
> +
> +	memreg = xe_mem_region(fd, region);
> +
> +	return memreg->mem_class;
> +}
> +
>   /**
>    * xe_min_page_size:
>    * @fd: xe device fd
> diff --git a/lib/xe/xe_query.h b/lib/xe/xe_query.h
> index ff328ab942..68ca5a680c 100644
> --- a/lib/xe/xe_query.h
> +++ b/lib/xe/xe_query.h
> @@ -85,6 +85,7 @@ struct drm_xe_engine_class_instance *xe_hw_engines(int fd);
>   struct drm_xe_engine_class_instance *xe_hw_engine(int fd, int idx);
>   struct drm_xe_query_mem_region *xe_mem_region(int fd, uint64_t region);
>   const char *xe_region_name(uint64_t region);
> +uint16_t xe_region_class(int fd, uint64_t region);
>   uint32_t xe_min_page_size(int fd, uint64_t region);
>   struct drm_xe_query_config *xe_config(int fd);
>   unsigned int xe_number_hw_engines(int fd);

Re: [igt-dev] [PATCH i-g-t 03/12] lib/drmtest: Add get_intel_driver() helper

[Karolina Stolarek]

On 4.07.2023 11:00, Zbigniew Kempczyński wrote:
> In libraries which diverges i915 and xe code we might use

nit: plural form, or could say something like "In libraries with i915 
and Xe code divergence"

> is_xe_device() or is_i915_device() to distinct code paths.
> But to avoid additional open and string compare we can cache
> this information in data structures.
> 
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> ---
>   lib/drmtest.c | 10 ++++++++++
>   lib/drmtest.h |  1 +
>   2 files changed, 11 insertions(+)
> 
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 5cdb0196d3..e1da66c877 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -151,6 +151,16 @@ bool is_intel_device(int fd)
>   	return is_i915_device(fd) || is_xe_device(fd);
>   }
>   
> +enum intel_driver get_intel_driver(int fd)
> +{
> +	if (is_xe_device(fd))
> +		return INTEL_DRIVER_XE;
> +	else if (is_i915_device(fd))
> +		return INTEL_DRIVER_I915;
> +
> +	igt_assert_f(0, "Device is not handled by Intel driver\n");
> +}

That looks like a helpful function. As a follow up to this series, we 
could update ibb->driver assignment in __intel_bb_create().

But for now:
Reviewed-by: Karolina Stolarek <karolina.stolarek@intel.com>

> +
>   static char _forced_driver[16] = "";
>   
>   /**
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index 9c3ea5d14c..97ab6e759e 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -124,6 +124,7 @@ bool is_nouveau_device(int fd);
>   bool is_vc4_device(int fd);
>   bool is_xe_device(int fd);
>   bool is_intel_device(int fd);
> +enum intel_driver get_intel_driver(int fd);
>   
>   /**
>    * do_or_die:

Re: [igt-dev] [PATCH i-g-t 04/12] lib/xe_util: Return dynamic subtest name for Xe

[Karolina Stolarek]

On 4.07.2023 11:00, Zbigniew Kempczyński wrote:
> For tests which are working on more than one region using name suffix
> like "vram01-system" etc. is common thing. Instead handcrafting this
> naming add xe_memregion_dynamic_subtest_name() function which is
> similar to memregion_dynamic_subtest_name() for i915.
> 
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> ---
>   lib/meson.build  |   3 +-
>   lib/xe/xe_util.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++
>   lib/xe/xe_util.h |  33 +++++++++++++++
>   3 files changed, 139 insertions(+), 1 deletion(-)
>   create mode 100644 lib/xe/xe_util.c
>   create mode 100644 lib/xe/xe_util.h
> 
> diff --git a/lib/meson.build b/lib/meson.build
> index 3e1ecdee2b..1d5b81ac8f 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -105,7 +105,8 @@ lib_sources = [
>   	'xe/xe_compute_square_kernels.c',
>   	'xe/xe_ioctl.c',
>   	'xe/xe_query.c',
> -	'xe/xe_spin.c'
> +	'xe/xe_spin.c',
> +	'xe/xe_util.c',
>   ]
>   
>   lib_deps = [
> diff --git a/lib/xe/xe_util.c b/lib/xe/xe_util.c
> new file mode 100644
> index 0000000000..448b3a3d27
> --- /dev/null
> +++ b/lib/xe/xe_util.c
> @@ -0,0 +1,104 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#include "igt.h"
> +#include "igt_syncobj.h"
> +#include "xe/xe_ioctl.h"
> +#include "xe/xe_query.h"
> +#include "xe/xe_util.h"
> +
> +static bool __region_belongs_to_regions_type(struct drm_xe_query_mem_region *region,
> +					     uint32_t *mem_regions_type,
> +					     int num_regions)
> +{
> +	for (int i = 0; i < num_regions; i++)
> +		if (mem_regions_type[i] == region->mem_class)
> +			return true;
> +	return false;
> +}
> +
> +struct igt_collection *
> +__xe_get_memory_region_set(int xe, uint32_t *mem_regions_type, int num_regions)
> +{
> +	struct drm_xe_query_mem_region *memregion;
> +	struct igt_collection *set = NULL;
> +	uint64_t memreg = all_memory_regions(xe), region;
> +	int count = 0, pos = 0;
> +
> +	xe_for_each_mem_region(xe, memreg, region) {
> +		memregion = xe_mem_region(xe, region);
> +		if (__region_belongs_to_regions_type(memregion,
> +						     mem_regions_type,
> +						     num_regions))
> +			count++;
> +	}
> +
> +	set = igt_collection_create(count);
> +
> +	xe_for_each_mem_region(xe, memreg, region) {
> +		memregion = xe_mem_region(xe, region);
> +		igt_assert(region < (1ull << 31));
> +		if (__region_belongs_to_regions_type(memregion,
> +						     mem_regions_type,
> +						     num_regions)) {
> +			igt_collection_set_value(set, pos++, (int)region);
> +		}
> +	}
> +
> +	igt_assert(count == pos);
> +
> +	return set;
> +}
> +
> +/**
> +  * xe_memregion_dynamic_subtest_name:
> +  * @xe: drm fd of Xe device
> +  * @igt_collection: memory region collection
> +  *
> +  * Function iterates over all memory regions inside the collection (keeped
> +  * in the value field) and generates the name which can be used during dynamic
> +  * subtest creation.
> +  *
> +  * Returns: newly allocated string, has to be freed by caller. Asserts if
> +  * caller tries to create a name using empty collection.
> +  */
> +char *xe_memregion_dynamic_subtest_name(int xe, struct igt_collection *set)

I'm aware that most functions here based on their i915 versions, but I'm 
not sure about this one. Having xe_memregion_dynamic_subtest_name and 
memregion_dynamic_subtest_name seems confusing to me.

Ideally, we would have a core function that does the formatting and 
wrappers that use it. I know that the need for fd in xe_region_class() 
complicated things, but I wonder if we could work around it. For 
example, why can we only store ints in igt_collection_data, and not 
something more generic? Could we switch to a generic pointer, for 
example? That would require more work, I'm aware of it, I'm just 
thinking about potential solutions here.

All the best,
Karolina

> +{
> +	struct igt_collection_data *data;
> +	char *name, *p;
> +	uint32_t region, len;
> +
> +	igt_assert(set && set->size);
> +	/* enough for "name%d-" * n */
> +	len = set->size * 8;
> +	p = name = malloc(len);
> +	igt_assert(name);
> +
> +	for_each_collection_data(data, set) {
> +		struct drm_xe_query_mem_region *memreg;
> +		int r;
> +
> +		region = data->value;
> +		memreg = xe_mem_region(xe, region);
> +
> +		if (XE_IS_CLASS_VRAM(memreg))
> +			r = snprintf(p, len, "%s%d-",
> +				     xe_region_name(region),
> +				     memreg->instance);
> +		else
> +			r = snprintf(p, len, "%s-",
> +				     xe_region_name(region));
> +
> +		igt_assert(r > 0);
> +		p += r;
> +		len -= r;
> +	}
> +
> +	/* remove last '-' */
> +	*(p - 1) = 0;
> +
> +	return name;
> +}
> +
> diff --git a/lib/xe/xe_util.h b/lib/xe/xe_util.h
> new file mode 100644
> index 0000000000..46b7e0d46b
> --- /dev/null
> +++ b/lib/xe/xe_util.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2023 Intel Corporation
> + *
> + */
> +
> +#ifndef XE_UTIL_H
> +#define XE_UTIL_H
> +
> +#include <stdbool.h>
> +#include <stddef.h>
> +#include <stdint.h>
> +#include <xe_drm.h>
> +
> +#define XE_REGION_SMEM XE_MEM_REGION_CLASS_SYSMEM
> +#define XE_REGION_LMEM XE_MEM_REGION_CLASS_VRAM
> +
> +#define XE_IS_SYSMEM_MEMORY_REGION(fd, region) \
> +	(xe_region_class(fd, region) == XE_MEM_REGION_CLASS_SYSMEM)
> +#define XE_IS_VRAM_MEMORY_REGION(fd, region) \
> +	(xe_region_class(fd, region) == XE_MEM_REGION_CLASS_VRAM)
> +
> +struct igt_collection *
> +__xe_get_memory_region_set(int xe, uint32_t *mem_regions_type, int num_regions);
> +
> +#define xe_get_memory_region_set(regions, mem_region_types...) ({ \
> +	unsigned int arr__[] = { mem_region_types }; \
> +	__xe_get_memory_region_set(regions, arr__, ARRAY_SIZE(arr__)); \
> +})
> +
> +char *xe_memregion_dynamic_subtest_name(int xe, struct igt_collection *set);
> +
> +#endif /* XE_UTIL_H */

Re: [igt-dev] [PATCH i-g-t 05/12] lib/xe_util: Add vm bind/unbind helper for Xe

[Karolina Stolarek]

On 4.07.2023 11:00, Zbigniew Kempczyński wrote:
> Before calling exec we need to prepare vm to contain valid entries.
> Bind/unbind in xe expects single bind_op or vector of bind_ops what
> makes preparation of it a little bit inconvinient. Add function
> which iterates over list of xe_object (auxiliary structure which
> describes bind information for object) and performs the bind/unbind
> in one step. It also supports passing syncobj in/out to work in
> pipelined executions.
> 
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> ---
>   lib/xe/xe_util.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++
>   lib/xe/xe_util.h |  21 ++++++--
>   2 files changed, 151 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/xe/xe_util.c b/lib/xe/xe_util.c
> index 448b3a3d27..8552db47d5 100644
> --- a/lib/xe/xe_util.c
> +++ b/lib/xe/xe_util.c
> @@ -102,3 +102,136 @@ char *xe_memregion_dynamic_subtest_name(int xe, struct igt_collection *set)
>   	return name;
>   }
>   
> +#ifdef XEBINDDBG
> +#define bind_info igt_info
> +#define bind_debug igt_debug
> +#else
> +#define bind_info(...) {}
> +#define bind_debug(...) {}
> +#endif
> +
> +static struct drm_xe_vm_bind_op *xe_alloc_bind_ops(struct igt_list_head *obj_list,
> +						   uint32_t *num_ops)
> +{
> +	struct drm_xe_vm_bind_op *bind_ops, *ops;
> +	struct xe_object *obj;
> +	uint32_t num_objects = 0, i = 0, op;
> +
> +	num_objects = 0;

It is already set to 0, you can remove that line

> +	igt_list_for_each_entry(obj, obj_list, link)
> +		num_objects++;
> +
> +	*num_ops = num_objects;
> +	if (!num_objects) {
> +		bind_info(" [nothing to bind]\n");
> +		return NULL;
> +	}
> +
> +	bind_ops = calloc(num_objects, sizeof(*bind_ops));
> +	igt_assert(bind_ops);
> +
> +	igt_list_for_each_entry(obj, obj_list, link) {
> +		ops = &bind_ops[i];
> +
> +		if (obj->bind_op == XE_OBJECT_BIND) {
> +			op = XE_VM_BIND_OP_MAP | XE_VM_BIND_FLAG_ASYNC;
> +			ops->obj = obj->handle;
> +		} else {
> +			op = XE_VM_BIND_OP_UNMAP | XE_VM_BIND_FLAG_ASYNC;
> +		}
> +
> +		ops->op = op;
> +		ops->obj_offset = 0;
> +		ops->addr = obj->offset;
> +		ops->range = obj->size;
> +		ops->region = 0;

Shouldn't we pass an actual region instance here?

> +
> +		bind_info("  [%d]: [%6s] handle: %u, offset: %llx, size: %llx\n",
> +			  i, obj->bind_op == XE_OBJECT_BIND ? "BIND" : "UNBIND",
> +			  ops->obj, (long long)ops->addr, (long long)ops->range);
> +		i++;
> +	}
> +
> +	return bind_ops;
> +}
> +
> +static void __xe_op_bind_async(int xe, uint32_t vm, uint32_t bind_engine,
> +			       struct igt_list_head *obj_list,
> +			       uint32_t sync_in, uint32_t sync_out)
> +{
> +	struct drm_xe_vm_bind_op *bind_ops;
> +	struct drm_xe_sync tabsyncs[2] = {
> +		{ .flags = DRM_XE_SYNC_SYNCOBJ, .handle = sync_in },
> +		{ .flags = DRM_XE_SYNC_SYNCOBJ | DRM_XE_SYNC_SIGNAL, .handle = sync_out },
> +	};
> +	struct drm_xe_sync *syncs;
> +	uint32_t num_binds = 0;
> +	int num_syncs;
> +
> +	bind_info("[Binding to vm: %u]\n", vm);
> +	bind_ops = xe_alloc_bind_ops(obj_list, &num_binds);
> +
> +	if (!num_binds) {
> +		if (sync_out)
> +			syncobj_signal(xe, &sync_out, 1);
> +		return;
> +	}
> +
> +	if (sync_in) {
> +		syncs = tabsyncs;
> +		num_syncs = 2;
> +	} else {
> +		syncs = &tabsyncs[1];
> +		num_syncs = 1;
> +	}
> +
> +	/* User didn't pass sync out, create it and wait for completion */
> +	if (!sync_out)
> +		tabsyncs[1].handle = syncobj_create(xe, 0);
> +
> +	bind_info("[Binding syncobjs: (in: %u, out: %u)]\n",
> +		  tabsyncs[0].handle, tabsyncs[1].handle);
> +
> +	if (num_binds == 1) {
> +		if ((bind_ops[0].op & 0xffff) == XE_VM_BIND_OP_MAP)
> +			xe_vm_bind_async(xe, vm, bind_engine, bind_ops[0].obj, 0,
> +					bind_ops[0].addr, bind_ops[0].range,
> +					syncs, num_syncs);
> +		else
> +			xe_vm_unbind_async(xe, vm, bind_engine, 0,
> +					   bind_ops[0].addr, bind_ops[0].range,
> +					   syncs, num_syncs);
> +	} else {
> +		xe_vm_bind_array(xe, vm, bind_engine, bind_ops,
> +				 num_binds, syncs, num_syncs);
> +	}
> +
> +	if (!sync_out) {
> +		igt_assert_eq(syncobj_wait_err(xe, &tabsyncs[1].handle, 1, INT64_MAX, 0), 0);

That timeout is quite long, but I see that in syncobj_wait.c test we 
have an even higher value... Should factor the CI timeout in when 
picking the fence timeout?

Apart from that one line thingy, the patch looks good, I think.

All the best,
Karolina

> +		syncobj_destroy(xe, tabsyncs[1].handle);
> +	}
> +
> +	free(bind_ops);
> +}
> +
> +/**
> +  * xe_bind_unbind_async:
> +  * @xe: drm fd of Xe device
> +  * @vm: vm to bind/unbind objects to/from
> +  * @bind_engine: bind engine, 0 if default
> +  * @obj_list: list of xe_object
> +  * @sync_in: sync object (fence-in), 0 if there's no input dependency
> +  * @sync_out: sync object (fence-out) to signal on bind/unbind completion,
> +  *            if 0 wait for bind/unbind completion.
> +  *
> +  * Function iterates over xe_object @obj_list, prepares binding operation
> +  * and does bind/unbind in one step. Providing sync_in / sync_out allows
> +  * working in pipelined mode. With sync_in and sync_out set to 0 function
> +  * waits until binding operation is complete.
> +  */
> +void xe_bind_unbind_async(int fd, uint32_t vm, uint32_t bind_engine,
> +			  struct igt_list_head *obj_list,
> +			  uint32_t sync_in, uint32_t sync_out)
> +{
> +	return __xe_op_bind_async(fd, vm, bind_engine, obj_list, sync_in, sync_out);
> +}
> diff --git a/lib/xe/xe_util.h b/lib/xe/xe_util.h
> index 46b7e0d46b..32f309923e 100644
> --- a/lib/xe/xe_util.h
> +++ b/lib/xe/xe_util.h
> @@ -12,9 +12,6 @@
>   #include <stdint.h>
>   #include <xe_drm.h>
>   
> -#define XE_REGION_SMEM XE_MEM_REGION_CLASS_SYSMEM
> -#define XE_REGION_LMEM XE_MEM_REGION_CLASS_VRAM
> -
>   #define XE_IS_SYSMEM_MEMORY_REGION(fd, region) \
>   	(xe_region_class(fd, region) == XE_MEM_REGION_CLASS_SYSMEM)
>   #define XE_IS_VRAM_MEMORY_REGION(fd, region) \
> @@ -30,4 +27,22 @@ __xe_get_memory_region_set(int xe, uint32_t *mem_regions_type, int num_regions);
>   
>   char *xe_memregion_dynamic_subtest_name(int xe, struct igt_collection *set);
>   
> +enum xe_bind_op {
> +	XE_OBJECT_BIND,
> +	XE_OBJECT_UNBIND,
> +};
> +
> +struct xe_object {
> +	uint32_t handle;
> +	uint64_t offset;
> +	uint64_t size;
> +	enum xe_bind_op bind_op;
> +	void *priv;
> +	struct igt_list_head link;
> +};
> +
> +void xe_bind_unbind_async(int fd, uint32_t vm, uint32_t bind_engine,
> +			  struct igt_list_head *obj_list,
> +			  uint32_t sync_in, uint32_t sync_out);
> +
>   #endif /* XE_UTIL_H */

Re: [igt-dev] [PATCH i-g-t 04/12] lib/xe_util: Return dynamic subtest name for Xe

[Zbigniew Kempczyński]

On Wed, Jul 05, 2023 at 02:54:44PM +0200, Karolina Stolarek wrote:
> On 4.07.2023 11:00, Zbigniew Kempczyński wrote:
> > For tests which are working on more than one region using name suffix
> > like "vram01-system" etc. is common thing. Instead handcrafting this
> > naming add xe_memregion_dynamic_subtest_name() function which is
> > similar to memregion_dynamic_subtest_name() for i915.
> > 
> > Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > ---
> >   lib/meson.build  |   3 +-
> >   lib/xe/xe_util.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++
> >   lib/xe/xe_util.h |  33 +++++++++++++++
> >   3 files changed, 139 insertions(+), 1 deletion(-)
> >   create mode 100644 lib/xe/xe_util.c
> >   create mode 100644 lib/xe/xe_util.h
> > 
> > diff --git a/lib/meson.build b/lib/meson.build
> > index 3e1ecdee2b..1d5b81ac8f 100644
> > --- a/lib/meson.build
> > +++ b/lib/meson.build
> > @@ -105,7 +105,8 @@ lib_sources = [
> >   	'xe/xe_compute_square_kernels.c',
> >   	'xe/xe_ioctl.c',
> >   	'xe/xe_query.c',
> > -	'xe/xe_spin.c'
> > +	'xe/xe_spin.c',
> > +	'xe/xe_util.c',
> >   ]
> >   lib_deps = [
> > diff --git a/lib/xe/xe_util.c b/lib/xe/xe_util.c
> > new file mode 100644
> > index 0000000000..448b3a3d27
> > --- /dev/null
> > +++ b/lib/xe/xe_util.c
> > @@ -0,0 +1,104 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright © 2023 Intel Corporation
> > + */
> > +
> > +#include "igt.h"
> > +#include "igt_syncobj.h"
> > +#include "xe/xe_ioctl.h"
> > +#include "xe/xe_query.h"
> > +#include "xe/xe_util.h"
> > +
> > +static bool __region_belongs_to_regions_type(struct drm_xe_query_mem_region *region,
> > +					     uint32_t *mem_regions_type,
> > +					     int num_regions)
> > +{
> > +	for (int i = 0; i < num_regions; i++)
> > +		if (mem_regions_type[i] == region->mem_class)
> > +			return true;
> > +	return false;
> > +}
> > +
> > +struct igt_collection *
> > +__xe_get_memory_region_set(int xe, uint32_t *mem_regions_type, int num_regions)
> > +{
> > +	struct drm_xe_query_mem_region *memregion;
> > +	struct igt_collection *set = NULL;
> > +	uint64_t memreg = all_memory_regions(xe), region;
> > +	int count = 0, pos = 0;
> > +
> > +	xe_for_each_mem_region(xe, memreg, region) {
> > +		memregion = xe_mem_region(xe, region);
> > +		if (__region_belongs_to_regions_type(memregion,
> > +						     mem_regions_type,
> > +						     num_regions))
> > +			count++;
> > +	}
> > +
> > +	set = igt_collection_create(count);
> > +
> > +	xe_for_each_mem_region(xe, memreg, region) {
> > +		memregion = xe_mem_region(xe, region);
> > +		igt_assert(region < (1ull << 31));
> > +		if (__region_belongs_to_regions_type(memregion,
> > +						     mem_regions_type,
> > +						     num_regions)) {
> > +			igt_collection_set_value(set, pos++, (int)region);
> > +		}
> > +	}
> > +
> > +	igt_assert(count == pos);
> > +
> > +	return set;
> > +}
> > +
> > +/**
> > +  * xe_memregion_dynamic_subtest_name:
> > +  * @xe: drm fd of Xe device
> > +  * @igt_collection: memory region collection
> > +  *
> > +  * Function iterates over all memory regions inside the collection (keeped
> > +  * in the value field) and generates the name which can be used during dynamic
> > +  * subtest creation.
> > +  *
> > +  * Returns: newly allocated string, has to be freed by caller. Asserts if
> > +  * caller tries to create a name using empty collection.
> > +  */
> > +char *xe_memregion_dynamic_subtest_name(int xe, struct igt_collection *set)
> 
> I'm aware that most functions here based on their i915 versions, but I'm not
> sure about this one. Having xe_memregion_dynamic_subtest_name and
> memregion_dynamic_subtest_name seems confusing to me.
> 
> Ideally, we would have a core function that does the formatting and wrappers
> that use it. I know that the need for fd in xe_region_class() complicated
> things, but I wonder if we could work around it. For example, why can we
> only store ints in igt_collection_data, and not something more generic?
> Could we switch to a generic pointer, for example? That would require more
> work, I'm aware of it, I'm just thinking about potential solutions here.

Thank you for the review.

Yes, I thought about unifying these wrappers but for this series my main
target is to address allocator support for Xe. I've noticed other helpers
which might be handy (like bo create, bo map, etc) but this series is long
enough to review. I was tempted to put xe code to gem_ccs and gem_exercise_blt
but I resisted and created separate xe_ccs and xe_exercise_blt. Most of the
code is common there but there's no reference code which shows how to use
allocator along with xe so I decided to add new tests without i915 accretions.

--
Zbigniew

> 
> All the best,
> Karolina
> 
> > +{
> > +	struct igt_collection_data *data;
> > +	char *name, *p;
> > +	uint32_t region, len;
> > +
> > +	igt_assert(set && set->size);
> > +	/* enough for "name%d-" * n */
> > +	len = set->size * 8;
> > +	p = name = malloc(len);
> > +	igt_assert(name);
> > +
> > +	for_each_collection_data(data, set) {
> > +		struct drm_xe_query_mem_region *memreg;
> > +		int r;
> > +
> > +		region = data->value;
> > +		memreg = xe_mem_region(xe, region);
> > +
> > +		if (XE_IS_CLASS_VRAM(memreg))
> > +			r = snprintf(p, len, "%s%d-",
> > +				     xe_region_name(region),
> > +				     memreg->instance);
> > +		else
> > +			r = snprintf(p, len, "%s-",
> > +				     xe_region_name(region));
> > +
> > +		igt_assert(r > 0);
> > +		p += r;
> > +		len -= r;
> > +	}
> > +
> > +	/* remove last '-' */
> > +	*(p - 1) = 0;
> > +
> > +	return name;
> > +}
> > +
> > diff --git a/lib/xe/xe_util.h b/lib/xe/xe_util.h
> > new file mode 100644
> > index 0000000000..46b7e0d46b
> > --- /dev/null
> > +++ b/lib/xe/xe_util.h
> > @@ -0,0 +1,33 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2023 Intel Corporation
> > + *
> > + */
> > +
> > +#ifndef XE_UTIL_H
> > +#define XE_UTIL_H
> > +
> > +#include <stdbool.h>
> > +#include <stddef.h>
> > +#include <stdint.h>
> > +#include <xe_drm.h>
> > +
> > +#define XE_REGION_SMEM XE_MEM_REGION_CLASS_SYSMEM
> > +#define XE_REGION_LMEM XE_MEM_REGION_CLASS_VRAM
> > +
> > +#define XE_IS_SYSMEM_MEMORY_REGION(fd, region) \
> > +	(xe_region_class(fd, region) == XE_MEM_REGION_CLASS_SYSMEM)
> > +#define XE_IS_VRAM_MEMORY_REGION(fd, region) \
> > +	(xe_region_class(fd, region) == XE_MEM_REGION_CLASS_VRAM)
> > +
> > +struct igt_collection *
> > +__xe_get_memory_region_set(int xe, uint32_t *mem_regions_type, int num_regions);
> > +
> > +#define xe_get_memory_region_set(regions, mem_region_types...) ({ \
> > +	unsigned int arr__[] = { mem_region_types }; \
> > +	__xe_get_memory_region_set(regions, arr__, ARRAY_SIZE(arr__)); \
> > +})
> > +
> > +char *xe_memregion_dynamic_subtest_name(int xe, struct igt_collection *set);
> > +
> > +#endif /* XE_UTIL_H */

Re: [igt-dev] [PATCH i-g-t 05/12] lib/xe_util: Add vm bind/unbind helper for Xe

[Zbigniew Kempczyński]

On Wed, Jul 05, 2023 at 05:12:23PM +0200, Karolina Stolarek wrote:
> On 4.07.2023 11:00, Zbigniew Kempczyński wrote:
> > Before calling exec we need to prepare vm to contain valid entries.
> > Bind/unbind in xe expects single bind_op or vector of bind_ops what
> > makes preparation of it a little bit inconvinient. Add function
> > which iterates over list of xe_object (auxiliary structure which
> > describes bind information for object) and performs the bind/unbind
> > in one step. It also supports passing syncobj in/out to work in
> > pipelined executions.
> > 
> > Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > ---
> >   lib/xe/xe_util.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++
> >   lib/xe/xe_util.h |  21 ++++++--
> >   2 files changed, 151 insertions(+), 3 deletions(-)
> > 
> > diff --git a/lib/xe/xe_util.c b/lib/xe/xe_util.c
> > index 448b3a3d27..8552db47d5 100644
> > --- a/lib/xe/xe_util.c
> > +++ b/lib/xe/xe_util.c
> > @@ -102,3 +102,136 @@ char *xe_memregion_dynamic_subtest_name(int xe, struct igt_collection *set)
> >   	return name;
> >   }
> > +#ifdef XEBINDDBG
> > +#define bind_info igt_info
> > +#define bind_debug igt_debug
> > +#else
> > +#define bind_info(...) {}
> > +#define bind_debug(...) {}
> > +#endif
> > +
> > +static struct drm_xe_vm_bind_op *xe_alloc_bind_ops(struct igt_list_head *obj_list,
> > +						   uint32_t *num_ops)
> > +{
> > +	struct drm_xe_vm_bind_op *bind_ops, *ops;
> > +	struct xe_object *obj;
> > +	uint32_t num_objects = 0, i = 0, op;
> > +
> > +	num_objects = 0;
> 
> It is already set to 0, you can remove that line
> 
> > +	igt_list_for_each_entry(obj, obj_list, link)
> > +		num_objects++;
> > +
> > +	*num_ops = num_objects;
> > +	if (!num_objects) {
> > +		bind_info(" [nothing to bind]\n");
> > +		return NULL;
> > +	}
> > +
> > +	bind_ops = calloc(num_objects, sizeof(*bind_ops));
> > +	igt_assert(bind_ops);
> > +
> > +	igt_list_for_each_entry(obj, obj_list, link) {
> > +		ops = &bind_ops[i];
> > +
> > +		if (obj->bind_op == XE_OBJECT_BIND) {
> > +			op = XE_VM_BIND_OP_MAP | XE_VM_BIND_FLAG_ASYNC;
> > +			ops->obj = obj->handle;
> > +		} else {
> > +			op = XE_VM_BIND_OP_UNMAP | XE_VM_BIND_FLAG_ASYNC;
> > +		}
> > +
> > +		ops->op = op;
> > +		ops->obj_offset = 0;
> > +		ops->addr = obj->offset;
> > +		ops->range = obj->size;
> > +		ops->region = 0;
> 
> Shouldn't we pass an actual region instance here?
> 

I think this is valid for prefetch mode (see xe_vm_prefetch_async() helper)
so for normal usage 0 is fine.

> > +
> > +		bind_info("  [%d]: [%6s] handle: %u, offset: %llx, size: %llx\n",
> > +			  i, obj->bind_op == XE_OBJECT_BIND ? "BIND" : "UNBIND",
> > +			  ops->obj, (long long)ops->addr, (long long)ops->range);
> > +		i++;
> > +	}
> > +
> > +	return bind_ops;
> > +}
> > +
> > +static void __xe_op_bind_async(int xe, uint32_t vm, uint32_t bind_engine,
> > +			       struct igt_list_head *obj_list,
> > +			       uint32_t sync_in, uint32_t sync_out)
> > +{
> > +	struct drm_xe_vm_bind_op *bind_ops;
> > +	struct drm_xe_sync tabsyncs[2] = {
> > +		{ .flags = DRM_XE_SYNC_SYNCOBJ, .handle = sync_in },
> > +		{ .flags = DRM_XE_SYNC_SYNCOBJ | DRM_XE_SYNC_SIGNAL, .handle = sync_out },
> > +	};
> > +	struct drm_xe_sync *syncs;
> > +	uint32_t num_binds = 0;
> > +	int num_syncs;
> > +
> > +	bind_info("[Binding to vm: %u]\n", vm);
> > +	bind_ops = xe_alloc_bind_ops(obj_list, &num_binds);
> > +
> > +	if (!num_binds) {
> > +		if (sync_out)
> > +			syncobj_signal(xe, &sync_out, 1);
> > +		return;
> > +	}
> > +
> > +	if (sync_in) {
> > +		syncs = tabsyncs;
> > +		num_syncs = 2;
> > +	} else {
> > +		syncs = &tabsyncs[1];
> > +		num_syncs = 1;
> > +	}
> > +
> > +	/* User didn't pass sync out, create it and wait for completion */
> > +	if (!sync_out)
> > +		tabsyncs[1].handle = syncobj_create(xe, 0);
> > +
> > +	bind_info("[Binding syncobjs: (in: %u, out: %u)]\n",
> > +		  tabsyncs[0].handle, tabsyncs[1].handle);
> > +
> > +	if (num_binds == 1) {
> > +		if ((bind_ops[0].op & 0xffff) == XE_VM_BIND_OP_MAP)
> > +			xe_vm_bind_async(xe, vm, bind_engine, bind_ops[0].obj, 0,
> > +					bind_ops[0].addr, bind_ops[0].range,
> > +					syncs, num_syncs);
> > +		else
> > +			xe_vm_unbind_async(xe, vm, bind_engine, 0,
> > +					   bind_ops[0].addr, bind_ops[0].range,
> > +					   syncs, num_syncs);
> > +	} else {
> > +		xe_vm_bind_array(xe, vm, bind_engine, bind_ops,
> > +				 num_binds, syncs, num_syncs);
> > +	}
> > +
> > +	if (!sync_out) {
> > +		igt_assert_eq(syncobj_wait_err(xe, &tabsyncs[1].handle, 1, INT64_MAX, 0), 0);
> 
> That timeout is quite long, but I see that in syncobj_wait.c test we have an
> even higher value... Should factor the CI timeout in when picking the fence
> timeout?

I think infinity is best selection - I mean we cannot move forward
if binding is not complete - we don't handle errors here apart
of asserting if sth wrong has happened. If we stuck on this fence
that's really bad but CI will kill and report a failure in thi case.

Thank you for the review.
--
Zbigniew

> 
> Apart from that one line thingy, the patch looks good, I think.
> 
> All the best,
> Karolina
> 
> > +		syncobj_destroy(xe, tabsyncs[1].handle);
> > +	}
> > +
> > +	free(bind_ops);
> > +}
> > +
> > +/**
> > +  * xe_bind_unbind_async:
> > +  * @xe: drm fd of Xe device
> > +  * @vm: vm to bind/unbind objects to/from
> > +  * @bind_engine: bind engine, 0 if default
> > +  * @obj_list: list of xe_object
> > +  * @sync_in: sync object (fence-in), 0 if there's no input dependency
> > +  * @sync_out: sync object (fence-out) to signal on bind/unbind completion,
> > +  *            if 0 wait for bind/unbind completion.
> > +  *
> > +  * Function iterates over xe_object @obj_list, prepares binding operation
> > +  * and does bind/unbind in one step. Providing sync_in / sync_out allows
> > +  * working in pipelined mode. With sync_in and sync_out set to 0 function
> > +  * waits until binding operation is complete.
> > +  */
> > +void xe_bind_unbind_async(int fd, uint32_t vm, uint32_t bind_engine,
> > +			  struct igt_list_head *obj_list,
> > +			  uint32_t sync_in, uint32_t sync_out)
> > +{
> > +	return __xe_op_bind_async(fd, vm, bind_engine, obj_list, sync_in, sync_out);
> > +}
> > diff --git a/lib/xe/xe_util.h b/lib/xe/xe_util.h
> > index 46b7e0d46b..32f309923e 100644
> > --- a/lib/xe/xe_util.h
> > +++ b/lib/xe/xe_util.h
> > @@ -12,9 +12,6 @@
> >   #include <stdint.h>
> >   #include <xe_drm.h>
> > -#define XE_REGION_SMEM XE_MEM_REGION_CLASS_SYSMEM
> > -#define XE_REGION_LMEM XE_MEM_REGION_CLASS_VRAM
> > -
> >   #define XE_IS_SYSMEM_MEMORY_REGION(fd, region) \
> >   	(xe_region_class(fd, region) == XE_MEM_REGION_CLASS_SYSMEM)
> >   #define XE_IS_VRAM_MEMORY_REGION(fd, region) \
> > @@ -30,4 +27,22 @@ __xe_get_memory_region_set(int xe, uint32_t *mem_regions_type, int num_regions);
> >   char *xe_memregion_dynamic_subtest_name(int xe, struct igt_collection *set);
> > +enum xe_bind_op {
> > +	XE_OBJECT_BIND,
> > +	XE_OBJECT_UNBIND,
> > +};
> > +
> > +struct xe_object {
> > +	uint32_t handle;
> > +	uint64_t offset;
> > +	uint64_t size;
> > +	enum xe_bind_op bind_op;
> > +	void *priv;
> > +	struct igt_list_head link;
> > +};
> > +
> > +void xe_bind_unbind_async(int fd, uint32_t vm, uint32_t bind_engine,
> > +			  struct igt_list_head *obj_list,
> > +			  uint32_t sync_in, uint32_t sync_out);
> > +
> >   #endif /* XE_UTIL_H */

Re: [igt-dev] [PATCH i-g-t 04/12] lib/xe_util: Return dynamic subtest name for Xe

[Karolina Stolarek]

On 6.07.2023 07:17, Zbigniew Kempczyński wrote:
> On Wed, Jul 05, 2023 at 02:54:44PM +0200, Karolina Stolarek wrote:
>> On 4.07.2023 11:00, Zbigniew Kempczyński wrote:
>>> For tests which are working on more than one region using name suffix
>>> like "vram01-system" etc. is common thing. Instead handcrafting this
>>> naming add xe_memregion_dynamic_subtest_name() function which is
>>> similar to memregion_dynamic_subtest_name() for i915.
>>>
>>> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
>>> ---
>>>    lib/meson.build  |   3 +-
>>>    lib/xe/xe_util.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++
>>>    lib/xe/xe_util.h |  33 +++++++++++++++
>>>    3 files changed, 139 insertions(+), 1 deletion(-)
>>>    create mode 100644 lib/xe/xe_util.c
>>>    create mode 100644 lib/xe/xe_util.h
>>>
>>> diff --git a/lib/meson.build b/lib/meson.build
>>> index 3e1ecdee2b..1d5b81ac8f 100644
>>> --- a/lib/meson.build
>>> +++ b/lib/meson.build
>>> @@ -105,7 +105,8 @@ lib_sources = [
>>>    	'xe/xe_compute_square_kernels.c',
>>>    	'xe/xe_ioctl.c',
>>>    	'xe/xe_query.c',
>>> -	'xe/xe_spin.c'
>>> +	'xe/xe_spin.c',
>>> +	'xe/xe_util.c',
>>>    ]
>>>    lib_deps = [
>>> diff --git a/lib/xe/xe_util.c b/lib/xe/xe_util.c
>>> new file mode 100644
>>> index 0000000000..448b3a3d27
>>> --- /dev/null
>>> +++ b/lib/xe/xe_util.c
>>> @@ -0,0 +1,104 @@
>>> +// SPDX-License-Identifier: MIT
>>> +/*
>>> + * Copyright © 2023 Intel Corporation
>>> + */
>>> +
>>> +#include "igt.h"
>>> +#include "igt_syncobj.h"
>>> +#include "xe/xe_ioctl.h"
>>> +#include "xe/xe_query.h"
>>> +#include "xe/xe_util.h"
>>> +
>>> +static bool __region_belongs_to_regions_type(struct drm_xe_query_mem_region *region,
>>> +					     uint32_t *mem_regions_type,
>>> +					     int num_regions)
>>> +{
>>> +	for (int i = 0; i < num_regions; i++)
>>> +		if (mem_regions_type[i] == region->mem_class)
>>> +			return true;
>>> +	return false;
>>> +}
>>> +
>>> +struct igt_collection *
>>> +__xe_get_memory_region_set(int xe, uint32_t *mem_regions_type, int num_regions)
>>> +{
>>> +	struct drm_xe_query_mem_region *memregion;
>>> +	struct igt_collection *set = NULL;
>>> +	uint64_t memreg = all_memory_regions(xe), region;
>>> +	int count = 0, pos = 0;
>>> +
>>> +	xe_for_each_mem_region(xe, memreg, region) {
>>> +		memregion = xe_mem_region(xe, region);
>>> +		if (__region_belongs_to_regions_type(memregion,
>>> +						     mem_regions_type,
>>> +						     num_regions))
>>> +			count++;
>>> +	}
>>> +
>>> +	set = igt_collection_create(count);
>>> +
>>> +	xe_for_each_mem_region(xe, memreg, region) {
>>> +		memregion = xe_mem_region(xe, region);
>>> +		igt_assert(region < (1ull << 31));
>>> +		if (__region_belongs_to_regions_type(memregion,
>>> +						     mem_regions_type,
>>> +						     num_regions)) {
>>> +			igt_collection_set_value(set, pos++, (int)region);
>>> +		}
>>> +	}
>>> +
>>> +	igt_assert(count == pos);
>>> +
>>> +	return set;
>>> +}
>>> +
>>> +/**
>>> +  * xe_memregion_dynamic_subtest_name:
>>> +  * @xe: drm fd of Xe device
>>> +  * @igt_collection: memory region collection
>>> +  *
>>> +  * Function iterates over all memory regions inside the collection (keeped
>>> +  * in the value field) and generates the name which can be used during dynamic
>>> +  * subtest creation.
>>> +  *
>>> +  * Returns: newly allocated string, has to be freed by caller. Asserts if
>>> +  * caller tries to create a name using empty collection.
>>> +  */
>>> +char *xe_memregion_dynamic_subtest_name(int xe, struct igt_collection *set)
>>
>> I'm aware that most functions here based on their i915 versions, but I'm not
>> sure about this one. Having xe_memregion_dynamic_subtest_name and
>> memregion_dynamic_subtest_name seems confusing to me.
>>
>> Ideally, we would have a core function that does the formatting and wrappers
>> that use it. I know that the need for fd in xe_region_class() complicated
>> things, but I wonder if we could work around it. For example, why can we
>> only store ints in igt_collection_data, and not something more generic?
>> Could we switch to a generic pointer, for example? That would require more
>> work, I'm aware of it, I'm just thinking about potential solutions here.
> 
> Thank you for the review.
> 
> Yes, I thought about unifying these wrappers but for this series my main
> target is to address allocator support for Xe. I've noticed other helpers
> which might be handy (like bo create, bo map, etc) but this series is long 
> enough to review. 

Right, I see your point. I'm just worried that once we introduce them, 
we won't have a chance to refactor them later. Still, I don't want it to 
be a stopper for the series, so probably I'll r-b it in v2.

> I was tempted to put xe code to gem_ccs and gem_exercise_blt
> but I resisted and created separate xe_ccs and xe_exercise_blt. Most of the
> code is common there but there's no reference code which shows how to use
> allocator along with xe so I decided to add new tests without i915 accretions.

I'm glad you've resisted that temptation ;) I think it'll be a good 
reference, even if some of the bits are repeated.

All the best,
Karolina

> 
> --
> Zbigniew
> 
>>
>> All the best,
>> Karolina
>>
>>> +{
>>> +	struct igt_collection_data *data;
>>> +	char *name, *p;
>>> +	uint32_t region, len;
>>> +
>>> +	igt_assert(set && set->size);
>>> +	/* enough for "name%d-" * n */
>>> +	len = set->size * 8;
>>> +	p = name = malloc(len);
>>> +	igt_assert(name);
>>> +
>>> +	for_each_collection_data(data, set) {
>>> +		struct drm_xe_query_mem_region *memreg;
>>> +		int r;
>>> +
>>> +		region = data->value;
>>> +		memreg = xe_mem_region(xe, region);
>>> +
>>> +		if (XE_IS_CLASS_VRAM(memreg))
>>> +			r = snprintf(p, len, "%s%d-",
>>> +				     xe_region_name(region),
>>> +				     memreg->instance);
>>> +		else
>>> +			r = snprintf(p, len, "%s-",
>>> +				     xe_region_name(region));
>>> +
>>> +		igt_assert(r > 0);
>>> +		p += r;
>>> +		len -= r;
>>> +	}
>>> +
>>> +	/* remove last '-' */
>>> +	*(p - 1) = 0;
>>> +
>>> +	return name;
>>> +}
>>> +
>>> diff --git a/lib/xe/xe_util.h b/lib/xe/xe_util.h
>>> new file mode 100644
>>> index 0000000000..46b7e0d46b
>>> --- /dev/null
>>> +++ b/lib/xe/xe_util.h
>>> @@ -0,0 +1,33 @@
>>> +/* SPDX-License-Identifier: MIT */
>>> +/*
>>> + * Copyright © 2023 Intel Corporation
>>> + *
>>> + */
>>> +
>>> +#ifndef XE_UTIL_H
>>> +#define XE_UTIL_H
>>> +
>>> +#include <stdbool.h>
>>> +#include <stddef.h>
>>> +#include <stdint.h>
>>> +#include <xe_drm.h>
>>> +
>>> +#define XE_REGION_SMEM XE_MEM_REGION_CLASS_SYSMEM
>>> +#define XE_REGION_LMEM XE_MEM_REGION_CLASS_VRAM
>>> +
>>> +#define XE_IS_SYSMEM_MEMORY_REGION(fd, region) \
>>> +	(xe_region_class(fd, region) == XE_MEM_REGION_CLASS_SYSMEM)
>>> +#define XE_IS_VRAM_MEMORY_REGION(fd, region) \
>>> +	(xe_region_class(fd, region) == XE_MEM_REGION_CLASS_VRAM)
>>> +
>>> +struct igt_collection *
>>> +__xe_get_memory_region_set(int xe, uint32_t *mem_regions_type, int num_regions);
>>> +
>>> +#define xe_get_memory_region_set(regions, mem_region_types...) ({ \
>>> +	unsigned int arr__[] = { mem_region_types }; \
>>> +	__xe_get_memory_region_set(regions, arr__, ARRAY_SIZE(arr__)); \
>>> +})
>>> +
>>> +char *xe_memregion_dynamic_subtest_name(int xe, struct igt_collection *set);
>>> +
>>> +#endif /* XE_UTIL_H */

Re: [igt-dev] [PATCH i-g-t 05/12] lib/xe_util: Add vm bind/unbind helper for Xe

[Karolina Stolarek]

On 6.07.2023 07:34, Zbigniew Kempczyński wrote:
> On Wed, Jul 05, 2023 at 05:12:23PM +0200, Karolina Stolarek wrote:
>> On 4.07.2023 11:00, Zbigniew Kempczyński wrote:
>>> Before calling exec we need to prepare vm to contain valid entries.
>>> Bind/unbind in xe expects single bind_op or vector of bind_ops what
>>> makes preparation of it a little bit inconvinient. Add function
>>> which iterates over list of xe_object (auxiliary structure which
>>> describes bind information for object) and performs the bind/unbind
>>> in one step. It also supports passing syncobj in/out to work in
>>> pipelined executions.
>>>
>>> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
>>> ---
>>>    lib/xe/xe_util.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++
>>>    lib/xe/xe_util.h |  21 ++++++--
>>>    2 files changed, 151 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/lib/xe/xe_util.c b/lib/xe/xe_util.c
>>> index 448b3a3d27..8552db47d5 100644
>>> --- a/lib/xe/xe_util.c
>>> +++ b/lib/xe/xe_util.c
>>> @@ -102,3 +102,136 @@ char *xe_memregion_dynamic_subtest_name(int xe, struct igt_collection *set)
>>>    	return name;
>>>    }
>>> +#ifdef XEBINDDBG
>>> +#define bind_info igt_info
>>> +#define bind_debug igt_debug
>>> +#else
>>> +#define bind_info(...) {}
>>> +#define bind_debug(...) {}
>>> +#endif
>>> +
>>> +static struct drm_xe_vm_bind_op *xe_alloc_bind_ops(struct igt_list_head *obj_list,
>>> +						   uint32_t *num_ops)
>>> +{
>>> +	struct drm_xe_vm_bind_op *bind_ops, *ops;
>>> +	struct xe_object *obj;
>>> +	uint32_t num_objects = 0, i = 0, op;
>>> +
>>> +	num_objects = 0;
>>
>> It is already set to 0, you can remove that line
>>
>>> +	igt_list_for_each_entry(obj, obj_list, link)
>>> +		num_objects++;
>>> +
>>> +	*num_ops = num_objects;
>>> +	if (!num_objects) {
>>> +		bind_info(" [nothing to bind]\n");
>>> +		return NULL;
>>> +	}
>>> +
>>> +	bind_ops = calloc(num_objects, sizeof(*bind_ops));
>>> +	igt_assert(bind_ops);
>>> +
>>> +	igt_list_for_each_entry(obj, obj_list, link) {
>>> +		ops = &bind_ops[i];
>>> +
>>> +		if (obj->bind_op == XE_OBJECT_BIND) {
>>> +			op = XE_VM_BIND_OP_MAP | XE_VM_BIND_FLAG_ASYNC;
>>> +			ops->obj = obj->handle;
>>> +		} else {
>>> +			op = XE_VM_BIND_OP_UNMAP | XE_VM_BIND_FLAG_ASYNC;
>>> +		}
>>> +
>>> +		ops->op = op;
>>> +		ops->obj_offset = 0;
>>> +		ops->addr = obj->offset;
>>> +		ops->range = obj->size;
>>> +		ops->region = 0;
>>
>> Shouldn't we pass an actual region instance here?
>>
> 
> I think this is valid for prefetch mode (see xe_vm_prefetch_async() helper)
> so for normal usage 0 is fine.

If I understand correctly, this will bring BO to the system memory, so 
it should be ok. Also, why prefetch? It looks like we're working with 
map and unmap here.

All the best,
Karolina

> 
>>> +
>>> +		bind_info("  [%d]: [%6s] handle: %u, offset: %llx, size: %llx\n",
>>> +			  i, obj->bind_op == XE_OBJECT_BIND ? "BIND" : "UNBIND",
>>> +			  ops->obj, (long long)ops->addr, (long long)ops->range);
>>> +		i++;
>>> +	}
>>> +
>>> +	return bind_ops;
>>> +}
>>> +
>>> +static void __xe_op_bind_async(int xe, uint32_t vm, uint32_t bind_engine,
>>> +			       struct igt_list_head *obj_list,
>>> +			       uint32_t sync_in, uint32_t sync_out)
>>> +{
>>> +	struct drm_xe_vm_bind_op *bind_ops;
>>> +	struct drm_xe_sync tabsyncs[2] = {
>>> +		{ .flags = DRM_XE_SYNC_SYNCOBJ, .handle = sync_in },
>>> +		{ .flags = DRM_XE_SYNC_SYNCOBJ | DRM_XE_SYNC_SIGNAL, .handle = sync_out },
>>> +	};
>>> +	struct drm_xe_sync *syncs;
>>> +	uint32_t num_binds = 0;
>>> +	int num_syncs;
>>> +
>>> +	bind_info("[Binding to vm: %u]\n", vm);
>>> +	bind_ops = xe_alloc_bind_ops(obj_list, &num_binds);
>>> +
>>> +	if (!num_binds) {
>>> +		if (sync_out)
>>> +			syncobj_signal(xe, &sync_out, 1);
>>> +		return;
>>> +	}
>>> +
>>> +	if (sync_in) {
>>> +		syncs = tabsyncs;
>>> +		num_syncs = 2;
>>> +	} else {
>>> +		syncs = &tabsyncs[1];
>>> +		num_syncs = 1;
>>> +	}
>>> +
>>> +	/* User didn't pass sync out, create it and wait for completion */
>>> +	if (!sync_out)
>>> +		tabsyncs[1].handle = syncobj_create(xe, 0);
>>> +
>>> +	bind_info("[Binding syncobjs: (in: %u, out: %u)]\n",
>>> +		  tabsyncs[0].handle, tabsyncs[1].handle);
>>> +
>>> +	if (num_binds == 1) {
>>> +		if ((bind_ops[0].op & 0xffff) == XE_VM_BIND_OP_MAP)
>>> +			xe_vm_bind_async(xe, vm, bind_engine, bind_ops[0].obj, 0,
>>> +					bind_ops[0].addr, bind_ops[0].range,
>>> +					syncs, num_syncs);
>>> +		else
>>> +			xe_vm_unbind_async(xe, vm, bind_engine, 0,
>>> +					   bind_ops[0].addr, bind_ops[0].range,
>>> +					   syncs, num_syncs);
>>> +	} else {
>>> +		xe_vm_bind_array(xe, vm, bind_engine, bind_ops,
>>> +				 num_binds, syncs, num_syncs);
>>> +	}
>>> +
>>> +	if (!sync_out) {
>>> +		igt_assert_eq(syncobj_wait_err(xe, &tabsyncs[1].handle, 1, INT64_MAX, 0), 0);
>>
>> That timeout is quite long, but I see that in syncobj_wait.c test we have an
>> even higher value... Should factor the CI timeout in when picking the fence
>> timeout?
> 
> I think infinity is best selection - I mean we cannot move forward
> if binding is not complete - we don't handle errors here apart
> of asserting if sth wrong has happened. If we stuck on this fence
> that's really bad but CI will kill and report a failure in thi case.
> 
> Thank you for the review.
> --
> Zbigniew
> 
>>
>> Apart from that one line thingy, the patch looks good, I think.
>>
>> All the best,
>> Karolina
>>
>>> +		syncobj_destroy(xe, tabsyncs[1].handle);
>>> +	}
>>> +
>>> +	free(bind_ops);
>>> +}
>>> +
>>> +/**
>>> +  * xe_bind_unbind_async:
>>> +  * @xe: drm fd of Xe device
>>> +  * @vm: vm to bind/unbind objects to/from
>>> +  * @bind_engine: bind engine, 0 if default
>>> +  * @obj_list: list of xe_object
>>> +  * @sync_in: sync object (fence-in), 0 if there's no input dependency
>>> +  * @sync_out: sync object (fence-out) to signal on bind/unbind completion,
>>> +  *            if 0 wait for bind/unbind completion.
>>> +  *
>>> +  * Function iterates over xe_object @obj_list, prepares binding operation
>>> +  * and does bind/unbind in one step. Providing sync_in / sync_out allows
>>> +  * working in pipelined mode. With sync_in and sync_out set to 0 function
>>> +  * waits until binding operation is complete.
>>> +  */
>>> +void xe_bind_unbind_async(int fd, uint32_t vm, uint32_t bind_engine,
>>> +			  struct igt_list_head *obj_list,
>>> +			  uint32_t sync_in, uint32_t sync_out)
>>> +{
>>> +	return __xe_op_bind_async(fd, vm, bind_engine, obj_list, sync_in, sync_out);
>>> +}
>>> diff --git a/lib/xe/xe_util.h b/lib/xe/xe_util.h
>>> index 46b7e0d46b..32f309923e 100644
>>> --- a/lib/xe/xe_util.h
>>> +++ b/lib/xe/xe_util.h
>>> @@ -12,9 +12,6 @@
>>>    #include <stdint.h>
>>>    #include <xe_drm.h>
>>> -#define XE_REGION_SMEM XE_MEM_REGION_CLASS_SYSMEM
>>> -#define XE_REGION_LMEM XE_MEM_REGION_CLASS_VRAM
>>> -
>>>    #define XE_IS_SYSMEM_MEMORY_REGION(fd, region) \
>>>    	(xe_region_class(fd, region) == XE_MEM_REGION_CLASS_SYSMEM)
>>>    #define XE_IS_VRAM_MEMORY_REGION(fd, region) \
>>> @@ -30,4 +27,22 @@ __xe_get_memory_region_set(int xe, uint32_t *mem_regions_type, int num_regions);
>>>    char *xe_memregion_dynamic_subtest_name(int xe, struct igt_collection *set);
>>> +enum xe_bind_op {
>>> +	XE_OBJECT_BIND,
>>> +	XE_OBJECT_UNBIND,
>>> +};
>>> +
>>> +struct xe_object {
>>> +	uint32_t handle;
>>> +	uint64_t offset;
>>> +	uint64_t size;
>>> +	enum xe_bind_op bind_op;
>>> +	void *priv;
>>> +	struct igt_list_head link;
>>> +};
>>> +
>>> +void xe_bind_unbind_async(int fd, uint32_t vm, uint32_t bind_engine,
>>> +			  struct igt_list_head *obj_list,
>>> +			  uint32_t sync_in, uint32_t sync_out);
>>> +
>>>    #endif /* XE_UTIL_H */