[igt-dev] [PATCH i-g-t 0/4] Add copy basic test to exercise blt commands

[igt-dev] [PATCH i-g-t 0/4] Add copy basic test to exercise blt commands

[sai.gowtham.ch]

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

Add copy basic test which exercies mem-se and mem-copy commands, this
patch series involves in following changes:

1. Add copy basic test to exercise blt commands.
2. check if blt commands are supported by the platforms.
3. Add copy commands to blt_cmd_type.
4. Add copy commands MEM_SET_CMD and MEM_COPY_CMD in the lib.

Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Sai Gowtham Ch (4):
  intel/xe_copy_basic: Add copy basic test to exercise blt commands
  lib/intel_blt: check if blt commands are supported by the platforms
  lib/intel_cmds_info: Add copy commands to blt_cmd_type
  lib/intel_reg: Add copy commands in the lib

 lib/intel_blt.c             |  32 ++++
 lib/intel_blt.h             |   2 +
 lib/intel_cmds_info.h       |   2 +
 lib/intel_reg.h             |   4 +
 tests/intel/xe_copy_basic.c | 284 ++++++++++++++++++++++++++++++++++++
 tests/meson.build           |   1 +
 6 files changed, 325 insertions(+)
 create mode 100644 tests/intel/xe_copy_basic.c

-- 
2.39.1

[igt-dev] [PATCH i-g-t 0/4] Add copy basic test to exercise blt commands

[sai.gowtham.ch]

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

Add copy basic test which exercies mem-se and mem-copy commands, this
patch series involves in following changes:

1. Add copy basic test to exercise blt commands.
2. check if blt commands are supported by the platforms.
3. Add wrappers for batch preparation and submit exec.
3. Add copy commands to blt_cmd_type.
4. Add copy commands MEM_SET_CMD and MEM_COPY_CMD in the lib.

Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Sai Gowtham Ch (4):
  lib/intel_cmds_info: Add copy commands to blt_cmd_type
  lib/intel_reg: Add copy commands in the lib
  lib/intel_blt: Add wrappers to prepare batch buffers and submit exec
  intel/xe_copy_basic: Add copy basic test to exercise blt commands

 lib/intel_blt.c             | 210 ++++++++++++++++++++++++++++++++++++
 lib/intel_blt.h             |  20 ++++
 lib/intel_cmds_info.h       |   2 +
 lib/intel_reg.h             |   4 +
 tests/intel/xe_copy_basic.c | 199 ++++++++++++++++++++++++++++++++++
 tests/meson.build           |   1 +
 6 files changed, 436 insertions(+)
 create mode 100644 tests/intel/xe_copy_basic.c

-- 
2.39.1

[igt-dev] [PATCH i-g-t 1/4] lib/intel_cmds_info: Add copy commands to blt_cmd_type

[sai.gowtham.ch]

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

Add copy commands to blt_cmd_type which is used latter,
to check it they are supported by fd.

Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
---
 lib/intel_cmds_info.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/intel_cmds_info.h b/lib/intel_cmds_info.h
index 91d0f15ec..ec8312d56 100644
--- a/lib/intel_cmds_info.h
+++ b/lib/intel_cmds_info.h
@@ -20,6 +20,8 @@ enum blt_tiling_type {
 
 enum blt_cmd_type {
 	SRC_COPY,
+	MEM_SET,
+	MEM_COPY,
 	XY_SRC_COPY,
 	XY_FAST_COPY,
 	XY_BLOCK_COPY,
-- 
2.39.1

[igt-dev] [PATCH i-g-t 2/4] lib/intel_reg: Add copy commands in the lib

[sai.gowtham.ch]

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

Add memory copy and set commands to the lib.

Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
---
 lib/intel_reg.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/intel_reg.h b/lib/intel_reg.h
index 3bf3676dc..322aec9fd 100644
--- a/lib/intel_reg.h
+++ b/lib/intel_reg.h
@@ -2564,6 +2564,10 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 #define XY_FAST_COLOR_BLT		((0x2<<29)|(0x44<<22)|0xe)
 
+/* RAW memory commands */
+#define MEM_COPY_CMD                    ((0x2 << 29)|(0x5a << 22)|0x8)
+#define MEM_SET_CMD                     ((0x2 << 29)|(0x5b << 22)|0x5)
+
 #define XY_FAST_COPY_BLT				((2<<29)|(0x42<<22)|0x8)
 /* dword 0 */
 #define   XY_FAST_COPY_SRC_TILING_LINEAR		(0 << 20)
-- 
2.39.1

[igt-dev] [PATCH i-g-t 3/4] lib/intel_blt: Add wrappers to prepare batch buffers and submit exec

[sai.gowtham.ch]

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

This commit has couple of changes in it:
1. Adding wrapper for mem-set and mem-copy instructions to
   prepare batch buffers and submit exec, (blt_mem_copy, blt_set_mem,
   emit_blt_mem_copy, emit,blt_set_mem).

2. Add check to see if blt commands are supported by the platforms.

Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
---
 lib/intel_blt.c | 210 ++++++++++++++++++++++++++++++++++++++++++++++++
 lib/intel_blt.h |  20 +++++
 2 files changed, 230 insertions(+)

diff --git a/lib/intel_blt.c b/lib/intel_blt.c
index 429511920..88ad7cae8 100644
--- a/lib/intel_blt.c
+++ b/lib/intel_blt.c
@@ -13,12 +13,14 @@
 #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"
 
 #define BITRANGE(start, end) (end - start + 1)
 #define GET_CMDS_INFO(__fd) intel_get_cmds_info(intel_get_drm_devid(__fd))
+#define MEM_COPY_MOCS_SHIFT                     25
 
 /* Blitter tiling definitions sanity checks */
 static_assert(T_LINEAR == I915_TILING_NONE, "Linear definitions have to match");
@@ -289,6 +291,38 @@ bool blt_has_block_copy(int fd)
 	return blt_supports_command(cmds_info, XY_BLOCK_COPY);
 }
 
+/**
+ * blt_has_mem_copy
+ * @fd: drm fd
+ *
+ * Check if mem copy is supported by @fd device
+ *
+ * Returns:
+ * true if it does, false otherwise.
+ */
+bool blt_has_mem_copy(int fd)
+{
+	const struct intel_cmds_info *cmds_info = GET_CMDS_INFO(fd);
+
+	return blt_supports_command(cmds_info, MEM_COPY);
+}
+
+/**
+ * blt_has_mem_set
+ * @fd: drm fd
+ *
+ * Check if mem set is supported by @fd device
+ *
+ * Returns:
+ * true if it does, false otherwise.
+ */
+bool blt_has_mem_set(int fd)
+{
+	const struct intel_cmds_info *cmds_info = GET_CMDS_INFO(fd);
+
+	return blt_supports_command(cmds_info, MEM_SET);
+}
+
 /**
  * blt_has_fast_copy
  * @fd: drm fd
@@ -1380,6 +1414,182 @@ int blt_fast_copy(int fd,
 	return ret;
 }
 
+void emit_blt_mem_copy(int fd, uint64_t ahnd, const struct blt_copy_data *blt, uint32_t col_size)
+{
+	uint64_t dst_offset, src_offset, alignment;
+	int i;
+	uint8_t src_mocs = intel_get_uc_mocs(fd);
+	uint8_t dst_mocs = src_mocs;
+	uint32_t size;
+	struct {
+		uint32_t batch[12];
+		uint32_t data;
+	} *data;
+
+	alignment = xe_get_default_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);
+	size = blt->src.size;
+
+	data = xe_bo_map(fd, blt->bb.handle, blt->bb.size);
+
+	i = 0;
+	data->batch[i++] = MEM_COPY_CMD;
+	data->batch[i++] = size - 1;
+	data->batch[i++] = col_size - 1;
+	data->batch[i++] = 0;
+	data->batch[i++] = 0;
+	data->batch[i++] = src_offset;
+	data->batch[i++] = src_offset << 32;
+	data->batch[i++] = dst_offset;
+	data->batch[i++] = dst_offset << 32;
+	data->batch[i++] = src_mocs << MEM_COPY_MOCS_SHIFT | dst_mocs;
+	data->batch[i++] = MI_BATCH_BUFFER_END;
+	data->batch[i++] = MI_NOOP;
+
+	igt_assert(i <= ARRAY_SIZE(data->batch));
+}
+
+/**
+ * blt_mem_copy:
+ * @fd: drm fd
+ * @ctx: intel_ctx_t context
+ * @e: blitter engine for @ctx
+ * @ahnd: allocator handle
+ * @blt: blitter data for mem-copy.
+ *
+ * Function does mem blit between @src and @dst described in @blt object.
+ *
+ * Returns:
+ * execbuffer status.
+ */
+int blt_mem_copy(int fd, const intel_ctx_t *ctx,
+		 const struct intel_execution_engine2 *e,
+		 uint64_t ahnd,
+		 const struct blt_copy_data *blt,
+		 uint32_t col_size)
+{
+	struct drm_i915_gem_execbuffer2 execbuf = {};
+	struct drm_i915_gem_exec_object2 obj[3] = {};
+	uint64_t dst_offset, src_offset, bb_offset, alignment;
+	int ret;
+
+	alignment = get_default_alignment(fd, blt->driver);
+	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_mem_copy(fd, ahnd, blt, col_size);
+
+	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;
+}
+
+void emit_blt_set_mem(int fd, uint64_t ahnd, const struct blt_copy_data *blt,
+		      uint32_t fill_data, uint32_t height)
+{
+	uint64_t dst_offset, alignment;
+	int b;
+	uint8_t dst_mocs = intel_get_uc_mocs(fd);
+	uint32_t size;
+	struct {
+		uint32_t batch[12];
+		uint32_t data;
+	} *data;
+
+	alignment = xe_get_default_alignment(fd);
+	dst_offset = get_offset(ahnd, blt->dst.handle, blt->dst.size, alignment);
+	size = blt->dst.size;
+
+	data = xe_bo_map(fd, blt->dst.handle, blt->dst.size);
+
+	b = 0;
+	data->batch[b++] = MEM_SET_CMD;
+	data->batch[b++] = size - 1;
+	data->batch[b++] = height;
+	data->batch[b++] = 0;
+	data->batch[b++] = dst_offset;
+	data->batch[b++] = dst_offset << 32;
+	data->batch[b++] = (fill_data << 24) | dst_mocs;
+	data->batch[b++] = MI_BATCH_BUFFER_END;
+	data->batch[b++] = MI_NOOP;
+
+	igt_assert(b <= ARRAY_SIZE(data->batch));
+}
+
+/**
+ * blt_set_mem:
+ * @fd: drm fd
+ * @ctx: intel_ctx_t context
+ * @e: blitter engine for @ctx
+ * @ahnd: allocator handle
+ * @blt: blitter data for mem-set.
+ *
+ * Function does mem set blit in described @blt object.
+ *
+ * Returns:
+ * execbuffer status.
+ */
+int blt_set_mem(int fd, const intel_ctx_t *ctx,
+		const struct intel_execution_engine2 *e,
+		uint64_t ahnd,
+		const struct blt_copy_data *blt,
+		uint32_t height,
+		uint32_t fill_data)
+{
+	struct drm_i915_gem_execbuffer2 execbuf = {};
+	struct drm_i915_gem_exec_object2 obj[2] = {};
+	uint64_t dst_offset, bb_offset, alignment;
+	int ret;
+
+	alignment = get_default_alignment(fd, blt->driver);
+	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_set_mem(fd, ahnd, blt, fill_data, height);
+
+	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(bb_offset);
+		obj[0].handle = blt->dst.handle;
+		obj[1].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;
+		execbuf.buffer_count = 2;
+		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->bb.handle);
+	}
+	return ret;
+}
+
 void blt_set_geom(struct blt_copy_object *obj, uint32_t pitch,
 		  int16_t x1, int16_t y1, int16_t x2, int16_t y2,
 		  uint16_t x_offset, uint16_t y_offset)
diff --git a/lib/intel_blt.h b/lib/intel_blt.h
index b8b3d724d..79edee0d5 100644
--- a/lib/intel_blt.h
+++ b/lib/intel_blt.h
@@ -175,6 +175,8 @@ bool blt_cmd_has_property(const struct intel_cmds_info *cmds_info,
 			  uint32_t prop);
 
 bool blt_has_block_copy(int fd);
+bool blt_has_mem_copy(int fd);
+bool blt_has_mem_set(int fd);
 bool blt_has_fast_copy(int fd);
 bool blt_has_xy_src_copy(int fd);
 bool blt_has_xy_color(int fd);
@@ -229,6 +231,24 @@ int blt_fast_copy(int fd,
 		  uint64_t ahnd,
 		  const struct blt_copy_data *blt);
 
+void emit_blt_mem_copy(int fd, uint64_t ahnd,
+		       const struct blt_copy_data *blt,
+		       uint32_t col_size);
+
+int blt_mem_copy(int fd, const intel_ctx_t *ctx,
+		const struct intel_execution_engine2 *e,
+		 uint64_t ahnd,
+		 const struct blt_copy_data *blt,
+		 uint32_t col_size);
+
+void emit_blt_set_mem(int fd, uint64_t ahnd, const struct blt_copy_data *blt,
+		      uint32_t fill_data, uint32_t height);
+
+int blt_set_mem(int fd, const intel_ctx_t *ctx,
+		const struct intel_execution_engine2 *e, uint64_t ahnd,
+		const struct blt_copy_data *blt, uint32_t height,
+		uint32_t fill_data);
+
 void blt_set_geom(struct blt_copy_object *obj, uint32_t pitch,
 		  int16_t x1, int16_t y1, int16_t x2, int16_t y2,
 		  uint16_t x_offset, uint16_t y_offset);
-- 
2.39.1

[igt-dev] [PATCH i-g-t 4/4] intel/xe_copy_basic: Add copy basic test to exercise blt commands

[sai.gowtham.ch]

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

Add copy basic test to exercise copy commands like mem-copy and mem-set.

Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
---
 tests/intel/xe_copy_basic.c | 199 ++++++++++++++++++++++++++++++++++++
 tests/meson.build           |   1 +
 2 files changed, 200 insertions(+)
 create mode 100644 tests/intel/xe_copy_basic.c

diff --git a/tests/intel/xe_copy_basic.c b/tests/intel/xe_copy_basic.c
new file mode 100644
index 000000000..0014b022e
--- /dev/null
+++ b/tests/intel/xe_copy_basic.c
@@ -0,0 +1,199 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2023 Intel Corporation
+ *
+ * Authors:
+ *	Sai Gowtham Ch <sai.gowtham.ch@intel.com>
+ */
+
+#include "igt.h"
+#include "lib/igt_syncobj.h"
+#include "intel_blt.h"
+#include "lib/intel_cmds_info.h"
+#include "lib/intel_mocs.h"
+#include "lib/intel_reg.h"
+#include "xe/xe_ioctl.h"
+#include "xe/xe_query.h"
+#include "xe/xe_util.h"
+
+/**
+ * TEST: Test to validate copy commands on xe
+ * Category: Software building block
+ * Sub-category: Copy
+ * Functionality: blitter
+ */
+
+/**
+ * SUBTEST: mem-copy
+ * Description: Test validates MEM_COPY command, it takes various
+ *		parameters needed for the filling batch buffer for MEM_COPY command.
+ * Test category: functionality test
+ */
+static void
+igt_mem_copy(int fd, uint32_t src_handle, uint32_t dst_handle,
+	     const intel_ctx_t *ctx, uint32_t row_size,
+	     uint32_t col_size, uint32_t region)
+{
+	struct blt_copy_data blt = {};
+	uint64_t bb_size = xe_get_default_alignment(fd);
+	uint64_t ahnd = intel_allocator_open_full(fd, ctx->vm, 0, 0,
+						  INTEL_ALLOCATOR_SIMPLE,
+						  ALLOC_STRATEGY_LOW_TO_HIGH, 0);
+	uint32_t bb;
+	int result;
+	uint8_t src_mocs = intel_get_uc_mocs(fd);
+	uint8_t dst_mocs = src_mocs;
+
+	bb = xe_bo_create_flags(fd, 0, bb_size, region);
+
+	blt_copy_init(fd, &blt);
+	blt_set_object(&blt.src, src_handle, row_size, region, src_mocs, 0,
+		       T_LINEAR, COMPRESSION_DISABLED);
+	blt_set_object(&blt.dst, dst_handle, row_size, region, dst_mocs, 0,
+		       T_LINEAR, COMPRESSION_DISABLED);
+	blt_set_batch(&blt.bb, bb, bb_size, region);
+	igt_assert(blt.src.size == blt.dst.size);
+
+	blt_mem_copy(fd, ctx, NULL, ahnd, &blt, col_size);
+	result = memcmp(blt.src.ptr, blt.dst.ptr, blt.src.size);
+	igt_assert_f(!result, "source and destination differ\n");
+
+	intel_allocator_bind(ahnd, 0, 0);
+	gem_close(fd, bb);
+	put_ahnd(ahnd);
+}
+
+/**
+ * SUBTEST: mem-set
+ * Description: Test validates MEM_SET command.
+ * Test category: functionality test
+ */
+static void igt_mem_set(int fd, uint32_t dst_handle, const intel_ctx_t *ctx,
+			size_t size, uint32_t height,
+			uint32_t fill_data, uint32_t region)
+{
+	struct blt_copy_data blt = {};
+	uint64_t bb_size = xe_get_default_alignment(fd);
+	uint64_t ahnd = intel_allocator_open_full(fd, ctx->vm, 0, 0,
+						  INTEL_ALLOCATOR_SIMPLE,
+						  ALLOC_STRATEGY_LOW_TO_HIGH, 0);
+	uint32_t bb;
+	int result;
+	uint8_t dst_mocs = intel_get_uc_mocs(fd);
+
+	bb = xe_bo_create_flags(fd, 0, bb_size, region);
+	blt_copy_init(fd, &blt);
+	blt_set_object(&blt.dst, dst_handle, size, region, dst_mocs, 0,
+		       T_LINEAR, COMPRESSION_DISABLED);
+	blt_set_batch(&blt.bb, bb, bb_size, region);
+	blt_set_mem(fd, ctx, NULL, ahnd, &blt, height, fill_data);
+	result = memcmp(blt.src.ptr, blt.dst.ptr, blt.src.size);
+	igt_assert_f(!result, "source and destination differ\n");
+
+	intel_allocator_bind(ahnd, 0, 0);
+	gem_close(fd, bb);
+	put_ahnd(ahnd);
+}
+
+static void copy_test(int fd, uint32_t size, enum blt_cmd_type cmd,
+		      struct drm_xe_engine_class_instance *hwe, uint32_t region)
+{
+	uint32_t src_handle, dst_handle, vm, exec_queue, src_size, dst_size;
+	char c = 'a';
+	uint32_t bo_size = ALIGN(size + xe_cs_prefetch_size(fd), xe_get_default_alignment(fd));
+	uint32_t temp_buffer[bo_size];
+	const intel_ctx_t *ctx;
+
+	src_handle = xe_bo_create_flags(fd, 0, bo_size, region);
+	dst_handle = xe_bo_create_flags(fd, 0, bo_size, region);
+	vm = xe_vm_create(fd, 0, 0);
+	exec_queue = xe_exec_queue_create(fd, vm, hwe, 0);
+	ctx = intel_ctx_xe(fd, vm, exec_queue, 0, 0, 0);
+
+	src_size = bo_size;
+	dst_size = bo_size;
+
+	/* Fill a pattern in the buffer */
+	for (int i = 0; i < bo_size; i++) {
+		temp_buffer[i] = c++ % 16;
+		temp_buffer[i] |= (c++ % 16) << 8;
+		temp_buffer[i] |= (c++ % 16) << 16;
+		temp_buffer[i] |= (c++ % 16) << 24;
+	}
+
+	if (cmd == MEM_COPY) {
+		igt_mem_copy(fd,
+			     src_handle,/*src_handle*/
+			     dst_handle,/*dst_handle*/
+			     ctx,
+			     src_size,/*row_size*/
+			     1,/*col_size*/
+			     region);
+	} else if (cmd == MEM_SET) {
+		igt_mem_set(fd,
+			    dst_handle, /*dst_handle*/
+			    ctx,
+			    dst_size,/*width*/
+			    1,/*height*/
+			    temp_buffer[0] & 0xff,/*fill_data*/
+			    region);
+		src_size = 1;
+	}
+
+	gem_close(fd, src_handle);
+	gem_close(fd, dst_handle);
+	xe_exec_queue_destroy(fd, exec_queue);
+	xe_vm_destroy(fd, vm);
+}
+
+igt_main
+{
+	struct drm_xe_engine_class_instance *hwe;
+	int fd;
+	struct igt_collection *set, *regions;
+	uint32_t region;
+	uint64_t size[] = {0xFD, 0x369, 0x369, 0x3FFF, 0xFFFF, 0x1FFFF, 0x3FFFF};
+
+	igt_fixture {
+		fd = drm_open_driver(DRIVER_XE);
+		xe_device_get(fd);
+		set = xe_get_memory_region_set(fd,
+					       XE_MEM_REGION_CLASS_SYSMEM,
+					       XE_MEM_REGION_CLASS_VRAM);
+	}
+
+	igt_subtest_with_dynamic_f("mem-copy") {
+		igt_require(blt_has_mem_copy(fd));
+		for_each_variation_r(regions, 1, set) {
+			region = igt_collection_get_value(regions, 0);
+			xe_for_each_hw_engine(fd, hwe) {
+				for (int i = 0; i < ARRAY_SIZE(size); i++) {
+					igt_dynamic_f("size-0x%lx", size[i]) {
+						copy_test(fd, size[i],
+							  MEM_COPY, hwe,
+							  region);
+					}
+				}
+			}
+		}
+	}
+
+	igt_subtest_with_dynamic_f("mem-set") {
+		igt_require(blt_has_mem_set(fd));
+		for_each_variation_r(regions, 1, set) {
+			region = igt_collection_get_value(regions, 0);
+			xe_for_each_hw_engine(fd, hwe) {
+				for (int i = 0; i < ARRAY_SIZE(size); i++) {
+					igt_dynamic_f("size-0x%lx", size[i]) {
+						copy_test(fd, size[i],
+							  MEM_SET, hwe, region);
+					}
+				}
+			}
+		}
+	}
+
+	igt_fixture {
+		drm_close_driver(fd);
+	}
+}
diff --git a/tests/meson.build b/tests/meson.build
index 7201958b7..753cde31e 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -274,6 +274,7 @@ intel_xe_progs = [
 	'xe_ccs',
 	'xe_create',
 	'xe_compute',
+	'xe_copy_basic',
 	'xe_dma_buf_sync',
 	'xe_debugfs',
 	'xe_evict',
-- 
2.39.1

[igt-dev] ✓ CI.xeBAT: success for Add copy basic test to exercise blt commands (rev3)

[Patchwork]

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

== Series Details ==

Series: Add copy basic test to exercise blt commands (rev3)
URL   : https://patchwork.freedesktop.org/series/122615/
State : success

== Summary ==

CI Bug Log - changes from XEIGT_7488_BAT -> XEIGTPW_9795_BAT
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

  No changes in participating hosts

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_flip@basic-flip-vs-wf_vblank@d-edp1:
    - bat-adlp-7:         [PASS][1] -> [FAIL][2] ([Intel XE#480]) +1 other test fail
   [1]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7488/bat-adlp-7/igt@kms_flip@basic-flip-vs-wf_vblank@d-edp1.html
   [2]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_9795/bat-adlp-7/igt@kms_flip@basic-flip-vs-wf_vblank@d-edp1.html

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

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


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

  * IGT: IGT_7488 -> IGTPW_9795
  * Linux: xe-373-aeac46cfaebff413254ac07837b97370c7ed3957 -> xe-375-4a9681392cecd56a372d06b648f7b3a37fdd5c63

  IGTPW_9795: 9795
  IGT_7488: 099e058c5dfb7a49942edf03cae88a52a77077a3 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  xe-373-aeac46cfaebff413254ac07837b97370c7ed3957: aeac46cfaebff413254ac07837b97370c7ed3957
  xe-375-4a9681392cecd56a372d06b648f7b3a37fdd5c63: 4a9681392cecd56a372d06b648f7b3a37fdd5c63

== Logs ==

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

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

[igt-dev] ✗ Fi.CI.BAT: failure for Add copy basic test to exercise blt commands (rev3)

[Patchwork]

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

== Series Details ==

Series: Add copy basic test to exercise blt commands (rev3)
URL   : https://patchwork.freedesktop.org/series/122615/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13635 -> IGTPW_9795
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_9795 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_9795, please notify your bug team (lgci.bug.filing@intel.com) 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_9795/index.html

Participating hosts (40 -> 38)
------------------------------

  Additional (1): fi-kbl-soraka 
  Missing    (3): fi-hsw-4770 fi-tgl-1115g4 fi-snb-2520m 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@ring_submission:
    - fi-kbl-soraka:      NOTRUN -> [ABORT][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9795/fi-kbl-soraka/igt@i915_selftest@live@ring_submission.html

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

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

### IGT changes ###

#### Issues hit ####

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

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

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][4] ([i915#5334] / [i915#7872])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9795/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html

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

  * igt@i915_selftest@live@hangcheck:
    - bat-dg2-11:         [PASS][6] -> [INCOMPLETE][7] ([i915#7913] / [i915#9253])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13635/bat-dg2-11/igt@i915_selftest@live@hangcheck.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9795/bat-dg2-11/igt@i915_selftest@live@hangcheck.html

  * igt@kms_dsc@dsc-basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][8] ([fdo#109271]) +9 other tests skip
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9795/fi-kbl-soraka/igt@kms_dsc@dsc-basic.html

  * igt@kms_force_connector_basic@force-connector-state:
    - fi-kbl-guc:         [PASS][9] -> [ABORT][10] ([i915#8299])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13635/fi-kbl-guc/igt@kms_force_connector_basic@force-connector-state.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9795/fi-kbl-guc/igt@kms_force_connector_basic@force-connector-state.html

  
  [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#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#7872]: https://gitlab.freedesktop.org/drm/intel/issues/7872
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#8299]: https://gitlab.freedesktop.org/drm/intel/issues/8299
  [i915#9253]: https://gitlab.freedesktop.org/drm/intel/issues/9253


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

  * CI: CI-20190529 -> None
  * IGT: IGT_7488 -> IGTPW_9795

  CI-20190529: 20190529
  CI_DRM_13635: c6b7f865a77a75af03c3b68baa4cf7eb66c1c6d5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_9795: 9795
  IGT_7488: 099e058c5dfb7a49942edf03cae88a52a77077a3 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git


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

+igt@xe_copy_basic@mem-copy
+igt@xe_copy_basic@mem-set

== Logs ==

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

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

Re: [igt-dev] [PATCH i-g-t 4/4] intel/xe_copy_basic: Add copy basic test to exercise blt commands

[Zbigniew Kempczyński]

On Fri, Sep 15, 2023 at 10:46:17AM +0530, sai.gowtham.ch@intel.com wrote:
> From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> 
> Add copy basic test to exercise copy commands like mem-copy and mem-set.
> 
> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> ---
>  tests/intel/xe_copy_basic.c | 199 ++++++++++++++++++++++++++++++++++++
>  tests/meson.build           |   1 +
>  2 files changed, 200 insertions(+)
>  create mode 100644 tests/intel/xe_copy_basic.c
> 
> diff --git a/tests/intel/xe_copy_basic.c b/tests/intel/xe_copy_basic.c
> new file mode 100644
> index 000000000..0014b022e
> --- /dev/null
> +++ b/tests/intel/xe_copy_basic.c
> @@ -0,0 +1,199 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2023 Intel Corporation
> + *
> + * Authors:
> + *	Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> + */
> +
> +#include "igt.h"
> +#include "lib/igt_syncobj.h"
> +#include "intel_blt.h"
> +#include "lib/intel_cmds_info.h"
> +#include "lib/intel_mocs.h"
> +#include "lib/intel_reg.h"
> +#include "xe/xe_ioctl.h"
> +#include "xe/xe_query.h"
> +#include "xe/xe_util.h"
> +
> +/**
> + * TEST: Test to validate copy commands on xe
> + * Category: Software building block
> + * Sub-category: Copy
> + * Functionality: blitter
> + */
> +
> +/**
> + * SUBTEST: mem-copy
> + * Description: Test validates MEM_COPY command, it takes various
> + *		parameters needed for the filling batch buffer for MEM_COPY command.
> + * Test category: functionality test
> + */
> +static void
> +igt_mem_copy(int fd, uint32_t src_handle, uint32_t dst_handle,
> +	     const intel_ctx_t *ctx, uint32_t row_size,
> +	     uint32_t col_size, uint32_t region)
> +{
> +	struct blt_copy_data blt = {};
> +	uint64_t bb_size = xe_get_default_alignment(fd);
> +	uint64_t ahnd = intel_allocator_open_full(fd, ctx->vm, 0, 0,
> +						  INTEL_ALLOCATOR_SIMPLE,
> +						  ALLOC_STRATEGY_LOW_TO_HIGH, 0);
> +	uint32_t bb;
> +	int result;
> +	uint8_t src_mocs = intel_get_uc_mocs(fd);
> +	uint8_t dst_mocs = src_mocs;
> +
> +	bb = xe_bo_create_flags(fd, 0, bb_size, region);
> +
> +	blt_copy_init(fd, &blt);
> +	blt_set_object(&blt.src, src_handle, row_size, region, src_mocs, 0,
> +		       T_LINEAR, COMPRESSION_DISABLED);
> +	blt_set_object(&blt.dst, dst_handle, row_size, region, dst_mocs, 0,
> +		       T_LINEAR, COMPRESSION_DISABLED);

Args are not properly passed:

./tests/intel/xe_copy_basic.c: In function ‘igt_mem_copy’:
../tests/intel/xe_copy_basic.c:51:24: warning: implicit conversion from ‘enum blt_tiling_type’ to ‘enum blt_compression’ [-Wenum-conversion]
   51 |                        T_LINEAR, COMPRESSION_DISABLED);
      |                        ^~~~~~~~
../tests/intel/xe_copy_basic.c:51:34: warning: implicit conversion from ‘enum blt_compression’ to ‘enum blt_compression_type’ [-Wenum-conversion]
   51 |                        T_LINEAR, COMPRESSION_DISABLED);
      |                                  ^~~~~~~~~~~~~~~~~~~~
../tests/intel/xe_copy_basic.c:53:24: warning: implicit conversion from ‘enum blt_tiling_type’ to ‘enum blt_compression’ [-Wenum-conversion]
   53 |                        T_LINEAR, COMPRESSION_DISABLED);
      |                        ^~~~~~~~
../tests/intel/xe_copy_basic.c:53:34: warning: implicit conversion from ‘enum blt_compression’ to ‘enum blt_compression_type’ [-Wenum-conversion]
   53 |                        T_LINEAR, COMPRESSION_DISABLED);
      |                                  ^~~~~~~~~~~~~~~~~~~~
../tests/intel/xe_copy_basic.c: In function ‘igt_mem_set’:
../tests/intel/xe_copy_basic.c:87:24: warning: implicit conversion from ‘enum blt_tiling_type’ to ‘enum blt_compression’ [-Wenum-conversion]
   87 |                        T_LINEAR, COMPRESSION_DISABLED);
      |                        ^~~~~~~~
../tests/intel/xe_copy_basic.c:87:34: warning: implicit conversion from ‘enum blt_compression’ to ‘enum blt_compression_type’ [-Wenum-conversion]
   87 |                        T_LINEAR, COMPRESSION_DISABLED);
      |                                  ^~~~~~~~~~~~~~~~~~~~

Please fix and resubmit.

--
Zbigniew

> +	blt_set_batch(&blt.bb, bb, bb_size, region);
> +	igt_assert(blt.src.size == blt.dst.size);
> +
> +	blt_mem_copy(fd, ctx, NULL, ahnd, &blt, col_size);
> +	result = memcmp(blt.src.ptr, blt.dst.ptr, blt.src.size);
> +	igt_assert_f(!result, "source and destination differ\n");
> +
> +	intel_allocator_bind(ahnd, 0, 0);
> +	gem_close(fd, bb);
> +	put_ahnd(ahnd);
> +}
> +
> +/**
> + * SUBTEST: mem-set
> + * Description: Test validates MEM_SET command.
> + * Test category: functionality test
> + */
> +static void igt_mem_set(int fd, uint32_t dst_handle, const intel_ctx_t *ctx,
> +			size_t size, uint32_t height,
> +			uint32_t fill_data, uint32_t region)
> +{
> +	struct blt_copy_data blt = {};
> +	uint64_t bb_size = xe_get_default_alignment(fd);
> +	uint64_t ahnd = intel_allocator_open_full(fd, ctx->vm, 0, 0,
> +						  INTEL_ALLOCATOR_SIMPLE,
> +						  ALLOC_STRATEGY_LOW_TO_HIGH, 0);
> +	uint32_t bb;
> +	int result;
> +	uint8_t dst_mocs = intel_get_uc_mocs(fd);
> +
> +	bb = xe_bo_create_flags(fd, 0, bb_size, region);
> +	blt_copy_init(fd, &blt);
> +	blt_set_object(&blt.dst, dst_handle, size, region, dst_mocs, 0,
> +		       T_LINEAR, COMPRESSION_DISABLED);
> +	blt_set_batch(&blt.bb, bb, bb_size, region);
> +	blt_set_mem(fd, ctx, NULL, ahnd, &blt, height, fill_data);
> +	result = memcmp(blt.src.ptr, blt.dst.ptr, blt.src.size);
> +	igt_assert_f(!result, "source and destination differ\n");
> +
> +	intel_allocator_bind(ahnd, 0, 0);
> +	gem_close(fd, bb);
> +	put_ahnd(ahnd);
> +}
> +
> +static void copy_test(int fd, uint32_t size, enum blt_cmd_type cmd,
> +		      struct drm_xe_engine_class_instance *hwe, uint32_t region)
> +{
> +	uint32_t src_handle, dst_handle, vm, exec_queue, src_size, dst_size;
> +	char c = 'a';
> +	uint32_t bo_size = ALIGN(size + xe_cs_prefetch_size(fd), xe_get_default_alignment(fd));
> +	uint32_t temp_buffer[bo_size];
> +	const intel_ctx_t *ctx;
> +
> +	src_handle = xe_bo_create_flags(fd, 0, bo_size, region);
> +	dst_handle = xe_bo_create_flags(fd, 0, bo_size, region);
> +	vm = xe_vm_create(fd, 0, 0);
> +	exec_queue = xe_exec_queue_create(fd, vm, hwe, 0);
> +	ctx = intel_ctx_xe(fd, vm, exec_queue, 0, 0, 0);
> +
> +	src_size = bo_size;
> +	dst_size = bo_size;
> +
> +	/* Fill a pattern in the buffer */
> +	for (int i = 0; i < bo_size; i++) {
> +		temp_buffer[i] = c++ % 16;
> +		temp_buffer[i] |= (c++ % 16) << 8;
> +		temp_buffer[i] |= (c++ % 16) << 16;
> +		temp_buffer[i] |= (c++ % 16) << 24;
> +	}
> +
> +	if (cmd == MEM_COPY) {
> +		igt_mem_copy(fd,
> +			     src_handle,/*src_handle*/
> +			     dst_handle,/*dst_handle*/
> +			     ctx,
> +			     src_size,/*row_size*/
> +			     1,/*col_size*/
> +			     region);
> +	} else if (cmd == MEM_SET) {
> +		igt_mem_set(fd,
> +			    dst_handle, /*dst_handle*/
> +			    ctx,
> +			    dst_size,/*width*/
> +			    1,/*height*/
> +			    temp_buffer[0] & 0xff,/*fill_data*/
> +			    region);
> +		src_size = 1;
> +	}
> +
> +	gem_close(fd, src_handle);
> +	gem_close(fd, dst_handle);
> +	xe_exec_queue_destroy(fd, exec_queue);
> +	xe_vm_destroy(fd, vm);
> +}
> +
> +igt_main
> +{
> +	struct drm_xe_engine_class_instance *hwe;
> +	int fd;
> +	struct igt_collection *set, *regions;
> +	uint32_t region;
> +	uint64_t size[] = {0xFD, 0x369, 0x369, 0x3FFF, 0xFFFF, 0x1FFFF, 0x3FFFF};
> +
> +	igt_fixture {
> +		fd = drm_open_driver(DRIVER_XE);
> +		xe_device_get(fd);
> +		set = xe_get_memory_region_set(fd,
> +					       XE_MEM_REGION_CLASS_SYSMEM,
> +					       XE_MEM_REGION_CLASS_VRAM);
> +	}
> +
> +	igt_subtest_with_dynamic_f("mem-copy") {
> +		igt_require(blt_has_mem_copy(fd));
> +		for_each_variation_r(regions, 1, set) {
> +			region = igt_collection_get_value(regions, 0);
> +			xe_for_each_hw_engine(fd, hwe) {
> +				for (int i = 0; i < ARRAY_SIZE(size); i++) {
> +					igt_dynamic_f("size-0x%lx", size[i]) {
> +						copy_test(fd, size[i],
> +							  MEM_COPY, hwe,
> +							  region);
> +					}
> +				}
> +			}
> +		}
> +	}
> +
> +	igt_subtest_with_dynamic_f("mem-set") {
> +		igt_require(blt_has_mem_set(fd));
> +		for_each_variation_r(regions, 1, set) {
> +			region = igt_collection_get_value(regions, 0);
> +			xe_for_each_hw_engine(fd, hwe) {
> +				for (int i = 0; i < ARRAY_SIZE(size); i++) {
> +					igt_dynamic_f("size-0x%lx", size[i]) {
> +						copy_test(fd, size[i],
> +							  MEM_SET, hwe, region);
> +					}
> +				}
> +			}
> +		}
> +	}
> +
> +	igt_fixture {
> +		drm_close_driver(fd);
> +	}
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index 7201958b7..753cde31e 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -274,6 +274,7 @@ intel_xe_progs = [
>  	'xe_ccs',
>  	'xe_create',
>  	'xe_compute',
> +	'xe_copy_basic',
>  	'xe_dma_buf_sync',
>  	'xe_debugfs',
>  	'xe_evict',
> -- 
> 2.39.1
> 

Re: [igt-dev] [PATCH i-g-t 1/4] lib/intel_cmds_info: Add copy commands to blt_cmd_type

[Karolina Stolarek]

On 15.09.2023 07:16, sai.gowtham.ch@intel.com wrote:
> From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> 
> Add copy commands to blt_cmd_type which is used latter,
> to check it they are supported by fd.

Hm, I'm slightly confused. I thought you'll merge this one with the "Add 
check to see if blt commands are supported by the platforms." patch.

This would count as one logical change, now it's mixed up a bit.

All the best,
Karolina

> 
> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> ---
>   lib/intel_cmds_info.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/lib/intel_cmds_info.h b/lib/intel_cmds_info.h
> index 91d0f15ec..ec8312d56 100644
> --- a/lib/intel_cmds_info.h
> +++ b/lib/intel_cmds_info.h
> @@ -20,6 +20,8 @@ enum blt_tiling_type {
>   
>   enum blt_cmd_type {
>   	SRC_COPY,
> +	MEM_SET,
> +	MEM_COPY,
>   	XY_SRC_COPY,
>   	XY_FAST_COPY,
>   	XY_BLOCK_COPY,

Re: [igt-dev] [PATCH i-g-t 2/4] lib/intel_reg: Add copy commands in the lib

[Karolina Stolarek]

On 15.09.2023 07:16, sai.gowtham.ch@intel.com wrote:
> From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> 
> Add memory copy and set commands to the lib.
> 
> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> ---
>   lib/intel_reg.h | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/lib/intel_reg.h b/lib/intel_reg.h
> index 3bf3676dc..322aec9fd 100644
> --- a/lib/intel_reg.h
> +++ b/lib/intel_reg.h
> @@ -2564,6 +2564,10 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>   
>   #define XY_FAST_COLOR_BLT		((0x2<<29)|(0x44<<22)|0xe)
>   
> +/* RAW memory commands */
> +#define MEM_COPY_CMD                    ((0x2 << 29)|(0x5a << 22)|0x8)
> +#define MEM_SET_CMD                     ((0x2 << 29)|(0x5b << 22)|0x5)
> +

This is inserted in the middle of BLT commands and their arguments. 
Could you move it so it's either before the BLT block or after 
XY_FAST_COPY_COLOR_DEPTH_128?

Many thanks,
Karolina

>   #define XY_FAST_COPY_BLT				((2<<29)|(0x42<<22)|0x8)
>   /* dword 0 */
>   #define   XY_FAST_COPY_SRC_TILING_LINEAR		(0 << 20)

Re: [igt-dev] [PATCH i-g-t 3/4] lib/intel_blt: Add wrappers to prepare batch buffers and submit exec

[Karolina Stolarek]

On 15.09.2023 07:16, sai.gowtham.ch@intel.com wrote:
> From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> 
> This commit has couple of changes in it:
> 1. Adding wrapper for mem-set and mem-copy instructions to
>     prepare batch buffers and submit exec, (blt_mem_copy, blt_set_mem,
>     emit_blt_mem_copy, emit,blt_set_mem).
> 
> 2. Add check to see if blt commands are supported by the platforms.

Like I said in 1/4, it's fine to merge (2) with that other patch, so 
here we only have mem_set and mem_copy defined. Also, you could reword 
the patch to something like "Introduce mem_set and mem_copy commands". 
Yes, technically they are wrappers, but I think of them more as library 
features.

> 
> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> ---
>   lib/intel_blt.c | 210 ++++++++++++++++++++++++++++++++++++++++++++++++
>   lib/intel_blt.h |  20 +++++
>   2 files changed, 230 insertions(+)
> 
> diff --git a/lib/intel_blt.c b/lib/intel_blt.c
> index 429511920..88ad7cae8 100644
> --- a/lib/intel_blt.c
> +++ b/lib/intel_blt.c
> @@ -13,12 +13,14 @@
>   #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"
>   
>   #define BITRANGE(start, end) (end - start + 1)
>   #define GET_CMDS_INFO(__fd) intel_get_cmds_info(intel_get_drm_devid(__fd))
> +#define MEM_COPY_MOCS_SHIFT                     25
>   
>   /* Blitter tiling definitions sanity checks */
>   static_assert(T_LINEAR == I915_TILING_NONE, "Linear definitions have to match");
> @@ -289,6 +291,38 @@ bool blt_has_block_copy(int fd)
>   	return blt_supports_command(cmds_info, XY_BLOCK_COPY);
>   }
>   
> +/**
> + * blt_has_mem_copy
> + * @fd: drm fd
> + *
> + * Check if mem copy is supported by @fd device
> + *
> + * Returns:
> + * true if it does, false otherwise.
> + */
> +bool blt_has_mem_copy(int fd)
> +{
> +	const struct intel_cmds_info *cmds_info = GET_CMDS_INFO(fd);
> +
> +	return blt_supports_command(cmds_info, MEM_COPY);
> +}
> +
> +/**
> + * blt_has_mem_set
> + * @fd: drm fd
> + *
> + * Check if mem set is supported by @fd device
> + *
> + * Returns:
> + * true if it does, false otherwise.
> + */
> +bool blt_has_mem_set(int fd)
> +{
> +	const struct intel_cmds_info *cmds_info = GET_CMDS_INFO(fd);
> +
> +	return blt_supports_command(cmds_info, MEM_SET);
> +}
> +
>   /**
>    * blt_has_fast_copy
>    * @fd: drm fd
> @@ -1380,6 +1414,182 @@ int blt_fast_copy(int fd,
>   	return ret;
>   }
>   
> +void emit_blt_mem_copy(int fd, uint64_t ahnd, const struct blt_copy_data *blt, uint32_t col_size)
> +{
> +	uint64_t dst_offset, src_offset, alignment;
> +	int i;
> +	uint8_t src_mocs = intel_get_uc_mocs(fd);
> +	uint8_t dst_mocs = src_mocs;
> +	uint32_t size;
> +	struct {
> +		uint32_t batch[12];
> +		uint32_t data;
> +	} *data;

It's a nit on my part, but I thought it would be nice to have a struct 
here, similar to gen12_fast_copy_data.

> +
> +	alignment = xe_get_default_alignment(fd);

If we want to provide support for both i915 and Xe, we should use 
get_default_alignment() function here instead.

> +	src_offset = get_offset(ahnd, blt->src.handle, blt->src.size, alignment);
> +	dst_offset = get_offset(ahnd, blt->dst.handle, blt->dst.size, alignment);
> +	size = blt->src.size;
> +
> +	data = xe_bo_map(fd, blt->bb.handle, blt->bb.size);

There's a helper to handle bo mapping for both i915 and Xe: bo_map(). 
It'll check the type of the driver for your.

> +
> +	i = 0;
> +	data->batch[i++] = MEM_COPY_CMD;
> +	data->batch[i++] = size - 1;
> +	data->batch[i++] = col_size - 1;
> +	data->batch[i++] = 0;
> +	data->batch[i++] = 0;
> +	data->batch[i++] = src_offset;
> +	data->batch[i++] = src_offset << 32;
> +	data->batch[i++] = dst_offset;
> +	data->batch[i++] = dst_offset << 32;
> +	data->batch[i++] = src_mocs << MEM_COPY_MOCS_SHIFT | dst_mocs;
> +	data->batch[i++] = MI_BATCH_BUFFER_END;

This means we can't pipeline mem_copies in one bb. You could add a flag 
similar to "bool emit_bbe" in emit_blt_fast_copy

> +	data->batch[i++] = MI_NOOP;
> +
> +	igt_assert(i <= ARRAY_SIZE(data->batch));

Hm, I wonder, don't we need to unmap the mapped bb here?

> +}
> +
> +/**
> + * blt_mem_copy:
> + * @fd: drm fd
> + * @ctx: intel_ctx_t context
> + * @e: blitter engine for @ctx
> + * @ahnd: allocator handle
> + * @blt: blitter data for mem-copy.
> + *
> + * Function does mem blit between @src and @dst described in @blt object.
> + *
> + * Returns:
> + * execbuffer status.
> + */
> +int blt_mem_copy(int fd, const intel_ctx_t *ctx,
> +		 const struct intel_execution_engine2 *e,
> +		 uint64_t ahnd,
> +		 const struct blt_copy_data *blt,
> +		 uint32_t col_size)
> +{
> +	struct drm_i915_gem_execbuffer2 execbuf = {};
> +	struct drm_i915_gem_exec_object2 obj[3] = {};
> +	uint64_t dst_offset, src_offset, bb_offset, alignment;
> +	int ret;
> +
> +	alignment = get_default_alignment(fd, blt->driver);
> +	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_mem_copy(fd, ahnd, blt, col_size);
> +
> +	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);
> +	}

Nit: add an empty line here

> +	return ret;
> +}
> +
> +void emit_blt_set_mem(int fd, uint64_t ahnd, const struct blt_copy_data *blt,
> +		      uint32_t fill_data, uint32_t height)

It should be emit_blt_mem_set

> +{
> +	uint64_t dst_offset, alignment;
> +	int b;
> +	uint8_t dst_mocs = intel_get_uc_mocs(fd);
> +	uint32_t size;
> +	struct {
> +		uint32_t batch[12];
> +		uint32_t data;
> +	} *data;
> +
> +	alignment = xe_get_default_alignment(fd);

Similar comments to what I have for mem_copy, commenting here to make 
sure this doesn't fall through the cracks

> +	dst_offset = get_offset(ahnd, blt->dst.handle, blt->dst.size, alignment);
> +	size = blt->dst.size;
> +
> +	data = xe_bo_map(fd, blt->dst.handle, blt->dst.size);

Also here, use a driver-independent wrapper bo_map()

> +
> +	b = 0;
> +	data->batch[b++] = MEM_SET_CMD;
> +	data->batch[b++] = size - 1;
> +	data->batch[b++] = height;
> +	data->batch[b++] = 0;
> +	data->batch[b++] = dst_offset;
> +	data->batch[b++] = dst_offset << 32;
> +	data->batch[b++] = (fill_data << 24) | dst_mocs;
> +	data->batch[b++] = MI_BATCH_BUFFER_END;

Again, the question about adding an buffer end in the middle of the bb.

> +	data->batch[b++] = MI_NOOP;
> +
> +	igt_assert(b <= ARRAY_SIZE(data->batch));
> +}
> +
> +/**
> + * blt_set_mem:
> + * @fd: drm fd
> + * @ctx: intel_ctx_t context
> + * @e: blitter engine for @ctx
> + * @ahnd: allocator handle
> + * @blt: blitter data for mem-set.
> + *
> + * Function does mem set blit in described @blt object.
> + *
> + * Returns:
> + * execbuffer status.
> + */
> +int blt_set_mem(int fd, const intel_ctx_t *ctx,

blt_mem_set, not set_mem

> +		const struct intel_execution_engine2 *e,
> +		uint64_t ahnd,
> +		const struct blt_copy_data *blt,
> +		uint32_t height,
> +		uint32_t fill_data)
> +{
> +	struct drm_i915_gem_execbuffer2 execbuf = {};
> +	struct drm_i915_gem_exec_object2 obj[2] = {};
> +	uint64_t dst_offset, bb_offset, alignment;
> +	int ret;
> +
> +	alignment = get_default_alignment(fd, blt->driver);
> +	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_set_mem(fd, ahnd, blt, fill_data, height);
> +
> +	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(bb_offset);
> +		obj[0].handle = blt->dst.handle;
> +		obj[1].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;
> +		execbuf.buffer_count = 2;
> +		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->bb.handle);
> +	}

Nit: empty line

Many thanks,
Karolina

> +	return ret;
> +}
> +
>   void blt_set_geom(struct blt_copy_object *obj, uint32_t pitch,
>   		  int16_t x1, int16_t y1, int16_t x2, int16_t y2,
>   		  uint16_t x_offset, uint16_t y_offset)
> diff --git a/lib/intel_blt.h b/lib/intel_blt.h
> index b8b3d724d..79edee0d5 100644
> --- a/lib/intel_blt.h
> +++ b/lib/intel_blt.h
> @@ -175,6 +175,8 @@ bool blt_cmd_has_property(const struct intel_cmds_info *cmds_info,
>   			  uint32_t prop);
>   
>   bool blt_has_block_copy(int fd);
> +bool blt_has_mem_copy(int fd);
> +bool blt_has_mem_set(int fd);
>   bool blt_has_fast_copy(int fd);
>   bool blt_has_xy_src_copy(int fd);
>   bool blt_has_xy_color(int fd);
> @@ -229,6 +231,24 @@ int blt_fast_copy(int fd,
>   		  uint64_t ahnd,
>   		  const struct blt_copy_data *blt);
>   
> +void emit_blt_mem_copy(int fd, uint64_t ahnd,
> +		       const struct blt_copy_data *blt,
> +		       uint32_t col_size);
> +
> +int blt_mem_copy(int fd, const intel_ctx_t *ctx,
> +		const struct intel_execution_engine2 *e,
> +		 uint64_t ahnd,
> +		 const struct blt_copy_data *blt,
> +		 uint32_t col_size);
> +
> +void emit_blt_set_mem(int fd, uint64_t ahnd, const struct blt_copy_data *blt,
> +		      uint32_t fill_data, uint32_t height);
> +
> +int blt_set_mem(int fd, const intel_ctx_t *ctx,
> +		const struct intel_execution_engine2 *e, uint64_t ahnd,
> +		const struct blt_copy_data *blt, uint32_t height,
> +		uint32_t fill_data);
> +
>   void blt_set_geom(struct blt_copy_object *obj, uint32_t pitch,
>   		  int16_t x1, int16_t y1, int16_t x2, int16_t y2,
>   		  uint16_t x_offset, uint16_t y_offset);

Re: [igt-dev] [PATCH i-g-t 4/4] intel/xe_copy_basic: Add copy basic test to exercise blt commands

[Karolina Stolarek]

On 15.09.2023 07:16, sai.gowtham.ch@intel.com wrote:
> From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> 
> Add copy basic test to exercise copy commands like mem-copy and mem-set.
> 
> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> ---
>   tests/intel/xe_copy_basic.c | 199 ++++++++++++++++++++++++++++++++++++
>   tests/meson.build           |   1 +
>   2 files changed, 200 insertions(+)
>   create mode 100644 tests/intel/xe_copy_basic.c
> 
> diff --git a/tests/intel/xe_copy_basic.c b/tests/intel/xe_copy_basic.c
> new file mode 100644
> index 000000000..0014b022e
> --- /dev/null
> +++ b/tests/intel/xe_copy_basic.c
> @@ -0,0 +1,199 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2023 Intel Corporation
> + *
> + * Authors:
> + *	Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> + */
> +
> +#include "igt.h"
> +#include "lib/igt_syncobj.h"
> +#include "intel_blt.h"
> +#include "lib/intel_cmds_info.h"
> +#include "lib/intel_mocs.h"
> +#include "lib/intel_reg.h"
> +#include "xe/xe_ioctl.h"
> +#include "xe/xe_query.h"
> +#include "xe/xe_util.h"
> +
> +/**
> + * TEST: Test to validate copy commands on xe
> + * Category: Software building block
> + * Sub-category: Copy
> + * Functionality: blitter
> + */
> +
> +/**
> + * SUBTEST: mem-copy
> + * Description: Test validates MEM_COPY command, it takes various
> + *		parameters needed for the filling batch buffer for MEM_COPY command.
> + * Test category: functionality test
> + */
> +static void
> +igt_mem_copy(int fd, uint32_t src_handle, uint32_t dst_handle,
> +	     const intel_ctx_t *ctx, uint32_t row_size,
> +	     uint32_t col_size, uint32_t region)
> +{
> +	struct blt_copy_data blt = {};
> +	uint64_t bb_size = xe_get_default_alignment(fd);
> +	uint64_t ahnd = intel_allocator_open_full(fd, ctx->vm, 0, 0,
> +						  INTEL_ALLOCATOR_SIMPLE,
> +						  ALLOC_STRATEGY_LOW_TO_HIGH, 0);
> +	uint32_t bb;
> +	int result;
> +	uint8_t src_mocs = intel_get_uc_mocs(fd);
> +	uint8_t dst_mocs = src_mocs;
> +
> +	bb = xe_bo_create_flags(fd, 0, bb_size, region);
> +
> +	blt_copy_init(fd, &blt);
> +	blt_set_object(&blt.src, src_handle, row_size, region, src_mocs, 0,
> +		       T_LINEAR, COMPRESSION_DISABLED);
> +	blt_set_object(&blt.dst, dst_handle, row_size, region, dst_mocs, 0,
> +		       T_LINEAR, COMPRESSION_DISABLED);
> +	blt_set_batch(&blt.bb, bb, bb_size, region);
> +	igt_assert(blt.src.size == blt.dst.size);
> +
> +	blt_mem_copy(fd, ctx, NULL, ahnd, &blt, col_size);
> +	result = memcmp(blt.src.ptr, blt.dst.ptr, blt.src.size);

This is in addition to Zbigniew's comments -- I'd move 
gem_close/intel_allocator_bind etc., so it's before the assertion. We 
already capture the result, so we can clean everything up before the 
check, and free the resources even if the assertion fails.

The same comment applies to igt_mem_set.

Also, I wonder if we could drop igt_ prefix. These function don't belong 
to the IGT framework or whatsoever.

And one more thing -- could we free the objects via 
blt_destroy_object()? I don't think they're reused between the tests.

> +	igt_assert_f(!result, "source and destination differ\n");
> +
> +	intel_allocator_bind(ahnd, 0, 0);
> +	gem_close(fd, bb);
> +	put_ahnd(ahnd);
> +}
> +
> +/**
> + * SUBTEST: mem-set
> + * Description: Test validates MEM_SET command.
> + * Test category: functionality test
> + */
> +static void igt_mem_set(int fd, uint32_t dst_handle, const intel_ctx_t *ctx,
> +			size_t size, uint32_t height,
> +			uint32_t fill_data, uint32_t region)
> +{
> +	struct blt_copy_data blt = {};
> +	uint64_t bb_size = xe_get_default_alignment(fd);
> +	uint64_t ahnd = intel_allocator_open_full(fd, ctx->vm, 0, 0,
> +						  INTEL_ALLOCATOR_SIMPLE,
> +						  ALLOC_STRATEGY_LOW_TO_HIGH, 0);
> +	uint32_t bb;
> +	int result;
> +	uint8_t dst_mocs = intel_get_uc_mocs(fd);
> +
> +	bb = xe_bo_create_flags(fd, 0, bb_size, region);
> +	blt_copy_init(fd, &blt);
> +	blt_set_object(&blt.dst, dst_handle, size, region, dst_mocs, 0,
> +		       T_LINEAR, COMPRESSION_DISABLED);
> +	blt_set_batch(&blt.bb, bb, bb_size, region);
> +	blt_set_mem(fd, ctx, NULL, ahnd, &blt, height, fill_data);
> +	result = memcmp(blt.src.ptr, blt.dst.ptr, blt.src.size);
> +	igt_assert_f(!result, "source and destination differ\n");
> +
> +	intel_allocator_bind(ahnd, 0, 0);
> +	gem_close(fd, bb);
> +	put_ahnd(ahnd);
> +}
> +
> +static void copy_test(int fd, uint32_t size, enum blt_cmd_type cmd,
> +		      struct drm_xe_engine_class_instance *hwe, uint32_t region)
> +{
> +	uint32_t src_handle, dst_handle, vm, exec_queue, src_size, dst_size;
> +	char c = 'a';
> +	uint32_t bo_size = ALIGN(size + xe_cs_prefetch_size(fd), xe_get_default_alignment(fd));
> +	uint32_t temp_buffer[bo_size];
> +	const intel_ctx_t *ctx;
> +
> +	src_handle = xe_bo_create_flags(fd, 0, bo_size, region);
> +	dst_handle = xe_bo_create_flags(fd, 0, bo_size, region);
> +	vm = xe_vm_create(fd, 0, 0);
> +	exec_queue = xe_exec_queue_create(fd, vm, hwe, 0);
> +	ctx = intel_ctx_xe(fd, vm, exec_queue, 0, 0, 0);
> +
> +	src_size = bo_size;
> +	dst_size = bo_size;
> +
> +	/* Fill a pattern in the buffer */
> +	for (int i = 0; i < bo_size; i++) {
> +		temp_buffer[i] = c++ % 16;
> +		temp_buffer[i] |= (c++ % 16) << 8;
> +		temp_buffer[i] |= (c++ % 16) << 16;
> +		temp_buffer[i] |= (c++ % 16) << 24;
> +	}
> +
> +	if (cmd == MEM_COPY) {
> +		igt_mem_copy(fd,
> +			     src_handle,/*src_handle*/
> +			     dst_handle,/*dst_handle*/
> +			     ctx,
> +			     src_size,/*row_size*/
> +			     1,/*col_size*/
> +			     region);
> +	} else if (cmd == MEM_SET) {
> +		igt_mem_set(fd,
> +			    dst_handle, /*dst_handle*/
> +			    ctx,
> +			    dst_size,/*width*/
> +			    1,/*height*/
> +			    temp_buffer[0] & 0xff,/*fill_data*/
> +			    region);
> +		src_size = 1;

Why do we set this after the mem set? Can't we set it before and pass to 
mem_set? Why to override it at all?

All the best,
Karolina

> +	}
> +
> +	gem_close(fd, src_handle);
> +	gem_close(fd, dst_handle);
> +	xe_exec_queue_destroy(fd, exec_queue);
> +	xe_vm_destroy(fd, vm);
> +}
> +
> +igt_main
> +{
> +	struct drm_xe_engine_class_instance *hwe;
> +	int fd;
> +	struct igt_collection *set, *regions;
> +	uint32_t region;
> +	uint64_t size[] = {0xFD, 0x369, 0x369, 0x3FFF, 0xFFFF, 0x1FFFF, 0x3FFFF};
> +
> +	igt_fixture {
> +		fd = drm_open_driver(DRIVER_XE);
> +		xe_device_get(fd);
> +		set = xe_get_memory_region_set(fd,
> +					       XE_MEM_REGION_CLASS_SYSMEM,
> +					       XE_MEM_REGION_CLASS_VRAM);
> +	}
> +
> +	igt_subtest_with_dynamic_f("mem-copy") {
> +		igt_require(blt_has_mem_copy(fd));
> +		for_each_variation_r(regions, 1, set) {
> +			region = igt_collection_get_value(regions, 0);
> +			xe_for_each_hw_engine(fd, hwe) {
> +				for (int i = 0; i < ARRAY_SIZE(size); i++) {
> +					igt_dynamic_f("size-0x%lx", size[i]) {
> +						copy_test(fd, size[i],
> +							  MEM_COPY, hwe,
> +							  region);
> +					}
> +				}
> +			}
> +		}
> +	}
> +
> +	igt_subtest_with_dynamic_f("mem-set") {
> +		igt_require(blt_has_mem_set(fd));
> +		for_each_variation_r(regions, 1, set) {
> +			region = igt_collection_get_value(regions, 0);
> +			xe_for_each_hw_engine(fd, hwe) {
> +				for (int i = 0; i < ARRAY_SIZE(size); i++) {
> +					igt_dynamic_f("size-0x%lx", size[i]) {
> +						copy_test(fd, size[i],
> +							  MEM_SET, hwe, region);
> +					}
> +				}
> +			}
> +		}
> +	}
> +
> +	igt_fixture {
> +		drm_close_driver(fd);
> +	}
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index 7201958b7..753cde31e 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -274,6 +274,7 @@ intel_xe_progs = [
>   	'xe_ccs',
>   	'xe_create',
>   	'xe_compute',
> +	'xe_copy_basic',
>   	'xe_dma_buf_sync',
>   	'xe_debugfs',
>   	'xe_evict',

Re: [igt-dev] [PATCH i-g-t 4/4] intel/xe_copy_basic: Add copy basic test to exercise blt commands

[Zbigniew Kempczyński]

On Fri, Sep 15, 2023 at 10:46:17AM +0530, sai.gowtham.ch@intel.com wrote:
> From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> 
> Add copy basic test to exercise copy commands like mem-copy and mem-set.
> 
> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> ---
>  tests/intel/xe_copy_basic.c | 199 ++++++++++++++++++++++++++++++++++++
>  tests/meson.build           |   1 +
>  2 files changed, 200 insertions(+)
>  create mode 100644 tests/intel/xe_copy_basic.c
> 
> diff --git a/tests/intel/xe_copy_basic.c b/tests/intel/xe_copy_basic.c
> new file mode 100644
> index 000000000..0014b022e
> --- /dev/null
> +++ b/tests/intel/xe_copy_basic.c
> @@ -0,0 +1,199 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2023 Intel Corporation
> + *
> + * Authors:
> + *	Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> + */
> +
> +#include "igt.h"
> +#include "lib/igt_syncobj.h"
> +#include "intel_blt.h"
> +#include "lib/intel_cmds_info.h"
> +#include "lib/intel_mocs.h"
> +#include "lib/intel_reg.h"
> +#include "xe/xe_ioctl.h"
> +#include "xe/xe_query.h"
> +#include "xe/xe_util.h"
> +
> +/**
> + * TEST: Test to validate copy commands on xe
> + * Category: Software building block
> + * Sub-category: Copy
> + * Functionality: blitter
> + */
> +
> +/**
> + * SUBTEST: mem-copy
> + * Description: Test validates MEM_COPY command, it takes various
> + *		parameters needed for the filling batch buffer for MEM_COPY command.
> + * Test category: functionality test
> + */
> +static void
> +igt_mem_copy(int fd, uint32_t src_handle, uint32_t dst_handle,
> +	     const intel_ctx_t *ctx, uint32_t row_size,
> +	     uint32_t col_size, uint32_t region)

For matrix copy you need pitch of each objects too.

> +{
> +	struct blt_copy_data blt = {};
> +	uint64_t bb_size = xe_get_default_alignment(fd);
> +	uint64_t ahnd = intel_allocator_open_full(fd, ctx->vm, 0, 0,
> +						  INTEL_ALLOCATOR_SIMPLE,
> +						  ALLOC_STRATEGY_LOW_TO_HIGH, 0);
> +	uint32_t bb;
> +	int result;
> +	uint8_t src_mocs = intel_get_uc_mocs(fd);
> +	uint8_t dst_mocs = src_mocs;
> +
> +	bb = xe_bo_create_flags(fd, 0, bb_size, region);
> +
> +	blt_copy_init(fd, &blt);

I wonder should you use blt_copy_data struct or define new one. If you
decide to use blt_copy_data you should add seperate wrapper which
initializes it for mem-set/copy operations. Be aware those commands supports
linear or matrix fills / transfers. Also compression is supported in case
of linear copy so it would be good to test this as well.

> +	blt_set_object(&blt.src, src_handle, row_size, region, src_mocs, 0,
> +		       T_LINEAR, COMPRESSION_DISABLED);

Adding T_MATRIX is problematic as this is not tiling at all and might
be confusing. Apart of this for_each_tiling() iterator would run over
it even it this is not we want. You could hide this after
__BLT_MAX_TILING enum but this is ugly hack and I got mixed feelings
about it. I would rather go toward new enum for supported set/copy
formats and new struct blt_mem_data.

> +	blt_set_object(&blt.dst, dst_handle, row_size, region, dst_mocs, 0,
> +		       T_LINEAR, COMPRESSION_DISABLED);
> +	blt_set_batch(&blt.bb, bb, bb_size, region);
> +	igt_assert(blt.src.size == blt.dst.size);
> +
> +	blt_mem_copy(fd, ctx, NULL, ahnd, &blt, col_size);
> +	result = memcmp(blt.src.ptr, blt.dst.ptr, blt.src.size);
> +	igt_assert_f(!result, "source and destination differ\n");
> +
> +	intel_allocator_bind(ahnd, 0, 0);
> +	gem_close(fd, bb);
> +	put_ahnd(ahnd);
> +}
> +
> +/**
> + * SUBTEST: mem-set
> + * Description: Test validates MEM_SET command.
> + * Test category: functionality test
> + */
> +static void igt_mem_set(int fd, uint32_t dst_handle, const intel_ctx_t *ctx,
> +			size_t size, uint32_t height,
> +			uint32_t fill_data, uint32_t region)
> +{
> +	struct blt_copy_data blt = {};
> +	uint64_t bb_size = xe_get_default_alignment(fd);
> +	uint64_t ahnd = intel_allocator_open_full(fd, ctx->vm, 0, 0,
> +						  INTEL_ALLOCATOR_SIMPLE,
> +						  ALLOC_STRATEGY_LOW_TO_HIGH, 0);
> +	uint32_t bb;
> +	int result;
> +	uint8_t dst_mocs = intel_get_uc_mocs(fd);
> +
> +	bb = xe_bo_create_flags(fd, 0, bb_size, region);
> +	blt_copy_init(fd, &blt);
> +	blt_set_object(&blt.dst, dst_handle, size, region, dst_mocs, 0,
> +		       T_LINEAR, COMPRESSION_DISABLED);
> +	blt_set_batch(&blt.bb, bb, bb_size, region);
> +	blt_set_mem(fd, ctx, NULL, ahnd, &blt, height, fill_data);
> +	result = memcmp(blt.src.ptr, blt.dst.ptr, blt.src.size);

For filled object I would expect byte-by-byte check, not memcmp().

> +	igt_assert_f(!result, "source and destination differ\n");
> +
> +	intel_allocator_bind(ahnd, 0, 0);
> +	gem_close(fd, bb);
> +	put_ahnd(ahnd);
> +}
> +
> +static void copy_test(int fd, uint32_t size, enum blt_cmd_type cmd,
> +		      struct drm_xe_engine_class_instance *hwe, uint32_t region)
> +{
> +	uint32_t src_handle, dst_handle, vm, exec_queue, src_size, dst_size;
> +	char c = 'a';
> +	uint32_t bo_size = ALIGN(size + xe_cs_prefetch_size(fd), xe_get_default_alignment(fd));

What for is adding xe_cs_prefetch_size(fd) to buffer size? This is
related to command streamer prefetcher, not accesses to buffers.

> +	uint32_t temp_buffer[bo_size];

I'm not fan of such big allocations on the stack, definitely
use dynamic allocations.

> +	const intel_ctx_t *ctx;
> +
> +	src_handle = xe_bo_create_flags(fd, 0, bo_size, region);
> +	dst_handle = xe_bo_create_flags(fd, 0, bo_size, region);
> +	vm = xe_vm_create(fd, 0, 0);
> +	exec_queue = xe_exec_queue_create(fd, vm, hwe, 0);
> +	ctx = intel_ctx_xe(fd, vm, exec_queue, 0, 0, 0);
> +
> +	src_size = bo_size;
> +	dst_size = bo_size;
> +
> +	/* Fill a pattern in the buffer */
> +	for (int i = 0; i < bo_size; i++) {
> +		temp_buffer[i] = c++ % 16;
> +		temp_buffer[i] |= (c++ % 16) << 8;
> +		temp_buffer[i] |= (c++ % 16) << 16;
> +		temp_buffer[i] |= (c++ % 16) << 24;
> +	}

Strange pattern, high nibbles will be always 0x0000.
as temp_buffer is uint32_t isn't better just simply assign i
there?

> +
> +	if (cmd == MEM_COPY) {
> +		igt_mem_copy(fd,
> +			     src_handle,/*src_handle*/
> +			     dst_handle,/*dst_handle*/

Useless comments.

> +			     ctx,
> +			     src_size,/*row_size*/
> +			     1,/*col_size*/

Same here.

> +			     region);
> +	} else if (cmd == MEM_SET) {
> +		igt_mem_set(fd,
> +			    dst_handle, /*dst_handle*/
> +			    ctx,
> +			    dst_size,/*width*/
> +			    1,/*height*/
> +			    temp_buffer[0] & 0xff,/*fill_data*/
> +			    region);
> +		src_size = 1;
> +	}
> +
> +	gem_close(fd, src_handle);
> +	gem_close(fd, dst_handle);
> +	xe_exec_queue_destroy(fd, exec_queue);
> +	xe_vm_destroy(fd, vm);
> +}
> +
> +igt_main
> +{
> +	struct drm_xe_engine_class_instance *hwe;
> +	int fd;
> +	struct igt_collection *set, *regions;
> +	uint32_t region;
> +	uint64_t size[] = {0xFD, 0x369, 0x369, 0x3FFF, 0xFFFF, 0x1FFFF, 0x3FFFF};
> +
> +	igt_fixture {
> +		fd = drm_open_driver(DRIVER_XE);
> +		xe_device_get(fd);
> +		set = xe_get_memory_region_set(fd,
> +					       XE_MEM_REGION_CLASS_SYSMEM,
> +					       XE_MEM_REGION_CLASS_VRAM);
> +	}
> +
> +	igt_subtest_with_dynamic_f("mem-copy") {
> +		igt_require(blt_has_mem_copy(fd));
> +		for_each_variation_r(regions, 1, set) {
> +			region = igt_collection_get_value(regions, 0);
> +			xe_for_each_hw_engine(fd, hwe) {
> +				for (int i = 0; i < ARRAY_SIZE(size); i++) {
> +					igt_dynamic_f("size-0x%lx", size[i]) {
> +						copy_test(fd, size[i],
> +							  MEM_COPY, hwe,
> +							  region);
> +					}
> +				}
> +			}
> +		}
> +	}
> +
> +	igt_subtest_with_dynamic_f("mem-set") {
> +		igt_require(blt_has_mem_set(fd));
> +		for_each_variation_r(regions, 1, set) {
> +			region = igt_collection_get_value(regions, 0);
> +			xe_for_each_hw_engine(fd, hwe) {
> +				for (int i = 0; i < ARRAY_SIZE(size); i++) {
> +					igt_dynamic_f("size-0x%lx", size[i]) {
> +						copy_test(fd, size[i],
> +							  MEM_SET, hwe, region);
> +					}
> +				}
> +			}
> +		}
> +	}
> +
> +	igt_fixture {
> +		drm_close_driver(fd);
> +	}
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index 7201958b7..753cde31e 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -274,6 +274,7 @@ intel_xe_progs = [
>  	'xe_ccs',
>  	'xe_create',
>  	'xe_compute',
> +	'xe_copy_basic',
>  	'xe_dma_buf_sync',
>  	'xe_debugfs',
>  	'xe_evict',
> -- 
> 2.39.1
>

On PVC I got:

IGT-Version: 1.28-NO-GIT (x86_64) (Linux: 6.5.0-rc7-xe+ x86_64)
Opened device: /dev/dri/card0
Starting subtest: mem-copy
Test requirement not met in function __igt_unique____real_main149, file ../tests/intel/xe_copy_basic.c:166:
Test requirement: blt_has_mem_copy(fd)
Subtest mem-copy: SKIP (0.000s)
Starting subtest: mem-set
Test requirement not met in function __igt_unique____real_main149, file ../tests/intel/xe_copy_basic.c:182:
Test requirement: blt_has_mem_set(fd)
Subtest mem-set: SKIP (0.000s)

Even if I enforced blt_has_mem_copy|set() returns true (you need
to properly configure this in intel_cmds_info.c) I got failures
on vm_bind_array.

--
Zbigniew