[PATCH igt 1/2] lib/igt_fb: make the automatic buffer sizes/strides smaller

[PATCH igt 1/2] lib/igt_fb: make the automatic buffer sizes/strides smaller

[Paulo Zanoni]

The big motivation behind this patch is that the current power-of-two
granularity from igt_fb is way too big. There was more than one
occasion where I had to work around this problem on
kms_frontbuffer_tracking, and during my last workaround I was
requested to just make igt_fb use more minimal buffers.

I also need to export the size computation function so I won't need to
reimplement it inside kms_frontbuffer_tracking.

The tiling size checking code is based on the Kernel's
intel_tile_height() and i915_tiling_ok().

Requested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 lib/igt_fb.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++----------
 lib/igt_fb.h |   2 ++
 2 files changed, 92 insertions(+), 17 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 3ea9915..e67dd24 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -32,6 +32,7 @@
 #include "drmtest.h"
 #include "igt_fb.h"
 #include "ioctl_wrappers.h"
+#include "intel_chipset.h"
 
 /**
  * SECTION:igt_fb
@@ -72,26 +73,84 @@ static struct format_desc_struct {
 #define for_each_format(f)	\
 	for (f = format_desc; f - format_desc < ARRAY_SIZE(format_desc); f++)
 
+static void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
+				 unsigned *width_ret, unsigned *height_ret)
+{
+	uint32_t devid = intel_get_drm_devid(fd);
 
-/* helpers to create nice-looking framebuffers */
-static int create_bo_for_fb(int fd, int width, int height, int bpp,
-			    uint64_t tiling, unsigned bo_size,
-			    unsigned bo_stride, uint32_t *gem_handle_ret,
-			    unsigned *size_ret, unsigned *stride_ret)
+	switch (tiling) {
+	case LOCAL_DRM_FORMAT_MOD_NONE:
+		*width_ret = 64;
+		*height_ret = 1;
+		break;
+	case LOCAL_I915_FORMAT_MOD_X_TILED:
+		if (intel_gen(devid) == 2) {
+			*width_ret = 128;
+			*height_ret = 16;
+		} else {
+			*width_ret = 512;
+			*height_ret = 8;
+		}
+		break;
+	case LOCAL_I915_FORMAT_MOD_Y_TILED:
+		if (IS_915(devid))
+			*width_ret = 512;
+		else
+			*width_ret = 128;
+		*height_ret = 32;
+		break;
+	case LOCAL_I915_FORMAT_MOD_Yf_TILED:
+		*width_ret = 128;
+		switch (fb_bpp) {
+		case 8:
+			*height_ret = 64;
+			break;
+		case 16:
+		case 32:
+			*height_ret = 32;
+			break;
+		case 64:
+			*height_ret = 16;
+			break;
+		default:
+			igt_assert(false);
+		}
+		break;
+	default:
+		igt_assert(false);
+	}
+}
+
+/**
+ * igt_calc_fb_size:
+ * @fd: the DRM file descriptor
+ * @width: width of the framebuffer in pixels
+ * @height: height of the framebuffer in pixels
+ * @bpp: bytes per pixel of the framebuffer
+ * @tiling: tiling layout of the framebuffer (as framebuffer modifier)
+ * @size_ret: returned size for the framebuffer
+ * @stride_ret: returned stride for the framebuffer
+ *
+ * This function returns valid stride and size values for a framebuffer with the
+ * specified parameters.
+ */
+void igt_calc_fb_size(int fd, int width, int height, int bpp, uint64_t tiling,
+		      unsigned *size_ret, unsigned *stride_ret)
 {
-	uint32_t gem_handle;
-	int size, ret = 0;
-	unsigned stride;
+	unsigned int tile_width, tile_height, stride, size;
+	int byte_width = width * (bpp / 8);
+
+	igt_get_fb_tile_size(fd, tiling, bpp, &tile_width, &tile_height);
 
-	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE) {
+	if (intel_gen(intel_get_drm_devid(fd)) <= 3) {
 		int v;
 
-		/* 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.
+		/* 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.
+		 * This can still fail if the framebuffer is too large to be
+		 * tiled. But then that failure is expected.
 		 */
 
 		v = width * bpp / 8;
@@ -102,11 +161,25 @@ static int create_bo_for_fb(int fd, int width, int height, int bpp,
 		for (size = 1024*1024; size < v; size *= 2)
 			;
 	} else {
-		/* Scan-out has a 64 byte alignment restriction */
-		stride = ALIGN(width * (bpp / 8), 64);
-		size = stride * height;
+		stride = ALIGN(byte_width, tile_width);
+		size = stride * ALIGN(height, tile_height);
 	}
 
+	*stride_ret = stride;
+	*size_ret = size;
+}
+
+/* helpers to create nice-looking framebuffers */
+static int create_bo_for_fb(int fd, int width, int height, int bpp,
+			    uint64_t tiling, unsigned bo_size,
+			    unsigned bo_stride, uint32_t *gem_handle_ret,
+			    unsigned *size_ret, unsigned *stride_ret)
+{
+	uint32_t gem_handle;
+	int ret = 0;
+	unsigned size, stride;
+
+	igt_calc_fb_size(fd, width, height, bpp, tiling, &size, &stride);
 	if (bo_size == 0)
 		bo_size = size;
 	if (bo_stride == 0)
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index 37892b5..1d32a9c 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -69,6 +69,8 @@ enum igt_text_align {
 	align_hcenter	= 0x08,
 };
 
+void igt_calc_fb_size(int fd, int width, int height, int bpp, uint64_t tiling,
+		      unsigned *size_ret, unsigned *stride_ret);
 unsigned int
 igt_create_fb_with_bo_size(int fd, int width, int height,
 			   uint32_t format, uint64_t tiling,
-- 
2.6.2

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

[PATCH igt 2/2] kms_frontbuffer_tracking: standardize the used FB sizes

[Paulo Zanoni]

We want to make sure that both tiled and untiled buffers have the same
size for the same width/height/format. This will allow better control
over the failure paths exercised by our tests: when we try to flip
from tiled to untiled, we'll be sure that we won't execute the error
path that checks for buffer sizes.

v2: Use the new igt_calc_fb_size() instead of implementing our own
size calculation (Daniel).

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 tests/kms_frontbuffer_tracking.c | 51 ++++++++++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 17 deletions(-)

diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 81703ec..3db95d2 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -479,10 +479,28 @@ static bool init_modeset_cached_params(void)
 	return true;
 }
 
+static int format_get_bpp(uint32_t format)
+{
+	switch (format) {
+	case DRM_FORMAT_RGB565:
+		return 16;
+	case DRM_FORMAT_XRGB8888:
+	case DRM_FORMAT_ARGB8888:
+	case DRM_FORMAT_ARGB2101010:
+	case DRM_FORMAT_XRGB2101010:
+		return 32;
+	default:
+		igt_assert(false);
+	}
+}
+
 static void create_fb(enum pixel_format pformat, int width, int height,
 		      uint64_t tiling, int plane, struct igt_fb *fb)
 {
 	uint32_t format;
+	unsigned int size, stride;
+	int bpp;
+	uint64_t tiling_for_size;
 
 	switch (pformat) {
 	case FORMAT_RGB888:
@@ -512,7 +530,21 @@ static void create_fb(enum pixel_format pformat, int width, int height,
 		igt_assert(false);
 	}
 
-	igt_create_fb(drm.fd, width, height, format, tiling, fb);
+	/* We want all frontbuffers with the same width/height/format to have
+	 * the same size regardless of tiling since we want to properly exercise
+	 * the Kernel's specific tiling-checking code paths without accidentally
+	 * hitting size-checking ones first. */
+	bpp = format_get_bpp(format);
+	if (plane == PLANE_CUR)
+		tiling_for_size = LOCAL_DRM_FORMAT_MOD_NONE;
+	else
+		tiling_for_size = LOCAL_I915_FORMAT_MOD_X_TILED;
+
+	igt_calc_fb_size(drm.fd, width, height, bpp, tiling_for_size, &size,
+			 &stride);
+
+	igt_create_fb_with_bo_size(drm.fd, width, height, format, tiling, fb,
+				   size, stride);
 }
 
 static uint32_t pick_color(struct igt_fb *fb, enum color ecolor)
@@ -1094,21 +1126,6 @@ static void *busy_thread_func(void *data)
 	pthread_exit(0);
 }
 
-static int fb_get_bpp(struct igt_fb *fb)
-{
-	switch (fb->drm_format) {
-	case DRM_FORMAT_RGB565:
-		return 16;
-	case DRM_FORMAT_XRGB8888:
-	case DRM_FORMAT_ARGB8888:
-	case DRM_FORMAT_ARGB2101010:
-	case DRM_FORMAT_XRGB2101010:
-		return 32;
-	default:
-		igt_assert(false);
-	}
-}
-
 static void start_busy_thread(struct igt_fb *fb)
 {
 	int rc;
@@ -1121,7 +1138,7 @@ static void start_busy_thread(struct igt_fb *fb)
 	busy_thread.width = fb->width;
 	busy_thread.height = fb->height;
 	busy_thread.color = pick_color(fb, COLOR_PRIM_BG);
-	busy_thread.bpp = fb_get_bpp(fb);
+	busy_thread.bpp = format_get_bpp(fb->drm_format);
 
 	rc = pthread_create(&busy_thread.thread, NULL, busy_thread_func, NULL);
 	igt_assert_eq(rc, 0);
-- 
2.6.2

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

Re: [PATCH igt 2/2] kms_frontbuffer_tracking: standardize the used FB sizes

[Daniel Vetter]

On Wed, Dec 02, 2015 at 10:47:01AM -0200, Paulo Zanoni wrote:
> We want to make sure that both tiled and untiled buffers have the same
> size for the same width/height/format. This will allow better control
> over the failure paths exercised by our tests: when we try to flip
> from tiled to untiled, we'll be sure that we won't execute the error
> path that checks for buffer sizes.
> 
> v2: Use the new igt_calc_fb_size() instead of implementing our own
> size calculation (Daniel).
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Yeah, this is what I had in mind, looks great. On the series:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  tests/kms_frontbuffer_tracking.c | 51 ++++++++++++++++++++++++++--------------
>  1 file changed, 34 insertions(+), 17 deletions(-)
> 
> diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
> index 81703ec..3db95d2 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -479,10 +479,28 @@ static bool init_modeset_cached_params(void)
>  	return true;
>  }
>  
> +static int format_get_bpp(uint32_t format)
> +{
> +	switch (format) {
> +	case DRM_FORMAT_RGB565:
> +		return 16;
> +	case DRM_FORMAT_XRGB8888:
> +	case DRM_FORMAT_ARGB8888:
> +	case DRM_FORMAT_ARGB2101010:
> +	case DRM_FORMAT_XRGB2101010:
> +		return 32;
> +	default:
> +		igt_assert(false);
> +	}
> +}
> +
>  static void create_fb(enum pixel_format pformat, int width, int height,
>  		      uint64_t tiling, int plane, struct igt_fb *fb)
>  {
>  	uint32_t format;
> +	unsigned int size, stride;
> +	int bpp;
> +	uint64_t tiling_for_size;
>  
>  	switch (pformat) {
>  	case FORMAT_RGB888:
> @@ -512,7 +530,21 @@ static void create_fb(enum pixel_format pformat, int width, int height,
>  		igt_assert(false);
>  	}
>  
> -	igt_create_fb(drm.fd, width, height, format, tiling, fb);
> +	/* We want all frontbuffers with the same width/height/format to have
> +	 * the same size regardless of tiling since we want to properly exercise
> +	 * the Kernel's specific tiling-checking code paths without accidentally
> +	 * hitting size-checking ones first. */
> +	bpp = format_get_bpp(format);
> +	if (plane == PLANE_CUR)
> +		tiling_for_size = LOCAL_DRM_FORMAT_MOD_NONE;
> +	else
> +		tiling_for_size = LOCAL_I915_FORMAT_MOD_X_TILED;
> +
> +	igt_calc_fb_size(drm.fd, width, height, bpp, tiling_for_size, &size,
> +			 &stride);
> +
> +	igt_create_fb_with_bo_size(drm.fd, width, height, format, tiling, fb,
> +				   size, stride);
>  }
>  
>  static uint32_t pick_color(struct igt_fb *fb, enum color ecolor)
> @@ -1094,21 +1126,6 @@ static void *busy_thread_func(void *data)
>  	pthread_exit(0);
>  }
>  
> -static int fb_get_bpp(struct igt_fb *fb)
> -{
> -	switch (fb->drm_format) {
> -	case DRM_FORMAT_RGB565:
> -		return 16;
> -	case DRM_FORMAT_XRGB8888:
> -	case DRM_FORMAT_ARGB8888:
> -	case DRM_FORMAT_ARGB2101010:
> -	case DRM_FORMAT_XRGB2101010:
> -		return 32;
> -	default:
> -		igt_assert(false);
> -	}
> -}
> -
>  static void start_busy_thread(struct igt_fb *fb)
>  {
>  	int rc;
> @@ -1121,7 +1138,7 @@ static void start_busy_thread(struct igt_fb *fb)
>  	busy_thread.width = fb->width;
>  	busy_thread.height = fb->height;
>  	busy_thread.color = pick_color(fb, COLOR_PRIM_BG);
> -	busy_thread.bpp = fb_get_bpp(fb);
> +	busy_thread.bpp = format_get_bpp(fb->drm_format);
>  
>  	rc = pthread_create(&busy_thread.thread, NULL, busy_thread_func, NULL);
>  	igt_assert_eq(rc, 0);
> -- 
> 2.6.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH igt 2/2] kms_frontbuffer_tracking: standardize the used FB sizes

[Daniel Vetter]

On Wed, Dec 02, 2015 at 10:47:01AM -0200, Paulo Zanoni wrote:
> We want to make sure that both tiled and untiled buffers have the same
> size for the same width/height/format. This will allow better control
> over the failure paths exercised by our tests: when we try to flip
> from tiled to untiled, we'll be sure that we won't execute the error
> path that checks for buffer sizes.
> 
> v2: Use the new igt_calc_fb_size() instead of implementing our own
> size calculation (Daniel).
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  tests/kms_frontbuffer_tracking.c | 51 ++++++++++++++++++++++++++--------------
>  1 file changed, 34 insertions(+), 17 deletions(-)
> 
> diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
> index 81703ec..3db95d2 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -479,10 +479,28 @@ static bool init_modeset_cached_params(void)
>  	return true;
>  }
>  
> +static int format_get_bpp(uint32_t format)

Ah, missed one: igt_drm_format_to_bpp please, and if it doesn't cover them all we
need to fix that asap.
-Daniel

> +{
> +	switch (format) {
> +	case DRM_FORMAT_RGB565:
> +		return 16;
> +	case DRM_FORMAT_XRGB8888:
> +	case DRM_FORMAT_ARGB8888:
> +	case DRM_FORMAT_ARGB2101010:
> +	case DRM_FORMAT_XRGB2101010:
> +		return 32;
> +	default:
> +		igt_assert(false);
> +	}
> +}
> +
>  static void create_fb(enum pixel_format pformat, int width, int height,
>  		      uint64_t tiling, int plane, struct igt_fb *fb)
>  {
>  	uint32_t format;
> +	unsigned int size, stride;
> +	int bpp;
> +	uint64_t tiling_for_size;
>  
>  	switch (pformat) {
>  	case FORMAT_RGB888:
> @@ -512,7 +530,21 @@ static void create_fb(enum pixel_format pformat, int width, int height,
>  		igt_assert(false);
>  	}
>  
> -	igt_create_fb(drm.fd, width, height, format, tiling, fb);
> +	/* We want all frontbuffers with the same width/height/format to have
> +	 * the same size regardless of tiling since we want to properly exercise
> +	 * the Kernel's specific tiling-checking code paths without accidentally
> +	 * hitting size-checking ones first. */
> +	bpp = format_get_bpp(format);
> +	if (plane == PLANE_CUR)
> +		tiling_for_size = LOCAL_DRM_FORMAT_MOD_NONE;
> +	else
> +		tiling_for_size = LOCAL_I915_FORMAT_MOD_X_TILED;
> +
> +	igt_calc_fb_size(drm.fd, width, height, bpp, tiling_for_size, &size,
> +			 &stride);
> +
> +	igt_create_fb_with_bo_size(drm.fd, width, height, format, tiling, fb,
> +				   size, stride);
>  }
>  
>  static uint32_t pick_color(struct igt_fb *fb, enum color ecolor)
> @@ -1094,21 +1126,6 @@ static void *busy_thread_func(void *data)
>  	pthread_exit(0);
>  }
>  
> -static int fb_get_bpp(struct igt_fb *fb)
> -{
> -	switch (fb->drm_format) {
> -	case DRM_FORMAT_RGB565:
> -		return 16;
> -	case DRM_FORMAT_XRGB8888:
> -	case DRM_FORMAT_ARGB8888:
> -	case DRM_FORMAT_ARGB2101010:
> -	case DRM_FORMAT_XRGB2101010:
> -		return 32;
> -	default:
> -		igt_assert(false);
> -	}
> -}
> -
>  static void start_busy_thread(struct igt_fb *fb)
>  {
>  	int rc;
> @@ -1121,7 +1138,7 @@ static void start_busy_thread(struct igt_fb *fb)
>  	busy_thread.width = fb->width;
>  	busy_thread.height = fb->height;
>  	busy_thread.color = pick_color(fb, COLOR_PRIM_BG);
> -	busy_thread.bpp = fb_get_bpp(fb);
> +	busy_thread.bpp = format_get_bpp(fb->drm_format);
>  
>  	rc = pthread_create(&busy_thread.thread, NULL, busy_thread_func, NULL);
>  	igt_assert_eq(rc, 0);
> -- 
> 2.6.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH igt 2/2] kms_frontbuffer_tracking: standardize the used FB sizes

[Zanoni, Paulo R]

Em Sex, 2015-12-04 às 16:28 +0100, Daniel Vetter escreveu:
> On Wed, Dec 02, 2015 at 10:47:01AM -0200, Paulo Zanoni wrote:
> > We want to make sure that both tiled and untiled buffers have the
> > same
> > size for the same width/height/format. This will allow better
> > control
> > over the failure paths exercised by our tests: when we try to flip
> > from tiled to untiled, we'll be sure that we won't execute the
> > error
> > path that checks for buffer sizes.
> > 
> > v2: Use the new igt_calc_fb_size() instead of implementing our own
> > size calculation (Daniel).
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  tests/kms_frontbuffer_tracking.c | 51 ++++++++++++++++++++++++++
> > --------------
> >  1 file changed, 34 insertions(+), 17 deletions(-)
> > 
> > diff --git a/tests/kms_frontbuffer_tracking.c
> > b/tests/kms_frontbuffer_tracking.c
> > index 81703ec..3db95d2 100644
> > --- a/tests/kms_frontbuffer_tracking.c
> > +++ b/tests/kms_frontbuffer_tracking.c
> > @@ -479,10 +479,28 @@ static bool init_modeset_cached_params(void)
> >  	return true;
> >  }
> >  
> > +static int format_get_bpp(uint32_t format)
> 
> Ah, missed one: igt_drm_format_to_bpp please, and if it doesn't cover
> them all we
> need to fix that asap.

I'm not sure if this is a good idea. Not every DRM format has a
matching cairo format, and it seems the whole igt_fb code is built
based on this assumption. On a quick look, it really seems that both
kms_render.c and kms_atomic.c will break if I add ARGB2101010 with a
matching CAIRO_INVALID (since they call igt_get_all_formats() and use
cairo).

I know, you can question the use of ARGB2101010 by
kms_frontbuffer_tracking (we don't use it anymore, the format is there
as an artifact of an older attempt when I initially added support for
multiple formats), but that doesn't solve the bigger problem that we
can't easily expand igt_drm_format_to_bpp().

If you still insist, the big plan should be to make sure that both
igt_fb and the libs can properly handle the cases where a DRM format
doesn't have a matching cairo format, but I don't want to block my
current tasks on this. So I'd vote to merge my patches as-is for now.

What do you think?

> -Daniel
> 
> > +{
> > +	switch (format) {
> > +	case DRM_FORMAT_RGB565:
> > +		return 16;
> > +	case DRM_FORMAT_XRGB8888:
> > +	case DRM_FORMAT_ARGB8888:
> > +	case DRM_FORMAT_ARGB2101010:
> > +	case DRM_FORMAT_XRGB2101010:
> > +		return 32;
> > +	default:
> > +		igt_assert(false);
> > +	}
> > +}
> > +
> >  static void create_fb(enum pixel_format pformat, int width, int
> > height,
> >  		      uint64_t tiling, int plane, struct igt_fb
> > *fb)
> >  {
> >  	uint32_t format;
> > +	unsigned int size, stride;
> > +	int bpp;
> > +	uint64_t tiling_for_size;
> >  
> >  	switch (pformat) {
> >  	case FORMAT_RGB888:
> > @@ -512,7 +530,21 @@ static void create_fb(enum pixel_format
> > pformat, int width, int height,
> >  		igt_assert(false);
> >  	}
> >  
> > -	igt_create_fb(drm.fd, width, height, format, tiling, fb);
> > +	/* We want all frontbuffers with the same
> > width/height/format to have
> > +	 * the same size regardless of tiling since we want to
> > properly exercise
> > +	 * the Kernel's specific tiling-checking code paths
> > without accidentally
> > +	 * hitting size-checking ones first. */
> > +	bpp = format_get_bpp(format);
> > +	if (plane == PLANE_CUR)
> > +		tiling_for_size = LOCAL_DRM_FORMAT_MOD_NONE;
> > +	else
> > +		tiling_for_size = LOCAL_I915_FORMAT_MOD_X_TILED;
> > +
> > +	igt_calc_fb_size(drm.fd, width, height, bpp,
> > tiling_for_size, &size,
> > +			 &stride);
> > +
> > +	igt_create_fb_with_bo_size(drm.fd, width, height, format,
> > tiling, fb,
> > +				   size, stride);
> >  }
> >  
> >  static uint32_t pick_color(struct igt_fb *fb, enum color ecolor)
> > @@ -1094,21 +1126,6 @@ static void *busy_thread_func(void *data)
> >  	pthread_exit(0);
> >  }
> >  
> > -static int fb_get_bpp(struct igt_fb *fb)
> > -{
> > -	switch (fb->drm_format) {
> > -	case DRM_FORMAT_RGB565:
> > -		return 16;
> > -	case DRM_FORMAT_XRGB8888:
> > -	case DRM_FORMAT_ARGB8888:
> > -	case DRM_FORMAT_ARGB2101010:
> > -	case DRM_FORMAT_XRGB2101010:
> > -		return 32;
> > -	default:
> > -		igt_assert(false);
> > -	}
> > -}
> > -
> >  static void start_busy_thread(struct igt_fb *fb)
> >  {
> >  	int rc;
> > @@ -1121,7 +1138,7 @@ static void start_busy_thread(struct igt_fb
> > *fb)
> >  	busy_thread.width = fb->width;
> >  	busy_thread.height = fb->height;
> >  	busy_thread.color = pick_color(fb, COLOR_PRIM_BG);
> > -	busy_thread.bpp = fb_get_bpp(fb);
> > +	busy_thread.bpp = format_get_bpp(fb->drm_format);
> >  
> >  	rc = pthread_create(&busy_thread.thread, NULL,
> > busy_thread_func, NULL);
> >  	igt_assert_eq(rc, 0);
> > -- 
> > 2.6.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH igt 2/2] kms_frontbuffer_tracking: standardize the used FB sizes

[Daniel Vetter]

On Mon, Dec 07, 2015 at 02:04:51PM +0000, Zanoni, Paulo R wrote:
> Em Sex, 2015-12-04 às 16:28 +0100, Daniel Vetter escreveu:
> > On Wed, Dec 02, 2015 at 10:47:01AM -0200, Paulo Zanoni wrote:
> > > We want to make sure that both tiled and untiled buffers have the
> > > same
> > > size for the same width/height/format. This will allow better
> > > control
> > > over the failure paths exercised by our tests: when we try to flip
> > > from tiled to untiled, we'll be sure that we won't execute the
> > > error
> > > path that checks for buffer sizes.
> > > 
> > > v2: Use the new igt_calc_fb_size() instead of implementing our own
> > > size calculation (Daniel).
> > > 
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > >  tests/kms_frontbuffer_tracking.c | 51 ++++++++++++++++++++++++++
> > > --------------
> > >  1 file changed, 34 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/tests/kms_frontbuffer_tracking.c
> > > b/tests/kms_frontbuffer_tracking.c
> > > index 81703ec..3db95d2 100644
> > > --- a/tests/kms_frontbuffer_tracking.c
> > > +++ b/tests/kms_frontbuffer_tracking.c
> > > @@ -479,10 +479,28 @@ static bool init_modeset_cached_params(void)
> > >  	return true;
> > >  }
> > >  
> > > +static int format_get_bpp(uint32_t format)
> > 
> > Ah, missed one: igt_drm_format_to_bpp please, and if it doesn't cover
> > them all we
> > need to fix that asap.
> 
> I'm not sure if this is a good idea. Not every DRM format has a
> matching cairo format, and it seems the whole igt_fb code is built
> based on this assumption. On a quick look, it really seems that both
> kms_render.c and kms_atomic.c will break if I add ARGB2101010 with a
> matching CAIRO_INVALID (since they call igt_get_all_formats() and use
> cairo).
> 
> I know, you can question the use of ARGB2101010 by
> kms_frontbuffer_tracking (we don't use it anymore, the format is there
> as an artifact of an older attempt when I initially added support for
> multiple formats), but that doesn't solve the bigger problem that we
> can't easily expand igt_drm_format_to_bpp().

Hm, if you don't need it maybe push these patches without the new format
and use the existing library function for now?

> If you still insist, the big plan should be to make sure that both
> igt_fb and the libs can properly handle the cases where a DRM format
> doesn't have a matching cairo format, but I don't want to block my
> current tasks on this. So I'd vote to merge my patches as-is for now.
> 
> What do you think?

igt_get_all_cairo_formats which skips on CAIRO_INVALID, roll it out in
existing users, extend what we have here?

Shouldn't be too much work, and we need this kind of stuff anyway.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx