All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats
@ 2018-04-24  7:46 Maxime Ripard
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 01/12] tests: Add vc4 test suite Maxime Ripard
                   ` (15 more replies)
  0 siblings, 16 replies; 29+ messages in thread
From: Maxime Ripard @ 2018-04-24  7:46 UTC (permalink / raw)
  To: igt-dev; +Cc: eben, Maxime Ripard, Paul Kocialkowski, Thomas Petazzoni

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.

There's been a number of changes from the RFC thanks to the feedback from
Eric Anholt.

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 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 (12):
  tests: Add vc4 test suite
  fb: Add buffer map/unmap functions
  fb: Add format conversion routine
  fb: Add more formats
  chamelium: Make chamelium_calculate_fb_crc private
  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_chamelium.c                      | 146 ++++++++---------
 lib/igt_chamelium.h                      |   1 +-
 lib/igt_fb.c                             | 145 +++++++++++++++--
 lib/igt_fb.h                             |   4 +-
 tests/kms_chamelium.c                    | 208 ++++++++++++++++++------
 tests/vc4_ci/README                      |  38 ++++-
 tests/vc4_ci/vc4-chamelium-fast.testlist |  11 +-
 tests/vc4_ci/vc4-chamelium.testlist      |  16 ++-
 tests/vc4_ci/vc4.testlist                |  35 ++++-
 9 files changed, 469 insertions(+), 135 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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [igt-dev] [PATCH i-g-t 01/12] tests: Add vc4 test suite
  2018-04-24  7:46 [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats Maxime Ripard
@ 2018-04-24  7:46 ` Maxime Ripard
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 02/12] fb: Add buffer map/unmap functions Maxime Ripard
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 29+ messages in thread
From: Maxime Ripard @ 2018-04-24  7:46 UTC (permalink / raw)
  To: igt-dev; +Cc: eben, Maxime Ripard, Paul Kocialkowski, Thomas Petazzoni

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [igt-dev] [PATCH i-g-t 02/12] fb: Add buffer map/unmap functions
  2018-04-24  7:46 [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats Maxime Ripard
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 01/12] tests: Add vc4 test suite Maxime Ripard
@ 2018-04-24  7:46 ` Maxime Ripard
  2018-05-14 13:27   ` Paul Kocialkowski
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 03/12] fb: Add format conversion routine Maxime Ripard
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2018-04-24  7:46 UTC (permalink / raw)
  To: igt-dev; +Cc: eben, Maxime Ripard, Paul Kocialkowski, Thomas Petazzoni

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 | 51 ++++++++++++++++++++++++++++++++++++++++++++++-----
 lib/igt_fb.h |  2 ++
 2 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 7404ba7cb0c0..e0d543aa1aeb 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,35 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
 }
 
 /**
+ * igt_fb_get_buffer:
+ * @fd: open drm file descriptor
+ * @fb: pointer to an #igt_fb structure
+ *
+ * This function will map and return a pointer to the content of the
+ * supplied framebuffer's plane.
+ *
+ * Returns:
+ * A pointer to a buffer with the contents of the framebuffer
+ */
+void *igt_fb_get_buffer(int fd, struct igt_fb *fb)
+{
+	return map_bo(fd, fb);
+}
+
+/**
+ * igt_fb_put_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_get_buffer().
+ */
+void igt_fb_put_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..631fe5db7318 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_get_buffer(int fd, struct igt_fb *fb);
+void igt_fb_put_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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [igt-dev] [PATCH i-g-t 03/12] fb: Add format conversion routine
  2018-04-24  7:46 [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats Maxime Ripard
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 01/12] tests: Add vc4 test suite Maxime Ripard
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 02/12] fb: Add buffer map/unmap functions Maxime Ripard
@ 2018-04-24  7:46 ` Maxime Ripard
  2018-05-14 13:41   ` Paul Kocialkowski
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 04/12] fb: Add more formats Maxime Ripard
                   ` (12 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2018-04-24  7:46 UTC (permalink / raw)
  To: igt-dev; +Cc: eben, Maxime Ripard, Paul Kocialkowski, Thomas Petazzoni

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 e0d543aa1aeb..1de6f9f6c275 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	-1
+
 /* 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;
@@ -1675,6 +1691,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_get_buffer(src->fd, src);
+	igt_assert(src_ptr);
+
+	dst_ptr = igt_fb_get_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_put_buffer(dst, dst_ptr);
+	igt_fb_put_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 631fe5db7318..09df52e34992 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_get_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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [igt-dev] [PATCH i-g-t 04/12] fb: Add more formats
  2018-04-24  7:46 [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats Maxime Ripard
                   ` (2 preceding siblings ...)
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 03/12] fb: Add format conversion routine Maxime Ripard
@ 2018-04-24  7:46 ` Maxime Ripard
  2018-05-14 13:45   ` Paul Kocialkowski
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 05/12] chamelium: Make chamelium_calculate_fb_crc private Maxime Ripard
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2018-04-24  7:46 UTC (permalink / raw)
  To: igt-dev; +Cc: eben, Maxime Ripard, Paul Kocialkowski, Thomas Petazzoni

We're going to need some DRM formats that are not defined in the
format_desc_struct. Make sure we add them.

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

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 1de6f9f6c275..c6e6b0377ae5 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -70,10 +70,14 @@ 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(BGR565,	INVALID,	b5g6r5,		16, 16),
 	//DF(RGB888,	INVALID,	r8g8b8,		24, 24),
 	DF(XRGB8888,	RGB24,		x8r8g8b8,	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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [igt-dev] [PATCH i-g-t 05/12] chamelium: Make chamelium_calculate_fb_crc private
  2018-04-24  7:46 [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats Maxime Ripard
                   ` (3 preceding siblings ...)
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 04/12] fb: Add more formats Maxime Ripard
@ 2018-04-24  7:46 ` Maxime Ripard
  2018-05-14 13:50   ` Paul Kocialkowski
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 06/12] chamelium: Split CRC test function in two Maxime Ripard
                   ` (10 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2018-04-24  7:46 UTC (permalink / raw)
  To: igt-dev; +Cc: eben, Maxime Ripard, Paul Kocialkowski, Thomas Petazzoni

The function chamelium_calculate_fb_crc has no user outside of
lib/igt_chamelium.c, but is still part of the global functions exposed in
lib/igt_chamelium.h.

Remove that.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_chamelium.c | 146 ++++++++++++++++++++++-----------------------
 lib/igt_chamelium.h |   1 +-
 2 files changed, 73 insertions(+), 74 deletions(-)

diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index b25855a41ceb..ce2f5d6e3bf5 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -1064,6 +1064,79 @@ void chamelium_assert_crc_eq_or_dump(struct chamelium *chamelium,
 	igt_assert(eq);
 }
 
+static uint32_t chamelium_xrgb_hash16(const unsigned char *buffer, int width,
+				      int height, int k, int m)
+{
+	unsigned char r, g, b;
+	uint64_t sum = 0;
+	uint64_t count = 0;
+	uint64_t value;
+	uint32_t hash;
+	int index;
+	int i;
+
+	for (i=0; i < width * height; i++) {
+		if ((i % m) != k)
+			continue;
+
+		index = i * 4;
+
+		r = buffer[index + 2];
+		g = buffer[index + 1];
+		b = buffer[index + 0];
+
+		value = r | (g << 8) | (b << 16);
+		sum += ++count * value;
+	}
+
+	hash = ((sum >> 0) ^ (sum >> 16) ^ (sum >> 32) ^ (sum >> 48)) & 0xffff;
+
+	return hash;
+}
+
+static void chamelium_do_calculate_fb_crc(cairo_surface_t *fb_surface,
+					  igt_crc_t *out)
+{
+	unsigned char *buffer;
+	int n = 4;
+	int w, h;
+	int i, j;
+
+	buffer = cairo_image_surface_get_data(fb_surface);
+	w = cairo_image_surface_get_width(fb_surface);
+	h = cairo_image_surface_get_height(fb_surface);
+
+	for (i = 0; i < n; i++) {
+		j = n - i - 1;
+		out->crc[i] = chamelium_xrgb_hash16(buffer, w, h, j, n);
+	}
+
+	out->n_words = n;
+}
+
+/**
+ * chamelium_calculate_fb_crc:
+ * @fd: The drm file descriptor
+ * @fb: The framebuffer to calculate the CRC for
+ *
+ * Calculates the CRC for the provided framebuffer, using the Chamelium's CRC
+ * algorithm. This calculates the CRC in a synchronous fashion.
+ *
+ * Returns: The calculated CRC
+ */
+static igt_crc_t *chamelium_calculate_fb_crc(int fd, struct igt_fb *fb)
+{
+	igt_crc_t *ret = calloc(1, sizeof(igt_crc_t));
+	cairo_surface_t *fb_surface;
+
+	/* Get the cairo surface for the framebuffer */
+	fb_surface = igt_get_cairo_surface(fd, fb);
+
+	chamelium_do_calculate_fb_crc(fb_surface, ret);
+
+	return ret;
+}
+
 /**
  * chamelium_assert_analog_frame_match_or_dump:
  * @chamelium: The chamelium instance the frame dump belongs to
@@ -1245,79 +1318,6 @@ int chamelium_get_frame_limit(struct chamelium *chamelium,
 	return ret;
 }
 
-static uint32_t chamelium_xrgb_hash16(const unsigned char *buffer, int width,
-				      int height, int k, int m)
-{
-	unsigned char r, g, b;
-	uint64_t sum = 0;
-	uint64_t count = 0;
-	uint64_t value;
-	uint32_t hash;
-	int index;
-	int i;
-
-	for (i=0; i < width * height; i++) {
-		if ((i % m) != k)
-			continue;
-
-		index = i * 4;
-
-		r = buffer[index + 2];
-		g = buffer[index + 1];
-		b = buffer[index + 0];
-
-		value = r | (g << 8) | (b << 16);
-		sum += ++count * value;
-	}
-
-	hash = ((sum >> 0) ^ (sum >> 16) ^ (sum >> 32) ^ (sum >> 48)) & 0xffff;
-
-	return hash;
-}
-
-static void chamelium_do_calculate_fb_crc(cairo_surface_t *fb_surface,
-					  igt_crc_t *out)
-{
-	unsigned char *buffer;
-	int n = 4;
-	int w, h;
-	int i, j;
-
-	buffer = cairo_image_surface_get_data(fb_surface);
-	w = cairo_image_surface_get_width(fb_surface);
-	h = cairo_image_surface_get_height(fb_surface);
-
-	for (i = 0; i < n; i++) {
-		j = n - i - 1;
-		out->crc[i] = chamelium_xrgb_hash16(buffer, w, h, j, n);
-	}
-
-	out->n_words = n;
-}
-
-/**
- * chamelium_calculate_fb_crc:
- * @fd: The drm file descriptor
- * @fb: The framebuffer to calculate the CRC for
- *
- * Calculates the CRC for the provided framebuffer, using the Chamelium's CRC
- * algorithm. This calculates the CRC in a synchronous fashion.
- *
- * Returns: The calculated CRC
- */
-igt_crc_t *chamelium_calculate_fb_crc(int fd, struct igt_fb *fb)
-{
-	igt_crc_t *ret = calloc(1, sizeof(igt_crc_t));
-	cairo_surface_t *fb_surface;
-
-	/* Get the cairo surface for the framebuffer */
-	fb_surface = igt_get_cairo_surface(fd, fb);
-
-	chamelium_do_calculate_fb_crc(fb_surface, ret);
-
-	return ret;
-}
-
 static void *chamelium_calculate_fb_crc_async_work(void *data)
 {
 	struct chamelium_fb_crc_async_data *fb_crc;
diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
index af9655a0b1cf..1a6ad9b93ee8 100644
--- a/lib/igt_chamelium.h
+++ b/lib/igt_chamelium.h
@@ -95,7 +95,6 @@ struct chamelium_frame_dump *chamelium_port_dump_pixels(struct chamelium *chamel
 							struct chamelium_port *port,
 							int x, int y,
 							int w, int h);
-igt_crc_t *chamelium_calculate_fb_crc(int fd, struct igt_fb *fb);
 struct chamelium_fb_crc_async_data *chamelium_calculate_fb_crc_async_start(int fd,
 									   struct igt_fb *fb);
 igt_crc_t *chamelium_calculate_fb_crc_async_finish(struct chamelium_fb_crc_async_data *fb_crc);
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [igt-dev] [PATCH i-g-t 06/12] chamelium: Split CRC test function in two
  2018-04-24  7:46 [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats Maxime Ripard
                   ` (4 preceding siblings ...)
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 05/12] chamelium: Make chamelium_calculate_fb_crc private Maxime Ripard
@ 2018-04-24  7:46 ` Maxime Ripard
  2018-05-14 13:55   ` Paul Kocialkowski
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 07/12] chamelium: Use preferred mode when testing a single mode Maxime Ripard
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2018-04-24  7:46 UTC (permalink / raw)
  To: igt-dev; +Cc: eben, Maxime Ripard, Paul Kocialkowski, Thomas Petazzoni

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.

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [igt-dev] [PATCH i-g-t 07/12] chamelium: Use preferred mode when testing a single mode
  2018-04-24  7:46 [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats Maxime Ripard
                   ` (5 preceding siblings ...)
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 06/12] chamelium: Split CRC test function in two Maxime Ripard
@ 2018-04-24  7:46 ` Maxime Ripard
  2018-05-14 14:00   ` Paul Kocialkowski
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 08/12] chamelium: Add function to generate our test pattern Maxime Ripard
                   ` (8 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2018-04-24  7:46 UTC (permalink / raw)
  To: igt-dev; +Cc: eben, Maxime Ripard, Paul Kocialkowski, Thomas Petazzoni

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [igt-dev] [PATCH i-g-t 08/12] chamelium: Add function to generate our test pattern
  2018-04-24  7:46 [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats Maxime Ripard
                   ` (6 preceding siblings ...)
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 07/12] chamelium: Use preferred mode when testing a single mode Maxime Ripard
@ 2018-04-24  7:46 ` Maxime Ripard
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 09/12] chamelium: Change our pattern for a custom one Maxime Ripard
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 29+ messages in thread
From: Maxime Ripard @ 2018-04-24  7:46 UTC (permalink / raw)
  To: igt-dev; +Cc: eben, Maxime Ripard, Paul Kocialkowski, Thomas Petazzoni

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [igt-dev] [PATCH i-g-t 09/12] chamelium: Change our pattern for a custom one
  2018-04-24  7:46 [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats Maxime Ripard
                   ` (7 preceding siblings ...)
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 08/12] chamelium: Add function to generate our test pattern Maxime Ripard
@ 2018-04-24  7:46 ` Maxime Ripard
  2018-05-14 14:24   ` Paul Kocialkowski
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 10/12] chamelium: Add format support Maxime Ripard
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2018-04-24  7:46 UTC (permalink / raw)
  To: igt-dev; +Cc: eben, Maxime Ripard, Paul Kocialkowski, Thomas Petazzoni

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..2b82f2597c73 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_get_buffer(fb->fd, fb);
+	igt_assert(ptr);
+
+	chamelium_paint_xr24_pattern(ptr, mode->vdisplay, mode->hdisplay);
+	igt_fb_put_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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [igt-dev] [PATCH i-g-t 10/12] chamelium: Add format support
  2018-04-24  7:46 [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats Maxime Ripard
                   ` (8 preceding siblings ...)
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 09/12] chamelium: Change our pattern for a custom one Maxime Ripard
@ 2018-04-24  7:46 ` Maxime Ripard
  2018-05-14 14:41   ` Paul Kocialkowski
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 11/12] chamelium: Add format subtests Maxime Ripard
                   ` (5 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2018-04-24  7:46 UTC (permalink / raw)
  To: igt-dev; +Cc: eben, Maxime Ripard, Paul Kocialkowski, Thomas Petazzoni

In order to introduce 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.

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 2b82f2597c73..9cd05b83076a 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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [igt-dev] [PATCH i-g-t 11/12] chamelium: Add format subtests
  2018-04-24  7:46 [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats Maxime Ripard
                   ` (9 preceding siblings ...)
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 10/12] chamelium: Add format support Maxime Ripard
@ 2018-04-24  7:46 ` Maxime Ripard
  2018-05-14 14:46   ` Paul Kocialkowski
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 12/12] tests: Add chamelium formats subtests to vc4 test lists Maxime Ripard
                   ` (4 subsequent siblings)
  15 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2018-04-24  7:46 UTC (permalink / raw)
  To: igt-dev; +Cc: eben, Maxime Ripard, Paul Kocialkowski, Thomas Petazzoni

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.

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

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 9cd05b83076a..4a634c9e456d 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -964,6 +964,34 @@ 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-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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [igt-dev] [PATCH i-g-t 12/12] tests: Add chamelium formats subtests to vc4 test lists
  2018-04-24  7:46 [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats Maxime Ripard
                   ` (10 preceding siblings ...)
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 11/12] chamelium: Add format subtests Maxime Ripard
@ 2018-04-24  7:46 ` Maxime Ripard
  2018-04-24  8:29 ` [igt-dev] ✗ Fi.CI.BAT: failure for chamelium: Test the plane formats Patchwork
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 29+ messages in thread
From: Maxime Ripard @ 2018-04-24  7:46 UTC (permalink / raw)
  To: igt-dev; +Cc: eben, Maxime Ripard, Paul Kocialkowski, Thomas Petazzoni

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 | 7 +++++++
 tests/vc4_ci/vc4-chamelium.testlist      | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/tests/vc4_ci/vc4-chamelium-fast.testlist b/tests/vc4_ci/vc4-chamelium-fast.testlist
index 964167b82d00..fa04fe3ff1f7 100644
--- a/tests/vc4_ci/vc4-chamelium-fast.testlist
+++ b/tests/vc4_ci/vc4-chamelium-fast.testlist
@@ -1,4 +1,11 @@
+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-fast
+igt@kms_chamelium@hdmi-crc-rgb565
+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..0f9df3fb1117 100644
--- a/tests/vc4_ci/vc4-chamelium.testlist
+++ b/tests/vc4_ci/vc4-chamelium.testlist
@@ -1,6 +1,13 @@
+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-fast
 igt@kms_chamelium@hdmi-crc-multiple
+igt@kms_chamelium@hdmi-crc-rgb565
 igt@kms_chamelium@hdmi-crc-single
+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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [igt-dev] ✗ Fi.CI.BAT: failure for chamelium: Test the plane formats
  2018-04-24  7:46 [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats Maxime Ripard
                   ` (11 preceding siblings ...)
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 12/12] tests: Add chamelium formats subtests to vc4 test lists Maxime Ripard
@ 2018-04-24  8:29 ` Patchwork
  2018-04-24 10:51 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 29+ messages in thread
From: Patchwork @ 2018-04-24  8:29 UTC (permalink / raw)
  To: Maxime Ripard; +Cc: igt-dev

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4083 -> IGTPW_1296 =

== Summary - FAILURE ==

  Serious unknown changes coming with IGTPW_1296 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1296, 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/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c-frame-sequence:
      fi-glk-j4005:       PASS -> DMESG-WARN

    


== Participating hosts (36 -> 33) ==

  Missing    (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * IGT: IGT_4444 -> IGTPW_1296

  CI_DRM_4083: b4e886c1f0e23ba0506206ea6758a60c05bd64c8 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1296: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1296/
  IGT_4444: dcc44347494231feabc588c2a76998cbc9afdf8c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4444: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit



== 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-rgb565
+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_1296/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for chamelium: Test the plane formats
  2018-04-24  7:46 [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats Maxime Ripard
                   ` (12 preceding siblings ...)
  2018-04-24  8:29 ` [igt-dev] ✗ Fi.CI.BAT: failure for chamelium: Test the plane formats Patchwork
@ 2018-04-24 10:51 ` Patchwork
  2018-04-24 12:17 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  2018-05-14 13:17 ` [igt-dev] [PATCH i-g-t 00/12] " Maxime Ripard
  15 siblings, 0 replies; 29+ messages in thread
From: Patchwork @ 2018-04-24 10:51 UTC (permalink / raw)
  To: Maxime Ripard; +Cc: igt-dev

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4083 -> IGTPW_1299 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-cnl-psr:         PASS -> FAIL (fdo#100368)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-skl-guc:         PASS -> FAIL (fdo#103191)

    
    ==== Possible fixes ====

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         FAIL (fdo#102575) -> PASS

    igt@kms_flip@basic-flip-vs-dpms:
      fi-ilk-650:         DMESG-WARN (fdo#106205) -> PASS +32

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#106205 https://bugs.freedesktop.org/show_bug.cgi?id=106205


== Participating hosts (36 -> 33) ==

  Missing    (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * IGT: IGT_4444 -> IGTPW_1299
    * Piglit: piglit_4444 -> piglit_4445

  CI_DRM_4083: b4e886c1f0e23ba0506206ea6758a60c05bd64c8 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1299: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1299/
  IGT_4444: dcc44347494231feabc588c2a76998cbc9afdf8c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4444: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit
  piglit_4445: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit



== 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-rgb565
+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_1299/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [igt-dev] ✓ Fi.CI.IGT: success for chamelium: Test the plane formats
  2018-04-24  7:46 [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats Maxime Ripard
                   ` (13 preceding siblings ...)
  2018-04-24 10:51 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork
@ 2018-04-24 12:17 ` Patchwork
  2018-05-14 13:17 ` [igt-dev] [PATCH i-g-t 00/12] " Maxime Ripard
  15 siblings, 0 replies; 29+ messages in thread
From: Patchwork @ 2018-04-24 12:17 UTC (permalink / raw)
  To: Maxime Ripard; +Cc: igt-dev

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from IGT_4444_full -> IGTPW_1299_full =

== Summary - WARNING ==

  Minor unknown changes coming with IGTPW_1299_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1299_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/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-vebox:
      shard-kbl:          PASS -> SKIP

    igt@gem_mocs_settings@mocs-rc6-blt:
      shard-kbl:          SKIP -> PASS +1

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_workarounds@suspend-resume-fd:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665)

    
    ==== Possible fixes ====

    igt@kms_flip@2x-dpms-vs-vblank-race:
      shard-hsw:          FAIL (fdo#103060) -> PASS

    igt@kms_sysfs_edid_timing:
      shard-apl:          WARN (fdo#100047) -> PASS

    
  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665


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

  Missing    (1): shard-glkb 


== Build changes ==

    * IGT: IGT_4444 -> IGTPW_1299
    * Linux: CI_DRM_4076 -> CI_DRM_4083
    * Piglit: piglit_4444 -> piglit_4445

  CI_DRM_4076: 01b171f6df7b2b55b3c1b851d40741179fa2a722 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4083: b4e886c1f0e23ba0506206ea6758a60c05bd64c8 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1299: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1299/
  IGT_4444: dcc44347494231feabc588c2a76998cbc9afdf8c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4444: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit
  piglit_4445: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit

== Logs ==

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats
  2018-04-24  7:46 [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats Maxime Ripard
                   ` (14 preceding siblings ...)
  2018-04-24 12:17 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
@ 2018-05-14 13:17 ` Maxime Ripard
  15 siblings, 0 replies; 29+ messages in thread
From: Maxime Ripard @ 2018-05-14 13:17 UTC (permalink / raw)
  To: igt-dev; +Cc: Paul Kocialkowski, eben, Thomas Petazzoni


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

On Tue, Apr 24, 2018 at 09:46:34AM +0200, Maxime Ripard wrote:
> 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.
> 
> There's been a number of changes from the RFC thanks to the feedback from
> Eric Anholt.
> 
> 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.

Any comments?

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 02/12] fb: Add buffer map/unmap functions
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 02/12] fb: Add buffer map/unmap functions Maxime Ripard
@ 2018-05-14 13:27   ` Paul Kocialkowski
  0 siblings, 0 replies; 29+ messages in thread
From: Paul Kocialkowski @ 2018-05-14 13:27 UTC (permalink / raw)
  To: Maxime Ripard, igt-dev; +Cc: eben, Thomas Petazzoni


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

Hi,

See a comment about function naming.

Cheers,

Paul

On Tue, 2018-04-24 at 09:46 +0200, Maxime Ripard wrote:
> 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 | 51 ++++++++++++++++++++++++++++++++++++++++++++++-----
>  lib/igt_fb.h |  2 ++
>  2 files changed, 48 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 7404ba7cb0c0..e0d543aa1aeb 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,35 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
>  }
>  
>  /**
> + * igt_fb_get_buffer:
> + * @fd: open drm file descriptor
> + * @fb: pointer to an #igt_fb structure
> + *
> + * This function will map and return a pointer to the content of the
> + * supplied framebuffer's plane.
> + *
> + * Returns:
> + * A pointer to a buffer with the contents of the framebuffer
> + */

From the description and name of the function, it's not fully clear what
happens and what needs to be done with the returned pointer. More
specifically, what if igt_fb_get_buffer is called multiple times in a
row: is a new pointer (resulting from a new mmap call) returned each
time or is there only one per buffer? Also, how must the returned buffer
be disposed of? igt_fb_get_buffer should probably be mentioned in this
function's documentation (otherwise users of this API may miss the call
and the reason to call it entirely).

I'm a bit confused here because the name does not suggest that this maps
a buffer (I would expect get_buffer to return a pointer to a previously-
mapped buffer instead) while it behaves similarly to map_bo here. Maybe
keeping map/unmap in the name instead of get/put would clarify things.

> +void *igt_fb_get_buffer(int fd, struct igt_fb *fb)
> +{
> +	return map_bo(fd, fb);
> +}
> +
> +/**
> + * igt_fb_put_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_get_buffer().
> + */
> +void igt_fb_put_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..631fe5db7318 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_get_buffer(int fd, struct igt_fb *fb);
> +void igt_fb_put_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,
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 03/12] fb: Add format conversion routine
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 03/12] fb: Add format conversion routine Maxime Ripard
@ 2018-05-14 13:41   ` Paul Kocialkowski
  0 siblings, 0 replies; 29+ messages in thread
From: Paul Kocialkowski @ 2018-05-14 13:41 UTC (permalink / raw)
  To: Maxime Ripard, igt-dev; +Cc: eben, Thomas Petazzoni


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

Hi,

On Tue, 2018-04-24 at 09:46 +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.
> 
> 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.

Thanks for the series!

See a few comment below, looks good to me otherwise.

> 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 e0d543aa1aeb..1de6f9f6c275 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	-1

I'm not sure -1 is the best call here. Given the definition of the
PIXMAN_FORMAT macro (pixman.h), it looks like -1 would result in a
somewhat valid format (although it's unlikely that it will be used in
the future).

More specifically, the fact that the bpp field would be != 0 makes that
choice not very straightforward. I would suggest using 0 instead so that
the bpp field gets set to 0 and only reading this field makes it clear
that it's an invalid format.

> +
>  /* 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));

Purely a cosmetic nit: instead of asserting 0 here, one might want to
introduce a found boolean, breaking instead of returning and setting
found when the format matches, asserting on the found variable and
finally returning f->pixman_id unconditionally. Up to you :)

> +}
> +
>  static cairo_format_t drm_format_to_cairo(uint32_t drm_format)
>  {
>  	struct format_desc_struct *f;
> @@ -1675,6 +1691,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_get_buffer(src->fd, src);
> +	igt_assert(src_ptr);
> +
> +	dst_ptr = igt_fb_get_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_put_buffer(dst, dst_ptr);
> +	igt_fb_put_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 631fe5db7318..09df52e34992 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_get_buffer(int fd, struct igt_fb *fb);
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 04/12] fb: Add more formats
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 04/12] fb: Add more formats Maxime Ripard
@ 2018-05-14 13:45   ` Paul Kocialkowski
  0 siblings, 0 replies; 29+ messages in thread
From: Paul Kocialkowski @ 2018-05-14 13:45 UTC (permalink / raw)
  To: Maxime Ripard, igt-dev; +Cc: eben, Thomas Petazzoni


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

Hi,

On Tue, 2018-04-24 at 09:46 +0200, Maxime Ripard wrote:
> We're going to need some DRM formats that are not defined in the
> format_desc_struct. Make sure we add them.

The commit subject line feels a bit vague here, as it doesn't mention
where these formats are added and what is added regarding these formats
(it's obvious that it's their definition when reading the patch, but it
could be a number of other things, which won't be obvious when reading
the subject line only).

Cheers,

> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  lib/igt_fb.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 1de6f9f6c275..c6e6b0377ae5 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -70,10 +70,14 @@ 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(BGR565,	INVALID,	b5g6r5,		16, 16),
>  	//DF(RGB888,	INVALID,	r8g8b8,		24, 24),
>  	DF(XRGB8888,	RGB24,		x8r8g8b8,	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}),
>  };

-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 05/12] chamelium: Make chamelium_calculate_fb_crc private
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 05/12] chamelium: Make chamelium_calculate_fb_crc private Maxime Ripard
@ 2018-05-14 13:50   ` Paul Kocialkowski
  2018-05-24  9:38     ` Maxime Ripard
  0 siblings, 1 reply; 29+ messages in thread
From: Paul Kocialkowski @ 2018-05-14 13:50 UTC (permalink / raw)
  To: Maxime Ripard, igt-dev; +Cc: eben, Thomas Petazzoni


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

Hi,

On Tue, 2018-04-24 at 09:46 +0200, Maxime Ripard wrote:
> The function chamelium_calculate_fb_crc has no user outside of
> lib/igt_chamelium.c, but is still part of the global functions exposed in
> lib/igt_chamelium.h.

I don't think the fact that it has no immediate user within IGT is a
reason to move this function out of the public API. Is there a specific
use case that justifies the need for this?

The way I see it, chamelium_calculate_fb_crc is simply the synchronous
version of chamelium_calculate_fb_crc_async and although its use is
simplified with helpers, I think both functions should be public on the
same grounds.

What do you think?

Cheers,

Paul

> Remove that.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  lib/igt_chamelium.c | 146 ++++++++++++++++++++++-----------------------
>  lib/igt_chamelium.h |   1 +-
>  2 files changed, 73 insertions(+), 74 deletions(-)
> 
> diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> index b25855a41ceb..ce2f5d6e3bf5 100644
> --- a/lib/igt_chamelium.c
> +++ b/lib/igt_chamelium.c
> @@ -1064,6 +1064,79 @@ void chamelium_assert_crc_eq_or_dump(struct chamelium *chamelium,
>  	igt_assert(eq);
>  }
>  
> +static uint32_t chamelium_xrgb_hash16(const unsigned char *buffer, int width,
> +				      int height, int k, int m)
> +{
> +	unsigned char r, g, b;
> +	uint64_t sum = 0;
> +	uint64_t count = 0;
> +	uint64_t value;
> +	uint32_t hash;
> +	int index;
> +	int i;
> +
> +	for (i=0; i < width * height; i++) {
> +		if ((i % m) != k)
> +			continue;
> +
> +		index = i * 4;
> +
> +		r = buffer[index + 2];
> +		g = buffer[index + 1];
> +		b = buffer[index + 0];
> +
> +		value = r | (g << 8) | (b << 16);
> +		sum += ++count * value;
> +	}
> +
> +	hash = ((sum >> 0) ^ (sum >> 16) ^ (sum >> 32) ^ (sum >> 48)) & 0xffff;
> +
> +	return hash;
> +}
> +
> +static void chamelium_do_calculate_fb_crc(cairo_surface_t *fb_surface,
> +					  igt_crc_t *out)
> +{
> +	unsigned char *buffer;
> +	int n = 4;
> +	int w, h;
> +	int i, j;
> +
> +	buffer = cairo_image_surface_get_data(fb_surface);
> +	w = cairo_image_surface_get_width(fb_surface);
> +	h = cairo_image_surface_get_height(fb_surface);
> +
> +	for (i = 0; i < n; i++) {
> +		j = n - i - 1;
> +		out->crc[i] = chamelium_xrgb_hash16(buffer, w, h, j, n);
> +	}
> +
> +	out->n_words = n;
> +}
> +
> +/**
> + * chamelium_calculate_fb_crc:
> + * @fd: The drm file descriptor
> + * @fb: The framebuffer to calculate the CRC for
> + *
> + * Calculates the CRC for the provided framebuffer, using the Chamelium's CRC
> + * algorithm. This calculates the CRC in a synchronous fashion.
> + *
> + * Returns: The calculated CRC
> + */
> +static igt_crc_t *chamelium_calculate_fb_crc(int fd, struct igt_fb *fb)
> +{
> +	igt_crc_t *ret = calloc(1, sizeof(igt_crc_t));
> +	cairo_surface_t *fb_surface;
> +
> +	/* Get the cairo surface for the framebuffer */
> +	fb_surface = igt_get_cairo_surface(fd, fb);
> +
> +	chamelium_do_calculate_fb_crc(fb_surface, ret);
> +
> +	return ret;
> +}
> +
>  /**
>   * chamelium_assert_analog_frame_match_or_dump:
>   * @chamelium: The chamelium instance the frame dump belongs to
> @@ -1245,79 +1318,6 @@ int chamelium_get_frame_limit(struct chamelium *chamelium,
>  	return ret;
>  }
>  
> -static uint32_t chamelium_xrgb_hash16(const unsigned char *buffer, int width,
> -				      int height, int k, int m)
> -{
> -	unsigned char r, g, b;
> -	uint64_t sum = 0;
> -	uint64_t count = 0;
> -	uint64_t value;
> -	uint32_t hash;
> -	int index;
> -	int i;
> -
> -	for (i=0; i < width * height; i++) {
> -		if ((i % m) != k)
> -			continue;
> -
> -		index = i * 4;
> -
> -		r = buffer[index + 2];
> -		g = buffer[index + 1];
> -		b = buffer[index + 0];
> -
> -		value = r | (g << 8) | (b << 16);
> -		sum += ++count * value;
> -	}
> -
> -	hash = ((sum >> 0) ^ (sum >> 16) ^ (sum >> 32) ^ (sum >> 48)) & 0xffff;
> -
> -	return hash;
> -}
> -
> -static void chamelium_do_calculate_fb_crc(cairo_surface_t *fb_surface,
> -					  igt_crc_t *out)
> -{
> -	unsigned char *buffer;
> -	int n = 4;
> -	int w, h;
> -	int i, j;
> -
> -	buffer = cairo_image_surface_get_data(fb_surface);
> -	w = cairo_image_surface_get_width(fb_surface);
> -	h = cairo_image_surface_get_height(fb_surface);
> -
> -	for (i = 0; i < n; i++) {
> -		j = n - i - 1;
> -		out->crc[i] = chamelium_xrgb_hash16(buffer, w, h, j, n);
> -	}
> -
> -	out->n_words = n;
> -}
> -
> -/**
> - * chamelium_calculate_fb_crc:
> - * @fd: The drm file descriptor
> - * @fb: The framebuffer to calculate the CRC for
> - *
> - * Calculates the CRC for the provided framebuffer, using the Chamelium's CRC
> - * algorithm. This calculates the CRC in a synchronous fashion.
> - *
> - * Returns: The calculated CRC
> - */
> -igt_crc_t *chamelium_calculate_fb_crc(int fd, struct igt_fb *fb)
> -{
> -	igt_crc_t *ret = calloc(1, sizeof(igt_crc_t));
> -	cairo_surface_t *fb_surface;
> -
> -	/* Get the cairo surface for the framebuffer */
> -	fb_surface = igt_get_cairo_surface(fd, fb);
> -
> -	chamelium_do_calculate_fb_crc(fb_surface, ret);
> -
> -	return ret;
> -}
> -
>  static void *chamelium_calculate_fb_crc_async_work(void *data)
>  {
>  	struct chamelium_fb_crc_async_data *fb_crc;
> diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
> index af9655a0b1cf..1a6ad9b93ee8 100644
> --- a/lib/igt_chamelium.h
> +++ b/lib/igt_chamelium.h
> @@ -95,7 +95,6 @@ struct chamelium_frame_dump *chamelium_port_dump_pixels(struct chamelium *chamel
>  							struct chamelium_port *port,
>  							int x, int y,
>  							int w, int h);
> -igt_crc_t *chamelium_calculate_fb_crc(int fd, struct igt_fb *fb);
>  struct chamelium_fb_crc_async_data *chamelium_calculate_fb_crc_async_start(int fd,
>  									   struct igt_fb *fb);
>  igt_crc_t *chamelium_calculate_fb_crc_async_finish(struct chamelium_fb_crc_async_data *fb_crc);
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 06/12] chamelium: Split CRC test function in two
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 06/12] chamelium: Split CRC test function in two Maxime Ripard
@ 2018-05-14 13:55   ` Paul Kocialkowski
  0 siblings, 0 replies; 29+ messages in thread
From: Paul Kocialkowski @ 2018-05-14 13:55 UTC (permalink / raw)
  To: Maxime Ripard, igt-dev; +Cc: eben, Thomas Petazzoni


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

Hi,

On Tue, 2018-04-24 at 09:46 +0200, Maxime Ripard wrote:
> 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.

Thanks for making that change!

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);
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 07/12] chamelium: Use preferred mode when testing a single mode
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 07/12] chamelium: Use preferred mode when testing a single mode Maxime Ripard
@ 2018-05-14 14:00   ` Paul Kocialkowski
  2018-05-24  9:48     ` Maxime Ripard
  0 siblings, 1 reply; 29+ messages in thread
From: Paul Kocialkowski @ 2018-05-14 14:00 UTC (permalink / raw)
  To: Maxime Ripard, igt-dev; +Cc: eben, Thomas Petazzoni


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

Hi,

On Tue, 2018-04-24 at 09:46 +0200, Maxime Ripard wrote:
> 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.

The rationale behind this decision was to use the highest resolution,
that is usually reported first, but I think your approach makes more
sense.

What happens when no mode is marked as preferred though? You put-in an
assert here, but does the DRM API ensure that at least one is marked as
preferred? If not, we should probably fallback to selecting the first
mode as before. This would also make things safer in case of a
descructive interaction with the mode pruning code, where the
recommended mode might get pruned (although that would be very
unfortunate).

What do you think?

Cheers,

Paul

> 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);
>  }
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 09/12] chamelium: Change our pattern for a custom one
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 09/12] chamelium: Change our pattern for a custom one Maxime Ripard
@ 2018-05-14 14:24   ` Paul Kocialkowski
  2018-05-18  7:26     ` Maxime Ripard
  0 siblings, 1 reply; 29+ messages in thread
From: Paul Kocialkowski @ 2018-05-14 14:24 UTC (permalink / raw)
  To: Maxime Ripard, igt-dev; +Cc: eben, Thomas Petazzoni


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

Hi,

On Tue, 2018-04-24 at 09:46 +0200, Maxime Ripard wrote:
> 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.

I understand the issue at hand, the proposed solution and I agree that
it an easy way to deal with the issues introduced by up/downsampling.

However, it all other cases (where the color bpp is the same for the
source and destination formats), I think we should definitely use a
complex pattern with gradients (igt_create_color_pattern_fb) in order to
increase the coverage of the tests (gradients are something we
definitely want to make sure are displayed correctly).

So I think we should have a test for the bpp somewhere and only rely on
this new pattern when there is downsampling involved.

What do you think?

> 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..2b82f2597c73 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_get_buffer(fb->fd, fb);
> +	igt_assert(ptr);
> +
> +	chamelium_paint_xr24_pattern(ptr, mode->vdisplay, mode->hdisplay);
> +	igt_fb_put_buffer(fb, ptr);
> +
> +	return fb_id;
>  }
>  
>  static void do_test_display_crc(data_t *data, struct chamelium_port *port,
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 10/12] chamelium: Add format support
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 10/12] chamelium: Add format support Maxime Ripard
@ 2018-05-14 14:41   ` Paul Kocialkowski
  0 siblings, 0 replies; 29+ messages in thread
From: Paul Kocialkowski @ 2018-05-14 14:41 UTC (permalink / raw)
  To: Maxime Ripard, igt-dev; +Cc: eben, Thomas Petazzoni


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

Hi,

On Tue, 2018-04-24 at 09:46 +0200, Maxime Ripard wrote:
> In order to introduce subtests for DRM formats, we need to take an
> intermediate step.

Maybe make it clear in the commit subject that this concerns the CRC
tests only. We could (and probably should) be doing the same with the
full-frame comparison tests.

> 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 2b82f2597c73..9cd05b83076a 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);
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 11/12] chamelium: Add format subtests
  2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 11/12] chamelium: Add format subtests Maxime Ripard
@ 2018-05-14 14:46   ` Paul Kocialkowski
  0 siblings, 0 replies; 29+ messages in thread
From: Paul Kocialkowski @ 2018-05-14 14:46 UTC (permalink / raw)
  To: Maxime Ripard, igt-dev; +Cc: eben, Thomas Petazzoni


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

Hi,

On Tue, 2018-04-24 at 09:46 +0200, Maxime Ripard wrote:
> 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 | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 9cd05b83076a..4a634c9e456d 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -964,6 +964,34 @@ 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-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);
>  	}
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 09/12] chamelium: Change our pattern for a custom one
  2018-05-14 14:24   ` Paul Kocialkowski
@ 2018-05-18  7:26     ` Maxime Ripard
  0 siblings, 0 replies; 29+ messages in thread
From: Maxime Ripard @ 2018-05-18  7:26 UTC (permalink / raw)
  To: Paul Kocialkowski; +Cc: igt-dev, eben, Thomas Petazzoni


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

On Mon, May 14, 2018 at 04:24:40PM +0200, Paul Kocialkowski wrote:
> On Tue, 2018-04-24 at 09:46 +0200, Maxime Ripard wrote:
> > 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.
> 
> I understand the issue at hand, the proposed solution and I agree that
> it an easy way to deal with the issues introduced by up/downsampling.
> 
> However, it all other cases (where the color bpp is the same for the
> source and destination formats), I think we should definitely use a
> complex pattern with gradients (igt_create_color_pattern_fb) in order to
> increase the coverage of the tests (gradients are something we
> definitely want to make sure are displayed correctly).

Testing a gradient and expecting a pixel-perfect result is pretty
difficult, even on a digital signal like HDMI. It would assume that
there's no loss in color depth between the plane and the final
connector. And you have two boards on your desk that break that
assumption :)

I guess this would work fine for setup that have an HDMI encoder as
part of the video pipeline. But if you don't, and you have to use a
bridge, then it starts to fall apart, since the bridge might very well
be connected to the upstream encoder through a bus with a lower color
depth than the signal will output. And the worst part is that this is
not exposed to the userspace, so you cannot just fallback to a simpler
pattern if you fall into that case.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 05/12] chamelium: Make chamelium_calculate_fb_crc private
  2018-05-14 13:50   ` Paul Kocialkowski
@ 2018-05-24  9:38     ` Maxime Ripard
  0 siblings, 0 replies; 29+ messages in thread
From: Maxime Ripard @ 2018-05-24  9:38 UTC (permalink / raw)
  To: Paul Kocialkowski; +Cc: igt-dev, eben, Thomas Petazzoni


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

On Mon, May 14, 2018 at 03:50:33PM +0200, Paul Kocialkowski wrote:
> Hi,
> 
> On Tue, 2018-04-24 at 09:46 +0200, Maxime Ripard wrote:
> > The function chamelium_calculate_fb_crc has no user outside of
> > lib/igt_chamelium.c, but is still part of the global functions exposed in
> > lib/igt_chamelium.h.
> 
> I don't think the fact that it has no immediate user within IGT is a
> reason to move this function out of the public API. Is there a specific
> use case that justifies the need for this?
> 
> The way I see it, chamelium_calculate_fb_crc is simply the synchronous
> version of chamelium_calculate_fb_crc_async and although its use is
> simplified with helpers, I think both functions should be public on the
> same grounds.
> 
> What do you think?

Fair enough :)

This was a spurious attempt at cleaning up, I'm fine either way.

I'll drop this patch.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 07/12] chamelium: Use preferred mode when testing a single mode
  2018-05-14 14:00   ` Paul Kocialkowski
@ 2018-05-24  9:48     ` Maxime Ripard
  0 siblings, 0 replies; 29+ messages in thread
From: Maxime Ripard @ 2018-05-24  9:48 UTC (permalink / raw)
  To: Paul Kocialkowski; +Cc: igt-dev, eben, Thomas Petazzoni


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

On Mon, May 14, 2018 at 04:00:53PM +0200, Paul Kocialkowski wrote:
> Hi,
> 
> On Tue, 2018-04-24 at 09:46 +0200, Maxime Ripard wrote:
> > 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.
> 
> The rationale behind this decision was to use the highest resolution,
> that is usually reported first, but I think your approach makes more
> sense.
> 
> What happens when no mode is marked as preferred though? You put-in an
> assert here, but does the DRM API ensure that at least one is marked as
> preferred?

As far as I understood it from the EDID spec, a preferred mode is
mandatory in EDID 1.3, which is the version used by HDMI, so we don't
really need to worry about this.

> If not, we should probably fallback to selecting the first
> mode as before. This would also make things safer in case of a
> descructive interaction with the mode pruning code, where the
> recommended mode might get pruned (although that would be very
> unfortunate).

That part would be more troublesome, but I guess the current code is
just as vulnerable to this. If we prune all the modes, even the logic
to take the first one will fail.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2018-05-24  9:48 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-24  7:46 [igt-dev] [PATCH i-g-t 00/12] chamelium: Test the plane formats Maxime Ripard
2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 01/12] tests: Add vc4 test suite Maxime Ripard
2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 02/12] fb: Add buffer map/unmap functions Maxime Ripard
2018-05-14 13:27   ` Paul Kocialkowski
2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 03/12] fb: Add format conversion routine Maxime Ripard
2018-05-14 13:41   ` Paul Kocialkowski
2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 04/12] fb: Add more formats Maxime Ripard
2018-05-14 13:45   ` Paul Kocialkowski
2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 05/12] chamelium: Make chamelium_calculate_fb_crc private Maxime Ripard
2018-05-14 13:50   ` Paul Kocialkowski
2018-05-24  9:38     ` Maxime Ripard
2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 06/12] chamelium: Split CRC test function in two Maxime Ripard
2018-05-14 13:55   ` Paul Kocialkowski
2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 07/12] chamelium: Use preferred mode when testing a single mode Maxime Ripard
2018-05-14 14:00   ` Paul Kocialkowski
2018-05-24  9:48     ` Maxime Ripard
2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 08/12] chamelium: Add function to generate our test pattern Maxime Ripard
2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 09/12] chamelium: Change our pattern for a custom one Maxime Ripard
2018-05-14 14:24   ` Paul Kocialkowski
2018-05-18  7:26     ` Maxime Ripard
2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 10/12] chamelium: Add format support Maxime Ripard
2018-05-14 14:41   ` Paul Kocialkowski
2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 11/12] chamelium: Add format subtests Maxime Ripard
2018-05-14 14:46   ` Paul Kocialkowski
2018-04-24  7:46 ` [igt-dev] [PATCH i-g-t 12/12] tests: Add chamelium formats subtests to vc4 test lists Maxime Ripard
2018-04-24  8:29 ` [igt-dev] ✗ Fi.CI.BAT: failure for chamelium: Test the plane formats Patchwork
2018-04-24 10:51 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork
2018-04-24 12:17 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2018-05-14 13:17 ` [igt-dev] [PATCH i-g-t 00/12] " Maxime Ripard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.