[igt-dev] [PATCH i-g-t 0/4] tests/i915: Enable XY_FAST_COPY_BLT for gen12+

[igt-dev] [PATCH i-g-t 0/4] tests/i915: Enable XY_FAST_COPY_BLT for gen12+

[Vikas Srivastava]

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

Arjun Melkaveri (1):
  tests/i915/gem_linear_blits: Enable XY_FAST_COPY_BLT copy instruction

Vikas Srivastava (3):
  lib/intel_batchbuffer: Enable XY_FAST_COPY_BLT support for
    api_intel_bb
  tests/i915/gem_caching: Enable XY_FAST_COPY_BLT for MTL
  tests/i915/gem_userptr_blits: Enable XY_FAST_COPY_BLT Command for
    gen12+

 lib/intel_batchbuffer.c        | 63 +++++++++++++++++---------
 tests/i915/gem_caching.c       | 25 +++++++----
 tests/i915/gem_linear_blits.c  | 66 +++++++++++++++++----------
 tests/i915/gem_userptr_blits.c | 82 +++++++++++++++++++++-------------
 4 files changed, 153 insertions(+), 83 deletions(-)

-- 
2.25.1

[igt-dev] [PATCH i-g-t 1/4] lib/intel_batchbuffer: Enable XY_FAST_COPY_BLT support for api_intel_bb

[Vikas Srivastava]

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

Signed-off-by: Vikas Srivastava <vikas.srivastava@intel.com>
---
 lib/intel_batchbuffer.c | 63 +++++++++++++++++++++++++++--------------
 1 file changed, 42 insertions(+), 21 deletions(-)

diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
index da4c238cae..b1eff42fcb 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -2407,11 +2407,17 @@ uint32_t intel_bb_copy_data(struct intel_bb *ibb,
  */
 void intel_bb_blit_start(struct intel_bb *ibb, uint32_t flags)
 {
-	intel_bb_out(ibb, XY_SRC_COPY_BLT_CMD |
-		     XY_SRC_COPY_BLT_WRITE_ALPHA |
-		     XY_SRC_COPY_BLT_WRITE_RGB |
-		     flags |
-		     (6 + 2 * (ibb->gen >= 8)));
+	if (blt_has_fast_copy(ibb->i915))
+		intel_bb_out(ibb, XY_FAST_COPY_BLT | flags);
+	else if (blt_has_xy_src_copy(ibb->i915))
+
+		intel_bb_out(ibb, XY_SRC_COPY_BLT_CMD |
+			     XY_SRC_COPY_BLT_WRITE_ALPHA |
+			     XY_SRC_COPY_BLT_WRITE_RGB |
+			     flags |
+			     (6 + 2 * (ibb->gen >= 8)));
+	else
+		igt_assert_f(0, "No supported blit command found\n");
 }
 
 /*
@@ -2449,12 +2455,22 @@ void intel_bb_emit_blt_copy(struct intel_bb *ibb,
 
 	if (gen >= 4 && src->tiling != I915_TILING_NONE) {
 		src_pitch /= 4;
-		cmd_bits |= XY_SRC_COPY_BLT_SRC_TILED;
+		if (blt_has_fast_copy(ibb->i915))
+			cmd_bits |= fast_copy_dword0(src->tiling, dst->tiling);
+		else if (blt_has_xy_src_copy(ibb->i915))
+			cmd_bits |= XY_SRC_COPY_BLT_SRC_TILED;
+		else
+			igt_assert_f(0, "No supported blit command found\n");
 	}
 
 	if (gen >= 4 && dst->tiling != I915_TILING_NONE) {
 		dst_pitch /= 4;
-		cmd_bits |= XY_SRC_COPY_BLT_DST_TILED;
+		if (blt_has_fast_copy(ibb->i915))
+			cmd_bits |= fast_copy_dword0(src->tiling, dst->tiling);
+		else if (blt_has_xy_src_copy(ibb->i915))
+			cmd_bits |= XY_SRC_COPY_BLT_DST_TILED;
+		else
+			igt_assert_f(0, "No supported blit command found\n");
 	}
 
 	CHECK_RANGE(src_x1); CHECK_RANGE(src_y1);
@@ -2465,20 +2481,25 @@ void intel_bb_emit_blt_copy(struct intel_bb *ibb,
 	CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
 
 	br13_bits = 0;
-	switch (bpp) {
-	case 8:
-		break;
-	case 16:		/* supporting only RGB565, not ARGB1555 */
-		br13_bits |= 1 << 24;
-		break;
-	case 32:
-		br13_bits |= 3 << 24;
-		cmd_bits |= (XY_SRC_COPY_BLT_WRITE_ALPHA |
-			     XY_SRC_COPY_BLT_WRITE_RGB);
-		break;
-	default:
-		igt_fail(IGT_EXIT_FAILURE);
-	}
+	if (blt_has_xy_src_copy(ibb->i915)) {
+		switch (bpp) {
+		case 8:
+			break;
+		case 16:		/* supporting only RGB565, not ARGB1555 */
+			br13_bits |= 1 << 24;
+			break;
+		case 32:
+			br13_bits |= 3 << 24;
+			cmd_bits |= (XY_SRC_COPY_BLT_WRITE_ALPHA |
+				     XY_SRC_COPY_BLT_WRITE_RGB);
+			break;
+		default:
+			igt_fail(IGT_EXIT_FAILURE);
+		}
+	} else if (blt_has_fast_copy(ibb->i915)) {
+		br13_bits = fast_copy_dword1(ibb->i915, src->tiling, dst->tiling, bpp);
+	} else
+		igt_assert_f(0, "No supported blit command found\n");
 
 	if ((src->tiling | dst->tiling) >= I915_TILING_Y) {
 		intel_bb_out(ibb, MI_LOAD_REGISTER_IMM(1));
-- 
2.25.1

[igt-dev] [PATCH i-g-t 2/4] tests/i915/gem_caching: Enable XY_FAST_COPY_BLT for MTL

[Vikas Srivastava]

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

Signed-off-by: Vikas Srivastava <vikas.srivastava@intel.com>
---
 tests/i915/gem_caching.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/tests/i915/gem_caching.c b/tests/i915/gem_caching.c
index eb0170abca..6a7f9fad06 100644
--- a/tests/i915/gem_caching.c
+++ b/tests/i915/gem_caching.c
@@ -39,6 +39,7 @@
 
 #include "i915/gem.h"
 #include "igt.h"
+#include "i915/i915_blt.h"
 
 IGT_TEST_DESCRIPTION("Test snoop consistency when touching partial"
 		     " cachelines.");
@@ -81,16 +82,22 @@ copy_bo(struct intel_bb *ibb, struct intel_buf *src, struct intel_buf *dst)
 
 	intel_bb_add_intel_buf(ibb, src, false);
 	intel_bb_add_intel_buf(ibb, dst, true);
+	if (blt_has_fast_copy(ibb->i915)) {
+		intel_bb_out(ibb, XY_FAST_COPY_BLT);
+		intel_bb_out(ibb, XY_FAST_COPY_COLOR_DEPTH_32 | 4096);
+	} else if (blt_has_xy_src_copy(ibb->i915)) {
+		intel_bb_out(ibb,
+			     XY_SRC_COPY_BLT_CMD |
+			     XY_SRC_COPY_BLT_WRITE_ALPHA |
+			     XY_SRC_COPY_BLT_WRITE_RGB |
+			     (6 + 2 * has_64b_reloc));
+
+		intel_bb_out(ibb, (3 << 24) | /* 32 bits */
+			     (0xcc << 16) | /* copy ROP */
+			     4096);
+	} else
+		igt_assert_f(0, "No supported blit command found\n");
 
-	intel_bb_out(ibb,
-		     XY_SRC_COPY_BLT_CMD |
-		     XY_SRC_COPY_BLT_WRITE_ALPHA |
-		     XY_SRC_COPY_BLT_WRITE_RGB |
-		     (6 + 2 * has_64b_reloc));
-
-	intel_bb_out(ibb, (3 << 24) | /* 32 bits */
-		     (0xcc << 16) | /* copy ROP */
-		     4096);
 	intel_bb_out(ibb, 0 << 16 | 0);
 	intel_bb_out(ibb, (BO_SIZE/4096) << 16 | 1024);
 	intel_bb_emit_reloc_fenced(ibb, dst->handle,
-- 
2.25.1

[igt-dev] [PATCH i-g-t 3/4] tests/i915/gem_userptr_blits: Enable XY_FAST_COPY_BLT Command for gen12+

[Vikas Srivastava]

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

Signed-off-by: Vikas Srivastava <vikas.srivastava@intel.com>
---
 tests/i915/gem_userptr_blits.c | 82 +++++++++++++++++++++-------------
 1 file changed, 52 insertions(+), 30 deletions(-)

diff --git a/tests/i915/gem_userptr_blits.c b/tests/i915/gem_userptr_blits.c
index 07a453229a..dba11efb88 100644
--- a/tests/i915/gem_userptr_blits.c
+++ b/tests/i915/gem_userptr_blits.c
@@ -66,6 +66,7 @@
 #include "sw_sync.h"
 
 #include "eviction_common.c"
+#include "i915/i915_blt.h"
 
 #ifndef PAGE_SIZE
 #define PAGE_SIZE 4096
@@ -99,6 +100,7 @@ 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;
 	uint64_t dst_offset, src_offset, bb_offset;
@@ -108,29 +110,48 @@ 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 (blt_has_fast_copy(fd)) {
+		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++] = lower_32_bits(dst_offset); /* dst address */
+		batch[i++] = upper_32_bits(CANONICAL(dst_offset));
+		batch[i++] = 0;/* src x1,y1 */
+		batch[i++] = WIDTH * 4;/* src pitch */
+		batch[i++] = lower_32_bits(src_offset); /* src address */
+		batch[i++] = upper_32_bits(CANONICAL(src_offset));
+		batch[i++] = MI_BATCH_BUFFER_END;
+		batch[i++] = MI_NOOP;
+	} else if (blt_has_xy_src_copy(fd)) {
+		batch[i++] = XY_SRC_COPY_BLT_CMD |
+			     XY_SRC_COPY_BLT_WRITE_ALPHA |
+			     XY_SRC_COPY_BLT_WRITE_RGB;
+
+		if (intel_gen(devid) >= 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++] = lower_32_bits(dst_offset); /*dst reloc*/
+		if (intel_gen(devid) >= 8)
+			batch[i++] = upper_32_bits(CANONICAL(dst_offset));
+		batch[i++] = 0; /* src x1,y1 */
+		batch[i++] = WIDTH * 4;
+		batch[i++] = lower_32_bits(src_offset); /* src reloc */
+		if (intel_gen(devid) >= 8)
+			batch[i++] = upper_32_bits(CANONICAL(src_offset));
+		batch[i++] = MI_BATCH_BUFFER_END;
+		batch[i++] = MI_NOOP;
+	} else
+		igt_assert_f(0, "No supported blit command found\n");
 
 	handle = gem_create(fd, 4096);
 	gem_write(fd, handle, 0, batch, sizeof(batch));
@@ -254,20 +275,20 @@ blit(int fd, uint32_t dst, uint32_t src, uint32_t *all_bo, int n_bo)
 	reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
 	reloc[1].write_domain = 0;
 
-	if (intel_graphics_ver(devid) >= IP_VER(12, 60)) {
+	if (blt_has_fast_copy(fd)) {
 		batch[i++] = XY_FAST_COPY_BLT;
-		batch[i++] = XY_FAST_COPY_COLOR_DEPTH_32 | WIDTH*4;
+		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++] = lower_32_bits(dst_offset); /* dst address */
 		batch[i++] = upper_32_bits(CANONICAL(dst_offset));
 		batch[i++] = 0; /* src x1,y1 */
-		batch[i++] = WIDTH*4; /* src pitch */
+		batch[i++] = WIDTH * 4; /* src pitch */
 		batch[i++] = lower_32_bits(src_offset); /* src address */
 		batch[i++] = upper_32_bits(CANONICAL(src_offset));
 		batch[i++] = MI_BATCH_BUFFER_END;
 		batch[i++] = MI_NOOP;
-	} else {
+	} else if (blt_has_xy_src_copy(fd)) {
 		batch[i++] = XY_SRC_COPY_BLT_CMD |
 			     XY_SRC_COPY_BLT_WRITE_ALPHA |
 			     XY_SRC_COPY_BLT_WRITE_RGB;
@@ -277,20 +298,21 @@ blit(int fd, uint32_t dst, uint32_t src, uint32_t *all_bo, int n_bo)
 			batch[i - 1] |= 6;
 		batch[i++] = (3 << 24) | /* 32 bits */
 			     (0xcc << 16) | /* copy ROP */
-			     WIDTH*4;
+			     WIDTH * 4;
 		batch[i++] = 0; /* dst x1,y1 */
 		batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
 		batch[i++] = lower_32_bits(dst_offset);
 		if (intel_gen(devid) >= 8)
 			batch[i++] = upper_32_bits(CANONICAL(dst_offset));
 		batch[i++] = 0; /* src x1,y1 */
-		batch[i++] = WIDTH*4;
+		batch[i++] = WIDTH * 4;
 		batch[i++] = lower_32_bits(src_offset);
 		if (intel_gen(devid) >= 8)
 			batch[i++] = upper_32_bits(CANONICAL(src_offset));
 		batch[i++] = MI_BATCH_BUFFER_END;
 		batch[i++] = MI_NOOP;
-	}
+	} else
+		igt_assert_f(0, "No supported blit command found\n");
 
 	gem_write(fd, handle, 0, batch, sizeof(batch));
 
-- 
2.25.1

[igt-dev] [PATCH i-g-t 4/4] tests/i915/gem_linear_blits: Enable XY_FAST_COPY_BLT copy instruction

[Vikas Srivastava]

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

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

Signed-off-by: Arjun Melkaveri <arjun.melkaveri@intel.com>
Co-developed-by: Vikas Srivastava <vikas.srivastava@intel.com>
Signed-off-by: Vikas Srivastava <vikas.srivastava@intel.com>
---
 tests/i915/gem_linear_blits.c | 66 +++++++++++++++++++++++------------
 1 file changed, 43 insertions(+), 23 deletions(-)

diff --git a/tests/i915/gem_linear_blits.c b/tests/i915/gem_linear_blits.c
index fac25095f5..6fa6cd4976 100644
--- a/tests/i915/gem_linear_blits.c
+++ b/tests/i915/gem_linear_blits.c
@@ -48,6 +48,7 @@
 #include "i915/gem_create.h"
 #include "igt.h"
 #include "igt_types.h"
+#include "i915/i915_blt.h"
 
 IGT_TEST_DESCRIPTION("Test doing many blits with a working set larger than the"
 		     " aperture size.");
@@ -67,6 +68,7 @@ static void copy(int fd, uint64_t ahnd, 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;
 	int i = 0;
 
 	memset(obj, 0, sizeof(obj));
@@ -83,29 +85,47 @@ static void copy(int fd, uint64_t ahnd, uint32_t dst, uint32_t src,
 	obj[2].offset = CANONICAL(obj[2].offset);
 	obj[2].flags = EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
 
-	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++] = obj[0].offset;
-	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
-		batch[i++] = obj[0].offset >> 32;
-	batch[i++] = 0; /* src x1,y1 */
-	batch[i++] = WIDTH*4;
-	batch[i++] = obj[1].offset;
-	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
-		batch[i++] = obj[1].offset >> 32;
-	batch[i++] = MI_BATCH_BUFFER_END;
-	batch[i++] = MI_NOOP;
+	devid = intel_get_drm_devid(fd);
+
+	if (blt_has_fast_copy(fd)) {
+		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++] = obj[0].offset; /* dst address lower bits */
+		batch[i++] = obj[0].offset >> 32; /* dst address upper bits */
+		batch[i++] = 0; /* src x1,y1 */
+		batch[i++] = WIDTH * 4; /* src pitch */
+		batch[i++] = obj[1].offset; /* src address lower bits */
+		batch[i++] = obj[1].offset >> 32; /* src address upper bits */
+		batch[i++] = MI_BATCH_BUFFER_END;
+		batch[i++] = MI_NOOP;
+	} else if (blt_has_xy_src_copy(fd)) {
+		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++] = obj[0].offset;
+		if (intel_gen(devid) >= 8)
+			batch[i++] = obj[0].offset >> 32;
+		batch[i++] = 0; /* src x1,y1 */
+		batch[i++] = WIDTH * 4;
+		batch[i++] = obj[1].offset;
+		if (intel_gen(devid) >= 8)
+			batch[i++] = obj[1].offset >> 32;
+		batch[i++] = MI_BATCH_BUFFER_END;
+		batch[i++] = MI_NOOP;
+	} else
+		igt_assert_f(0, "No supported blit command found\n");
 
 	gem_write(fd, obj[2].handle, 0, batch, i * sizeof(batch[0]));
 
-- 
2.25.1

[igt-dev] ✗ Fi.CI.BAT: failure for tests/i915: Enable XY_FAST_COPY_BLT for gen12+

[Patchwork]

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

== Series Details ==

Series: tests/i915: Enable XY_FAST_COPY_BLT for gen12+
URL   : https://patchwork.freedesktop.org/series/115602/
State : failure

== Summary ==

CI Bug Log - changes from IGT_7218 -> IGTPW_8680
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_8680 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_8680, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

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

Participating hosts (38 -> 36)
------------------------------

  Missing    (2): fi-kbl-soraka fi-snb-2520m 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_tiled_blits@basic:
    - fi-kbl-guc:         [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/fi-kbl-guc/igt@gem_tiled_blits@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/fi-kbl-guc/igt@gem_tiled_blits@basic.html
    - bat-jsl-1:          [PASS][3] -> [FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/bat-jsl-1/igt@gem_tiled_blits@basic.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-jsl-1/igt@gem_tiled_blits@basic.html
    - fi-tgl-1115g4:      [PASS][5] -> [FAIL][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/fi-tgl-1115g4/igt@gem_tiled_blits@basic.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/fi-tgl-1115g4/igt@gem_tiled_blits@basic.html
    - bat-rpls-1:         [PASS][7] -> [FAIL][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/bat-rpls-1/igt@gem_tiled_blits@basic.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-rpls-1/igt@gem_tiled_blits@basic.html
    - fi-cfl-guc:         [PASS][9] -> [FAIL][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/fi-cfl-guc/igt@gem_tiled_blits@basic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/fi-cfl-guc/igt@gem_tiled_blits@basic.html
    - fi-skl-6600u:       [PASS][11] -> [FAIL][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/fi-skl-6600u/igt@gem_tiled_blits@basic.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/fi-skl-6600u/igt@gem_tiled_blits@basic.html
    - bat-rpls-2:         [PASS][13] -> [FAIL][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/bat-rpls-2/igt@gem_tiled_blits@basic.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-rpls-2/igt@gem_tiled_blits@basic.html
    - fi-glk-j4005:       [PASS][15] -> [FAIL][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/fi-glk-j4005/igt@gem_tiled_blits@basic.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/fi-glk-j4005/igt@gem_tiled_blits@basic.html
    - bat-adlp-9:         [PASS][17] -> [FAIL][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/bat-adlp-9/igt@gem_tiled_blits@basic.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-adlp-9/igt@gem_tiled_blits@basic.html
    - fi-skl-guc:         [PASS][19] -> [FAIL][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/fi-skl-guc/igt@gem_tiled_blits@basic.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/fi-skl-guc/igt@gem_tiled_blits@basic.html
    - fi-cfl-8700k:       [PASS][21] -> [FAIL][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/fi-cfl-8700k/igt@gem_tiled_blits@basic.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/fi-cfl-8700k/igt@gem_tiled_blits@basic.html
    - bat-rplp-1:         [PASS][23] -> [FAIL][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/bat-rplp-1/igt@gem_tiled_blits@basic.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-rplp-1/igt@gem_tiled_blits@basic.html
    - fi-rkl-11600:       [PASS][25] -> [FAIL][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/fi-rkl-11600/igt@gem_tiled_blits@basic.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/fi-rkl-11600/igt@gem_tiled_blits@basic.html
    - bat-adls-5:         [PASS][27] -> [FAIL][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/bat-adls-5/igt@gem_tiled_blits@basic.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-adls-5/igt@gem_tiled_blits@basic.html
    - fi-apl-guc:         [PASS][29] -> [FAIL][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/fi-apl-guc/igt@gem_tiled_blits@basic.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/fi-apl-guc/igt@gem_tiled_blits@basic.html
    - bat-jsl-3:          [PASS][31] -> [FAIL][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/bat-jsl-3/igt@gem_tiled_blits@basic.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-jsl-3/igt@gem_tiled_blits@basic.html
    - fi-kbl-x1275:       [PASS][33] -> [FAIL][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/fi-kbl-x1275/igt@gem_tiled_blits@basic.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/fi-kbl-x1275/igt@gem_tiled_blits@basic.html
    - fi-cfl-8109u:       [PASS][35] -> [FAIL][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/fi-cfl-8109u/igt@gem_tiled_blits@basic.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/fi-cfl-8109u/igt@gem_tiled_blits@basic.html
    - fi-kbl-7567u:       [PASS][37] -> [FAIL][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/fi-kbl-7567u/igt@gem_tiled_blits@basic.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/fi-kbl-7567u/igt@gem_tiled_blits@basic.html
    - bat-adln-1:         [PASS][39] -> [FAIL][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/bat-adln-1/igt@gem_tiled_blits@basic.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-adln-1/igt@gem_tiled_blits@basic.html
    - fi-kbl-8809g:       [PASS][41] -> [FAIL][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/fi-kbl-8809g/igt@gem_tiled_blits@basic.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/fi-kbl-8809g/igt@gem_tiled_blits@basic.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_tiled_blits@basic:
    - {bat-kbl-2}:        [PASS][43] -> [FAIL][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/bat-kbl-2/igt@gem_tiled_blits@basic.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-kbl-2/igt@gem_tiled_blits@basic.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@gt_lrc:
    - bat-adln-1:         [PASS][45] -> [INCOMPLETE][46] ([i915#7609])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/bat-adln-1/igt@i915_selftest@live@gt_lrc.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-adln-1/igt@i915_selftest@live@gt_lrc.html

  * igt@i915_selftest@live@mman:
    - bat-rpls-2:         [PASS][47] -> [TIMEOUT][48] ([i915#6794])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/bat-rpls-2/igt@i915_selftest@live@mman.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-rpls-2/igt@i915_selftest@live@mman.html

  * igt@kms_chamelium_hpd@common-hpd-after-suspend:
    - bat-rpls-2:         NOTRUN -> [SKIP][49] ([i915#7828])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-rpls-2/igt@kms_chamelium_hpd@common-hpd-after-suspend.html
    - bat-rpls-1:         NOTRUN -> [SKIP][50] ([i915#7828])
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-rpls-1/igt@kms_chamelium_hpd@common-hpd-after-suspend.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
    - bat-dg2-11:         NOTRUN -> [SKIP][51] ([i915#5354]) +1 similar issue
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html

  * igt@kms_pipe_crc_basic@suspend-read-crc:
    - bat-rpls-1:         NOTRUN -> [SKIP][52] ([i915#1845])
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-rpls-1/igt@kms_pipe_crc_basic@suspend-read-crc.html
    - bat-rpls-2:         NOTRUN -> [SKIP][53] ([i915#1845])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-rpls-2/igt@kms_pipe_crc_basic@suspend-read-crc.html
    - fi-hsw-4770:        NOTRUN -> [SKIP][54] ([fdo#109271])
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/fi-hsw-4770/igt@kms_pipe_crc_basic@suspend-read-crc.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3@smem:
    - bat-rpls-1:         [ABORT][55] ([i915#6687] / [i915#7978]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/bat-rpls-1/igt@gem_exec_suspend@basic-s3@smem.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-rpls-1/igt@gem_exec_suspend@basic-s3@smem.html

  
#### Warnings ####

  * igt@i915_selftest@live@slpc:
    - bat-rpls-1:         [DMESG-FAIL][57] ([i915#6367]) -> [DMESG-FAIL][58] ([i915#6997])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7218/bat-rpls-1/igt@i915_selftest@live@slpc.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/bat-rpls-1/igt@i915_selftest@live@slpc.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
  [i915#6794]: https://gitlab.freedesktop.org/drm/intel/issues/6794
  [i915#6997]: https://gitlab.freedesktop.org/drm/intel/issues/6997
  [i915#7609]: https://gitlab.freedesktop.org/drm/intel/issues/7609
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7978]: https://gitlab.freedesktop.org/drm/intel/issues/7978


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

  * CI: CI-20190529 -> None
  * IGT: IGT_7218 -> IGTPW_8680

  CI-20190529: 20190529
  CI_DRM_12910: 34e37caa656a3f5907fd3afcacb4ef69b1d3062c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_8680: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8680/index.html
  IGT_7218: 8036123f781059c54a31240756794b17bd3d15dc @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

== Logs ==

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

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

Re: [igt-dev] [PATCH i-g-t 1/4] lib/intel_batchbuffer: Enable XY_FAST_COPY_BLT support for api_intel_bb

[Karolina Stolarek]

Hi Vikas,

On 24.03.2023 14:33, Vikas Srivastava wrote:
> Test case uses legacy command XY_SRC_COPY_BLT_CMD which is
> not supported on newer platforms. Modified test to use

nit: The recommendation of using imperative mood in commit comments also 
applies to descriptions. s/Modified/Modify/

> XY_FAST_COPY_BLT.
> 
> Signed-off-by: Vikas Srivastava <vikas.srivastava@intel.com>
> ---
>   lib/intel_batchbuffer.c | 63 +++++++++++++++++++++++++++--------------
>   1 file changed, 42 insertions(+), 21 deletions(-)
> 
> diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
> index da4c238cae..b1eff42fcb 100644
> --- a/lib/intel_batchbuffer.c
> +++ b/lib/intel_batchbuffer.c
> @@ -2407,11 +2407,17 @@ uint32_t intel_bb_copy_data(struct intel_bb *ibb,
>    */
>   void intel_bb_blit_start(struct intel_bb *ibb, uint32_t flags)
>   {
> -	intel_bb_out(ibb, XY_SRC_COPY_BLT_CMD |
> -		     XY_SRC_COPY_BLT_WRITE_ALPHA |
> -		     XY_SRC_COPY_BLT_WRITE_RGB |
> -		     flags |
> -		     (6 + 2 * (ibb->gen >= 8)));
> +	if (blt_has_fast_copy(ibb->i915))
> +		intel_bb_out(ibb, XY_FAST_COPY_BLT | flags);
> +	else if (blt_has_xy_src_copy(ibb->i915))
> +

^--- empty line, it'd be better to delete it

> +		intel_bb_out(ibb, XY_SRC_COPY_BLT_CMD |
> +			     XY_SRC_COPY_BLT_WRITE_ALPHA |
> +			     XY_SRC_COPY_BLT_WRITE_RGB |
> +			     flags |
> +			     (6 + 2 * (ibb->gen >= 8)));
> +	else
> +		igt_assert_f(0, "No supported blit command found\n");
>   }
>   
>   /*
> @@ -2449,12 +2455,22 @@ void intel_bb_emit_blt_copy(struct intel_bb *ibb,
>   
>   	if (gen >= 4 && src->tiling != I915_TILING_NONE) {
>   		src_pitch /= 4;
> -		cmd_bits |= XY_SRC_COPY_BLT_SRC_TILED;
> +		if (blt_has_fast_copy(ibb->i915))
> +			cmd_bits |= fast_copy_dword0(src->tiling, dst->tiling);

I think you've run in a strange bug with that fast-copy that I didn't 
have time to investigate. I was able to fix the issue locally by 
reordering all of your checks (that is, first check for XY_SRC_COPY_BLT 
and then do if else on XY_FAST_COPY_BLT support). Remember to test it, 
on KBL for example, before sending the next version.

> +		else if (blt_has_xy_src_copy(ibb->i915))
> +			cmd_bits |= XY_SRC_COPY_BLT_SRC_TILED;
> +		else
> +			igt_assert_f(0, "No supported blit command found\n");
>   	}
>   
>   	if (gen >= 4 && dst->tiling != I915_TILING_NONE) {
>   		dst_pitch /= 4;
> -		cmd_bits |= XY_SRC_COPY_BLT_DST_TILED;
> +		if (blt_has_fast_copy(ibb->i915))
> +			cmd_bits |= fast_copy_dword0(src->tiling, dst->tiling);
> +		else if (blt_has_xy_src_copy(ibb->i915))
> +			cmd_bits |= XY_SRC_COPY_BLT_DST_TILED;
> +		else
> +			igt_assert_f(0, "No supported blit command found\n");
>   	}
>   
>   	CHECK_RANGE(src_x1); CHECK_RANGE(src_y1);
> @@ -2465,20 +2481,25 @@ void intel_bb_emit_blt_copy(struct intel_bb *ibb,
>   	CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
>   
>   	br13_bits = 0;
> -	switch (bpp) {
> -	case 8:
> -		break;
> -	case 16:		/* supporting only RGB565, not ARGB1555 */
> -		br13_bits |= 1 << 24;
> -		break;
> -	case 32:
> -		br13_bits |= 3 << 24;
> -		cmd_bits |= (XY_SRC_COPY_BLT_WRITE_ALPHA |
> -			     XY_SRC_COPY_BLT_WRITE_RGB);
> -		break;
> -	default:
> -		igt_fail(IGT_EXIT_FAILURE);
> -	}
> +	if (blt_has_xy_src_copy(ibb->i915)) {
> +		switch (bpp) {
> +		case 8:
> +			break;
> +		case 16:		/* supporting only RGB565, not ARGB1555 */
> +			br13_bits |= 1 << 24;
> +			break;
> +		case 32:
> +			br13_bits |= 3 << 24;
> +			cmd_bits |= (XY_SRC_COPY_BLT_WRITE_ALPHA |
> +				     XY_SRC_COPY_BLT_WRITE_RGB);
> +			break;
> +		default:
> +			igt_fail(IGT_EXIT_FAILURE);
> +		}
> +	} else if (blt_has_fast_copy(ibb->i915)) {
> +		br13_bits = fast_copy_dword1(ibb->i915, src->tiling, dst->tiling, bpp);
> +	} else
> +		igt_assert_f(0, "No supported blit command found\n");

Please add brackets to "else" as well.

All the best,
Karolina

>   
>   	if ((src->tiling | dst->tiling) >= I915_TILING_Y) {
>   		intel_bb_out(ibb, MI_LOAD_REGISTER_IMM(1));

Re: [igt-dev] [PATCH i-g-t 1/4] lib/intel_batchbuffer: Enable XY_FAST_COPY_BLT support for api_intel_bb

[Kamil Konieczny]

Hi Vikas,

On 2023-03-24 at 19:03:43 +0530, Vikas Srivastava wrote:
> Test case uses legacy command XY_SRC_COPY_BLT_CMD which is
> not supported on newer platforms. Modified test to use
> XY_FAST_COPY_BLT.
> 

Put here Cc like:

Cc: Karolina Stolarek <karolina.stolarek@intel.com>
> Signed-off-by: Vikas Srivastava <vikas.srivastava@intel.com>
> ---
>  lib/intel_batchbuffer.c | 63 +++++++++++++++++++++++++++--------------
>  1 file changed, 42 insertions(+), 21 deletions(-)
> 
> diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
> index da4c238cae..b1eff42fcb 100644
> --- a/lib/intel_batchbuffer.c
> +++ b/lib/intel_batchbuffer.c
> @@ -2407,11 +2407,17 @@ uint32_t intel_bb_copy_data(struct intel_bb *ibb,
>   */
>  void intel_bb_blit_start(struct intel_bb *ibb, uint32_t flags)
>  {
> -	intel_bb_out(ibb, XY_SRC_COPY_BLT_CMD |
> -		     XY_SRC_COPY_BLT_WRITE_ALPHA |
> -		     XY_SRC_COPY_BLT_WRITE_RGB |
> -		     flags |
> -		     (6 + 2 * (ibb->gen >= 8)));
> +	if (blt_has_fast_copy(ibb->i915))
> +		intel_bb_out(ibb, XY_FAST_COPY_BLT | flags);
> +	else if (blt_has_xy_src_copy(ibb->i915))
> +
> +		intel_bb_out(ibb, XY_SRC_COPY_BLT_CMD |
> +			     XY_SRC_COPY_BLT_WRITE_ALPHA |
> +			     XY_SRC_COPY_BLT_WRITE_RGB |
> +			     flags |
> +			     (6 + 2 * (ibb->gen >= 8)));
> +	else
> +		igt_assert_f(0, "No supported blit command found\n");
>  }
>  
>  /*
> @@ -2449,12 +2455,22 @@ void intel_bb_emit_blt_copy(struct intel_bb *ibb,
>  
>  	if (gen >= 4 && src->tiling != I915_TILING_NONE) {
>  		src_pitch /= 4;
> -		cmd_bits |= XY_SRC_COPY_BLT_SRC_TILED;
> +		if (blt_has_fast_copy(ibb->i915))
> +			cmd_bits |= fast_copy_dword0(src->tiling, dst->tiling);
> +		else if (blt_has_xy_src_copy(ibb->i915))
> +			cmd_bits |= XY_SRC_COPY_BLT_SRC_TILED;
> +		else
> +			igt_assert_f(0, "No supported blit command found\n");
>  	}
>  
>  	if (gen >= 4 && dst->tiling != I915_TILING_NONE) {
>  		dst_pitch /= 4;
> -		cmd_bits |= XY_SRC_COPY_BLT_DST_TILED;
> +		if (blt_has_fast_copy(ibb->i915))
> +			cmd_bits |= fast_copy_dword0(src->tiling, dst->tiling);
> +		else if (blt_has_xy_src_copy(ibb->i915))
-------------------- ^
Here you can use else without if() as it was already checked above.

> +			cmd_bits |= XY_SRC_COPY_BLT_DST_TILED;
> +		else
> +			igt_assert_f(0, "No supported blit command found\n");

This was ruled out with SRC_TILED above so no need to repeat
"else igt_assert" here.

>  	}
>  
>  	CHECK_RANGE(src_x1); CHECK_RANGE(src_y1);
> @@ -2465,20 +2481,25 @@ void intel_bb_emit_blt_copy(struct intel_bb *ibb,
>  	CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
>  
>  	br13_bits = 0;
> -	switch (bpp) {
> -	case 8:
> -		break;
> -	case 16:		/* supporting only RGB565, not ARGB1555 */
> -		br13_bits |= 1 << 24;
> -		break;
> -	case 32:
> -		br13_bits |= 3 << 24;
> -		cmd_bits |= (XY_SRC_COPY_BLT_WRITE_ALPHA |
> -			     XY_SRC_COPY_BLT_WRITE_RGB);
> -		break;
> -	default:
> -		igt_fail(IGT_EXIT_FAILURE);
> -	}
> +	if (blt_has_xy_src_copy(ibb->i915)) {

The instruction for dword0 and dword1 should be of the same kind,
so if you will use fast_copy if it is supported then it should
be used for both dw0 and dw1 as Karolina observed.

> +		switch (bpp) {
> +		case 8:
> +			break;
> +		case 16:		/* supporting only RGB565, not ARGB1555 */
> +			br13_bits |= 1 << 24;
> +			break;
> +		case 32:
> +			br13_bits |= 3 << 24;
> +			cmd_bits |= (XY_SRC_COPY_BLT_WRITE_ALPHA |
> +				     XY_SRC_COPY_BLT_WRITE_RGB);
> +			break;
> +		default:
> +			igt_fail(IGT_EXIT_FAILURE);
> +		}
> +	} else if (blt_has_fast_copy(ibb->i915)) {
-------------- ^
Here you can use else without if().

> +		br13_bits = fast_copy_dword1(ibb->i915, src->tiling, dst->tiling, bpp);
> +	} else
> +		igt_assert_f(0, "No supported blit command found\n");

Same here, no need for that final else.

Regards,
Kamil

>  
>  	if ((src->tiling | dst->tiling) >= I915_TILING_Y) {
>  		intel_bb_out(ibb, MI_LOAD_REGISTER_IMM(1));
> -- 
> 2.25.1
> 

Re: [igt-dev] [PATCH i-g-t 2/4] tests/i915/gem_caching: Enable XY_FAST_COPY_BLT for MTL

[Karolina Stolarek]

On 24.03.2023 14:33, Vikas Srivastava wrote:
> Test case uses legacy command XY_SRC_COPY_BLT_CMD which
> is not supported on newer platforms. Modified test to
> use XY_FAST_COPY_BLT.
> 
> Signed-off-by: Vikas Srivastava <vikas.srivastava@intel.com>
> ---
>   tests/i915/gem_caching.c | 25 ++++++++++++++++---------
>   1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/i915/gem_caching.c b/tests/i915/gem_caching.c
> index eb0170abca..6a7f9fad06 100644
> --- a/tests/i915/gem_caching.c
> +++ b/tests/i915/gem_caching.c
> @@ -39,6 +39,7 @@
>   
>   #include "i915/gem.h"
>   #include "igt.h"
> +#include "i915/i915_blt.h"
>   
>   IGT_TEST_DESCRIPTION("Test snoop consistency when touching partial"
>   		     " cachelines.");
> @@ -81,16 +82,22 @@ copy_bo(struct intel_bb *ibb, struct intel_buf *src, struct intel_buf *dst)
>   
>   	intel_bb_add_intel_buf(ibb, src, false);
>   	intel_bb_add_intel_buf(ibb, dst, true);
> +	if (blt_has_fast_copy(ibb->i915)) {
> +		intel_bb_out(ibb, XY_FAST_COPY_BLT);
> +		intel_bb_out(ibb, XY_FAST_COPY_COLOR_DEPTH_32 | 4096);
> +	} else if (blt_has_xy_src_copy(ibb->i915)) {

Please double-check if this works on older gens, because we might have a 
problem similar to the one I described in review for the first patch.

> +		intel_bb_out(ibb,
> +			     XY_SRC_COPY_BLT_CMD |
> +			     XY_SRC_COPY_BLT_WRITE_ALPHA |
> +			     XY_SRC_COPY_BLT_WRITE_RGB |
> +			     (6 + 2 * has_64b_reloc));
> +
> +		intel_bb_out(ibb, (3 << 24) | /* 32 bits */
> +			     (0xcc << 16) | /* copy ROP */
> +			     4096);
> +	} else
> +		igt_assert_f(0, "No supported blit command found\n");

It would be good to also add brackets here (checkpatch.pl also complains 
about it)

Thanks,
Karolina

>   
> -	intel_bb_out(ibb,
> -		     XY_SRC_COPY_BLT_CMD |
> -		     XY_SRC_COPY_BLT_WRITE_ALPHA |
> -		     XY_SRC_COPY_BLT_WRITE_RGB |
> -		     (6 + 2 * has_64b_reloc));
> -
> -	intel_bb_out(ibb, (3 << 24) | /* 32 bits */
> -		     (0xcc << 16) | /* copy ROP */
> -		     4096);
>   	intel_bb_out(ibb, 0 << 16 | 0);
>   	intel_bb_out(ibb, (BO_SIZE/4096) << 16 | 1024);
>   	intel_bb_emit_reloc_fenced(ibb, dst->handle,

Re: [igt-dev] [PATCH i-g-t 1/4] lib/intel_batchbuffer: Enable XY_FAST_COPY_BLT support for api_intel_bb

[Karolina Stolarek]

On 27.03.2023 14:52, Kamil Konieczny wrote:
> Hi Vikas,
> 
> On 2023-03-24 at 19:03:43 +0530, Vikas Srivastava wrote:
>> Test case uses legacy command XY_SRC_COPY_BLT_CMD which is
>> not supported on newer platforms. Modified test to use
>> XY_FAST_COPY_BLT.
>>
> 
> Put here Cc like:
> 
> Cc: Karolina Stolarek <karolina.stolarek@intel.com>
>> Signed-off-by: Vikas Srivastava <vikas.srivastava@intel.com>
>> ---
>>   lib/intel_batchbuffer.c | 63 +++++++++++++++++++++++++++--------------
>>   1 file changed, 42 insertions(+), 21 deletions(-)
>>
>> diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
>> index da4c238cae..b1eff42fcb 100644
>> --- a/lib/intel_batchbuffer.c
>> +++ b/lib/intel_batchbuffer.c
>> @@ -2407,11 +2407,17 @@ uint32_t intel_bb_copy_data(struct intel_bb *ibb,
>>    */
>>   void intel_bb_blit_start(struct intel_bb *ibb, uint32_t flags)
>>   {
>> -	intel_bb_out(ibb, XY_SRC_COPY_BLT_CMD |
>> -		     XY_SRC_COPY_BLT_WRITE_ALPHA |
>> -		     XY_SRC_COPY_BLT_WRITE_RGB |
>> -		     flags |
>> -		     (6 + 2 * (ibb->gen >= 8)));
>> +	if (blt_has_fast_copy(ibb->i915))
>> +		intel_bb_out(ibb, XY_FAST_COPY_BLT | flags);
>> +	else if (blt_has_xy_src_copy(ibb->i915))
>> +
>> +		intel_bb_out(ibb, XY_SRC_COPY_BLT_CMD |
>> +			     XY_SRC_COPY_BLT_WRITE_ALPHA |
>> +			     XY_SRC_COPY_BLT_WRITE_RGB |
>> +			     flags |
>> +			     (6 + 2 * (ibb->gen >= 8)));
>> +	else
>> +		igt_assert_f(0, "No supported blit command found\n");
>>   }
>>   
>>   /*
>> @@ -2449,12 +2455,22 @@ void intel_bb_emit_blt_copy(struct intel_bb *ibb,
>>   
>>   	if (gen >= 4 && src->tiling != I915_TILING_NONE) {
>>   		src_pitch /= 4;
>> -		cmd_bits |= XY_SRC_COPY_BLT_SRC_TILED;
>> +		if (blt_has_fast_copy(ibb->i915))
>> +			cmd_bits |= fast_copy_dword0(src->tiling, dst->tiling);
>> +		else if (blt_has_xy_src_copy(ibb->i915))
>> +			cmd_bits |= XY_SRC_COPY_BLT_SRC_TILED;
>> +		else
>> +			igt_assert_f(0, "No supported blit command found\n");
>>   	}
>>   
>>   	if (gen >= 4 && dst->tiling != I915_TILING_NONE) {
>>   		dst_pitch /= 4;
>> -		cmd_bits |= XY_SRC_COPY_BLT_DST_TILED;
>> +		if (blt_has_fast_copy(ibb->i915))
>> +			cmd_bits |= fast_copy_dword0(src->tiling, dst->tiling);
>> +		else if (blt_has_xy_src_copy(ibb->i915))
> -------------------- ^
> Here you can use else without if() as it was already checked above.

I think we want to select either fast or xy_src blit, not both. Most 
platforms would go with both ifs.

> 
>> +			cmd_bits |= XY_SRC_COPY_BLT_DST_TILED;
>> +		else
>> +			igt_assert_f(0, "No supported blit command found\n");
> 
> This was ruled out with SRC_TILED above so no need to repeat
> "else igt_assert" here.

It's a belt and suspenders approach, as this shouldn't be happening, but 
this assert will show us quickly if we miss important definitions in 
intel_cmds_info. I did something similar in my gem_blits patches, so 
that's why I recommended doing it here.

Many thanks,
Karolina

> 
>>   	}
>>   
>>   	CHECK_RANGE(src_x1); CHECK_RANGE(src_y1);
>> @@ -2465,20 +2481,25 @@ void intel_bb_emit_blt_copy(struct intel_bb *ibb,
>>   	CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
>>   
>>   	br13_bits = 0;
>> -	switch (bpp) {
>> -	case 8:
>> -		break;
>> -	case 16:		/* supporting only RGB565, not ARGB1555 */
>> -		br13_bits |= 1 << 24;
>> -		break;
>> -	case 32:
>> -		br13_bits |= 3 << 24;
>> -		cmd_bits |= (XY_SRC_COPY_BLT_WRITE_ALPHA |
>> -			     XY_SRC_COPY_BLT_WRITE_RGB);
>> -		break;
>> -	default:
>> -		igt_fail(IGT_EXIT_FAILURE);
>> -	}
>> +	if (blt_has_xy_src_copy(ibb->i915)) {
> 
> The instruction for dword0 and dword1 should be of the same kind,
> so if you will use fast_copy if it is supported then it should
> be used for both dw0 and dw1 as Karolina observed.
> 
>> +		switch (bpp) {
>> +		case 8:
>> +			break;
>> +		case 16:		/* supporting only RGB565, not ARGB1555 */
>> +			br13_bits |= 1 << 24;
>> +			break;
>> +		case 32:
>> +			br13_bits |= 3 << 24;
>> +			cmd_bits |= (XY_SRC_COPY_BLT_WRITE_ALPHA |
>> +				     XY_SRC_COPY_BLT_WRITE_RGB);
>> +			break;
>> +		default:
>> +			igt_fail(IGT_EXIT_FAILURE);
>> +		}
>> +	} else if (blt_has_fast_copy(ibb->i915)) {
> -------------- ^
> Here you can use else without if().
> 
>> +		br13_bits = fast_copy_dword1(ibb->i915, src->tiling, dst->tiling, bpp);
>> +	} else
>> +		igt_assert_f(0, "No supported blit command found\n");
> 
> Same here, no need for that final else.
> 
> Regards,
> Kamil
> 
>>   
>>   	if ((src->tiling | dst->tiling) >= I915_TILING_Y) {
>>   		intel_bb_out(ibb, MI_LOAD_REGISTER_IMM(1));
>> -- 
>> 2.25.1
>>

Re: [igt-dev] [PATCH i-g-t 3/4] tests/i915/gem_userptr_blits: Enable XY_FAST_COPY_BLT Command for gen12+

[Karolina Stolarek]

On 24.03.2023 14:33, Vikas Srivastava wrote:
> Test case uses legacy command XY_SRC_COPY_BLT_CMD which is
> not supported on newer platforms. Modified test
> to use XY_FAST_COPY_BLT.
> 
> Signed-off-by: Vikas Srivastava <vikas.srivastava@intel.com>
> ---
>   tests/i915/gem_userptr_blits.c | 82 +++++++++++++++++++++-------------
>   1 file changed, 52 insertions(+), 30 deletions(-)
> 
> diff --git a/tests/i915/gem_userptr_blits.c b/tests/i915/gem_userptr_blits.c
> index 07a453229a..dba11efb88 100644
> --- a/tests/i915/gem_userptr_blits.c
> +++ b/tests/i915/gem_userptr_blits.c
> @@ -66,6 +66,7 @@
>   #include "sw_sync.h"
>   
>   #include "eviction_common.c"
> +#include "i915/i915_blt.h"
>   
>   #ifndef PAGE_SIZE
>   #define PAGE_SIZE 4096
> @@ -99,6 +100,7 @@ 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;
>   	uint64_t dst_offset, src_offset, bb_offset;
> @@ -108,29 +110,48 @@ 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 (blt_has_fast_copy(fd)) {

Apart from two small comments, the patch looks good, but please could 
you check if you have to re-order the checks here as well? We still 
don't have full run results, so you can either wait for the results or 
test it locally on SKL or KBL.

> +		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++] = lower_32_bits(dst_offset); /* dst address */
> +		batch[i++] = upper_32_bits(CANONICAL(dst_offset));
> +		batch[i++] = 0;/* src x1,y1 */
> +		batch[i++] = WIDTH * 4;/* src pitch */
> +		batch[i++] = lower_32_bits(src_offset); /* src address */
> +		batch[i++] = upper_32_bits(CANONICAL(src_offset));
> +		batch[i++] = MI_BATCH_BUFFER_END;
> +		batch[i++] = MI_NOOP;
> +	} else if (blt_has_xy_src_copy(fd)) {
> +		batch[i++] = XY_SRC_COPY_BLT_CMD |
> +			     XY_SRC_COPY_BLT_WRITE_ALPHA |
> +			     XY_SRC_COPY_BLT_WRITE_RGB;
> +
> +		if (intel_gen(devid) >= 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++] = lower_32_bits(dst_offset); /*dst reloc*/

nit: Add spaces to "dst reloc", as it seems like it was how it was 
defined before.

> +		if (intel_gen(devid) >= 8)
> +			batch[i++] = upper_32_bits(CANONICAL(dst_offset));
> +		batch[i++] = 0; /* src x1,y1 */
> +		batch[i++] = WIDTH * 4;
> +		batch[i++] = lower_32_bits(src_offset); /* src reloc */
> +		if (intel_gen(devid) >= 8)
> +			batch[i++] = upper_32_bits(CANONICAL(src_offset));
> +		batch[i++] = MI_BATCH_BUFFER_END;
> +		batch[i++] = MI_NOOP;
> +	} else
> +		igt_assert_f(0, "No supported blit command found\n");

Similar comment to the one I gave earlier -- it would be good to balance 
the brackets in this section.

Many thanks,
Karolina
>   
>   	handle = gem_create(fd, 4096);
>   	gem_write(fd, handle, 0, batch, sizeof(batch));
> @@ -254,20 +275,20 @@ blit(int fd, uint32_t dst, uint32_t src, uint32_t *all_bo, int n_bo)
>   	reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
>   	reloc[1].write_domain = 0;
>   
> -	if (intel_graphics_ver(devid) >= IP_VER(12, 60)) {
> +	if (blt_has_fast_copy(fd)) {
>   		batch[i++] = XY_FAST_COPY_BLT;
> -		batch[i++] = XY_FAST_COPY_COLOR_DEPTH_32 | WIDTH*4;
> +		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++] = lower_32_bits(dst_offset); /* dst address */
>   		batch[i++] = upper_32_bits(CANONICAL(dst_offset));
>   		batch[i++] = 0; /* src x1,y1 */
> -		batch[i++] = WIDTH*4; /* src pitch */
> +		batch[i++] = WIDTH * 4; /* src pitch */
>   		batch[i++] = lower_32_bits(src_offset); /* src address */
>   		batch[i++] = upper_32_bits(CANONICAL(src_offset));
>   		batch[i++] = MI_BATCH_BUFFER_END;
>   		batch[i++] = MI_NOOP;
> -	} else {
> +	} else if (blt_has_xy_src_copy(fd)) {
>   		batch[i++] = XY_SRC_COPY_BLT_CMD |
>   			     XY_SRC_COPY_BLT_WRITE_ALPHA |
>   			     XY_SRC_COPY_BLT_WRITE_RGB;
> @@ -277,20 +298,21 @@ blit(int fd, uint32_t dst, uint32_t src, uint32_t *all_bo, int n_bo)
>   			batch[i - 1] |= 6;
>   		batch[i++] = (3 << 24) | /* 32 bits */
>   			     (0xcc << 16) | /* copy ROP */
> -			     WIDTH*4;
> +			     WIDTH * 4;
>   		batch[i++] = 0; /* dst x1,y1 */
>   		batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
>   		batch[i++] = lower_32_bits(dst_offset);
>   		if (intel_gen(devid) >= 8)
>   			batch[i++] = upper_32_bits(CANONICAL(dst_offset));
>   		batch[i++] = 0; /* src x1,y1 */
> -		batch[i++] = WIDTH*4;
> +		batch[i++] = WIDTH * 4;
>   		batch[i++] = lower_32_bits(src_offset);
>   		if (intel_gen(devid) >= 8)
>   			batch[i++] = upper_32_bits(CANONICAL(src_offset));
>   		batch[i++] = MI_BATCH_BUFFER_END;
>   		batch[i++] = MI_NOOP;
> -	}
> +	} else
> +		igt_assert_f(0, "No supported blit command found\n");
>   
>   	gem_write(fd, handle, 0, batch, sizeof(batch));
>   

Re: [igt-dev] [PATCH i-g-t 4/4] tests/i915/gem_linear_blits: Enable XY_FAST_COPY_BLT copy instruction

[Karolina Stolarek]

On 24.03.2023 14:33, Vikas Srivastava wrote:
> From: Arjun Melkaveri <arjun.melkaveri@intel.com>
> 
> Test case uses legacy command XY_SRC_COPY_BLT_CMD which is
> not supported on newer platforms. Modified test to
> use XY_FAST_COPY_BLT.
> 
> Signed-off-by: Arjun Melkaveri <arjun.melkaveri@intel.com>
> Co-developed-by: Vikas Srivastava <vikas.srivastava@intel.com>

Nice use of the tag, I should probably start using it in my patches as
well...

> Signed-off-by: Vikas Srivastava <vikas.srivastava@intel.com>
> ---
>   tests/i915/gem_linear_blits.c | 66 +++++++++++++++++++++++------------
>   1 file changed, 43 insertions(+), 23 deletions(-)
> 
> diff --git a/tests/i915/gem_linear_blits.c b/tests/i915/gem_linear_blits.c
> index fac25095f5..6fa6cd4976 100644
> --- a/tests/i915/gem_linear_blits.c
> +++ b/tests/i915/gem_linear_blits.c
> @@ -48,6 +48,7 @@
>   #include "i915/gem_create.h"
>   #include "igt.h"
>   #include "igt_types.h"
> +#include "i915/i915_blt.h"
>   
>   IGT_TEST_DESCRIPTION("Test doing many blits with a working set larger than the"
>   		     " aperture size.");
> @@ -67,6 +68,7 @@ static void copy(int fd, uint64_t ahnd, 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;
>   	int i = 0;
>   
>   	memset(obj, 0, sizeof(obj));
> @@ -83,29 +85,47 @@ static void copy(int fd, uint64_t ahnd, uint32_t dst, uint32_t src,
>   	obj[2].offset = CANONICAL(obj[2].offset);
>   	obj[2].flags = EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
>   
> -	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++] = obj[0].offset;
> -	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> -		batch[i++] = obj[0].offset >> 32;
> -	batch[i++] = 0; /* src x1,y1 */
> -	batch[i++] = WIDTH*4;
> -	batch[i++] = obj[1].offset;
> -	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> -		batch[i++] = obj[1].offset >> 32;
> -	batch[i++] = MI_BATCH_BUFFER_END;
> -	batch[i++] = MI_NOOP;
> +	devid = intel_get_drm_devid(fd);
> +
> +	if (blt_has_fast_copy(fd)) {

Don't forget to check the order, like I said in 1/4.

> +		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++] = obj[0].offset; /* dst address lower bits */
> +		batch[i++] = obj[0].offset >> 32; /* dst address upper bits */
> +		batch[i++] = 0; /* src x1,y1 */
> +		batch[i++] = WIDTH * 4; /* src pitch */

Just wanted to thank you for addressing my comments, even these for such 
minor changes :)

> +		batch[i++] = obj[1].offset; /* src address lower bits */
> +		batch[i++] = obj[1].offset >> 32; /* src address upper bits */
> +		batch[i++] = MI_BATCH_BUFFER_END;
> +		batch[i++] = MI_NOOP;
> +	} else if (blt_has_xy_src_copy(fd)) {
> +		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++] = obj[0].offset;
> +		if (intel_gen(devid) >= 8)
> +			batch[i++] = obj[0].offset >> 32;
> +		batch[i++] = 0; /* src x1,y1 */
> +		batch[i++] = WIDTH * 4;
> +		batch[i++] = obj[1].offset;
> +		if (intel_gen(devid) >= 8)
> +			batch[i++] = obj[1].offset >> 32;
> +		batch[i++] = MI_BATCH_BUFFER_END;
> +		batch[i++] = MI_NOOP;
> +	} else
> +		igt_assert_f(0, "No supported blit command found\n");

My comment on the brackets applies here as well.

So, let's wait for the CI results (or check this and 3/4 locally), and 
if everything is fine and brackets are balanced, I can r-b these patches.

Many thanks,
Karolina

>   
>   	gem_write(fd, obj[2].handle, 0, batch, i * sizeof(batch[0]));
>