[igt-dev] [PATCH v7 00/14] chamelium: Test the plane formats

[igt-dev] [PATCH v7 00/14] chamelium: Test the plane formats

[Maxime Ripard]

Hi,

Here is another attempt at a serie that aims at starting to test the plane
formats using the Chamelium over the HDMI. This was tested using the vc4
DRM driver found on the RaspberryPi.

There's been a number of changes from the RFC thanks to the feedback from
Eric Anholt, Paul Kocialkowski and Ville Syrjälä.

The series now relies mostly on igt_fb, with some decoupling from cairo and
a bunch of new helpers to aim at being able to convert igt_fb to arbitrary
DRM formats. The list of formats have been extended a bit, but is quite
small at this point. We also rely solely on either pixman or the existing
format conversions functions at them moment, and we allow cairo surfaces to
be created for framebuffers in a format not supported by cairo, through an
intermediate conversion to RGB24.

However, since it's now abstracted away from the igt_fb users, we can
easily add some routines to convert to additional formats if needed. And
since the code was so close now, it's been integrated into kms_chamelium.

Let me know what you think,
Maxime

Changes from v7:
  * Convert the igt_fb cairo shadow buffer to a igt_fb of its own
  * Fix a regression in the igt_fb common code that will prevent it from
    running on anything else than i915

Changes from v5:
  * Fixed a typo

Changes from v4:
  * Added Eric's Reviewed-by
  * Dropped the preferred mode patch
  * Use the igt_fb pattern if we can
  * Rebased on top of master

Changes from v3:
  * Plug into the cairo surface conversion code, and share the code between
    the explicit conversion and the implicit one done when asking for a
    cairo surface for a framebuffer in a format not supported by Cairo.
  * Rebased on top of master

Changes from v2:
  * Reworded a commit log
  * Dropped commit making chamelium_calculate_fb_crc static
  * Added a few more missing formats
  * Changed PIXMAN_invalid to 0 to make sure it never conflicts with a
    pixman format

Changes from v1:
  * Add a README for the test lists
  * Add igt_fb buffer mapping / unmapping functions
  * Add igt_fb buffer format conversion function
  * Add pixman formats to the format descriptors
  * Made some refactoring to kms_chamelium to support format tests
  * Created sub-tests for the formats

Maxime Ripard (14):
  fb: Add buffer map/unmap functions
  fb: Use an igt_fb for the cairo shadow buffer
  fb: convert: Remove swizzle from the arguments
  fb: Create common function to convert frame formats
  fb: Add format conversion routine
  fb: Fix ARGB8888 color depth
  fb: Add support for conversions through pixman
  fb: Add depth lookup function
  fb: Add more formats
  chamelium: Split CRC test function in two
  chamelium: Change our pattern for a custom one if needed
  chamelium: Add format support
  chamelium: Add format subtests
  tests: Add chamelium formats subtests to vc4 test lists

 lib/igt_fb.c                             | 464 ++++++++++++++++++------
 lib/igt_fb.h                             |   5 +-
 tests/kms_chamelium.c                    | 218 ++++++++---
 tests/vc4_ci/vc4-chamelium-fast.testlist |  10 +-
 tests/vc4_ci/vc4-chamelium.testlist      |  10 +-
 5 files changed, 546 insertions(+), 161 deletions(-)

base-commit: 57e3d826dee154cb8664667db7660d854a707fc6
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH v7 01/14] fb: Add buffer map/unmap functions

[Maxime Ripard]

The current code to manipulate the buffer has the assumption that we can
create an underlying cairo instance.

However, when it comes to supporting various formats, cairo is very limited
so we would need to decouple the buffer access from cairo surfaces.

Let's create a function that allows to map the underlying GEM buffer from
an igt_fb structure, which will then allow use to manipulate as we wish.

Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_fb.c | 65 +++++++++++++++++++++++++++++++++++++++++++----------
 lib/igt_fb.h |  2 ++-
 2 files changed, 55 insertions(+), 12 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 1085d25d33ce..542c62610412 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1330,23 +1330,29 @@ int igt_dirty_fb(int fd, struct igt_fb *fb)
 	return drmModeDirtyFB(fb->fd, fb->fb_id, NULL, 0);
 }
 
+static void unmap_bo(struct igt_fb *fb, void *ptr)
+{
+	gem_munmap(ptr, fb->size);
+
+	if (fb->is_dumb)
+		igt_dirty_fb(fb->fd, fb);
+}
+
 static void destroy_cairo_surface__gtt(void *arg)
 {
 	struct igt_fb *fb = arg;
 
-	gem_munmap(cairo_image_surface_get_data(fb->cairo_surface), fb->size);
+	unmap_bo(fb, cairo_image_surface_get_data(fb->cairo_surface));
 	fb->cairo_surface = NULL;
-
-	if (fb->is_dumb)
-		igt_dirty_fb(fb->fd, fb);
 }
 
-static void create_cairo_surface__gtt(int fd, struct igt_fb *fb)
+static void *map_bo(int fd, struct igt_fb *fb)
 {
 	void *ptr;
 
-	gem_set_domain(fd, fb->gem_handle,
-		       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
+	if (is_i915_device(fd))
+		gem_set_domain(fd, fb->gem_handle,
+			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
 
 	if (fb->is_dumb)
 		ptr = kmstest_dumb_map_buffer(fd, fb->gem_handle, fb->size,
@@ -1355,6 +1361,13 @@ static void create_cairo_surface__gtt(int fd, struct igt_fb *fb)
 		ptr = gem_mmap__gtt(fd, fb->gem_handle, fb->size,
 				    PROT_READ | PROT_WRITE);
 
+	return ptr;
+}
+
+static void create_cairo_surface__gtt(int fd, struct igt_fb *fb)
+{
+	void *ptr = map_bo(fd, fb);
+
 	fb->cairo_surface =
 		cairo_image_surface_create_for_data(ptr,
 						    drm_format_to_cairo(fb->drm_format),
@@ -1772,7 +1785,7 @@ static void destroy_cairo_surface__convert(void *arg)
 	if (blit->linear.handle)
 		free_linear_mapping(blit->fd, blit->fb, &blit->linear);
 	else
-		gem_munmap(blit->linear.map, fb->size);
+		unmap_bo(fb, blit->linear.map);
 
 	free(blit);
 
@@ -1796,11 +1809,9 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
 		setup_linear_mapping(fd, fb, &blit->linear);
 	} else {
 		blit->linear.handle = 0;
-		gem_set_domain(fd, fb->gem_handle,
-			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
-		blit->linear.map = gem_mmap__gtt(fd, fb->gem_handle, fb->size,
-					      PROT_READ | PROT_WRITE);
+		blit->linear.map = map_bo(fd, fb);
 		igt_assert(blit->linear.map);
+
 		blit->linear.stride = fb->stride;
 		blit->linear.size = fb->size;
 		memcpy(blit->linear.offsets, fb->offsets, sizeof(fb->offsets));
@@ -1834,6 +1845,36 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
 }
 
 /**
+ * igt_fb_map_buffer:
+ * @fd: open drm file descriptor
+ * @fb: pointer to an #igt_fb structure
+ *
+ * This function will creating a new mapping of the buffer and return a pointer
+ * to the content of the supplied framebuffer's plane. This mapping needs to be
+ * deleted using igt_fb_unmap_buffer().
+ *
+ * Returns:
+ * A pointer to a buffer with the contents of the framebuffer
+ */
+void *igt_fb_map_buffer(int fd, struct igt_fb *fb)
+{
+	return map_bo(fd, fb);
+}
+
+/**
+ * igt_fb_unmap_buffer:
+ * @fb: pointer to the backing igt_fb structure
+ * @buffer: pointer to the buffer previously mappped
+ *
+ * This function will unmap a buffer mapped previously with
+ * igt_fb_map_buffer().
+ */
+void igt_fb_unmap_buffer(struct igt_fb *fb, void *buffer)
+{
+	return unmap_bo(fb, buffer);
+}
+
+/**
  * igt_get_cairo_surface:
  * @fd: open drm file descriptor
  * @fb: pointer to an #igt_fb structure
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index d28bc0c4110a..ca028f550ab8 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -132,6 +132,8 @@ unsigned int igt_create_stereo_fb(int drm_fd, drmModeModeInfo *mode,
 				  uint32_t format, uint64_t tiling);
 void igt_remove_fb(int fd, struct igt_fb *fb);
 int igt_dirty_fb(int fd, struct igt_fb *fb);
+void *igt_fb_map_buffer(int fd, struct igt_fb *fb);
+void igt_fb_unmap_buffer(struct igt_fb *fb, void *buffer);
 
 int igt_create_bo_with_dimensions(int fd, int width, int height, uint32_t format,
 				  uint64_t modifier, unsigned stride,
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH v7 02/14] fb: Use an igt_fb for the cairo shadow buffer

[Maxime Ripard]

In the case where an igt_fb user wants to get a cairo surface out of that
framebuffer, if the format of that framebuffer cannot be imported as-is in
Cairo, the current code will do an anonymous mapping to create a shadow
buffer where an RGB24 copy of that buffer will be created.

However, making that shadow buffer into an igt_fb itself will help us do
further improvements on the conversion code.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_fb.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 542c62610412..dd180c6c8016 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1381,12 +1381,12 @@ static void create_cairo_surface__gtt(int fd, struct igt_fb *fb)
 
 struct fb_convert_blit_upload {
 	int fd;
+
 	struct igt_fb *fb;
+	uint8_t *ptr;
 
-	struct {
-		uint8_t *map;
-		unsigned stride, size;
-	} rgb24;
+	struct igt_fb shadow_fb;
+	uint8_t *shadow_ptr;
 
 	struct fb_blit_linear linear;
 };
@@ -1780,7 +1780,8 @@ static void destroy_cairo_surface__convert(void *arg)
 			     fb->drm_format);
 	}
 
-	munmap(blit->rgb24.map, blit->rgb24.size);
+	igt_fb_unmap_buffer(&blit->shadow_fb, blit->shadow_ptr);
+	igt_remove_fb(blit->fd, &blit->shadow_fb);
 
 	if (blit->linear.handle)
 		free_linear_mapping(blit->fd, blit->fb, &blit->linear);
@@ -1795,14 +1796,19 @@ static void destroy_cairo_surface__convert(void *arg)
 static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
 {
 	struct fb_convert_blit_upload *blit = malloc(sizeof(*blit));
+	int shadow_id;
+
 	igt_assert(blit);
 
 	blit->fd = fd;
 	blit->fb = fb;
-	blit->rgb24.stride = ALIGN(fb->width * 4, 16);
-	blit->rgb24.size = ALIGN(blit->rgb24.stride * fb->height, sysconf(_SC_PAGESIZE));
-	blit->rgb24.map = mmap(NULL, blit->rgb24.size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-	igt_assert(blit->rgb24.map != MAP_FAILED);
+
+	shadow_id = igt_create_fb(fd, fb->width, fb->height, DRM_FORMAT_RGB888,
+				  LOCAL_DRM_FORMAT_MOD_NONE, &blit->shadow_fb);
+	igt_assert(shadow_id > 0);
+
+	blit->shadow_ptr = igt_fb_map_buffer(fd, &blit->shadow_fb);
+	igt_assert(blit->shadow_ptr);
 
 	if (fb->tiling == LOCAL_I915_FORMAT_MOD_Y_TILED ||
 	    fb->tiling == LOCAL_I915_FORMAT_MOD_Yf_TILED) {
@@ -1834,10 +1840,10 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
 	}
 
 	fb->cairo_surface =
-		cairo_image_surface_create_for_data(blit->rgb24.map,
+		cairo_image_surface_create_for_data(blit->shadow_ptr,
 						    CAIRO_FORMAT_RGB24,
 						    fb->width, fb->height,
-						    blit->rgb24.stride);
+						    blit->shadow_fb.stride);
 
 	cairo_surface_set_user_data(fb->cairo_surface,
 				    (cairo_user_data_key_t *)create_cairo_surface__convert,
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH v7 03/14] fb: convert: Remove swizzle from the arguments

[Maxime Ripard]

Since it can be inferred from the framebuffer that is already given as an
argument, it is redundant and can be removed.

Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_fb.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index dd180c6c8016..66bdc02a0ce7 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1647,8 +1647,7 @@ static const unsigned char *yuyv_swizzle(uint32_t format)
 	}
 }
 
-static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_upload *blit,
-				  const unsigned char swz[4])
+static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
 {
 	int i, j;
 	const uint8_t *yuyv;
@@ -1657,6 +1656,7 @@ static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
 	uint8_t *buf = malloc(blit->linear.size);
 	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(fb->color_encoding,
 						    fb->color_range);
+	const unsigned char *swz = yuyv_swizzle(fb->drm_format);
 
 	/*
 	 * Reading from the BO is awfully slow because of lack of read caching,
@@ -1706,8 +1706,7 @@ static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
 	free(buf);
 }
 
-static void convert_rgb24_to_yuyv(struct igt_fb *fb, struct fb_convert_blit_upload *blit,
-				  const unsigned char swz[4])
+static void convert_rgb24_to_yuyv(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
 {
 	int i, j;
 	uint8_t *yuyv = blit->linear.map;
@@ -1716,6 +1715,7 @@ static void convert_rgb24_to_yuyv(struct igt_fb *fb, struct fb_convert_blit_uplo
 	unsigned yuyv_stride = blit->linear.stride;
 	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(fb->color_encoding,
 						    fb->color_range);
+	const unsigned char *swz = yuyv_swizzle(fb->drm_format);
 
 	igt_assert_f(fb->drm_format == DRM_FORMAT_YUYV ||
 		     fb->drm_format == DRM_FORMAT_YVYU ||
@@ -1773,7 +1773,7 @@ static void destroy_cairo_surface__convert(void *arg)
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_VYUY:
-		convert_rgb24_to_yuyv(fb, blit, yuyv_swizzle(fb->drm_format));
+		convert_rgb24_to_yuyv(fb, blit);
 		break;
 	default:
 		igt_assert_f(false, "Conversion not implemented for formats 0x%x\n",
@@ -1832,7 +1832,7 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_VYUY:
-		convert_yuyv_to_rgb24(fb, blit, yuyv_swizzle(fb->drm_format));
+		convert_yuyv_to_rgb24(fb, blit);
 		break;
 	default:
 		igt_assert_f(false, "Conversion not implemented for formats 0x%x\n",
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH v7 04/14] fb: Create common function to convert frame formats

[Maxime Ripard]

The current code to convert between two buffer formats is quite tied to the
cairo surface infrastructure. Since we'll want to reuse it, make that
function more generic by introducing a common structure that passes all the
arguments and a common function that will call the right functions we
needed.

Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_fb.c | 220 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 130 insertions(+), 90 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 66bdc02a0ce7..f829df59e9fa 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1391,6 +1391,19 @@ struct fb_convert_blit_upload {
 	struct fb_blit_linear linear;
 };
 
+struct fb_convert_buf {
+	void			*ptr;
+	struct igt_fb		*fb;
+};
+
+struct fb_convert {
+	unsigned int		width;
+	unsigned int		height;
+
+	struct fb_convert_buf	dst;
+	struct fb_convert_buf	src;
+};
+
 static uint8_t clamprgb(float val)
 {
 	return clamp((int)(val + 0.5f), 0, 255);
@@ -1411,27 +1424,28 @@ static void write_rgb(uint8_t *rgb24, const struct igt_vec4 *rgb)
 	rgb24[0] = clamprgb(rgb->d[2]);
 }
 
-static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
+static void convert_nv12_to_rgb24(struct fb_convert *cvt)
 {
 	int i, j;
 	const uint8_t *y, *uv;
-	uint8_t *rgb24 = blit->rgb24.map;
-	unsigned rgb24_stride = blit->rgb24.stride, planar_stride = blit->linear.stride;
-	uint8_t *buf = malloc(blit->linear.size);
-	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(fb->color_encoding,
-						    fb->color_range);
+	uint8_t *rgb24 = cvt->dst.ptr;
+	unsigned int rgb24_stride = cvt->dst.fb->stride;
+	unsigned int planar_stride = cvt->src.fb->stride;
+	uint8_t *buf = malloc(cvt->src.fb->size);
+	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.fb->color_encoding,
+						    cvt->src.fb->color_range);
 
 	/*
 	 * Reading from the BO is awfully slow because of lack of read caching,
 	 * it's faster to copy the whole BO to a temporary buffer and convert
 	 * from there.
 	 */
-	igt_memcpy_from_wc(buf, blit->linear.map, blit->linear.size);
-	y = &buf[blit->linear.offsets[0]];
-	uv = &buf[blit->linear.offsets[1]];
+	igt_memcpy_from_wc(buf, cvt->src.ptr, cvt->src.fb->size);
+	y = cvt->src.ptr + cvt->src.fb->offsets[0];
+	uv = cvt->src.ptr + cvt->src.fb->offsets[1];
 
-	for (i = 0; i < fb->height / 2; i++) {
-		for (j = 0; j < fb->width / 2; j++) {
+	for (i = 0; i < cvt->height / 2; i++) {
+		for (j = 0; j < cvt->width / 2; j++) {
 			/* Convert 2x2 pixel blocks */
 			struct igt_vec4 yuv[4];
 			struct igt_vec4 rgb[4];
@@ -1456,7 +1470,7 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
 			write_rgb(&rgb24[j * 8 + 4 + rgb24_stride], &rgb[3]);
 		}
 
-		if (fb->width & 1) {
+		if (cvt->width & 1) {
 			/* Convert 1x2 pixel block */
 			struct igt_vec4 yuv[2];
 			struct igt_vec4 rgb[2];
@@ -1480,9 +1494,9 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
 		uv += planar_stride;
 	}
 
-	if (fb->height & 1) {
+	if (cvt->height & 1) {
 		/* Convert last row */
-		for (j = 0; j < fb->width / 2; j++) {
+		for (j = 0; j < cvt->width / 2; j++) {
 			/* Convert 2x1 pixel blocks */
 			struct igt_vec4 yuv[2];
 			struct igt_vec4 rgb[2];
@@ -1500,7 +1514,7 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
 			write_rgb(&rgb24[j * 8 + 4], &rgb[0]);
 		}
 
-		if (fb->width & 1) {
+		if (cvt->width & 1) {
 			/* Convert single pixel */
 			struct igt_vec4 yuv;
 			struct igt_vec4 rgb;
@@ -1519,22 +1533,22 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
 	free(buf);
 }
 
-static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
+static void convert_rgb24_to_nv12(struct fb_convert *cvt)
 {
 	int i, j;
-	uint8_t *y = &blit->linear.map[blit->linear.offsets[0]];
-	uint8_t *uv = &blit->linear.map[blit->linear.offsets[1]];
-	const uint8_t *rgb24 = blit->rgb24.map;
-	unsigned rgb24_stride = blit->rgb24.stride;
-	unsigned planar_stride = blit->linear.stride;
-	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(fb->color_encoding,
-						    fb->color_range);
-
-	igt_assert_f(fb->drm_format == DRM_FORMAT_NV12,
+	uint8_t *y = cvt->dst.ptr + cvt->dst.fb->offsets[0];
+	uint8_t *uv = cvt->dst.ptr + cvt->dst.fb->offsets[1];
+	const uint8_t *rgb24 = cvt->src.ptr;
+	unsigned rgb24_stride = cvt->src.fb->stride;
+	unsigned planar_stride = cvt->dst.fb->stride;
+	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.fb->color_encoding,
+						    cvt->dst.fb->color_range);
+
+	igt_assert_f(cvt->dst.fb->drm_format == DRM_FORMAT_NV12,
 		     "Conversion not implemented for !NV12 planar formats\n");
 
-	for (i = 0; i < fb->height / 2; i++) {
-		for (j = 0; j < fb->width / 2; j++) {
+	for (i = 0; i < cvt->height / 2; i++) {
+		for (j = 0; j < cvt->width / 2; j++) {
 			/* Convert 2x2 pixel blocks */
 			struct igt_vec4 rgb[4];
 			struct igt_vec4 yuv[4];
@@ -1563,7 +1577,7 @@ static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_uplo
 			uv[j * 2 + 1] = (yuv[0].d[2] + yuv[2].d[2]) / 2.0f;
 		}
 
-		if (fb->width & 1) {
+		if (cvt->width & 1) {
 			/* Convert 1x2 pixel block */
 			struct igt_vec4 rgb[2];
 			struct igt_vec4 yuv[2];
@@ -1592,8 +1606,8 @@ static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_uplo
 	}
 
 	/* Last row cannot be interpolated between 2 pixels, take the single value */
-	if (fb->height & 1) {
-		for (j = 0; j < fb->width / 2; j++) {
+	if (cvt->height & 1) {
+		for (j = 0; j < cvt->width / 2; j++) {
 			/* Convert 2x1 pixel blocks */
 			struct igt_vec4 rgb[2];
 			struct igt_vec4 yuv[2];
@@ -1610,7 +1624,7 @@ static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_uplo
 			uv[j * 2 + 1] = yuv[0].d[2];
 		}
 
-		if (fb->width & 1) {
+		if (cvt->width & 1) {
 			/* Convert single pixel */
 			struct igt_vec4 rgb;
 			struct igt_vec4 yuv;
@@ -1647,27 +1661,28 @@ static const unsigned char *yuyv_swizzle(uint32_t format)
 	}
 }
 
-static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
+static void convert_yuyv_to_rgb24(struct fb_convert *cvt)
 {
 	int i, j;
 	const uint8_t *yuyv;
-	uint8_t *rgb24 = blit->rgb24.map;
-	unsigned rgb24_stride = blit->rgb24.stride, yuyv_stride = blit->linear.stride;
-	uint8_t *buf = malloc(blit->linear.size);
-	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(fb->color_encoding,
-						    fb->color_range);
-	const unsigned char *swz = yuyv_swizzle(fb->drm_format);
+	uint8_t *rgb24 = cvt->dst.ptr;
+	unsigned int rgb24_stride = cvt->dst.fb->stride;
+	unsigned int yuyv_stride = cvt->src.fb->stride;
+	uint8_t *buf = malloc(cvt->src.fb->size);
+	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.fb->color_encoding,
+						    cvt->src.fb->color_range);
+	const unsigned char *swz = yuyv_swizzle(cvt->src.fb->drm_format);
 
 	/*
 	 * Reading from the BO is awfully slow because of lack of read caching,
 	 * it's faster to copy the whole BO to a temporary buffer and convert
 	 * from there.
 	 */
-	igt_memcpy_from_wc(buf, blit->linear.map, blit->linear.size);
+	igt_memcpy_from_wc(buf, cvt->src.ptr, cvt->src.fb->size);
 	yuyv = buf;
 
-	for (i = 0; i < fb->height; i++) {
-		for (j = 0; j < fb->width / 2; j++) {
+	for (i = 0; i < cvt->height; i++) {
+		for (j = 0; j < cvt->width / 2; j++) {
 			/* Convert 2x1 pixel blocks */
 			struct igt_vec4 yuv[2];
 			struct igt_vec4 rgb[2];
@@ -1685,7 +1700,7 @@ static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
 			write_rgb(&rgb24[j * 8 + 4], &rgb[1]);
 		}
 
-		if (fb->width & 1) {
+		if (cvt->width & 1) {
 			struct igt_vec4 yuv;
 			struct igt_vec4 rgb;
 
@@ -1706,25 +1721,25 @@ static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
 	free(buf);
 }
 
-static void convert_rgb24_to_yuyv(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
+static void convert_rgb24_to_yuyv(struct fb_convert *cvt)
 {
 	int i, j;
-	uint8_t *yuyv = blit->linear.map;
-	const uint8_t *rgb24 = blit->rgb24.map;
-	unsigned rgb24_stride = blit->rgb24.stride;
-	unsigned yuyv_stride = blit->linear.stride;
-	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(fb->color_encoding,
-						    fb->color_range);
-	const unsigned char *swz = yuyv_swizzle(fb->drm_format);
-
-	igt_assert_f(fb->drm_format == DRM_FORMAT_YUYV ||
-		     fb->drm_format == DRM_FORMAT_YVYU ||
-		     fb->drm_format == DRM_FORMAT_UYVY ||
-		     fb->drm_format == DRM_FORMAT_VYUY,
+	uint8_t *yuyv = cvt->dst.ptr;
+	const uint8_t *rgb24 = cvt->src.ptr;
+	unsigned rgb24_stride = cvt->src.fb->stride;
+	unsigned yuyv_stride = cvt->dst.fb->stride;
+	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.fb->color_encoding,
+						    cvt->dst.fb->color_range);
+	const unsigned char *swz = yuyv_swizzle(cvt->dst.fb->drm_format);
+
+	igt_assert_f(cvt->dst.fb->drm_format == DRM_FORMAT_YUYV ||
+		     cvt->dst.fb->drm_format == DRM_FORMAT_YVYU ||
+		     cvt->dst.fb->drm_format == DRM_FORMAT_UYVY ||
+		     cvt->dst.fb->drm_format == DRM_FORMAT_VYUY,
 		     "Conversion not implemented for !YUYV planar formats\n");
 
-	for (i = 0; i < fb->height; i++) {
-		for (j = 0; j < fb->width / 2; j++) {
+	for (i = 0; i < cvt->height; i++) {
+		for (j = 0; j < cvt->width / 2; j++) {
 			/* Convert 2x1 pixel blocks */
 			struct igt_vec4 rgb[2];
 			struct igt_vec4 yuv[2];
@@ -1741,7 +1756,7 @@ static void convert_rgb24_to_yuyv(struct igt_fb *fb, struct fb_convert_blit_uplo
 			yuyv[j * 4 + swz[3]] = (yuv[0].d[2] + yuv[1].d[2]) / 2.0f;
 		}
 
-		if (fb->width & 1) {
+		if (cvt->width & 1) {
 			struct igt_vec4 rgb;
 			struct igt_vec4 yuv;
 
@@ -1759,27 +1774,59 @@ static void convert_rgb24_to_yuyv(struct igt_fb *fb, struct fb_convert_blit_uplo
 	}
 }
 
+static void fb_convert(struct fb_convert *cvt)
+{
+	if (cvt->dst.fb->drm_format == DRM_FORMAT_RGB888) {
+		switch (cvt->src.fb->drm_format) {
+		case DRM_FORMAT_NV12:
+			convert_nv12_to_rgb24(cvt);
+			return;
+		case DRM_FORMAT_YUYV:
+		case DRM_FORMAT_YVYU:
+		case DRM_FORMAT_UYVY:
+		case DRM_FORMAT_VYUY:
+			convert_yuyv_to_rgb24(cvt);
+			return;
+		}
+	} else if (cvt->src.fb->drm_format == DRM_FORMAT_RGB888) {
+		switch (cvt->dst.fb->drm_format) {
+		case DRM_FORMAT_NV12:
+			convert_rgb24_to_nv12(cvt);
+			return;
+		case DRM_FORMAT_YUYV:
+		case DRM_FORMAT_YVYU:
+		case DRM_FORMAT_UYVY:
+		case DRM_FORMAT_VYUY:
+			convert_rgb24_to_yuyv(cvt);
+			return;
+		}
+	}
+
+	igt_assert_f(false,
+		     "Conversion not implemented (from format 0x%x to 0x%x)\n",
+		     cvt->src.fb->drm_format, cvt->dst.fb->drm_format);
+}
+
 static void destroy_cairo_surface__convert(void *arg)
 {
 	struct fb_convert_blit_upload *blit = arg;
 	struct igt_fb *fb = blit->fb;
-
-	/* Convert linear rgb back! */
-	switch(fb->drm_format) {
-	case DRM_FORMAT_NV12:
-		convert_rgb24_to_nv12(fb, blit);
-		break;
-	case DRM_FORMAT_YUYV:
-	case DRM_FORMAT_YVYU:
-	case DRM_FORMAT_UYVY:
-	case DRM_FORMAT_VYUY:
-		convert_rgb24_to_yuyv(fb, blit);
-		break;
-	default:
-		igt_assert_f(false, "Conversion not implemented for formats 0x%x\n",
-			     fb->drm_format);
-	}
-
+	struct fb_convert cvt = {
+		.width	= fb->width,
+		.height	= fb->height,
+
+		.dst	= {
+			.ptr	= blit->linear.map,
+			.fb	= blit->fb,
+		},
+
+		.src	= {
+			.ptr	= blit->shadow_ptr,
+			.fb	= &blit->shadow_fb,
+		},
+	};
+
+	fb_convert(&cvt);
 	igt_fb_unmap_buffer(&blit->shadow_fb, blit->shadow_ptr);
 	igt_remove_fb(blit->fd, &blit->shadow_fb);
 
@@ -1796,6 +1843,7 @@ static void destroy_cairo_surface__convert(void *arg)
 static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
 {
 	struct fb_convert_blit_upload *blit = malloc(sizeof(*blit));
+	struct fb_convert cvt = { 0 };
 	int shadow_id;
 
 	igt_assert(blit);
@@ -1823,21 +1871,13 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
 		memcpy(blit->linear.offsets, fb->offsets, sizeof(fb->offsets));
 	}
 
-	/* Convert to linear rgb! */
-	switch(fb->drm_format) {
-	case DRM_FORMAT_NV12:
-		convert_nv12_to_rgb24(fb, blit);
-		break;
-	case DRM_FORMAT_YUYV:
-	case DRM_FORMAT_YVYU:
-	case DRM_FORMAT_UYVY:
-	case DRM_FORMAT_VYUY:
-		convert_yuyv_to_rgb24(fb, blit);
-		break;
-	default:
-		igt_assert_f(false, "Conversion not implemented for formats 0x%x\n",
-			     fb->drm_format);
-	}
+	cvt.width = fb->width;
+	cvt.height = fb->height;
+	cvt.dst.ptr = blit->shadow_ptr;
+	cvt.dst.fb = &blit->shadow_fb;
+	cvt.src.ptr = blit->linear.map;
+	cvt.src.fb = blit->fb;
+	fb_convert(&cvt);
 
 	fb->cairo_surface =
 		cairo_image_surface_create_for_data(blit->shadow_ptr,
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH v7 05/14] fb: Add format conversion routine

[Maxime Ripard]

The chamelium format subtests will need to convert the reference pattern to
the format to be tested on the DRM device.

However, Cairo is very limited when it comes to format, and while pixman
has much more support for formats, it's still falling short compared to
what DRM exposes, especially on the YUV side. Plus, since we want to run
CRC checks on the frame, we can't afford having conversions back and forth
between RGB24, as the current API is doing.

In order to abstract this away, let's create a function that will convert a
igt_fb structure to another DRM format and return the converted igt_fb.

For now, we will use only cairo to do the conversion, but we will use other
libraries or roll our own routines to convert to more exotic formats and
abstract it away from the users.

Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_fb.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_fb.h |  2 ++
 2 files changed, 49 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index f829df59e9fa..4f63e0335f86 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2016,6 +2016,53 @@ void igt_remove_fb(int fd, struct igt_fb *fb)
 }
 
 /**
+ * igt_fb_convert:
+ * @dst: pointer to the #igt_fb structure that will store the conversion result
+ * @src: pointer to the #igt_fb structure that stores the frame we convert
+ * @dst_fourcc: DRM format specifier to convert to
+ *
+ * This will convert a given @src content to the @dst_fourcc format,
+ * storing the result in the @dst fb, allocating the @dst fb
+ * underlying buffer.
+ *
+ * Once done with @dst, the caller will have to call igt_remove_fb()
+ * on it to free the associated resources.
+ *
+ * Returns:
+ * The kms id of the created framebuffer.
+ */
+unsigned int igt_fb_convert(struct igt_fb *dst, struct igt_fb *src,
+			    uint32_t dst_fourcc)
+{
+	struct fb_convert cvt = { 0 };
+	void *dst_ptr, *src_ptr;
+	int fb_id;
+
+	fb_id = igt_create_fb(src->fd, src->width, src->height,
+			      dst_fourcc, LOCAL_DRM_FORMAT_MOD_NONE, dst);
+	igt_assert(fb_id > 0);
+
+	src_ptr = igt_fb_map_buffer(src->fd, src);
+	igt_assert(src_ptr);
+
+	dst_ptr = igt_fb_map_buffer(dst->fd, dst);
+	igt_assert(dst_ptr);
+
+	cvt.width = src->width;
+	cvt.height = src->height;
+	cvt.dst.ptr = dst_ptr;
+	cvt.dst.fb = dst;
+	cvt.src.ptr = src_ptr;
+	cvt.src.fb = src;
+	fb_convert(&cvt);
+
+	igt_fb_unmap_buffer(dst, dst_ptr);
+	igt_fb_unmap_buffer(src, src_ptr);
+
+	return fb_id;
+}
+
+/**
  * igt_bpp_depth_to_drm_format:
  * @bpp: desired bits per pixel
  * @depth: desired depth
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index ca028f550ab8..eb68b236530e 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -130,6 +130,8 @@ unsigned int igt_create_image_fb(int drm_fd,  int width, int height,
 				 struct igt_fb *fb /* out */);
 unsigned int igt_create_stereo_fb(int drm_fd, drmModeModeInfo *mode,
 				  uint32_t format, uint64_t tiling);
+unsigned int igt_fb_convert(struct igt_fb *dst, struct igt_fb *src,
+			    uint32_t dst_fourcc);
 void igt_remove_fb(int fd, struct igt_fb *fb);
 int igt_dirty_fb(int fd, struct igt_fb *fb);
 void *igt_fb_map_buffer(int fd, struct igt_fb *fb);
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH v7 06/14] fb: Fix ARGB8888 color depth

[Maxime Ripard]

The ARGB8888 has been listed as having a 32 bits depth, while it actually
is a 24 bits format. Fix this.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 4f63e0335f86..624fddbd465b 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -80,7 +80,7 @@ static struct format_desc_struct {
 	  .cairo_id = CAIRO_FORMAT_RGB30,
 	  .num_planes = 1, .plane_bpp = { 32, },
 	},
-	{ .name = "ARGB8888", .depth = 32, .drm_id = DRM_FORMAT_ARGB8888,
+	{ .name = "ARGB8888", .depth = 24, .drm_id = DRM_FORMAT_ARGB8888,
 	  .cairo_id = CAIRO_FORMAT_ARGB32,
 	  .num_planes = 1, .plane_bpp = { 32, },
 	},
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH v7 07/14] fb: Add support for conversions through pixman

[Maxime Ripard]

Pixman allows for much more conversions than cairo, and we don't want to
open code conversions routine for the common formats.

Let's plug pixman in our conversion function so that we can benefit from it
when possible.

Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_fb.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 2 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 624fddbd465b..64370e01c1d6 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -29,6 +29,7 @@
 #include <math.h>
 #include <wchar.h>
 #include <inttypes.h>
+#include <pixman.h>
 
 #include "drmtest.h"
 #include "igt_aux.h"
@@ -59,29 +60,36 @@
  * functions to work with these pixel format codes.
  */
 
+#define PIXMAN_invalid	0
+
 /* drm fourcc/cairo format maps */
 static struct format_desc_struct {
 	const char *name;
 	uint32_t drm_id;
 	cairo_format_t cairo_id;
+	pixman_format_code_t pixman_id;
 	int depth;
 	int num_planes;
 	int plane_bpp[4];
 } format_desc[] = {
 	{ .name = "RGB565", .depth = 16, .drm_id = DRM_FORMAT_RGB565,
 	  .cairo_id = CAIRO_FORMAT_RGB16_565,
+	  .pixman_id = PIXMAN_r5g6b5,
 	  .num_planes = 1, .plane_bpp = { 16, },
 	},
 	{ .name = "XRGB8888", .depth = 24, .drm_id = DRM_FORMAT_XRGB8888,
 	  .cairo_id = CAIRO_FORMAT_RGB24,
+	  .pixman_id = PIXMAN_x8r8g8b8,
 	  .num_planes = 1, .plane_bpp = { 32, },
 	},
 	{ .name = "XRGB2101010", .depth = 30, .drm_id = DRM_FORMAT_XRGB2101010,
 	  .cairo_id = CAIRO_FORMAT_RGB30,
+	  .pixman_id = PIXMAN_x2r10g10b10,
 	  .num_planes = 1, .plane_bpp = { 32, },
 	},
 	{ .name = "ARGB8888", .depth = 24, .drm_id = DRM_FORMAT_ARGB8888,
 	  .cairo_id = CAIRO_FORMAT_ARGB32,
+	  .pixman_id = PIXMAN_a8r8g8b8,
 	  .num_planes = 1, .plane_bpp = { 32, },
 	},
 	{ .name = "NV12", .depth = -1, .drm_id = DRM_FORMAT_NV12,
@@ -90,6 +98,7 @@ static struct format_desc_struct {
 	},
 	{ .name = "YUYV", .depth = -1, .drm_id = DRM_FORMAT_YUYV,
 	  .cairo_id = CAIRO_FORMAT_RGB24,
+	  .pixman_id = PIXMAN_yuy2,
 	  .num_planes = 1, .plane_bpp = { 16, },
 	},
 	{ .name = "YVYU", .depth = -1, .drm_id = DRM_FORMAT_YVYU,
@@ -1178,6 +1187,18 @@ unsigned int igt_create_stereo_fb(int drm_fd, drmModeModeInfo *mode,
 	return fb_id;
 }
 
+static pixman_format_code_t drm_format_to_pixman(uint32_t drm_format)
+{
+	struct format_desc_struct *f;
+
+	for_each_format(f)
+		if (f->drm_id == drm_format)
+			return f->pixman_id;
+
+	igt_assert_f(0, "can't find a pixman format for %08x (%s)\n",
+		     drm_format, igt_format_str(drm_format));
+}
+
 static cairo_format_t drm_format_to_cairo(uint32_t drm_format)
 {
 	struct format_desc_struct *f;
@@ -1774,9 +1795,38 @@ static void convert_rgb24_to_yuyv(struct fb_convert *cvt)
 	}
 }
 
+static void convert_pixman(struct fb_convert *cvt)
+{
+	pixman_format_code_t src_pixman = drm_format_to_pixman(cvt->src.fb->drm_format);
+	pixman_format_code_t dst_pixman = drm_format_to_pixman(cvt->dst.fb->drm_format);
+	pixman_image_t *dst_image, *src_image;
+
+	igt_assert((src_pixman != PIXMAN_invalid) &&
+		   (dst_pixman != PIXMAN_invalid));
+
+	src_image = pixman_image_create_bits(src_pixman,
+					     cvt->width, cvt->height,
+					     cvt->src.ptr, cvt->src.fb->stride);
+	igt_assert(src_image);
+
+	dst_image = pixman_image_create_bits(dst_pixman,
+					     cvt->width, cvt->height,
+					     cvt->dst.ptr, cvt->dst.fb->stride);
+	igt_assert(dst_image);
+
+	pixman_image_composite(PIXMAN_OP_SRC, src_image, NULL, dst_image,
+			       0, 0, 0, 0, 0, 0, cvt->width, cvt->height);
+	pixman_image_unref(dst_image);
+	pixman_image_unref(src_image);
+}
+
 static void fb_convert(struct fb_convert *cvt)
 {
-	if (cvt->dst.fb->drm_format == DRM_FORMAT_RGB888) {
+	if ((drm_format_to_pixman(cvt->src.fb->drm_format) != PIXMAN_invalid) &&
+	    (drm_format_to_pixman(cvt->dst.fb->drm_format) != PIXMAN_invalid)) {
+		convert_pixman(cvt);
+		return;
+	} else if (cvt->dst.fb->drm_format == DRM_FORMAT_RGB888) {
 		switch (cvt->src.fb->drm_format) {
 		case DRM_FORMAT_NV12:
 			convert_nv12_to_rgb24(cvt);
@@ -2130,7 +2180,8 @@ bool igt_fb_supported_format(uint32_t drm_format)
 
 	for_each_format(f)
 		if (f->drm_id == drm_format)
-			return f->cairo_id != CAIRO_FORMAT_INVALID;
+			return (f->cairo_id != CAIRO_FORMAT_INVALID) ||
+				(f->pixman_id != PIXMAN_invalid);
 
 	return false;
 }
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH v7 08/14] fb: Add depth lookup function

[Maxime Ripard]

There is currently a function to lookup the bits per pixels for a given DRM
format, but not for the depth of that format. Add this function.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_fb.c | 18 ++++++++++++++++++
 lib/igt_fb.h |  1 +
 2 files changed, 19 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 64370e01c1d6..f83ad0d2679c 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2153,6 +2153,24 @@ uint32_t igt_drm_format_to_bpp(uint32_t drm_format)
 }
 
 /**
+ * igt_drm_format_to_depth:
+ * @drm_format: drm fourcc pixel format code
+ *
+ * Returns:
+ * The bits per pixel for the given drm fourcc pixel format code. Fails hard if
+ * no match was found.
+ */
+uint32_t igt_drm_format_to_depth(uint32_t drm_format)
+{
+	struct format_desc_struct *f = lookup_drm_format(drm_format);
+
+	igt_assert_f(f, "can't find a depth for format %08x (%s)\n",
+		     drm_format, igt_format_str(drm_format));
+
+	return f->depth;
+}
+
+/**
  * igt_format_str:
  * @drm_format: drm fourcc pixel format code
  *
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index eb68b236530e..0273ad13899d 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -169,6 +169,7 @@ int igt_cairo_printf_line(cairo_t *cr, enum igt_text_align align,
 /* helpers to handle drm fourcc codes */
 uint32_t igt_bpp_depth_to_drm_format(int bpp, int depth);
 uint32_t igt_drm_format_to_bpp(uint32_t drm_format);
+uint32_t igt_drm_format_to_depth(uint32_t drm_format);
 const char *igt_format_str(uint32_t drm_format);
 bool igt_fb_supported_format(uint32_t drm_format);
 bool igt_format_is_yuv(uint32_t drm_format);
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH v7 09/14] fb: Add more formats

[Maxime Ripard]

We're going to need some DRM formats, and we're going to need the igt_fb
code to handle them. Since it relies on the format_desc structure to map
the DRM fourcc to the pixman and cairo formats, we need to add these new
formats to that structure.

Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_fb.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index f83ad0d2679c..425f98e23380 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -72,16 +72,46 @@ static struct format_desc_struct {
 	int num_planes;
 	int plane_bpp[4];
 } format_desc[] = {
+	{ .name = "ARGB1555", .depth = 16, .drm_id = DRM_FORMAT_ARGB1555,
+	  .cairo_id = CAIRO_FORMAT_INVALID,
+	  .pixman_id = PIXMAN_a1r5g5b5,
+	  .num_planes = 1, .plane_bpp = { 16, },
+	},
+	{ .name = "XRGB1555", .depth = 15, .drm_id = DRM_FORMAT_XRGB1555,
+	  .cairo_id = CAIRO_FORMAT_INVALID,
+	  .pixman_id = PIXMAN_x1r5g5b5,
+	  .num_planes = 1, .plane_bpp = { 16, },
+	},
 	{ .name = "RGB565", .depth = 16, .drm_id = DRM_FORMAT_RGB565,
 	  .cairo_id = CAIRO_FORMAT_RGB16_565,
 	  .pixman_id = PIXMAN_r5g6b5,
 	  .num_planes = 1, .plane_bpp = { 16, },
 	},
+	{ .name = "BGR565", .depth = 16, .drm_id = DRM_FORMAT_BGR565,
+	  .cairo_id = CAIRO_FORMAT_INVALID,
+	  .pixman_id = PIXMAN_b5g6r5,
+	  .num_planes = 1, .plane_bpp = { 16, },
+	},
+	{ .name = "BGR888", .depth = 24, .drm_id = DRM_FORMAT_BGR888,
+	  .cairo_id = CAIRO_FORMAT_INVALID,
+	  .pixman_id = PIXMAN_b8g8r8,
+	  .num_planes = 1, .plane_bpp = { 24, },
+	},
+	{ .name = "RGB888", .depth = 24, .drm_id = DRM_FORMAT_RGB888,
+	  .cairo_id = CAIRO_FORMAT_INVALID,
+	  .pixman_id = PIXMAN_r8g8b8,
+	  .num_planes = 1, .plane_bpp = { 24, },
+	},
 	{ .name = "XRGB8888", .depth = 24, .drm_id = DRM_FORMAT_XRGB8888,
 	  .cairo_id = CAIRO_FORMAT_RGB24,
 	  .pixman_id = PIXMAN_x8r8g8b8,
 	  .num_planes = 1, .plane_bpp = { 32, },
 	},
+	{ .name = "XBGR8888", .depth = 24, .drm_id = DRM_FORMAT_XBGR8888,
+	  .cairo_id = CAIRO_FORMAT_INVALID,
+	  .pixman_id = PIXMAN_x8b8g8r8,
+	  .num_planes = 1, .plane_bpp = { 32, },
+	},
 	{ .name = "XRGB2101010", .depth = 30, .drm_id = DRM_FORMAT_XRGB2101010,
 	  .cairo_id = CAIRO_FORMAT_RGB30,
 	  .pixman_id = PIXMAN_x2r10g10b10,
@@ -92,6 +122,11 @@ static struct format_desc_struct {
 	  .pixman_id = PIXMAN_a8r8g8b8,
 	  .num_planes = 1, .plane_bpp = { 32, },
 	},
+	{ .name = "ABGR8888", .depth = 24, .drm_id = DRM_FORMAT_ABGR8888,
+	  .cairo_id = CAIRO_FORMAT_INVALID,
+	  .pixman_id = PIXMAN_a8b8g8r8,
+	  .num_planes = 1, .plane_bpp = { 32, },
+	},
 	{ .name = "NV12", .depth = -1, .drm_id = DRM_FORMAT_NV12,
 	  .cairo_id = CAIRO_FORMAT_RGB24,
 	  .num_planes = 2, .plane_bpp = { 8, 16, },
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH v7 10/14] chamelium: Split CRC test function in two

[Maxime Ripard]

We have two use cases in our current sub-test suites: the tests that test
all the modes exposed by the driver, and the ones just picking up one.

Instead of having to deal with this two cases in the same function as it is
currently done, move the part that test a single mode into a separate
function, and just call it for every mode that we want to test if needs be.

This will result in a simpler function that will be easier to extend to
support formats.

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 tests/kms_chamelium.c | 120 +++++++++++++++++++++++++------------------
 1 file changed, 72 insertions(+), 48 deletions(-)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 2bc34d07788d..1e3a6fdaa44d 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -486,20 +486,60 @@ enable_output(data_t *data,
 	drmModeFreeConnector(connector);
 }
 
-static void
-test_display_crc(data_t *data, struct chamelium_port *port, int count,
-		 bool fast)
+static void do_test_display_crc(data_t *data, struct chamelium_port *port,
+				igt_output_t *output, drmModeModeInfo *mode,
+				int count)
 {
-	igt_output_t *output;
-	igt_plane_t *primary;
 	igt_crc_t *crc;
 	igt_crc_t *expected_crc;
 	struct chamelium_fb_crc_async_data *fb_crc;
 	struct igt_fb fb;
-	drmModeModeInfo *mode;
+	int i, fb_id, captured_frame_count;
+
+	fb_id = igt_create_color_pattern_fb(data->drm_fd,
+					    mode->hdisplay,
+					    mode->vdisplay,
+					    DRM_FORMAT_XRGB8888,
+					    LOCAL_DRM_FORMAT_MOD_NONE,
+					    0, 0, 0, &fb);
+	igt_assert(fb_id > 0);
+
+	fb_crc = chamelium_calculate_fb_crc_async_start(data->drm_fd,
+							&fb);
+
+	enable_output(data, port, output, mode, &fb);
+
+	/* We want to keep the display running for a little bit, since
+	 * there's always the potential the driver isn't able to keep
+	 * the display running properly for very long
+	 */
+	chamelium_capture(data->chamelium, port, 0, 0, 0, 0, count);
+	crc = chamelium_read_captured_crcs(data->chamelium,
+					   &captured_frame_count);
+
+	igt_assert(captured_frame_count == count);
+
+	igt_debug("Captured %d frames\n", captured_frame_count);
+
+	expected_crc = chamelium_calculate_fb_crc_async_finish(fb_crc);
+
+	for (i = 0; i < captured_frame_count; i++)
+		chamelium_assert_crc_eq_or_dump(data->chamelium,
+						expected_crc, &crc[i],
+						&fb, i);
+
+	free(expected_crc);
+	free(crc);
+
+	igt_remove_fb(data->drm_fd, &fb);
+}
+
+static void test_display_crc_one_mode(data_t *data, struct chamelium_port *port,
+				      int count)
+{
+	igt_output_t *output;
 	drmModeConnector *connector;
-	int fb_id, i, j, captured_frame_count;
-	int count_modes;
+	igt_plane_t *primary;
 
 	reset_state(data, port);
 
@@ -508,46 +548,30 @@ test_display_crc(data_t *data, struct chamelium_port *port, int count,
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	igt_assert(primary);
 
-	count_modes = fast ? 1 : connector->count_modes;
+	do_test_display_crc(data, port, output, &connector->modes[0], count);
 
-	for (i = 0; i < count_modes; i++) {
-		mode = &connector->modes[i];
-		fb_id = igt_create_color_pattern_fb(data->drm_fd,
-						    mode->hdisplay,
-						    mode->vdisplay,
-						    DRM_FORMAT_XRGB8888,
-						    LOCAL_DRM_FORMAT_MOD_NONE,
-						    0, 0, 0, &fb);
-		igt_assert(fb_id > 0);
-
-		fb_crc = chamelium_calculate_fb_crc_async_start(data->drm_fd,
-								&fb);
-
-		enable_output(data, port, output, mode, &fb);
-
-		/* We want to keep the display running for a little bit, since
-		 * there's always the potential the driver isn't able to keep
-		 * the display running properly for very long
-		 */
-		chamelium_capture(data->chamelium, port, 0, 0, 0, 0, count);
-		crc = chamelium_read_captured_crcs(data->chamelium,
-						   &captured_frame_count);
-
-		igt_assert(captured_frame_count == count);
+	drmModeFreeConnector(connector);
+}
 
-		igt_debug("Captured %d frames\n", captured_frame_count);
+static void test_display_crc_all_modes(data_t *data, struct chamelium_port *port,
+					int count)
+{
+	igt_output_t *output;
+	igt_plane_t *primary;
+	drmModeConnector *connector;
+	int i;
 
-		expected_crc = chamelium_calculate_fb_crc_async_finish(fb_crc);
+	reset_state(data, port);
 
-		for (j = 0; j < captured_frame_count; j++)
-			chamelium_assert_crc_eq_or_dump(data->chamelium,
-							expected_crc, &crc[j],
-							&fb, j);
+	output = prepare_output(data, port);
+	connector = chamelium_port_get_connector(data->chamelium, port, false);
+	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+	igt_assert(primary);
 
-		free(expected_crc);
-		free(crc);
+	for (i = 0; i < connector->count_modes; i++) {
+		drmModeModeInfo *mode = &connector->modes[i];
 
-		igt_remove_fb(data->drm_fd, &fb);
+		do_test_display_crc(data, port, output, mode, count);
 	}
 
 	drmModeFreeConnector(connector);
@@ -808,13 +832,13 @@ igt_main
 							edid_id, alt_edid_id);
 
 		connector_subtest("dp-crc-single", DisplayPort)
-			test_display_crc(&data, port, 1, false);
+			test_display_crc_all_modes(&data, port, 1);
 
 		connector_subtest("dp-crc-fast", DisplayPort)
-			test_display_crc(&data, port, 1, true);
+			test_display_crc_one_mode(&data, port, 1);
 
 		connector_subtest("dp-crc-multiple", DisplayPort)
-			test_display_crc(&data, port, 3, false);
+			test_display_crc_all_modes(&data, port, 3);
 
 		connector_subtest("dp-frame-dump", DisplayPort)
 			test_display_frame_dump(&data, port);
@@ -872,13 +896,13 @@ igt_main
 							edid_id, alt_edid_id);
 
 		connector_subtest("hdmi-crc-single", HDMIA)
-			test_display_crc(&data, port, 1, false);
+			test_display_crc_all_modes(&data, port, 1);
 
 		connector_subtest("hdmi-crc-fast", HDMIA)
-			test_display_crc(&data, port, 1, true);
+			test_display_crc_one_mode(&data, port, 1);
 
 		connector_subtest("hdmi-crc-multiple", HDMIA)
-			test_display_crc(&data, port, 3, false);
+			test_display_crc_all_modes(&data, port, 3);
 
 		connector_subtest("hdmi-frame-dump", HDMIA)
 			test_display_frame_dump(&data, port);
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH v7 11/14] chamelium: Change our pattern for a custom one if needed

[Maxime Ripard]

The current pattern being used is the one generated through the
igt_create_color_pattern_fb.

However, in order to deal with multiple formats and the upsampling /
downsampling issues that might arise from converting back and forth between
formats, we will need to have a pattern with quite precise color values,
and without any shades or gradient of colors.

Let's create a function that will generate that pattern in the chamelium
code if we need to convert the framebuffer to a smaller depth, and use the
current pattern otherwise.

The easiest way to do that will be to only use values that would have the
same part on the common most significant bits (5, to deal with most
formats) and have the same bit repeated on the least significant bits that
are going to be dropped and / or padded when converting between formats.

Pixman will fill the lowest bits with 1, and our hardware (this has been
tested on a Raspberry Pi's VC4) is able to support that, so the easiest is
to just use all 1's for our components in order to still be able to compute
the CRCs.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 tests/kms_chamelium.c | 59 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 53 insertions(+), 6 deletions(-)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 1e3a6fdaa44d..8d2b3f8e170d 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -486,6 +486,42 @@ enable_output(data_t *data,
 	drmModeFreeConnector(connector);
 }
 
+static void chamelium_paint_xr24_pattern(uint32_t *data,
+					 size_t width, size_t height)
+{
+	uint32_t colors[] = { 0xff000000,
+			      0xffff0000,
+			      0xff00ff00,
+			      0xff0000ff,
+			      0xffffffff };
+	unsigned i, j;
+
+	for (i = 0; i < height; i++)
+		for (j = 0; j < width; j++)
+			*(data + i * width + j) = colors[((j / 64) + (i / 64)) % 5];
+}
+
+static int chamelium_get_pattern_fb(data_t *data, drmModeModeInfo *mode,
+				    uint32_t fourcc, struct igt_fb *fb)
+{
+	int fb_id;
+	void *ptr;
+
+	igt_assert(fourcc == DRM_FORMAT_XRGB8888);
+
+	fb_id = igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+			      fourcc, LOCAL_DRM_FORMAT_MOD_NONE, fb);
+	igt_assert(fb_id > 0);
+
+	ptr = igt_fb_map_buffer(fb->fd, fb);
+	igt_assert(ptr);
+
+	chamelium_paint_xr24_pattern(ptr, mode->vdisplay, mode->hdisplay);
+	igt_fb_unmap_buffer(fb, ptr);
+
+	return fb_id;
+}
+
 static void do_test_display_crc(data_t *data, struct chamelium_port *port,
 				igt_output_t *output, drmModeModeInfo *mode,
 				int count)
@@ -496,12 +532,23 @@ static void do_test_display_crc(data_t *data, struct chamelium_port *port,
 	struct igt_fb fb;
 	int i, fb_id, captured_frame_count;
 
-	fb_id = igt_create_color_pattern_fb(data->drm_fd,
-					    mode->hdisplay,
-					    mode->vdisplay,
-					    DRM_FORMAT_XRGB8888,
-					    LOCAL_DRM_FORMAT_MOD_NONE,
-					    0, 0, 0, &fb);
+	/*
+	 * We only need to create a custom pattern if we want to
+	 * display a format that is either XRGB8888, or has the same
+	 * depth since we won't have any downsampling.
+	 */
+	if (igt_drm_format_to_depth(DRM_FORMAT_XRGB8888) ==
+	    igt_drm_format_to_depth(fourcc))
+		fb_id = igt_create_color_pattern_fb(data->drm_fd,
+						    mode->hdisplay,
+						    mode->vdisplay,
+						    DRM_FORMAT_XRGB8888,
+						    LOCAL_DRM_FORMAT_MOD_NONE,
+						    0, 0, 0, &fb);
+	else
+		fb_id = chamelium_get_pattern_fb(data, mode,
+						 DRM_FORMAT_XRGB8888, &fb);
+
 	igt_assert(fb_id > 0);
 
 	fb_crc = chamelium_calculate_fb_crc_async_start(data->drm_fd,
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH v7 12/14] chamelium: Add format support

[Maxime Ripard]

In order to introduce CRC subtests for DRM formats, we need to take an
intermediate step.

The current code will generate a pattern in XR24, and will try to compare
the CRC returned by the Chamelium for each frames.

This CRC is computed on an XR24 format as well, so it works. However, as
soon as we will start implementing other formats, if we just change the
format of the pattern, the raw content of the buffer, and therefore the
CRC's won't match anymore.

In order to address that, we will need an intermediate step, and we will
now still create the XR24 pattern, and compute its CRC, then convert it to
the format we want to test, and finally retrieve the CRC from the Chamelium
to compare it with the one from the XR24 pattern.

The current code is converted to the new prototype that will take the
fourcc of the format to test, even though we're still using XR24 everywhere
for now.

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 tests/kms_chamelium.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 8d2b3f8e170d..dbc020459a75 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -524,13 +524,14 @@ static int chamelium_get_pattern_fb(data_t *data, drmModeModeInfo *mode,
 
 static void do_test_display_crc(data_t *data, struct chamelium_port *port,
 				igt_output_t *output, drmModeModeInfo *mode,
-				int count)
+				uint32_t fourcc, int count)
 {
 	igt_crc_t *crc;
 	igt_crc_t *expected_crc;
 	struct chamelium_fb_crc_async_data *fb_crc;
-	struct igt_fb fb;
+	struct igt_fb frame_fb, fb;
 	int i, fb_id, captured_frame_count;
+	int frame_id;
 
 	/*
 	 * We only need to create a custom pattern if we want to
@@ -551,10 +552,13 @@ static void do_test_display_crc(data_t *data, struct chamelium_port *port,
 
 	igt_assert(fb_id > 0);
 
+	frame_id = igt_fb_convert(&frame_fb, &fb, fourcc);
+	igt_assert(frame_id > 0);
+
 	fb_crc = chamelium_calculate_fb_crc_async_start(data->drm_fd,
 							&fb);
 
-	enable_output(data, port, output, mode, &fb);
+	enable_output(data, port, output, mode, &frame_fb);
 
 	/* We want to keep the display running for a little bit, since
 	 * there's always the potential the driver isn't able to keep
@@ -578,11 +582,12 @@ static void do_test_display_crc(data_t *data, struct chamelium_port *port,
 	free(expected_crc);
 	free(crc);
 
+	igt_remove_fb(data->drm_fd, &frame_fb);
 	igt_remove_fb(data->drm_fd, &fb);
 }
 
 static void test_display_crc_one_mode(data_t *data, struct chamelium_port *port,
-				      int count)
+				      uint32_t fourcc, int count)
 {
 	igt_output_t *output;
 	drmModeConnector *connector;
@@ -595,13 +600,13 @@ static void test_display_crc_one_mode(data_t *data, struct chamelium_port *port,
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	igt_assert(primary);
 
-	do_test_display_crc(data, port, output, &connector->modes[0], count);
+	do_test_display_crc(data, port, output, &connector->modes[0], fourcc, count);
 
 	drmModeFreeConnector(connector);
 }
 
 static void test_display_crc_all_modes(data_t *data, struct chamelium_port *port,
-					int count)
+				       uint32_t fourcc, int count)
 {
 	igt_output_t *output;
 	igt_plane_t *primary;
@@ -618,7 +623,7 @@ static void test_display_crc_all_modes(data_t *data, struct chamelium_port *port
 	for (i = 0; i < connector->count_modes; i++) {
 		drmModeModeInfo *mode = &connector->modes[i];
 
-		do_test_display_crc(data, port, output, mode, count);
+		do_test_display_crc(data, port, output, mode, fourcc, count);
 	}
 
 	drmModeFreeConnector(connector);
@@ -879,13 +884,16 @@ igt_main
 							edid_id, alt_edid_id);
 
 		connector_subtest("dp-crc-single", DisplayPort)
-			test_display_crc_all_modes(&data, port, 1);
+			test_display_crc_all_modes(&data, port,
+						   DRM_FORMAT_XRGB8888, 1);
 
 		connector_subtest("dp-crc-fast", DisplayPort)
-			test_display_crc_one_mode(&data, port, 1);
+			test_display_crc_one_mode(&data, port,
+						  DRM_FORMAT_XRGB8888, 1);
 
 		connector_subtest("dp-crc-multiple", DisplayPort)
-			test_display_crc_all_modes(&data, port, 3);
+			test_display_crc_all_modes(&data, port,
+						   DRM_FORMAT_XRGB8888, 3);
 
 		connector_subtest("dp-frame-dump", DisplayPort)
 			test_display_frame_dump(&data, port);
@@ -943,13 +951,16 @@ igt_main
 							edid_id, alt_edid_id);
 
 		connector_subtest("hdmi-crc-single", HDMIA)
-			test_display_crc_all_modes(&data, port, 1);
+			test_display_crc_all_modes(&data, port,
+						   DRM_FORMAT_XRGB8888, 1);
 
 		connector_subtest("hdmi-crc-fast", HDMIA)
-			test_display_crc_one_mode(&data, port, 1);
+			test_display_crc_one_mode(&data, port,
+						  DRM_FORMAT_XRGB8888, 1);
 
 		connector_subtest("hdmi-crc-multiple", HDMIA)
-			test_display_crc_all_modes(&data, port, 3);
+			test_display_crc_all_modes(&data, port,
+						   DRM_FORMAT_XRGB8888, 3);
 
 		connector_subtest("hdmi-frame-dump", HDMIA)
 			test_display_frame_dump(&data, port);
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH v7 13/14] chamelium: Add format subtests

[Maxime Ripard]

Now that we have everything in place, we can add the support for the
subtests testing the output of planes setup with formats other than XR24.
Since YUV will be a bit trickier to handle, start with various common RGB
formats.

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 tests/kms_chamelium.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index dbc020459a75..1e78ab5cbfd6 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -962,6 +962,46 @@ igt_main
 			test_display_crc_all_modes(&data, port,
 						   DRM_FORMAT_XRGB8888, 3);
 
+		connector_subtest("hdmi-crc-argb8888", HDMIA)
+			test_display_crc_one_mode(&data, port,
+						  DRM_FORMAT_ARGB8888, 1);
+
+		connector_subtest("hdmi-crc-abgr8888", HDMIA)
+			test_display_crc_one_mode(&data, port,
+						  DRM_FORMAT_ABGR8888, 1);
+
+		connector_subtest("hdmi-crc-xrgb8888", HDMIA)
+			test_display_crc_one_mode(&data, port,
+						  DRM_FORMAT_XRGB8888, 1);
+
+		connector_subtest("hdmi-crc-xbgr8888", HDMIA)
+			test_display_crc_one_mode(&data, port,
+						  DRM_FORMAT_XBGR8888, 1);
+
+		connector_subtest("hdmi-crc-rgb888", HDMIA)
+			test_display_crc_one_mode(&data, port,
+						  DRM_FORMAT_RGB888, 1);
+
+		connector_subtest("hdmi-crc-bgr888", HDMIA)
+			test_display_crc_one_mode(&data, port,
+						  DRM_FORMAT_BGR888, 1);
+
+		connector_subtest("hdmi-crc-rgb565", HDMIA)
+			test_display_crc_one_mode(&data, port,
+						  DRM_FORMAT_RGB565, 1);
+
+		connector_subtest("hdmi-crc-bgr565", HDMIA)
+			test_display_crc_one_mode(&data, port,
+						  DRM_FORMAT_BGR565, 1);
+
+		connector_subtest("hdmi-crc-argb1555", HDMIA)
+			test_display_crc_one_mode(&data, port,
+						  DRM_FORMAT_ARGB1555, 1);
+
+		connector_subtest("hdmi-crc-xrgb1555", HDMIA)
+			test_display_crc_one_mode(&data, port,
+						  DRM_FORMAT_XRGB1555, 1);
+
 		connector_subtest("hdmi-frame-dump", HDMIA)
 			test_display_frame_dump(&data, port);
 	}
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH v7 14/14] tests: Add chamelium formats subtests to vc4 test lists

[Maxime Ripard]

Now that we have support for the format sub-tests, enable them in the vc4
chamelium test lists.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 tests/vc4_ci/vc4-chamelium-fast.testlist | 10 ++++++++++
 tests/vc4_ci/vc4-chamelium.testlist      | 10 ++++++++++
 2 files changed, 20 insertions(+)

diff --git a/tests/vc4_ci/vc4-chamelium-fast.testlist b/tests/vc4_ci/vc4-chamelium-fast.testlist
index 964167b82d00..dd45d12ab487 100644
--- a/tests/vc4_ci/vc4-chamelium-fast.testlist
+++ b/tests/vc4_ci/vc4-chamelium-fast.testlist
@@ -1,4 +1,14 @@
+igt@kms_chamelium@hdmi-crc-abgr8888
+igt@kms_chamelium@hdmi-crc-argb1555
+igt@kms_chamelium@hdmi-crc-argb8888
+igt@kms_chamelium@hdmi-crc-bgr565
+igt@kms_chamelium@hdmi-crc-bgr888
 igt@kms_chamelium@hdmi-crc-fast
+igt@kms_chamelium@hdmi-crc-rgb565
+igt@kms_chamelium@hdmi-crc-rgb888
+igt@kms_chamelium@hdmi-crc-xbgr8888
+igt@kms_chamelium@hdmi-crc-xrgb1555
+igt@kms_chamelium@hdmi-crc-xrgb8888
 igt@kms_chamelium@hdmi-edid-read
 igt@kms_chamelium@hdmi-hpd
 igt@kms_chamelium@hdmi-hpd-fast
diff --git a/tests/vc4_ci/vc4-chamelium.testlist b/tests/vc4_ci/vc4-chamelium.testlist
index b00f54cd9c46..d3d4104acf48 100644
--- a/tests/vc4_ci/vc4-chamelium.testlist
+++ b/tests/vc4_ci/vc4-chamelium.testlist
@@ -1,6 +1,16 @@
+igt@kms_chamelium@hdmi-crc-abgr8888
+igt@kms_chamelium@hdmi-crc-argb1555
+igt@kms_chamelium@hdmi-crc-argb8888
+igt@kms_chamelium@hdmi-crc-bgr565
+igt@kms_chamelium@hdmi-crc-bgr888
 igt@kms_chamelium@hdmi-crc-fast
 igt@kms_chamelium@hdmi-crc-multiple
+igt@kms_chamelium@hdmi-crc-rgb565
+igt@kms_chamelium@hdmi-crc-rgb888
 igt@kms_chamelium@hdmi-crc-single
+igt@kms_chamelium@hdmi-crc-xbgr8888
+igt@kms_chamelium@hdmi-crc-xrgb1555
+igt@kms_chamelium@hdmi-crc-xrgb8888
 igt@kms_chamelium@hdmi-edid-read
 igt@kms_chamelium@hdmi-frame-dump
 igt@kms_chamelium@hdmi-hpd
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for chamelium: Test the plane formats (rev6)

[Patchwork]

== Series Details ==

Series: chamelium: Test the plane formats (rev6)
URL   : https://patchwork.freedesktop.org/series/42165/
State : failure

== Summary ==

Applying: fb: Add buffer map/unmap functions
Applying: fb: Use an igt_fb for the cairo shadow buffer
Patch failed at 0002 fb: Use an igt_fb for the cairo shadow buffer
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

Re: [igt-dev] [PATCH v7 00/14] chamelium: Test the plane formats

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1267 bytes --]

On Tue, Sep 11, 2018 at 10:47:27AM +0200, Maxime Ripard wrote:
> Here is another attempt at a serie that aims at starting to test the plane
> formats using the Chamelium over the HDMI. This was tested using the vc4
> DRM driver found on the RaspberryPi.
> 
> There's been a number of changes from the RFC thanks to the feedback from
> Eric Anholt, Paul Kocialkowski and Ville Syrjälä.
> 
> The series now relies mostly on igt_fb, with some decoupling from cairo and
> a bunch of new helpers to aim at being able to convert igt_fb to arbitrary
> DRM formats. The list of formats have been extended a bit, but is quite
> small at this point. We also rely solely on either pixman or the existing
> format conversions functions at them moment, and we allow cairo surfaces to
> be created for framebuffers in a format not supported by cairo, through an
> intermediate conversion to RGB24.
> 
> However, since it's now abstracted away from the igt_fb users, we can
> easily add some routines to convert to additional formats if needed. And
> since the code was so close now, it's been integrated into kms_chamelium.

Can we make any progress on this?

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH v7 04/14] fb: Create common function to convert frame formats

[Ville Syrjälä]

On Tue, Sep 11, 2018 at 10:47:31AM +0200, Maxime Ripard wrote:
> The current code to convert between two buffer formats is quite tied to the
> cairo surface infrastructure. Since we'll want to reuse it, make that
> function more generic by introducing a common structure that passes all the
> arguments and a common function that will call the right functions we
> needed.
> 
> Reviewed-by: Eric Anholt <eric@anholt.net>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  lib/igt_fb.c | 220 ++++++++++++++++++++++++++++++----------------------
>  1 file changed, 130 insertions(+), 90 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 66bdc02a0ce7..f829df59e9fa 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -1391,6 +1391,19 @@ struct fb_convert_blit_upload {
>  	struct fb_blit_linear linear;
>  };
>  
> +struct fb_convert_buf {
> +	void			*ptr;
> +	struct igt_fb		*fb;
> +};
> +
> +struct fb_convert {
> +	unsigned int		width;
> +	unsigned int		height;

Are we expecting to convert between different sized fbs, or why do we
need these?

> +
> +	struct fb_convert_buf	dst;
> +	struct fb_convert_buf	src;
> +};
> +
>  static uint8_t clamprgb(float val)
>  {
>  	return clamp((int)(val + 0.5f), 0, 255);
> @@ -1411,27 +1424,28 @@ static void write_rgb(uint8_t *rgb24, const struct igt_vec4 *rgb)
>  	rgb24[0] = clamprgb(rgb->d[2]);
>  }
>  
> -static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
> +static void convert_nv12_to_rgb24(struct fb_convert *cvt)
>  {
>  	int i, j;
>  	const uint8_t *y, *uv;
> -	uint8_t *rgb24 = blit->rgb24.map;
> -	unsigned rgb24_stride = blit->rgb24.stride, planar_stride = blit->linear.stride;
> -	uint8_t *buf = malloc(blit->linear.size);
> -	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(fb->color_encoding,
> -						    fb->color_range);
> +	uint8_t *rgb24 = cvt->dst.ptr;
> +	unsigned int rgb24_stride = cvt->dst.fb->stride;
> +	unsigned int planar_stride = cvt->src.fb->stride;
> +	uint8_t *buf = malloc(cvt->src.fb->size);
> +	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.fb->color_encoding,
> +						    cvt->src.fb->color_range);
>  
>  	/*
>  	 * Reading from the BO is awfully slow because of lack of read caching,
>  	 * it's faster to copy the whole BO to a temporary buffer and convert
>  	 * from there.
>  	 */
> -	igt_memcpy_from_wc(buf, blit->linear.map, blit->linear.size);
> -	y = &buf[blit->linear.offsets[0]];
> -	uv = &buf[blit->linear.offsets[1]];
> +	igt_memcpy_from_wc(buf, cvt->src.ptr, cvt->src.fb->size);
> +	y = cvt->src.ptr + cvt->src.fb->offsets[0];
> +	uv = cvt->src.ptr + cvt->src.fb->offsets[1];
>  
> -	for (i = 0; i < fb->height / 2; i++) {
> -		for (j = 0; j < fb->width / 2; j++) {
> +	for (i = 0; i < cvt->height / 2; i++) {
> +		for (j = 0; j < cvt->width / 2; j++) {
>  			/* Convert 2x2 pixel blocks */
>  			struct igt_vec4 yuv[4];
>  			struct igt_vec4 rgb[4];
> @@ -1456,7 +1470,7 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
>  			write_rgb(&rgb24[j * 8 + 4 + rgb24_stride], &rgb[3]);
>  		}
>  
> -		if (fb->width & 1) {
> +		if (cvt->width & 1) {
>  			/* Convert 1x2 pixel block */
>  			struct igt_vec4 yuv[2];
>  			struct igt_vec4 rgb[2];
> @@ -1480,9 +1494,9 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
>  		uv += planar_stride;
>  	}
>  
> -	if (fb->height & 1) {
> +	if (cvt->height & 1) {
>  		/* Convert last row */
> -		for (j = 0; j < fb->width / 2; j++) {
> +		for (j = 0; j < cvt->width / 2; j++) {
>  			/* Convert 2x1 pixel blocks */
>  			struct igt_vec4 yuv[2];
>  			struct igt_vec4 rgb[2];
> @@ -1500,7 +1514,7 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
>  			write_rgb(&rgb24[j * 8 + 4], &rgb[0]);
>  		}
>  
> -		if (fb->width & 1) {
> +		if (cvt->width & 1) {
>  			/* Convert single pixel */
>  			struct igt_vec4 yuv;
>  			struct igt_vec4 rgb;
> @@ -1519,22 +1533,22 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
>  	free(buf);
>  }
>  
> -static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
> +static void convert_rgb24_to_nv12(struct fb_convert *cvt)
>  {
>  	int i, j;
> -	uint8_t *y = &blit->linear.map[blit->linear.offsets[0]];
> -	uint8_t *uv = &blit->linear.map[blit->linear.offsets[1]];
> -	const uint8_t *rgb24 = blit->rgb24.map;
> -	unsigned rgb24_stride = blit->rgb24.stride;
> -	unsigned planar_stride = blit->linear.stride;
> -	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(fb->color_encoding,
> -						    fb->color_range);
> -
> -	igt_assert_f(fb->drm_format == DRM_FORMAT_NV12,
> +	uint8_t *y = cvt->dst.ptr + cvt->dst.fb->offsets[0];
> +	uint8_t *uv = cvt->dst.ptr + cvt->dst.fb->offsets[1];
> +	const uint8_t *rgb24 = cvt->src.ptr;
> +	unsigned rgb24_stride = cvt->src.fb->stride;
> +	unsigned planar_stride = cvt->dst.fb->stride;
> +	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.fb->color_encoding,
> +						    cvt->dst.fb->color_range);
> +
> +	igt_assert_f(cvt->dst.fb->drm_format == DRM_FORMAT_NV12,
>  		     "Conversion not implemented for !NV12 planar formats\n");
>  
> -	for (i = 0; i < fb->height / 2; i++) {
> -		for (j = 0; j < fb->width / 2; j++) {
> +	for (i = 0; i < cvt->height / 2; i++) {
> +		for (j = 0; j < cvt->width / 2; j++) {
>  			/* Convert 2x2 pixel blocks */
>  			struct igt_vec4 rgb[4];
>  			struct igt_vec4 yuv[4];
> @@ -1563,7 +1577,7 @@ static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_uplo
>  			uv[j * 2 + 1] = (yuv[0].d[2] + yuv[2].d[2]) / 2.0f;
>  		}
>  
> -		if (fb->width & 1) {
> +		if (cvt->width & 1) {
>  			/* Convert 1x2 pixel block */
>  			struct igt_vec4 rgb[2];
>  			struct igt_vec4 yuv[2];
> @@ -1592,8 +1606,8 @@ static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_uplo
>  	}
>  
>  	/* Last row cannot be interpolated between 2 pixels, take the single value */
> -	if (fb->height & 1) {
> -		for (j = 0; j < fb->width / 2; j++) {
> +	if (cvt->height & 1) {
> +		for (j = 0; j < cvt->width / 2; j++) {
>  			/* Convert 2x1 pixel blocks */
>  			struct igt_vec4 rgb[2];
>  			struct igt_vec4 yuv[2];
> @@ -1610,7 +1624,7 @@ static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_uplo
>  			uv[j * 2 + 1] = yuv[0].d[2];
>  		}
>  
> -		if (fb->width & 1) {
> +		if (cvt->width & 1) {
>  			/* Convert single pixel */
>  			struct igt_vec4 rgb;
>  			struct igt_vec4 yuv;
> @@ -1647,27 +1661,28 @@ static const unsigned char *yuyv_swizzle(uint32_t format)
>  	}
>  }
>  
> -static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
> +static void convert_yuyv_to_rgb24(struct fb_convert *cvt)
>  {
>  	int i, j;
>  	const uint8_t *yuyv;
> -	uint8_t *rgb24 = blit->rgb24.map;
> -	unsigned rgb24_stride = blit->rgb24.stride, yuyv_stride = blit->linear.stride;
> -	uint8_t *buf = malloc(blit->linear.size);
> -	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(fb->color_encoding,
> -						    fb->color_range);
> -	const unsigned char *swz = yuyv_swizzle(fb->drm_format);
> +	uint8_t *rgb24 = cvt->dst.ptr;
> +	unsigned int rgb24_stride = cvt->dst.fb->stride;
> +	unsigned int yuyv_stride = cvt->src.fb->stride;
> +	uint8_t *buf = malloc(cvt->src.fb->size);
> +	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.fb->color_encoding,
> +						    cvt->src.fb->color_range);
> +	const unsigned char *swz = yuyv_swizzle(cvt->src.fb->drm_format);
>  
>  	/*
>  	 * Reading from the BO is awfully slow because of lack of read caching,
>  	 * it's faster to copy the whole BO to a temporary buffer and convert
>  	 * from there.
>  	 */
> -	igt_memcpy_from_wc(buf, blit->linear.map, blit->linear.size);
> +	igt_memcpy_from_wc(buf, cvt->src.ptr, cvt->src.fb->size);
>  	yuyv = buf;
>  
> -	for (i = 0; i < fb->height; i++) {
> -		for (j = 0; j < fb->width / 2; j++) {
> +	for (i = 0; i < cvt->height; i++) {
> +		for (j = 0; j < cvt->width / 2; j++) {
>  			/* Convert 2x1 pixel blocks */
>  			struct igt_vec4 yuv[2];
>  			struct igt_vec4 rgb[2];
> @@ -1685,7 +1700,7 @@ static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
>  			write_rgb(&rgb24[j * 8 + 4], &rgb[1]);
>  		}
>  
> -		if (fb->width & 1) {
> +		if (cvt->width & 1) {
>  			struct igt_vec4 yuv;
>  			struct igt_vec4 rgb;
>  
> @@ -1706,25 +1721,25 @@ static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
>  	free(buf);
>  }
>  
> -static void convert_rgb24_to_yuyv(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
> +static void convert_rgb24_to_yuyv(struct fb_convert *cvt)
>  {
>  	int i, j;
> -	uint8_t *yuyv = blit->linear.map;
> -	const uint8_t *rgb24 = blit->rgb24.map;
> -	unsigned rgb24_stride = blit->rgb24.stride;
> -	unsigned yuyv_stride = blit->linear.stride;
> -	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(fb->color_encoding,
> -						    fb->color_range);
> -	const unsigned char *swz = yuyv_swizzle(fb->drm_format);
> -
> -	igt_assert_f(fb->drm_format == DRM_FORMAT_YUYV ||
> -		     fb->drm_format == DRM_FORMAT_YVYU ||
> -		     fb->drm_format == DRM_FORMAT_UYVY ||
> -		     fb->drm_format == DRM_FORMAT_VYUY,
> +	uint8_t *yuyv = cvt->dst.ptr;
> +	const uint8_t *rgb24 = cvt->src.ptr;
> +	unsigned rgb24_stride = cvt->src.fb->stride;
> +	unsigned yuyv_stride = cvt->dst.fb->stride;
> +	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.fb->color_encoding,
> +						    cvt->dst.fb->color_range);
> +	const unsigned char *swz = yuyv_swizzle(cvt->dst.fb->drm_format);
> +
> +	igt_assert_f(cvt->dst.fb->drm_format == DRM_FORMAT_YUYV ||
> +		     cvt->dst.fb->drm_format == DRM_FORMAT_YVYU ||
> +		     cvt->dst.fb->drm_format == DRM_FORMAT_UYVY ||
> +		     cvt->dst.fb->drm_format == DRM_FORMAT_VYUY,
>  		     "Conversion not implemented for !YUYV planar formats\n");
>  
> -	for (i = 0; i < fb->height; i++) {
> -		for (j = 0; j < fb->width / 2; j++) {
> +	for (i = 0; i < cvt->height; i++) {
> +		for (j = 0; j < cvt->width / 2; j++) {
>  			/* Convert 2x1 pixel blocks */
>  			struct igt_vec4 rgb[2];
>  			struct igt_vec4 yuv[2];
> @@ -1741,7 +1756,7 @@ static void convert_rgb24_to_yuyv(struct igt_fb *fb, struct fb_convert_blit_uplo
>  			yuyv[j * 4 + swz[3]] = (yuv[0].d[2] + yuv[1].d[2]) / 2.0f;
>  		}
>  
> -		if (fb->width & 1) {
> +		if (cvt->width & 1) {
>  			struct igt_vec4 rgb;
>  			struct igt_vec4 yuv;
>  
> @@ -1759,27 +1774,59 @@ static void convert_rgb24_to_yuyv(struct igt_fb *fb, struct fb_convert_blit_uplo
>  	}
>  }
>  
> +static void fb_convert(struct fb_convert *cvt)
> +{
> +	if (cvt->dst.fb->drm_format == DRM_FORMAT_RGB888) {
> +		switch (cvt->src.fb->drm_format) {
> +		case DRM_FORMAT_NV12:
> +			convert_nv12_to_rgb24(cvt);
> +			return;
> +		case DRM_FORMAT_YUYV:
> +		case DRM_FORMAT_YVYU:
> +		case DRM_FORMAT_UYVY:
> +		case DRM_FORMAT_VYUY:
> +			convert_yuyv_to_rgb24(cvt);
> +			return;
> +		}
> +	} else if (cvt->src.fb->drm_format == DRM_FORMAT_RGB888) {
> +		switch (cvt->dst.fb->drm_format) {
> +		case DRM_FORMAT_NV12:
> +			convert_rgb24_to_nv12(cvt);
> +			return;
> +		case DRM_FORMAT_YUYV:
> +		case DRM_FORMAT_YVYU:
> +		case DRM_FORMAT_UYVY:
> +		case DRM_FORMAT_VYUY:
> +			convert_rgb24_to_yuyv(cvt);
> +			return;
> +		}
> +	}
> +
> +	igt_assert_f(false,
> +		     "Conversion not implemented (from format 0x%x to 0x%x)\n",
> +		     cvt->src.fb->drm_format, cvt->dst.fb->drm_format);
> +}
> +
>  static void destroy_cairo_surface__convert(void *arg)
>  {
>  	struct fb_convert_blit_upload *blit = arg;
>  	struct igt_fb *fb = blit->fb;
> -
> -	/* Convert linear rgb back! */
> -	switch(fb->drm_format) {
> -	case DRM_FORMAT_NV12:
> -		convert_rgb24_to_nv12(fb, blit);
> -		break;
> -	case DRM_FORMAT_YUYV:
> -	case DRM_FORMAT_YVYU:
> -	case DRM_FORMAT_UYVY:
> -	case DRM_FORMAT_VYUY:
> -		convert_rgb24_to_yuyv(fb, blit);
> -		break;
> -	default:
> -		igt_assert_f(false, "Conversion not implemented for formats 0x%x\n",
> -			     fb->drm_format);
> -	}
> -
> +	struct fb_convert cvt = {
> +		.width	= fb->width,
> +		.height	= fb->height,
> +
> +		.dst	= {
> +			.ptr	= blit->linear.map,
> +			.fb	= blit->fb,
> +		},
> +
> +		.src	= {
> +			.ptr	= blit->shadow_ptr,
> +			.fb	= &blit->shadow_fb,
> +		},
> +	};
> +
> +	fb_convert(&cvt);
>  	igt_fb_unmap_buffer(&blit->shadow_fb, blit->shadow_ptr);
>  	igt_remove_fb(blit->fd, &blit->shadow_fb);
>  
> @@ -1796,6 +1843,7 @@ static void destroy_cairo_surface__convert(void *arg)
>  static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
>  {
>  	struct fb_convert_blit_upload *blit = malloc(sizeof(*blit));
> +	struct fb_convert cvt = { 0 };
>  	int shadow_id;
>  
>  	igt_assert(blit);
> @@ -1823,21 +1871,13 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
>  		memcpy(blit->linear.offsets, fb->offsets, sizeof(fb->offsets));
>  	}
>  
> -	/* Convert to linear rgb! */
> -	switch(fb->drm_format) {
> -	case DRM_FORMAT_NV12:
> -		convert_nv12_to_rgb24(fb, blit);
> -		break;
> -	case DRM_FORMAT_YUYV:
> -	case DRM_FORMAT_YVYU:
> -	case DRM_FORMAT_UYVY:
> -	case DRM_FORMAT_VYUY:
> -		convert_yuyv_to_rgb24(fb, blit);
> -		break;
> -	default:
> -		igt_assert_f(false, "Conversion not implemented for formats 0x%x\n",
> -			     fb->drm_format);
> -	}
> +	cvt.width = fb->width;
> +	cvt.height = fb->height;
> +	cvt.dst.ptr = blit->shadow_ptr;
> +	cvt.dst.fb = &blit->shadow_fb;
> +	cvt.src.ptr = blit->linear.map;
> +	cvt.src.fb = blit->fb;
> +	fb_convert(&cvt);
>  
>  	fb->cairo_surface =
>  		cairo_image_surface_create_for_data(blit->shadow_ptr,
> -- 
> git-series 0.9.1

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

Re: [igt-dev] [PATCH v7 09/14] fb: Add more formats

[Ville Syrjälä]

On Tue, Sep 11, 2018 at 10:47:36AM +0200, Maxime Ripard wrote:
> We're going to need some DRM formats, and we're going to need the igt_fb
> code to handle them. Since it relies on the format_desc structure to map
> the DRM fourcc to the pixman and cairo formats, we need to add these new
> formats to that structure.
> 
> Reviewed-by: Eric Anholt <eric@anholt.net>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  lib/igt_fb.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index f83ad0d2679c..425f98e23380 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -72,16 +72,46 @@ static struct format_desc_struct {
>  	int num_planes;
>  	int plane_bpp[4];
>  } format_desc[] = {
> +	{ .name = "ARGB1555", .depth = 16, .drm_id = DRM_FORMAT_ARGB1555,
> +	  .cairo_id = CAIRO_FORMAT_INVALID,
> +	  .pixman_id = PIXMAN_a1r5g5b5,
> +	  .num_planes = 1, .plane_bpp = { 16, },
> +	},
> +	{ .name = "XRGB1555", .depth = 15, .drm_id = DRM_FORMAT_XRGB1555,
> +	  .cairo_id = CAIRO_FORMAT_INVALID,
> +	  .pixman_id = PIXMAN_x1r5g5b5,
> +	  .num_planes = 1, .plane_bpp = { 16, },
> +	},

I think this is going to mess up the depth->format behaviour that
bunch of tests expect. So nak on adding .depth to these.

>  	{ .name = "RGB565", .depth = 16, .drm_id = DRM_FORMAT_RGB565,
>  	  .cairo_id = CAIRO_FORMAT_RGB16_565,
>  	  .pixman_id = PIXMAN_r5g6b5,
>  	  .num_planes = 1, .plane_bpp = { 16, },
>  	},
> +	{ .name = "BGR565", .depth = 16, .drm_id = DRM_FORMAT_BGR565,
> +	  .cairo_id = CAIRO_FORMAT_INVALID,
> +	  .pixman_id = PIXMAN_b5g6r5,
> +	  .num_planes = 1, .plane_bpp = { 16, },
> +	},
> +	{ .name = "BGR888", .depth = 24, .drm_id = DRM_FORMAT_BGR888,
> +	  .cairo_id = CAIRO_FORMAT_INVALID,
> +	  .pixman_id = PIXMAN_b8g8r8,
> +	  .num_planes = 1, .plane_bpp = { 24, },
> +	},
> +	{ .name = "RGB888", .depth = 24, .drm_id = DRM_FORMAT_RGB888,
> +	  .cairo_id = CAIRO_FORMAT_INVALID,
> +	  .pixman_id = PIXMAN_r8g8b8,
> +	  .num_planes = 1, .plane_bpp = { 24, },
> +	},
>  	{ .name = "XRGB8888", .depth = 24, .drm_id = DRM_FORMAT_XRGB8888,
>  	  .cairo_id = CAIRO_FORMAT_RGB24,
>  	  .pixman_id = PIXMAN_x8r8g8b8,
>  	  .num_planes = 1, .plane_bpp = { 32, },
>  	},
> +	{ .name = "XBGR8888", .depth = 24, .drm_id = DRM_FORMAT_XBGR8888,
> +	  .cairo_id = CAIRO_FORMAT_INVALID,
> +	  .pixman_id = PIXMAN_x8b8g8r8,
> +	  .num_planes = 1, .plane_bpp = { 32, },
> +	},
>  	{ .name = "XRGB2101010", .depth = 30, .drm_id = DRM_FORMAT_XRGB2101010,
>  	  .cairo_id = CAIRO_FORMAT_RGB30,
>  	  .pixman_id = PIXMAN_x2r10g10b10,
> @@ -92,6 +122,11 @@ static struct format_desc_struct {
>  	  .pixman_id = PIXMAN_a8r8g8b8,
>  	  .num_planes = 1, .plane_bpp = { 32, },
>  	},
> +	{ .name = "ABGR8888", .depth = 24, .drm_id = DRM_FORMAT_ABGR8888,
> +	  .cairo_id = CAIRO_FORMAT_INVALID,
> +	  .pixman_id = PIXMAN_a8b8g8r8,
> +	  .num_planes = 1, .plane_bpp = { 32, },
> +	},
>  	{ .name = "NV12", .depth = -1, .drm_id = DRM_FORMAT_NV12,
>  	  .cairo_id = CAIRO_FORMAT_RGB24,
>  	  .num_planes = 2, .plane_bpp = { 8, 16, },
> -- 
> git-series 0.9.1

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

Re: [igt-dev] [PATCH v7 02/14] fb: Use an igt_fb for the cairo shadow buffer

[Ville Syrjälä]

On Tue, Sep 11, 2018 at 10:47:29AM +0200, Maxime Ripard wrote:
> In the case where an igt_fb user wants to get a cairo surface out of that
> framebuffer, if the format of that framebuffer cannot be imported as-is in
> Cairo, the current code will do an anonymous mapping to create a shadow
> buffer where an RGB24 copy of that buffer will be created.
> 
> However, making that shadow buffer into an igt_fb itself will help us do
> further improvements on the conversion code.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  lib/igt_fb.c | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 542c62610412..dd180c6c8016 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -1381,12 +1381,12 @@ static void create_cairo_surface__gtt(int fd, struct igt_fb *fb)
>  
>  struct fb_convert_blit_upload {
>  	int fd;
> +
>  	struct igt_fb *fb;
> +	uint8_t *ptr;
>  
> -	struct {
> -		uint8_t *map;
> -		unsigned stride, size;
> -	} rgb24;
> +	struct igt_fb shadow_fb;
> +	uint8_t *shadow_ptr;

We seem to be pairing fb and ptr a lot. I wonder if we should suck the
the ptr into the fb struct. But maybe someone wants to map the same
thing multiple times? Not sure.

>  
>  	struct fb_blit_linear linear;
>  };
> @@ -1780,7 +1780,8 @@ static void destroy_cairo_surface__convert(void *arg)
>  			     fb->drm_format);
>  	}
>  
> -	munmap(blit->rgb24.map, blit->rgb24.size);
> +	igt_fb_unmap_buffer(&blit->shadow_fb, blit->shadow_ptr);
> +	igt_remove_fb(blit->fd, &blit->shadow_fb);
>  
>  	if (blit->linear.handle)
>  		free_linear_mapping(blit->fd, blit->fb, &blit->linear);
> @@ -1795,14 +1796,19 @@ static void destroy_cairo_surface__convert(void *arg)
>  static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
>  {
>  	struct fb_convert_blit_upload *blit = malloc(sizeof(*blit));
> +	int shadow_id;
> +
>  	igt_assert(blit);
>  
>  	blit->fd = fd;
>  	blit->fb = fb;
> -	blit->rgb24.stride = ALIGN(fb->width * 4, 16);
> -	blit->rgb24.size = ALIGN(blit->rgb24.stride * fb->height, sysconf(_SC_PAGESIZE));
> -	blit->rgb24.map = mmap(NULL, blit->rgb24.size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> -	igt_assert(blit->rgb24.map != MAP_FAILED);
> +
> +	shadow_id = igt_create_fb(fd, fb->width, fb->height, DRM_FORMAT_RGB888,
> +				  LOCAL_DRM_FORMAT_MOD_NONE, &blit->shadow_fb);
> +	igt_assert(shadow_id > 0);

We can't actually expect addfb to work for RGB888. That thing is not
supported by most hardware. Since my series adding fb_init() etc. hasn't
gotten reviewed I guess we'll just need to populate the igt_fb by hand
here, or something.

> +
> +	blit->shadow_ptr = igt_fb_map_buffer(fd, &blit->shadow_fb);
> +	igt_assert(blit->shadow_ptr);
>  
>  	if (fb->tiling == LOCAL_I915_FORMAT_MOD_Y_TILED ||
>  	    fb->tiling == LOCAL_I915_FORMAT_MOD_Yf_TILED) {
> @@ -1834,10 +1840,10 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
>  	}
>  
>  	fb->cairo_surface =
> -		cairo_image_surface_create_for_data(blit->rgb24.map,
> +		cairo_image_surface_create_for_data(blit->shadow_ptr,
>  						    CAIRO_FORMAT_RGB24,
>  						    fb->width, fb->height,
> -						    blit->rgb24.stride);
> +						    blit->shadow_fb.stride);
>  
>  	cairo_surface_set_user_data(fb->cairo_surface,
>  				    (cairo_user_data_key_t *)create_cairo_surface__convert,
> -- 
> git-series 0.9.1

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

Re: [igt-dev] [PATCH v7 09/14] fb: Add more formats

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1592 bytes --]

On Wed, Sep 19, 2018 at 04:14:22PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 11, 2018 at 10:47:36AM +0200, Maxime Ripard wrote:
> > We're going to need some DRM formats, and we're going to need the igt_fb
> > code to handle them. Since it relies on the format_desc structure to map
> > the DRM fourcc to the pixman and cairo formats, we need to add these new
> > formats to that structure.
> > 
> > Reviewed-by: Eric Anholt <eric@anholt.net>
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> >  lib/igt_fb.c | 35 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index f83ad0d2679c..425f98e23380 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -72,16 +72,46 @@ static struct format_desc_struct {
> >  	int num_planes;
> >  	int plane_bpp[4];
> >  } format_desc[] = {
> > +	{ .name = "ARGB1555", .depth = 16, .drm_id = DRM_FORMAT_ARGB1555,
> > +	  .cairo_id = CAIRO_FORMAT_INVALID,
> > +	  .pixman_id = PIXMAN_a1r5g5b5,
> > +	  .num_planes = 1, .plane_bpp = { 16, },
> > +	},
> > +	{ .name = "XRGB1555", .depth = 15, .drm_id = DRM_FORMAT_XRGB1555,
> > +	  .cairo_id = CAIRO_FORMAT_INVALID,
> > +	  .pixman_id = PIXMAN_x1r5g5b5,
> > +	  .num_planes = 1, .plane_bpp = { 16, },
> > +	},
> 
> I think this is going to mess up the depth->format behaviour that
> bunch of tests expect. So nak on adding .depth to these.

What is your suggestion then?

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH v7 04/14] fb: Create common function to convert frame formats

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1531 bytes --]

On Wed, Sep 19, 2018 at 04:03:25PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 11, 2018 at 10:47:31AM +0200, Maxime Ripard wrote:
> > The current code to convert between two buffer formats is quite tied to the
> > cairo surface infrastructure. Since we'll want to reuse it, make that
> > function more generic by introducing a common structure that passes all the
> > arguments and a common function that will call the right functions we
> > needed.
> > 
> > Reviewed-by: Eric Anholt <eric@anholt.net>
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> >  lib/igt_fb.c | 220 ++++++++++++++++++++++++++++++----------------------
> >  1 file changed, 130 insertions(+), 90 deletions(-)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 66bdc02a0ce7..f829df59e9fa 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -1391,6 +1391,19 @@ struct fb_convert_blit_upload {
> >  	struct fb_blit_linear linear;
> >  };
> >  
> > +struct fb_convert_buf {
> > +	void			*ptr;
> > +	struct igt_fb		*fb;
> > +};
> > +
> > +struct fb_convert {
> > +	unsigned int		width;
> > +	unsigned int		height;
> 
> Are we expecting to convert between different sized fbs, or why do we
> need these?

This was meant to express that a conversion should happen on a given
size, common to both fb. I guess we can remove it, but which one
should we choose in such a case? The destination?

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH v7 02/14] fb: Use an igt_fb for the cairo shadow buffer

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 3575 bytes --]

On Wed, Sep 19, 2018 at 04:21:38PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 11, 2018 at 10:47:29AM +0200, Maxime Ripard wrote:
> > In the case where an igt_fb user wants to get a cairo surface out of that
> > framebuffer, if the format of that framebuffer cannot be imported as-is in
> > Cairo, the current code will do an anonymous mapping to create a shadow
> > buffer where an RGB24 copy of that buffer will be created.
> > 
> > However, making that shadow buffer into an igt_fb itself will help us do
> > further improvements on the conversion code.
> > 
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> >  lib/igt_fb.c | 28 +++++++++++++++++-----------
> >  1 file changed, 17 insertions(+), 11 deletions(-)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 542c62610412..dd180c6c8016 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -1381,12 +1381,12 @@ static void create_cairo_surface__gtt(int fd, struct igt_fb *fb)
> >  
> >  struct fb_convert_blit_upload {
> >  	int fd;
> > +
> >  	struct igt_fb *fb;
> > +	uint8_t *ptr;
> >  
> > -	struct {
> > -		uint8_t *map;
> > -		unsigned stride, size;
> > -	} rgb24;
> > +	struct igt_fb shadow_fb;
> > +	uint8_t *shadow_ptr;
> 
> We seem to be pairing fb and ptr a lot. I wonder if we should suck the
> the ptr into the fb struct. But maybe someone wants to map the same
> thing multiple times? Not sure.

Is it something you want me to change then?

> >  	struct fb_blit_linear linear;
> >  };
> > @@ -1780,7 +1780,8 @@ static void destroy_cairo_surface__convert(void *arg)
> >  			     fb->drm_format);
> >  	}
> >  
> > -	munmap(blit->rgb24.map, blit->rgb24.size);
> > +	igt_fb_unmap_buffer(&blit->shadow_fb, blit->shadow_ptr);
> > +	igt_remove_fb(blit->fd, &blit->shadow_fb);
> >  
> >  	if (blit->linear.handle)
> >  		free_linear_mapping(blit->fd, blit->fb, &blit->linear);
> > @@ -1795,14 +1796,19 @@ static void destroy_cairo_surface__convert(void *arg)
> >  static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
> >  {
> >  	struct fb_convert_blit_upload *blit = malloc(sizeof(*blit));
> > +	int shadow_id;
> > +
> >  	igt_assert(blit);
> >  
> >  	blit->fd = fd;
> >  	blit->fb = fb;
> > -	blit->rgb24.stride = ALIGN(fb->width * 4, 16);
> > -	blit->rgb24.size = ALIGN(blit->rgb24.stride * fb->height, sysconf(_SC_PAGESIZE));
> > -	blit->rgb24.map = mmap(NULL, blit->rgb24.size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > -	igt_assert(blit->rgb24.map != MAP_FAILED);
> > +
> > +	shadow_id = igt_create_fb(fd, fb->width, fb->height, DRM_FORMAT_RGB888,
> > +				  LOCAL_DRM_FORMAT_MOD_NONE, &blit->shadow_fb);
> > +	igt_assert(shadow_id > 0);
> 
> We can't actually expect addfb to work for RGB888.

Because the driver might not support it?

> That thing is not supported by most hardware. Since my series adding
> fb_init() etc. hasn't gotten reviewed I guess we'll just need to
> populate the igt_fb by hand here, or something.

At this point, I have to ask: is there any actual will to have this
patch merged at some point?

This series is just a long history of either being ignored, dealing
with the i915-specific changes that got in and are breaking the other
users, or huge reworks for i915-specific uses cases I have no way to
test.

If the current idea is back to igt is for i915 solely, then there's no
point in wasting time on this.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH v7 09/14] fb: Add more formats

[Ville Syrjälä]

On Tue, Sep 25, 2018 at 09:49:43AM +0200, Maxime Ripard wrote:
> On Wed, Sep 19, 2018 at 04:14:22PM +0300, Ville Syrjälä wrote:
> > On Tue, Sep 11, 2018 at 10:47:36AM +0200, Maxime Ripard wrote:
> > > We're going to need some DRM formats, and we're going to need the igt_fb
> > > code to handle them. Since it relies on the format_desc structure to map
> > > the DRM fourcc to the pixman and cairo formats, we need to add these new
> > > formats to that structure.
> > > 
> > > Reviewed-by: Eric Anholt <eric@anholt.net>
> > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > > ---
> > >  lib/igt_fb.c | 35 +++++++++++++++++++++++++++++++++++
> > >  1 file changed, 35 insertions(+)
> > > 
> > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > index f83ad0d2679c..425f98e23380 100644
> > > --- a/lib/igt_fb.c
> > > +++ b/lib/igt_fb.c
> > > @@ -72,16 +72,46 @@ static struct format_desc_struct {
> > >  	int num_planes;
> > >  	int plane_bpp[4];
> > >  } format_desc[] = {
> > > +	{ .name = "ARGB1555", .depth = 16, .drm_id = DRM_FORMAT_ARGB1555,
> > > +	  .cairo_id = CAIRO_FORMAT_INVALID,
> > > +	  .pixman_id = PIXMAN_a1r5g5b5,
> > > +	  .num_planes = 1, .plane_bpp = { 16, },
> > > +	},
> > > +	{ .name = "XRGB1555", .depth = 15, .drm_id = DRM_FORMAT_XRGB1555,
> > > +	  .cairo_id = CAIRO_FORMAT_INVALID,
> > > +	  .pixman_id = PIXMAN_x1r5g5b5,
> > > +	  .num_planes = 1, .plane_bpp = { 16, },
> > > +	},
> > 
> > I think this is going to mess up the depth->format behaviour that
> > bunch of tests expect. So nak on adding .depth to these.
> 
> What is your suggestion then?

Do you actually need the depth for something?

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

Re: [igt-dev] [PATCH v7 04/14] fb: Create common function to convert frame formats

[Ville Syrjälä]

On Tue, Sep 25, 2018 at 09:51:15AM +0200, Maxime Ripard wrote:
> On Wed, Sep 19, 2018 at 04:03:25PM +0300, Ville Syrjälä wrote:
> > On Tue, Sep 11, 2018 at 10:47:31AM +0200, Maxime Ripard wrote:
> > > The current code to convert between two buffer formats is quite tied to the
> > > cairo surface infrastructure. Since we'll want to reuse it, make that
> > > function more generic by introducing a common structure that passes all the
> > > arguments and a common function that will call the right functions we
> > > needed.
> > > 
> > > Reviewed-by: Eric Anholt <eric@anholt.net>
> > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > > ---
> > >  lib/igt_fb.c | 220 ++++++++++++++++++++++++++++++----------------------
> > >  1 file changed, 130 insertions(+), 90 deletions(-)
> > > 
> > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > index 66bdc02a0ce7..f829df59e9fa 100644
> > > --- a/lib/igt_fb.c
> > > +++ b/lib/igt_fb.c
> > > @@ -1391,6 +1391,19 @@ struct fb_convert_blit_upload {
> > >  	struct fb_blit_linear linear;
> > >  };
> > >  
> > > +struct fb_convert_buf {
> > > +	void			*ptr;
> > > +	struct igt_fb		*fb;
> > > +};
> > > +
> > > +struct fb_convert {
> > > +	unsigned int		width;
> > > +	unsigned int		height;
> > 
> > Are we expecting to convert between different sized fbs, or why do we
> > need these?
> 
> This was meant to express that a conversion should happen on a given
> size, common to both fb. I guess we can remove it, but which one
> should we choose in such a case? The destination?

Seems good enough to me.

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

Re: [igt-dev] [PATCH v7 07/14] fb: Add support for conversions through pixman

[Petri Latvala]

On Tue, Sep 11, 2018 at 10:47:34AM +0200, Maxime Ripard wrote:
> Pixman allows for much more conversions than cairo, and we don't want to
> open code conversions routine for the common formats.
> 
> Let's plug pixman in our conversion function so that we can benefit from it
> when possible.
> 
> Reviewed-by: Eric Anholt <eric@anholt.net>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  lib/igt_fb.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 624fddbd465b..64370e01c1d6 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -29,6 +29,7 @@
>  #include <math.h>
>  #include <wchar.h>
>  #include <inttypes.h>
> +#include <pixman.h>
>  

Currently pixman is only there if you enable chamelium. Doing this
requires making it mandatory, doing some refactoring on meson.build
and configure.ac. And README.


-- 
Petri Latvala


>  #include "drmtest.h"
>  #include "igt_aux.h"
> @@ -59,29 +60,36 @@
>   * functions to work with these pixel format codes.
>   */
>  
> +#define PIXMAN_invalid	0
> +
>  /* drm fourcc/cairo format maps */
>  static struct format_desc_struct {
>  	const char *name;
>  	uint32_t drm_id;
>  	cairo_format_t cairo_id;
> +	pixman_format_code_t pixman_id;
>  	int depth;
>  	int num_planes;
>  	int plane_bpp[4];
>  } format_desc[] = {
>  	{ .name = "RGB565", .depth = 16, .drm_id = DRM_FORMAT_RGB565,
>  	  .cairo_id = CAIRO_FORMAT_RGB16_565,
> +	  .pixman_id = PIXMAN_r5g6b5,
>  	  .num_planes = 1, .plane_bpp = { 16, },
>  	},
>  	{ .name = "XRGB8888", .depth = 24, .drm_id = DRM_FORMAT_XRGB8888,
>  	  .cairo_id = CAIRO_FORMAT_RGB24,
> +	  .pixman_id = PIXMAN_x8r8g8b8,
>  	  .num_planes = 1, .plane_bpp = { 32, },
>  	},
>  	{ .name = "XRGB2101010", .depth = 30, .drm_id = DRM_FORMAT_XRGB2101010,
>  	  .cairo_id = CAIRO_FORMAT_RGB30,
> +	  .pixman_id = PIXMAN_x2r10g10b10,
>  	  .num_planes = 1, .plane_bpp = { 32, },
>  	},
>  	{ .name = "ARGB8888", .depth = 24, .drm_id = DRM_FORMAT_ARGB8888,
>  	  .cairo_id = CAIRO_FORMAT_ARGB32,
> +	  .pixman_id = PIXMAN_a8r8g8b8,
>  	  .num_planes = 1, .plane_bpp = { 32, },
>  	},
>  	{ .name = "NV12", .depth = -1, .drm_id = DRM_FORMAT_NV12,
> @@ -90,6 +98,7 @@ static struct format_desc_struct {
>  	},
>  	{ .name = "YUYV", .depth = -1, .drm_id = DRM_FORMAT_YUYV,
>  	  .cairo_id = CAIRO_FORMAT_RGB24,
> +	  .pixman_id = PIXMAN_yuy2,
>  	  .num_planes = 1, .plane_bpp = { 16, },
>  	},
>  	{ .name = "YVYU", .depth = -1, .drm_id = DRM_FORMAT_YVYU,
> @@ -1178,6 +1187,18 @@ unsigned int igt_create_stereo_fb(int drm_fd, drmModeModeInfo *mode,
>  	return fb_id;
>  }
>  
> +static pixman_format_code_t drm_format_to_pixman(uint32_t drm_format)
> +{
> +	struct format_desc_struct *f;
> +
> +	for_each_format(f)
> +		if (f->drm_id == drm_format)
> +			return f->pixman_id;
> +
> +	igt_assert_f(0, "can't find a pixman format for %08x (%s)\n",
> +		     drm_format, igt_format_str(drm_format));
> +}
> +
>  static cairo_format_t drm_format_to_cairo(uint32_t drm_format)
>  {
>  	struct format_desc_struct *f;
> @@ -1774,9 +1795,38 @@ static void convert_rgb24_to_yuyv(struct fb_convert *cvt)
>  	}
>  }
>  
> +static void convert_pixman(struct fb_convert *cvt)
> +{
> +	pixman_format_code_t src_pixman = drm_format_to_pixman(cvt->src.fb->drm_format);
> +	pixman_format_code_t dst_pixman = drm_format_to_pixman(cvt->dst.fb->drm_format);
> +	pixman_image_t *dst_image, *src_image;
> +
> +	igt_assert((src_pixman != PIXMAN_invalid) &&
> +		   (dst_pixman != PIXMAN_invalid));
> +
> +	src_image = pixman_image_create_bits(src_pixman,
> +					     cvt->width, cvt->height,
> +					     cvt->src.ptr, cvt->src.fb->stride);
> +	igt_assert(src_image);
> +
> +	dst_image = pixman_image_create_bits(dst_pixman,
> +					     cvt->width, cvt->height,
> +					     cvt->dst.ptr, cvt->dst.fb->stride);
> +	igt_assert(dst_image);
> +
> +	pixman_image_composite(PIXMAN_OP_SRC, src_image, NULL, dst_image,
> +			       0, 0, 0, 0, 0, 0, cvt->width, cvt->height);
> +	pixman_image_unref(dst_image);
> +	pixman_image_unref(src_image);
> +}
> +
>  static void fb_convert(struct fb_convert *cvt)
>  {
> -	if (cvt->dst.fb->drm_format == DRM_FORMAT_RGB888) {
> +	if ((drm_format_to_pixman(cvt->src.fb->drm_format) != PIXMAN_invalid) &&
> +	    (drm_format_to_pixman(cvt->dst.fb->drm_format) != PIXMAN_invalid)) {
> +		convert_pixman(cvt);
> +		return;
> +	} else if (cvt->dst.fb->drm_format == DRM_FORMAT_RGB888) {
>  		switch (cvt->src.fb->drm_format) {
>  		case DRM_FORMAT_NV12:
>  			convert_nv12_to_rgb24(cvt);
> @@ -2130,7 +2180,8 @@ bool igt_fb_supported_format(uint32_t drm_format)
>  
>  	for_each_format(f)
>  		if (f->drm_id == drm_format)
> -			return f->cairo_id != CAIRO_FORMAT_INVALID;
> +			return (f->cairo_id != CAIRO_FORMAT_INVALID) ||
> +				(f->pixman_id != PIXMAN_invalid);
>  
>  	return false;
>  }
> -- 
> git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH v7 02/14] fb: Use an igt_fb for the cairo shadow buffer

[Arkadiusz Hiler]

On Tue, Sep 25, 2018 at 10:51:24AM +0200, Maxime Ripard wrote:
> On Wed, Sep 19, 2018 at 04:21:38PM +0300, Ville Syrjälä wrote:
> > On Tue, Sep 11, 2018 at 10:47:29AM +0200, Maxime Ripard wrote:
> > > In the case where an igt_fb user wants to get a cairo surface out of that
> > > framebuffer, if the format of that framebuffer cannot be imported as-is in
> > > Cairo, the current code will do an anonymous mapping to create a shadow
> > > buffer where an RGB24 copy of that buffer will be created.
> > > 
> > > However, making that shadow buffer into an igt_fb itself will help us do
> > > further improvements on the conversion code.
> > > 
> > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > > ---
> > >  lib/igt_fb.c | 28 +++++++++++++++++-----------
> > >  1 file changed, 17 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > index 542c62610412..dd180c6c8016 100644
> > > --- a/lib/igt_fb.c
> > > +++ b/lib/igt_fb.c
> > > @@ -1381,12 +1381,12 @@ static void create_cairo_surface__gtt(int fd, struct igt_fb *fb)
> > >  
> > >  struct fb_convert_blit_upload {
> > >  	int fd;
> > > +
> > >  	struct igt_fb *fb;
> > > +	uint8_t *ptr;
> > >  
> > > -	struct {
> > > -		uint8_t *map;
> > > -		unsigned stride, size;
> > > -	} rgb24;
> > > +	struct igt_fb shadow_fb;
> > > +	uint8_t *shadow_ptr;
> > 
> > We seem to be pairing fb and ptr a lot. I wonder if we should suck the
> > the ptr into the fb struct. But maybe someone wants to map the same
> > thing multiple times? Not sure.
> 
> Is it something you want me to change then?
> 
> > >  	struct fb_blit_linear linear;
> > >  };
> > > @@ -1780,7 +1780,8 @@ static void destroy_cairo_surface__convert(void *arg)
> > >  			     fb->drm_format);
> > >  	}
> > >  
> > > -	munmap(blit->rgb24.map, blit->rgb24.size);
> > > +	igt_fb_unmap_buffer(&blit->shadow_fb, blit->shadow_ptr);
> > > +	igt_remove_fb(blit->fd, &blit->shadow_fb);
> > >  
> > >  	if (blit->linear.handle)
> > >  		free_linear_mapping(blit->fd, blit->fb, &blit->linear);
> > > @@ -1795,14 +1796,19 @@ static void destroy_cairo_surface__convert(void *arg)
> > >  static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
> > >  {
> > >  	struct fb_convert_blit_upload *blit = malloc(sizeof(*blit));
> > > +	int shadow_id;
> > > +
> > >  	igt_assert(blit);
> > >  
> > >  	blit->fd = fd;
> > >  	blit->fb = fb;
> > > -	blit->rgb24.stride = ALIGN(fb->width * 4, 16);
> > > -	blit->rgb24.size = ALIGN(blit->rgb24.stride * fb->height, sysconf(_SC_PAGESIZE));
> > > -	blit->rgb24.map = mmap(NULL, blit->rgb24.size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > > -	igt_assert(blit->rgb24.map != MAP_FAILED);
> > > +
> > > +	shadow_id = igt_create_fb(fd, fb->width, fb->height, DRM_FORMAT_RGB888,
> > > +				  LOCAL_DRM_FORMAT_MOD_NONE, &blit->shadow_fb);
> > > +	igt_assert(shadow_id > 0);
> > 
> > We can't actually expect addfb to work for RGB888.
> 
> Because the driver might not support it?
> 
> > That thing is not supported by most hardware. Since my series adding
> > fb_init() etc. hasn't gotten reviewed I guess we'll just need to
> > populate the igt_fb by hand here, or something.
> 
> At this point, I have to ask: is there any actual will to have this
> patch merged at some point?
> 
> This series is just a long history of either being ignored, dealing
> with the i915-specific changes that got in and are breaking the other
> users, or huge reworks for i915-specific uses cases I have no way to
> test.
> 
> If the current idea is back to igt is for i915 solely, then there's no
> point in wasting time on this.

Sorry for wasting your time up to this point. If you will ever feel
blocked, please do contact me or Petri.

Seeing some comments here and there, as well as reviewed-bys I was
wrongly assuming that this is progressing. This is my failure here, and
getting to v7 should have alarmed me. I am truly sorry.

Back to the topic - yes your contributions are welcome.

I really like the idea, that instead of manually fiddling with rgb24 we
could deal with a proper igt_fb here, because it makes life easier later.

@Ville: We have your fb_init() merged now, so we can use it here, so it
should be hardware-that-does-not-support-rgb888-proof. And it's not like
we should have blocked here in the first place, as all the hardware we
run on seems to support it, and it could be reworked later on.

@Maxime: Let's merge this in chunks now. Let's start with the first two
patches. I'll rebase them, switch them to use fb_init(), do the changes
in convert_nv12_to_rgb24 and send them for CI approval.

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

Re: [igt-dev] [PATCH v7 02/14] fb: Use an igt_fb for the cairo shadow buffer

[Ville Syrjälä]

On Mon, Oct 01, 2018 at 01:47:12PM +0300, Arkadiusz Hiler wrote:
> On Tue, Sep 25, 2018 at 10:51:24AM +0200, Maxime Ripard wrote:
> > On Wed, Sep 19, 2018 at 04:21:38PM +0300, Ville Syrjälä wrote:
> > > On Tue, Sep 11, 2018 at 10:47:29AM +0200, Maxime Ripard wrote:
> > > > In the case where an igt_fb user wants to get a cairo surface out of that
> > > > framebuffer, if the format of that framebuffer cannot be imported as-is in
> > > > Cairo, the current code will do an anonymous mapping to create a shadow
> > > > buffer where an RGB24 copy of that buffer will be created.
> > > > 
> > > > However, making that shadow buffer into an igt_fb itself will help us do
> > > > further improvements on the conversion code.
> > > > 
> > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > > > ---
> > > >  lib/igt_fb.c | 28 +++++++++++++++++-----------
> > > >  1 file changed, 17 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > > index 542c62610412..dd180c6c8016 100644
> > > > --- a/lib/igt_fb.c
> > > > +++ b/lib/igt_fb.c
> > > > @@ -1381,12 +1381,12 @@ static void create_cairo_surface__gtt(int fd, struct igt_fb *fb)
> > > >  
> > > >  struct fb_convert_blit_upload {
> > > >  	int fd;
> > > > +
> > > >  	struct igt_fb *fb;
> > > > +	uint8_t *ptr;
> > > >  
> > > > -	struct {
> > > > -		uint8_t *map;
> > > > -		unsigned stride, size;
> > > > -	} rgb24;
> > > > +	struct igt_fb shadow_fb;
> > > > +	uint8_t *shadow_ptr;
> > > 
> > > We seem to be pairing fb and ptr a lot. I wonder if we should suck the
> > > the ptr into the fb struct. But maybe someone wants to map the same
> > > thing multiple times? Not sure.
> > 
> > Is it something you want me to change then?
> > 
> > > >  	struct fb_blit_linear linear;
> > > >  };
> > > > @@ -1780,7 +1780,8 @@ static void destroy_cairo_surface__convert(void *arg)
> > > >  			     fb->drm_format);
> > > >  	}
> > > >  
> > > > -	munmap(blit->rgb24.map, blit->rgb24.size);
> > > > +	igt_fb_unmap_buffer(&blit->shadow_fb, blit->shadow_ptr);
> > > > +	igt_remove_fb(blit->fd, &blit->shadow_fb);
> > > >  
> > > >  	if (blit->linear.handle)
> > > >  		free_linear_mapping(blit->fd, blit->fb, &blit->linear);
> > > > @@ -1795,14 +1796,19 @@ static void destroy_cairo_surface__convert(void *arg)
> > > >  static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
> > > >  {
> > > >  	struct fb_convert_blit_upload *blit = malloc(sizeof(*blit));
> > > > +	int shadow_id;
> > > > +
> > > >  	igt_assert(blit);
> > > >  
> > > >  	blit->fd = fd;
> > > >  	blit->fb = fb;
> > > > -	blit->rgb24.stride = ALIGN(fb->width * 4, 16);
> > > > -	blit->rgb24.size = ALIGN(blit->rgb24.stride * fb->height, sysconf(_SC_PAGESIZE));
> > > > -	blit->rgb24.map = mmap(NULL, blit->rgb24.size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > > > -	igt_assert(blit->rgb24.map != MAP_FAILED);
> > > > +
> > > > +	shadow_id = igt_create_fb(fd, fb->width, fb->height, DRM_FORMAT_RGB888,
> > > > +				  LOCAL_DRM_FORMAT_MOD_NONE, &blit->shadow_fb);
> > > > +	igt_assert(shadow_id > 0);
> > > 
> > > We can't actually expect addfb to work for RGB888.
> > 
> > Because the driver might not support it?
> > 
> > > That thing is not supported by most hardware. Since my series adding
> > > fb_init() etc. hasn't gotten reviewed I guess we'll just need to
> > > populate the igt_fb by hand here, or something.
> > 
> > At this point, I have to ask: is there any actual will to have this
> > patch merged at some point?
> > 
> > This series is just a long history of either being ignored, dealing
> > with the i915-specific changes that got in and are breaking the other
> > users, or huge reworks for i915-specific uses cases I have no way to
> > test.
> > 
> > If the current idea is back to igt is for i915 solely, then there's no
> > point in wasting time on this.
> 
> Sorry for wasting your time up to this point. If you will ever feel
> blocked, please do contact me or Petri.
> 
> Seeing some comments here and there, as well as reviewed-bys I was
> wrongly assuming that this is progressing. This is my failure here, and
> getting to v7 should have alarmed me. I am truly sorry.
> 
> Back to the topic - yes your contributions are welcome.
> 
> I really like the idea, that instead of manually fiddling with rgb24 we
> could deal with a proper igt_fb here, because it makes life easier later.
> 
> @Ville: We have your fb_init() merged now, so we can use it here, so it
> should be hardware-that-does-not-support-rgb888-proof. And it's not like
> we should have blocked here in the first place, as all the hardware we
> run on seems to support it, and it could be reworked later on.

Practically nothing supports 24bpp.

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

Re: [igt-dev] [PATCH v7 02/14] fb: Use an igt_fb for the cairo shadow buffer

[Arkadiusz Hiler]

On Mon, Oct 01, 2018 at 04:55:22PM +0300, Ville Syrjälä wrote:
> On Mon, Oct 01, 2018 at 01:47:12PM +0300, Arkadiusz Hiler wrote:
> > On Tue, Sep 25, 2018 at 10:51:24AM +0200, Maxime Ripard wrote:
> > > On Wed, Sep 19, 2018 at 04:21:38PM +0300, Ville Syrjälä wrote:
> > > > On Tue, Sep 11, 2018 at 10:47:29AM +0200, Maxime Ripard wrote:
> > > > > In the case where an igt_fb user wants to get a cairo surface out of that
> > > > > framebuffer, if the format of that framebuffer cannot be imported as-is in
> > > > > Cairo, the current code will do an anonymous mapping to create a shadow
> > > > > buffer where an RGB24 copy of that buffer will be created.
> > > > > 
> > > > > However, making that shadow buffer into an igt_fb itself will help us do
> > > > > further improvements on the conversion code.
> > > > > 
> > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > > > > ---
> > > > >  lib/igt_fb.c | 28 +++++++++++++++++-----------
> > > > >  1 file changed, 17 insertions(+), 11 deletions(-)
> > > > > 
> > > > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > > > index 542c62610412..dd180c6c8016 100644
> > > > > --- a/lib/igt_fb.c
> > > > > +++ b/lib/igt_fb.c
> > > > > @@ -1381,12 +1381,12 @@ static void create_cairo_surface__gtt(int fd, struct igt_fb *fb)
> > > > >  
> > > > >  struct fb_convert_blit_upload {
> > > > >  	int fd;
> > > > > +
> > > > >  	struct igt_fb *fb;
> > > > > +	uint8_t *ptr;
> > > > >  
> > > > > -	struct {
> > > > > -		uint8_t *map;
> > > > > -		unsigned stride, size;
> > > > > -	} rgb24;
> > > > > +	struct igt_fb shadow_fb;
> > > > > +	uint8_t *shadow_ptr;
> > > > 
> > > > We seem to be pairing fb and ptr a lot. I wonder if we should suck the
> > > > the ptr into the fb struct. But maybe someone wants to map the same
> > > > thing multiple times? Not sure.
> > > 
> > > Is it something you want me to change then?
> > > 
> > > > >  	struct fb_blit_linear linear;
> > > > >  };
> > > > > @@ -1780,7 +1780,8 @@ static void destroy_cairo_surface__convert(void *arg)
> > > > >  			     fb->drm_format);
> > > > >  	}
> > > > >  
> > > > > -	munmap(blit->rgb24.map, blit->rgb24.size);
> > > > > +	igt_fb_unmap_buffer(&blit->shadow_fb, blit->shadow_ptr);
> > > > > +	igt_remove_fb(blit->fd, &blit->shadow_fb);
> > > > >  
> > > > >  	if (blit->linear.handle)
> > > > >  		free_linear_mapping(blit->fd, blit->fb, &blit->linear);
> > > > > @@ -1795,14 +1796,19 @@ static void destroy_cairo_surface__convert(void *arg)
> > > > >  static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
> > > > >  {
> > > > >  	struct fb_convert_blit_upload *blit = malloc(sizeof(*blit));
> > > > > +	int shadow_id;
> > > > > +
> > > > >  	igt_assert(blit);
> > > > >  
> > > > >  	blit->fd = fd;
> > > > >  	blit->fb = fb;
> > > > > -	blit->rgb24.stride = ALIGN(fb->width * 4, 16);
> > > > > -	blit->rgb24.size = ALIGN(blit->rgb24.stride * fb->height, sysconf(_SC_PAGESIZE));
> > > > > -	blit->rgb24.map = mmap(NULL, blit->rgb24.size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > > > > -	igt_assert(blit->rgb24.map != MAP_FAILED);
> > > > > +
> > > > > +	shadow_id = igt_create_fb(fd, fb->width, fb->height, DRM_FORMAT_RGB888,
> > > > > +				  LOCAL_DRM_FORMAT_MOD_NONE, &blit->shadow_fb);
> > > > > +	igt_assert(shadow_id > 0);
> > > > 
> > > > We can't actually expect addfb to work for RGB888.
> > > 
> > > Because the driver might not support it?
> > > 
> > > > That thing is not supported by most hardware. Since my series adding
> > > > fb_init() etc. hasn't gotten reviewed I guess we'll just need to
> > > > populate the igt_fb by hand here, or something.
> > > 
> > > At this point, I have to ask: is there any actual will to have this
> > > patch merged at some point?
> > > 
> > > This series is just a long history of either being ignored, dealing
> > > with the i915-specific changes that got in and are breaking the other
> > > users, or huge reworks for i915-specific uses cases I have no way to
> > > test.
> > > 
> > > If the current idea is back to igt is for i915 solely, then there's no
> > > point in wasting time on this.
> > 
> > Sorry for wasting your time up to this point. If you will ever feel
> > blocked, please do contact me or Petri.
> > 
> > Seeing some comments here and there, as well as reviewed-bys I was
> > wrongly assuming that this is progressing. This is my failure here, and
> > getting to v7 should have alarmed me. I am truly sorry.
> > 
> > Back to the topic - yes your contributions are welcome.
> > 
> > I really like the idea, that instead of manually fiddling with rgb24 we
> > could deal with a proper igt_fb here, because it makes life easier later.
> > 
> > @Ville: We have your fb_init() merged now, so we can use it here, so it
> > should be hardware-that-does-not-support-rgb888-proof. And it's not like
> > we should have blocked here in the first place, as all the hardware we
> > run on seems to support it, and it could be reworked later on.
> 
> Practically nothing supports 24bpp.

Misread that as XRGB8888, my bad. It would have worked with that.

@Maxime: I guess fbinit() + malloc for the dummy fb will do the trick
for you, as you seem use that just for the conversion purposes.

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

Re: [igt-dev] [PATCH v7 02/14] fb: Use an igt_fb for the cairo shadow buffer

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 6142 bytes --]

Hi!

On Mon, Oct 01, 2018 at 05:10:14PM +0300, Arkadiusz Hiler wrote:
> On Mon, Oct 01, 2018 at 04:55:22PM +0300, Ville Syrjälä wrote:
> > On Mon, Oct 01, 2018 at 01:47:12PM +0300, Arkadiusz Hiler wrote:
> > > On Tue, Sep 25, 2018 at 10:51:24AM +0200, Maxime Ripard wrote:
> > > > On Wed, Sep 19, 2018 at 04:21:38PM +0300, Ville Syrjälä wrote:
> > > > > On Tue, Sep 11, 2018 at 10:47:29AM +0200, Maxime Ripard wrote:
> > > > > > In the case where an igt_fb user wants to get a cairo surface out of that
> > > > > > framebuffer, if the format of that framebuffer cannot be imported as-is in
> > > > > > Cairo, the current code will do an anonymous mapping to create a shadow
> > > > > > buffer where an RGB24 copy of that buffer will be created.
> > > > > > 
> > > > > > However, making that shadow buffer into an igt_fb itself will help us do
> > > > > > further improvements on the conversion code.
> > > > > > 
> > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > > > > > ---
> > > > > >  lib/igt_fb.c | 28 +++++++++++++++++-----------
> > > > > >  1 file changed, 17 insertions(+), 11 deletions(-)
> > > > > > 
> > > > > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > > > > index 542c62610412..dd180c6c8016 100644
> > > > > > --- a/lib/igt_fb.c
> > > > > > +++ b/lib/igt_fb.c
> > > > > > @@ -1381,12 +1381,12 @@ static void create_cairo_surface__gtt(int fd, struct igt_fb *fb)
> > > > > >  
> > > > > >  struct fb_convert_blit_upload {
> > > > > >  	int fd;
> > > > > > +
> > > > > >  	struct igt_fb *fb;
> > > > > > +	uint8_t *ptr;
> > > > > >  
> > > > > > -	struct {
> > > > > > -		uint8_t *map;
> > > > > > -		unsigned stride, size;
> > > > > > -	} rgb24;
> > > > > > +	struct igt_fb shadow_fb;
> > > > > > +	uint8_t *shadow_ptr;
> > > > > 
> > > > > We seem to be pairing fb and ptr a lot. I wonder if we should suck the
> > > > > the ptr into the fb struct. But maybe someone wants to map the same
> > > > > thing multiple times? Not sure.
> > > > 
> > > > Is it something you want me to change then?
> > > > 
> > > > > >  	struct fb_blit_linear linear;
> > > > > >  };
> > > > > > @@ -1780,7 +1780,8 @@ static void destroy_cairo_surface__convert(void *arg)
> > > > > >  			     fb->drm_format);
> > > > > >  	}
> > > > > >  
> > > > > > -	munmap(blit->rgb24.map, blit->rgb24.size);
> > > > > > +	igt_fb_unmap_buffer(&blit->shadow_fb, blit->shadow_ptr);
> > > > > > +	igt_remove_fb(blit->fd, &blit->shadow_fb);
> > > > > >  
> > > > > >  	if (blit->linear.handle)
> > > > > >  		free_linear_mapping(blit->fd, blit->fb, &blit->linear);
> > > > > > @@ -1795,14 +1796,19 @@ static void destroy_cairo_surface__convert(void *arg)
> > > > > >  static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
> > > > > >  {
> > > > > >  	struct fb_convert_blit_upload *blit = malloc(sizeof(*blit));
> > > > > > +	int shadow_id;
> > > > > > +
> > > > > >  	igt_assert(blit);
> > > > > >  
> > > > > >  	blit->fd = fd;
> > > > > >  	blit->fb = fb;
> > > > > > -	blit->rgb24.stride = ALIGN(fb->width * 4, 16);
> > > > > > -	blit->rgb24.size = ALIGN(blit->rgb24.stride * fb->height, sysconf(_SC_PAGESIZE));
> > > > > > -	blit->rgb24.map = mmap(NULL, blit->rgb24.size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > > > > > -	igt_assert(blit->rgb24.map != MAP_FAILED);
> > > > > > +
> > > > > > +	shadow_id = igt_create_fb(fd, fb->width, fb->height, DRM_FORMAT_RGB888,
> > > > > > +				  LOCAL_DRM_FORMAT_MOD_NONE, &blit->shadow_fb);
> > > > > > +	igt_assert(shadow_id > 0);
> > > > > 
> > > > > We can't actually expect addfb to work for RGB888.
> > > > 
> > > > Because the driver might not support it?
> > > > 
> > > > > That thing is not supported by most hardware. Since my series adding
> > > > > fb_init() etc. hasn't gotten reviewed I guess we'll just need to
> > > > > populate the igt_fb by hand here, or something.
> > > > 
> > > > At this point, I have to ask: is there any actual will to have this
> > > > patch merged at some point?
> > > > 
> > > > This series is just a long history of either being ignored, dealing
> > > > with the i915-specific changes that got in and are breaking the other
> > > > users, or huge reworks for i915-specific uses cases I have no way to
> > > > test.
> > > > 
> > > > If the current idea is back to igt is for i915 solely, then there's no
> > > > point in wasting time on this.
> > > 
> > > Sorry for wasting your time up to this point. If you will ever feel
> > > blocked, please do contact me or Petri.
> > > 
> > > Seeing some comments here and there, as well as reviewed-bys I was
> > > wrongly assuming that this is progressing. This is my failure here, and
> > > getting to v7 should have alarmed me. I am truly sorry.
> > > 
> > > Back to the topic - yes your contributions are welcome.
> > > 
> > > I really like the idea, that instead of manually fiddling with rgb24 we
> > > could deal with a proper igt_fb here, because it makes life easier later.
> > > 
> > > @Ville: We have your fb_init() merged now, so we can use it here, so it
> > > should be hardware-that-does-not-support-rgb888-proof. And it's not like
> > > we should have blocked here in the first place, as all the hardware we
> > > run on seems to support it, and it could be reworked later on.
> > 
> > Practically nothing supports 24bpp.
> 
> Misread that as XRGB8888, my bad. It would have worked with that.
> 
> @Maxime: I guess fbinit() + malloc for the dummy fb will do the trick
> for you, as you seem use that just for the conversion purposes.

Yep. I actually rolled back to the previous approach that was usisng
an anonymous mapping. It should work for everyone.

I've also rebased on current master, and it turns out I introduced a
regression on Intel platforms, so I've setup a platform here so that I
can test on some intel hardware at least. I'll resend a new version
with hopefully all the comments and breakages addressed.

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH v7 07/14] fb: Add support for conversions through pixman

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1240 bytes --]

Hi Petri,

On Mon, Oct 01, 2018 at 12:26:58PM +0300, Petri Latvala wrote:
> On Tue, Sep 11, 2018 at 10:47:34AM +0200, Maxime Ripard wrote:
> > Pixman allows for much more conversions than cairo, and we don't want to
> > open code conversions routine for the common formats.
> > 
> > Let's plug pixman in our conversion function so that we can benefit from it
> > when possible.
> > 
> > Reviewed-by: Eric Anholt <eric@anholt.net>
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> >  lib/igt_fb.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 53 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 624fddbd465b..64370e01c1d6 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -29,6 +29,7 @@
> >  #include <math.h>
> >  #include <wchar.h>
> >  #include <inttypes.h>
> > +#include <pixman.h>
> >  
> 
> Currently pixman is only there if you enable chamelium. Doing this
> requires making it mandatory, doing some refactoring on meson.build
> and configure.ac. And README.

Ah, right. I'll fix that.

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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