[igt-dev] [PATCH i-g-t v3]tests/i915/gem_userptr_blits: Added XY_FAST_COPY_BLT Command for MTL

[igt-dev] [PATCH i-g-t v3]tests/i915/gem_userptr_blits: Added XY_FAST_COPY_BLT Command for MTL

[Vikas Srivastava]

From: Arjun Melkaveri <arjun.melkaveri@intel.com>

Test case uses legacy command which is not supported on MTL.
Modified test to use XY_FAST_COPY_BLT.

Signed-off-by: Arjun Melkaveri <arjun.melkaveri@intel.com>
Acked-by: Priyanka Dandamudi <priyanka.dandamudi@intel.com>
---
 tests/i915/gem_userptr_blits.c | 129 +++++++++++++++++++++------------
 1 file changed, 81 insertions(+), 48 deletions(-)

diff --git a/tests/i915/gem_userptr_blits.c b/tests/i915/gem_userptr_blits.c
index 698508669..edcc3418b 100644
--- a/tests/i915/gem_userptr_blits.c
+++ b/tests/i915/gem_userptr_blits.c
@@ -99,8 +99,9 @@ static int copy(int fd, uint32_t dst, uint32_t src)
 	struct drm_i915_gem_relocation_entry reloc[2];
 	struct drm_i915_gem_exec_object2 obj[3];
 	struct drm_i915_gem_execbuffer2 exec;
+	static uint32_t devid;
 	uint32_t handle;
-	int ret, i=0;
+	int ret, i = 0;
 	uint64_t dst_offset, src_offset, bb_offset;
 	bool has_relocs = gem_has_relocations(fd);
 
@@ -108,29 +109,44 @@ static int copy(int fd, uint32_t dst, uint32_t src)
 	dst_offset = bb_offset + 4096;
 	src_offset = dst_offset + WIDTH * HEIGHT * sizeof(uint32_t) * (src != dst);
 
-	batch[i++] = XY_SRC_COPY_BLT_CMD |
-		  XY_SRC_COPY_BLT_WRITE_ALPHA |
-		  XY_SRC_COPY_BLT_WRITE_RGB;
-	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
-		batch[i - 1] |= 8;
-	else
-		batch[i - 1] |= 6;
-
-	batch[i++] = (3 << 24) | /* 32 bits */
-		  (0xcc << 16) | /* copy ROP */
-		  WIDTH*4;
-	batch[i++] = 0; /* dst x1,y1 */
-	batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
-	batch[i++] = dst_offset; /* dst reloc */
-	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
-		batch[i++] = dst_offset >> 32;
-	batch[i++] = 0; /* src x1,y1 */
-	batch[i++] = WIDTH*4;
-	batch[i++] = src_offset; /* src reloc */
-	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
-		batch[i++] = src_offset >> 32;
-	batch[i++] = MI_BATCH_BUFFER_END;
-	batch[i++] = MI_NOOP;
+	devid = intel_get_drm_devid(fd);
+
+	if (intel_graphics_ver(devid) >= IP_VER(12, 60)) {
+		batch[i++] = XY_FAST_COPY_BLT;
+		batch[i++] = XY_FAST_COPY_COLOR_DEPTH_32 | WIDTH*4;
+		batch[i++] = 0;/* dst x1,y1 */
+		batch[i++] = (HEIGHT << 16) | WIDTH;/* dst x2,y2 */
+		batch[i++] = dst_offset;/* dst address */
+		batch[i++] = 0;/* src x1,y1 */
+		batch[i++] = WIDTH*4;/* src pitch */
+		batch[i++] = src_offset;/* src address */
+		batch[i++] = MI_BATCH_BUFFER_END;
+		batch[i++] = MI_NOOP;
+	} else {
+		batch[i++] = XY_SRC_COPY_BLT_CMD |
+			     XY_SRC_COPY_BLT_WRITE_ALPHA |
+			     XY_SRC_COPY_BLT_WRITE_RGB;
+		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
+			batch[i - 1] |= 8;
+		else
+			batch[i - 1] |= 6;
+
+		batch[i++] = (3 << 24) | /* 32 bits */
+			  (0xcc << 16) | /* copy ROP */
+			  WIDTH*4;
+		batch[i++] = 0; /* dst x1,y1 */
+		batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
+		batch[i++] = dst_offset; /* dst reloc */
+		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
+			batch[i++] = dst_offset >> 32;
+		batch[i++] = 0; /* src x1,y1 */
+		batch[i++] = WIDTH*4;
+		batch[i++] = src_offset; /* src reloc */
+		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
+			batch[i++] = src_offset >> 32;
+		batch[i++] = MI_BATCH_BUFFER_END;
+		batch[i++] = MI_NOOP;
+	}
 
 	handle = gem_create(fd, 4096);
 	gem_write(fd, handle, 0, batch, sizeof(batch));
@@ -204,31 +220,48 @@ blit(int fd, uint32_t dst, uint32_t src, uint32_t *all_bo, int n_bo)
 	struct drm_i915_gem_relocation_entry reloc[2];
 	struct drm_i915_gem_exec_object2 *obj;
 	struct drm_i915_gem_execbuffer2 exec;
-	uint32_t handle;
-	int n, ret, i=0;
+	static uint32_t devid;
 
-	batch[i++] = XY_SRC_COPY_BLT_CMD |
-		  XY_SRC_COPY_BLT_WRITE_ALPHA |
-		  XY_SRC_COPY_BLT_WRITE_RGB;
-	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
-		batch[i - 1] |= 8;
-	else
-		batch[i - 1] |= 6;
-	batch[i++] = (3 << 24) | /* 32 bits */
-		  (0xcc << 16) | /* copy ROP */
-		  WIDTH*4;
-	batch[i++] = 0; /* dst x1,y1 */
-	batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
-	batch[i++] = 0; /* dst reloc */
-	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
-		batch[i++] = 0;
-	batch[i++] = 0; /* src x1,y1 */
-	batch[i++] = WIDTH*4;
-	batch[i++] = 0; /* src reloc */
-	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
-		batch[i++] = 0;
-	batch[i++] = MI_BATCH_BUFFER_END;
-	batch[i++] = MI_NOOP;
+	uint32_t handle;
+	int n, ret, i = 0;
+
+	devid = intel_get_drm_devid(fd);
+
+	if (intel_graphics_ver(devid) >= IP_VER(12, 60)) {
+		batch[i++] = XY_FAST_COPY_BLT;
+		batch[i++] = XY_FAST_COPY_COLOR_DEPTH_32 | WIDTH*4;
+		batch[i++] = 0;/* dst x1,y1 */
+		batch[i++] = (HEIGHT << 16) | WIDTH;/* dst x2,y2 */
+		batch[i++] = ;/* dst address */
+		batch[i++] = 0;/* src x1,y1 */
+		batch[i++] = WIDTH*4;/* src pitch */
+		batch[i++] = 0;// src address lower bits
+		batch[i++] = MI_BATCH_BUFFER_END;
+		batch[i++] = MI_NOOP;
+	} else {
+		batch[i++] = XY_SRC_COPY_BLT_CMD |
+			     XY_SRC_COPY_BLT_WRITE_ALPHA |
+			     XY_SRC_COPY_BLT_WRITE_RGB;
+		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
+			batch[i - 1] |= 8;
+		else
+			batch[i - 1] |= 6;
+		batch[i++] = (3 << 24) | /* 32 bits */
+			     (0xcc << 16) | /* copy ROP */
+			     WIDTH*4;
+		batch[i++] = 0; /* dst x1,y1 */
+		batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
+		batch[i++] = 0; /* dst reloc */
+		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
+			batch[i++] = 0;
+		batch[i++] = 0; /* src x1,y1 */
+		batch[i++] = WIDTH*4;
+		batch[i++] = 0; /* src reloc */
+		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
+			batch[i++] = 0;
+		batch[i++] = MI_BATCH_BUFFER_END;
+		batch[i++] = MI_NOOP;
+	}
 
 	handle = gem_create(fd, 4096);
 	gem_write(fd, handle, 0, batch, sizeof(batch));
-- 
2.25.1

[igt-dev] ✗ Fi.CI.BUILD: failure for tests/i915/gem_userptr_blits: Added XY_FAST_COPY_BLT Command for MTL (rev3)

[Patchwork]

== Series Details ==

Series: tests/i915/gem_userptr_blits: Added XY_FAST_COPY_BLT Command for MTL (rev3)
URL   : https://patchwork.freedesktop.org/series/111133/
State : failure

== Summary ==

IGT patchset build failed on latest successful build
40a2de5cc6a6b43af7da7905bfe1ede9d9a3200c tests/i915/gem_exec_fence: Replacing custom spinner with igt_spin

[242/616] Linking target tests/gem_huc_copy.
[243/616] Linking target tests/gem_linear_blits.
[244/616] Linking target tests/gem_lmem_swapping.
[245/616] Linking target tests/gem_lut_handle.
[246/616] Linking target tests/gem_madvise.
[247/616] Linking target tests/gem_media_fill.
[248/616] Linking target tests/gem_media_vme.
[249/616] Linking target tests/gem_mmap.
[250/616] Linking target tests/gem_mmap_gtt.
[251/616] Linking target tests/gem_partial_pwrite_pread.
[252/616] Linking target tests/gem_ppgtt.
[253/616] Linking target tests/gem_pread.
[254/616] Linking target tests/gem_pread_after_blit.
[255/616] Linking target tests/gem_pwrite.
[256/616] Linking target tests/gem_pwrite_snooped.
[257/616] Linking target tests/gem_pxp.
[258/616] Linking target tests/gem_read_read_speed.
[259/616] Linking target tests/gem_readwrite.
[260/616] Linking target tests/gem_reg_read.
[261/616] Linking target tests/gem_render_copy.
[262/616] Linking target tests/gem_render_copy_redux.
[263/616] Linking target tests/gem_request_retire.
[264/616] Linking target tests/gem_render_tiled_blits.
[265/616] Linking target tests/gem_render_linear_blits.
[266/616] Linking target tests/gem_ringfill.
[267/616] Linking target tests/gem_reset_stats.
[268/616] Linking target tests/gem_set_tiling_vs_blt.
[269/616] Linking target tests/gem_set_tiling_vs_gtt.
[270/616] Linking target tests/gem_set_tiling_vs_pwrite.
[271/616] Linking target tests/gem_shrink.
[272/616] Linking target tests/gem_softpin.
[273/616] Linking target tests/gem_spin_batch.
[274/616] Linking target tests/gem_streaming_writes.
[275/616] Linking target tests/gem_sync.
[276/616] Linking target tests/gem_tiled_blits.
[277/616] Linking target tests/gem_tiled_partial_pwrite_pread.
[278/616] Linking target tests/gem_tiled_fence_blits.
[279/616] Linking target tests/gem_tiled_pread_basic.
[280/616] Linking target tests/gem_tiled_pread_pwrite.
[281/616] Linking target tests/gem_tiled_swapping.
[282/616] Linking target tests/gem_tiled_wb.
[283/616] Linking target tests/gem_tiled_wc.
[284/616] Compiling C object 'tests/59830eb@@gem_userptr_blits@exe/i915_gem_userptr_blits.c.o'.
FAILED: tests/59830eb@@gem_userptr_blits@exe/i915_gem_userptr_blits.c.o 
cc -Itests/59830eb@@gem_userptr_blits@exe -Itests -I../../../usr/src/igt-gpu-tools/tests -I../../../usr/src/igt-gpu-tools/include/drm-uapi -I../../../usr/src/igt-gpu-tools/include/linux-uapi -Ilib -I../../../usr/src/igt-gpu-tools/lib -I../../../usr/src/igt-gpu-tools/lib/stubs/syscalls -I. -I../../../usr/src/igt-gpu-tools/ -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/libdrm -I/usr/include/x86_64-linux-gnu -I/usr/include/valgrind -I/usr/include -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O2 -g -D_GNU_SOURCE -include config.h -D_FORTIFY_SOURCE=2 -Wbad-function-cast -Wdeclaration-after-statement -Wformat=2 -Wimplicit-fallthrough=0 -Wlogical-op -Wmissing-declarations -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wuninitialized -Wunused -Wno-clobbered -Wno-maybe-uninitialized -Wno-missing-field-initializers -Wno-pointer-arith -Wno-address-of-packed-member -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-unused-result -Werror=address -Werror=array-bounds -Werror=implicit -Werror=init-self -Werror=int-to-pointer-cast -Werror=main -Werror=missing-braces -Werror=nonnull -Werror=pointer-to-int-cast -Werror=return-type -Werror=sequence-point -Werror=trigraphs -Werror=write-strings -fno-builtin-malloc -fno-builtin-calloc -fcommon -pthread -MD -MQ 'tests/59830eb@@gem_userptr_blits@exe/i915_gem_userptr_blits.c.o' -MF 'tests/59830eb@@gem_userptr_blits@exe/i915_gem_userptr_blits.c.o.d' -o 'tests/59830eb@@gem_userptr_blits@exe/i915_gem_userptr_blits.c.o' -c ../../../usr/src/igt-gpu-tools/tests/i915/gem_userptr_blits.c
../../../usr/src/igt-gpu-tools/tests/i915/gem_userptr_blits.c: In function ‘blit’:
../../../usr/src/igt-gpu-tools/tests/i915/gem_userptr_blits.c:235:16: error: expected expression before ‘;’ token
  235 |   batch[i++] = ;/* dst address */
      |                ^
ninja: build stopped: subcommand failed.

Re: [igt-dev] [PATCH i-g-t v3]tests/i915/gem_userptr_blits: Added XY_FAST_COPY_BLT Command for MTL

[Lucas De Marchi]

On Mon, Nov 21, 2022 at 10:32:57PM +0530, Vikas Srivastava wrote:
>From: Arjun Melkaveri <arjun.melkaveri@intel.com>
>
>Test case uses legacy command which is not supported on MTL.
>Modified test to use XY_FAST_COPY_BLT.
>
>Signed-off-by: Arjun Melkaveri <arjun.melkaveri@intel.com>
>Acked-by: Priyanka Dandamudi <priyanka.dandamudi@intel.com>

$ git grep XY_SRC_COPY_BLT_CMD
lib/i830_reg.h:#define XY_SRC_COPY_BLT_CMD              ((2<<29)|(0x53<<22))
lib/intel_batchbuffer.c:        dword0 |= XY_SRC_COPY_BLT_CMD;
lib/intel_batchbuffer.c:        intel_bb_out(ibb, XY_SRC_COPY_BLT_CMD |
lib/intel_batchbuffer.h:        OUT_BATCH(XY_SRC_COPY_BLT_CMD | \
lib/intel_reg.h:#define XY_SRC_COPY_BLT_CMD             ((2<<29)|(0x53<<22))
tests/i915/api_intel_allocator.c:               batch[i++] = XY_SRC_COPY_BLT_CMD |
tests/i915/gem_blits.c: batch[i] = (XY_SRC_COPY_BLT_CMD |
tests/i915/gem_blits.c: batch[i] = (XY_SRC_COPY_BLT_CMD |
tests/i915/gem_blits.c: batch[i] = (XY_SRC_COPY_BLT_CMD |
tests/i915/gem_caching.c:                    XY_SRC_COPY_BLT_CMD |
tests/i915/gem_evict_alignment.c:       batch[i++] = (XY_SRC_COPY_BLT_CMD |
tests/i915/gem_evict_everything.c:      batch[i++] = (XY_SRC_COPY_BLT_CMD |
tests/i915/gem_linear_blits.c:  batch[i++] = XY_SRC_COPY_BLT_CMD |
tests/i915/gem_partial_pwrite_pread.c:               XY_SRC_COPY_BLT_CMD |
tests/i915/gem_tiled_fence_blits.c:     batch[i] = (XY_SRC_COPY_BLT_CMD |
tests/i915/gem_userptr_blits.c: batch[i++] = XY_SRC_COPY_BLT_CMD |
tests/i915/gem_userptr_blits.c: batch[i++] = XY_SRC_COPY_BLT_CMD |
tests/i915/gen3_mixed_blits.c:  *b++ = (XY_SRC_COPY_BLT_CMD |

Currently we have one abstraction for bb creation, but some tests
not using it.  This is covering just 1 test... maybe we should rather
convert tests to use a common abstraction and change it there?

+Zbigniew, who I think was working on some common abstraction.


Lucas De Marchi



>---
> tests/i915/gem_userptr_blits.c | 129 +++++++++++++++++++++------------
> 1 file changed, 81 insertions(+), 48 deletions(-)
>
>diff --git a/tests/i915/gem_userptr_blits.c b/tests/i915/gem_userptr_blits.c
>index 698508669..edcc3418b 100644
>--- a/tests/i915/gem_userptr_blits.c
>+++ b/tests/i915/gem_userptr_blits.c
>@@ -99,8 +99,9 @@ static int copy(int fd, uint32_t dst, uint32_t src)
> 	struct drm_i915_gem_relocation_entry reloc[2];
> 	struct drm_i915_gem_exec_object2 obj[3];
> 	struct drm_i915_gem_execbuffer2 exec;
>+	static uint32_t devid;
> 	uint32_t handle;
>-	int ret, i=0;
>+	int ret, i = 0;
> 	uint64_t dst_offset, src_offset, bb_offset;
> 	bool has_relocs = gem_has_relocations(fd);
>
>@@ -108,29 +109,44 @@ static int copy(int fd, uint32_t dst, uint32_t src)
> 	dst_offset = bb_offset + 4096;
> 	src_offset = dst_offset + WIDTH * HEIGHT * sizeof(uint32_t) * (src != dst);
>
>-	batch[i++] = XY_SRC_COPY_BLT_CMD |
>-		  XY_SRC_COPY_BLT_WRITE_ALPHA |
>-		  XY_SRC_COPY_BLT_WRITE_RGB;
>-	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
>-		batch[i - 1] |= 8;
>-	else
>-		batch[i - 1] |= 6;
>-
>-	batch[i++] = (3 << 24) | /* 32 bits */
>-		  (0xcc << 16) | /* copy ROP */
>-		  WIDTH*4;
>-	batch[i++] = 0; /* dst x1,y1 */
>-	batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
>-	batch[i++] = dst_offset; /* dst reloc */
>-	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
>-		batch[i++] = dst_offset >> 32;
>-	batch[i++] = 0; /* src x1,y1 */
>-	batch[i++] = WIDTH*4;
>-	batch[i++] = src_offset; /* src reloc */
>-	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
>-		batch[i++] = src_offset >> 32;
>-	batch[i++] = MI_BATCH_BUFFER_END;
>-	batch[i++] = MI_NOOP;
>+	devid = intel_get_drm_devid(fd);
>+
>+	if (intel_graphics_ver(devid) >= IP_VER(12, 60)) {
>+		batch[i++] = XY_FAST_COPY_BLT;
>+		batch[i++] = XY_FAST_COPY_COLOR_DEPTH_32 | WIDTH*4;
>+		batch[i++] = 0;/* dst x1,y1 */
>+		batch[i++] = (HEIGHT << 16) | WIDTH;/* dst x2,y2 */
>+		batch[i++] = dst_offset;/* dst address */
>+		batch[i++] = 0;/* src x1,y1 */
>+		batch[i++] = WIDTH*4;/* src pitch */
>+		batch[i++] = src_offset;/* src address */
>+		batch[i++] = MI_BATCH_BUFFER_END;
>+		batch[i++] = MI_NOOP;
>+	} else {
>+		batch[i++] = XY_SRC_COPY_BLT_CMD |
>+			     XY_SRC_COPY_BLT_WRITE_ALPHA |
>+			     XY_SRC_COPY_BLT_WRITE_RGB;
>+		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
>+			batch[i - 1] |= 8;
>+		else
>+			batch[i - 1] |= 6;
>+
>+		batch[i++] = (3 << 24) | /* 32 bits */
>+			  (0xcc << 16) | /* copy ROP */
>+			  WIDTH*4;
>+		batch[i++] = 0; /* dst x1,y1 */
>+		batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
>+		batch[i++] = dst_offset; /* dst reloc */
>+		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
>+			batch[i++] = dst_offset >> 32;
>+		batch[i++] = 0; /* src x1,y1 */
>+		batch[i++] = WIDTH*4;
>+		batch[i++] = src_offset; /* src reloc */
>+		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
>+			batch[i++] = src_offset >> 32;
>+		batch[i++] = MI_BATCH_BUFFER_END;
>+		batch[i++] = MI_NOOP;
>+	}
>
> 	handle = gem_create(fd, 4096);
> 	gem_write(fd, handle, 0, batch, sizeof(batch));
>@@ -204,31 +220,48 @@ blit(int fd, uint32_t dst, uint32_t src, uint32_t *all_bo, int n_bo)
> 	struct drm_i915_gem_relocation_entry reloc[2];
> 	struct drm_i915_gem_exec_object2 *obj;
> 	struct drm_i915_gem_execbuffer2 exec;
>-	uint32_t handle;
>-	int n, ret, i=0;
>+	static uint32_t devid;
>
>-	batch[i++] = XY_SRC_COPY_BLT_CMD |
>-		  XY_SRC_COPY_BLT_WRITE_ALPHA |
>-		  XY_SRC_COPY_BLT_WRITE_RGB;
>-	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
>-		batch[i - 1] |= 8;
>-	else
>-		batch[i - 1] |= 6;
>-	batch[i++] = (3 << 24) | /* 32 bits */
>-		  (0xcc << 16) | /* copy ROP */
>-		  WIDTH*4;
>-	batch[i++] = 0; /* dst x1,y1 */
>-	batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
>-	batch[i++] = 0; /* dst reloc */
>-	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
>-		batch[i++] = 0;
>-	batch[i++] = 0; /* src x1,y1 */
>-	batch[i++] = WIDTH*4;
>-	batch[i++] = 0; /* src reloc */
>-	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
>-		batch[i++] = 0;
>-	batch[i++] = MI_BATCH_BUFFER_END;
>-	batch[i++] = MI_NOOP;
>+	uint32_t handle;
>+	int n, ret, i = 0;
>+
>+	devid = intel_get_drm_devid(fd);
>+
>+	if (intel_graphics_ver(devid) >= IP_VER(12, 60)) {
>+		batch[i++] = XY_FAST_COPY_BLT;
>+		batch[i++] = XY_FAST_COPY_COLOR_DEPTH_32 | WIDTH*4;
>+		batch[i++] = 0;/* dst x1,y1 */
>+		batch[i++] = (HEIGHT << 16) | WIDTH;/* dst x2,y2 */
>+		batch[i++] = ;/* dst address */
>+		batch[i++] = 0;/* src x1,y1 */
>+		batch[i++] = WIDTH*4;/* src pitch */
>+		batch[i++] = 0;// src address lower bits
>+		batch[i++] = MI_BATCH_BUFFER_END;
>+		batch[i++] = MI_NOOP;
>+	} else {
>+		batch[i++] = XY_SRC_COPY_BLT_CMD |
>+			     XY_SRC_COPY_BLT_WRITE_ALPHA |
>+			     XY_SRC_COPY_BLT_WRITE_RGB;
>+		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
>+			batch[i - 1] |= 8;
>+		else
>+			batch[i - 1] |= 6;
>+		batch[i++] = (3 << 24) | /* 32 bits */
>+			     (0xcc << 16) | /* copy ROP */
>+			     WIDTH*4;
>+		batch[i++] = 0; /* dst x1,y1 */
>+		batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
>+		batch[i++] = 0; /* dst reloc */
>+		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
>+			batch[i++] = 0;
>+		batch[i++] = 0; /* src x1,y1 */
>+		batch[i++] = WIDTH*4;
>+		batch[i++] = 0; /* src reloc */
>+		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
>+			batch[i++] = 0;
>+		batch[i++] = MI_BATCH_BUFFER_END;
>+		batch[i++] = MI_NOOP;
>+	}
>
> 	handle = gem_create(fd, 4096);
> 	gem_write(fd, handle, 0, batch, sizeof(batch));
>-- 
>2.25.1
>

Re: [igt-dev] [PATCH i-g-t v3]tests/i915/gem_userptr_blits: Added XY_FAST_COPY_BLT Command for MTL

[Zbigniew Kempczyński]

On Mon, Nov 21, 2022 at 02:46:02PM -0800, Lucas De Marchi wrote:
> On Mon, Nov 21, 2022 at 10:32:57PM +0530, Vikas Srivastava wrote:
> > From: Arjun Melkaveri <arjun.melkaveri@intel.com>
> > 
> > Test case uses legacy command which is not supported on MTL.
> > Modified test to use XY_FAST_COPY_BLT.
> > 
> > Signed-off-by: Arjun Melkaveri <arjun.melkaveri@intel.com>
> > Acked-by: Priyanka Dandamudi <priyanka.dandamudi@intel.com>
> 
> $ git grep XY_SRC_COPY_BLT_CMD
> lib/i830_reg.h:#define XY_SRC_COPY_BLT_CMD              ((2<<29)|(0x53<<22))
> lib/intel_batchbuffer.c:        dword0 |= XY_SRC_COPY_BLT_CMD;
> lib/intel_batchbuffer.c:        intel_bb_out(ibb, XY_SRC_COPY_BLT_CMD |
> lib/intel_batchbuffer.h:        OUT_BATCH(XY_SRC_COPY_BLT_CMD | \

I would like to get rid of all libdrm stuff in intel_batchbuffer but there's
one test which still references to it - prime_nv_test.c. 

+ Petri, Kamil

May I remove this test? There're no users of it (there's no such hw we can
test this on) so I see no reason to keep outdated and dead test.
Anyway, I'm going to send the series, please decide in the meantime to
accept it or reject.

> lib/intel_reg.h:#define XY_SRC_COPY_BLT_CMD             ((2<<29)|(0x53<<22))
> tests/i915/api_intel_allocator.c:               batch[i++] = XY_SRC_COPY_BLT_CMD |
> tests/i915/gem_blits.c: batch[i] = (XY_SRC_COPY_BLT_CMD |
> tests/i915/gem_blits.c: batch[i] = (XY_SRC_COPY_BLT_CMD |
> tests/i915/gem_blits.c: batch[i] = (XY_SRC_COPY_BLT_CMD |
> tests/i915/gem_caching.c:                    XY_SRC_COPY_BLT_CMD |
> tests/i915/gem_evict_alignment.c:       batch[i++] = (XY_SRC_COPY_BLT_CMD |
> tests/i915/gem_evict_everything.c:      batch[i++] = (XY_SRC_COPY_BLT_CMD |
> tests/i915/gem_linear_blits.c:  batch[i++] = XY_SRC_COPY_BLT_CMD |
> tests/i915/gem_partial_pwrite_pread.c:               XY_SRC_COPY_BLT_CMD |
> tests/i915/gem_tiled_fence_blits.c:     batch[i] = (XY_SRC_COPY_BLT_CMD |
> tests/i915/gem_userptr_blits.c: batch[i++] = XY_SRC_COPY_BLT_CMD |
> tests/i915/gem_userptr_blits.c: batch[i++] = XY_SRC_COPY_BLT_CMD |
> tests/i915/gen3_mixed_blits.c:  *b++ = (XY_SRC_COPY_BLT_CMD |
> 
> Currently we have one abstraction for bb creation, but some tests
> not using it.  This is covering just 1 test... maybe we should rather
> convert tests to use a common abstraction and change it there?
> 
> +Zbigniew, who I think was working on some common abstraction.

+ Karolina

TL;DR
------
We're working on more universal blitter library, but currently it
supports/will support xy-block-copy-blt and xy-fast-copy-blt. So any
above changes (xy-src-copy-blt -> xy-fast-copy-blt) I still would do
manually. 

Long:
-----
Question is - do we really want to move to intel-bb everywhere. Intel-bb
was create to remove libdrm in render-copy/gpgpu-fill/media-fill. Nothing
prevents us from creating batches this way, but this still doesn't
handle conditional code to feed batches. I think we should rather go way
where we prepare data in structures, pass to build/execute batch (see
gem_ccs/i915_blt for reference). We may integrate this with intel-bb too
(I mean feeding intel-bb with instructions produced by the blit library
function is easy). Currently this is standalone blit (with execbuf
itself), but I want to change this - current design doesn't allow to
execute two blits in a row what may reveal some problems (cache flushes,
etc.). I opt for structures because new blit commands are quite big
(xy-block-copy-blt on dg2 has 21 dwords - I don't want to make function
argument list monsters - readability of this is questionable thing).

Another thing we're considering are tilings and compression support in
specific platform. I was surprised that in same gen we have different
tilings/compressions (dg1 and dg2 as an example). If I'm not wrong 
main bcs engine also may differ from other engines...

--
Zbigniew

> 
> 
> Lucas De Marchi
> 
> 
> 
> > ---
> > tests/i915/gem_userptr_blits.c | 129 +++++++++++++++++++++------------
> > 1 file changed, 81 insertions(+), 48 deletions(-)
> > 
> > diff --git a/tests/i915/gem_userptr_blits.c b/tests/i915/gem_userptr_blits.c
> > index 698508669..edcc3418b 100644
> > --- a/tests/i915/gem_userptr_blits.c
> > +++ b/tests/i915/gem_userptr_blits.c
> > @@ -99,8 +99,9 @@ static int copy(int fd, uint32_t dst, uint32_t src)
> > 	struct drm_i915_gem_relocation_entry reloc[2];
> > 	struct drm_i915_gem_exec_object2 obj[3];
> > 	struct drm_i915_gem_execbuffer2 exec;
> > +	static uint32_t devid;
> > 	uint32_t handle;
> > -	int ret, i=0;
> > +	int ret, i = 0;
> > 	uint64_t dst_offset, src_offset, bb_offset;
> > 	bool has_relocs = gem_has_relocations(fd);
> > 
> > @@ -108,29 +109,44 @@ static int copy(int fd, uint32_t dst, uint32_t src)
> > 	dst_offset = bb_offset + 4096;
> > 	src_offset = dst_offset + WIDTH * HEIGHT * sizeof(uint32_t) * (src != dst);
> > 
> > -	batch[i++] = XY_SRC_COPY_BLT_CMD |
> > -		  XY_SRC_COPY_BLT_WRITE_ALPHA |
> > -		  XY_SRC_COPY_BLT_WRITE_RGB;
> > -	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > -		batch[i - 1] |= 8;
> > -	else
> > -		batch[i - 1] |= 6;
> > -
> > -	batch[i++] = (3 << 24) | /* 32 bits */
> > -		  (0xcc << 16) | /* copy ROP */
> > -		  WIDTH*4;
> > -	batch[i++] = 0; /* dst x1,y1 */
> > -	batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
> > -	batch[i++] = dst_offset; /* dst reloc */
> > -	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > -		batch[i++] = dst_offset >> 32;
> > -	batch[i++] = 0; /* src x1,y1 */
> > -	batch[i++] = WIDTH*4;
> > -	batch[i++] = src_offset; /* src reloc */
> > -	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > -		batch[i++] = src_offset >> 32;
> > -	batch[i++] = MI_BATCH_BUFFER_END;
> > -	batch[i++] = MI_NOOP;
> > +	devid = intel_get_drm_devid(fd);
> > +
> > +	if (intel_graphics_ver(devid) >= IP_VER(12, 60)) {
> > +		batch[i++] = XY_FAST_COPY_BLT;
> > +		batch[i++] = XY_FAST_COPY_COLOR_DEPTH_32 | WIDTH*4;
> > +		batch[i++] = 0;/* dst x1,y1 */
> > +		batch[i++] = (HEIGHT << 16) | WIDTH;/* dst x2,y2 */
> > +		batch[i++] = dst_offset;/* dst address */
> > +		batch[i++] = 0;/* src x1,y1 */
> > +		batch[i++] = WIDTH*4;/* src pitch */
> > +		batch[i++] = src_offset;/* src address */
> > +		batch[i++] = MI_BATCH_BUFFER_END;
> > +		batch[i++] = MI_NOOP;
> > +	} else {
> > +		batch[i++] = XY_SRC_COPY_BLT_CMD |
> > +			     XY_SRC_COPY_BLT_WRITE_ALPHA |
> > +			     XY_SRC_COPY_BLT_WRITE_RGB;
> > +		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > +			batch[i - 1] |= 8;
> > +		else
> > +			batch[i - 1] |= 6;
> > +
> > +		batch[i++] = (3 << 24) | /* 32 bits */
> > +			  (0xcc << 16) | /* copy ROP */
> > +			  WIDTH*4;
> > +		batch[i++] = 0; /* dst x1,y1 */
> > +		batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
> > +		batch[i++] = dst_offset; /* dst reloc */
> > +		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > +			batch[i++] = dst_offset >> 32;
> > +		batch[i++] = 0; /* src x1,y1 */
> > +		batch[i++] = WIDTH*4;
> > +		batch[i++] = src_offset; /* src reloc */
> > +		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > +			batch[i++] = src_offset >> 32;
> > +		batch[i++] = MI_BATCH_BUFFER_END;
> > +		batch[i++] = MI_NOOP;
> > +	}
> > 
> > 	handle = gem_create(fd, 4096);
> > 	gem_write(fd, handle, 0, batch, sizeof(batch));
> > @@ -204,31 +220,48 @@ blit(int fd, uint32_t dst, uint32_t src, uint32_t *all_bo, int n_bo)
> > 	struct drm_i915_gem_relocation_entry reloc[2];
> > 	struct drm_i915_gem_exec_object2 *obj;
> > 	struct drm_i915_gem_execbuffer2 exec;
> > -	uint32_t handle;
> > -	int n, ret, i=0;
> > +	static uint32_t devid;
> > 
> > -	batch[i++] = XY_SRC_COPY_BLT_CMD |
> > -		  XY_SRC_COPY_BLT_WRITE_ALPHA |
> > -		  XY_SRC_COPY_BLT_WRITE_RGB;
> > -	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > -		batch[i - 1] |= 8;
> > -	else
> > -		batch[i - 1] |= 6;
> > -	batch[i++] = (3 << 24) | /* 32 bits */
> > -		  (0xcc << 16) | /* copy ROP */
> > -		  WIDTH*4;
> > -	batch[i++] = 0; /* dst x1,y1 */
> > -	batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
> > -	batch[i++] = 0; /* dst reloc */
> > -	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > -		batch[i++] = 0;
> > -	batch[i++] = 0; /* src x1,y1 */
> > -	batch[i++] = WIDTH*4;
> > -	batch[i++] = 0; /* src reloc */
> > -	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > -		batch[i++] = 0;
> > -	batch[i++] = MI_BATCH_BUFFER_END;
> > -	batch[i++] = MI_NOOP;
> > +	uint32_t handle;
> > +	int n, ret, i = 0;
> > +
> > +	devid = intel_get_drm_devid(fd);
> > +
> > +	if (intel_graphics_ver(devid) >= IP_VER(12, 60)) {
> > +		batch[i++] = XY_FAST_COPY_BLT;
> > +		batch[i++] = XY_FAST_COPY_COLOR_DEPTH_32 | WIDTH*4;
> > +		batch[i++] = 0;/* dst x1,y1 */
> > +		batch[i++] = (HEIGHT << 16) | WIDTH;/* dst x2,y2 */
> > +		batch[i++] = ;/* dst address */
> > +		batch[i++] = 0;/* src x1,y1 */
> > +		batch[i++] = WIDTH*4;/* src pitch */
> > +		batch[i++] = 0;// src address lower bits
> > +		batch[i++] = MI_BATCH_BUFFER_END;
> > +		batch[i++] = MI_NOOP;
> > +	} else {
> > +		batch[i++] = XY_SRC_COPY_BLT_CMD |
> > +			     XY_SRC_COPY_BLT_WRITE_ALPHA |
> > +			     XY_SRC_COPY_BLT_WRITE_RGB;
> > +		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > +			batch[i - 1] |= 8;
> > +		else
> > +			batch[i - 1] |= 6;
> > +		batch[i++] = (3 << 24) | /* 32 bits */
> > +			     (0xcc << 16) | /* copy ROP */
> > +			     WIDTH*4;
> > +		batch[i++] = 0; /* dst x1,y1 */
> > +		batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
> > +		batch[i++] = 0; /* dst reloc */
> > +		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > +			batch[i++] = 0;
> > +		batch[i++] = 0; /* src x1,y1 */
> > +		batch[i++] = WIDTH*4;
> > +		batch[i++] = 0; /* src reloc */
> > +		if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > +			batch[i++] = 0;
> > +		batch[i++] = MI_BATCH_BUFFER_END;
> > +		batch[i++] = MI_NOOP;
> > +	}
> > 
> > 	handle = gem_create(fd, 4096);
> > 	gem_write(fd, handle, 0, batch, sizeof(batch));
> > -- 
> > 2.25.1
> > 

Re: [igt-dev] [PATCH i-g-t v3]tests/i915/gem_userptr_blits: Added XY_FAST_COPY_BLT Command for MTL

[Lucas De Marchi]

On Tue, Nov 22, 2022 at 09:46:46AM +0100, Zbigniew Kempczyński wrote:
>On Mon, Nov 21, 2022 at 02:46:02PM -0800, Lucas De Marchi wrote:
>> On Mon, Nov 21, 2022 at 10:32:57PM +0530, Vikas Srivastava wrote:
>> > From: Arjun Melkaveri <arjun.melkaveri@intel.com>
>> >
>> > Test case uses legacy command which is not supported on MTL.
>> > Modified test to use XY_FAST_COPY_BLT.
>> >
>> > Signed-off-by: Arjun Melkaveri <arjun.melkaveri@intel.com>
>> > Acked-by: Priyanka Dandamudi <priyanka.dandamudi@intel.com>
>>
>> $ git grep XY_SRC_COPY_BLT_CMD
>> lib/i830_reg.h:#define XY_SRC_COPY_BLT_CMD              ((2<<29)|(0x53<<22))
>> lib/intel_batchbuffer.c:        dword0 |= XY_SRC_COPY_BLT_CMD;
>> lib/intel_batchbuffer.c:        intel_bb_out(ibb, XY_SRC_COPY_BLT_CMD |
>> lib/intel_batchbuffer.h:        OUT_BATCH(XY_SRC_COPY_BLT_CMD | \
>
>I would like to get rid of all libdrm stuff in intel_batchbuffer but there's
>one test which still references to it - prime_nv_test.c.
>
>+ Petri, Kamil
>
>May I remove this test? There're no users of it (there's no such hw we can
>test this on) so I see no reason to keep outdated and dead test.
>Anyway, I'm going to send the series, please decide in the meantime to
>accept it or reject.
>
>> lib/intel_reg.h:#define XY_SRC_COPY_BLT_CMD             ((2<<29)|(0x53<<22))
>> tests/i915/api_intel_allocator.c:               batch[i++] = XY_SRC_COPY_BLT_CMD |
>> tests/i915/gem_blits.c: batch[i] = (XY_SRC_COPY_BLT_CMD |
>> tests/i915/gem_blits.c: batch[i] = (XY_SRC_COPY_BLT_CMD |
>> tests/i915/gem_blits.c: batch[i] = (XY_SRC_COPY_BLT_CMD |
>> tests/i915/gem_caching.c:                    XY_SRC_COPY_BLT_CMD |
>> tests/i915/gem_evict_alignment.c:       batch[i++] = (XY_SRC_COPY_BLT_CMD |
>> tests/i915/gem_evict_everything.c:      batch[i++] = (XY_SRC_COPY_BLT_CMD |
>> tests/i915/gem_linear_blits.c:  batch[i++] = XY_SRC_COPY_BLT_CMD |
>> tests/i915/gem_partial_pwrite_pread.c:               XY_SRC_COPY_BLT_CMD |
>> tests/i915/gem_tiled_fence_blits.c:     batch[i] = (XY_SRC_COPY_BLT_CMD |
>> tests/i915/gem_userptr_blits.c: batch[i++] = XY_SRC_COPY_BLT_CMD |
>> tests/i915/gem_userptr_blits.c: batch[i++] = XY_SRC_COPY_BLT_CMD |
>> tests/i915/gen3_mixed_blits.c:  *b++ = (XY_SRC_COPY_BLT_CMD |
>>
>> Currently we have one abstraction for bb creation, but some tests
>> not using it.  This is covering just 1 test... maybe we should rather
>> convert tests to use a common abstraction and change it there?
>>
>> +Zbigniew, who I think was working on some common abstraction.
>
>+ Karolina
>
>TL;DR
>------
>We're working on more universal blitter library, but currently it
>supports/will support xy-block-copy-blt and xy-fast-copy-blt. So any
>above changes (xy-src-copy-blt -> xy-fast-copy-blt) I still would do
>manually.
>
>Long:
>-----
>Question is - do we really want to move to intel-bb everywhere. Intel-bb

if we don't have one function, that is fine, but I think we should at
least abstract out this if/else for XY_SRC_COPY_BLT vs XY_FAST_COPY_BLT,
because there are 10+ users of it doing the similar stuff.

>was create to remove libdrm in render-copy/gpgpu-fill/media-fill. Nothing
>prevents us from creating batches this way, but this still doesn't
>handle conditional code to feed batches. I think we should rather go way
>where we prepare data in structures, pass to build/execute batch (see
>gem_ccs/i915_blt for reference). We may integrate this with intel-bb too
>(I mean feeding intel-bb with instructions produced by the blit library
>function is easy). Currently this is standalone blit (with execbuf
>itself), but I want to change this - current design doesn't allow to
>execute two blits in a row what may reveal some problems (cache flushes,
>etc.). I opt for structures because new blit commands are quite big
>(xy-block-copy-blt on dg2 has 21 dwords - I don't want to make function
>argument list monsters - readability of this is questionable thing).
>
>Another thing we're considering are tilings and compression support in
>specific platform. I was surprised that in same gen we have different
>tilings/compressions (dg1 and dg2 as an example). If I'm not wrong

there isn't such a "gen" terminology anymore... this ended in 12. The
ones after that we shouldn't considered as "generation".. so they are
not really the same. DG1 has graphics 12.1 while DG2 has 12.55

>main bcs engine also may differ from other engines...

In PVC, yes. Link Copy engines don't support all the instructions of the
main copy engine.

Lucas De Marchi

Re: [igt-dev] [PATCH i-g-t v3]tests/i915/gem_userptr_blits: Added XY_FAST_COPY_BLT Command for MTL

[Petri Latvala]

On Tue, Nov 22, 2022 at 09:46:46AM +0100, Zbigniew Kempczyński wrote:
> On Mon, Nov 21, 2022 at 02:46:02PM -0800, Lucas De Marchi wrote:
> > On Mon, Nov 21, 2022 at 10:32:57PM +0530, Vikas Srivastava wrote:
> > > From: Arjun Melkaveri <arjun.melkaveri@intel.com>
> > > 
> > > Test case uses legacy command which is not supported on MTL.
> > > Modified test to use XY_FAST_COPY_BLT.
> > > 
> > > Signed-off-by: Arjun Melkaveri <arjun.melkaveri@intel.com>
> > > Acked-by: Priyanka Dandamudi <priyanka.dandamudi@intel.com>
> > 
> > $ git grep XY_SRC_COPY_BLT_CMD
> > lib/i830_reg.h:#define XY_SRC_COPY_BLT_CMD              ((2<<29)|(0x53<<22))
> > lib/intel_batchbuffer.c:        dword0 |= XY_SRC_COPY_BLT_CMD;
> > lib/intel_batchbuffer.c:        intel_bb_out(ibb, XY_SRC_COPY_BLT_CMD |
> > lib/intel_batchbuffer.h:        OUT_BATCH(XY_SRC_COPY_BLT_CMD | \
> 
> I would like to get rid of all libdrm stuff in intel_batchbuffer but there's
> one test which still references to it - prime_nv_test.c. 
> 
> + Petri, Kamil
> 
> May I remove this test? There're no users of it (there's no such hw we can
> test this on) so I see no reason to keep outdated and dead test.
> Anyway, I'm going to send the series, please decide in the meantime to
> accept it or reject.

tl;dr: Bitrotten test that could use a serious cleanup and
modernization. Go ahead and remove.


-- 
Petri Latvala