[igt-dev] [PATCH i-g-t v6 00/10] igt: chamelium: Test YUV buffers using the Chamelium

[igt-dev] [PATCH i-g-t v6 00/10] igt: chamelium: Test YUV buffers using the Chamelium

[Maxime Ripard]

Hi,

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

It builds on top of the current infrastructure for the YUV convertions, and
the format suppport of the Chamelium. It reworks igt_fb quite significantly
to add generic support for YUV formats, and adds a bunch of new formats
along the way.

Let me know what you think,
Maxime

Changes from v5:
  - Fixed breakage on x86 platforms
  - Fixed typo in a comment
  - Split the size and strides check in a separate patch
  - Added the various tags from Lyude and Paul
  - Rebased on master

Changes from v4:
  - Added Paul Reviewed-by's
  - Removed the patch that moves calc_fb_size for everybody, and added some
    asserts

Changes from v3:
  - Fixed build breakage

Changes from v2:
  - Added Paul Reviewed-by's
  - Fixed a stride issue on the buffer allocation
  - Take into account the plane bpp so that we don't overallocate
  - Rebased on master

Changes from v1:
  - Support more YUV formats than just NV12
  - Rework the YUV convertion functions to add generic routines
  - Took the comments into account.

Maxime Ripard (10):
  igt: fb: Refactor dumb buffer allocation path
  igt: fb: Add size checks and recalculation before dumb allocation
  igt: fb: Account for all planes bpp
  igt: fb: Don't pass the stride when allocating a dumb, multi-planar
    buffer
  igt: fb: Clear YUV dumb buffers
  igt: fb: Rework YUV i915 allocation path
  igt: fb: Add a bunch of new YUV formats
  igt: tests: chamelium: Start to unify tests
  igt: tests: chamelium: Convert VGA tests to do_test_display
  igt: tests: chamelium: Add YUV formats tests

 lib/igt_fb.c          | 175 ++++++++++++++++++++++++-----
 tests/kms_chamelium.c | 254 ++++++++++++++++++++++--------------------
 2 files changed, 282 insertions(+), 147 deletions(-)

-- 
2.20.1

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

[igt-dev] [PATCH i-g-t v6 01/10] igt: fb: Refactor dumb buffer allocation path

[Maxime Ripard]

The else condition is not needed, since all the other conditions return
when they are done.

Move the KMS dumb buffer allocation outside of the outer else condition,
this will also allow to ease later changes.

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

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 5d8c187a9ac9..c61e6805ff7a 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -559,15 +559,14 @@ static int create_bo_for_fb(struct igt_fb *fb)
 			igt_require(driver_has_gem_api);
 			return -EINVAL;
 		}
-	} else {
-		fb->is_dumb = true;
-
-		fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
-						     fb->plane_bpp[0],
-						     &fb->strides[0], &fb->size);
-
-		return fb->gem_handle;
 	}
+
+	fb->is_dumb = true;
+	fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
+					     fb->plane_bpp[0],
+					     &fb->strides[0], &fb->size);
+
+	return fb->gem_handle;
 }
 
 /**
-- 
2.20.1

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

[igt-dev] [PATCH i-g-t v6 02/10] igt: fb: Add size checks and recalculation before dumb allocation

[Maxime Ripard]

Since we want to use the dumb buffers to store frames in formats with
multiple planes, we need to add checks for unsupported corner cases, and we
need to calculate the offsets and sizes of each planes. Let's use
calc_fb_size for that.

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

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index c61e6805ff7a..ca4865519c13 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -561,6 +561,16 @@ static int create_bo_for_fb(struct igt_fb *fb)
 		}
 	}
 
+	/*
+	 * The current dumb buffer allocation API doesn't really allow to
+	 * specify a custom size or stride. Yet the caller is free to specify
+	 * them, so we need to make sure to error out in this case.
+	 */
+	igt_assert(fb->size == 0);
+	igt_assert(fb->strides[0] == 0);
+
+	fb->size = calc_fb_size(fb);
+
 	fb->is_dumb = true;
 	fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
 					     fb->plane_bpp[0],
-- 
2.20.1

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

[igt-dev] [PATCH i-g-t v6 03/10] igt: fb: Account for all planes bpp

[Maxime Ripard]

When allocating a dumb buffer for a format with multiple plane, we need to
account for all plane's bpp in order to allocate the proper size.

Let's add all the planes bpp and use the result to allocate our buffer.

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_fb.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index ca4865519c13..ca19c034f4a3 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -528,6 +528,9 @@ static void clear_yuv_buffer(struct igt_fb *fb)
 /* helpers to create nice-looking framebuffers */
 static int create_bo_for_fb(struct igt_fb *fb)
 {
+	const struct format_desc_struct *fmt = lookup_drm_format(fb->drm_format);
+	unsigned int bpp = 0;
+	unsigned int plane;
 	int fd = fb->fd;
 
 	if (fb->tiling || fb->size || fb->strides[0] || igt_format_is_yuv(fb->drm_format)) {
@@ -570,11 +573,13 @@ static int create_bo_for_fb(struct igt_fb *fb)
 	igt_assert(fb->strides[0] == 0);
 
 	fb->size = calc_fb_size(fb);
+	for (plane = 0; plane < fb->num_planes; plane++)
+		bpp += DIV_ROUND_UP(fb->plane_bpp[plane],
+				    plane ? fmt->hsub * fmt->vsub : 1);
 
 	fb->is_dumb = true;
 	fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
-					     fb->plane_bpp[0],
-					     &fb->strides[0], &fb->size);
+					     bpp, &fb->strides[0], &fb->size);
 
 	return fb->gem_handle;
 }
-- 
2.20.1

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

[igt-dev] [PATCH i-g-t v6 04/10] igt: fb: Don't pass the stride when allocating a dumb, multi-planar buffer

[Maxime Ripard]

The dumb buffer allocation API only considers a single plane, and even
though allocating multi-planar buffers through it is allowed, the stride it
gives back is the the width times the bpp passed as an argument.

That doesn't work in our case, since the bpp is going to be the one we give
as an argument, but split over three planes so the stride doesn't match
anymore.

A proper fix for this would be to have a better dumb buffer allocation API,
but for the time being, let's do it that way.

Reviewed-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_fb.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index ca19c034f4a3..dea390b062de 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -531,6 +531,7 @@ static int create_bo_for_fb(struct igt_fb *fb)
 	const struct format_desc_struct *fmt = lookup_drm_format(fb->drm_format);
 	unsigned int bpp = 0;
 	unsigned int plane;
+	unsigned *strides = &fb->strides[0];
 	int fd = fb->fd;
 
 	if (fb->tiling || fb->size || fb->strides[0] || igt_format_is_yuv(fb->drm_format)) {
@@ -578,8 +579,22 @@ static int create_bo_for_fb(struct igt_fb *fb)
 				    plane ? fmt->hsub * fmt->vsub : 1);
 
 	fb->is_dumb = true;
+
+	/*
+	 * We can't really pass the stride array here since the dumb
+	 * buffer allocation is assuming that it operates on one
+	 * plane, and therefore will calculate the stride as if each
+	 * pixel was stored on a single plane.
+	 *
+	 * This might cause issues at some point on drivers that would
+	 * change the stride of YUV buffers, but we haven't
+	 * encountered any yet.
+	 */
+	if (fb->num_planes > 1)
+		strides = NULL;
+
 	fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
-					     bpp, &fb->strides[0], &fb->size);
+					     bpp, strides, &fb->size);
 
 	return fb->gem_handle;
 }
-- 
2.20.1

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

[igt-dev] [PATCH i-g-t v6 05/10] igt: fb: Clear YUV dumb buffers

[Maxime Ripard]

YUV dumb buffers, just like i915 GEM buffers also need to be cleared once
allocated. Make sure it happens.

Reviewed-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_fb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index dea390b062de..6fe0d0847aef 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -596,6 +596,9 @@ static int create_bo_for_fb(struct igt_fb *fb)
 	fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
 					     bpp, strides, &fb->size);
 
+	if (igt_format_is_yuv(fb->drm_format))
+		clear_yuv_buffer(fb);
+
 	return fb->gem_handle;
 }
 
-- 
2.20.1

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

[igt-dev] [PATCH i-g-t v6 06/10] igt: fb: Rework YUV i915 allocation path

[Maxime Ripard]

We only need to allocate a buffer using the GEM API when we're in the
condition that we currently test, and that we're running on i915.

All the other cases can be handled by a fallback to a dumb buffer
allocation. Let's simplify the code a bit to reflect that.

Reviewed-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_fb.c | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 6fe0d0847aef..e9728c0c03f4 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -534,7 +534,8 @@ static int create_bo_for_fb(struct igt_fb *fb)
 	unsigned *strides = &fb->strides[0];
 	int fd = fb->fd;
 
-	if (fb->tiling || fb->size || fb->strides[0] || igt_format_is_yuv(fb->drm_format)) {
+	if (is_i915_device(fd) &&
+	   (fb->tiling || fb->size || fb->strides[0] || igt_format_is_yuv(fb->drm_format))) {
 		uint64_t size;
 
 		size = calc_fb_size(fb);
@@ -544,25 +545,12 @@ static int create_bo_for_fb(struct igt_fb *fb)
 			fb->size = size;
 
 		fb->is_dumb = false;
+		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]);
 
-		if (is_i915_device(fd)) {
-
-			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]);
-
-			if (igt_format_is_yuv(fb->drm_format))
-				clear_yuv_buffer(fb);
-
-			return fb->gem_handle;
-		} else {
-			bool driver_has_gem_api = false;
-
-			igt_require(driver_has_gem_api);
-			return -EINVAL;
-		}
+		goto out;
 	}
 
 	/*
@@ -596,6 +584,7 @@ static int create_bo_for_fb(struct igt_fb *fb)
 	fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
 					     bpp, strides, &fb->size);
 
+out:
 	if (igt_format_is_yuv(fb->drm_format))
 		clear_yuv_buffer(fb);
 
-- 
2.20.1

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

[igt-dev] [PATCH i-g-t v6 07/10] igt: fb: Add a bunch of new YUV formats

[Maxime Ripard]

Thanks to the previous reworks, we can now add new YUV formats pretty
easily. This patch adds support for the NV12, NV16, NV21, NV61, YUV420,
YVU420, YUV422 and YVU422 formats.

Reviewed-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_fb.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 98 insertions(+), 2 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index e9728c0c03f4..cad8bfeacebc 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -150,6 +150,21 @@ static const struct format_desc_struct {
 	  .num_planes = 2, .plane_bpp = { 8, 16, },
 	  .hsub = 2, .vsub = 2,
 	},
+	{ .name = "NV16", .depth = -1, .drm_id = DRM_FORMAT_NV16,
+	  .cairo_id = CAIRO_FORMAT_RGB24,
+	  .num_planes = 2, .plane_bpp = { 8, 16, },
+	  .hsub = 2, .vsub = 1,
+	},
+	{ .name = "NV21", .depth = -1, .drm_id = DRM_FORMAT_NV21,
+	  .cairo_id = CAIRO_FORMAT_RGB24,
+	  .num_planes = 2, .plane_bpp = { 8, 16, },
+	  .hsub = 2, .vsub = 2,
+	},
+	{ .name = "NV61", .depth = -1, .drm_id = DRM_FORMAT_NV61,
+	  .cairo_id = CAIRO_FORMAT_RGB24,
+	  .num_planes = 2, .plane_bpp = { 8, 16, },
+	  .hsub = 2, .vsub = 1,
+	},
 	{ .name = "YUYV", .depth = -1, .drm_id = DRM_FORMAT_YUYV,
 	  .cairo_id = CAIRO_FORMAT_RGB24,
 	  .num_planes = 1, .plane_bpp = { 16, },
@@ -170,6 +185,26 @@ static const struct format_desc_struct {
 	  .num_planes = 1, .plane_bpp = { 16, },
 	  .hsub = 2, .vsub = 1,
 	},
+	{ .name = "YU12", .depth = -1, .drm_id = DRM_FORMAT_YUV420,
+	  .cairo_id = CAIRO_FORMAT_RGB24,
+	  .num_planes = 3, .plane_bpp = { 8, 8, 8, },
+	  .hsub = 2, .vsub = 2,
+	},
+	{ .name = "YU16", .depth = -1, .drm_id = DRM_FORMAT_YUV422,
+	  .cairo_id = CAIRO_FORMAT_RGB24,
+	  .num_planes = 3, .plane_bpp = { 8, 8, 8, },
+	  .hsub = 2, .vsub = 1,
+	},
+	{ .name = "YV12", .depth = -1, .drm_id = DRM_FORMAT_YVU420,
+	  .cairo_id = CAIRO_FORMAT_RGB24,
+	  .num_planes = 3, .plane_bpp = { 8, 8, 8, },
+	  .hsub = 2, .vsub = 2,
+	},
+	{ .name = "YV16", .depth = -1, .drm_id = DRM_FORMAT_YVU422,
+	  .cairo_id = CAIRO_FORMAT_RGB24,
+	  .num_planes = 3, .plane_bpp = { 8, 8, 8, },
+	  .hsub = 2, .vsub = 1,
+	},
 };
 #define for_each_format(f)	\
 	for (f = format_desc; f - format_desc < ARRAY_SIZE(format_desc); f++)
@@ -1620,10 +1655,21 @@ static void get_yuv_parameters(struct igt_fb *fb, struct yuv_parameters *params)
 
 	switch (fb->drm_format) {
 	case DRM_FORMAT_NV12:
+	case DRM_FORMAT_NV16:
+	case DRM_FORMAT_NV21:
+	case DRM_FORMAT_NV61:
 		params->y_inc = 1;
 		params->uv_inc = 2;
 		break;
 
+	case DRM_FORMAT_YUV420:
+	case DRM_FORMAT_YUV422:
+	case DRM_FORMAT_YVU420:
+	case DRM_FORMAT_YVU422:
+		params->y_inc = 1;
+		params->uv_inc = 1;
+		break;
+
 	case DRM_FORMAT_YUYV:
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_UYVY:
@@ -1640,6 +1686,13 @@ static void get_yuv_parameters(struct igt_fb *fb, struct yuv_parameters *params)
 
 	switch (fb->drm_format) {
 	case DRM_FORMAT_NV12:
+	case DRM_FORMAT_NV16:
+	case DRM_FORMAT_NV21:
+	case DRM_FORMAT_NV61:
+	case DRM_FORMAT_YUV420:
+	case DRM_FORMAT_YUV422:
+	case DRM_FORMAT_YVU420:
+	case DRM_FORMAT_YVU422:
 		params->y_stride = fb->strides[0];
 		params->uv_stride = fb->strides[1];
 		break;
@@ -1656,11 +1709,33 @@ static void get_yuv_parameters(struct igt_fb *fb, struct yuv_parameters *params)
 
 	switch (fb->drm_format) {
 	case DRM_FORMAT_NV12:
+	case DRM_FORMAT_NV16:
 		params->y_offset = fb->offsets[0];
 		params->u_offset = fb->offsets[1];
 		params->v_offset = fb->offsets[1] + 1;
 		break;
 
+	case DRM_FORMAT_NV21:
+	case DRM_FORMAT_NV61:
+		params->y_offset = fb->offsets[0];
+		params->u_offset = fb->offsets[1] + 1;
+		params->v_offset = fb->offsets[1];
+		break;
+
+	case DRM_FORMAT_YUV420:
+	case DRM_FORMAT_YUV422:
+		params->y_offset = fb->offsets[0];
+		params->u_offset = fb->offsets[1];
+		params->v_offset = fb->offsets[2];
+		break;
+
+	case DRM_FORMAT_YVU420:
+	case DRM_FORMAT_YVU422:
+		params->y_offset = fb->offsets[0];
+		params->u_offset = fb->offsets[2];
+		params->v_offset = fb->offsets[1];
+		break;
+
 	case DRM_FORMAT_YUYV:
 		params->y_offset = fb->offsets[0];
 		params->u_offset = fb->offsets[0] + 1;
@@ -1887,9 +1962,16 @@ static void fb_convert(struct fb_convert *cvt)
 		switch (cvt->src.fb->drm_format) {
 		case DRM_FORMAT_XYUV8888:
 		case DRM_FORMAT_NV12:
+		case DRM_FORMAT_NV16:
+		case DRM_FORMAT_NV21:
+		case DRM_FORMAT_NV61:
 		case DRM_FORMAT_UYVY:
 		case DRM_FORMAT_VYUY:
+		case DRM_FORMAT_YUV420:
+		case DRM_FORMAT_YUV422:
 		case DRM_FORMAT_YUYV:
+		case DRM_FORMAT_YVU420:
+		case DRM_FORMAT_YVU422:
 		case DRM_FORMAT_YVYU:
 			convert_yuv_to_rgb24(cvt);
 			return;
@@ -1898,10 +1980,17 @@ static void fb_convert(struct fb_convert *cvt)
 		switch (cvt->dst.fb->drm_format) {
 		case DRM_FORMAT_XYUV8888:
 		case DRM_FORMAT_NV12:
-		case DRM_FORMAT_YUYV:
-		case DRM_FORMAT_YVYU:
+		case DRM_FORMAT_NV16:
+		case DRM_FORMAT_NV21:
+		case DRM_FORMAT_NV61:
 		case DRM_FORMAT_UYVY:
 		case DRM_FORMAT_VYUY:
+		case DRM_FORMAT_YUV420:
+		case DRM_FORMAT_YUV422:
+		case DRM_FORMAT_YUYV:
+		case DRM_FORMAT_YVU420:
+		case DRM_FORMAT_YVU422:
+		case DRM_FORMAT_YVYU:
 			convert_rgb24_to_yuv(cvt);
 			return;
 		}
@@ -2248,6 +2337,13 @@ bool igt_format_is_yuv(uint32_t drm_format)
 {
 	switch (drm_format) {
 	case DRM_FORMAT_NV12:
+	case DRM_FORMAT_NV16:
+	case DRM_FORMAT_NV21:
+	case DRM_FORMAT_NV61:
+	case DRM_FORMAT_YUV420:
+	case DRM_FORMAT_YUV422:
+	case DRM_FORMAT_YVU420:
+	case DRM_FORMAT_YVU422:
 	case DRM_FORMAT_YUYV:
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_UYVY:
-- 
2.20.1

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

[igt-dev] [PATCH i-g-t v6 08/10] 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.

Reviewed-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
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 2d848c2f0620..8230cb852cfb 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -527,12 +527,15 @@ static int chamelium_get_pattern_fb(data_t *data, size_t width, size_t height,
 	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;
@@ -545,39 +548,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;
@@ -590,13 +600,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;
@@ -613,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, fourcc, count);
+		do_test_display(data, port, output, mode, fourcc, check, count);
 	}
 
 	drmModeFreeConnector(connector);
@@ -874,16 +886,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);
@@ -941,56 +953,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.20.1

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

[igt-dev] [PATCH i-g-t v6 09/10] 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.

Reviewed-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
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 8230cb852cfb..eaea9c18832a 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -528,6 +528,7 @@ static int chamelium_get_pattern_fb(data_t *data, size_t width, size_t height,
 }
 
 enum chamelium_check {
+	CHAMELIUM_CHECK_ANALOG,
 	CHAMELIUM_CHECK_CRC,
 };
 
@@ -579,6 +580,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);
@@ -589,8 +602,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);
@@ -600,8 +614,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);
 }
@@ -613,6 +633,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);
@@ -622,9 +643,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)
 	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)
 {
@@ -1041,7 +1014,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.20.1

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

[igt-dev] [PATCH i-g-t v6 10/10] igt: tests: chamelium: Add YUV formats tests

[Maxime Ripard]

The NV12, NV16, NV21, NV61, YUV420, YVU420, YUV422 and YVU422 are YUV
formats 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.

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

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index eaea9c18832a..64f87d3ae474 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -977,6 +977,38 @@ igt_main
 			test_display_one_mode(&data, port, DRM_FORMAT_XRGB1555,
 					      CHAMELIUM_CHECK_CRC, 1);
 
+		connector_subtest("hdmi-cmp-nv12", HDMIA)
+			test_display_one_mode(&data, port, DRM_FORMAT_NV12,
+					      CHAMELIUM_CHECK_ANALOG, 1);
+
+		connector_subtest("hdmi-cmp-nv16", HDMIA)
+			test_display_one_mode(&data, port, DRM_FORMAT_NV16,
+					      CHAMELIUM_CHECK_ANALOG, 1);
+
+		connector_subtest("hdmi-cmp-nv21", HDMIA)
+			test_display_one_mode(&data, port, DRM_FORMAT_NV21,
+					      CHAMELIUM_CHECK_ANALOG, 1);
+
+		connector_subtest("hdmi-cmp-nv61", HDMIA)
+			test_display_one_mode(&data, port, DRM_FORMAT_NV61,
+					      CHAMELIUM_CHECK_ANALOG, 1);
+
+		connector_subtest("hdmi-cmp-yu12", HDMIA)
+			test_display_one_mode(&data, port, DRM_FORMAT_YUV420,
+					      CHAMELIUM_CHECK_ANALOG, 1);
+
+		connector_subtest("hdmi-cmp-yu16", HDMIA)
+			test_display_one_mode(&data, port, DRM_FORMAT_YUV422,
+					      CHAMELIUM_CHECK_ANALOG, 1);
+
+		connector_subtest("hdmi-cmp-yv12", HDMIA)
+			test_display_one_mode(&data, port, DRM_FORMAT_YVU420,
+					      CHAMELIUM_CHECK_ANALOG, 1);
+
+		connector_subtest("hdmi-cmp-yv16", HDMIA)
+			test_display_one_mode(&data, port, DRM_FORMAT_YVU422,
+					      CHAMELIUM_CHECK_ANALOG, 1);
+
 		connector_subtest("hdmi-frame-dump", HDMIA)
 			test_display_frame_dump(&data, port);
 	}
-- 
2.20.1

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

[igt-dev] ✓ Fi.CI.BAT: success for igt: chamelium: Test YUV buffers using the Chamelium (rev8)

[Patchwork]

== Series Details ==

Series: igt: chamelium: Test YUV buffers using the Chamelium (rev8)
URL   : https://patchwork.freedesktop.org/series/53468/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5567 -> IGTPW_2364
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/53468/revisions/8/mbox/

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  * igt@kms_flip@basic-flip-vs-modeset:
    - fi-skl-6700hq:      PASS -> DMESG-WARN [fdo#105998]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#107362] +1

  * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@pm_rpm@module-reload:
    - fi-skl-6770hq:      PASS -> FAIL [fdo#108511]

  
#### Possible fixes ####

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         INCOMPLETE [fdo#103927] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#105998]: https://bugs.freedesktop.org/show_bug.cgi?id=105998
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511


Participating hosts (49 -> 44)
------------------------------

  Missing    (5): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-icl-y 


Build changes
-------------

    * IGT: IGT_4815 -> IGTPW_2364

  CI_DRM_5567: 5725867972123d46f4a4d382ed5fa2eb0348b00c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2364: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2364/
  IGT_4815: 947301563259726b65ce47d3a3fe37931ed42efe @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@kms_chamelium@hdmi-cmp-nv12
+igt@kms_chamelium@hdmi-cmp-nv16
+igt@kms_chamelium@hdmi-cmp-nv21
+igt@kms_chamelium@hdmi-cmp-nv61
+igt@kms_chamelium@hdmi-cmp-yu12
+igt@kms_chamelium@hdmi-cmp-yu16
+igt@kms_chamelium@hdmi-cmp-yv12
+igt@kms_chamelium@hdmi-cmp-yv16

== Logs ==

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

Re: [igt-dev] [PATCH i-g-t v6 02/10] igt: fb: Add size checks and recalculation before dumb allocation

[Paul Kocialkowski]

Hi,

On Fri, 2019-02-08 at 14:18 +0100, Maxime Ripard wrote:
> Since we want to use the dumb buffers to store frames in formats with
> multiple planes, we need to add checks for unsupported corner cases, and we
> need to calculate the offsets and sizes of each planes. Let's use
> calc_fb_size for that.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>

Looks good to me, thanks!

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> ---
>  lib/igt_fb.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index c61e6805ff7a..ca4865519c13 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -561,6 +561,16 @@ static int create_bo_for_fb(struct igt_fb *fb)
>  		}
>  	}
>  
> +	/*
> +	 * The current dumb buffer allocation API doesn't really allow to
> +	 * specify a custom size or stride. Yet the caller is free to specify
> +	 * them, so we need to make sure to error out in this case.
> +	 */
> +	igt_assert(fb->size == 0);
> +	igt_assert(fb->strides[0] == 0);
> +
> +	fb->size = calc_fb_size(fb);
> +
>  	fb->is_dumb = true;
>  	fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
>  					     fb->plane_bpp[0],
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

[igt-dev] ✓ Fi.CI.IGT: success for igt: chamelium: Test YUV buffers using the Chamelium (rev8)

[Patchwork]

== Series Details ==

Series: igt: chamelium: Test YUV buffers using the Chamelium (rev8)
URL   : https://patchwork.freedesktop.org/series/53468/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5567_full -> IGTPW_2364_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/53468/revisions/8/mbox/

New tests
---------

  New tests have been introduced between CI_DRM_5567_full and IGTPW_2364_full:
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-apl:          PASS -> FAIL [fdo#103191] / [fdo#103232]

  * igt@kms_cursor_crc@cursor-128x42-sliding:
    - shard-kbl:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-glk:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-256x85-onscreen:
    - shard-apl:          PASS -> FAIL [fdo#103232] +6

  * igt@kms_cursor_crc@cursor-size-change:
    - shard-glk:          PASS -> FAIL [fdo#103232]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
    - shard-apl:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
    - shard-glk:          PASS -> FAIL [fdo#103167] +11

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-glk:          PASS -> FAIL [fdo#108948]

  * igt@kms_plane@pixel-format-pipe-c-planes:
    - shard-apl:          PASS -> FAIL [fdo#103166] +3

  * igt@kms_plane@plane-position-covered-pipe-a-planes:
    - shard-glk:          PASS -> FAIL [fdo#103166] +3

  * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
    - shard-glk:          PASS -> FAIL [fdo#108145] +1

  
#### Possible fixes ####

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +3

  * igt@kms_cursor_crc@cursor-64x64-onscreen:
    - shard-kbl:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-apl:          FAIL [fdo#103191] / [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-alpha-opaque:
    - shard-apl:          FAIL [fdo#109350] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
    - shard-apl:          FAIL [fdo#103167] -> PASS +2

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-pwrite:
    - shard-glk:          FAIL [fdo#103167] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbc-farfromfence:
    - shard-snb:          DMESG-FAIL [fdo#107469] -> PASS

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-apl:          FAIL [fdo#108948] -> PASS

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-glk:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
    - shard-apl:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-glk:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - shard-kbl:          FAIL [fdo#103166] -> PASS +2

  * igt@kms_setmode@basic:
    - shard-kbl:          FAIL [fdo#99912] -> PASS

  
#### Warnings ####

  * igt@i915_suspend@shrink:
    - shard-kbl:          DMESG-WARN [fdo#109244] -> INCOMPLETE [fdo#103665] / [fdo#106886]

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

  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
  [fdo#107469]: https://bugs.freedesktop.org/show_bug.cgi?id=107469
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * IGT: IGT_4815 -> IGTPW_2364
    * Piglit: piglit_4509 -> None

  CI_DRM_5567: 5725867972123d46f4a4d382ed5fa2eb0348b00c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2364: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2364/
  IGT_4815: 947301563259726b65ce47d3a3fe37931ed42efe @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [igt-dev] [PATCH i-g-t v6 01/10] igt: fb: Refactor dumb buffer allocation path

[Deepak Singh Rawat]

Hi Maxime,

Thanks for doing this.

Reviewed-by: Deepak Rawat <drawat@vmware.com>

On Fri, 2019-02-08 at 14:18 +0100, Maxime Ripard wrote:
> The else condition is not needed, since all the other conditions
> return
> when they are done.
> 
> Move the KMS dumb buffer allocation outside of the outer else
> condition,
> this will also allow to ease later changes.
> 
> Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  lib/igt_fb.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 5d8c187a9ac9..c61e6805ff7a 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -559,15 +559,14 @@ static int create_bo_for_fb(struct igt_fb *fb)
>  			igt_require(driver_has_gem_api);
>  			return -EINVAL;
>  		}
> -	} else {
> -		fb->is_dumb = true;
> -
> -		fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb-
> >height,
> -						     fb->plane_bpp[0],
> -						     &fb->strides[0],
> &fb->size);
> -
> -		return fb->gem_handle;
>  	}
> +
> +	fb->is_dumb = true;
> +	fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
> +					     fb->plane_bpp[0],
> +					     &fb->strides[0], &fb-
> >size);
> +
> +	return fb->gem_handle;
>  }
>  
>  /**

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

Re: [igt-dev] [PATCH i-g-t v6 06/10] igt: fb: Rework YUV i915 allocation path

[Deepak Singh Rawat]

This looks good to me.

Reviewed-by: Deepak Rawat <drawat@vmware.com>

On Fri, 2019-02-08 at 14:18 +0100, Maxime Ripard wrote:
> We only need to allocate a buffer using the GEM API when we're in the
> condition that we currently test, and that we're running on i915.
> 
> All the other cases can be handled by a fallback to a dumb buffer
> allocation. Let's simplify the code a bit to reflect that.
> 
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  lib/igt_fb.c | 27 ++++++++-------------------
>  1 file changed, 8 insertions(+), 19 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 6fe0d0847aef..e9728c0c03f4 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -534,7 +534,8 @@ static int create_bo_for_fb(struct igt_fb *fb)
>  	unsigned *strides = &fb->strides[0];
>  	int fd = fb->fd;
>  
> -	if (fb->tiling || fb->size || fb->strides[0] ||
> igt_format_is_yuv(fb->drm_format)) {
> +	if (is_i915_device(fd) &&
> +	   (fb->tiling || fb->size || fb->strides[0] ||
> igt_format_is_yuv(fb->drm_format))) {
>  		uint64_t size;
>  
>  		size = calc_fb_size(fb);
> @@ -544,25 +545,12 @@ static int create_bo_for_fb(struct igt_fb *fb)
>  			fb->size = size;
>  
>  		fb->is_dumb = false;
> +		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]);
>  
> -		if (is_i915_device(fd)) {
> -
> -			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]);
> -
> -			if (igt_format_is_yuv(fb->drm_format))
> -				clear_yuv_buffer(fb);
> -
> -			return fb->gem_handle;
> -		} else {
> -			bool driver_has_gem_api = false;
> -
> -			igt_require(driver_has_gem_api);
> -			return -EINVAL;
> -		}
> +		goto out;
>  	}
>  
>  	/*
> @@ -596,6 +584,7 @@ static int create_bo_for_fb(struct igt_fb *fb)
>  	fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
>  					     bpp, strides, &fb->size);
>  
> +out:
>  	if (igt_format_is_yuv(fb->drm_format))
>  		clear_yuv_buffer(fb);
>  

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

Re: [igt-dev] [PATCH i-g-t v6 01/10] igt: fb: Refactor dumb buffer allocation path

[Lyude Paul]

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Fri, 2019-02-08 at 14:18 +0100, Maxime Ripard wrote:
> The else condition is not needed, since all the other conditions return
> when they are done.
> 
> Move the KMS dumb buffer allocation outside of the outer else condition,
> this will also allow to ease later changes.
> 
> Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  lib/igt_fb.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 5d8c187a9ac9..c61e6805ff7a 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -559,15 +559,14 @@ static int create_bo_for_fb(struct igt_fb *fb)
>  			igt_require(driver_has_gem_api);
>  			return -EINVAL;
>  		}
> -	} else {
> -		fb->is_dumb = true;
> -
> -		fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb-
> >height,
> -						     fb->plane_bpp[0],
> -						     &fb->strides[0], &fb-
> >size);
> -
> -		return fb->gem_handle;
>  	}
> +
> +	fb->is_dumb = true;
> +	fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
> +					     fb->plane_bpp[0],
> +					     &fb->strides[0], &fb->size);
> +
> +	return fb->gem_handle;
>  }
>  
>  /**
-- 
Cheers,
	Lyude Paul

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

Re: [igt-dev] [PATCH i-g-t v6 02/10] igt: fb: Add size checks and recalculation before dumb allocation

[Lyude Paul]

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Fri, 2019-02-08 at 14:18 +0100, Maxime Ripard wrote:
> Since we want to use the dumb buffers to store frames in formats with
> multiple planes, we need to add checks for unsupported corner cases, and we
> need to calculate the offsets and sizes of each planes. Let's use
> calc_fb_size for that.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  lib/igt_fb.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index c61e6805ff7a..ca4865519c13 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -561,6 +561,16 @@ static int create_bo_for_fb(struct igt_fb *fb)
>  		}
>  	}
>  
> +	/*
> +	 * The current dumb buffer allocation API doesn't really allow to
> +	 * specify a custom size or stride. Yet the caller is free to specify
> +	 * them, so we need to make sure to error out in this case.
> +	 */
> +	igt_assert(fb->size == 0);
> +	igt_assert(fb->strides[0] == 0);
> +
> +	fb->size = calc_fb_size(fb);
> +
>  	fb->is_dumb = true;
>  	fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
>  					     fb->plane_bpp[0],
-- 
Cheers,
	Lyude Paul

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

Re: [igt-dev] [PATCH i-g-t v6 03/10] igt: fb: Account for all planes bpp

[Lyude Paul]

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Fri, 2019-02-08 at 14:18 +0100, Maxime Ripard wrote:
> When allocating a dumb buffer for a format with multiple plane, we need to
> account for all plane's bpp in order to allocate the proper size.
> 
> Let's add all the planes bpp and use the result to allocate our buffer.
> 
> Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  lib/igt_fb.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index ca4865519c13..ca19c034f4a3 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -528,6 +528,9 @@ static void clear_yuv_buffer(struct igt_fb *fb)
>  /* helpers to create nice-looking framebuffers */
>  static int create_bo_for_fb(struct igt_fb *fb)
>  {
> +	const struct format_desc_struct *fmt = lookup_drm_format(fb-
> >drm_format);
> +	unsigned int bpp = 0;
> +	unsigned int plane;
>  	int fd = fb->fd;
>  
>  	if (fb->tiling || fb->size || fb->strides[0] || igt_format_is_yuv(fb-
> >drm_format)) {
> @@ -570,11 +573,13 @@ static int create_bo_for_fb(struct igt_fb *fb)
>  	igt_assert(fb->strides[0] == 0);
>  
>  	fb->size = calc_fb_size(fb);
> +	for (plane = 0; plane < fb->num_planes; plane++)
> +		bpp += DIV_ROUND_UP(fb->plane_bpp[plane],
> +				    plane ? fmt->hsub * fmt->vsub : 1);
>  
>  	fb->is_dumb = true;
>  	fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
> -					     fb->plane_bpp[0],
> -					     &fb->strides[0], &fb->size);
> +					     bpp, &fb->strides[0], &fb->size);
>  
>  	return fb->gem_handle;
>  }
-- 
Cheers,
	Lyude Paul

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

Re: [igt-dev] [PATCH i-g-t v6 00/10] igt: chamelium: Test YUV buffers using the Chamelium

[Paul Kocialkowski]

Hi,

On Fri, 2019-02-08 at 14:18 +0100, Maxime Ripard wrote:
> Hi,
> 
> Here is a first attempt at expanding the current Chamelium formats
> test to YUV formats.
> 
> It builds on top of the current infrastructure for the YUV convertions, and
> the format suppport of the Chamelium. It reworks igt_fb quite significantly
> to add generic support for YUV formats, and adds a bunch of new formats
> along the way.

The whole series was merged, thanks a lot!

Cheers,

Paul

> Let me know what you think,
> Maxime
> 
> Changes from v5:
>   - Fixed breakage on x86 platforms
>   - Fixed typo in a comment
>   - Split the size and strides check in a separate patch
>   - Added the various tags from Lyude and Paul
>   - Rebased on master
> 
> Changes from v4:
>   - Added Paul Reviewed-by's
>   - Removed the patch that moves calc_fb_size for everybody, and added some
>     asserts
> 
> Changes from v3:
>   - Fixed build breakage
> 
> Changes from v2:
>   - Added Paul Reviewed-by's
>   - Fixed a stride issue on the buffer allocation
>   - Take into account the plane bpp so that we don't overallocate
>   - Rebased on master
> 
> Changes from v1:
>   - Support more YUV formats than just NV12
>   - Rework the YUV convertion functions to add generic routines
>   - Took the comments into account.
> 
> Maxime Ripard (10):
>   igt: fb: Refactor dumb buffer allocation path
>   igt: fb: Add size checks and recalculation before dumb allocation
>   igt: fb: Account for all planes bpp
>   igt: fb: Don't pass the stride when allocating a dumb, multi-planar
>     buffer
>   igt: fb: Clear YUV dumb buffers
>   igt: fb: Rework YUV i915 allocation path
>   igt: fb: Add a bunch of new YUV formats
>   igt: tests: chamelium: Start to unify tests
>   igt: tests: chamelium: Convert VGA tests to do_test_display
>   igt: tests: chamelium: Add YUV formats tests
> 
>  lib/igt_fb.c          | 175 ++++++++++++++++++++++++-----
>  tests/kms_chamelium.c | 254 ++++++++++++++++++++++--------------------
>  2 files changed, 282 insertions(+), 147 deletions(-)
> 
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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