[PATCH 1/4] igt_fb: Add Y-tiling support

[PATCH 1/4] igt_fb: Add Y-tiling support

[Praveen Paneri]

This adds Y-tiling check in igt_create_fb_with_bo_size as
now we should also be able to create Y-tiled FBs.

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

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 47472f4..bf1d372 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -608,7 +608,8 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
 		  __func__, fb->gem_handle, fb->stride);
 
 	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
-	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED) {
+	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED &&
+	    tiling != LOCAL_I915_FORMAT_MOD_Y_TILED) {
 		do_or_die(__kms_addfb(fd, fb->gem_handle, width, height,
 				      fb->stride, format, tiling,
 				      LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id));
-- 
1.9.1

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

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

[Praveen Paneri]

This patch adds Y-tiling support for igt_draw_rect function.

Change-Id: I139e9773b7df286febe9ffa3dce358df079dac14
Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
---
 lib/igt_draw.c | 148 ++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 110 insertions(+), 38 deletions(-)

diff --git a/lib/igt_draw.c b/lib/igt_draw.c
index 3afb827..29a725f 100644
--- a/lib/igt_draw.c
+++ b/lib/igt_draw.c
@@ -137,7 +137,7 @@ 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)
+				   int bpp, int tiling)
 {
 	int x_tile_size, y_tile_size;
 	int x_tile_n, y_tile_n, x_tile_off, y_tile_off;
@@ -146,23 +146,54 @@ static int linear_x_y_to_tiled_pos(int x, int y, uint32_t stride, int swizzle,
 	int tiled_pos, tiles_per_line;
 	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;
-
-	y_tile_n = y / y_tile_size;
-	y_tile_off = y % y_tile_size;
-
-	x_tile_n = (x * pixel_size) / x_tile_size;
-	x_tile_off = (x * pixel_size) % x_tile_size;
-
-	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;
-
-	tiled_pos = swizzle_addr(tiled_pos, swizzle);
+	if (tiling == I915_TILING_X ) {
+		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;
+
+		y_tile_n = y / y_tile_size;
+		y_tile_off = y % y_tile_size;
+
+		x_tile_n = (x * pixel_size) / x_tile_size;
+		x_tile_off = (x * pixel_size) % x_tile_size;
+
+		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;
+
+		tiled_pos = swizzle_addr(tiled_pos, swizzle);
+	} else {
+		int x_oword_n, x_oword_off;
+		int oword_size;
+
+		/* Y-tile arrangement */
+		line_size = stride;
+		oword_size = 16;
+		x_tile_size = 128;
+		y_tile_size = 32;
+		tile_size = x_tile_size * y_tile_size;
+		tiles_per_line = line_size / x_tile_size;
+
+		y_tile_n = y / y_tile_size;
+		y_tile_off = y % y_tile_size;
+
+		x_tile_n = (x * pixel_size) / x_tile_size;
+		x_tile_off = (x * pixel_size) % x_tile_size;
+
+		tile_n = y_tile_n * tiles_per_line + x_tile_n;
+
+		/* computation inside the tile */
+		x_oword_n = x_tile_off / oword_size;
+		x_oword_off = x_tile_off % oword_size;
+		tile_off = x_oword_n * y_tile_size * oword_size
+			   + y_tile_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;
 }
@@ -170,7 +201,8 @@ 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)
+				    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;
@@ -181,22 +213,50 @@ static void tiled_pos_to_x_y_linear(int tiled_pos, uint32_t stride,
 	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;
+	if (tiling == I915_TILING_X ) {
+		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_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_n = tiled_pos / tile_size;
-	tile_off = tiled_pos % tile_size;
+		x_tile_n = tile_n % tiles_per_line;
+		y_tile_n = tile_n / tiles_per_line;
+
+		*x = (x_tile_n * x_tile_size + x_tile_off) / pixel_size;
+		*y = y_tile_n * y_tile_size + y_tile_off;
+	} else {
+		int x_oword_n, x_oword_off;
+		int oword_size;
 
-	y_tile_off = tile_off / x_tile_size;
-	x_tile_off = tile_off % x_tile_size;
+		oword_size = 16;
+		x_tile_size = 128;
+		y_tile_size = 32;
+		tile_size = x_tile_size * y_tile_size;
+		tiles_per_line = line_size / x_tile_size;
 
-	x_tile_n = tile_n % tiles_per_line;
-	y_tile_n = tile_n / tiles_per_line;
+		tile_n = tiled_pos / tile_size;
+		tile_off = tiled_pos % tile_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_n = tile_n % tiles_per_line;
+		y_tile_n = tile_n / tiles_per_line;
+
+		x_oword_n = tile_off / (oword_size * y_tile_size);
+		x_oword_off = tile_off % oword_size;
+
+		x_tile_off = x_oword_n * oword_size + x_oword_off;
+		y_tile_off = (tile_off - x_oword_n * oword_size * y_tile_size)
+			     / oword_size;
+
+		*x = (x_tile_n * x_tile_size + x_tile_off) / pixel_size;
+		*y = y_tile_n * y_tile_size + y_tile_off;
+
+	}
 }
 
 static void set_pixel(void *_ptr, int index, uint32_t color, int bpp)
@@ -225,14 +285,15 @@ 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)
+				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);
+						      bpp, tiling);
 			set_pixel(ptr, pos, color, bpp);
 		}
 	}
@@ -260,7 +321,11 @@ static void draw_rect_mmap_cpu(int fd, struct buf_data *buf, struct rect *rect,
 		break;
 	case I915_TILING_X:
 		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
-				    buf->bpp);
+				    buf->bpp, tiling);
+		break;
+	case I915_TILING_Y:
+		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
+				    buf->bpp, tiling);
 		break;
 	default:
 		igt_assert(false);
@@ -310,7 +375,11 @@ static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect *rect,
 		break;
 	case I915_TILING_X:
 		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
-				    buf->bpp);
+				    buf->bpp, tiling);
+		break;
+	case I915_TILING_Y:
+		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
+				    buf->bpp, tiling);
 		break;
 	default:
 		igt_assert(false);
@@ -338,7 +407,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;
@@ -362,7 +431,7 @@ static void draw_rect_pwrite_tiled(int fd, struct buf_data *buf,
 
 	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);
+					buf->bpp, &x, &y, tiling);
 
 		if (x >= rect->x && x < rect->x + rect->w &&
 		    y >= rect->y && y < rect->y + rect->h) {
@@ -399,7 +468,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 3/4] lib/igt_draw: Add Y-tiling support for IGT_DRAW_BLT method

[Praveen Paneri]

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

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

diff --git a/lib/igt_draw.c b/lib/igt_draw.c
index 29a725f..32600f0 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
@@ -517,6 +518,23 @@ 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;
 
+	if (tiling == I915_TILING_Y) {
+		/* 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(((0x3 << 16) | 0x3)); /* enable the Y tiling */
+		OUT_BATCH(MI_NOOP);
+		ADVANCE_BATCH();
+	}
+
 	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);
@@ -527,6 +545,23 @@ static void draw_rect_blt(int fd, struct cmd_data *cmd_data,
 	OUT_BATCH(color);
 	ADVANCE_BATCH();
 
+	if (tiling == I915_TILING_Y) {
+		/* 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((0x3 << 16)); /* Reset back to X-Tiling (default) */
+		OUT_BATCH(MI_NOOP);
+		ADVANCE_BATCH();
+	}
+
 	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 4/4] 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.

Change-Id: Id0a805aab2d804cf82c188326a8562f0ec75c874
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 tests/kms_draw_crc.c | 55 ++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 40 insertions(+), 15 deletions(-)

diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
index cb28052..e8c1fe1 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;
@@ -152,6 +159,9 @@ 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
@@ -214,6 +224,11 @@ 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);
 }
@@ -272,28 +287,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

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

[Ville Syrjälä]

On Mon, Oct 24, 2016 at 10:25:56PM +0530, Praveen Paneri wrote:
> This patch adds Y-tiling support for igt_draw_rect function.
> 
> Change-Id: I139e9773b7df286febe9ffa3dce358df079dac14
> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> ---
>  lib/igt_draw.c | 148 ++++++++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 110 insertions(+), 38 deletions(-)
> 
> diff --git a/lib/igt_draw.c b/lib/igt_draw.c
> index 3afb827..29a725f 100644
> --- a/lib/igt_draw.c
> +++ b/lib/igt_draw.c
> @@ -137,7 +137,7 @@ 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)
> +				   int bpp, int tiling)
>  {
>  	int x_tile_size, y_tile_size;
>  	int x_tile_n, y_tile_n, x_tile_off, y_tile_off;
> @@ -146,23 +146,54 @@ static int linear_x_y_to_tiled_pos(int x, int y, uint32_t stride, int swizzle,
>  	int tiled_pos, tiles_per_line;
>  	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;
> -
> -	y_tile_n = y / y_tile_size;
> -	y_tile_off = y % y_tile_size;
> -
> -	x_tile_n = (x * pixel_size) / x_tile_size;
> -	x_tile_off = (x * pixel_size) % x_tile_size;
> -
> -	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;
> -
> -	tiled_pos = swizzle_addr(tiled_pos, swizzle);
> +	if (tiling == I915_TILING_X ) {

We should already have some helpers to get you the tile dimensions so
I don't think you should have to have to duplicate all of this for
every tiling format. Also this doesn't even seem to be handling the
gen2/gen3 tile sizes, so we'd get most of that for free (apart from
the intra tile layout).

> +		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;
> +
> +		y_tile_n = y / y_tile_size;
> +		y_tile_off = y % y_tile_size;
> +
> +		x_tile_n = (x * pixel_size) / x_tile_size;
> +		x_tile_off = (x * pixel_size) % x_tile_size;
> +
> +		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;
> +
> +		tiled_pos = swizzle_addr(tiled_pos, swizzle);
> +	} else {
> +		int x_oword_n, x_oword_off;
> +		int oword_size;
> +
> +		/* Y-tile arrangement */
> +		line_size = stride;
> +		oword_size = 16;
> +		x_tile_size = 128;
> +		y_tile_size = 32;
> +		tile_size = x_tile_size * y_tile_size;
> +		tiles_per_line = line_size / x_tile_size;
> +
> +		y_tile_n = y / y_tile_size;
> +		y_tile_off = y % y_tile_size;
> +
> +		x_tile_n = (x * pixel_size) / x_tile_size;
> +		x_tile_off = (x * pixel_size) % x_tile_size;
> +
> +		tile_n = y_tile_n * tiles_per_line + x_tile_n;
> +
> +		/* computation inside the tile */
> +		x_oword_n = x_tile_off / oword_size;
> +		x_oword_off = x_tile_off % oword_size;
> +		tile_off = x_oword_n * y_tile_size * oword_size
> +			   + y_tile_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;
>  }
> @@ -170,7 +201,8 @@ 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)
> +				    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;
> @@ -181,22 +213,50 @@ static void tiled_pos_to_x_y_linear(int tiled_pos, uint32_t stride,
>  	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;
> +	if (tiling == I915_TILING_X ) {
> +		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_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_n = tiled_pos / tile_size;
> -	tile_off = tiled_pos % tile_size;
> +		x_tile_n = tile_n % tiles_per_line;
> +		y_tile_n = tile_n / tiles_per_line;
> +
> +		*x = (x_tile_n * x_tile_size + x_tile_off) / pixel_size;
> +		*y = y_tile_n * y_tile_size + y_tile_off;
> +	} else {
> +		int x_oword_n, x_oword_off;
> +		int oword_size;
>  
> -	y_tile_off = tile_off / x_tile_size;
> -	x_tile_off = tile_off % x_tile_size;
> +		oword_size = 16;
> +		x_tile_size = 128;
> +		y_tile_size = 32;
> +		tile_size = x_tile_size * y_tile_size;
> +		tiles_per_line = line_size / x_tile_size;
>  
> -	x_tile_n = tile_n % tiles_per_line;
> -	y_tile_n = tile_n / tiles_per_line;
> +		tile_n = tiled_pos / tile_size;
> +		tile_off = tiled_pos % tile_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_n = tile_n % tiles_per_line;
> +		y_tile_n = tile_n / tiles_per_line;
> +
> +		x_oword_n = tile_off / (oword_size * y_tile_size);
> +		x_oword_off = tile_off % oword_size;
> +
> +		x_tile_off = x_oword_n * oword_size + x_oword_off;
> +		y_tile_off = (tile_off - x_oword_n * oword_size * y_tile_size)
> +			     / oword_size;
> +
> +		*x = (x_tile_n * x_tile_size + x_tile_off) / pixel_size;
> +		*y = y_tile_n * y_tile_size + y_tile_off;
> +
> +	}
>  }
>  
>  static void set_pixel(void *_ptr, int index, uint32_t color, int bpp)
> @@ -225,14 +285,15 @@ 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)
> +				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);
> +						      bpp, tiling);
>  			set_pixel(ptr, pos, color, bpp);
>  		}
>  	}
> @@ -260,7 +321,11 @@ static void draw_rect_mmap_cpu(int fd, struct buf_data *buf, struct rect *rect,
>  		break;
>  	case I915_TILING_X:
>  		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
> -				    buf->bpp);
> +				    buf->bpp, tiling);
> +		break;
> +	case I915_TILING_Y:
> +		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
> +				    buf->bpp, tiling);
>  		break;
>  	default:
>  		igt_assert(false);
> @@ -310,7 +375,11 @@ static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect *rect,
>  		break;
>  	case I915_TILING_X:
>  		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
> -				    buf->bpp);
> +				    buf->bpp, tiling);
> +		break;
> +	case I915_TILING_Y:
> +		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
> +				    buf->bpp, tiling);
>  		break;
>  	default:
>  		igt_assert(false);
> @@ -338,7 +407,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;
> @@ -362,7 +431,7 @@ static void draw_rect_pwrite_tiled(int fd, struct buf_data *buf,
>  
>  	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);
> +					buf->bpp, &x, &y, tiling);
>  
>  		if (x >= rect->x && x < rect->x + rect->w &&
>  		    y >= rect->y && y < rect->y + rect->h) {
> @@ -399,7 +468,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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] igt_fb: Add Y-tiling support

[Tvrtko Ursulin]

On 24/10/2016 17:55, Praveen Paneri wrote:
> This adds Y-tiling check in igt_create_fb_with_bo_size as
> now we should also be able to create Y-tiled FBs.
>
> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> ---
>  lib/igt_fb.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 47472f4..bf1d372 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -608,7 +608,8 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
>  		  __func__, fb->gem_handle, fb->stride);
>
>  	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> -	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED) {
> +	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED &&
> +	    tiling != LOCAL_I915_FORMAT_MOD_Y_TILED) {
>  		do_or_die(__kms_addfb(fd, fb->gem_handle, width, height,
>  				      fb->stride, format, tiling,
>  				      LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id));
>

This works now? Ie. doesn't hit "No Y Tiling for legacy addfb" error in 
the driver?

Regards,

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

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

[Tvrtko Ursulin]

On 24/10/2016 17:55, Praveen Paneri wrote:
> This patch adds Y-tiling support for igt_draw_rect function.
>
> Change-Id: I139e9773b7df286febe9ffa3dce358df079dac14

You can remove (and should) remove Gerrit tags when sending stuff upstream.

Regards,

Tvrtko

> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> ---
>  lib/igt_draw.c | 148 ++++++++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 110 insertions(+), 38 deletions(-)
>
> diff --git a/lib/igt_draw.c b/lib/igt_draw.c
> index 3afb827..29a725f 100644
> --- a/lib/igt_draw.c
> +++ b/lib/igt_draw.c
> @@ -137,7 +137,7 @@ 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)
> +				   int bpp, int tiling)
>  {
>  	int x_tile_size, y_tile_size;
>  	int x_tile_n, y_tile_n, x_tile_off, y_tile_off;
> @@ -146,23 +146,54 @@ static int linear_x_y_to_tiled_pos(int x, int y, uint32_t stride, int swizzle,
>  	int tiled_pos, tiles_per_line;
>  	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;
> -
> -	y_tile_n = y / y_tile_size;
> -	y_tile_off = y % y_tile_size;
> -
> -	x_tile_n = (x * pixel_size) / x_tile_size;
> -	x_tile_off = (x * pixel_size) % x_tile_size;
> -
> -	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;
> -
> -	tiled_pos = swizzle_addr(tiled_pos, swizzle);
> +	if (tiling == I915_TILING_X ) {
> +		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;
> +
> +		y_tile_n = y / y_tile_size;
> +		y_tile_off = y % y_tile_size;
> +
> +		x_tile_n = (x * pixel_size) / x_tile_size;
> +		x_tile_off = (x * pixel_size) % x_tile_size;
> +
> +		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;
> +
> +		tiled_pos = swizzle_addr(tiled_pos, swizzle);
> +	} else {
> +		int x_oword_n, x_oword_off;
> +		int oword_size;
> +
> +		/* Y-tile arrangement */
> +		line_size = stride;
> +		oword_size = 16;
> +		x_tile_size = 128;
> +		y_tile_size = 32;
> +		tile_size = x_tile_size * y_tile_size;
> +		tiles_per_line = line_size / x_tile_size;
> +
> +		y_tile_n = y / y_tile_size;
> +		y_tile_off = y % y_tile_size;
> +
> +		x_tile_n = (x * pixel_size) / x_tile_size;
> +		x_tile_off = (x * pixel_size) % x_tile_size;
> +
> +		tile_n = y_tile_n * tiles_per_line + x_tile_n;
> +
> +		/* computation inside the tile */
> +		x_oword_n = x_tile_off / oword_size;
> +		x_oword_off = x_tile_off % oword_size;
> +		tile_off = x_oword_n * y_tile_size * oword_size
> +			   + y_tile_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;
>  }
> @@ -170,7 +201,8 @@ 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)
> +				    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;
> @@ -181,22 +213,50 @@ static void tiled_pos_to_x_y_linear(int tiled_pos, uint32_t stride,
>  	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;
> +	if (tiling == I915_TILING_X ) {
> +		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_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_n = tiled_pos / tile_size;
> -	tile_off = tiled_pos % tile_size;
> +		x_tile_n = tile_n % tiles_per_line;
> +		y_tile_n = tile_n / tiles_per_line;
> +
> +		*x = (x_tile_n * x_tile_size + x_tile_off) / pixel_size;
> +		*y = y_tile_n * y_tile_size + y_tile_off;
> +	} else {
> +		int x_oword_n, x_oword_off;
> +		int oword_size;
>
> -	y_tile_off = tile_off / x_tile_size;
> -	x_tile_off = tile_off % x_tile_size;
> +		oword_size = 16;
> +		x_tile_size = 128;
> +		y_tile_size = 32;
> +		tile_size = x_tile_size * y_tile_size;
> +		tiles_per_line = line_size / x_tile_size;
>
> -	x_tile_n = tile_n % tiles_per_line;
> -	y_tile_n = tile_n / tiles_per_line;
> +		tile_n = tiled_pos / tile_size;
> +		tile_off = tiled_pos % tile_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_n = tile_n % tiles_per_line;
> +		y_tile_n = tile_n / tiles_per_line;
> +
> +		x_oword_n = tile_off / (oword_size * y_tile_size);
> +		x_oword_off = tile_off % oword_size;
> +
> +		x_tile_off = x_oword_n * oword_size + x_oword_off;
> +		y_tile_off = (tile_off - x_oword_n * oword_size * y_tile_size)
> +			     / oword_size;
> +
> +		*x = (x_tile_n * x_tile_size + x_tile_off) / pixel_size;
> +		*y = y_tile_n * y_tile_size + y_tile_off;
> +
> +	}
>  }
>
>  static void set_pixel(void *_ptr, int index, uint32_t color, int bpp)
> @@ -225,14 +285,15 @@ 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)
> +				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);
> +						      bpp, tiling);
>  			set_pixel(ptr, pos, color, bpp);
>  		}
>  	}
> @@ -260,7 +321,11 @@ static void draw_rect_mmap_cpu(int fd, struct buf_data *buf, struct rect *rect,
>  		break;
>  	case I915_TILING_X:
>  		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
> -				    buf->bpp);
> +				    buf->bpp, tiling);
> +		break;
> +	case I915_TILING_Y:
> +		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
> +				    buf->bpp, tiling);
>  		break;
>  	default:
>  		igt_assert(false);
> @@ -310,7 +375,11 @@ static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect *rect,
>  		break;
>  	case I915_TILING_X:
>  		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
> -				    buf->bpp);
> +				    buf->bpp, tiling);
> +		break;
> +	case I915_TILING_Y:
> +		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
> +				    buf->bpp, tiling);
>  		break;
>  	default:
>  		igt_assert(false);
> @@ -338,7 +407,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;
> @@ -362,7 +431,7 @@ static void draw_rect_pwrite_tiled(int fd, struct buf_data *buf,
>
>  	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);
> +					buf->bpp, &x, &y, tiling);
>
>  		if (x >= rect->x && x < rect->x + rect->w &&
>  		    y >= rect->y && y < rect->y + rect->h) {
> @@ -399,7 +468,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 1/4] igt_fb: Add Y-tiling support

[Chris Wilson]

On Tue, Oct 25, 2016 at 09:06:26AM +0100, Tvrtko Ursulin wrote:
> 
> On 24/10/2016 17:55, Praveen Paneri wrote:
> >This adds Y-tiling check in igt_create_fb_with_bo_size as
> >now we should also be able to create Y-tiled FBs.
> >
> >Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> >---
> > lib/igt_fb.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> >index 47472f4..bf1d372 100644
> >--- a/lib/igt_fb.c
> >+++ b/lib/igt_fb.c
> >@@ -608,7 +608,8 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
> > 		  __func__, fb->gem_handle, fb->stride);
> >
> > 	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> >-	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED) {
> >+	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED &&
> >+	    tiling != LOCAL_I915_FORMAT_MOD_Y_TILED) {
> > 		do_or_die(__kms_addfb(fd, fb->gem_handle, width, height,
> > 				      fb->stride, format, tiling,
> > 				      LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id));
> >
> 
> This works now? Ie. doesn't hit "No Y Tiling for legacy addfb" error
> in the driver?

That legacy check still exists :(, but __kms_addfb() lies and calls addfb2.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] igt_fb: Add Y-tiling support

[Tvrtko Ursulin]

On 25/10/2016 09:18, Chris Wilson wrote:
> On Tue, Oct 25, 2016 at 09:06:26AM +0100, Tvrtko Ursulin wrote:
>>
>> On 24/10/2016 17:55, Praveen Paneri wrote:
>>> This adds Y-tiling check in igt_create_fb_with_bo_size as
>>> now we should also be able to create Y-tiled FBs.
>>>
>>> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
>>> ---
>>> lib/igt_fb.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
>>> index 47472f4..bf1d372 100644
>>> --- a/lib/igt_fb.c
>>> +++ b/lib/igt_fb.c
>>> @@ -608,7 +608,8 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
>>> 		  __func__, fb->gem_handle, fb->stride);
>>>
>>> 	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
>>> -	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED) {
>>> +	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED &&
>>> +	    tiling != LOCAL_I915_FORMAT_MOD_Y_TILED) {
>>> 		do_or_die(__kms_addfb(fd, fb->gem_handle, width, height,
>>> 				      fb->stride, format, tiling,
>>> 				      LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id));
>>>
>>
>> This works now? Ie. doesn't hit "No Y Tiling for legacy addfb" error
>> in the driver?
>
> That legacy check still exists :(, but __kms_addfb() lies and calls addfb2.

And this change makes the code not call __kms_addfb but the else branch 
which does not set the modifiers at all AFAICS.

REgards,

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

Re: [PATCH 1/4] igt_fb: Add Y-tiling support

[Chris Wilson]

On Tue, Oct 25, 2016 at 09:20:31AM +0100, Tvrtko Ursulin wrote:
> 
> 
> On 25/10/2016 09:18, Chris Wilson wrote:
> >On Tue, Oct 25, 2016 at 09:06:26AM +0100, Tvrtko Ursulin wrote:
> >>
> >>On 24/10/2016 17:55, Praveen Paneri wrote:
> >>>This adds Y-tiling check in igt_create_fb_with_bo_size as
> >>>now we should also be able to create Y-tiled FBs.
> >>>
> >>>Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> >>>---
> >>>lib/igt_fb.c | 3 ++-
> >>>1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>>diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> >>>index 47472f4..bf1d372 100644
> >>>--- a/lib/igt_fb.c
> >>>+++ b/lib/igt_fb.c
> >>>@@ -608,7 +608,8 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
> >>>		  __func__, fb->gem_handle, fb->stride);
> >>>
> >>>	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> >>>-	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED) {
> >>>+	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED &&
> >>>+	    tiling != LOCAL_I915_FORMAT_MOD_Y_TILED) {
> >>>		do_or_die(__kms_addfb(fd, fb->gem_handle, width, height,
> >>>				      fb->stride, format, tiling,
> >>>				      LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id));
> >>>
> >>
> >>This works now? Ie. doesn't hit "No Y Tiling for legacy addfb" error
> >>in the driver?
> >
> >That legacy check still exists :(, but __kms_addfb() lies and calls addfb2.
> 
> And this change makes the code not call __kms_addfb but the else
> branch which does not set the modifiers at all AFAICS.

Ah. Was looking at the wrong thing, say addfb and thought it was the
vanilla ADDFB (which should of course support y-tiling!!!). :(
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] igt_fb: Add Y-tiling support

[Paneri, Praveen]

Hi Tvrtko,

Along with this change I made following change in the kernel side. I was not sure if this is a hack of a legitimate change. Could you please give me a pointer about how to move fwd from here? Without this all Y-tiling tests would fail.

--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15406,8 +15406,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
                if (obj->tiling_mode == I915_TILING_X)
                        mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED;
                else if (obj->tiling_mode == I915_TILING_Y) {
-                       DRM_DEBUG("No Y tiling for legacy addfb\n");
-                       return -EINVAL;
+                       mode_cmd->modifier[0] = I915_FORMAT_MOD_Y_TILED;
                }
        }

Regards,
Praveen


-----Original Message-----
From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com] 
Sent: Tuesday, October 25, 2016 1:36 PM
To: Paneri, Praveen <praveen.paneri@intel.com>; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/4] igt_fb: Add Y-tiling support


On 24/10/2016 17:55, Praveen Paneri wrote:
> This adds Y-tiling check in igt_create_fb_with_bo_size as now we 
> should also be able to create Y-tiled FBs.
>
> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> ---
>  lib/igt_fb.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c index 47472f4..bf1d372 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -608,7 +608,8 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
>  		  __func__, fb->gem_handle, fb->stride);
>
>  	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> -	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED) {
> +	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED &&
> +	    tiling != LOCAL_I915_FORMAT_MOD_Y_TILED) {
>  		do_or_die(__kms_addfb(fd, fb->gem_handle, width, height,
>  				      fb->stride, format, tiling,
>  				      LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id));
>

This works now? Ie. doesn't hit "No Y Tiling for legacy addfb" error in the driver?

Regards,

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

Re: [PATCH 1/4] igt_fb: Add Y-tiling support

[Tvrtko Ursulin]

On 25/10/2016 13:06, Paneri, Praveen wrote:
> Hi Tvrtko,
>
> Along with this change I made following change in the kernel side. I was not sure if this is a hack of a legitimate change. Could you please give me a pointer about how to move fwd from here? Without this all Y-tiling tests would fail.
>
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15406,8 +15406,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
>                 if (obj->tiling_mode == I915_TILING_X)
>                         mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED;
>                 else if (obj->tiling_mode == I915_TILING_Y) {
> -                       DRM_DEBUG("No Y tiling for legacy addfb\n");
> -                       return -EINVAL;
> +                       mode_cmd->modifier[0] = I915_FORMAT_MOD_Y_TILED;
>                 }
>         }

It would be a controversial change, "beyond my pay grade". :)

If you drop this particular IGT patch, you can still create Y tiled 
framebuffers and display them (as the existing tests already do that). 
So when you say that all Y tiling tests fail without this kernel hack, 
which tests you are referring to?

Regards,

Tvrtko

> Regards,
> Praveen
>
>
> -----Original Message-----
> From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com]
> Sent: Tuesday, October 25, 2016 1:36 PM
> To: Paneri, Praveen <praveen.paneri@intel.com>; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 1/4] igt_fb: Add Y-tiling support
>
>
> On 24/10/2016 17:55, Praveen Paneri wrote:
>> This adds Y-tiling check in igt_create_fb_with_bo_size as now we
>> should also be able to create Y-tiled FBs.
>>
>> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
>> ---
>>  lib/igt_fb.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/igt_fb.c b/lib/igt_fb.c index 47472f4..bf1d372 100644
>> --- a/lib/igt_fb.c
>> +++ b/lib/igt_fb.c
>> @@ -608,7 +608,8 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
>>  		  __func__, fb->gem_handle, fb->stride);
>>
>>  	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
>> -	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED) {
>> +	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED &&
>> +	    tiling != LOCAL_I915_FORMAT_MOD_Y_TILED) {
>>  		do_or_die(__kms_addfb(fd, fb->gem_handle, width, height,
>>  				      fb->stride, format, tiling,
>>  				      LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id));
>>
>
> This works now? Ie. doesn't hit "No Y Tiling for legacy addfb" error in the driver?
>
> Regards,
>
> Tvrtko
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] igt_fb: Add Y-tiling support

[Paneri, Praveen]

> So when you say that all Y tiling tests fail without this kernel hack, which tests you are referring to?
If I revert this IGT patch and do not make below kernel change, kms_draw_crc (ytiled cases, last patch in this series) fail with following error.

Test assertion failure function igt_assert_crc_equal, file hardware/intel/intel-gpu-tools/lib/igt_debugfs.c:266:
Failed assertion: a->crc[i] == b->crc[i]
error: 0x68c96b8b != 0x948e53b

I think, I shall debug it further and try to fix it without making this change.

Regards,
Praveen


-----Original Message-----
From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com] 
Sent: Tuesday, October 25, 2016 6:07 PM
To: Paneri, Praveen <praveen.paneri@intel.com>; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/4] igt_fb: Add Y-tiling support


On 25/10/2016 13:06, Paneri, Praveen wrote:
> Hi Tvrtko,
>
> Along with this change I made following change in the kernel side. I was not sure if this is a hack of a legitimate change. Could you please give me a pointer about how to move fwd from here? Without this all Y-tiling tests would fail.
>
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15406,8 +15406,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
>                 if (obj->tiling_mode == I915_TILING_X)
>                         mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED;
>                 else if (obj->tiling_mode == I915_TILING_Y) {
> -                       DRM_DEBUG("No Y tiling for legacy addfb\n");
> -                       return -EINVAL;
> +                       mode_cmd->modifier[0] = 
> + I915_FORMAT_MOD_Y_TILED;
>                 }
>         }

It would be a controversial change, "beyond my pay grade". :)

If you drop this particular IGT patch, you can still create Y tiled framebuffers and display them (as the existing tests already do that). 
So when you say that all Y tiling tests fail without this kernel hack, which tests you are referring to?

Regards,

Tvrtko

> Regards,
> Praveen
>
>
> -----Original Message-----
> From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com]
> Sent: Tuesday, October 25, 2016 1:36 PM
> To: Paneri, Praveen <praveen.paneri@intel.com>; 
> intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 1/4] igt_fb: Add Y-tiling support
>
>
> On 24/10/2016 17:55, Praveen Paneri wrote:
>> This adds Y-tiling check in igt_create_fb_with_bo_size as now we 
>> should also be able to create Y-tiled FBs.
>>
>> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
>> ---
>>  lib/igt_fb.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/igt_fb.c b/lib/igt_fb.c index 47472f4..bf1d372 
>> 100644
>> --- a/lib/igt_fb.c
>> +++ b/lib/igt_fb.c
>> @@ -608,7 +608,8 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
>>  		  __func__, fb->gem_handle, fb->stride);
>>
>>  	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
>> -	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED) {
>> +	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED &&
>> +	    tiling != LOCAL_I915_FORMAT_MOD_Y_TILED) {
>>  		do_or_die(__kms_addfb(fd, fb->gem_handle, width, height,
>>  				      fb->stride, format, tiling,
>>  				      LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id));
>>
>
> This works now? Ie. doesn't hit "No Y Tiling for legacy addfb" error in the driver?
>
> Regards,
>
> Tvrtko
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

On Tue, Oct 25, 2016 at 09:08:37AM +0100, Tvrtko Ursulin wrote:
> 
> On 24/10/2016 17:55, Praveen Paneri wrote:
> > This patch adds Y-tiling support for igt_draw_rect function.
> > 
> > Change-Id: I139e9773b7df286febe9ffa3dce358df079dac14
> 
> You can remove (and should) remove Gerrit tags when sending stuff upstream.

Personally I don't care. There's definitely some upstream folks who want
those removed, and will reject your patches if gerrit tags are still there
(so really remove them for anything touching non-intel code). But
personally I don't care, and if it somehow helps you track stuff
internally I'm ok with carrying them around.

Iirc a while back we had the idea of an VPG-Id: or similar, for exactly
this purpose.

/maintainer out

Cheers, Daniel

> 
> Regards,
> 
> Tvrtko
> 
> > Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> > ---
> >  lib/igt_draw.c | 148 ++++++++++++++++++++++++++++++++++++++++++---------------
> >  1 file changed, 110 insertions(+), 38 deletions(-)
> > 
> > diff --git a/lib/igt_draw.c b/lib/igt_draw.c
> > index 3afb827..29a725f 100644
> > --- a/lib/igt_draw.c
> > +++ b/lib/igt_draw.c
> > @@ -137,7 +137,7 @@ 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)
> > +				   int bpp, int tiling)
> >  {
> >  	int x_tile_size, y_tile_size;
> >  	int x_tile_n, y_tile_n, x_tile_off, y_tile_off;
> > @@ -146,23 +146,54 @@ static int linear_x_y_to_tiled_pos(int x, int y, uint32_t stride, int swizzle,
> >  	int tiled_pos, tiles_per_line;
> >  	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;
> > -
> > -	y_tile_n = y / y_tile_size;
> > -	y_tile_off = y % y_tile_size;
> > -
> > -	x_tile_n = (x * pixel_size) / x_tile_size;
> > -	x_tile_off = (x * pixel_size) % x_tile_size;
> > -
> > -	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;
> > -
> > -	tiled_pos = swizzle_addr(tiled_pos, swizzle);
> > +	if (tiling == I915_TILING_X ) {
> > +		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;
> > +
> > +		y_tile_n = y / y_tile_size;
> > +		y_tile_off = y % y_tile_size;
> > +
> > +		x_tile_n = (x * pixel_size) / x_tile_size;
> > +		x_tile_off = (x * pixel_size) % x_tile_size;
> > +
> > +		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;
> > +
> > +		tiled_pos = swizzle_addr(tiled_pos, swizzle);
> > +	} else {
> > +		int x_oword_n, x_oword_off;
> > +		int oword_size;
> > +
> > +		/* Y-tile arrangement */
> > +		line_size = stride;
> > +		oword_size = 16;
> > +		x_tile_size = 128;
> > +		y_tile_size = 32;
> > +		tile_size = x_tile_size * y_tile_size;
> > +		tiles_per_line = line_size / x_tile_size;
> > +
> > +		y_tile_n = y / y_tile_size;
> > +		y_tile_off = y % y_tile_size;
> > +
> > +		x_tile_n = (x * pixel_size) / x_tile_size;
> > +		x_tile_off = (x * pixel_size) % x_tile_size;
> > +
> > +		tile_n = y_tile_n * tiles_per_line + x_tile_n;
> > +
> > +		/* computation inside the tile */
> > +		x_oword_n = x_tile_off / oword_size;
> > +		x_oword_off = x_tile_off % oword_size;
> > +		tile_off = x_oword_n * y_tile_size * oword_size
> > +			   + y_tile_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;
> >  }
> > @@ -170,7 +201,8 @@ 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)
> > +				    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;
> > @@ -181,22 +213,50 @@ static void tiled_pos_to_x_y_linear(int tiled_pos, uint32_t stride,
> >  	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;
> > +	if (tiling == I915_TILING_X ) {
> > +		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_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_n = tiled_pos / tile_size;
> > -	tile_off = tiled_pos % tile_size;
> > +		x_tile_n = tile_n % tiles_per_line;
> > +		y_tile_n = tile_n / tiles_per_line;
> > +
> > +		*x = (x_tile_n * x_tile_size + x_tile_off) / pixel_size;
> > +		*y = y_tile_n * y_tile_size + y_tile_off;
> > +	} else {
> > +		int x_oword_n, x_oword_off;
> > +		int oword_size;
> > 
> > -	y_tile_off = tile_off / x_tile_size;
> > -	x_tile_off = tile_off % x_tile_size;
> > +		oword_size = 16;
> > +		x_tile_size = 128;
> > +		y_tile_size = 32;
> > +		tile_size = x_tile_size * y_tile_size;
> > +		tiles_per_line = line_size / x_tile_size;
> > 
> > -	x_tile_n = tile_n % tiles_per_line;
> > -	y_tile_n = tile_n / tiles_per_line;
> > +		tile_n = tiled_pos / tile_size;
> > +		tile_off = tiled_pos % tile_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_n = tile_n % tiles_per_line;
> > +		y_tile_n = tile_n / tiles_per_line;
> > +
> > +		x_oword_n = tile_off / (oword_size * y_tile_size);
> > +		x_oword_off = tile_off % oword_size;
> > +
> > +		x_tile_off = x_oword_n * oword_size + x_oword_off;
> > +		y_tile_off = (tile_off - x_oword_n * oword_size * y_tile_size)
> > +			     / oword_size;
> > +
> > +		*x = (x_tile_n * x_tile_size + x_tile_off) / pixel_size;
> > +		*y = y_tile_n * y_tile_size + y_tile_off;
> > +
> > +	}
> >  }
> > 
> >  static void set_pixel(void *_ptr, int index, uint32_t color, int bpp)
> > @@ -225,14 +285,15 @@ 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)
> > +				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);
> > +						      bpp, tiling);
> >  			set_pixel(ptr, pos, color, bpp);
> >  		}
> >  	}
> > @@ -260,7 +321,11 @@ static void draw_rect_mmap_cpu(int fd, struct buf_data *buf, struct rect *rect,
> >  		break;
> >  	case I915_TILING_X:
> >  		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
> > -				    buf->bpp);
> > +				    buf->bpp, tiling);
> > +		break;
> > +	case I915_TILING_Y:
> > +		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
> > +				    buf->bpp, tiling);
> >  		break;
> >  	default:
> >  		igt_assert(false);
> > @@ -310,7 +375,11 @@ static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect *rect,
> >  		break;
> >  	case I915_TILING_X:
> >  		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
> > -				    buf->bpp);
> > +				    buf->bpp, tiling);
> > +		break;
> > +	case I915_TILING_Y:
> > +		draw_rect_ptr_tiled(ptr, buf->stride, swizzle, rect, color,
> > +				    buf->bpp, tiling);
> >  		break;
> >  	default:
> >  		igt_assert(false);
> > @@ -338,7 +407,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;
> > @@ -362,7 +431,7 @@ static void draw_rect_pwrite_tiled(int fd, struct buf_data *buf,
> > 
> >  	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);
> > +					buf->bpp, &x, &y, tiling);
> > 
> >  		if (x >= rect->x && x < rect->x + rect->w &&
> >  		    y >= rect->y && y < rect->y + rect->h) {
> > @@ -399,7 +468,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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] igt_fb: Add Y-tiling support

[Daniel Vetter]

On Tue, Oct 25, 2016 at 03:02:33PM +0000, Paneri, Praveen wrote:
> > So when you say that all Y tiling tests fail without this kernel hack, which tests you are referring to?
> If I revert this IGT patch and do not make below kernel change, kms_draw_crc (ytiled cases, last patch in this series) fail with following error.
> 
> Test assertion failure function igt_assert_crc_equal, file hardware/intel/intel-gpu-tools/lib/igt_debugfs.c:266:
> Failed assertion: a->crc[i] == b->crc[i]
> error: 0x68c96b8b != 0x948e53b
> 
> I think, I shall debug it further and try to fix it without making this change.

Can't we just set the fb modifiers when creating the drm_fb in the igt
helpers?
-Daniel

> 
> Regards,
> Praveen
> 
> 
> -----Original Message-----
> From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com] 
> Sent: Tuesday, October 25, 2016 6:07 PM
> To: Paneri, Praveen <praveen.paneri@intel.com>; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 1/4] igt_fb: Add Y-tiling support
> 
> 
> On 25/10/2016 13:06, Paneri, Praveen wrote:
> > Hi Tvrtko,
> >
> > Along with this change I made following change in the kernel side. I was not sure if this is a hack of a legitimate change. Could you please give me a pointer about how to move fwd from here? Without this all Y-tiling tests would fail.
> >
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -15406,8 +15406,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
> >                 if (obj->tiling_mode == I915_TILING_X)
> >                         mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED;
> >                 else if (obj->tiling_mode == I915_TILING_Y) {
> > -                       DRM_DEBUG("No Y tiling for legacy addfb\n");
> > -                       return -EINVAL;
> > +                       mode_cmd->modifier[0] = 
> > + I915_FORMAT_MOD_Y_TILED;
> >                 }
> >         }
> 
> It would be a controversial change, "beyond my pay grade". :)
> 
> If you drop this particular IGT patch, you can still create Y tiled framebuffers and display them (as the existing tests already do that). 
> So when you say that all Y tiling tests fail without this kernel hack, which tests you are referring to?
> 
> Regards,
> 
> Tvrtko
> 
> > Regards,
> > Praveen
> >
> >
> > -----Original Message-----
> > From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com]
> > Sent: Tuesday, October 25, 2016 1:36 PM
> > To: Paneri, Praveen <praveen.paneri@intel.com>; 
> > intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH 1/4] igt_fb: Add Y-tiling support
> >
> >
> > On 24/10/2016 17:55, Praveen Paneri wrote:
> >> This adds Y-tiling check in igt_create_fb_with_bo_size as now we 
> >> should also be able to create Y-tiled FBs.
> >>
> >> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> >> ---
> >>  lib/igt_fb.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/lib/igt_fb.c b/lib/igt_fb.c index 47472f4..bf1d372 
> >> 100644
> >> --- a/lib/igt_fb.c
> >> +++ b/lib/igt_fb.c
> >> @@ -608,7 +608,8 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
> >>  		  __func__, fb->gem_handle, fb->stride);
> >>
> >>  	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> >> -	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED) {
> >> +	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED &&
> >> +	    tiling != LOCAL_I915_FORMAT_MOD_Y_TILED) {
> >>  		do_or_die(__kms_addfb(fd, fb->gem_handle, width, height,
> >>  				      fb->stride, format, tiling,
> >>  				      LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id));
> >>
> >
> > This works now? Ie. doesn't hit "No Y Tiling for legacy addfb" error in the driver?
> >
> > Regards,
> >
> > Tvrtko
> >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] igt_fb: Add Y-tiling support

[Tvrtko Ursulin]

On 26/10/2016 07:32, Daniel Vetter wrote:
> On Tue, Oct 25, 2016 at 03:02:33PM +0000, Paneri, Praveen wrote:
>>> So when you say that all Y tiling tests fail without this kernel hack, which tests you are referring to?
>> If I revert this IGT patch and do not make below kernel change, kms_draw_crc (ytiled cases, last patch in this series) fail with following error.
>>
>> Test assertion failure function igt_assert_crc_equal, file hardware/intel/intel-gpu-tools/lib/igt_debugfs.c:266:
>> Failed assertion: a->crc[i] == b->crc[i]
>> error: 0x68c96b8b != 0x948e53b
>>
>> I think, I shall debug it further and try to fix it without making this change.
>
> Can't we just set the fb modifiers when creating the drm_fb in the igt
> helpers?

Thats already there, if I understand what you mean. But it only does the 
modifer and does not set the object tiling. So I suspect this test might 
be depending on object tiling being set as well? In which case we  may 
be missing the capability to create the object separately from the fb 
creation, because otherwise it is too late to try to change the object 
tiling.

Regards,

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

Re: [PATCH 1/4] igt_fb: Add Y-tiling support

[Ville Syrjälä]

On Wed, Oct 26, 2016 at 09:09:26AM +0100, Tvrtko Ursulin wrote:
> 
> On 26/10/2016 07:32, Daniel Vetter wrote:
> > On Tue, Oct 25, 2016 at 03:02:33PM +0000, Paneri, Praveen wrote:
> >>> So when you say that all Y tiling tests fail without this kernel hack, which tests you are referring to?
> >> If I revert this IGT patch and do not make below kernel change, kms_draw_crc (ytiled cases, last patch in this series) fail with following error.
> >>
> >> Test assertion failure function igt_assert_crc_equal, file hardware/intel/intel-gpu-tools/lib/igt_debugfs.c:266:
> >> Failed assertion: a->crc[i] == b->crc[i]
> >> error: 0x68c96b8b != 0x948e53b
> >>
> >> I think, I shall debug it further and try to fix it without making this change.
> >
> > Can't we just set the fb modifiers when creating the drm_fb in the igt
> > helpers?
> 
> Thats already there, if I understand what you mean. But it only does the 
> modifer and does not set the object tiling. So I suspect this test might 
> be depending on object tiling being set as well? In which case we  may 
> be missing the capability to create the object separately from the fb 
> creation, because otherwise it is too late to try to change the object 
> tiling.

I'm pretty sure I had patches to split the two apart. Assuming I never
posted them, they must be on a branch... somewhere.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] igt_fb: Add Y-tiling support

[Ville Syrjälä]

On Wed, Oct 26, 2016 at 11:20:51AM +0300, Ville Syrjälä wrote:
> On Wed, Oct 26, 2016 at 09:09:26AM +0100, Tvrtko Ursulin wrote:
> > 
> > On 26/10/2016 07:32, Daniel Vetter wrote:
> > > On Tue, Oct 25, 2016 at 03:02:33PM +0000, Paneri, Praveen wrote:
> > >>> So when you say that all Y tiling tests fail without this kernel hack, which tests you are referring to?
> > >> If I revert this IGT patch and do not make below kernel change, kms_draw_crc (ytiled cases, last patch in this series) fail with following error.
> > >>
> > >> Test assertion failure function igt_assert_crc_equal, file hardware/intel/intel-gpu-tools/lib/igt_debugfs.c:266:
> > >> Failed assertion: a->crc[i] == b->crc[i]
> > >> error: 0x68c96b8b != 0x948e53b
> > >>
> > >> I think, I shall debug it further and try to fix it without making this change.
> > >
> > > Can't we just set the fb modifiers when creating the drm_fb in the igt
> > > helpers?
> > 
> > Thats already there, if I understand what you mean. But it only does the 
> > modifer and does not set the object tiling. So I suspect this test might 
> > be depending on object tiling being set as well? In which case we  may 
> > be missing the capability to create the object separately from the fb 
> > creation, because otherwise it is too late to try to change the object 
> > tiling.
> 
> I'm pretty sure I had patches to split the two apart. Assuming I never
> posted them, they must be on a branch... somewhere.

Failed to find them. So someone will have to reimplement if needed.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] igt_fb: Add Y-tiling support

[Daniel Vetter]

On Wed, Oct 26, 2016 at 09:09:26AM +0100, Tvrtko Ursulin wrote:
> 
> On 26/10/2016 07:32, Daniel Vetter wrote:
> > On Tue, Oct 25, 2016 at 03:02:33PM +0000, Paneri, Praveen wrote:
> > > > So when you say that all Y tiling tests fail without this kernel hack, which tests you are referring to?
> > > If I revert this IGT patch and do not make below kernel change, kms_draw_crc (ytiled cases, last patch in this series) fail with following error.
> > > 
> > > Test assertion failure function igt_assert_crc_equal, file hardware/intel/intel-gpu-tools/lib/igt_debugfs.c:266:
> > > Failed assertion: a->crc[i] == b->crc[i]
> > > error: 0x68c96b8b != 0x948e53b
> > > 
> > > I think, I shall debug it further and try to fix it without making this change.
> > 
> > Can't we just set the fb modifiers when creating the drm_fb in the igt
> > helpers?
> 
> Thats already there, if I understand what you mean. But it only does the
> modifer and does not set the object tiling. So I suspect this test might be
> depending on object tiling being set as well? In which case we  may be
> missing the capability to create the object separately from the fb creation,
> because otherwise it is too late to try to change the object tiling.

Yeah I guess then my question is: Why can't we use that? This patch sounds
like duct-tape instead of proper bugfix ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx