[PATCH v2 0/7] Add Y-tiling support into IGTs

[PATCH v2 0/7] Add Y-tiling support into IGTs

[Praveen Paneri]

This series adds Y-tiled buffer creation support into IGT libraries and
goes on to use this capability to add support into FBC tests to use
Y-tiled buffers.

Akash Goel (1):
  lib/igt_draw: Add Y-tiling support for IGT_DRAW_BLT method

Paulo Zanoni (1):
  tests/kms_draw_crc: add support for Y tiling

Praveen Paneri (5):
  lib/igt_fb: Let others use igt_get_fb_tile_size
  lib/igt_fb: Add helper function for tile_to_mod
  lib/igt_draw: Add Y-tiling support
  igt/kms_frontbuffer_tracking: Add Y-tiling support
  igt/kms_fbc_crc.c : Add Y-tile tests

 lib/igt_draw.c                   | 165 ++++++++++++++++++++++++++++-----------
 lib/igt_fb.c                     |  41 +++++++++-
 lib/igt_fb.h                     |   4 +-
 tests/kms_draw_crc.c             |  58 ++++++++++----
 tests/kms_fbc_crc.c              |  71 +++++++++--------
 tests/kms_frontbuffer_tracking.c |  48 +++++++-----
 6 files changed, 273 insertions(+), 114 deletions(-)

-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 1/7] lib/igt_fb: Let others use igt_get_fb_tile_size

[Praveen Paneri]

This function can be used by igt_draw to get accurate
tile dimensions for all tile formats.

v2: Added comments to function igt_get_fb_tile_size (Daniel)

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 lib/igt_fb.c | 16 +++++++++++++---
 lib/igt_fb.h |  3 ++-
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index d2b7e9e..9ba1e3b 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -73,9 +73,19 @@ static struct format_desc_struct {
 
 #define for_each_format(f)	\
 	for (f = format_desc; f - format_desc < ARRAY_SIZE(format_desc); f++)
-
-static void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
-				 unsigned *width_ret, unsigned *height_ret)
+/**
+ * igt_get_fb_tile_size:
+ * @fd: The DRM file descriptor
+ * @tiling: Tiling layout of the framebuffer (as framebuffer modifier)
+ * @fb_bpp: Bytes per pixel of the framebuffer
+ * @width_ret: Width of the tile in pixels
+ * @height_ret: Height of the tile in pixels
+ *
+ * This function returns width and height of a tile based on the given tiling
+ * format.
+ */
+void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
+			  unsigned *width_ret, unsigned *height_ret)
 {
 	switch (tiling) {
 	case LOCAL_DRM_FORMAT_MOD_NONE:
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index 4a680ce..414cb3d 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -94,7 +94,8 @@ enum igt_text_align {
 	align_vcenter	= 0x04,
 	align_hcenter	= 0x08,
 };
-
+void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
+			  unsigned *width_ret, unsigned *height_ret);
 void igt_calc_fb_size(int fd, int width, int height, int bpp, uint64_t tiling,
 		      unsigned *size_ret, unsigned *stride_ret);
 unsigned int
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/7] lib/igt_fb: Add helper function for tile_to_mod

[Praveen Paneri]

igt_get_fb_tile_size function takes modifer as an argument
This helper function will let users to convert tiling to
modifier and use igt_get_fb_tile_size()

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 lib/igt_fb.c | 26 ++++++++++++++++++++++++++
 lib/igt_fb.h |  1 +
 2 files changed, 27 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 9ba1e3b..27baf79 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -216,6 +216,32 @@ uint64_t igt_fb_mod_to_tiling(uint64_t modifier)
 	}
 }
 
+/**
+ * igt_fb_tiling_to_mod:
+ * @tiling: DRM framebuffer tiling
+ *
+ * This function converts a DRM framebuffer tiling to its corresponding
+ * modifier constant.
+ *
+ * Returns:
+ * A tiling constant
+ */
+uint64_t igt_fb_tiling_to_mod(uint64_t tiling)
+{
+	switch (tiling) {
+	case I915_TILING_NONE:
+		return LOCAL_DRM_FORMAT_MOD_NONE;
+	case I915_TILING_X:
+		return LOCAL_I915_FORMAT_MOD_X_TILED;
+	case I915_TILING_Y:
+		return LOCAL_I915_FORMAT_MOD_Y_TILED;
+	case I915_TILING_Yf:
+		return LOCAL_I915_FORMAT_MOD_Yf_TILED;
+	default:
+		igt_assert(0);
+	}
+}
+
 /* helpers to create nice-looking framebuffers */
 static int create_bo_for_fb(int fd, int width, int height, uint32_t format,
 			    uint64_t tiling, unsigned size, unsigned stride,
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index 414cb3d..a01671b 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -131,6 +131,7 @@ int igt_create_bo_with_dimensions(int fd, int width, int height, uint32_t format
 				  bool *is_dumb);
 
 uint64_t igt_fb_mod_to_tiling(uint64_t modifier);
+uint64_t igt_fb_tiling_to_mod(uint64_t tiling);
 
 /* cairo-based painting */
 cairo_t *igt_get_cairo_ctx(int fd, struct igt_fb *fb);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 3/7] lib/igt_draw: Add Y-tiling support

[Praveen Paneri]

This patch adds Y-tiling support for igt_draw_rect function.

v2:
Use helper function to get tile sizes (Ville)

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 lib/igt_draw.c | 132 +++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 87 insertions(+), 45 deletions(-)

diff --git a/lib/igt_draw.c b/lib/igt_draw.c
index 29aec85..fcf8fba 100644
--- a/lib/igt_draw.c
+++ b/lib/igt_draw.c
@@ -136,32 +136,47 @@ static int swizzle_addr(int addr, int swizzle)
 
 /* It's all in "pixel coordinates", so make sure you multiply/divide by the bpp
  * if you need to. */
-static int linear_x_y_to_tiled_pos(int x, int y, uint32_t stride, int swizzle,
-				   int bpp)
+static int linear_x_y_to_tiled_pos(int fd, int x, int y, uint32_t stride, int swizzle,
+				   int bpp, int tiling)
 {
-	int x_tile_size, y_tile_size;
-	int x_tile_n, y_tile_n, x_tile_off, y_tile_off;
-	int line_size, tile_size;
+	uint32_t tile_width, tile_height, tile_size;
+	int tile_x, tile_y; /* Co-ordinates of the tile holding the pixel */
+	int tile_x_off, tile_y_off; /* pixel position inside the tile */
 	int tile_n, tile_off;
-	int tiled_pos, tiles_per_line;
+	int line_size, tiles_per_line;
+	int tiled_pos;
 	int pixel_size = bpp / 8;
 
+	igt_get_fb_tile_size(fd, (uint64_t)igt_fb_tiling_to_mod(tiling), bpp,
+			&tile_width, &tile_height);
+
 	line_size = stride;
-	x_tile_size = 512;
-	y_tile_size = 8;
-	tile_size = x_tile_size * y_tile_size;
-	tiles_per_line = line_size / x_tile_size;
+	tile_size = tile_width * tile_height;
+	tiles_per_line = line_size / tile_width;
 
-	y_tile_n = y / y_tile_size;
-	y_tile_off = y % y_tile_size;
+	tile_y = y / tile_height;
+	tile_y_off = y % tile_height;
 
-	x_tile_n = (x * pixel_size) / x_tile_size;
-	x_tile_off = (x * pixel_size) % x_tile_size;
+	tile_x = (x * pixel_size) / tile_width;
+	tile_x_off = (x * pixel_size) % tile_width;
 
-	tile_n = y_tile_n * tiles_per_line + x_tile_n;
-	tile_off = y_tile_off * x_tile_size + x_tile_off;
-	tiled_pos = tile_n * tile_size + tile_off;
+	tile_n = tile_y * tiles_per_line + tile_x;
+
+	if (tiling == I915_TILING_X ) {
+		tile_off = tile_y_off * tile_width + tile_x_off;
+	} else { /* (tiling == I915_TILING_Y ) */
+		int x_oword_n, x_oword_off;
+		int oword_size = 16;
+
+		/* computation inside the tile */
+		x_oword_n = tile_x_off / oword_size;
+		x_oword_off = tile_x_off % oword_size;
+		tile_off = x_oword_n * tile_height * oword_size
+			   + tile_y_off * oword_size
+			   + x_oword_off;
+	}
 
+	tiled_pos = tile_n * tile_size + tile_off;
 	tiled_pos = swizzle_addr(tiled_pos, swizzle);
 
 	return tiled_pos / pixel_size;
@@ -169,34 +184,49 @@ static int linear_x_y_to_tiled_pos(int x, int y, uint32_t stride, int swizzle,
 
 /* It's all in "pixel coordinates", so make sure you multiply/divide by the bpp
  * if you need to. */
-static void tiled_pos_to_x_y_linear(int tiled_pos, uint32_t stride,
-				    int swizzle, int bpp, int *x, int *y)
+static void tiled_pos_to_x_y_linear(int fd, int tiled_pos, uint32_t stride,
+				    int swizzle, int bpp, int *x, int *y,
+				    int tiling)
 {
-	int tile_n, tile_off, tiles_per_line, line_size;
-	int x_tile_off, y_tile_off;
-	int x_tile_n, y_tile_n;
-	int x_tile_size, y_tile_size, tile_size;
+	uint32_t tile_width, tile_height, tile_size;
+	int tile_x, tile_y; /* Co-ordinates of the tile holding the pixel */
+	int tile_x_off, tile_y_off; /* pixel position inside the tile */
+	int tile_n, tile_off;
+	int line_size, tiles_per_line;
 	int pixel_size = bpp / 8;
 
+	igt_get_fb_tile_size(fd, (uint64_t)igt_fb_tiling_to_mod(tiling), bpp,
+                                 &tile_width, &tile_height);
 	tiled_pos = swizzle_addr(tiled_pos, swizzle);
 
 	line_size = stride;
-	x_tile_size = 512;
-	y_tile_size = 8;
-	tile_size = x_tile_size * y_tile_size;
-	tiles_per_line = line_size / x_tile_size;
+	tile_size = tile_width * tile_height;
+	tiles_per_line = line_size / tile_width;
 
 	tile_n = tiled_pos / tile_size;
 	tile_off = tiled_pos % tile_size;
 
-	y_tile_off = tile_off / x_tile_size;
-	x_tile_off = tile_off % x_tile_size;
+	tile_x = tile_n % tiles_per_line;
+	tile_y = tile_n / tiles_per_line;
 
-	x_tile_n = tile_n % tiles_per_line;
-	y_tile_n = tile_n / tiles_per_line;
+	if (tiling == I915_TILING_X ) {
 
-	*x = (x_tile_n * x_tile_size + x_tile_off) / pixel_size;
-	*y = y_tile_n * y_tile_size + y_tile_off;
+		tile_y_off = tile_off / tile_width;
+		tile_x_off = tile_off % tile_width;
+	} else {
+		int x_oword_n, x_oword_off;
+		int oword_size = 16;
+
+		x_oword_n = tile_off / (oword_size * tile_height);
+		x_oword_off = tile_off % oword_size;
+
+		tile_x_off = x_oword_n * oword_size + x_oword_off;
+		tile_y_off = (tile_off - x_oword_n * oword_size * tile_height)
+			     / oword_size;
+	}
+
+	*x = (tile_x * tile_width + tile_x_off) / pixel_size;
+	*y = tile_y * tile_height + tile_y_off;
 }
 
 static void set_pixel(void *_ptr, int index, uint32_t color, int bpp)
@@ -224,15 +254,16 @@ static void draw_rect_ptr_linear(void *ptr, uint32_t stride,
 	}
 }
 
-static void draw_rect_ptr_tiled(void *ptr, uint32_t stride, int swizzle,
-				struct rect *rect, uint32_t color, int bpp)
+static void draw_rect_ptr_tiled(int fd, void *ptr, uint32_t stride, int swizzle,
+				struct rect *rect, uint32_t color, int bpp,
+				int tiling)
 {
 	int x, y, pos;
 
 	for (y = rect->y; y < rect->y + rect->h; y++) {
 		for (x = rect->x; x < rect->x + rect->w; x++) {
-			pos = linear_x_y_to_tiled_pos(x, y, stride, swizzle,
-						      bpp);
+			pos = linear_x_y_to_tiled_pos(fd, x, y, stride, swizzle,
+						      bpp, tiling);
 			set_pixel(ptr, pos, color, bpp);
 		}
 	}
@@ -259,8 +290,12 @@ static void draw_rect_mmap_cpu(int fd, struct buf_data *buf, struct rect *rect,
 		draw_rect_ptr_linear(ptr, buf->stride, rect, color, buf->bpp);
 		break;
 	case I915_TILING_X:
-		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
-				    buf->bpp);
+		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle, rect, color,
+				    buf->bpp, tiling);
+		break;
+	case I915_TILING_Y:
+		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle, rect, color,
+				    buf->bpp, tiling);
 		break;
 	default:
 		igt_assert(false);
@@ -309,8 +344,12 @@ static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect *rect,
 		draw_rect_ptr_linear(ptr, buf->stride, rect, color, buf->bpp);
 		break;
 	case I915_TILING_X:
-		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
-				    buf->bpp);
+		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle, rect, color,
+				    buf->bpp, tiling);
+		break;
+	case I915_TILING_Y:
+		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle, rect, color,
+				    buf->bpp, tiling);
 		break;
 	default:
 		igt_assert(false);
@@ -338,7 +377,7 @@ static void draw_rect_pwrite_untiled(int fd, struct buf_data *buf,
 
 static void draw_rect_pwrite_tiled(int fd, struct buf_data *buf,
 				   struct rect *rect, uint32_t color,
-				   uint32_t swizzle)
+				   uint32_t swizzle, int tiling)
 {
 	int i;
 	int tiled_pos, x, y, pixel_size;
@@ -361,8 +400,8 @@ static void draw_rect_pwrite_tiled(int fd, struct buf_data *buf,
 		set_pixel(tmp, i, color, buf->bpp);
 
 	for (tiled_pos = 0; tiled_pos < buf->size; tiled_pos += pixel_size) {
-		tiled_pos_to_x_y_linear(tiled_pos, buf->stride, swizzle,
-					buf->bpp, &x, &y);
+		tiled_pos_to_x_y_linear(fd, tiled_pos, buf->stride, swizzle,
+					buf->bpp, &x, &y, tiling);
 
 		if (x >= rect->x && x < rect->x + rect->w &&
 		    y >= rect->y && y < rect->y + rect->h) {
@@ -399,7 +438,10 @@ static void draw_rect_pwrite(int fd, struct buf_data *buf,
 		draw_rect_pwrite_untiled(fd, buf, rect, color);
 		break;
 	case I915_TILING_X:
-		draw_rect_pwrite_tiled(fd, buf, rect, color, swizzle);
+		draw_rect_pwrite_tiled(fd, buf, rect, color, swizzle, tiling);
+		break;
+	case I915_TILING_Y:
+		draw_rect_pwrite_tiled(fd, buf, rect, color, swizzle, tiling);
 		break;
 	default:
 		igt_assert(false);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 4/7] lib/igt_draw: Add Y-tiling support for IGT_DRAW_BLT method

[Praveen Paneri]

From: Akash Goel <akash.goel@intel.com>

v2: Moved identical code into a single function (Paulo)

Signed-off-by: Akash Goel <akash.goel@intel.com>
Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 lib/igt_draw.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/lib/igt_draw.c b/lib/igt_draw.c
index fcf8fba..27a69cd 100644
--- a/lib/igt_draw.c
+++ b/lib/igt_draw.c
@@ -31,6 +31,7 @@
 #include "igt_core.h"
 #include "igt_fb.h"
 #include "ioctl_wrappers.h"
+#include "i830_reg.h"
 
 /**
  * SECTION:igt_draw
@@ -242,6 +243,34 @@ static void set_pixel(void *_ptr, int index, uint32_t color, int bpp)
 	}
 }
 
+static void switch_blt_tiling(struct intel_batchbuffer *batch, uint32_t tiling, bool on)
+{
+	uint32_t bcs_swctrl;
+
+	/* Default is X-tile */
+	if (tiling != I915_TILING_Y)
+		return;
+
+	bcs_swctrl = (0x3 << 16) | (on ? 0x3 : 0x0);
+
+	/* To change the tile register, insert an MI_FLUSH_DW followed by an
+	 * MI_LOAD_REGISTER_IMM
+	 */
+	BEGIN_BATCH(4, 0);
+	OUT_BATCH(MI_FLUSH_DW | 2);
+	OUT_BATCH(0x0);
+	OUT_BATCH(0x0);
+	OUT_BATCH(0x0);
+	ADVANCE_BATCH();
+
+	BEGIN_BATCH(4, 0);
+	OUT_BATCH(MI_LOAD_REGISTER_IMM);
+	OUT_BATCH(0x22200); /* BCS_SWCTRL */
+	OUT_BATCH(bcs_swctrl);
+	OUT_BATCH(MI_NOOP);
+	ADVANCE_BATCH();
+}
+
 static void draw_rect_ptr_linear(void *ptr, uint32_t stride,
 				 struct rect *rect, uint32_t color, int bpp)
 {
@@ -487,6 +516,8 @@ static void draw_rect_blt(int fd, struct cmd_data *cmd_data,
 	blt_cmd_tiling = (tiling) ? XY_COLOR_BLT_TILED : 0;
 	pitch = (tiling) ? buf->stride / 4 : buf->stride;
 
+	switch_blt_tiling(batch, tiling, true);
+
 	BEGIN_BATCH(6, 1);
 	OUT_BATCH(XY_COLOR_BLT_CMD_NOLEN | XY_COLOR_BLT_WRITE_ALPHA |
 		  XY_COLOR_BLT_WRITE_RGB | blt_cmd_tiling | blt_cmd_len);
@@ -497,6 +528,8 @@ static void draw_rect_blt(int fd, struct cmd_data *cmd_data,
 	OUT_BATCH(color);
 	ADVANCE_BATCH();
 
+	switch_blt_tiling(batch, tiling, false);
+
 	intel_batchbuffer_flush(batch);
 	intel_batchbuffer_free(batch);
 }
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 5/7] tests/kms_draw_crc: add support for Y tiling

[Praveen Paneri]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

This is the program that's supposed to test lib/igt_draw. We just
added Y tiling support for the library, so add the tests now.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 tests/kms_draw_crc.c | 58 ++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 43 insertions(+), 15 deletions(-)

diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
index c57d3a3..1d91b48 100644
--- a/tests/kms_draw_crc.c
+++ b/tests/kms_draw_crc.c
@@ -47,6 +47,13 @@ static const uint32_t formats[N_FORMATS] = {
 	DRM_FORMAT_XRGB2101010,
 };
 
+#define N_TILING_METHODS 3
+static const uint64_t tilings[N_TILING_METHODS] = {
+	LOCAL_DRM_FORMAT_MOD_NONE,
+	LOCAL_I915_FORMAT_MOD_X_TILED,
+	LOCAL_I915_FORMAT_MOD_Y_TILED,
+};
+
 struct base_crc {
 	bool set;
 	igt_crc_t crc;
@@ -151,6 +158,11 @@ static void draw_method_subtest(enum igt_draw_method method,
 {
 	igt_crc_t crc;
 
+	if (tiling == LOCAL_I915_FORMAT_MOD_Y_TILED)
+		igt_require(intel_gen(intel_get_drm_devid(drm_fd)) >= 9);
+
+	kmstest_unset_all_crtcs(drm_fd, drm_res);
+
 	/* Use IGT_DRAW_MMAP_GTT on an untiled buffer as the parameter for
 	 * comparison. Cache the value so we don't recompute it for every single
 	 * subtest. */
@@ -208,6 +220,12 @@ static void fill_fb_subtest(void)
 	get_fill_crc(LOCAL_I915_FORMAT_MOD_X_TILED, &crc);
 	igt_assert_crc_equal(&crc, &base_crc);
 
+	if (intel_gen(intel_get_drm_devid(drm_fd)) >= 9) {
+		get_fill_crc(LOCAL_I915_FORMAT_MOD_Y_TILED, &crc);
+		igt_assert_crc_equal(&crc, &base_crc);
+	}
+
+	kmstest_unset_all_crtcs(drm_fd, drm_res);
 	igt_remove_fb(drm_fd, &fb);
 }
 
@@ -265,28 +283,38 @@ static const char *format_str(int format_index)
 	}
 }
 
+static const char *tiling_str(int tiling_index)
+{
+	switch (tilings[tiling_index]) {
+	case LOCAL_DRM_FORMAT_MOD_NONE:
+		return "untiled";
+	case LOCAL_I915_FORMAT_MOD_X_TILED:
+		return "xtiled";
+	case LOCAL_I915_FORMAT_MOD_Y_TILED:
+		return "ytiled";
+	default:
+		igt_assert(false);
+	}
+}
+
 igt_main
 {
 	enum igt_draw_method method;
-	int format_index;
+	int format_idx, tiling_idx;
 
 	igt_fixture
 		setup_environment();
 
-	for (format_index = 0; format_index < N_FORMATS; format_index++) {
-		for (method = 0; method < IGT_DRAW_METHOD_COUNT; method++) {
-			igt_subtest_f("draw-method-%s-%s-untiled",
-				      format_str(format_index),
-				      igt_draw_get_method_name(method))
-				draw_method_subtest(method, format_index,
-						    LOCAL_DRM_FORMAT_MOD_NONE);
-			igt_subtest_f("draw-method-%s-%s-tiled",
-				      format_str(format_index),
-				      igt_draw_get_method_name(method))
-				draw_method_subtest(method, format_index,
-						LOCAL_I915_FORMAT_MOD_X_TILED);
-		}
-	}
+	for (format_idx = 0; format_idx < N_FORMATS; format_idx++) {
+	for (method = 0; method < IGT_DRAW_METHOD_COUNT; method++) {
+	for (tiling_idx = 0; tiling_idx < N_TILING_METHODS; tiling_idx++) {
+		igt_subtest_f("draw-method-%s-%s-%s",
+			      format_str(format_idx),
+			      igt_draw_get_method_name(method),
+			      tiling_str(tiling_idx))
+			draw_method_subtest(method, format_idx,
+					    tilings[tiling_idx]);
+	} } }
 
 	igt_subtest("fill-fb")
 		fill_fb_subtest();
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 6/7] igt/kms_frontbuffer_tracking: Add Y-tiling support

[Praveen Paneri]

Allow tests to create Y-tiled bufferes using a separate
argument to the test without increasing the number of
subtests.

v2: Changed tiling option to string (Paulo)

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 tests/kms_frontbuffer_tracking.c | 48 ++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 7cea4de..62ae33a 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -252,6 +252,7 @@ struct {
 	int only_pipes;
 	int shared_fb_x_offset;
 	int shared_fb_y_offset;
+	uint64_t tiling;
 } opt = {
 	.check_status = true,
 	.check_crc = true,
@@ -264,6 +265,7 @@ struct {
 	.only_pipes = PIPE_COUNT,
 	.shared_fb_x_offset = 500,
 	.shared_fb_y_offset = 500,
+	.tiling = LOCAL_I915_FORMAT_MOD_X_TILED,
 };
 
 struct modeset_params {
@@ -578,7 +580,7 @@ static void create_fb(enum pixel_format pformat, int width, int height,
 	if (plane == PLANE_CUR)
 		tiling_for_size = LOCAL_DRM_FORMAT_MOD_NONE;
 	else
-		tiling_for_size = LOCAL_I915_FORMAT_MOD_X_TILED;
+		tiling_for_size = opt.tiling;
 
 	igt_calc_fb_size(drm.fd, width, height, bpp, tiling_for_size, &size,
 			 &stride);
@@ -710,7 +712,7 @@ static void create_shared_fb(enum pixel_format format)
 
 	big_h = prim_h + scnd_h + offs_h + opt.shared_fb_y_offset;
 
-	create_fb(format, big_w, big_h, LOCAL_I915_FORMAT_MOD_X_TILED,
+	create_fb(format, big_w, big_h, opt.tiling,
 		  PLANE_PRI, &s->big);
 }
 
@@ -744,16 +746,16 @@ static void create_fbs(enum pixel_format format)
 
 	create_fb(format, prim_mode_params.mode->hdisplay,
 		  prim_mode_params.mode->vdisplay,
-		  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_PRI, &s->prim_pri);
+		  opt.tiling, PLANE_PRI, &s->prim_pri);
 	create_fb(format, prim_mode_params.cursor.w,
 		  prim_mode_params.cursor.h, LOCAL_DRM_FORMAT_MOD_NONE,
 		  PLANE_CUR, &s->prim_cur);
 	create_fb(format, prim_mode_params.sprite.w,
-		  prim_mode_params.sprite.h, LOCAL_I915_FORMAT_MOD_X_TILED,
+		  prim_mode_params.sprite.h, opt.tiling,
 		  PLANE_SPR, &s->prim_spr);
 
 	create_fb(format, offscreen_fb.w, offscreen_fb.h,
-		  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_PRI, &s->offscreen);
+		  opt.tiling, PLANE_PRI, &s->offscreen);
 
 	create_shared_fb(format);
 
@@ -762,11 +764,11 @@ static void create_fbs(enum pixel_format format)
 
 	create_fb(format, scnd_mode_params.mode->hdisplay,
 		  scnd_mode_params.mode->vdisplay,
-		  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_PRI, &s->scnd_pri);
+		  opt.tiling, PLANE_PRI, &s->scnd_pri);
 	create_fb(format, scnd_mode_params.cursor.w, scnd_mode_params.cursor.h,
 		  LOCAL_DRM_FORMAT_MOD_NONE, PLANE_CUR, &s->scnd_cur);
 	create_fb(format, scnd_mode_params.sprite.w, scnd_mode_params.sprite.h,
-		  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_SPR, &s->scnd_spr);
+		  opt.tiling, PLANE_SPR, &s->scnd_spr);
 }
 
 static bool set_mode_for_params(struct modeset_params *params)
@@ -1252,7 +1254,7 @@ static void init_blue_crc(enum pixel_format format, bool mandatory_sink_crc)
 
 	create_fb(format, prim_mode_params.mode->hdisplay,
 		  prim_mode_params.mode->vdisplay,
-		  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_PRI, &blue);
+		  opt.tiling, PLANE_PRI, &blue);
 
 	fill_fb(&blue, COLOR_PRIM_BG);
 
@@ -1287,7 +1289,7 @@ static void init_crcs(enum pixel_format format,
 	for (r = 0; r < pattern->n_rects; r++)
 		create_fb(format, prim_mode_params.mode->hdisplay,
 			  prim_mode_params.mode->vdisplay,
-			  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_PRI, &tmp_fbs[r]);
+			  opt.tiling, PLANE_PRI, &tmp_fbs[r]);
 
 	for (r = 0; r < pattern->n_rects; r++)
 		fill_fb(&tmp_fbs[r], COLOR_PRIM_BG);
@@ -2386,7 +2388,7 @@ static void flip_subtest(const struct test_mode *t)
 	prepare_subtest(t, pattern);
 
 	create_fb(t->format, params->fb.fb->width, params->fb.fb->height,
-		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &fb2);
+		  opt.tiling, t->plane, &fb2);
 	fill_fb(&fb2, bg_color);
 	orig_fb = params->fb.fb;
 
@@ -2432,7 +2434,7 @@ static void fliptrack_subtest(const struct test_mode *t, enum flip_type type)
 	prepare_subtest(t, pattern);
 
 	create_fb(t->format, params->fb.fb->width, params->fb.fb->height,
-		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &fb2);
+		  opt.tiling, t->plane, &fb2);
 	fill_fb(&fb2, COLOR_PRIM_BG);
 	orig_fb = params->fb.fb;
 
@@ -2643,7 +2645,7 @@ static void fullscreen_plane_subtest(const struct test_mode *t)
 	prepare_subtest(t, pattern);
 
 	rect = pattern->get_rect(&params->fb, 0);
-	create_fb(t->format, rect.w, rect.h, LOCAL_I915_FORMAT_MOD_X_TILED,
+	create_fb(t->format, rect.w, rect.h, opt.tiling,
 		  t->plane, &fullscreen_fb);
 	/* Call pick_color() again since PRI and SPR may not support the same
 	 * pixel formats. */
@@ -2722,7 +2724,7 @@ static void scaledprimary_subtest(const struct test_mode *t)
 	old_fb = params->fb.fb;
 
 	create_fb(t->format, params->fb.fb->width, params->fb.fb->height,
-		  LOCAL_I915_FORMAT_MOD_X_TILED,
+		  opt.tiling,
 		  t->plane, &new_fb);
 	fill_fb(&new_fb, COLOR_BLUE);
 
@@ -2832,7 +2834,7 @@ static void modesetfrombusy_subtest(const struct test_mode *t)
 	prepare_subtest(t, NULL);
 
 	create_fb(t->format, params->fb.fb->width, params->fb.fb->height,
-		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &fb2);
+		  opt.tiling, t->plane, &fb2);
 	fill_fb(&fb2, COLOR_PRIM_BG);
 
 	start_busy_thread(params->fb.fb);
@@ -2937,7 +2939,7 @@ static void farfromfence_subtest(const struct test_mode *t)
 	target = pick_target(t, params);
 
 	create_fb(t->format, params->mode->hdisplay, max_height,
-		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &tall_fb);
+		  opt.tiling, t->plane, &tall_fb);
 
 	fill_fb(&tall_fb, COLOR_PRIM_BG);
 
@@ -3012,7 +3014,7 @@ static void badstride_subtest(const struct test_mode *t)
 	old_fb = params->fb.fb;
 
 	create_fb(t->format, params->fb.fb->width + 4096, params->fb.fb->height,
-		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &wide_fb);
+		  opt.tiling, t->plane, &wide_fb);
 	igt_assert(wide_fb.stride > 16384);
 
 	fill_fb(&wide_fb, COLOR_PRIM_BG);
@@ -3079,7 +3081,7 @@ static void stridechange_subtest(const struct test_mode *t)
 	old_fb = params->fb.fb;
 
 	create_fb(t->format, params->fb.fb->width + 512, params->fb.fb->height,
-		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &new_fb);
+		  opt.tiling, t->plane, &new_fb);
 	fill_fb(&new_fb, COLOR_PRIM_BG);
 
 	igt_assert(old_fb->stride != new_fb.stride);
@@ -3198,7 +3200,7 @@ static void basic_subtest(const struct test_mode *t)
 	prepare_subtest(t, pattern);
 
 	create_fb(t->format, params->fb.fb->width, params->fb.fb->height,
-		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &fb2);
+		  opt.tiling, t->plane, &fb2);
 	fb1 = params->fb.fb;
 
 	for (r = 0, method = 0; method < IGT_DRAW_METHOD_COUNT; method++, r++) {
@@ -3267,6 +3269,12 @@ static int opt_handler(int option, int option_index, void *data)
 		igt_assert_eq(opt.only_pipes, PIPE_COUNT);
 		opt.only_pipes = PIPE_DUAL;
 		break;
+	case 'l':
+		if (!strncmp(optarg, "y", 1))
+			opt.tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
+		else
+			opt.tiling = LOCAL_I915_FORMAT_MOD_X_TILED;
+		break;
 	default:
 		igt_assert(false);
 	}
@@ -3286,7 +3294,8 @@ const char *help_str =
 "  --shared-fb-x offset        Use 'offset' as the X offset for the shared FB\n"
 "  --shared-fb-y offset        Use 'offset' as the Y offset for the shared FB\n"
 "  --1p-only                   Only run subtests that use 1 pipe\n"
-"  --2p-only                   Only run subtests that use 2 pipes\n";
+"  --2p-only                   Only run subtests that use 2 pipes\n"
+"  --tiling                    Select tiling mode-'x' or 'y'\n";
 
 static const char *pipes_str(int pipes)
 {
@@ -3425,6 +3434,7 @@ int main(int argc, char *argv[])
 		{ "shared-fb-y",              1, 0, 'y'},
 		{ "1p-only",                  0, 0, '1'},
 		{ "2p-only",                  0, 0, '2'},
+		{ "tiling",                   1, 0, 'l'},
 		{ 0, 0, 0, 0 }
 	};
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 7/7] igt/kms_fbc_crc.c : Add Y-tile tests

[Praveen Paneri]

Now that we have support for Y-tiled buffers, add another
iteration of tests for Y-tiled buffers.

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 tests/kms_fbc_crc.c | 71 +++++++++++++++++++++++++++++------------------------
 1 file changed, 39 insertions(+), 32 deletions(-)

diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
index 7964e05..cdf04c1 100644
--- a/tests/kms_fbc_crc.c
+++ b/tests/kms_fbc_crc.c
@@ -318,12 +318,10 @@ static void prepare_crtc(data_t *data)
 	igt_output_set_pipe(output, data->pipe);
 }
 
-static void create_fbs(data_t *data, bool tiled, struct igt_fb *fbs)
+static void create_fbs(data_t *data, uint64_t tiling, struct igt_fb *fbs)
 {
 	int rc;
 	drmModeModeInfo *mode = igt_output_get_mode(data->output);
-	uint64_t tiling = tiled ? LOCAL_I915_FORMAT_MOD_X_TILED :
-				  LOCAL_DRM_FORMAT_MOD_NONE;
 
 	rc = igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
 				 DRM_FORMAT_XRGB8888, tiling,
@@ -344,8 +342,8 @@ static void get_ref_crcs(data_t *data)
 	struct igt_fb fbs[4];
 	int i;
 
-	create_fbs(data, false, &fbs[0]);
-	create_fbs(data, false, &fbs[2]);
+	create_fbs(data, LOCAL_DRM_FORMAT_MOD_NONE, &fbs[0]);
+	create_fbs(data, LOCAL_DRM_FORMAT_MOD_NONE, &fbs[2]);
 
 	fill_mmap_gtt(data, fbs[2].gem_handle, 0xff);
 	fill_mmap_gtt(data, fbs[3].gem_handle, 0xff);
@@ -366,7 +364,7 @@ static void get_ref_crcs(data_t *data)
 		igt_remove_fb(data->drm_fd, &fbs[i]);
 }
 
-static bool prepare_test(data_t *data, enum test_mode test_mode)
+static bool prepare_test(data_t *data, enum test_mode test_mode, uint64_t tiling)
 {
 	igt_display_t *display = &data->display;
 	igt_output_t *output = data->output;
@@ -374,7 +372,7 @@ static bool prepare_test(data_t *data, enum test_mode test_mode)
 
 	data->primary = igt_output_get_plane_type(data->output, DRM_PLANE_TYPE_PRIMARY);
 
-	create_fbs(data, true, data->fb);
+	create_fbs(data, tiling, data->fb);
 
 	igt_pipe_crc_free(data->pipe_crc);
 	data->pipe_crc = NULL;
@@ -484,32 +482,41 @@ static void run_test(data_t *data, enum test_mode mode)
 
 	reset_display(data);
 
-	for_each_pipe_with_valid_output(display, data->pipe, data->output) {
-		prepare_crtc(data);
-
-		igt_info("Beginning %s on pipe %s, connector %s\n",
-			  igt_subtest_name(),
-			  kmstest_pipe_name(data->pipe),
-			  igt_output_name(data->output));
-
-		if (!prepare_test(data, mode)) {
-			igt_info("%s on pipe %s, connector %s: SKIPPED\n",
-				  igt_subtest_name(),
-				  kmstest_pipe_name(data->pipe),
-				  igt_output_name(data->output));
-			continue;
+	for (int tiling = I915_TILING_X;
+	     tiling <= I915_TILING_Y; tiling++) {
+		for_each_pipe_with_valid_output(display,
+						data->pipe, data->output) {
+			prepare_crtc(data);
+
+			igt_info("Beginning %s on pipe %s, connector "
+					"%s, %s-tiled\n",
+					igt_subtest_name(),
+					kmstest_pipe_name(data->pipe),
+					igt_output_name(data->output),
+					(tiling == I915_TILING_X) ? "x":"y" );
+
+			if (!prepare_test(data, mode,
+					  igt_fb_tiling_to_mod(tiling))) {
+				igt_info("%s on pipe %s, connector %s: SKIPPED\n",
+						igt_subtest_name(),
+						kmstest_pipe_name(data->pipe),
+						igt_output_name(data->output));
+				continue;
+			}
+
+			valid_tests++;
+
+			test_crc(data, mode);
+
+			igt_info("%s on pipe %s, connector %s"
+					"%s-tiled: PASSED\n",
+					igt_subtest_name(),
+					kmstest_pipe_name(data->pipe),
+					igt_output_name(data->output),
+					(tiling == I915_TILING_X) ? "x":"y" );
+
+			finish_crtc(data, mode);
 		}
-
-		valid_tests++;
-
-		test_crc(data, mode);
-
-		igt_info("%s on pipe %s, connector %s: PASSED\n",
-			  igt_subtest_name(),
-			  kmstest_pipe_name(data->pipe),
-			  igt_output_name(data->output));
-
-		finish_crtc(data, mode);
 	}
 
 	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 0/7] Add Y-tiling support into IGTs

[Paulo Zanoni]

Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
> This series adds Y-tiled buffer creation support into IGT libraries
> and
> goes on to use this capability to add support into FBC tests to use
> Y-tiled buffers.

The problem I reported before still happens to me. Please fix it before
resending.

> 
> Akash Goel (1):
>   lib/igt_draw: Add Y-tiling support for IGT_DRAW_BLT method
> 
> Paulo Zanoni (1):
>   tests/kms_draw_crc: add support for Y tiling
> 
> Praveen Paneri (5):
>   lib/igt_fb: Let others use igt_get_fb_tile_size
>   lib/igt_fb: Add helper function for tile_to_mod
>   lib/igt_draw: Add Y-tiling support
>   igt/kms_frontbuffer_tracking: Add Y-tiling support
>   igt/kms_fbc_crc.c : Add Y-tile tests
> 
>  lib/igt_draw.c                   | 165 ++++++++++++++++++++++++++++-
> ----------
>  lib/igt_fb.c                     |  41 +++++++++-
>  lib/igt_fb.h                     |   4 +-
>  tests/kms_draw_crc.c             |  58 ++++++++++----
>  tests/kms_fbc_crc.c              |  71 +++++++++--------
>  tests/kms_frontbuffer_tracking.c |  48 +++++++-----
>  6 files changed, 273 insertions(+), 114 deletions(-)
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 0/7] Add Y-tiling support into IGTs

[Praveen Paneri]

HI Paulo,



On Sat, Apr 29, 2017 at 12:51 AM, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
>> This series adds Y-tiled buffer creation support into IGT libraries
>> and
>> goes on to use this capability to add support into FBC tests to use
>> Y-tiled buffers.
>
> The problem I reported before still happens to me. Please fix it before
> resending.
I tried reproducing it on SKL. For me the test was just running fine.

I ran it in shell mode on ubuntu running the latest kernel.

Could you plz give some hint to repro it at my end.
Thanks,
Praveen
>
>>
>> Akash Goel (1):
>>   lib/igt_draw: Add Y-tiling support for IGT_DRAW_BLT method
>>
>> Paulo Zanoni (1):
>>   tests/kms_draw_crc: add support for Y tiling
>>
>> Praveen Paneri (5):
>>   lib/igt_fb: Let others use igt_get_fb_tile_size
>>   lib/igt_fb: Add helper function for tile_to_mod
>>   lib/igt_draw: Add Y-tiling support
>>   igt/kms_frontbuffer_tracking: Add Y-tiling support
>>   igt/kms_fbc_crc.c : Add Y-tile tests
>>
>>  lib/igt_draw.c                   | 165 ++++++++++++++++++++++++++++-
>> ----------
>>  lib/igt_fb.c                     |  41 +++++++++-
>>  lib/igt_fb.h                     |   4 +-
>>  tests/kms_draw_crc.c             |  58 ++++++++++----
>>  tests/kms_fbc_crc.c              |  71 +++++++++--------
>>  tests/kms_frontbuffer_tracking.c |  48 +++++++-----
>>  6 files changed, 273 insertions(+), 114 deletions(-)
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 0/7] Add Y-tiling support into IGTs

[Paulo Zanoni]

Em Sáb, 2017-04-29 às 08:44 +0530, Praveen Paneri escreveu:
> HI Paulo,
> 
> 
> 
> On Sat, Apr 29, 2017 at 12:51 AM, Paulo Zanoni <paulo.r.zanoni@intel.
> com> wrote:
> > Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
> > > This series adds Y-tiled buffer creation support into IGT
> > > libraries
> > > and
> > > goes on to use this capability to add support into FBC tests to
> > > use
> > > Y-tiled buffers.
> > 
> > The problem I reported before still happens to me. Please fix it
> > before
> > resending.
> 
> I tried reproducing it on SKL. For me the test was just running fine.
> 
> I ran it in shell mode on ubuntu running the latest kernel.
> 
> Could you plz give some hint to repro it at my end.

How much time did each subtest take to complete? I have tests that
should take 1s but take up to 150s with the patch.

> Thanks,
> Praveen
> > 
> > > 
> > > Akash Goel (1):
> > >   lib/igt_draw: Add Y-tiling support for IGT_DRAW_BLT method
> > > 
> > > Paulo Zanoni (1):
> > >   tests/kms_draw_crc: add support for Y tiling
> > > 
> > > Praveen Paneri (5):
> > >   lib/igt_fb: Let others use igt_get_fb_tile_size
> > >   lib/igt_fb: Add helper function for tile_to_mod
> > >   lib/igt_draw: Add Y-tiling support
> > >   igt/kms_frontbuffer_tracking: Add Y-tiling support
> > >   igt/kms_fbc_crc.c : Add Y-tile tests
> > > 
> > >  lib/igt_draw.c                   | 165
> > > ++++++++++++++++++++++++++++-
> > > ----------
> > >  lib/igt_fb.c                     |  41 +++++++++-
> > >  lib/igt_fb.h                     |   4 +-
> > >  tests/kms_draw_crc.c             |  58 ++++++++++----
> > >  tests/kms_fbc_crc.c              |  71 +++++++++--------
> > >  tests/kms_frontbuffer_tracking.c |  48 +++++++-----
> > >  6 files changed, 273 insertions(+), 114 deletions(-)
> > > 
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] lib/igt_draw: Add Y-tiling support

[Praveen Paneri]

This patch adds Y-tiling support for igt_draw_rect function.

v2: Use helper function to get tile sizes (Ville)

v3: Moved igt_get_fb_tile_size() out of the for loop
 for better performance (Paulo)

Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 lib/igt_draw.c | 139 ++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 94 insertions(+), 45 deletions(-)

diff --git a/lib/igt_draw.c b/lib/igt_draw.c
index 29aec85..2138bf7 100644
--- a/lib/igt_draw.c
+++ b/lib/igt_draw.c
@@ -136,32 +136,45 @@ static int swizzle_addr(int addr, int swizzle)
 
 /* It's all in "pixel coordinates", so make sure you multiply/divide by the bpp
  * if you need to. */
-static int linear_x_y_to_tiled_pos(int x, int y, uint32_t stride, int swizzle,
-				   int bpp)
+static int linear_x_y_to_tiled_pos(int fd, int x, int y, uint32_t stride, int swizzle,
+				   int bpp, int tiling, uint32_t tile_width,
+				   uint32_t tile_height)
 {
-	int x_tile_size, y_tile_size;
-	int x_tile_n, y_tile_n, x_tile_off, y_tile_off;
-	int line_size, tile_size;
+	uint32_t tile_size;
+	int tile_x, tile_y; /* Co-ordinates of the tile holding the pixel */
+	int tile_x_off, tile_y_off; /* pixel position inside the tile */
 	int tile_n, tile_off;
-	int tiled_pos, tiles_per_line;
+	int line_size, tiles_per_line;
+	int tiled_pos;
 	int pixel_size = bpp / 8;
 
 	line_size = stride;
-	x_tile_size = 512;
-	y_tile_size = 8;
-	tile_size = x_tile_size * y_tile_size;
-	tiles_per_line = line_size / x_tile_size;
+	tile_size = tile_width * tile_height;
+	tiles_per_line = line_size / tile_width;
 
-	y_tile_n = y / y_tile_size;
-	y_tile_off = y % y_tile_size;
+	tile_y = y / tile_height;
+	tile_y_off = y % tile_height;
 
-	x_tile_n = (x * pixel_size) / x_tile_size;
-	x_tile_off = (x * pixel_size) % x_tile_size;
+	tile_x = (x * pixel_size) / tile_width;
+	tile_x_off = (x * pixel_size) % tile_width;
 
-	tile_n = y_tile_n * tiles_per_line + x_tile_n;
-	tile_off = y_tile_off * x_tile_size + x_tile_off;
-	tiled_pos = tile_n * tile_size + tile_off;
+	tile_n = tile_y * tiles_per_line + tile_x;
+
+	if (tiling == I915_TILING_X ) {
+		tile_off = tile_y_off * tile_width + tile_x_off;
+	} else { /* (tiling == I915_TILING_Y ) */
+		int x_oword_n, x_oword_off;
+		int oword_size = 16;
+
+		/* computation inside the tile */
+		x_oword_n = tile_x_off / oword_size;
+		x_oword_off = tile_x_off % oword_size;
+		tile_off = x_oword_n * tile_height * oword_size
+			   + tile_y_off * oword_size
+			   + x_oword_off;
+	}
 
+	tiled_pos = tile_n * tile_size + tile_off;
 	tiled_pos = swizzle_addr(tiled_pos, swizzle);
 
 	return tiled_pos / pixel_size;
@@ -169,34 +182,48 @@ static int linear_x_y_to_tiled_pos(int x, int y, uint32_t stride, int swizzle,
 
 /* It's all in "pixel coordinates", so make sure you multiply/divide by the bpp
  * if you need to. */
-static void tiled_pos_to_x_y_linear(int tiled_pos, uint32_t stride,
-				    int swizzle, int bpp, int *x, int *y)
+static void tiled_pos_to_x_y_linear(int fd, int tiled_pos, uint32_t stride,
+				    int swizzle, int bpp, int *x, int *y,
+				    int tiling, uint32_t tile_width,
+				    uint32_t tile_height)
 {
-	int tile_n, tile_off, tiles_per_line, line_size;
-	int x_tile_off, y_tile_off;
-	int x_tile_n, y_tile_n;
-	int x_tile_size, y_tile_size, tile_size;
+	uint32_t tile_size;
+	int tile_x, tile_y; /* Co-ordinates of the tile holding the pixel */
+	int tile_x_off, tile_y_off; /* pixel position inside the tile */
+	int tile_n, tile_off;
+	int line_size, tiles_per_line;
 	int pixel_size = bpp / 8;
 
 	tiled_pos = swizzle_addr(tiled_pos, swizzle);
 
 	line_size = stride;
-	x_tile_size = 512;
-	y_tile_size = 8;
-	tile_size = x_tile_size * y_tile_size;
-	tiles_per_line = line_size / x_tile_size;
+	tile_size = tile_width * tile_height;
+	tiles_per_line = line_size / tile_width;
 
 	tile_n = tiled_pos / tile_size;
 	tile_off = tiled_pos % tile_size;
 
-	y_tile_off = tile_off / x_tile_size;
-	x_tile_off = tile_off % x_tile_size;
+	tile_x = tile_n % tiles_per_line;
+	tile_y = tile_n / tiles_per_line;
+
+	if (tiling == I915_TILING_X ) {
+
+		tile_y_off = tile_off / tile_width;
+		tile_x_off = tile_off % tile_width;
+	} else {
+		int x_oword_n, x_oword_off;
+		int oword_size = 16;
+
+		x_oword_n = tile_off / (oword_size * tile_height);
+		x_oword_off = tile_off % oword_size;
 
-	x_tile_n = tile_n % tiles_per_line;
-	y_tile_n = tile_n / tiles_per_line;
+		tile_x_off = x_oword_n * oword_size + x_oword_off;
+		tile_y_off = (tile_off - x_oword_n * oword_size * tile_height)
+			     / oword_size;
+	}
 
-	*x = (x_tile_n * x_tile_size + x_tile_off) / pixel_size;
-	*y = y_tile_n * y_tile_size + y_tile_off;
+	*x = (tile_x * tile_width + tile_x_off) / pixel_size;
+	*y = tile_y * tile_height + tile_y_off;
 }
 
 static void set_pixel(void *_ptr, int index, uint32_t color, int bpp)
@@ -224,15 +251,21 @@ static void draw_rect_ptr_linear(void *ptr, uint32_t stride,
 	}
 }
 
-static void draw_rect_ptr_tiled(void *ptr, uint32_t stride, int swizzle,
-				struct rect *rect, uint32_t color, int bpp)
+static void draw_rect_ptr_tiled(int fd, void *ptr, uint32_t stride, int swizzle,
+				struct rect *rect, uint32_t color, int bpp,
+				int tiling)
 {
 	int x, y, pos;
+	uint32_t tile_width, tile_height;
+
+	igt_get_fb_tile_size(fd, (uint64_t)igt_fb_tiling_to_mod(tiling), bpp,
+			&tile_width, &tile_height);
 
 	for (y = rect->y; y < rect->y + rect->h; y++) {
 		for (x = rect->x; x < rect->x + rect->w; x++) {
-			pos = linear_x_y_to_tiled_pos(x, y, stride, swizzle,
-						      bpp);
+			pos = linear_x_y_to_tiled_pos(fd, x, y, stride, swizzle,
+						      bpp, tiling, tile_width,
+						      tile_height);
 			set_pixel(ptr, pos, color, bpp);
 		}
 	}
@@ -259,8 +292,12 @@ static void draw_rect_mmap_cpu(int fd, struct buf_data *buf, struct rect *rect,
 		draw_rect_ptr_linear(ptr, buf->stride, rect, color, buf->bpp);
 		break;
 	case I915_TILING_X:
-		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
-				    buf->bpp);
+		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle, rect, color,
+				    buf->bpp, tiling);
+		break;
+	case I915_TILING_Y:
+		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle, rect, color,
+				    buf->bpp, tiling);
 		break;
 	default:
 		igt_assert(false);
@@ -309,8 +346,12 @@ static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect *rect,
 		draw_rect_ptr_linear(ptr, buf->stride, rect, color, buf->bpp);
 		break;
 	case I915_TILING_X:
-		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
-				    buf->bpp);
+		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle, rect, color,
+				    buf->bpp, tiling);
+		break;
+	case I915_TILING_Y:
+		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle, rect, color,
+				    buf->bpp, tiling);
 		break;
 	default:
 		igt_assert(false);
@@ -338,7 +379,7 @@ static void draw_rect_pwrite_untiled(int fd, struct buf_data *buf,
 
 static void draw_rect_pwrite_tiled(int fd, struct buf_data *buf,
 				   struct rect *rect, uint32_t color,
-				   uint32_t swizzle)
+				   uint32_t swizzle, int tiling)
 {
 	int i;
 	int tiled_pos, x, y, pixel_size;
@@ -347,6 +388,7 @@ static void draw_rect_pwrite_tiled(int fd, struct buf_data *buf,
 	bool flush_tmp = false;
 	int tmp_start_pos = 0;
 	int pixels_written = 0;
+	uint32_t tile_width, tile_height;
 
 	/* We didn't implement suport for the older tiling methods yet. */
 	igt_require(intel_gen(intel_get_drm_devid(fd)) >= 5);
@@ -360,9 +402,13 @@ static void draw_rect_pwrite_tiled(int fd, struct buf_data *buf,
 	for (i = 0; i < tmp_size; i++)
 		set_pixel(tmp, i, color, buf->bpp);
 
+	igt_get_fb_tile_size(fd, (uint64_t)igt_fb_tiling_to_mod(tiling),
+			     buf->bpp, &tile_width, &tile_height);
+
 	for (tiled_pos = 0; tiled_pos < buf->size; tiled_pos += pixel_size) {
-		tiled_pos_to_x_y_linear(tiled_pos, buf->stride, swizzle,
-					buf->bpp, &x, &y);
+		tiled_pos_to_x_y_linear(fd, tiled_pos, buf->stride, swizzle,
+					buf->bpp, &x, &y, tiling, tile_width,
+					tile_height);
 
 		if (x >= rect->x && x < rect->x + rect->w &&
 		    y >= rect->y && y < rect->y + rect->h) {
@@ -399,7 +445,10 @@ static void draw_rect_pwrite(int fd, struct buf_data *buf,
 		draw_rect_pwrite_untiled(fd, buf, rect, color);
 		break;
 	case I915_TILING_X:
-		draw_rect_pwrite_tiled(fd, buf, rect, color, swizzle);
+		draw_rect_pwrite_tiled(fd, buf, rect, color, swizzle, tiling);
+		break;
+	case I915_TILING_Y:
+		draw_rect_pwrite_tiled(fd, buf, rect, color, swizzle, tiling);
 		break;
 	default:
 		igt_assert(false);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 0/7] Add Y-tiling support into IGTs

[Praveen Paneri]

Hi Paulo,

On Tuesday 06 June 2017 10:28 PM, Paulo Zanoni wrote:
> Em Sáb, 2017-04-29 às 08:44 +0530, Praveen Paneri escreveu:
>> HI Paulo,
>>
>>
>>
>> On Sat, Apr 29, 2017 at 12:51 AM, Paulo Zanoni <paulo.r.zanoni@intel.
>> com> wrote:
>>> Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
>>>> This series adds Y-tiled buffer creation support into IGT
>>>> libraries
>>>> and
>>>> goes on to use this capability to add support into FBC tests to
>>>> use
>>>> Y-tiled buffers.
>>>
>>> The problem I reported before still happens to me. Please fix it
>>> before
>>> resending.
>>
>> I tried reproducing it on SKL. For me the test was just running fine.
>>
>> I ran it in shell mode on ubuntu running the latest kernel.
>>
>> Could you plz give some hint to repro it at my end.
>
> How much time did each subtest take to complete? I have tests that
> should take 1s but take up to 150s with the patch.
It was not taking 150s for me but some tests were taking around 7-8 
secs. I realized that call to igt_get_fb_tile_size() was happening 
inside a busy loop.
I have fixed the issue and I see most of the tests completing within a 
sec now. I have floated the updated patch and here is the link to new 
series. Plz review.
https://patchwork.freedesktop.org/series/21467/

Thanks,
Praveen
>
>> Thanks,
>> Praveen
>>>
>>>>
>>>> Akash Goel (1):
>>>>   lib/igt_draw: Add Y-tiling support for IGT_DRAW_BLT method
>>>>
>>>> Paulo Zanoni (1):
>>>>   tests/kms_draw_crc: add support for Y tiling
>>>>
>>>> Praveen Paneri (5):
>>>>   lib/igt_fb: Let others use igt_get_fb_tile_size
>>>>   lib/igt_fb: Add helper function for tile_to_mod
>>>>   lib/igt_draw: Add Y-tiling support
>>>>   igt/kms_frontbuffer_tracking: Add Y-tiling support
>>>>   igt/kms_fbc_crc.c : Add Y-tile tests
>>>>
>>>>  lib/igt_draw.c                   | 165
>>>> ++++++++++++++++++++++++++++-
>>>> ----------
>>>>  lib/igt_fb.c                     |  41 +++++++++-
>>>>  lib/igt_fb.h                     |   4 +-
>>>>  tests/kms_draw_crc.c             |  58 ++++++++++----
>>>>  tests/kms_fbc_crc.c              |  71 +++++++++--------
>>>>  tests/kms_frontbuffer_tracking.c |  48 +++++++-----
>>>>  6 files changed, 273 insertions(+), 114 deletions(-)
>>>>
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] lib/igt_draw: Add Y-tiling support

[Praveen Paneri]

Hi Paulo,

Could you plz review this patch in context of this series.

https://patchwork.freedesktop.org/series/21467/

It should fix the issue that you highlighted.


Thanks,
Praveen

On Friday 09 June 2017 03:48 PM, Praveen Paneri wrote:
> This patch adds Y-tiling support for igt_draw_rect function.
>
> v2: Use helper function to get tile sizes (Ville)
>
> v3: Moved igt_get_fb_tile_size() out of the for loop
>  for better performance (Paulo)
>
> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> ---
>  lib/igt_draw.c | 139 ++++++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 94 insertions(+), 45 deletions(-)
>
> diff --git a/lib/igt_draw.c b/lib/igt_draw.c
> index 29aec85..2138bf7 100644
> --- a/lib/igt_draw.c
> +++ b/lib/igt_draw.c
> @@ -136,32 +136,45 @@ static int swizzle_addr(int addr, int swizzle)
>
>  /* It's all in "pixel coordinates", so make sure you multiply/divide by the bpp
>   * if you need to. */
> -static int linear_x_y_to_tiled_pos(int x, int y, uint32_t stride, int swizzle,
> -				   int bpp)
> +static int linear_x_y_to_tiled_pos(int fd, int x, int y, uint32_t stride, int swizzle,
> +				   int bpp, int tiling, uint32_t tile_width,
> +				   uint32_t tile_height)
>  {
> -	int x_tile_size, y_tile_size;
> -	int x_tile_n, y_tile_n, x_tile_off, y_tile_off;
> -	int line_size, tile_size;
> +	uint32_t tile_size;
> +	int tile_x, tile_y; /* Co-ordinates of the tile holding the pixel */
> +	int tile_x_off, tile_y_off; /* pixel position inside the tile */
>  	int tile_n, tile_off;
> -	int tiled_pos, tiles_per_line;
> +	int line_size, tiles_per_line;
> +	int tiled_pos;
>  	int pixel_size = bpp / 8;
>
>  	line_size = stride;
> -	x_tile_size = 512;
> -	y_tile_size = 8;
> -	tile_size = x_tile_size * y_tile_size;
> -	tiles_per_line = line_size / x_tile_size;
> +	tile_size = tile_width * tile_height;
> +	tiles_per_line = line_size / tile_width;
>
> -	y_tile_n = y / y_tile_size;
> -	y_tile_off = y % y_tile_size;
> +	tile_y = y / tile_height;
> +	tile_y_off = y % tile_height;
>
> -	x_tile_n = (x * pixel_size) / x_tile_size;
> -	x_tile_off = (x * pixel_size) % x_tile_size;
> +	tile_x = (x * pixel_size) / tile_width;
> +	tile_x_off = (x * pixel_size) % tile_width;
>
> -	tile_n = y_tile_n * tiles_per_line + x_tile_n;
> -	tile_off = y_tile_off * x_tile_size + x_tile_off;
> -	tiled_pos = tile_n * tile_size + tile_off;
> +	tile_n = tile_y * tiles_per_line + tile_x;
> +
> +	if (tiling == I915_TILING_X ) {
> +		tile_off = tile_y_off * tile_width + tile_x_off;
> +	} else { /* (tiling == I915_TILING_Y ) */
> +		int x_oword_n, x_oword_off;
> +		int oword_size = 16;
> +
> +		/* computation inside the tile */
> +		x_oword_n = tile_x_off / oword_size;
> +		x_oword_off = tile_x_off % oword_size;
> +		tile_off = x_oword_n * tile_height * oword_size
> +			   + tile_y_off * oword_size
> +			   + x_oword_off;
> +	}
>
> +	tiled_pos = tile_n * tile_size + tile_off;
>  	tiled_pos = swizzle_addr(tiled_pos, swizzle);
>
>  	return tiled_pos / pixel_size;
> @@ -169,34 +182,48 @@ static int linear_x_y_to_tiled_pos(int x, int y, uint32_t stride, int swizzle,
>
>  /* It's all in "pixel coordinates", so make sure you multiply/divide by the bpp
>   * if you need to. */
> -static void tiled_pos_to_x_y_linear(int tiled_pos, uint32_t stride,
> -				    int swizzle, int bpp, int *x, int *y)
> +static void tiled_pos_to_x_y_linear(int fd, int tiled_pos, uint32_t stride,
> +				    int swizzle, int bpp, int *x, int *y,
> +				    int tiling, uint32_t tile_width,
> +				    uint32_t tile_height)
>  {
> -	int tile_n, tile_off, tiles_per_line, line_size;
> -	int x_tile_off, y_tile_off;
> -	int x_tile_n, y_tile_n;
> -	int x_tile_size, y_tile_size, tile_size;
> +	uint32_t tile_size;
> +	int tile_x, tile_y; /* Co-ordinates of the tile holding the pixel */
> +	int tile_x_off, tile_y_off; /* pixel position inside the tile */
> +	int tile_n, tile_off;
> +	int line_size, tiles_per_line;
>  	int pixel_size = bpp / 8;
>
>  	tiled_pos = swizzle_addr(tiled_pos, swizzle);
>
>  	line_size = stride;
> -	x_tile_size = 512;
> -	y_tile_size = 8;
> -	tile_size = x_tile_size * y_tile_size;
> -	tiles_per_line = line_size / x_tile_size;
> +	tile_size = tile_width * tile_height;
> +	tiles_per_line = line_size / tile_width;
>
>  	tile_n = tiled_pos / tile_size;
>  	tile_off = tiled_pos % tile_size;
>
> -	y_tile_off = tile_off / x_tile_size;
> -	x_tile_off = tile_off % x_tile_size;
> +	tile_x = tile_n % tiles_per_line;
> +	tile_y = tile_n / tiles_per_line;
> +
> +	if (tiling == I915_TILING_X ) {
> +
> +		tile_y_off = tile_off / tile_width;
> +		tile_x_off = tile_off % tile_width;
> +	} else {
> +		int x_oword_n, x_oword_off;
> +		int oword_size = 16;
> +
> +		x_oword_n = tile_off / (oword_size * tile_height);
> +		x_oword_off = tile_off % oword_size;
>
> -	x_tile_n = tile_n % tiles_per_line;
> -	y_tile_n = tile_n / tiles_per_line;
> +		tile_x_off = x_oword_n * oword_size + x_oword_off;
> +		tile_y_off = (tile_off - x_oword_n * oword_size * tile_height)
> +			     / oword_size;
> +	}
>
> -	*x = (x_tile_n * x_tile_size + x_tile_off) / pixel_size;
> -	*y = y_tile_n * y_tile_size + y_tile_off;
> +	*x = (tile_x * tile_width + tile_x_off) / pixel_size;
> +	*y = tile_y * tile_height + tile_y_off;
>  }
>
>  static void set_pixel(void *_ptr, int index, uint32_t color, int bpp)
> @@ -224,15 +251,21 @@ static void draw_rect_ptr_linear(void *ptr, uint32_t stride,
>  	}
>  }
>
> -static void draw_rect_ptr_tiled(void *ptr, uint32_t stride, int swizzle,
> -				struct rect *rect, uint32_t color, int bpp)
> +static void draw_rect_ptr_tiled(int fd, void *ptr, uint32_t stride, int swizzle,
> +				struct rect *rect, uint32_t color, int bpp,
> +				int tiling)
>  {
>  	int x, y, pos;
> +	uint32_t tile_width, tile_height;
> +
> +	igt_get_fb_tile_size(fd, (uint64_t)igt_fb_tiling_to_mod(tiling), bpp,
> +			&tile_width, &tile_height);
>
>  	for (y = rect->y; y < rect->y + rect->h; y++) {
>  		for (x = rect->x; x < rect->x + rect->w; x++) {
> -			pos = linear_x_y_to_tiled_pos(x, y, stride, swizzle,
> -						      bpp);
> +			pos = linear_x_y_to_tiled_pos(fd, x, y, stride, swizzle,
> +						      bpp, tiling, tile_width,
> +						      tile_height);
>  			set_pixel(ptr, pos, color, bpp);
>  		}
>  	}
> @@ -259,8 +292,12 @@ static void draw_rect_mmap_cpu(int fd, struct buf_data *buf, struct rect *rect,
>  		draw_rect_ptr_linear(ptr, buf->stride, rect, color, buf->bpp);
>  		break;
>  	case I915_TILING_X:
> -		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
> -				    buf->bpp);
> +		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle, rect, color,
> +				    buf->bpp, tiling);
> +		break;
> +	case I915_TILING_Y:
> +		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle, rect, color,
> +				    buf->bpp, tiling);
>  		break;
>  	default:
>  		igt_assert(false);
> @@ -309,8 +346,12 @@ static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect *rect,
>  		draw_rect_ptr_linear(ptr, buf->stride, rect, color, buf->bpp);
>  		break;
>  	case I915_TILING_X:
> -		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
> -				    buf->bpp);
> +		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle, rect, color,
> +				    buf->bpp, tiling);
> +		break;
> +	case I915_TILING_Y:
> +		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle, rect, color,
> +				    buf->bpp, tiling);
>  		break;
>  	default:
>  		igt_assert(false);
> @@ -338,7 +379,7 @@ static void draw_rect_pwrite_untiled(int fd, struct buf_data *buf,
>
>  static void draw_rect_pwrite_tiled(int fd, struct buf_data *buf,
>  				   struct rect *rect, uint32_t color,
> -				   uint32_t swizzle)
> +				   uint32_t swizzle, int tiling)
>  {
>  	int i;
>  	int tiled_pos, x, y, pixel_size;
> @@ -347,6 +388,7 @@ static void draw_rect_pwrite_tiled(int fd, struct buf_data *buf,
>  	bool flush_tmp = false;
>  	int tmp_start_pos = 0;
>  	int pixels_written = 0;
> +	uint32_t tile_width, tile_height;
>
>  	/* We didn't implement suport for the older tiling methods yet. */
>  	igt_require(intel_gen(intel_get_drm_devid(fd)) >= 5);
> @@ -360,9 +402,13 @@ static void draw_rect_pwrite_tiled(int fd, struct buf_data *buf,
>  	for (i = 0; i < tmp_size; i++)
>  		set_pixel(tmp, i, color, buf->bpp);
>
> +	igt_get_fb_tile_size(fd, (uint64_t)igt_fb_tiling_to_mod(tiling),
> +			     buf->bpp, &tile_width, &tile_height);
> +
>  	for (tiled_pos = 0; tiled_pos < buf->size; tiled_pos += pixel_size) {
> -		tiled_pos_to_x_y_linear(tiled_pos, buf->stride, swizzle,
> -					buf->bpp, &x, &y);
> +		tiled_pos_to_x_y_linear(fd, tiled_pos, buf->stride, swizzle,
> +					buf->bpp, &x, &y, tiling, tile_width,
> +					tile_height);
>
>  		if (x >= rect->x && x < rect->x + rect->w &&
>  		    y >= rect->y && y < rect->y + rect->h) {
> @@ -399,7 +445,10 @@ static void draw_rect_pwrite(int fd, struct buf_data *buf,
>  		draw_rect_pwrite_untiled(fd, buf, rect, color);
>  		break;
>  	case I915_TILING_X:
> -		draw_rect_pwrite_tiled(fd, buf, rect, color, swizzle);
> +		draw_rect_pwrite_tiled(fd, buf, rect, color, swizzle, tiling);
> +		break;
> +	case I915_TILING_Y:
> +		draw_rect_pwrite_tiled(fd, buf, rect, color, swizzle, tiling);
>  		break;
>  	default:
>  		igt_assert(false);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/7] lib/igt_fb: Let others use igt_get_fb_tile_size

[Paulo Zanoni]

Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
> This function can be used by igt_draw to get accurate
> tile dimensions for all tile formats.
> 
> v2: Added comments to function igt_get_fb_tile_size (Daniel)
> 
> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> ---
>  lib/igt_fb.c | 16 +++++++++++++---
>  lib/igt_fb.h |  3 ++-
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index d2b7e9e..9ba1e3b 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -73,9 +73,19 @@ static struct format_desc_struct {
>  
>  #define for_each_format(f)	\
>  	for (f = format_desc; f - format_desc <
> ARRAY_SIZE(format_desc); f++)
> -

Please don't remove the blank line above.


> -static void igt_get_fb_tile_size(int fd, uint64_t tiling, int
> fb_bpp,
> -				 unsigned *width_ret, unsigned
> *height_ret)
> +/**
> + * igt_get_fb_tile_size:
> + * @fd: The DRM file descriptor
> + * @tiling: Tiling layout of the framebuffer (as framebuffer
> modifier)
> + * @fb_bpp: Bytes per pixel of the framebuffer

Bits, not bytes.


> + * @width_ret: Width of the tile in pixels

I think this is in bytes, not pixels.


> + * @height_ret: Height of the tile in pixels

I think saying "lines" instead of pixels would be more appropriate
here.


Oh, and since we're already going to have to change some other things,
here's an OCD bikeshed: the rest of the file seems to use:
 * @param: description
instead of:
 * @param: Description
so you may opt to use the current standard.


> + *
> + * This function returns width and height of a tile based on the
> given tiling
> + * format.
> + */
> +void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
> +			  unsigned *width_ret, unsigned *height_ret)
>  {
>  	switch (tiling) {
>  	case LOCAL_DRM_FORMAT_MOD_NONE:
> diff --git a/lib/igt_fb.h b/lib/igt_fb.h
> index 4a680ce..414cb3d 100644
> --- a/lib/igt_fb.h
> +++ b/lib/igt_fb.h
> @@ -94,7 +94,8 @@ enum igt_text_align {
>  	align_vcenter	= 0x04,
>  	align_hcenter	= 0x08,
>  };
> 
> -

Please don't remove the blank line above.


Thanks,
Paulo



> +void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
> +			  unsigned *width_ret, unsigned
> *height_ret);
>  void igt_calc_fb_size(int fd, int width, int height, int bpp,
> uint64_t tiling,
>  		      unsigned *size_ret, unsigned *stride_ret);
>  unsigned int
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/7] lib/igt_fb: Add helper function for tile_to_mod

[Paulo Zanoni]

Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
> igt_get_fb_tile_size function takes modifer as an argument
> This helper function will let users to convert tiling to
> modifier and use igt_get_fb_tile_size()
> 
> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> ---
>  lib/igt_fb.c | 26 ++++++++++++++++++++++++++
>  lib/igt_fb.h |  1 +
>  2 files changed, 27 insertions(+)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 9ba1e3b..27baf79 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -216,6 +216,32 @@ uint64_t igt_fb_mod_to_tiling(uint64_t modifier)
>  	}
>  }
>  
> +/**
> + * igt_fb_tiling_to_mod:
> + * @tiling: DRM framebuffer tiling
> + *
> + * This function converts a DRM framebuffer tiling to its
> corresponding
> + * modifier constant.
> + *
> + * Returns:
> + * A tiling constant

A modifier constant.

Besides this, the patch looks good.

> + */
> +uint64_t igt_fb_tiling_to_mod(uint64_t tiling)
> +{
> +	switch (tiling) {
> +	case I915_TILING_NONE:
> +		return LOCAL_DRM_FORMAT_MOD_NONE;
> +	case I915_TILING_X:
> +		return LOCAL_I915_FORMAT_MOD_X_TILED;
> +	case I915_TILING_Y:
> +		return LOCAL_I915_FORMAT_MOD_Y_TILED;
> +	case I915_TILING_Yf:
> +		return LOCAL_I915_FORMAT_MOD_Yf_TILED;
> +	default:
> +		igt_assert(0);
> +	}
> +}
> +
>  /* helpers to create nice-looking framebuffers */
>  static int create_bo_for_fb(int fd, int width, int height, uint32_t
> format,
>  			    uint64_t tiling, unsigned size, unsigned
> stride,
> diff --git a/lib/igt_fb.h b/lib/igt_fb.h
> index 414cb3d..a01671b 100644
> --- a/lib/igt_fb.h
> +++ b/lib/igt_fb.h
> @@ -131,6 +131,7 @@ int igt_create_bo_with_dimensions(int fd, int
> width, int height, uint32_t format
>  				  bool *is_dumb);
>  
>  uint64_t igt_fb_mod_to_tiling(uint64_t modifier);
> +uint64_t igt_fb_tiling_to_mod(uint64_t tiling);
>  
>  /* cairo-based painting */
>  cairo_t *igt_get_cairo_ctx(int fd, struct igt_fb *fb);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/7] tests/kms_draw_crc: add support for Y tiling

[Paulo Zanoni]

Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> This is the program that's supposed to test lib/igt_draw. We just
> added Y tiling support for the library, so add the tests now.
> 

This is not my original version of the patch, yet there's no changelog
mentioning that this is a new version.

https://patchwork.freedesktop.org/patch/72040/

Why were the kmstest_unset_all_crtcs() calls added? My guess is that
they ended up here as a rebase artifact, but please clarify if there's
an actual reason.


> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> ---
>  tests/kms_draw_crc.c | 58 ++++++++++++++++++++++++++++++++++++++--
> ------------
>  1 file changed, 43 insertions(+), 15 deletions(-)
> 
> diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
> index c57d3a3..1d91b48 100644
> --- a/tests/kms_draw_crc.c
> +++ b/tests/kms_draw_crc.c
> @@ -47,6 +47,13 @@ static const uint32_t formats[N_FORMATS] = {
>  	DRM_FORMAT_XRGB2101010,
>  };
>  
> +#define N_TILING_METHODS 3
> +static const uint64_t tilings[N_TILING_METHODS] = {
> +	LOCAL_DRM_FORMAT_MOD_NONE,
> +	LOCAL_I915_FORMAT_MOD_X_TILED,
> +	LOCAL_I915_FORMAT_MOD_Y_TILED,
> +};
> +
>  struct base_crc {
>  	bool set;
>  	igt_crc_t crc;
> @@ -151,6 +158,11 @@ static void draw_method_subtest(enum
> igt_draw_method method,
>  {
>  	igt_crc_t crc;
>  
> +	if (tiling == LOCAL_I915_FORMAT_MOD_Y_TILED)
> +		igt_require(intel_gen(intel_get_drm_devid(drm_fd))
> >= 9);
> +
> +	kmstest_unset_all_crtcs(drm_fd, drm_res);
> +
>  	/* Use IGT_DRAW_MMAP_GTT on an untiled buffer as the
> parameter for
>  	 * comparison. Cache the value so we don't recompute it for
> every single
>  	 * subtest. */
> @@ -208,6 +220,12 @@ static void fill_fb_subtest(void)
>  	get_fill_crc(LOCAL_I915_FORMAT_MOD_X_TILED, &crc);
>  	igt_assert_crc_equal(&crc, &base_crc);
>  
> +	if (intel_gen(intel_get_drm_devid(drm_fd)) >= 9) {
> +		get_fill_crc(LOCAL_I915_FORMAT_MOD_Y_TILED, &crc);
> +		igt_assert_crc_equal(&crc, &base_crc);
> +	}
> +
> +	kmstest_unset_all_crtcs(drm_fd, drm_res);
>  	igt_remove_fb(drm_fd, &fb);
>  }
>  
> @@ -265,28 +283,38 @@ static const char *format_str(int format_index)
>  	}
>  }
>  
> +static const char *tiling_str(int tiling_index)
> +{
> +	switch (tilings[tiling_index]) {
> +	case LOCAL_DRM_FORMAT_MOD_NONE:
> +		return "untiled";
> +	case LOCAL_I915_FORMAT_MOD_X_TILED:
> +		return "xtiled";
> +	case LOCAL_I915_FORMAT_MOD_Y_TILED:
> +		return "ytiled";
> +	default:
> +		igt_assert(false);
> +	}
> +}
> +
>  igt_main
>  {
>  	enum igt_draw_method method;
> -	int format_index;
> +	int format_idx, tiling_idx;
>  
>  	igt_fixture
>  		setup_environment();
>  
> -	for (format_index = 0; format_index < N_FORMATS;
> format_index++) {
> -		for (method = 0; method < IGT_DRAW_METHOD_COUNT;
> method++) {
> -			igt_subtest_f("draw-method-%s-%s-untiled",
> -				      format_str(format_index),
> -				      igt_draw_get_method_name(metho
> d))
> -				draw_method_subtest(method,
> format_index,
> -						    LOCAL_DRM_FORMAT
> _MOD_NONE);
> -			igt_subtest_f("draw-method-%s-%s-tiled",
> -				      format_str(format_index),
> -				      igt_draw_get_method_name(metho
> d))
> -				draw_method_subtest(method,
> format_index,
> -						LOCAL_I915_FORMAT_MO
> D_X_TILED);
> -		}
> -	}
> +	for (format_idx = 0; format_idx < N_FORMATS; format_idx++) {
> +	for (method = 0; method < IGT_DRAW_METHOD_COUNT; method++) {
> +	for (tiling_idx = 0; tiling_idx < N_TILING_METHODS;
> tiling_idx++) {
> +		igt_subtest_f("draw-method-%s-%s-%s",
> +			      format_str(format_idx),
> +			      igt_draw_get_method_name(method),
> +			      tiling_str(tiling_idx))
> +			draw_method_subtest(method, format_idx,
> +					    tilings[tiling_idx]);
> +	} } }
>  
>  	igt_subtest("fill-fb")
>  		fill_fb_subtest();
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/7] tests/kms_draw_crc: add support for Y tiling

[Praveen Paneri]

Hi Paulo,

On Wednesday 12 July 2017 12:33 AM, Paulo Zanoni wrote:
> Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> This is the program that's supposed to test lib/igt_draw. We just
>> added Y tiling support for the library, so add the tests now.
>>
>
> This is not my original version of the patch, yet there's no changelog
> mentioning that this is a new version.
>
> https://patchwork.freedesktop.org/patch/72040/
>
> Why were the kmstest_unset_all_crtcs() calls added? My guess is that
> they ended up here as a rebase artifact, but please clarify if there's
> an actual reason.
Yes this is a rebase artifact and is not actually required. I will fix it.

Thanks,
Praveen
>
>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
>> ---
>>  tests/kms_draw_crc.c | 58 ++++++++++++++++++++++++++++++++++++++--
>> ------------
>>  1 file changed, 43 insertions(+), 15 deletions(-)
>>
>> diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
>> index c57d3a3..1d91b48 100644
>> --- a/tests/kms_draw_crc.c
>> +++ b/tests/kms_draw_crc.c
>> @@ -47,6 +47,13 @@ static const uint32_t formats[N_FORMATS] = {
>>  	DRM_FORMAT_XRGB2101010,
>>  };
>>
>> +#define N_TILING_METHODS 3
>> +static const uint64_t tilings[N_TILING_METHODS] = {
>> +	LOCAL_DRM_FORMAT_MOD_NONE,
>> +	LOCAL_I915_FORMAT_MOD_X_TILED,
>> +	LOCAL_I915_FORMAT_MOD_Y_TILED,
>> +};
>> +
>>  struct base_crc {
>>  	bool set;
>>  	igt_crc_t crc;
>> @@ -151,6 +158,11 @@ static void draw_method_subtest(enum
>> igt_draw_method method,
>>  {
>>  	igt_crc_t crc;
>>
>> +	if (tiling == LOCAL_I915_FORMAT_MOD_Y_TILED)
>> +		igt_require(intel_gen(intel_get_drm_devid(drm_fd))
>>> = 9);
>> +
>> +	kmstest_unset_all_crtcs(drm_fd, drm_res);
>> +
>>  	/* Use IGT_DRAW_MMAP_GTT on an untiled buffer as the
>> parameter for
>>  	 * comparison. Cache the value so we don't recompute it for
>> every single
>>  	 * subtest. */
>> @@ -208,6 +220,12 @@ static void fill_fb_subtest(void)
>>  	get_fill_crc(LOCAL_I915_FORMAT_MOD_X_TILED, &crc);
>>  	igt_assert_crc_equal(&crc, &base_crc);
>>
>> +	if (intel_gen(intel_get_drm_devid(drm_fd)) >= 9) {
>> +		get_fill_crc(LOCAL_I915_FORMAT_MOD_Y_TILED, &crc);
>> +		igt_assert_crc_equal(&crc, &base_crc);
>> +	}
>> +
>> +	kmstest_unset_all_crtcs(drm_fd, drm_res);
>>  	igt_remove_fb(drm_fd, &fb);
>>  }
>>
>> @@ -265,28 +283,38 @@ static const char *format_str(int format_index)
>>  	}
>>  }
>>
>> +static const char *tiling_str(int tiling_index)
>> +{
>> +	switch (tilings[tiling_index]) {
>> +	case LOCAL_DRM_FORMAT_MOD_NONE:
>> +		return "untiled";
>> +	case LOCAL_I915_FORMAT_MOD_X_TILED:
>> +		return "xtiled";
>> +	case LOCAL_I915_FORMAT_MOD_Y_TILED:
>> +		return "ytiled";
>> +	default:
>> +		igt_assert(false);
>> +	}
>> +}
>> +
>>  igt_main
>>  {
>>  	enum igt_draw_method method;
>> -	int format_index;
>> +	int format_idx, tiling_idx;
>>
>>  	igt_fixture
>>  		setup_environment();
>>
>> -	for (format_index = 0; format_index < N_FORMATS;
>> format_index++) {
>> -		for (method = 0; method < IGT_DRAW_METHOD_COUNT;
>> method++) {
>> -			igt_subtest_f("draw-method-%s-%s-untiled",
>> -				      format_str(format_index),
>> -				      igt_draw_get_method_name(metho
>> d))
>> -				draw_method_subtest(method,
>> format_index,
>> -						    LOCAL_DRM_FORMAT
>> _MOD_NONE);
>> -			igt_subtest_f("draw-method-%s-%s-tiled",
>> -				      format_str(format_index),
>> -				      igt_draw_get_method_name(metho
>> d))
>> -				draw_method_subtest(method,
>> format_index,
>> -						LOCAL_I915_FORMAT_MO
>> D_X_TILED);
>> -		}
>> -	}
>> +	for (format_idx = 0; format_idx < N_FORMATS; format_idx++) {
>> +	for (method = 0; method < IGT_DRAW_METHOD_COUNT; method++) {
>> +	for (tiling_idx = 0; tiling_idx < N_TILING_METHODS;
>> tiling_idx++) {
>> +		igt_subtest_f("draw-method-%s-%s-%s",
>> +			      format_str(format_idx),
>> +			      igt_draw_get_method_name(method),
>> +			      tiling_str(tiling_idx))
>> +			draw_method_subtest(method, format_idx,
>> +					    tilings[tiling_idx]);
>> +	} } }
>>
>>  	igt_subtest("fill-fb")
>>  		fill_fb_subtest();
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 6/7] igt/kms_frontbuffer_tracking: Add Y-tiling support

[Paulo Zanoni]

Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
> Allow tests to create Y-tiled bufferes using a separate
> argument to the test without increasing the number of
> subtests.
> 
> v2: Changed tiling option to string (Paulo)

I had some minor nitpicks for this patch: reshuffling parameters
between lines and the way we handle the new argument and its help
message. Instead of requesting those trivial changes and creating yet
another review round-trip I just went ahead and implemented them as v3.
I hope you're fine with this approach. The patch can be applied without
the rest of the patches in the series, so I applied it & pushed to IGT
with my nitpicks and R-B.


> 
> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> ---
>  tests/kms_frontbuffer_tracking.c | 48 ++++++++++++++++++++++++----
> ------------
>  1 file changed, 29 insertions(+), 19 deletions(-)
> 
> diff --git a/tests/kms_frontbuffer_tracking.c
> b/tests/kms_frontbuffer_tracking.c
> index 7cea4de..62ae33a 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -252,6 +252,7 @@ struct {
>  	int only_pipes;
>  	int shared_fb_x_offset;
>  	int shared_fb_y_offset;
> +	uint64_t tiling;
>  } opt = {
>  	.check_status = true,
>  	.check_crc = true,
> @@ -264,6 +265,7 @@ struct {
>  	.only_pipes = PIPE_COUNT,
>  	.shared_fb_x_offset = 500,
>  	.shared_fb_y_offset = 500,
> +	.tiling = LOCAL_I915_FORMAT_MOD_X_TILED,
>  };
>  
>  struct modeset_params {
> @@ -578,7 +580,7 @@ static void create_fb(enum pixel_format pformat,
> int width, int height,
>  	if (plane == PLANE_CUR)
>  		tiling_for_size = LOCAL_DRM_FORMAT_MOD_NONE;
>  	else
> -		tiling_for_size = LOCAL_I915_FORMAT_MOD_X_TILED;
> +		tiling_for_size = opt.tiling;
>  
>  	igt_calc_fb_size(drm.fd, width, height, bpp,
> tiling_for_size, &size,
>  			 &stride);
> @@ -710,7 +712,7 @@ static void create_shared_fb(enum pixel_format
> format)
>  
>  	big_h = prim_h + scnd_h + offs_h + opt.shared_fb_y_offset;
>  
> -	create_fb(format, big_w, big_h,
> LOCAL_I915_FORMAT_MOD_X_TILED,
> +	create_fb(format, big_w, big_h, opt.tiling,
>  		  PLANE_PRI, &s->big);
>  }
>  
> @@ -744,16 +746,16 @@ static void create_fbs(enum pixel_format
> format)
>  
>  	create_fb(format, prim_mode_params.mode->hdisplay,
>  		  prim_mode_params.mode->vdisplay,
> -		  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_PRI, &s-
> >prim_pri);
> +		  opt.tiling, PLANE_PRI, &s->prim_pri);
>  	create_fb(format, prim_mode_params.cursor.w,
>  		  prim_mode_params.cursor.h,
> LOCAL_DRM_FORMAT_MOD_NONE,
>  		  PLANE_CUR, &s->prim_cur);
>  	create_fb(format, prim_mode_params.sprite.w,
> -		  prim_mode_params.sprite.h,
> LOCAL_I915_FORMAT_MOD_X_TILED,
> +		  prim_mode_params.sprite.h, opt.tiling,
>  		  PLANE_SPR, &s->prim_spr);
>  
>  	create_fb(format, offscreen_fb.w, offscreen_fb.h,
> -		  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_PRI, &s-
> >offscreen);
> +		  opt.tiling, PLANE_PRI, &s->offscreen);
>  
>  	create_shared_fb(format);
>  
> @@ -762,11 +764,11 @@ static void create_fbs(enum pixel_format
> format)
>  
>  	create_fb(format, scnd_mode_params.mode->hdisplay,
>  		  scnd_mode_params.mode->vdisplay,
> -		  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_PRI, &s-
> >scnd_pri);
> +		  opt.tiling, PLANE_PRI, &s->scnd_pri);
>  	create_fb(format, scnd_mode_params.cursor.w,
> scnd_mode_params.cursor.h,
>  		  LOCAL_DRM_FORMAT_MOD_NONE, PLANE_CUR, &s-
> >scnd_cur);
>  	create_fb(format, scnd_mode_params.sprite.w,
> scnd_mode_params.sprite.h,
> -		  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_SPR, &s-
> >scnd_spr);
> +		  opt.tiling, PLANE_SPR, &s->scnd_spr);
>  }
>  
>  static bool set_mode_for_params(struct modeset_params *params)
> @@ -1252,7 +1254,7 @@ static void init_blue_crc(enum pixel_format
> format, bool mandatory_sink_crc)
>  
>  	create_fb(format, prim_mode_params.mode->hdisplay,
>  		  prim_mode_params.mode->vdisplay,
> -		  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_PRI, &blue);
> +		  opt.tiling, PLANE_PRI, &blue);
>  
>  	fill_fb(&blue, COLOR_PRIM_BG);
>  
> @@ -1287,7 +1289,7 @@ static void init_crcs(enum pixel_format format,
>  	for (r = 0; r < pattern->n_rects; r++)
>  		create_fb(format, prim_mode_params.mode->hdisplay,
>  			  prim_mode_params.mode->vdisplay,
> -			  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_PRI,
> &tmp_fbs[r]);
> +			  opt.tiling, PLANE_PRI, &tmp_fbs[r]);
>  
>  	for (r = 0; r < pattern->n_rects; r++)
>  		fill_fb(&tmp_fbs[r], COLOR_PRIM_BG);
> @@ -2386,7 +2388,7 @@ static void flip_subtest(const struct test_mode
> *t)
>  	prepare_subtest(t, pattern);
>  
>  	create_fb(t->format, params->fb.fb->width, params->fb.fb-
> >height,
> -		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &fb2);
> +		  opt.tiling, t->plane, &fb2);
>  	fill_fb(&fb2, bg_color);
>  	orig_fb = params->fb.fb;
>  
> @@ -2432,7 +2434,7 @@ static void fliptrack_subtest(const struct
> test_mode *t, enum flip_type type)
>  	prepare_subtest(t, pattern);
>  
>  	create_fb(t->format, params->fb.fb->width, params->fb.fb-
> >height,
> -		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &fb2);
> +		  opt.tiling, t->plane, &fb2);
>  	fill_fb(&fb2, COLOR_PRIM_BG);
>  	orig_fb = params->fb.fb;
>  
> @@ -2643,7 +2645,7 @@ static void fullscreen_plane_subtest(const
> struct test_mode *t)
>  	prepare_subtest(t, pattern);
>  
>  	rect = pattern->get_rect(&params->fb, 0);
> -	create_fb(t->format, rect.w, rect.h,
> LOCAL_I915_FORMAT_MOD_X_TILED,
> +	create_fb(t->format, rect.w, rect.h, opt.tiling,
>  		  t->plane, &fullscreen_fb);
>  	/* Call pick_color() again since PRI and SPR may not support
> the same
>  	 * pixel formats. */
> @@ -2722,7 +2724,7 @@ static void scaledprimary_subtest(const struct
> test_mode *t)
>  	old_fb = params->fb.fb;
>  
>  	create_fb(t->format, params->fb.fb->width, params->fb.fb-
> >height,
> -		  LOCAL_I915_FORMAT_MOD_X_TILED,
> +		  opt.tiling,
>  		  t->plane, &new_fb);
>  	fill_fb(&new_fb, COLOR_BLUE);
>  
> @@ -2832,7 +2834,7 @@ static void modesetfrombusy_subtest(const
> struct test_mode *t)
>  	prepare_subtest(t, NULL);
>  
>  	create_fb(t->format, params->fb.fb->width, params->fb.fb-
> >height,
> -		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &fb2);
> +		  opt.tiling, t->plane, &fb2);
>  	fill_fb(&fb2, COLOR_PRIM_BG);
>  
>  	start_busy_thread(params->fb.fb);
> @@ -2937,7 +2939,7 @@ static void farfromfence_subtest(const struct
> test_mode *t)
>  	target = pick_target(t, params);
>  
>  	create_fb(t->format, params->mode->hdisplay, max_height,
> -		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane,
> &tall_fb);
> +		  opt.tiling, t->plane, &tall_fb);
>  
>  	fill_fb(&tall_fb, COLOR_PRIM_BG);
>  
> @@ -3012,7 +3014,7 @@ static void badstride_subtest(const struct
> test_mode *t)
>  	old_fb = params->fb.fb;
>  
>  	create_fb(t->format, params->fb.fb->width + 4096, params-
> >fb.fb->height,
> -		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane,
> &wide_fb);
> +		  opt.tiling, t->plane, &wide_fb);
>  	igt_assert(wide_fb.stride > 16384);
>  
>  	fill_fb(&wide_fb, COLOR_PRIM_BG);
> @@ -3079,7 +3081,7 @@ static void stridechange_subtest(const struct
> test_mode *t)
>  	old_fb = params->fb.fb;
>  
>  	create_fb(t->format, params->fb.fb->width + 512, params-
> >fb.fb->height,
> -		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &new_fb);
> +		  opt.tiling, t->plane, &new_fb);
>  	fill_fb(&new_fb, COLOR_PRIM_BG);
>  
>  	igt_assert(old_fb->stride != new_fb.stride);
> @@ -3198,7 +3200,7 @@ static void basic_subtest(const struct
> test_mode *t)
>  	prepare_subtest(t, pattern);
>  
>  	create_fb(t->format, params->fb.fb->width, params->fb.fb-
> >height,
> -		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &fb2);
> +		  opt.tiling, t->plane, &fb2);
>  	fb1 = params->fb.fb;
>  
>  	for (r = 0, method = 0; method < IGT_DRAW_METHOD_COUNT;
> method++, r++) {
> @@ -3267,6 +3269,12 @@ static int opt_handler(int option, int
> option_index, void *data)
>  		igt_assert_eq(opt.only_pipes, PIPE_COUNT);
>  		opt.only_pipes = PIPE_DUAL;
>  		break;
> +	case 'l':
> +		if (!strncmp(optarg, "y", 1))
> +			opt.tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
> +		else
> +			opt.tiling = LOCAL_I915_FORMAT_MOD_X_TILED;
> +		break;
>  	default:
>  		igt_assert(false);
>  	}
> @@ -3286,7 +3294,8 @@ const char *help_str =
>  "  --shared-fb-x offset        Use 'offset' as the X offset for the
> shared FB\n"
>  "  --shared-fb-y offset        Use 'offset' as the Y offset for the
> shared FB\n"
>  "  --1p-only                   Only run subtests that use 1 pipe\n"
> -"  --2p-only                   Only run subtests that use 2
> pipes\n";
> +"  --2p-only                   Only run subtests that use 2 pipes\n"
> +"  --tiling                    Select tiling mode-'x' or 'y'\n";
>  
>  static const char *pipes_str(int pipes)
>  {
> @@ -3425,6 +3434,7 @@ int main(int argc, char *argv[])
>  		{ "shared-fb-y",              1, 0, 'y'},
>  		{ "1p-only",                  0, 0, '1'},
>  		{ "2p-only",                  0, 0, '2'},
> +		{ "tiling",                   1, 0, 'l'},
>  		{ 0, 0, 0, 0 }
>  	};
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 7/7] igt/kms_fbc_crc.c : Add Y-tile tests

[Paulo Zanoni]

Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
> Now that we have support for Y-tiled buffers, add another
> iteration of tests for Y-tiled buffers.

Have you tested this on platforms that don't support Y-tiled buffers? I
don't see a check for that, so I wonder if we'll just fail some
assertion or correctly hit some igt_skip() call I couldn't find.

More below.

> 
> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> ---
>  tests/kms_fbc_crc.c | 71 +++++++++++++++++++++++++++++------------
> ------------
>  1 file changed, 39 insertions(+), 32 deletions(-)
> 
> diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
> index 7964e05..cdf04c1 100644
> --- a/tests/kms_fbc_crc.c
> +++ b/tests/kms_fbc_crc.c
> @@ -318,12 +318,10 @@ static void prepare_crtc(data_t *data)
>  	igt_output_set_pipe(output, data->pipe);
>  }
>  
> -static void create_fbs(data_t *data, bool tiled, struct igt_fb *fbs)
> +static void create_fbs(data_t *data, uint64_t tiling, struct igt_fb
> *fbs)
>  {
>  	int rc;
>  	drmModeModeInfo *mode = igt_output_get_mode(data->output);
> -	uint64_t tiling = tiled ? LOCAL_I915_FORMAT_MOD_X_TILED :
> -				  LOCAL_DRM_FORMAT_MOD_NONE;
>  
>  	rc = igt_create_color_fb(data->drm_fd, mode->hdisplay, mode-
> >vdisplay,
>  				 DRM_FORMAT_XRGB8888, tiling,
> @@ -344,8 +342,8 @@ static void get_ref_crcs(data_t *data)
>  	struct igt_fb fbs[4];
>  	int i;
>  
> -	create_fbs(data, false, &fbs[0]);
> -	create_fbs(data, false, &fbs[2]);
> +	create_fbs(data, LOCAL_DRM_FORMAT_MOD_NONE, &fbs[0]);
> +	create_fbs(data, LOCAL_DRM_FORMAT_MOD_NONE, &fbs[2]);
>  
>  	fill_mmap_gtt(data, fbs[2].gem_handle, 0xff);
>  	fill_mmap_gtt(data, fbs[3].gem_handle, 0xff);
> @@ -366,7 +364,7 @@ static void get_ref_crcs(data_t *data)
>  		igt_remove_fb(data->drm_fd, &fbs[i]);
>  }
>  
> -static bool prepare_test(data_t *data, enum test_mode test_mode)
> +static bool prepare_test(data_t *data, enum test_mode test_mode,
> uint64_t tiling)
>  {
>  	igt_display_t *display = &data->display;
>  	igt_output_t *output = data->output;
> @@ -374,7 +372,7 @@ static bool prepare_test(data_t *data, enum
> test_mode test_mode)
>  
>  	data->primary = igt_output_get_plane_type(data->output,
> DRM_PLANE_TYPE_PRIMARY);
>  
> -	create_fbs(data, true, data->fb);
> +	create_fbs(data, tiling, data->fb);
>  
>  	igt_pipe_crc_free(data->pipe_crc);
>  	data->pipe_crc = NULL;
> @@ -484,32 +482,41 @@ static void run_test(data_t *data, enum
> test_mode mode)
>  
>  	reset_display(data);
>  
> -	for_each_pipe_with_valid_output(display, data->pipe, data-
> >output) {
> -		prepare_crtc(data);
> -
> -		igt_info("Beginning %s on pipe %s, connector %s\n",
> -			  igt_subtest_name(),
> -			  kmstest_pipe_name(data->pipe),
> -			  igt_output_name(data->output));
> -
> -		if (!prepare_test(data, mode)) {
> -			igt_info("%s on pipe %s, connector %s:
> SKIPPED\n",
> -				  igt_subtest_name(),
> -				  kmstest_pipe_name(data->pipe),
> -				  igt_output_name(data->output));
> -			continue;
> +	for (int tiling = I915_TILING_X;
> +	     tiling <= I915_TILING_Y; tiling++) {

What I don't understand is why this part of the code chooses to go with
the tiling constants (I915_TILING_) only to later convert them to
modifiers with igt_fb_tiling_to_mod(). If this loop iterated over the
modifiers directly we wouldn't need that. The rest of the code only
cares about the modifiers.


> +		for_each_pipe_with_valid_output(display,
> +						data->pipe, data-
> >output) {
> +			prepare_crtc(data);
> +
> +			igt_info("Beginning %s on pipe %s, connector
> "
> +					"%s, %s-tiled\n",
> +					igt_subtest_name(),
> +					kmstest_pipe_name(data-
> >pipe),
> +					igt_output_name(data-
> >output),
> +					(tiling == I915_TILING_X) ?
> "x":"y" );

This change is not keeping the indentation style, things should be
aligned with the parens (although I see they're actually aligned with
the quote, which is also weird). The same can be said for the other two
igt_info() calls in this patch.


> +
> +			if (!prepare_test(data, mode,
> +					  igt_fb_tiling_to_mod(tilin
> g))) {
> +				igt_info("%s on pipe %s, connector
> %s: SKIPPED\n",
> +						igt_subtest_name(),
> +						kmstest_pipe_name(da
> ta->pipe),
> +						igt_output_name(data
> ->output));

This one is missing the "%s-tiled" part that was added in the other two
messages.

And we can probably create a "const char *tiling_name" variable to
store the %s part in order to avoid the same ternary operator in the 3
if statements.


> +				continue;
> +			}
> +
> +			valid_tests++;
> +
> +			test_crc(data, mode);
> +
> +			igt_info("%s on pipe %s, connector %s"
> +					"%s-tiled: PASSED\n",
> +					igt_subtest_name(),
> +					kmstest_pipe_name(data-
> >pipe),
> +					igt_output_name(data-
> >output),
> +					(tiling == I915_TILING_X) ?
> "x":"y" );
> +
> +			finish_crtc(data, mode);
>  		}
> -
> -		valid_tests++;
> -
> -		test_crc(data, mode);
> -
> -		igt_info("%s on pipe %s, connector %s: PASSED\n",
> -			  igt_subtest_name(),
> -			  kmstest_pipe_name(data->pipe),
> -			  igt_output_name(data->output));
> -
> -		finish_crtc(data, mode);
>  	}
>  
>  	igt_require_f(valid_tests, "no valid crtc/connector
> combinations found\n");
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 4/7] lib/igt_draw: Add Y-tiling support for IGT_DRAW_BLT method

[Paulo Zanoni]

Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
> From: Akash Goel <akash.goel@intel.com>
> 
> v2: Moved identical code into a single function (Paulo)

My MI-fu is not very strong, but I tried to check this against BSpec
and I believe this patch is correct. I also tested it, and it
definitely solves the problems with the BLT operations on Y tiling.

There's only a minor nitpick with a line going beyond 80 columns, but I
can fix that while applying.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> 
> Signed-off-by: Akash Goel <akash.goel@intel.com>
> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> ---
>  lib/igt_draw.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/lib/igt_draw.c b/lib/igt_draw.c
> index fcf8fba..27a69cd 100644
> --- a/lib/igt_draw.c
> +++ b/lib/igt_draw.c
> @@ -31,6 +31,7 @@
>  #include "igt_core.h"
>  #include "igt_fb.h"
>  #include "ioctl_wrappers.h"
> +#include "i830_reg.h"
>  
>  /**
>   * SECTION:igt_draw
> @@ -242,6 +243,34 @@ static void set_pixel(void *_ptr, int index,
> uint32_t color, int bpp)
>  	}
>  }
>  
> +static void switch_blt_tiling(struct intel_batchbuffer *batch,
> uint32_t tiling, bool on)
> +{
> +	uint32_t bcs_swctrl;
> +
> +	/* Default is X-tile */
> +	if (tiling != I915_TILING_Y)
> +		return;
> +
> +	bcs_swctrl = (0x3 << 16) | (on ? 0x3 : 0x0);
> +
> +	/* To change the tile register, insert an MI_FLUSH_DW
> followed by an
> +	 * MI_LOAD_REGISTER_IMM
> +	 */
> +	BEGIN_BATCH(4, 0);
> +	OUT_BATCH(MI_FLUSH_DW | 2);
> +	OUT_BATCH(0x0);
> +	OUT_BATCH(0x0);
> +	OUT_BATCH(0x0);
> +	ADVANCE_BATCH();
> +
> +	BEGIN_BATCH(4, 0);
> +	OUT_BATCH(MI_LOAD_REGISTER_IMM);
> +	OUT_BATCH(0x22200); /* BCS_SWCTRL */
> +	OUT_BATCH(bcs_swctrl);
> +	OUT_BATCH(MI_NOOP);
> +	ADVANCE_BATCH();
> +}
> +
>  static void draw_rect_ptr_linear(void *ptr, uint32_t stride,
>  				 struct rect *rect, uint32_t color,
> int bpp)
>  {
> @@ -487,6 +516,8 @@ static void draw_rect_blt(int fd, struct cmd_data
> *cmd_data,
>  	blt_cmd_tiling = (tiling) ? XY_COLOR_BLT_TILED : 0;
>  	pitch = (tiling) ? buf->stride / 4 : buf->stride;
>  
> +	switch_blt_tiling(batch, tiling, true);
> +
>  	BEGIN_BATCH(6, 1);
>  	OUT_BATCH(XY_COLOR_BLT_CMD_NOLEN | XY_COLOR_BLT_WRITE_ALPHA
> |
>  		  XY_COLOR_BLT_WRITE_RGB | blt_cmd_tiling |
> blt_cmd_len);
> @@ -497,6 +528,8 @@ static void draw_rect_blt(int fd, struct cmd_data
> *cmd_data,
>  	OUT_BATCH(color);
>  	ADVANCE_BATCH();
>  
> +	switch_blt_tiling(batch, tiling, false);
> +
>  	intel_batchbuffer_flush(batch);
>  	intel_batchbuffer_free(batch);
>  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/7] tests/kms_draw_crc: add support for Y tiling

[Paulo Zanoni]

Em Qua, 2017-07-12 às 13:45 +0530, Praveen Paneri escreveu:
> Hi Paulo,
> 
> On Wednesday 12 July 2017 12:33 AM, Paulo Zanoni wrote:
> > Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
> > > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > 
> > > This is the program that's supposed to test lib/igt_draw. We just
> > > added Y tiling support for the library, so add the tests now.
> > > 
> > 
> > This is not my original version of the patch, yet there's no
> > changelog
> > mentioning that this is a new version.
> > 
> > https://patchwork.freedesktop.org/patch/72040/
> > 
> > Why were the kmstest_unset_all_crtcs() calls added? My guess is
> > that
> > they ended up here as a rebase artifact, but please clarify if
> > there's
> > an actual reason.
> 
> Yes this is a rebase artifact and is not actually required. I will
> fix it.

Or you can just give a reviewed-by tag to my original patch :).

> 
> Thanks,
> Praveen
> > 
> > 
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> > > ---
> > >  tests/kms_draw_crc.c | 58
> > > ++++++++++++++++++++++++++++++++++++++--
> > > ------------
> > >  1 file changed, 43 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
> > > index c57d3a3..1d91b48 100644
> > > --- a/tests/kms_draw_crc.c
> > > +++ b/tests/kms_draw_crc.c
> > > @@ -47,6 +47,13 @@ static const uint32_t formats[N_FORMATS] = {
> > >  	DRM_FORMAT_XRGB2101010,
> > >  };
> > > 
> > > +#define N_TILING_METHODS 3
> > > +static const uint64_t tilings[N_TILING_METHODS] = {
> > > +	LOCAL_DRM_FORMAT_MOD_NONE,
> > > +	LOCAL_I915_FORMAT_MOD_X_TILED,
> > > +	LOCAL_I915_FORMAT_MOD_Y_TILED,
> > > +};
> > > +
> > >  struct base_crc {
> > >  	bool set;
> > >  	igt_crc_t crc;
> > > @@ -151,6 +158,11 @@ static void draw_method_subtest(enum
> > > igt_draw_method method,
> > >  {
> > >  	igt_crc_t crc;
> > > 
> > > +	if (tiling == LOCAL_I915_FORMAT_MOD_Y_TILED)
> > > +		igt_require(intel_gen(intel_get_drm_devid(drm_fd
> > > ))
> > > > = 9);
> > > 
> > > +
> > > +	kmstest_unset_all_crtcs(drm_fd, drm_res);
> > > +
> > >  	/* Use IGT_DRAW_MMAP_GTT on an untiled buffer as the
> > > parameter for
> > >  	 * comparison. Cache the value so we don't recompute it
> > > for
> > > every single
> > >  	 * subtest. */
> > > @@ -208,6 +220,12 @@ static void fill_fb_subtest(void)
> > >  	get_fill_crc(LOCAL_I915_FORMAT_MOD_X_TILED, &crc);
> > >  	igt_assert_crc_equal(&crc, &base_crc);
> > > 
> > > +	if (intel_gen(intel_get_drm_devid(drm_fd)) >= 9) {
> > > +		get_fill_crc(LOCAL_I915_FORMAT_MOD_Y_TILED,
> > > &crc);
> > > +		igt_assert_crc_equal(&crc, &base_crc);
> > > +	}
> > > +
> > > +	kmstest_unset_all_crtcs(drm_fd, drm_res);
> > >  	igt_remove_fb(drm_fd, &fb);
> > >  }
> > > 
> > > @@ -265,28 +283,38 @@ static const char *format_str(int
> > > format_index)
> > >  	}
> > >  }
> > > 
> > > +static const char *tiling_str(int tiling_index)
> > > +{
> > > +	switch (tilings[tiling_index]) {
> > > +	case LOCAL_DRM_FORMAT_MOD_NONE:
> > > +		return "untiled";
> > > +	case LOCAL_I915_FORMAT_MOD_X_TILED:
> > > +		return "xtiled";
> > > +	case LOCAL_I915_FORMAT_MOD_Y_TILED:
> > > +		return "ytiled";
> > > +	default:
> > > +		igt_assert(false);
> > > +	}
> > > +}
> > > +
> > >  igt_main
> > >  {
> > >  	enum igt_draw_method method;
> > > -	int format_index;
> > > +	int format_idx, tiling_idx;
> > > 
> > >  	igt_fixture
> > >  		setup_environment();
> > > 
> > > -	for (format_index = 0; format_index < N_FORMATS;
> > > format_index++) {
> > > -		for (method = 0; method < IGT_DRAW_METHOD_COUNT;
> > > method++) {
> > > -			igt_subtest_f("draw-method-%s-%s-
> > > untiled",
> > > -				      format_str(format_index),
> > > -				      igt_draw_get_method_name(m
> > > etho
> > > d))
> > > -				draw_method_subtest(method,
> > > format_index,
> > > -						    LOCAL_DRM_FO
> > > RMAT
> > > _MOD_NONE);
> > > -			igt_subtest_f("draw-method-%s-%s-tiled",
> > > -				      format_str(format_index),
> > > -				      igt_draw_get_method_name(m
> > > etho
> > > d))
> > > -				draw_method_subtest(method,
> > > format_index,
> > > -						LOCAL_I915_FORMA
> > > T_MO
> > > D_X_TILED);
> > > -		}
> > > -	}
> > > +	for (format_idx = 0; format_idx < N_FORMATS;
> > > format_idx++) {
> > > +	for (method = 0; method < IGT_DRAW_METHOD_COUNT;
> > > method++) {
> > > +	for (tiling_idx = 0; tiling_idx < N_TILING_METHODS;
> > > tiling_idx++) {
> > > +		igt_subtest_f("draw-method-%s-%s-%s",
> > > +			      format_str(format_idx),
> > > +			      igt_draw_get_method_name(method),
> > > +			      tiling_str(tiling_idx))
> > > +			draw_method_subtest(method, format_idx,
> > > +					    tilings[tiling_idx])
> > > ;
> > > +	} } }
> > > 
> > >  	igt_subtest("fill-fb")
> > >  		fill_fb_subtest();
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] lib/igt_draw: Add Y-tiling support

[Paulo Zanoni]

Em Sex, 2017-06-09 às 15:48 +0530, Praveen Paneri escreveu:
> This patch adds Y-tiling support for igt_draw_rect function.
> 
> v2: Use helper function to get tile sizes (Ville)
> 
> v3: Moved igt_get_fb_tile_size() out of the for loop
>  for better performance (Paulo)

For some reason I thought this series was using my original patch, but
I realized it's not:

https://patchwork.freedesktop.org/patch/72039/

Now that I read our past emails, I wonder that maybe I didn't send you
this specific patch of that series. I'm sorry for that. Obviously I'm
biased towards my version. Which one do you think is better? Would you
be willing to give my version a r-b, in case you think it's better?

Thanks,
Paulo


> 
> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> ---
>  lib/igt_draw.c | 139 ++++++++++++++++++++++++++++++++++++++---------
> ----------
>  1 file changed, 94 insertions(+), 45 deletions(-)
> 
> diff --git a/lib/igt_draw.c b/lib/igt_draw.c
> index 29aec85..2138bf7 100644
> --- a/lib/igt_draw.c
> +++ b/lib/igt_draw.c
> @@ -136,32 +136,45 @@ static int swizzle_addr(int addr, int swizzle)
>  
>  /* It's all in "pixel coordinates", so make sure you multiply/divide
> by the bpp
>   * if you need to. */
> -static int linear_x_y_to_tiled_pos(int x, int y, uint32_t stride,
> int swizzle,
> -				   int bpp)
> +static int linear_x_y_to_tiled_pos(int fd, int x, int y, uint32_t
> stride, int swizzle,
> +				   int bpp, int tiling, uint32_t
> tile_width,
> +				   uint32_t tile_height)
>  {
> -	int x_tile_size, y_tile_size;
> -	int x_tile_n, y_tile_n, x_tile_off, y_tile_off;
> -	int line_size, tile_size;
> +	uint32_t tile_size;
> +	int tile_x, tile_y; /* Co-ordinates of the tile holding the
> pixel */
> +	int tile_x_off, tile_y_off; /* pixel position inside the
> tile */
>  	int tile_n, tile_off;
> -	int tiled_pos, tiles_per_line;
> +	int line_size, tiles_per_line;
> +	int tiled_pos;
>  	int pixel_size = bpp / 8;
>  
>  	line_size = stride;
> -	x_tile_size = 512;
> -	y_tile_size = 8;
> -	tile_size = x_tile_size * y_tile_size;
> -	tiles_per_line = line_size / x_tile_size;
> +	tile_size = tile_width * tile_height;
> +	tiles_per_line = line_size / tile_width;
>  
> -	y_tile_n = y / y_tile_size;
> -	y_tile_off = y % y_tile_size;
> +	tile_y = y / tile_height;
> +	tile_y_off = y % tile_height;
>  
> -	x_tile_n = (x * pixel_size) / x_tile_size;
> -	x_tile_off = (x * pixel_size) % x_tile_size;
> +	tile_x = (x * pixel_size) / tile_width;
> +	tile_x_off = (x * pixel_size) % tile_width;
>  
> -	tile_n = y_tile_n * tiles_per_line + x_tile_n;
> -	tile_off = y_tile_off * x_tile_size + x_tile_off;
> -	tiled_pos = tile_n * tile_size + tile_off;
> +	tile_n = tile_y * tiles_per_line + tile_x;
> +
> +	if (tiling == I915_TILING_X ) {
> +		tile_off = tile_y_off * tile_width + tile_x_off;
> +	} else { /* (tiling == I915_TILING_Y ) */
> +		int x_oword_n, x_oword_off;
> +		int oword_size = 16;
> +
> +		/* computation inside the tile */
> +		x_oword_n = tile_x_off / oword_size;
> +		x_oword_off = tile_x_off % oword_size;
> +		tile_off = x_oword_n * tile_height * oword_size
> +			   + tile_y_off * oword_size
> +			   + x_oword_off;
> +	}
>  
> +	tiled_pos = tile_n * tile_size + tile_off;
>  	tiled_pos = swizzle_addr(tiled_pos, swizzle);
>  
>  	return tiled_pos / pixel_size;
> @@ -169,34 +182,48 @@ static int linear_x_y_to_tiled_pos(int x, int
> y, uint32_t stride, int swizzle,
>  
>  /* It's all in "pixel coordinates", so make sure you multiply/divide
> by the bpp
>   * if you need to. */
> -static void tiled_pos_to_x_y_linear(int tiled_pos, uint32_t stride,
> -				    int swizzle, int bpp, int *x,
> int *y)
> +static void tiled_pos_to_x_y_linear(int fd, int tiled_pos, uint32_t
> stride,
> +				    int swizzle, int bpp, int *x,
> int *y,
> +				    int tiling, uint32_t tile_width,
> +				    uint32_t tile_height)
>  {
> -	int tile_n, tile_off, tiles_per_line, line_size;
> -	int x_tile_off, y_tile_off;
> -	int x_tile_n, y_tile_n;
> -	int x_tile_size, y_tile_size, tile_size;
> +	uint32_t tile_size;
> +	int tile_x, tile_y; /* Co-ordinates of the tile holding the
> pixel */
> +	int tile_x_off, tile_y_off; /* pixel position inside the
> tile */
> +	int tile_n, tile_off;
> +	int line_size, tiles_per_line;
>  	int pixel_size = bpp / 8;
>  
>  	tiled_pos = swizzle_addr(tiled_pos, swizzle);
>  
>  	line_size = stride;
> -	x_tile_size = 512;
> -	y_tile_size = 8;
> -	tile_size = x_tile_size * y_tile_size;
> -	tiles_per_line = line_size / x_tile_size;
> +	tile_size = tile_width * tile_height;
> +	tiles_per_line = line_size / tile_width;
>  
>  	tile_n = tiled_pos / tile_size;
>  	tile_off = tiled_pos % tile_size;
>  
> -	y_tile_off = tile_off / x_tile_size;
> -	x_tile_off = tile_off % x_tile_size;
> +	tile_x = tile_n % tiles_per_line;
> +	tile_y = tile_n / tiles_per_line;
> +
> +	if (tiling == I915_TILING_X ) {
> +
> +		tile_y_off = tile_off / tile_width;
> +		tile_x_off = tile_off % tile_width;
> +	} else {
> +		int x_oword_n, x_oword_off;
> +		int oword_size = 16;
> +
> +		x_oword_n = tile_off / (oword_size * tile_height);
> +		x_oword_off = tile_off % oword_size;
>  
> -	x_tile_n = tile_n % tiles_per_line;
> -	y_tile_n = tile_n / tiles_per_line;
> +		tile_x_off = x_oword_n * oword_size + x_oword_off;
> +		tile_y_off = (tile_off - x_oword_n * oword_size *
> tile_height)
> +			     / oword_size;
> +	}
>  
> -	*x = (x_tile_n * x_tile_size + x_tile_off) / pixel_size;
> -	*y = y_tile_n * y_tile_size + y_tile_off;
> +	*x = (tile_x * tile_width + tile_x_off) / pixel_size;
> +	*y = tile_y * tile_height + tile_y_off;
>  }
>  
>  static void set_pixel(void *_ptr, int index, uint32_t color, int
> bpp)
> @@ -224,15 +251,21 @@ static void draw_rect_ptr_linear(void *ptr,
> uint32_t stride,
>  	}
>  }
>  
> -static void draw_rect_ptr_tiled(void *ptr, uint32_t stride, int
> swizzle,
> -				struct rect *rect, uint32_t color,
> int bpp)
> +static void draw_rect_ptr_tiled(int fd, void *ptr, uint32_t stride,
> int swizzle,
> +				struct rect *rect, uint32_t color,
> int bpp,
> +				int tiling)
>  {
>  	int x, y, pos;
> +	uint32_t tile_width, tile_height;
> +
> +	igt_get_fb_tile_size(fd,
> (uint64_t)igt_fb_tiling_to_mod(tiling), bpp,
> +			&tile_width, &tile_height);
>  
>  	for (y = rect->y; y < rect->y + rect->h; y++) {
>  		for (x = rect->x; x < rect->x + rect->w; x++) {
> -			pos = linear_x_y_to_tiled_pos(x, y, stride,
> swizzle,
> -						      bpp);
> +			pos = linear_x_y_to_tiled_pos(fd, x, y,
> stride, swizzle,
> +						      bpp, tiling,
> tile_width,
> +						      tile_height);
>  			set_pixel(ptr, pos, color, bpp);
>  		}
>  	}
> @@ -259,8 +292,12 @@ static void draw_rect_mmap_cpu(int fd, struct
> buf_data *buf, struct rect *rect,
>  		draw_rect_ptr_linear(ptr, buf->stride, rect, color,
> buf->bpp);
>  		break;
>  	case I915_TILING_X:
> -		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect,
> color,
> -				    buf->bpp);
> +		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle,
> rect, color,
> +				    buf->bpp, tiling);
> +		break;
> +	case I915_TILING_Y:
> +		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle,
> rect, color,
> +				    buf->bpp, tiling);
>  		break;
>  	default:
>  		igt_assert(false);
> @@ -309,8 +346,12 @@ static void draw_rect_mmap_wc(int fd, struct
> buf_data *buf, struct rect *rect,
>  		draw_rect_ptr_linear(ptr, buf->stride, rect, color,
> buf->bpp);
>  		break;
>  	case I915_TILING_X:
> -		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect,
> color,
> -				    buf->bpp);
> +		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle,
> rect, color,
> +				    buf->bpp, tiling);
> +		break;
> +	case I915_TILING_Y:
> +		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle,
> rect, color,
> +				    buf->bpp, tiling);
>  		break;
>  	default:
>  		igt_assert(false);
> @@ -338,7 +379,7 @@ static void draw_rect_pwrite_untiled(int fd,
> struct buf_data *buf,
>  
>  static void draw_rect_pwrite_tiled(int fd, struct buf_data *buf,
>  				   struct rect *rect, uint32_t
> color,
> -				   uint32_t swizzle)
> +				   uint32_t swizzle, int tiling)
>  {
>  	int i;
>  	int tiled_pos, x, y, pixel_size;
> @@ -347,6 +388,7 @@ static void draw_rect_pwrite_tiled(int fd, struct
> buf_data *buf,
>  	bool flush_tmp = false;
>  	int tmp_start_pos = 0;
>  	int pixels_written = 0;
> +	uint32_t tile_width, tile_height;
>  
>  	/* We didn't implement suport for the older tiling methods
> yet. */
>  	igt_require(intel_gen(intel_get_drm_devid(fd)) >= 5);
> @@ -360,9 +402,13 @@ static void draw_rect_pwrite_tiled(int fd,
> struct buf_data *buf,
>  	for (i = 0; i < tmp_size; i++)
>  		set_pixel(tmp, i, color, buf->bpp);
>  
> +	igt_get_fb_tile_size(fd,
> (uint64_t)igt_fb_tiling_to_mod(tiling),
> +			     buf->bpp, &tile_width, &tile_height);
> +
>  	for (tiled_pos = 0; tiled_pos < buf->size; tiled_pos +=
> pixel_size) {
> -		tiled_pos_to_x_y_linear(tiled_pos, buf->stride,
> swizzle,
> -					buf->bpp, &x, &y);
> +		tiled_pos_to_x_y_linear(fd, tiled_pos, buf->stride,
> swizzle,
> +					buf->bpp, &x, &y, tiling,
> tile_width,
> +					tile_height);
>  
>  		if (x >= rect->x && x < rect->x + rect->w &&
>  		    y >= rect->y && y < rect->y + rect->h) {
> @@ -399,7 +445,10 @@ static void draw_rect_pwrite(int fd, struct
> buf_data *buf,
>  		draw_rect_pwrite_untiled(fd, buf, rect, color);
>  		break;
>  	case I915_TILING_X:
> -		draw_rect_pwrite_tiled(fd, buf, rect, color,
> swizzle);
> +		draw_rect_pwrite_tiled(fd, buf, rect, color,
> swizzle, tiling);
> +		break;
> +	case I915_TILING_Y:
> +		draw_rect_pwrite_tiled(fd, buf, rect, color,
> swizzle, tiling);
>  		break;
>  	default:
>  		igt_assert(false);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 6/7] igt/kms_frontbuffer_tracking: Add Y-tiling support

[Praveen Paneri]

Hi Paulo,

On Thursday 13 July 2017 01:47 AM, Paulo Zanoni wrote:
> Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
>> Allow tests to create Y-tiled bufferes using a separate
>> argument to the test without increasing the number of
>> subtests.
>>
>> v2: Changed tiling option to string (Paulo)
>
> I had some minor nitpicks for this patch: reshuffling parameters
> between lines and the way we handle the new argument and its help
> message. Instead of requesting those trivial changes and creating yet
> another review round-trip I just went ahead and implemented them as v3.
> I hope you're fine with this approach. The patch can be applied without
> the rest of the patches in the series, so I applied it & pushed to IGT
> with my nitpicks and R-B.
>
>
Thank you for your review

Praveen
>>
>> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
>> ---
>>  tests/kms_frontbuffer_tracking.c | 48 ++++++++++++++++++++++++----
>> ------------
>>  1 file changed, 29 insertions(+), 19 deletions(-)
>>
>> diff --git a/tests/kms_frontbuffer_tracking.c
>> b/tests/kms_frontbuffer_tracking.c
>> index 7cea4de..62ae33a 100644
>> --- a/tests/kms_frontbuffer_tracking.c
>> +++ b/tests/kms_frontbuffer_tracking.c
>> @@ -252,6 +252,7 @@ struct {
>>  	int only_pipes;
>>  	int shared_fb_x_offset;
>>  	int shared_fb_y_offset;
>> +	uint64_t tiling;
>>  } opt = {
>>  	.check_status = true,
>>  	.check_crc = true,
>> @@ -264,6 +265,7 @@ struct {
>>  	.only_pipes = PIPE_COUNT,
>>  	.shared_fb_x_offset = 500,
>>  	.shared_fb_y_offset = 500,
>> +	.tiling = LOCAL_I915_FORMAT_MOD_X_TILED,
>>  };
>>
>>  struct modeset_params {
>> @@ -578,7 +580,7 @@ static void create_fb(enum pixel_format pformat,
>> int width, int height,
>>  	if (plane == PLANE_CUR)
>>  		tiling_for_size = LOCAL_DRM_FORMAT_MOD_NONE;
>>  	else
>> -		tiling_for_size = LOCAL_I915_FORMAT_MOD_X_TILED;
>> +		tiling_for_size = opt.tiling;
>>
>>  	igt_calc_fb_size(drm.fd, width, height, bpp,
>> tiling_for_size, &size,
>>  			 &stride);
>> @@ -710,7 +712,7 @@ static void create_shared_fb(enum pixel_format
>> format)
>>
>>  	big_h = prim_h + scnd_h + offs_h + opt.shared_fb_y_offset;
>>
>> -	create_fb(format, big_w, big_h,
>> LOCAL_I915_FORMAT_MOD_X_TILED,
>> +	create_fb(format, big_w, big_h, opt.tiling,
>>  		  PLANE_PRI, &s->big);
>>  }
>>
>> @@ -744,16 +746,16 @@ static void create_fbs(enum pixel_format
>> format)
>>
>>  	create_fb(format, prim_mode_params.mode->hdisplay,
>>  		  prim_mode_params.mode->vdisplay,
>> -		  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_PRI, &s-
>>> prim_pri);
>> +		  opt.tiling, PLANE_PRI, &s->prim_pri);
>>  	create_fb(format, prim_mode_params.cursor.w,
>>  		  prim_mode_params.cursor.h,
>> LOCAL_DRM_FORMAT_MOD_NONE,
>>  		  PLANE_CUR, &s->prim_cur);
>>  	create_fb(format, prim_mode_params.sprite.w,
>> -		  prim_mode_params.sprite.h,
>> LOCAL_I915_FORMAT_MOD_X_TILED,
>> +		  prim_mode_params.sprite.h, opt.tiling,
>>  		  PLANE_SPR, &s->prim_spr);
>>
>>  	create_fb(format, offscreen_fb.w, offscreen_fb.h,
>> -		  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_PRI, &s-
>>> offscreen);
>> +		  opt.tiling, PLANE_PRI, &s->offscreen);
>>
>>  	create_shared_fb(format);
>>
>> @@ -762,11 +764,11 @@ static void create_fbs(enum pixel_format
>> format)
>>
>>  	create_fb(format, scnd_mode_params.mode->hdisplay,
>>  		  scnd_mode_params.mode->vdisplay,
>> -		  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_PRI, &s-
>>> scnd_pri);
>> +		  opt.tiling, PLANE_PRI, &s->scnd_pri);
>>  	create_fb(format, scnd_mode_params.cursor.w,
>> scnd_mode_params.cursor.h,
>>  		  LOCAL_DRM_FORMAT_MOD_NONE, PLANE_CUR, &s-
>>> scnd_cur);
>>  	create_fb(format, scnd_mode_params.sprite.w,
>> scnd_mode_params.sprite.h,
>> -		  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_SPR, &s-
>>> scnd_spr);
>> +		  opt.tiling, PLANE_SPR, &s->scnd_spr);
>>  }
>>
>>  static bool set_mode_for_params(struct modeset_params *params)
>> @@ -1252,7 +1254,7 @@ static void init_blue_crc(enum pixel_format
>> format, bool mandatory_sink_crc)
>>
>>  	create_fb(format, prim_mode_params.mode->hdisplay,
>>  		  prim_mode_params.mode->vdisplay,
>> -		  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_PRI, &blue);
>> +		  opt.tiling, PLANE_PRI, &blue);
>>
>>  	fill_fb(&blue, COLOR_PRIM_BG);
>>
>> @@ -1287,7 +1289,7 @@ static void init_crcs(enum pixel_format format,
>>  	for (r = 0; r < pattern->n_rects; r++)
>>  		create_fb(format, prim_mode_params.mode->hdisplay,
>>  			  prim_mode_params.mode->vdisplay,
>> -			  LOCAL_I915_FORMAT_MOD_X_TILED, PLANE_PRI,
>> &tmp_fbs[r]);
>> +			  opt.tiling, PLANE_PRI, &tmp_fbs[r]);
>>
>>  	for (r = 0; r < pattern->n_rects; r++)
>>  		fill_fb(&tmp_fbs[r], COLOR_PRIM_BG);
>> @@ -2386,7 +2388,7 @@ static void flip_subtest(const struct test_mode
>> *t)
>>  	prepare_subtest(t, pattern);
>>
>>  	create_fb(t->format, params->fb.fb->width, params->fb.fb-
>>> height,
>> -		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &fb2);
>> +		  opt.tiling, t->plane, &fb2);
>>  	fill_fb(&fb2, bg_color);
>>  	orig_fb = params->fb.fb;
>>
>> @@ -2432,7 +2434,7 @@ static void fliptrack_subtest(const struct
>> test_mode *t, enum flip_type type)
>>  	prepare_subtest(t, pattern);
>>
>>  	create_fb(t->format, params->fb.fb->width, params->fb.fb-
>>> height,
>> -		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &fb2);
>> +		  opt.tiling, t->plane, &fb2);
>>  	fill_fb(&fb2, COLOR_PRIM_BG);
>>  	orig_fb = params->fb.fb;
>>
>> @@ -2643,7 +2645,7 @@ static void fullscreen_plane_subtest(const
>> struct test_mode *t)
>>  	prepare_subtest(t, pattern);
>>
>>  	rect = pattern->get_rect(&params->fb, 0);
>> -	create_fb(t->format, rect.w, rect.h,
>> LOCAL_I915_FORMAT_MOD_X_TILED,
>> +	create_fb(t->format, rect.w, rect.h, opt.tiling,
>>  		  t->plane, &fullscreen_fb);
>>  	/* Call pick_color() again since PRI and SPR may not support
>> the same
>>  	 * pixel formats. */
>> @@ -2722,7 +2724,7 @@ static void scaledprimary_subtest(const struct
>> test_mode *t)
>>  	old_fb = params->fb.fb;
>>
>>  	create_fb(t->format, params->fb.fb->width, params->fb.fb-
>>> height,
>> -		  LOCAL_I915_FORMAT_MOD_X_TILED,
>> +		  opt.tiling,
>>  		  t->plane, &new_fb);
>>  	fill_fb(&new_fb, COLOR_BLUE);
>>
>> @@ -2832,7 +2834,7 @@ static void modesetfrombusy_subtest(const
>> struct test_mode *t)
>>  	prepare_subtest(t, NULL);
>>
>>  	create_fb(t->format, params->fb.fb->width, params->fb.fb-
>>> height,
>> -		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &fb2);
>> +		  opt.tiling, t->plane, &fb2);
>>  	fill_fb(&fb2, COLOR_PRIM_BG);
>>
>>  	start_busy_thread(params->fb.fb);
>> @@ -2937,7 +2939,7 @@ static void farfromfence_subtest(const struct
>> test_mode *t)
>>  	target = pick_target(t, params);
>>
>>  	create_fb(t->format, params->mode->hdisplay, max_height,
>> -		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane,
>> &tall_fb);
>> +		  opt.tiling, t->plane, &tall_fb);
>>
>>  	fill_fb(&tall_fb, COLOR_PRIM_BG);
>>
>> @@ -3012,7 +3014,7 @@ static void badstride_subtest(const struct
>> test_mode *t)
>>  	old_fb = params->fb.fb;
>>
>>  	create_fb(t->format, params->fb.fb->width + 4096, params-
>>> fb.fb->height,
>> -		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane,
>> &wide_fb);
>> +		  opt.tiling, t->plane, &wide_fb);
>>  	igt_assert(wide_fb.stride > 16384);
>>
>>  	fill_fb(&wide_fb, COLOR_PRIM_BG);
>> @@ -3079,7 +3081,7 @@ static void stridechange_subtest(const struct
>> test_mode *t)
>>  	old_fb = params->fb.fb;
>>
>>  	create_fb(t->format, params->fb.fb->width + 512, params-
>>> fb.fb->height,
>> -		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &new_fb);
>> +		  opt.tiling, t->plane, &new_fb);
>>  	fill_fb(&new_fb, COLOR_PRIM_BG);
>>
>>  	igt_assert(old_fb->stride != new_fb.stride);
>> @@ -3198,7 +3200,7 @@ static void basic_subtest(const struct
>> test_mode *t)
>>  	prepare_subtest(t, pattern);
>>
>>  	create_fb(t->format, params->fb.fb->width, params->fb.fb-
>>> height,
>> -		  LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &fb2);
>> +		  opt.tiling, t->plane, &fb2);
>>  	fb1 = params->fb.fb;
>>
>>  	for (r = 0, method = 0; method < IGT_DRAW_METHOD_COUNT;
>> method++, r++) {
>> @@ -3267,6 +3269,12 @@ static int opt_handler(int option, int
>> option_index, void *data)
>>  		igt_assert_eq(opt.only_pipes, PIPE_COUNT);
>>  		opt.only_pipes = PIPE_DUAL;
>>  		break;
>> +	case 'l':
>> +		if (!strncmp(optarg, "y", 1))
>> +			opt.tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
>> +		else
>> +			opt.tiling = LOCAL_I915_FORMAT_MOD_X_TILED;
>> +		break;
>>  	default:
>>  		igt_assert(false);
>>  	}
>> @@ -3286,7 +3294,8 @@ const char *help_str =
>>  "  --shared-fb-x offset        Use 'offset' as the X offset for the
>> shared FB\n"
>>  "  --shared-fb-y offset        Use 'offset' as the Y offset for the
>> shared FB\n"
>>  "  --1p-only                   Only run subtests that use 1 pipe\n"
>> -"  --2p-only                   Only run subtests that use 2
>> pipes\n";
>> +"  --2p-only                   Only run subtests that use 2 pipes\n"
>> +"  --tiling                    Select tiling mode-'x' or 'y'\n";
>>
>>  static const char *pipes_str(int pipes)
>>  {
>> @@ -3425,6 +3434,7 @@ int main(int argc, char *argv[])
>>  		{ "shared-fb-y",              1, 0, 'y'},
>>  		{ "1p-only",                  0, 0, '1'},
>>  		{ "2p-only",                  0, 0, '2'},
>> +		{ "tiling",                   1, 0, 'l'},
>>  		{ 0, 0, 0, 0 }
>>  	};
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 7/7] igt/kms_fbc_crc.c : Add Y-tile tests

[Praveen Paneri]

Hi Paulo,

On Thursday 13 July 2017 02:31 AM, Paulo Zanoni wrote:
> Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
>> Now that we have support for Y-tiled buffers, add another
>> iteration of tests for Y-tiled buffers.
>
> Have you tested this on platforms that don't support Y-tiled buffers? I
Unfortunately I haven't...
> don't see a check for that, so I wonder if we'll just fail some
> assertion or correctly hit some igt_skip() call I couldn't find.
...but I will add the check as you have mentioned
>
> More below.
>
>>
>> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
>> ---
>>  tests/kms_fbc_crc.c | 71 +++++++++++++++++++++++++++++------------
>> ------------
>>  1 file changed, 39 insertions(+), 32 deletions(-)
>>
>> diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
>> index 7964e05..cdf04c1 100644
>> --- a/tests/kms_fbc_crc.c
>> +++ b/tests/kms_fbc_crc.c
>> @@ -318,12 +318,10 @@ static void prepare_crtc(data_t *data)
>>  	igt_output_set_pipe(output, data->pipe);
>>  }
>>
>> -static void create_fbs(data_t *data, bool tiled, struct igt_fb *fbs)
>> +static void create_fbs(data_t *data, uint64_t tiling, struct igt_fb
>> *fbs)
>>  {
>>  	int rc;
>>  	drmModeModeInfo *mode = igt_output_get_mode(data->output);
>> -	uint64_t tiling = tiled ? LOCAL_I915_FORMAT_MOD_X_TILED :
>> -				  LOCAL_DRM_FORMAT_MOD_NONE;
>>
>>  	rc = igt_create_color_fb(data->drm_fd, mode->hdisplay, mode-
>>> vdisplay,
>>  				 DRM_FORMAT_XRGB8888, tiling,
>> @@ -344,8 +342,8 @@ static void get_ref_crcs(data_t *data)
>>  	struct igt_fb fbs[4];
>>  	int i;
>>
>> -	create_fbs(data, false, &fbs[0]);
>> -	create_fbs(data, false, &fbs[2]);
>> +	create_fbs(data, LOCAL_DRM_FORMAT_MOD_NONE, &fbs[0]);
>> +	create_fbs(data, LOCAL_DRM_FORMAT_MOD_NONE, &fbs[2]);
>>
>>  	fill_mmap_gtt(data, fbs[2].gem_handle, 0xff);
>>  	fill_mmap_gtt(data, fbs[3].gem_handle, 0xff);
>> @@ -366,7 +364,7 @@ static void get_ref_crcs(data_t *data)
>>  		igt_remove_fb(data->drm_fd, &fbs[i]);
>>  }
>>
>> -static bool prepare_test(data_t *data, enum test_mode test_mode)
>> +static bool prepare_test(data_t *data, enum test_mode test_mode,
>> uint64_t tiling)
>>  {
>>  	igt_display_t *display = &data->display;
>>  	igt_output_t *output = data->output;
>> @@ -374,7 +372,7 @@ static bool prepare_test(data_t *data, enum
>> test_mode test_mode)
>>
>>  	data->primary = igt_output_get_plane_type(data->output,
>> DRM_PLANE_TYPE_PRIMARY);
>>
>> -	create_fbs(data, true, data->fb);
>> +	create_fbs(data, tiling, data->fb);
>>
>>  	igt_pipe_crc_free(data->pipe_crc);
>>  	data->pipe_crc = NULL;
>> @@ -484,32 +482,41 @@ static void run_test(data_t *data, enum
>> test_mode mode)
>>
>>  	reset_display(data);
>>
>> -	for_each_pipe_with_valid_output(display, data->pipe, data-
>>> output) {
>> -		prepare_crtc(data);
>> -
>> -		igt_info("Beginning %s on pipe %s, connector %s\n",
>> -			  igt_subtest_name(),
>> -			  kmstest_pipe_name(data->pipe),
>> -			  igt_output_name(data->output));
>> -
>> -		if (!prepare_test(data, mode)) {
>> -			igt_info("%s on pipe %s, connector %s:
>> SKIPPED\n",
>> -				  igt_subtest_name(),
>> -				  kmstest_pipe_name(data->pipe),
>> -				  igt_output_name(data->output));
>> -			continue;
>> +	for (int tiling = I915_TILING_X;
>> +	     tiling <= I915_TILING_Y; tiling++) {
>
> What I don't understand is why this part of the code chooses to go with
> the tiling constants (I915_TILING_) only to later convert them to
> modifiers with igt_fb_tiling_to_mod(). If this loop iterated over the
> modifiers directly we wouldn't need that. The rest of the code only
> cares about the modifiers.
I chose to loop over tiling constants as they are in a simple arithmetic 
order. anyhow I will just change that.

Also as mentioned above can I just add a check to skip Y-tiling tests 
for older platforms?

igt_skip_on(intel_gen(intel_get_drm_devid(drm_fd)) < 9 &&
                                tiling == I915_TILING_Y);

>
>
>> +		for_each_pipe_with_valid_output(display,
>> +						data->pipe, data-
>>> output) {
>> +			prepare_crtc(data);
>> +
>> +			igt_info("Beginning %s on pipe %s, connector
>> "
>> +					"%s, %s-tiled\n",
>> +					igt_subtest_name(),
>> +					kmstest_pipe_name(data-
>>> pipe),
>> +					igt_output_name(data-
>>> output),
>> +					(tiling == I915_TILING_X) ?
>> "x":"y" );
>
> This change is not keeping the indentation style, things should be
> aligned with the parens (although I see they're actually aligned with
> the quote, which is also weird). The same can be said for the other two
> igt_info() calls in this patch.
Will fix it
>
>
>> +
>> +			if (!prepare_test(data, mode,
>> +					  igt_fb_tiling_to_mod(tilin
>> g))) {
>> +				igt_info("%s on pipe %s, connector
>> %s: SKIPPED\n",
>> +						igt_subtest_name(),
>> +						kmstest_pipe_name(da
>> ta->pipe),
>> +						igt_output_name(data
>> ->output));
>
> This one is missing the "%s-tiled" part that was added in the other two
> messages.
>
> And we can probably create a "const char *tiling_name" variable to
> store the %s part in order to avoid the same ternary operator in the 3
> if statements.
make sense, will add.
>
>
>> +				continue;
>> +			}
>> +
>> +			valid_tests++;
>> +
>> +			test_crc(data, mode);
>> +
>> +			igt_info("%s on pipe %s, connector %s"
>> +					"%s-tiled: PASSED\n",
>> +					igt_subtest_name(),
>> +					kmstest_pipe_name(data-
>>> pipe),
>> +					igt_output_name(data-
>>> output),
>> +					(tiling == I915_TILING_X) ?
>> "x":"y" );
>> +
>> +			finish_crtc(data, mode);
>>  		}
>> -
>> -		valid_tests++;
>> -
>> -		test_crc(data, mode);
>> -
>> -		igt_info("%s on pipe %s, connector %s: PASSED\n",
>> -			  igt_subtest_name(),
>> -			  kmstest_pipe_name(data->pipe),
>> -			  igt_output_name(data->output));
>> -
>> -		finish_crtc(data, mode);
>>  	}
>>
>>  	igt_require_f(valid_tests, "no valid crtc/connector
>> combinations found\n");
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 4/7] lib/igt_draw: Add Y-tiling support for IGT_DRAW_BLT method

[Praveen Paneri]

On Friday 14 July 2017 01:29 AM, Paulo Zanoni wrote:
> Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
>> From: Akash Goel <akash.goel@intel.com>
>>
>> v2: Moved identical code into a single function (Paulo)
>
> My MI-fu is not very strong, but I tried to check this against BSpec
> and I believe this patch is correct. I also tested it, and it
> definitely solves the problems with the BLT operations on Y tiling.
>
> There's only a minor nitpick with a line going beyond 80 columns, but I
> can fix that while applying.
>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Thanks
Praveen
>
>>
>> Signed-off-by: Akash Goel <akash.goel@intel.com>
>> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
>> ---
>>  lib/igt_draw.c | 33 +++++++++++++++++++++++++++++++++
>>  1 file changed, 33 insertions(+)
>>
>> diff --git a/lib/igt_draw.c b/lib/igt_draw.c
>> index fcf8fba..27a69cd 100644
>> --- a/lib/igt_draw.c
>> +++ b/lib/igt_draw.c
>> @@ -31,6 +31,7 @@
>>  #include "igt_core.h"
>>  #include "igt_fb.h"
>>  #include "ioctl_wrappers.h"
>> +#include "i830_reg.h"
>>
>>  /**
>>   * SECTION:igt_draw
>> @@ -242,6 +243,34 @@ static void set_pixel(void *_ptr, int index,
>> uint32_t color, int bpp)
>>  	}
>>  }
>>
>> +static void switch_blt_tiling(struct intel_batchbuffer *batch,
>> uint32_t tiling, bool on)
>> +{
>> +	uint32_t bcs_swctrl;
>> +
>> +	/* Default is X-tile */
>> +	if (tiling != I915_TILING_Y)
>> +		return;
>> +
>> +	bcs_swctrl = (0x3 << 16) | (on ? 0x3 : 0x0);
>> +
>> +	/* To change the tile register, insert an MI_FLUSH_DW
>> followed by an
>> +	 * MI_LOAD_REGISTER_IMM
>> +	 */
>> +	BEGIN_BATCH(4, 0);
>> +	OUT_BATCH(MI_FLUSH_DW | 2);
>> +	OUT_BATCH(0x0);
>> +	OUT_BATCH(0x0);
>> +	OUT_BATCH(0x0);
>> +	ADVANCE_BATCH();
>> +
>> +	BEGIN_BATCH(4, 0);
>> +	OUT_BATCH(MI_LOAD_REGISTER_IMM);
>> +	OUT_BATCH(0x22200); /* BCS_SWCTRL */
>> +	OUT_BATCH(bcs_swctrl);
>> +	OUT_BATCH(MI_NOOP);
>> +	ADVANCE_BATCH();
>> +}
>> +
>>  static void draw_rect_ptr_linear(void *ptr, uint32_t stride,
>>  				 struct rect *rect, uint32_t color,
>> int bpp)
>>  {
>> @@ -487,6 +516,8 @@ static void draw_rect_blt(int fd, struct cmd_data
>> *cmd_data,
>>  	blt_cmd_tiling = (tiling) ? XY_COLOR_BLT_TILED : 0;
>>  	pitch = (tiling) ? buf->stride / 4 : buf->stride;
>>
>> +	switch_blt_tiling(batch, tiling, true);
>> +
>>  	BEGIN_BATCH(6, 1);
>>  	OUT_BATCH(XY_COLOR_BLT_CMD_NOLEN | XY_COLOR_BLT_WRITE_ALPHA
>> |
>>  		  XY_COLOR_BLT_WRITE_RGB | blt_cmd_tiling |
>> blt_cmd_len);
>> @@ -497,6 +528,8 @@ static void draw_rect_blt(int fd, struct cmd_data
>> *cmd_data,
>>  	OUT_BATCH(color);
>>  	ADVANCE_BATCH();
>>
>> +	switch_blt_tiling(batch, tiling, false);
>> +
>>  	intel_batchbuffer_flush(batch);
>>  	intel_batchbuffer_free(batch);
>>  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/7] tests/kms_draw_crc: add support for Y tiling

[Praveen Paneri]

On Friday 14 July 2017 01:49 AM, Paulo Zanoni wrote:
> Em Qua, 2017-07-12 às 13:45 +0530, Praveen Paneri escreveu:
>> Hi Paulo,
>>
>> On Wednesday 12 July 2017 12:33 AM, Paulo Zanoni wrote:
>>> Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
>>>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>>>
>>>> This is the program that's supposed to test lib/igt_draw. We just
>>>> added Y tiling support for the library, so add the tests now.
>>>>
>>>
>>> This is not my original version of the patch, yet there's no
>>> changelog
>>> mentioning that this is a new version.
>>>
>>> https://patchwork.freedesktop.org/patch/72040/
>>>
>>> Why were the kmstest_unset_all_crtcs() calls added? My guess is
>>> that
>>> they ended up here as a rebase artifact, but please clarify if
>>> there's
>>> an actual reason.
>>
>> Yes this is a rebase artifact and is not actually required. I will
>> fix it.
>
> Or you can just give a reviewed-by tag to my original patch :).
Sure! I will add that in the next series :)
>
>>
>> Thanks,
>> Praveen
>>>
>>>
>>>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>>> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
>>>> ---
>>>>  tests/kms_draw_crc.c | 58
>>>> ++++++++++++++++++++++++++++++++++++++--
>>>> ------------
>>>>  1 file changed, 43 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
>>>> index c57d3a3..1d91b48 100644
>>>> --- a/tests/kms_draw_crc.c
>>>> +++ b/tests/kms_draw_crc.c
>>>> @@ -47,6 +47,13 @@ static const uint32_t formats[N_FORMATS] = {
>>>>  	DRM_FORMAT_XRGB2101010,
>>>>  };
>>>>
>>>> +#define N_TILING_METHODS 3
>>>> +static const uint64_t tilings[N_TILING_METHODS] = {
>>>> +	LOCAL_DRM_FORMAT_MOD_NONE,
>>>> +	LOCAL_I915_FORMAT_MOD_X_TILED,
>>>> +	LOCAL_I915_FORMAT_MOD_Y_TILED,
>>>> +};
>>>> +
>>>>  struct base_crc {
>>>>  	bool set;
>>>>  	igt_crc_t crc;
>>>> @@ -151,6 +158,11 @@ static void draw_method_subtest(enum
>>>> igt_draw_method method,
>>>>  {
>>>>  	igt_crc_t crc;
>>>>
>>>> +	if (tiling == LOCAL_I915_FORMAT_MOD_Y_TILED)
>>>> +		igt_require(intel_gen(intel_get_drm_devid(drm_fd
>>>> ))
>>>>> = 9);
>>>>
>>>> +
>>>> +	kmstest_unset_all_crtcs(drm_fd, drm_res);
>>>> +
>>>>  	/* Use IGT_DRAW_MMAP_GTT on an untiled buffer as the
>>>> parameter for
>>>>  	 * comparison. Cache the value so we don't recompute it
>>>> for
>>>> every single
>>>>  	 * subtest. */
>>>> @@ -208,6 +220,12 @@ static void fill_fb_subtest(void)
>>>>  	get_fill_crc(LOCAL_I915_FORMAT_MOD_X_TILED, &crc);
>>>>  	igt_assert_crc_equal(&crc, &base_crc);
>>>>
>>>> +	if (intel_gen(intel_get_drm_devid(drm_fd)) >= 9) {
>>>> +		get_fill_crc(LOCAL_I915_FORMAT_MOD_Y_TILED,
>>>> &crc);
>>>> +		igt_assert_crc_equal(&crc, &base_crc);
>>>> +	}
>>>> +
>>>> +	kmstest_unset_all_crtcs(drm_fd, drm_res);
>>>>  	igt_remove_fb(drm_fd, &fb);
>>>>  }
>>>>
>>>> @@ -265,28 +283,38 @@ static const char *format_str(int
>>>> format_index)
>>>>  	}
>>>>  }
>>>>
>>>> +static const char *tiling_str(int tiling_index)
>>>> +{
>>>> +	switch (tilings[tiling_index]) {
>>>> +	case LOCAL_DRM_FORMAT_MOD_NONE:
>>>> +		return "untiled";
>>>> +	case LOCAL_I915_FORMAT_MOD_X_TILED:
>>>> +		return "xtiled";
>>>> +	case LOCAL_I915_FORMAT_MOD_Y_TILED:
>>>> +		return "ytiled";
>>>> +	default:
>>>> +		igt_assert(false);
>>>> +	}
>>>> +}
>>>> +
>>>>  igt_main
>>>>  {
>>>>  	enum igt_draw_method method;
>>>> -	int format_index;
>>>> +	int format_idx, tiling_idx;
>>>>
>>>>  	igt_fixture
>>>>  		setup_environment();
>>>>
>>>> -	for (format_index = 0; format_index < N_FORMATS;
>>>> format_index++) {
>>>> -		for (method = 0; method < IGT_DRAW_METHOD_COUNT;
>>>> method++) {
>>>> -			igt_subtest_f("draw-method-%s-%s-
>>>> untiled",
>>>> -				      format_str(format_index),
>>>> -				      igt_draw_get_method_name(m
>>>> etho
>>>> d))
>>>> -				draw_method_subtest(method,
>>>> format_index,
>>>> -						    LOCAL_DRM_FO
>>>> RMAT
>>>> _MOD_NONE);
>>>> -			igt_subtest_f("draw-method-%s-%s-tiled",
>>>> -				      format_str(format_index),
>>>> -				      igt_draw_get_method_name(m
>>>> etho
>>>> d))
>>>> -				draw_method_subtest(method,
>>>> format_index,
>>>> -						LOCAL_I915_FORMA
>>>> T_MO
>>>> D_X_TILED);
>>>> -		}
>>>> -	}
>>>> +	for (format_idx = 0; format_idx < N_FORMATS;
>>>> format_idx++) {
>>>> +	for (method = 0; method < IGT_DRAW_METHOD_COUNT;
>>>> method++) {
>>>> +	for (tiling_idx = 0; tiling_idx < N_TILING_METHODS;
>>>> tiling_idx++) {
>>>> +		igt_subtest_f("draw-method-%s-%s-%s",
>>>> +			      format_str(format_idx),
>>>> +			      igt_draw_get_method_name(method),
>>>> +			      tiling_str(tiling_idx))
>>>> +			draw_method_subtest(method, format_idx,
>>>> +					    tilings[tiling_idx])
>>>> ;
>>>> +	} } }
>>>>
>>>>  	igt_subtest("fill-fb")
>>>>  		fill_fb_subtest();
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] lib/igt_draw: Add Y-tiling support

[Praveen Paneri]

On Friday 14 July 2017 03:03 AM, Paulo Zanoni wrote:
> Em Sex, 2017-06-09 às 15:48 +0530, Praveen Paneri escreveu:
>> This patch adds Y-tiling support for igt_draw_rect function.
>>
>> v2: Use helper function to get tile sizes (Ville)
>>
>> v3: Moved igt_get_fb_tile_size() out of the for loop
>>  for better performance (Paulo)
>
> For some reason I thought this series was using my original patch, but
> I realized it's not:
>
> https://patchwork.freedesktop.org/patch/72039/
>
> Now that I read our past emails, I wonder that maybe I didn't send you
> this specific patch of that series. I'm sorry for that. Obviously I'm
> biased towards my version. Which one do you think is better? Would you
> be willing to give my version a r-b, in case you think it's better?
Your version looks better. I will add my r-b and post it
Thanks
Praveen
>
> Thanks,
> Paulo
>
>
>>
>> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
>> ---
>>  lib/igt_draw.c | 139 ++++++++++++++++++++++++++++++++++++++---------
>> ----------
>>  1 file changed, 94 insertions(+), 45 deletions(-)
>>
>> diff --git a/lib/igt_draw.c b/lib/igt_draw.c
>> index 29aec85..2138bf7 100644
>> --- a/lib/igt_draw.c
>> +++ b/lib/igt_draw.c
>> @@ -136,32 +136,45 @@ static int swizzle_addr(int addr, int swizzle)
>>
>>  /* It's all in "pixel coordinates", so make sure you multiply/divide
>> by the bpp
>>   * if you need to. */
>> -static int linear_x_y_to_tiled_pos(int x, int y, uint32_t stride,
>> int swizzle,
>> -				   int bpp)
>> +static int linear_x_y_to_tiled_pos(int fd, int x, int y, uint32_t
>> stride, int swizzle,
>> +				   int bpp, int tiling, uint32_t
>> tile_width,
>> +				   uint32_t tile_height)
>>  {
>> -	int x_tile_size, y_tile_size;
>> -	int x_tile_n, y_tile_n, x_tile_off, y_tile_off;
>> -	int line_size, tile_size;
>> +	uint32_t tile_size;
>> +	int tile_x, tile_y; /* Co-ordinates of the tile holding the
>> pixel */
>> +	int tile_x_off, tile_y_off; /* pixel position inside the
>> tile */
>>  	int tile_n, tile_off;
>> -	int tiled_pos, tiles_per_line;
>> +	int line_size, tiles_per_line;
>> +	int tiled_pos;
>>  	int pixel_size = bpp / 8;
>>
>>  	line_size = stride;
>> -	x_tile_size = 512;
>> -	y_tile_size = 8;
>> -	tile_size = x_tile_size * y_tile_size;
>> -	tiles_per_line = line_size / x_tile_size;
>> +	tile_size = tile_width * tile_height;
>> +	tiles_per_line = line_size / tile_width;
>>
>> -	y_tile_n = y / y_tile_size;
>> -	y_tile_off = y % y_tile_size;
>> +	tile_y = y / tile_height;
>> +	tile_y_off = y % tile_height;
>>
>> -	x_tile_n = (x * pixel_size) / x_tile_size;
>> -	x_tile_off = (x * pixel_size) % x_tile_size;
>> +	tile_x = (x * pixel_size) / tile_width;
>> +	tile_x_off = (x * pixel_size) % tile_width;
>>
>> -	tile_n = y_tile_n * tiles_per_line + x_tile_n;
>> -	tile_off = y_tile_off * x_tile_size + x_tile_off;
>> -	tiled_pos = tile_n * tile_size + tile_off;
>> +	tile_n = tile_y * tiles_per_line + tile_x;
>> +
>> +	if (tiling == I915_TILING_X ) {
>> +		tile_off = tile_y_off * tile_width + tile_x_off;
>> +	} else { /* (tiling == I915_TILING_Y ) */
>> +		int x_oword_n, x_oword_off;
>> +		int oword_size = 16;
>> +
>> +		/* computation inside the tile */
>> +		x_oword_n = tile_x_off / oword_size;
>> +		x_oword_off = tile_x_off % oword_size;
>> +		tile_off = x_oword_n * tile_height * oword_size
>> +			   + tile_y_off * oword_size
>> +			   + x_oword_off;
>> +	}
>>
>> +	tiled_pos = tile_n * tile_size + tile_off;
>>  	tiled_pos = swizzle_addr(tiled_pos, swizzle);
>>
>>  	return tiled_pos / pixel_size;
>> @@ -169,34 +182,48 @@ static int linear_x_y_to_tiled_pos(int x, int
>> y, uint32_t stride, int swizzle,
>>
>>  /* It's all in "pixel coordinates", so make sure you multiply/divide
>> by the bpp
>>   * if you need to. */
>> -static void tiled_pos_to_x_y_linear(int tiled_pos, uint32_t stride,
>> -				    int swizzle, int bpp, int *x,
>> int *y)
>> +static void tiled_pos_to_x_y_linear(int fd, int tiled_pos, uint32_t
>> stride,
>> +				    int swizzle, int bpp, int *x,
>> int *y,
>> +				    int tiling, uint32_t tile_width,
>> +				    uint32_t tile_height)
>>  {
>> -	int tile_n, tile_off, tiles_per_line, line_size;
>> -	int x_tile_off, y_tile_off;
>> -	int x_tile_n, y_tile_n;
>> -	int x_tile_size, y_tile_size, tile_size;
>> +	uint32_t tile_size;
>> +	int tile_x, tile_y; /* Co-ordinates of the tile holding the
>> pixel */
>> +	int tile_x_off, tile_y_off; /* pixel position inside the
>> tile */
>> +	int tile_n, tile_off;
>> +	int line_size, tiles_per_line;
>>  	int pixel_size = bpp / 8;
>>
>>  	tiled_pos = swizzle_addr(tiled_pos, swizzle);
>>
>>  	line_size = stride;
>> -	x_tile_size = 512;
>> -	y_tile_size = 8;
>> -	tile_size = x_tile_size * y_tile_size;
>> -	tiles_per_line = line_size / x_tile_size;
>> +	tile_size = tile_width * tile_height;
>> +	tiles_per_line = line_size / tile_width;
>>
>>  	tile_n = tiled_pos / tile_size;
>>  	tile_off = tiled_pos % tile_size;
>>
>> -	y_tile_off = tile_off / x_tile_size;
>> -	x_tile_off = tile_off % x_tile_size;
>> +	tile_x = tile_n % tiles_per_line;
>> +	tile_y = tile_n / tiles_per_line;
>> +
>> +	if (tiling == I915_TILING_X ) {
>> +
>> +		tile_y_off = tile_off / tile_width;
>> +		tile_x_off = tile_off % tile_width;
>> +	} else {
>> +		int x_oword_n, x_oword_off;
>> +		int oword_size = 16;
>> +
>> +		x_oword_n = tile_off / (oword_size * tile_height);
>> +		x_oword_off = tile_off % oword_size;
>>
>> -	x_tile_n = tile_n % tiles_per_line;
>> -	y_tile_n = tile_n / tiles_per_line;
>> +		tile_x_off = x_oword_n * oword_size + x_oword_off;
>> +		tile_y_off = (tile_off - x_oword_n * oword_size *
>> tile_height)
>> +			     / oword_size;
>> +	}
>>
>> -	*x = (x_tile_n * x_tile_size + x_tile_off) / pixel_size;
>> -	*y = y_tile_n * y_tile_size + y_tile_off;
>> +	*x = (tile_x * tile_width + tile_x_off) / pixel_size;
>> +	*y = tile_y * tile_height + tile_y_off;
>>  }
>>
>>  static void set_pixel(void *_ptr, int index, uint32_t color, int
>> bpp)
>> @@ -224,15 +251,21 @@ static void draw_rect_ptr_linear(void *ptr,
>> uint32_t stride,
>>  	}
>>  }
>>
>> -static void draw_rect_ptr_tiled(void *ptr, uint32_t stride, int
>> swizzle,
>> -				struct rect *rect, uint32_t color,
>> int bpp)
>> +static void draw_rect_ptr_tiled(int fd, void *ptr, uint32_t stride,
>> int swizzle,
>> +				struct rect *rect, uint32_t color,
>> int bpp,
>> +				int tiling)
>>  {
>>  	int x, y, pos;
>> +	uint32_t tile_width, tile_height;
>> +
>> +	igt_get_fb_tile_size(fd,
>> (uint64_t)igt_fb_tiling_to_mod(tiling), bpp,
>> +			&tile_width, &tile_height);
>>
>>  	for (y = rect->y; y < rect->y + rect->h; y++) {
>>  		for (x = rect->x; x < rect->x + rect->w; x++) {
>> -			pos = linear_x_y_to_tiled_pos(x, y, stride,
>> swizzle,
>> -						      bpp);
>> +			pos = linear_x_y_to_tiled_pos(fd, x, y,
>> stride, swizzle,
>> +						      bpp, tiling,
>> tile_width,
>> +						      tile_height);
>>  			set_pixel(ptr, pos, color, bpp);
>>  		}
>>  	}
>> @@ -259,8 +292,12 @@ static void draw_rect_mmap_cpu(int fd, struct
>> buf_data *buf, struct rect *rect,
>>  		draw_rect_ptr_linear(ptr, buf->stride, rect, color,
>> buf->bpp);
>>  		break;
>>  	case I915_TILING_X:
>> -		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect,
>> color,
>> -				    buf->bpp);
>> +		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle,
>> rect, color,
>> +				    buf->bpp, tiling);
>> +		break;
>> +	case I915_TILING_Y:
>> +		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle,
>> rect, color,
>> +				    buf->bpp, tiling);
>>  		break;
>>  	default:
>>  		igt_assert(false);
>> @@ -309,8 +346,12 @@ static void draw_rect_mmap_wc(int fd, struct
>> buf_data *buf, struct rect *rect,
>>  		draw_rect_ptr_linear(ptr, buf->stride, rect, color,
>> buf->bpp);
>>  		break;
>>  	case I915_TILING_X:
>> -		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect,
>> color,
>> -				    buf->bpp);
>> +		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle,
>> rect, color,
>> +				    buf->bpp, tiling);
>> +		break;
>> +	case I915_TILING_Y:
>> +		draw_rect_ptr_tiled(fd, ptr, buf->stride, swizzle,
>> rect, color,
>> +				    buf->bpp, tiling);
>>  		break;
>>  	default:
>>  		igt_assert(false);
>> @@ -338,7 +379,7 @@ static void draw_rect_pwrite_untiled(int fd,
>> struct buf_data *buf,
>>
>>  static void draw_rect_pwrite_tiled(int fd, struct buf_data *buf,
>>  				   struct rect *rect, uint32_t
>> color,
>> -				   uint32_t swizzle)
>> +				   uint32_t swizzle, int tiling)
>>  {
>>  	int i;
>>  	int tiled_pos, x, y, pixel_size;
>> @@ -347,6 +388,7 @@ static void draw_rect_pwrite_tiled(int fd, struct
>> buf_data *buf,
>>  	bool flush_tmp = false;
>>  	int tmp_start_pos = 0;
>>  	int pixels_written = 0;
>> +	uint32_t tile_width, tile_height;
>>
>>  	/* We didn't implement suport for the older tiling methods
>> yet. */
>>  	igt_require(intel_gen(intel_get_drm_devid(fd)) >= 5);
>> @@ -360,9 +402,13 @@ static void draw_rect_pwrite_tiled(int fd,
>> struct buf_data *buf,
>>  	for (i = 0; i < tmp_size; i++)
>>  		set_pixel(tmp, i, color, buf->bpp);
>>
>> +	igt_get_fb_tile_size(fd,
>> (uint64_t)igt_fb_tiling_to_mod(tiling),
>> +			     buf->bpp, &tile_width, &tile_height);
>> +
>>  	for (tiled_pos = 0; tiled_pos < buf->size; tiled_pos +=
>> pixel_size) {
>> -		tiled_pos_to_x_y_linear(tiled_pos, buf->stride,
>> swizzle,
>> -					buf->bpp, &x, &y);
>> +		tiled_pos_to_x_y_linear(fd, tiled_pos, buf->stride,
>> swizzle,
>> +					buf->bpp, &x, &y, tiling,
>> tile_width,
>> +					tile_height);
>>
>>  		if (x >= rect->x && x < rect->x + rect->w &&
>>  		    y >= rect->y && y < rect->y + rect->h) {
>> @@ -399,7 +445,10 @@ static void draw_rect_pwrite(int fd, struct
>> buf_data *buf,
>>  		draw_rect_pwrite_untiled(fd, buf, rect, color);
>>  		break;
>>  	case I915_TILING_X:
>> -		draw_rect_pwrite_tiled(fd, buf, rect, color,
>> swizzle);
>> +		draw_rect_pwrite_tiled(fd, buf, rect, color,
>> swizzle, tiling);
>> +		break;
>> +	case I915_TILING_Y:
>> +		draw_rect_pwrite_tiled(fd, buf, rect, color,
>> swizzle, tiling);
>>  		break;
>>  	default:
>>  		igt_assert(false);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 7/7] igt/kms_fbc_crc.c : Add Y-tile tests

[Paulo Zanoni]

Em Sex, 2017-07-14 às 19:25 +0530, Praveen Paneri escreveu:
> Hi Paulo,
> 
> On Thursday 13 July 2017 02:31 AM, Paulo Zanoni wrote:
> > Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
> > > Now that we have support for Y-tiled buffers, add another
> > > iteration of tests for Y-tiled buffers.
> > 
> > Have you tested this on platforms that don't support Y-tiled
> > buffers? I
> 
> Unfortunately I haven't...
> > don't see a check for that, so I wonder if we'll just fail some
> > assertion or correctly hit some igt_skip() call I couldn't find.
> 
> ...but I will add the check as you have mentioned
> > 
> > More below.
> > 
> > > 
> > > Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> > > ---
> > >  tests/kms_fbc_crc.c | 71 +++++++++++++++++++++++++++++--------
> > > ----
> > > ------------
> > >  1 file changed, 39 insertions(+), 32 deletions(-)
> > > 
> > > diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
> > > index 7964e05..cdf04c1 100644
> > > --- a/tests/kms_fbc_crc.c
> > > +++ b/tests/kms_fbc_crc.c
> > > @@ -318,12 +318,10 @@ static void prepare_crtc(data_t *data)
> > >  	igt_output_set_pipe(output, data->pipe);
> > >  }
> > > 
> > > -static void create_fbs(data_t *data, bool tiled, struct igt_fb
> > > *fbs)
> > > +static void create_fbs(data_t *data, uint64_t tiling, struct
> > > igt_fb
> > > *fbs)
> > >  {
> > >  	int rc;
> > >  	drmModeModeInfo *mode = igt_output_get_mode(data-
> > > >output);
> > > -	uint64_t tiling = tiled ? LOCAL_I915_FORMAT_MOD_X_TILED
> > > :
> > > -				  LOCAL_DRM_FORMAT_MOD_NONE;
> > > 
> > >  	rc = igt_create_color_fb(data->drm_fd, mode->hdisplay,
> > > mode-
> > > > vdisplay,
> > > 
> > >  				 DRM_FORMAT_XRGB8888, tiling,
> > > @@ -344,8 +342,8 @@ static void get_ref_crcs(data_t *data)
> > >  	struct igt_fb fbs[4];
> > >  	int i;
> > > 
> > > -	create_fbs(data, false, &fbs[0]);
> > > -	create_fbs(data, false, &fbs[2]);
> > > +	create_fbs(data, LOCAL_DRM_FORMAT_MOD_NONE, &fbs[0]);
> > > +	create_fbs(data, LOCAL_DRM_FORMAT_MOD_NONE, &fbs[2]);
> > > 
> > >  	fill_mmap_gtt(data, fbs[2].gem_handle, 0xff);
> > >  	fill_mmap_gtt(data, fbs[3].gem_handle, 0xff);
> > > @@ -366,7 +364,7 @@ static void get_ref_crcs(data_t *data)
> > >  		igt_remove_fb(data->drm_fd, &fbs[i]);
> > >  }
> > > 
> > > -static bool prepare_test(data_t *data, enum test_mode test_mode)
> > > +static bool prepare_test(data_t *data, enum test_mode test_mode,
> > > uint64_t tiling)
> > >  {
> > >  	igt_display_t *display = &data->display;
> > >  	igt_output_t *output = data->output;
> > > @@ -374,7 +372,7 @@ static bool prepare_test(data_t *data, enum
> > > test_mode test_mode)
> > > 
> > >  	data->primary = igt_output_get_plane_type(data->output,
> > > DRM_PLANE_TYPE_PRIMARY);
> > > 
> > > -	create_fbs(data, true, data->fb);
> > > +	create_fbs(data, tiling, data->fb);
> > > 
> > >  	igt_pipe_crc_free(data->pipe_crc);
> > >  	data->pipe_crc = NULL;
> > > @@ -484,32 +482,41 @@ static void run_test(data_t *data, enum
> > > test_mode mode)
> > > 
> > >  	reset_display(data);
> > > 
> > > -	for_each_pipe_with_valid_output(display, data->pipe,
> > > data-
> > > > output) {
> > > 
> > > -		prepare_crtc(data);
> > > -
> > > -		igt_info("Beginning %s on pipe %s, connector
> > > %s\n",
> > > -			  igt_subtest_name(),
> > > -			  kmstest_pipe_name(data->pipe),
> > > -			  igt_output_name(data->output));
> > > -
> > > -		if (!prepare_test(data, mode)) {
> > > -			igt_info("%s on pipe %s, connector %s:
> > > SKIPPED\n",
> > > -				  igt_subtest_name(),
> > > -				  kmstest_pipe_name(data->pipe),
> > > -				  igt_output_name(data-
> > > >output));
> > > -			continue;
> > > +	for (int tiling = I915_TILING_X;
> > > +	     tiling <= I915_TILING_Y; tiling++) {
> > 
> > What I don't understand is why this part of the code chooses to go
> > with
> > the tiling constants (I915_TILING_) only to later convert them to
> > modifiers with igt_fb_tiling_to_mod(). If this loop iterated over
> > the
> > modifiers directly we wouldn't need that. The rest of the code only
> > cares about the modifiers.
> 
> I chose to loop over tiling constants as they are in a simple
> arithmetic 
> order. anyhow I will just change that.

Just put the two local_format stuff in an array and iterate over it.

> 
> Also as mentioned above can I just add a check to skip Y-tiling
> tests 
> for older platforms?
> 
> igt_skip_on(intel_gen(intel_get_drm_devid(drm_fd)) < 9 &&
>                                 tiling == I915_TILING_Y);

You can't do this here because the same subtest tests X tiling.

Perhaps we could make Y tiling be a separate subtest? I'm not a huge
fan of single tests that do tons of stuff.

> 
> > 
> > 
> > > +		for_each_pipe_with_valid_output(display,
> > > +						data->pipe,
> > > data-
> > > > output) {
> > > 
> > > +			prepare_crtc(data);
> > > +
> > > +			igt_info("Beginning %s on pipe %s,
> > > connector
> > > "
> > > +					"%s, %s-tiled\n",
> > > +					igt_subtest_name(),
> > > +					kmstest_pipe_name(data-
> > > > pipe),
> > > 
> > > +					igt_output_name(data-
> > > > output),
> > > 
> > > +					(tiling ==
> > > I915_TILING_X) ?
> > > "x":"y" );
> > 
> > This change is not keeping the indentation style, things should be
> > aligned with the parens (although I see they're actually aligned
> > with
> > the quote, which is also weird). The same can be said for the other
> > two
> > igt_info() calls in this patch.
> 
> Will fix it
> > 
> > 
> > > +
> > > +			if (!prepare_test(data, mode,
> > > +					  igt_fb_tiling_to_mod(t
> > > ilin
> > > g))) {
> > > +				igt_info("%s on pipe %s,
> > > connector
> > > %s: SKIPPED\n",
> > > +						igt_subtest_name
> > > (),
> > > +						kmstest_pipe_nam
> > > e(da
> > > ta->pipe),
> > > +						igt_output_name(
> > > data
> > > ->output));
> > 
> > This one is missing the "%s-tiled" part that was added in the other
> > two
> > messages.
> > 
> > And we can probably create a "const char *tiling_name" variable to
> > store the %s part in order to avoid the same ternary operator in
> > the 3
> > if statements.
> 
> make sense, will add.
> > 
> > 
> > > +				continue;
> > > +			}
> > > +
> > > +			valid_tests++;
> > > +
> > > +			test_crc(data, mode);
> > > +
> > > +			igt_info("%s on pipe %s, connector %s"
> > > +					"%s-tiled: PASSED\n",
> > > +					igt_subtest_name(),
> > > +					kmstest_pipe_name(data-
> > > > pipe),
> > > 
> > > +					igt_output_name(data-
> > > > output),
> > > 
> > > +					(tiling ==
> > > I915_TILING_X) ?
> > > "x":"y" );
> > > +
> > > +			finish_crtc(data, mode);
> > >  		}
> > > -
> > > -		valid_tests++;
> > > -
> > > -		test_crc(data, mode);
> > > -
> > > -		igt_info("%s on pipe %s, connector %s:
> > > PASSED\n",
> > > -			  igt_subtest_name(),
> > > -			  kmstest_pipe_name(data->pipe),
> > > -			  igt_output_name(data->output));
> > > -
> > > -		finish_crtc(data, mode);
> > >  	}
> > > 
> > >  	igt_require_f(valid_tests, "no valid crtc/connector
> > > combinations found\n");
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 7/7] igt/kms_fbc_crc.c : Add Y-tile tests

[Praveen Paneri]

On Friday 14 July 2017 07:55 PM, Paulo Zanoni wrote:
> Em Sex, 2017-07-14 às 19:25 +0530, Praveen Paneri escreveu:
>> Hi Paulo,
>>
>> On Thursday 13 July 2017 02:31 AM, Paulo Zanoni wrote:
>>> Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
>>>> Now that we have support for Y-tiled buffers, add another
>>>> iteration of tests for Y-tiled buffers.
>>>
>>> Have you tested this on platforms that don't support Y-tiled
>>> buffers? I
>>
>> Unfortunately I haven't...
>>> don't see a check for that, so I wonder if we'll just fail some
>>> assertion or correctly hit some igt_skip() call I couldn't find.
>>
>> ...but I will add the check as you have mentioned
>>>
>>> More below.
>>>
>>>>
>>>> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
>>>> ---
>>>>  tests/kms_fbc_crc.c | 71 +++++++++++++++++++++++++++++--------
>>>> ----
>>>> ------------
>>>>  1 file changed, 39 insertions(+), 32 deletions(-)
>>>>
>>>> diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
>>>> index 7964e05..cdf04c1 100644
>>>> --- a/tests/kms_fbc_crc.c
>>>> +++ b/tests/kms_fbc_crc.c
>>>> @@ -318,12 +318,10 @@ static void prepare_crtc(data_t *data)
>>>>  	igt_output_set_pipe(output, data->pipe);
>>>>  }
>>>>
>>>> -static void create_fbs(data_t *data, bool tiled, struct igt_fb
>>>> *fbs)
>>>> +static void create_fbs(data_t *data, uint64_t tiling, struct
>>>> igt_fb
>>>> *fbs)
>>>>  {
>>>>  	int rc;
>>>>  	drmModeModeInfo *mode = igt_output_get_mode(data-
>>>>> output);
>>>> -	uint64_t tiling = tiled ? LOCAL_I915_FORMAT_MOD_X_TILED
>>>> :
>>>> -				  LOCAL_DRM_FORMAT_MOD_NONE;
>>>>
>>>>  	rc = igt_create_color_fb(data->drm_fd, mode->hdisplay,
>>>> mode-
>>>>> vdisplay,
>>>>
>>>>  				 DRM_FORMAT_XRGB8888, tiling,
>>>> @@ -344,8 +342,8 @@ static void get_ref_crcs(data_t *data)
>>>>  	struct igt_fb fbs[4];
>>>>  	int i;
>>>>
>>>> -	create_fbs(data, false, &fbs[0]);
>>>> -	create_fbs(data, false, &fbs[2]);
>>>> +	create_fbs(data, LOCAL_DRM_FORMAT_MOD_NONE, &fbs[0]);
>>>> +	create_fbs(data, LOCAL_DRM_FORMAT_MOD_NONE, &fbs[2]);
>>>>
>>>>  	fill_mmap_gtt(data, fbs[2].gem_handle, 0xff);
>>>>  	fill_mmap_gtt(data, fbs[3].gem_handle, 0xff);
>>>> @@ -366,7 +364,7 @@ static void get_ref_crcs(data_t *data)
>>>>  		igt_remove_fb(data->drm_fd, &fbs[i]);
>>>>  }
>>>>
>>>> -static bool prepare_test(data_t *data, enum test_mode test_mode)
>>>> +static bool prepare_test(data_t *data, enum test_mode test_mode,
>>>> uint64_t tiling)
>>>>  {
>>>>  	igt_display_t *display = &data->display;
>>>>  	igt_output_t *output = data->output;
>>>> @@ -374,7 +372,7 @@ static bool prepare_test(data_t *data, enum
>>>> test_mode test_mode)
>>>>
>>>>  	data->primary = igt_output_get_plane_type(data->output,
>>>> DRM_PLANE_TYPE_PRIMARY);
>>>>
>>>> -	create_fbs(data, true, data->fb);
>>>> +	create_fbs(data, tiling, data->fb);
>>>>
>>>>  	igt_pipe_crc_free(data->pipe_crc);
>>>>  	data->pipe_crc = NULL;
>>>> @@ -484,32 +482,41 @@ static void run_test(data_t *data, enum
>>>> test_mode mode)
>>>>
>>>>  	reset_display(data);
>>>>
>>>> -	for_each_pipe_with_valid_output(display, data->pipe,
>>>> data-
>>>>> output) {
>>>>
>>>> -		prepare_crtc(data);
>>>> -
>>>> -		igt_info("Beginning %s on pipe %s, connector
>>>> %s\n",
>>>> -			  igt_subtest_name(),
>>>> -			  kmstest_pipe_name(data->pipe),
>>>> -			  igt_output_name(data->output));
>>>> -
>>>> -		if (!prepare_test(data, mode)) {
>>>> -			igt_info("%s on pipe %s, connector %s:
>>>> SKIPPED\n",
>>>> -				  igt_subtest_name(),
>>>> -				  kmstest_pipe_name(data->pipe),
>>>> -				  igt_output_name(data-
>>>>> output));
>>>> -			continue;
>>>> +	for (int tiling = I915_TILING_X;
>>>> +	     tiling <= I915_TILING_Y; tiling++) {
>>>
>>> What I don't understand is why this part of the code chooses to go
>>> with
>>> the tiling constants (I915_TILING_) only to later convert them to
>>> modifiers with igt_fb_tiling_to_mod(). If this loop iterated over
>>> the
>>> modifiers directly we wouldn't need that. The rest of the code only
>>> cares about the modifiers.
>>
>> I chose to loop over tiling constants as they are in a simple
>> arithmetic
>> order. anyhow I will just change that.
>
> Just put the two local_format stuff in an array and iterate over it.
>
>>
>> Also as mentioned above can I just add a check to skip Y-tiling
>> tests
>> for older platforms?
>>
>> igt_skip_on(intel_gen(intel_get_drm_devid(drm_fd)) < 9 &&
>>                                 tiling == I915_TILING_Y);
>
> You can't do this here because the same subtest tests X tiling.
Okay! what if I just skip the loop for Y tiling with some message?
>
> Perhaps we could make Y tiling be a separate subtest? I'm not a huge
> fan of single tests that do tons of stuff.
Will it be possible in just one subtest. I thought we will have to 
duplicate all the subtests for Y-tile case? Plz suggest

Thanks,
Praveen
>
>>
>>>
>>>
>>>> +		for_each_pipe_with_valid_output(display,
>>>> +						data->pipe,
>>>> data-
>>>>> output) {
>>>>
>>>> +			prepare_crtc(data);
>>>> +
>>>> +			igt_info("Beginning %s on pipe %s,
>>>> connector
>>>> "
>>>> +					"%s, %s-tiled\n",
>>>> +					igt_subtest_name(),
>>>> +					kmstest_pipe_name(data-
>>>>> pipe),
>>>>
>>>> +					igt_output_name(data-
>>>>> output),
>>>>
>>>> +					(tiling ==
>>>> I915_TILING_X) ?
>>>> "x":"y" );
>>>
>>> This change is not keeping the indentation style, things should be
>>> aligned with the parens (although I see they're actually aligned
>>> with
>>> the quote, which is also weird). The same can be said for the other
>>> two
>>> igt_info() calls in this patch.
>>
>> Will fix it
>>>
>>>
>>>> +
>>>> +			if (!prepare_test(data, mode,
>>>> +					  igt_fb_tiling_to_mod(t
>>>> ilin
>>>> g))) {
>>>> +				igt_info("%s on pipe %s,
>>>> connector
>>>> %s: SKIPPED\n",
>>>> +						igt_subtest_name
>>>> (),
>>>> +						kmstest_pipe_nam
>>>> e(da
>>>> ta->pipe),
>>>> +						igt_output_name(
>>>> data
>>>> ->output));
>>>
>>> This one is missing the "%s-tiled" part that was added in the other
>>> two
>>> messages.
>>>
>>> And we can probably create a "const char *tiling_name" variable to
>>> store the %s part in order to avoid the same ternary operator in
>>> the 3
>>> if statements.
>>
>> make sense, will add.
>>>
>>>
>>>> +				continue;
>>>> +			}
>>>> +
>>>> +			valid_tests++;
>>>> +
>>>> +			test_crc(data, mode);
>>>> +
>>>> +			igt_info("%s on pipe %s, connector %s"
>>>> +					"%s-tiled: PASSED\n",
>>>> +					igt_subtest_name(),
>>>> +					kmstest_pipe_name(data-
>>>>> pipe),
>>>>
>>>> +					igt_output_name(data-
>>>>> output),
>>>>
>>>> +					(tiling ==
>>>> I915_TILING_X) ?
>>>> "x":"y" );
>>>> +
>>>> +			finish_crtc(data, mode);
>>>>  		}
>>>> -
>>>> -		valid_tests++;
>>>> -
>>>> -		test_crc(data, mode);
>>>> -
>>>> -		igt_info("%s on pipe %s, connector %s:
>>>> PASSED\n",
>>>> -			  igt_subtest_name(),
>>>> -			  kmstest_pipe_name(data->pipe),
>>>> -			  igt_output_name(data->output));
>>>> -
>>>> -		finish_crtc(data, mode);
>>>>  	}
>>>>
>>>>  	igt_require_f(valid_tests, "no valid crtc/connector
>>>> combinations found\n");
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx