[igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Extract calc_plane_stride()

[igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Extract calc_plane_stride()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Extract the stride calculation from calc_fb_size_packed() to its own
thing so that we can use it to calculate just the stride.

v2: Rebase due to overflow fixes and roundup_power_of_two()

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/igt_fb.c | 50 +++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 39 insertions(+), 11 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 486b5d3015ad..ed49a3ede874 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -226,6 +226,39 @@ static int fb_num_planes(struct format_desc_struct *format)
 	return format->num_planes;
 }
 
+static unsigned calc_plane_stride(int fd,
+				  struct format_desc_struct *format,
+				  int width, uint64_t tiling, int plane)
+{
+	uint32_t min_stride = fb_plane_min_stride(format, width, plane);
+
+	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
+	    intel_gen(intel_get_drm_devid(fd)) <= 3) {
+		uint32_t stride;
+
+		/* Round the tiling up to the next power-of-two and the region
+		 * up to the next pot fence size so that this works on all
+		 * generations.
+		 *
+		 * This can still fail if the framebuffer is too large to be
+		 * tiled. But then that failure is expected.
+		 */
+
+		stride = max(min_stride, 512);
+		stride = roundup_power_of_two(stride);
+
+		return stride;
+	} else {
+		unsigned int tile_width, tile_height;
+
+		igt_get_fb_tile_size(fd, tiling,
+				     fb_plane_bpp(format, plane),
+				     &tile_width, &tile_height);
+
+		return ALIGN(min_stride, tile_width);
+	}
+}
+
 static void calc_fb_size_planar(int fd, int width, int height,
 				struct format_desc_struct *format,
 				uint64_t tiling, unsigned stride,
@@ -272,12 +305,10 @@ static void calc_fb_size_packed(int fd, int width, int height,
 				struct format_desc_struct *format, uint64_t tiling,
 				unsigned stride, uint64_t *size_ret, unsigned *stride_ret)
 {
-	unsigned int tile_width, tile_height;
 	uint64_t size;
-	unsigned int byte_width = fb_plane_min_stride(format, width, 0);
 
-	igt_get_fb_tile_size(fd, tiling, fb_plane_bpp(format, 0),
-			     &tile_width, &tile_height);
+	if (!stride)
+		stride = calc_plane_stride(fd, format, width, tiling, 0);
 
 	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
 	    intel_gen(intel_get_drm_devid(fd)) <= 3) {
@@ -289,16 +320,13 @@ static void calc_fb_size_packed(int fd, int width, int height,
 		 * tiled. But then that failure is expected.
 		 */
 
-		if (!stride) {
-			stride = max(byte_width, 512);
-			stride = roundup_power_of_two(stride);
-		}
-
 		size = max((uint64_t) stride * height, 1024*1024);
 		size = roundup_power_of_two(size);
 	} else {
-		if (!stride)
-			stride = ALIGN(byte_width, tile_width);
+		unsigned int tile_width, tile_height;
+
+		igt_get_fb_tile_size(fd, tiling, fb_plane_bpp(format, 0),
+				     &tile_width, &tile_height);
 
 		size = (uint64_t) stride * ALIGN(height, tile_height);
 	}
-- 
2.16.4

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

[igt-dev] [PATCH i-g-t 2/5] lib/igt_fb: Consolidate fb size calculation to one function

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Eliminate the planar vs. packed size calculation differences and just
use one function for the entire thing.

v2: Rebase due to uint64_t size

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/igt_fb.c | 143 ++++++++++++++++++++++++++---------------------------------
 1 file changed, 64 insertions(+), 79 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index ed49a3ede874..ad728863758e 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -259,80 +259,66 @@ static unsigned calc_plane_stride(int fd,
 	}
 }
 
-static void calc_fb_size_planar(int fd, int width, int height,
+static uint64_t calc_plane_size(int fd, int width, int height,
 				struct format_desc_struct *format,
-				uint64_t tiling, unsigned stride,
-				uint64_t *size_ret, unsigned *stride_ret,
-				unsigned *offsets)
+				uint64_t tiling, int plane,
+				uint32_t stride)
 {
+	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
+	    intel_gen(intel_get_drm_devid(fd)) <= 3) {
+		uint64_t min_size = (uint64_t) stride * height;
+		uint64_t size;
+
+		/* Round the tiling up to the next power-of-two and the region
+		 * up to the next pot fence size so that this works on all
+		 * generations.
+		 *
+		 * This can still fail if the framebuffer is too large to be
+		 * tiled. But then that failure is expected.
+		 */
+
+		size = max(min_size, 1024*1024);
+		size = roundup_power_of_two(size);
+
+		return size;
+	} else {
+		unsigned int tile_width, tile_height;
+
+		igt_get_fb_tile_size(fd, tiling, fb_plane_bpp(format, plane),
+				     &tile_width, &tile_height);
+
+		return (uint64_t) stride * ALIGN(height, tile_height);
+	}
+}
+
+static uint64_t calc_fb_size(int fd, int width, int height,
+			     struct format_desc_struct *format,
+			     uint64_t tiling,
+			     uint32_t strides[4], uint32_t offsets[4])
+{
+	uint64_t size = 0;
 	int plane;
-	unsigned max_stride = 0, tile_width, tile_height;
-
-	*size_ret = 0;
 
 	for (plane = 0; plane < fb_num_planes(format); plane++) {
-		unsigned plane_stride;
-
-		igt_get_fb_tile_size(fd, tiling, fb_plane_bpp(format, plane),
-				     &tile_width, &tile_height);
-
-		plane_stride = ALIGN(fb_plane_min_stride(format, width, plane), tile_width);
-		if (max_stride < plane_stride)
-			max_stride = plane_stride;
-	}
+		if (!strides[plane])
+			strides[plane] = calc_plane_stride(fd, format,
+							   width, tiling, plane);
 
-	if (!stride)
-		stride = max_stride;
-
-	for (plane = 0; plane < fb_num_planes(format); plane++) {
 		if (offsets)
-			offsets[plane] = *size_ret;
+			offsets[plane] = size;
 
-		igt_get_fb_tile_size(fd, tiling, fb_plane_bpp(format, plane),
-				     &tile_width, &tile_height);
-
-		*size_ret += stride * ALIGN(fb_plane_height(format, height, plane), tile_height);
+		size += calc_plane_size(fd, width, height,
+					format, tiling, plane,
+					strides[plane]);
 	}
 
-	if (offsets)
-		for (; plane < ARRAY_SIZE(format->plane_bpp); plane++)
+	for (; plane < ARRAY_SIZE(format->plane_bpp); plane++) {
+		strides[plane] = 0;
+		if (offsets)
 			offsets[plane] = 0;
-
-	*stride_ret = stride;
-}
-
-static void calc_fb_size_packed(int fd, int width, int height,
-				struct format_desc_struct *format, uint64_t tiling,
-				unsigned stride, uint64_t *size_ret, unsigned *stride_ret)
-{
-	uint64_t size;
-
-	if (!stride)
-		stride = calc_plane_stride(fd, format, width, tiling, 0);
-
-	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
-	    intel_gen(intel_get_drm_devid(fd)) <= 3) {
-		/* Round the tiling up to the next power-of-two and the region
-		 * up to the next pot fence size so that this works on all
-		 * generations.
-		 *
-		 * This can still fail if the framebuffer is too large to be
-		 * tiled. But then that failure is expected.
-		 */
-
-		size = max((uint64_t) stride * height, 1024*1024);
-		size = roundup_power_of_two(size);
-	} else {
-		unsigned int tile_width, tile_height;
-
-		igt_get_fb_tile_size(fd, tiling, fb_plane_bpp(format, 0),
-				     &tile_width, &tile_height);
-
-		size = (uint64_t) stride * ALIGN(height, tile_height);
 	}
 
-	*stride_ret = stride;
-	*size_ret = size;
+	return size;
 }
 
 /**
@@ -352,12 +338,12 @@ void igt_calc_fb_size(int fd, int width, int height, uint32_t drm_format, uint64
 		      uint64_t *size_ret, unsigned *stride_ret)
 {
 	struct format_desc_struct *format = lookup_drm_format(drm_format);
+	uint32_t strides[4] = {};
+
 	igt_assert(format);
 
-	if (fb_num_planes(format) > 1)
-		calc_fb_size_planar(fd, width, height, format, tiling, 0, size_ret, stride_ret, NULL);
-	else
-		calc_fb_size_packed(fd, width, height, format, tiling, 0, size_ret, stride_ret);
+	*size_ret = calc_fb_size(fd, width, height, format, tiling, strides, NULL);
+	*stride_ret = strides[0];
 }
 
 /**
@@ -419,7 +405,7 @@ static int create_bo_for_fb(int fd, int width, int height,
 			    struct format_desc_struct *format,
 			    uint64_t tiling, uint64_t size, unsigned stride,
 			    uint64_t *size_ret, unsigned *stride_ret,
-			    uint32_t *offsets, bool *is_dumb)
+			    uint32_t offsets[4], bool *is_dumb)
 {
 	int bo;
 
@@ -430,17 +416,16 @@ static int create_bo_for_fb(int fd, int width, int height,
 
 	if (tiling || size || stride || igt_format_is_yuv(format->drm_id)) {
 		uint64_t calculated_size;
-		unsigned int calculated_stride;
+		uint32_t strides[4] = {
+			stride,
+		};
 
-		if (fb_num_planes(format) > 1)
-			calc_fb_size_planar(fd, width, height, format, tiling, stride,
-					    &calculated_size, &calculated_stride, offsets);
-		else
-			calc_fb_size_packed(fd, width, height, format, tiling, stride,
-					    &calculated_size, &calculated_stride);
+		calculated_size = calc_fb_size(fd, width, height,
+					       format, tiling,
+					       strides, offsets);
 
 		if (stride == 0)
-			stride = calculated_stride;
+			stride = strides[0];
 		if (size == 0)
 			size = calculated_size;
 
@@ -464,19 +449,19 @@ static int create_bo_for_fb(int fd, int width, int height,
 			switch (format->drm_id) {
 			case DRM_FORMAT_NV12:
 				memset(ptr + offsets[0], full_range ? 0x00 : 0x10,
-				       calculated_stride * height);
+				       strides[0] * height);
 				memset(ptr + offsets[1], 0x80,
-				       calculated_stride * height/2);
+				       strides[1] * height/2);
 				break;
 			case DRM_FORMAT_YUYV:
 			case DRM_FORMAT_YVYU:
 				wmemset(ptr, full_range ? 0x80008000 : 0x80108010,
-					calculated_stride * height / sizeof(wchar_t));
+					strides[0] * height / sizeof(wchar_t));
 				break;
 			case DRM_FORMAT_UYVY:
 			case DRM_FORMAT_VYUY:
 				wmemset(ptr, full_range ? 0x00800080 : 0x10801080,
-					calculated_stride * height / sizeof(wchar_t));
+					strides[0] * height / sizeof(wchar_t));
 				break;
 			}
 			gem_munmap(ptr, size);
@@ -485,7 +470,7 @@ static int create_bo_for_fb(int fd, int width, int height,
 				*size_ret = size;
 
 			if (stride_ret)
-				*stride_ret = stride;
+				*stride_ret = strides[0];
 
 			return bo;
 		} else {
-- 
2.16.4

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

Re: [igt-dev] [PATCH i-g-t 2/5] lib/igt_fb: Consolidate fb size calculation to one function

[Paulo Zanoni]

Em Ter, 2018-09-25 às 16:47 +0300, Ville Syrjala escreveu:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Eliminate the planar vs. packed size calculation differences and just
> use one function for the entire thing.

Much better!

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

> 
> v2: Rebase due to uint64_t size
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  lib/igt_fb.c | 143 ++++++++++++++++++++++++++-----------------------
> ----------
>  1 file changed, 64 insertions(+), 79 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index ed49a3ede874..ad728863758e 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -259,80 +259,66 @@ static unsigned calc_plane_stride(int fd,
>  	}
>  }
>  
> -static void calc_fb_size_planar(int fd, int width, int height,
> +static uint64_t calc_plane_size(int fd, int width, int height,
>  				struct format_desc_struct *format,
> -				uint64_t tiling, unsigned stride,
> -				uint64_t *size_ret, unsigned
> *stride_ret,
> -				unsigned *offsets)
> +				uint64_t tiling, int plane,
> +				uint32_t stride)
>  {
> +	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> +	    intel_gen(intel_get_drm_devid(fd)) <= 3) {
> +		uint64_t min_size = (uint64_t) stride * height;
> +		uint64_t size;
> +
> +		/* Round the tiling up to the next power-of-two and
> the region
> +		 * up to the next pot fence size so that this works
> on all
> +		 * generations.
> +		 *
> +		 * This can still fail if the framebuffer is too
> large to be
> +		 * tiled. But then that failure is expected.
> +		 */
> +
> +		size = max(min_size, 1024*1024);
> +		size = roundup_power_of_two(size);
> +
> +		return size;
> +	} else {
> +		unsigned int tile_width, tile_height;
> +
> +		igt_get_fb_tile_size(fd, tiling,
> fb_plane_bpp(format, plane),
> +				     &tile_width, &tile_height);
> +
> +		return (uint64_t) stride * ALIGN(height,
> tile_height);
> +	}
> +}
> +
> +static uint64_t calc_fb_size(int fd, int width, int height,
> +			     struct format_desc_struct *format,
> +			     uint64_t tiling,
> +			     uint32_t strides[4], uint32_t
> offsets[4])
> +{
> +	uint64_t size = 0;
>  	int plane;
> -	unsigned max_stride = 0, tile_width, tile_height;
> -
> -	*size_ret = 0;
>  
>  	for (plane = 0; plane < fb_num_planes(format); plane++) {
> -		unsigned plane_stride;
> -
> -		igt_get_fb_tile_size(fd, tiling,
> fb_plane_bpp(format, plane),
> -				     &tile_width, &tile_height);
> -
> -		plane_stride = ALIGN(fb_plane_min_stride(format,
> width, plane), tile_width);
> -		if (max_stride < plane_stride)
> -			max_stride = plane_stride;
> -	}
> +		if (!strides[plane])
> +			strides[plane] = calc_plane_stride(fd,
> format,
> +							   width,
> tiling, plane);
>  
> -	if (!stride)
> -		stride = max_stride;
> -
> -	for (plane = 0; plane < fb_num_planes(format); plane++) {
>  		if (offsets)
> -			offsets[plane] = *size_ret;
> +			offsets[plane] = size;
>  
> -		igt_get_fb_tile_size(fd, tiling,
> fb_plane_bpp(format, plane),
> -				     &tile_width, &tile_height);
> -
> -		*size_ret += stride * ALIGN(fb_plane_height(format,
> height, plane), tile_height);
> +		size += calc_plane_size(fd, width, height,
> +					format, tiling, plane,
> +					strides[plane]);
>  	}
>  
> -	if (offsets)
> -		for (; plane < ARRAY_SIZE(format->plane_bpp);
> plane++)
> +	for (; plane < ARRAY_SIZE(format->plane_bpp); plane++) {
> +		strides[plane] = 0;
> +		if (offsets)
>  			offsets[plane] = 0;
> -
> -	*stride_ret = stride;
> -}
> -
> -static void calc_fb_size_packed(int fd, int width, int height,
> -				struct format_desc_struct *format,
> uint64_t tiling,
> -				unsigned stride, uint64_t *size_ret,
> unsigned *stride_ret)
> -{
> -	uint64_t size;
> -
> -	if (!stride)
> -		stride = calc_plane_stride(fd, format, width,
> tiling, 0);
> -
> -	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> -	    intel_gen(intel_get_drm_devid(fd)) <= 3) {
> -		/* Round the tiling up to the next power-of-two and
> the region
> -		 * up to the next pot fence size so that this works
> on all
> -		 * generations.
> -		 *
> -		 * This can still fail if the framebuffer is too
> large to be
> -		 * tiled. But then that failure is expected.
> -		 */
> -
> -		size = max((uint64_t) stride * height, 1024*1024);
> -		size = roundup_power_of_two(size);
> -	} else {
> -		unsigned int tile_width, tile_height;
> -
> -		igt_get_fb_tile_size(fd, tiling,
> fb_plane_bpp(format, 0),
> -				     &tile_width, &tile_height);
> -
> -		size = (uint64_t) stride * ALIGN(height,
> tile_height);
>  	}
>  
> -	*stride_ret = stride;
> -	*size_ret = size;
> +	return size;
>  }
>  
>  /**
> @@ -352,12 +338,12 @@ void igt_calc_fb_size(int fd, int width, int
> height, uint32_t drm_format, uint64
>  		      uint64_t *size_ret, unsigned *stride_ret)
>  {
>  	struct format_desc_struct *format =
> lookup_drm_format(drm_format);
> +	uint32_t strides[4] = {};
> +
>  	igt_assert(format);
>  
> -	if (fb_num_planes(format) > 1)
> -		calc_fb_size_planar(fd, width, height, format,
> tiling, 0, size_ret, stride_ret, NULL);
> -	else
> -		calc_fb_size_packed(fd, width, height, format,
> tiling, 0, size_ret, stride_ret);
> +	*size_ret = calc_fb_size(fd, width, height, format, tiling,
> strides, NULL);
> +	*stride_ret = strides[0];
>  }
>  
>  /**
> @@ -419,7 +405,7 @@ static int create_bo_for_fb(int fd, int width,
> int height,
>  			    struct format_desc_struct *format,
>  			    uint64_t tiling, uint64_t size, unsigned
> stride,
>  			    uint64_t *size_ret, unsigned
> *stride_ret,
> -			    uint32_t *offsets, bool *is_dumb)
> +			    uint32_t offsets[4], bool *is_dumb)
>  {
>  	int bo;
>  
> @@ -430,17 +416,16 @@ static int create_bo_for_fb(int fd, int width,
> int height,
>  
>  	if (tiling || size || stride || igt_format_is_yuv(format-
> >drm_id)) {
>  		uint64_t calculated_size;
> -		unsigned int calculated_stride;
> +		uint32_t strides[4] = {
> +			stride,
> +		};
>  
> -		if (fb_num_planes(format) > 1)
> -			calc_fb_size_planar(fd, width, height,
> format, tiling, stride,
> -					    &calculated_size,
> &calculated_stride, offsets);
> -		else
> -			calc_fb_size_packed(fd, width, height,
> format, tiling, stride,
> -					    &calculated_size,
> &calculated_stride);
> +		calculated_size = calc_fb_size(fd, width, height,
> +					       format, tiling,
> +					       strides, offsets);
>  
>  		if (stride == 0)
> -			stride = calculated_stride;
> +			stride = strides[0];
>  		if (size == 0)
>  			size = calculated_size;
>  
> @@ -464,19 +449,19 @@ static int create_bo_for_fb(int fd, int width,
> int height,
>  			switch (format->drm_id) {
>  			case DRM_FORMAT_NV12:
>  				memset(ptr + offsets[0], full_range
> ? 0x00 : 0x10,
> -				       calculated_stride * height);
> +				       strides[0] * height);
>  				memset(ptr + offsets[1], 0x80,
> -				       calculated_stride *
> height/2);
> +				       strides[1] * height/2);
>  				break;
>  			case DRM_FORMAT_YUYV:
>  			case DRM_FORMAT_YVYU:
>  				wmemset(ptr, full_range ? 0x80008000
> : 0x80108010,
> -					calculated_stride * height /
> sizeof(wchar_t));
> +					strides[0] * height /
> sizeof(wchar_t));
>  				break;
>  			case DRM_FORMAT_UYVY:
>  			case DRM_FORMAT_VYUY:
>  				wmemset(ptr, full_range ? 0x00800080
> : 0x10801080,
> -					calculated_stride * height /
> sizeof(wchar_t));
> +					strides[0] * height /
> sizeof(wchar_t));
>  				break;
>  			}
>  			gem_munmap(ptr, size);
> @@ -485,7 +470,7 @@ static int create_bo_for_fb(int fd, int width,
> int height,
>  				*size_ret = size;
>  
>  			if (stride_ret)
> -				*stride_ret = stride;
> +				*stride_ret = strides[0];
>  
>  			return bo;
>  		} else {
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 3/5] lib/igt_fb: Constify format_desc_struct

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We don't want anyone to modify the format description structures. Make
them const.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 lib/igt_fb.c | 44 ++++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index ad728863758e..3c6974503313 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -60,7 +60,7 @@
  */
 
 /* drm fourcc/cairo format maps */
-static struct format_desc_struct {
+static const struct format_desc_struct {
 	const char *name;
 	uint32_t drm_id;
 	cairo_format_t cairo_id;
@@ -108,9 +108,9 @@ static struct format_desc_struct {
 #define for_each_format(f)	\
 	for (f = format_desc; f - format_desc < ARRAY_SIZE(format_desc); f++)
 
-static struct format_desc_struct *lookup_drm_format(uint32_t drm_format)
+static const struct format_desc_struct *lookup_drm_format(uint32_t drm_format)
 {
-	struct format_desc_struct *format;
+	const struct format_desc_struct *format;
 
 	for_each_format(format) {
 		if (format->drm_id != drm_format)
@@ -190,8 +190,8 @@ void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
 	}
 }
 
-static unsigned fb_plane_width(struct format_desc_struct *format,
-			       unsigned width, int plane)
+static unsigned fb_plane_width(const struct format_desc_struct *format,
+			       int plane, unsigned width)
 {
 	if (format->drm_id == DRM_FORMAT_NV12 && plane == 1)
 		return DIV_ROUND_UP(width, 2);
@@ -199,21 +199,21 @@ static unsigned fb_plane_width(struct format_desc_struct *format,
 	return width;
 }
 
-static unsigned fb_plane_bpp(struct format_desc_struct *format, int plane)
+static unsigned fb_plane_bpp(const struct format_desc_struct *format, int plane)
 {
 	return format->plane_bpp[plane];
 }
 
-static unsigned fb_plane_min_stride(struct format_desc_struct *format,
-				    unsigned width, int plane)
+static unsigned fb_plane_min_stride(const struct format_desc_struct *format,
+				    int plane, unsigned width)
 {
 	unsigned cpp = fb_plane_bpp(format, plane) / 8;
 
 	return fb_plane_width(format, width, plane) * cpp;
 }
 
-static unsigned fb_plane_height(struct format_desc_struct *format,
-				unsigned height, int plane)
+static unsigned fb_plane_height(const struct format_desc_struct *format,
+				int plane, unsigned height)
 {
 	if (format->drm_id == DRM_FORMAT_NV12 && plane == 1)
 		return DIV_ROUND_UP(height, 2);
@@ -221,13 +221,13 @@ static unsigned fb_plane_height(struct format_desc_struct *format,
 	return height;
 }
 
-static int fb_num_planes(struct format_desc_struct *format)
+static int fb_num_planes(const struct format_desc_struct *format)
 {
 	return format->num_planes;
 }
 
 static unsigned calc_plane_stride(int fd,
-				  struct format_desc_struct *format,
+				  const struct format_desc_struct *format,
 				  int width, uint64_t tiling, int plane)
 {
 	uint32_t min_stride = fb_plane_min_stride(format, width, plane);
@@ -260,7 +260,7 @@ static unsigned calc_plane_stride(int fd,
 }
 
 static uint64_t calc_plane_size(int fd, int width, int height,
-				struct format_desc_struct *format,
+				const struct format_desc_struct *format,
 				uint64_t tiling, int plane,
 				uint32_t stride)
 {
@@ -292,7 +292,7 @@ static uint64_t calc_plane_size(int fd, int width, int height,
 }
 
 static uint64_t calc_fb_size(int fd, int width, int height,
-			     struct format_desc_struct *format,
+			     const struct format_desc_struct *format,
 			     uint64_t tiling,
 			     uint32_t strides[4], uint32_t offsets[4])
 {
@@ -337,7 +337,7 @@ static uint64_t calc_fb_size(int fd, int width, int height,
 void igt_calc_fb_size(int fd, int width, int height, uint32_t drm_format, uint64_t tiling,
 		      uint64_t *size_ret, unsigned *stride_ret)
 {
-	struct format_desc_struct *format = lookup_drm_format(drm_format);
+	const struct format_desc_struct *format = lookup_drm_format(drm_format);
 	uint32_t strides[4] = {};
 
 	igt_assert(format);
@@ -402,7 +402,7 @@ uint64_t igt_fb_tiling_to_mod(uint64_t tiling)
 static int create_bo_for_fb(int fd, int width, int height,
 			    enum igt_color_encoding color_encoding,
 			    enum igt_color_range color_range,
-			    struct format_desc_struct *format,
+			    const struct format_desc_struct *format,
 			    uint64_t tiling, uint64_t size, unsigned stride,
 			    uint64_t *size_ret, unsigned *stride_ret,
 			    uint32_t offsets[4], bool *is_dumb)
@@ -860,7 +860,7 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
 	/* FIXME allow the caller to pass these in */
 	enum igt_color_encoding color_encoding = IGT_COLOR_YCBCR_BT709;
 	enum igt_color_range color_range = IGT_COLOR_YCBCR_LIMITED_RANGE;
-	struct format_desc_struct *f = lookup_drm_format(format);
+	const struct format_desc_struct *f = lookup_drm_format(format);
 	uint32_t pitches[4];
 	uint32_t fb_id;
 	int i;
@@ -1200,7 +1200,7 @@ unsigned int igt_create_stereo_fb(int drm_fd, drmModeModeInfo *mode,
 
 static cairo_format_t drm_format_to_cairo(uint32_t drm_format)
 {
-	struct format_desc_struct *f;
+	const struct format_desc_struct *f;
 
 	for_each_format(f)
 		if (f->drm_id == drm_format)
@@ -1961,7 +1961,7 @@ void igt_remove_fb(int fd, struct igt_fb *fb)
  */
 uint32_t igt_bpp_depth_to_drm_format(int bpp, int depth)
 {
-	struct format_desc_struct *f;
+	const struct format_desc_struct *f;
 
 	for_each_format(f)
 		if (f->plane_bpp[0] == bpp && f->depth == depth)
@@ -1982,7 +1982,7 @@ uint32_t igt_bpp_depth_to_drm_format(int bpp, int depth)
  */
 uint32_t igt_drm_format_to_bpp(uint32_t drm_format)
 {
-	struct format_desc_struct *f = lookup_drm_format(drm_format);
+	const struct format_desc_struct *f = lookup_drm_format(drm_format);
 
 	igt_assert_f(f, "can't find a bpp format for %08x (%s)\n",
 		     drm_format, igt_format_str(drm_format));
@@ -2000,7 +2000,7 @@ uint32_t igt_drm_format_to_bpp(uint32_t drm_format)
  */
 const char *igt_format_str(uint32_t drm_format)
 {
-	struct format_desc_struct *f = lookup_drm_format(drm_format);
+	const struct format_desc_struct *f = lookup_drm_format(drm_format);
 
 	return f ? f->name : "invalid";
 }
@@ -2014,7 +2014,7 @@ const char *igt_format_str(uint32_t drm_format)
  */
 bool igt_fb_supported_format(uint32_t drm_format)
 {
-	struct format_desc_struct *f;
+	const struct format_desc_struct *f;
 
 	for_each_format(f)
 		if (f->drm_id == drm_format)
-- 
2.16.4

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

[igt-dev] [PATCH i-g-t 4/5] lib/igt_fb: Pass around igt_fb internally

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Instead of passing around a boatload of integers everywhere let's
just pass around the igt_fb struct. That obviously means we have to
populate it first sufficiently, to which end we'll add a small helper.
Later on the stride/size calculations will consult the already
pre-populated igt_fb and fill in the rest as needed.

This makes the whole thing a lot less error prone as it's impossible
to accidentally pass the arguments in the wrong order when there's
just the one of them, and it's a pointer.

v2: Rebase due to uint64_t size

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/igt_draw.c                   |   2 +-
 lib/igt_fb.c                     | 408 +++++++++++++++++++--------------------
 lib/igt_fb.h                     |   4 +-
 tests/kms_ccs.c                  |   2 +-
 tests/kms_flip.c                 |   4 +-
 tests/kms_frontbuffer_tracking.c |   6 +-
 tests/pm_rpm.c                   |   2 +-
 7 files changed, 207 insertions(+), 221 deletions(-)

diff --git a/lib/igt_draw.c b/lib/igt_draw.c
index c7d5770dca28..05821480bc80 100644
--- a/lib/igt_draw.c
+++ b/lib/igt_draw.c
@@ -720,7 +720,7 @@ void igt_draw_rect_fb(int fd, drm_intel_bufmgr *bufmgr,
 		      enum igt_draw_method method, int rect_x, int rect_y,
 		      int rect_w, int rect_h, uint32_t color)
 {
-	igt_draw_rect(fd, bufmgr, context, fb->gem_handle, fb->size, fb->stride,
+	igt_draw_rect(fd, bufmgr, context, fb->gem_handle, fb->size, fb->strides[0],
 		      method, rect_x, rect_y, rect_w, rect_h, color,
 		      igt_drm_format_to_bpp(fb->drm_format));
 }
diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 3c6974503313..26019f0420dc 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -190,50 +190,72 @@ void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
 	}
 }
 
-static unsigned fb_plane_width(const struct format_desc_struct *format,
-			       int plane, unsigned width)
+static unsigned fb_plane_width(const struct igt_fb *fb, int plane)
 {
-	if (format->drm_id == DRM_FORMAT_NV12 && plane == 1)
-		return DIV_ROUND_UP(width, 2);
+	if (fb->drm_format == DRM_FORMAT_NV12 && plane == 1)
+		return DIV_ROUND_UP(fb->width, 2);
 
-	return width;
+	return fb->width;
 }
 
-static unsigned fb_plane_bpp(const struct format_desc_struct *format, int plane)
+static unsigned fb_plane_bpp(const struct igt_fb *fb, int plane)
 {
+	const struct format_desc_struct *format = lookup_drm_format(fb->drm_format);
+
 	return format->plane_bpp[plane];
 }
 
-static unsigned fb_plane_min_stride(const struct format_desc_struct *format,
-				    int plane, unsigned width)
+static unsigned fb_plane_height(const struct igt_fb *fb, int plane)
 {
-	unsigned cpp = fb_plane_bpp(format, plane) / 8;
+	if (fb->drm_format == DRM_FORMAT_NV12 && plane == 1)
+		return DIV_ROUND_UP(fb->height, 2);
 
-	return fb_plane_width(format, width, plane) * cpp;
+	return fb->height;
 }
 
-static unsigned fb_plane_height(const struct format_desc_struct *format,
-				int plane, unsigned height)
+static int fb_num_planes(const struct igt_fb *fb)
 {
-	if (format->drm_id == DRM_FORMAT_NV12 && plane == 1)
-		return DIV_ROUND_UP(height, 2);
+	const struct format_desc_struct *format = lookup_drm_format(fb->drm_format);
 
-	return height;
-}
-
-static int fb_num_planes(const struct format_desc_struct *format)
-{
 	return format->num_planes;
 }
 
-static unsigned calc_plane_stride(int fd,
-				  const struct format_desc_struct *format,
-				  int width, uint64_t tiling, int plane)
+static void fb_init(struct igt_fb *fb,
+		    int fd, int width, int height,
+		    uint32_t drm_format,
+		    uint64_t modifier,
+		    enum igt_color_encoding color_encoding,
+		    enum igt_color_range color_range)
 {
-	uint32_t min_stride = fb_plane_min_stride(format, width, plane);
+	const struct format_desc_struct *f = lookup_drm_format(drm_format);
 
-	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
-	    intel_gen(intel_get_drm_devid(fd)) <= 3) {
+	igt_assert_f(f, "DRM format %08x not found\n", drm_format);
+
+	memset(fb, 0, sizeof(*fb));
+
+	fb->width = width;
+	fb->height = height;
+	fb->tiling = modifier;
+	fb->drm_format = drm_format;
+	fb->fd = fd;
+	fb->num_planes = fb_num_planes(fb);
+	fb->color_encoding = color_encoding;
+	fb->color_range = color_range;
+
+	for (int i = 0; i < fb->num_planes; i++) {
+		fb->plane_bpp[i] = fb_plane_bpp(fb, i);
+		fb->plane_height[i] = fb_plane_height(fb, i);
+		fb->plane_width[i] = fb_plane_width(fb, i);
+	}
+}
+
+static uint32_t calc_plane_stride(struct igt_fb *fb, int plane)
+{
+	uint32_t min_stride = fb->plane_width[plane] *
+		(fb->plane_bpp[plane] / 8);
+
+	if (fb->tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
+	    intel_gen(intel_get_drm_devid(fb->fd)) <= 3) {
 		uint32_t stride;
 
 		/* Round the tiling up to the next power-of-two and the region
@@ -251,22 +273,19 @@ static unsigned calc_plane_stride(int fd,
 	} else {
 		unsigned int tile_width, tile_height;
 
-		igt_get_fb_tile_size(fd, tiling,
-				     fb_plane_bpp(format, plane),
+		igt_get_fb_tile_size(fb->fd, fb->tiling, fb->plane_bpp[plane],
 				     &tile_width, &tile_height);
 
 		return ALIGN(min_stride, tile_width);
 	}
 }
 
-static uint64_t calc_plane_size(int fd, int width, int height,
-				const struct format_desc_struct *format,
-				uint64_t tiling, int plane,
-				uint32_t stride)
+static uint64_t calc_plane_size(struct igt_fb *fb, int plane)
 {
-	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
-	    intel_gen(intel_get_drm_devid(fd)) <= 3) {
-		uint64_t min_size = (uint64_t) stride * height;
+	if (fb->tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
+	    intel_gen(intel_get_drm_devid(fb->fd)) <= 3) {
+		uint64_t min_size = (uint64_t) fb->strides[plane] *
+			fb->plane_height[plane];
 		uint64_t size;
 
 		/* Round the tiling up to the next power-of-two and the region
@@ -284,38 +303,27 @@ static uint64_t calc_plane_size(int fd, int width, int height,
 	} else {
 		unsigned int tile_width, tile_height;
 
-		igt_get_fb_tile_size(fd, tiling, fb_plane_bpp(format, plane),
+		igt_get_fb_tile_size(fb->fd, fb->tiling, fb->plane_bpp[plane],
 				     &tile_width, &tile_height);
 
-		return (uint64_t) stride * ALIGN(height, tile_height);
+		return (uint64_t) fb->strides[plane] *
+			ALIGN(fb->plane_height[plane], tile_height);
 	}
 }
 
-static uint64_t calc_fb_size(int fd, int width, int height,
-			     const struct format_desc_struct *format,
-			     uint64_t tiling,
-			     uint32_t strides[4], uint32_t offsets[4])
+static uint64_t calc_fb_size(struct igt_fb *fb)
 {
 	uint64_t size = 0;
 	int plane;
 
-	for (plane = 0; plane < fb_num_planes(format); plane++) {
-		if (!strides[plane])
-			strides[plane] = calc_plane_stride(fd, format,
-							   width, tiling, plane);
+	for (plane = 0; plane < fb->num_planes; plane++) {
+		/* respect the stride requested by the caller */
+		if (!fb->strides[plane])
+			fb->strides[plane] = calc_plane_stride(fb, plane);
 
-		if (offsets)
-			offsets[plane] = size;
+		fb->offsets[plane] = size;
 
-		size += calc_plane_size(fd, width, height,
-					format, tiling, plane,
-					strides[plane]);
-	}
-
-	for (; plane < ARRAY_SIZE(format->plane_bpp); plane++) {
-		strides[plane] = 0;
-		if (offsets)
-			offsets[plane] = 0;
+		size += calc_plane_size(fb, plane);
 	}
 
 	return size;
@@ -337,13 +345,17 @@ static uint64_t calc_fb_size(int fd, int width, int height,
 void igt_calc_fb_size(int fd, int width, int height, uint32_t drm_format, uint64_t tiling,
 		      uint64_t *size_ret, unsigned *stride_ret)
 {
-	const struct format_desc_struct *format = lookup_drm_format(drm_format);
-	uint32_t strides[4] = {};
+	struct igt_fb fb;
 
-	igt_assert(format);
+	fb_init(&fb, fd, width, height, drm_format, tiling,
+		IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
 
-	*size_ret = calc_fb_size(fd, width, height, format, tiling, strides, NULL);
-	*stride_ret = strides[0];
+	fb.size = calc_fb_size(&fb);
+
+	if (size_ret)
+		*size_ret = fb.size;
+	if (stride_ret)
+		*stride_ret = fb.strides[0];
 }
 
 /**
@@ -399,80 +411,64 @@ uint64_t igt_fb_tiling_to_mod(uint64_t tiling)
 }
 
 /* helpers to create nice-looking framebuffers */
-static int create_bo_for_fb(int fd, int width, int height,
-			    enum igt_color_encoding color_encoding,
-			    enum igt_color_range color_range,
-			    const struct format_desc_struct *format,
-			    uint64_t tiling, uint64_t size, unsigned stride,
-			    uint64_t *size_ret, unsigned *stride_ret,
-			    uint32_t offsets[4], bool *is_dumb)
+static int create_bo_for_fb(struct igt_fb *fb)
 {
-	int bo;
+	int fd = fb->fd;
 
-	igt_assert(format);
+	if (fb->tiling || fb->size || fb->strides[0] || igt_format_is_yuv(fb->drm_format)) {
+		uint64_t size;
 
-	if (offsets)
-		memset(offsets, 0, ARRAY_SIZE(format->plane_bpp) * sizeof(*offsets));
+		size = calc_fb_size(fb);
 
-	if (tiling || size || stride || igt_format_is_yuv(format->drm_id)) {
-		uint64_t calculated_size;
-		uint32_t strides[4] = {
-			stride,
-		};
+		/* respect the size requested by the caller */
+		if (fb->size == 0)
+			fb->size = size;
 
-		calculated_size = calc_fb_size(fd, width, height,
-					       format, tiling,
-					       strides, offsets);
-
-		if (stride == 0)
-			stride = strides[0];
-		if (size == 0)
-			size = calculated_size;
-
-		if (is_dumb)
-			*is_dumb = false;
+		fb->is_dumb = false;
 
 		if (is_i915_device(fd)) {
 			void *ptr;
-			bool full_range = color_range == IGT_COLOR_YCBCR_FULL_RANGE;
+			bool full_range = fb->color_range == IGT_COLOR_YCBCR_FULL_RANGE;
 
-			bo = gem_create(fd, size);
-			gem_set_tiling(fd, bo, igt_fb_mod_to_tiling(tiling), stride);
+			fb->gem_handle = gem_create(fd, fb->size);
 
-			gem_set_domain(fd, bo,
+			gem_set_tiling(fd, fb->gem_handle,
+				       igt_fb_mod_to_tiling(fb->tiling),
+				       fb->strides[0]);
+
+			gem_set_domain(fd, fb->gem_handle,
 				       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
 
 			/* Ensure the framebuffer is preallocated */
-			ptr = gem_mmap__gtt(fd, bo, size, PROT_READ | PROT_WRITE);
+			ptr = gem_mmap__gtt(fd, fb->gem_handle,
+					    fb->size, PROT_READ | PROT_WRITE);
 			igt_assert(*(uint32_t *)ptr == 0);
 
-			switch (format->drm_id) {
+			switch (fb->drm_format) {
 			case DRM_FORMAT_NV12:
-				memset(ptr + offsets[0], full_range ? 0x00 : 0x10,
-				       strides[0] * height);
-				memset(ptr + offsets[1], 0x80,
-				       strides[1] * height/2);
+				memset(ptr + fb->offsets[0],
+				       full_range ? 0x00 : 0x10,
+				       fb->strides[0] * fb->plane_height[0]);
+				memset(ptr + fb->offsets[1],
+				       0x80,
+				       fb->strides[1] * fb->plane_height[1]);
 				break;
 			case DRM_FORMAT_YUYV:
 			case DRM_FORMAT_YVYU:
-				wmemset(ptr, full_range ? 0x80008000 : 0x80108010,
-					strides[0] * height / sizeof(wchar_t));
+				wmemset(ptr + fb->offsets[0],
+					full_range ? 0x80008000 : 0x80108010,
+					fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
 				break;
 			case DRM_FORMAT_UYVY:
 			case DRM_FORMAT_VYUY:
-				wmemset(ptr, full_range ? 0x00800080 : 0x10801080,
-					strides[0] * height / sizeof(wchar_t));
+				wmemset(ptr + fb->offsets[0],
+					full_range ? 0x00800080 : 0x10801080,
+					fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
 				break;
 			}
-			gem_munmap(ptr, size);
+			gem_munmap(ptr, fb->size);
 
-			if (size_ret)
-				*size_ret = size;
-
-			if (stride_ret)
-				*stride_ret = strides[0];
-
-			return bo;
+			return fb->gem_handle;
 		} else {
 			bool driver_has_gem_api = false;
 
@@ -480,12 +476,13 @@ static int create_bo_for_fb(int fd, int width, int height,
 			return -EINVAL;
 		}
 	} else {
-		if (is_dumb)
-			*is_dumb = true;
+		fb->is_dumb = true;
 
-		return kmstest_dumb_create(fd, width, height,
-					   fb_plane_bpp(format, 0),
-					   stride_ret, size_ret);
+		fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
+						     fb->plane_bpp[0],
+						     &fb->strides[0], &fb->size);
+
+		return fb->gem_handle;
 	}
 }
 
@@ -512,11 +509,24 @@ int igt_create_bo_with_dimensions(int fd, int width, int height,
 				  unsigned stride, uint64_t *size_ret,
 				  unsigned *stride_ret, bool *is_dumb)
 {
-	return create_bo_for_fb(fd, width, height,
-				IGT_COLOR_YCBCR_BT709,
-				IGT_COLOR_YCBCR_LIMITED_RANGE,
-				lookup_drm_format(format),
-				modifier, 0, stride, size_ret, stride_ret, NULL, is_dumb);
+	struct igt_fb fb;
+
+	fb_init(&fb, fd, width, height, format, modifier,
+		IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
+
+	for (int i = 0; i < fb.num_planes; i++)
+		fb.strides[i] = stride;
+
+	create_bo_for_fb(&fb);
+
+	if (size_ret)
+		*size_ret = fb.size;
+	if (stride_ret)
+		*stride_ret = fb.strides[0];
+	if (is_dumb)
+		*is_dumb = fb.is_dumb;
+
+	return fb.gem_handle;
 }
 
 /**
@@ -860,52 +870,32 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
 	/* FIXME allow the caller to pass these in */
 	enum igt_color_encoding color_encoding = IGT_COLOR_YCBCR_BT709;
 	enum igt_color_range color_range = IGT_COLOR_YCBCR_LIMITED_RANGE;
-	const struct format_desc_struct *f = lookup_drm_format(format);
-	uint32_t pitches[4];
-	uint32_t fb_id;
-	int i;
 
-	igt_assert_f(f, "DRM format %08x not found\n", format);
+	fb_init(fb, fd, width, height, format, tiling,
+		color_encoding, color_range);
 
-	memset(fb, 0, sizeof(*fb));
+	for (int i = 0; i < fb->num_planes; i++)
+		fb->strides[i] = bo_stride;
+
+	fb->size = bo_size;
 
 	igt_debug("%s(width=%d, height=%d, format=0x%x, tiling=0x%"PRIx64", size=%"PRIu64")\n",
 		  __func__, width, height, format, tiling, bo_size);
-	fb->gem_handle = create_bo_for_fb(fd, width, height,
-					  color_encoding, color_range,
-					  f, tiling, bo_size, bo_stride,
-					  &fb->size, &fb->stride,
-					  fb->offsets, &fb->is_dumb);
+
+	create_bo_for_fb(fb);
 	igt_assert(fb->gem_handle > 0);
 
 	igt_debug("%s(handle=%d, pitch=%d)\n",
-		  __func__, fb->gem_handle, fb->stride);
+		  __func__, fb->gem_handle, fb->strides[0]);
 
-	for (i = 0; i < fb_num_planes(f); i++)
-		pitches[i] = fb->stride;
+	do_or_die(__kms_addfb(fb->fd, fb->gem_handle,
+			      fb->width, fb->height,
+			      fb->drm_format, fb->tiling,
+			      fb->strides, fb->offsets, fb->num_planes,
+			      LOCAL_DRM_MODE_FB_MODIFIERS,
+			      &fb->fb_id));
 
-	do_or_die(__kms_addfb(fd, fb->gem_handle, width, height,
-			      format, tiling, pitches, fb->offsets,
-			      fb_num_planes(f),
-			      LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id));
-
-	fb->width = width;
-	fb->height = height;
-	fb->tiling = tiling;
-	fb->drm_format = format;
-	fb->fb_id = fb_id;
-	fb->fd = fd;
-	fb->num_planes = fb_num_planes(f);
-	fb->color_encoding = color_encoding;
-	fb->color_range = color_range;
-
-	for (i = 0; i < fb_num_planes(f); i++) {
-		fb->plane_bpp[i] = fb_plane_bpp(f, i);
-		fb->plane_height[i] = fb_plane_height(f, height, i);
-		fb->plane_width[i] = fb_plane_width(f, width, i);
-	}
-
-	return fb_id;
+	return fb->fb_id;
 }
 
 /**
@@ -1211,12 +1201,8 @@ static cairo_format_t drm_format_to_cairo(uint32_t drm_format)
 }
 
 struct fb_blit_linear {
-	uint64_t size;
-	uint32_t handle;
-	unsigned int stride;
+	struct igt_fb fb;
 	uint8_t *map;
-	bool is_dumb;
-	uint32_t offsets[4];
 };
 
 struct fb_blit_upload {
@@ -1233,27 +1219,27 @@ static void free_linear_mapping(struct fb_blit_upload *blit)
 	unsigned int obj_tiling = igt_fb_mod_to_tiling(fb->tiling);
 	int i;
 
-	gem_munmap(linear->map, linear->size);
-	gem_set_domain(fd, linear->handle,
+	gem_munmap(linear->map, linear->fb.size);
+	gem_set_domain(fd, linear->fb.gem_handle,
 		       I915_GEM_DOMAIN_GTT, 0);
 
 	for (i = 0; i < fb->num_planes; i++)
 		igt_blitter_fast_copy__raw(fd,
-					   linear->handle,
-					   linear->offsets[i],
-					   linear->stride,
+					   linear->fb.gem_handle,
+					   linear->fb.offsets[i],
+					   linear->fb.strides[i],
 					   I915_TILING_NONE,
 					   0, 0, /* src_x, src_y */
 					   fb->plane_width[i], fb->plane_height[i],
 					   fb->plane_bpp[i],
 					   fb->gem_handle,
 					   fb->offsets[i],
-					   fb->stride,
+					   fb->strides[i],
 					   obj_tiling,
 					   0, 0 /* dst_x, dst_y */);
 
-	gem_sync(fd, linear->handle);
-	gem_close(fd, linear->handle);
+	gem_sync(fd, linear->fb.gem_handle);
+	gem_close(fd, linear->fb.gem_handle);
 }
 
 static void destroy_cairo_surface__blit(void *arg)
@@ -1277,42 +1263,42 @@ static void setup_linear_mapping(int fd, struct igt_fb *fb, struct fb_blit_linea
 	 * cairo). This linear bo will be then blitted to its final
 	 * destination, tiling it at the same time.
 	 */
-	linear->handle = create_bo_for_fb(fd, fb->width, fb->height,
-					  fb->color_encoding, fb->color_range,
-					  lookup_drm_format(fb->drm_format),
-					  LOCAL_DRM_FORMAT_MOD_NONE, 0,
-					  0, &linear->size,
-					  &linear->stride,
-					  linear->offsets, &linear->is_dumb);
 
-	igt_assert(linear->handle > 0);
+	fb_init(&linear->fb, fb->fd, fb->width, fb->height,
+		fb->drm_format, LOCAL_DRM_FORMAT_MOD_NONE,
+		fb->color_encoding, fb->color_range);
+
+	create_bo_for_fb(&linear->fb);
+
+	igt_assert(linear->fb.gem_handle > 0);
 
 	/* Copy fb content to linear BO */
-	gem_set_domain(fd, linear->handle,
+	gem_set_domain(fd, linear->fb.gem_handle,
 			I915_GEM_DOMAIN_GTT, 0);
 
 	for (i = 0; i < fb->num_planes; i++)
 		igt_blitter_fast_copy__raw(fd,
-					  fb->gem_handle,
-					  fb->offsets[i],
-					  fb->stride,
-					  obj_tiling,
-					  0, 0, /* src_x, src_y */
-					  fb->plane_width[i], fb->plane_height[i],
-					  fb->plane_bpp[i],
-					  linear->handle, linear->offsets[i],
-					  linear->stride,
-					  I915_TILING_NONE,
-					  0, 0 /* dst_x, dst_y */);
+					   fb->gem_handle,
+					   fb->offsets[i],
+					   fb->strides[i],
+					   obj_tiling,
+					   0, 0, /* src_x, src_y */
+					   fb->plane_width[i], fb->plane_height[i],
+					   fb->plane_bpp[i],
+					   linear->fb.gem_handle,
+					   linear->fb.offsets[i],
+					   linear->fb.strides[i],
+					   I915_TILING_NONE,
+					   0, 0 /* dst_x, dst_y */);
 
-	gem_sync(fd, linear->handle);
+	gem_sync(fd, linear->fb.gem_handle);
 
-	gem_set_domain(fd, linear->handle,
+	gem_set_domain(fd, linear->fb.gem_handle,
 		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
 
 	/* Setup cairo context */
-	linear->map = gem_mmap__cpu(fd, linear->handle,
-				    0, linear->size, PROT_READ | PROT_WRITE);
+	linear->map = gem_mmap__cpu(fd, linear->fb.gem_handle,
+				    0, linear->fb.size, PROT_READ | PROT_WRITE);
 }
 
 static void create_cairo_surface__blit(int fd, struct igt_fb *fb)
@@ -1332,7 +1318,7 @@ static void create_cairo_surface__blit(int fd, struct igt_fb *fb)
 		cairo_image_surface_create_for_data(blit->linear.map,
 						    cairo_format,
 						    fb->width, fb->height,
-						    blit->linear.stride);
+						    blit->linear.fb.strides[0]);
 	fb->domain = I915_GEM_DOMAIN_GTT;
 
 	cairo_surface_set_user_data(fb->cairo_surface,
@@ -1382,7 +1368,7 @@ static void create_cairo_surface__gtt(int fd, struct igt_fb *fb)
 	fb->cairo_surface =
 		cairo_image_surface_create_for_data(ptr,
 						    drm_format_to_cairo(fb->drm_format),
-						    fb->width, fb->height, fb->stride);
+						    fb->width, fb->height, fb->strides[0]);
 	fb->domain = I915_GEM_DOMAIN_GTT;
 
 	cairo_surface_set_user_data(fb->cairo_surface,
@@ -1425,8 +1411,8 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
 	int i, j;
 	const uint8_t *y, *uv;
 	uint8_t *rgb24 = blit->rgb24.map;
-	unsigned rgb24_stride = blit->rgb24.stride, planar_stride = blit->base.linear.stride;
-	uint8_t *buf = malloc(blit->base.linear.size);
+	unsigned rgb24_stride = blit->rgb24.stride, planar_stride = blit->base.linear.fb.strides[0];
+	uint8_t *buf = malloc(blit->base.linear.fb.size);
 	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(fb->color_encoding,
 						    fb->color_range);
 
@@ -1435,9 +1421,9 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
 	 * it's faster to copy the whole BO to a temporary buffer and convert
 	 * from there.
 	 */
-	igt_memcpy_from_wc(buf, blit->base.linear.map, blit->base.linear.size);
-	y = &buf[blit->base.linear.offsets[0]];
-	uv = &buf[blit->base.linear.offsets[1]];
+	igt_memcpy_from_wc(buf, blit->base.linear.map, blit->base.linear.fb.size);
+	y = &buf[blit->base.linear.fb.offsets[0]];
+	uv = &buf[blit->base.linear.fb.offsets[1]];
 
 	for (i = 0; i < fb->height / 2; i++) {
 		for (j = 0; j < fb->width / 2; j++) {
@@ -1531,11 +1517,11 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
 static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
 {
 	int i, j;
-	uint8_t *y = &blit->base.linear.map[blit->base.linear.offsets[0]];
-	uint8_t *uv = &blit->base.linear.map[blit->base.linear.offsets[1]];
+	uint8_t *y = &blit->base.linear.map[blit->base.linear.fb.offsets[0]];
+	uint8_t *uv = &blit->base.linear.map[blit->base.linear.fb.offsets[1]];
 	const uint8_t *rgb24 = blit->rgb24.map;
 	unsigned rgb24_stride = blit->rgb24.stride;
-	unsigned planar_stride = blit->base.linear.stride;
+	unsigned planar_stride = blit->base.linear.fb.strides[0];
 	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(fb->color_encoding,
 						    fb->color_range);
 
@@ -1662,8 +1648,8 @@ static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
 	int i, j;
 	const uint8_t *yuyv;
 	uint8_t *rgb24 = blit->rgb24.map;
-	unsigned rgb24_stride = blit->rgb24.stride, yuyv_stride = blit->base.linear.stride;
-	uint8_t *buf = malloc(blit->base.linear.size);
+	unsigned rgb24_stride = blit->rgb24.stride, yuyv_stride = blit->base.linear.fb.strides[0];
+	uint8_t *buf = malloc(blit->base.linear.fb.size);
 	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(fb->color_encoding,
 						    fb->color_range);
 
@@ -1672,7 +1658,7 @@ static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
 	 * it's faster to copy the whole BO to a temporary buffer and convert
 	 * from there.
 	 */
-	igt_memcpy_from_wc(buf, blit->base.linear.map, blit->base.linear.size);
+	igt_memcpy_from_wc(buf, blit->base.linear.map, blit->base.linear.fb.size);
 	yuyv = buf;
 
 	for (i = 0; i < fb->height; i++) {
@@ -1722,7 +1708,7 @@ static void convert_rgb24_to_yuyv(struct igt_fb *fb, struct fb_convert_blit_uplo
 	uint8_t *yuyv = blit->base.linear.map;
 	const uint8_t *rgb24 = blit->rgb24.map;
 	unsigned rgb24_stride = blit->rgb24.stride;
-	unsigned yuyv_stride = blit->base.linear.stride;
+	unsigned yuyv_stride = blit->base.linear.fb.strides[0];
 	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(fb->color_encoding,
 						    fb->color_range);
 
@@ -1791,7 +1777,7 @@ static void destroy_cairo_surface__convert(void *arg)
 
 	munmap(blit->rgb24.map, blit->rgb24.size);
 
-	if (blit->base.linear.handle)
+	if (blit->base.linear.fb.gem_handle)
 		free_linear_mapping(&blit->base);
 	else
 		gem_munmap(blit->base.linear.map, fb->size);
@@ -1817,15 +1803,15 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
 	    fb->tiling == LOCAL_I915_FORMAT_MOD_Yf_TILED) {
 		setup_linear_mapping(fd, fb, &blit->base.linear);
 	} else {
-		blit->base.linear.handle = 0;
+		blit->base.linear.fb.gem_handle = 0;
 		gem_set_domain(fd, fb->gem_handle,
 			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
 		blit->base.linear.map = gem_mmap__gtt(fd, fb->gem_handle, fb->size,
 						      PROT_READ | PROT_WRITE);
 		igt_assert(blit->base.linear.map);
-		blit->base.linear.stride = fb->stride;
-		blit->base.linear.size = fb->size;
-		memcpy(blit->base.linear.offsets, fb->offsets, sizeof(fb->offsets));
+		blit->base.linear.fb.size = fb->size;
+		memcpy(blit->base.linear.fb.strides, fb->strides, sizeof(fb->strides));
+		memcpy(blit->base.linear.fb.offsets, fb->offsets, sizeof(fb->offsets));
 	}
 
 	/* Convert to linear rgb! */
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index 2343fe505f28..35bf307a930b 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -47,12 +47,12 @@
  * @drm_format: DRM FOURCC code
  * @width: width in pixels
  * @height: height in pixels
- * @stride: line stride in bytes
  * @tiling: tiling mode as a DRM framebuffer modifier
  * @size: size in bytes of the underlying backing storage
  * @cairo_surface: optionally attached cairo drawing surface
  * @domain: current domain for cache flushing tracking on i915.ko
  * @num_planes: Amount of planes on this fb. >1 for planar formats.
+ * @strides: line stride for each plane in bytes
  * @offsets: Offset for each plane in bytes.
  * @plane_bpp: The bpp for each plane.
  * @plane_width: The width for each plane.
@@ -70,12 +70,12 @@ typedef struct igt_fb {
 	int height;
 	enum igt_color_encoding color_encoding;
 	enum igt_color_range color_range;
-	unsigned int stride;
 	uint64_t tiling;
 	uint64_t size;
 	cairo_surface_t *cairo_surface;
 	unsigned int domain;
 	unsigned int num_planes;
+	uint32_t strides[4];
 	uint32_t offsets[4];
 	unsigned int plane_bpp[4];
 	unsigned int plane_width[4];
diff --git a/tests/kms_ccs.c b/tests/kms_ccs.c
index e1ee58801ac3..e03f947eae11 100644
--- a/tests/kms_ccs.c
+++ b/tests/kms_ccs.c
@@ -378,7 +378,7 @@ static void generate_fb(data_t *data, struct igt_fb *fb,
 	fb->drm_format = f.pixel_format;
 	fb->width = f.width;
 	fb->height = f.height;
-	fb->stride = f.pitches[0];
+	fb->strides[0] = f.pitches[0];
 	fb->tiling = f.modifier[0];
 	fb->size = size[0];
 	fb->cairo_surface = NULL;
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 393d690ab535..f7d08a60aeea 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -592,7 +592,7 @@ static void recreate_fb(struct test_output *o)
 	igt_assert(r);
 
 	do_or_die(drmModeAddFB(drm_fd, o->fb_width, o->fb_height, o->depth,
-			       o->bpp, fb_info->stride,
+			       o->bpp, fb_info->strides[0],
 			       r->handle, &new_fb_id));
 
 	gem_close(drm_fd, r->handle);
@@ -612,7 +612,7 @@ static void set_y_tiling(struct test_output *o, int fb_idx)
 	r = drmModeGetFB(drm_fd, fb_info->fb_id);
 	igt_assert(r);
 	/* Newer kernels don't allow such shenagians any more, so skip the test. */
-	igt_require(__gem_set_tiling(drm_fd, r->handle, I915_TILING_Y, fb_info->stride) == 0);
+	igt_require(__gem_set_tiling(drm_fd, r->handle, I915_TILING_Y, fb_info->strides[0]) == 0);
 	gem_close(drm_fd, r->handle);
 	drmFree(r);
 }
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 1bce676081b4..265c313a3886 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -1157,7 +1157,7 @@ static void start_busy_thread(struct igt_fb *fb)
 	busy_thread.stop = false;
 	busy_thread.handle = fb->gem_handle;
 	busy_thread.size = fb->size;
-	busy_thread.stride = fb->stride;
+	busy_thread.stride = fb->strides[0];
 	busy_thread.width = fb->width;
 	busy_thread.height = fb->height;
 	busy_thread.color = pick_color(fb, COLOR_PRIM_BG);
@@ -2859,7 +2859,7 @@ static void badstride_subtest(const struct test_mode *t)
 
 	create_fb(t->format, params->primary.fb->width + 4096, params->primary.fb->height,
 		  opt.tiling, t->plane, &wide_fb);
-	igt_assert(wide_fb.stride > 16384);
+	igt_assert(wide_fb.strides[0] > 16384);
 
 	fill_fb(&wide_fb, COLOR_PRIM_BG);
 
@@ -2928,7 +2928,7 @@ static void stridechange_subtest(const struct test_mode *t)
 		  opt.tiling, t->plane, &new_fb);
 	fill_fb(&new_fb, COLOR_PRIM_BG);
 
-	igt_assert(old_fb->stride != new_fb.stride);
+	igt_assert(old_fb->strides[0] != new_fb.strides[0]);
 
 	/* We can't assert that FBC will be enabled since there may not be
 	 * enough space for the CFB, but we can check the CRC. */
diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
index c24fd95bb0f3..2d6c5b49c9d5 100644
--- a/tests/pm_rpm.c
+++ b/tests/pm_rpm.c
@@ -1547,7 +1547,7 @@ static void cursor_subtest(bool dpms)
 	 * hopefully it has some fences around it. */
 	rc = drmModeRmFB(drm_fd, cursor_fb3.fb_id);
 	igt_assert_eq(rc, 0);
-	gem_set_tiling(drm_fd, cursor_fb3.gem_handle, false, cursor_fb3.stride);
+	gem_set_tiling(drm_fd, cursor_fb3.gem_handle, false, cursor_fb3.strides[0]);
 	igt_assert(wait_for_suspended());
 
 	rc = drmModeSetCursor(drm_fd, crtc_id, cursor_fb3.gem_handle,
-- 
2.16.4

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

Re: [igt-dev] [PATCH i-g-t 4/5] lib/igt_fb: Pass around igt_fb internally

[Paulo Zanoni]

Em Ter, 2018-09-25 às 16:47 +0300, Ville Syrjala escreveu:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Instead of passing around a boatload of integers everywhere let's
> just pass around the igt_fb struct. That obviously means we have to
> populate it first sufficiently, to which end we'll add a small
> helper.
> Later on the stride/size calculations will consult the already
> pre-populated igt_fb and fill in the rest as needed.
> 
> This makes the whole thing a lot less error prone as it's impossible
> to accidentally pass the arguments in the wrong order when there's
> just the one of them, and it's a pointer.

While I completely agree with these arguments, this change does not
come without its own minor downsides. Code that deals-with/initializes
igt_fb_t is now a little riskier due to the the fact that the order
which things are initialized is way more important now. And we also
moved a whole layer of our abstractions up, but that didn't seem to be
a problem now so let's hope it still won't be a problem later.

Still, a positive change even when downsides are considered.

> 
> v2: Rebase due to uint64_t size
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  lib/igt_draw.c                   |   2 +-
>  lib/igt_fb.c                     | 408 +++++++++++++++++++--------
> ------------
>  lib/igt_fb.h                     |   4 +-
>  tests/kms_ccs.c                  |   2 +-
>  tests/kms_flip.c                 |   4 +-
>  tests/kms_frontbuffer_tracking.c |   6 +-
>  tests/pm_rpm.c                   |   2 +-
>  7 files changed, 207 insertions(+), 221 deletions(-)
> 
> diff --git a/lib/igt_draw.c b/lib/igt_draw.c
> index c7d5770dca28..05821480bc80 100644
> --- a/lib/igt_draw.c
> +++ b/lib/igt_draw.c
> @@ -720,7 +720,7 @@ void igt_draw_rect_fb(int fd, drm_intel_bufmgr
> *bufmgr,
>  		      enum igt_draw_method method, int rect_x, int
> rect_y,
>  		      int rect_w, int rect_h, uint32_t color)
>  {
> -	igt_draw_rect(fd, bufmgr, context, fb->gem_handle, fb->size, 
> fb->stride,
> +	igt_draw_rect(fd, bufmgr, context, fb->gem_handle, fb->size, 
> fb->strides[0],
>  		      method, rect_x, rect_y, rect_w, rect_h, color,
>  		      igt_drm_format_to_bpp(fb->drm_format));
>  }
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 3c6974503313..26019f0420dc 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -190,50 +190,72 @@ void igt_get_fb_tile_size(int fd, uint64_t
> tiling, int fb_bpp,
>  	}
>  }
>  
> -static unsigned fb_plane_width(const struct format_desc_struct
> *format,
> -			       int plane, unsigned width)
> +static unsigned fb_plane_width(const struct igt_fb *fb, int plane)
>  {
> -	if (format->drm_id == DRM_FORMAT_NV12 && plane == 1)
> -		return DIV_ROUND_UP(width, 2);
> +	if (fb->drm_format == DRM_FORMAT_NV12 && plane == 1)
> +		return DIV_ROUND_UP(fb->width, 2);
>  
> -	return width;
> +	return fb->width;
>  }
>  
> -static unsigned fb_plane_bpp(const struct format_desc_struct
> *format, int plane)
> +static unsigned fb_plane_bpp(const struct igt_fb *fb, int plane)
>  {
> +	const struct format_desc_struct *format =
> lookup_drm_format(fb->drm_format);
> +
>  	return format->plane_bpp[plane];
>  }
>  
> -static unsigned fb_plane_min_stride(const struct format_desc_struct
> *format,
> -				    int plane, unsigned width)
> +static unsigned fb_plane_height(const struct igt_fb *fb, int plane)
>  {
> -	unsigned cpp = fb_plane_bpp(format, plane) / 8;
> +	if (fb->drm_format == DRM_FORMAT_NV12 && plane == 1)
> +		return DIV_ROUND_UP(fb->height, 2);
>  
> -	return fb_plane_width(format, width, plane) * cpp;
> +	return fb->height;
>  }
>  
> -static unsigned fb_plane_height(const struct format_desc_struct
> *format,
> -				int plane, unsigned height)
> +static int fb_num_planes(const struct igt_fb *fb)
>  {
> -	if (format->drm_id == DRM_FORMAT_NV12 && plane == 1)
> -		return DIV_ROUND_UP(height, 2);
> +	const struct format_desc_struct *format =
> lookup_drm_format(fb->drm_format);
>  
> -	return height;
> -}
> -
> -static int fb_num_planes(const struct format_desc_struct *format)
> -{
>  	return format->num_planes;
>  }
>  
> -static unsigned calc_plane_stride(int fd,
> -				  const struct format_desc_struct
> *format,
> -				  int width, uint64_t tiling, int
> plane)
> +static void fb_init(struct igt_fb *fb,
> +		    int fd, int width, int height,
> +		    uint32_t drm_format,
> +		    uint64_t modifier,
> +		    enum igt_color_encoding color_encoding,
> +		    enum igt_color_range color_range)
>  {
> -	uint32_t min_stride = fb_plane_min_stride(format, width,
> plane);
> +	const struct format_desc_struct *f =
> lookup_drm_format(drm_format);
>  
> -	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> -	    intel_gen(intel_get_drm_devid(fd)) <= 3) {
> +	igt_assert_f(f, "DRM format %08x not found\n", drm_format);
> +
> +	memset(fb, 0, sizeof(*fb));
> +
> +	fb->width = width;
> +	fb->height = height;
> +	fb->tiling = modifier;

Shouldn't we rename fb->tiling now? (of course, in a separate patch)

Everything else looks correct.

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

> +	fb->drm_format = drm_format;
> +	fb->fd = fd;
> +	fb->num_planes = fb_num_planes(fb);
> +	fb->color_encoding = color_encoding;
> +	fb->color_range = color_range;
> +
> +	for (int i = 0; i < fb->num_planes; i++) {
> +		fb->plane_bpp[i] = fb_plane_bpp(fb, i);
> +		fb->plane_height[i] = fb_plane_height(fb, i);
> +		fb->plane_width[i] = fb_plane_width(fb, i);
> +	}
> +}
> +
> +static uint32_t calc_plane_stride(struct igt_fb *fb, int plane)
> +{
> +	uint32_t min_stride = fb->plane_width[plane] *
> +		(fb->plane_bpp[plane] / 8);
> +
> +	if (fb->tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> +	    intel_gen(intel_get_drm_devid(fb->fd)) <= 3) {
>  		uint32_t stride;
>  
>  		/* Round the tiling up to the next power-of-two and
> the region
> @@ -251,22 +273,19 @@ static unsigned calc_plane_stride(int fd,
>  	} else {
>  		unsigned int tile_width, tile_height;
>  
> -		igt_get_fb_tile_size(fd, tiling,
> -				     fb_plane_bpp(format, plane),
> +		igt_get_fb_tile_size(fb->fd, fb->tiling, fb-
> >plane_bpp[plane],
>  				     &tile_width, &tile_height);
>  
>  		return ALIGN(min_stride, tile_width);
>  	}
>  }
>  
> -static uint64_t calc_plane_size(int fd, int width, int height,
> -				const struct format_desc_struct
> *format,
> -				uint64_t tiling, int plane,
> -				uint32_t stride)
> +static uint64_t calc_plane_size(struct igt_fb *fb, int plane)
>  {
> -	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> -	    intel_gen(intel_get_drm_devid(fd)) <= 3) {
> -		uint64_t min_size = (uint64_t) stride * height;
> +	if (fb->tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> +	    intel_gen(intel_get_drm_devid(fb->fd)) <= 3) {
> +		uint64_t min_size = (uint64_t) fb->strides[plane] *
> +			fb->plane_height[plane];
>  		uint64_t size;
>  
>  		/* Round the tiling up to the next power-of-two and
> the region
> @@ -284,38 +303,27 @@ static uint64_t calc_plane_size(int fd, int
> width, int height,
>  	} else {
>  		unsigned int tile_width, tile_height;
>  
> -		igt_get_fb_tile_size(fd, tiling,
> fb_plane_bpp(format, plane),
> +		igt_get_fb_tile_size(fb->fd, fb->tiling, fb-
> >plane_bpp[plane],
>  				     &tile_width, &tile_height);
>  
> -		return (uint64_t) stride * ALIGN(height,
> tile_height);
> +		return (uint64_t) fb->strides[plane] *
> +			ALIGN(fb->plane_height[plane], tile_height);
>  	}
>  }
>  
> -static uint64_t calc_fb_size(int fd, int width, int height,
> -			     const struct format_desc_struct
> *format,
> -			     uint64_t tiling,
> -			     uint32_t strides[4], uint32_t
> offsets[4])
> +static uint64_t calc_fb_size(struct igt_fb *fb)
>  {
>  	uint64_t size = 0;
>  	int plane;
>  
> -	for (plane = 0; plane < fb_num_planes(format); plane++) {
> -		if (!strides[plane])
> -			strides[plane] = calc_plane_stride(fd,
> format,
> -							   width,
> tiling, plane);
> +	for (plane = 0; plane < fb->num_planes; plane++) {
> +		/* respect the stride requested by the caller */
> +		if (!fb->strides[plane])
> +			fb->strides[plane] = calc_plane_stride(fb,
> plane);
>  
> -		if (offsets)
> -			offsets[plane] = size;
> +		fb->offsets[plane] = size;
>  
> -		size += calc_plane_size(fd, width, height,
> -					format, tiling, plane,
> -					strides[plane]);
> -	}
> -
> -	for (; plane < ARRAY_SIZE(format->plane_bpp); plane++) {
> -		strides[plane] = 0;
> -		if (offsets)
> -			offsets[plane] = 0;
> +		size += calc_plane_size(fb, plane);
>  	}
>  
>  	return size;
> @@ -337,13 +345,17 @@ static uint64_t calc_fb_size(int fd, int width,
> int height,
>  void igt_calc_fb_size(int fd, int width, int height, uint32_t
> drm_format, uint64_t tiling,
>  		      uint64_t *size_ret, unsigned *stride_ret)
>  {
> -	const struct format_desc_struct *format =
> lookup_drm_format(drm_format);
> -	uint32_t strides[4] = {};
> +	struct igt_fb fb;
>  
> -	igt_assert(format);
> +	fb_init(&fb, fd, width, height, drm_format, tiling,
> +		IGT_COLOR_YCBCR_BT709,
> IGT_COLOR_YCBCR_LIMITED_RANGE);
>  
> -	*size_ret = calc_fb_size(fd, width, height, format, tiling,
> strides, NULL);
> -	*stride_ret = strides[0];
> +	fb.size = calc_fb_size(&fb);
> +
> +	if (size_ret)
> +		*size_ret = fb.size;
> +	if (stride_ret)
> +		*stride_ret = fb.strides[0];
>  }
>  
>  /**
> @@ -399,80 +411,64 @@ uint64_t igt_fb_tiling_to_mod(uint64_t tiling)
>  }
>  
>  /* helpers to create nice-looking framebuffers */
> -static int create_bo_for_fb(int fd, int width, int height,
> -			    enum igt_color_encoding color_encoding,
> -			    enum igt_color_range color_range,
> -			    const struct format_desc_struct *format,
> -			    uint64_t tiling, uint64_t size, unsigned
> stride,
> -			    uint64_t *size_ret, unsigned
> *stride_ret,
> -			    uint32_t offsets[4], bool *is_dumb)
> +static int create_bo_for_fb(struct igt_fb *fb)
>  {
> -	int bo;
> +	int fd = fb->fd;
>  
> -	igt_assert(format);
> +	if (fb->tiling || fb->size || fb->strides[0] ||
> igt_format_is_yuv(fb->drm_format)) {
> +		uint64_t size;
>  
> -	if (offsets)
> -		memset(offsets, 0, ARRAY_SIZE(format->plane_bpp) *
> sizeof(*offsets));
> +		size = calc_fb_size(fb);
>  
> -	if (tiling || size || stride || igt_format_is_yuv(format-
> >drm_id)) {
> -		uint64_t calculated_size;
> -		uint32_t strides[4] = {
> -			stride,
> -		};
> +		/* respect the size requested by the caller */
> +		if (fb->size == 0)
> +			fb->size = size;
>  
> -		calculated_size = calc_fb_size(fd, width, height,
> -					       format, tiling,
> -					       strides, offsets);
> -
> -		if (stride == 0)
> -			stride = strides[0];
> -		if (size == 0)
> -			size = calculated_size;
> -
> -		if (is_dumb)
> -			*is_dumb = false;
> +		fb->is_dumb = false;
>  
>  		if (is_i915_device(fd)) {
>  			void *ptr;
> -			bool full_range = color_range ==
> IGT_COLOR_YCBCR_FULL_RANGE;
> +			bool full_range = fb->color_range ==
> IGT_COLOR_YCBCR_FULL_RANGE;
>  
> -			bo = gem_create(fd, size);
> -			gem_set_tiling(fd, bo,
> igt_fb_mod_to_tiling(tiling), stride);
> +			fb->gem_handle = gem_create(fd, fb->size);
>  
> -			gem_set_domain(fd, bo,
> +			gem_set_tiling(fd, fb->gem_handle,
> +				       igt_fb_mod_to_tiling(fb-
> >tiling),
> +				       fb->strides[0]);
> +
> +			gem_set_domain(fd, fb->gem_handle,
>  				       I915_GEM_DOMAIN_GTT,
> I915_GEM_DOMAIN_GTT);
>  
>  			/* Ensure the framebuffer is preallocated */
> -			ptr = gem_mmap__gtt(fd, bo, size, PROT_READ
> | PROT_WRITE);
> +			ptr = gem_mmap__gtt(fd, fb->gem_handle,
> +					    fb->size, PROT_READ |
> PROT_WRITE);
>  			igt_assert(*(uint32_t *)ptr == 0);
>  
> -			switch (format->drm_id) {
> +			switch (fb->drm_format) {
>  			case DRM_FORMAT_NV12:
> -				memset(ptr + offsets[0], full_range
> ? 0x00 : 0x10,
> -				       strides[0] * height);
> -				memset(ptr + offsets[1], 0x80,
> -				       strides[1] * height/2);
> +				memset(ptr + fb->offsets[0],
> +				       full_range ? 0x00 : 0x10,
> +				       fb->strides[0] * fb-
> >plane_height[0]);
> +				memset(ptr + fb->offsets[1],
> +				       0x80,
> +				       fb->strides[1] * fb-
> >plane_height[1]);
>  				break;
>  			case DRM_FORMAT_YUYV:
>  			case DRM_FORMAT_YVYU:
> -				wmemset(ptr, full_range ? 0x80008000
> : 0x80108010,
> -					strides[0] * height /
> sizeof(wchar_t));
> +				wmemset(ptr + fb->offsets[0],
> +					full_range ? 0x80008000 :
> 0x80108010,
> +					fb->strides[0] * fb-
> >plane_height[0] / sizeof(wchar_t));
>  				break;
>  			case DRM_FORMAT_UYVY:
>  			case DRM_FORMAT_VYUY:
> -				wmemset(ptr, full_range ? 0x00800080
> : 0x10801080,
> -					strides[0] * height /
> sizeof(wchar_t));
> +				wmemset(ptr + fb->offsets[0],
> +					full_range ? 0x00800080 :
> 0x10801080,
> +					fb->strides[0] * fb-
> >plane_height[0] / sizeof(wchar_t));
>  				break;
>  			}
> -			gem_munmap(ptr, size);
> +			gem_munmap(ptr, fb->size);
>  
> -			if (size_ret)
> -				*size_ret = size;
> -
> -			if (stride_ret)
> -				*stride_ret = strides[0];
> -
> -			return bo;
> +			return fb->gem_handle;
>  		} else {
>  			bool driver_has_gem_api = false;
>  
> @@ -480,12 +476,13 @@ static int create_bo_for_fb(int fd, int width,
> int height,
>  			return -EINVAL;
>  		}
>  	} else {
> -		if (is_dumb)
> -			*is_dumb = true;
> +		fb->is_dumb = true;
>  
> -		return kmstest_dumb_create(fd, width, height,
> -					   fb_plane_bpp(format, 0),
> -					   stride_ret, size_ret);
> +		fb->gem_handle = kmstest_dumb_create(fd, fb->width,
> fb->height,
> +						     fb-
> >plane_bpp[0],
> +						     &fb-
> >strides[0], &fb->size);
> +
> +		return fb->gem_handle;
>  	}
>  }
>  
> @@ -512,11 +509,24 @@ int igt_create_bo_with_dimensions(int fd, int
> width, int height,
>  				  unsigned stride, uint64_t
> *size_ret,
>  				  unsigned *stride_ret, bool
> *is_dumb)
>  {
> -	return create_bo_for_fb(fd, width, height,
> -				IGT_COLOR_YCBCR_BT709,
> -				IGT_COLOR_YCBCR_LIMITED_RANGE,
> -				lookup_drm_format(format),
> -				modifier, 0, stride, size_ret,
> stride_ret, NULL, is_dumb);
> +	struct igt_fb fb;
> +
> +	fb_init(&fb, fd, width, height, format, modifier,
> +		IGT_COLOR_YCBCR_BT709,
> IGT_COLOR_YCBCR_LIMITED_RANGE);
> +
> +	for (int i = 0; i < fb.num_planes; i++)
> +		fb.strides[i] = stride;
> +
> +	create_bo_for_fb(&fb);
> +
> +	if (size_ret)
> +		*size_ret = fb.size;
> +	if (stride_ret)
> +		*stride_ret = fb.strides[0];
> +	if (is_dumb)
> +		*is_dumb = fb.is_dumb;
> +
> +	return fb.gem_handle;
>  }
>  
>  /**
> @@ -860,52 +870,32 @@ igt_create_fb_with_bo_size(int fd, int width,
> int height,
>  	/* FIXME allow the caller to pass these in */
>  	enum igt_color_encoding color_encoding =
> IGT_COLOR_YCBCR_BT709;
>  	enum igt_color_range color_range =
> IGT_COLOR_YCBCR_LIMITED_RANGE;
> -	const struct format_desc_struct *f =
> lookup_drm_format(format);
> -	uint32_t pitches[4];
> -	uint32_t fb_id;
> -	int i;
>  
> -	igt_assert_f(f, "DRM format %08x not found\n", format);
> +	fb_init(fb, fd, width, height, format, tiling,
> +		color_encoding, color_range);
>  
> -	memset(fb, 0, sizeof(*fb));
> +	for (int i = 0; i < fb->num_planes; i++)
> +		fb->strides[i] = bo_stride;
> +
> +	fb->size = bo_size;
>  
>  	igt_debug("%s(width=%d, height=%d, format=0x%x,
> tiling=0x%"PRIx64", size=%"PRIu64")\n",
>  		  __func__, width, height, format, tiling, bo_size);
> -	fb->gem_handle = create_bo_for_fb(fd, width, height,
> -					  color_encoding,
> color_range,
> -					  f, tiling, bo_size,
> bo_stride,
> -					  &fb->size, &fb->stride,
> -					  fb->offsets, &fb-
> >is_dumb);
> +
> +	create_bo_for_fb(fb);
>  	igt_assert(fb->gem_handle > 0);
>  
>  	igt_debug("%s(handle=%d, pitch=%d)\n",
> -		  __func__, fb->gem_handle, fb->stride);
> +		  __func__, fb->gem_handle, fb->strides[0]);
>  
> -	for (i = 0; i < fb_num_planes(f); i++)
> -		pitches[i] = fb->stride;
> +	do_or_die(__kms_addfb(fb->fd, fb->gem_handle,
> +			      fb->width, fb->height,
> +			      fb->drm_format, fb->tiling,
> +			      fb->strides, fb->offsets, fb-
> >num_planes,
> +			      LOCAL_DRM_MODE_FB_MODIFIERS,
> +			      &fb->fb_id));
>  
> -	do_or_die(__kms_addfb(fd, fb->gem_handle, width, height,
> -			      format, tiling, pitches, fb->offsets,
> -			      fb_num_planes(f),
> -			      LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id));
> -
> -	fb->width = width;
> -	fb->height = height;
> -	fb->tiling = tiling;
> -	fb->drm_format = format;
> -	fb->fb_id = fb_id;
> -	fb->fd = fd;
> -	fb->num_planes = fb_num_planes(f);
> -	fb->color_encoding = color_encoding;
> -	fb->color_range = color_range;
> -
> -	for (i = 0; i < fb_num_planes(f); i++) {
> -		fb->plane_bpp[i] = fb_plane_bpp(f, i);
> -		fb->plane_height[i] = fb_plane_height(f, height, i);
> -		fb->plane_width[i] = fb_plane_width(f, width, i);
> -	}
> -
> -	return fb_id;
> +	return fb->fb_id;
>  }
>  
>  /**
> @@ -1211,12 +1201,8 @@ static cairo_format_t
> drm_format_to_cairo(uint32_t drm_format)
>  }
>  
>  struct fb_blit_linear {
> -	uint64_t size;
> -	uint32_t handle;
> -	unsigned int stride;
> +	struct igt_fb fb;
>  	uint8_t *map;
> -	bool is_dumb;
> -	uint32_t offsets[4];
>  };
>  
>  struct fb_blit_upload {
> @@ -1233,27 +1219,27 @@ static void free_linear_mapping(struct
> fb_blit_upload *blit)
>  	unsigned int obj_tiling = igt_fb_mod_to_tiling(fb->tiling);
>  	int i;
>  
> -	gem_munmap(linear->map, linear->size);
> -	gem_set_domain(fd, linear->handle,
> +	gem_munmap(linear->map, linear->fb.size);
> +	gem_set_domain(fd, linear->fb.gem_handle,
>  		       I915_GEM_DOMAIN_GTT, 0);
>  
>  	for (i = 0; i < fb->num_planes; i++)
>  		igt_blitter_fast_copy__raw(fd,
> -					   linear->handle,
> -					   linear->offsets[i],
> -					   linear->stride,
> +					   linear->fb.gem_handle,
> +					   linear->fb.offsets[i],
> +					   linear->fb.strides[i],
>  					   I915_TILING_NONE,
>  					   0, 0, /* src_x, src_y */
>  					   fb->plane_width[i], fb-
> >plane_height[i],
>  					   fb->plane_bpp[i],
>  					   fb->gem_handle,
>  					   fb->offsets[i],
> -					   fb->stride,
> +					   fb->strides[i],
>  					   obj_tiling,
>  					   0, 0 /* dst_x, dst_y */);
>  
> -	gem_sync(fd, linear->handle);
> -	gem_close(fd, linear->handle);
> +	gem_sync(fd, linear->fb.gem_handle);
> +	gem_close(fd, linear->fb.gem_handle);
>  }
>  
>  static void destroy_cairo_surface__blit(void *arg)
> @@ -1277,42 +1263,42 @@ static void setup_linear_mapping(int fd,
> struct igt_fb *fb, struct fb_blit_linea
>  	 * cairo). This linear bo will be then blitted to its final
>  	 * destination, tiling it at the same time.
>  	 */
> -	linear->handle = create_bo_for_fb(fd, fb->width, fb->height,
> -					  fb->color_encoding, fb-
> >color_range,
> -					  lookup_drm_format(fb-
> >drm_format),
> -					  LOCAL_DRM_FORMAT_MOD_NONE,
> 0,
> -					  0, &linear->size,
> -					  &linear->stride,
> -					  linear->offsets, &linear-
> >is_dumb);
>  
> -	igt_assert(linear->handle > 0);
> +	fb_init(&linear->fb, fb->fd, fb->width, fb->height,
> +		fb->drm_format, LOCAL_DRM_FORMAT_MOD_NONE,
> +		fb->color_encoding, fb->color_range);
> +
> +	create_bo_for_fb(&linear->fb);
> +
> +	igt_assert(linear->fb.gem_handle > 0);
>  
>  	/* Copy fb content to linear BO */
> -	gem_set_domain(fd, linear->handle,
> +	gem_set_domain(fd, linear->fb.gem_handle,
>  			I915_GEM_DOMAIN_GTT, 0);
>  
>  	for (i = 0; i < fb->num_planes; i++)
>  		igt_blitter_fast_copy__raw(fd,
> -					  fb->gem_handle,
> -					  fb->offsets[i],
> -					  fb->stride,
> -					  obj_tiling,
> -					  0, 0, /* src_x, src_y */
> -					  fb->plane_width[i], fb-
> >plane_height[i],
> -					  fb->plane_bpp[i],
> -					  linear->handle, linear-
> >offsets[i],
> -					  linear->stride,
> -					  I915_TILING_NONE,
> -					  0, 0 /* dst_x, dst_y */);
> +					   fb->gem_handle,
> +					   fb->offsets[i],
> +					   fb->strides[i],
> +					   obj_tiling,
> +					   0, 0, /* src_x, src_y */
> +					   fb->plane_width[i], fb-
> >plane_height[i],
> +					   fb->plane_bpp[i],
> +					   linear->fb.gem_handle,
> +					   linear->fb.offsets[i],
> +					   linear->fb.strides[i],
> +					   I915_TILING_NONE,
> +					   0, 0 /* dst_x, dst_y */);
>  
> -	gem_sync(fd, linear->handle);
> +	gem_sync(fd, linear->fb.gem_handle);
>  
> -	gem_set_domain(fd, linear->handle,
> +	gem_set_domain(fd, linear->fb.gem_handle,
>  		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
>  
>  	/* Setup cairo context */
> -	linear->map = gem_mmap__cpu(fd, linear->handle,
> -				    0, linear->size, PROT_READ |
> PROT_WRITE);
> +	linear->map = gem_mmap__cpu(fd, linear->fb.gem_handle,
> +				    0, linear->fb.size, PROT_READ |
> PROT_WRITE);
>  }
>  
>  static void create_cairo_surface__blit(int fd, struct igt_fb *fb)
> @@ -1332,7 +1318,7 @@ static void create_cairo_surface__blit(int fd,
> struct igt_fb *fb)
>  		cairo_image_surface_create_for_data(blit-
> >linear.map,
>  						    cairo_format,
>  						    fb->width, fb-
> >height,
> -						    blit-
> >linear.stride);
> +						    blit-
> >linear.fb.strides[0]);
>  	fb->domain = I915_GEM_DOMAIN_GTT;
>  
>  	cairo_surface_set_user_data(fb->cairo_surface,
> @@ -1382,7 +1368,7 @@ static void create_cairo_surface__gtt(int fd,
> struct igt_fb *fb)
>  	fb->cairo_surface =
>  		cairo_image_surface_create_for_data(ptr,
>  						    drm_format_to_ca
> iro(fb->drm_format),
> -						    fb->width, fb-
> >height, fb->stride);
> +						    fb->width, fb-
> >height, fb->strides[0]);
>  	fb->domain = I915_GEM_DOMAIN_GTT;
>  
>  	cairo_surface_set_user_data(fb->cairo_surface,
> @@ -1425,8 +1411,8 @@ static void convert_nv12_to_rgb24(struct igt_fb
> *fb, struct fb_convert_blit_uplo
>  	int i, j;
>  	const uint8_t *y, *uv;
>  	uint8_t *rgb24 = blit->rgb24.map;
> -	unsigned rgb24_stride = blit->rgb24.stride, planar_stride =
> blit->base.linear.stride;
> -	uint8_t *buf = malloc(blit->base.linear.size);
> +	unsigned rgb24_stride = blit->rgb24.stride, planar_stride =
> blit->base.linear.fb.strides[0];
> +	uint8_t *buf = malloc(blit->base.linear.fb.size);
>  	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(fb-
> >color_encoding,
>  						    fb-
> >color_range);
>  
> @@ -1435,9 +1421,9 @@ static void convert_nv12_to_rgb24(struct igt_fb
> *fb, struct fb_convert_blit_uplo
>  	 * it's faster to copy the whole BO to a temporary buffer
> and convert
>  	 * from there.
>  	 */
> -	igt_memcpy_from_wc(buf, blit->base.linear.map, blit-
> >base.linear.size);
> -	y = &buf[blit->base.linear.offsets[0]];
> -	uv = &buf[blit->base.linear.offsets[1]];
> +	igt_memcpy_from_wc(buf, blit->base.linear.map, blit-
> >base.linear.fb.size);
> +	y = &buf[blit->base.linear.fb.offsets[0]];
> +	uv = &buf[blit->base.linear.fb.offsets[1]];
>  
>  	for (i = 0; i < fb->height / 2; i++) {
>  		for (j = 0; j < fb->width / 2; j++) {
> @@ -1531,11 +1517,11 @@ static void convert_nv12_to_rgb24(struct
> igt_fb *fb, struct fb_convert_blit_uplo
>  static void convert_rgb24_to_nv12(struct igt_fb *fb, struct
> fb_convert_blit_upload *blit)
>  {
>  	int i, j;
> -	uint8_t *y = &blit->base.linear.map[blit-
> >base.linear.offsets[0]];
> -	uint8_t *uv = &blit->base.linear.map[blit-
> >base.linear.offsets[1]];
> +	uint8_t *y = &blit->base.linear.map[blit-
> >base.linear.fb.offsets[0]];
> +	uint8_t *uv = &blit->base.linear.map[blit-
> >base.linear.fb.offsets[1]];
>  	const uint8_t *rgb24 = blit->rgb24.map;
>  	unsigned rgb24_stride = blit->rgb24.stride;
> -	unsigned planar_stride = blit->base.linear.stride;
> +	unsigned planar_stride = blit->base.linear.fb.strides[0];
>  	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(fb-
> >color_encoding,
>  						    fb-
> >color_range);
>  
> @@ -1662,8 +1648,8 @@ static void convert_yuyv_to_rgb24(struct igt_fb
> *fb, struct fb_convert_blit_uplo
>  	int i, j;
>  	const uint8_t *yuyv;
>  	uint8_t *rgb24 = blit->rgb24.map;
> -	unsigned rgb24_stride = blit->rgb24.stride, yuyv_stride =
> blit->base.linear.stride;
> -	uint8_t *buf = malloc(blit->base.linear.size);
> +	unsigned rgb24_stride = blit->rgb24.stride, yuyv_stride =
> blit->base.linear.fb.strides[0];
> +	uint8_t *buf = malloc(blit->base.linear.fb.size);
>  	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(fb-
> >color_encoding,
>  						    fb-
> >color_range);
>  
> @@ -1672,7 +1658,7 @@ static void convert_yuyv_to_rgb24(struct igt_fb
> *fb, struct fb_convert_blit_uplo
>  	 * it's faster to copy the whole BO to a temporary buffer
> and convert
>  	 * from there.
>  	 */
> -	igt_memcpy_from_wc(buf, blit->base.linear.map, blit-
> >base.linear.size);
> +	igt_memcpy_from_wc(buf, blit->base.linear.map, blit-
> >base.linear.fb.size);
>  	yuyv = buf;
>  
>  	for (i = 0; i < fb->height; i++) {
> @@ -1722,7 +1708,7 @@ static void convert_rgb24_to_yuyv(struct igt_fb
> *fb, struct fb_convert_blit_uplo
>  	uint8_t *yuyv = blit->base.linear.map;
>  	const uint8_t *rgb24 = blit->rgb24.map;
>  	unsigned rgb24_stride = blit->rgb24.stride;
> -	unsigned yuyv_stride = blit->base.linear.stride;
> +	unsigned yuyv_stride = blit->base.linear.fb.strides[0];
>  	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(fb-
> >color_encoding,
>  						    fb-
> >color_range);
>  
> @@ -1791,7 +1777,7 @@ static void destroy_cairo_surface__convert(void
> *arg)
>  
>  	munmap(blit->rgb24.map, blit->rgb24.size);
>  
> -	if (blit->base.linear.handle)
> +	if (blit->base.linear.fb.gem_handle)
>  		free_linear_mapping(&blit->base);
>  	else
>  		gem_munmap(blit->base.linear.map, fb->size);
> @@ -1817,15 +1803,15 @@ static void create_cairo_surface__convert(int
> fd, struct igt_fb *fb)
>  	    fb->tiling == LOCAL_I915_FORMAT_MOD_Yf_TILED) {
>  		setup_linear_mapping(fd, fb, &blit->base.linear);
>  	} else {
> -		blit->base.linear.handle = 0;
> +		blit->base.linear.fb.gem_handle = 0;
>  		gem_set_domain(fd, fb->gem_handle,
>  			       I915_GEM_DOMAIN_GTT,
> I915_GEM_DOMAIN_GTT);
>  		blit->base.linear.map = gem_mmap__gtt(fd, fb-
> >gem_handle, fb->size,
>  						      PROT_READ |
> PROT_WRITE);
>  		igt_assert(blit->base.linear.map);
> -		blit->base.linear.stride = fb->stride;
> -		blit->base.linear.size = fb->size;
> -		memcpy(blit->base.linear.offsets, fb->offsets,
> sizeof(fb->offsets));
> +		blit->base.linear.fb.size = fb->size;
> +		memcpy(blit->base.linear.fb.strides, fb->strides,
> sizeof(fb->strides));
> +		memcpy(blit->base.linear.fb.offsets, fb->offsets,
> sizeof(fb->offsets));
>  	}
>  
>  	/* Convert to linear rgb! */
> diff --git a/lib/igt_fb.h b/lib/igt_fb.h
> index 2343fe505f28..35bf307a930b 100644
> --- a/lib/igt_fb.h
> +++ b/lib/igt_fb.h
> @@ -47,12 +47,12 @@
>   * @drm_format: DRM FOURCC code
>   * @width: width in pixels
>   * @height: height in pixels
> - * @stride: line stride in bytes
>   * @tiling: tiling mode as a DRM framebuffer modifier
>   * @size: size in bytes of the underlying backing storage
>   * @cairo_surface: optionally attached cairo drawing surface
>   * @domain: current domain for cache flushing tracking on i915.ko
>   * @num_planes: Amount of planes on this fb. >1 for planar formats.
> + * @strides: line stride for each plane in bytes
>   * @offsets: Offset for each plane in bytes.
>   * @plane_bpp: The bpp for each plane.
>   * @plane_width: The width for each plane.
> @@ -70,12 +70,12 @@ typedef struct igt_fb {
>  	int height;
>  	enum igt_color_encoding color_encoding;
>  	enum igt_color_range color_range;
> -	unsigned int stride;
>  	uint64_t tiling;
>  	uint64_t size;
>  	cairo_surface_t *cairo_surface;
>  	unsigned int domain;
>  	unsigned int num_planes;
> +	uint32_t strides[4];
>  	uint32_t offsets[4];
>  	unsigned int plane_bpp[4];
>  	unsigned int plane_width[4];
> diff --git a/tests/kms_ccs.c b/tests/kms_ccs.c
> index e1ee58801ac3..e03f947eae11 100644
> --- a/tests/kms_ccs.c
> +++ b/tests/kms_ccs.c
> @@ -378,7 +378,7 @@ static void generate_fb(data_t *data, struct
> igt_fb *fb,
>  	fb->drm_format = f.pixel_format;
>  	fb->width = f.width;
>  	fb->height = f.height;
> -	fb->stride = f.pitches[0];
> +	fb->strides[0] = f.pitches[0];
>  	fb->tiling = f.modifier[0];
>  	fb->size = size[0];
>  	fb->cairo_surface = NULL;
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index 393d690ab535..f7d08a60aeea 100644
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -592,7 +592,7 @@ static void recreate_fb(struct test_output *o)
>  	igt_assert(r);
>  
>  	do_or_die(drmModeAddFB(drm_fd, o->fb_width, o->fb_height, o-
> >depth,
> -			       o->bpp, fb_info->stride,
> +			       o->bpp, fb_info->strides[0],
>  			       r->handle, &new_fb_id));
>  
>  	gem_close(drm_fd, r->handle);
> @@ -612,7 +612,7 @@ static void set_y_tiling(struct test_output *o,
> int fb_idx)
>  	r = drmModeGetFB(drm_fd, fb_info->fb_id);
>  	igt_assert(r);
>  	/* Newer kernels don't allow such shenagians any more, so
> skip the test. */
> -	igt_require(__gem_set_tiling(drm_fd, r->handle,
> I915_TILING_Y, fb_info->stride) == 0);
> +	igt_require(__gem_set_tiling(drm_fd, r->handle,
> I915_TILING_Y, fb_info->strides[0]) == 0);
>  	gem_close(drm_fd, r->handle);
>  	drmFree(r);
>  }
> diff --git a/tests/kms_frontbuffer_tracking.c
> b/tests/kms_frontbuffer_tracking.c
> index 1bce676081b4..265c313a3886 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -1157,7 +1157,7 @@ static void start_busy_thread(struct igt_fb
> *fb)
>  	busy_thread.stop = false;
>  	busy_thread.handle = fb->gem_handle;
>  	busy_thread.size = fb->size;
> -	busy_thread.stride = fb->stride;
> +	busy_thread.stride = fb->strides[0];
>  	busy_thread.width = fb->width;
>  	busy_thread.height = fb->height;
>  	busy_thread.color = pick_color(fb, COLOR_PRIM_BG);
> @@ -2859,7 +2859,7 @@ static void badstride_subtest(const struct
> test_mode *t)
>  
>  	create_fb(t->format, params->primary.fb->width + 4096,
> params->primary.fb->height,
>  		  opt.tiling, t->plane, &wide_fb);
> -	igt_assert(wide_fb.stride > 16384);
> +	igt_assert(wide_fb.strides[0] > 16384);
>  
>  	fill_fb(&wide_fb, COLOR_PRIM_BG);
>  
> @@ -2928,7 +2928,7 @@ static void stridechange_subtest(const struct
> test_mode *t)
>  		  opt.tiling, t->plane, &new_fb);
>  	fill_fb(&new_fb, COLOR_PRIM_BG);
>  
> -	igt_assert(old_fb->stride != new_fb.stride);
> +	igt_assert(old_fb->strides[0] != new_fb.strides[0]);
>  
>  	/* We can't assert that FBC will be enabled since there may
> not be
>  	 * enough space for the CFB, but we can check the CRC. */
> diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
> index c24fd95bb0f3..2d6c5b49c9d5 100644
> --- a/tests/pm_rpm.c
> +++ b/tests/pm_rpm.c
> @@ -1547,7 +1547,7 @@ static void cursor_subtest(bool dpms)
>  	 * hopefully it has some fences around it. */
>  	rc = drmModeRmFB(drm_fd, cursor_fb3.fb_id);
>  	igt_assert_eq(rc, 0);
> -	gem_set_tiling(drm_fd, cursor_fb3.gem_handle, false,
> cursor_fb3.stride);
> +	gem_set_tiling(drm_fd, cursor_fb3.gem_handle, false,
> cursor_fb3.strides[0]);
>  	igt_assert(wait_for_suspended());
>  
>  	rc = drmModeSetCursor(drm_fd, crtc_id,
> cursor_fb3.gem_handle,
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/5] lib/igt_fb: Pass around igt_fb internally

[Ville Syrjälä]

On Tue, Sep 25, 2018 at 05:49:10PM -0700, Paulo Zanoni wrote:
> Em Ter, 2018-09-25 às 16:47 +0300, Ville Syrjala escreveu:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Instead of passing around a boatload of integers everywhere let's
> > just pass around the igt_fb struct. That obviously means we have to
> > populate it first sufficiently, to which end we'll add a small
> > helper.
> > Later on the stride/size calculations will consult the already
> > pre-populated igt_fb and fill in the rest as needed.
> > 
> > This makes the whole thing a lot less error prone as it's impossible
> > to accidentally pass the arguments in the wrong order when there's
> > just the one of them, and it's a pointer.
> 
> While I completely agree with these arguments, this change does not
> come without its own minor downsides. Code that deals-with/initializes
> igt_fb_t is now a little riskier due to the the fact that the order
> which things are initialized is way more important now. And we also
> moved a whole layer of our abstractions up, but that didn't seem to be
> a problem now so let's hope it still won't be a problem later.
> 
> Still, a positive change even when downsides are considered.
> 
> > 
> > v2: Rebase due to uint64_t size
> > 
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  lib/igt_draw.c                   |   2 +-
> >  lib/igt_fb.c                     | 408 +++++++++++++++++++--------
> > ------------
> >  lib/igt_fb.h                     |   4 +-
> >  tests/kms_ccs.c                  |   2 +-
> >  tests/kms_flip.c                 |   4 +-
> >  tests/kms_frontbuffer_tracking.c |   6 +-
> >  tests/pm_rpm.c                   |   2 +-
> >  7 files changed, 207 insertions(+), 221 deletions(-)
> > 
> > diff --git a/lib/igt_draw.c b/lib/igt_draw.c
> > index c7d5770dca28..05821480bc80 100644
> > --- a/lib/igt_draw.c
> > +++ b/lib/igt_draw.c
> > @@ -720,7 +720,7 @@ void igt_draw_rect_fb(int fd, drm_intel_bufmgr
> > *bufmgr,
> >  		      enum igt_draw_method method, int rect_x, int
> > rect_y,
> >  		      int rect_w, int rect_h, uint32_t color)
> >  {
> > -	igt_draw_rect(fd, bufmgr, context, fb->gem_handle, fb->size, 
> > fb->stride,
> > +	igt_draw_rect(fd, bufmgr, context, fb->gem_handle, fb->size, 
> > fb->strides[0],
> >  		      method, rect_x, rect_y, rect_w, rect_h, color,
> >  		      igt_drm_format_to_bpp(fb->drm_format));
> >  }
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 3c6974503313..26019f0420dc 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -190,50 +190,72 @@ void igt_get_fb_tile_size(int fd, uint64_t
> > tiling, int fb_bpp,
> >  	}
> >  }
> >  
> > -static unsigned fb_plane_width(const struct format_desc_struct
> > *format,
> > -			       int plane, unsigned width)
> > +static unsigned fb_plane_width(const struct igt_fb *fb, int plane)
> >  {
> > -	if (format->drm_id == DRM_FORMAT_NV12 && plane == 1)
> > -		return DIV_ROUND_UP(width, 2);
> > +	if (fb->drm_format == DRM_FORMAT_NV12 && plane == 1)
> > +		return DIV_ROUND_UP(fb->width, 2);
> >  
> > -	return width;
> > +	return fb->width;
> >  }
> >  
> > -static unsigned fb_plane_bpp(const struct format_desc_struct
> > *format, int plane)
> > +static unsigned fb_plane_bpp(const struct igt_fb *fb, int plane)
> >  {
> > +	const struct format_desc_struct *format =
> > lookup_drm_format(fb->drm_format);
> > +
> >  	return format->plane_bpp[plane];
> >  }
> >  
> > -static unsigned fb_plane_min_stride(const struct format_desc_struct
> > *format,
> > -				    int plane, unsigned width)
> > +static unsigned fb_plane_height(const struct igt_fb *fb, int plane)
> >  {
> > -	unsigned cpp = fb_plane_bpp(format, plane) / 8;
> > +	if (fb->drm_format == DRM_FORMAT_NV12 && plane == 1)
> > +		return DIV_ROUND_UP(fb->height, 2);
> >  
> > -	return fb_plane_width(format, width, plane) * cpp;
> > +	return fb->height;
> >  }
> >  
> > -static unsigned fb_plane_height(const struct format_desc_struct
> > *format,
> > -				int plane, unsigned height)
> > +static int fb_num_planes(const struct igt_fb *fb)
> >  {
> > -	if (format->drm_id == DRM_FORMAT_NV12 && plane == 1)
> > -		return DIV_ROUND_UP(height, 2);
> > +	const struct format_desc_struct *format =
> > lookup_drm_format(fb->drm_format);
> >  
> > -	return height;
> > -}
> > -
> > -static int fb_num_planes(const struct format_desc_struct *format)
> > -{
> >  	return format->num_planes;
> >  }
> >  
> > -static unsigned calc_plane_stride(int fd,
> > -				  const struct format_desc_struct
> > *format,
> > -				  int width, uint64_t tiling, int
> > plane)
> > +static void fb_init(struct igt_fb *fb,
> > +		    int fd, int width, int height,
> > +		    uint32_t drm_format,
> > +		    uint64_t modifier,
> > +		    enum igt_color_encoding color_encoding,
> > +		    enum igt_color_range color_range)
> >  {
> > -	uint32_t min_stride = fb_plane_min_stride(format, width,
> > plane);
> > +	const struct format_desc_struct *f =
> > lookup_drm_format(drm_format);
> >  
> > -	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> > -	    intel_gen(intel_get_drm_devid(fd)) <= 3) {
> > +	igt_assert_f(f, "DRM format %08x not found\n", drm_format);
> > +
> > +	memset(fb, 0, sizeof(*fb));
> > +
> > +	fb->width = width;
> > +	fb->height = height;
> > +	fb->tiling = modifier;
> 
> Shouldn't we rename fb->tiling now? (of course, in a separate patch)

Yeah, would be good to unconfused the whole tiling vs. modifier
thing in this code.

> 
> Everything else looks correct.
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Thanks.

> 
> > +	fb->drm_format = drm_format;
> > +	fb->fd = fd;
> > +	fb->num_planes = fb_num_planes(fb);
> > +	fb->color_encoding = color_encoding;
> > +	fb->color_range = color_range;
> > +
> > +	for (int i = 0; i < fb->num_planes; i++) {
> > +		fb->plane_bpp[i] = fb_plane_bpp(fb, i);
> > +		fb->plane_height[i] = fb_plane_height(fb, i);
> > +		fb->plane_width[i] = fb_plane_width(fb, i);
> > +	}
> > +}
> > +
> > +static uint32_t calc_plane_stride(struct igt_fb *fb, int plane)
> > +{
> > +	uint32_t min_stride = fb->plane_width[plane] *
> > +		(fb->plane_bpp[plane] / 8);
> > +
> > +	if (fb->tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> > +	    intel_gen(intel_get_drm_devid(fb->fd)) <= 3) {
> >  		uint32_t stride;
> >  
> >  		/* Round the tiling up to the next power-of-two and
> > the region
> > @@ -251,22 +273,19 @@ static unsigned calc_plane_stride(int fd,
> >  	} else {
> >  		unsigned int tile_width, tile_height;
> >  
> > -		igt_get_fb_tile_size(fd, tiling,
> > -				     fb_plane_bpp(format, plane),
> > +		igt_get_fb_tile_size(fb->fd, fb->tiling, fb-
> > >plane_bpp[plane],
> >  				     &tile_width, &tile_height);
> >  
> >  		return ALIGN(min_stride, tile_width);
> >  	}
> >  }
> >  
> > -static uint64_t calc_plane_size(int fd, int width, int height,
> > -				const struct format_desc_struct
> > *format,
> > -				uint64_t tiling, int plane,
> > -				uint32_t stride)
> > +static uint64_t calc_plane_size(struct igt_fb *fb, int plane)
> >  {
> > -	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> > -	    intel_gen(intel_get_drm_devid(fd)) <= 3) {
> > -		uint64_t min_size = (uint64_t) stride * height;
> > +	if (fb->tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> > +	    intel_gen(intel_get_drm_devid(fb->fd)) <= 3) {
> > +		uint64_t min_size = (uint64_t) fb->strides[plane] *
> > +			fb->plane_height[plane];
> >  		uint64_t size;
> >  
> >  		/* Round the tiling up to the next power-of-two and
> > the region
> > @@ -284,38 +303,27 @@ static uint64_t calc_plane_size(int fd, int
> > width, int height,
> >  	} else {
> >  		unsigned int tile_width, tile_height;
> >  
> > -		igt_get_fb_tile_size(fd, tiling,
> > fb_plane_bpp(format, plane),
> > +		igt_get_fb_tile_size(fb->fd, fb->tiling, fb-
> > >plane_bpp[plane],
> >  				     &tile_width, &tile_height);
> >  
> > -		return (uint64_t) stride * ALIGN(height,
> > tile_height);
> > +		return (uint64_t) fb->strides[plane] *
> > +			ALIGN(fb->plane_height[plane], tile_height);
> >  	}
> >  }
> >  
> > -static uint64_t calc_fb_size(int fd, int width, int height,
> > -			     const struct format_desc_struct
> > *format,
> > -			     uint64_t tiling,
> > -			     uint32_t strides[4], uint32_t
> > offsets[4])
> > +static uint64_t calc_fb_size(struct igt_fb *fb)
> >  {
> >  	uint64_t size = 0;
> >  	int plane;
> >  
> > -	for (plane = 0; plane < fb_num_planes(format); plane++) {
> > -		if (!strides[plane])
> > -			strides[plane] = calc_plane_stride(fd,
> > format,
> > -							   width,
> > tiling, plane);
> > +	for (plane = 0; plane < fb->num_planes; plane++) {
> > +		/* respect the stride requested by the caller */
> > +		if (!fb->strides[plane])
> > +			fb->strides[plane] = calc_plane_stride(fb,
> > plane);
> >  
> > -		if (offsets)
> > -			offsets[plane] = size;
> > +		fb->offsets[plane] = size;
> >  
> > -		size += calc_plane_size(fd, width, height,
> > -					format, tiling, plane,
> > -					strides[plane]);
> > -	}
> > -
> > -	for (; plane < ARRAY_SIZE(format->plane_bpp); plane++) {
> > -		strides[plane] = 0;
> > -		if (offsets)
> > -			offsets[plane] = 0;
> > +		size += calc_plane_size(fb, plane);
> >  	}
> >  
> >  	return size;
> > @@ -337,13 +345,17 @@ static uint64_t calc_fb_size(int fd, int width,
> > int height,
> >  void igt_calc_fb_size(int fd, int width, int height, uint32_t
> > drm_format, uint64_t tiling,
> >  		      uint64_t *size_ret, unsigned *stride_ret)
> >  {
> > -	const struct format_desc_struct *format =
> > lookup_drm_format(drm_format);
> > -	uint32_t strides[4] = {};
> > +	struct igt_fb fb;
> >  
> > -	igt_assert(format);
> > +	fb_init(&fb, fd, width, height, drm_format, tiling,
> > +		IGT_COLOR_YCBCR_BT709,
> > IGT_COLOR_YCBCR_LIMITED_RANGE);
> >  
> > -	*size_ret = calc_fb_size(fd, width, height, format, tiling,
> > strides, NULL);
> > -	*stride_ret = strides[0];
> > +	fb.size = calc_fb_size(&fb);
> > +
> > +	if (size_ret)
> > +		*size_ret = fb.size;
> > +	if (stride_ret)
> > +		*stride_ret = fb.strides[0];
> >  }
> >  
> >  /**
> > @@ -399,80 +411,64 @@ uint64_t igt_fb_tiling_to_mod(uint64_t tiling)
> >  }
> >  
> >  /* helpers to create nice-looking framebuffers */
> > -static int create_bo_for_fb(int fd, int width, int height,
> > -			    enum igt_color_encoding color_encoding,
> > -			    enum igt_color_range color_range,
> > -			    const struct format_desc_struct *format,
> > -			    uint64_t tiling, uint64_t size, unsigned
> > stride,
> > -			    uint64_t *size_ret, unsigned
> > *stride_ret,
> > -			    uint32_t offsets[4], bool *is_dumb)
> > +static int create_bo_for_fb(struct igt_fb *fb)
> >  {
> > -	int bo;
> > +	int fd = fb->fd;
> >  
> > -	igt_assert(format);
> > +	if (fb->tiling || fb->size || fb->strides[0] ||
> > igt_format_is_yuv(fb->drm_format)) {
> > +		uint64_t size;
> >  
> > -	if (offsets)
> > -		memset(offsets, 0, ARRAY_SIZE(format->plane_bpp) *
> > sizeof(*offsets));
> > +		size = calc_fb_size(fb);
> >  
> > -	if (tiling || size || stride || igt_format_is_yuv(format-
> > >drm_id)) {
> > -		uint64_t calculated_size;
> > -		uint32_t strides[4] = {
> > -			stride,
> > -		};
> > +		/* respect the size requested by the caller */
> > +		if (fb->size == 0)
> > +			fb->size = size;
> >  
> > -		calculated_size = calc_fb_size(fd, width, height,
> > -					       format, tiling,
> > -					       strides, offsets);
> > -
> > -		if (stride == 0)
> > -			stride = strides[0];
> > -		if (size == 0)
> > -			size = calculated_size;
> > -
> > -		if (is_dumb)
> > -			*is_dumb = false;
> > +		fb->is_dumb = false;
> >  
> >  		if (is_i915_device(fd)) {
> >  			void *ptr;
> > -			bool full_range = color_range ==
> > IGT_COLOR_YCBCR_FULL_RANGE;
> > +			bool full_range = fb->color_range ==
> > IGT_COLOR_YCBCR_FULL_RANGE;
> >  
> > -			bo = gem_create(fd, size);
> > -			gem_set_tiling(fd, bo,
> > igt_fb_mod_to_tiling(tiling), stride);
> > +			fb->gem_handle = gem_create(fd, fb->size);
> >  
> > -			gem_set_domain(fd, bo,
> > +			gem_set_tiling(fd, fb->gem_handle,
> > +				       igt_fb_mod_to_tiling(fb-
> > >tiling),
> > +				       fb->strides[0]);
> > +
> > +			gem_set_domain(fd, fb->gem_handle,
> >  				       I915_GEM_DOMAIN_GTT,
> > I915_GEM_DOMAIN_GTT);
> >  
> >  			/* Ensure the framebuffer is preallocated */
> > -			ptr = gem_mmap__gtt(fd, bo, size, PROT_READ
> > | PROT_WRITE);
> > +			ptr = gem_mmap__gtt(fd, fb->gem_handle,
> > +					    fb->size, PROT_READ |
> > PROT_WRITE);
> >  			igt_assert(*(uint32_t *)ptr == 0);
> >  
> > -			switch (format->drm_id) {
> > +			switch (fb->drm_format) {
> >  			case DRM_FORMAT_NV12:
> > -				memset(ptr + offsets[0], full_range
> > ? 0x00 : 0x10,
> > -				       strides[0] * height);
> > -				memset(ptr + offsets[1], 0x80,
> > -				       strides[1] * height/2);
> > +				memset(ptr + fb->offsets[0],
> > +				       full_range ? 0x00 : 0x10,
> > +				       fb->strides[0] * fb-
> > >plane_height[0]);
> > +				memset(ptr + fb->offsets[1],
> > +				       0x80,
> > +				       fb->strides[1] * fb-
> > >plane_height[1]);
> >  				break;
> >  			case DRM_FORMAT_YUYV:
> >  			case DRM_FORMAT_YVYU:
> > -				wmemset(ptr, full_range ? 0x80008000
> > : 0x80108010,
> > -					strides[0] * height /
> > sizeof(wchar_t));
> > +				wmemset(ptr + fb->offsets[0],
> > +					full_range ? 0x80008000 :
> > 0x80108010,
> > +					fb->strides[0] * fb-
> > >plane_height[0] / sizeof(wchar_t));
> >  				break;
> >  			case DRM_FORMAT_UYVY:
> >  			case DRM_FORMAT_VYUY:
> > -				wmemset(ptr, full_range ? 0x00800080
> > : 0x10801080,
> > -					strides[0] * height /
> > sizeof(wchar_t));
> > +				wmemset(ptr + fb->offsets[0],
> > +					full_range ? 0x00800080 :
> > 0x10801080,
> > +					fb->strides[0] * fb-
> > >plane_height[0] / sizeof(wchar_t));
> >  				break;
> >  			}
> > -			gem_munmap(ptr, size);
> > +			gem_munmap(ptr, fb->size);
> >  
> > -			if (size_ret)
> > -				*size_ret = size;
> > -
> > -			if (stride_ret)
> > -				*stride_ret = strides[0];
> > -
> > -			return bo;
> > +			return fb->gem_handle;
> >  		} else {
> >  			bool driver_has_gem_api = false;
> >  
> > @@ -480,12 +476,13 @@ static int create_bo_for_fb(int fd, int width,
> > int height,
> >  			return -EINVAL;
> >  		}
> >  	} else {
> > -		if (is_dumb)
> > -			*is_dumb = true;
> > +		fb->is_dumb = true;
> >  
> > -		return kmstest_dumb_create(fd, width, height,
> > -					   fb_plane_bpp(format, 0),
> > -					   stride_ret, size_ret);
> > +		fb->gem_handle = kmstest_dumb_create(fd, fb->width,
> > fb->height,
> > +						     fb-
> > >plane_bpp[0],
> > +						     &fb-
> > >strides[0], &fb->size);
> > +
> > +		return fb->gem_handle;
> >  	}
> >  }
> >  
> > @@ -512,11 +509,24 @@ int igt_create_bo_with_dimensions(int fd, int
> > width, int height,
> >  				  unsigned stride, uint64_t
> > *size_ret,
> >  				  unsigned *stride_ret, bool
> > *is_dumb)
> >  {
> > -	return create_bo_for_fb(fd, width, height,
> > -				IGT_COLOR_YCBCR_BT709,
> > -				IGT_COLOR_YCBCR_LIMITED_RANGE,
> > -				lookup_drm_format(format),
> > -				modifier, 0, stride, size_ret,
> > stride_ret, NULL, is_dumb);
> > +	struct igt_fb fb;
> > +
> > +	fb_init(&fb, fd, width, height, format, modifier,
> > +		IGT_COLOR_YCBCR_BT709,
> > IGT_COLOR_YCBCR_LIMITED_RANGE);
> > +
> > +	for (int i = 0; i < fb.num_planes; i++)
> > +		fb.strides[i] = stride;
> > +
> > +	create_bo_for_fb(&fb);
> > +
> > +	if (size_ret)
> > +		*size_ret = fb.size;
> > +	if (stride_ret)
> > +		*stride_ret = fb.strides[0];
> > +	if (is_dumb)
> > +		*is_dumb = fb.is_dumb;
> > +
> > +	return fb.gem_handle;
> >  }
> >  
> >  /**
> > @@ -860,52 +870,32 @@ igt_create_fb_with_bo_size(int fd, int width,
> > int height,
> >  	/* FIXME allow the caller to pass these in */
> >  	enum igt_color_encoding color_encoding =
> > IGT_COLOR_YCBCR_BT709;
> >  	enum igt_color_range color_range =
> > IGT_COLOR_YCBCR_LIMITED_RANGE;
> > -	const struct format_desc_struct *f =
> > lookup_drm_format(format);
> > -	uint32_t pitches[4];
> > -	uint32_t fb_id;
> > -	int i;
> >  
> > -	igt_assert_f(f, "DRM format %08x not found\n", format);
> > +	fb_init(fb, fd, width, height, format, tiling,
> > +		color_encoding, color_range);
> >  
> > -	memset(fb, 0, sizeof(*fb));
> > +	for (int i = 0; i < fb->num_planes; i++)
> > +		fb->strides[i] = bo_stride;
> > +
> > +	fb->size = bo_size;
> >  
> >  	igt_debug("%s(width=%d, height=%d, format=0x%x,
> > tiling=0x%"PRIx64", size=%"PRIu64")\n",
> >  		  __func__, width, height, format, tiling, bo_size);
> > -	fb->gem_handle = create_bo_for_fb(fd, width, height,
> > -					  color_encoding,
> > color_range,
> > -					  f, tiling, bo_size,
> > bo_stride,
> > -					  &fb->size, &fb->stride,
> > -					  fb->offsets, &fb-
> > >is_dumb);
> > +
> > +	create_bo_for_fb(fb);
> >  	igt_assert(fb->gem_handle > 0);
> >  
> >  	igt_debug("%s(handle=%d, pitch=%d)\n",
> > -		  __func__, fb->gem_handle, fb->stride);
> > +		  __func__, fb->gem_handle, fb->strides[0]);
> >  
> > -	for (i = 0; i < fb_num_planes(f); i++)
> > -		pitches[i] = fb->stride;
> > +	do_or_die(__kms_addfb(fb->fd, fb->gem_handle,
> > +			      fb->width, fb->height,
> > +			      fb->drm_format, fb->tiling,
> > +			      fb->strides, fb->offsets, fb-
> > >num_planes,
> > +			      LOCAL_DRM_MODE_FB_MODIFIERS,
> > +			      &fb->fb_id));
> >  
> > -	do_or_die(__kms_addfb(fd, fb->gem_handle, width, height,
> > -			      format, tiling, pitches, fb->offsets,
> > -			      fb_num_planes(f),
> > -			      LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id));
> > -
> > -	fb->width = width;
> > -	fb->height = height;
> > -	fb->tiling = tiling;
> > -	fb->drm_format = format;
> > -	fb->fb_id = fb_id;
> > -	fb->fd = fd;
> > -	fb->num_planes = fb_num_planes(f);
> > -	fb->color_encoding = color_encoding;
> > -	fb->color_range = color_range;
> > -
> > -	for (i = 0; i < fb_num_planes(f); i++) {
> > -		fb->plane_bpp[i] = fb_plane_bpp(f, i);
> > -		fb->plane_height[i] = fb_plane_height(f, height, i);
> > -		fb->plane_width[i] = fb_plane_width(f, width, i);
> > -	}
> > -
> > -	return fb_id;
> > +	return fb->fb_id;
> >  }
> >  
> >  /**
> > @@ -1211,12 +1201,8 @@ static cairo_format_t
> > drm_format_to_cairo(uint32_t drm_format)
> >  }
> >  
> >  struct fb_blit_linear {
> > -	uint64_t size;
> > -	uint32_t handle;
> > -	unsigned int stride;
> > +	struct igt_fb fb;
> >  	uint8_t *map;
> > -	bool is_dumb;
> > -	uint32_t offsets[4];
> >  };
> >  
> >  struct fb_blit_upload {
> > @@ -1233,27 +1219,27 @@ static void free_linear_mapping(struct
> > fb_blit_upload *blit)
> >  	unsigned int obj_tiling = igt_fb_mod_to_tiling(fb->tiling);
> >  	int i;
> >  
> > -	gem_munmap(linear->map, linear->size);
> > -	gem_set_domain(fd, linear->handle,
> > +	gem_munmap(linear->map, linear->fb.size);
> > +	gem_set_domain(fd, linear->fb.gem_handle,
> >  		       I915_GEM_DOMAIN_GTT, 0);
> >  
> >  	for (i = 0; i < fb->num_planes; i++)
> >  		igt_blitter_fast_copy__raw(fd,
> > -					   linear->handle,
> > -					   linear->offsets[i],
> > -					   linear->stride,
> > +					   linear->fb.gem_handle,
> > +					   linear->fb.offsets[i],
> > +					   linear->fb.strides[i],
> >  					   I915_TILING_NONE,
> >  					   0, 0, /* src_x, src_y */
> >  					   fb->plane_width[i], fb-
> > >plane_height[i],
> >  					   fb->plane_bpp[i],
> >  					   fb->gem_handle,
> >  					   fb->offsets[i],
> > -					   fb->stride,
> > +					   fb->strides[i],
> >  					   obj_tiling,
> >  					   0, 0 /* dst_x, dst_y */);
> >  
> > -	gem_sync(fd, linear->handle);
> > -	gem_close(fd, linear->handle);
> > +	gem_sync(fd, linear->fb.gem_handle);
> > +	gem_close(fd, linear->fb.gem_handle);
> >  }
> >  
> >  static void destroy_cairo_surface__blit(void *arg)
> > @@ -1277,42 +1263,42 @@ static void setup_linear_mapping(int fd,
> > struct igt_fb *fb, struct fb_blit_linea
> >  	 * cairo). This linear bo will be then blitted to its final
> >  	 * destination, tiling it at the same time.
> >  	 */
> > -	linear->handle = create_bo_for_fb(fd, fb->width, fb->height,
> > -					  fb->color_encoding, fb-
> > >color_range,
> > -					  lookup_drm_format(fb-
> > >drm_format),
> > -					  LOCAL_DRM_FORMAT_MOD_NONE,
> > 0,
> > -					  0, &linear->size,
> > -					  &linear->stride,
> > -					  linear->offsets, &linear-
> > >is_dumb);
> >  
> > -	igt_assert(linear->handle > 0);
> > +	fb_init(&linear->fb, fb->fd, fb->width, fb->height,
> > +		fb->drm_format, LOCAL_DRM_FORMAT_MOD_NONE,
> > +		fb->color_encoding, fb->color_range);
> > +
> > +	create_bo_for_fb(&linear->fb);
> > +
> > +	igt_assert(linear->fb.gem_handle > 0);
> >  
> >  	/* Copy fb content to linear BO */
> > -	gem_set_domain(fd, linear->handle,
> > +	gem_set_domain(fd, linear->fb.gem_handle,
> >  			I915_GEM_DOMAIN_GTT, 0);
> >  
> >  	for (i = 0; i < fb->num_planes; i++)
> >  		igt_blitter_fast_copy__raw(fd,
> > -					  fb->gem_handle,
> > -					  fb->offsets[i],
> > -					  fb->stride,
> > -					  obj_tiling,
> > -					  0, 0, /* src_x, src_y */
> > -					  fb->plane_width[i], fb-
> > >plane_height[i],
> > -					  fb->plane_bpp[i],
> > -					  linear->handle, linear-
> > >offsets[i],
> > -					  linear->stride,
> > -					  I915_TILING_NONE,
> > -					  0, 0 /* dst_x, dst_y */);
> > +					   fb->gem_handle,
> > +					   fb->offsets[i],
> > +					   fb->strides[i],
> > +					   obj_tiling,
> > +					   0, 0, /* src_x, src_y */
> > +					   fb->plane_width[i], fb-
> > >plane_height[i],
> > +					   fb->plane_bpp[i],
> > +					   linear->fb.gem_handle,
> > +					   linear->fb.offsets[i],
> > +					   linear->fb.strides[i],
> > +					   I915_TILING_NONE,
> > +					   0, 0 /* dst_x, dst_y */);
> >  
> > -	gem_sync(fd, linear->handle);
> > +	gem_sync(fd, linear->fb.gem_handle);
> >  
> > -	gem_set_domain(fd, linear->handle,
> > +	gem_set_domain(fd, linear->fb.gem_handle,
> >  		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
> >  
> >  	/* Setup cairo context */
> > -	linear->map = gem_mmap__cpu(fd, linear->handle,
> > -				    0, linear->size, PROT_READ |
> > PROT_WRITE);
> > +	linear->map = gem_mmap__cpu(fd, linear->fb.gem_handle,
> > +				    0, linear->fb.size, PROT_READ |
> > PROT_WRITE);
> >  }
> >  
> >  static void create_cairo_surface__blit(int fd, struct igt_fb *fb)
> > @@ -1332,7 +1318,7 @@ static void create_cairo_surface__blit(int fd,
> > struct igt_fb *fb)
> >  		cairo_image_surface_create_for_data(blit-
> > >linear.map,
> >  						    cairo_format,
> >  						    fb->width, fb-
> > >height,
> > -						    blit-
> > >linear.stride);
> > +						    blit-
> > >linear.fb.strides[0]);
> >  	fb->domain = I915_GEM_DOMAIN_GTT;
> >  
> >  	cairo_surface_set_user_data(fb->cairo_surface,
> > @@ -1382,7 +1368,7 @@ static void create_cairo_surface__gtt(int fd,
> > struct igt_fb *fb)
> >  	fb->cairo_surface =
> >  		cairo_image_surface_create_for_data(ptr,
> >  						    drm_format_to_ca
> > iro(fb->drm_format),
> > -						    fb->width, fb-
> > >height, fb->stride);
> > +						    fb->width, fb-
> > >height, fb->strides[0]);
> >  	fb->domain = I915_GEM_DOMAIN_GTT;
> >  
> >  	cairo_surface_set_user_data(fb->cairo_surface,
> > @@ -1425,8 +1411,8 @@ static void convert_nv12_to_rgb24(struct igt_fb
> > *fb, struct fb_convert_blit_uplo
> >  	int i, j;
> >  	const uint8_t *y, *uv;
> >  	uint8_t *rgb24 = blit->rgb24.map;
> > -	unsigned rgb24_stride = blit->rgb24.stride, planar_stride =
> > blit->base.linear.stride;
> > -	uint8_t *buf = malloc(blit->base.linear.size);
> > +	unsigned rgb24_stride = blit->rgb24.stride, planar_stride =
> > blit->base.linear.fb.strides[0];
> > +	uint8_t *buf = malloc(blit->base.linear.fb.size);
> >  	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(fb-
> > >color_encoding,
> >  						    fb-
> > >color_range);
> >  
> > @@ -1435,9 +1421,9 @@ static void convert_nv12_to_rgb24(struct igt_fb
> > *fb, struct fb_convert_blit_uplo
> >  	 * it's faster to copy the whole BO to a temporary buffer
> > and convert
> >  	 * from there.
> >  	 */
> > -	igt_memcpy_from_wc(buf, blit->base.linear.map, blit-
> > >base.linear.size);
> > -	y = &buf[blit->base.linear.offsets[0]];
> > -	uv = &buf[blit->base.linear.offsets[1]];
> > +	igt_memcpy_from_wc(buf, blit->base.linear.map, blit-
> > >base.linear.fb.size);
> > +	y = &buf[blit->base.linear.fb.offsets[0]];
> > +	uv = &buf[blit->base.linear.fb.offsets[1]];
> >  
> >  	for (i = 0; i < fb->height / 2; i++) {
> >  		for (j = 0; j < fb->width / 2; j++) {
> > @@ -1531,11 +1517,11 @@ static void convert_nv12_to_rgb24(struct
> > igt_fb *fb, struct fb_convert_blit_uplo
> >  static void convert_rgb24_to_nv12(struct igt_fb *fb, struct
> > fb_convert_blit_upload *blit)
> >  {
> >  	int i, j;
> > -	uint8_t *y = &blit->base.linear.map[blit-
> > >base.linear.offsets[0]];
> > -	uint8_t *uv = &blit->base.linear.map[blit-
> > >base.linear.offsets[1]];
> > +	uint8_t *y = &blit->base.linear.map[blit-
> > >base.linear.fb.offsets[0]];
> > +	uint8_t *uv = &blit->base.linear.map[blit-
> > >base.linear.fb.offsets[1]];
> >  	const uint8_t *rgb24 = blit->rgb24.map;
> >  	unsigned rgb24_stride = blit->rgb24.stride;
> > -	unsigned planar_stride = blit->base.linear.stride;
> > +	unsigned planar_stride = blit->base.linear.fb.strides[0];
> >  	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(fb-
> > >color_encoding,
> >  						    fb-
> > >color_range);
> >  
> > @@ -1662,8 +1648,8 @@ static void convert_yuyv_to_rgb24(struct igt_fb
> > *fb, struct fb_convert_blit_uplo
> >  	int i, j;
> >  	const uint8_t *yuyv;
> >  	uint8_t *rgb24 = blit->rgb24.map;
> > -	unsigned rgb24_stride = blit->rgb24.stride, yuyv_stride =
> > blit->base.linear.stride;
> > -	uint8_t *buf = malloc(blit->base.linear.size);
> > +	unsigned rgb24_stride = blit->rgb24.stride, yuyv_stride =
> > blit->base.linear.fb.strides[0];
> > +	uint8_t *buf = malloc(blit->base.linear.fb.size);
> >  	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(fb-
> > >color_encoding,
> >  						    fb-
> > >color_range);
> >  
> > @@ -1672,7 +1658,7 @@ static void convert_yuyv_to_rgb24(struct igt_fb
> > *fb, struct fb_convert_blit_uplo
> >  	 * it's faster to copy the whole BO to a temporary buffer
> > and convert
> >  	 * from there.
> >  	 */
> > -	igt_memcpy_from_wc(buf, blit->base.linear.map, blit-
> > >base.linear.size);
> > +	igt_memcpy_from_wc(buf, blit->base.linear.map, blit-
> > >base.linear.fb.size);
> >  	yuyv = buf;
> >  
> >  	for (i = 0; i < fb->height; i++) {
> > @@ -1722,7 +1708,7 @@ static void convert_rgb24_to_yuyv(struct igt_fb
> > *fb, struct fb_convert_blit_uplo
> >  	uint8_t *yuyv = blit->base.linear.map;
> >  	const uint8_t *rgb24 = blit->rgb24.map;
> >  	unsigned rgb24_stride = blit->rgb24.stride;
> > -	unsigned yuyv_stride = blit->base.linear.stride;
> > +	unsigned yuyv_stride = blit->base.linear.fb.strides[0];
> >  	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(fb-
> > >color_encoding,
> >  						    fb-
> > >color_range);
> >  
> > @@ -1791,7 +1777,7 @@ static void destroy_cairo_surface__convert(void
> > *arg)
> >  
> >  	munmap(blit->rgb24.map, blit->rgb24.size);
> >  
> > -	if (blit->base.linear.handle)
> > +	if (blit->base.linear.fb.gem_handle)
> >  		free_linear_mapping(&blit->base);
> >  	else
> >  		gem_munmap(blit->base.linear.map, fb->size);
> > @@ -1817,15 +1803,15 @@ static void create_cairo_surface__convert(int
> > fd, struct igt_fb *fb)
> >  	    fb->tiling == LOCAL_I915_FORMAT_MOD_Yf_TILED) {
> >  		setup_linear_mapping(fd, fb, &blit->base.linear);
> >  	} else {
> > -		blit->base.linear.handle = 0;
> > +		blit->base.linear.fb.gem_handle = 0;
> >  		gem_set_domain(fd, fb->gem_handle,
> >  			       I915_GEM_DOMAIN_GTT,
> > I915_GEM_DOMAIN_GTT);
> >  		blit->base.linear.map = gem_mmap__gtt(fd, fb-
> > >gem_handle, fb->size,
> >  						      PROT_READ |
> > PROT_WRITE);
> >  		igt_assert(blit->base.linear.map);
> > -		blit->base.linear.stride = fb->stride;
> > -		blit->base.linear.size = fb->size;
> > -		memcpy(blit->base.linear.offsets, fb->offsets,
> > sizeof(fb->offsets));
> > +		blit->base.linear.fb.size = fb->size;
> > +		memcpy(blit->base.linear.fb.strides, fb->strides,
> > sizeof(fb->strides));
> > +		memcpy(blit->base.linear.fb.offsets, fb->offsets,
> > sizeof(fb->offsets));
> >  	}
> >  
> >  	/* Convert to linear rgb! */
> > diff --git a/lib/igt_fb.h b/lib/igt_fb.h
> > index 2343fe505f28..35bf307a930b 100644
> > --- a/lib/igt_fb.h
> > +++ b/lib/igt_fb.h
> > @@ -47,12 +47,12 @@
> >   * @drm_format: DRM FOURCC code
> >   * @width: width in pixels
> >   * @height: height in pixels
> > - * @stride: line stride in bytes
> >   * @tiling: tiling mode as a DRM framebuffer modifier
> >   * @size: size in bytes of the underlying backing storage
> >   * @cairo_surface: optionally attached cairo drawing surface
> >   * @domain: current domain for cache flushing tracking on i915.ko
> >   * @num_planes: Amount of planes on this fb. >1 for planar formats.
> > + * @strides: line stride for each plane in bytes
> >   * @offsets: Offset for each plane in bytes.
> >   * @plane_bpp: The bpp for each plane.
> >   * @plane_width: The width for each plane.
> > @@ -70,12 +70,12 @@ typedef struct igt_fb {
> >  	int height;
> >  	enum igt_color_encoding color_encoding;
> >  	enum igt_color_range color_range;
> > -	unsigned int stride;
> >  	uint64_t tiling;
> >  	uint64_t size;
> >  	cairo_surface_t *cairo_surface;
> >  	unsigned int domain;
> >  	unsigned int num_planes;
> > +	uint32_t strides[4];
> >  	uint32_t offsets[4];
> >  	unsigned int plane_bpp[4];
> >  	unsigned int plane_width[4];
> > diff --git a/tests/kms_ccs.c b/tests/kms_ccs.c
> > index e1ee58801ac3..e03f947eae11 100644
> > --- a/tests/kms_ccs.c
> > +++ b/tests/kms_ccs.c
> > @@ -378,7 +378,7 @@ static void generate_fb(data_t *data, struct
> > igt_fb *fb,
> >  	fb->drm_format = f.pixel_format;
> >  	fb->width = f.width;
> >  	fb->height = f.height;
> > -	fb->stride = f.pitches[0];
> > +	fb->strides[0] = f.pitches[0];
> >  	fb->tiling = f.modifier[0];
> >  	fb->size = size[0];
> >  	fb->cairo_surface = NULL;
> > diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> > index 393d690ab535..f7d08a60aeea 100644
> > --- a/tests/kms_flip.c
> > +++ b/tests/kms_flip.c
> > @@ -592,7 +592,7 @@ static void recreate_fb(struct test_output *o)
> >  	igt_assert(r);
> >  
> >  	do_or_die(drmModeAddFB(drm_fd, o->fb_width, o->fb_height, o-
> > >depth,
> > -			       o->bpp, fb_info->stride,
> > +			       o->bpp, fb_info->strides[0],
> >  			       r->handle, &new_fb_id));
> >  
> >  	gem_close(drm_fd, r->handle);
> > @@ -612,7 +612,7 @@ static void set_y_tiling(struct test_output *o,
> > int fb_idx)
> >  	r = drmModeGetFB(drm_fd, fb_info->fb_id);
> >  	igt_assert(r);
> >  	/* Newer kernels don't allow such shenagians any more, so
> > skip the test. */
> > -	igt_require(__gem_set_tiling(drm_fd, r->handle,
> > I915_TILING_Y, fb_info->stride) == 0);
> > +	igt_require(__gem_set_tiling(drm_fd, r->handle,
> > I915_TILING_Y, fb_info->strides[0]) == 0);
> >  	gem_close(drm_fd, r->handle);
> >  	drmFree(r);
> >  }
> > diff --git a/tests/kms_frontbuffer_tracking.c
> > b/tests/kms_frontbuffer_tracking.c
> > index 1bce676081b4..265c313a3886 100644
> > --- a/tests/kms_frontbuffer_tracking.c
> > +++ b/tests/kms_frontbuffer_tracking.c
> > @@ -1157,7 +1157,7 @@ static void start_busy_thread(struct igt_fb
> > *fb)
> >  	busy_thread.stop = false;
> >  	busy_thread.handle = fb->gem_handle;
> >  	busy_thread.size = fb->size;
> > -	busy_thread.stride = fb->stride;
> > +	busy_thread.stride = fb->strides[0];
> >  	busy_thread.width = fb->width;
> >  	busy_thread.height = fb->height;
> >  	busy_thread.color = pick_color(fb, COLOR_PRIM_BG);
> > @@ -2859,7 +2859,7 @@ static void badstride_subtest(const struct
> > test_mode *t)
> >  
> >  	create_fb(t->format, params->primary.fb->width + 4096,
> > params->primary.fb->height,
> >  		  opt.tiling, t->plane, &wide_fb);
> > -	igt_assert(wide_fb.stride > 16384);
> > +	igt_assert(wide_fb.strides[0] > 16384);
> >  
> >  	fill_fb(&wide_fb, COLOR_PRIM_BG);
> >  
> > @@ -2928,7 +2928,7 @@ static void stridechange_subtest(const struct
> > test_mode *t)
> >  		  opt.tiling, t->plane, &new_fb);
> >  	fill_fb(&new_fb, COLOR_PRIM_BG);
> >  
> > -	igt_assert(old_fb->stride != new_fb.stride);
> > +	igt_assert(old_fb->strides[0] != new_fb.strides[0]);
> >  
> >  	/* We can't assert that FBC will be enabled since there may
> > not be
> >  	 * enough space for the CFB, but we can check the CRC. */
> > diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
> > index c24fd95bb0f3..2d6c5b49c9d5 100644
> > --- a/tests/pm_rpm.c
> > +++ b/tests/pm_rpm.c
> > @@ -1547,7 +1547,7 @@ static void cursor_subtest(bool dpms)
> >  	 * hopefully it has some fences around it. */
> >  	rc = drmModeRmFB(drm_fd, cursor_fb3.fb_id);
> >  	igt_assert_eq(rc, 0);
> > -	gem_set_tiling(drm_fd, cursor_fb3.gem_handle, false,
> > cursor_fb3.stride);
> > +	gem_set_tiling(drm_fd, cursor_fb3.gem_handle, false,
> > cursor_fb3.strides[0]);
> >  	igt_assert(wait_for_suspended());
> >  
> >  	rc = drmModeSetCursor(drm_fd, crtc_id,
> > cursor_fb3.gem_handle,

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

[igt-dev] [PATCH i-g-t 5/5] lib/igt_fb: Refactor blitter usage

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Deduplicate the blitter code.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 lib/igt_fb.c | 62 ++++++++++++++++++++++++++++--------------------------------
 1 file changed, 29 insertions(+), 33 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 26019f0420dc..cba67f41bcc1 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1211,32 +1211,44 @@ struct fb_blit_upload {
 	struct fb_blit_linear linear;
 };
 
+static void blitcopy(const struct igt_fb *dst_fb,
+		     const struct igt_fb *src_fb)
+{
+	igt_assert_eq(dst_fb->fd, src_fb->fd);
+	igt_assert_eq(dst_fb->num_planes, src_fb->num_planes);
+
+	for (int i = 0; i < dst_fb->num_planes; i++) {
+		igt_assert_eq(dst_fb->plane_bpp[i], src_fb->plane_bpp[i]);
+		igt_assert_eq(dst_fb->plane_width[i], src_fb->plane_width[i]);
+		igt_assert_eq(dst_fb->plane_height[i], src_fb->plane_height[i]);
+
+		igt_blitter_fast_copy__raw(dst_fb->fd,
+					   src_fb->gem_handle,
+					   src_fb->offsets[i],
+					   src_fb->strides[i],
+					   igt_fb_mod_to_tiling(src_fb->tiling),
+					   0, 0, /* src_x, src_y */
+					   dst_fb->plane_width[i], dst_fb->plane_height[i],
+					   dst_fb->plane_bpp[i],
+					   dst_fb->gem_handle,
+					   dst_fb->offsets[i],
+					   dst_fb->strides[i],
+					   igt_fb_mod_to_tiling(dst_fb->tiling),
+					   0, 0 /* dst_x, dst_y */);
+	}
+}
+
 static void free_linear_mapping(struct fb_blit_upload *blit)
 {
 	int fd = blit->fd;
 	struct igt_fb *fb = blit->fb;
 	struct fb_blit_linear *linear = &blit->linear;
-	unsigned int obj_tiling = igt_fb_mod_to_tiling(fb->tiling);
-	int i;
 
 	gem_munmap(linear->map, linear->fb.size);
 	gem_set_domain(fd, linear->fb.gem_handle,
 		       I915_GEM_DOMAIN_GTT, 0);
 
-	for (i = 0; i < fb->num_planes; i++)
-		igt_blitter_fast_copy__raw(fd,
-					   linear->fb.gem_handle,
-					   linear->fb.offsets[i],
-					   linear->fb.strides[i],
-					   I915_TILING_NONE,
-					   0, 0, /* src_x, src_y */
-					   fb->plane_width[i], fb->plane_height[i],
-					   fb->plane_bpp[i],
-					   fb->gem_handle,
-					   fb->offsets[i],
-					   fb->strides[i],
-					   obj_tiling,
-					   0, 0 /* dst_x, dst_y */);
+	blitcopy(fb, &linear->fb);
 
 	gem_sync(fd, linear->fb.gem_handle);
 	gem_close(fd, linear->fb.gem_handle);
@@ -1255,9 +1267,6 @@ static void destroy_cairo_surface__blit(void *arg)
 
 static void setup_linear_mapping(int fd, struct igt_fb *fb, struct fb_blit_linear *linear)
 {
-	unsigned int obj_tiling = igt_fb_mod_to_tiling(fb->tiling);
-	int i;
-
 	/*
 	 * We create a linear BO that we'll map for the CPU to write to (using
 	 * cairo). This linear bo will be then blitted to its final
@@ -1276,20 +1285,7 @@ static void setup_linear_mapping(int fd, struct igt_fb *fb, struct fb_blit_linea
 	gem_set_domain(fd, linear->fb.gem_handle,
 			I915_GEM_DOMAIN_GTT, 0);
 
-	for (i = 0; i < fb->num_planes; i++)
-		igt_blitter_fast_copy__raw(fd,
-					   fb->gem_handle,
-					   fb->offsets[i],
-					   fb->strides[i],
-					   obj_tiling,
-					   0, 0, /* src_x, src_y */
-					   fb->plane_width[i], fb->plane_height[i],
-					   fb->plane_bpp[i],
-					   linear->fb.gem_handle,
-					   linear->fb.offsets[i],
-					   linear->fb.strides[i],
-					   I915_TILING_NONE,
-					   0, 0 /* dst_x, dst_y */);
+	blitcopy(&linear->fb, fb);
 
 	gem_sync(fd, linear->fb.gem_handle);
 
-- 
2.16.4

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

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/5] lib/igt_fb: Extract calc_plane_stride()

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/5] lib/igt_fb: Extract calc_plane_stride()
URL   : https://patchwork.freedesktop.org/series/50148/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4874 -> IGTPW_1870 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/50148/revisions/1/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-bdw-samus:       PASS -> INCOMPLETE (fdo#107773)

    igt@kms_flip@basic-flip-vs-modeset:
      fi-skl-6700hq:      NOTRUN -> DMESG-WARN (fdo#105998)

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       PASS -> DMESG-WARN (fdo#102614)

    
    ==== Possible fixes ====

    igt@drv_module_reload@basic-reload:
      fi-blb-e6850:       INCOMPLETE (fdo#107718) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-cfl-8109u:       INCOMPLETE (fdo#106070) -> PASS

    igt@kms_psr@primary_page_flip:
      fi-cnl-u:           FAIL (fdo#107336) -> PASS

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
  fdo#106070 https://bugs.freedesktop.org/show_bug.cgi?id=106070
  fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773


== Participating hosts (48 -> 40) ==

  Additional (1): fi-skl-6700hq 
  Missing    (9): fi-ilk-m540 fi-icl-u fi-hsw-4200u fi-byt-squawks fi-icl-u2 fi-bsw-cyan fi-ctg-p8600 fi-gdg-551 fi-kbl-7560u 


== Build changes ==

    * IGT: IGT_4649 -> IGTPW_1870

  CI_DRM_4874: 7bb42432ef1cc63b4a6a223e19455f6c4f9a1538 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1870: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1870/
  IGT_4649: 19b0c74d20d9b53d4c82be14af0909a3b6846010 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

Re: [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Extract calc_plane_stride()

[Paulo Zanoni]

Em Ter, 2018-09-25 às 16:47 +0300, Ville Syrjala escreveu:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Extract the stride calculation from calc_fb_size_packed() to its own
> thing so that we can use it to calculate just the stride.
> 
> v2: Rebase due to overflow fixes and roundup_power_of_two()
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>

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

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  lib/igt_fb.c | 50 +++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 39 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 486b5d3015ad..ed49a3ede874 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -226,6 +226,39 @@ static int fb_num_planes(struct
> format_desc_struct *format)
>  	return format->num_planes;
>  }
>  
> +static unsigned calc_plane_stride(int fd,
> +				  struct format_desc_struct *format,
> +				  int width, uint64_t tiling, int
> plane)
> +{
> +	uint32_t min_stride = fb_plane_min_stride(format, width,
> plane);
> +
> +	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> +	    intel_gen(intel_get_drm_devid(fd)) <= 3) {
> +		uint32_t stride;
> +
> +		/* Round the tiling up to the next power-of-two and
> the region
> +		 * up to the next pot fence size so that this works
> on all
> +		 * generations.
> +		 *
> +		 * This can still fail if the framebuffer is too
> large to be
> +		 * tiled. But then that failure is expected.
> +		 */
> +
> +		stride = max(min_stride, 512);
> +		stride = roundup_power_of_two(stride);
> +
> +		return stride;
> +	} else {
> +		unsigned int tile_width, tile_height;
> +
> +		igt_get_fb_tile_size(fd, tiling,
> +				     fb_plane_bpp(format, plane),
> +				     &tile_width, &tile_height);
> +
> +		return ALIGN(min_stride, tile_width);
> +	}
> +}
> +
>  static void calc_fb_size_planar(int fd, int width, int height,
>  				struct format_desc_struct *format,
>  				uint64_t tiling, unsigned stride,
> @@ -272,12 +305,10 @@ static void calc_fb_size_packed(int fd, int
> width, int height,
>  				struct format_desc_struct *format,
> uint64_t tiling,
>  				unsigned stride, uint64_t *size_ret,
> unsigned *stride_ret)
>  {
> -	unsigned int tile_width, tile_height;
>  	uint64_t size;
> -	unsigned int byte_width = fb_plane_min_stride(format, width,
> 0);
>  
> -	igt_get_fb_tile_size(fd, tiling, fb_plane_bpp(format, 0),
> -			     &tile_width, &tile_height);
> +	if (!stride)
> +		stride = calc_plane_stride(fd, format, width,
> tiling, 0);
>  
>  	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
>  	    intel_gen(intel_get_drm_devid(fd)) <= 3) {
> @@ -289,16 +320,13 @@ static void calc_fb_size_packed(int fd, int
> width, int height,
>  		 * tiled. But then that failure is expected.
>  		 */
>  
> -		if (!stride) {
> -			stride = max(byte_width, 512);
> -			stride = roundup_power_of_two(stride);
> -		}
> -
>  		size = max((uint64_t) stride * height, 1024*1024);
>  		size = roundup_power_of_two(size);
>  	} else {
> -		if (!stride)
> -			stride = ALIGN(byte_width, tile_width);
> +		unsigned int tile_width, tile_height;
> +
> +		igt_get_fb_tile_size(fd, tiling,
> fb_plane_bpp(format, 0),
> +				     &tile_width, &tile_height);
>  
>  		size = (uint64_t) stride * ALIGN(height,
> tile_height);
>  	}
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev