[igt-dev] [PATCH i-g-t 0/2] switch lib/igt_fb.c to use lib/i915/i915_blt functions for blitter on Intel hw

[igt-dev] [PATCH i-g-t 0/2] switch lib/igt_fb.c to use lib/i915/i915_blt functions for blitter on Intel hw

[Juha-Pekka Heikkila]

Switch to new blitter functions on framebuffer creation. On Intel CI there's
still some legacy machines with relocation support hence left XY_SRC path as
legacy path.

Juha-Pekka Heikkila (2):
  lib/i915/i915_blt: Add offset to block and fast copy
  lib/igt_fb: switch blitcopy to use lib/i915/i915_blt functions

 lib/i915/i915_blt.c |  12 ++-
 lib/i915/i915_blt.h |   1 +
 lib/igt_fb.c        | 214 +++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 201 insertions(+), 26 deletions(-)

-- 
2.39.0

[igt-dev] [PATCH i-g-t 1/2] lib/i915/i915_blt: Add offset to block and fast copy

[Juha-Pekka Heikkila]

Add offset to src and dst blits, this allow to use i915_blt with multiplane
framebuffers.

Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
---
 lib/i915/i915_blt.c | 12 ++++++++----
 lib/i915/i915_blt.h |  1 +
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/lib/i915/i915_blt.c b/lib/i915/i915_blt.c
index ef67fe26f..ffaa97b42 100644
--- a/lib/i915/i915_blt.c
+++ b/lib/i915/i915_blt.c
@@ -708,8 +708,10 @@ uint64_t emit_blt_block_copy(int i915,
 	igt_assert_f(blt, "block-copy requires data to do blit\n");
 
 	alignment = gem_detect_safe_alignment(i915);
-	src_offset = get_offset(ahnd, blt->src.handle, blt->src.size, alignment);
-	dst_offset = get_offset(ahnd, blt->dst.handle, blt->dst.size, alignment);
+	src_offset = get_offset(ahnd, blt->src.handle, blt->src.size, alignment)
+		     + blt->src.offset;
+	dst_offset = get_offset(ahnd, blt->dst.handle, blt->dst.size, alignment)
+		     + blt->dst.offset;
 	bb_offset = get_offset(ahnd, blt->bb.handle, blt->bb.size, alignment);
 
 	fill_data(&data, blt, src_offset, dst_offset, ext);
@@ -1179,8 +1181,10 @@ uint64_t emit_blt_fast_copy(int i915,
 	data.dw03.dst_x2 = blt->dst.x2;
 	data.dw03.dst_y2 = blt->dst.y2;
 
-	src_offset = get_offset(ahnd, blt->src.handle, blt->src.size, alignment);
-	dst_offset = get_offset(ahnd, blt->dst.handle, blt->dst.size, alignment);
+	src_offset = get_offset(ahnd, blt->src.handle, blt->src.size, alignment)
+		     + blt->src.offset;
+	dst_offset = get_offset(ahnd, blt->dst.handle, blt->dst.size, alignment)
+		     + blt->dst.offset;
 	bb_offset = get_offset(ahnd, blt->bb.handle, blt->bb.size, alignment);
 
 	data.dw04.dst_address_lo = dst_offset;
diff --git a/lib/i915/i915_blt.h b/lib/i915/i915_blt.h
index a5f0edd15..3e5bec496 100644
--- a/lib/i915/i915_blt.h
+++ b/lib/i915/i915_blt.h
@@ -80,6 +80,7 @@ struct blt_copy_object {
 	enum blt_compression compression;  /* BC only */
 	enum blt_compression_type compression_type; /* BC only */
 	uint32_t pitch;
+	uint32_t offset;
 	uint16_t x_offset, y_offset;
 	int16_t x1, y1, x2, y2;
 
-- 
2.39.0

[igt-dev] [PATCH i-g-t 2/2] lib/igt_fb: switch blitcopy to use lib/i915/i915_blt functions

[Juha-Pekka Heikkila]

reduce code duplication

Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
---
 lib/igt_fb.c | 214 +++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 192 insertions(+), 22 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index ba89e1f60..06ebe4818 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -35,6 +35,8 @@
 #include "drmtest.h"
 #include "i915/gem_create.h"
 #include "i915/gem_mman.h"
+#include "i915/i915_blt.h"
+#include "i915/intel_mocs.h"
 #include "igt_aux.h"
 #include "igt_color_encoding.h"
 #include "igt_fb.h"
@@ -2680,12 +2682,134 @@ static void copy_with_engine(struct fb_blit_upload *blit,
 	fini_buf(src);
 }
 
+static struct blt_copy_object *blt_fb_init(const struct igt_fb *fb,
+					   uint32_t plane, uint32_t memregion)
+{
+	uint32_t name, handle;
+	struct blt_copy_object *blt;
+	enum blt_tiling_type blt_tile;
+	uint64_t stride;
+
+	blt = malloc(sizeof(*blt));
+	igt_assert(blt);
+
+	name = gem_flink(fb->fd, fb->gem_handle);
+	handle = gem_open(fb->fd, name);
+
+	switch (igt_fb_mod_to_tiling(fb->modifier)) {
+	case I915_TILING_X:
+		blt_tile = T_XMAJOR;
+		stride = fb->strides[plane] / 4;
+		break;
+	case I915_TILING_Y:
+		blt_tile = T_YMAJOR;
+		stride = fb->strides[plane] / 4;
+		break;
+	case I915_TILING_4:
+		blt_tile = T_TILE4;
+		stride = fb->strides[plane] / 4;
+		break;
+	case I915_TILING_NONE:
+	default:
+		blt_tile = T_LINEAR;
+		stride = fb->strides[plane];
+		break;
+	}
+
+	blt_set_object(blt, handle, fb->size, memregion,
+		       intel_get_uc_mocs(fb->fd),
+		       blt_tile,
+		       is_ccs_modifier(fb->modifier) ? COMPRESSION_ENABLED : COMPRESSION_DISABLED,
+		       is_gen12_mc_ccs_modifier(fb->modifier) ? COMPRESSION_TYPE_MEDIA : COMPRESSION_TYPE_3D);
+
+	blt_set_geom(blt, stride, 0, 0, fb->width, fb->plane_height[plane], 0, 0);
+
+	blt->offset = fb->offsets[plane];
+
+	blt->ptr = gem_mmap__device_coherent(fb->fd, handle, 0, fb->size,
+					     PROT_READ | PROT_WRITE);
+	return blt;
+}
+
+static enum blt_color_depth blt_get_bpp(const struct igt_fb *fb)
+{
+	switch (fb->plane_bpp[0]) {
+	case 8:
+		return CD_8bit;
+	case 16:
+		return CD_16bit;
+	case 32:
+		return CD_32bit;
+	case 64:
+		return CD_64bit;
+	case 96:
+		return CD_96bit;
+	case 128:
+		return CD_128bit;
+	default:
+		igt_assert(0);
+	}
+}
+
+#define BLT_TARGET_RC(x) (x.compression == COMPRESSION_ENABLED && \
+			  x.compression_type == COMPRESSION_TYPE_3D)
+
+#define BLT_TARGET_MC(x) (x.compression == COMPRESSION_ENABLED && \
+			  x.compression_type == COMPRESSION_TYPE_MEDIA)
+
+static uint32_t blt_compression_format(struct blt_copy_data *blt,
+				       const struct igt_fb *fb)
+{
+	if (blt->src.compression == COMPRESSION_DISABLED &&
+	    blt->dst.compression == COMPRESSION_DISABLED)
+		return 0;
+
+	if (BLT_TARGET_RC(blt->src) || BLT_TARGET_RC(blt->dst)) {
+		switch (blt->color_depth)
+		{
+		case CD_32bit:
+			return 8;
+		default:
+			igt_assert_f(0, "COMPRESSION_TYPE_3D unknown color depth\n");
+		}
+	}
+
+	if (BLT_TARGET_MC(blt->src) || BLT_TARGET_MC(blt->dst)) {
+		switch (fb->drm_format)
+		{
+		case DRM_FORMAT_XRGB8888:
+			return 8;
+		case DRM_FORMAT_XYUV8888:
+			return 9;
+		case DRM_FORMAT_NV12:
+			return 9;
+		case DRM_FORMAT_P010:
+		case DRM_FORMAT_P012:
+		case DRM_FORMAT_P016:
+			return 8;
+		default:
+			igt_assert_f(0, "COMPRESSION_TYPE_MEDIA unknown format\n");
+		}
+	} else {
+		igt_assert_f(0, "unknown compression\n");
+	}
+}
+
 static void blitcopy(const struct igt_fb *dst_fb,
 		     const struct igt_fb *src_fb)
 {
 	uint32_t src_tiling, dst_tiling;
 	uint32_t ctx = 0;
 	uint64_t ahnd = 0;
+	const intel_ctx_t *ictx = intel_ctx_create_all_physical(src_fb->fd);
+	struct intel_execution_engine2 *e;
+	uint32_t bb;
+	uint64_t bb_size = 4096;
+	struct blt_copy_data blt = {};
+	struct blt_copy_object *src, *dst;
+	struct blt_block_copy_data_ext ext = {}, *pext = NULL;
+	uint32_t mem_region = HAS_FLATCCS(intel_get_drm_devid(src_fb->fd))
+			   ? REGION_LMEM(0) : REGION_SMEM;
 
 	igt_assert_eq(dst_fb->fd, src_fb->fd);
 	igt_assert_eq(dst_fb->num_planes, src_fb->num_planes);
@@ -2697,36 +2821,81 @@ static void blitcopy(const struct igt_fb *dst_fb,
 		igt_require(gem_has_contexts(dst_fb->fd));
 		ctx = gem_context_create(dst_fb->fd);
 		ahnd = get_reloc_ahnd(dst_fb->fd, ctx);
+
+		igt_assert(__gem_create_in_memory_regions(src_fb->fd,
+							  &bb,
+							  &bb_size,
+							  mem_region) == 0);
+
+		for_each_ctx_engine(src_fb->fd, ictx, e) {
+			if (gem_engine_can_block_copy(src_fb->fd, e))
+				break;
+		}
 	}
 
 	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]);
-		/*
-		 * On GEN12+ X-tiled format support is removed from the fast
-		 * blit command, so use the XY_SRC blit command for it
-		 * instead.
-		 */
+
 		if (fast_blit_ok(src_fb) && fast_blit_ok(dst_fb)) {
-			igt_blitter_fast_copy__raw(dst_fb->fd,
-						   ahnd, ctx, NULL,
-						   src_fb->gem_handle,
-						   src_fb->offsets[i],
-						   src_fb->strides[i],
-						   src_tiling,
-						   0, 0, /* src_x, src_y */
-						   src_fb->size,
-						   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 */,
-						   dst_fb->size);
+			memset(&blt, 0, sizeof(blt));
+			blt.color_depth = blt_get_bpp(src_fb);
+
+			src = blt_fb_init(src_fb, i, mem_region);
+			dst = blt_fb_init(dst_fb, i, mem_region);
+
+			blt_set_copy_object(&blt.src, src);
+			blt_set_copy_object(&blt.dst, dst);
+
+			blt_set_batch(&blt.bb, bb, bb_size, mem_region);
+
+			blt_fast_copy(src_fb->fd, ictx, e, ahnd, &blt);
+			gem_sync(src_fb->fd, blt.dst.handle);
+
+			blt_destroy_object(src_fb->fd, src);
+			blt_destroy_object(dst_fb->fd, dst);
+		} else if (ahnd) {
+			/*
+			* On GEN12+ X-tiled format support is removed from
+			* the fast blit command, so use the block copy blit
+			* command for it instead.
+			*/
+			src = blt_fb_init(src_fb, i, mem_region);
+			dst = blt_fb_init(dst_fb, i, mem_region);
+
+			memset(&blt, 0, sizeof(blt));
+			blt.print_bb = true;
+			blt.color_depth = blt_get_bpp(src_fb);
+			blt_set_copy_object(&blt.src, src);
+			blt_set_copy_object(&blt.dst, dst);
+
+			if (HAS_FLATCCS(intel_get_drm_devid(src_fb->fd))) {
+				blt_set_object_ext(&ext.src,
+						   blt_compression_format(&blt, src_fb),
+						   src_fb->width, src_fb->height,
+						   SURFACE_TYPE_2D);
+
+				blt_set_object_ext(&ext.dst,
+						   blt_compression_format(&blt, dst_fb),
+						   dst_fb->width, dst_fb->height,
+						   SURFACE_TYPE_2D);
+
+				pext = &ext;
+			}
+
+			blt_set_batch(&blt.bb, bb, bb_size, mem_region);
+
+			blt_block_copy(src_fb->fd, ictx, e, ahnd, &blt, pext);
+			gem_sync(src_fb->fd, blt.dst.handle);
+
+			blt_destroy_object(src_fb->fd, src);
+			blt_destroy_object(dst_fb->fd, dst);
 		} else {
+			/*
+			 * If on legacy hardware where relocations are supported
+			 * we'll use XY_SRC blit command instead
+			 */
 			igt_blitter_src_copy(dst_fb->fd,
 					     ahnd, ctx, NULL,
 					     src_fb->gem_handle,
@@ -2750,6 +2919,7 @@ static void blitcopy(const struct igt_fb *dst_fb,
 	if (ctx)
 		gem_context_destroy(dst_fb->fd, ctx);
 	put_ahnd(ahnd);
+	intel_ctx_destroy(src_fb->fd, ictx);
 }
 
 static void free_linear_mapping(struct fb_blit_upload *blit)
-- 
2.39.0

Re: [igt-dev] [PATCH i-g-t 2/2] lib/igt_fb: switch blitcopy to use lib/i915/i915_blt functions

[Zbigniew Kempczyński]

On Mon, Mar 27, 2023 at 11:12:54PM +0300, Juha-Pekka Heikkila wrote:
> reduce code duplication
> 
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> ---
>  lib/igt_fb.c | 214 +++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 192 insertions(+), 22 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index ba89e1f60..06ebe4818 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -35,6 +35,8 @@
>  #include "drmtest.h"
>  #include "i915/gem_create.h"
>  #include "i915/gem_mman.h"
> +#include "i915/i915_blt.h"
> +#include "i915/intel_mocs.h"
>  #include "igt_aux.h"
>  #include "igt_color_encoding.h"
>  #include "igt_fb.h"
> @@ -2680,12 +2682,134 @@ static void copy_with_engine(struct fb_blit_upload *blit,
>  	fini_buf(src);
>  }
>  
> +static struct blt_copy_object *blt_fb_init(const struct igt_fb *fb,
> +					   uint32_t plane, uint32_t memregion)
> +{
> +	uint32_t name, handle;
> +	struct blt_copy_object *blt;
> +	enum blt_tiling_type blt_tile;
> +	uint64_t stride;
> +
> +	blt = malloc(sizeof(*blt));
> +	igt_assert(blt);
> +
> +	name = gem_flink(fb->fd, fb->gem_handle);
> +	handle = gem_open(fb->fd, name);
> +
> +	switch (igt_fb_mod_to_tiling(fb->modifier)) {
> +	case I915_TILING_X:
> +		blt_tile = T_XMAJOR;

I wonder to add helper in i915_blt: i915_tiling_to_blt_tile(int tile);

BTW Karolina added asserts which should map 1:1 i915 -> blt. Tile4 is
not added there yet but it can be, so i915_tiline_to_blt_tile() can
simply return tile;

then those few cases could looks like:

case I915_TILING_X:
case I915_TILING_Y:
case I915_TILING_4:
	blt_tile = i915_tiling_to_blt_tile(...);
	stride = fb->strides[plane] / 4;
	break;
case I915_TILING_NONE:
	...

> +		stride = fb->strides[plane] / 4;
> +		break;
> +	case I915_TILING_Y:
> +		blt_tile = T_YMAJOR;
> +		stride = fb->strides[plane] / 4;
> +		break;
> +	case I915_TILING_4:
> +		blt_tile = T_TILE4;
> +		stride = fb->strides[plane] / 4;
> +		break;
> +	case I915_TILING_NONE:
> +	default:
> +		blt_tile = T_LINEAR;
> +		stride = fb->strides[plane];
> +		break;
> +	}
> +
> +	blt_set_object(blt, handle, fb->size, memregion,
> +		       intel_get_uc_mocs(fb->fd),
> +		       blt_tile,
> +		       is_ccs_modifier(fb->modifier) ? COMPRESSION_ENABLED : COMPRESSION_DISABLED,
> +		       is_gen12_mc_ccs_modifier(fb->modifier) ? COMPRESSION_TYPE_MEDIA : COMPRESSION_TYPE_3D);
> +
> +	blt_set_geom(blt, stride, 0, 0, fb->width, fb->plane_height[plane], 0, 0);
> +
> +	blt->offset = fb->offsets[plane];
> +
> +	blt->ptr = gem_mmap__device_coherent(fb->fd, handle, 0, fb->size,
> +					     PROT_READ | PROT_WRITE);
> +	return blt;
> +}
> +
> +static enum blt_color_depth blt_get_bpp(const struct igt_fb *fb)
> +{
> +	switch (fb->plane_bpp[0]) {
> +	case 8:
> +		return CD_8bit;
> +	case 16:
> +		return CD_16bit;
> +	case 32:
> +		return CD_32bit;
> +	case 64:
> +		return CD_64bit;
> +	case 96:
> +		return CD_96bit;
> +	case 128:
> +		return CD_128bit;
> +	default:
> +		igt_assert(0);
> +	}
> +}
> +
> +#define BLT_TARGET_RC(x) (x.compression == COMPRESSION_ENABLED && \
> +			  x.compression_type == COMPRESSION_TYPE_3D)
> +
> +#define BLT_TARGET_MC(x) (x.compression == COMPRESSION_ENABLED && \
> +			  x.compression_type == COMPRESSION_TYPE_MEDIA)
> +
> +static uint32_t blt_compression_format(struct blt_copy_data *blt,
> +				       const struct igt_fb *fb)
> +{
> +	if (blt->src.compression == COMPRESSION_DISABLED &&
> +	    blt->dst.compression == COMPRESSION_DISABLED)
> +		return 0;
> +
> +	if (BLT_TARGET_RC(blt->src) || BLT_TARGET_RC(blt->dst)) {
> +		switch (blt->color_depth)
> +		{
> +		case CD_32bit:
> +			return 8;
> +		default:
> +			igt_assert_f(0, "COMPRESSION_TYPE_3D unknown color depth\n");
> +		}
> +	}
> +
> +	if (BLT_TARGET_MC(blt->src) || BLT_TARGET_MC(blt->dst)) {
> +		switch (fb->drm_format)
> +		{
> +		case DRM_FORMAT_XRGB8888:
> +			return 8;
> +		case DRM_FORMAT_XYUV8888:
> +			return 9;
> +		case DRM_FORMAT_NV12:
> +			return 9;
> +		case DRM_FORMAT_P010:
> +		case DRM_FORMAT_P012:
> +		case DRM_FORMAT_P016:
> +			return 8;
> +		default:
> +			igt_assert_f(0, "COMPRESSION_TYPE_MEDIA unknown format\n");
> +		}
> +	} else {
> +		igt_assert_f(0, "unknown compression\n");
> +	}
> +}
> +
>  static void blitcopy(const struct igt_fb *dst_fb,
>  		     const struct igt_fb *src_fb)
>  {
>  	uint32_t src_tiling, dst_tiling;
>  	uint32_t ctx = 0;
>  	uint64_t ahnd = 0;
> +	const intel_ctx_t *ictx = intel_ctx_create_all_physical(src_fb->fd);
> +	struct intel_execution_engine2 *e;
> +	uint32_t bb;
> +	uint64_t bb_size = 4096;
> +	struct blt_copy_data blt = {};
> +	struct blt_copy_object *src, *dst;
> +	struct blt_block_copy_data_ext ext = {}, *pext = NULL;
> +	uint32_t mem_region = HAS_FLATCCS(intel_get_drm_devid(src_fb->fd))
> +			   ? REGION_LMEM(0) : REGION_SMEM;
>  
>  	igt_assert_eq(dst_fb->fd, src_fb->fd);
>  	igt_assert_eq(dst_fb->num_planes, src_fb->num_planes);
> @@ -2697,36 +2821,81 @@ static void blitcopy(const struct igt_fb *dst_fb,
>  		igt_require(gem_has_contexts(dst_fb->fd));
>  		ctx = gem_context_create(dst_fb->fd);
>  		ahnd = get_reloc_ahnd(dst_fb->fd, ctx);
> +
> +		igt_assert(__gem_create_in_memory_regions(src_fb->fd,
> +							  &bb,
> +							  &bb_size,
> +							  mem_region) == 0);
> +
> +		for_each_ctx_engine(src_fb->fd, ictx, e) {
> +			if (gem_engine_can_block_copy(src_fb->fd, e))
> +				break;
> +		}
>  	}
>  
>  	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]);
> -		/*
> -		 * On GEN12+ X-tiled format support is removed from the fast
> -		 * blit command, so use the XY_SRC blit command for it
> -		 * instead.
> -		 */
> +
>  		if (fast_blit_ok(src_fb) && fast_blit_ok(dst_fb)) {

I think this blit fail when ahnd == 0 as i915_blt requires softpinning.

'if (ahnd && fast_blit_ok(src_fb) && fast_blit_ok(dst_fb))' should work.

> -			igt_blitter_fast_copy__raw(dst_fb->fd,
> -						   ahnd, ctx, NULL,
> -						   src_fb->gem_handle,
> -						   src_fb->offsets[i],
> -						   src_fb->strides[i],
> -						   src_tiling,
> -						   0, 0, /* src_x, src_y */
> -						   src_fb->size,
> -						   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 */,
> -						   dst_fb->size);
> +			memset(&blt, 0, sizeof(blt));
> +			blt.color_depth = blt_get_bpp(src_fb);
> +
> +			src = blt_fb_init(src_fb, i, mem_region);
> +			dst = blt_fb_init(dst_fb, i, mem_region);
> +
> +			blt_set_copy_object(&blt.src, src);
> +			blt_set_copy_object(&blt.dst, dst);
> +
> +			blt_set_batch(&blt.bb, bb, bb_size, mem_region);
> +
> +			blt_fast_copy(src_fb->fd, ictx, e, ahnd, &blt);
> +			gem_sync(src_fb->fd, blt.dst.handle);
> +
> +			blt_destroy_object(src_fb->fd, src);
> +			blt_destroy_object(dst_fb->fd, dst);
> +		} else if (ahnd) {

I would also check if this else supports block-copy.

If you prefer newer instructions (like fast-copy) with i915_blt it
requires valid ahnd (>0). Instead of using wrapper:

ahnd = get_reloc_ahnd(fd, ctx);

you may enforce softpinning by:

ahnd = intel_allocator_open(fd, ctx, INTEL_ALLOCATOR_RELOC);

Then i915_blt functions will work as you expect. But this will change
previous logic to prefer older blits instead new ones.

--
Zbigniew

> +			/*
> +			* On GEN12+ X-tiled format support is removed from
> +			* the fast blit command, so use the block copy blit
> +			* command for it instead.
> +			*/
> +			src = blt_fb_init(src_fb, i, mem_region);
> +			dst = blt_fb_init(dst_fb, i, mem_region);
> +
> +			memset(&blt, 0, sizeof(blt));
> +			blt.print_bb = true;
> +			blt.color_depth = blt_get_bpp(src_fb);
> +			blt_set_copy_object(&blt.src, src);
> +			blt_set_copy_object(&blt.dst, dst);
> +
> +			if (HAS_FLATCCS(intel_get_drm_devid(src_fb->fd))) {
> +				blt_set_object_ext(&ext.src,
> +						   blt_compression_format(&blt, src_fb),
> +						   src_fb->width, src_fb->height,
> +						   SURFACE_TYPE_2D);
> +
> +				blt_set_object_ext(&ext.dst,
> +						   blt_compression_format(&blt, dst_fb),
> +						   dst_fb->width, dst_fb->height,
> +						   SURFACE_TYPE_2D);
> +
> +				pext = &ext;
> +			}
> +
> +			blt_set_batch(&blt.bb, bb, bb_size, mem_region);
> +
> +			blt_block_copy(src_fb->fd, ictx, e, ahnd, &blt, pext);
> +			gem_sync(src_fb->fd, blt.dst.handle);
> +
> +			blt_destroy_object(src_fb->fd, src);
> +			blt_destroy_object(dst_fb->fd, dst);
>  		} else {
> +			/*
> +			 * If on legacy hardware where relocations are supported
> +			 * we'll use XY_SRC blit command instead
> +			 */
>  			igt_blitter_src_copy(dst_fb->fd,
>  					     ahnd, ctx, NULL,
>  					     src_fb->gem_handle,
> @@ -2750,6 +2919,7 @@ static void blitcopy(const struct igt_fb *dst_fb,
>  	if (ctx)
>  		gem_context_destroy(dst_fb->fd, ctx);
>  	put_ahnd(ahnd);
> +	intel_ctx_destroy(src_fb->fd, ictx);
>  }
>  
>  static void free_linear_mapping(struct fb_blit_upload *blit)
> -- 
> 2.39.0
> 

Re: [igt-dev] [PATCH i-g-t 2/2] lib/igt_fb: switch blitcopy to use lib/i915/i915_blt functions

[Karolina Stolarek]

On 28.03.2023 08:50, Zbigniew Kempczyński wrote:
> On Mon, Mar 27, 2023 at 11:12:54PM +0300, Juha-Pekka Heikkila wrote:
>> reduce code duplication
>>
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>> ---
>>   lib/igt_fb.c | 214 +++++++++++++++++++++++++++++++++++++++++++++------
>>   1 file changed, 192 insertions(+), 22 deletions(-)
>>
>> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
>> index ba89e1f60..06ebe4818 100644
>> --- a/lib/igt_fb.c
>> +++ b/lib/igt_fb.c
>> @@ -35,6 +35,8 @@
>>   #include "drmtest.h"
>>   #include "i915/gem_create.h"
>>   #include "i915/gem_mman.h"
>> +#include "i915/i915_blt.h"
>> +#include "i915/intel_mocs.h"
>>   #include "igt_aux.h"
>>   #include "igt_color_encoding.h"
>>   #include "igt_fb.h"
>> @@ -2680,12 +2682,134 @@ static void copy_with_engine(struct fb_blit_upload *blit,
>>   	fini_buf(src);
>>   }
>>   
>> +static struct blt_copy_object *blt_fb_init(const struct igt_fb *fb,
>> +					   uint32_t plane, uint32_t memregion)
>> +{
>> +	uint32_t name, handle;
>> +	struct blt_copy_object *blt;
>> +	enum blt_tiling_type blt_tile;
>> +	uint64_t stride;
>> +
>> +	blt = malloc(sizeof(*blt));
>> +	igt_assert(blt);
>> +
>> +	name = gem_flink(fb->fd, fb->gem_handle);
>> +	handle = gem_open(fb->fd, name);
>> +
>> +	switch (igt_fb_mod_to_tiling(fb->modifier)) {
>> +	case I915_TILING_X:
>> +		blt_tile = T_XMAJOR;
> 
> I wonder to add helper in i915_blt: i915_tiling_to_blt_tile(int tile);

I wish we could move on from using I915_TILING_* for tiling formats that 
don't require fences. It would be great if we could switch on 
blt_tiling_type when doing blit copy, especially when we have checks 
that ensure the correct mapping.

I mean, we could add such a helper, but do we want to keep it as a 
permanent solution?

> 
> BTW Karolina added asserts which should map 1:1 i915 -> blt. Tile4 is
> not added there yet but it can be, so i915_tiline_to_blt_tile() can
> simply return tile;
> 
> then those few cases could looks like:
> 
> case I915_TILING_X:
> case I915_TILING_Y:
> case I915_TILING_4:
> 	blt_tile = i915_tiling_to_blt_tile(...);
> 	stride = fb->strides[plane] / 4;
> 	break;
> case I915_TILING_NONE:
> 	...
> 
>> +		stride = fb->strides[plane] / 4;
>> +		break;
>> +	case I915_TILING_Y:
>> +		blt_tile = T_YMAJOR;
>> +		stride = fb->strides[plane] / 4;
>> +		break;
>> +	case I915_TILING_4:
>> +		blt_tile = T_TILE4;
>> +		stride = fb->strides[plane] / 4;
>> +		break;
>> +	case I915_TILING_NONE:
>> +	default:
>> +		blt_tile = T_LINEAR;
>> +		stride = fb->strides[plane];
>> +		break;
>> +	}
>> +
>> +	blt_set_object(blt, handle, fb->size, memregion,
>> +		       intel_get_uc_mocs(fb->fd),
>> +		       blt_tile,
>> +		       is_ccs_modifier(fb->modifier) ? COMPRESSION_ENABLED : COMPRESSION_DISABLED,
>> +		       is_gen12_mc_ccs_modifier(fb->modifier) ? COMPRESSION_TYPE_MEDIA : COMPRESSION_TYPE_3D);
>> +
>> +	blt_set_geom(blt, stride, 0, 0, fb->width, fb->plane_height[plane], 0, 0);
>> +
>> +	blt->offset = fb->offsets[plane];
>> +
>> +	blt->ptr = gem_mmap__device_coherent(fb->fd, handle, 0, fb->size,
>> +					     PROT_READ | PROT_WRITE);
>> +	return blt;
>> +}
>> +
>> +static enum blt_color_depth blt_get_bpp(const struct igt_fb *fb)
>> +{
>> +	switch (fb->plane_bpp[0]) {
>> +	case 8:
>> +		return CD_8bit;
>> +	case 16:
>> +		return CD_16bit;
>> +	case 32:
>> +		return CD_32bit;
>> +	case 64:
>> +		return CD_64bit;
>> +	case 96:
>> +		return CD_96bit;
>> +	case 128:
>> +		return CD_128bit;
>> +	default:
>> +		igt_assert(0);
>> +	}
>> +}
>> +
>> +#define BLT_TARGET_RC(x) (x.compression == COMPRESSION_ENABLED && \
>> +			  x.compression_type == COMPRESSION_TYPE_3D)
>> +
>> +#define BLT_TARGET_MC(x) (x.compression == COMPRESSION_ENABLED && \
>> +			  x.compression_type == COMPRESSION_TYPE_MEDIA)
>> +
>> +static uint32_t blt_compression_format(struct blt_copy_data *blt,
>> +				       const struct igt_fb *fb)
>> +{
>> +	if (blt->src.compression == COMPRESSION_DISABLED &&
>> +	    blt->dst.compression == COMPRESSION_DISABLED)
>> +		return 0;
>> +
>> +	if (BLT_TARGET_RC(blt->src) || BLT_TARGET_RC(blt->dst)) {
>> +		switch (blt->color_depth)
>> +		{
>> +		case CD_32bit:
>> +			return 8;
>> +		default:
>> +			igt_assert_f(0, "COMPRESSION_TYPE_3D unknown color depth\n");
>> +		}
>> +	}
>> +
>> +	if (BLT_TARGET_MC(blt->src) || BLT_TARGET_MC(blt->dst)) {
>> +		switch (fb->drm_format)
>> +		{
>> +		case DRM_FORMAT_XRGB8888:
>> +			return 8;
>> +		case DRM_FORMAT_XYUV8888:
>> +			return 9;
>> +		case DRM_FORMAT_NV12:
>> +			return 9;
>> +		case DRM_FORMAT_P010:
>> +		case DRM_FORMAT_P012:
>> +		case DRM_FORMAT_P016:
>> +			return 8;
>> +		default:
>> +			igt_assert_f(0, "COMPRESSION_TYPE_MEDIA unknown format\n");
>> +		}
>> +	} else {
>> +		igt_assert_f(0, "unknown compression\n");
>> +	}
>> +}
>> +
>>   static void blitcopy(const struct igt_fb *dst_fb,
>>   		     const struct igt_fb *src_fb)
>>   {
>>   	uint32_t src_tiling, dst_tiling;
>>   	uint32_t ctx = 0;
>>   	uint64_t ahnd = 0;
>> +	const intel_ctx_t *ictx = intel_ctx_create_all_physical(src_fb->fd);
>> +	struct intel_execution_engine2 *e;
>> +	uint32_t bb;
>> +	uint64_t bb_size = 4096;
>> +	struct blt_copy_data blt = {};
>> +	struct blt_copy_object *src, *dst;
>> +	struct blt_block_copy_data_ext ext = {}, *pext = NULL;
>> +	uint32_t mem_region = HAS_FLATCCS(intel_get_drm_devid(src_fb->fd))
>> +			   ? REGION_LMEM(0) : REGION_SMEM;
>>   
>>   	igt_assert_eq(dst_fb->fd, src_fb->fd);
>>   	igt_assert_eq(dst_fb->num_planes, src_fb->num_planes);
>> @@ -2697,36 +2821,81 @@ static void blitcopy(const struct igt_fb *dst_fb,
>>   		igt_require(gem_has_contexts(dst_fb->fd));
>>   		ctx = gem_context_create(dst_fb->fd);
>>   		ahnd = get_reloc_ahnd(dst_fb->fd, ctx);
>> +
>> +		igt_assert(__gem_create_in_memory_regions(src_fb->fd,
>> +							  &bb,
>> +							  &bb_size,
>> +							  mem_region) == 0);
>> +
>> +		for_each_ctx_engine(src_fb->fd, ictx, e) {
>> +			if (gem_engine_can_block_copy(src_fb->fd, e))
>> +				break;
>> +		}
>>   	}
>>   
>>   	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]);
>> -		/*
>> -		 * On GEN12+ X-tiled format support is removed from the fast
>> -		 * blit command, so use the XY_SRC blit command for it
>> -		 * instead.
>> -		 */
>> +
>>   		if (fast_blit_ok(src_fb) && fast_blit_ok(dst_fb)) {
> 
> I think this blit fail when ahnd == 0 as i915_blt requires softpinning.

Also, are we fine with the fact that we'll only cover gen 8 and above 
(due to the softpinning requirement)?

> 
> 'if (ahnd && fast_blit_ok(src_fb) && fast_blit_ok(dst_fb))' should work.

I had a quick look at the function, and I wonder if we could i915_blt 
helpers here instead. There seem to be other factors that we have to 
consider here, but the display version checks seem similar to what we 
have in intel_cmd_info lib.


All the best,
Karolina

> 
>> -			igt_blitter_fast_copy__raw(dst_fb->fd,
>> -						   ahnd, ctx, NULL,
>> -						   src_fb->gem_handle,
>> -						   src_fb->offsets[i],
>> -						   src_fb->strides[i],
>> -						   src_tiling,
>> -						   0, 0, /* src_x, src_y */
>> -						   src_fb->size,
>> -						   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 */,
>> -						   dst_fb->size);
>> +			memset(&blt, 0, sizeof(blt));
>> +			blt.color_depth = blt_get_bpp(src_fb);
>> +
>> +			src = blt_fb_init(src_fb, i, mem_region);
>> +			dst = blt_fb_init(dst_fb, i, mem_region);
>> +
>> +			blt_set_copy_object(&blt.src, src);
>> +			blt_set_copy_object(&blt.dst, dst);
>> +
>> +			blt_set_batch(&blt.bb, bb, bb_size, mem_region);
>> +
>> +			blt_fast_copy(src_fb->fd, ictx, e, ahnd, &blt);
>> +			gem_sync(src_fb->fd, blt.dst.handle);
>> +
>> +			blt_destroy_object(src_fb->fd, src);
>> +			blt_destroy_object(dst_fb->fd, dst);
>> +		} else if (ahnd) {
> 
> I would also check if this else supports block-copy.
> 
> If you prefer newer instructions (like fast-copy) with i915_blt it
> requires valid ahnd (>0). Instead of using wrapper:
> 
> ahnd = get_reloc_ahnd(fd, ctx);
> 
> you may enforce softpinning by:
> 
> ahnd = intel_allocator_open(fd, ctx, INTEL_ALLOCATOR_RELOC);
> 
> Then i915_blt functions will work as you expect. But this will change
> previous logic to prefer older blits instead new ones.
> 
> --
> Zbigniew
> 
>> +			/*
>> +			* On GEN12+ X-tiled format support is removed from
>> +			* the fast blit command, so use the block copy blit
>> +			* command for it instead.
>> +			*/
>> +			src = blt_fb_init(src_fb, i, mem_region);
>> +			dst = blt_fb_init(dst_fb, i, mem_region);
>> +
>> +			memset(&blt, 0, sizeof(blt));
>> +			blt.print_bb = true;
>> +			blt.color_depth = blt_get_bpp(src_fb);
>> +			blt_set_copy_object(&blt.src, src);
>> +			blt_set_copy_object(&blt.dst, dst);
>> +
>> +			if (HAS_FLATCCS(intel_get_drm_devid(src_fb->fd))) {
>> +				blt_set_object_ext(&ext.src,
>> +						   blt_compression_format(&blt, src_fb),
>> +						   src_fb->width, src_fb->height,
>> +						   SURFACE_TYPE_2D);
>> +
>> +				blt_set_object_ext(&ext.dst,
>> +						   blt_compression_format(&blt, dst_fb),
>> +						   dst_fb->width, dst_fb->height,
>> +						   SURFACE_TYPE_2D);
>> +
>> +				pext = &ext;
>> +			}
>> +
>> +			blt_set_batch(&blt.bb, bb, bb_size, mem_region);
>> +
>> +			blt_block_copy(src_fb->fd, ictx, e, ahnd, &blt, pext);
>> +			gem_sync(src_fb->fd, blt.dst.handle);
>> +
>> +			blt_destroy_object(src_fb->fd, src);
>> +			blt_destroy_object(dst_fb->fd, dst);
>>   		} else {
>> +			/*
>> +			 * If on legacy hardware where relocations are supported
>> +			 * we'll use XY_SRC blit command instead
>> +			 */
>>   			igt_blitter_src_copy(dst_fb->fd,
>>   					     ahnd, ctx, NULL,
>>   					     src_fb->gem_handle,
>> @@ -2750,6 +2919,7 @@ static void blitcopy(const struct igt_fb *dst_fb,
>>   	if (ctx)
>>   		gem_context_destroy(dst_fb->fd, ctx);
>>   	put_ahnd(ahnd);
>> +	intel_ctx_destroy(src_fb->fd, ictx);
>>   }
>>   
>>   static void free_linear_mapping(struct fb_blit_upload *blit)
>> -- 
>> 2.39.0
>>

Re: [igt-dev] [PATCH i-g-t 2/2] lib/igt_fb: switch blitcopy to use lib/i915/i915_blt functions

[Juha-Pekka Heikkila]

Hei,

thanks for the comments. This was my first attempt on this and I got 
those failures which Zbigniew's comment explained how those happen. I'll 
soon send new version.. I was also wondering about insertion of that 
offset in the other patch, it somehow feels bit out of place but didn't 
find better place for it.

couple of comments below.

On 28.3.2023 17.10, Karolina Stolarek wrote:
> On 28.03.2023 08:50, Zbigniew Kempczyński wrote:
>> On Mon, Mar 27, 2023 at 11:12:54PM +0300, Juha-Pekka Heikkila wrote:
>>> reduce code duplication
>>>
>>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>>> ---
>>>   lib/igt_fb.c | 214 +++++++++++++++++++++++++++++++++++++++++++++------
>>>   1 file changed, 192 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
>>> index ba89e1f60..06ebe4818 100644
>>> --- a/lib/igt_fb.c
>>> +++ b/lib/igt_fb.c
>>> @@ -35,6 +35,8 @@
>>>   #include "drmtest.h"
>>>   #include "i915/gem_create.h"
>>>   #include "i915/gem_mman.h"
>>> +#include "i915/i915_blt.h"
>>> +#include "i915/intel_mocs.h"
>>>   #include "igt_aux.h"
>>>   #include "igt_color_encoding.h"
>>>   #include "igt_fb.h"
>>> @@ -2680,12 +2682,134 @@ static void copy_with_engine(struct 
>>> fb_blit_upload *blit,
>>>       fini_buf(src);
>>>   }
>>> +static struct blt_copy_object *blt_fb_init(const struct igt_fb *fb,
>>> +                       uint32_t plane, uint32_t memregion)
>>> +{
>>> +    uint32_t name, handle;
>>> +    struct blt_copy_object *blt;
>>> +    enum blt_tiling_type blt_tile;
>>> +    uint64_t stride;
>>> +
>>> +    blt = malloc(sizeof(*blt));
>>> +    igt_assert(blt);
>>> +
>>> +    name = gem_flink(fb->fd, fb->gem_handle);
>>> +    handle = gem_open(fb->fd, name);
>>> +
>>> +    switch (igt_fb_mod_to_tiling(fb->modifier)) {
>>> +    case I915_TILING_X:
>>> +        blt_tile = T_XMAJOR;
>>
>> I wonder to add helper in i915_blt: i915_tiling_to_blt_tile(int tile);
> 
> I wish we could move on from using I915_TILING_* for tiling formats that 
> don't require fences. It would be great if we could switch on 
> blt_tiling_type when doing blit copy, especially when we have checks 
> that ensure the correct mapping.
> 
> I mean, we could add such a helper, but do we want to keep it as a 
> permanent solution?
> 
>>
>> BTW Karolina added asserts which should map 1:1 i915 -> blt. Tile4 is
>> not added there yet but it can be, so i915_tiline_to_blt_tile() can
>> simply return tile;
>>
>> then those few cases could looks like:
>>
>> case I915_TILING_X:
>> case I915_TILING_Y:
>> case I915_TILING_4:
>>     blt_tile = i915_tiling_to_blt_tile(...);
>>     stride = fb->strides[plane] / 4;
>>     break;
>> case I915_TILING_NONE:
>>     ...

i915_tiling_to_blt_tile is coming in some yet unmerged set? I didn't 
find it in my igt.

>>
>>> +        stride = fb->strides[plane] / 4;
>>> +        break;
>>> +    case I915_TILING_Y:
>>> +        blt_tile = T_YMAJOR;
>>> +        stride = fb->strides[plane] / 4;
>>> +        break;
>>> +    case I915_TILING_4:
>>> +        blt_tile = T_TILE4;
>>> +        stride = fb->strides[plane] / 4;
>>> +        break;
>>> +    case I915_TILING_NONE:
>>> +    default:
>>> +        blt_tile = T_LINEAR;
>>> +        stride = fb->strides[plane];
>>> +        break;
>>> +    }
>>> +
>>> +    blt_set_object(blt, handle, fb->size, memregion,
>>> +               intel_get_uc_mocs(fb->fd),
>>> +               blt_tile,
>>> +               is_ccs_modifier(fb->modifier) ? COMPRESSION_ENABLED : 
>>> COMPRESSION_DISABLED,
>>> +               is_gen12_mc_ccs_modifier(fb->modifier) ? 
>>> COMPRESSION_TYPE_MEDIA : COMPRESSION_TYPE_3D);
>>> +
>>> +    blt_set_geom(blt, stride, 0, 0, fb->width, 
>>> fb->plane_height[plane], 0, 0);
>>> +
>>> +    blt->offset = fb->offsets[plane];
>>> +
>>> +    blt->ptr = gem_mmap__device_coherent(fb->fd, handle, 0, fb->size,
>>> +                         PROT_READ | PROT_WRITE);
>>> +    return blt;
>>> +}
>>> +
>>> +static enum blt_color_depth blt_get_bpp(const struct igt_fb *fb)
>>> +{
>>> +    switch (fb->plane_bpp[0]) {
>>> +    case 8:
>>> +        return CD_8bit;
>>> +    case 16:
>>> +        return CD_16bit;
>>> +    case 32:
>>> +        return CD_32bit;
>>> +    case 64:
>>> +        return CD_64bit;
>>> +    case 96:
>>> +        return CD_96bit;
>>> +    case 128:
>>> +        return CD_128bit;
>>> +    default:
>>> +        igt_assert(0);
>>> +    }
>>> +}
>>> +
>>> +#define BLT_TARGET_RC(x) (x.compression == COMPRESSION_ENABLED && \
>>> +              x.compression_type == COMPRESSION_TYPE_3D)
>>> +
>>> +#define BLT_TARGET_MC(x) (x.compression == COMPRESSION_ENABLED && \
>>> +              x.compression_type == COMPRESSION_TYPE_MEDIA)
>>> +
>>> +static uint32_t blt_compression_format(struct blt_copy_data *blt,
>>> +                       const struct igt_fb *fb)
>>> +{
>>> +    if (blt->src.compression == COMPRESSION_DISABLED &&
>>> +        blt->dst.compression == COMPRESSION_DISABLED)
>>> +        return 0;
>>> +
>>> +    if (BLT_TARGET_RC(blt->src) || BLT_TARGET_RC(blt->dst)) {
>>> +        switch (blt->color_depth)
>>> +        {
>>> +        case CD_32bit:
>>> +            return 8;
>>> +        default:
>>> +            igt_assert_f(0, "COMPRESSION_TYPE_3D unknown color 
>>> depth\n");
>>> +        }
>>> +    }
>>> +
>>> +    if (BLT_TARGET_MC(blt->src) || BLT_TARGET_MC(blt->dst)) {
>>> +        switch (fb->drm_format)
>>> +        {
>>> +        case DRM_FORMAT_XRGB8888:
>>> +            return 8;
>>> +        case DRM_FORMAT_XYUV8888:
>>> +            return 9;
>>> +        case DRM_FORMAT_NV12:
>>> +            return 9;
>>> +        case DRM_FORMAT_P010:
>>> +        case DRM_FORMAT_P012:
>>> +        case DRM_FORMAT_P016:
>>> +            return 8;
>>> +        default:
>>> +            igt_assert_f(0, "COMPRESSION_TYPE_MEDIA unknown format\n");
>>> +        }
>>> +    } else {
>>> +        igt_assert_f(0, "unknown compression\n");
>>> +    }
>>> +}
>>> +
>>>   static void blitcopy(const struct igt_fb *dst_fb,
>>>                const struct igt_fb *src_fb)
>>>   {
>>>       uint32_t src_tiling, dst_tiling;
>>>       uint32_t ctx = 0;
>>>       uint64_t ahnd = 0;
>>> +    const intel_ctx_t *ictx = 
>>> intel_ctx_create_all_physical(src_fb->fd);
>>> +    struct intel_execution_engine2 *e;
>>> +    uint32_t bb;
>>> +    uint64_t bb_size = 4096;
>>> +    struct blt_copy_data blt = {};
>>> +    struct blt_copy_object *src, *dst;
>>> +    struct blt_block_copy_data_ext ext = {}, *pext = NULL;
>>> +    uint32_t mem_region = HAS_FLATCCS(intel_get_drm_devid(src_fb->fd))
>>> +               ? REGION_LMEM(0) : REGION_SMEM;
>>>       igt_assert_eq(dst_fb->fd, src_fb->fd);
>>>       igt_assert_eq(dst_fb->num_planes, src_fb->num_planes);
>>> @@ -2697,36 +2821,81 @@ static void blitcopy(const struct igt_fb 
>>> *dst_fb,
>>>           igt_require(gem_has_contexts(dst_fb->fd));
>>>           ctx = gem_context_create(dst_fb->fd);
>>>           ahnd = get_reloc_ahnd(dst_fb->fd, ctx);
>>> +
>>> +        igt_assert(__gem_create_in_memory_regions(src_fb->fd,
>>> +                              &bb,
>>> +                              &bb_size,
>>> +                              mem_region) == 0);
>>> +
>>> +        for_each_ctx_engine(src_fb->fd, ictx, e) {
>>> +            if (gem_engine_can_block_copy(src_fb->fd, e))
>>> +                break;
>>> +        }
>>>       }
>>>       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]);
>>> -        /*
>>> -         * On GEN12+ X-tiled format support is removed from the fast
>>> -         * blit command, so use the XY_SRC blit command for it
>>> -         * instead.
>>> -         */
>>> +
>>>           if (fast_blit_ok(src_fb) && fast_blit_ok(dst_fb)) {
>>
>> I think this blit fail when ahnd == 0 as i915_blt requires softpinning.

I think this is what I saw as failures in ci on tgl and some gen9 
testboxes. Before sending I tested only with dg2 and adlp so I didn't 
see this problem earlier.

> 
> Also, are we fine with the fact that we'll only cover gen 8 and above 
> (due to the softpinning requirement)?
> 
>>
>> 'if (ahnd && fast_blit_ok(src_fb) && fast_blit_ok(dst_fb))' should work.
> 
> I had a quick look at the function, and I wonder if we could i915_blt 
> helpers here instead. There seem to be other factors that we have to 
> consider here, but the display version checks seem similar to what we 
> have in intel_cmd_info lib.
> 

fast_blit_ok(..) checks also fb for x-tiling so I think I'll stay with 
that for now.

> 
>>
>>> -            igt_blitter_fast_copy__raw(dst_fb->fd,
>>> -                           ahnd, ctx, NULL,
>>> -                           src_fb->gem_handle,
>>> -                           src_fb->offsets[i],
>>> -                           src_fb->strides[i],
>>> -                           src_tiling,
>>> -                           0, 0, /* src_x, src_y */
>>> -                           src_fb->size,
>>> -                           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 */,
>>> -                           dst_fb->size);
>>> +            memset(&blt, 0, sizeof(blt));
>>> +            blt.color_depth = blt_get_bpp(src_fb);
>>> +
>>> +            src = blt_fb_init(src_fb, i, mem_region);
>>> +            dst = blt_fb_init(dst_fb, i, mem_region);
>>> +
>>> +            blt_set_copy_object(&blt.src, src);
>>> +            blt_set_copy_object(&blt.dst, dst);
>>> +
>>> +            blt_set_batch(&blt.bb, bb, bb_size, mem_region);
>>> +
>>> +            blt_fast_copy(src_fb->fd, ictx, e, ahnd, &blt);
>>> +            gem_sync(src_fb->fd, blt.dst.handle);
>>> +
>>> +            blt_destroy_object(src_fb->fd, src);
>>> +            blt_destroy_object(dst_fb->fd, dst);
>>> +        } else if (ahnd) {
>>
>> I would also check if this else supports block-copy.
>>
>> If you prefer newer instructions (like fast-copy) with i915_blt it
>> requires valid ahnd (>0). Instead of using wrapper:
>>
>> ahnd = get_reloc_ahnd(fd, ctx);
>>
>> you may enforce softpinning by:
>>
>> ahnd = intel_allocator_open(fd, ctx, INTEL_ALLOCATOR_RELOC);
>>
>> Then i915_blt functions will work as you expect. But this will change
>> previous logic to prefer older blits instead new ones.
>>

I now put above "ahnd && blt_has_block_copy(src_fb->fd)". I'll need to 
check test runtimes what will come from different boxes to see if 
there's something more needed to do with these, as long as runtimes wont 
start to grow I'll be ok either way of getting ahnd.

/Juha-Pekka

>>
>>> +            /*
>>> +            * On GEN12+ X-tiled format support is removed from
>>> +            * the fast blit command, so use the block copy blit
>>> +            * command for it instead.
>>> +            */
>>> +            src = blt_fb_init(src_fb, i, mem_region);
>>> +            dst = blt_fb_init(dst_fb, i, mem_region);
>>> +
>>> +            memset(&blt, 0, sizeof(blt));
>>> +            blt.print_bb = true;
>>> +            blt.color_depth = blt_get_bpp(src_fb);
>>> +            blt_set_copy_object(&blt.src, src);
>>> +            blt_set_copy_object(&blt.dst, dst);
>>> +
>>> +            if (HAS_FLATCCS(intel_get_drm_devid(src_fb->fd))) {
>>> +                blt_set_object_ext(&ext.src,
>>> +                           blt_compression_format(&blt, src_fb),
>>> +                           src_fb->width, src_fb->height,
>>> +                           SURFACE_TYPE_2D);
>>> +
>>> +                blt_set_object_ext(&ext.dst,
>>> +                           blt_compression_format(&blt, dst_fb),
>>> +                           dst_fb->width, dst_fb->height,
>>> +                           SURFACE_TYPE_2D);
>>> +
>>> +                pext = &ext;
>>> +            }
>>> +
>>> +            blt_set_batch(&blt.bb, bb, bb_size, mem_region);
>>> +
>>> +            blt_block_copy(src_fb->fd, ictx, e, ahnd, &blt, pext);
>>> +            gem_sync(src_fb->fd, blt.dst.handle);
>>> +
>>> +            blt_destroy_object(src_fb->fd, src);
>>> +            blt_destroy_object(dst_fb->fd, dst);
>>>           } else {
>>> +            /*
>>> +             * If on legacy hardware where relocations are supported
>>> +             * we'll use XY_SRC blit command instead
>>> +             */
>>>               igt_blitter_src_copy(dst_fb->fd,
>>>                            ahnd, ctx, NULL,
>>>                            src_fb->gem_handle,
>>> @@ -2750,6 +2919,7 @@ static void blitcopy(const struct igt_fb *dst_fb,
>>>       if (ctx)
>>>           gem_context_destroy(dst_fb->fd, ctx);
>>>       put_ahnd(ahnd);
>>> +    intel_ctx_destroy(src_fb->fd, ictx);
>>>   }
>>>   static void free_linear_mapping(struct fb_blit_upload *blit)
>>> -- 
>>> 2.39.0
>>>

Re: [igt-dev] [PATCH i-g-t 2/2] lib/igt_fb: switch blitcopy to use lib/i915/i915_blt functions

[Karolina Stolarek]

On 28.03.2023 19:29, Juha-Pekka Heikkila wrote:
> Hei,
> 
> thanks for the comments. This was my first attempt on this and I got 
> those failures which Zbigniew's comment explained how those happen. I'll 
> soon send new version.. I was also wondering about insertion of that 
> offset in the other patch, it somehow feels bit out of place but didn't 
> find better place for it.
> 
> couple of comments below.
> 
> On 28.3.2023 17.10, Karolina Stolarek wrote:
>> On 28.03.2023 08:50, Zbigniew Kempczyński wrote:
>>> On Mon, Mar 27, 2023 at 11:12:54PM +0300, Juha-Pekka Heikkila wrote:
>>>> reduce code duplication
>>>>
>>>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>>>> ---
>>>>   lib/igt_fb.c | 214 
>>>> +++++++++++++++++++++++++++++++++++++++++++++------
>>>>   1 file changed, 192 insertions(+), 22 deletions(-)
>>>>
>>>> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
>>>> index ba89e1f60..06ebe4818 100644
>>>> --- a/lib/igt_fb.c
>>>> +++ b/lib/igt_fb.c
>>>> @@ -35,6 +35,8 @@
>>>>   #include "drmtest.h"
>>>>   #include "i915/gem_create.h"
>>>>   #include "i915/gem_mman.h"
>>>> +#include "i915/i915_blt.h"
>>>> +#include "i915/intel_mocs.h"
>>>>   #include "igt_aux.h"
>>>>   #include "igt_color_encoding.h"
>>>>   #include "igt_fb.h"
>>>> @@ -2680,12 +2682,134 @@ static void copy_with_engine(struct 
>>>> fb_blit_upload *blit,
>>>>       fini_buf(src);
>>>>   }
>>>> +static struct blt_copy_object *blt_fb_init(const struct igt_fb *fb,
>>>> +                       uint32_t plane, uint32_t memregion)
>>>> +{
>>>> +    uint32_t name, handle;
>>>> +    struct blt_copy_object *blt;
>>>> +    enum blt_tiling_type blt_tile;
>>>> +    uint64_t stride;
>>>> +
>>>> +    blt = malloc(sizeof(*blt));
>>>> +    igt_assert(blt);
>>>> +
>>>> +    name = gem_flink(fb->fd, fb->gem_handle);
>>>> +    handle = gem_open(fb->fd, name);
>>>> +
>>>> +    switch (igt_fb_mod_to_tiling(fb->modifier)) {
>>>> +    case I915_TILING_X:
>>>> +        blt_tile = T_XMAJOR;
>>>
>>> I wonder to add helper in i915_blt: i915_tiling_to_blt_tile(int tile);
>>
>> I wish we could move on from using I915_TILING_* for tiling formats 
>> that don't require fences. It would be great if we could switch on 
>> blt_tiling_type when doing blit copy, especially when we have checks 
>> that ensure the correct mapping.
>>
>> I mean, we could add such a helper, but do we want to keep it as a 
>> permanent solution?
>>
>>>
>>> BTW Karolina added asserts which should map 1:1 i915 -> blt. Tile4 is
>>> not added there yet but it can be, so i915_tiline_to_blt_tile() can
>>> simply return tile;
>>>
>>> then those few cases could looks like:
>>>
>>> case I915_TILING_X:
>>> case I915_TILING_Y:
>>> case I915_TILING_4:
>>>     blt_tile = i915_tiling_to_blt_tile(...);
>>>     stride = fb->strides[plane] / 4;
>>>     break;
>>> case I915_TILING_NONE:
>>>     ...
> 
> i915_tiling_to_blt_tile is coming in some yet unmerged set? I didn't 
> find it in my igt.

No, I believe it was a suggestion to add such a function in. I'm not 
working on adding such function at least. In my patches I just use 
blt_tiling_type instead of I915_TILING_* definitions (but not sure how 
such approach would fit in igt_fb).

> 
>>>
>>>> +        stride = fb->strides[plane] / 4;
>>>> +        break;
>>>> +    case I915_TILING_Y:
>>>> +        blt_tile = T_YMAJOR;
>>>> +        stride = fb->strides[plane] / 4;
>>>> +        break;
>>>> +    case I915_TILING_4:
>>>> +        blt_tile = T_TILE4;
>>>> +        stride = fb->strides[plane] / 4;
>>>> +        break;
>>>> +    case I915_TILING_NONE:
>>>> +    default:
>>>> +        blt_tile = T_LINEAR;
>>>> +        stride = fb->strides[plane];
>>>> +        break;
>>>> +    }
>>>> +
>>>> +    blt_set_object(blt, handle, fb->size, memregion,
>>>> +               intel_get_uc_mocs(fb->fd),
>>>> +               blt_tile,
>>>> +               is_ccs_modifier(fb->modifier) ? COMPRESSION_ENABLED 
>>>> : COMPRESSION_DISABLED,
>>>> +               is_gen12_mc_ccs_modifier(fb->modifier) ? 
>>>> COMPRESSION_TYPE_MEDIA : COMPRESSION_TYPE_3D);
>>>> +
>>>> +    blt_set_geom(blt, stride, 0, 0, fb->width, 
>>>> fb->plane_height[plane], 0, 0);
>>>> +
>>>> +    blt->offset = fb->offsets[plane];
>>>> +
>>>> +    blt->ptr = gem_mmap__device_coherent(fb->fd, handle, 0, fb->size,
>>>> +                         PROT_READ | PROT_WRITE);
>>>> +    return blt;
>>>> +}
>>>> +
>>>> +static enum blt_color_depth blt_get_bpp(const struct igt_fb *fb)
>>>> +{
>>>> +    switch (fb->plane_bpp[0]) {
>>>> +    case 8:
>>>> +        return CD_8bit;
>>>> +    case 16:
>>>> +        return CD_16bit;
>>>> +    case 32:
>>>> +        return CD_32bit;
>>>> +    case 64:
>>>> +        return CD_64bit;
>>>> +    case 96:
>>>> +        return CD_96bit;
>>>> +    case 128:
>>>> +        return CD_128bit;
>>>> +    default:
>>>> +        igt_assert(0);
>>>> +    }
>>>> +}
>>>> +
>>>> +#define BLT_TARGET_RC(x) (x.compression == COMPRESSION_ENABLED && \
>>>> +              x.compression_type == COMPRESSION_TYPE_3D)
>>>> +
>>>> +#define BLT_TARGET_MC(x) (x.compression == COMPRESSION_ENABLED && \
>>>> +              x.compression_type == COMPRESSION_TYPE_MEDIA)
>>>> +
>>>> +static uint32_t blt_compression_format(struct blt_copy_data *blt,
>>>> +                       const struct igt_fb *fb)
>>>> +{
>>>> +    if (blt->src.compression == COMPRESSION_DISABLED &&
>>>> +        blt->dst.compression == COMPRESSION_DISABLED)
>>>> +        return 0;
>>>> +
>>>> +    if (BLT_TARGET_RC(blt->src) || BLT_TARGET_RC(blt->dst)) {
>>>> +        switch (blt->color_depth)
>>>> +        {
>>>> +        case CD_32bit:
>>>> +            return 8;
>>>> +        default:
>>>> +            igt_assert_f(0, "COMPRESSION_TYPE_3D unknown color 
>>>> depth\n");
>>>> +        }
>>>> +    }
>>>> +
>>>> +    if (BLT_TARGET_MC(blt->src) || BLT_TARGET_MC(blt->dst)) {
>>>> +        switch (fb->drm_format)
>>>> +        {
>>>> +        case DRM_FORMAT_XRGB8888:
>>>> +            return 8;
>>>> +        case DRM_FORMAT_XYUV8888:
>>>> +            return 9;
>>>> +        case DRM_FORMAT_NV12:
>>>> +            return 9;
>>>> +        case DRM_FORMAT_P010:
>>>> +        case DRM_FORMAT_P012:
>>>> +        case DRM_FORMAT_P016:
>>>> +            return 8;
>>>> +        default:
>>>> +            igt_assert_f(0, "COMPRESSION_TYPE_MEDIA unknown 
>>>> format\n");
>>>> +        }
>>>> +    } else {
>>>> +        igt_assert_f(0, "unknown compression\n");
>>>> +    }
>>>> +}
>>>> +
>>>>   static void blitcopy(const struct igt_fb *dst_fb,
>>>>                const struct igt_fb *src_fb)
>>>>   {
>>>>       uint32_t src_tiling, dst_tiling;
>>>>       uint32_t ctx = 0;
>>>>       uint64_t ahnd = 0;
>>>> +    const intel_ctx_t *ictx = 
>>>> intel_ctx_create_all_physical(src_fb->fd);
>>>> +    struct intel_execution_engine2 *e;
>>>> +    uint32_t bb;
>>>> +    uint64_t bb_size = 4096;
>>>> +    struct blt_copy_data blt = {};
>>>> +    struct blt_copy_object *src, *dst;
>>>> +    struct blt_block_copy_data_ext ext = {}, *pext = NULL;
>>>> +    uint32_t mem_region = HAS_FLATCCS(intel_get_drm_devid(src_fb->fd))
>>>> +               ? REGION_LMEM(0) : REGION_SMEM;
>>>>       igt_assert_eq(dst_fb->fd, src_fb->fd);
>>>>       igt_assert_eq(dst_fb->num_planes, src_fb->num_planes);
>>>> @@ -2697,36 +2821,81 @@ static void blitcopy(const struct igt_fb 
>>>> *dst_fb,
>>>>           igt_require(gem_has_contexts(dst_fb->fd));
>>>>           ctx = gem_context_create(dst_fb->fd);
>>>>           ahnd = get_reloc_ahnd(dst_fb->fd, ctx);
>>>> +
>>>> +        igt_assert(__gem_create_in_memory_regions(src_fb->fd,
>>>> +                              &bb,
>>>> +                              &bb_size,
>>>> +                              mem_region) == 0);
>>>> +
>>>> +        for_each_ctx_engine(src_fb->fd, ictx, e) {
>>>> +            if (gem_engine_can_block_copy(src_fb->fd, e))
>>>> +                break;
>>>> +        }
>>>>       }
>>>>       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]);
>>>> -        /*
>>>> -         * On GEN12+ X-tiled format support is removed from the fast
>>>> -         * blit command, so use the XY_SRC blit command for it
>>>> -         * instead.
>>>> -         */
>>>> +
>>>>           if (fast_blit_ok(src_fb) && fast_blit_ok(dst_fb)) {
>>>
>>> I think this blit fail when ahnd == 0 as i915_blt requires softpinning.
> 
> I think this is what I saw as failures in ci on tgl and some gen9 
> testboxes. Before sending I tested only with dg2 and adlp so I didn't 
> see this problem earlier.
> 
>>
>> Also, are we fine with the fact that we'll only cover gen 8 and above 
>> (due to the softpinning requirement)?
>>
>>>
>>> 'if (ahnd && fast_blit_ok(src_fb) && fast_blit_ok(dst_fb))' should work.
>>
>> I had a quick look at the function, and I wonder if we could i915_blt 
>> helpers here instead. There seem to be other factors that we have to 
>> consider here, but the display version checks seem similar to what we 
>> have in intel_cmd_info lib.
>>
> 
> fast_blit_ok(..) checks also fb for x-tiling so I think I'll stay with 
> that for now.

Right, that's fine

Many thanks,
Karolina

> 
>>
>>>
>>>> -            igt_blitter_fast_copy__raw(dst_fb->fd,
>>>> -                           ahnd, ctx, NULL,
>>>> -                           src_fb->gem_handle,
>>>> -                           src_fb->offsets[i],
>>>> -                           src_fb->strides[i],
>>>> -                           src_tiling,
>>>> -                           0, 0, /* src_x, src_y */
>>>> -                           src_fb->size,
>>>> -                           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 */,
>>>> -                           dst_fb->size);
>>>> +            memset(&blt, 0, sizeof(blt));
>>>> +            blt.color_depth = blt_get_bpp(src_fb);
>>>> +
>>>> +            src = blt_fb_init(src_fb, i, mem_region);
>>>> +            dst = blt_fb_init(dst_fb, i, mem_region);
>>>> +
>>>> +            blt_set_copy_object(&blt.src, src);
>>>> +            blt_set_copy_object(&blt.dst, dst);
>>>> +
>>>> +            blt_set_batch(&blt.bb, bb, bb_size, mem_region);
>>>> +
>>>> +            blt_fast_copy(src_fb->fd, ictx, e, ahnd, &blt);
>>>> +            gem_sync(src_fb->fd, blt.dst.handle);
>>>> +
>>>> +            blt_destroy_object(src_fb->fd, src);
>>>> +            blt_destroy_object(dst_fb->fd, dst);
>>>> +        } else if (ahnd) {
>>>
>>> I would also check if this else supports block-copy.
>>>
>>> If you prefer newer instructions (like fast-copy) with i915_blt it
>>> requires valid ahnd (>0). Instead of using wrapper:
>>>
>>> ahnd = get_reloc_ahnd(fd, ctx);
>>>
>>> you may enforce softpinning by:
>>>
>>> ahnd = intel_allocator_open(fd, ctx, INTEL_ALLOCATOR_RELOC);
>>>
>>> Then i915_blt functions will work as you expect. But this will change
>>> previous logic to prefer older blits instead new ones.
>>>
> 
> I now put above "ahnd && blt_has_block_copy(src_fb->fd)". I'll need to 
> check test runtimes what will come from different boxes to see if 
> there's something more needed to do with these, as long as runtimes wont 
> start to grow I'll be ok either way of getting ahnd.
> 
> /Juha-Pekka
> 
>>>
>>>> +            /*
>>>> +            * On GEN12+ X-tiled format support is removed from
>>>> +            * the fast blit command, so use the block copy blit
>>>> +            * command for it instead.
>>>> +            */
>>>> +            src = blt_fb_init(src_fb, i, mem_region);
>>>> +            dst = blt_fb_init(dst_fb, i, mem_region);
>>>> +
>>>> +            memset(&blt, 0, sizeof(blt));
>>>> +            blt.print_bb = true;
>>>> +            blt.color_depth = blt_get_bpp(src_fb);
>>>> +            blt_set_copy_object(&blt.src, src);
>>>> +            blt_set_copy_object(&blt.dst, dst);
>>>> +
>>>> +            if (HAS_FLATCCS(intel_get_drm_devid(src_fb->fd))) {
>>>> +                blt_set_object_ext(&ext.src,
>>>> +                           blt_compression_format(&blt, src_fb),
>>>> +                           src_fb->width, src_fb->height,
>>>> +                           SURFACE_TYPE_2D);
>>>> +
>>>> +                blt_set_object_ext(&ext.dst,
>>>> +                           blt_compression_format(&blt, dst_fb),
>>>> +                           dst_fb->width, dst_fb->height,
>>>> +                           SURFACE_TYPE_2D);
>>>> +
>>>> +                pext = &ext;
>>>> +            }
>>>> +
>>>> +            blt_set_batch(&blt.bb, bb, bb_size, mem_region);
>>>> +
>>>> +            blt_block_copy(src_fb->fd, ictx, e, ahnd, &blt, pext);
>>>> +            gem_sync(src_fb->fd, blt.dst.handle);
>>>> +
>>>> +            blt_destroy_object(src_fb->fd, src);
>>>> +            blt_destroy_object(dst_fb->fd, dst);
>>>>           } else {
>>>> +            /*
>>>> +             * If on legacy hardware where relocations are supported
>>>> +             * we'll use XY_SRC blit command instead
>>>> +             */
>>>>               igt_blitter_src_copy(dst_fb->fd,
>>>>                            ahnd, ctx, NULL,
>>>>                            src_fb->gem_handle,
>>>> @@ -2750,6 +2919,7 @@ static void blitcopy(const struct igt_fb *dst_fb,
>>>>       if (ctx)
>>>>           gem_context_destroy(dst_fb->fd, ctx);
>>>>       put_ahnd(ahnd);
>>>> +    intel_ctx_destroy(src_fb->fd, ictx);
>>>>   }
>>>>   static void free_linear_mapping(struct fb_blit_upload *blit)
>>>> -- 
>>>> 2.39.0
>>>>
> 

Re: [igt-dev] [PATCH i-g-t 2/2] lib/igt_fb: switch blitcopy to use lib/i915/i915_blt functions

[Zbigniew Kempczyński]

On Tue, Mar 28, 2023 at 08:29:40PM +0300, Juha-Pekka Heikkila wrote:
> Hei,
> 
> thanks for the comments. This was my first attempt on this and I got those
> failures which Zbigniew's comment explained how those happen. I'll soon send
> new version.. I was also wondering about insertion of that offset in the
> other patch, it somehow feels bit out of place but didn't find better place
> for it.

I'm sorry, I forgot comment 'offset' field.

There's no problem from adding it, I wondered to add those at the end
of the structure with detailed description. And 'offset' is a little bit
confusing. Maybe 'plane_offset' would be better?


> 
> couple of comments below.
> 
> On 28.3.2023 17.10, Karolina Stolarek wrote:
> > On 28.03.2023 08:50, Zbigniew Kempczyński wrote:
> > > On Mon, Mar 27, 2023 at 11:12:54PM +0300, Juha-Pekka Heikkila wrote:
> > > > reduce code duplication
> > > > 
> > > > Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> > > > ---
> > > >   lib/igt_fb.c | 214 +++++++++++++++++++++++++++++++++++++++++++++------
> > > >   1 file changed, 192 insertions(+), 22 deletions(-)
> > > > 
> > > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > > index ba89e1f60..06ebe4818 100644
> > > > --- a/lib/igt_fb.c
> > > > +++ b/lib/igt_fb.c
> > > > @@ -35,6 +35,8 @@
> > > >   #include "drmtest.h"
> > > >   #include "i915/gem_create.h"
> > > >   #include "i915/gem_mman.h"
> > > > +#include "i915/i915_blt.h"
> > > > +#include "i915/intel_mocs.h"
> > > >   #include "igt_aux.h"
> > > >   #include "igt_color_encoding.h"
> > > >   #include "igt_fb.h"
> > > > @@ -2680,12 +2682,134 @@ static void copy_with_engine(struct
> > > > fb_blit_upload *blit,
> > > >       fini_buf(src);
> > > >   }
> > > > +static struct blt_copy_object *blt_fb_init(const struct igt_fb *fb,
> > > > +                       uint32_t plane, uint32_t memregion)
> > > > +{
> > > > +    uint32_t name, handle;
> > > > +    struct blt_copy_object *blt;
> > > > +    enum blt_tiling_type blt_tile;
> > > > +    uint64_t stride;
> > > > +
> > > > +    blt = malloc(sizeof(*blt));
> > > > +    igt_assert(blt);
> > > > +
> > > > +    name = gem_flink(fb->fd, fb->gem_handle);
> > > > +    handle = gem_open(fb->fd, name);
> > > > +
> > > > +    switch (igt_fb_mod_to_tiling(fb->modifier)) {
> > > > +    case I915_TILING_X:
> > > > +        blt_tile = T_XMAJOR;
> > > 
> > > I wonder to add helper in i915_blt: i915_tiling_to_blt_tile(int tile);
> > 
> > I wish we could move on from using I915_TILING_* for tiling formats that
> > don't require fences. It would be great if we could switch on
> > blt_tiling_type when doing blit copy, especially when we have checks
> > that ensure the correct mapping.
> > 
> > I mean, we could add such a helper, but do we want to keep it as a
> > permanent solution?
> > 
> > > 
> > > BTW Karolina added asserts which should map 1:1 i915 -> blt. Tile4 is
> > > not added there yet but it can be, so i915_tiline_to_blt_tile() can
> > > simply return tile;
> > > 
> > > then those few cases could looks like:
> > > 
> > > case I915_TILING_X:
> > > case I915_TILING_Y:
> > > case I915_TILING_4:
> > >     blt_tile = i915_tiling_to_blt_tile(...);
> > >     stride = fb->strides[plane] / 4;
> > >     break;
> > > case I915_TILING_NONE:
> > >     ...
> 
> i915_tiling_to_blt_tile is coming in some yet unmerged set? I didn't find it
> in my igt.

No, I suggested you could add this. Assumption is we want 1:1 mapping
between I915_TILING_<fmt> and T_<fmt> so simple return int to enum value
is enough.

> 
> > > 
> > > > +        stride = fb->strides[plane] / 4;
> > > > +        break;
> > > > +    case I915_TILING_Y:
> > > > +        blt_tile = T_YMAJOR;
> > > > +        stride = fb->strides[plane] / 4;
> > > > +        break;
> > > > +    case I915_TILING_4:
> > > > +        blt_tile = T_TILE4;
> > > > +        stride = fb->strides[plane] / 4;
> > > > +        break;
> > > > +    case I915_TILING_NONE:
> > > > +    default:
> > > > +        blt_tile = T_LINEAR;
> > > > +        stride = fb->strides[plane];
> > > > +        break;
> > > > +    }
> > > > +
> > > > +    blt_set_object(blt, handle, fb->size, memregion,
> > > > +               intel_get_uc_mocs(fb->fd),
> > > > +               blt_tile,
> > > > +               is_ccs_modifier(fb->modifier) ?
> > > > COMPRESSION_ENABLED : COMPRESSION_DISABLED,
> > > > +               is_gen12_mc_ccs_modifier(fb->modifier) ?
> > > > COMPRESSION_TYPE_MEDIA : COMPRESSION_TYPE_3D);
> > > > +
> > > > +    blt_set_geom(blt, stride, 0, 0, fb->width,
> > > > fb->plane_height[plane], 0, 0);
> > > > +
> > > > +    blt->offset = fb->offsets[plane];
> > > > +
> > > > +    blt->ptr = gem_mmap__device_coherent(fb->fd, handle, 0, fb->size,
> > > > +                         PROT_READ | PROT_WRITE);
> > > > +    return blt;
> > > > +}
> > > > +
> > > > +static enum blt_color_depth blt_get_bpp(const struct igt_fb *fb)
> > > > +{
> > > > +    switch (fb->plane_bpp[0]) {
> > > > +    case 8:
> > > > +        return CD_8bit;
> > > > +    case 16:
> > > > +        return CD_16bit;
> > > > +    case 32:
> > > > +        return CD_32bit;
> > > > +    case 64:
> > > > +        return CD_64bit;
> > > > +    case 96:
> > > > +        return CD_96bit;
> > > > +    case 128:
> > > > +        return CD_128bit;
> > > > +    default:
> > > > +        igt_assert(0);
> > > > +    }
> > > > +}
> > > > +
> > > > +#define BLT_TARGET_RC(x) (x.compression == COMPRESSION_ENABLED && \
> > > > +              x.compression_type == COMPRESSION_TYPE_3D)
> > > > +
> > > > +#define BLT_TARGET_MC(x) (x.compression == COMPRESSION_ENABLED && \
> > > > +              x.compression_type == COMPRESSION_TYPE_MEDIA)
> > > > +
> > > > +static uint32_t blt_compression_format(struct blt_copy_data *blt,
> > > > +                       const struct igt_fb *fb)
> > > > +{
> > > > +    if (blt->src.compression == COMPRESSION_DISABLED &&
> > > > +        blt->dst.compression == COMPRESSION_DISABLED)
> > > > +        return 0;
> > > > +
> > > > +    if (BLT_TARGET_RC(blt->src) || BLT_TARGET_RC(blt->dst)) {
> > > > +        switch (blt->color_depth)
> > > > +        {
> > > > +        case CD_32bit:
> > > > +            return 8;
> > > > +        default:
> > > > +            igt_assert_f(0, "COMPRESSION_TYPE_3D unknown color
> > > > depth\n");
> > > > +        }
> > > > +    }
> > > > +
> > > > +    if (BLT_TARGET_MC(blt->src) || BLT_TARGET_MC(blt->dst)) {
> > > > +        switch (fb->drm_format)
> > > > +        {
> > > > +        case DRM_FORMAT_XRGB8888:
> > > > +            return 8;
> > > > +        case DRM_FORMAT_XYUV8888:
> > > > +            return 9;
> > > > +        case DRM_FORMAT_NV12:
> > > > +            return 9;
> > > > +        case DRM_FORMAT_P010:
> > > > +        case DRM_FORMAT_P012:
> > > > +        case DRM_FORMAT_P016:
> > > > +            return 8;
> > > > +        default:
> > > > +            igt_assert_f(0, "COMPRESSION_TYPE_MEDIA unknown format\n");
> > > > +        }
> > > > +    } else {
> > > > +        igt_assert_f(0, "unknown compression\n");
> > > > +    }
> > > > +}
> > > > +
> > > >   static void blitcopy(const struct igt_fb *dst_fb,
> > > >                const struct igt_fb *src_fb)
> > > >   {
> > > >       uint32_t src_tiling, dst_tiling;
> > > >       uint32_t ctx = 0;
> > > >       uint64_t ahnd = 0;
> > > > +    const intel_ctx_t *ictx =
> > > > intel_ctx_create_all_physical(src_fb->fd);
> > > > +    struct intel_execution_engine2 *e;
> > > > +    uint32_t bb;
> > > > +    uint64_t bb_size = 4096;
> > > > +    struct blt_copy_data blt = {};
> > > > +    struct blt_copy_object *src, *dst;
> > > > +    struct blt_block_copy_data_ext ext = {}, *pext = NULL;
> > > > +    uint32_t mem_region = HAS_FLATCCS(intel_get_drm_devid(src_fb->fd))
> > > > +               ? REGION_LMEM(0) : REGION_SMEM;
> > > >       igt_assert_eq(dst_fb->fd, src_fb->fd);
> > > >       igt_assert_eq(dst_fb->num_planes, src_fb->num_planes);
> > > > @@ -2697,36 +2821,81 @@ static void blitcopy(const struct igt_fb
> > > > *dst_fb,
> > > >           igt_require(gem_has_contexts(dst_fb->fd));
> > > >           ctx = gem_context_create(dst_fb->fd);
> > > >           ahnd = get_reloc_ahnd(dst_fb->fd, ctx);
> > > > +
> > > > +        igt_assert(__gem_create_in_memory_regions(src_fb->fd,
> > > > +                              &bb,
> > > > +                              &bb_size,
> > > > +                              mem_region) == 0);
> > > > +
> > > > +        for_each_ctx_engine(src_fb->fd, ictx, e) {
> > > > +            if (gem_engine_can_block_copy(src_fb->fd, e))
> > > > +                break;
> > > > +        }
> > > >       }
> > > >       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]);
> > > > -        /*
> > > > -         * On GEN12+ X-tiled format support is removed from the fast
> > > > -         * blit command, so use the XY_SRC blit command for it
> > > > -         * instead.
> > > > -         */
> > > > +
> > > >           if (fast_blit_ok(src_fb) && fast_blit_ok(dst_fb)) {
> > > 
> > > I think this blit fail when ahnd == 0 as i915_blt requires softpinning.
> 
> I think this is what I saw as failures in ci on tgl and some gen9 testboxes.
> Before sending I tested only with dg2 and adlp so I didn't see this problem
> earlier.

Karolina did a good job with intel_cmds_info and i915_blt helpers which
allows test if command/tiling is supported on dedicated platform.
We may start to replacing above and not fully reliable functions
(fast_blit_ok()) with i915_blt checkers.


> 
> > 
> > Also, are we fine with the fact that we'll only cover gen 8 and above
> > (due to the softpinning requirement)?
> > 
> > > 
> > > 'if (ahnd && fast_blit_ok(src_fb) && fast_blit_ok(dst_fb))' should work.
> > 
> > I had a quick look at the function, and I wonder if we could i915_blt
> > helpers here instead. There seem to be other factors that we have to
> > consider here, but the display version checks seem similar to what we
> > have in intel_cmd_info lib.
> > 
> 
> fast_blit_ok(..) checks also fb for x-tiling so I think I'll stay with that
> for now.

I suggest replace fast_blit_ok() body and use blt_has_fast_copy() and
blt_block_copy_supports_tiling() but I won't insist.

> 
> > 
> > > 
> > > > -            igt_blitter_fast_copy__raw(dst_fb->fd,
> > > > -                           ahnd, ctx, NULL,
> > > > -                           src_fb->gem_handle,
> > > > -                           src_fb->offsets[i],
> > > > -                           src_fb->strides[i],
> > > > -                           src_tiling,
> > > > -                           0, 0, /* src_x, src_y */
> > > > -                           src_fb->size,
> > > > -                           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 */,
> > > > -                           dst_fb->size);
> > > > +            memset(&blt, 0, sizeof(blt));
> > > > +            blt.color_depth = blt_get_bpp(src_fb);
> > > > +
> > > > +            src = blt_fb_init(src_fb, i, mem_region);
> > > > +            dst = blt_fb_init(dst_fb, i, mem_region);
> > > > +
> > > > +            blt_set_copy_object(&blt.src, src);
> > > > +            blt_set_copy_object(&blt.dst, dst);
> > > > +
> > > > +            blt_set_batch(&blt.bb, bb, bb_size, mem_region);
> > > > +
> > > > +            blt_fast_copy(src_fb->fd, ictx, e, ahnd, &blt);
> > > > +            gem_sync(src_fb->fd, blt.dst.handle);
> > > > +
> > > > +            blt_destroy_object(src_fb->fd, src);
> > > > +            blt_destroy_object(dst_fb->fd, dst);
> > > > +        } else if (ahnd) {
> > > 
> > > I would also check if this else supports block-copy.
> > > 
> > > If you prefer newer instructions (like fast-copy) with i915_blt it
> > > requires valid ahnd (>0). Instead of using wrapper:
> > > 
> > > ahnd = get_reloc_ahnd(fd, ctx);
> > > 
> > > you may enforce softpinning by:
> > > 
> > > ahnd = intel_allocator_open(fd, ctx, INTEL_ALLOCATOR_RELOC);
> > > 
> > > Then i915_blt functions will work as you expect. But this will change
> > > previous logic to prefer older blits instead new ones.
> > > 
> 
> I now put above "ahnd && blt_has_block_copy(src_fb->fd)". I'll need to check
> test runtimes what will come from different boxes to see if there's
> something more needed to do with these, as long as runtimes wont start to
> grow I'll be ok either way of getting ahnd.

Ok, looks good to me (I would also check tilings supported by the command,
but this is not a must).

--
Zbigniew

> 
> /Juha-Pekka
> 
> > > 
> > > > +            /*
> > > > +            * On GEN12+ X-tiled format support is removed from
> > > > +            * the fast blit command, so use the block copy blit
> > > > +            * command for it instead.
> > > > +            */
> > > > +            src = blt_fb_init(src_fb, i, mem_region);
> > > > +            dst = blt_fb_init(dst_fb, i, mem_region);
> > > > +
> > > > +            memset(&blt, 0, sizeof(blt));
> > > > +            blt.print_bb = true;
> > > > +            blt.color_depth = blt_get_bpp(src_fb);
> > > > +            blt_set_copy_object(&blt.src, src);
> > > > +            blt_set_copy_object(&blt.dst, dst);
> > > > +
> > > > +            if (HAS_FLATCCS(intel_get_drm_devid(src_fb->fd))) {
> > > > +                blt_set_object_ext(&ext.src,
> > > > +                           blt_compression_format(&blt, src_fb),
> > > > +                           src_fb->width, src_fb->height,
> > > > +                           SURFACE_TYPE_2D);
> > > > +
> > > > +                blt_set_object_ext(&ext.dst,
> > > > +                           blt_compression_format(&blt, dst_fb),
> > > > +                           dst_fb->width, dst_fb->height,
> > > > +                           SURFACE_TYPE_2D);
> > > > +
> > > > +                pext = &ext;
> > > > +            }
> > > > +
> > > > +            blt_set_batch(&blt.bb, bb, bb_size, mem_region);
> > > > +
> > > > +            blt_block_copy(src_fb->fd, ictx, e, ahnd, &blt, pext);
> > > > +            gem_sync(src_fb->fd, blt.dst.handle);
> > > > +
> > > > +            blt_destroy_object(src_fb->fd, src);
> > > > +            blt_destroy_object(dst_fb->fd, dst);
> > > >           } else {
> > > > +            /*
> > > > +             * If on legacy hardware where relocations are supported
> > > > +             * we'll use XY_SRC blit command instead
> > > > +             */
> > > >               igt_blitter_src_copy(dst_fb->fd,
> > > >                            ahnd, ctx, NULL,
> > > >                            src_fb->gem_handle,
> > > > @@ -2750,6 +2919,7 @@ static void blitcopy(const struct igt_fb *dst_fb,
> > > >       if (ctx)
> > > >           gem_context_destroy(dst_fb->fd, ctx);
> > > >       put_ahnd(ahnd);
> > > > +    intel_ctx_destroy(src_fb->fd, ictx);
> > > >   }
> > > >   static void free_linear_mapping(struct fb_blit_upload *blit)
> > > > -- 
> > > > 2.39.0
> > > > 
> 

Re: [igt-dev] [PATCH i-g-t 2/2] lib/igt_fb: switch blitcopy to use lib/i915/i915_blt functions

[Zbigniew Kempczyński]

On Tue, Mar 28, 2023 at 04:10:12PM +0200, Karolina Stolarek wrote:
> On 28.03.2023 08:50, Zbigniew Kempczyński wrote:
> > On Mon, Mar 27, 2023 at 11:12:54PM +0300, Juha-Pekka Heikkila wrote:
> > > reduce code duplication
> > > 
> > > Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> > > ---
> > >   lib/igt_fb.c | 214 +++++++++++++++++++++++++++++++++++++++++++++------
> > >   1 file changed, 192 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > index ba89e1f60..06ebe4818 100644
> > > --- a/lib/igt_fb.c
> > > +++ b/lib/igt_fb.c
> > > @@ -35,6 +35,8 @@
> > >   #include "drmtest.h"
> > >   #include "i915/gem_create.h"
> > >   #include "i915/gem_mman.h"
> > > +#include "i915/i915_blt.h"
> > > +#include "i915/intel_mocs.h"
> > >   #include "igt_aux.h"
> > >   #include "igt_color_encoding.h"
> > >   #include "igt_fb.h"
> > > @@ -2680,12 +2682,134 @@ static void copy_with_engine(struct fb_blit_upload *blit,
> > >   	fini_buf(src);
> > >   }
> > > +static struct blt_copy_object *blt_fb_init(const struct igt_fb *fb,
> > > +					   uint32_t plane, uint32_t memregion)
> > > +{
> > > +	uint32_t name, handle;
> > > +	struct blt_copy_object *blt;
> > > +	enum blt_tiling_type blt_tile;
> > > +	uint64_t stride;
> > > +
> > > +	blt = malloc(sizeof(*blt));
> > > +	igt_assert(blt);
> > > +
> > > +	name = gem_flink(fb->fd, fb->gem_handle);
> > > +	handle = gem_open(fb->fd, name);
> > > +
> > > +	switch (igt_fb_mod_to_tiling(fb->modifier)) {
> > > +	case I915_TILING_X:
> > > +		blt_tile = T_XMAJOR;
> > 
> > I wonder to add helper in i915_blt: i915_tiling_to_blt_tile(int tile);
> 
> I wish we could move on from using I915_TILING_* for tiling formats that
> don't require fences. It would be great if we could switch on
> blt_tiling_type when doing blit copy, especially when we have checks that
> ensure the correct mapping.
> 
> I mean, we could add such a helper, but do we want to keep it as a permanent
> solution?
> 

I assume we will keep I915_TILING_<fmt> == T_<fmt>. I don't know what
the future will bring, but I assume for each new format we will have
both I915_TILING_<newfmt> and T_<newfmt>.

As a legacy code still uses I915_TILING_<fmt> I would add converter to
enum to make developers happy to not to convert this manually (even if
this is same value).

> > 
> > BTW Karolina added asserts which should map 1:1 i915 -> blt. Tile4 is
> > not added there yet but it can be, so i915_tiline_to_blt_tile() can
> > simply return tile;
> > 
> > then those few cases could looks like:
> > 
> > case I915_TILING_X:
> > case I915_TILING_Y:
> > case I915_TILING_4:
> > 	blt_tile = i915_tiling_to_blt_tile(...);
> > 	stride = fb->strides[plane] / 4;
> > 	break;
> > case I915_TILING_NONE:
> > 	...
> > 
> > > +		stride = fb->strides[plane] / 4;
> > > +		break;
> > > +	case I915_TILING_Y:
> > > +		blt_tile = T_YMAJOR;
> > > +		stride = fb->strides[plane] / 4;
> > > +		break;
> > > +	case I915_TILING_4:
> > > +		blt_tile = T_TILE4;
> > > +		stride = fb->strides[plane] / 4;
> > > +		break;
> > > +	case I915_TILING_NONE:
> > > +	default:
> > > +		blt_tile = T_LINEAR;
> > > +		stride = fb->strides[plane];
> > > +		break;
> > > +	}
> > > +
> > > +	blt_set_object(blt, handle, fb->size, memregion,
> > > +		       intel_get_uc_mocs(fb->fd),
> > > +		       blt_tile,
> > > +		       is_ccs_modifier(fb->modifier) ? COMPRESSION_ENABLED : COMPRESSION_DISABLED,
> > > +		       is_gen12_mc_ccs_modifier(fb->modifier) ? COMPRESSION_TYPE_MEDIA : COMPRESSION_TYPE_3D);
> > > +
> > > +	blt_set_geom(blt, stride, 0, 0, fb->width, fb->plane_height[plane], 0, 0);
> > > +
> > > +	blt->offset = fb->offsets[plane];
> > > +
> > > +	blt->ptr = gem_mmap__device_coherent(fb->fd, handle, 0, fb->size,
> > > +					     PROT_READ | PROT_WRITE);
> > > +	return blt;
> > > +}
> > > +
> > > +static enum blt_color_depth blt_get_bpp(const struct igt_fb *fb)
> > > +{
> > > +	switch (fb->plane_bpp[0]) {
> > > +	case 8:
> > > +		return CD_8bit;
> > > +	case 16:
> > > +		return CD_16bit;
> > > +	case 32:
> > > +		return CD_32bit;
> > > +	case 64:
> > > +		return CD_64bit;
> > > +	case 96:
> > > +		return CD_96bit;
> > > +	case 128:
> > > +		return CD_128bit;
> > > +	default:
> > > +		igt_assert(0);
> > > +	}
> > > +}
> > > +
> > > +#define BLT_TARGET_RC(x) (x.compression == COMPRESSION_ENABLED && \
> > > +			  x.compression_type == COMPRESSION_TYPE_3D)
> > > +
> > > +#define BLT_TARGET_MC(x) (x.compression == COMPRESSION_ENABLED && \
> > > +			  x.compression_type == COMPRESSION_TYPE_MEDIA)
> > > +
> > > +static uint32_t blt_compression_format(struct blt_copy_data *blt,
> > > +				       const struct igt_fb *fb)
> > > +{
> > > +	if (blt->src.compression == COMPRESSION_DISABLED &&
> > > +	    blt->dst.compression == COMPRESSION_DISABLED)
> > > +		return 0;
> > > +
> > > +	if (BLT_TARGET_RC(blt->src) || BLT_TARGET_RC(blt->dst)) {
> > > +		switch (blt->color_depth)
> > > +		{
> > > +		case CD_32bit:
> > > +			return 8;
> > > +		default:
> > > +			igt_assert_f(0, "COMPRESSION_TYPE_3D unknown color depth\n");
> > > +		}
> > > +	}
> > > +
> > > +	if (BLT_TARGET_MC(blt->src) || BLT_TARGET_MC(blt->dst)) {
> > > +		switch (fb->drm_format)
> > > +		{
> > > +		case DRM_FORMAT_XRGB8888:
> > > +			return 8;
> > > +		case DRM_FORMAT_XYUV8888:
> > > +			return 9;
> > > +		case DRM_FORMAT_NV12:
> > > +			return 9;
> > > +		case DRM_FORMAT_P010:
> > > +		case DRM_FORMAT_P012:
> > > +		case DRM_FORMAT_P016:
> > > +			return 8;
> > > +		default:
> > > +			igt_assert_f(0, "COMPRESSION_TYPE_MEDIA unknown format\n");
> > > +		}
> > > +	} else {
> > > +		igt_assert_f(0, "unknown compression\n");
> > > +	}
> > > +}
> > > +
> > >   static void blitcopy(const struct igt_fb *dst_fb,
> > >   		     const struct igt_fb *src_fb)
> > >   {
> > >   	uint32_t src_tiling, dst_tiling;
> > >   	uint32_t ctx = 0;
> > >   	uint64_t ahnd = 0;
> > > +	const intel_ctx_t *ictx = intel_ctx_create_all_physical(src_fb->fd);
> > > +	struct intel_execution_engine2 *e;
> > > +	uint32_t bb;
> > > +	uint64_t bb_size = 4096;
> > > +	struct blt_copy_data blt = {};
> > > +	struct blt_copy_object *src, *dst;
> > > +	struct blt_block_copy_data_ext ext = {}, *pext = NULL;
> > > +	uint32_t mem_region = HAS_FLATCCS(intel_get_drm_devid(src_fb->fd))
> > > +			   ? REGION_LMEM(0) : REGION_SMEM;
> > >   	igt_assert_eq(dst_fb->fd, src_fb->fd);
> > >   	igt_assert_eq(dst_fb->num_planes, src_fb->num_planes);
> > > @@ -2697,36 +2821,81 @@ static void blitcopy(const struct igt_fb *dst_fb,
> > >   		igt_require(gem_has_contexts(dst_fb->fd));
> > >   		ctx = gem_context_create(dst_fb->fd);
> > >   		ahnd = get_reloc_ahnd(dst_fb->fd, ctx);
> > > +
> > > +		igt_assert(__gem_create_in_memory_regions(src_fb->fd,
> > > +							  &bb,
> > > +							  &bb_size,
> > > +							  mem_region) == 0);
> > > +
> > > +		for_each_ctx_engine(src_fb->fd, ictx, e) {
> > > +			if (gem_engine_can_block_copy(src_fb->fd, e))
> > > +				break;
> > > +		}
> > >   	}
> > >   	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]);
> > > -		/*
> > > -		 * On GEN12+ X-tiled format support is removed from the fast
> > > -		 * blit command, so use the XY_SRC blit command for it
> > > -		 * instead.
> > > -		 */
> > > +
> > >   		if (fast_blit_ok(src_fb) && fast_blit_ok(dst_fb)) {
> > 
> > I think this blit fail when ahnd == 0 as i915_blt requires softpinning.
> 
> Also, are we fine with the fact that we'll only cover gen 8 and above (due
> to the softpinning requirement)?

Hmm, instead of fast_blit_ok() we should use here blt_has_fast_copy() and
blt_fast_copy_supports_tiling(). Gen8 won't be covered here as fast copy
doesn't exists there. BTW it looks a little bit weird .cmds_info on gen9
pointing to gen11_cmds_info. I started to look on _cmds_info first to find
out is fast_copy supported on gen8 and gen9 and I thought it is not.
Consider adding separated entry for gen9, even at the cost of additional
cmds_info variable.

> 
> > 
> > 'if (ahnd && fast_blit_ok(src_fb) && fast_blit_ok(dst_fb))' should work.
> 
> I had a quick look at the function, and I wonder if we could i915_blt
> helpers here instead. There seem to be other factors that we have to
> consider here, but the display version checks seem similar to what we have
> in intel_cmd_info lib.

Yes, i915_blt helpers are more reliable for platforms.

--
Zbigniew

> 
> 
> All the best,
> Karolina
> 
> > 
> > > -			igt_blitter_fast_copy__raw(dst_fb->fd,
> > > -						   ahnd, ctx, NULL,
> > > -						   src_fb->gem_handle,
> > > -						   src_fb->offsets[i],
> > > -						   src_fb->strides[i],
> > > -						   src_tiling,
> > > -						   0, 0, /* src_x, src_y */
> > > -						   src_fb->size,
> > > -						   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 */,
> > > -						   dst_fb->size);
> > > +			memset(&blt, 0, sizeof(blt));
> > > +			blt.color_depth = blt_get_bpp(src_fb);
> > > +
> > > +			src = blt_fb_init(src_fb, i, mem_region);
> > > +			dst = blt_fb_init(dst_fb, i, mem_region);
> > > +
> > > +			blt_set_copy_object(&blt.src, src);
> > > +			blt_set_copy_object(&blt.dst, dst);
> > > +
> > > +			blt_set_batch(&blt.bb, bb, bb_size, mem_region);
> > > +
> > > +			blt_fast_copy(src_fb->fd, ictx, e, ahnd, &blt);
> > > +			gem_sync(src_fb->fd, blt.dst.handle);
> > > +
> > > +			blt_destroy_object(src_fb->fd, src);
> > > +			blt_destroy_object(dst_fb->fd, dst);
> > > +		} else if (ahnd) {
> > 
> > I would also check if this else supports block-copy.
> > 
> > If you prefer newer instructions (like fast-copy) with i915_blt it
> > requires valid ahnd (>0). Instead of using wrapper:
> > 
> > ahnd = get_reloc_ahnd(fd, ctx);
> > 
> > you may enforce softpinning by:
> > 
> > ahnd = intel_allocator_open(fd, ctx, INTEL_ALLOCATOR_RELOC);
> > 
> > Then i915_blt functions will work as you expect. But this will change
> > previous logic to prefer older blits instead new ones.
> > 
> > --
> > Zbigniew
> > 
> > > +			/*
> > > +			* On GEN12+ X-tiled format support is removed from
> > > +			* the fast blit command, so use the block copy blit
> > > +			* command for it instead.
> > > +			*/
> > > +			src = blt_fb_init(src_fb, i, mem_region);
> > > +			dst = blt_fb_init(dst_fb, i, mem_region);
> > > +
> > > +			memset(&blt, 0, sizeof(blt));
> > > +			blt.print_bb = true;
> > > +			blt.color_depth = blt_get_bpp(src_fb);
> > > +			blt_set_copy_object(&blt.src, src);
> > > +			blt_set_copy_object(&blt.dst, dst);
> > > +
> > > +			if (HAS_FLATCCS(intel_get_drm_devid(src_fb->fd))) {
> > > +				blt_set_object_ext(&ext.src,
> > > +						   blt_compression_format(&blt, src_fb),
> > > +						   src_fb->width, src_fb->height,
> > > +						   SURFACE_TYPE_2D);
> > > +
> > > +				blt_set_object_ext(&ext.dst,
> > > +						   blt_compression_format(&blt, dst_fb),
> > > +						   dst_fb->width, dst_fb->height,
> > > +						   SURFACE_TYPE_2D);
> > > +
> > > +				pext = &ext;
> > > +			}
> > > +
> > > +			blt_set_batch(&blt.bb, bb, bb_size, mem_region);
> > > +
> > > +			blt_block_copy(src_fb->fd, ictx, e, ahnd, &blt, pext);
> > > +			gem_sync(src_fb->fd, blt.dst.handle);
> > > +
> > > +			blt_destroy_object(src_fb->fd, src);
> > > +			blt_destroy_object(dst_fb->fd, dst);
> > >   		} else {
> > > +			/*
> > > +			 * If on legacy hardware where relocations are supported
> > > +			 * we'll use XY_SRC blit command instead
> > > +			 */
> > >   			igt_blitter_src_copy(dst_fb->fd,
> > >   					     ahnd, ctx, NULL,
> > >   					     src_fb->gem_handle,
> > > @@ -2750,6 +2919,7 @@ static void blitcopy(const struct igt_fb *dst_fb,
> > >   	if (ctx)
> > >   		gem_context_destroy(dst_fb->fd, ctx);
> > >   	put_ahnd(ahnd);
> > > +	intel_ctx_destroy(src_fb->fd, ictx);
> > >   }
> > >   static void free_linear_mapping(struct fb_blit_upload *blit)
> > > -- 
> > > 2.39.0
> > > 

Re: [igt-dev] [PATCH i-g-t 2/2] lib/igt_fb: switch blitcopy to use lib/i915/i915_blt functions

[Karolina Stolarek]

On 29.03.2023 19:19, Zbigniew Kempczyński wrote:
> On Tue, Mar 28, 2023 at 04:10:12PM +0200, Karolina Stolarek wrote:
>> On 28.03.2023 08:50, Zbigniew Kempczyński wrote:
>>> On Mon, Mar 27, 2023 at 11:12:54PM +0300, Juha-Pekka Heikkila wrote:
>>>> reduce code duplication
>>>>
>>>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>>>> ---
>>>>    lib/igt_fb.c | 214 +++++++++++++++++++++++++++++++++++++++++++++------
>>>>    1 file changed, 192 insertions(+), 22 deletions(-)
>>>>
>>>> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
>>>> index ba89e1f60..06ebe4818 100644
>>>> --- a/lib/igt_fb.c
>>>> +++ b/lib/igt_fb.c
>>>> @@ -35,6 +35,8 @@
>>>>    #include "drmtest.h"
>>>>    #include "i915/gem_create.h"
>>>>    #include "i915/gem_mman.h"
>>>> +#include "i915/i915_blt.h"
>>>> +#include "i915/intel_mocs.h"
>>>>    #include "igt_aux.h"
>>>>    #include "igt_color_encoding.h"
>>>>    #include "igt_fb.h"
>>>> @@ -2680,12 +2682,134 @@ static void copy_with_engine(struct fb_blit_upload *blit,
>>>>    	fini_buf(src);
>>>>    }
>>>> +static struct blt_copy_object *blt_fb_init(const struct igt_fb *fb,
>>>> +					   uint32_t plane, uint32_t memregion)
>>>> +{
>>>> +	uint32_t name, handle;
>>>> +	struct blt_copy_object *blt;
>>>> +	enum blt_tiling_type blt_tile;
>>>> +	uint64_t stride;
>>>> +
>>>> +	blt = malloc(sizeof(*blt));
>>>> +	igt_assert(blt);
>>>> +
>>>> +	name = gem_flink(fb->fd, fb->gem_handle);
>>>> +	handle = gem_open(fb->fd, name);
>>>> +
>>>> +	switch (igt_fb_mod_to_tiling(fb->modifier)) {
>>>> +	case I915_TILING_X:
>>>> +		blt_tile = T_XMAJOR;
>>>
>>> I wonder to add helper in i915_blt: i915_tiling_to_blt_tile(int tile);
>>
>> I wish we could move on from using I915_TILING_* for tiling formats that
>> don't require fences. It would be great if we could switch on
>> blt_tiling_type when doing blit copy, especially when we have checks that
>> ensure the correct mapping.
>>
>> I mean, we could add such a helper, but do we want to keep it as a permanent
>> solution?
>>
> 
> I assume we will keep I915_TILING_<fmt> == T_<fmt>. I don't know what
> the future will bring, but I assume for each new format we will have
> both I915_TILING_<newfmt> and T_<newfmt>.
> 
> As a legacy code still uses I915_TILING_<fmt> I would add converter to
> enum to make developers happy to not to convert this manually (even if
> this is same value).
> 

Right, I see what you mean. I just pass "newer" definitions in :)
But I know that others might not know that they're equivalent, and we
should provide a function that will convert the values.

>>>
>>> BTW Karolina added asserts which should map 1:1 i915 -> blt. Tile4 is
>>> not added there yet but it can be, so i915_tiline_to_blt_tile() can
>>> simply return tile;
>>>
>>> then those few cases could looks like:
>>>
>>> case I915_TILING_X:
>>> case I915_TILING_Y:
>>> case I915_TILING_4:
>>> 	blt_tile = i915_tiling_to_blt_tile(...);
>>> 	stride = fb->strides[plane] / 4;
>>> 	break;
>>> case I915_TILING_NONE:
>>> 	...
>>>
>>>> +		stride = fb->strides[plane] / 4;
>>>> +		break;
>>>> +	case I915_TILING_Y:
>>>> +		blt_tile = T_YMAJOR;
>>>> +		stride = fb->strides[plane] / 4;
>>>> +		break;
>>>> +	case I915_TILING_4:
>>>> +		blt_tile = T_TILE4;
>>>> +		stride = fb->strides[plane] / 4;
>>>> +		break;
>>>> +	case I915_TILING_NONE:
>>>> +	default:
>>>> +		blt_tile = T_LINEAR;
>>>> +		stride = fb->strides[plane];
>>>> +		break;
>>>> +	}
>>>> +
>>>> +	blt_set_object(blt, handle, fb->size, memregion,
>>>> +		       intel_get_uc_mocs(fb->fd),
>>>> +		       blt_tile,
>>>> +		       is_ccs_modifier(fb->modifier) ? COMPRESSION_ENABLED : COMPRESSION_DISABLED,
>>>> +		       is_gen12_mc_ccs_modifier(fb->modifier) ? COMPRESSION_TYPE_MEDIA : COMPRESSION_TYPE_3D);
>>>> +
>>>> +	blt_set_geom(blt, stride, 0, 0, fb->width, fb->plane_height[plane], 0, 0);
>>>> +
>>>> +	blt->offset = fb->offsets[plane];
>>>> +
>>>> +	blt->ptr = gem_mmap__device_coherent(fb->fd, handle, 0, fb->size,
>>>> +					     PROT_READ | PROT_WRITE);
>>>> +	return blt;
>>>> +}
>>>> +
>>>> +static enum blt_color_depth blt_get_bpp(const struct igt_fb *fb)
>>>> +{
>>>> +	switch (fb->plane_bpp[0]) {
>>>> +	case 8:
>>>> +		return CD_8bit;
>>>> +	case 16:
>>>> +		return CD_16bit;
>>>> +	case 32:
>>>> +		return CD_32bit;
>>>> +	case 64:
>>>> +		return CD_64bit;
>>>> +	case 96:
>>>> +		return CD_96bit;
>>>> +	case 128:
>>>> +		return CD_128bit;
>>>> +	default:
>>>> +		igt_assert(0);
>>>> +	}
>>>> +}
>>>> +
>>>> +#define BLT_TARGET_RC(x) (x.compression == COMPRESSION_ENABLED && \
>>>> +			  x.compression_type == COMPRESSION_TYPE_3D)
>>>> +
>>>> +#define BLT_TARGET_MC(x) (x.compression == COMPRESSION_ENABLED && \
>>>> +			  x.compression_type == COMPRESSION_TYPE_MEDIA)
>>>> +
>>>> +static uint32_t blt_compression_format(struct blt_copy_data *blt,
>>>> +				       const struct igt_fb *fb)
>>>> +{
>>>> +	if (blt->src.compression == COMPRESSION_DISABLED &&
>>>> +	    blt->dst.compression == COMPRESSION_DISABLED)
>>>> +		return 0;
>>>> +
>>>> +	if (BLT_TARGET_RC(blt->src) || BLT_TARGET_RC(blt->dst)) {
>>>> +		switch (blt->color_depth)
>>>> +		{
>>>> +		case CD_32bit:
>>>> +			return 8;
>>>> +		default:
>>>> +			igt_assert_f(0, "COMPRESSION_TYPE_3D unknown color depth\n");
>>>> +		}
>>>> +	}
>>>> +
>>>> +	if (BLT_TARGET_MC(blt->src) || BLT_TARGET_MC(blt->dst)) {
>>>> +		switch (fb->drm_format)
>>>> +		{
>>>> +		case DRM_FORMAT_XRGB8888:
>>>> +			return 8;
>>>> +		case DRM_FORMAT_XYUV8888:
>>>> +			return 9;
>>>> +		case DRM_FORMAT_NV12:
>>>> +			return 9;
>>>> +		case DRM_FORMAT_P010:
>>>> +		case DRM_FORMAT_P012:
>>>> +		case DRM_FORMAT_P016:
>>>> +			return 8;
>>>> +		default:
>>>> +			igt_assert_f(0, "COMPRESSION_TYPE_MEDIA unknown format\n");
>>>> +		}
>>>> +	} else {
>>>> +		igt_assert_f(0, "unknown compression\n");
>>>> +	}
>>>> +}
>>>> +
>>>>    static void blitcopy(const struct igt_fb *dst_fb,
>>>>    		     const struct igt_fb *src_fb)
>>>>    {
>>>>    	uint32_t src_tiling, dst_tiling;
>>>>    	uint32_t ctx = 0;
>>>>    	uint64_t ahnd = 0;
>>>> +	const intel_ctx_t *ictx = intel_ctx_create_all_physical(src_fb->fd);
>>>> +	struct intel_execution_engine2 *e;
>>>> +	uint32_t bb;
>>>> +	uint64_t bb_size = 4096;
>>>> +	struct blt_copy_data blt = {};
>>>> +	struct blt_copy_object *src, *dst;
>>>> +	struct blt_block_copy_data_ext ext = {}, *pext = NULL;
>>>> +	uint32_t mem_region = HAS_FLATCCS(intel_get_drm_devid(src_fb->fd))
>>>> +			   ? REGION_LMEM(0) : REGION_SMEM;
>>>>    	igt_assert_eq(dst_fb->fd, src_fb->fd);
>>>>    	igt_assert_eq(dst_fb->num_planes, src_fb->num_planes);
>>>> @@ -2697,36 +2821,81 @@ static void blitcopy(const struct igt_fb *dst_fb,
>>>>    		igt_require(gem_has_contexts(dst_fb->fd));
>>>>    		ctx = gem_context_create(dst_fb->fd);
>>>>    		ahnd = get_reloc_ahnd(dst_fb->fd, ctx);
>>>> +
>>>> +		igt_assert(__gem_create_in_memory_regions(src_fb->fd,
>>>> +							  &bb,
>>>> +							  &bb_size,
>>>> +							  mem_region) == 0);
>>>> +
>>>> +		for_each_ctx_engine(src_fb->fd, ictx, e) {
>>>> +			if (gem_engine_can_block_copy(src_fb->fd, e))
>>>> +				break;
>>>> +		}
>>>>    	}
>>>>    	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]);
>>>> -		/*
>>>> -		 * On GEN12+ X-tiled format support is removed from the fast
>>>> -		 * blit command, so use the XY_SRC blit command for it
>>>> -		 * instead.
>>>> -		 */
>>>> +
>>>>    		if (fast_blit_ok(src_fb) && fast_blit_ok(dst_fb)) {
>>>
>>> I think this blit fail when ahnd == 0 as i915_blt requires softpinning.
>>
>> Also, are we fine with the fact that we'll only cover gen 8 and above (due
>> to the softpinning requirement)?
> 
> Hmm, instead of fast_blit_ok() we should use here blt_has_fast_copy() and
> blt_fast_copy_supports_tiling().

I'm just confused by I915_FORMAT_MOD_X_TILED check. Is it the same as 
checking for TileX? If so, then we could indeed rewrite it in such a way.

> Gen8 won't be covered here as fast copy doesn't exists there.

Right, by this comment I meant if we want to use a different blit as 
well, or are we fine with using fast_copy only.

> BTW it looks a little bit weird .cmds_info on gen9
> pointing to gen11_cmds_info. I started to look on _cmds_info first to find
> out is fast_copy supported on gen8 and gen9 and I thought it is not.

Oh my, things are complicated over there, I agree. If time permits, I 
shall re-review the naming convention used there.

> Consider adding separated entry for gen9, even at the cost of additional
> cmds_info variable.

I believe it'll duplicate what's in gen11, wouldn't it? Nevermind, I'll 
check it out and see if I can make it less confusing.


Many thanks,
Karolina
> 
>>
>>>
>>> 'if (ahnd && fast_blit_ok(src_fb) && fast_blit_ok(dst_fb))' should work.
>>
>> I had a quick look at the function, and I wonder if we could i915_blt
>> helpers here instead. There seem to be other factors that we have to
>> consider here, but the display version checks seem similar to what we have
>> in intel_cmd_info lib.
> 
> Yes, i915_blt helpers are more reliable for platforms.
> 
> --
> Zbigniew
> 
>>
>>
>> All the best,
>> Karolina
>>
>>>
>>>> -			igt_blitter_fast_copy__raw(dst_fb->fd,
>>>> -						   ahnd, ctx, NULL,
>>>> -						   src_fb->gem_handle,
>>>> -						   src_fb->offsets[i],
>>>> -						   src_fb->strides[i],
>>>> -						   src_tiling,
>>>> -						   0, 0, /* src_x, src_y */
>>>> -						   src_fb->size,
>>>> -						   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 */,
>>>> -						   dst_fb->size);
>>>> +			memset(&blt, 0, sizeof(blt));
>>>> +			blt.color_depth = blt_get_bpp(src_fb);
>>>> +
>>>> +			src = blt_fb_init(src_fb, i, mem_region);
>>>> +			dst = blt_fb_init(dst_fb, i, mem_region);
>>>> +
>>>> +			blt_set_copy_object(&blt.src, src);
>>>> +			blt_set_copy_object(&blt.dst, dst);
>>>> +
>>>> +			blt_set_batch(&blt.bb, bb, bb_size, mem_region);
>>>> +
>>>> +			blt_fast_copy(src_fb->fd, ictx, e, ahnd, &blt);
>>>> +			gem_sync(src_fb->fd, blt.dst.handle);
>>>> +
>>>> +			blt_destroy_object(src_fb->fd, src);
>>>> +			blt_destroy_object(dst_fb->fd, dst);
>>>> +		} else if (ahnd) {
>>>
>>> I would also check if this else supports block-copy.
>>>
>>> If you prefer newer instructions (like fast-copy) with i915_blt it
>>> requires valid ahnd (>0). Instead of using wrapper:
>>>
>>> ahnd = get_reloc_ahnd(fd, ctx);
>>>
>>> you may enforce softpinning by:
>>>
>>> ahnd = intel_allocator_open(fd, ctx, INTEL_ALLOCATOR_RELOC);
>>>
>>> Then i915_blt functions will work as you expect. But this will change
>>> previous logic to prefer older blits instead new ones.
>>>
>>> --
>>> Zbigniew
>>>
>>>> +			/*
>>>> +			* On GEN12+ X-tiled format support is removed from
>>>> +			* the fast blit command, so use the block copy blit
>>>> +			* command for it instead.
>>>> +			*/
>>>> +			src = blt_fb_init(src_fb, i, mem_region);
>>>> +			dst = blt_fb_init(dst_fb, i, mem_region);
>>>> +
>>>> +			memset(&blt, 0, sizeof(blt));
>>>> +			blt.print_bb = true;
>>>> +			blt.color_depth = blt_get_bpp(src_fb);
>>>> +			blt_set_copy_object(&blt.src, src);
>>>> +			blt_set_copy_object(&blt.dst, dst);
>>>> +
>>>> +			if (HAS_FLATCCS(intel_get_drm_devid(src_fb->fd))) {
>>>> +				blt_set_object_ext(&ext.src,
>>>> +						   blt_compression_format(&blt, src_fb),
>>>> +						   src_fb->width, src_fb->height,
>>>> +						   SURFACE_TYPE_2D);
>>>> +
>>>> +				blt_set_object_ext(&ext.dst,
>>>> +						   blt_compression_format(&blt, dst_fb),
>>>> +						   dst_fb->width, dst_fb->height,
>>>> +						   SURFACE_TYPE_2D);
>>>> +
>>>> +				pext = &ext;
>>>> +			}
>>>> +
>>>> +			blt_set_batch(&blt.bb, bb, bb_size, mem_region);
>>>> +
>>>> +			blt_block_copy(src_fb->fd, ictx, e, ahnd, &blt, pext);
>>>> +			gem_sync(src_fb->fd, blt.dst.handle);
>>>> +
>>>> +			blt_destroy_object(src_fb->fd, src);
>>>> +			blt_destroy_object(dst_fb->fd, dst);
>>>>    		} else {
>>>> +			/*
>>>> +			 * If on legacy hardware where relocations are supported
>>>> +			 * we'll use XY_SRC blit command instead
>>>> +			 */
>>>>    			igt_blitter_src_copy(dst_fb->fd,
>>>>    					     ahnd, ctx, NULL,
>>>>    					     src_fb->gem_handle,
>>>> @@ -2750,6 +2919,7 @@ static void blitcopy(const struct igt_fb *dst_fb,
>>>>    	if (ctx)
>>>>    		gem_context_destroy(dst_fb->fd, ctx);
>>>>    	put_ahnd(ahnd);
>>>> +	intel_ctx_destroy(src_fb->fd, ictx);
>>>>    }
>>>>    static void free_linear_mapping(struct fb_blit_upload *blit)
>>>> -- 
>>>> 2.39.0
>>>>

[igt-dev] ✗ GitLab.Pipeline: warning for switch lib/igt_fb.c to use lib/i915/i915_blt functions for blitter on Intel hw

[Patchwork]

== Series Details ==

Series: switch lib/igt_fb.c to use lib/i915/i915_blt functions for blitter on Intel hw
URL   : https://patchwork.freedesktop.org/series/115691/
State : warning

== Summary ==

Pipeline status: FAILED.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/840569 for the overview.

build:tests-debian-meson-mips has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/38841929):
  $ /host/bin/curl -s -L --cacert /host/ca-certificates.crt --retry 4 -f --retry-delay 60 https://gitlab.freedesktop.org/freedesktop/helm-gitlab-infra/-/raw/main/runner-gating/runner-gating.sh | sh -s -- pre_get_sources_script
  Checking if the user of the pipeline is allowed...
  Checking if the job's project is part of a well-known group...
  Thank you for contributing to freedesktop.org
  Fetching changes...
  Reinitialized existing Git repository in /builds/gfx-ci/igt-ci-tags/.git/
  Checking out da24ac32 as detached HEAD (ref is intel/IGTPW_8690)...
  Removing build/
  Removing lib/i915/perf-configs/__pycache__/
  
  Skipping Git submodules setup
  section_end:1679948917:get_sources
  section_start:1679948917:step_script
  Executing "step_script" stage of the job script
  Using docker image sha256:cc55efdc667be826910d414a562c76ce1130a9c15255a0dd115431bc42f83448 for registry.freedesktop.org/gfx-ci/igt-ci-tags/build-debian-mips:commit-da24ac3246920fd14c02a16591084221e278645c with digest registry.freedesktop.org/gfx-ci/igt-ci-tags/build-debian-mips@sha256:c829c44880da4782b7a72626c259ac6904b4bd5f08601e66b3be889ca1c0cf79 ...
  section_end:1679948918:step_script
  section_start:1679948918:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1679948919:cleanup_file_variables
  ERROR: Job failed (system failure): Error response from daemon: no such image: docker.io/library/sha256:cc55efdc667be826910d414a562c76ce1130a9c15255a0dd115431bc42f83448: image not known (docker.go:535:0s)

build:tests-debian-minimal has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/38841926):
  $ /host/bin/curl -s -L --cacert /host/ca-certificates.crt --retry 4 -f --retry-delay 60 https://gitlab.freedesktop.org/freedesktop/helm-gitlab-infra/-/raw/main/runner-gating/runner-gating.sh | sh -s -- pre_get_sources_script
  Checking if the user of the pipeline is allowed...
  Checking if the job's project is part of a well-known group...
  Thank you for contributing to freedesktop.org
  Fetching changes...
  Reinitialized existing Git repository in /builds/gfx-ci/igt-ci-tags/.git/
  Checking out da24ac32 as detached HEAD (ref is intel/IGTPW_8690)...
  Removing build/
  Removing lib/i915/perf-configs/__pycache__/
  
  Skipping Git submodules setup
  section_end:1679948917:get_sources
  section_start:1679948917:step_script
  Executing "step_script" stage of the job script
  Using docker image sha256:08904a47f4efcc161569a9b7f88c458fc6e3e85a2225132246af9cf5cc6c4e5b for registry.freedesktop.org/gfx-ci/igt-ci-tags/build-debian-minimal:commit-da24ac3246920fd14c02a16591084221e278645c with digest registry.freedesktop.org/gfx-ci/igt-ci-tags/build-debian-minimal@sha256:8b51a86fd81e64c501c9521c37fc8ad6f2550976931c510eb0ff4f6a328d477a ...
  section_end:1679948917:step_script
  section_start:1679948917:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1679948919:cleanup_file_variables
  ERROR: Job failed (system failure): Error response from daemon: container f7d517199d16935569d86fdb090a966a47e97f52eb5388f64c53cecb0a855e5c does not exist in database: no such container (exec.go:78:0s)

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/840569

[igt-dev] ✗ Fi.CI.BAT: failure for switch lib/igt_fb.c to use lib/i915/i915_blt functions for blitter on Intel hw

[Patchwork]

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

== Series Details ==

Series: switch lib/igt_fb.c to use lib/i915/i915_blt functions for blitter on Intel hw
URL   : https://patchwork.freedesktop.org/series/115691/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12921 -> IGTPW_8690
====================================================

Summary
-------

  **FAILURE**

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

Participating hosts (37 -> 36)
------------------------------

  Missing    (1): fi-kbl-soraka 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck@pipe-d-dp-1:
    - bat-dg2-8:          [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/bat-dg2-8/igt@kms_pipe_crc_basic@compare-crc-sanitycheck@pipe-d-dp-1.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8690/bat-dg2-8/igt@kms_pipe_crc_basic@compare-crc-sanitycheck@pipe-d-dp-1.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@requests:
    - bat-rpls-2:         [PASS][3] -> [ABORT][4] ([i915#4983] / [i915#7913])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/bat-rpls-2/igt@i915_selftest@live@requests.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8690/bat-rpls-2/igt@i915_selftest@live@requests.html

  * igt@i915_selftest@live@reset:
    - bat-rpls-1:         [PASS][5] -> [ABORT][6] ([i915#4983])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/bat-rpls-1/igt@i915_selftest@live@reset.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8690/bat-rpls-1/igt@i915_selftest@live@reset.html

  
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913


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

  * CI: CI-20190529 -> None
  * IGT: IGT_7221 -> IGTPW_8690

  CI-20190529: 20190529
  CI_DRM_12921: 3de6040ce9900a94ec626662d5c6a227b37eeb1c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_8690: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8690/index.html
  IGT_7221: 4b77c6d85024d22ca521d510f8eee574128fe04f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

== Logs ==

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

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

[igt-dev] ✓ Fi.CI.BAT: success for switch lib/igt_fb.c to use lib/i915/i915_blt functions for blitter on Intel hw (rev2)

[Patchwork]

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

== Series Details ==

Series: switch lib/igt_fb.c to use lib/i915/i915_blt functions for blitter on Intel hw (rev2)
URL   : https://patchwork.freedesktop.org/series/115691/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12923 -> IGTPW_8694
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

  No changes in participating hosts

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-cfl-8109u:       [PASS][1] -> [DMESG-FAIL][2] ([i915#5334])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/fi-cfl-8109u/igt@i915_selftest@live@gt_heartbeat.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/fi-cfl-8109u/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@slpc:
    - bat-rplp-1:         [PASS][3] -> [DMESG-FAIL][4] ([i915#6367] / [i915#7913])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/bat-rplp-1/igt@i915_selftest@live@slpc.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/bat-rplp-1/igt@i915_selftest@live@slpc.html

  * igt@kms_chamelium_hpd@common-hpd-after-suspend:
    - bat-dg2-11:         NOTRUN -> [SKIP][5] ([i915#7828])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/bat-dg2-11/igt@kms_chamelium_hpd@common-hpd-after-suspend.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck@pipe-d-dp-1:
    - bat-dg2-8:          [PASS][6] -> [FAIL][7] ([i915#7932])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/bat-dg2-8/igt@kms_pipe_crc_basic@compare-crc-sanitycheck@pipe-d-dp-1.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/bat-dg2-8/igt@kms_pipe_crc_basic@compare-crc-sanitycheck@pipe-d-dp-1.html

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

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_lrc:
    - bat-dg2-11:         [INCOMPLETE][9] ([i915#7609] / [i915#7913]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/bat-dg2-11/igt@i915_selftest@live@gt_lrc.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/bat-dg2-11/igt@i915_selftest@live@gt_lrc.html

  * igt@i915_selftest@live@guc:
    - bat-rpls-1:         [DMESG-WARN][11] ([i915#7852]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/bat-rpls-1/igt@i915_selftest@live@guc.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/bat-rpls-1/igt@i915_selftest@live@guc.html

  * igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1:
    - bat-dg2-8:          [FAIL][13] ([i915#7932]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1.html

  
#### Warnings ####

  * igt@i915_selftest@live@slpc:
    - bat-rpls-2:         [DMESG-FAIL][15] ([i915#6367] / [i915#7913] / [i915#7996]) -> [DMESG-FAIL][16] ([i915#6367] / [i915#7913])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/bat-rpls-2/igt@i915_selftest@live@slpc.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/bat-rpls-2/igt@i915_selftest@live@slpc.html

  
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#7609]: https://gitlab.freedesktop.org/drm/intel/issues/7609
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7852]: https://gitlab.freedesktop.org/drm/intel/issues/7852
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7932]: https://gitlab.freedesktop.org/drm/intel/issues/7932
  [i915#7996]: https://gitlab.freedesktop.org/drm/intel/issues/7996


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

  * CI: CI-20190529 -> None
  * IGT: IGT_7221 -> IGTPW_8694

  CI-20190529: 20190529
  CI_DRM_12923: cdd32ac83137835a85bad4ca4b4751ea90960e72 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_8694: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/index.html
  IGT_7221: 4b77c6d85024d22ca521d510f8eee574128fe04f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

== Logs ==

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

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

[igt-dev] ✗ Fi.CI.IGT: failure for switch lib/igt_fb.c to use lib/i915/i915_blt functions for blitter on Intel hw (rev2)

[Patchwork]

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

== Series Details ==

Series: switch lib/igt_fb.c to use lib/i915/i915_blt functions for blitter on Intel hw (rev2)
URL   : https://patchwork.freedesktop.org/series/115691/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12923_full -> IGTPW_8694_full
====================================================

Summary
-------

  **FAILURE**

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

Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_plane_scaling@plane-scaler-with-modifiers-unity-scaling@pipe-a-hdmi-a-1:
    - shard-glk:          [PASS][1] -> [FAIL][2] +195 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/shard-glk8/igt@kms_plane_scaling@plane-scaler-with-modifiers-unity-scaling@pipe-a-hdmi-a-1.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-glk1/igt@kms_plane_scaling@plane-scaler-with-modifiers-unity-scaling@pipe-a-hdmi-a-1.html

  * igt@kms_plane_scaling@plane-upscale-with-modifiers-20x20@pipe-b-dp-1:
    - shard-apl:          [PASS][3] -> [FAIL][4] +128 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/shard-apl7/igt@kms_plane_scaling@plane-upscale-with-modifiers-20x20@pipe-b-dp-1.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-apl4/igt@kms_plane_scaling@plane-upscale-with-modifiers-20x20@pipe-b-dp-1.html

  
#### Warnings ####

  * igt@i915_pipe_stress@stress-xrgb8888-ytiled:
    - shard-apl:          [FAIL][5] ([i915#7036]) -> [FAIL][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/shard-apl1/igt@i915_pipe_stress@stress-xrgb8888-ytiled.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-apl6/igt@i915_pipe_stress@stress-xrgb8888-ytiled.html

  * igt@kms_flip_scaled_crc@flip-32bpp-yftile-to-64bpp-yftile-downscaling@pipe-a-valid-mode:
    - shard-glk:          [SKIP][7] ([fdo#109271]) -> [FAIL][8] +9 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/shard-glk9/igt@kms_flip_scaled_crc@flip-32bpp-yftile-to-64bpp-yftile-downscaling@pipe-a-valid-mode.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-glk4/igt@kms_flip_scaled_crc@flip-32bpp-yftile-to-64bpp-yftile-downscaling@pipe-a-valid-mode.html

  * igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-32bpp-ytilegen12rcccs-upscaling@pipe-a-valid-mode:
    - shard-apl:          [SKIP][9] ([fdo#109271]) -> [FAIL][10] +9 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/shard-apl2/igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-32bpp-ytilegen12rcccs-upscaling@pipe-a-valid-mode.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-apl2/igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-32bpp-ytilegen12rcccs-upscaling@pipe-a-valid-mode.html

  
#### Suppressed ####

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

  * igt@kms_big_fb@y-tiled-64bpp-rotate-180:
    - {shard-tglu}:       NOTRUN -> [FAIL][11]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-tglu-3/igt@kms_big_fb@y-tiled-64bpp-rotate-180.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs-upscaling@pipe-a-valid-mode:
    - {shard-tglu}:       [SKIP][12] ([i915#2587] / [i915#2672]) -> [FAIL][13] +1 similar issue
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/shard-tglu-9/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs-upscaling@pipe-a-valid-mode.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-tglu-2/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs-upscaling@pipe-a-valid-mode.html

  * igt@kms_flip_scaled_crc@flip-64bpp-xtile-to-16bpp-xtile-upscaling@pipe-a-valid-mode:
    - {shard-dg1}:        [PASS][14] -> [FAIL][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/shard-dg1-16/igt@kms_flip_scaled_crc@flip-64bpp-xtile-to-16bpp-xtile-upscaling@pipe-a-valid-mode.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-dg1-15/igt@kms_flip_scaled_crc@flip-64bpp-xtile-to-16bpp-xtile-upscaling@pipe-a-valid-mode.html

  * igt@kms_plane_lowres@tiling-x@pipe-d-hdmi-a-4:
    - {shard-dg1}:        NOTRUN -> [FAIL][16] +5 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-dg1-15/igt@kms_plane_lowres@tiling-x@pipe-d-hdmi-a-4.html

  * igt@kms_plane_scaling@plane-scaler-with-clipping-clamping-modifiers@pipe-c-hdmi-a-1:
    - {shard-tglu}:       [PASS][17] -> [FAIL][18] +118 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/shard-tglu-6/igt@kms_plane_scaling@plane-scaler-with-clipping-clamping-modifiers@pipe-c-hdmi-a-1.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-tglu-7/igt@kms_plane_scaling@plane-scaler-with-clipping-clamping-modifiers@pipe-c-hdmi-a-1.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_mmap_gtt@fault-concurrent-x:
    - shard-snb:          [PASS][19] -> [ABORT][20] ([i915#5161])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/shard-snb7/igt@gem_mmap_gtt@fault-concurrent-x.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-snb7/igt@gem_mmap_gtt@fault-concurrent-x.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-apl:          [PASS][21] -> [FAIL][22] ([i915#2346]) +1 similar issue
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/shard-apl3/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-apl7/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_flip@plain-flip-ts-check-interruptible@b-hdmi-a1:
    - shard-glk:          [PASS][23] -> [FAIL][24] ([i915#2122])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/shard-glk2/igt@kms_flip@plain-flip-ts-check-interruptible@b-hdmi-a1.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-glk9/igt@kms_flip@plain-flip-ts-check-interruptible@b-hdmi-a1.html

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-pri-indfb-draw-mmap-cpu:
    - shard-snb:          NOTRUN -> [SKIP][25] ([fdo#109271]) +55 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-snb4/igt@kms_frontbuffer_tracking@psr-2p-scndscrn-pri-indfb-draw-mmap-cpu.html

  * igt@kms_rotation_crc@sprite-rotation-90-pos-100-0:
    - shard-apl:          [PASS][26] -> [DMESG-FAIL][27] ([IGT#6]) +1 similar issue
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/shard-apl3/igt@kms_rotation_crc@sprite-rotation-90-pos-100-0.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-apl2/igt@kms_rotation_crc@sprite-rotation-90-pos-100-0.html

  
#### Possible fixes ####

  * igt@gem_exec_fair@basic-pace-solo@rcs0:
    - shard-apl:          [FAIL][28] ([i915#2842]) -> [PASS][29]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/shard-apl4/igt@gem_exec_fair@basic-pace-solo@rcs0.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-apl1/igt@gem_exec_fair@basic-pace-solo@rcs0.html

  * igt@i915_pm_dc@dc9-dpms:
    - {shard-tglu}:       [SKIP][30] ([i915#4281]) -> [PASS][31]
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/shard-tglu-5/igt@i915_pm_dc@dc9-dpms.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-tglu-2/igt@i915_pm_dc@dc9-dpms.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-glk:          [FAIL][32] ([i915#2346]) -> [PASS][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/shard-glk2/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-glk5/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_cursor_legacy@forked-move@pipe-b:
    - {shard-dg1}:        [INCOMPLETE][34] ([i915#8011]) -> [PASS][35]
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/shard-dg1-14/igt@kms_cursor_legacy@forked-move@pipe-b.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-dg1-18/igt@kms_cursor_legacy@forked-move@pipe-b.html

  
#### Warnings ####

  * igt@i915_pm_rps@reset:
    - shard-snb:          [INCOMPLETE][36] ([i915#7790]) -> [DMESG-FAIL][37] ([i915#8319])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12923/shard-snb4/igt@i915_pm_rps@reset.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/shard-snb7/igt@i915_pm_rps@reset.html

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

  [IGT#6]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/6
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109303]: https://bugs.freedesktop.org/show_bug.cgi?id=109303
  [fdo#109307]: https://bugs.freedesktop.org/show_bug.cgi?id=109307
  [fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [fdo#112054]: https://bugs.freedesktop.org/show_bug.cgi?id=112054
  [fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1755]: https://gitlab.freedesktop.org/drm/intel/issues/1755
  [i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2433]: https://gitlab.freedesktop.org/drm/intel/issues/2433
  [i915#2434]: https://gitlab.freedesktop.org/drm/intel/issues/2434
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
  [i915#2705]: https://gitlab.freedesktop.org/drm/intel/issues/2705
  [i915#284]: https://gitlab.freedesktop.org/drm/intel/issues/284
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
  [i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3742]: https://gitlab.freedesktop.org/drm/intel/issues/3742
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3952]: https://gitlab.freedesktop.org/drm/intel/issues/3952
  [i915#4036]: https://gitlab.freedesktop.org/drm/intel/issues/4036
  [i915#404]: https://gitlab.freedesktop.org/drm/intel/issues/404
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
  [i915#426]: https://gitlab.freedesktop.org/drm/intel/issues/426
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4281]: https://gitlab.freedesktop.org/drm/intel/issues/4281
  [i915#4387]: https://gitlab.freedesktop.org/drm/intel/issues/4387
  [i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
  [i915#4565]: https://gitlab.freedesktop.org/drm/intel/issues/4565
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4771]: https://gitlab.freedesktop.org/drm/intel/issues/4771
  [i915#4812]: https://gitlab.freedesktop.org/drm/intel/issues/4812
  [i915#4818]: https://gitlab.freedesktop.org/drm/intel/issues/4818
  [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
  [i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
  [i915#4854]: https://gitlab.freedesktop.org/drm/intel/issues/4854
  [i915#4859]: https://gitlab.freedesktop.org/drm/intel/issues/4859
  [i915#4860]: https://gitlab.freedesktop.org/drm/intel/issues/4860
  [i915#4880]: https://gitlab.freedesktop.org/drm/intel/issues/4880
  [i915#4881]: https://gitlab.freedesktop.org/drm/intel/issues/4881
  [i915#5161]: https://gitlab.freedesktop.org/drm/intel/issues/5161
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5288]: https://gitlab.freedesktop.org/drm/intel/issues/5288
  [i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
  [i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
  [i915#5563]: https://gitlab.freedesktop.org/drm/intel/issues/5563
  [i915#5852]: https://gitlab.freedesktop.org/drm/intel/issues/5852
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
  [i915#6301]: https://gitlab.freedesktop.org/drm/intel/issues/6301
  [i915#6433]: https://gitlab.freedesktop.org/drm/intel/issues/6433
  [i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6946]: https://gitlab.freedesktop.org/drm/intel/issues/6946
  [i915#6953]: https://gitlab.freedesktop.org/drm/intel/issues/6953
  [i915#7036]: https://gitlab.freedesktop.org/drm/intel/issues/7036
  [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
  [i915#7479]: https://gitlab.freedesktop.org/drm/intel/issues/7479
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7790]: https://gitlab.freedesktop.org/drm/intel/issues/7790
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
  [i915#8247]: https://gitlab.freedesktop.org/drm/intel/issues/8247
  [i915#8292]: https://gitlab.freedesktop.org/drm/intel/issues/8292
  [i915#8319]: https://gitlab.freedesktop.org/drm/intel/issues/8319


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

  * CI: CI-20190529 -> None
  * IGT: IGT_7221 -> IGTPW_8694
  * Piglit: piglit_4509 -> None

  CI-20190529: 20190529
  CI_DRM_12923: cdd32ac83137835a85bad4ca4b4751ea90960e72 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_8694: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8694/index.html
  IGT_7221: 4b77c6d85024d22ca521d510f8eee574128fe04f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

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