[PATCH igt 1/3] lib/igt_fb: also call __gem_set_tiling for Y tiling

[PATCH igt 1/3] lib/igt_fb: also call __gem_set_tiling for Y tiling

[Paulo Zanoni]

The interesting thing is that if we don't do this, we still get a
Y tiled framebuffer, but there won't be a fence around it, which makes
the GTT mmaps less interesting. Is this a Kernel bug?

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 lib/igt_fb.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 5f23136..efdd793 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -73,6 +73,22 @@ static struct format_desc_struct {
 #define for_each_format(f)	\
 	for (f = format_desc; f - format_desc < ARRAY_SIZE(format_desc); f++)
 
+static unsigned int fb_mod_to_obj_tiling(uint64_t fb_mod)
+{
+	switch (fb_mod) {
+	case LOCAL_DRM_FORMAT_MOD_NONE:
+		return I915_TILING_NONE;
+	case LOCAL_I915_FORMAT_MOD_X_TILED:
+		return I915_TILING_X;
+	case LOCAL_I915_FORMAT_MOD_Y_TILED:
+		return I915_TILING_Y;
+	case LOCAL_I915_FORMAT_MOD_Yf_TILED:
+		return I915_TILING_Yf;
+	default:
+		igt_assert(0);
+	}
+}
+
 static void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
 				 unsigned *width_ret, unsigned *height_ret)
 {
@@ -191,9 +207,10 @@ static int create_bo_for_fb(int fd, int width, int height, int bpp,
 
 	gem_handle = gem_create(fd, bo_size);
 
-	if (tiling == LOCAL_I915_FORMAT_MOD_X_TILED)
-		ret = __gem_set_tiling(fd, gem_handle, I915_TILING_X,
-				       bo_stride);
+	if (tiling == LOCAL_I915_FORMAT_MOD_X_TILED ||
+	    tiling == LOCAL_I915_FORMAT_MOD_Y_TILED)
+		ret = __gem_set_tiling(fd, gem_handle,
+				      fb_mod_to_obj_tiling(tiling), bo_stride);
 
 	*stride_ret = bo_stride;
 	*size_ret = bo_size;
@@ -862,22 +879,6 @@ struct fb_blit_upload {
 	} linear;
 };
 
-static unsigned int fb_mod_to_obj_tiling(uint64_t fb_mod)
-{
-	switch (fb_mod) {
-	case LOCAL_DRM_FORMAT_MOD_NONE:
-		return I915_TILING_NONE;
-	case LOCAL_I915_FORMAT_MOD_X_TILED:
-		return I915_TILING_X;
-	case LOCAL_I915_FORMAT_MOD_Y_TILED:
-		return I915_TILING_Y;
-	case LOCAL_I915_FORMAT_MOD_Yf_TILED:
-		return I915_TILING_Yf;
-	default:
-		igt_assert(0);
-	}
-}
-
 static void destroy_cairo_surface__blit(void *arg)
 {
 	struct fb_blit_upload *blit = arg;
-- 
2.7.0.rc3

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

[PATCH igt 2/3] lib/igt_draw: add support for Y tiling

[Paulo Zanoni]

Most of the patch is to change the tile/untile functions so they can
work with Y-major tiling.

We're also skipping on BLT for Y-tiling. Some parts of the spec
suggest this doesn't work and some parts of the spec suggest it will
work. Since I couldn't make it work, SKIP for now.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 lib/igt_draw.c | 173 ++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 123 insertions(+), 50 deletions(-)

diff --git a/lib/igt_draw.c b/lib/igt_draw.c
index 45fa10f..468a1eb 100644
--- a/lib/igt_draw.c
+++ b/lib/igt_draw.c
@@ -134,71 +134,118 @@ static int swizzle_addr(int addr, int swizzle)
 	return addr;
 }
 
-/* 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 tile(int x, int y, uint32_t x_tile_size, uint32_t y_tile_size,
+		uint32_t line_size, bool xmajor)
 {
-	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;
-	int tile_n, tile_off;
-	int tiled_pos, tiles_per_line;
-	int pixel_size = bpp / 8;
+	int tile_size, tiles_per_line, x_tile_n, y_tile_n, tile_off, pos;
+	int tile_n, x_tile_off, y_tile_off;
 
-	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 = x_tile_size * y_tile_size;
 
+	x_tile_n = x / x_tile_size;
 	y_tile_n = y / y_tile_size;
+	tile_n = y_tile_n * tiles_per_line + x_tile_n;
+
+	x_tile_off = x % x_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;
+	if (xmajor)
+		tile_off = y_tile_off * x_tile_size + x_tile_off;
+	else
+		tile_off = x_tile_off * y_tile_size + y_tile_off;
 
-	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;
+	pos = tile_n * tile_size + tile_off;
 
-	tiled_pos = swizzle_addr(tiled_pos, swizzle);
-
-	return tiled_pos / pixel_size;
+	return pos;
 }
 
-/* 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 untile(int tiled_pos, int x_tile_size, int y_tile_size,
+		   uint32_t line_size, bool xmajor, int *x, int *y)
 {
-	int tile_n, tile_off, tiles_per_line, line_size;
+	int tile_n, tile_off, tiles_per_line;
 	int x_tile_off, y_tile_off;
 	int x_tile_n, y_tile_n;
-	int x_tile_size, y_tile_size, tile_size;
-	int pixel_size = bpp / 8;
-
-	tiled_pos = swizzle_addr(tiled_pos, swizzle);
+	int tile_size;
 
-	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_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;
+	if (xmajor) {
+		y_tile_off = tile_off / x_tile_size;
+		x_tile_off = tile_off % x_tile_size;
+	} else {
+		y_tile_off = tile_off % y_tile_size;
+		x_tile_off = tile_off / y_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;
+	*x = (x_tile_n * x_tile_size + x_tile_off);
 	*y = y_tile_n * y_tile_size + y_tile_off;
 }
 
+static int linear_x_y_to_xtiled_pos(int x, int y, uint32_t stride, int swizzle,
+				    int bpp)
+{
+	int pos;
+	int pixel_size = bpp / 8;
+
+	x *= pixel_size;
+	pos = tile(x, y, 512, 8, stride, true);
+	pos = swizzle_addr(pos, swizzle);
+	return pos / pixel_size;
+}
+
+static int linear_x_y_to_ytiled_pos(int x, int y, uint32_t stride, int swizzle,
+				    int bpp)
+{
+	int ow_tile_n, pos;
+	int ow_size = 16;
+	int pixel_size = bpp / 8;
+
+	/* We have an Y tiling of OWords, so use the tile() function to get the
+	 * OW number, then adjust to the fact that the OW may have more than one
+	 * pixel. */
+	x *= pixel_size;
+	ow_tile_n = tile(x / ow_size, y, 128 / ow_size, 32,
+			 stride / ow_size, false);
+	pos = ow_tile_n * ow_size + (x % ow_size);
+	pos = swizzle_addr(pos, swizzle);
+	return pos / pixel_size;
+}
+
+static void xtiled_pos_to_x_y_linear(int tiled_pos, uint32_t stride,
+				     int swizzle, int bpp, int *x, int *y)
+{
+	int pixel_size = bpp / 8;
+
+	tiled_pos = swizzle_addr(tiled_pos, swizzle);
+
+	untile(tiled_pos, 512, 8, stride, true, x, y);
+	*x /= pixel_size;
+}
+
+static void ytiled_pos_to_x_y_linear(int tiled_pos, uint32_t stride,
+				     int swizzle, int bpp, int *x, int *y)
+{
+	int ow_tile_n;
+	int ow_size = 16;
+	int pixel_size = bpp / 8;
+
+	tiled_pos = swizzle_addr(tiled_pos, swizzle);
+
+	ow_tile_n = tiled_pos / ow_size;
+	untile(ow_tile_n, 128 / ow_size, 32, stride / ow_size, false, x, y);
+	*x *= ow_size;
+	*x += tiled_pos % ow_size;
+	*x /= pixel_size;
+}
+
 static void set_pixel(void *_ptr, int index, uint32_t color, int bpp)
 {
 	if (bpp == 16) {
@@ -224,15 +271,26 @@ 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(void *ptr, uint32_t stride, uint32_t tiling,
+				int swizzle, struct rect *rect, uint32_t color,
+				int bpp)
 {
 	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);
+			switch (tiling) {
+			case I915_TILING_X:
+				pos = linear_x_y_to_xtiled_pos(x, y, stride,
+							       swizzle, bpp);
+				break;
+			case I915_TILING_Y:
+				pos = linear_x_y_to_ytiled_pos(x, y, stride,
+							       swizzle, bpp);
+				break;
+			default:
+				igt_assert(false);
+			}
 			set_pixel(ptr, pos, color, bpp);
 		}
 	}
@@ -259,8 +317,9 @@ 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);
+	case I915_TILING_Y:
+		draw_rect_ptr_tiled(ptr, buf->stride, tiling, swizzle, rect,
+				    color, buf->bpp);
 		break;
 	default:
 		igt_assert(false);
@@ -309,8 +368,9 @@ 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);
+	case I915_TILING_Y:
+		draw_rect_ptr_tiled(ptr, buf->stride, tiling, swizzle, rect,
+				    color, buf->bpp);
 		break;
 	default:
 		igt_assert(false);
@@ -337,8 +397,8 @@ 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 tiling, struct rect *rect,
+				   uint32_t color, uint32_t swizzle)
 {
 	int i;
 	int tiled_pos, x, y, pixel_size;
@@ -361,8 +421,18 @@ 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);
+		switch (tiling) {
+		case I915_TILING_X:
+			xtiled_pos_to_x_y_linear(tiled_pos, buf->stride,
+						 swizzle, buf->bpp, &x, &y);
+			break;
+		case I915_TILING_Y:
+			ytiled_pos_to_x_y_linear(tiled_pos, buf->stride,
+						 swizzle, buf->bpp, &x, &y);
+			break;
+		default:
+			igt_assert(false);
+		}
 
 		if (x >= rect->x && x < rect->x + rect->w &&
 		    y >= rect->y && y < rect->y + rect->h) {
@@ -399,7 +469,8 @@ 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);
+	case I915_TILING_Y:
+		draw_rect_pwrite_tiled(fd, buf, tiling, rect, color, swizzle);
 		break;
 	default:
 		igt_assert(false);
@@ -421,6 +492,8 @@ static void draw_rect_blt(int fd, struct cmd_data *cmd_data,
 
 	gem_get_tiling(fd, buf->handle, &tiling, &swizzle);
 
+	igt_require(tiling != I915_TILING_Y);
+
 	dst = gem_handle_to_libdrm_bo(cmd_data->bufmgr, fd, "", buf->handle);
 	igt_assert(dst);
 
-- 
2.7.0.rc3

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

[PATCH igt 3/3] tests/kms_draw_crc: add support for Y tiling

[Paulo Zanoni]

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>
---
 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 3f80174..78d1e53 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,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);
 
 	find_modeset_params();
@@ -217,6 +227,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);
 }
@@ -273,28 +288,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();
-- 
2.7.0.rc3

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

Re: [PATCH igt 1/3] lib/igt_fb: also call __gem_set_tiling for Y tiling

[Ville Syrjälä]

On Fri, Jan 29, 2016 at 04:46:30PM -0200, Paulo Zanoni wrote:
> The interesting thing is that if we don't do this, we still get a
> Y tiled framebuffer, but there won't be a fence around it, which makes
> the GTT mmaps less interesting. Is this a Kernel bug?

I think some tests currently depend on not having a fence for Y tiled
fbs. So this could break stuff.

> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  lib/igt_fb.c | 39 ++++++++++++++++++++-------------------
>  1 file changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 5f23136..efdd793 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -73,6 +73,22 @@ static struct format_desc_struct {
>  #define for_each_format(f)	\
>  	for (f = format_desc; f - format_desc < ARRAY_SIZE(format_desc); f++)
>  
> +static unsigned int fb_mod_to_obj_tiling(uint64_t fb_mod)
> +{
> +	switch (fb_mod) {
> +	case LOCAL_DRM_FORMAT_MOD_NONE:
> +		return I915_TILING_NONE;
> +	case LOCAL_I915_FORMAT_MOD_X_TILED:
> +		return I915_TILING_X;
> +	case LOCAL_I915_FORMAT_MOD_Y_TILED:
> +		return I915_TILING_Y;
> +	case LOCAL_I915_FORMAT_MOD_Yf_TILED:
> +		return I915_TILING_Yf;
> +	default:
> +		igt_assert(0);
> +	}
> +}
> +
>  static void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
>  				 unsigned *width_ret, unsigned *height_ret)
>  {
> @@ -191,9 +207,10 @@ static int create_bo_for_fb(int fd, int width, int height, int bpp,
>  
>  	gem_handle = gem_create(fd, bo_size);
>  
> -	if (tiling == LOCAL_I915_FORMAT_MOD_X_TILED)
> -		ret = __gem_set_tiling(fd, gem_handle, I915_TILING_X,
> -				       bo_stride);
> +	if (tiling == LOCAL_I915_FORMAT_MOD_X_TILED ||
> +	    tiling == LOCAL_I915_FORMAT_MOD_Y_TILED)
> +		ret = __gem_set_tiling(fd, gem_handle,
> +				      fb_mod_to_obj_tiling(tiling), bo_stride);
>  
>  	*stride_ret = bo_stride;
>  	*size_ret = bo_size;
> @@ -862,22 +879,6 @@ struct fb_blit_upload {
>  	} linear;
>  };
>  
> -static unsigned int fb_mod_to_obj_tiling(uint64_t fb_mod)
> -{
> -	switch (fb_mod) {
> -	case LOCAL_DRM_FORMAT_MOD_NONE:
> -		return I915_TILING_NONE;
> -	case LOCAL_I915_FORMAT_MOD_X_TILED:
> -		return I915_TILING_X;
> -	case LOCAL_I915_FORMAT_MOD_Y_TILED:
> -		return I915_TILING_Y;
> -	case LOCAL_I915_FORMAT_MOD_Yf_TILED:
> -		return I915_TILING_Yf;
> -	default:
> -		igt_assert(0);
> -	}
> -}
> -
>  static void destroy_cairo_surface__blit(void *arg)
>  {
>  	struct fb_blit_upload *blit = arg;
> -- 
> 2.7.0.rc3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH igt 1/3] lib/igt_fb: also call __gem_set_tiling for Y tiling

[Zanoni, Paulo R]

Em Sex, 2016-01-29 às 21:06 +0200, Ville Syrjälä escreveu:
> On Fri, Jan 29, 2016 at 04:46:30PM -0200, Paulo Zanoni wrote:
> > The interesting thing is that if we don't do this, we still get a
> > Y tiled framebuffer, but there won't be a fence around it, which
> > makes
> > the GTT mmaps less interesting. Is this a Kernel bug?
> 
> I think some tests currently depend on not having a fence for Y tiled
> fbs. So this could break stuff.

Do you have any additional information that could help me discover
which ones? A quick look on the IGT tests mentioning tiling didn't
point anything obvious.

Besides, I think it's probably not a good idea to have such a high
level helper function behaving differently depending on the tiling
type, I'd vote to either call set_tiling on both or on none.

> 
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  lib/igt_fb.c | 39 ++++++++++++++++++++-------------------
> >  1 file changed, 20 insertions(+), 19 deletions(-)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 5f23136..efdd793 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -73,6 +73,22 @@ static struct format_desc_struct {
> >  #define for_each_format(f)	\
> >  	for (f = format_desc; f - format_desc <
> > ARRAY_SIZE(format_desc); f++)
> >  
> > +static unsigned int fb_mod_to_obj_tiling(uint64_t fb_mod)
> > +{
> > +	switch (fb_mod) {
> > +	case LOCAL_DRM_FORMAT_MOD_NONE:
> > +		return I915_TILING_NONE;
> > +	case LOCAL_I915_FORMAT_MOD_X_TILED:
> > +		return I915_TILING_X;
> > +	case LOCAL_I915_FORMAT_MOD_Y_TILED:
> > +		return I915_TILING_Y;
> > +	case LOCAL_I915_FORMAT_MOD_Yf_TILED:
> > +		return I915_TILING_Yf;
> > +	default:
> > +		igt_assert(0);
> > +	}
> > +}
> > +
> >  static void igt_get_fb_tile_size(int fd, uint64_t tiling, int
> > fb_bpp,
> >  				 unsigned *width_ret, unsigned
> > *height_ret)
> >  {
> > @@ -191,9 +207,10 @@ static int create_bo_for_fb(int fd, int width,
> > int height, int bpp,
> >  
> >  	gem_handle = gem_create(fd, bo_size);
> >  
> > -	if (tiling == LOCAL_I915_FORMAT_MOD_X_TILED)
> > -		ret = __gem_set_tiling(fd, gem_handle,
> > I915_TILING_X,
> > -				       bo_stride);
> > +	if (tiling == LOCAL_I915_FORMAT_MOD_X_TILED ||
> > +	    tiling == LOCAL_I915_FORMAT_MOD_Y_TILED)
> > +		ret = __gem_set_tiling(fd, gem_handle,
> > +				      fb_mod_to_obj_tiling(tiling)
> > , bo_stride);
> >  
> >  	*stride_ret = bo_stride;
> >  	*size_ret = bo_size;
> > @@ -862,22 +879,6 @@ struct fb_blit_upload {
> >  	} linear;
> >  };
> >  
> > -static unsigned int fb_mod_to_obj_tiling(uint64_t fb_mod)
> > -{
> > -	switch (fb_mod) {
> > -	case LOCAL_DRM_FORMAT_MOD_NONE:
> > -		return I915_TILING_NONE;
> > -	case LOCAL_I915_FORMAT_MOD_X_TILED:
> > -		return I915_TILING_X;
> > -	case LOCAL_I915_FORMAT_MOD_Y_TILED:
> > -		return I915_TILING_Y;
> > -	case LOCAL_I915_FORMAT_MOD_Yf_TILED:
> > -		return I915_TILING_Yf;
> > -	default:
> > -		igt_assert(0);
> > -	}
> > -}
> > -
> >  static void destroy_cairo_surface__blit(void *arg)
> >  {
> >  	struct fb_blit_upload *blit = arg;
> > -- 
> > 2.7.0.rc3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH igt 1/3] lib/igt_fb: also call __gem_set_tiling for Y tiling

[Ville Syrjälä]

On Mon, Feb 01, 2016 at 05:16:25PM +0000, Zanoni, Paulo R wrote:
> Em Sex, 2016-01-29 às 21:06 +0200, Ville Syrjälä escreveu:
> > On Fri, Jan 29, 2016 at 04:46:30PM -0200, Paulo Zanoni wrote:
> > > The interesting thing is that if we don't do this, we still get a
> > > Y tiled framebuffer, but there won't be a fence around it, which
> > > makes
> > > the GTT mmaps less interesting. Is this a Kernel bug?
> > 
> > I think some tests currently depend on not having a fence for Y tiled
> > fbs. So this could break stuff.
> 
> Do you have any additional information that could help me discover
> which ones? A quick look on the IGT tests mentioning tiling didn't
> point anything obvious.

Some Y tiling/rotation ones, can't recall the specific details. There
was some discussion on this recently when I suggested that we could
just use a fence on Y tiled buffers as well.

Also if you look further, the cairo glue will actually use a linear temp
buffer for rendering and then blit between the linear temp buffer and
the Y tiled fb. Though it's only implemented for SKL+ fast blit so far.
I have a patch locally adding support for the regular blit as well
since I wanted to play around with that stuff on gen7/X tiled too.

> 
> Besides, I think it's probably not a good idea to have such a high
> level helper function behaving differently depending on the tiling
> type, I'd vote to either call set_tiling on both or on none.
> 
> > 
> > > 
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > >  lib/igt_fb.c | 39 ++++++++++++++++++++-------------------
> > >  1 file changed, 20 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > index 5f23136..efdd793 100644
> > > --- a/lib/igt_fb.c
> > > +++ b/lib/igt_fb.c
> > > @@ -73,6 +73,22 @@ static struct format_desc_struct {
> > >  #define for_each_format(f)	\
> > >  	for (f = format_desc; f - format_desc <
> > > ARRAY_SIZE(format_desc); f++)
> > >  
> > > +static unsigned int fb_mod_to_obj_tiling(uint64_t fb_mod)
> > > +{
> > > +	switch (fb_mod) {
> > > +	case LOCAL_DRM_FORMAT_MOD_NONE:
> > > +		return I915_TILING_NONE;
> > > +	case LOCAL_I915_FORMAT_MOD_X_TILED:
> > > +		return I915_TILING_X;
> > > +	case LOCAL_I915_FORMAT_MOD_Y_TILED:
> > > +		return I915_TILING_Y;
> > > +	case LOCAL_I915_FORMAT_MOD_Yf_TILED:
> > > +		return I915_TILING_Yf;
> > > +	default:
> > > +		igt_assert(0);
> > > +	}
> > > +}
> > > +
> > >  static void igt_get_fb_tile_size(int fd, uint64_t tiling, int
> > > fb_bpp,
> > >  				 unsigned *width_ret, unsigned
> > > *height_ret)
> > >  {
> > > @@ -191,9 +207,10 @@ static int create_bo_for_fb(int fd, int width,
> > > int height, int bpp,
> > >  
> > >  	gem_handle = gem_create(fd, bo_size);
> > >  
> > > -	if (tiling == LOCAL_I915_FORMAT_MOD_X_TILED)
> > > -		ret = __gem_set_tiling(fd, gem_handle,
> > > I915_TILING_X,
> > > -				       bo_stride);
> > > +	if (tiling == LOCAL_I915_FORMAT_MOD_X_TILED ||
> > > +	    tiling == LOCAL_I915_FORMAT_MOD_Y_TILED)
> > > +		ret = __gem_set_tiling(fd, gem_handle,
> > > +				      fb_mod_to_obj_tiling(tiling)
> > > , bo_stride);
> > >  
> > >  	*stride_ret = bo_stride;
> > >  	*size_ret = bo_size;
> > > @@ -862,22 +879,6 @@ struct fb_blit_upload {
> > >  	} linear;
> > >  };
> > >  
> > > -static unsigned int fb_mod_to_obj_tiling(uint64_t fb_mod)
> > > -{
> > > -	switch (fb_mod) {
> > > -	case LOCAL_DRM_FORMAT_MOD_NONE:
> > > -		return I915_TILING_NONE;
> > > -	case LOCAL_I915_FORMAT_MOD_X_TILED:
> > > -		return I915_TILING_X;
> > > -	case LOCAL_I915_FORMAT_MOD_Y_TILED:
> > > -		return I915_TILING_Y;
> > > -	case LOCAL_I915_FORMAT_MOD_Yf_TILED:
> > > -		return I915_TILING_Yf;
> > > -	default:
> > > -		igt_assert(0);
> > > -	}
> > > -}
> > > -
> > >  static void destroy_cairo_surface__blit(void *arg)
> > >  {
> > >  	struct fb_blit_upload *blit = arg;
> > > -- 
> > > 2.7.0.rc3
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 

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

Re: [PATCH igt 1/3] lib/igt_fb: also call __gem_set_tiling for Y tiling

[Tvrtko Ursulin]

On 01/02/16 17:16, Zanoni, Paulo R wrote:
> Em Sex, 2016-01-29 às 21:06 +0200, Ville Syrjälä escreveu:
>> On Fri, Jan 29, 2016 at 04:46:30PM -0200, Paulo Zanoni wrote:
>>> The interesting thing is that if we don't do this, we still get a
>>> Y tiled framebuffer, but there won't be a fence around it, which
>>> makes
>>> the GTT mmaps less interesting. Is this a Kernel bug?
>>
>> I think some tests currently depend on not having a fence for Y tiled
>> fbs. So this could break stuff.
>
> Do you have any additional information that could help me discover
> which ones? A quick look on the IGT tests mentioning tiling didn't
> point anything obvious.
>
> Besides, I think it's probably not a good idea to have such a high
> level helper function behaving differently depending on the tiling
> type, I'd vote to either call set_tiling on both or on none.

Noticed the thread by accident. :)

I can't help with the question of which tests might be affected by this. 
Some low level ones like kms_addfb don't use the fb helpers so they 
shouldn't be. Can't remember if any other would be.

But just a little bit of background:

Basically with the introduction of Y tiled (and Yf) scanout in Gen9 we 
have forked the path and destroyed the coupling between obj->tiling and 
framebuffer tiling.

The X special casing in create_bo_for_fb is for compatibility with old 
userspace, but going forward it was decided fb  modifiers should be used 
to tell the driver about tiling and get/set_tiling ioctl is about 
fencing and only that.

Paths implemented in IGT back then were rendering to Y and Yf tiling fbs 
via a temporary linear surface which is then blitted (blit?) to the real 
fb obj. (With the blitter doing the appropriate transformation.)

So in that respect adding Y tiling to create_bo_for_fb would be wrong 
because it is not aligned with the above, and also you cannot support Yf 
this way at all.

But I do agree this creates a problem for some use cases within the IGT 
since the fb and backing obj are created atomically and once that is 
done you cannot fiddle with obj->tiling (aka fencing).

Regards,

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

Re: [PATCH igt 1/3] lib/igt_fb: also call __gem_set_tiling for Y tiling

[Ville Syrjälä]

On Mon, Feb 01, 2016 at 05:44:42PM +0000, Tvrtko Ursulin wrote:
> 
> On 01/02/16 17:16, Zanoni, Paulo R wrote:
> > Em Sex, 2016-01-29 às 21:06 +0200, Ville Syrjälä escreveu:
> >> On Fri, Jan 29, 2016 at 04:46:30PM -0200, Paulo Zanoni wrote:
> >>> The interesting thing is that if we don't do this, we still get a
> >>> Y tiled framebuffer, but there won't be a fence around it, which
> >>> makes
> >>> the GTT mmaps less interesting. Is this a Kernel bug?
> >>
> >> I think some tests currently depend on not having a fence for Y tiled
> >> fbs. So this could break stuff.
> >
> > Do you have any additional information that could help me discover
> > which ones? A quick look on the IGT tests mentioning tiling didn't
> > point anything obvious.
> >
> > Besides, I think it's probably not a good idea to have such a high
> > level helper function behaving differently depending on the tiling
> > type, I'd vote to either call set_tiling on both or on none.
> 
> Noticed the thread by accident. :)
> 
> I can't help with the question of which tests might be affected by this. 
> Some low level ones like kms_addfb don't use the fb helpers so they 
> shouldn't be. Can't remember if any other would be.
> 
> But just a little bit of background:
> 
> Basically with the introduction of Y tiled (and Yf) scanout in Gen9 we 
> have forked the path and destroyed the coupling between obj->tiling and 
> framebuffer tiling.
> 
> The X special casing in create_bo_for_fb is for compatibility with old 
> userspace, but going forward it was decided fb  modifiers should be used 
> to tell the driver about tiling and get/set_tiling ioctl is about 
> fencing and only that.
> 
> Paths implemented in IGT back then were rendering to Y and Yf tiling fbs 
> via a temporary linear surface which is then blitted (blit?) to the real 
> fb obj. (With the blitter doing the appropriate transformation.)
> 
> So in that respect adding Y tiling to create_bo_for_fb would be wrong 
> because it is not aligned with the above, and also you cannot support Yf 
> this way at all.
> 
> But I do agree this creates a problem for some use cases within the IGT 
> since the fb and backing obj are created atomically and once that is 
> done you cannot fiddle with obj->tiling (aka fencing).

I suppose we could either make it easier to create the obj and fb
separately, or we could add a parameter to the fb funcs to indicate
whether we want a fence or not.

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

Re: [PATCH igt 1/3] lib/igt_fb: also call __gem_set_tiling for Y tiling

[Tvrtko Ursulin]

On 01/02/16 17:57, Ville Syrjälä wrote:
> On Mon, Feb 01, 2016 at 05:44:42PM +0000, Tvrtko Ursulin wrote:
>>
>> On 01/02/16 17:16, Zanoni, Paulo R wrote:
>>> Em Sex, 2016-01-29 às 21:06 +0200, Ville Syrjälä escreveu:
>>>> On Fri, Jan 29, 2016 at 04:46:30PM -0200, Paulo Zanoni wrote:
>>>>> The interesting thing is that if we don't do this, we still get a
>>>>> Y tiled framebuffer, but there won't be a fence around it, which
>>>>> makes
>>>>> the GTT mmaps less interesting. Is this a Kernel bug?
>>>>
>>>> I think some tests currently depend on not having a fence for Y tiled
>>>> fbs. So this could break stuff.
>>>
>>> Do you have any additional information that could help me discover
>>> which ones? A quick look on the IGT tests mentioning tiling didn't
>>> point anything obvious.
>>>
>>> Besides, I think it's probably not a good idea to have such a high
>>> level helper function behaving differently depending on the tiling
>>> type, I'd vote to either call set_tiling on both or on none.
>>
>> Noticed the thread by accident. :)
>>
>> I can't help with the question of which tests might be affected by this.
>> Some low level ones like kms_addfb don't use the fb helpers so they
>> shouldn't be. Can't remember if any other would be.
>>
>> But just a little bit of background:
>>
>> Basically with the introduction of Y tiled (and Yf) scanout in Gen9 we
>> have forked the path and destroyed the coupling between obj->tiling and
>> framebuffer tiling.
>>
>> The X special casing in create_bo_for_fb is for compatibility with old
>> userspace, but going forward it was decided fb  modifiers should be used
>> to tell the driver about tiling and get/set_tiling ioctl is about
>> fencing and only that.
>>
>> Paths implemented in IGT back then were rendering to Y and Yf tiling fbs
>> via a temporary linear surface which is then blitted (blit?) to the real
>> fb obj. (With the blitter doing the appropriate transformation.)
>>
>> So in that respect adding Y tiling to create_bo_for_fb would be wrong
>> because it is not aligned with the above, and also you cannot support Yf
>> this way at all.
>>
>> But I do agree this creates a problem for some use cases within the IGT
>> since the fb and backing obj are created atomically and once that is
>> done you cannot fiddle with obj->tiling (aka fencing).
>
> I suppose we could either make it easier to create the obj and fb
> separately, or we could add a parameter to the fb funcs to indicate
> whether we want a fence or not.

Either way sounds good to me. Will depend on whatever fits better with 
what Paulo is working on at the moment.

Regards,

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

Re: [PATCH igt 1/3] lib/igt_fb: also call __gem_set_tiling for Y tiling

[Daniel Vetter]

On Tue, Feb 02, 2016 at 09:34:11AM +0000, Tvrtko Ursulin wrote:
> 
> On 01/02/16 17:57, Ville Syrjälä wrote:
> >On Mon, Feb 01, 2016 at 05:44:42PM +0000, Tvrtko Ursulin wrote:
> >>
> >>On 01/02/16 17:16, Zanoni, Paulo R wrote:
> >>>Em Sex, 2016-01-29 às 21:06 +0200, Ville Syrjälä escreveu:
> >>>>On Fri, Jan 29, 2016 at 04:46:30PM -0200, Paulo Zanoni wrote:
> >>>>>The interesting thing is that if we don't do this, we still get a
> >>>>>Y tiled framebuffer, but there won't be a fence around it, which
> >>>>>makes
> >>>>>the GTT mmaps less interesting. Is this a Kernel bug?
> >>>>
> >>>>I think some tests currently depend on not having a fence for Y tiled
> >>>>fbs. So this could break stuff.
> >>>
> >>>Do you have any additional information that could help me discover
> >>>which ones? A quick look on the IGT tests mentioning tiling didn't
> >>>point anything obvious.
> >>>
> >>>Besides, I think it's probably not a good idea to have such a high
> >>>level helper function behaving differently depending on the tiling
> >>>type, I'd vote to either call set_tiling on both or on none.
> >>
> >>Noticed the thread by accident. :)
> >>
> >>I can't help with the question of which tests might be affected by this.
> >>Some low level ones like kms_addfb don't use the fb helpers so they
> >>shouldn't be. Can't remember if any other would be.
> >>
> >>But just a little bit of background:
> >>
> >>Basically with the introduction of Y tiled (and Yf) scanout in Gen9 we
> >>have forked the path and destroyed the coupling between obj->tiling and
> >>framebuffer tiling.
> >>
> >>The X special casing in create_bo_for_fb is for compatibility with old
> >>userspace, but going forward it was decided fb  modifiers should be used
> >>to tell the driver about tiling and get/set_tiling ioctl is about
> >>fencing and only that.
> >>
> >>Paths implemented in IGT back then were rendering to Y and Yf tiling fbs
> >>via a temporary linear surface which is then blitted (blit?) to the real
> >>fb obj. (With the blitter doing the appropriate transformation.)
> >>
> >>So in that respect adding Y tiling to create_bo_for_fb would be wrong
> >>because it is not aligned with the above, and also you cannot support Yf
> >>this way at all.
> >>
> >>But I do agree this creates a problem for some use cases within the IGT
> >>since the fb and backing obj are created atomically and once that is
> >>done you cannot fiddle with obj->tiling (aka fencing).
> >
> >I suppose we could either make it easier to create the obj and fb
> >separately, or we could add a parameter to the fb funcs to indicate
> >whether we want a fence or not.
> 
> Either way sounds good to me. Will depend on whatever fits better with what
> Paulo is working on at the moment.

I'm ok with not tiling by default for anything but X-tiled in the igt_fb
code. Users can just call set_tiling on the underlying bo if they want it,
but in the shiny new world of Yf/Ys we should by default not use gtt mmaps
really for anything. See also some of the changes planned and then
cancelled/delayed for future products, where the gtt might go poof
entirely.
-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

Re: [PATCH igt 2/3] lib/igt_draw: add support for Y tiling

[Daniel Vetter]

On Fri, Jan 29, 2016 at 04:46:31PM -0200, Paulo Zanoni wrote:
> Most of the patch is to change the tile/untile functions so they can
> work with Y-major tiling.
> 
> We're also skipping on BLT for Y-tiling. Some parts of the spec
> suggest this doesn't work and some parts of the spec suggest it will
> work. Since I couldn't make it work, SKIP for now.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

So if we go with not setting tiling by default for Y-tiled then we'd need
a call to get_tiling for the blt functions in igt_draw and an igt_assert
that it matches. Then callers know when exactly they need to set up tiling
themselves.

Of course we also need to document this in the gtkdoc.

Sounds reasonable as an overall plan, or totally nonsense?
-Daniel

> ---
>  lib/igt_draw.c | 173 ++++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 123 insertions(+), 50 deletions(-)
> 
> diff --git a/lib/igt_draw.c b/lib/igt_draw.c
> index 45fa10f..468a1eb 100644
> --- a/lib/igt_draw.c
> +++ b/lib/igt_draw.c
> @@ -134,71 +134,118 @@ static int swizzle_addr(int addr, int swizzle)
>  	return addr;
>  }
>  
> -/* 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 tile(int x, int y, uint32_t x_tile_size, uint32_t y_tile_size,
> +		uint32_t line_size, bool xmajor)
>  {
> -	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;
> -	int tile_n, tile_off;
> -	int tiled_pos, tiles_per_line;
> -	int pixel_size = bpp / 8;
> +	int tile_size, tiles_per_line, x_tile_n, y_tile_n, tile_off, pos;
> +	int tile_n, x_tile_off, y_tile_off;
>  
> -	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 = x_tile_size * y_tile_size;
>  
> +	x_tile_n = x / x_tile_size;
>  	y_tile_n = y / y_tile_size;
> +	tile_n = y_tile_n * tiles_per_line + x_tile_n;
> +
> +	x_tile_off = x % x_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;
> +	if (xmajor)
> +		tile_off = y_tile_off * x_tile_size + x_tile_off;
> +	else
> +		tile_off = x_tile_off * y_tile_size + y_tile_off;
>  
> -	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;
> +	pos = tile_n * tile_size + tile_off;
>  
> -	tiled_pos = swizzle_addr(tiled_pos, swizzle);
> -
> -	return tiled_pos / pixel_size;
> +	return pos;
>  }
>  
> -/* 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 untile(int tiled_pos, int x_tile_size, int y_tile_size,
> +		   uint32_t line_size, bool xmajor, int *x, int *y)
>  {
> -	int tile_n, tile_off, tiles_per_line, line_size;
> +	int tile_n, tile_off, tiles_per_line;
>  	int x_tile_off, y_tile_off;
>  	int x_tile_n, y_tile_n;
> -	int x_tile_size, y_tile_size, tile_size;
> -	int pixel_size = bpp / 8;
> -
> -	tiled_pos = swizzle_addr(tiled_pos, swizzle);
> +	int tile_size;
>  
> -	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_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;
> +	if (xmajor) {
> +		y_tile_off = tile_off / x_tile_size;
> +		x_tile_off = tile_off % x_tile_size;
> +	} else {
> +		y_tile_off = tile_off % y_tile_size;
> +		x_tile_off = tile_off / y_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;
> +	*x = (x_tile_n * x_tile_size + x_tile_off);
>  	*y = y_tile_n * y_tile_size + y_tile_off;
>  }
>  
> +static int linear_x_y_to_xtiled_pos(int x, int y, uint32_t stride, int swizzle,
> +				    int bpp)
> +{
> +	int pos;
> +	int pixel_size = bpp / 8;
> +
> +	x *= pixel_size;
> +	pos = tile(x, y, 512, 8, stride, true);
> +	pos = swizzle_addr(pos, swizzle);
> +	return pos / pixel_size;
> +}
> +
> +static int linear_x_y_to_ytiled_pos(int x, int y, uint32_t stride, int swizzle,
> +				    int bpp)
> +{
> +	int ow_tile_n, pos;
> +	int ow_size = 16;
> +	int pixel_size = bpp / 8;
> +
> +	/* We have an Y tiling of OWords, so use the tile() function to get the
> +	 * OW number, then adjust to the fact that the OW may have more than one
> +	 * pixel. */
> +	x *= pixel_size;
> +	ow_tile_n = tile(x / ow_size, y, 128 / ow_size, 32,
> +			 stride / ow_size, false);
> +	pos = ow_tile_n * ow_size + (x % ow_size);
> +	pos = swizzle_addr(pos, swizzle);
> +	return pos / pixel_size;
> +}
> +
> +static void xtiled_pos_to_x_y_linear(int tiled_pos, uint32_t stride,
> +				     int swizzle, int bpp, int *x, int *y)
> +{
> +	int pixel_size = bpp / 8;
> +
> +	tiled_pos = swizzle_addr(tiled_pos, swizzle);
> +
> +	untile(tiled_pos, 512, 8, stride, true, x, y);
> +	*x /= pixel_size;
> +}
> +
> +static void ytiled_pos_to_x_y_linear(int tiled_pos, uint32_t stride,
> +				     int swizzle, int bpp, int *x, int *y)
> +{
> +	int ow_tile_n;
> +	int ow_size = 16;
> +	int pixel_size = bpp / 8;
> +
> +	tiled_pos = swizzle_addr(tiled_pos, swizzle);
> +
> +	ow_tile_n = tiled_pos / ow_size;
> +	untile(ow_tile_n, 128 / ow_size, 32, stride / ow_size, false, x, y);
> +	*x *= ow_size;
> +	*x += tiled_pos % ow_size;
> +	*x /= pixel_size;
> +}
> +
>  static void set_pixel(void *_ptr, int index, uint32_t color, int bpp)
>  {
>  	if (bpp == 16) {
> @@ -224,15 +271,26 @@ 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(void *ptr, uint32_t stride, uint32_t tiling,
> +				int swizzle, struct rect *rect, uint32_t color,
> +				int bpp)
>  {
>  	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);
> +			switch (tiling) {
> +			case I915_TILING_X:
> +				pos = linear_x_y_to_xtiled_pos(x, y, stride,
> +							       swizzle, bpp);
> +				break;
> +			case I915_TILING_Y:
> +				pos = linear_x_y_to_ytiled_pos(x, y, stride,
> +							       swizzle, bpp);
> +				break;
> +			default:
> +				igt_assert(false);
> +			}
>  			set_pixel(ptr, pos, color, bpp);
>  		}
>  	}
> @@ -259,8 +317,9 @@ 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);
> +	case I915_TILING_Y:
> +		draw_rect_ptr_tiled(ptr, buf->stride, tiling, swizzle, rect,
> +				    color, buf->bpp);
>  		break;
>  	default:
>  		igt_assert(false);
> @@ -309,8 +368,9 @@ 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);
> +	case I915_TILING_Y:
> +		draw_rect_ptr_tiled(ptr, buf->stride, tiling, swizzle, rect,
> +				    color, buf->bpp);
>  		break;
>  	default:
>  		igt_assert(false);
> @@ -337,8 +397,8 @@ 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 tiling, struct rect *rect,
> +				   uint32_t color, uint32_t swizzle)
>  {
>  	int i;
>  	int tiled_pos, x, y, pixel_size;
> @@ -361,8 +421,18 @@ 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);
> +		switch (tiling) {
> +		case I915_TILING_X:
> +			xtiled_pos_to_x_y_linear(tiled_pos, buf->stride,
> +						 swizzle, buf->bpp, &x, &y);
> +			break;
> +		case I915_TILING_Y:
> +			ytiled_pos_to_x_y_linear(tiled_pos, buf->stride,
> +						 swizzle, buf->bpp, &x, &y);
> +			break;
> +		default:
> +			igt_assert(false);
> +		}
>  
>  		if (x >= rect->x && x < rect->x + rect->w &&
>  		    y >= rect->y && y < rect->y + rect->h) {
> @@ -399,7 +469,8 @@ 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);
> +	case I915_TILING_Y:
> +		draw_rect_pwrite_tiled(fd, buf, tiling, rect, color, swizzle);
>  		break;
>  	default:
>  		igt_assert(false);
> @@ -421,6 +492,8 @@ static void draw_rect_blt(int fd, struct cmd_data *cmd_data,
>  
>  	gem_get_tiling(fd, buf->handle, &tiling, &swizzle);
>  
> +	igt_require(tiling != I915_TILING_Y);
> +
>  	dst = gem_handle_to_libdrm_bo(cmd_data->bufmgr, fd, "", buf->handle);
>  	igt_assert(dst);
>  
> -- 
> 2.7.0.rc3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://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