diff mbox

[RFC,i-g-t,2/3] tests/chamelium: Add test case for plane formats

Message ID 20180305142129.18352-3-maxime.ripard@bootlin.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maxime Ripard March 5, 2018, 2:21 p.m. UTC
KMS can support a lot of different plane formats that are not being tested
by the current chamelium tests.

Add some preliminary tests to exert the RGB formats exposed by the KMS
planes.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 tests/Makefile.am             |   1 +
 tests/Makefile.sources        |   5 +
 tests/kms_chamelium_formats.c | 305 ++++++++++++++++++++++++++++++++++++++++++
 tests/meson.build             |   1 +
 4 files changed, 312 insertions(+)
 create mode 100644 tests/kms_chamelium_formats.c

Comments

Eric Anholt March 12, 2018, 7:02 p.m. UTC | #1
Maxime Ripard <maxime.ripard@bootlin.com> writes:

> KMS can support a lot of different plane formats that are not being tested
> by the current chamelium tests.
>
> Add some preliminary tests to exert the RGB formats exposed by the KMS
> planes.

I'm really excited for this test.  A few comments...

> ---
>  tests/Makefile.am             |   1 +
>  tests/Makefile.sources        |   5 +
>  tests/kms_chamelium_formats.c | 305 ++++++++++++++++++++++++++++++++++++++++++
>  tests/meson.build             |   1 +
>  4 files changed, 312 insertions(+)
>  create mode 100644 tests/kms_chamelium_formats.c
>
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 8472a6bf0a73..becc23de895b 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -17,6 +17,7 @@ endif
>  if HAVE_CHAMELIUM
>  TESTS_progs += \
>  	kms_chamelium \
> +	kms_chamelium_formats \
>  	$(NULL)
>  endif
>  
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index c27226fc96c9..8476b63a245c 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -280,6 +280,11 @@ kms_chamelium_SOURCES = \
>  	helpers_chamelium.h \
>  	helpers_chamelium.c
>  
> +kms_chamelium_formats_SOURCES = \
> +	kms_chamelium_formats.c \
> +	helpers_chamelium.h \
> +	helpers_chamelium.c
> +
>  testdisplay_SOURCES = \
>  	testdisplay.c \
>  	testdisplay.h \
> diff --git a/tests/kms_chamelium_formats.c b/tests/kms_chamelium_formats.c
> new file mode 100644
> index 000000000000..6d61f2fa34d8
> --- /dev/null
> +++ b/tests/kms_chamelium_formats.c
> @@ -0,0 +1,305 @@
> +/*
> + * Copyright © 2016 Red Hat Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *    Lyude Paul <lyude@redhat.com>
> + */
> +
> +#include "config.h"
> +#include "helpers_chamelium.h"
> +#include "igt.h"
> +
> +#include <fcntl.h>
> +#include <pixman.h>
> +#include <string.h>
> +
> +struct formats {
> +	uint32_t		drm_fmt;
> +	pixman_format_code_t	pixman_fmt;
> +} formats_map[] = {
> +	{ DRM_FORMAT_XRGB8888, PIXMAN_x8r8g8b8 },
> +	{ DRM_FORMAT_ARGB8888, PIXMAN_a8r8g8b8 },
> +	{ DRM_FORMAT_ABGR8888, PIXMAN_a8b8g8r8 },
> +	{ DRM_FORMAT_RGB565, PIXMAN_r5g6b5 },
> +	{ DRM_FORMAT_BGR565, PIXMAN_b5g6r5 },
> +	{ DRM_FORMAT_ARGB1555, PIXMAN_a1r5g5b5 },
> +	{ DRM_FORMAT_XRGB1555, PIXMAN_x1r5g5b5 },
> +};
> +
> +static pixman_image_t *paint_ar24_pattern(size_t width, size_t height)
> +{
> +	uint32_t colors[] = { 0xff000000,
> +			      0xffff0000,
> +			      0xff00ff00,
> +			      0xff0000ff,
> +			      0xffffffff };
> +	unsigned i, j;
> +	uint32_t *data;
> +
> +	data = malloc(width * height * sizeof(*data));
> +	igt_assert(data);
> +
> +	for (i = 0; i < height; i++)
> +		for (j = 0; j < width; j++)
> +			*(data + i * width + j) = colors[((j / 64) + (i / 64)) % 5];
> +
> +	return pixman_image_create_bits(PIXMAN_a8r8g8b8, width, height,
> +					data, width * 4);
> +}
> +
> +static void free_pattern(pixman_image_t *pattern)
> +{
> +	void *data = pixman_image_get_data(pattern);
> +
> +	pixman_image_unref(pattern);
> +	free(data);
> +}
> +
> +static pixman_image_t *pattern_to_fb(pixman_image_t *pattern, struct igt_fb *fb,
> +				     pixman_format_code_t pixman_fmt)
> +{
> +	pixman_image_t *converted;
> +	void *ptr;
> +
> +	igt_assert(fb->is_dumb);
> +
> +	ptr = kmstest_dumb_map_buffer(fb->fd, fb->gem_handle, fb->size,
> +				      PROT_READ | PROT_WRITE);
> +	igt_assert(ptr);
> +
> +	converted = pixman_image_create_bits(pixman_fmt, fb->width, fb->height,
> +					     ptr, fb->stride);
> +	pixman_image_composite(PIXMAN_OP_ADD, pattern, NULL, converted,
> +			       0, 0, 0, 0, 0, 0, fb->width, fb->height);

If you're trying to fill the FB with the incoming pattern, then
PIXMAN_OP_SRC is the thing you want (and will be *much* faster).

> +
> +	return converted;
> +}
> +
> +static pixman_image_t *convert_frame_format(pixman_image_t *src,
> +					    int format)
> +{
> +	pixman_image_t *converted;
> +	unsigned int w = pixman_image_get_width(src);
> +	unsigned int h = pixman_image_get_height(src);
> +	void *data = malloc(w * h * 4);
> +
> +	memset(data, 0, w * h * 4);
> +	converted = pixman_image_create_bits(format, w, h, data,
> +					     PIXMAN_FORMAT_BPP(format) / 8 * w);
> +	pixman_image_composite(PIXMAN_OP_ADD, src, NULL, converted,
> +			       0, 0, 0, 0, 0, 0, w, h);
> +	return converted;
> +}

Instead of the memset, you could just use PIXMAN_OP_SRC.

Also, instead of "* 4", probably want "* PIXMAN_FORMAT_BPP(format) / 8"
there too.

> +#define PIXEL_MASK	0x00f8f8f8

If we're going to have some tolerance, the tolerance should probably
depend on the lowest depth of the channel involved.  However, I'm not
sure this is needed -- we should be able to get bit-exact from VC4 by
filling in the size-extension bits in the HVS.
Maxime Ripard March 13, 2018, 3:05 p.m. UTC | #2
Hi Eric,

On Mon, Mar 12, 2018 at 12:02:26PM -0700, Eric Anholt wrote:
> > +static pixman_image_t *convert_frame_format(pixman_image_t *src,
> > +					    int format)
> > +{
> > +	pixman_image_t *converted;
> > +	unsigned int w = pixman_image_get_width(src);
> > +	unsigned int h = pixman_image_get_height(src);
> > +	void *data = malloc(w * h * 4);
> > +
> > +	memset(data, 0, w * h * 4);
> > +	converted = pixman_image_create_bits(format, w, h, data,
> > +					     PIXMAN_FORMAT_BPP(format) / 8 * w);
> > +	pixman_image_composite(PIXMAN_OP_ADD, src, NULL, converted,
> > +			       0, 0, 0, 0, 0, 0, w, h);
> > +	return converted;
> > +}
> 
> Instead of the memset, you could just use PIXMAN_OP_SRC.

I guess PIXMAN_OP_ADD will do a composition between the existing
buffer content and the new one, while PIXMAN_OP_SRC will just take
whatever comes from the buffer and overwrite the previous content?

> Also, instead of "* 4", probably want "* PIXMAN_FORMAT_BPP(format) / 8"
> there too.

Ah, yes, definitely.

> > +#define PIXEL_MASK	0x00f8f8f8
> 
> If we're going to have some tolerance, the tolerance should probably
> depend on the lowest depth of the channel involved.

Indeed, yes.

> However, I'm not sure this is needed -- we should be able to get
> bit-exact from VC4 by filling in the size-extension bits in the HVS.

I guess it really depends on what we're aiming for here. While it
would probably work with VC4, I think this part is rather generic, so
I guess we could end up in scenarios where the CRTC will not fill the
bits to our expected value and where it wouldn't be something we can
change.

I just looked at the Allwinner display engine for example, and it
didn't have such configuration (documented, at least).

That's why I went with this approach here (but I can definitely change
it if we want to deal with that theorical case later).

Maxime
diff mbox

Patch

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 8472a6bf0a73..becc23de895b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -17,6 +17,7 @@  endif
 if HAVE_CHAMELIUM
 TESTS_progs += \
 	kms_chamelium \
+	kms_chamelium_formats \
 	$(NULL)
 endif
 
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c27226fc96c9..8476b63a245c 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -280,6 +280,11 @@  kms_chamelium_SOURCES = \
 	helpers_chamelium.h \
 	helpers_chamelium.c
 
+kms_chamelium_formats_SOURCES = \
+	kms_chamelium_formats.c \
+	helpers_chamelium.h \
+	helpers_chamelium.c
+
 testdisplay_SOURCES = \
 	testdisplay.c \
 	testdisplay.h \
diff --git a/tests/kms_chamelium_formats.c b/tests/kms_chamelium_formats.c
new file mode 100644
index 000000000000..6d61f2fa34d8
--- /dev/null
+++ b/tests/kms_chamelium_formats.c
@@ -0,0 +1,305 @@ 
+/*
+ * Copyright © 2016 Red Hat Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *    Lyude Paul <lyude@redhat.com>
+ */
+
+#include "config.h"
+#include "helpers_chamelium.h"
+#include "igt.h"
+
+#include <fcntl.h>
+#include <pixman.h>
+#include <string.h>
+
+struct formats {
+	uint32_t		drm_fmt;
+	pixman_format_code_t	pixman_fmt;
+} formats_map[] = {
+	{ DRM_FORMAT_XRGB8888, PIXMAN_x8r8g8b8 },
+	{ DRM_FORMAT_ARGB8888, PIXMAN_a8r8g8b8 },
+	{ DRM_FORMAT_ABGR8888, PIXMAN_a8b8g8r8 },
+	{ DRM_FORMAT_RGB565, PIXMAN_r5g6b5 },
+	{ DRM_FORMAT_BGR565, PIXMAN_b5g6r5 },
+	{ DRM_FORMAT_ARGB1555, PIXMAN_a1r5g5b5 },
+	{ DRM_FORMAT_XRGB1555, PIXMAN_x1r5g5b5 },
+};
+
+static pixman_image_t *paint_ar24_pattern(size_t width, size_t height)
+{
+	uint32_t colors[] = { 0xff000000,
+			      0xffff0000,
+			      0xff00ff00,
+			      0xff0000ff,
+			      0xffffffff };
+	unsigned i, j;
+	uint32_t *data;
+
+	data = malloc(width * height * sizeof(*data));
+	igt_assert(data);
+
+	for (i = 0; i < height; i++)
+		for (j = 0; j < width; j++)
+			*(data + i * width + j) = colors[((j / 64) + (i / 64)) % 5];
+
+	return pixman_image_create_bits(PIXMAN_a8r8g8b8, width, height,
+					data, width * 4);
+}
+
+static void free_pattern(pixman_image_t *pattern)
+{
+	void *data = pixman_image_get_data(pattern);
+
+	pixman_image_unref(pattern);
+	free(data);
+}
+
+static pixman_image_t *pattern_to_fb(pixman_image_t *pattern, struct igt_fb *fb,
+				     pixman_format_code_t pixman_fmt)
+{
+	pixman_image_t *converted;
+	void *ptr;
+
+	igt_assert(fb->is_dumb);
+
+	ptr = kmstest_dumb_map_buffer(fb->fd, fb->gem_handle, fb->size,
+				      PROT_READ | PROT_WRITE);
+	igt_assert(ptr);
+
+	converted = pixman_image_create_bits(pixman_fmt, fb->width, fb->height,
+					     ptr, fb->stride);
+	pixman_image_composite(PIXMAN_OP_ADD, pattern, NULL, converted,
+			       0, 0, 0, 0, 0, 0, fb->width, fb->height);
+
+	return converted;
+}
+
+static pixman_image_t *convert_frame_format(pixman_image_t *src,
+					    int format)
+{
+	pixman_image_t *converted;
+	unsigned int w = pixman_image_get_width(src);
+	unsigned int h = pixman_image_get_height(src);
+	void *data = malloc(w * h * 4);
+
+	memset(data, 0, w * h * 4);
+	converted = pixman_image_create_bits(format, w, h, data,
+					     PIXMAN_FORMAT_BPP(format) / 8 * w);
+	pixman_image_composite(PIXMAN_OP_ADD, src, NULL, converted,
+			       0, 0, 0, 0, 0, 0, w, h);
+	return converted;
+}
+
+#define PIXEL_MASK	0x00f8f8f8
+
+static bool format_compare_pixels(void *reference, void *frame,
+				  size_t height, size_t width)
+{
+	uint32_t *ref_frame = reference;
+	uint32_t *new_frame = frame;
+	unsigned int h, w;
+
+	for (h = 0; h < height; h++) {
+		for (w = 0; w < width; w++) {
+			uint32_t *ref = ref_frame + h * width + w;
+			uint32_t m_ref = *ref & PIXEL_MASK;
+			uint32_t *new = new_frame + h * width + w;
+			uint32_t m_new = *new & PIXEL_MASK;
+
+			if (m_new != m_ref) {
+				igt_info("pix %d:%d mismatch: 0x%08x vs 0x%08x\n",
+					 h, w, m_ref, m_new);
+				return false;
+			}
+		}
+	}
+
+	return true;
+}	
+
+static bool format_compare_frames(pixman_image_t *reference,
+				  struct chamelium_frame_dump *frame,
+				  uint32_t format, int index)
+{
+	pixman_image_t *frame_bgr, *frame_ar24;
+	bool match;
+
+	frame_bgr = pixman_image_create_bits(PIXMAN_b8g8r8,
+					     frame->width, frame->height,
+					     (uint32_t *)frame->bgr,
+					     PIXMAN_FORMAT_BPP(PIXMAN_b8g8r8) / 8 * frame->width);
+	/*
+	 * Convert the reference image into the same format as the chamelium
+	 * image
+	 */
+
+        frame_ar24 = convert_frame_format(frame_bgr, PIXMAN_a8r8g8b8);
+
+	match = format_compare_pixels(pixman_image_get_data(reference),
+				      pixman_image_get_data(frame_ar24),
+				      frame->height, frame->width);
+
+	pixman_image_unref(frame_ar24);
+	pixman_image_unref(frame_bgr);
+
+	return match;
+}
+
+static void
+test_display_frame_dump(data_t *data, struct chamelium_port *port)
+{
+	igt_output_t *output;
+	igt_plane_t *primary;
+	drmModeConnector *connector;
+	int i;
+
+	reset_state(data, port);
+
+	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);
+
+	for (i = 0; i < connector->count_modes; i++) {
+		drmModePlanePtr plane = primary->drm_plane;
+		drmModeModeInfo *mode;
+		pixman_image_t *pattern;
+		int j;
+
+		mode = &connector->modes[i];
+		if (!(mode->type & DRM_MODE_TYPE_PREFERRED))
+			continue;
+
+		pattern = paint_ar24_pattern(mode->hdisplay, mode->vdisplay);
+		igt_assert(pattern);
+
+		for (j = 0; j < plane->count_formats; j++) {
+			uint32_t format = plane->formats[j];
+			struct formats *format_map = NULL;
+			pixman_image_t *patternfb;
+			struct igt_fb fb;
+			int fb_id, k;
+
+			for (k = 0; k < ARRAY_SIZE(formats_map); k++)
+				if (formats_map[k].drm_fmt == format)
+					format_map = &formats_map[k];
+
+			if (!format_map)
+				continue;
+
+			igt_info("Using mode %ux%u with format %s\n",
+				 mode->hdisplay, mode->vdisplay,
+				 igt_format_str(format));
+
+			fb_id = igt_create_fb(data->drm_fd,
+					      mode->hdisplay,
+					      mode->vdisplay,
+					      format_map->drm_fmt,
+					      LOCAL_DRM_FORMAT_MOD_NONE,
+					      &fb);
+			igt_assert(fb_id > 0);
+
+			patternfb = pattern_to_fb(pattern, &fb,
+						  format_map->pixman_fmt);
+			enable_output(data, port, output, mode, &fb);
+			chamelium_capture(data->chamelium, port, 0, 0, 0, 0, 5);
+
+			for (k = 0; k < 5; k++) {
+				struct chamelium_frame_dump *frame;
+				bool match;
+
+				frame = chamelium_read_captured_frame(data->chamelium, k);
+				match = format_compare_frames(pattern, frame, format, k);
+				chamelium_destroy_frame_dump(frame);
+
+				igt_info("Frame %d/5: %s\n",
+					 k + 1, match ? "passed" : "failed");
+			}
+
+			pixman_image_unref(patternfb);
+			igt_remove_fb(data->drm_fd, &fb);
+		}
+
+		free_pattern(pattern);
+	}
+
+	drmModeFreeConnector(connector);
+}
+
+#define for_each_port(p, port)            \
+	for (p = 0, port = data.ports[p]; \
+	     p < data.port_count;         \
+	     p++, port = data.ports[p])
+
+#define connector_subtest(name__, type__)                    \
+	igt_subtest(name__)                                  \
+		for_each_port(p, port)                       \
+			if (chamelium_port_get_type(port) == \
+			    DRM_MODE_CONNECTOR_ ## type__)
+
+static data_t data;
+
+igt_main
+{
+	struct chamelium_port *port;
+	int edid_id, alt_edid_id, p;
+
+	igt_fixture {
+		igt_skip_on_simulation();
+
+		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
+		data.chamelium = chamelium_init(data.drm_fd);
+		igt_require(data.chamelium);
+
+		data.ports = chamelium_get_ports(data.chamelium,
+						 &data.port_count);
+
+		edid_id = chamelium_new_edid(data.chamelium,
+					     igt_kms_get_base_edid());
+		alt_edid_id = chamelium_new_edid(data.chamelium,
+						 igt_kms_get_alt_edid());
+		data.edid_id = edid_id;
+		data.alt_edid_id = alt_edid_id;
+
+		/* So fbcon doesn't try to reprobe things itself */
+		kmstest_set_vt_graphics_mode();
+
+		igt_display_init(&data.display, data.drm_fd);
+		igt_require(data.display.is_atomic);
+	}
+
+	igt_subtest_group {
+		igt_fixture {
+			require_connector_present(
+			    &data, DRM_MODE_CONNECTOR_HDMIA);
+		}
+
+		connector_subtest("hdmi-frame-dump", HDMIA)
+			test_display_frame_dump(&data, port);
+	}
+
+	igt_fixture {
+		igt_display_fini(&data.display);
+		close(data.drm_fd);
+	}
+}
diff --git a/tests/meson.build b/tests/meson.build
index 2a1e6f19e374..98365c33e3ce 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -244,6 +244,7 @@  endif
 if chamelium.found()
 	test_progs += [
 		'kms_chamelium',
+		'kms_chamelium_formats',
 	]
 	test_deps += chamelium
 endif