[igt-dev] [PATCH i-g-t 0/8] igt: chamelium: Test YUV buffers using the Chamelium

[igt-dev] [PATCH i-g-t 0/8] igt: chamelium: Test YUV buffers using the Chamelium

[Maxime Ripard]

Hi,

Here is a first attempt at expanding the current Chamelium formats
test to the NV12 format.

It builds on top of the current infrastructure for the NV12
convertion, and the format suppport of the Chamelium.

Let me know what you think,
Maxime

Maxime Ripard (6):
  igt: tests: chamelium: Start to unify tests
  igt: tests: chamelium: Convert VGA tests to do_test_display
  igt: fb: Move i915 buffer allocation to a function of its own
  igt: fb: Separate YUV allocation from the rest
  igt: fb: Fallback on KMS dumb buffer allocation for YUV buffers
  igt: tests: chamelium: Add NV12 format

Paul Kocialkowski (2):
  chamelium: Fix inverted xr24 pattern paint dimensions
  chamelium: Pass and use stride for xr24 pattern paint

 lib/igt_fb.c          | 140 ++++++++++++++++++--------
 tests/kms_chamelium.c | 234 ++++++++++++++++++++-----------------------
 2 files changed, 210 insertions(+), 164 deletions(-)

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

[igt-dev] [PATCH i-g-t 1/8] chamelium: Fix inverted xr24 pattern paint dimensions

[Maxime Ripard]

From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

The xr24 pattern for chamelium testing appears mangled when checking it
on an actual display. This is because the horizontal and vertical
display sizes are inverted when used as width and height.

Put them back in order.

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

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 55b346a9e1c4..ee7580b54e69 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -518,7 +518,7 @@ static int chamelium_get_pattern_fb(data_t *data, drmModeModeInfo *mode,
 	ptr = igt_fb_map_buffer(fb->fd, fb);
 	igt_assert(ptr);
 
-	chamelium_paint_xr24_pattern(ptr, mode->vdisplay, mode->hdisplay);
+	chamelium_paint_xr24_pattern(ptr, mode->hdisplay, mode->vdisplay);
 	igt_fb_unmap_buffer(fb, ptr);
 
 	return fb_id;
-- 
2.19.1

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

[igt-dev] [PATCH i-g-t 2/8] chamelium: Pass and use stride for xr24 pattern paint

[Maxime Ripard]

From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Using the width from the selected mode is not sufficient to correctly
paint a pattern on the framebuffer memory: the stride also has to be
taken in account for proper line start alignment.

Pass the stride and use it in chamelium_paint_xr24_pattern.

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

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index ee7580b54e69..ebca5eefd222 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -489,7 +489,8 @@ enable_output(data_t *data,
 }
 
 static void chamelium_paint_xr24_pattern(uint32_t *data,
-					 size_t width, size_t height)
+					 size_t width, size_t height,
+					 size_t stride)
 {
 	uint32_t colors[] = { 0xff000000,
 			      0xffff0000,
@@ -500,7 +501,7 @@ static void chamelium_paint_xr24_pattern(uint32_t *data,
 
 	for (i = 0; i < height; i++)
 		for (j = 0; j < width; j++)
-			*(data + i * width + j) = colors[((j / 64) + (i / 64)) % 5];
+			*(data + i * stride / 4 + j) = colors[((j / 64) + (i / 64)) % 5];
 }
 
 static int chamelium_get_pattern_fb(data_t *data, drmModeModeInfo *mode,
@@ -518,7 +519,8 @@ static int chamelium_get_pattern_fb(data_t *data, drmModeModeInfo *mode,
 	ptr = igt_fb_map_buffer(fb->fd, fb);
 	igt_assert(ptr);
 
-	chamelium_paint_xr24_pattern(ptr, mode->hdisplay, mode->vdisplay);
+	chamelium_paint_xr24_pattern(ptr, mode->hdisplay, mode->vdisplay,
+				     fb->strides[0]);
 	igt_fb_unmap_buffer(fb, ptr);
 
 	return fb_id;
-- 
2.19.1

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

[igt-dev] [PATCH i-g-t 3/8] igt: tests: chamelium: Start to unify tests

[Maxime Ripard]

The various tests in kms_chamelium are really variants of one another
but share little code.

In addition to the duplication, this gets in the way of the
introduction of more tests, or to be able to run all the tests on all
the output, which isn't the case at the moment, with the HDMI and DP
tests and the VGA tests being different.

Start by introducing a check parameter to the do_test_display
function, that will tell which test method we want to use to compare
the frames.

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

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index ebca5eefd222..8b9872a17044 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -526,12 +526,15 @@ static int chamelium_get_pattern_fb(data_t *data, drmModeModeInfo *mode,
 	return fb_id;
 }
 
-static void do_test_display_crc(data_t *data, struct chamelium_port *port,
-				igt_output_t *output, drmModeModeInfo *mode,
-				uint32_t fourcc, int count)
+enum chamelium_check {
+	CHAMELIUM_CHECK_CRC,
+};
+
+static void do_test_display(data_t *data, struct chamelium_port *port,
+			    igt_output_t *output, drmModeModeInfo *mode,
+			    uint32_t fourcc, enum chamelium_check check,
+			    int count)
 {
-	igt_crc_t *crc;
-	igt_crc_t *expected_crc;
 	struct chamelium_fb_crc_async_data *fb_crc;
 	struct igt_fb frame_fb, fb;
 	int i, fb_id, captured_frame_count;
@@ -544,39 +547,46 @@ static void do_test_display_crc(data_t *data, struct chamelium_port *port,
 	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);
+	if (check == CHAMELIUM_CHECK_CRC)
+		fb_crc = chamelium_calculate_fb_crc_async_start(data->drm_fd,
+								&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
-	 * 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);
+	if (check == CHAMELIUM_CHECK_CRC) {
+		igt_crc_t *expected_crc;
+		igt_crc_t *crc;
 
-	igt_assert(captured_frame_count == count);
+		/* 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_debug("Captured %d frames\n", captured_frame_count);
+		igt_assert(captured_frame_count == count);
 
-	expected_crc = chamelium_calculate_fb_crc_async_finish(fb_crc);
+		igt_debug("Captured %d frames\n", captured_frame_count);
 
-	for (i = 0; i < captured_frame_count; i++)
-		chamelium_assert_crc_eq_or_dump(data->chamelium,
-						expected_crc, &crc[i],
-						&fb, i);
+		expected_crc = chamelium_calculate_fb_crc_async_finish(fb_crc);
 
-	free(expected_crc);
-	free(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, &frame_fb);
 	igt_remove_fb(data->drm_fd, &fb);
 }
 
-static void test_display_crc_one_mode(data_t *data, struct chamelium_port *port,
-				      uint32_t fourcc, int count)
+static void test_display_one_mode(data_t *data, struct chamelium_port *port,
+				  uint32_t fourcc, enum chamelium_check check,
+				  int count)
 {
 	igt_output_t *output;
 	drmModeConnector *connector;
@@ -589,13 +599,15 @@ 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], fourcc, count);
+	do_test_display(data, port, output, &connector->modes[0], fourcc,
+			check, count);
 
 	drmModeFreeConnector(connector);
 }
 
-static void test_display_crc_all_modes(data_t *data, struct chamelium_port *port,
-				       uint32_t fourcc, int count)
+static void test_display_all_modes(data_t *data, struct chamelium_port *port,
+				   uint32_t fourcc, enum chamelium_check check,
+				   int count)
 {
 	igt_output_t *output;
 	igt_plane_t *primary;
@@ -612,7 +624,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, fourcc, count);
+		do_test_display(data, port, output, mode, fourcc, check, count);
 	}
 
 	drmModeFreeConnector(connector);
@@ -873,16 +885,16 @@ igt_main
 							edid_id, alt_edid_id);
 
 		connector_subtest("dp-crc-single", DisplayPort)
-			test_display_crc_all_modes(&data, port,
-						   DRM_FORMAT_XRGB8888, 1);
+			test_display_all_modes(&data, port, DRM_FORMAT_XRGB8888,
+					       CHAMELIUM_CHECK_CRC, 1);
 
 		connector_subtest("dp-crc-fast", DisplayPort)
-			test_display_crc_one_mode(&data, port,
-						  DRM_FORMAT_XRGB8888, 1);
+			test_display_one_mode(&data, port, DRM_FORMAT_XRGB8888,
+					      CHAMELIUM_CHECK_CRC, 1);
 
 		connector_subtest("dp-crc-multiple", DisplayPort)
-			test_display_crc_all_modes(&data, port,
-						   DRM_FORMAT_XRGB8888, 3);
+			test_display_all_modes(&data, port, DRM_FORMAT_XRGB8888,
+					       CHAMELIUM_CHECK_CRC, 3);
 
 		connector_subtest("dp-frame-dump", DisplayPort)
 			test_display_frame_dump(&data, port);
@@ -940,56 +952,56 @@ igt_main
 							edid_id, alt_edid_id);
 
 		connector_subtest("hdmi-crc-single", HDMIA)
-			test_display_crc_all_modes(&data, port,
-						   DRM_FORMAT_XRGB8888, 1);
+			test_display_all_modes(&data, port, DRM_FORMAT_XRGB8888,
+					       CHAMELIUM_CHECK_CRC, 1);
 
 		connector_subtest("hdmi-crc-fast", HDMIA)
-			test_display_crc_one_mode(&data, port,
-						  DRM_FORMAT_XRGB8888, 1);
+			test_display_one_mode(&data, port, DRM_FORMAT_XRGB8888,
+					      CHAMELIUM_CHECK_CRC, 1);
 
 		connector_subtest("hdmi-crc-multiple", HDMIA)
-			test_display_crc_all_modes(&data, port,
-						   DRM_FORMAT_XRGB8888, 3);
+			test_display_all_modes(&data, port, DRM_FORMAT_XRGB8888,
+					       CHAMELIUM_CHECK_CRC, 3);
 
 		connector_subtest("hdmi-crc-argb8888", HDMIA)
-			test_display_crc_one_mode(&data, port,
-						  DRM_FORMAT_ARGB8888, 1);
+			test_display_one_mode(&data, port, DRM_FORMAT_ARGB8888,
+					      CHAMELIUM_CHECK_CRC, 1);
 
 		connector_subtest("hdmi-crc-abgr8888", HDMIA)
-			test_display_crc_one_mode(&data, port,
-						  DRM_FORMAT_ABGR8888, 1);
+			test_display_one_mode(&data, port, DRM_FORMAT_ABGR8888,
+					      CHAMELIUM_CHECK_CRC, 1);
 
 		connector_subtest("hdmi-crc-xrgb8888", HDMIA)
-			test_display_crc_one_mode(&data, port,
-						  DRM_FORMAT_XRGB8888, 1);
+			test_display_one_mode(&data, port, DRM_FORMAT_XRGB8888,
+					      CHAMELIUM_CHECK_CRC, 1);
 
 		connector_subtest("hdmi-crc-xbgr8888", HDMIA)
-			test_display_crc_one_mode(&data, port,
-						  DRM_FORMAT_XBGR8888, 1);
+			test_display_one_mode(&data, port, DRM_FORMAT_XBGR8888,
+					      CHAMELIUM_CHECK_CRC, 1);
 
 		connector_subtest("hdmi-crc-rgb888", HDMIA)
-			test_display_crc_one_mode(&data, port,
-						  DRM_FORMAT_RGB888, 1);
+			test_display_one_mode(&data, port, DRM_FORMAT_RGB888,
+					      CHAMELIUM_CHECK_CRC, 1);
 
 		connector_subtest("hdmi-crc-bgr888", HDMIA)
-			test_display_crc_one_mode(&data, port,
-						  DRM_FORMAT_BGR888, 1);
+			test_display_one_mode(&data, port, DRM_FORMAT_BGR888,
+					      CHAMELIUM_CHECK_CRC, 1);
 
 		connector_subtest("hdmi-crc-rgb565", HDMIA)
-			test_display_crc_one_mode(&data, port,
-						  DRM_FORMAT_RGB565, 1);
+			test_display_one_mode(&data, port, DRM_FORMAT_RGB565,
+					      CHAMELIUM_CHECK_CRC, 1);
 
 		connector_subtest("hdmi-crc-bgr565", HDMIA)
-			test_display_crc_one_mode(&data, port,
-						  DRM_FORMAT_BGR565, 1);
+			test_display_one_mode(&data, port, DRM_FORMAT_BGR565,
+					      CHAMELIUM_CHECK_CRC, 1);
 
 		connector_subtest("hdmi-crc-argb1555", HDMIA)
-			test_display_crc_one_mode(&data, port,
-						  DRM_FORMAT_ARGB1555, 1);
+			test_display_one_mode(&data, port, DRM_FORMAT_ARGB1555,
+					      CHAMELIUM_CHECK_CRC, 1);
 
 		connector_subtest("hdmi-crc-xrgb1555", HDMIA)
-			test_display_crc_one_mode(&data, port,
-						  DRM_FORMAT_XRGB1555, 1);
+			test_display_one_mode(&data, port, DRM_FORMAT_XRGB1555,
+					      CHAMELIUM_CHECK_CRC, 1);
 
 		connector_subtest("hdmi-frame-dump", HDMIA)
 			test_display_frame_dump(&data, port);
-- 
2.19.1

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

[igt-dev] [PATCH i-g-t 4/8] igt: tests: chamelium: Convert VGA tests to do_test_display

[Maxime Ripard]

The VGA tests were run so far as part of a separate function. However,
the only difference between that function and the do_test_display
function was the kind of comparison we wanted to use. Indeed, VGA
being an analog output, we can't rely on a pixel perfect image, and
thus only compare the CRCs, but we need to make some more advanced
comparison.

Let's add a new check method, add support for the analog check to
do_test_display and remove the old version.

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

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 8b9872a17044..80b279c4f1c2 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -527,6 +527,7 @@ static int chamelium_get_pattern_fb(data_t *data, drmModeModeInfo *mode,
 }
 
 enum chamelium_check {
+	CHAMELIUM_CHECK_ANALOG,
 	CHAMELIUM_CHECK_CRC,
 };
 
@@ -578,6 +579,18 @@ static void do_test_display(data_t *data, struct chamelium_port *port,
 
 		free(expected_crc);
 		free(crc);
+	} else if (check == CHAMELIUM_CHECK_ANALOG) {
+		struct chamelium_frame_dump *dump;
+
+		igt_assert(count == 1);
+
+		dump = chamelium_port_dump_pixels(data->chamelium, port, 0, 0,
+						  0, 0);
+		chamelium_crop_analog_frame(dump, mode->hdisplay,
+					    mode->vdisplay);
+		chamelium_assert_analog_frame_match_or_dump(data->chamelium,
+							    port, dump, &fb);
+		chamelium_destroy_frame_dump(dump);
 	}
 
 	igt_remove_fb(data->drm_fd, &frame_fb);
@@ -588,8 +601,9 @@ static void test_display_one_mode(data_t *data, struct chamelium_port *port,
 				  uint32_t fourcc, enum chamelium_check check,
 				  int count)
 {
-	igt_output_t *output;
 	drmModeConnector *connector;
+	drmModeModeInfo *mode;
+	igt_output_t *output;
 	igt_plane_t *primary;
 
 	reset_state(data, port);
@@ -599,8 +613,14 @@ static void test_display_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(data, port, output, &connector->modes[0], fourcc,
-			check, count);
+	mode = &connector->modes[0];
+	if (check == CHAMELIUM_CHECK_ANALOG) {
+		bool bridge = check_analog_bridge(data, port);
+
+		igt_assert(!(bridge && prune_vga_mode(data, mode)));
+	}
+
+	do_test_display(data, port, output, mode, fourcc, check, count);
 
 	drmModeFreeConnector(connector);
 }
@@ -612,6 +632,7 @@ static void test_display_all_modes(data_t *data, struct chamelium_port *port,
 	igt_output_t *output;
 	igt_plane_t *primary;
 	drmModeConnector *connector;
+	bool bridge;
 	int i;
 
 	reset_state(data, port);
@@ -621,9 +642,16 @@ static void test_display_all_modes(data_t *data, struct chamelium_port *port,
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	igt_assert(primary);
 
+	if (check == CHAMELIUM_CHECK_ANALOG)
+		bridge = check_analog_bridge(data, port);
+
 	for (i = 0; i < connector->count_modes; i++) {
 		drmModeModeInfo *mode = &connector->modes[i];
 
+		if (check == CHAMELIUM_CHECK_ANALOG && bridge &&
+		    prune_vga_mode(data, mode))
+			continue;
+
 		do_test_display(data, port, output, mode, fourcc, check, count);
 	}
 
@@ -674,61 +702,6 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
 	drmModeFreeConnector(connector);
 }
 
-static void
-test_analog_frame_dump(data_t *data, struct chamelium_port *port)
-{
-	igt_output_t *output;
-	igt_plane_t *primary;
-	struct igt_fb fb;
-	struct chamelium_frame_dump *frame;
-	drmModeModeInfo *mode;
-	drmModeConnector *connector;
-	int fb_id, i;
-	bool bridge;
-
-	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);
-
-	bridge = check_analog_bridge(data, port);
-
-	for (i = 0; i < connector->count_modes; i++) {
-		mode = &connector->modes[i];
-
-		if (bridge && prune_vga_mode(data, mode))
-			continue;
-
-		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);
-
-		enable_output(data, port, output, mode, &fb);
-
-		igt_debug("Reading frame dumps from Chamelium...\n");
-
-		frame = chamelium_port_dump_pixels(data->chamelium, port, 0, 0,
-						   0, 0);
-
-		chamelium_crop_analog_frame(frame, mode->hdisplay,
-					    mode->vdisplay);
-
-		chamelium_assert_analog_frame_match_or_dump(data->chamelium,
-							    port, frame, &fb);
-
-		chamelium_destroy_frame_dump(frame);
-
-		igt_remove_fb(data->drm_fd, &fb);
-	}
-
-	drmModeFreeConnector(connector);
-}
-
 static void
 test_hpd_without_ddc(data_t *data, struct chamelium_port *port)
 {
@@ -1040,7 +1013,8 @@ igt_main
 			test_hpd_without_ddc(&data, port);
 
 		connector_subtest("vga-frame-dump", VGA)
-			test_analog_frame_dump(&data, port);
+			test_display_all_modes(&data, port, DRM_FORMAT_XRGB8888,
+					       CHAMELIUM_CHECK_ANALOG, 1);
 	}
 
 	igt_subtest_group {
-- 
2.19.1

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

[igt-dev] [PATCH 4/8] igt: tests: chamelium: Convert VGA tests to do_test_display

[Maxime Ripard]

The VGA tests were run so far as part of a separate function. However,
the only difference between that function and the do_test_display
function was the kind of comparison we wanted to use. Indeed, VGA
being an analog output, we can't rely on a pixel perfect image, and
thus only compare the CRCs, but we need to make some more advanced
comparison.

Let's add a new check method, add support for the analog check to
do_test_display and remove the old version.

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

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 8b9872a17044..80b279c4f1c2 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -527,6 +527,7 @@ static int chamelium_get_pattern_fb(data_t *data, drmModeModeInfo *mode,
 }
 
 enum chamelium_check {
+	CHAMELIUM_CHECK_ANALOG,
 	CHAMELIUM_CHECK_CRC,
 };
 
@@ -578,6 +579,18 @@ static void do_test_display(data_t *data, struct chamelium_port *port,
 
 		free(expected_crc);
 		free(crc);
+	} else if (check == CHAMELIUM_CHECK_ANALOG) {
+		struct chamelium_frame_dump *dump;
+
+		igt_assert(count == 1);
+
+		dump = chamelium_port_dump_pixels(data->chamelium, port, 0, 0,
+						  0, 0);
+		chamelium_crop_analog_frame(dump, mode->hdisplay,
+					    mode->vdisplay);
+		chamelium_assert_analog_frame_match_or_dump(data->chamelium,
+							    port, dump, &fb);
+		chamelium_destroy_frame_dump(dump);
 	}
 
 	igt_remove_fb(data->drm_fd, &frame_fb);
@@ -588,8 +601,9 @@ static void test_display_one_mode(data_t *data, struct chamelium_port *port,
 				  uint32_t fourcc, enum chamelium_check check,
 				  int count)
 {
-	igt_output_t *output;
 	drmModeConnector *connector;
+	drmModeModeInfo *mode;
+	igt_output_t *output;
 	igt_plane_t *primary;
 
 	reset_state(data, port);
@@ -599,8 +613,14 @@ static void test_display_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(data, port, output, &connector->modes[0], fourcc,
-			check, count);
+	mode = &connector->modes[0];
+	if (check == CHAMELIUM_CHECK_ANALOG) {
+		bool bridge = check_analog_bridge(data, port);
+
+		igt_assert(!(bridge && prune_vga_mode(data, mode)));
+	}
+
+	do_test_display(data, port, output, mode, fourcc, check, count);
 
 	drmModeFreeConnector(connector);
 }
@@ -612,6 +632,7 @@ static void test_display_all_modes(data_t *data, struct chamelium_port *port,
 	igt_output_t *output;
 	igt_plane_t *primary;
 	drmModeConnector *connector;
+	bool bridge;
 	int i;
 
 	reset_state(data, port);
@@ -621,9 +642,16 @@ static void test_display_all_modes(data_t *data, struct chamelium_port *port,
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	igt_assert(primary);
 
+	if (check == CHAMELIUM_CHECK_ANALOG)
+		bridge = check_analog_bridge(data, port);
+
 	for (i = 0; i < connector->count_modes; i++) {
 		drmModeModeInfo *mode = &connector->modes[i];
 
+		if (check == CHAMELIUM_CHECK_ANALOG && bridge &&
+		    prune_vga_mode(data, mode))
+			continue;
+
 		do_test_display(data, port, output, mode, fourcc, check, count);
 	}
 
@@ -675,61 +703,6 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
 }
 
 static void
-test_analog_frame_dump(data_t *data, struct chamelium_port *port)
-{
-	igt_output_t *output;
-	igt_plane_t *primary;
-	struct igt_fb fb;
-	struct chamelium_frame_dump *frame;
-	drmModeModeInfo *mode;
-	drmModeConnector *connector;
-	int fb_id, i;
-	bool bridge;
-
-	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);
-
-	bridge = check_analog_bridge(data, port);
-
-	for (i = 0; i < connector->count_modes; i++) {
-		mode = &connector->modes[i];
-
-		if (bridge && prune_vga_mode(data, mode))
-			continue;
-
-		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);
-
-		enable_output(data, port, output, mode, &fb);
-
-		igt_debug("Reading frame dumps from Chamelium...\n");
-
-		frame = chamelium_port_dump_pixels(data->chamelium, port, 0, 0,
-						   0, 0);
-
-		chamelium_crop_analog_frame(frame, mode->hdisplay,
-					    mode->vdisplay);
-
-		chamelium_assert_analog_frame_match_or_dump(data->chamelium,
-							    port, frame, &fb);
-
-		chamelium_destroy_frame_dump(frame);
-
-		igt_remove_fb(data->drm_fd, &fb);
-	}
-
-	drmModeFreeConnector(connector);
-}
-
-static void
 test_hpd_without_ddc(data_t *data, struct chamelium_port *port)
 {
 	struct udev_monitor *mon = igt_watch_hotplug();
@@ -1040,7 +1013,8 @@ igt_main
 			test_hpd_without_ddc(&data, port);
 
 		connector_subtest("vga-frame-dump", VGA)
-			test_analog_frame_dump(&data, port);
+			test_display_all_modes(&data, port, DRM_FORMAT_XRGB8888,
+					       CHAMELIUM_CHECK_ANALOG, 1);
 	}
 
 	igt_subtest_group {
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH 5/8] igt: fb: Move i915 buffer allocation to a function of its own

[Maxime Ripard]

The create_bo_for_fb function has a rather big, non trivial block of
code to allocate buffers in some non-trivial situation on the i915
driver.

Since this is both non-trivial and driver specific, move that code
into a function of its own so that we can rework that function later
on.

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

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 2462d6ba6fab..d96689aa5fe6 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -453,6 +453,52 @@ uint64_t igt_fb_tiling_to_mod(uint64_t tiling)
 	}
 }
 
+static int i915_create_gem_for_fb(struct igt_fb *fb)
+{
+	bool full_range = fb->color_range == IGT_COLOR_YCBCR_FULL_RANGE;
+	int fd = fb->fd;
+	void *ptr;
+
+	fb->gem_handle = gem_create(fd, fb->size);
+	gem_set_tiling(fd, fb->gem_handle,
+		       igt_fb_mod_to_tiling(fb->tiling),
+		       fb->strides[0]);
+
+	gem_set_domain(fd, fb->gem_handle,
+		       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
+
+	/* Ensure the framebuffer is preallocated */
+	ptr = gem_mmap__gtt(fd, fb->gem_handle,
+			    fb->size, PROT_READ | PROT_WRITE);
+	igt_assert(*(uint32_t *)ptr == 0);
+
+	switch (fb->drm_format) {
+	case DRM_FORMAT_NV12:
+		memset(ptr + fb->offsets[0],
+		       full_range ? 0x00 : 0x10,
+		       fb->strides[0] * fb->plane_height[0]);
+		memset(ptr + fb->offsets[1],
+		       0x80,
+		       fb->strides[1] * fb->plane_height[1]);
+		break;
+	case DRM_FORMAT_YUYV:
+	case DRM_FORMAT_YVYU:
+		wmemset(ptr + fb->offsets[0],
+			full_range ? 0x80008000 : 0x80108010,
+			fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
+		break;
+	case DRM_FORMAT_UYVY:
+	case DRM_FORMAT_VYUY:
+		wmemset(ptr + fb->offsets[0],
+			full_range ? 0x00800080 : 0x10801080,
+			fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
+		break;
+	}
+	gem_munmap(ptr, fb->size);
+
+	return fb->gem_handle;
+}
+
 /* helpers to create nice-looking framebuffers */
 static int create_bo_for_fb(struct igt_fb *fb)
 {
@@ -470,48 +516,7 @@ static int create_bo_for_fb(struct igt_fb *fb)
 		fb->is_dumb = false;
 
 		if (is_i915_device(fd)) {
-			void *ptr;
-			bool full_range = fb->color_range == IGT_COLOR_YCBCR_FULL_RANGE;
-
-			fb->gem_handle = gem_create(fd, fb->size);
-
-			gem_set_tiling(fd, fb->gem_handle,
-				       igt_fb_mod_to_tiling(fb->tiling),
-				       fb->strides[0]);
-
-			gem_set_domain(fd, fb->gem_handle,
-				       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
-
-			/* Ensure the framebuffer is preallocated */
-			ptr = gem_mmap__gtt(fd, fb->gem_handle,
-					    fb->size, PROT_READ | PROT_WRITE);
-			igt_assert(*(uint32_t *)ptr == 0);
-
-			switch (fb->drm_format) {
-			case DRM_FORMAT_NV12:
-				memset(ptr + fb->offsets[0],
-				       full_range ? 0x00 : 0x10,
-				       fb->strides[0] * fb->plane_height[0]);
-				memset(ptr + fb->offsets[1],
-				       0x80,
-				       fb->strides[1] * fb->plane_height[1]);
-				break;
-			case DRM_FORMAT_YUYV:
-			case DRM_FORMAT_YVYU:
-				wmemset(ptr + fb->offsets[0],
-					full_range ? 0x80008000 : 0x80108010,
-					fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
-				break;
-			case DRM_FORMAT_UYVY:
-			case DRM_FORMAT_VYUY:
-				wmemset(ptr + fb->offsets[0],
-					full_range ? 0x00800080 : 0x10801080,
-					fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
-				break;
-			}
-			gem_munmap(ptr, fb->size);
-
-			return fb->gem_handle;
+			return i915_create_gem_for_fb(fb);
 		} else {
 			bool driver_has_gem_api = false;
 
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 5/8] igt: fb: Move i915 buffer allocation to a function of its own

[Maxime Ripard]

The create_bo_for_fb function has a rather big, non trivial block of
code to allocate buffers in some non-trivial situation on the i915
driver.

Since this is both non-trivial and driver specific, move that code
into a function of its own so that we can rework that function later
on.

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

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 2462d6ba6fab..d96689aa5fe6 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -453,6 +453,52 @@ uint64_t igt_fb_tiling_to_mod(uint64_t tiling)
 	}
 }
 
+static int i915_create_gem_for_fb(struct igt_fb *fb)
+{
+	bool full_range = fb->color_range == IGT_COLOR_YCBCR_FULL_RANGE;
+	int fd = fb->fd;
+	void *ptr;
+
+	fb->gem_handle = gem_create(fd, fb->size);
+	gem_set_tiling(fd, fb->gem_handle,
+		       igt_fb_mod_to_tiling(fb->tiling),
+		       fb->strides[0]);
+
+	gem_set_domain(fd, fb->gem_handle,
+		       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
+
+	/* Ensure the framebuffer is preallocated */
+	ptr = gem_mmap__gtt(fd, fb->gem_handle,
+			    fb->size, PROT_READ | PROT_WRITE);
+	igt_assert(*(uint32_t *)ptr == 0);
+
+	switch (fb->drm_format) {
+	case DRM_FORMAT_NV12:
+		memset(ptr + fb->offsets[0],
+		       full_range ? 0x00 : 0x10,
+		       fb->strides[0] * fb->plane_height[0]);
+		memset(ptr + fb->offsets[1],
+		       0x80,
+		       fb->strides[1] * fb->plane_height[1]);
+		break;
+	case DRM_FORMAT_YUYV:
+	case DRM_FORMAT_YVYU:
+		wmemset(ptr + fb->offsets[0],
+			full_range ? 0x80008000 : 0x80108010,
+			fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
+		break;
+	case DRM_FORMAT_UYVY:
+	case DRM_FORMAT_VYUY:
+		wmemset(ptr + fb->offsets[0],
+			full_range ? 0x00800080 : 0x10801080,
+			fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
+		break;
+	}
+	gem_munmap(ptr, fb->size);
+
+	return fb->gem_handle;
+}
+
 /* helpers to create nice-looking framebuffers */
 static int create_bo_for_fb(struct igt_fb *fb)
 {
@@ -470,48 +516,7 @@ static int create_bo_for_fb(struct igt_fb *fb)
 		fb->is_dumb = false;
 
 		if (is_i915_device(fd)) {
-			void *ptr;
-			bool full_range = fb->color_range == IGT_COLOR_YCBCR_FULL_RANGE;
-
-			fb->gem_handle = gem_create(fd, fb->size);
-
-			gem_set_tiling(fd, fb->gem_handle,
-				       igt_fb_mod_to_tiling(fb->tiling),
-				       fb->strides[0]);
-
-			gem_set_domain(fd, fb->gem_handle,
-				       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
-
-			/* Ensure the framebuffer is preallocated */
-			ptr = gem_mmap__gtt(fd, fb->gem_handle,
-					    fb->size, PROT_READ | PROT_WRITE);
-			igt_assert(*(uint32_t *)ptr == 0);
-
-			switch (fb->drm_format) {
-			case DRM_FORMAT_NV12:
-				memset(ptr + fb->offsets[0],
-				       full_range ? 0x00 : 0x10,
-				       fb->strides[0] * fb->plane_height[0]);
-				memset(ptr + fb->offsets[1],
-				       0x80,
-				       fb->strides[1] * fb->plane_height[1]);
-				break;
-			case DRM_FORMAT_YUYV:
-			case DRM_FORMAT_YVYU:
-				wmemset(ptr + fb->offsets[0],
-					full_range ? 0x80008000 : 0x80108010,
-					fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
-				break;
-			case DRM_FORMAT_UYVY:
-			case DRM_FORMAT_VYUY:
-				wmemset(ptr + fb->offsets[0],
-					full_range ? 0x00800080 : 0x10801080,
-					fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
-				break;
-			}
-			gem_munmap(ptr, fb->size);
-
-			return fb->gem_handle;
+			return i915_create_gem_for_fb(fb);
 		} else {
 			bool driver_has_gem_api = false;
 
-- 
2.19.1

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

[igt-dev] [PATCH i-g-t 6/8] igt: fb: Separate YUV allocation from the rest

[Maxime Ripard]

Allocating YUV buffers is a different operation than allocating
buffers with non-trivial setups.

Let's separate the YUV buffer allocation to a case and function of its
own.

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

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index d96689aa5fe6..d6242a6652f1 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -499,12 +499,30 @@ static int i915_create_gem_for_fb(struct igt_fb *fb)
 	return fb->gem_handle;
 }
 
+static int create_yuv_bo_for_fb(struct igt_fb *fb)
+{
+	uint64_t size = calc_fb_size(fb);
+	int fd = fb->fd;
+
+	/* respect the size requested by the caller */
+	if (fb->size == 0)
+		fb->size = size;
+
+	if (is_i915_device(fd))
+		return i915_create_gem_for_fb(fb);
+
+	/* We cannot allocate any other buffer type */
+	igt_assert(true);
+}
+
 /* helpers to create nice-looking framebuffers */
 static int create_bo_for_fb(struct igt_fb *fb)
 {
 	int fd = fb->fd;
 
-	if (fb->tiling || fb->size || fb->strides[0] || igt_format_is_yuv(fb->drm_format)) {
+	if (igt_format_is_yuv(fb->drm_format)) {
+		return create_yuv_bo_for_fb(fb);
+	} else if (fb->tiling || fb->size || fb->strides[0]) {
 		uint64_t size;
 
 		size = calc_fb_size(fb);
-- 
2.19.1

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

[igt-dev] [PATCH i-g-t 7/8] igt: fb: Fallback on KMS dumb buffer allocation for YUV buffers

[Maxime Ripard]

The current YUV buffer allocation only works on the i915 driver, since
it uses some private ioctl. However, we can to use that code on other
drivers that implement only KMS, so if the driver is something else
than the i915 driver, let's allocate a dumb buffer.

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

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index d6242a6652f1..f2e6c89f3884 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -501,6 +501,8 @@ static int i915_create_gem_for_fb(struct igt_fb *fb)
 
 static int create_yuv_bo_for_fb(struct igt_fb *fb)
 {
+	unsigned int virtual_height;
+	unsigned int bpp;
 	uint64_t size = calc_fb_size(fb);
 	int fd = fb->fd;
 
@@ -511,8 +513,37 @@ static int create_yuv_bo_for_fb(struct igt_fb *fb)
 	if (is_i915_device(fd))
 		return i915_create_gem_for_fb(fb);
 
-	/* We cannot allocate any other buffer type */
-	igt_assert(true);
+	switch (fb->drm_format) {
+	case DRM_FORMAT_NV12:
+		bpp = 8;
+		break;
+
+	case DRM_FORMAT_UYVY:
+	case DRM_FORMAT_VYUY:
+	case DRM_FORMAT_YUYV:
+	case DRM_FORMAT_YVYU:
+		bpp = 16;
+		break;
+
+	default:
+		igt_assert_f(false, "Unsupported YUV format\n");
+	}
+
+	switch (fb->drm_format) {
+	case DRM_FORMAT_NV12:
+		virtual_height = fb->height * 3 / 2;
+		break;
+
+	default:
+		virtual_height = fb->height;
+		break;
+	}
+
+	fb->is_dumb = true;
+	fb->gem_handle = kmstest_dumb_create(fd, fb->width, virtual_height,
+					     bpp, NULL, &fb->size);
+
+	return fb->gem_handle;
 }
 
 /* helpers to create nice-looking framebuffers */
-- 
2.19.1

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

[igt-dev] [PATCH 7/8] igt: fb: Fallback on KMS dumb buffer allocation for YUV buffers

[Maxime Ripard]

The current YUV buffer allocation only works on the i915 driver, since
it uses some private ioctl. However, we can to use that code on other
drivers that implement only KMS, so if the driver is something else
than the i915 driver, let's allocate a dumb buffer.

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

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index d6242a6652f1..f2e6c89f3884 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -501,6 +501,8 @@ static int i915_create_gem_for_fb(struct igt_fb *fb)
 
 static int create_yuv_bo_for_fb(struct igt_fb *fb)
 {
+	unsigned int virtual_height;
+	unsigned int bpp;
 	uint64_t size = calc_fb_size(fb);
 	int fd = fb->fd;
 
@@ -511,8 +513,37 @@ static int create_yuv_bo_for_fb(struct igt_fb *fb)
 	if (is_i915_device(fd))
 		return i915_create_gem_for_fb(fb);
 
-	/* We cannot allocate any other buffer type */
-	igt_assert(true);
+	switch (fb->drm_format) {
+	case DRM_FORMAT_NV12:
+		bpp = 8;
+		break;
+
+	case DRM_FORMAT_UYVY:
+	case DRM_FORMAT_VYUY:
+	case DRM_FORMAT_YUYV:
+	case DRM_FORMAT_YVYU:
+		bpp = 16;
+		break;
+
+	default:
+		igt_assert_f(false, "Unsupported YUV format\n");
+	}
+
+	switch (fb->drm_format) {
+	case DRM_FORMAT_NV12:
+		virtual_height = fb->height * 3 / 2;
+		break;
+
+	default:
+		virtual_height = fb->height;
+		break;
+	}
+
+	fb->is_dumb = true;
+	fb->gem_handle = kmstest_dumb_create(fd, fb->width, virtual_height,
+					     bpp, NULL, &fb->size);
+
+	return fb->gem_handle;
 }
 
 /* helpers to create nice-looking framebuffers */
-- 
git-series 0.9.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 8/8] igt: tests: chamelium: Add NV12 format

[Maxime Ripard]

The NV12 is a YUV format that are currently supported in IGT.

We'll want to test those formats in addition to the RGB formats, so
let's add some subtests. One thing worth noting is some hardware isn't
able to output a pixel-perfect image, so we do the same kind of
comparison than for VGA.

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

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 80b279c4f1c2..8a9f6bfe9235 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -976,6 +976,10 @@ igt_main
 			test_display_one_mode(&data, port, DRM_FORMAT_XRGB1555,
 					      CHAMELIUM_CHECK_CRC, 1);
 
+		connector_subtest("hdmi-crc-nv12", HDMIA)
+			test_display_one_mode(&data, port, DRM_FORMAT_NV12,
+					      CHAMELIUM_CHECK_ANALOG, 1);
+
 		connector_subtest("hdmi-frame-dump", HDMIA)
 			test_display_frame_dump(&data, port);
 	}
-- 
2.19.1

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

Re: [igt-dev] [PATCH 5/8] igt: fb: Move i915 buffer allocation to a function of its own

[Ville Syrjälä]

On Tue, Dec 04, 2018 at 11:08:19AM +0100, Maxime Ripard wrote:
> The create_bo_for_fb function has a rather big, non trivial block of
> code to allocate buffers in some non-trivial situation on the i915
> driver.
> 
> Since this is both non-trivial and driver specific, move that code
> into a function of its own so that we can rework that function later
> on.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  lib/igt_fb.c | 89 +++++++++++++++++++++++++++--------------------------
>  1 file changed, 47 insertions(+), 42 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 2462d6ba6fab..d96689aa5fe6 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -453,6 +453,52 @@ uint64_t igt_fb_tiling_to_mod(uint64_t tiling)
>  	}
>  }
>  
> +static int i915_create_gem_for_fb(struct igt_fb *fb)
> +{
> +	bool full_range = fb->color_range == IGT_COLOR_YCBCR_FULL_RANGE;
> +	int fd = fb->fd;
> +	void *ptr;
> +
> +	fb->gem_handle = gem_create(fd, fb->size);
> +	gem_set_tiling(fd, fb->gem_handle,
> +		       igt_fb_mod_to_tiling(fb->tiling),
> +		       fb->strides[0]);
> +
> +	gem_set_domain(fd, fb->gem_handle,
> +		       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
> +
> +	/* Ensure the framebuffer is preallocated */
> +	ptr = gem_mmap__gtt(fd, fb->gem_handle,
> +			    fb->size, PROT_READ | PROT_WRITE);
> +	igt_assert(*(uint32_t *)ptr == 0);
> +
> +	switch (fb->drm_format) {
> +	case DRM_FORMAT_NV12:
> +		memset(ptr + fb->offsets[0],
> +		       full_range ? 0x00 : 0x10,
> +		       fb->strides[0] * fb->plane_height[0]);
> +		memset(ptr + fb->offsets[1],
> +		       0x80,
> +		       fb->strides[1] * fb->plane_height[1]);
> +		break;
> +	case DRM_FORMAT_YUYV:
> +	case DRM_FORMAT_YVYU:
> +		wmemset(ptr + fb->offsets[0],
> +			full_range ? 0x80008000 : 0x80108010,
> +			fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
> +		break;
> +	case DRM_FORMAT_UYVY:
> +	case DRM_FORMAT_VYUY:
> +		wmemset(ptr + fb->offsets[0],
> +			full_range ? 0x00800080 : 0x10801080,
> +			fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
> +		break;
> +	}
> +	gem_munmap(ptr, fb->size);

We should probably extract this memset() stuff into a separate
function because every driver will require it.

> +
> +	return fb->gem_handle;
> +}
> +
>  /* helpers to create nice-looking framebuffers */
>  static int create_bo_for_fb(struct igt_fb *fb)
>  {
> @@ -470,48 +516,7 @@ static int create_bo_for_fb(struct igt_fb *fb)
>  		fb->is_dumb = false;
>  
>  		if (is_i915_device(fd)) {
> -			void *ptr;
> -			bool full_range = fb->color_range == IGT_COLOR_YCBCR_FULL_RANGE;
> -
> -			fb->gem_handle = gem_create(fd, fb->size);
> -
> -			gem_set_tiling(fd, fb->gem_handle,
> -				       igt_fb_mod_to_tiling(fb->tiling),
> -				       fb->strides[0]);
> -
> -			gem_set_domain(fd, fb->gem_handle,
> -				       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
> -
> -			/* Ensure the framebuffer is preallocated */
> -			ptr = gem_mmap__gtt(fd, fb->gem_handle,
> -					    fb->size, PROT_READ | PROT_WRITE);
> -			igt_assert(*(uint32_t *)ptr == 0);
> -
> -			switch (fb->drm_format) {
> -			case DRM_FORMAT_NV12:
> -				memset(ptr + fb->offsets[0],
> -				       full_range ? 0x00 : 0x10,
> -				       fb->strides[0] * fb->plane_height[0]);
> -				memset(ptr + fb->offsets[1],
> -				       0x80,
> -				       fb->strides[1] * fb->plane_height[1]);
> -				break;
> -			case DRM_FORMAT_YUYV:
> -			case DRM_FORMAT_YVYU:
> -				wmemset(ptr + fb->offsets[0],
> -					full_range ? 0x80008000 : 0x80108010,
> -					fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
> -				break;
> -			case DRM_FORMAT_UYVY:
> -			case DRM_FORMAT_VYUY:
> -				wmemset(ptr + fb->offsets[0],
> -					full_range ? 0x00800080 : 0x10801080,
> -					fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
> -				break;
> -			}
> -			gem_munmap(ptr, fb->size);
> -
> -			return fb->gem_handle;
> +			return i915_create_gem_for_fb(fb);
>  		} else {
>  			bool driver_has_gem_api = false;
>  
> -- 
> git-series 0.9.1

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

Re: [igt-dev] [PATCH i-g-t 7/8] igt: fb: Fallback on KMS dumb buffer allocation for YUV buffers

[Ville Syrjälä]

On Tue, Dec 04, 2018 at 11:08:22AM +0100, Maxime Ripard wrote:
> The current YUV buffer allocation only works on the i915 driver, since
> it uses some private ioctl. However, we can to use that code on other
> drivers that implement only KMS, so if the driver is something else
> than the i915 driver, let's allocate a dumb buffer.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  lib/igt_fb.c | 35 +++++++++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index d6242a6652f1..f2e6c89f3884 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -501,6 +501,8 @@ static int i915_create_gem_for_fb(struct igt_fb *fb)
>  
>  static int create_yuv_bo_for_fb(struct igt_fb *fb)
>  {
> +	unsigned int virtual_height;
> +	unsigned int bpp;
>  	uint64_t size = calc_fb_size(fb);
>  	int fd = fb->fd;
>  
> @@ -511,8 +513,37 @@ static int create_yuv_bo_for_fb(struct igt_fb *fb)
>  	if (is_i915_device(fd))
>  		return i915_create_gem_for_fb(fb);
>  
> -	/* We cannot allocate any other buffer type */
> -	igt_assert(true);
> +	switch (fb->drm_format) {
> +	case DRM_FORMAT_NV12:
> +		bpp = 8;
> +		break;
> +
> +	case DRM_FORMAT_UYVY:
> +	case DRM_FORMAT_VYUY:
> +	case DRM_FORMAT_YUYV:
> +	case DRM_FORMAT_YVYU:
> +		bpp = 16;
> +		break;
> +
> +	default:
> +		igt_assert_f(false, "Unsupported YUV format\n");
> +	}
> +
> +	switch (fb->drm_format) {
> +	case DRM_FORMAT_NV12:
> +		virtual_height = fb->height * 3 / 2;
> +		break;

A bit of a hack. The main problem with this is that the kernel won't
know anything about any stride alignment limitations etc. If we do use
this hack I guess it would be more technically correct to write it as
DIV_ROUND_UP(fb->size, fb->stride[0]), or something along those lines.

Which also brings me to the other issue we have. calc_plane_stride()
and calc_plane_size() are somewhat i915 specific. We should probably
do something about that.

> +
> +	default:
> +		virtual_height = fb->height;
> +		break;
> +	}
> +
> +	fb->is_dumb = true;
> +	fb->gem_handle = kmstest_dumb_create(fd, fb->width, virtual_height,
> +					     bpp, NULL, &fb->size);
> +
> +	return fb->gem_handle;
>  }
>  
>  /* helpers to create nice-looking framebuffers */
> -- 
> 2.19.1

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

Re: [igt-dev] [PATCH i-g-t 7/8] igt: fb: Fallback on KMS dumb buffer allocation for YUV buffers

[Maxime Ripard]

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

On Tue, Dec 04, 2018 at 09:32:05PM +0200, Ville Syrjälä wrote:
> On Tue, Dec 04, 2018 at 11:08:22AM +0100, Maxime Ripard wrote:
> > The current YUV buffer allocation only works on the i915 driver, since
> > it uses some private ioctl. However, we can to use that code on other
> > drivers that implement only KMS, so if the driver is something else
> > than the i915 driver, let's allocate a dumb buffer.
> > 
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> >  lib/igt_fb.c | 35 +++++++++++++++++++++++++++++++++--
> >  1 file changed, 33 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index d6242a6652f1..f2e6c89f3884 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -501,6 +501,8 @@ static int i915_create_gem_for_fb(struct igt_fb *fb)
> >  
> >  static int create_yuv_bo_for_fb(struct igt_fb *fb)
> >  {
> > +	unsigned int virtual_height;
> > +	unsigned int bpp;
> >  	uint64_t size = calc_fb_size(fb);
> >  	int fd = fb->fd;
> >  
> > @@ -511,8 +513,37 @@ static int create_yuv_bo_for_fb(struct igt_fb *fb)
> >  	if (is_i915_device(fd))
> >  		return i915_create_gem_for_fb(fb);
> >  
> > -	/* We cannot allocate any other buffer type */
> > -	igt_assert(true);
> > +	switch (fb->drm_format) {
> > +	case DRM_FORMAT_NV12:
> > +		bpp = 8;
> > +		break;
> > +
> > +	case DRM_FORMAT_UYVY:
> > +	case DRM_FORMAT_VYUY:
> > +	case DRM_FORMAT_YUYV:
> > +	case DRM_FORMAT_YVYU:
> > +		bpp = 16;
> > +		break;
> > +
> > +	default:
> > +		igt_assert_f(false, "Unsupported YUV format\n");
> > +	}
> > +
> > +	switch (fb->drm_format) {
> > +	case DRM_FORMAT_NV12:
> > +		virtual_height = fb->height * 3 / 2;
> > +		break;
> 
> A bit of a hack. The main problem with this is that the kernel won't
> know anything about any stride alignment limitations etc. If we do use
> this hack I guess it would be more technically correct to write it as
> DIV_ROUND_UP(fb->size, fb->stride[0]), or something along those lines.

It's the same algorithm that modetest is using, which is why I went
for that. It must have been tested on a wide variety of platforms.

> Which also brings me to the other issue we have. calc_plane_stride()
> and calc_plane_size() are somewhat i915 specific. We should probably
> do something about that.

Ah, right, I missed that.

When will we start to put some intel specific code in a generic code
path?

Maxime

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

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

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

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

Re: [igt-dev] [PATCH i-g-t 7/8] igt: fb: Fallback on KMS dumb buffer allocation for YUV buffers

[Ville Syrjälä]

On Fri, Dec 07, 2018 at 04:29:54PM +0100, Maxime Ripard wrote:
> On Tue, Dec 04, 2018 at 09:32:05PM +0200, Ville Syrjälä wrote:
> > On Tue, Dec 04, 2018 at 11:08:22AM +0100, Maxime Ripard wrote:
> > > The current YUV buffer allocation only works on the i915 driver, since
> > > it uses some private ioctl. However, we can to use that code on other
> > > drivers that implement only KMS, so if the driver is something else
> > > than the i915 driver, let's allocate a dumb buffer.
> > > 
> > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > > ---
> > >  lib/igt_fb.c | 35 +++++++++++++++++++++++++++++++++--
> > >  1 file changed, 33 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > index d6242a6652f1..f2e6c89f3884 100644
> > > --- a/lib/igt_fb.c
> > > +++ b/lib/igt_fb.c
> > > @@ -501,6 +501,8 @@ static int i915_create_gem_for_fb(struct igt_fb *fb)
> > >  
> > >  static int create_yuv_bo_for_fb(struct igt_fb *fb)
> > >  {
> > > +	unsigned int virtual_height;
> > > +	unsigned int bpp;
> > >  	uint64_t size = calc_fb_size(fb);
> > >  	int fd = fb->fd;
> > >  
> > > @@ -511,8 +513,37 @@ static int create_yuv_bo_for_fb(struct igt_fb *fb)
> > >  	if (is_i915_device(fd))
> > >  		return i915_create_gem_for_fb(fb);
> > >  
> > > -	/* We cannot allocate any other buffer type */
> > > -	igt_assert(true);
> > > +	switch (fb->drm_format) {
> > > +	case DRM_FORMAT_NV12:
> > > +		bpp = 8;
> > > +		break;
> > > +
> > > +	case DRM_FORMAT_UYVY:
> > > +	case DRM_FORMAT_VYUY:
> > > +	case DRM_FORMAT_YUYV:
> > > +	case DRM_FORMAT_YVYU:
> > > +		bpp = 16;
> > > +		break;
> > > +
> > > +	default:
> > > +		igt_assert_f(false, "Unsupported YUV format\n");
> > > +	}
> > > +
> > > +	switch (fb->drm_format) {
> > > +	case DRM_FORMAT_NV12:
> > > +		virtual_height = fb->height * 3 / 2;
> > > +		break;
> > 
> > A bit of a hack. The main problem with this is that the kernel won't
> > know anything about any stride alignment limitations etc. If we do use
> > this hack I guess it would be more technically correct to write it as
> > DIV_ROUND_UP(fb->size, fb->stride[0]), or something along those lines.
> 
> It's the same algorithm that modetest is using, which is why I went
> for that. It must have been tested on a wide variety of platforms.

People actually use modetest?

Anyways, I guess it's sort of correct in the sense that we override
fb->strides[0] and fb->size based on the data returned from the
crerate_dumb ioctl. Though DIV_ROUND_UP(height * 3, 2) would
seem like a sane idea anyway.

But what's missing is the handling for the nv12 chroma plane. We'd
probably want to rewrite offsets[1] and strides[1] as well. Although
we'll still be totally guessing since the kernel wasn't told that
we want NV12 and so there's is no guarantee that it'll accept the
resulting fb.

> 
> > Which also brings me to the other issue we have. calc_plane_stride()
> > and calc_plane_size() are somewhat i915 specific. We should probably
> > do something about that.
> 
> Ah, right, I missed that.
> 
> When will we start to put some intel specific code in a generic code
> path?

Do you mean stop? Hopefully we've already stopped.

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