[igt-dev] [PATCH v3 00/11] chamelium: Test the plane formats

[igt-dev] [PATCH v3 00/11] chamelium: Test the plane formats

[Maxime Ripard]

Hi,

Here is a first non-RFC version of 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.

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 pixman at the moment to perform
the format conversion.

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 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 (11):
  tests: Add vc4 test suite
  fb: Add buffer map/unmap functions
  fb: Add format conversion routine
  fb: Add more formats
  chamelium: Split CRC test function in two
  chamelium: Use preferred mode when testing a single mode
  chamelium: Add function to generate our test pattern
  chamelium: Change our pattern for a custom one
  chamelium: Add format support
  chamelium: Add format subtests
  tests: Add chamelium formats subtests to vc4 test lists

 lib/igt_fb.c                             | 148 ++++++++++++++--
 lib/igt_fb.h                             |   4 +-
 tests/kms_chamelium.c                    | 220 ++++++++++++++++++------
 tests/vc4_ci/README                      |  38 ++++-
 tests/vc4_ci/vc4-chamelium-fast.testlist |  14 ++-
 tests/vc4_ci/vc4-chamelium.testlist      |  19 ++-
 tests/vc4_ci/vc4.testlist                |  35 ++++-
 7 files changed, 417 insertions(+), 61 deletions(-)
 create mode 100644 tests/vc4_ci/README
 create mode 100644 tests/vc4_ci/vc4-chamelium-fast.testlist
 create mode 100644 tests/vc4_ci/vc4-chamelium.testlist
 create mode 100644 tests/vc4_ci/vc4.testlist

base-commit: 83ba5b7d3bde48b383df41792fc9c955a5a23bdb
-- 
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 v3 01/11] tests: Add vc4 test suite

[Maxime Ripard]

Add some various test suites relevant for the vc4 drm driver.

Acked-by: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 tests/vc4_ci/README                      | 38 +++++++++++++++++++++++++-
 tests/vc4_ci/vc4-chamelium-fast.testlist |  4 +++-
 tests/vc4_ci/vc4-chamelium.testlist      |  9 ++++++-
 tests/vc4_ci/vc4.testlist                | 35 +++++++++++++++++++++++-
 4 files changed, 86 insertions(+)
 create mode 100644 tests/vc4_ci/README
 create mode 100644 tests/vc4_ci/vc4-chamelium-fast.testlist
 create mode 100644 tests/vc4_ci/vc4-chamelium.testlist
 create mode 100644 tests/vc4_ci/vc4.testlist

diff --git a/tests/vc4_ci/README b/tests/vc4_ci/README
new file mode 100644
index 000000000000..e1de5c6120da
--- /dev/null
+++ b/tests/vc4_ci/README
@@ -0,0 +1,38 @@
+This directory contains test lists to be used for vc4's DRM support. The files
+are passed to piglit with the --test-list parameter directly.
+
+The test lists are contained in the IGT repository for several
+reasons:
+
+- The lists stay synchronized with the IGT codebase.
+- Public availability. Kernel developers can see what tests are run,
+  and can see what changes are done to the set, when, and why.
+- Explicit test lists in general make it possible to implement a new
+  test without having it run by everyone else before the tests and / or setup
+  are ready for it.
+
+Changing the test lists should only happen with approval from the vc4
+maintainer, Eric Anholt (eric@anholt.net).
+
+============
+vc4.testlist
+============
+
+This test list is meant as a general test suite without any time
+restriction for the vc4 DRM driver, combining generic DRM and KMS tests.
+
+======================
+vc4-chamelium.testlist
+======================
+
+This test list is meant to test the vc4 driver using Google's Chamelium
+board as a testing device. This doesn't have any time restriction, and
+will include test that will run for a significant time.
+
+===========================
+vc4-chamelium-fast.testlist
+===========================
+
+This test list is a variant of the previous one, with only tests
+supposed to run quickly. Therefore, the whole suite should be pretty
+fast to execute and is meant to be used as a fast-loop CI.
diff --git a/tests/vc4_ci/vc4-chamelium-fast.testlist b/tests/vc4_ci/vc4-chamelium-fast.testlist
new file mode 100644
index 000000000000..964167b82d00
--- /dev/null
+++ b/tests/vc4_ci/vc4-chamelium-fast.testlist
@@ -0,0 +1,4 @@
+igt@kms_chamelium@hdmi-crc-fast
+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
new file mode 100644
index 000000000000..b00f54cd9c46
--- /dev/null
+++ b/tests/vc4_ci/vc4-chamelium.testlist
@@ -0,0 +1,9 @@
+igt@kms_chamelium@hdmi-crc-fast
+igt@kms_chamelium@hdmi-crc-multiple
+igt@kms_chamelium@hdmi-crc-single
+igt@kms_chamelium@hdmi-edid-read
+igt@kms_chamelium@hdmi-frame-dump
+igt@kms_chamelium@hdmi-hpd
+igt@kms_chamelium@hdmi-hpd-fast
+igt@kms_chamelium@hdmi-hpd-storm
+igt@kms_chamelium@hdmi-hpd-storm-disable
diff --git a/tests/vc4_ci/vc4.testlist b/tests/vc4_ci/vc4.testlist
new file mode 100644
index 000000000000..e86d4c815c56
--- /dev/null
+++ b/tests/vc4_ci/vc4.testlist
@@ -0,0 +1,35 @@
+igt@vc4_create_bo@create-bo-0
+igt@vc4_create_bo@create-bo-4096
+igt@vc4_create_bo@create-bo-zeroed
+igt@vc4_dmabuf_poll@poll-read-waits-until-write-done
+igt@vc4_dmabuf_poll@poll-write-waits-until-write-done
+igt@vc4_label_bo@set-label
+igt@vc4_label_bo@set-bad-handle
+igt@vc4_label_bo@set-bad-name
+igt@vc4_label_bo@set-kernel-name
+igt@vc4_lookup_fail@bad-color-write
+igt@vc4_purgeable_bo@mark-willneed
+igt@vc4_purgeable_bo@mark-purgeable
+igt@vc4_purgeable_bo@mark-purgeable-twice
+igt@vc4_purgeable_bo@access-purgeable-bo-mem
+igt@vc4_purgeable_bo@access-purged-bo-mem
+igt@vc4_purgeable_bo@mark-unpurgeable-check-retained
+igt@vc4_purgeable_bo@mark-unpurgeable-purged
+igt@vc4_purgeable_bo@free-purged-bo
+igt@vc4_tiling@get-bad-handle
+igt@vc4_tiling@set-bad-handle
+igt@vc4_tiling@get-bad-flags
+igt@vc4_tiling@set-bad-flags
+igt@vc4_tiling@get-bad-modifier
+igt@vc4_tiling@set-bad-modifier
+igt@vc4_tiling@set-get
+igt@vc4_tiling@get-after-free
+igt@vc4_wait_bo@bad-bo
+igt@vc4_wait_bo@bad-pad
+igt@vc4_wait_bo@unused-bo-0ns
+igt@vc4_wait_bo@unused-bo-1ns
+igt@vc4_wait_bo@used-bo
+igt@vc4_wait_bo@used-bo-0ns
+igt@vc4_wait_bo@used-bo-1ns
+igt@vc4_wait_seqno@bad-seqno-0ns
+igt@vc4_wait_seqno@bad-seqno-1ns
-- 
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 v3 02/11] 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.

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

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 7404ba7cb0c0..66ded11a73ca 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1274,18 +1274,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;
 
@@ -1296,6 +1301,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),
@@ -1535,6 +1547,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 023b069db592..f5f6d31445a0 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -128,6 +128,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 v3 03/11] 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.

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 pixman 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.

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

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 66ded11a73ca..e5e66f7df7f5 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -28,6 +28,7 @@
 #include <stdio.h>
 #include <math.h>
 #include <inttypes.h>
+#include <pixman.h>
 
 #include "drmtest.h"
 #include "igt_fb.h"
@@ -54,24 +55,27 @@
  * functions to work with these pixel format codes.
  */
 
+#define PIXMAN_invalid	0
+
 /* drm fourcc/cairo format maps */
-#define DF(did, cid, ...)	\
-	{ DRM_FORMAT_##did, CAIRO_FORMAT_##cid, # did, __VA_ARGS__ }
+#define DF(did, cid, pid, ...)					\
+	{ DRM_FORMAT_##did, CAIRO_FORMAT_##cid, PIXMAN_##pid, # did, __VA_ARGS__ }
 static struct format_desc_struct {
 	uint32_t drm_id;
 	cairo_format_t cairo_id;
+	pixman_format_code_t pixman_id;
 	const char *name;
 	int bpp;
 	int depth;
 	int planes;
 	int plane_bpp[4];
 } format_desc[] = {
-	DF(RGB565,	RGB16_565,	16, 16),
-	//DF(RGB888,	INVALID,	24, 24),
-	DF(XRGB8888,	RGB24,		32, 24),
-	DF(XRGB2101010,	RGB30,		32, 30),
-	DF(ARGB8888,	ARGB32,		32, 32),
-	DF(NV12,	RGB24,		32, -1, 2, {8, 16}),
+	DF(RGB565,	RGB16_565,	r5g6b5,		16, 16),
+	//DF(RGB888,	INVALID,	r8g8b8,		24, 24),
+	DF(XRGB8888,	RGB24,		x8r8g8b8,	32, 24),
+	DF(XRGB2101010,	RGB30,		x2r10g10b10,	32, 30),
+	DF(ARGB8888,	ARGB32,		a8r8g8b8,	32, 32),
+	DF(NV12,	RGB24,		invalid,	32, -1, 2, {8, 16}),
 };
 #undef DF
 
@@ -1123,6 +1127,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;
@@ -1676,6 +1692,64 @@ 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)
+{
+	pixman_format_code_t src_pixman = drm_format_to_pixman(src->drm_format);
+	pixman_format_code_t dst_pixman = drm_format_to_pixman(dst_fourcc);
+	pixman_image_t *dst_image, *src_image;
+	void *dst_ptr, *src_ptr;
+	int fb_id;
+
+	igt_assert(src_pixman != PIXMAN_invalid);
+	igt_assert(dst_pixman != PIXMAN_invalid);
+
+	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);
+
+	src_image = pixman_image_create_bits(src_pixman,
+					     src->width, src->height,
+					     src_ptr, src->stride);
+	igt_assert(src_image);
+
+	dst_image = pixman_image_create_bits(dst_pixman,
+					     dst->width, dst->height,
+					     dst_ptr, dst->stride);
+	igt_assert(dst_image);
+
+	pixman_image_composite(PIXMAN_OP_SRC, src_image, NULL, dst_image,
+			       0, 0, 0, 0, 0, 0, dst->width, dst->height);
+	pixman_image_unref(dst_image);
+	pixman_image_unref(src_image);
+	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 f5f6d31445a0..c4ca39866b9a 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -126,6 +126,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 v3 04/11] 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.

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

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index e5e66f7df7f5..7ae3dbcab5f1 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -70,10 +70,16 @@ static struct format_desc_struct {
 	int planes;
 	int plane_bpp[4];
 } format_desc[] = {
+	DF(ARGB1555,	INVALID,	a1r5g5b5,	16, 16),
+	DF(XRGB1555,	INVALID,	x1r5g5b5,	16, 16),
 	DF(RGB565,	RGB16_565,	r5g6b5,		16, 16),
-	//DF(RGB888,	INVALID,	r8g8b8,		24, 24),
+	DF(BGR565,	INVALID,	b5g6r5,		16, 16),
+	DF(BGR888,	INVALID,	b8g8r8,		24, 24),
+	DF(RGB888,	INVALID,	r8g8b8,		24, 24),
 	DF(XRGB8888,	RGB24,		x8r8g8b8,	32, 24),
+	DF(XBGR8888,	INVALID,	x8b8g8r8,	32, 24),
 	DF(XRGB2101010,	RGB30,		x2r10g10b10,	32, 30),
+	DF(ABGR8888,	INVALID,	a8b8g8r8,	32, 32),
 	DF(ARGB8888,	ARGB32,		a8r8g8b8,	32, 32),
 	DF(NV12,	RGB24,		invalid,	32, -1, 2, {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 v3 05/11] 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 | 121 +++++++++++++++++++++++++------------------
 1 file changed, 73 insertions(+), 48 deletions(-)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 2bc34d07788d..490bacc6f5fe 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -486,20 +486,61 @@ 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;
+	drmModeModeInfo *mode;
 
 	reset_state(data, port);
 
@@ -508,46 +549,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;
-
-	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);
+	do_test_display_crc(data, port, output, &connector->modes[0], count);
 
-		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 +833,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 +897,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 v3 06/11] chamelium: Use preferred mode when testing a single mode

[Maxime Ripard]

The current code is testing the first mode in the connector list when it's
testing a single mode. While this is arbitrarily chosen, it makes more
sense to use the preferred mode reported by the display.

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

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 490bacc6f5fe..5d7fb83fb74f 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -534,6 +534,20 @@ static void do_test_display_crc(data_t *data, struct chamelium_port *port,
 	igt_remove_fb(data->drm_fd, &fb);
 }
 
+static drmModeModeInfo *find_preferred_mode(drmModeConnector *connector)
+{
+	int i;
+
+	for (i = 0; i < connector->count_modes; i++) {
+		drmModeModeInfo *mode = &connector->modes[i];
+
+		if (mode->type & DRM_MODE_TYPE_PREFERRED)
+			return mode;
+	}
+
+	return NULL;
+}
+
 static void test_display_crc_one_mode(data_t *data, struct chamelium_port *port,
 				      int count)
 {
@@ -549,7 +563,10 @@ 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);
+	mode = find_preferred_mode(connector);
+	igt_assert(mode);
+
+	do_test_display_crc(data, port, output, mode, count);
 
 	drmModeFreeConnector(connector);
 }
-- 
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 v3 07/11] chamelium: Add function to generate our test pattern

[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 our pattern in the chamelium
code, keeping the current pattern for now. We will extend it in a later
patch.

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

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 5d7fb83fb74f..f14e3ebb4b87 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -486,6 +486,17 @@ enable_output(data_t *data,
 	drmModeFreeConnector(connector);
 }
 
+static int chamelium_get_pattern_fb(data_t *data, drmModeModeInfo *mode,
+				    uint32_t fourcc, struct igt_fb *fb)
+{
+	igt_assert(fourcc == DRM_FORMAT_XRGB8888);
+
+	return igt_create_color_pattern_fb(data->drm_fd, mode->hdisplay,
+					   mode->vdisplay, fourcc,
+					   LOCAL_DRM_FORMAT_MOD_NONE,
+					   0, 0, 0, fb);
+}
+
 static void do_test_display_crc(data_t *data, struct chamelium_port *port,
 				igt_output_t *output, drmModeModeInfo *mode,
 				int count)
@@ -496,12 +507,7 @@ 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);
+	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 v3 08/11] chamelium: Change our pattern for a custom one

[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.

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 the 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 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 | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index f14e3ebb4b87..4f4984b51b41 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -486,15 +486,40 @@ 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);
 
-	return igt_create_color_pattern_fb(data->drm_fd, mode->hdisplay,
-					   mode->vdisplay, fourcc,
-					   LOCAL_DRM_FORMAT_MOD_NONE,
-					   0, 0, 0, fb);
+	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,
-- 
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 v3 09/11] 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 4f4984b51b41..aa0aa4fd89ef 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -524,21 +524,25 @@ 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;
 
 	fb_id = chamelium_get_pattern_fb(data, mode, DRM_FORMAT_XRGB8888, &fb);
 	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
@@ -562,6 +566,7 @@ 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);
 }
 
@@ -580,7 +585,7 @@ static drmModeModeInfo *find_preferred_mode(drmModeConnector *connector)
 }
 
 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;
@@ -597,13 +602,13 @@ static void test_display_crc_one_mode(data_t *data, struct chamelium_port *port,
 	mode = find_preferred_mode(connector);
 	igt_assert(mode);
 
-	do_test_display_crc(data, port, output, mode, count);
+	do_test_display_crc(data, port, output, mode, 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;
@@ -620,7 +625,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);
@@ -881,13 +886,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);
@@ -945,13 +953,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 v3 10/11] 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 aa0aa4fd89ef..e77b930a8298 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -964,6 +964,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 v3 11/11] 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 v3 03/11] fb: Add format conversion routine

[Ville Syrjälä]

On Thu, May 24, 2018 at 04:24:42PM +0200, Maxime Ripard wrote:
> 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.

We already have the capability to do format conversions automagically.
I'm extending it to handle more YUV stuff here:
https://patchwork.freedesktop.org/series/43651/

Can you hook up the pixman stuff in the same way so that we don't
have to any explicit conversion stuff in the tests themselves?

> 
> 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 pixman 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.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  lib/igt_fb.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>  lib/igt_fb.h |  2 +-
>  2 files changed, 84 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 66ded11a73ca..e5e66f7df7f5 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -28,6 +28,7 @@
>  #include <stdio.h>
>  #include <math.h>
>  #include <inttypes.h>
> +#include <pixman.h>
>  
>  #include "drmtest.h"
>  #include "igt_fb.h"
> @@ -54,24 +55,27 @@
>   * functions to work with these pixel format codes.
>   */
>  
> +#define PIXMAN_invalid	0
> +
>  /* drm fourcc/cairo format maps */
> -#define DF(did, cid, ...)	\
> -	{ DRM_FORMAT_##did, CAIRO_FORMAT_##cid, # did, __VA_ARGS__ }
> +#define DF(did, cid, pid, ...)					\
> +	{ DRM_FORMAT_##did, CAIRO_FORMAT_##cid, PIXMAN_##pid, # did, __VA_ARGS__ }
>  static struct format_desc_struct {
>  	uint32_t drm_id;
>  	cairo_format_t cairo_id;
> +	pixman_format_code_t pixman_id;
>  	const char *name;
>  	int bpp;
>  	int depth;
>  	int planes;
>  	int plane_bpp[4];
>  } format_desc[] = {
> -	DF(RGB565,	RGB16_565,	16, 16),
> -	//DF(RGB888,	INVALID,	24, 24),
> -	DF(XRGB8888,	RGB24,		32, 24),
> -	DF(XRGB2101010,	RGB30,		32, 30),
> -	DF(ARGB8888,	ARGB32,		32, 32),
> -	DF(NV12,	RGB24,		32, -1, 2, {8, 16}),
> +	DF(RGB565,	RGB16_565,	r5g6b5,		16, 16),
> +	//DF(RGB888,	INVALID,	r8g8b8,		24, 24),
> +	DF(XRGB8888,	RGB24,		x8r8g8b8,	32, 24),
> +	DF(XRGB2101010,	RGB30,		x2r10g10b10,	32, 30),
> +	DF(ARGB8888,	ARGB32,		a8r8g8b8,	32, 32),
> +	DF(NV12,	RGB24,		invalid,	32, -1, 2, {8, 16}),
>  };
>  #undef DF
>  
> @@ -1123,6 +1127,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;
> @@ -1676,6 +1692,64 @@ 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)
> +{
> +	pixman_format_code_t src_pixman = drm_format_to_pixman(src->drm_format);
> +	pixman_format_code_t dst_pixman = drm_format_to_pixman(dst_fourcc);
> +	pixman_image_t *dst_image, *src_image;
> +	void *dst_ptr, *src_ptr;
> +	int fb_id;
> +
> +	igt_assert(src_pixman != PIXMAN_invalid);
> +	igt_assert(dst_pixman != PIXMAN_invalid);
> +
> +	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);
> +
> +	src_image = pixman_image_create_bits(src_pixman,
> +					     src->width, src->height,
> +					     src_ptr, src->stride);
> +	igt_assert(src_image);
> +
> +	dst_image = pixman_image_create_bits(dst_pixman,
> +					     dst->width, dst->height,
> +					     dst_ptr, dst->stride);
> +	igt_assert(dst_image);
> +
> +	pixman_image_composite(PIXMAN_OP_SRC, src_image, NULL, dst_image,
> +			       0, 0, 0, 0, 0, 0, dst->width, dst->height);
> +	pixman_image_unref(dst_image);
> +	pixman_image_unref(src_image);
> +	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 f5f6d31445a0..c4ca39866b9a 100644
> --- a/lib/igt_fb.h
> +++ b/lib/igt_fb.h
> @@ -126,6 +126,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

-- 
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 v3 03/11] fb: Add format conversion routine

[Maxime Ripard]

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

On Thu, May 24, 2018 at 05:58:13PM +0300, Ville Syrjälä wrote:
> On Thu, May 24, 2018 at 04:24:42PM +0200, Maxime Ripard wrote:
> > 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.
> 
> We already have the capability to do format conversions automagically.
> I'm extending it to handle more YUV stuff here:
> https://patchwork.freedesktop.org/series/43651/
> 
> Can you hook up the pixman stuff in the same way so that we don't
> have to any explicit conversion stuff in the tests themselves?

I really think that the assumption that cairo will handle it is not a
proper fit. Cairo support is very limited, just like pixman is to a
lesser extent, and we should just break the assumption that we will
get a cairo surface all the time.

If we push things further (and we have the intention to do so), we
will have to support proprietary format that are very unlikely to be
supported by cairo at some point.

However, we do intend to support YUV at some point, so reusing the
routines definitely make some sense.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
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 v3 03/11] fb: Add format conversion routine

[Ville Syrjälä]

On Thu, May 24, 2018 at 05:19:04PM +0200, Maxime Ripard wrote:
> On Thu, May 24, 2018 at 05:58:13PM +0300, Ville Syrjälä wrote:
> > On Thu, May 24, 2018 at 04:24:42PM +0200, Maxime Ripard wrote:
> > > 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.
> > 
> > We already have the capability to do format conversions automagically.
> > I'm extending it to handle more YUV stuff here:
> > https://patchwork.freedesktop.org/series/43651/
> > 
> > Can you hook up the pixman stuff in the same way so that we don't
> > have to any explicit conversion stuff in the tests themselves?
> 
> I really think that the assumption that cairo will handle it is not a
> proper fit. Cairo support is very limited, just like pixman is to a
> lesser extent, and we should just break the assumption that we will
> get a cairo surface all the time.

Not sure what you're saying here. We don't assume cairo handles all
the formats, well, because it doesn't. We just magically convert
to/from XRGB888 so that cairo doesn't have to handle anything else.

-- 
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: failure for chamelium: Test the plane formats (rev2)

[Patchwork]

== Series Details ==

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

== Summary ==

Applying: tests: Add vc4 test suite
Applying: fb: Add buffer map/unmap functions
Applying: fb: Add format conversion routine
Patch failed at 0003 fb: Add format conversion routine
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

Re: [igt-dev] [PATCH v3 03/11] fb: Add format conversion routine

[Maxime Ripard]

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

On Thu, May 24, 2018 at 06:40:36PM +0300, Ville Syrjälä wrote:
> On Thu, May 24, 2018 at 05:19:04PM +0200, Maxime Ripard wrote:
> > On Thu, May 24, 2018 at 05:58:13PM +0300, Ville Syrjälä wrote:
> > > On Thu, May 24, 2018 at 04:24:42PM +0200, Maxime Ripard wrote:
> > > > 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.
> > > 
> > > We already have the capability to do format conversions automagically.
> > > I'm extending it to handle more YUV stuff here:
> > > https://patchwork.freedesktop.org/series/43651/
> > > 
> > > Can you hook up the pixman stuff in the same way so that we don't
> > > have to any explicit conversion stuff in the tests themselves?
> > 
> > I really think that the assumption that cairo will handle it is not a
> > proper fit. Cairo support is very limited, just like pixman is to a
> > lesser extent, and we should just break the assumption that we will
> > get a cairo surface all the time.
> 
> Not sure what you're saying here. We don't assume cairo handles all
> the formats, well, because it doesn't. We just magically convert
> to/from XRGB888 so that cairo doesn't have to handle anything else.

My point was that from what I can tell from your patches, you allow to
create an igt_fb with whatever format we want, and then convert it to
XRGB8888 when one calls igt_get_cairo_surface.

I need something more generic, but even if we simplify to the
operation that I need now, I basically needs the exact opposite: to be
able to take an XRGB8888 buffer (that is our pattern) and get an
igt_fb in an arbitrary format.

And that buffer cannot be created by cairo, so relying on the
destroy_cairo_surface__convert to convert it back from XRGB8888 to
whatever format doesn't work.

So yeah, we can't really rely on cairo in this particuliar case, and
the buffer conversions need be to made explicit.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
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 v3 03/11] fb: Add format conversion routine

[Ville Syrjälä]

On Mon, Jun 04, 2018 at 11:54:03AM +0200, Maxime Ripard wrote:
> On Thu, May 24, 2018 at 06:40:36PM +0300, Ville Syrjälä wrote:
> > On Thu, May 24, 2018 at 05:19:04PM +0200, Maxime Ripard wrote:
> > > On Thu, May 24, 2018 at 05:58:13PM +0300, Ville Syrjälä wrote:
> > > > On Thu, May 24, 2018 at 04:24:42PM +0200, Maxime Ripard wrote:
> > > > > 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.
> > > > 
> > > > We already have the capability to do format conversions automagically.
> > > > I'm extending it to handle more YUV stuff here:
> > > > https://patchwork.freedesktop.org/series/43651/
> > > > 
> > > > Can you hook up the pixman stuff in the same way so that we don't
> > > > have to any explicit conversion stuff in the tests themselves?
> > > 
> > > I really think that the assumption that cairo will handle it is not a
> > > proper fit. Cairo support is very limited, just like pixman is to a
> > > lesser extent, and we should just break the assumption that we will
> > > get a cairo surface all the time.
> > 
> > Not sure what you're saying here. We don't assume cairo handles all
> > the formats, well, because it doesn't. We just magically convert
> > to/from XRGB888 so that cairo doesn't have to handle anything else.
> 
> My point was that from what I can tell from your patches, you allow to
> create an igt_fb with whatever format we want, and then convert it to
> XRGB8888 when one calls igt_get_cairo_surface.
> 
> I need something more generic, but even if we simplify to the
> operation that I need now, I basically needs the exact opposite: to be
> able to take an XRGB8888 buffer (that is our pattern) and get an
> igt_fb in an arbitrary format.

The current thing (and my patches) converts both ways. Can't get much
more generic than that.

> 
> And that buffer cannot be created by cairo,

Why not?

> so relying on the
> destroy_cairo_surface__convert to convert it back from XRGB8888 to
> whatever format doesn't work.
> 
> So yeah, we can't really rely on cairo in this particuliar case, and
> the buffer conversions need be to made explicit.
> 
> Maxime
> 
> -- 
> Maxime Ripard, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com

-- 
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 v3 01/11] tests: Add vc4 test suite

[Eric Anholt]

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

Maxime Ripard <maxime.ripard@bootlin.com> writes:

> Add some various test suites relevant for the vc4 drm driver.
>
> Acked-by: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>

It sounds like there were concerns about the format conversion
functions.  I've merged this patch in the meantime.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 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 v3 03/11] fb: Add format conversion routine

[Maxime Ripard]

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

On Mon, Jun 04, 2018 at 01:13:53PM +0300, Ville Syrjälä wrote:
> On Mon, Jun 04, 2018 at 11:54:03AM +0200, Maxime Ripard wrote:
> > On Thu, May 24, 2018 at 06:40:36PM +0300, Ville Syrjälä wrote:
> > > On Thu, May 24, 2018 at 05:19:04PM +0200, Maxime Ripard wrote:
> > > > On Thu, May 24, 2018 at 05:58:13PM +0300, Ville Syrjälä wrote:
> > > > > On Thu, May 24, 2018 at 04:24:42PM +0200, Maxime Ripard wrote:
> > > > > > 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.
> > > > > 
> > > > > We already have the capability to do format conversions automagically.
> > > > > I'm extending it to handle more YUV stuff here:
> > > > > https://patchwork.freedesktop.org/series/43651/
> > > > > 
> > > > > Can you hook up the pixman stuff in the same way so that we don't
> > > > > have to any explicit conversion stuff in the tests themselves?
> > > > 
> > > > I really think that the assumption that cairo will handle it is not a
> > > > proper fit. Cairo support is very limited, just like pixman is to a
> > > > lesser extent, and we should just break the assumption that we will
> > > > get a cairo surface all the time.
> > > 
> > > Not sure what you're saying here. We don't assume cairo handles all
> > > the formats, well, because it doesn't. We just magically convert
> > > to/from XRGB888 so that cairo doesn't have to handle anything else.
> > 
> > My point was that from what I can tell from your patches, you allow to
> > create an igt_fb with whatever format we want, and then convert it to
> > XRGB8888 when one calls igt_get_cairo_surface.
> > 
> > I need something more generic, but even if we simplify to the
> > operation that I need now, I basically needs the exact opposite: to be
> > able to take an XRGB8888 buffer (that is our pattern) and get an
> > igt_fb in an arbitrary format.
> 
> The current thing (and my patches) converts both ways. Can't get much
> more generic than that.
> 
> > 
> > And that buffer cannot be created by cairo,
> 
> Why not?

In order to have control over the lower bits that would be lost in the
resampling. Let's say you convert the initial XRGB8888 pattern to an
RGB565 one, feed that to the scanout, and capture the frame on the
other end using a chamelium.

The Chamelium will capture a CRC over an XRGB8888 frame internall, and
you will have the CRC of the XRGB8888 pattern. However, since that
pattern was downsampled to RGB565 and the resampled back to 8 bits,
you lost the least significant bits, and without a pattern that was
carefully chosen, the CRCs don't match anymore.

As far as I know, Cairo only allows you to specify a color component
as a float, and tries to map that to a 24bits color components. But
you have no way to force it to use a particular value for the 3 (or
more) lower bits.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
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 v3 03/11] fb: Add format conversion routine

[Ville Syrjälä]

On Thu, Jun 14, 2018 at 04:53:39PM +0200, Maxime Ripard wrote:
> On Mon, Jun 04, 2018 at 01:13:53PM +0300, Ville Syrjälä wrote:
> > On Mon, Jun 04, 2018 at 11:54:03AM +0200, Maxime Ripard wrote:
> > > On Thu, May 24, 2018 at 06:40:36PM +0300, Ville Syrjälä wrote:
> > > > On Thu, May 24, 2018 at 05:19:04PM +0200, Maxime Ripard wrote:
> > > > > On Thu, May 24, 2018 at 05:58:13PM +0300, Ville Syrjälä wrote:
> > > > > > On Thu, May 24, 2018 at 04:24:42PM +0200, Maxime Ripard wrote:
> > > > > > > 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.
> > > > > > 
> > > > > > We already have the capability to do format conversions automagically.
> > > > > > I'm extending it to handle more YUV stuff here:
> > > > > > https://patchwork.freedesktop.org/series/43651/
> > > > > > 
> > > > > > Can you hook up the pixman stuff in the same way so that we don't
> > > > > > have to any explicit conversion stuff in the tests themselves?
> > > > > 
> > > > > I really think that the assumption that cairo will handle it is not a
> > > > > proper fit. Cairo support is very limited, just like pixman is to a
> > > > > lesser extent, and we should just break the assumption that we will
> > > > > get a cairo surface all the time.
> > > > 
> > > > Not sure what you're saying here. We don't assume cairo handles all
> > > > the formats, well, because it doesn't. We just magically convert
> > > > to/from XRGB888 so that cairo doesn't have to handle anything else.
> > > 
> > > My point was that from what I can tell from your patches, you allow to
> > > create an igt_fb with whatever format we want, and then convert it to
> > > XRGB8888 when one calls igt_get_cairo_surface.
> > > 
> > > I need something more generic, but even if we simplify to the
> > > operation that I need now, I basically needs the exact opposite: to be
> > > able to take an XRGB8888 buffer (that is our pattern) and get an
> > > igt_fb in an arbitrary format.
> > 
> > The current thing (and my patches) converts both ways. Can't get much
> > more generic than that.
> > 
> > > 
> > > And that buffer cannot be created by cairo,
> > 
> > Why not?
> 
> In order to have control over the lower bits that would be lost in the
> resampling. Let's say you convert the initial XRGB8888 pattern to an
> RGB565 one, feed that to the scanout, and capture the frame on the
> other end using a chamelium.
> 
> The Chamelium will capture a CRC over an XRGB8888 frame internall, and
> you will have the CRC of the XRGB8888 pattern. However, since that
> pattern was downsampled to RGB565 and the resampled back to 8 bits,
> you lost the least significant bits, and without a pattern that was
> carefully chosen, the CRCs don't match anymore.
> 
> As far as I know, Cairo only allows you to specify a color component
> as a float, and tries to map that to a 24bits color components. But
> you have no way to force it to use a particular value for the 3 (or
> more) lower bits.

You can always draw solid rectangles and such with cairo. Should retain
full control over the bits that way.

Alternative is to just throw away the low bits with a gamma unit etc.
Granted, you might actually want to check that the low bits have the
correct data. Depedns on what you're testing I suppose.

That said, I don't have any real objections to adding a function to
do the format conversion explicitly, but I'd definitely want to see
this stuff hooked up to the cairo path as well. And conversely the
things we already have in the cairo path should probably be hooked
up to the explicit format conversion function.

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