[igt-dev] [PATCH v6 00/13] chamelium: Test the plane formats

[igt-dev] [PATCH v6 00/13] 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 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 (13):
  fb: Add buffer map/unmap functions
  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                             | 467 ++++++++++++++++++------
 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, 562 insertions(+), 148 deletions(-)

base-commit: 94ebd21177feedf03e8f6dd1e73dca1a6ec7a0ac
-- 
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 v6 01/13] 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 | 57 +++++++++++++++++++++++++++++++++++++++++++++--------
 lib/igt_fb.h |  2 ++-
 2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index ae71d9673228..a4e38b7a8963 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1327,18 +1327,23 @@ 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;
 
@@ -1349,6 +1354,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),
@@ -1766,7 +1778,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);
 
@@ -1790,8 +1802,7 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
 		setup_linear_mapping(fd, fb, &blit->linear);
 	} else {
 		blit->linear.handle = 0;
-		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;
@@ -1826,6 +1837,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 v6 02/13] 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 a4e38b7a8963..4061fedec0c1 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1640,8 +1640,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;
@@ -1650,6 +1649,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,
@@ -1699,8 +1699,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;
@@ -1709,6 +1708,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 ||
@@ -1766,7 +1766,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",
@@ -1818,7 +1818,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 v6 03/13] 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 | 243 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 153 insertions(+), 90 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 4061fedec0c1..1914233786a5 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1384,6 +1384,24 @@ struct fb_convert_blit_upload {
 	struct fb_blit_linear linear;
 };
 
+struct fb_convert_buf {
+	void			*ptr;
+	unsigned int		stride;
+	unsigned int		size;
+	uint32_t		fmt;
+	enum igt_color_encoding	color_encoding;
+	enum igt_color_range	color_range;
+	uint32_t		offsets[4];
+};
+
+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);
@@ -1404,27 +1422,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.stride;
+	unsigned int planar_stride = cvt->src.stride;
+	uint8_t *buf = malloc(cvt->src.size);
+	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.color_encoding,
+						    cvt->src.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.size);
+	y = cvt->src.ptr + cvt->src.offsets[0];
+	uv = cvt->src.ptr + cvt->src.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];
@@ -1449,7 +1468,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];
@@ -1473,9 +1492,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];
@@ -1493,7 +1512,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;
@@ -1512,22 +1531,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.offsets[0];
+	uint8_t *uv = cvt->dst.ptr + cvt->dst.offsets[1];
+	const uint8_t *rgb24 = cvt->src.ptr;
+	unsigned rgb24_stride = cvt->src.stride;
+	unsigned planar_stride = cvt->dst.stride;
+	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.color_encoding,
+						    cvt->dst.color_range);
+
+	igt_assert_f(cvt->dst.fmt == 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];
@@ -1556,7 +1575,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];
@@ -1585,8 +1604,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];
@@ -1603,7 +1622,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;
@@ -1640,27 +1659,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.stride;
+	unsigned int yuyv_stride = cvt->src.stride;
+	uint8_t *buf = malloc(cvt->src.size);
+	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.color_encoding,
+						    cvt->src.color_range);
+	const unsigned char *swz = yuyv_swizzle(cvt->src.fmt);
 
 	/*
 	 * 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.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];
@@ -1678,7 +1698,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;
 
@@ -1699,25 +1719,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.stride;
+	unsigned yuyv_stride = cvt->dst.stride;
+	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.color_encoding,
+						    cvt->dst.color_range);
+	const unsigned char *swz = yuyv_swizzle(cvt->dst.fmt);
+
+	igt_assert_f(cvt->dst.fmt == DRM_FORMAT_YUYV ||
+		     cvt->dst.fmt == DRM_FORMAT_YVYU ||
+		     cvt->dst.fmt == DRM_FORMAT_UYVY ||
+		     cvt->dst.fmt == 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];
@@ -1734,7 +1754,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;
 
@@ -1752,27 +1772,65 @@ 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.fmt == DRM_FORMAT_RGB888) {
+		switch (cvt->src.fmt) {
+		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.fmt == DRM_FORMAT_RGB888) {
+		switch (cvt->dst.fmt) {
+		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.fmt, cvt->dst.fmt);
+}
+
 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,
+			.fmt		= fb->drm_format,
+			.stride		= blit->linear.stride,
+			.size		= blit->linear.size,
+			.color_encoding	= fb->color_encoding,
+			.color_range	= fb->color_range,
+		},
+
+		.src	= {
+			.ptr		= blit->rgb24.map,
+			.fmt		= DRM_FORMAT_RGB888,
+			.stride		= blit->rgb24.stride,
+			.size		= blit->rgb24.size,
+		},
+	};
+
+	fb_convert(&cvt);
 	munmap(blit->rgb24.map, blit->rgb24.size);
 
 	if (blit->linear.handle)
@@ -1788,6 +1846,8 @@ 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 };
+
 	igt_assert(blit);
 
 	blit->fd = fd;
@@ -1809,21 +1869,24 @@ 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->rgb24.map;
+	cvt.dst.fmt = DRM_FORMAT_RGB888;
+	cvt.dst.stride = blit->rgb24.stride;
+	cvt.dst.size = blit->rgb24.size;
+
+	cvt.src.ptr = blit->linear.map;
+	cvt.src.fmt = fb->drm_format;
+	cvt.src.stride	= blit->linear.stride;
+	cvt.src.size = blit->linear.size;
+	cvt.src.color_encoding = fb->color_encoding;
+	cvt.src.color_range = fb->color_range;
+	memcpy(cvt.dst.offsets, blit->linear.offsets,
+	       sizeof(blit->linear.offsets));
+
+	fb_convert(&cvt);
 
 	fb->cairo_surface =
 		cairo_image_surface_create_for_data(blit->rgb24.map,
-- 
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 v6 04/13] 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 | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 lib/igt_fb.h |  2 ++-
 2 files changed, 61 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 1914233786a5..9aee02c35c16 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2029,6 +2029,65 @@ 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.fmt = dst_fourcc;
+	cvt.dst.stride = dst->stride;
+	cvt.dst.size = dst->size;
+	cvt.dst.color_encoding = dst->color_encoding;
+	cvt.dst.color_range = dst->color_range;
+	memcpy(&cvt.dst.offsets, dst->offsets, sizeof(dst->offsets));
+
+	cvt.src.ptr = src_ptr;
+	cvt.src.fmt = src->drm_format;
+	cvt.src.stride = src->stride;
+	cvt.src.size = src->size;
+	cvt.src.color_encoding = src->color_encoding;
+	cvt.src.color_range = src->color_range;
+	memcpy(&cvt.src.offsets, src->offsets, sizeof(src->offsets));
+	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 v6 05/13] 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 9aee02c35c16..7d46cfdb45d8 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 v6 06/13] 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 7d46cfdb45d8..d98b733adecd 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,
@@ -1175,6 +1184,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;
@@ -1772,9 +1793,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.fmt);
+	pixman_format_code_t dst_pixman = drm_format_to_pixman(cvt->dst.fmt);
+	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.stride);
+	igt_assert(src_image);
+
+	dst_image = pixman_image_create_bits(dst_pixman,
+					     cvt->width, cvt->height,
+					     cvt->dst.ptr, cvt->dst.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.fmt == DRM_FORMAT_RGB888) {
+	if ((drm_format_to_pixman(cvt->src.fmt) != PIXMAN_invalid) &&
+	    (drm_format_to_pixman(cvt->dst.fmt) != PIXMAN_invalid)) {
+		convert_pixman(cvt);
+		return;
+	} else if (cvt->dst.fmt == DRM_FORMAT_RGB888) {
 		switch (cvt->src.fmt) {
 		case DRM_FORMAT_NV12:
 			convert_nv12_to_rgb24(cvt);
@@ -2155,7 +2205,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 v6 07/13] 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 d98b733adecd..b712faa616a0 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2178,6 +2178,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 v6 08/13] 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 b712faa616a0..1286d52bbec0 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 v6 09/13] 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 v6 10/13] 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 v6 11/13] 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 v6 12/13] 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 v6 13/13] 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

Re: [igt-dev] [PATCH v6 03/13] fb: Create common function to convert frame formats

[Ville Syrjälä]

On Fri, Aug 31, 2018 at 03:12:12PM +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 | 243 ++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 153 insertions(+), 90 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 4061fedec0c1..1914233786a5 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -1384,6 +1384,24 @@ struct fb_convert_blit_upload {
>  	struct fb_blit_linear linear;
>  };
>  
> +struct fb_convert_buf {
> +	void			*ptr;
> +	unsigned int		stride;
> +	unsigned int		size;
> +	uint32_t		fmt;
> +	enum igt_color_encoding	color_encoding;
> +	enum igt_color_range	color_range;
> +	uint32_t		offsets[4];
> +};

I wonder if we can just use igt_fb for that instead of duplicating most
of it?

I have a patch that did something like that for the fb_blit_upload
stuff: https://patchwork.freedesktop.org/patch/239832/

> +
> +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);
> @@ -1404,27 +1422,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.stride;
> +	unsigned int planar_stride = cvt->src.stride;
> +	uint8_t *buf = malloc(cvt->src.size);
> +	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.color_encoding,
> +						    cvt->src.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.size);
> +	y = cvt->src.ptr + cvt->src.offsets[0];
> +	uv = cvt->src.ptr + cvt->src.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];
> @@ -1449,7 +1468,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];
> @@ -1473,9 +1492,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];
> @@ -1493,7 +1512,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;
> @@ -1512,22 +1531,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.offsets[0];
> +	uint8_t *uv = cvt->dst.ptr + cvt->dst.offsets[1];
> +	const uint8_t *rgb24 = cvt->src.ptr;
> +	unsigned rgb24_stride = cvt->src.stride;
> +	unsigned planar_stride = cvt->dst.stride;
> +	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.color_encoding,
> +						    cvt->dst.color_range);
> +
> +	igt_assert_f(cvt->dst.fmt == 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];
> @@ -1556,7 +1575,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];
> @@ -1585,8 +1604,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];
> @@ -1603,7 +1622,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;
> @@ -1640,27 +1659,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.stride;
> +	unsigned int yuyv_stride = cvt->src.stride;
> +	uint8_t *buf = malloc(cvt->src.size);
> +	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.color_encoding,
> +						    cvt->src.color_range);
> +	const unsigned char *swz = yuyv_swizzle(cvt->src.fmt);
>  
>  	/*
>  	 * 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.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];
> @@ -1678,7 +1698,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;
>  
> @@ -1699,25 +1719,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.stride;
> +	unsigned yuyv_stride = cvt->dst.stride;
> +	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.color_encoding,
> +						    cvt->dst.color_range);
> +	const unsigned char *swz = yuyv_swizzle(cvt->dst.fmt);
> +
> +	igt_assert_f(cvt->dst.fmt == DRM_FORMAT_YUYV ||
> +		     cvt->dst.fmt == DRM_FORMAT_YVYU ||
> +		     cvt->dst.fmt == DRM_FORMAT_UYVY ||
> +		     cvt->dst.fmt == 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];
> @@ -1734,7 +1754,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;
>  
> @@ -1752,27 +1772,65 @@ 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.fmt == DRM_FORMAT_RGB888) {
> +		switch (cvt->src.fmt) {
> +		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.fmt == DRM_FORMAT_RGB888) {
> +		switch (cvt->dst.fmt) {
> +		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.fmt, cvt->dst.fmt);
> +}
> +
>  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,
> +			.fmt		= fb->drm_format,
> +			.stride		= blit->linear.stride,
> +			.size		= blit->linear.size,
> +			.color_encoding	= fb->color_encoding,
> +			.color_range	= fb->color_range,
> +		},
> +
> +		.src	= {
> +			.ptr		= blit->rgb24.map,
> +			.fmt		= DRM_FORMAT_RGB888,
> +			.stride		= blit->rgb24.stride,
> +			.size		= blit->rgb24.size,
> +		},
> +	};
> +
> +	fb_convert(&cvt);
>  	munmap(blit->rgb24.map, blit->rgb24.size);
>  
>  	if (blit->linear.handle)
> @@ -1788,6 +1846,8 @@ 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 };
> +
>  	igt_assert(blit);
>  
>  	blit->fd = fd;
> @@ -1809,21 +1869,24 @@ 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->rgb24.map;
> +	cvt.dst.fmt = DRM_FORMAT_RGB888;
> +	cvt.dst.stride = blit->rgb24.stride;
> +	cvt.dst.size = blit->rgb24.size;
> +
> +	cvt.src.ptr = blit->linear.map;
> +	cvt.src.fmt = fb->drm_format;
> +	cvt.src.stride	= blit->linear.stride;
> +	cvt.src.size = blit->linear.size;
> +	cvt.src.color_encoding = fb->color_encoding;
> +	cvt.src.color_range = fb->color_range;
> +	memcpy(cvt.dst.offsets, blit->linear.offsets,
> +	       sizeof(blit->linear.offsets));
> +
> +	fb_convert(&cvt);
>  
>  	fb->cairo_surface =
>  		cairo_image_surface_create_for_data(blit->rgb24.map,
> -- 
> 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 v6 05/13] fb: Fix ARGB8888 color depth

[Ville Syrjälä]

On Fri, Aug 31, 2018 at 03:12:14PM +0200, Maxime Ripard wrote:
> The ARGB8888 has been listed as having a 32 bits depth, while it actually
> is a 24 bits format. Fix this.

I think 32 is the more usual definition for 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 9aee02c35c16..7d46cfdb45d8 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

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

[igt-dev] ✓ Fi.CI.BAT: success for chamelium: Test the plane formats (rev5)

[Patchwork]

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4751 -> IGTPW_1773 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42165/revisions/5/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_evict:
      {fi-bsw-kefka}:     PASS -> DMESG-WARN (fdo#107709)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#103191, fdo#107362)

    {igt@pm_rpm@module-reload}:
      fi-glk-dsi:         NOTRUN -> WARN (fdo#107602, fdo#107708)

    
    ==== Possible fixes ====

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

    igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
      {fi-byt-clapper}:   FAIL (fdo#103191, fdo#107362) -> PASS +1

    igt@prime_vgem@basic-fence-flip:
      fi-skl-6770hq:      FAIL (fdo#104008) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107602 https://bugs.freedesktop.org/show_bug.cgi?id=107602
  fdo#107708 https://bugs.freedesktop.org/show_bug.cgi?id=107708
  fdo#107709 https://bugs.freedesktop.org/show_bug.cgi?id=107709


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

  Additional (2): fi-glk-dsi fi-cfl-8109u 
  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * IGT: IGT_4618 -> IGTPW_1773

  CI_DRM_4751: 2679376a6fe30a27b8734442d9b1af6a411f7bc7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1773: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1773/
  IGT_4618: 9d83154c898b5acc8b462d17104df50cfd71e9a0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+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-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

== Logs ==

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

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

[Patchwork]

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from IGT_4618_full -> IGTPW_1773_full =

== Summary - FAILURE ==

  Serious unknown changes coming with IGTPW_1773_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1773_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42165/revisions/5/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1773_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_atomic@plane_overlay_legacy:
      shard-hsw:          PASS -> FAIL +3
      shard-snb:          PASS -> FAIL +1

    igt@kms_plane@pixel-format-pipe-b-planes:
      shard-kbl:          PASS -> FAIL +10

    igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping:
      shard-glk:          PASS -> FAIL +10

    igt@kms_plane_scaling@pipe-c-scaler-with-pixel-format:
      shard-apl:          PASS -> FAIL +10

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ppgtt@flink-and-exit-vma-leak:
      shard-snb:          SKIP -> INCOMPLETE (fdo#105411)

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-apl:          PASS -> FAIL (fdo#105363, fdo#102887)

    igt@pm_rps@waitboost:
      shard-apl:          PASS -> FAIL (fdo#102250)

    
    ==== Possible fixes ====

    igt@gem_exec_await@wide-contexts:
      shard-glk:          FAIL (fdo#105900) -> PASS

    igt@gem_exec_big:
      shard-hsw:          INCOMPLETE (fdo#103540) -> PASS

    igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
      shard-hsw:          FAIL (fdo#105767) -> PASS

    igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
      shard-glk:          FAIL (fdo#105454, fdo#106509) -> PASS

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-mmap-cpu:
      shard-glk:          FAIL (fdo#103167) -> PASS

    igt@kms_setmode@basic:
      shard-kbl:          FAIL (fdo#99912) -> PASS

    igt@kms_vblank@pipe-a-ts-continuation-suspend:
      shard-kbl:          INCOMPLETE (fdo#103665, fdo#107556) -> PASS

    
  fdo#102250 https://bugs.freedesktop.org/show_bug.cgi?id=102250
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
  fdo#105767 https://bugs.freedesktop.org/show_bug.cgi?id=105767
  fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900
  fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
  fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * IGT: IGT_4618 -> IGTPW_1773
    * Linux: CI_DRM_4749 -> CI_DRM_4751

  CI_DRM_4749: 4a46c18fad0de38a78b4b0c848892de494324a17 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4751: 2679376a6fe30a27b8734442d9b1af6a411f7bc7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1773: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1773/
  IGT_4618: 9d83154c898b5acc8b462d17104df50cfd71e9a0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

Re: [igt-dev] [PATCH v6 05/13] fb: Fix ARGB8888 color depth

[Maxime Ripard]

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

On Fri, Aug 31, 2018 at 04:42:18PM +0300, Ville Syrjälä wrote:
> On Fri, Aug 31, 2018 at 03:12:14PM +0200, Maxime Ripard wrote:
> > The ARGB8888 has been listed as having a 32 bits depth, while it actually
> > is a 24 bits format. Fix this.
> 
> I think 32 is the more usual definition for this.

Isn't color depth about the color itself? I guess alpha wouldn't apply
in this case.

In the same format list:
  - RGB565 has a depth of 16
  - XRGB8888 has a depth of 24
  - XRGB2101010 has a depth of 30
  - ARGB8888 has a depth of 32

Which seems to indicate that the list indeed meant that the color
depth is about the color, and that ARGB8888 is wrong.

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 v6 05/13] fb: Fix ARGB8888 color depth

[Ville Syrjälä]

On Wed, Sep 05, 2018 at 11:35:49AM +0200, Maxime Ripard wrote:
> On Fri, Aug 31, 2018 at 04:42:18PM +0300, Ville Syrjälä wrote:
> > On Fri, Aug 31, 2018 at 03:12:14PM +0200, Maxime Ripard wrote:
> > > The ARGB8888 has been listed as having a 32 bits depth, while it actually
> > > is a 24 bits format. Fix this.
> > 
> > I think 32 is the more usual definition for this.
> 
> Isn't color depth about the color itself? I guess alpha wouldn't apply
> in this case.

Not according to X at least.

  visual:
    visual id:    0x6d
    class:    TrueColor
    depth:    32 planes
    available colormap entries:    256 per subfield
    red, green, blue masks:    0xff0000, 0xff00, 0xff
    significant bits in color specification:    8 bits

Same for eg. drm_mode_legacy_fb_format().

> 
> In the same format list:
>   - RGB565 has a depth of 16
>   - XRGB8888 has a depth of 24
>   - XRGB2101010 has a depth of 30
>   - ARGB8888 has a depth of 32
> 
> Which seems to indicate that the list indeed meant that the color
> depth is about the color, and that ARGB8888 is wrong.

None of those other things have alpha, so they are still
consistent with the depth==32 definition for ARGB888.

-- 
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 v6 03/13] fb: Create common function to convert frame formats

[Maxime Ripard]

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

On Fri, Aug 31, 2018 at 04:41:10PM +0300, Ville Syrjälä wrote:
> On Fri, Aug 31, 2018 at 03:12:12PM +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 | 243 ++++++++++++++++++++++++++++++++--------------------
> >  1 file changed, 153 insertions(+), 90 deletions(-)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 4061fedec0c1..1914233786a5 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -1384,6 +1384,24 @@ struct fb_convert_blit_upload {
> >  	struct fb_blit_linear linear;
> >  };
> >  
> > +struct fb_convert_buf {
> > +	void			*ptr;
> > +	unsigned int		stride;
> > +	unsigned int		size;
> > +	uint32_t		fmt;
> > +	enum igt_color_encoding	color_encoding;
> > +	enum igt_color_range	color_range;
> > +	uint32_t		offsets[4];
> > +};
> 
> I wonder if we can just use igt_fb for that instead of duplicating most
> of it?

I guess we could yes. I'll try to adjust the patches to do that.

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 v6 05/13] fb: Fix ARGB8888 color depth

[Maxime Ripard]

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

On Wed, Sep 05, 2018 at 01:14:34PM +0300, Ville Syrjälä wrote:
> > In the same format list:
> >   - RGB565 has a depth of 16
> >   - XRGB8888 has a depth of 24
> >   - XRGB2101010 has a depth of 30
> >   - ARGB8888 has a depth of 32
> > 
> > Which seems to indicate that the list indeed meant that the color
> > depth is about the color, and that ARGB8888 is wrong.
> 
> None of those other things have alpha, so they are still
> consistent with the depth==32 definition for ARGB888.

"24 bits almost always uses 8 bits of each of R, G, B. As of 2018
24-bit color depth is used by virtually every computer and phone
display and the vast majority of image storage formats. Almost all
cases where there are 32 bits per pixel mean that 24 are used for the
color, and the remaining 8 are the alpha channel or unused."

There's 32bpp, but the depth is 24 bits.

But maybe the X11 legacy says otherwise, I don't know. What's your
suggestion then? adding yet another field?

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 v6 05/13] fb: Fix ARGB8888 color depth

[Ville Syrjälä]

On Wed, Sep 05, 2018 at 03:19:03PM +0200, Maxime Ripard wrote:
> On Wed, Sep 05, 2018 at 01:14:34PM +0300, Ville Syrjälä wrote:
> > > In the same format list:
> > >   - RGB565 has a depth of 16
> > >   - XRGB8888 has a depth of 24
> > >   - XRGB2101010 has a depth of 30
> > >   - ARGB8888 has a depth of 32
> > > 
> > > Which seems to indicate that the list indeed meant that the color
> > > depth is about the color, and that ARGB8888 is wrong.
> > 
> > None of those other things have alpha, so they are still
> > consistent with the depth==32 definition for ARGB888.
> 
> "24 bits almost always uses 8 bits of each of R, G, B. As of 2018
> 24-bit color depth is used by virtually every computer and phone
> display and the vast majority of image storage formats. Almost all
> cases where there are 32 bits per pixel mean that 24 are used for the
> color, and the remaining 8 are the alpha channel or unused."
> 
> There's 32bpp, but the depth is 24 bits.
> 
> But maybe the X11 legacy says otherwise, I don't know. What's your
> suggestion then? adding yet another field?

Just leave it as is?

-- 
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 v6 05/13] fb: Fix ARGB8888 color depth

[Maxime Ripard]

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

On Wed, Sep 05, 2018 at 04:37:26PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 05, 2018 at 03:19:03PM +0200, Maxime Ripard wrote:
> > On Wed, Sep 05, 2018 at 01:14:34PM +0300, Ville Syrjälä wrote:
> > > > In the same format list:
> > > >   - RGB565 has a depth of 16
> > > >   - XRGB8888 has a depth of 24
> > > >   - XRGB2101010 has a depth of 30
> > > >   - ARGB8888 has a depth of 32
> > > > 
> > > > Which seems to indicate that the list indeed meant that the color
> > > > depth is about the color, and that ARGB8888 is wrong.
> > > 
> > > None of those other things have alpha, so they are still
> > > consistent with the depth==32 definition for ARGB888.
> > 
> > "24 bits almost always uses 8 bits of each of R, G, B. As of 2018
> > 24-bit color depth is used by virtually every computer and phone
> > display and the vast majority of image storage formats. Almost all
> > cases where there are 32 bits per pixel mean that 24 are used for the
> > color, and the remaining 8 are the alpha channel or unused."
> > 
> > There's 32bpp, but the depth is 24 bits.
> > 
> > But maybe the X11 legacy says otherwise, I don't know. What's your
> > suggestion then? adding yet another field?
> 
> Just leave it as is?

I need the actual color depth in order to check whether the conversion
from one format to another would lead to a lesser number of bits per
color, and as such should fall back to a more conservative
pattern. See the later patches in this series.

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 v6 03/13] fb: Create common function to convert frame formats

[Maxime Ripard]

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

On Fri, Aug 31, 2018 at 04:41:10PM +0300, Ville Syrjälä wrote:
> On Fri, Aug 31, 2018 at 03:12:12PM +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 | 243 ++++++++++++++++++++++++++++++++--------------------
> >  1 file changed, 153 insertions(+), 90 deletions(-)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 4061fedec0c1..1914233786a5 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -1384,6 +1384,24 @@ struct fb_convert_blit_upload {
> >  	struct fb_blit_linear linear;
> >  };
> >  
> > +struct fb_convert_buf {
> > +	void			*ptr;
> > +	unsigned int		stride;
> > +	unsigned int		size;
> > +	uint32_t		fmt;
> > +	enum igt_color_encoding	color_encoding;
> > +	enum igt_color_range	color_range;
> > +	uint32_t		offsets[4];
> > +};
> 
> I wonder if we can just use igt_fb for that instead of duplicating most
> of it?

After looking into it again, now I remember why I didn't do what you
suggested. In the case where you are in the YUYV path, with a "shadow"
buffer being used to perform the cairo operations on top of an YUV
buffer, you end up with conversions in create_cairo_surface__convert
and destroy_cairo_surface__convert, getting a fb_convert_blit_upload
structure as an argument.

That structure however is used to convert one buffer to the shadow
buffer, or the other way around at destroy time. However, a single
igt_fb is allocated for that structure, which makes it impossible to
assign the second fb in that particular case. All the informations
needed to perform the conversion are stored in multiple structures
(fb_convert_blit_upload.linear, fb_convert_blit_upload.blit, and the
actual igt_fb instance) that we need to aggregate to find the correct
conversion parameters.

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 v6 03/13] fb: Create common function to convert frame formats

[Ville Syrjälä]

On Thu, Sep 06, 2018 at 01:41:29PM +0200, Maxime Ripard wrote:
> On Fri, Aug 31, 2018 at 04:41:10PM +0300, Ville Syrjälä wrote:
> > On Fri, Aug 31, 2018 at 03:12:12PM +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 | 243 ++++++++++++++++++++++++++++++++--------------------
> > >  1 file changed, 153 insertions(+), 90 deletions(-)
> > > 
> > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > index 4061fedec0c1..1914233786a5 100644
> > > --- a/lib/igt_fb.c
> > > +++ b/lib/igt_fb.c
> > > @@ -1384,6 +1384,24 @@ struct fb_convert_blit_upload {
> > >  	struct fb_blit_linear linear;
> > >  };
> > >  
> > > +struct fb_convert_buf {
> > > +	void			*ptr;
> > > +	unsigned int		stride;
> > > +	unsigned int		size;
> > > +	uint32_t		fmt;
> > > +	enum igt_color_encoding	color_encoding;
> > > +	enum igt_color_range	color_range;
> > > +	uint32_t		offsets[4];
> > > +};
> > 
> > I wonder if we can just use igt_fb for that instead of duplicating most
> > of it?
> 
> After looking into it again, now I remember why I didn't do what you
> suggested. In the case where you are in the YUYV path, with a "shadow"
> buffer being used to perform the cairo operations on top of an YUV
> buffer, you end up with conversions in create_cairo_surface__convert
> and destroy_cairo_surface__convert, getting a fb_convert_blit_upload
> structure as an argument.
> 
> That structure however is used to convert one buffer to the shadow
> buffer, or the other way around at destroy time. However, a single
> igt_fb is allocated for that structure, which makes it impossible to
> assign the second fb in that particular case. All the informations
> needed to perform the conversion are stored in multiple structures
> (fb_convert_blit_upload.linear, fb_convert_blit_upload.blit, and the
> actual igt_fb instance) that we need to aggregate to find the correct
> conversion parameters.

The patch series I mentioned before [1] changes fb_blit_linear to inherit
from igt_fb. So presumably that should help?

[1] https://patchwork.freedesktop.org/series/46876/

-- 
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 v6 03/13] fb: Create common function to convert frame formats

[Maxime Ripard]

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

On Thu, Sep 06, 2018 at 03:11:16PM +0300, Ville Syrjälä wrote:
> On Thu, Sep 06, 2018 at 01:41:29PM +0200, Maxime Ripard wrote:
> > On Fri, Aug 31, 2018 at 04:41:10PM +0300, Ville Syrjälä wrote:
> > > On Fri, Aug 31, 2018 at 03:12:12PM +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 | 243 ++++++++++++++++++++++++++++++++--------------------
> > > >  1 file changed, 153 insertions(+), 90 deletions(-)
> > > > 
> > > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > > index 4061fedec0c1..1914233786a5 100644
> > > > --- a/lib/igt_fb.c
> > > > +++ b/lib/igt_fb.c
> > > > @@ -1384,6 +1384,24 @@ struct fb_convert_blit_upload {
> > > >  	struct fb_blit_linear linear;
> > > >  };
> > > >  
> > > > +struct fb_convert_buf {
> > > > +	void			*ptr;
> > > > +	unsigned int		stride;
> > > > +	unsigned int		size;
> > > > +	uint32_t		fmt;
> > > > +	enum igt_color_encoding	color_encoding;
> > > > +	enum igt_color_range	color_range;
> > > > +	uint32_t		offsets[4];
> > > > +};
> > > 
> > > I wonder if we can just use igt_fb for that instead of duplicating most
> > > of it?
> > 
> > After looking into it again, now I remember why I didn't do what you
> > suggested. In the case where you are in the YUYV path, with a "shadow"
> > buffer being used to perform the cairo operations on top of an YUV
> > buffer, you end up with conversions in create_cairo_surface__convert
> > and destroy_cairo_surface__convert, getting a fb_convert_blit_upload
> > structure as an argument.
> > 
> > That structure however is used to convert one buffer to the shadow
> > buffer, or the other way around at destroy time. However, a single
> > igt_fb is allocated for that structure, which makes it impossible to
> > assign the second fb in that particular case. All the informations
> > needed to perform the conversion are stored in multiple structures
> > (fb_convert_blit_upload.linear, fb_convert_blit_upload.blit, and the
> > actual igt_fb instance) that we need to aggregate to find the correct
> > conversion parameters.
> 
> The patch series I mentioned before [1] changes fb_blit_linear to inherit
> from igt_fb. So presumably that should help?
> 
> [1] https://patchwork.freedesktop.org/series/46876/

Yeah, except that the shadow buffer would still not have a backing
igt_fb, and we would need to assume that all the parameters for that
buffer are the same than for the source, including the offsets and
stride. This looks a bit fragile.

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 v6 03/13] fb: Create common function to convert frame formats

[Ville Syrjälä]

On Thu, Sep 06, 2018 at 02:49:17PM +0200, Maxime Ripard wrote:
> On Thu, Sep 06, 2018 at 03:11:16PM +0300, Ville Syrjälä wrote:
> > On Thu, Sep 06, 2018 at 01:41:29PM +0200, Maxime Ripard wrote:
> > > On Fri, Aug 31, 2018 at 04:41:10PM +0300, Ville Syrjälä wrote:
> > > > On Fri, Aug 31, 2018 at 03:12:12PM +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 | 243 ++++++++++++++++++++++++++++++++--------------------
> > > > >  1 file changed, 153 insertions(+), 90 deletions(-)
> > > > > 
> > > > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > > > index 4061fedec0c1..1914233786a5 100644
> > > > > --- a/lib/igt_fb.c
> > > > > +++ b/lib/igt_fb.c
> > > > > @@ -1384,6 +1384,24 @@ struct fb_convert_blit_upload {
> > > > >  	struct fb_blit_linear linear;
> > > > >  };
> > > > >  
> > > > > +struct fb_convert_buf {
> > > > > +	void			*ptr;
> > > > > +	unsigned int		stride;
> > > > > +	unsigned int		size;
> > > > > +	uint32_t		fmt;
> > > > > +	enum igt_color_encoding	color_encoding;
> > > > > +	enum igt_color_range	color_range;
> > > > > +	uint32_t		offsets[4];
> > > > > +};
> > > > 
> > > > I wonder if we can just use igt_fb for that instead of duplicating most
> > > > of it?
> > > 
> > > After looking into it again, now I remember why I didn't do what you
> > > suggested. In the case where you are in the YUYV path, with a "shadow"
> > > buffer being used to perform the cairo operations on top of an YUV
> > > buffer, you end up with conversions in create_cairo_surface__convert
> > > and destroy_cairo_surface__convert, getting a fb_convert_blit_upload
> > > structure as an argument.
> > > 
> > > That structure however is used to convert one buffer to the shadow
> > > buffer, or the other way around at destroy time. However, a single
> > > igt_fb is allocated for that structure, which makes it impossible to
> > > assign the second fb in that particular case. All the informations
> > > needed to perform the conversion are stored in multiple structures
> > > (fb_convert_blit_upload.linear, fb_convert_blit_upload.blit, and the
> > > actual igt_fb instance) that we need to aggregate to find the correct
> > > conversion parameters.
> > 
> > The patch series I mentioned before [1] changes fb_blit_linear to inherit
> > from igt_fb. So presumably that should help?
> > 
> > [1] https://patchwork.freedesktop.org/series/46876/
> 
> Yeah, except that the shadow buffer would still not have a backing
> igt_fb,

Ah, the rgb24 thing? Hmm. I guess we could make that into an igt_fb as
well.

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