[igt-dev] [PATCH 00/10] Prepare IGT display test for removal of

[igt-dev] [PATCH 00/10] Prepare IGT display test for removal of

[Vanshidhar Konda]

The GET/SET_TILING IOCTLs will not be supported on devices that do not 
have the CPU (de)tiler. For kms_* tests that use modifiers the IGT 
library and tests have been updated to skip calling get/set_tiling calls 
on devices that don't support these IOCTLs.

Vanshidhar Konda (10):
  lib/ioctl_wrappers: Query if device supports set/get legacy tiling
  lib/intel_batchbuffer: Add blitter copy using XY_SRC_COPY_BLT
  lib/igt_fb: Switch from XY_FAST_COPY_BLT to XY_SRC_COPY_BLT
  lib/igt_fb: Remove set_tiling calls on devices without HW tiling
    support
  lib/igt_draw: Refactor get_tiling calls
  i915/i915_fb_tiling: Skip on devices that don't support HW tiling
  tests/kms_frontbuffer_tracking: Skip set tiling calls if not supported
  tests/kms_addfb_basic: Avoid tiling subtests on device without HW
    tiling support
  tests/kms_fence_pin_leak: Skip test on devices without HW tiling
    support
  tests/kms_available_modes_crc: Don't set tiling for framebuffer

 lib/igt_draw.c                   |  56 +++++-----
 lib/igt_draw.h                   |   5 +-
 lib/igt_fb.c                     |  72 ++++++++----
 lib/intel_batchbuffer.c          | 183 +++++++++++++++++++++++++++++++
 lib/intel_batchbuffer.h          |  21 ++++
 lib/ioctl_wrappers.c             |  17 +++
 lib/ioctl_wrappers.h             |   1 +
 tests/i915/i915_fb_tiling.c      |   2 +
 tests/kms_addfb_basic.c          |   4 +
 tests/kms_available_modes_crc.c  |   6 -
 tests/kms_fence_pin_leak.c       |   2 +
 tests/kms_frontbuffer_tracking.c |  11 +-
 12 files changed, 321 insertions(+), 59 deletions(-)

-- 
2.24.0

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

[igt-dev] [PATCH 01/10] lib/ioctl_wrappers: Query if device supports set/get legacy tiling

[Vanshidhar Konda]

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>
---
 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.24.0

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

[igt-dev] [PATCH 02/10] lib/intel_batchbuffer: Add blitter copy using XY_SRC_COPY_BLT

[Vanshidhar Konda]

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

Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@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 51aae4dc..1352aa95 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -46,6 +46,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
@@ -661,6 +667,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,
+				unsigned int src_delta,
+				unsigned int src_stride,
+				unsigned int src_tiling,
+				unsigned int src_x, unsigned src_y,
+
+				/* size */
+				unsigned int width, unsigned int height,
+
+				/* bpp */
+				int bpp,
+
+				/* dst */
+				uint32_t dst_handle,
+				unsigned dst_delta,
+				unsigned int dst_stride,
+				unsigned int dst_tiling,
+				unsigned int dst_x, unsigned 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 = fast_copy_pitch(src_stride, src_tiling);
+	dst_pitch = fast_copy_pitch(dst_stride, dst_tiling);
+
+	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 37e3affe..4820cebb 100644
--- a/lib/intel_batchbuffer.h
+++ b/lib/intel_batchbuffer.h
@@ -252,6 +252,27 @@ struct igt_buf {
 unsigned igt_buf_width(const struct igt_buf *buf);
 unsigned igt_buf_height(const struct igt_buf *buf);
 
+void igt_blitter_src_copy__raw(int fd,
+				/* src */
+				uint32_t src_handle,
+				unsigned int src_delta,
+				unsigned int src_stride,
+				unsigned int src_tiling,
+				unsigned int src_x, unsigned src_y,
+
+				/* size */
+				unsigned int width, unsigned int height,
+
+				/* bpp */
+				int bpp,
+
+				/* dst */
+				uint32_t dst_handle,
+				unsigned int dst_delta,
+				unsigned int dst_stride,
+				unsigned int dst_tiling,
+				unsigned int dst_x, unsigned 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.24.0

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

[igt-dev] [PATCH 03/10] lib/igt_fb: Switch from XY_FAST_COPY_BLT to XY_SRC_COPY_BLT

[Vanshidhar Konda]

The XY_SRC_COPY_BLT instruction is supported on more platforms than
XY_FAST_COPY_BLT - use it for X and Y tiling copying using blitter. For
other tiling modes use the XY_FAST_COPY_BLT.

Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
---
 lib/igt_fb.c | 51 +++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index e6eb39ac..35a50eeb 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2056,27 +2056,50 @@ 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++) {
 		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 */);
+		if ((src_tiling > I915_TILING_Y) || (dst_tiling > I915_TILING_Y)) {
+			igt_assert(intel_gen(intel_get_drm_devid(src_fb->fd)) >= 9);
+			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.24.0

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

[igt-dev] [PATCH 04/10] lib/igt_fb: Remove set_tiling calls on devices without HW tiling support

[Vanshidhar Konda]

On devices that don't support tiling/de-tiling in HW, skip the usage
of the SET_TILING IOCTL and use blitter for drawing.

Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@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 35a50eeb..2843ab4d 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -922,9 +922,11 @@ 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);
 
@@ -1962,9 +1964,14 @@ static bool use_enginecopy(const struct igt_fb *fb)
 
 static bool use_blitter(const struct igt_fb *fb)
 {
+	if (!blitter_ok(fb))
+		return false;
+
+	if (!gem_has_legacy_hw_tiling(fb->fd))
+		return true;
+
 	return (fb->modifier == I915_FORMAT_MOD_Y_TILED ||
-		fb->modifier == I915_FORMAT_MOD_Yf_TILED) &&
-		blitter_ok(fb);
+		fb->modifier == I915_FORMAT_MOD_Yf_TILED);
 }
 
 static void init_buf(struct fb_blit_upload *blit,
@@ -3381,8 +3388,10 @@ cairo_surface_t *igt_get_cairo_surface(int fd, struct igt_fb *fb)
 		else if (use_blitter(fb) || use_enginecopy(fb) ||
 			 igt_vc4_is_tiled(fb->modifier))
 			create_cairo_surface__gpu(fd, fb);
-		else
+		else if (gem_has_mappable_ggtt(fd))
 			create_cairo_surface__gtt(fd, fb);
+		else
+			igt_assert_f(false, "Configuration not supported.\n");
 	}
 
 	igt_assert(cairo_surface_status(fb->cairo_surface) == CAIRO_STATUS_SUCCESS);
-- 
2.24.0

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

[igt-dev] [PATCH 05/10] lib/igt_draw: Refactor get_tiling calls

[Vanshidhar Konda]

Simplify the number of places from which gem_get_tiling method is called
and call it only if the device has support in hardware for tiling.

Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
---
 lib/igt_draw.c                   | 56 +++++++++++++++++---------------
 lib/igt_draw.h                   |  5 +--
 tests/kms_frontbuffer_tracking.c |  8 +++--
 3 files changed, 37 insertions(+), 32 deletions(-)

diff --git a/lib/igt_draw.c b/lib/igt_draw.c
index 7e0edec1..cc2efc3d 100644
--- a/lib/igt_draw.c
+++ b/lib/igt_draw.c
@@ -333,20 +333,19 @@ static void draw_rect_ptr_tiled(void *ptr, uint32_t stride, uint32_t tiling,
 }
 
 static void draw_rect_mmap_cpu(int fd, struct buf_data *buf, struct rect *rect,
-			       uint32_t color)
+			       uint32_t tiling, uint32_t swizzle, uint32_t color)
 {
 	uint32_t *ptr;
-	uint32_t tiling, swizzle;
 
 	gem_set_domain(fd, buf->handle, I915_GEM_DOMAIN_CPU,
 		       I915_GEM_DOMAIN_CPU);
-	igt_require(gem_get_tiling(fd, buf->handle, &tiling, &swizzle));
 
 	/* We didn't implement suport for the older tiling methods yet. */
 	if (tiling != I915_TILING_NONE)
 		igt_require(intel_gen(intel_get_drm_devid(fd)) >= 5);
 
-	ptr = gem_mmap__cpu(fd, buf->handle, 0, PAGE_ALIGN(buf->size), 0);
+	ptr = gem_mmap__cpu(fd, buf->handle, 0, PAGE_ALIGN(buf->size),
+			    PROT_READ | PROT_WRITE);
 
 	switch (tiling) {
 	case I915_TILING_NONE:
@@ -384,14 +383,12 @@ static void draw_rect_mmap_gtt(int fd, struct buf_data *buf, struct rect *rect,
 }
 
 static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect *rect,
-			      uint32_t color)
+			      uint32_t tiling, uint32_t swizzle, uint32_t color)
 {
 	uint32_t *ptr;
-	uint32_t tiling, swizzle;
 
 	gem_set_domain(fd, buf->handle, I915_GEM_DOMAIN_GTT,
 		       I915_GEM_DOMAIN_GTT);
-	igt_require(gem_get_tiling(fd, buf->handle, &tiling, &swizzle));
 
 	/* We didn't implement suport for the older tiling methods yet. */
 	if (tiling != I915_TILING_NONE)
@@ -495,12 +492,9 @@ static void draw_rect_pwrite_tiled(int fd, struct buf_data *buf,
 }
 
 static void draw_rect_pwrite(int fd, struct buf_data *buf,
-			     struct rect *rect, uint32_t color)
+			     struct rect *rect, uint32_t tiling,
+			     uint32_t swizzle, uint32_t color)
 {
-	uint32_t tiling, swizzle;
-
-	igt_require(gem_get_tiling(fd, buf->handle, &tiling, &swizzle));
-
 	switch (tiling) {
 	case I915_TILING_NONE:
 		draw_rect_pwrite_untiled(fd, buf, rect, color);
@@ -517,6 +511,7 @@ static void draw_rect_pwrite(int fd, struct buf_data *buf,
 
 static void draw_rect_blt(int fd, struct cmd_data *cmd_data,
 			  struct buf_data *buf, struct rect *rect,
+			  uint32_t tiling, uint32_t swizzle,
 			  uint32_t color)
 {
 	drm_intel_bo *dst;
@@ -524,11 +519,8 @@ static void draw_rect_blt(int fd, struct cmd_data *cmd_data,
 	int blt_cmd_len, blt_cmd_tiling, blt_cmd_depth;
 	uint32_t devid = intel_get_drm_devid(fd);
 	int gen = intel_gen(devid);
-	uint32_t tiling, swizzle;
 	int pitch;
 
-	igt_require(gem_get_tiling(fd, buf->handle, &tiling, &swizzle));
-
 	dst = gem_handle_to_libdrm_bo(cmd_data->bufmgr, fd, "", buf->handle);
 	igt_assert(dst);
 
@@ -574,6 +566,7 @@ static void draw_rect_blt(int fd, struct cmd_data *cmd_data,
 
 static void draw_rect_render(int fd, struct cmd_data *cmd_data,
 			     struct buf_data *buf, struct rect *rect,
+			     uint32_t tiling, uint32_t swizzle,
 			     uint32_t color)
 {
 	drm_intel_bo *src, *dst;
@@ -581,21 +574,18 @@ static void draw_rect_render(int fd, struct cmd_data *cmd_data,
 	igt_render_copyfunc_t rendercopy = igt_get_render_copyfunc(devid);
 	struct igt_buf src_buf = {}, dst_buf = {};
 	struct intel_batchbuffer *batch;
-	uint32_t tiling, swizzle;
 	struct buf_data tmp;
 	int pixel_size = buf->bpp / 8;
 
 	igt_skip_on(!rendercopy);
 
-	igt_require(gem_get_tiling(fd, buf->handle, &tiling, &swizzle));
-
 	/* We create a temporary buffer and copy from it using rendercopy. */
 	tmp.size = rect->w * rect->h * pixel_size;
 	tmp.handle = gem_create(fd, tmp.size);
 	tmp.stride = rect->w * pixel_size;
 	tmp.bpp = buf->bpp;
 	draw_rect_mmap_cpu(fd, &tmp, &(struct rect){0, 0, rect->w, rect->h},
-			   color);
+			   I915_TILING_NONE, I915_BIT_6_SWIZZLE_NONE, color);
 
 	src = gem_handle_to_libdrm_bo(cmd_data->bufmgr, fd, "", tmp.handle);
 	igt_assert(src);
@@ -647,9 +637,12 @@ static void draw_rect_render(int fd, struct cmd_data *cmd_data,
  */
 void igt_draw_rect(int fd, drm_intel_bufmgr *bufmgr, drm_intel_context *context,
 		   uint32_t buf_handle, uint32_t buf_size, uint32_t buf_stride,
-		   enum igt_draw_method method, int rect_x, int rect_y,
-		   int rect_w, int rect_h, uint32_t color, int bpp)
+		   uint32_t tiling, enum igt_draw_method method,
+		   int rect_x, int rect_y, int rect_w, int rect_h,
+		   uint32_t color, int bpp)
 {
+	uint32_t buf_tiling, swizzle;
+
 	struct cmd_data cmd_data = {
 		.bufmgr = bufmgr,
 		.context = context,
@@ -667,24 +660,32 @@ void igt_draw_rect(int fd, drm_intel_bufmgr *bufmgr, drm_intel_context *context,
 		.h = rect_h,
 	};
 
+	swizzle = I915_BIT_6_SWIZZLE_NONE;
+	if (tiling != I915_TILING_NONE && gem_has_legacy_hw_tiling(fd)) {
+		gem_get_tiling(fd, buf_handle, &buf_tiling, &swizzle);
+		igt_assert(tiling == buf_tiling);
+	}
+
 	switch (method) {
 	case IGT_DRAW_MMAP_CPU:
-		draw_rect_mmap_cpu(fd, &buf, &rect, color);
+		draw_rect_mmap_cpu(fd, &buf, &rect, tiling, swizzle, color);
 		break;
 	case IGT_DRAW_MMAP_GTT:
 		draw_rect_mmap_gtt(fd, &buf, &rect, color);
 		break;
 	case IGT_DRAW_MMAP_WC:
-		draw_rect_mmap_wc(fd, &buf, &rect, color);
+		draw_rect_mmap_wc(fd, &buf, &rect, tiling, swizzle, color);
 		break;
 	case IGT_DRAW_PWRITE:
-		draw_rect_pwrite(fd, &buf, &rect, color);
+		draw_rect_pwrite(fd, &buf, &rect, tiling, swizzle, color);
 		break;
 	case IGT_DRAW_BLT:
-		draw_rect_blt(fd, &cmd_data, &buf, &rect, color);
+		draw_rect_blt(fd, &cmd_data, &buf, &rect, tiling, swizzle,
+			      color);
 		break;
 	case IGT_DRAW_RENDER:
-		draw_rect_render(fd, &cmd_data, &buf, &rect, color);
+		draw_rect_render(fd, &cmd_data, &buf, &rect, tiling, swizzle,
+				 color);
 		break;
 	default:
 		igt_assert(false);
@@ -715,7 +716,8 @@ void igt_draw_rect_fb(int fd, drm_intel_bufmgr *bufmgr,
 		      int rect_w, int rect_h, uint32_t color)
 {
 	igt_draw_rect(fd, bufmgr, context, fb->gem_handle, fb->size, fb->strides[0],
-		      method, rect_x, rect_y, rect_w, rect_h, color,
+		      igt_fb_mod_to_tiling(fb->modifier), method,
+		      rect_x, rect_y, rect_w, rect_h, color,
 		      igt_drm_format_to_bpp(fb->drm_format));
 }
 
diff --git a/lib/igt_draw.h b/lib/igt_draw.h
index b030131e..ec146754 100644
--- a/lib/igt_draw.h
+++ b/lib/igt_draw.h
@@ -52,8 +52,9 @@ const char *igt_draw_get_method_name(enum igt_draw_method method);
 
 void igt_draw_rect(int fd, drm_intel_bufmgr *bufmgr, drm_intel_context *context,
 		   uint32_t buf_handle, uint32_t buf_size, uint32_t buf_stride,
-		   enum igt_draw_method method, int rect_x, int rect_y,
-		   int rect_w, int rect_h, uint32_t color, int bpp);
+		   uint32_t tiling, enum igt_draw_method method,
+		   int rect_x, int rect_y, int rect_w, int rect_h,
+		   uint32_t color, int bpp);
 
 void igt_draw_rect_fb(int fd, drm_intel_bufmgr *bufmgr,
 		      drm_intel_context *context, struct igt_fb *fb,
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index c788b59e..ae3b087e 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -291,6 +291,7 @@ struct {
 	int height;
 	uint32_t color;
 	int bpp;
+	uint32_t tiling;
 } busy_thread = {
 	.stop = true,
 };
@@ -1126,9 +1127,9 @@ static void *busy_thread_func(void *data)
 	while (!busy_thread.stop)
 		igt_draw_rect(drm.fd, drm.bufmgr, NULL, busy_thread.handle,
 			      busy_thread.size, busy_thread.stride,
-			      IGT_DRAW_BLT, 0, 0, busy_thread.width,
-			      busy_thread.height, busy_thread.color,
-			      busy_thread.bpp);
+			      busy_thread.tiling, IGT_DRAW_BLT, 0, 0,
+			      busy_thread.width, busy_thread.height,
+			      busy_thread.color, busy_thread.bpp);
 
 	pthread_exit(0);
 }
@@ -1146,6 +1147,7 @@ static void start_busy_thread(struct igt_fb *fb)
 	busy_thread.height = fb->height;
 	busy_thread.color = pick_color(fb, COLOR_PRIM_BG);
 	busy_thread.bpp = igt_drm_format_to_bpp(fb->drm_format);
+	busy_thread.tiling = igt_fb_mod_to_tiling(fb->modifier);
 
 	rc = pthread_create(&busy_thread.thread, NULL, busy_thread_func, NULL);
 	igt_assert_eq(rc, 0);
-- 
2.24.0

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

[igt-dev] [PATCH 06/10] i915/i915_fb_tiling: Skip on devices that don't support HW tiling

[Vanshidhar Konda]

The test tries to change the tiling of a framebuffer object after it has
been set using a framebuffer modifier. If the device does not support HW
tiling/de-tiling the test should not be run.

Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
---
 tests/i915/i915_fb_tiling.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/i915/i915_fb_tiling.c b/tests/i915/i915_fb_tiling.c
index 7d5c3f1f..4ec84962 100644
--- a/tests/i915/i915_fb_tiling.c
+++ b/tests/i915/i915_fb_tiling.c
@@ -32,6 +32,8 @@ igt_simple_main
 	struct igt_fb fb;
 	int ret;
 
+	igt_require(gem_has_legacy_hw_tiling(drm_fd));
+
 	igt_create_fb(drm_fd, 512, 512, DRM_FORMAT_XRGB8888,
 		      LOCAL_I915_FORMAT_MOD_X_TILED, &fb);
 
-- 
2.24.0

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

[igt-dev] [PATCH 07/10] tests/kms_frontbuffer_tracking: Skip set tiling calls if not supported

[Vanshidhar Konda]

Skip the method that is setting tiling with invalid strides if the
hardware does not support HW for tiling/de-tiling.

Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
---
 tests/kms_frontbuffer_tracking.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index ae3b087e..89023bbe 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -2833,7 +2833,8 @@ static void badstride_subtest(const struct test_mode *t)
 	struct modeset_params *params = pick_params(t);
 	int rc;
 
-	try_invalid_strides();
+	if (gem_has_legacy_hw_tiling(drm.fd))
+		try_invalid_strides();
 
 	prepare_subtest(t, NULL);
 
-- 
2.24.0

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

[igt-dev] [PATCH 08/10] tests/kms_addfb_basic: Avoid tiling subtests on device without HW tiling support

[Vanshidhar Konda]

Skip subtests that are testing interoperability of FB modifiers and
hardware tiling if the device does not support HW tiling

Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
---
 tests/kms_addfb_basic.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c
index 666e7165..2e39ffe7 100644
--- a/tests/kms_addfb_basic.c
+++ b/tests/kms_addfb_basic.c
@@ -132,6 +132,7 @@ static void invalid_tests(int fd)
 
 	igt_subtest("clobberred-modifier") {
 		igt_require_intel(fd);
+		igt_require(gem_has_legacy_hw_tiling(fd));
 		f.flags = 0;
 		f.modifier[0] = 0;
 		gem_set_tiling(fd, gem_bo, I915_TILING_X, 512*4);
@@ -304,6 +305,7 @@ static void tiling_tests(int fd)
 	igt_subtest_group {
 		igt_fixture {
 			igt_require_intel(fd);
+			igt_require(gem_has_legacy_hw_tiling(fd));
 			tiled_x_bo = igt_create_bo_with_dimensions(fd, 1024, 1024,
 				DRM_FORMAT_XRGB8888, LOCAL_I915_FORMAT_MOD_X_TILED,
 				1024*4, NULL, NULL, NULL);
@@ -450,6 +452,7 @@ static void size_tests(int fd)
 
 	igt_subtest("bo-too-small-due-to-tiling") {
 		igt_require_intel(fd);
+		igt_require(gem_has_legacy_hw_tiling(fd));
 		gem_set_tiling(fd, gem_bo_small, I915_TILING_X, 1024*4);
 		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) == -1 &&
 			   errno == EINVAL);
@@ -502,6 +505,7 @@ static void addfb25_tests(int fd)
 	igt_subtest_group {
 		igt_fixture {
 			igt_require_intel(fd);
+			igt_require(gem_has_legacy_hw_tiling(fd));
 			gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4);
 			igt_require_fb_modifiers(fd);
 		}
-- 
2.24.0

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

[igt-dev] [PATCH 09/10] tests/kms_fence_pin_leak: Skip test on devices without HW tiling support

[Vanshidhar Konda]

This test is utilizing fences associated with tiled buffer objects. On
devices that don't have HW support for tiling/de-tiling these fences are
not available.

Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
---
 tests/kms_fence_pin_leak.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/kms_fence_pin_leak.c b/tests/kms_fence_pin_leak.c
index 8c9e10a5..a05039df 100644
--- a/tests/kms_fence_pin_leak.c
+++ b/tests/kms_fence_pin_leak.c
@@ -200,6 +200,8 @@ igt_simple_main
 
 	data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
 	igt_require_gem(data.drm_fd);
+	igt_require(gem_has_legacy_hw_tiling(data.drm_fd));
+	igt_require(gem_has_mappable_ggtt(data.drm_fd));
 
 	data.devid = intel_get_drm_devid(data.drm_fd);
 
-- 
2.24.0

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

[igt-dev] [PATCH 10/10] tests/kms_available_modes_crc: Don't set tiling for framebuffer

[Vanshidhar Konda]

The GEM object used for the framebuffer does not need tiling to be set
on it as the entire framebuffer is being filled with the same value -
tiling will not impact the end value of the buffer.

Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
---
 tests/kms_available_modes_crc.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/tests/kms_available_modes_crc.c b/tests/kms_available_modes_crc.c
index d1b7b517..4113b4c0 100644
--- a/tests/kms_available_modes_crc.c
+++ b/tests/kms_available_modes_crc.c
@@ -211,17 +211,11 @@ static bool setup_fb(data_t *data, igt_output_t *output, igt_plane_t *plane,
 	data->buf = (unsigned char *)calloc(data->size*2, 1);
 
 	data->gem_handle = gem_create(data->gfx_fd, gemsize);
-	ret = __gem_set_tiling(data->gfx_fd, data->gem_handle,
-			       igt_fb_mod_to_tiling(tiling),
-			       data->fb.strides[0]);
-
 	data->fb.gem_handle = data->gem_handle;
 	data->fb.width = w;
 	data->fb.height = h;
 	fill_in_fb(data, output, plane, format);
 
-	igt_assert_eq(ret, 0);
-
 	ret = __kms_addfb(data->gfx_fd, data->gem_handle, w, h,
 			  format, tiling, data->fb.strides, data->fb.offsets,
 			  num_planes, LOCAL_DRM_MODE_FB_MODIFIERS,
-- 
2.24.0

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

[igt-dev] ✗ Fi.CI.BAT: failure for Prepare IGT display test for removal of

[Patchwork]

== Series Details ==

Series: Prepare IGT display test for removal of
URL   : https://patchwork.freedesktop.org/series/71083/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7591 -> IGTPW_3874
====================================================

Summary
-------

  **FAILURE**

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770r:       [PASS][1] -> [DMESG-FAIL][2] ([i915#553] / [i915#725])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7591/fi-hsw-4770r/igt@i915_selftest@live_blt.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3874/fi-hsw-4770r/igt@i915_selftest@live_blt.html
    - fi-ivb-3770:        [PASS][3] -> [DMESG-FAIL][4] ([i915#725])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7591/fi-ivb-3770/igt@i915_selftest@live_blt.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3874/fi-ivb-3770/igt@i915_selftest@live_blt.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-byt-n2820:       [PASS][5] -> [DMESG-FAIL][6] ([i915#722])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7591/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3874/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html

  
#### Possible fixes ####

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

  
  [i915#178]: https://gitlab.freedesktop.org/drm/intel/issues/178
  [i915#553]: https://gitlab.freedesktop.org/drm/intel/issues/553
  [i915#722]: https://gitlab.freedesktop.org/drm/intel/issues/722
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725


Participating hosts (51 -> 24)
------------------------------

  ERROR: It appears as if the changes made in IGTPW_3874 prevented too many machines from booting.

  Missing    (27): fi-snb-2520m fi-skl-lmem fi-icl-guc fi-cml-u2 fi-bxt-dsi fi-bdw-5557u fi-byt-j1900 fi-glk-dsi fi-bwr-2160 fi-ilk-650 fi-kbl-7500u fi-ctg-p8600 fi-hsw-4770 fi-elk-e7500 fi-tgl-y fi-tgl-guc fi-ilk-m540 fi-cfl-8700k fi-hsw-4200u fi-hsw-peppy fi-byt-squawks fi-bsw-cyan fi-cfl-guc fi-kbl-guc fi-kbl-x1275 fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * IGT: IGT_5350 -> IGTPW_3874

  CI-20190529: 20190529
  CI_DRM_7591: 977eb2b7ca4efceca4baf88a612e751f5f819999 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3874: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3874/index.html
  IGT_5350: 36431c5923099582e87379aec8e16d43090d06a7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

Re: [igt-dev] [PATCH 02/10] lib/intel_batchbuffer: Add blitter copy using XY_SRC_COPY_BLT

[Ville Syrjälä]

On Tue, Dec 17, 2019 at 09:59:17PM -0800, Vanshidhar Konda wrote:
> Add a method that uses the XY_SRC_COPY_BLT instruction for copying
> buffers using the blitter engine.
> 
> Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@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 51aae4dc..1352aa95 100644
> --- a/lib/intel_batchbuffer.c
> +++ b/lib/intel_batchbuffer.c
> @@ -46,6 +46,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
> @@ -661,6 +667,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,
> +				unsigned int src_delta,
> +				unsigned int src_stride,
> +				unsigned int src_tiling,
> +				unsigned int src_x, unsigned src_y,

Inconsistent unsigned int vs. unsigned

> +
> +				/* size */
> +				unsigned int width, unsigned int height,
> +
> +				/* bpp */
> +				int bpp,
> +
> +				/* dst */
> +				uint32_t dst_handle,
> +				unsigned dst_delta,
> +				unsigned int dst_stride,
> +				unsigned int dst_tiling,
> +				unsigned int dst_x, unsigned 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 = fast_copy_pitch(src_stride, src_tiling);
> +	dst_pitch = fast_copy_pitch(dst_stride, dst_tiling);

I believe those do the wrong thing for pre-gen4 tiling.

I also wonder how many implementations of this we already have in igt.
I suspect the answer is more than two. Maybe there's a way to reduce the
duplication a bit?

> +
> +	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 37e3affe..4820cebb 100644
> --- a/lib/intel_batchbuffer.h
> +++ b/lib/intel_batchbuffer.h
> @@ -252,6 +252,27 @@ struct igt_buf {
>  unsigned igt_buf_width(const struct igt_buf *buf);
>  unsigned igt_buf_height(const struct igt_buf *buf);
>  
> +void igt_blitter_src_copy__raw(int fd,
> +				/* src */
> +				uint32_t src_handle,
> +				unsigned int src_delta,
> +				unsigned int src_stride,
> +				unsigned int src_tiling,
> +				unsigned int src_x, unsigned src_y,
> +
> +				/* size */
> +				unsigned int width, unsigned int height,
> +
> +				/* bpp */
> +				int bpp,
> +
> +				/* dst */
> +				uint32_t dst_handle,
> +				unsigned int dst_delta,
> +				unsigned int dst_stride,
> +				unsigned int dst_tiling,
> +				unsigned int dst_x, unsigned 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.24.0
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH 02/10] lib/intel_batchbuffer: Add blitter copy using XY_SRC_COPY_BLT

[Vanshidhar Konda]

On Wed, Dec 18, 2019 at 05:13:28PM +0200, Ville Syrjälä wrote:
>On Tue, Dec 17, 2019 at 09:59:17PM -0800, Vanshidhar Konda wrote:
>> Add a method that uses the XY_SRC_COPY_BLT instruction for copying
>> buffers using the blitter engine.
>>
>> Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@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 51aae4dc..1352aa95 100644
>> --- a/lib/intel_batchbuffer.c
>> +++ b/lib/intel_batchbuffer.c
>> @@ -46,6 +46,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
>> @@ -661,6 +667,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,
>> +				unsigned int src_delta,
>> +				unsigned int src_stride,
>> +				unsigned int src_tiling,
>> +				unsigned int src_x, unsigned src_y,
>
>Inconsistent unsigned int vs. unsigned

I'll fix this in the next version.

>
>> +
>> +				/* size */
>> +				unsigned int width, unsigned int height,
>> +
>> +				/* bpp */
>> +				int bpp,
>> +
>> +				/* dst */
>> +				uint32_t dst_handle,
>> +				unsigned dst_delta,
>> +				unsigned int dst_stride,
>> +				unsigned int dst_tiling,
>> +				unsigned int dst_x, unsigned 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 = fast_copy_pitch(src_stride, src_tiling);
>> +	dst_pitch = fast_copy_pitch(dst_stride, dst_tiling);
>
>I believe those do the wrong thing for pre-gen4 tiling.

The implementation that I've added is based on the latest one in
gem_blits.c. The logic I added is consistent with all the
implementations in IGT that are setting tiling for XY_SRC_COPY_BLT.
Not being aware of the history of the hardware I wanted to keep it
inline with what I found.

Would adding an assert to use this only on >= gen 4 be the right
approach?

>
>I also wonder how many implementations of this we already have in igt.
>I suspect the answer is more than two. Maybe there's a way to reduce the
>duplication a bit?
>

There are at least 13 implementations that I found. From what I can tell
there are two different forms - one using gem object based batch buffer
and another using libdrm batchbuffer and macros associated with that.
Merging them should be another patch series I think.

Vanshi

>> +
>> +	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 37e3affe..4820cebb 100644
>> --- a/lib/intel_batchbuffer.h
>> +++ b/lib/intel_batchbuffer.h
>> @@ -252,6 +252,27 @@ struct igt_buf {
>>  unsigned igt_buf_width(const struct igt_buf *buf);
>>  unsigned igt_buf_height(const struct igt_buf *buf);
>>
>> +void igt_blitter_src_copy__raw(int fd,
>> +				/* src */
>> +				uint32_t src_handle,
>> +				unsigned int src_delta,
>> +				unsigned int src_stride,
>> +				unsigned int src_tiling,
>> +				unsigned int src_x, unsigned src_y,
>> +
>> +				/* size */
>> +				unsigned int width, unsigned int height,
>> +
>> +				/* bpp */
>> +				int bpp,
>> +
>> +				/* dst */
>> +				uint32_t dst_handle,
>> +				unsigned int dst_delta,
>> +				unsigned int dst_stride,
>> +				unsigned int dst_tiling,
>> +				unsigned int dst_x, unsigned 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.24.0
>>
>> _______________________________________________
>> igt-dev mailing list
>> igt-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
>
>-- 
>Ville Syrjälä
>Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH 02/10] lib/intel_batchbuffer: Add blitter copy using XY_SRC_COPY_BLT

[Ville Syrjälä]

On Wed, Dec 18, 2019 at 09:39:30AM -0800, Vanshidhar Konda wrote:
> On Wed, Dec 18, 2019 at 05:13:28PM +0200, Ville Syrjälä wrote:
> >On Tue, Dec 17, 2019 at 09:59:17PM -0800, Vanshidhar Konda wrote:
> >> Add a method that uses the XY_SRC_COPY_BLT instruction for copying
> >> buffers using the blitter engine.
> >>
> >> Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@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 51aae4dc..1352aa95 100644
> >> --- a/lib/intel_batchbuffer.c
> >> +++ b/lib/intel_batchbuffer.c
> >> @@ -46,6 +46,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
> >> @@ -661,6 +667,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,
> >> +				unsigned int src_delta,
> >> +				unsigned int src_stride,
> >> +				unsigned int src_tiling,
> >> +				unsigned int src_x, unsigned src_y,
> >
> >Inconsistent unsigned int vs. unsigned
> 
> I'll fix this in the next version.
> 
> >
> >> +
> >> +				/* size */
> >> +				unsigned int width, unsigned int height,
> >> +
> >> +				/* bpp */
> >> +				int bpp,
> >> +
> >> +				/* dst */
> >> +				uint32_t dst_handle,
> >> +				unsigned dst_delta,
> >> +				unsigned int dst_stride,
> >> +				unsigned int dst_tiling,
> >> +				unsigned int dst_x, unsigned 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 = fast_copy_pitch(src_stride, src_tiling);
> >> +	dst_pitch = fast_copy_pitch(dst_stride, dst_tiling);
> >
> >I believe those do the wrong thing for pre-gen4 tiling.
> 
> The implementation that I've added is based on the latest one in
> gem_blits.c. The logic I added is consistent with all the
> implementations in IGT that are setting tiling for XY_SRC_COPY_BLT.
> Not being aware of the history of the hardware I wanted to keep it
> inline with what I found.

See commit 9df50aef49e0 ("lib/igt_draw: Fix blt tiled stride for
gen2/3")

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH 02/10] lib/intel_batchbuffer: Add blitter copy using XY_SRC_COPY_BLT

[Janusz Krzysztofik]

Hi Vanshi,

On Wednesday, December 18, 2019 8:39:07 PM CET Vanshidhar Konda wrote:
> 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>
> ---
>  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 51aae4dc..bc44b44f 100644
> --- a/lib/intel_batchbuffer.c
> +++ b/lib/intel_batchbuffer.c
> @@ -46,6 +46,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
> @@ -661,6 +667,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 */

Shouldn't the upper address bits be skipped on Gen < 8?

> +	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 */

ditto

Thanks,
Janusz

> +
> +	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 37e3affe..a0a22030 100644
> --- a/lib/intel_batchbuffer.h
> +++ b/lib/intel_batchbuffer.h
> @@ -252,6 +252,27 @@ struct igt_buf {
>  unsigned igt_buf_width(const struct igt_buf *buf);
>  unsigned igt_buf_height(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,
> 




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

[igt-dev] [PATCH 02/10] lib/intel_batchbuffer: Add blitter copy using XY_SRC_COPY_BLT

[Vanshidhar Konda]

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>
---
 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 51aae4dc..bc44b44f 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -46,6 +46,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
@@ -661,6 +667,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 37e3affe..a0a22030 100644
--- a/lib/intel_batchbuffer.h
+++ b/lib/intel_batchbuffer.h
@@ -252,6 +252,27 @@ struct igt_buf {
 unsigned igt_buf_width(const struct igt_buf *buf);
 unsigned igt_buf_height(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.24.0

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