[igt-dev] [PATCH i-g-t 1/9] lib/ioctl_wrappers: Query if device supports set/get legacy tiling

[igt-dev] [PATCH i-g-t 1/9] lib/ioctl_wrappers: Query if device supports set/get legacy tiling

[Imre Deak]

From: Vanshidhar Konda <vanshidhar.r.konda@intel.com>

Add a method to query if the device supports setting and getting legacy
tiling formats for buffer objects.

Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 lib/ioctl_wrappers.c | 17 +++++++++++++++++
 lib/ioctl_wrappers.h |  1 +
 2 files changed, 18 insertions(+)

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 627717d2..c1abb575 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -133,6 +133,23 @@ __gem_get_tiling(int fd, struct drm_i915_gem_get_tiling *arg)
 	return err;
 }
 
+/**
+ * gem_has_legacy_hw_tiling:
+ * @fd: open i915 drm file descriptor
+ *
+ * Feature check to query if the device supports setting/getting
+ * legacy tiling formats for buffer objects
+ *
+ * Returns: True if tiling is supported
+ */
+bool
+gem_has_legacy_hw_tiling(int fd)
+{
+	struct drm_i915_gem_get_tiling arg = {};
+
+	return (__gem_get_tiling(fd, &arg) != -EOPNOTSUPP);
+}
+
 /**
  * gem_get_tiling:
  * @fd: open i915 drm file descriptor
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index 7614e688..0ea77738 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -146,6 +146,7 @@ void gem_require_caching(int fd);
 void gem_require_ring(int fd, unsigned ring);
 bool gem_has_mocs_registers(int fd);
 void gem_require_mocs_registers(int fd);
+bool gem_has_legacy_hw_tiling(int fd);
 
 #define gem_has_ring(f, r) gem_context_has_engine(f, 0, r)
 
-- 
2.23.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 2/9] lib/igt_fb: Fix creating FBs on platforms w/o HW detiling

[Imre Deak]

On platforms w/o HW detiling don't fail creating the FB due to the
expected error from the set_tiling IOCTL.

Most of the tests use a cairo surface to draw, which don't depend on the
HW detiling. Other tests (using lib/igt_draw.c or drawing to the FB
directly, like kms_draw_crc, kms_frontbuffer) are failing atm and will
need to be fixed separately.

Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 lib/igt_fb.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index c81b9de8..ec7e9991 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -944,9 +944,10 @@ static int create_bo_for_fb(struct igt_fb *fb)
 
 		if (is_i915_device(fd)) {
 			fb->gem_handle = gem_create(fd, fb->size);
-			gem_set_tiling(fd, fb->gem_handle,
-				       igt_fb_mod_to_tiling(fb->modifier),
-				       fb->strides[0]);
+			if (gem_has_legacy_hw_tiling(fd))
+				gem_set_tiling(fd, fb->gem_handle,
+					       igt_fb_mod_to_tiling(fb->modifier),
+					       fb->strides[0]);
 		} else if (is_vc4_device(fd)) {
 			fb->gem_handle = igt_vc4_create_bo(fd, fb->size);
 
-- 
2.23.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 3/9] lib/intel_batchbuffer: Add blitter copy using XY_SRC_COPY_BLT

[Imre Deak]

From: Vanshidhar Konda <vanshidhar.r.konda@intel.com>

Add a method that uses the XY_SRC_COPY_BLT instruction for copying
buffers using the blitter engine.

v2: Use uint32_t for parameters; fix stride for Gen2/3

Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 lib/intel_batchbuffer.c | 183 ++++++++++++++++++++++++++++++++++++++++
 lib/intel_batchbuffer.h |  21 +++++
 2 files changed, 204 insertions(+)

diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
index ab907913..c4bf7ef4 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -47,6 +47,12 @@
 
 #include <i915_drm.h>
 
+#define MI_FLUSH_DW (0x26 << 23)
+
+#define BCS_SWCTRL 0x22200
+#define BCS_SRC_Y (1 << 0)
+#define BCS_DST_Y (1 << 1)
+
 /**
  * SECTION:intel_batchbuffer
  * @short_description: Batchbuffer and blitter support
@@ -708,6 +714,183 @@ static void exec_blit(int fd,
 	gem_execbuf(fd, &exec);
 }
 
+static uint32_t src_copy_dword0(uint32_t src_tiling, uint32_t dst_tiling,
+				uint32_t bpp, uint32_t device_gen)
+{
+	uint32_t dword0 = 0;
+
+	dword0 |= XY_SRC_COPY_BLT_CMD;
+	if (bpp == 32)
+		dword0 |= XY_SRC_COPY_BLT_WRITE_RGB |
+			XY_SRC_COPY_BLT_WRITE_ALPHA;
+
+	if (device_gen >= 4 && src_tiling)
+		dword0 |= XY_SRC_COPY_BLT_SRC_TILED;
+	if (device_gen >= 4 && dst_tiling)
+		dword0 |= XY_SRC_COPY_BLT_DST_TILED;
+
+	return dword0;
+}
+
+static uint32_t src_copy_dword1(uint32_t dst_pitch, uint32_t bpp)
+{
+	uint32_t dword1 = 0;
+
+	switch (bpp) {
+	case 8:
+		break;
+	case 16:
+		dword1 |= (1 << 24); /* Only support 565 color */
+		break;
+	case 32:
+		dword1 |= (3 << 24);
+		break;
+	default:
+		igt_assert(0);
+	}
+
+	dword1 |= 0xcc << 16;
+	dword1 |= dst_pitch;
+
+	return dword1;
+}
+/**
+ * igt_blitter_src_copy__raw:
+ * @fd: file descriptor of the i915 driver
+ * @src_handle: GEM handle of the source buffer
+ * @src_delta: offset into the source GEM bo, in bytes
+ * @src_stride: Stride (in bytes) of the source buffer
+ * @src_tiling: Tiling mode of the source buffer
+ * @src_x: X coordinate of the source region to copy
+ * @src_y: Y coordinate of the source region to copy
+ * @width: Width of the region to copy
+ * @height: Height of the region to copy
+ * @bpp: source and destination bits per pixel
+ * @dst_handle: GEM handle of the destination buffer
+ * @dst_delta: offset into the destination GEM bo, in bytes
+ * @dst_stride: Stride (in bytes) of the destination buffer
+ * @dst_tiling: Tiling mode of the destination buffer
+ * @dst_x: X coordinate of destination
+ * @dst_y: Y coordinate of destination
+ *
+ */
+void igt_blitter_src_copy__raw(int fd,
+				/* src */
+				uint32_t src_handle,
+				uint32_t src_delta,
+				uint32_t src_stride,
+				uint32_t src_tiling,
+				uint32_t src_x, uint32_t src_y,
+
+				/* size */
+				uint32_t width, uint32_t height,
+
+				/* bpp */
+				uint32_t bpp,
+
+				/* dst */
+				uint32_t dst_handle,
+				uint32_t dst_delta,
+				uint32_t dst_stride,
+				uint32_t dst_tiling,
+				uint32_t dst_x, uint32_t dst_y)
+{
+	uint32_t batch[32];
+	struct drm_i915_gem_exec_object2 objs[3];
+	struct drm_i915_gem_relocation_entry relocs[2];
+	uint32_t batch_handle;
+	uint32_t src_pitch, dst_pitch;
+	uint32_t dst_reloc_offset, src_reloc_offset;
+	int i = 0;
+	uint32_t gen = intel_gen(intel_get_drm_devid(fd));
+	const bool has_64b_reloc = gen >= 8;
+
+	memset(batch, 0, sizeof(batch));
+
+	igt_assert((src_tiling == I915_TILING_NONE) ||
+		   (src_tiling == I915_TILING_X) ||
+		   (src_tiling == I915_TILING_Y));
+	igt_assert((dst_tiling == I915_TILING_NONE) ||
+		   (dst_tiling == I915_TILING_X) ||
+		   (dst_tiling == I915_TILING_Y));
+
+	src_pitch = (gen >= 4 && src_tiling) ? src_stride / 4 : src_stride;
+	dst_pitch = (gen >= 4 && dst_tiling) ? dst_stride / 4 : dst_stride;
+
+	CHECK_RANGE(src_x); CHECK_RANGE(src_y);
+	CHECK_RANGE(dst_x); CHECK_RANGE(dst_y);
+	CHECK_RANGE(width); CHECK_RANGE(height);
+	CHECK_RANGE(src_x + width); CHECK_RANGE(src_y + height);
+	CHECK_RANGE(dst_x + width); CHECK_RANGE(dst_y + height);
+	CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
+
+	if ((src_tiling | dst_tiling) >= I915_TILING_Y) {
+		unsigned int mask;
+
+		batch[i++] = MI_LOAD_REGISTER_IMM;
+		batch[i++] = BCS_SWCTRL;
+
+		mask = (BCS_SRC_Y | BCS_DST_Y) << 16;
+		if (src_tiling == I915_TILING_Y)
+			mask |= BCS_SRC_Y;
+		if (dst_tiling == I915_TILING_Y)
+			mask |= BCS_DST_Y;
+		batch[i++] = mask;
+	}
+
+	batch[i] = src_copy_dword0(src_tiling, dst_tiling, bpp, gen);
+	batch[i++] |= 6 + 2 * has_64b_reloc;
+	batch[i++] = src_copy_dword1(dst_pitch, bpp);
+	batch[i++] = (dst_y << 16) | dst_x; /* dst x1,y1 */
+	batch[i++] = ((dst_y + height) << 16) | (dst_x + width); /* dst x2,y2 */
+	dst_reloc_offset = i;
+	batch[i++] = dst_delta; /* dst address lower bits */
+	batch[i++] = 0;	/* dst address upper bits */
+	batch[i++] = (src_y << 16) | src_x; /* src x1,y1 */
+	batch[i++] = src_pitch;
+	src_reloc_offset = i;
+	batch[i++] = src_delta; /* src address lower bits */
+	batch[i++] = 0;	/* src address upper bits */
+
+	if ((src_tiling | dst_tiling) >= I915_TILING_Y) {
+		igt_assert(gen >= 6);
+		batch[i++] = MI_FLUSH_DW | 2;
+		batch[i++] = 0;
+		batch[i++] = 0;
+		batch[i++] = 0;
+
+		batch[i++] = MI_LOAD_REGISTER_IMM;
+		batch[i++] = BCS_SWCTRL;
+		batch[i++] = (BCS_SRC_Y | BCS_DST_Y) << 16;
+	}
+
+	batch[i++] = MI_BATCH_BUFFER_END;
+	batch[i++] = MI_NOOP;
+
+	igt_assert(i <= ARRAY_SIZE(batch));
+
+	batch_handle = gem_create(fd, 4096);
+	gem_write(fd, batch_handle, 0, batch, sizeof(batch));
+
+	fill_relocation(&relocs[0], dst_handle, dst_delta, dst_reloc_offset,
+			I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER);
+	fill_relocation(&relocs[1], src_handle, src_delta, src_reloc_offset,
+			I915_GEM_DOMAIN_RENDER, 0);
+
+	fill_object(&objs[0], dst_handle, NULL, 0);
+	fill_object(&objs[1], src_handle, NULL, 0);
+	fill_object(&objs[2], batch_handle, relocs, 2);
+
+	if (dst_tiling)
+		objs[0].flags |= EXEC_OBJECT_NEEDS_FENCE;
+	if (src_tiling)
+		objs[1].flags |= EXEC_OBJECT_NEEDS_FENCE;
+
+	exec_blit(fd, objs, 3, ARRAY_SIZE(batch));
+
+	gem_close(fd, batch_handle);
+}
+
 /**
  * igt_blitter_fast_copy__raw:
  * @fd: file descriptor of the i915 driver
diff --git a/lib/intel_batchbuffer.h b/lib/intel_batchbuffer.h
index 85eb3ffd..0fc18db1 100644
--- a/lib/intel_batchbuffer.h
+++ b/lib/intel_batchbuffer.h
@@ -265,6 +265,27 @@ unsigned igt_buf_height(const struct igt_buf *buf);
 unsigned int igt_buf_intel_ccs_width(int gen, const struct igt_buf *buf);
 unsigned int igt_buf_intel_ccs_height(int gen, const struct igt_buf *buf);
 
+void igt_blitter_src_copy__raw(int fd,
+				/* src */
+				uint32_t src_handle,
+				uint32_t src_delta,
+				uint32_t src_stride,
+				uint32_t src_tiling,
+				uint32_t src_x, uint32_t src_y,
+
+				/* size */
+				uint32_t width, uint32_t height,
+
+				/* bpp */
+				uint32_t bpp,
+
+				/* dst */
+				uint32_t dst_handle,
+				uint32_t dst_delta,
+				uint32_t dst_stride,
+				uint32_t dst_tiling,
+				uint32_t dst_x, uint32_t dst_y);
+
 void igt_blitter_fast_copy(struct intel_batchbuffer *batch,
 			   const struct igt_buf *src, unsigned src_delta,
 			   unsigned src_x, unsigned src_y,
-- 
2.23.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 4/9] lib/igt_fb: Switch from XY_FAST_COPY_BLT to XY_SRC_COPY_BLT

[Imre Deak]

From: Vanshidhar Konda <vanshidhar.r.konda@intel.com>

The XY_SRC_COPY_BLT instruction is supported on more platforms than
XY_FAST_COPY_BLT - use it for X tiling on GEN12+ copying using blitter.
For other tiling modes/platforms use the XY_FAST_COPY_BLT as before.

v2:
- Use xy_src blit only - when necessary - on GEN12+/X-tiled mods.

Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 lib/igt_fb.c | 59 +++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 45 insertions(+), 14 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index ec7e9991..53b43528 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2146,27 +2146,58 @@ static void copy_with_engine(struct fb_blit_upload *blit,
 static void blitcopy(const struct igt_fb *dst_fb,
 		     const struct igt_fb *src_fb)
 {
+	uint32_t src_tiling, dst_tiling;
+
 	igt_assert_eq(dst_fb->fd, src_fb->fd);
 	igt_assert_eq(dst_fb->num_planes, src_fb->num_planes);
 
+	src_tiling = igt_fb_mod_to_tiling(src_fb->modifier);
+	dst_tiling = igt_fb_mod_to_tiling(dst_fb->modifier);
+
 	for (int i = 0; i < dst_fb->num_planes; i++) {
+		int gen = intel_gen(intel_get_drm_devid(src_fb->fd));
+
 		igt_assert_eq(dst_fb->plane_bpp[i], src_fb->plane_bpp[i]);
 		igt_assert_eq(dst_fb->plane_width[i], src_fb->plane_width[i]);
 		igt_assert_eq(dst_fb->plane_height[i], src_fb->plane_height[i]);
-
-		igt_blitter_fast_copy__raw(dst_fb->fd,
-					   src_fb->gem_handle,
-					   src_fb->offsets[i],
-					   src_fb->strides[i],
-					   igt_fb_mod_to_tiling(src_fb->modifier),
-					   0, 0, /* src_x, src_y */
-					   dst_fb->plane_width[i], dst_fb->plane_height[i],
-					   dst_fb->plane_bpp[i],
-					   dst_fb->gem_handle,
-					   dst_fb->offsets[i],
-					   dst_fb->strides[i],
-					   igt_fb_mod_to_tiling(dst_fb->modifier),
-					   0, 0 /* dst_x, dst_y */);
+		/*
+		 * On GEN12+ X-tiled format support is removed from the fast
+		 * blit command, so use the XY_SRC blit command for it
+		 * instead.
+		 */
+		if ((gen >= 9 && gen < 12) ||
+		    (gen >= 12 && (src_tiling != I915_TILING_X &&
+				   dst_tiling != I915_TILING_X))) {
+			igt_blitter_fast_copy__raw(dst_fb->fd,
+						   src_fb->gem_handle,
+						   src_fb->offsets[i],
+						   src_fb->strides[i],
+						   src_tiling,
+						   0, 0, /* src_x, src_y */
+						   dst_fb->plane_width[i],
+						   dst_fb->plane_height[i],
+						   dst_fb->plane_bpp[i],
+						   dst_fb->gem_handle,
+						   dst_fb->offsets[i],
+						   dst_fb->strides[i],
+						   dst_tiling,
+						   0, 0 /* dst_x, dst_y */);
+		} else {
+			igt_blitter_src_copy__raw(dst_fb->fd,
+						  src_fb->gem_handle,
+						  src_fb->offsets[i],
+						  src_fb->strides[i],
+						  src_tiling,
+						  0, 0, /* src_x, src_y */
+						  dst_fb->plane_width[i],
+						  dst_fb->plane_height[i],
+						  dst_fb->plane_bpp[i],
+						  dst_fb->gem_handle,
+						  dst_fb->offsets[i],
+						  dst_fb->strides[i],
+						  dst_tiling,
+						  0, 0 /* dst_x, dst_y */);
+		}
 	}
 }
 
-- 
2.23.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 5/9] lib/igt_fb: Add 64bpp support to the XY_SRC blit command

[Imre Deak]

While the XY_SRC blit command lacks native 64bpp copy support, we can
emulate it using a 32bpp copy by treating the 64bpp plane as a 2x wide
32bpp plane. Add support for this, as we need the XY_SRC command at
least for GEN12+ X-tiled formats.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 lib/igt_fb.c            | 30 ++++++++++++++++++++++++------
 lib/intel_batchbuffer.c |  5 +++++
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 53b43528..fa294a38 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1957,9 +1957,31 @@ struct fb_blit_upload {
 	struct intel_batchbuffer *batch;
 };
 
+static bool fast_blit_ok(const struct igt_fb *fb)
+{
+	int gen = intel_gen(intel_get_drm_devid(fb->fd));
+
+	if (gen < 9)
+		return false;
+
+	if (gen < 12)
+		return true;
+
+	return fb->modifier != I915_FORMAT_MOD_X_TILED;
+}
+
 static bool blitter_ok(const struct igt_fb *fb)
 {
 	for (int i = 0; i < fb->num_planes; i++) {
+		int width = fb->plane_width[i];
+
+		/*
+		 * XY_SRC blit supports only 32bpp, but we can still use it
+		 * for a 64bpp plane by treating that as a 2x wide 32bpp plane.
+		 */
+		if (!fast_blit_ok(fb) && fb->plane_bpp[i] == 64)
+			width *= 2;
+
 		/*
 		 * gen4+ stride limit is 4x this with tiling,
 		 * but since our blits are always between tiled
@@ -1967,7 +1989,7 @@ static bool blitter_ok(const struct igt_fb *fb)
 		 * for the tiled surface) we must use the lower
 		 * linear stride limit here.
 		 */
-		if (fb->plane_width[i] > 32767 ||
+		if (width > 32767 ||
 		    fb->plane_height[i] > 32767 ||
 		    fb->strides[i] > 32767)
 			return false;
@@ -2155,8 +2177,6 @@ static void blitcopy(const struct igt_fb *dst_fb,
 	dst_tiling = igt_fb_mod_to_tiling(dst_fb->modifier);
 
 	for (int i = 0; i < dst_fb->num_planes; i++) {
-		int gen = intel_gen(intel_get_drm_devid(src_fb->fd));
-
 		igt_assert_eq(dst_fb->plane_bpp[i], src_fb->plane_bpp[i]);
 		igt_assert_eq(dst_fb->plane_width[i], src_fb->plane_width[i]);
 		igt_assert_eq(dst_fb->plane_height[i], src_fb->plane_height[i]);
@@ -2165,9 +2185,7 @@ static void blitcopy(const struct igt_fb *dst_fb,
 		 * blit command, so use the XY_SRC blit command for it
 		 * instead.
 		 */
-		if ((gen >= 9 && gen < 12) ||
-		    (gen >= 12 && (src_tiling != I915_TILING_X &&
-				   dst_tiling != I915_TILING_X))) {
+		if (fast_blit_ok(src_fb) && fast_blit_ok(dst_fb)) {
 			igt_blitter_fast_copy__raw(dst_fb->fd,
 						   src_fb->gem_handle,
 						   src_fb->offsets[i],
diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
index c4bf7ef4..2fd3a75a 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -817,6 +817,11 @@ void igt_blitter_src_copy__raw(int fd,
 	src_pitch = (gen >= 4 && src_tiling) ? src_stride / 4 : src_stride;
 	dst_pitch = (gen >= 4 && dst_tiling) ? dst_stride / 4 : dst_stride;
 
+	if (bpp == 64) {
+		bpp /= 2;
+		width *= 2;
+	}
+
 	CHECK_RANGE(src_x); CHECK_RANGE(src_y);
 	CHECK_RANGE(dst_x); CHECK_RANGE(dst_y);
 	CHECK_RANGE(width); CHECK_RANGE(height);
-- 
2.23.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 6/9] lib/igt_fb: Fall back from gtt map to coherent map on platforms w/o aperture

[Imre Deak]

Use a device coherent map instead of GGTT map on platforms w/o an
aperture.

Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 lib/igt_fb.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index fa294a38..cb594634 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2376,18 +2376,23 @@ static void destroy_cairo_surface__gtt(void *arg)
 
 static void *map_bo(int fd, struct igt_fb *fb)
 {
+	bool is_i915 = is_i915_device(fd);
 	void *ptr;
 
-	if (is_i915_device(fd))
+	if (is_i915)
 		gem_set_domain(fd, fb->gem_handle,
 			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
 
 	if (fb->is_dumb)
 		ptr = kmstest_dumb_map_buffer(fd, fb->gem_handle, fb->size,
 					      PROT_READ | PROT_WRITE);
-	else if (is_i915_device(fd))
+	else if (is_i915 && gem_has_mappable_ggtt(fd))
 		ptr = gem_mmap__gtt(fd, fb->gem_handle, fb->size,
 				    PROT_READ | PROT_WRITE);
+	else if (is_i915)
+		ptr = gem_mmap__device_coherent(fd, fb->gem_handle, 0,
+						fb->size,
+						PROT_READ | PROT_WRITE);
 	else if (is_vc4_device(fd))
 		ptr = igt_vc4_mmap_bo(fd, fb->gem_handle, fb->size,
 				      PROT_READ | PROT_WRITE);
-- 
2.23.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 7/9] lib/igt_fb: Use render copy/blit on platforms w/o HW detiling

[Imre Deak]

On platforms without HW detiling use render copy or blitting to convert
a framebuffer to a cairo surface.

Replaces [1] which used a slow device_coherent map, instead of the
faster engine copy in this version.

[1] https://patchwork.freedesktop.org/patch/349742/?series=72308&rev=1

Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 lib/igt_fb.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index cb594634..30f3bfba 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1972,6 +1972,9 @@ static bool fast_blit_ok(const struct igt_fb *fb)
 
 static bool blitter_ok(const struct igt_fb *fb)
 {
+	if (is_ccs_modifier(fb->modifier))
+		return false;
+
 	for (int i = 0; i < fb->num_planes; i++) {
 		int width = fb->plane_width[i];
 
@@ -2000,16 +2003,22 @@ static bool blitter_ok(const struct igt_fb *fb)
 
 static bool use_enginecopy(const struct igt_fb *fb)
 {
-	return is_ccs_modifier(fb->modifier) ||
-		(fb->modifier == I915_FORMAT_MOD_Yf_TILED &&
-		 !blitter_ok(fb));
+	if (blitter_ok(fb))
+		return false;
+
+	return fb->modifier == I915_FORMAT_MOD_Yf_TILED ||
+	       is_ccs_modifier(fb->modifier) ||
+	       !gem_has_mappable_ggtt(fb->fd);
 }
 
 static bool use_blitter(const struct igt_fb *fb)
 {
-	return (fb->modifier == I915_FORMAT_MOD_Y_TILED ||
-		fb->modifier == I915_FORMAT_MOD_Yf_TILED) &&
-		blitter_ok(fb);
+	if (!blitter_ok(fb))
+		return false;
+
+	return fb->modifier == I915_FORMAT_MOD_Y_TILED ||
+	       fb->modifier == I915_FORMAT_MOD_Yf_TILED ||
+	       !gem_has_mappable_ggtt(fb->fd);
 }
 
 static void init_buf_ccs(struct igt_buf *buf, int ccs_idx,
-- 
2.23.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 8/9] lib/igt_fb: Speed up format conversion for local memory

[Imre Deak]

To speed up the conversion that needs to read from a dGFX local memory
use the same trick as what's used for GTT apertures and make a copy
first into system memory.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 lib/igt_fb.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 30f3bfba..ef3fa2ed 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -3427,6 +3427,10 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
 	if (use_enginecopy(fb) || use_blitter(fb) ||
 	    igt_vc4_is_tiled(fb->modifier)) {
 		setup_linear_mapping(&blit->base);
+
+		/* speed things up by working from a copy in system memory */
+		cvt.src.slow_reads =
+			is_i915_device(fd) && !gem_has_mappable_ggtt(fd);
 	} else {
 		blit->base.linear.fb = *fb;
 		blit->base.linear.fb.gem_handle = 0;
-- 
2.23.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 9/9] tests/kms_plane: Fix format/mod for reference FB

[Imre Deak]

Fix the use of uninited format, modifier.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 tests/kms_plane.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index 9ef3a7f3..7d7316d5 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -683,7 +683,7 @@ static bool test_format_plane(data_t *data, enum pipe pipe,
 		struct igt_fb test_fb;
 		int ret;
 
-		igt_create_fb(data->drm_fd, 64, 64, format,
+		igt_create_fb(data->drm_fd, 64, 64, ref_format,
 			      LOCAL_DRM_FORMAT_MOD_NONE, &test_fb);
 
 		igt_plane_set_fb(plane, &test_fb);
@@ -704,7 +704,7 @@ static bool test_format_plane(data_t *data, enum pipe pipe,
 		const color_t *c = &data->colors[i];
 
 		test_format_plane_color(data, pipe, plane,
-					format, modifier,
+					ref_format, ref_modifier,
 					width, height,
 					IGT_COLOR_YCBCR_BT709,
 					IGT_COLOR_YCBCR_LIMITED_RANGE,
-- 
2.23.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 9/9] tests/kms_plane: Fix format/mod for reference FB

[Imre Deak]

On Wed, Jan 29, 2020 at 08:16:01PM +0200, Imre Deak wrote:
> Fix the use of uninited format, modifier.

Ignore this patch, missed the place where they are inited.

> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  tests/kms_plane.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index 9ef3a7f3..7d7316d5 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -683,7 +683,7 @@ static bool test_format_plane(data_t *data, enum pipe pipe,
>  		struct igt_fb test_fb;
>  		int ret;
>  
> -		igt_create_fb(data->drm_fd, 64, 64, format,
> +		igt_create_fb(data->drm_fd, 64, 64, ref_format,
>  			      LOCAL_DRM_FORMAT_MOD_NONE, &test_fb);
>  
>  		igt_plane_set_fb(plane, &test_fb);
> @@ -704,7 +704,7 @@ static bool test_format_plane(data_t *data, enum pipe pipe,
>  		const color_t *c = &data->colors[i];
>  
>  		test_format_plane_color(data, pipe, plane,
> -					format, modifier,
> +					ref_format, ref_modifier,
>  					width, height,
>  					IGT_COLOR_YCBCR_BT709,
>  					IGT_COLOR_YCBCR_LIMITED_RANGE,
> -- 
> 2.23.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,1/9] lib/ioctl_wrappers: Query if device supports set/get legacy tiling

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/9] lib/ioctl_wrappers: Query if device supports set/get legacy tiling
URL   : https://patchwork.freedesktop.org/series/72734/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7838 -> IGTPW_4031
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_4031 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_4031, 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_4031/index.html

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@runner@aborted:
    - fi-hsw-peppy:       NOTRUN -> [FAIL][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4031/fi-hsw-peppy/igt@runner@aborted.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_close_race@basic-threads:
    - fi-hsw-peppy:       [PASS][2] -> [TIMEOUT][3] ([fdo#112271])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-hsw-peppy/igt@gem_close_race@basic-threads.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4031/fi-hsw-peppy/igt@gem_close_race@basic-threads.html

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-tgl-y:           [PASS][4] -> [FAIL][5] ([CI#94])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-tgl-y/igt@gem_exec_suspend@basic-s4-devices.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4031/fi-tgl-y/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [PASS][6] -> [FAIL][7] ([i915#178])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4031/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_blt:
    - fi-ivb-3770:        [PASS][8] -> [DMESG-FAIL][9] ([i915#725])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-ivb-3770/igt@i915_selftest@live_blt.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4031/fi-ivb-3770/igt@i915_selftest@live_blt.html

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-skl-6770hq:      [PASS][10] -> [SKIP][11] ([fdo#109271]) +27 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-dpms.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4031/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-dpms.html

  * igt@prime_self_import@basic-llseek-bad:
    - fi-tgl-y:           [PASS][12] -> [DMESG-WARN][13] ([CI#94] / [i915#402]) +1 similar issue
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-tgl-y/igt@prime_self_import@basic-llseek-bad.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4031/fi-tgl-y/igt@prime_self_import@basic-llseek-bad.html

  
#### Possible fixes ####

  * igt@i915_selftest@live_gem_contexts:
    - fi-cfl-8700k:       [INCOMPLETE][14] ([i915#424]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4031/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html

  * igt@i915_selftest@live_gtt:
    - fi-bdw-5557u:       [TIMEOUT][16] ([fdo#112271]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-bdw-5557u/igt@i915_selftest@live_gtt.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4031/fi-bdw-5557u/igt@i915_selftest@live_gtt.html

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
    - fi-icl-dsi:         [DMESG-WARN][18] ([i915#109]) -> [PASS][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-icl-dsi/igt@kms_pipe_crc_basic@hang-read-crc-pipe-a.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4031/fi-icl-dsi/igt@kms_pipe_crc_basic@hang-read-crc-pipe-a.html

  * igt@prime_self_import@basic-llseek-size:
    - fi-tgl-y:           [DMESG-WARN][20] ([CI#94] / [i915#402]) -> [PASS][21] +1 similar issue
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-tgl-y/igt@prime_self_import@basic-llseek-size.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4031/fi-tgl-y/igt@prime_self_import@basic-llseek-size.html

  
#### Warnings ####

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770r:       [DMESG-FAIL][22] ([i915#725]) -> [DMESG-FAIL][23] ([i915#563])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-hsw-4770r/igt@i915_selftest@live_blt.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4031/fi-hsw-4770r/igt@i915_selftest@live_blt.html
    - fi-hsw-4770:        [DMESG-FAIL][24] ([i915#553] / [i915#725]) -> [DMESG-FAIL][25] ([i915#725])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-hsw-4770/igt@i915_selftest@live_blt.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4031/fi-hsw-4770/igt@i915_selftest@live_blt.html

  * igt@i915_selftest@live_execlists:
    - fi-icl-y:           [INCOMPLETE][26] ([i915#140]) -> [DMESG-FAIL][27] ([fdo#108569])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-icl-y/igt@i915_selftest@live_execlists.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4031/fi-icl-y/igt@i915_selftest@live_execlists.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][28] ([fdo#111096] / [i915#323]) -> [FAIL][29] ([fdo#111407])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4031/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
  [CI#94]: https://gitlab.freedesktop.org/gfx-ci/i915-infra/issues/94
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
  [fdo#112271]: https://bugs.freedesktop.org/show_bug.cgi?id=112271
  [i915#109]: https://gitlab.freedesktop.org/drm/intel/issues/109
  [i915#140]: https://gitlab.freedesktop.org/drm/intel/issues/140
  [i915#178]: https://gitlab.freedesktop.org/drm/intel/issues/178
  [i915#323]: https://gitlab.freedesktop.org/drm/intel/issues/323
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#424]: https://gitlab.freedesktop.org/drm/intel/issues/424
  [i915#553]: https://gitlab.freedesktop.org/drm/intel/issues/553
  [i915#563]: https://gitlab.freedesktop.org/drm/intel/issues/563
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725


Participating hosts (50 -> 47)
------------------------------

  Additional (2): fi-byt-n2820 fi-bwr-2160 
  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-kbl-7560u fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * IGT: IGT_5404 -> IGTPW_4031

  CI-20190529: 20190529
  CI_DRM_7838: d3d96beea538c8de906a1c4d7e6793a47d17a471 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_4031: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4031/index.html
  IGT_5404: 4147bab8e3dcaf11bd657b5fb4c109708e94e60c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4031/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/9] lib/ioctl_wrappers: Query if device supports set/get legacy tiling

[Chris Wilson]

Quoting Imre Deak (2020-01-29 18:15:53)
> From: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> 
> Add a method to query if the device supports setting and getting legacy
> tiling formats for buffer objects.

Legacy is never a good choice for a name longterm. Say what you mean

gem_mmap_gtt_uses_tiling or
gem_mmap_aperture_has_detiling

> 
> Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  lib/ioctl_wrappers.c | 17 +++++++++++++++++
>  lib/ioctl_wrappers.h |  1 +
>  2 files changed, 18 insertions(+)
> 
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index 627717d2..c1abb575 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -133,6 +133,23 @@ __gem_get_tiling(int fd, struct drm_i915_gem_get_tiling *arg)
>         return err;
>  }
>  
> +/**
> + * gem_has_legacy_hw_tiling:
> + * @fd: open i915 drm file descriptor
> + *
> + * Feature check to query if the device supports setting/getting
> + * legacy tiling formats for buffer objects
> + *
> + * Returns: True if tiling is supported
> + */
> +bool
> +gem_has_legacy_hw_tiling(int fd)
> +{
> +       struct drm_i915_gem_get_tiling arg = {};
> +
> +       return (__gem_get_tiling(fd, &arg) != -EOPNOTSUPP);

There was already an interface for this, the number of available fences.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/9] lib/igt_fb: Fix creating FBs on platforms w/o HW detiling

[Chris Wilson]

Quoting Imre Deak (2020-01-29 18:15:54)
> On platforms w/o HW detiling don't fail creating the FB due to the
> expected error from the set_tiling IOCTL.
> 
> Most of the tests use a cairo surface to draw, which don't depend on the
> HW detiling. Other tests (using lib/igt_draw.c or drawing to the FB
> directly, like kms_draw_crc, kms_frontbuffer) are failing atm and will
> need to be fixed separately.
> 
> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  lib/igt_fb.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index c81b9de8..ec7e9991 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -944,9 +944,10 @@ static int create_bo_for_fb(struct igt_fb *fb)
>  
>                 if (is_i915_device(fd)) {
>                         fb->gem_handle = gem_create(fd, fb->size);
> -                       gem_set_tiling(fd, fb->gem_handle,
> -                                      igt_fb_mod_to_tiling(fb->modifier),
> -                                      fb->strides[0]);
> +                       if (gem_has_legacy_hw_tiling(fd))
> +                               gem_set_tiling(fd, fb->gem_handle,
> +                                              igt_fb_mod_to_tiling(fb->modifier),
> +                                              fb->strides[0]);

/* If we can't use fences, we won't use ggtt detiling later */
__gem_set_tiling()
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 3/9] lib/intel_batchbuffer: Add blitter copy using XY_SRC_COPY_BLT

[Chris Wilson]

Quoting Imre Deak (2020-01-29 18:15:55)
> From: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> 
> Add a method that uses the XY_SRC_COPY_BLT instruction for copying
> buffers using the blitter engine.
> 
> v2: Use uint32_t for parameters; fix stride for Gen2/3
> 
> Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  lib/intel_batchbuffer.c | 183 ++++++++++++++++++++++++++++++++++++++++
>  lib/intel_batchbuffer.h |  21 +++++
>  2 files changed, 204 insertions(+)
> 
> diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
> index ab907913..c4bf7ef4 100644
> --- a/lib/intel_batchbuffer.c
> +++ b/lib/intel_batchbuffer.c
> @@ -47,6 +47,12 @@
>  
>  #include <i915_drm.h>
>  
> +#define MI_FLUSH_DW (0x26 << 23)
> +
> +#define BCS_SWCTRL 0x22200
> +#define BCS_SRC_Y (1 << 0)
> +#define BCS_DST_Y (1 << 1)
> +
>  /**
>   * SECTION:intel_batchbuffer
>   * @short_description: Batchbuffer and blitter support
> @@ -708,6 +714,183 @@ static void exec_blit(int fd,
>         gem_execbuf(fd, &exec);
>  }
>  
> +static uint32_t src_copy_dword0(uint32_t src_tiling, uint32_t dst_tiling,
> +                               uint32_t bpp, uint32_t device_gen)
> +{
> +       uint32_t dword0 = 0;
> +
> +       dword0 |= XY_SRC_COPY_BLT_CMD;
> +       if (bpp == 32)
> +               dword0 |= XY_SRC_COPY_BLT_WRITE_RGB |
> +                       XY_SRC_COPY_BLT_WRITE_ALPHA;
> +
> +       if (device_gen >= 4 && src_tiling)
> +               dword0 |= XY_SRC_COPY_BLT_SRC_TILED;
> +       if (device_gen >= 4 && dst_tiling)
> +               dword0 |= XY_SRC_COPY_BLT_DST_TILED;
> +
> +       return dword0;
> +}
> +
> +static uint32_t src_copy_dword1(uint32_t dst_pitch, uint32_t bpp)
> +{
> +       uint32_t dword1 = 0;
> +
> +       switch (bpp) {
> +       case 8:
> +               break;
> +       case 16:
> +               dword1 |= (1 << 24); /* Only support 565 color */
> +               break;
> +       case 32:
> +               dword1 |= (3 << 24);
> +               break;
> +       default:
> +               igt_assert(0);
> +       }
> +
> +       dword1 |= 0xcc << 16;
> +       dword1 |= dst_pitch;
> +
> +       return dword1;
> +}
> +/**
> + * igt_blitter_src_copy__raw:
> + * @fd: file descriptor of the i915 driver
> + * @src_handle: GEM handle of the source buffer
> + * @src_delta: offset into the source GEM bo, in bytes
> + * @src_stride: Stride (in bytes) of the source buffer
> + * @src_tiling: Tiling mode of the source buffer
> + * @src_x: X coordinate of the source region to copy
> + * @src_y: Y coordinate of the source region to copy
> + * @width: Width of the region to copy
> + * @height: Height of the region to copy
> + * @bpp: source and destination bits per pixel
> + * @dst_handle: GEM handle of the destination buffer
> + * @dst_delta: offset into the destination GEM bo, in bytes
> + * @dst_stride: Stride (in bytes) of the destination buffer
> + * @dst_tiling: Tiling mode of the destination buffer
> + * @dst_x: X coordinate of destination
> + * @dst_y: Y coordinate of destination
> + *
> + */
> +void igt_blitter_src_copy__raw(int fd,
> +                               /* src */
> +                               uint32_t src_handle,
> +                               uint32_t src_delta,
> +                               uint32_t src_stride,
> +                               uint32_t src_tiling,
> +                               uint32_t src_x, uint32_t src_y,
> +
> +                               /* size */
> +                               uint32_t width, uint32_t height,
> +
> +                               /* bpp */
> +                               uint32_t bpp,
> +
> +                               /* dst */
> +                               uint32_t dst_handle,
> +                               uint32_t dst_delta,
> +                               uint32_t dst_stride,
> +                               uint32_t dst_tiling,
> +                               uint32_t dst_x, uint32_t dst_y)
> +{
> +       uint32_t batch[32];
> +       struct drm_i915_gem_exec_object2 objs[3];
> +       struct drm_i915_gem_relocation_entry relocs[2];
> +       uint32_t batch_handle;
> +       uint32_t src_pitch, dst_pitch;
> +       uint32_t dst_reloc_offset, src_reloc_offset;
> +       int i = 0;
> +       uint32_t gen = intel_gen(intel_get_drm_devid(fd));
> +       const bool has_64b_reloc = gen >= 8;

Could try to be a little less haphazard.

> +
> +       memset(batch, 0, sizeof(batch));
> +
> +       igt_assert((src_tiling == I915_TILING_NONE) ||
> +                  (src_tiling == I915_TILING_X) ||
> +                  (src_tiling == I915_TILING_Y));
> +       igt_assert((dst_tiling == I915_TILING_NONE) ||
> +                  (dst_tiling == I915_TILING_X) ||
> +                  (dst_tiling == I915_TILING_Y));
> +
> +       src_pitch = (gen >= 4 && src_tiling) ? src_stride / 4 : src_stride;
> +       dst_pitch = (gen >= 4 && dst_tiling) ? dst_stride / 4 : dst_stride;
> +
> +       CHECK_RANGE(src_x); CHECK_RANGE(src_y);
> +       CHECK_RANGE(dst_x); CHECK_RANGE(dst_y);
> +       CHECK_RANGE(width); CHECK_RANGE(height);
> +       CHECK_RANGE(src_x + width); CHECK_RANGE(src_y + height);
> +       CHECK_RANGE(dst_x + width); CHECK_RANGE(dst_y + height);
> +       CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
> +
> +       if ((src_tiling | dst_tiling) >= I915_TILING_Y) {
> +               unsigned int mask;
> +
> +               batch[i++] = MI_LOAD_REGISTER_IMM;
> +               batch[i++] = BCS_SWCTRL;
> +
> +               mask = (BCS_SRC_Y | BCS_DST_Y) << 16;
> +               if (src_tiling == I915_TILING_Y)
> +                       mask |= BCS_SRC_Y;
> +               if (dst_tiling == I915_TILING_Y)
> +                       mask |= BCS_DST_Y;
> +               batch[i++] = mask;
> +       }
> +
> +       batch[i] = src_copy_dword0(src_tiling, dst_tiling, bpp, gen);
> +       batch[i++] |= 6 + 2 * has_64b_reloc;
> +       batch[i++] = src_copy_dword1(dst_pitch, bpp);
> +       batch[i++] = (dst_y << 16) | dst_x; /* dst x1,y1 */
> +       batch[i++] = ((dst_y + height) << 16) | (dst_x + width); /* dst x2,y2 */
> +       dst_reloc_offset = i;
> +       batch[i++] = dst_delta; /* dst address lower bits */

if (has_64b_reloc)
> +       batch[i++] = 0; /* dst address upper bits */

> +       batch[i++] = (src_y << 16) | src_x; /* src x1,y1 */
> +       batch[i++] = src_pitch;
> +       src_reloc_offset = i;
> +       batch[i++] = src_delta; /* src address lower bits */

if (has_64b_reloc)
> +       batch[i++] = 0; /* src address upper bits */
> +
> +       if ((src_tiling | dst_tiling) >= I915_TILING_Y) {
> +               igt_assert(gen >= 6);
> +               batch[i++] = MI_FLUSH_DW | 2;

MI_FLUSH_DW is only 3 dwords on gen6, so

batch[i] = MI_FLUSH_DW | 1;
if (has_64b_reloc)
	batch[i]++;
i++;

or something prettier.


> +               batch[i++] = 0;
> +               batch[i++] = 0;
> +               batch[i++] = 0;
> +
> +               batch[i++] = MI_LOAD_REGISTER_IMM;
> +               batch[i++] = BCS_SWCTRL;
> +               batch[i++] = (BCS_SRC_Y | BCS_DST_Y) << 16;
> +       }
> +
> +       batch[i++] = MI_BATCH_BUFFER_END;
> +       batch[i++] = MI_NOOP;
> +
> +       igt_assert(i <= ARRAY_SIZE(batch));
> +
> +       batch_handle = gem_create(fd, 4096);
> +       gem_write(fd, batch_handle, 0, batch, sizeof(batch));
> +
> +       fill_relocation(&relocs[0], dst_handle, dst_delta, dst_reloc_offset,
> +                       I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER);
> +       fill_relocation(&relocs[1], src_handle, src_delta, src_reloc_offset,
> +                       I915_GEM_DOMAIN_RENDER, 0);
> +
> +       fill_object(&objs[0], dst_handle, NULL, 0);
> +       fill_object(&objs[1], src_handle, NULL, 0);
> +       fill_object(&objs[2], batch_handle, relocs, 2);
> +
> +       if (dst_tiling)
> +               objs[0].flags |= EXEC_OBJECT_NEEDS_FENCE;

Strictly speaking, NEEDS_FENCE anyway for gen2. Basically any blitter
operation needs to be so marked so that a fence may be removed instead.

> +       if (src_tiling)
> +               objs[1].flags |= EXEC_OBJECT_NEEDS_FENCE;
> +
> +       exec_blit(fd, objs, 3, ARRAY_SIZE(batch));
> +
> +       gem_close(fd, batch_handle);
> +}
> +
>  /**
>   * igt_blitter_fast_copy__raw:
>   * @fd: file descriptor of the i915 driver
> diff --git a/lib/intel_batchbuffer.h b/lib/intel_batchbuffer.h
> index 85eb3ffd..0fc18db1 100644
> --- a/lib/intel_batchbuffer.h
> +++ b/lib/intel_batchbuffer.h
> @@ -265,6 +265,27 @@ unsigned igt_buf_height(const struct igt_buf *buf);
>  unsigned int igt_buf_intel_ccs_width(int gen, const struct igt_buf *buf);
>  unsigned int igt_buf_intel_ccs_height(int gen, const struct igt_buf *buf);
>  
> +void igt_blitter_src_copy__raw(int fd,

Just src_copy after the instruction.

What is __raw meant to mean?
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/9] lib/igt_fb: Switch from XY_FAST_COPY_BLT to XY_SRC_COPY_BLT

[Chris Wilson]

Quoting Imre Deak (2020-01-29 18:15:56)
> From: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> 
> The XY_SRC_COPY_BLT instruction is supported on more platforms than
> XY_FAST_COPY_BLT - use it for X tiling on GEN12+ copying using blitter.
> For other tiling modes/platforms use the XY_FAST_COPY_BLT as before.
> 
> v2:
> - Use xy_src blit only - when necessary - on GEN12+/X-tiled mods.
> 
> Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  lib/igt_fb.c | 59 +++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 45 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index ec7e9991..53b43528 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -2146,27 +2146,58 @@ static void copy_with_engine(struct fb_blit_upload *blit,
>  static void blitcopy(const struct igt_fb *dst_fb,
>                      const struct igt_fb *src_fb)
>  {
> +       uint32_t src_tiling, dst_tiling;
> +
>         igt_assert_eq(dst_fb->fd, src_fb->fd);
>         igt_assert_eq(dst_fb->num_planes, src_fb->num_planes);
>  
> +       src_tiling = igt_fb_mod_to_tiling(src_fb->modifier);
> +       dst_tiling = igt_fb_mod_to_tiling(dst_fb->modifier);
> +
>         for (int i = 0; i < dst_fb->num_planes; i++) {
> +               int gen = intel_gen(intel_get_drm_devid(src_fb->fd));
> +
>                 igt_assert_eq(dst_fb->plane_bpp[i], src_fb->plane_bpp[i]);
>                 igt_assert_eq(dst_fb->plane_width[i], src_fb->plane_width[i]);
>                 igt_assert_eq(dst_fb->plane_height[i], src_fb->plane_height[i]);
> -
> -               igt_blitter_fast_copy__raw(dst_fb->fd,
> -                                          src_fb->gem_handle,
> -                                          src_fb->offsets[i],
> -                                          src_fb->strides[i],
> -                                          igt_fb_mod_to_tiling(src_fb->modifier),
> -                                          0, 0, /* src_x, src_y */
> -                                          dst_fb->plane_width[i], dst_fb->plane_height[i],
> -                                          dst_fb->plane_bpp[i],
> -                                          dst_fb->gem_handle,
> -                                          dst_fb->offsets[i],
> -                                          dst_fb->strides[i],
> -                                          igt_fb_mod_to_tiling(dst_fb->modifier),
> -                                          0, 0 /* dst_x, dst_y */);
> +               /*
> +                * On GEN12+ X-tiled format support is removed from the fast
> +                * blit command, so use the XY_SRC blit command for it
> +                * instead.
> +                */

Just use the normal SRC_COPY for linear/X/Y, keep it simple?
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 6/9] lib/igt_fb: Fall back from gtt map to coherent map on platforms w/o aperture

[Chris Wilson]

Quoting Imre Deak (2020-01-29 18:15:58)
> Use a device coherent map instead of GGTT map on platforms w/o an
> aperture.

The decision tree here leads much to be desired. The usual practice is
to use sw detiling since that has been faster than HW since byt.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 8/9] lib/igt_fb: Speed up format conversion for local memory

[Chris Wilson]

Quoting Imre Deak (2020-01-29 18:16:00)
> To speed up the conversion that needs to read from a dGFX local memory
> use the same trick as what's used for GTT apertures and make a copy
> first into system memory.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  lib/igt_fb.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 30f3bfba..ef3fa2ed 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -3427,6 +3427,10 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
>         if (use_enginecopy(fb) || use_blitter(fb) ||
>             igt_vc4_is_tiled(fb->modifier)) {
>                 setup_linear_mapping(&blit->base);
> +
> +               /* speed things up by working from a copy in system memory */
> +               cvt.src.slow_reads =
> +                       is_i915_device(fd) && !gem_has_mappable_ggtt(fd);

Any read from WC (including from GGTT) is greatly improved by using
memcpy_from_wc, and it even works with X/Y-tiling through fences.

slow_reads is a question of the mapping, at the time of use.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 3/9] lib/intel_batchbuffer: Add blitter copy using XY_SRC_COPY_BLT

[Chris Wilson]

Quoting Chris Wilson (2020-01-30 10:17:40)
> Quoting Imre Deak (2020-01-29 18:15:55)
> > +       if ((src_tiling | dst_tiling) >= I915_TILING_Y) {
> > +               igt_assert(gen >= 6);
> > +               batch[i++] = MI_FLUSH_DW | 2;
> 
> MI_FLUSH_DW is only 3 dwords on gen6, so
> 
> batch[i] = MI_FLUSH_DW | 1;
> if (has_64b_reloc)
>         batch[i]++;
> i++;
> 
> or something prettier.

Actually, you can get away with MI_FLUSH_DW | 2 as that should indicate
a qword write instead for gen6/7.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 8/9] lib/igt_fb: Speed up format conversion for local memory

[Imre Deak]

On Thu, Jan 30, 2020 at 10:24:05AM +0000, Chris Wilson wrote:
> Quoting Imre Deak (2020-01-29 18:16:00)
> > To speed up the conversion that needs to read from a dGFX local memory
> > use the same trick as what's used for GTT apertures and make a copy
> > first into system memory.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  lib/igt_fb.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 30f3bfba..ef3fa2ed 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -3427,6 +3427,10 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
> >         if (use_enginecopy(fb) || use_blitter(fb) ||
> >             igt_vc4_is_tiled(fb->modifier)) {
> >                 setup_linear_mapping(&blit->base);
> > +
> > +               /* speed things up by working from a copy in system memory */
> > +               cvt.src.slow_reads =
> > +                       is_i915_device(fd) && !gem_has_mappable_ggtt(fd);
> 
> Any read from WC (including from GGTT) is greatly improved by using
> memcpy_from_wc, and it even works with X/Y-tiling through fences.
> 
> slow_reads is a question of the mapping, at the time of use.

Yes, setting slow_reads above will lead to a memcpy_from_wc() of the
source to a system memory buf and the conversion will happen from the
latter. What's slow is the random access read during conversion from
local memory.

> -Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 8/9] lib/igt_fb: Speed up format conversion for local memory

[Chris Wilson]

Quoting Imre Deak (2020-01-30 12:04:09)
> On Thu, Jan 30, 2020 at 10:24:05AM +0000, Chris Wilson wrote:
> > Quoting Imre Deak (2020-01-29 18:16:00)
> > > To speed up the conversion that needs to read from a dGFX local memory
> > > use the same trick as what's used for GTT apertures and make a copy
> > > first into system memory.
> > > 
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  lib/igt_fb.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > index 30f3bfba..ef3fa2ed 100644
> > > --- a/lib/igt_fb.c
> > > +++ b/lib/igt_fb.c
> > > @@ -3427,6 +3427,10 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
> > >         if (use_enginecopy(fb) || use_blitter(fb) ||
> > >             igt_vc4_is_tiled(fb->modifier)) {
> > >                 setup_linear_mapping(&blit->base);
> > > +
> > > +               /* speed things up by working from a copy in system memory */
> > > +               cvt.src.slow_reads =
> > > +                       is_i915_device(fd) && !gem_has_mappable_ggtt(fd);
> > 
> > Any read from WC (including from GGTT) is greatly improved by using
> > memcpy_from_wc, and it even works with X/Y-tiling through fences.
> > 
> > slow_reads is a question of the mapping, at the time of use.
> 
> Yes, setting slow_reads above will lead to a memcpy_from_wc() of the
> source to a system memory buf and the conversion will happen from the
> latter. What's slow is the random access read during conversion from
> local memory.

We have fast routines for those, if needed. We can read from tiled
surfaces faster than the HW can detile in most cases.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 8/9] lib/igt_fb: Speed up format conversion for local memory

[Imre Deak]

On Thu, Jan 30, 2020 at 12:12:06PM +0000, Chris Wilson wrote:
> Quoting Imre Deak (2020-01-30 12:04:09)
> > On Thu, Jan 30, 2020 at 10:24:05AM +0000, Chris Wilson wrote:
> > > Quoting Imre Deak (2020-01-29 18:16:00)
> > > > To speed up the conversion that needs to read from a dGFX local memory
> > > > use the same trick as what's used for GTT apertures and make a copy
> > > > first into system memory.
> > > > 
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > ---
> > > >  lib/igt_fb.c | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > > 
> > > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > > index 30f3bfba..ef3fa2ed 100644
> > > > --- a/lib/igt_fb.c
> > > > +++ b/lib/igt_fb.c
> > > > @@ -3427,6 +3427,10 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
> > > >         if (use_enginecopy(fb) || use_blitter(fb) ||
> > > >             igt_vc4_is_tiled(fb->modifier)) {
> > > >                 setup_linear_mapping(&blit->base);
> > > > +
> > > > +               /* speed things up by working from a copy in system memory */
> > > > +               cvt.src.slow_reads =
> > > > +                       is_i915_device(fd) && !gem_has_mappable_ggtt(fd);
> > > 
> > > Any read from WC (including from GGTT) is greatly improved by using
> > > memcpy_from_wc, and it even works with X/Y-tiling through fences.
> > > 
> > > slow_reads is a question of the mapping, at the time of use.
> > 
> > Yes, setting slow_reads above will lead to a memcpy_from_wc() of the
> > source to a system memory buf and the conversion will happen from the
> > latter. What's slow is the random access read during conversion from
> > local memory.
> 
> We have fast routines for those, if needed. We can read from tiled
> surfaces faster than the HW can detile in most cases.

Not sure what you mean here. The above change is to make sure
conversions between YUV/RGB formats are fast even if the buffer is not
tiled.

> -Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 6/9] lib/igt_fb: Fall back from gtt map to coherent map on platforms w/o aperture

[Imre Deak]

On Thu, Jan 30, 2020 at 10:22:29AM +0000, Chris Wilson wrote:
> Quoting Imre Deak (2020-01-29 18:15:58)
> > Use a device coherent map instead of GGTT map on platforms w/o an
> > aperture.
> 
> The decision tree here leads much to be desired. The usual practice is
> to use sw detiling since that has been faster than HW since byt.

This fixes the use of map_bo() when the user just wants to fill the
buffer with a pattern (for instance clearing for yuv), so detiling is
not needed in that case.

> -Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/9] lib/igt_fb: Switch from XY_FAST_COPY_BLT to XY_SRC_COPY_BLT

[Imre Deak]

On Thu, Jan 30, 2020 at 10:19:09AM +0000, Chris Wilson wrote:
> Quoting Imre Deak (2020-01-29 18:15:56)
> > From: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> > 
> > The XY_SRC_COPY_BLT instruction is supported on more platforms than
> > XY_FAST_COPY_BLT - use it for X tiling on GEN12+ copying using blitter.
> > For other tiling modes/platforms use the XY_FAST_COPY_BLT as before.
> > 
> > v2:
> > - Use xy_src blit only - when necessary - on GEN12+/X-tiled mods.
> > 
> > Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  lib/igt_fb.c | 59 +++++++++++++++++++++++++++++++++++++++-------------
> >  1 file changed, 45 insertions(+), 14 deletions(-)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index ec7e9991..53b43528 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -2146,27 +2146,58 @@ static void copy_with_engine(struct fb_blit_upload *blit,
> >  static void blitcopy(const struct igt_fb *dst_fb,
> >                      const struct igt_fb *src_fb)
> >  {
> > +       uint32_t src_tiling, dst_tiling;
> > +
> >         igt_assert_eq(dst_fb->fd, src_fb->fd);
> >         igt_assert_eq(dst_fb->num_planes, src_fb->num_planes);
> >  
> > +       src_tiling = igt_fb_mod_to_tiling(src_fb->modifier);
> > +       dst_tiling = igt_fb_mod_to_tiling(dst_fb->modifier);
> > +
> >         for (int i = 0; i < dst_fb->num_planes; i++) {
> > +               int gen = intel_gen(intel_get_drm_devid(src_fb->fd));
> > +
> >                 igt_assert_eq(dst_fb->plane_bpp[i], src_fb->plane_bpp[i]);
> >                 igt_assert_eq(dst_fb->plane_width[i], src_fb->plane_width[i]);
> >                 igt_assert_eq(dst_fb->plane_height[i], src_fb->plane_height[i]);
> > -
> > -               igt_blitter_fast_copy__raw(dst_fb->fd,
> > -                                          src_fb->gem_handle,
> > -                                          src_fb->offsets[i],
> > -                                          src_fb->strides[i],
> > -                                          igt_fb_mod_to_tiling(src_fb->modifier),
> > -                                          0, 0, /* src_x, src_y */
> > -                                          dst_fb->plane_width[i], dst_fb->plane_height[i],
> > -                                          dst_fb->plane_bpp[i],
> > -                                          dst_fb->gem_handle,
> > -                                          dst_fb->offsets[i],
> > -                                          dst_fb->strides[i],
> > -                                          igt_fb_mod_to_tiling(dst_fb->modifier),
> > -                                          0, 0 /* dst_x, dst_y */);
> > +               /*
> > +                * On GEN12+ X-tiled format support is removed from the fast
> > +                * blit command, so use the XY_SRC blit command for it
> > +                * instead.
> > +                */
> 
> Just use the normal SRC_COPY for linear/X/Y, keep it simple?

The assumption is that fast blit is faster than src_copy if it's
available. If that's not true I think we should switch all current users
to src copy in a separate patch. 

> -Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/9] lib/igt_fb: Fix creating FBs on platforms w/o HW detiling

[Imre Deak]

On Thu, Jan 30, 2020 at 10:11:46AM +0000, Chris Wilson wrote:
> Quoting Imre Deak (2020-01-29 18:15:54)
> > On platforms w/o HW detiling don't fail creating the FB due to the
> > expected error from the set_tiling IOCTL.
> > 
> > Most of the tests use a cairo surface to draw, which don't depend on the
> > HW detiling. Other tests (using lib/igt_draw.c or drawing to the FB
> > directly, like kms_draw_crc, kms_frontbuffer) are failing atm and will
> > need to be fixed separately.
> > 
> > Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  lib/igt_fb.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index c81b9de8..ec7e9991 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -944,9 +944,10 @@ static int create_bo_for_fb(struct igt_fb *fb)
> >  
> >                 if (is_i915_device(fd)) {
> >                         fb->gem_handle = gem_create(fd, fb->size);
> > -                       gem_set_tiling(fd, fb->gem_handle,
> > -                                      igt_fb_mod_to_tiling(fb->modifier),
> > -                                      fb->strides[0]);
> > +                       if (gem_has_legacy_hw_tiling(fd))
> > +                               gem_set_tiling(fd, fb->gem_handle,
> > +                                              igt_fb_mod_to_tiling(fb->modifier),
> > +                                              fb->strides[0]);
> 
> /* If we can't use fences, we won't use ggtt detiling later */
> __gem_set_tiling()

Ok.

> -Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 3/9] lib/intel_batchbuffer: Add blitter copy using XY_SRC_COPY_BLT

[Imre Deak]

On Thu, Jan 30, 2020 at 10:17:40AM +0000, Chris Wilson wrote:
> Quoting Imre Deak (2020-01-29 18:15:55)
> > From: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> > 
> > Add a method that uses the XY_SRC_COPY_BLT instruction for copying
> > buffers using the blitter engine.
> > 
> > v2: Use uint32_t for parameters; fix stride for Gen2/3
> > 
> > Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  lib/intel_batchbuffer.c | 183 ++++++++++++++++++++++++++++++++++++++++
> >  lib/intel_batchbuffer.h |  21 +++++
> >  2 files changed, 204 insertions(+)
> > 
> > diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
> > index ab907913..c4bf7ef4 100644
> > --- a/lib/intel_batchbuffer.c
> > +++ b/lib/intel_batchbuffer.c
> > @@ -47,6 +47,12 @@
> >  
> >  #include <i915_drm.h>
> >  
> > +#define MI_FLUSH_DW (0x26 << 23)
> > +
> > +#define BCS_SWCTRL 0x22200
> > +#define BCS_SRC_Y (1 << 0)
> > +#define BCS_DST_Y (1 << 1)
> > +
> >  /**
> >   * SECTION:intel_batchbuffer
> >   * @short_description: Batchbuffer and blitter support
> > @@ -708,6 +714,183 @@ static void exec_blit(int fd,
> >         gem_execbuf(fd, &exec);
> >  }
> >  
> > +static uint32_t src_copy_dword0(uint32_t src_tiling, uint32_t dst_tiling,
> > +                               uint32_t bpp, uint32_t device_gen)
> > +{
> > +       uint32_t dword0 = 0;
> > +
> > +       dword0 |= XY_SRC_COPY_BLT_CMD;
> > +       if (bpp == 32)
> > +               dword0 |= XY_SRC_COPY_BLT_WRITE_RGB |
> > +                       XY_SRC_COPY_BLT_WRITE_ALPHA;
> > +
> > +       if (device_gen >= 4 && src_tiling)
> > +               dword0 |= XY_SRC_COPY_BLT_SRC_TILED;
> > +       if (device_gen >= 4 && dst_tiling)
> > +               dword0 |= XY_SRC_COPY_BLT_DST_TILED;
> > +
> > +       return dword0;
> > +}
> > +
> > +static uint32_t src_copy_dword1(uint32_t dst_pitch, uint32_t bpp)
> > +{
> > +       uint32_t dword1 = 0;
> > +
> > +       switch (bpp) {
> > +       case 8:
> > +               break;
> > +       case 16:
> > +               dword1 |= (1 << 24); /* Only support 565 color */
> > +               break;
> > +       case 32:
> > +               dword1 |= (3 << 24);
> > +               break;
> > +       default:
> > +               igt_assert(0);
> > +       }
> > +
> > +       dword1 |= 0xcc << 16;
> > +       dword1 |= dst_pitch;
> > +
> > +       return dword1;
> > +}
> > +/**
> > + * igt_blitter_src_copy__raw:
> > + * @fd: file descriptor of the i915 driver
> > + * @src_handle: GEM handle of the source buffer
> > + * @src_delta: offset into the source GEM bo, in bytes
> > + * @src_stride: Stride (in bytes) of the source buffer
> > + * @src_tiling: Tiling mode of the source buffer
> > + * @src_x: X coordinate of the source region to copy
> > + * @src_y: Y coordinate of the source region to copy
> > + * @width: Width of the region to copy
> > + * @height: Height of the region to copy
> > + * @bpp: source and destination bits per pixel
> > + * @dst_handle: GEM handle of the destination buffer
> > + * @dst_delta: offset into the destination GEM bo, in bytes
> > + * @dst_stride: Stride (in bytes) of the destination buffer
> > + * @dst_tiling: Tiling mode of the destination buffer
> > + * @dst_x: X coordinate of destination
> > + * @dst_y: Y coordinate of destination
> > + *
> > + */
> > +void igt_blitter_src_copy__raw(int fd,
> > +                               /* src */
> > +                               uint32_t src_handle,
> > +                               uint32_t src_delta,
> > +                               uint32_t src_stride,
> > +                               uint32_t src_tiling,
> > +                               uint32_t src_x, uint32_t src_y,
> > +
> > +                               /* size */
> > +                               uint32_t width, uint32_t height,
> > +
> > +                               /* bpp */
> > +                               uint32_t bpp,
> > +
> > +                               /* dst */
> > +                               uint32_t dst_handle,
> > +                               uint32_t dst_delta,
> > +                               uint32_t dst_stride,
> > +                               uint32_t dst_tiling,
> > +                               uint32_t dst_x, uint32_t dst_y)
> > +{
> > +       uint32_t batch[32];
> > +       struct drm_i915_gem_exec_object2 objs[3];
> > +       struct drm_i915_gem_relocation_entry relocs[2];
> > +       uint32_t batch_handle;
> > +       uint32_t src_pitch, dst_pitch;
> > +       uint32_t dst_reloc_offset, src_reloc_offset;
> > +       int i = 0;
> > +       uint32_t gen = intel_gen(intel_get_drm_devid(fd));
> > +       const bool has_64b_reloc = gen >= 8;
> 
> Could try to be a little less haphazard.

Ok, will use has_64b_reloc() instead.

> 
> > +
> > +       memset(batch, 0, sizeof(batch));
> > +
> > +       igt_assert((src_tiling == I915_TILING_NONE) ||
> > +                  (src_tiling == I915_TILING_X) ||
> > +                  (src_tiling == I915_TILING_Y));
> > +       igt_assert((dst_tiling == I915_TILING_NONE) ||
> > +                  (dst_tiling == I915_TILING_X) ||
> > +                  (dst_tiling == I915_TILING_Y));
> > +
> > +       src_pitch = (gen >= 4 && src_tiling) ? src_stride / 4 : src_stride;
> > +       dst_pitch = (gen >= 4 && dst_tiling) ? dst_stride / 4 : dst_stride;
> > +
> > +       CHECK_RANGE(src_x); CHECK_RANGE(src_y);
> > +       CHECK_RANGE(dst_x); CHECK_RANGE(dst_y);
> > +       CHECK_RANGE(width); CHECK_RANGE(height);
> > +       CHECK_RANGE(src_x + width); CHECK_RANGE(src_y + height);
> > +       CHECK_RANGE(dst_x + width); CHECK_RANGE(dst_y + height);
> > +       CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
> > +
> > +       if ((src_tiling | dst_tiling) >= I915_TILING_Y) {
> > +               unsigned int mask;
> > +
> > +               batch[i++] = MI_LOAD_REGISTER_IMM;
> > +               batch[i++] = BCS_SWCTRL;
> > +
> > +               mask = (BCS_SRC_Y | BCS_DST_Y) << 16;
> > +               if (src_tiling == I915_TILING_Y)
> > +                       mask |= BCS_SRC_Y;
> > +               if (dst_tiling == I915_TILING_Y)
> > +                       mask |= BCS_DST_Y;
> > +               batch[i++] = mask;
> > +       }
> > +
> > +       batch[i] = src_copy_dword0(src_tiling, dst_tiling, bpp, gen);
> > +       batch[i++] |= 6 + 2 * has_64b_reloc;
> > +       batch[i++] = src_copy_dword1(dst_pitch, bpp);
> > +       batch[i++] = (dst_y << 16) | dst_x; /* dst x1,y1 */
> > +       batch[i++] = ((dst_y + height) << 16) | (dst_x + width); /* dst x2,y2 */
> > +       dst_reloc_offset = i;
> > +       batch[i++] = dst_delta; /* dst address lower bits */
> 
> if (has_64b_reloc)
> > +       batch[i++] = 0; /* dst address upper bits */
> 
> > +       batch[i++] = (src_y << 16) | src_x; /* src x1,y1 */
> > +       batch[i++] = src_pitch;
> > +       src_reloc_offset = i;
> > +       batch[i++] = src_delta; /* src address lower bits */
> 
> if (has_64b_reloc)
> > +       batch[i++] = 0; /* src address upper bits */
> > +
> > +       if ((src_tiling | dst_tiling) >= I915_TILING_Y) {
> > +               igt_assert(gen >= 6);
> > +               batch[i++] = MI_FLUSH_DW | 2;
> 
> MI_FLUSH_DW is only 3 dwords on gen6, so
> 
> batch[i] = MI_FLUSH_DW | 1;
> if (has_64b_reloc)
> 	batch[i]++;
> i++;
> 
> or something prettier.

Ok.

> > +               batch[i++] = 0;
> > +               batch[i++] = 0;
> > +               batch[i++] = 0;
> > +
> > +               batch[i++] = MI_LOAD_REGISTER_IMM;
> > +               batch[i++] = BCS_SWCTRL;
> > +               batch[i++] = (BCS_SRC_Y | BCS_DST_Y) << 16;
> > +       }
> > +
> > +       batch[i++] = MI_BATCH_BUFFER_END;
> > +       batch[i++] = MI_NOOP;
> > +
> > +       igt_assert(i <= ARRAY_SIZE(batch));
> > +
> > +       batch_handle = gem_create(fd, 4096);
> > +       gem_write(fd, batch_handle, 0, batch, sizeof(batch));
> > +
> > +       fill_relocation(&relocs[0], dst_handle, dst_delta, dst_reloc_offset,
> > +                       I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER);
> > +       fill_relocation(&relocs[1], src_handle, src_delta, src_reloc_offset,
> > +                       I915_GEM_DOMAIN_RENDER, 0);
> > +
> > +       fill_object(&objs[0], dst_handle, NULL, 0);
> > +       fill_object(&objs[1], src_handle, NULL, 0);
> > +       fill_object(&objs[2], batch_handle, relocs, 2);
> > +
> > +       if (dst_tiling)
> > +               objs[0].flags |= EXEC_OBJECT_NEEDS_FENCE;
> 
> Strictly speaking, NEEDS_FENCE anyway for gen2. Basically any blitter
> operation needs to be so marked so that a fence may be removed instead.

Ok, so set NEEDS_FENCE unconditionally (removed by kernel for gen>=4?).

> 
> > +       if (src_tiling)
> > +               objs[1].flags |= EXEC_OBJECT_NEEDS_FENCE;
> > +
> > +       exec_blit(fd, objs, 3, ARRAY_SIZE(batch));
> > +
> > +       gem_close(fd, batch_handle);
> > +}
> > +
> >  /**
> >   * igt_blitter_fast_copy__raw:
> >   * @fd: file descriptor of the i915 driver
> > diff --git a/lib/intel_batchbuffer.h b/lib/intel_batchbuffer.h
> > index 85eb3ffd..0fc18db1 100644
> > --- a/lib/intel_batchbuffer.h
> > +++ b/lib/intel_batchbuffer.h
> > @@ -265,6 +265,27 @@ unsigned igt_buf_height(const struct igt_buf *buf);
> >  unsigned int igt_buf_intel_ccs_width(int gen, const struct igt_buf *buf);
> >  unsigned int igt_buf_intel_ccs_height(int gen, const struct igt_buf *buf);
> >  
> > +void igt_blitter_src_copy__raw(int fd,
> 
> Just src_copy after the instruction.
> 
> What is __raw meant to mean?

Looks like to align with the name of fast_copy__raw() which is the
version of fast_copy() not depending on libdrm. We don't have a libdrm
version of src_copy though so I can rename it to src_copy().

> -Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 3/9] lib/intel_batchbuffer: Add blitter copy using XY_SRC_COPY_BLT

[Imre Deak]

On Thu, Jan 30, 2020 at 10:26:28AM +0000, Chris Wilson wrote:
> Quoting Chris Wilson (2020-01-30 10:17:40)
> > Quoting Imre Deak (2020-01-29 18:15:55)
> > > +       if ((src_tiling | dst_tiling) >= I915_TILING_Y) {
> > > +               igt_assert(gen >= 6);
> > > +               batch[i++] = MI_FLUSH_DW | 2;
> > 
> > MI_FLUSH_DW is only 3 dwords on gen6, so
> > 
> > batch[i] = MI_FLUSH_DW | 1;
> > if (has_64b_reloc)
> >         batch[i]++;
> > i++;
> > 
> > or something prettier.
> 
> Actually, you can get away with MI_FLUSH_DW | 2 as that should indicate
> a qword write instead for gen6/7.

Ok, making it qword for gen6/7 and dword for gen8+.

--Imre
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 8/9] lib/igt_fb: Speed up format conversion for local memory

[Imre Deak]

On Thu, Jan 30, 2020 at 03:10:59PM +0200, Imre Deak wrote:
> On Thu, Jan 30, 2020 at 12:12:06PM +0000, Chris Wilson wrote:
> > Quoting Imre Deak (2020-01-30 12:04:09)
> > > On Thu, Jan 30, 2020 at 10:24:05AM +0000, Chris Wilson wrote:
> > > > Quoting Imre Deak (2020-01-29 18:16:00)
> > > > > To speed up the conversion that needs to read from a dGFX local memory
> > > > > use the same trick as what's used for GTT apertures and make a copy
> > > > > first into system memory.
> > > > > 
> > > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > > ---
> > > > >  lib/igt_fb.c | 4 ++++
> > > > >  1 file changed, 4 insertions(+)
> > > > > 
> > > > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > > > index 30f3bfba..ef3fa2ed 100644
> > > > > --- a/lib/igt_fb.c
> > > > > +++ b/lib/igt_fb.c
> > > > > @@ -3427,6 +3427,10 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
> > > > >         if (use_enginecopy(fb) || use_blitter(fb) ||
> > > > >             igt_vc4_is_tiled(fb->modifier)) {
> > > > >                 setup_linear_mapping(&blit->base);
> > > > > +
> > > > > +               /* speed things up by working from a copy in system memory */
> > > > > +               cvt.src.slow_reads =
> > > > > +                       is_i915_device(fd) && !gem_has_mappable_ggtt(fd);
> > > > 
> > > > Any read from WC (including from GGTT) is greatly improved by using
> > > > memcpy_from_wc, and it even works with X/Y-tiling through fences.
> > > > 
> > > > slow_reads is a question of the mapping, at the time of use.
> > > 
> > > Yes, setting slow_reads above will lead to a memcpy_from_wc() of the
> > > source to a system memory buf and the conversion will happen from the
> > > latter. What's slow is the random access read during conversion from
> > > local memory.
> > 
> > We have fast routines for those, if needed. We can read from tiled
> > surfaces faster than the HW can detile in most cases.
> 
> Not sure what you mean here. The above change is to make sure
> conversions between YUV/RGB formats are fast even if the buffer is not
> tiled.

FTR: After discussing with Ville, I checked a version that blits to
system memory directly, but that was somewhat slower than blitting to
local memory and then memcpy_from_wc() that.

> 
> > -Chris
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev