* [igt-dev] [PATCH i-g-t v2 01/13] igt: fb: Add subsampling parameters to the formats
2019-01-08 15:19 [igt-dev] [PATCH i-g-t v2 00/13] igt: chamelium: Test YUV buffers using the Chamelium Maxime Ripard
@ 2019-01-08 15:19 ` Maxime Ripard
2019-01-10 9:59 ` Paul Kocialkowski
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 02/13] igt: fb: Reduce tile size alignment for non intel platforms Maxime Ripard
` (14 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2019-01-08 15:19 UTC (permalink / raw)
To: igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
In order to improve the YUV support, let's add the horizontal and vertical
subsampling parameters to the IGT formats.
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
lib/igt_fb.c | 35 +++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 8244e5176a6f..00db2ee57cad 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -71,85 +71,104 @@ static const struct format_desc_struct {
int depth;
int num_planes;
int plane_bpp[4];
+ uint8_t hsub;
+ uint8_t vsub;
} format_desc[] = {
{ .name = "ARGB1555", .depth = -1, .drm_id = DRM_FORMAT_ARGB1555,
.cairo_id = CAIRO_FORMAT_INVALID,
.pixman_id = PIXMAN_a1r5g5b5,
.num_planes = 1, .plane_bpp = { 16, },
+ .hsub = 1, .vsub = 1,
},
{ .name = "XRGB1555", .depth = -1, .drm_id = DRM_FORMAT_XRGB1555,
.cairo_id = CAIRO_FORMAT_INVALID,
.pixman_id = PIXMAN_x1r5g5b5,
.num_planes = 1, .plane_bpp = { 16, },
+ .hsub = 1, .vsub = 1,
},
{ .name = "RGB565", .depth = 16, .drm_id = DRM_FORMAT_RGB565,
.cairo_id = CAIRO_FORMAT_RGB16_565,
.pixman_id = PIXMAN_r5g6b5,
.num_planes = 1, .plane_bpp = { 16, },
+ .hsub = 1, .vsub = 1,
},
{ .name = "BGR565", .depth = -1, .drm_id = DRM_FORMAT_BGR565,
.cairo_id = CAIRO_FORMAT_INVALID,
.pixman_id = PIXMAN_b5g6r5,
.num_planes = 1, .plane_bpp = { 16, },
+ .hsub = 1, .vsub = 1,
},
{ .name = "BGR888", .depth = -1, .drm_id = DRM_FORMAT_BGR888,
.cairo_id = CAIRO_FORMAT_INVALID,
.pixman_id = PIXMAN_b8g8r8,
.num_planes = 1, .plane_bpp = { 24, },
+ .hsub = 1, .vsub = 1,
},
{ .name = "RGB888", .depth = -1, .drm_id = DRM_FORMAT_RGB888,
.cairo_id = CAIRO_FORMAT_INVALID,
.pixman_id = PIXMAN_r8g8b8,
.num_planes = 1, .plane_bpp = { 24, },
+ .hsub = 1, .vsub = 1,
},
{ .name = "XYUV8888", .depth = -1, .drm_id = DRM_FORMAT_XYUV8888,
.cairo_id = CAIRO_FORMAT_RGB24,
.num_planes = 1, .plane_bpp = { 32, },
+ .hsub = 1, .vsub = 1,
},
{ .name = "XRGB8888", .depth = 24, .drm_id = DRM_FORMAT_XRGB8888,
.cairo_id = CAIRO_FORMAT_RGB24,
.pixman_id = PIXMAN_x8r8g8b8,
.num_planes = 1, .plane_bpp = { 32, },
+ .hsub = 1, .vsub = 1,
},
{ .name = "XBGR8888", .depth = -1, .drm_id = DRM_FORMAT_XBGR8888,
.cairo_id = CAIRO_FORMAT_INVALID,
.pixman_id = PIXMAN_x8b8g8r8,
.num_planes = 1, .plane_bpp = { 32, },
+ .hsub = 1, .vsub = 1,
},
{ .name = "XRGB2101010", .depth = 30, .drm_id = DRM_FORMAT_XRGB2101010,
.cairo_id = CAIRO_FORMAT_RGB30,
.pixman_id = PIXMAN_x2r10g10b10,
.num_planes = 1, .plane_bpp = { 32, },
+ .hsub = 1, .vsub = 1,
},
{ .name = "ARGB8888", .depth = 32, .drm_id = DRM_FORMAT_ARGB8888,
.cairo_id = CAIRO_FORMAT_ARGB32,
.pixman_id = PIXMAN_a8r8g8b8,
.num_planes = 1, .plane_bpp = { 32, },
+ .hsub = 1, .vsub = 1,
},
{ .name = "ABGR8888", .depth = -1, .drm_id = DRM_FORMAT_ABGR8888,
.cairo_id = CAIRO_FORMAT_INVALID,
.pixman_id = PIXMAN_a8b8g8r8,
.num_planes = 1, .plane_bpp = { 32, },
+ .hsub = 1, .vsub = 1,
},
{ .name = "NV12", .depth = -1, .drm_id = DRM_FORMAT_NV12,
.cairo_id = CAIRO_FORMAT_RGB24,
.num_planes = 2, .plane_bpp = { 8, 16, },
+ .hsub = 2, .vsub = 2,
},
{ .name = "YUYV", .depth = -1, .drm_id = DRM_FORMAT_YUYV,
.cairo_id = CAIRO_FORMAT_RGB24,
.num_planes = 1, .plane_bpp = { 16, },
+ .hsub = 2, .vsub = 1,
},
{ .name = "YVYU", .depth = -1, .drm_id = DRM_FORMAT_YVYU,
.cairo_id = CAIRO_FORMAT_RGB24,
.num_planes = 1, .plane_bpp = { 16, },
+ .hsub = 2, .vsub = 1,
},
{ .name = "UYVY", .depth = -1, .drm_id = DRM_FORMAT_UYVY,
.cairo_id = CAIRO_FORMAT_RGB24,
.num_planes = 1, .plane_bpp = { 16, },
+ .hsub = 2, .vsub = 1,
},
{ .name = "VYUY", .depth = -1, .drm_id = DRM_FORMAT_VYUY,
.cairo_id = CAIRO_FORMAT_RGB24,
.num_planes = 1, .plane_bpp = { 16, },
+ .hsub = 2, .vsub = 1,
},
};
#define for_each_format(f) \
@@ -239,10 +258,12 @@ void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
static unsigned fb_plane_width(const struct igt_fb *fb, int plane)
{
- if (fb->drm_format == DRM_FORMAT_NV12 && plane == 1)
- return DIV_ROUND_UP(fb->width, 2);
+ const struct format_desc_struct *format = lookup_drm_format(fb->drm_format);
- return fb->width;
+ if (plane == 0)
+ return fb->width;
+
+ return DIV_ROUND_UP(fb->width, format->hsub);
}
static unsigned fb_plane_bpp(const struct igt_fb *fb, int plane)
@@ -254,10 +275,12 @@ static unsigned fb_plane_bpp(const struct igt_fb *fb, int plane)
static unsigned fb_plane_height(const struct igt_fb *fb, int plane)
{
- if (fb->drm_format == DRM_FORMAT_NV12 && plane == 1)
- return DIV_ROUND_UP(fb->height, 2);
+ const struct format_desc_struct *format = lookup_drm_format(fb->drm_format);
- return fb->height;
+ if (plane == 0)
+ return fb->height;
+
+ return DIV_ROUND_UP(fb->height, format->vsub);
}
static int fb_num_planes(const struct igt_fb *fb)
--
2.20.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 01/13] igt: fb: Add subsampling parameters to the formats
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 01/13] igt: fb: Add subsampling parameters to the formats Maxime Ripard
@ 2019-01-10 9:59 ` Paul Kocialkowski
0 siblings, 0 replies; 36+ messages in thread
From: Paul Kocialkowski @ 2019-01-10 9:59 UTC (permalink / raw)
To: Maxime Ripard, igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> In order to improve the YUV support, let's add the horizontal and vertical
> subsampling parameters to the IGT formats.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
> lib/igt_fb.c | 35 +++++++++++++++++++++++++++++------
> 1 file changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 8244e5176a6f..00db2ee57cad 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -71,85 +71,104 @@ static const struct format_desc_struct {
> int depth;
> int num_planes;
> int plane_bpp[4];
> + uint8_t hsub;
> + uint8_t vsub;
> } format_desc[] = {
> { .name = "ARGB1555", .depth = -1, .drm_id = DRM_FORMAT_ARGB1555,
> .cairo_id = CAIRO_FORMAT_INVALID,
> .pixman_id = PIXMAN_a1r5g5b5,
> .num_planes = 1, .plane_bpp = { 16, },
> + .hsub = 1, .vsub = 1,
> },
> { .name = "XRGB1555", .depth = -1, .drm_id = DRM_FORMAT_XRGB1555,
> .cairo_id = CAIRO_FORMAT_INVALID,
> .pixman_id = PIXMAN_x1r5g5b5,
> .num_planes = 1, .plane_bpp = { 16, },
> + .hsub = 1, .vsub = 1,
> },
> { .name = "RGB565", .depth = 16, .drm_id = DRM_FORMAT_RGB565,
> .cairo_id = CAIRO_FORMAT_RGB16_565,
> .pixman_id = PIXMAN_r5g6b5,
> .num_planes = 1, .plane_bpp = { 16, },
> + .hsub = 1, .vsub = 1,
> },
> { .name = "BGR565", .depth = -1, .drm_id = DRM_FORMAT_BGR565,
> .cairo_id = CAIRO_FORMAT_INVALID,
> .pixman_id = PIXMAN_b5g6r5,
> .num_planes = 1, .plane_bpp = { 16, },
> + .hsub = 1, .vsub = 1,
> },
> { .name = "BGR888", .depth = -1, .drm_id = DRM_FORMAT_BGR888,
> .cairo_id = CAIRO_FORMAT_INVALID,
> .pixman_id = PIXMAN_b8g8r8,
> .num_planes = 1, .plane_bpp = { 24, },
> + .hsub = 1, .vsub = 1,
> },
> { .name = "RGB888", .depth = -1, .drm_id = DRM_FORMAT_RGB888,
> .cairo_id = CAIRO_FORMAT_INVALID,
> .pixman_id = PIXMAN_r8g8b8,
> .num_planes = 1, .plane_bpp = { 24, },
> + .hsub = 1, .vsub = 1,
> },
> { .name = "XYUV8888", .depth = -1, .drm_id = DRM_FORMAT_XYUV8888,
> .cairo_id = CAIRO_FORMAT_RGB24,
> .num_planes = 1, .plane_bpp = { 32, },
> + .hsub = 1, .vsub = 1,
> },
> { .name = "XRGB8888", .depth = 24, .drm_id = DRM_FORMAT_XRGB8888,
> .cairo_id = CAIRO_FORMAT_RGB24,
> .pixman_id = PIXMAN_x8r8g8b8,
> .num_planes = 1, .plane_bpp = { 32, },
> + .hsub = 1, .vsub = 1,
> },
> { .name = "XBGR8888", .depth = -1, .drm_id = DRM_FORMAT_XBGR8888,
> .cairo_id = CAIRO_FORMAT_INVALID,
> .pixman_id = PIXMAN_x8b8g8r8,
> .num_planes = 1, .plane_bpp = { 32, },
> + .hsub = 1, .vsub = 1,
> },
> { .name = "XRGB2101010", .depth = 30, .drm_id = DRM_FORMAT_XRGB2101010,
> .cairo_id = CAIRO_FORMAT_RGB30,
> .pixman_id = PIXMAN_x2r10g10b10,
> .num_planes = 1, .plane_bpp = { 32, },
> + .hsub = 1, .vsub = 1,
> },
> { .name = "ARGB8888", .depth = 32, .drm_id = DRM_FORMAT_ARGB8888,
> .cairo_id = CAIRO_FORMAT_ARGB32,
> .pixman_id = PIXMAN_a8r8g8b8,
> .num_planes = 1, .plane_bpp = { 32, },
> + .hsub = 1, .vsub = 1,
> },
> { .name = "ABGR8888", .depth = -1, .drm_id = DRM_FORMAT_ABGR8888,
> .cairo_id = CAIRO_FORMAT_INVALID,
> .pixman_id = PIXMAN_a8b8g8r8,
> .num_planes = 1, .plane_bpp = { 32, },
> + .hsub = 1, .vsub = 1,
> },
> { .name = "NV12", .depth = -1, .drm_id = DRM_FORMAT_NV12,
> .cairo_id = CAIRO_FORMAT_RGB24,
> .num_planes = 2, .plane_bpp = { 8, 16, },
> + .hsub = 2, .vsub = 2,
> },
> { .name = "YUYV", .depth = -1, .drm_id = DRM_FORMAT_YUYV,
> .cairo_id = CAIRO_FORMAT_RGB24,
> .num_planes = 1, .plane_bpp = { 16, },
> + .hsub = 2, .vsub = 1,
> },
> { .name = "YVYU", .depth = -1, .drm_id = DRM_FORMAT_YVYU,
> .cairo_id = CAIRO_FORMAT_RGB24,
> .num_planes = 1, .plane_bpp = { 16, },
> + .hsub = 2, .vsub = 1,
> },
> { .name = "UYVY", .depth = -1, .drm_id = DRM_FORMAT_UYVY,
> .cairo_id = CAIRO_FORMAT_RGB24,
> .num_planes = 1, .plane_bpp = { 16, },
> + .hsub = 2, .vsub = 1,
> },
> { .name = "VYUY", .depth = -1, .drm_id = DRM_FORMAT_VYUY,
> .cairo_id = CAIRO_FORMAT_RGB24,
> .num_planes = 1, .plane_bpp = { 16, },
> + .hsub = 2, .vsub = 1,
> },
> };
> #define for_each_format(f) \
> @@ -239,10 +258,12 @@ void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
>
> static unsigned fb_plane_width(const struct igt_fb *fb, int plane)
> {
> - if (fb->drm_format == DRM_FORMAT_NV12 && plane == 1)
> - return DIV_ROUND_UP(fb->width, 2);
> + const struct format_desc_struct *format = lookup_drm_format(fb->drm_format);
>
> - return fb->width;
> + if (plane == 0)
> + return fb->width;
> +
> + return DIV_ROUND_UP(fb->width, format->hsub);
> }
>
> static unsigned fb_plane_bpp(const struct igt_fb *fb, int plane)
> @@ -254,10 +275,12 @@ static unsigned fb_plane_bpp(const struct igt_fb *fb, int plane)
>
> static unsigned fb_plane_height(const struct igt_fb *fb, int plane)
> {
> - if (fb->drm_format == DRM_FORMAT_NV12 && plane == 1)
> - return DIV_ROUND_UP(fb->height, 2);
> + const struct format_desc_struct *format = lookup_drm_format(fb->drm_format);
>
> - return fb->height;
> + if (plane == 0)
> + return fb->height;
> +
> + return DIV_ROUND_UP(fb->height, format->vsub);
> }
>
> static int fb_num_planes(const struct igt_fb *fb)
--
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [igt-dev] [PATCH i-g-t v2 02/13] igt: fb: Reduce tile size alignment for non intel platforms
2019-01-08 15:19 [igt-dev] [PATCH i-g-t v2 00/13] igt: chamelium: Test YUV buffers using the Chamelium Maxime Ripard
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 01/13] igt: fb: Add subsampling parameters to the formats Maxime Ripard
@ 2019-01-08 15:19 ` Maxime Ripard
2019-01-10 9:59 ` Paul Kocialkowski
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 03/13] igt: fb: generic YUV convertion function Maxime Ripard
` (13 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2019-01-08 15:19 UTC (permalink / raw)
To: igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
Aligning the width on 64 pixels only make sense on intel platforms, make
sure to add a check against this.
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
lib/igt_fb.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 00db2ee57cad..6ab4e3039166 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -204,7 +204,11 @@ void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
{
switch (tiling) {
case LOCAL_DRM_FORMAT_MOD_NONE:
- *width_ret = 64;
+ if (is_i915_device(fd))
+ *width_ret = 64;
+ else
+ *width_ret = 1;
+
*height_ret = 1;
break;
case LOCAL_I915_FORMAT_MOD_X_TILED:
--
2.20.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 02/13] igt: fb: Reduce tile size alignment for non intel platforms
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 02/13] igt: fb: Reduce tile size alignment for non intel platforms Maxime Ripard
@ 2019-01-10 9:59 ` Paul Kocialkowski
0 siblings, 0 replies; 36+ messages in thread
From: Paul Kocialkowski @ 2019-01-10 9:59 UTC (permalink / raw)
To: Maxime Ripard, igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> Aligning the width on 64 pixels only make sense on intel platforms, make
> sure to add a check against this.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
> lib/igt_fb.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 00db2ee57cad..6ab4e3039166 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -204,7 +204,11 @@ void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
> {
> switch (tiling) {
> case LOCAL_DRM_FORMAT_MOD_NONE:
> - *width_ret = 64;
> + if (is_i915_device(fd))
> + *width_ret = 64;
> + else
> + *width_ret = 1;
> +
> *height_ret = 1;
> break;
> case LOCAL_I915_FORMAT_MOD_X_TILED:
--
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [igt-dev] [PATCH i-g-t v2 03/13] igt: fb: generic YUV convertion function
2019-01-08 15:19 [igt-dev] [PATCH i-g-t v2 00/13] igt: chamelium: Test YUV buffers using the Chamelium Maxime Ripard
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 01/13] igt: fb: Add subsampling parameters to the formats Maxime Ripard
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 02/13] igt: fb: Reduce tile size alignment for non intel platforms Maxime Ripard
@ 2019-01-08 15:19 ` Maxime Ripard
2019-01-10 10:07 ` Paul Kocialkowski
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 04/13] igt: fb: Move i915 YUV buffer clearing code to a function Maxime Ripard
` (12 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2019-01-08 15:19 UTC (permalink / raw)
To: igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
The current way we work when we want to support a new YUV format to IGT, we
also need to add new function to convert to and from XRGB8888.
This doesn't really scale however, and creates a lot of code to maintain.
In order to work around this, create a generic function to convert to RGB
and one to convert from RGB.
The only thing that is needed now is to add new parameters, and that's it.
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
lib/igt_fb.c | 537 ++++++++++++++++-----------------------------------
1 file changed, 171 insertions(+), 366 deletions(-)
diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 6ab4e3039166..45691fd441d9 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1577,424 +1577,237 @@ static void convert_src_put(const struct fb_convert *cvt,
free(src_buf);
}
-static void convert_nv12_to_rgb24(struct fb_convert *cvt)
+struct yuv_parameters {
+ unsigned y_inc;
+ unsigned uv_inc;
+ unsigned y_stride;
+ unsigned uv_stride;
+ unsigned y_offset;
+ unsigned u_offset;
+ unsigned v_offset;
+};
+
+static void get_yuv_parameters(struct igt_fb *fb, struct yuv_parameters *params)
{
- int i, j;
- const uint8_t *y, *uv;
- uint8_t *rgb24 = cvt->dst.ptr;
- unsigned int rgb24_stride = cvt->dst.fb->strides[0];
- unsigned int planar_stride = cvt->src.fb->strides[0];
- struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.fb->color_encoding,
- cvt->src.fb->color_range);
- uint8_t *buf;
-
- igt_assert(cvt->src.fb->drm_format == DRM_FORMAT_NV12 &&
- cvt->dst.fb->drm_format == DRM_FORMAT_XRGB8888);
-
- buf = convert_src_get(cvt);
- y = buf + cvt->src.fb->offsets[0];
- uv = buf + cvt->src.fb->offsets[1];
-
- for (i = 0; i < cvt->dst.fb->height / 2; i++) {
- for (j = 0; j < cvt->dst.fb->width / 2; j++) {
- /* Convert 2x2 pixel blocks */
- struct igt_vec4 yuv[4];
- struct igt_vec4 rgb[4];
-
- yuv[0].d[0] = y[j * 2 + 0];
- yuv[1].d[0] = y[j * 2 + 1];
- yuv[2].d[0] = y[j * 2 + 0 + planar_stride];
- yuv[3].d[0] = y[j * 2 + 1 + planar_stride];
-
- yuv[0].d[1] = yuv[1].d[1] = yuv[2].d[1] = yuv[3].d[1] = uv[j * 2 + 0];
- yuv[0].d[2] = yuv[1].d[2] = yuv[2].d[2] = yuv[3].d[2] = uv[j * 2 + 1];
- yuv[0].d[3] = yuv[1].d[3] = yuv[2].d[3] = yuv[3].d[3] = 1.0f;
-
- rgb[0] = igt_matrix_transform(&m, &yuv[0]);
- rgb[1] = igt_matrix_transform(&m, &yuv[1]);
- rgb[2] = igt_matrix_transform(&m, &yuv[2]);
- rgb[3] = igt_matrix_transform(&m, &yuv[3]);
-
- write_rgb(&rgb24[j * 8 + 0], &rgb[0]);
- write_rgb(&rgb24[j * 8 + 4], &rgb[1]);
- write_rgb(&rgb24[j * 8 + 0 + rgb24_stride], &rgb[2]);
- write_rgb(&rgb24[j * 8 + 4 + rgb24_stride], &rgb[3]);
- }
-
- if (cvt->dst.fb->width & 1) {
- /* Convert 1x2 pixel block */
- struct igt_vec4 yuv[2];
- struct igt_vec4 rgb[2];
-
- yuv[0].d[0] = y[j * 2 + 0];
- yuv[1].d[0] = y[j * 2 + 0 + planar_stride];
+ igt_assert(igt_format_is_yuv(fb->drm_format));
- yuv[0].d[1] = yuv[1].d[1] = uv[j * 2 + 0];
- yuv[0].d[2] = yuv[1].d[2] = uv[j * 2 + 1];
- yuv[0].d[3] = yuv[1].d[3] = 1.0f;
+ switch (fb->drm_format) {
+ case DRM_FORMAT_NV12:
+ params->y_inc = 1;
+ params->uv_inc = 2;
+ break;
- rgb[0] = igt_matrix_transform(&m, &yuv[0]);
- rgb[1] = igt_matrix_transform(&m, &yuv[1]);
-
- write_rgb(&rgb24[j * 8 + 0], &rgb[0]);
- write_rgb(&rgb24[j * 8 + 0 + rgb24_stride], &rgb[1]);
- }
-
- rgb24 += 2 * rgb24_stride;
- y += 2 * planar_stride;
- uv += planar_stride;
+ case DRM_FORMAT_YUYV:
+ case DRM_FORMAT_YVYU:
+ case DRM_FORMAT_UYVY:
+ case DRM_FORMAT_VYUY:
+ params->y_inc = 2;
+ params->uv_inc = 4;
+ break;
+
+ case DRM_FORMAT_XYUV8888:
+ params->y_inc = 4;
+ params->uv_inc = 4;
+ break;
}
- if (cvt->dst.fb->height & 1) {
- /* Convert last row */
- for (j = 0; j < cvt->dst.fb->width / 2; j++) {
- /* Convert 2x1 pixel blocks */
- struct igt_vec4 yuv[2];
- struct igt_vec4 rgb[2];
-
- yuv[0].d[0] = y[j * 2 + 0];
- yuv[1].d[0] = y[j * 2 + 1];
- yuv[0].d[1] = yuv[1].d[1] = uv[j * 2 + 0];
- yuv[0].d[2] = yuv[1].d[2] = uv[j * 2 + 1];
- yuv[0].d[3] = yuv[1].d[3] = 1.0f;
+ switch (fb->drm_format) {
+ case DRM_FORMAT_NV12:
+ params->y_stride = fb->strides[0];
+ params->uv_stride = fb->strides[1];
+ break;
- rgb[0] = igt_matrix_transform(&m, &yuv[0]);
- rgb[1] = igt_matrix_transform(&m, &yuv[1]);
+ case DRM_FORMAT_YUYV:
+ case DRM_FORMAT_YVYU:
+ case DRM_FORMAT_UYVY:
+ case DRM_FORMAT_VYUY:
+ case DRM_FORMAT_XYUV8888:
+ params->y_stride = fb->strides[0];
+ params->uv_stride = fb->strides[0];
+ break;
+ }
- write_rgb(&rgb24[j * 8 + 0], &rgb[0]);
- write_rgb(&rgb24[j * 8 + 4], &rgb[0]);
- }
+ switch (fb->drm_format) {
+ case DRM_FORMAT_NV12:
+ params->y_offset = fb->offsets[0];
+ params->u_offset = fb->offsets[1];
+ params->v_offset = fb->offsets[1] + 1;
+ break;
- if (cvt->dst.fb->width & 1) {
- /* Convert single pixel */
- struct igt_vec4 yuv;
- struct igt_vec4 rgb;
+ case DRM_FORMAT_YUYV:
+ params->y_offset = fb->offsets[0];
+ params->u_offset = fb->offsets[0] + 1;
+ params->v_offset = fb->offsets[0] + 3;
+ break;
- yuv.d[0] = y[j * 2 + 0];
- yuv.d[1] = uv[j * 2 + 0];
- yuv.d[2] = uv[j * 2 + 1];
- yuv.d[3] = 1.0f;
+ case DRM_FORMAT_YVYU:
+ params->y_offset = fb->offsets[0];
+ params->u_offset = fb->offsets[0] + 3;
+ params->v_offset = fb->offsets[0] + 1;
+ break;
- rgb = igt_matrix_transform(&m, &yuv);
+ case DRM_FORMAT_UYVY:
+ params->y_offset = fb->offsets[0] + 1;
+ params->u_offset = fb->offsets[0];
+ params->v_offset = fb->offsets[0] + 2;
+ break;
- write_rgb(&rgb24[j * 8 + 0], &rgb);
- }
+ case DRM_FORMAT_VYUY:
+ params->y_offset = fb->offsets[0] + 1;
+ params->u_offset = fb->offsets[0] + 2;
+ params->v_offset = fb->offsets[0];
+ break;
+
+ case DRM_FORMAT_XYUV8888:
+ params->y_offset = fb->offsets[0] + 1;
+ params->u_offset = fb->offsets[0] + 2;
+ params->v_offset = fb->offsets[0] + 3;
+ break;
}
-
- convert_src_put(cvt, buf);
}
-static void convert_yuv444_to_rgb24(struct fb_convert *cvt)
+static void convert_yuv_to_rgb24(struct fb_convert *cvt)
{
+ const struct format_desc_struct *src_fmt =
+ lookup_drm_format(cvt->src.fb->drm_format);
int i, j;
- uint8_t *yuv24;
+ uint8_t bpp = 4;
+ uint8_t *y, *u, *v;
uint8_t *rgb24 = cvt->dst.ptr;
- unsigned rgb24_stride = cvt->dst.fb->strides[0], xyuv_stride = cvt->src.fb->strides[0];
- uint8_t *buf = malloc(cvt->src.fb->size);
+ unsigned int rgb24_stride = cvt->dst.fb->strides[0];
struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.fb->color_encoding,
cvt->src.fb->color_range);
+ uint8_t *buf;
+ struct yuv_parameters params = { };
- /*
- * Reading from the BO is awfully slow because of lack of read caching,
- * it's faster to copy the whole BO to a temporary buffer and convert
- * from there.
- */
- igt_memcpy_from_wc(buf, cvt->src.ptr + cvt->src.fb->offsets[0], cvt->src.fb->size);
- yuv24 = buf;
+ igt_assert(cvt->dst.fb->drm_format == DRM_FORMAT_XRGB8888 &&
+ igt_format_is_yuv(cvt->src.fb->drm_format));
+
+ buf = convert_src_get(cvt);
+ get_yuv_parameters(cvt->src.fb, ¶ms);
+ y = buf + params.y_offset;
+ u = buf + params.u_offset;
+ v = buf + params.v_offset;
for (i = 0; i < cvt->dst.fb->height; i++) {
+ const uint8_t *y_tmp = y;
+ const uint8_t *u_tmp = u;
+ const uint8_t *v_tmp = v;
+ uint8_t *rgb_tmp = rgb24;
+
for (j = 0; j < cvt->dst.fb->width; j++) {
- float y, u, v;
- struct igt_vec4 yuv;
- struct igt_vec4 rgb;
-
- v = yuv24[i * xyuv_stride + j * 4];
- u = yuv24[i * xyuv_stride + j * 4 + 1];
- y = yuv24[i * xyuv_stride + j * 4 + 2];
- yuv.d[0] = y;
- yuv.d[1] = u;
- yuv.d[2] = v;
+ struct igt_vec4 rgb, yuv;
+
+ yuv.d[0] = *y_tmp;
+ yuv.d[1] = *u_tmp;
+ yuv.d[2] = *v_tmp;
yuv.d[3] = 1.0f;
rgb = igt_matrix_transform(&m, &yuv);
+ write_rgb(rgb_tmp, &rgb);
- write_rgb(&rgb24[i * rgb24_stride + j * 4], &rgb);
- }
- }
-
- free(buf);
-}
-
-
-static void convert_rgb24_to_yuv444(struct fb_convert *cvt)
-{
- int i, j;
- uint8_t *rgb24;
- uint8_t *yuv444 = cvt->dst.ptr + cvt->dst.fb->offsets[0];
- unsigned int rgb24_stride = cvt->src.fb->strides[0], xyuv_stride = cvt->dst.fb->strides[0];
- struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.fb->color_encoding,
- cvt->dst.fb->color_range);
-
- rgb24 = cvt->src.ptr;
+ rgb_tmp += bpp;
+ y_tmp += params.y_inc;
- igt_assert_f(cvt->dst.fb->drm_format == DRM_FORMAT_XYUV8888,
- "Conversion not implemented for !XYUV packed formats\n");
-
- for (i = 0; i < cvt->dst.fb->height; i++) {
- for (j = 0; j < cvt->dst.fb->width; j++) {
- struct igt_vec4 rgb;
- struct igt_vec4 yuv;
-
- read_rgb(&rgb, &rgb24[i * rgb24_stride + j * 4]);
+ if ((src_fmt->hsub == 1) || (j % src_fmt->hsub)) {
+ u_tmp += params.uv_inc;
+ v_tmp += params.uv_inc;
+ }
+ }
- yuv = igt_matrix_transform(&m, &rgb);
+ rgb24 += rgb24_stride;
+ y += params.y_stride;
- yuv444[i * xyuv_stride + j * 4] = yuv.d[2];
- yuv444[i * xyuv_stride + j * 4 + 1] = yuv.d[1];
- yuv444[i * xyuv_stride + j * 4 + 2] = yuv.d[0];
+ if ((src_fmt->vsub == 1) || (i % src_fmt->vsub)) {
+ u += params.uv_stride;
+ v += params.uv_stride;
}
}
+
+ convert_src_put(cvt, buf);
}
-static void convert_rgb24_to_nv12(struct fb_convert *cvt)
+static void convert_rgb24_to_yuv(struct fb_convert *cvt)
{
+ const struct format_desc_struct *dst_fmt =
+ lookup_drm_format(cvt->dst.fb->drm_format);
int i, j;
- uint8_t *y = cvt->dst.ptr + cvt->dst.fb->offsets[0];
- uint8_t *uv = cvt->dst.ptr + cvt->dst.fb->offsets[1];
+ uint8_t *y, *u, *v;
const uint8_t *rgb24 = cvt->src.ptr;
+ uint8_t bpp = 4;
unsigned rgb24_stride = cvt->src.fb->strides[0];
- unsigned planar_stride = cvt->dst.fb->strides[0];
struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.fb->color_encoding,
cvt->dst.fb->color_range);
+ struct yuv_parameters params = { };
igt_assert(cvt->src.fb->drm_format == DRM_FORMAT_XRGB8888 &&
- cvt->dst.fb->drm_format == DRM_FORMAT_NV12);
+ igt_format_is_yuv(cvt->dst.fb->drm_format));
- for (i = 0; i < cvt->dst.fb->height / 2; i++) {
- for (j = 0; j < cvt->dst.fb->width / 2; j++) {
- /* Convert 2x2 pixel blocks */
- struct igt_vec4 rgb[4];
- struct igt_vec4 yuv[4];
+ get_yuv_parameters(cvt->dst.fb, ¶ms);
+ y = cvt->dst.ptr + params.y_offset;
+ u = cvt->dst.ptr + params.u_offset;
+ v = cvt->dst.ptr + params.v_offset;
- read_rgb(&rgb[0], &rgb24[j * 8 + 0]);
- read_rgb(&rgb[1], &rgb24[j * 8 + 4]);
- read_rgb(&rgb[2], &rgb24[j * 8 + 0 + rgb24_stride]);
- read_rgb(&rgb[3], &rgb24[j * 8 + 4 + rgb24_stride]);
-
- yuv[0] = igt_matrix_transform(&m, &rgb[0]);
- yuv[1] = igt_matrix_transform(&m, &rgb[1]);
- yuv[2] = igt_matrix_transform(&m, &rgb[2]);
- yuv[3] = igt_matrix_transform(&m, &rgb[3]);
-
- y[j * 2 + 0] = yuv[0].d[0];
- y[j * 2 + 1] = yuv[1].d[0];
- y[j * 2 + 0 + planar_stride] = yuv[2].d[0];
- y[j * 2 + 1 + planar_stride] = yuv[3].d[0];
+ for (i = 0; i < cvt->dst.fb->height; i++) {
+ const uint8_t *rgb_tmp = rgb24;
+ uint8_t *y_tmp = y;
+ uint8_t *u_tmp = u;
+ uint8_t *v_tmp = v;
- /*
- * We assume the MPEG2 chroma siting convention, where
- * pixel center for Cb'Cr' is between the left top and
- * bottom pixel in a 2x2 block, so take the average.
- */
- uv[j * 2 + 0] = (yuv[0].d[1] + yuv[2].d[1]) / 2.0f;
- uv[j * 2 + 1] = (yuv[0].d[2] + yuv[2].d[2]) / 2.0f;
- }
+ for (j = 0; j < cvt->dst.fb->width; j++) {
+ const uint8_t *pair_rgb24 = rgb_tmp;
+ struct igt_vec4 pair_rgb, rgb;
+ struct igt_vec4 pair_yuv, yuv;
- if (cvt->dst.fb->width & 1) {
- /* Convert 1x2 pixel block */
- struct igt_vec4 rgb[2];
- struct igt_vec4 yuv[2];
+ read_rgb(&rgb, rgb_tmp);
+ yuv = igt_matrix_transform(&m, &rgb);
- read_rgb(&rgb[0], &rgb24[j * 8 + 0]);
- read_rgb(&rgb[2], &rgb24[j * 8 + 0 + rgb24_stride]);
+ rgb_tmp += bpp;
- yuv[0] = igt_matrix_transform(&m, &rgb[0]);
- yuv[1] = igt_matrix_transform(&m, &rgb[1]);
+ *y_tmp = yuv.d[0];
+ y_tmp += params.y_inc;
- y[j * 2 + 0] = yuv[0].d[0];
- y[j * 2 + 0 + planar_stride] = yuv[1].d[0];
+ if ((i % dst_fmt->vsub) || (j % dst_fmt->hsub))
+ continue;
/*
* We assume the MPEG2 chroma siting convention, where
* pixel center for Cb'Cr' is between the left top and
* bottom pixel in a 2x2 block, so take the average.
+ *
+ * Therefore, if we use subsampling, we only really care
+ * about two pixels all the time, either the two
+ * subsequent pixels horizontally, vertically, or the
+ * two corners in a 2x2 block.
+ *
+ * The only corner case is when we have an odd number of
+ * pixels, but this can be handled pretty easily by not
+ * incrementing the paired pixel pointer in the
+ * direction it's odd in.
*/
- uv[j * 2 + 0] = (yuv[0].d[1] + yuv[1].d[1]) / 2.0f;
- uv[j * 2 + 1] = (yuv[0].d[2] + yuv[1].d[2]) / 2.0f;
- }
-
- rgb24 += 2 * rgb24_stride;
- y += 2 * planar_stride;
- uv += planar_stride;
- }
-
- /* Last row cannot be interpolated between 2 pixels, take the single value */
- if (cvt->dst.fb->height & 1) {
- for (j = 0; j < cvt->dst.fb->width / 2; j++) {
- /* Convert 2x1 pixel blocks */
- struct igt_vec4 rgb[2];
- struct igt_vec4 yuv[2];
-
- read_rgb(&rgb[0], &rgb24[j * 8 + 0]);
- read_rgb(&rgb[1], &rgb24[j * 8 + 4]);
-
- yuv[0] = igt_matrix_transform(&m, &rgb[0]);
- yuv[1] = igt_matrix_transform(&m, &rgb[1]);
+ if (j != (cvt->dst.fb->width - 1))
+ pair_rgb24 += (dst_fmt->hsub - 1) * bpp;
- y[j * 2 + 0] = yuv[0].d[0];
- y[j * 2 + 1] = yuv[1].d[0];
- uv[j * 2 + 0] = yuv[0].d[1];
- uv[j * 2 + 1] = yuv[0].d[2];
- }
-
- if (cvt->dst.fb->width & 1) {
- /* Convert single pixel */
- struct igt_vec4 rgb;
- struct igt_vec4 yuv;
+ if (i != (cvt->dst.fb->height - 1))
+ pair_rgb24 += rgb24_stride * (dst_fmt->vsub - 1);
- read_rgb(&rgb, &rgb24[j * 8 + 0]);
+ read_rgb(&pair_rgb, pair_rgb24);
+ pair_yuv = igt_matrix_transform(&m, &pair_rgb);
- yuv = igt_matrix_transform(&m, &rgb);
+ *u_tmp = (yuv.d[1] + pair_yuv.d[1]) / 2.0f;
+ *v_tmp = (yuv.d[2] + pair_yuv.d[2]) / 2.0f;
- y[j * 2 + 0] = yuv.d[0];
- uv[j * 2 + 0] = yuv.d[1];
- uv[j * 2 + 1] = yuv.d[2];
- }
- }
-}
-
-/* { Y0, U, Y1, V } */
-static const unsigned char swizzle_yuyv[] = { 0, 1, 2, 3 };
-static const unsigned char swizzle_yvyu[] = { 0, 3, 2, 1 };
-static const unsigned char swizzle_uyvy[] = { 1, 0, 3, 2 };
-static const unsigned char swizzle_vyuy[] = { 1, 2, 3, 0 };
-
-static const unsigned char *yuyv_swizzle(uint32_t format)
-{
- switch (format) {
- default:
- case DRM_FORMAT_YUYV:
- return swizzle_yuyv;
- case DRM_FORMAT_YVYU:
- return swizzle_yvyu;
- case DRM_FORMAT_UYVY:
- return swizzle_uyvy;
- case DRM_FORMAT_VYUY:
- return swizzle_vyuy;
- }
-}
-
-static void convert_yuyv_to_rgb24(struct fb_convert *cvt)
-{
- int i, j;
- const uint8_t *yuyv;
- uint8_t *rgb24 = cvt->dst.ptr;
- unsigned int rgb24_stride = cvt->dst.fb->strides[0];
- unsigned int yuyv_stride = cvt->src.fb->strides[0];
- struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.fb->color_encoding,
- cvt->src.fb->color_range);
- const unsigned char *swz = yuyv_swizzle(cvt->src.fb->drm_format);
- uint8_t *buf;
-
- igt_assert((cvt->src.fb->drm_format == DRM_FORMAT_YUYV ||
- cvt->src.fb->drm_format == DRM_FORMAT_UYVY ||
- cvt->src.fb->drm_format == DRM_FORMAT_YVYU ||
- cvt->src.fb->drm_format == DRM_FORMAT_VYUY) &&
- cvt->dst.fb->drm_format == DRM_FORMAT_XRGB8888);
-
- buf = convert_src_get(cvt);
- yuyv = buf;
-
- for (i = 0; i < cvt->dst.fb->height; i++) {
- for (j = 0; j < cvt->dst.fb->width / 2; j++) {
- /* Convert 2x1 pixel blocks */
- struct igt_vec4 yuv[2];
- struct igt_vec4 rgb[2];
-
- yuv[0].d[0] = yuyv[j * 4 + swz[0]];
- yuv[1].d[0] = yuyv[j * 4 + swz[2]];
- yuv[0].d[1] = yuv[1].d[1] = yuyv[j * 4 + swz[1]];
- yuv[0].d[2] = yuv[1].d[2] = yuyv[j * 4 + swz[3]];
- yuv[0].d[3] = yuv[1].d[3] = 1.0f;
-
- rgb[0] = igt_matrix_transform(&m, &yuv[0]);
- rgb[1] = igt_matrix_transform(&m, &yuv[1]);
-
- write_rgb(&rgb24[j * 8 + 0], &rgb[0]);
- write_rgb(&rgb24[j * 8 + 4], &rgb[1]);
- }
-
- if (cvt->dst.fb->width & 1) {
- struct igt_vec4 yuv;
- struct igt_vec4 rgb;
-
- yuv.d[0] = yuyv[j * 4 + swz[0]];
- yuv.d[1] = yuyv[j * 4 + swz[1]];
- yuv.d[2] = yuyv[j * 4 + swz[3]];
- yuv.d[3] = 1.0f;
-
- rgb = igt_matrix_transform(&m, &yuv);
-
- write_rgb(&rgb24[j * 8 + 0], &rgb);
+ u_tmp += params.uv_inc;
+ v_tmp += params.uv_inc;
}
rgb24 += rgb24_stride;
- yuyv += yuyv_stride;
- }
-
- convert_src_put(cvt, buf);
-}
-
-static void convert_rgb24_to_yuyv(struct fb_convert *cvt)
-{
- int i, j;
- uint8_t *yuyv = cvt->dst.ptr;
- const uint8_t *rgb24 = cvt->src.ptr;
- unsigned rgb24_stride = cvt->src.fb->strides[0];
- unsigned yuyv_stride = cvt->dst.fb->strides[0];
- struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.fb->color_encoding,
- cvt->dst.fb->color_range);
- const unsigned char *swz = yuyv_swizzle(cvt->dst.fb->drm_format);
-
- igt_assert(cvt->src.fb->drm_format == DRM_FORMAT_XRGB8888 &&
- (cvt->dst.fb->drm_format == DRM_FORMAT_YUYV ||
- cvt->dst.fb->drm_format == DRM_FORMAT_UYVY ||
- cvt->dst.fb->drm_format == DRM_FORMAT_YVYU ||
- cvt->dst.fb->drm_format == DRM_FORMAT_VYUY));
-
- for (i = 0; i < cvt->dst.fb->height; i++) {
- for (j = 0; j < cvt->dst.fb->width / 2; j++) {
- /* Convert 2x1 pixel blocks */
- struct igt_vec4 rgb[2];
- struct igt_vec4 yuv[2];
-
- read_rgb(&rgb[0], &rgb24[j * 8 + 0]);
- read_rgb(&rgb[1], &rgb24[j * 8 + 4]);
-
- yuv[0] = igt_matrix_transform(&m, &rgb[0]);
- yuv[1] = igt_matrix_transform(&m, &rgb[1]);
+ y += params.y_stride;
- yuyv[j * 4 + swz[0]] = yuv[0].d[0];
- yuyv[j * 4 + swz[2]] = yuv[1].d[0];
- yuyv[j * 4 + swz[1]] = (yuv[0].d[1] + yuv[1].d[1]) / 2.0f;
- yuyv[j * 4 + swz[3]] = (yuv[0].d[2] + yuv[1].d[2]) / 2.0f;
+ if ((i % dst_fmt->vsub) == (dst_fmt->vsub - 1)) {
+ u += params.uv_stride;
+ v += params.uv_stride;
}
-
- if (cvt->dst.fb->width & 1) {
- struct igt_vec4 rgb;
- struct igt_vec4 yuv;
-
- read_rgb(&rgb, &rgb24[j * 8 + 0]);
-
- yuv = igt_matrix_transform(&m, &rgb);
-
- yuyv[j * 4 + swz[0]] = yuv.d[0];
- yuyv[j * 4 + swz[1]] = yuv.d[1];
- yuyv[j * 4 + swz[3]] = yuv.d[2];
- }
-
- rgb24 += rgb24_stride;
- yuyv += yuyv_stride;
}
}
@@ -2046,31 +1859,23 @@ static void fb_convert(struct fb_convert *cvt)
} else if (cvt->dst.fb->drm_format == DRM_FORMAT_XRGB8888) {
switch (cvt->src.fb->drm_format) {
case DRM_FORMAT_XYUV8888:
- convert_yuv444_to_rgb24(cvt);
- return;
case DRM_FORMAT_NV12:
- convert_nv12_to_rgb24(cvt);
- return;
- case DRM_FORMAT_YUYV:
- case DRM_FORMAT_YVYU:
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
- convert_yuyv_to_rgb24(cvt);
+ case DRM_FORMAT_YUYV:
+ case DRM_FORMAT_YVYU:
+ convert_yuv_to_rgb24(cvt);
return;
}
} else if (cvt->src.fb->drm_format == DRM_FORMAT_XRGB8888) {
switch (cvt->dst.fb->drm_format) {
case DRM_FORMAT_XYUV8888:
- convert_rgb24_to_yuv444(cvt);
- return;
case DRM_FORMAT_NV12:
- convert_rgb24_to_nv12(cvt);
- return;
case DRM_FORMAT_YUYV:
case DRM_FORMAT_YVYU:
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
- convert_rgb24_to_yuyv(cvt);
+ convert_rgb24_to_yuv(cvt);
return;
}
}
--
2.20.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 03/13] igt: fb: generic YUV convertion function
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 03/13] igt: fb: generic YUV convertion function Maxime Ripard
@ 2019-01-10 10:07 ` Paul Kocialkowski
0 siblings, 0 replies; 36+ messages in thread
From: Paul Kocialkowski @ 2019-01-10 10:07 UTC (permalink / raw)
To: Maxime Ripard, igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> The current way we work when we want to support a new YUV format to IGT, we
> also need to add new function to convert to and from XRGB8888.
>
> This doesn't really scale however, and creates a lot of code to maintain.
> In order to work around this, create a generic function to convert to RGB
> and one to convert from RGB.
>
> The only thing that is needed now is to add new parameters, and that's it.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
> lib/igt_fb.c | 537 ++++++++++++++++-----------------------------------
> 1 file changed, 171 insertions(+), 366 deletions(-)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 6ab4e3039166..45691fd441d9 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -1577,424 +1577,237 @@ static void convert_src_put(const struct fb_convert *cvt,
> free(src_buf);
> }
>
> -static void convert_nv12_to_rgb24(struct fb_convert *cvt)
> +struct yuv_parameters {
> + unsigned y_inc;
> + unsigned uv_inc;
> + unsigned y_stride;
> + unsigned uv_stride;
> + unsigned y_offset;
> + unsigned u_offset;
> + unsigned v_offset;
> +};
> +
> +static void get_yuv_parameters(struct igt_fb *fb, struct yuv_parameters *params)
> {
> - int i, j;
> - const uint8_t *y, *uv;
> - uint8_t *rgb24 = cvt->dst.ptr;
> - unsigned int rgb24_stride = cvt->dst.fb->strides[0];
> - unsigned int planar_stride = cvt->src.fb->strides[0];
> - struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.fb->color_encoding,
> - cvt->src.fb->color_range);
> - uint8_t *buf;
> -
> - igt_assert(cvt->src.fb->drm_format == DRM_FORMAT_NV12 &&
> - cvt->dst.fb->drm_format == DRM_FORMAT_XRGB8888);
> -
> - buf = convert_src_get(cvt);
> - y = buf + cvt->src.fb->offsets[0];
> - uv = buf + cvt->src.fb->offsets[1];
> -
> - for (i = 0; i < cvt->dst.fb->height / 2; i++) {
> - for (j = 0; j < cvt->dst.fb->width / 2; j++) {
> - /* Convert 2x2 pixel blocks */
> - struct igt_vec4 yuv[4];
> - struct igt_vec4 rgb[4];
> -
> - yuv[0].d[0] = y[j * 2 + 0];
> - yuv[1].d[0] = y[j * 2 + 1];
> - yuv[2].d[0] = y[j * 2 + 0 + planar_stride];
> - yuv[3].d[0] = y[j * 2 + 1 + planar_stride];
> -
> - yuv[0].d[1] = yuv[1].d[1] = yuv[2].d[1] = yuv[3].d[1] = uv[j * 2 + 0];
> - yuv[0].d[2] = yuv[1].d[2] = yuv[2].d[2] = yuv[3].d[2] = uv[j * 2 + 1];
> - yuv[0].d[3] = yuv[1].d[3] = yuv[2].d[3] = yuv[3].d[3] = 1.0f;
> -
> - rgb[0] = igt_matrix_transform(&m, &yuv[0]);
> - rgb[1] = igt_matrix_transform(&m, &yuv[1]);
> - rgb[2] = igt_matrix_transform(&m, &yuv[2]);
> - rgb[3] = igt_matrix_transform(&m, &yuv[3]);
> -
> - write_rgb(&rgb24[j * 8 + 0], &rgb[0]);
> - write_rgb(&rgb24[j * 8 + 4], &rgb[1]);
> - write_rgb(&rgb24[j * 8 + 0 + rgb24_stride], &rgb[2]);
> - write_rgb(&rgb24[j * 8 + 4 + rgb24_stride], &rgb[3]);
> - }
> -
> - if (cvt->dst.fb->width & 1) {
> - /* Convert 1x2 pixel block */
> - struct igt_vec4 yuv[2];
> - struct igt_vec4 rgb[2];
> -
> - yuv[0].d[0] = y[j * 2 + 0];
> - yuv[1].d[0] = y[j * 2 + 0 + planar_stride];
> + igt_assert(igt_format_is_yuv(fb->drm_format));
>
> - yuv[0].d[1] = yuv[1].d[1] = uv[j * 2 + 0];
> - yuv[0].d[2] = yuv[1].d[2] = uv[j * 2 + 1];
> - yuv[0].d[3] = yuv[1].d[3] = 1.0f;
> + switch (fb->drm_format) {
> + case DRM_FORMAT_NV12:
> + params->y_inc = 1;
> + params->uv_inc = 2;
> + break;
>
> - rgb[0] = igt_matrix_transform(&m, &yuv[0]);
> - rgb[1] = igt_matrix_transform(&m, &yuv[1]);
> -
> - write_rgb(&rgb24[j * 8 + 0], &rgb[0]);
> - write_rgb(&rgb24[j * 8 + 0 + rgb24_stride], &rgb[1]);
> - }
> -
> - rgb24 += 2 * rgb24_stride;
> - y += 2 * planar_stride;
> - uv += planar_stride;
> + case DRM_FORMAT_YUYV:
> + case DRM_FORMAT_YVYU:
> + case DRM_FORMAT_UYVY:
> + case DRM_FORMAT_VYUY:
> + params->y_inc = 2;
> + params->uv_inc = 4;
> + break;
> +
> + case DRM_FORMAT_XYUV8888:
> + params->y_inc = 4;
> + params->uv_inc = 4;
> + break;
> }
>
> - if (cvt->dst.fb->height & 1) {
> - /* Convert last row */
> - for (j = 0; j < cvt->dst.fb->width / 2; j++) {
> - /* Convert 2x1 pixel blocks */
> - struct igt_vec4 yuv[2];
> - struct igt_vec4 rgb[2];
> -
> - yuv[0].d[0] = y[j * 2 + 0];
> - yuv[1].d[0] = y[j * 2 + 1];
> - yuv[0].d[1] = yuv[1].d[1] = uv[j * 2 + 0];
> - yuv[0].d[2] = yuv[1].d[2] = uv[j * 2 + 1];
> - yuv[0].d[3] = yuv[1].d[3] = 1.0f;
> + switch (fb->drm_format) {
> + case DRM_FORMAT_NV12:
> + params->y_stride = fb->strides[0];
> + params->uv_stride = fb->strides[1];
> + break;
>
> - rgb[0] = igt_matrix_transform(&m, &yuv[0]);
> - rgb[1] = igt_matrix_transform(&m, &yuv[1]);
> + case DRM_FORMAT_YUYV:
> + case DRM_FORMAT_YVYU:
> + case DRM_FORMAT_UYVY:
> + case DRM_FORMAT_VYUY:
> + case DRM_FORMAT_XYUV8888:
> + params->y_stride = fb->strides[0];
> + params->uv_stride = fb->strides[0];
> + break;
> + }
>
> - write_rgb(&rgb24[j * 8 + 0], &rgb[0]);
> - write_rgb(&rgb24[j * 8 + 4], &rgb[0]);
> - }
> + switch (fb->drm_format) {
> + case DRM_FORMAT_NV12:
> + params->y_offset = fb->offsets[0];
> + params->u_offset = fb->offsets[1];
> + params->v_offset = fb->offsets[1] + 1;
> + break;
>
> - if (cvt->dst.fb->width & 1) {
> - /* Convert single pixel */
> - struct igt_vec4 yuv;
> - struct igt_vec4 rgb;
> + case DRM_FORMAT_YUYV:
> + params->y_offset = fb->offsets[0];
> + params->u_offset = fb->offsets[0] + 1;
> + params->v_offset = fb->offsets[0] + 3;
> + break;
>
> - yuv.d[0] = y[j * 2 + 0];
> - yuv.d[1] = uv[j * 2 + 0];
> - yuv.d[2] = uv[j * 2 + 1];
> - yuv.d[3] = 1.0f;
> + case DRM_FORMAT_YVYU:
> + params->y_offset = fb->offsets[0];
> + params->u_offset = fb->offsets[0] + 3;
> + params->v_offset = fb->offsets[0] + 1;
> + break;
>
> - rgb = igt_matrix_transform(&m, &yuv);
> + case DRM_FORMAT_UYVY:
> + params->y_offset = fb->offsets[0] + 1;
> + params->u_offset = fb->offsets[0];
> + params->v_offset = fb->offsets[0] + 2;
> + break;
>
> - write_rgb(&rgb24[j * 8 + 0], &rgb);
> - }
> + case DRM_FORMAT_VYUY:
> + params->y_offset = fb->offsets[0] + 1;
> + params->u_offset = fb->offsets[0] + 2;
> + params->v_offset = fb->offsets[0];
> + break;
> +
> + case DRM_FORMAT_XYUV8888:
> + params->y_offset = fb->offsets[0] + 1;
> + params->u_offset = fb->offsets[0] + 2;
> + params->v_offset = fb->offsets[0] + 3;
> + break;
> }
> -
> - convert_src_put(cvt, buf);
> }
>
> -static void convert_yuv444_to_rgb24(struct fb_convert *cvt)
> +static void convert_yuv_to_rgb24(struct fb_convert *cvt)
> {
> + const struct format_desc_struct *src_fmt =
> + lookup_drm_format(cvt->src.fb->drm_format);
> int i, j;
> - uint8_t *yuv24;
> + uint8_t bpp = 4;
> + uint8_t *y, *u, *v;
> uint8_t *rgb24 = cvt->dst.ptr;
> - unsigned rgb24_stride = cvt->dst.fb->strides[0], xyuv_stride = cvt->src.fb->strides[0];
> - uint8_t *buf = malloc(cvt->src.fb->size);
> + unsigned int rgb24_stride = cvt->dst.fb->strides[0];
> struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.fb->color_encoding,
> cvt->src.fb->color_range);
> + uint8_t *buf;
> + struct yuv_parameters params = { };
>
> - /*
> - * Reading from the BO is awfully slow because of lack of read caching,
> - * it's faster to copy the whole BO to a temporary buffer and convert
> - * from there.
> - */
> - igt_memcpy_from_wc(buf, cvt->src.ptr + cvt->src.fb->offsets[0], cvt->src.fb->size);
> - yuv24 = buf;
> + igt_assert(cvt->dst.fb->drm_format == DRM_FORMAT_XRGB8888 &&
> + igt_format_is_yuv(cvt->src.fb->drm_format));
> +
> + buf = convert_src_get(cvt);
> + get_yuv_parameters(cvt->src.fb, ¶ms);
> + y = buf + params.y_offset;
> + u = buf + params.u_offset;
> + v = buf + params.v_offset;
>
> for (i = 0; i < cvt->dst.fb->height; i++) {
> + const uint8_t *y_tmp = y;
> + const uint8_t *u_tmp = u;
> + const uint8_t *v_tmp = v;
> + uint8_t *rgb_tmp = rgb24;
> +
> for (j = 0; j < cvt->dst.fb->width; j++) {
> - float y, u, v;
> - struct igt_vec4 yuv;
> - struct igt_vec4 rgb;
> -
> - v = yuv24[i * xyuv_stride + j * 4];
> - u = yuv24[i * xyuv_stride + j * 4 + 1];
> - y = yuv24[i * xyuv_stride + j * 4 + 2];
> - yuv.d[0] = y;
> - yuv.d[1] = u;
> - yuv.d[2] = v;
> + struct igt_vec4 rgb, yuv;
> +
> + yuv.d[0] = *y_tmp;
> + yuv.d[1] = *u_tmp;
> + yuv.d[2] = *v_tmp;
> yuv.d[3] = 1.0f;
>
> rgb = igt_matrix_transform(&m, &yuv);
> + write_rgb(rgb_tmp, &rgb);
>
> - write_rgb(&rgb24[i * rgb24_stride + j * 4], &rgb);
> - }
> - }
> -
> - free(buf);
> -}
> -
> -
> -static void convert_rgb24_to_yuv444(struct fb_convert *cvt)
> -{
> - int i, j;
> - uint8_t *rgb24;
> - uint8_t *yuv444 = cvt->dst.ptr + cvt->dst.fb->offsets[0];
> - unsigned int rgb24_stride = cvt->src.fb->strides[0], xyuv_stride = cvt->dst.fb->strides[0];
> - struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.fb->color_encoding,
> - cvt->dst.fb->color_range);
> -
> - rgb24 = cvt->src.ptr;
> + rgb_tmp += bpp;
> + y_tmp += params.y_inc;
>
> - igt_assert_f(cvt->dst.fb->drm_format == DRM_FORMAT_XYUV8888,
> - "Conversion not implemented for !XYUV packed formats\n");
> -
> - for (i = 0; i < cvt->dst.fb->height; i++) {
> - for (j = 0; j < cvt->dst.fb->width; j++) {
> - struct igt_vec4 rgb;
> - struct igt_vec4 yuv;
> -
> - read_rgb(&rgb, &rgb24[i * rgb24_stride + j * 4]);
> + if ((src_fmt->hsub == 1) || (j % src_fmt->hsub)) {
> + u_tmp += params.uv_inc;
> + v_tmp += params.uv_inc;
> + }
> + }
>
> - yuv = igt_matrix_transform(&m, &rgb);
> + rgb24 += rgb24_stride;
> + y += params.y_stride;
>
> - yuv444[i * xyuv_stride + j * 4] = yuv.d[2];
> - yuv444[i * xyuv_stride + j * 4 + 1] = yuv.d[1];
> - yuv444[i * xyuv_stride + j * 4 + 2] = yuv.d[0];
> + if ((src_fmt->vsub == 1) || (i % src_fmt->vsub)) {
> + u += params.uv_stride;
> + v += params.uv_stride;
> }
> }
> +
> + convert_src_put(cvt, buf);
> }
>
> -static void convert_rgb24_to_nv12(struct fb_convert *cvt)
> +static void convert_rgb24_to_yuv(struct fb_convert *cvt)
> {
> + const struct format_desc_struct *dst_fmt =
> + lookup_drm_format(cvt->dst.fb->drm_format);
> int i, j;
> - uint8_t *y = cvt->dst.ptr + cvt->dst.fb->offsets[0];
> - uint8_t *uv = cvt->dst.ptr + cvt->dst.fb->offsets[1];
> + uint8_t *y, *u, *v;
> const uint8_t *rgb24 = cvt->src.ptr;
> + uint8_t bpp = 4;
> unsigned rgb24_stride = cvt->src.fb->strides[0];
> - unsigned planar_stride = cvt->dst.fb->strides[0];
> struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.fb->color_encoding,
> cvt->dst.fb->color_range);
> + struct yuv_parameters params = { };
>
> igt_assert(cvt->src.fb->drm_format == DRM_FORMAT_XRGB8888 &&
> - cvt->dst.fb->drm_format == DRM_FORMAT_NV12);
> + igt_format_is_yuv(cvt->dst.fb->drm_format));
>
> - for (i = 0; i < cvt->dst.fb->height / 2; i++) {
> - for (j = 0; j < cvt->dst.fb->width / 2; j++) {
> - /* Convert 2x2 pixel blocks */
> - struct igt_vec4 rgb[4];
> - struct igt_vec4 yuv[4];
> + get_yuv_parameters(cvt->dst.fb, ¶ms);
> + y = cvt->dst.ptr + params.y_offset;
> + u = cvt->dst.ptr + params.u_offset;
> + v = cvt->dst.ptr + params.v_offset;
>
> - read_rgb(&rgb[0], &rgb24[j * 8 + 0]);
> - read_rgb(&rgb[1], &rgb24[j * 8 + 4]);
> - read_rgb(&rgb[2], &rgb24[j * 8 + 0 + rgb24_stride]);
> - read_rgb(&rgb[3], &rgb24[j * 8 + 4 + rgb24_stride]);
> -
> - yuv[0] = igt_matrix_transform(&m, &rgb[0]);
> - yuv[1] = igt_matrix_transform(&m, &rgb[1]);
> - yuv[2] = igt_matrix_transform(&m, &rgb[2]);
> - yuv[3] = igt_matrix_transform(&m, &rgb[3]);
> -
> - y[j * 2 + 0] = yuv[0].d[0];
> - y[j * 2 + 1] = yuv[1].d[0];
> - y[j * 2 + 0 + planar_stride] = yuv[2].d[0];
> - y[j * 2 + 1 + planar_stride] = yuv[3].d[0];
> + for (i = 0; i < cvt->dst.fb->height; i++) {
> + const uint8_t *rgb_tmp = rgb24;
> + uint8_t *y_tmp = y;
> + uint8_t *u_tmp = u;
> + uint8_t *v_tmp = v;
>
> - /*
> - * We assume the MPEG2 chroma siting convention, where
> - * pixel center for Cb'Cr' is between the left top and
> - * bottom pixel in a 2x2 block, so take the average.
> - */
> - uv[j * 2 + 0] = (yuv[0].d[1] + yuv[2].d[1]) / 2.0f;
> - uv[j * 2 + 1] = (yuv[0].d[2] + yuv[2].d[2]) / 2.0f;
> - }
> + for (j = 0; j < cvt->dst.fb->width; j++) {
> + const uint8_t *pair_rgb24 = rgb_tmp;
> + struct igt_vec4 pair_rgb, rgb;
> + struct igt_vec4 pair_yuv, yuv;
>
> - if (cvt->dst.fb->width & 1) {
> - /* Convert 1x2 pixel block */
> - struct igt_vec4 rgb[2];
> - struct igt_vec4 yuv[2];
> + read_rgb(&rgb, rgb_tmp);
> + yuv = igt_matrix_transform(&m, &rgb);
>
> - read_rgb(&rgb[0], &rgb24[j * 8 + 0]);
> - read_rgb(&rgb[2], &rgb24[j * 8 + 0 + rgb24_stride]);
> + rgb_tmp += bpp;
>
> - yuv[0] = igt_matrix_transform(&m, &rgb[0]);
> - yuv[1] = igt_matrix_transform(&m, &rgb[1]);
> + *y_tmp = yuv.d[0];
> + y_tmp += params.y_inc;
>
> - y[j * 2 + 0] = yuv[0].d[0];
> - y[j * 2 + 0 + planar_stride] = yuv[1].d[0];
> + if ((i % dst_fmt->vsub) || (j % dst_fmt->hsub))
> + continue;
>
> /*
> * We assume the MPEG2 chroma siting convention, where
> * pixel center for Cb'Cr' is between the left top and
> * bottom pixel in a 2x2 block, so take the average.
> + *
> + * Therefore, if we use subsampling, we only really care
> + * about two pixels all the time, either the two
> + * subsequent pixels horizontally, vertically, or the
> + * two corners in a 2x2 block.
> + *
> + * The only corner case is when we have an odd number of
> + * pixels, but this can be handled pretty easily by not
> + * incrementing the paired pixel pointer in the
> + * direction it's odd in.
> */
> - uv[j * 2 + 0] = (yuv[0].d[1] + yuv[1].d[1]) / 2.0f;
> - uv[j * 2 + 1] = (yuv[0].d[2] + yuv[1].d[2]) / 2.0f;
> - }
> -
> - rgb24 += 2 * rgb24_stride;
> - y += 2 * planar_stride;
> - uv += planar_stride;
> - }
> -
> - /* Last row cannot be interpolated between 2 pixels, take the single value */
> - if (cvt->dst.fb->height & 1) {
> - for (j = 0; j < cvt->dst.fb->width / 2; j++) {
> - /* Convert 2x1 pixel blocks */
> - struct igt_vec4 rgb[2];
> - struct igt_vec4 yuv[2];
> -
> - read_rgb(&rgb[0], &rgb24[j * 8 + 0]);
> - read_rgb(&rgb[1], &rgb24[j * 8 + 4]);
> -
> - yuv[0] = igt_matrix_transform(&m, &rgb[0]);
> - yuv[1] = igt_matrix_transform(&m, &rgb[1]);
> + if (j != (cvt->dst.fb->width - 1))
> + pair_rgb24 += (dst_fmt->hsub - 1) * bpp;
>
> - y[j * 2 + 0] = yuv[0].d[0];
> - y[j * 2 + 1] = yuv[1].d[0];
> - uv[j * 2 + 0] = yuv[0].d[1];
> - uv[j * 2 + 1] = yuv[0].d[2];
> - }
> -
> - if (cvt->dst.fb->width & 1) {
> - /* Convert single pixel */
> - struct igt_vec4 rgb;
> - struct igt_vec4 yuv;
> + if (i != (cvt->dst.fb->height - 1))
> + pair_rgb24 += rgb24_stride * (dst_fmt->vsub - 1);
>
> - read_rgb(&rgb, &rgb24[j * 8 + 0]);
> + read_rgb(&pair_rgb, pair_rgb24);
> + pair_yuv = igt_matrix_transform(&m, &pair_rgb);
>
> - yuv = igt_matrix_transform(&m, &rgb);
> + *u_tmp = (yuv.d[1] + pair_yuv.d[1]) / 2.0f;
> + *v_tmp = (yuv.d[2] + pair_yuv.d[2]) / 2.0f;
>
> - y[j * 2 + 0] = yuv.d[0];
> - uv[j * 2 + 0] = yuv.d[1];
> - uv[j * 2 + 1] = yuv.d[2];
> - }
> - }
> -}
> -
> -/* { Y0, U, Y1, V } */
> -static const unsigned char swizzle_yuyv[] = { 0, 1, 2, 3 };
> -static const unsigned char swizzle_yvyu[] = { 0, 3, 2, 1 };
> -static const unsigned char swizzle_uyvy[] = { 1, 0, 3, 2 };
> -static const unsigned char swizzle_vyuy[] = { 1, 2, 3, 0 };
> -
> -static const unsigned char *yuyv_swizzle(uint32_t format)
> -{
> - switch (format) {
> - default:
> - case DRM_FORMAT_YUYV:
> - return swizzle_yuyv;
> - case DRM_FORMAT_YVYU:
> - return swizzle_yvyu;
> - case DRM_FORMAT_UYVY:
> - return swizzle_uyvy;
> - case DRM_FORMAT_VYUY:
> - return swizzle_vyuy;
> - }
> -}
> -
> -static void convert_yuyv_to_rgb24(struct fb_convert *cvt)
> -{
> - int i, j;
> - const uint8_t *yuyv;
> - uint8_t *rgb24 = cvt->dst.ptr;
> - unsigned int rgb24_stride = cvt->dst.fb->strides[0];
> - unsigned int yuyv_stride = cvt->src.fb->strides[0];
> - struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.fb->color_encoding,
> - cvt->src.fb->color_range);
> - const unsigned char *swz = yuyv_swizzle(cvt->src.fb->drm_format);
> - uint8_t *buf;
> -
> - igt_assert((cvt->src.fb->drm_format == DRM_FORMAT_YUYV ||
> - cvt->src.fb->drm_format == DRM_FORMAT_UYVY ||
> - cvt->src.fb->drm_format == DRM_FORMAT_YVYU ||
> - cvt->src.fb->drm_format == DRM_FORMAT_VYUY) &&
> - cvt->dst.fb->drm_format == DRM_FORMAT_XRGB8888);
> -
> - buf = convert_src_get(cvt);
> - yuyv = buf;
> -
> - for (i = 0; i < cvt->dst.fb->height; i++) {
> - for (j = 0; j < cvt->dst.fb->width / 2; j++) {
> - /* Convert 2x1 pixel blocks */
> - struct igt_vec4 yuv[2];
> - struct igt_vec4 rgb[2];
> -
> - yuv[0].d[0] = yuyv[j * 4 + swz[0]];
> - yuv[1].d[0] = yuyv[j * 4 + swz[2]];
> - yuv[0].d[1] = yuv[1].d[1] = yuyv[j * 4 + swz[1]];
> - yuv[0].d[2] = yuv[1].d[2] = yuyv[j * 4 + swz[3]];
> - yuv[0].d[3] = yuv[1].d[3] = 1.0f;
> -
> - rgb[0] = igt_matrix_transform(&m, &yuv[0]);
> - rgb[1] = igt_matrix_transform(&m, &yuv[1]);
> -
> - write_rgb(&rgb24[j * 8 + 0], &rgb[0]);
> - write_rgb(&rgb24[j * 8 + 4], &rgb[1]);
> - }
> -
> - if (cvt->dst.fb->width & 1) {
> - struct igt_vec4 yuv;
> - struct igt_vec4 rgb;
> -
> - yuv.d[0] = yuyv[j * 4 + swz[0]];
> - yuv.d[1] = yuyv[j * 4 + swz[1]];
> - yuv.d[2] = yuyv[j * 4 + swz[3]];
> - yuv.d[3] = 1.0f;
> -
> - rgb = igt_matrix_transform(&m, &yuv);
> -
> - write_rgb(&rgb24[j * 8 + 0], &rgb);
> + u_tmp += params.uv_inc;
> + v_tmp += params.uv_inc;
> }
>
> rgb24 += rgb24_stride;
> - yuyv += yuyv_stride;
> - }
> -
> - convert_src_put(cvt, buf);
> -}
> -
> -static void convert_rgb24_to_yuyv(struct fb_convert *cvt)
> -{
> - int i, j;
> - uint8_t *yuyv = cvt->dst.ptr;
> - const uint8_t *rgb24 = cvt->src.ptr;
> - unsigned rgb24_stride = cvt->src.fb->strides[0];
> - unsigned yuyv_stride = cvt->dst.fb->strides[0];
> - struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.fb->color_encoding,
> - cvt->dst.fb->color_range);
> - const unsigned char *swz = yuyv_swizzle(cvt->dst.fb->drm_format);
> -
> - igt_assert(cvt->src.fb->drm_format == DRM_FORMAT_XRGB8888 &&
> - (cvt->dst.fb->drm_format == DRM_FORMAT_YUYV ||
> - cvt->dst.fb->drm_format == DRM_FORMAT_UYVY ||
> - cvt->dst.fb->drm_format == DRM_FORMAT_YVYU ||
> - cvt->dst.fb->drm_format == DRM_FORMAT_VYUY));
> -
> - for (i = 0; i < cvt->dst.fb->height; i++) {
> - for (j = 0; j < cvt->dst.fb->width / 2; j++) {
> - /* Convert 2x1 pixel blocks */
> - struct igt_vec4 rgb[2];
> - struct igt_vec4 yuv[2];
> -
> - read_rgb(&rgb[0], &rgb24[j * 8 + 0]);
> - read_rgb(&rgb[1], &rgb24[j * 8 + 4]);
> -
> - yuv[0] = igt_matrix_transform(&m, &rgb[0]);
> - yuv[1] = igt_matrix_transform(&m, &rgb[1]);
> + y += params.y_stride;
>
> - yuyv[j * 4 + swz[0]] = yuv[0].d[0];
> - yuyv[j * 4 + swz[2]] = yuv[1].d[0];
> - yuyv[j * 4 + swz[1]] = (yuv[0].d[1] + yuv[1].d[1]) / 2.0f;
> - yuyv[j * 4 + swz[3]] = (yuv[0].d[2] + yuv[1].d[2]) / 2.0f;
> + if ((i % dst_fmt->vsub) == (dst_fmt->vsub - 1)) {
> + u += params.uv_stride;
> + v += params.uv_stride;
> }
> -
> - if (cvt->dst.fb->width & 1) {
> - struct igt_vec4 rgb;
> - struct igt_vec4 yuv;
> -
> - read_rgb(&rgb, &rgb24[j * 8 + 0]);
> -
> - yuv = igt_matrix_transform(&m, &rgb);
> -
> - yuyv[j * 4 + swz[0]] = yuv.d[0];
> - yuyv[j * 4 + swz[1]] = yuv.d[1];
> - yuyv[j * 4 + swz[3]] = yuv.d[2];
> - }
> -
> - rgb24 += rgb24_stride;
> - yuyv += yuyv_stride;
> }
> }
>
> @@ -2046,31 +1859,23 @@ static void fb_convert(struct fb_convert *cvt)
> } else if (cvt->dst.fb->drm_format == DRM_FORMAT_XRGB8888) {
> switch (cvt->src.fb->drm_format) {
> case DRM_FORMAT_XYUV8888:
> - convert_yuv444_to_rgb24(cvt);
> - return;
> case DRM_FORMAT_NV12:
> - convert_nv12_to_rgb24(cvt);
> - return;
> - case DRM_FORMAT_YUYV:
> - case DRM_FORMAT_YVYU:
> case DRM_FORMAT_UYVY:
> case DRM_FORMAT_VYUY:
> - convert_yuyv_to_rgb24(cvt);
> + case DRM_FORMAT_YUYV:
> + case DRM_FORMAT_YVYU:
> + convert_yuv_to_rgb24(cvt);
> return;
> }
> } else if (cvt->src.fb->drm_format == DRM_FORMAT_XRGB8888) {
> switch (cvt->dst.fb->drm_format) {
> case DRM_FORMAT_XYUV8888:
> - convert_rgb24_to_yuv444(cvt);
> - return;
> case DRM_FORMAT_NV12:
> - convert_rgb24_to_nv12(cvt);
> - return;
> case DRM_FORMAT_YUYV:
> case DRM_FORMAT_YVYU:
> case DRM_FORMAT_UYVY:
> case DRM_FORMAT_VYUY:
> - convert_rgb24_to_yuyv(cvt);
> + convert_rgb24_to_yuv(cvt);
> return;
> }
> }
--
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [igt-dev] [PATCH i-g-t v2 04/13] igt: fb: Move i915 YUV buffer clearing code to a function
2019-01-08 15:19 [igt-dev] [PATCH i-g-t v2 00/13] igt: chamelium: Test YUV buffers using the Chamelium Maxime Ripard
` (2 preceding siblings ...)
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 03/13] igt: fb: generic YUV convertion function Maxime Ripard
@ 2019-01-08 15:19 ` Maxime Ripard
2019-01-10 10:10 ` Paul Kocialkowski
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 05/13] igt: fb: Move size computation to the common path Maxime Ripard
` (11 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2019-01-08 15:19 UTC (permalink / raw)
To: igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
The YUV buffer allocation path for the i915 driver also has some code to
clear a YUV buffer.
As that is going to be useful for all the other drivers, let's move it to
a separate function.
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
lib/igt_fb.c | 80 +++++++++++++++++++++++++++-------------------------
1 file changed, 42 insertions(+), 38 deletions(-)
diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 45691fd441d9..d69c3fb2d38d 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -484,6 +484,47 @@ uint64_t igt_fb_tiling_to_mod(uint64_t tiling)
}
}
+static void clear_yuv_buffer(struct igt_fb *fb)
+{
+ bool full_range = fb->color_range == IGT_COLOR_YCBCR_FULL_RANGE;
+ void *ptr;
+
+ igt_assert(igt_format_is_yuv(fb->drm_format));
+
+ /* Ensure the framebuffer is preallocated */
+ ptr = igt_fb_map_buffer(fb->fd, fb);
+ 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_XYUV8888:
+ wmemset(ptr + fb->offsets[0], full_range ? 0x00008080 : 0x00108080,
+ fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
+ 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;
+ }
+
+ igt_fb_unmap_buffer(fb, ptr);
+}
+
/* helpers to create nice-looking framebuffers */
static int create_bo_for_fb(struct igt_fb *fb)
{
@@ -501,50 +542,13 @@ 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_XYUV8888:
- wmemset(ptr + fb->offsets[0], full_range ? 0x00008080 : 0x00108080,
- fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
- 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);
+ clear_yuv_buffer(fd);
return fb->gem_handle;
} else {
--
2.20.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 04/13] igt: fb: Move i915 YUV buffer clearing code to a function
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 04/13] igt: fb: Move i915 YUV buffer clearing code to a function Maxime Ripard
@ 2019-01-10 10:10 ` Paul Kocialkowski
2019-01-10 14:38 ` Ville Syrjälä
0 siblings, 1 reply; 36+ messages in thread
From: Paul Kocialkowski @ 2019-01-10 10:10 UTC (permalink / raw)
To: Maxime Ripard, igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> The YUV buffer allocation path for the i915 driver also has some code to
> clear a YUV buffer.
>
> As that is going to be useful for all the other drivers, let's move it to
> a separate function.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
I'm not really sure why clearing the buffers is a useful thing to do
and why we should only do it for YUV and not RGB formats.
Anyway, since this code was already there and the refactoring
definitely makes sense:
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Cheers,
Paul
> ---
> lib/igt_fb.c | 80 +++++++++++++++++++++++++++-------------------------
> 1 file changed, 42 insertions(+), 38 deletions(-)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 45691fd441d9..d69c3fb2d38d 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -484,6 +484,47 @@ uint64_t igt_fb_tiling_to_mod(uint64_t tiling)
> }
> }
>
> +static void clear_yuv_buffer(struct igt_fb *fb)
> +{
> + bool full_range = fb->color_range == IGT_COLOR_YCBCR_FULL_RANGE;
> + void *ptr;
> +
> + igt_assert(igt_format_is_yuv(fb->drm_format));
> +
> + /* Ensure the framebuffer is preallocated */
> + ptr = igt_fb_map_buffer(fb->fd, fb);
> + 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_XYUV8888:
> + wmemset(ptr + fb->offsets[0], full_range ? 0x00008080 : 0x00108080,
> + fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
> + 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;
> + }
> +
> + igt_fb_unmap_buffer(fb, ptr);
> +}
> +
> /* helpers to create nice-looking framebuffers */
> static int create_bo_for_fb(struct igt_fb *fb)
> {
> @@ -501,50 +542,13 @@ 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_XYUV8888:
> - wmemset(ptr + fb->offsets[0], full_range ? 0x00008080 : 0x00108080,
> - fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
> - 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);
> + clear_yuv_buffer(fd);
>
> return fb->gem_handle;
> } else {
--
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
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 04/13] igt: fb: Move i915 YUV buffer clearing code to a function
2019-01-10 10:10 ` Paul Kocialkowski
@ 2019-01-10 14:38 ` Ville Syrjälä
2019-01-14 15:13 ` Paul Kocialkowski
0 siblings, 1 reply; 36+ messages in thread
From: Ville Syrjälä @ 2019-01-10 14:38 UTC (permalink / raw)
To: Paul Kocialkowski; +Cc: Petri Latvala, eben, igt-dev, Thomas Petazzoni
On Thu, Jan 10, 2019 at 11:10:09AM +0100, Paul Kocialkowski wrote:
> On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> > The YUV buffer allocation path for the i915 driver also has some code to
> > clear a YUV buffer.
> >
> > As that is going to be useful for all the other drivers, let's move it to
> > a separate function.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
>
> I'm not really sure why clearing the buffers is a useful thing to do
> and why we should only do it for YUV and not RGB formats.
0 = black for RGB, 0 != black for YUV. The memset was just to make
things more consistent. But not sure if any test would actually
fail if we just drop the memset.
>
> Anyway, since this code was already there and the refactoring
> definitely makes sense:
>
> Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
>
> Cheers,
>
> Paul
>
> > ---
> > lib/igt_fb.c | 80 +++++++++++++++++++++++++++-------------------------
> > 1 file changed, 42 insertions(+), 38 deletions(-)
> >
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 45691fd441d9..d69c3fb2d38d 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -484,6 +484,47 @@ uint64_t igt_fb_tiling_to_mod(uint64_t tiling)
> > }
> > }
> >
> > +static void clear_yuv_buffer(struct igt_fb *fb)
> > +{
> > + bool full_range = fb->color_range == IGT_COLOR_YCBCR_FULL_RANGE;
> > + void *ptr;
> > +
> > + igt_assert(igt_format_is_yuv(fb->drm_format));
> > +
> > + /* Ensure the framebuffer is preallocated */
> > + ptr = igt_fb_map_buffer(fb->fd, fb);
> > + 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_XYUV8888:
> > + wmemset(ptr + fb->offsets[0], full_range ? 0x00008080 : 0x00108080,
> > + fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
> > + 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;
> > + }
> > +
> > + igt_fb_unmap_buffer(fb, ptr);
> > +}
> > +
> > /* helpers to create nice-looking framebuffers */
> > static int create_bo_for_fb(struct igt_fb *fb)
> > {
> > @@ -501,50 +542,13 @@ 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_XYUV8888:
> > - wmemset(ptr + fb->offsets[0], full_range ? 0x00008080 : 0x00108080,
> > - fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
> > - 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);
> > + clear_yuv_buffer(fd);
> >
> > return fb->gem_handle;
> > } else {
> --
> Paul Kocialkowski, Bootlin
> Embedded Linux and kernel engineering
> https://bootlin.com
--
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 04/13] igt: fb: Move i915 YUV buffer clearing code to a function
2019-01-10 14:38 ` Ville Syrjälä
@ 2019-01-14 15:13 ` Paul Kocialkowski
2019-01-14 16:40 ` Ville Syrjälä
0 siblings, 1 reply; 36+ messages in thread
From: Paul Kocialkowski @ 2019-01-14 15:13 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Petri Latvala, eben, igt-dev, Thomas Petazzoni
Hi,
On Thu, 2019-01-10 at 16:38 +0200, Ville Syrjälä wrote:
> On Thu, Jan 10, 2019 at 11:10:09AM +0100, Paul Kocialkowski wrote:
> > On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> > > The YUV buffer allocation path for the i915 driver also has some code to
> > > clear a YUV buffer.
> > >
> > > As that is going to be useful for all the other drivers, let's move it to
> > > a separate function.
> > >
> > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> >
> > I'm not really sure why clearing the buffers is a useful thing to do
> > and why we should only do it for YUV and not RGB formats.
>
> 0 = black for RGB, 0 != black for YUV. The memset was just to make
> things more consistent. But not sure if any test would actually
> fail if we just drop the memset.
Right, I understand that this is required for setting the framebuffer
to black, but I'm just wondering why this has to be done in the first
place.
As far as I know, (RGB) buffers allocated with GEM are not zero-ed by
the allocator, nor by any component in IGT. So I'm wondering why we're
taking the extra time for clearing buffers in the YUV case.
Cheers,
Paul
> > Anyway, since this code was already there and the refactoring
> > definitely makes sense:
> >
> > Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> >
> > Cheers,
> >
> > Paul
> >
> > > ---
> > > lib/igt_fb.c | 80 +++++++++++++++++++++++++++-------------------------
> > > 1 file changed, 42 insertions(+), 38 deletions(-)
> > >
> > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > index 45691fd441d9..d69c3fb2d38d 100644
> > > --- a/lib/igt_fb.c
> > > +++ b/lib/igt_fb.c
> > > @@ -484,6 +484,47 @@ uint64_t igt_fb_tiling_to_mod(uint64_t tiling)
> > > }
> > > }
> > >
> > > +static void clear_yuv_buffer(struct igt_fb *fb)
> > > +{
> > > + bool full_range = fb->color_range == IGT_COLOR_YCBCR_FULL_RANGE;
> > > + void *ptr;
> > > +
> > > + igt_assert(igt_format_is_yuv(fb->drm_format));
> > > +
> > > + /* Ensure the framebuffer is preallocated */
> > > + ptr = igt_fb_map_buffer(fb->fd, fb);
> > > + 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_XYUV8888:
> > > + wmemset(ptr + fb->offsets[0], full_range ? 0x00008080 : 0x00108080,
> > > + fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
> > > + 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;
> > > + }
> > > +
> > > + igt_fb_unmap_buffer(fb, ptr);
> > > +}
> > > +
> > > /* helpers to create nice-looking framebuffers */
> > > static int create_bo_for_fb(struct igt_fb *fb)
> > > {
> > > @@ -501,50 +542,13 @@ 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_XYUV8888:
> > > - wmemset(ptr + fb->offsets[0], full_range ? 0x00008080 : 0x00108080,
> > > - fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
> > > - 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);
> > > + clear_yuv_buffer(fd);
> > >
> > > return fb->gem_handle;
> > > } else {
> > --
> > Paul Kocialkowski, Bootlin
> > Embedded Linux and kernel engineering
> > https://bootlin.com
--
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
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 04/13] igt: fb: Move i915 YUV buffer clearing code to a function
2019-01-14 15:13 ` Paul Kocialkowski
@ 2019-01-14 16:40 ` Ville Syrjälä
2019-01-15 16:15 ` Paul Kocialkowski
0 siblings, 1 reply; 36+ messages in thread
From: Ville Syrjälä @ 2019-01-14 16:40 UTC (permalink / raw)
To: Paul Kocialkowski; +Cc: Petri Latvala, eben, igt-dev, Thomas Petazzoni
On Mon, Jan 14, 2019 at 04:13:02PM +0100, Paul Kocialkowski wrote:
> Hi,
>
> On Thu, 2019-01-10 at 16:38 +0200, Ville Syrjälä wrote:
> > On Thu, Jan 10, 2019 at 11:10:09AM +0100, Paul Kocialkowski wrote:
> > > On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> > > > The YUV buffer allocation path for the i915 driver also has some code to
> > > > clear a YUV buffer.
> > > >
> > > > As that is going to be useful for all the other drivers, let's move it to
> > > > a separate function.
> > > >
> > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > >
> > > I'm not really sure why clearing the buffers is a useful thing to do
> > > and why we should only do it for YUV and not RGB formats.
> >
> > 0 = black for RGB, 0 != black for YUV. The memset was just to make
> > things more consistent. But not sure if any test would actually
> > fail if we just drop the memset.
>
> Right, I understand that this is required for setting the framebuffer
> to black, but I'm just wondering why this has to be done in the first
> place.
>
> As far as I know, (RGB) buffers allocated with GEM are not zero-ed by
> the allocator,
They are zeroed. You wouldn't want the kernel to leak random
memory contents all over.
> nor by any component in IGT. So I'm wondering why we're
> taking the extra time for clearing buffers in the YUV case.
>
> Cheers,
>
> Paul
>
> > > Anyway, since this code was already there and the refactoring
> > > definitely makes sense:
> > >
> > > Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > >
> > > Cheers,
> > >
> > > Paul
> > >
> > > > ---
> > > > lib/igt_fb.c | 80 +++++++++++++++++++++++++++-------------------------
> > > > 1 file changed, 42 insertions(+), 38 deletions(-)
> > > >
> > > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > > index 45691fd441d9..d69c3fb2d38d 100644
> > > > --- a/lib/igt_fb.c
> > > > +++ b/lib/igt_fb.c
> > > > @@ -484,6 +484,47 @@ uint64_t igt_fb_tiling_to_mod(uint64_t tiling)
> > > > }
> > > > }
> > > >
> > > > +static void clear_yuv_buffer(struct igt_fb *fb)
> > > > +{
> > > > + bool full_range = fb->color_range == IGT_COLOR_YCBCR_FULL_RANGE;
> > > > + void *ptr;
> > > > +
> > > > + igt_assert(igt_format_is_yuv(fb->drm_format));
> > > > +
> > > > + /* Ensure the framebuffer is preallocated */
> > > > + ptr = igt_fb_map_buffer(fb->fd, fb);
> > > > + 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_XYUV8888:
> > > > + wmemset(ptr + fb->offsets[0], full_range ? 0x00008080 : 0x00108080,
> > > > + fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
> > > > + 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;
> > > > + }
> > > > +
> > > > + igt_fb_unmap_buffer(fb, ptr);
> > > > +}
> > > > +
> > > > /* helpers to create nice-looking framebuffers */
> > > > static int create_bo_for_fb(struct igt_fb *fb)
> > > > {
> > > > @@ -501,50 +542,13 @@ 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_XYUV8888:
> > > > - wmemset(ptr + fb->offsets[0], full_range ? 0x00008080 : 0x00108080,
> > > > - fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
> > > > - 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);
> > > > + clear_yuv_buffer(fd);
> > > >
> > > > return fb->gem_handle;
> > > > } else {
> > > --
> > > Paul Kocialkowski, Bootlin
> > > Embedded Linux and kernel engineering
> > > https://bootlin.com
> --
> Paul Kocialkowski, Bootlin
> Embedded Linux and kernel engineering
> https://bootlin.com
--
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 04/13] igt: fb: Move i915 YUV buffer clearing code to a function
2019-01-14 16:40 ` Ville Syrjälä
@ 2019-01-15 16:15 ` Paul Kocialkowski
0 siblings, 0 replies; 36+ messages in thread
From: Paul Kocialkowski @ 2019-01-15 16:15 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Petri Latvala, eben, igt-dev, Thomas Petazzoni
Hi,
On Mon, 2019-01-14 at 18:40 +0200, Ville Syrjälä wrote:
> On Mon, Jan 14, 2019 at 04:13:02PM +0100, Paul Kocialkowski wrote:
> > Hi,
> >
> > On Thu, 2019-01-10 at 16:38 +0200, Ville Syrjälä wrote:
> > > On Thu, Jan 10, 2019 at 11:10:09AM +0100, Paul Kocialkowski wrote:
> > > > On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> > > > > The YUV buffer allocation path for the i915 driver also has some code to
> > > > > clear a YUV buffer.
> > > > >
> > > > > As that is going to be useful for all the other drivers, let's move it to
> > > > > a separate function.
> > > > >
> > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > > >
> > > > I'm not really sure why clearing the buffers is a useful thing to do
> > > > and why we should only do it for YUV and not RGB formats.
> > >
> > > 0 = black for RGB, 0 != black for YUV. The memset was just to make
> > > things more consistent. But not sure if any test would actually
> > > fail if we just drop the memset.
> >
> > Right, I understand that this is required for setting the framebuffer
> > to black, but I'm just wondering why this has to be done in the first
> > place.
> >
> > As far as I know, (RGB) buffers allocated with GEM are not zero-ed by
> > the allocator,
>
> They are zeroed. You wouldn't want the kernel to leak random
> memory contents all over.
Oh my mistake! It definitely makes sense that the kernel does that.
Let's keep things consistent and have all buffers initialized with
black then.
Cheers,
Paul
> > nor by any component in IGT. So I'm wondering why we're
> > taking the extra time for clearing buffers in the YUV case.
> >
> > Cheers,
> >
> > Paul
> >
> > > > Anyway, since this code was already there and the refactoring
> > > > definitely makes sense:
> > > >
> > > > Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > >
> > > > Cheers,
> > > >
> > > > Paul
> > > >
> > > > > ---
> > > > > lib/igt_fb.c | 80 +++++++++++++++++++++++++++-------------------------
> > > > > 1 file changed, 42 insertions(+), 38 deletions(-)
> > > > >
> > > > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > > > index 45691fd441d9..d69c3fb2d38d 100644
> > > > > --- a/lib/igt_fb.c
> > > > > +++ b/lib/igt_fb.c
> > > > > @@ -484,6 +484,47 @@ uint64_t igt_fb_tiling_to_mod(uint64_t tiling)
> > > > > }
> > > > > }
> > > > >
> > > > > +static void clear_yuv_buffer(struct igt_fb *fb)
> > > > > +{
> > > > > + bool full_range = fb->color_range == IGT_COLOR_YCBCR_FULL_RANGE;
> > > > > + void *ptr;
> > > > > +
> > > > > + igt_assert(igt_format_is_yuv(fb->drm_format));
> > > > > +
> > > > > + /* Ensure the framebuffer is preallocated */
> > > > > + ptr = igt_fb_map_buffer(fb->fd, fb);
> > > > > + 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_XYUV8888:
> > > > > + wmemset(ptr + fb->offsets[0], full_range ? 0x00008080 : 0x00108080,
> > > > > + fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
> > > > > + 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;
> > > > > + }
> > > > > +
> > > > > + igt_fb_unmap_buffer(fb, ptr);
> > > > > +}
> > > > > +
> > > > > /* helpers to create nice-looking framebuffers */
> > > > > static int create_bo_for_fb(struct igt_fb *fb)
> > > > > {
> > > > > @@ -501,50 +542,13 @@ 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_XYUV8888:
> > > > > - wmemset(ptr + fb->offsets[0], full_range ? 0x00008080 : 0x00108080,
> > > > > - fb->strides[0] * fb->plane_height[0] / sizeof(wchar_t));
> > > > > - 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);
> > > > > + clear_yuv_buffer(fd);
> > > > >
> > > > > return fb->gem_handle;
> > > > > } else {
> > > > --
> > > > Paul Kocialkowski, Bootlin
> > > > Embedded Linux and kernel engineering
> > > > https://bootlin.com
> > --
> > Paul Kocialkowski, Bootlin
> > Embedded Linux and kernel engineering
> > https://bootlin.com
--
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [igt-dev] [PATCH i-g-t v2 05/13] igt: fb: Move size computation to the common path
2019-01-08 15:19 [igt-dev] [PATCH i-g-t v2 00/13] igt: chamelium: Test YUV buffers using the Chamelium Maxime Ripard
` (3 preceding siblings ...)
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 04/13] igt: fb: Move i915 YUV buffer clearing code to a function Maxime Ripard
@ 2019-01-08 15:19 ` Maxime Ripard
2019-01-10 10:10 ` Paul Kocialkowski
2019-01-10 10:43 ` Paul Kocialkowski
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 06/13] igt: fb: Refactor dumb buffer allocation path Maxime Ripard
` (10 subsequent siblings)
15 siblings, 2 replies; 36+ messages in thread
From: Maxime Ripard @ 2019-01-08 15:19 UTC (permalink / raw)
To: igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
In order to properly support the YUV buffer on non-i915 platforms, we need
to run calc_fb_size for the dumb buffers allocation path too. Move it out
of the special case and in the common code path.
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
lib/igt_fb.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index d69c3fb2d38d..31e44d6188d9 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -528,16 +528,14 @@ 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)
{
+ 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 (fb->tiling || fb->size || fb->strides[0] || igt_format_is_yuv(fb->drm_format)) {
- uint64_t size;
-
- size = calc_fb_size(fb);
-
- /* respect the size requested by the caller */
- if (fb->size == 0)
- fb->size = size;
fb->is_dumb = false;
--
2.20.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 05/13] igt: fb: Move size computation to the common path
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 05/13] igt: fb: Move size computation to the common path Maxime Ripard
@ 2019-01-10 10:10 ` Paul Kocialkowski
2019-01-10 10:43 ` Paul Kocialkowski
1 sibling, 0 replies; 36+ messages in thread
From: Paul Kocialkowski @ 2019-01-10 10:10 UTC (permalink / raw)
To: Maxime Ripard, igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> In order to properly support the YUV buffer on non-i915 platforms, we need
> to run calc_fb_size for the dumb buffers allocation path too. Move it out
> of the special case and in the common code path.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
> lib/igt_fb.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index d69c3fb2d38d..31e44d6188d9 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -528,16 +528,14 @@ 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)
> {
> + 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 (fb->tiling || fb->size || fb->strides[0] || igt_format_is_yuv(fb->drm_format)) {
> - uint64_t size;
> -
> - size = calc_fb_size(fb);
> -
> - /* respect the size requested by the caller */
> - if (fb->size == 0)
> - fb->size = size;
>
> fb->is_dumb = false;
>
--
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
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 05/13] igt: fb: Move size computation to the common path
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 05/13] igt: fb: Move size computation to the common path Maxime Ripard
2019-01-10 10:10 ` Paul Kocialkowski
@ 2019-01-10 10:43 ` Paul Kocialkowski
1 sibling, 0 replies; 36+ messages in thread
From: Paul Kocialkowski @ 2019-01-10 10:43 UTC (permalink / raw)
To: Maxime Ripard, igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
Hi,
On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> In order to properly support the YUV buffer on non-i915 platforms, we need
> to run calc_fb_size for the dumb buffers allocation path too. Move it out
> of the special case and in the common code path.
Actually, I have one minor comment below.
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
> lib/igt_fb.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index d69c3fb2d38d..31e44d6188d9 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -528,16 +528,14 @@ 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)
> {
> + 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 (fb->tiling || fb->size || fb->strides[0] || igt_format_is_yuv(fb->drm_format)) {
Calling calc_fb_size when the function begins will automatically set
the strides if it was previously 0, so we can drop the "|| fb-
>strides[0]" here (will never happen).
Generally speaking, I think it would be much more simple to always go
with i915 gem create instead of dumb alloc as soon as i915 is detected
and drop all these conditions.
I can't see why dumb buffer would be preferable, but I might be missing
something here (i915 developers, feel free to point it out if that's
the case)!
Cheers,
Paul
> - uint64_t size;
> -
> - size = calc_fb_size(fb);
> -
> - /* respect the size requested by the caller */
> - if (fb->size == 0)
> - fb->size = size;
>
> fb->is_dumb = false;
>
--
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [igt-dev] [PATCH i-g-t v2 06/13] igt: fb: Refactor dumb buffer allocation path
2019-01-08 15:19 [igt-dev] [PATCH i-g-t v2 00/13] igt: chamelium: Test YUV buffers using the Chamelium Maxime Ripard
` (4 preceding siblings ...)
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 05/13] igt: fb: Move size computation to the common path Maxime Ripard
@ 2019-01-08 15:19 ` Maxime Ripard
2019-01-10 10:11 ` Paul Kocialkowski
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 07/13] igt: fb: Account for all planes bpp Maxime Ripard
` (9 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2019-01-08 15:19 UTC (permalink / raw)
To: igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
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.
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 31e44d6188d9..2c33a899bc04 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -555,15 +555,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
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 06/13] igt: fb: Refactor dumb buffer allocation path
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 06/13] igt: fb: Refactor dumb buffer allocation path Maxime Ripard
@ 2019-01-10 10:11 ` Paul Kocialkowski
0 siblings, 0 replies; 36+ messages in thread
From: Paul Kocialkowski @ 2019-01-10 10:11 UTC (permalink / raw)
To: Maxime Ripard, igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
On Tue, 2019-01-08 at 16:19 +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.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@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 31e44d6188d9..2c33a899bc04 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -555,15 +555,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;
> }
>
> /**
--
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [igt-dev] [PATCH i-g-t v2 07/13] igt: fb: Account for all planes bpp
2019-01-08 15:19 [igt-dev] [PATCH i-g-t v2 00/13] igt: chamelium: Test YUV buffers using the Chamelium Maxime Ripard
` (5 preceding siblings ...)
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 06/13] igt: fb: Refactor dumb buffer allocation path Maxime Ripard
@ 2019-01-08 15:19 ` Maxime Ripard
2019-01-10 10:21 ` Paul Kocialkowski
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 08/13] igt: fb: Clear YUV dumb buffers Maxime Ripard
` (8 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2019-01-08 15:19 UTC (permalink / raw)
To: igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
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.
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
lib/igt_fb.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 2c33a899bc04..96d56d0e63be 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -529,6 +529,7 @@ static void clear_yuv_buffer(struct igt_fb *fb)
static int create_bo_for_fb(struct igt_fb *fb)
{
uint64_t size = calc_fb_size(fb);
+ unsigned int plane, bpp;
int fd = fb->fd;
/* respect the size requested by the caller */
@@ -557,10 +558,12 @@ static int create_bo_for_fb(struct igt_fb *fb)
}
}
+ for (plane = 0; plane < fb->num_planes; plane++)
+ bpp += fb->plane_bpp[plane];
+
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
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 07/13] igt: fb: Account for all planes bpp
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 07/13] igt: fb: Account for all planes bpp Maxime Ripard
@ 2019-01-10 10:21 ` Paul Kocialkowski
2019-01-21 13:10 ` Maxime Ripard
0 siblings, 1 reply; 36+ messages in thread
From: Paul Kocialkowski @ 2019-01-10 10:21 UTC (permalink / raw)
To: Maxime Ripard, igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
On Tue, 2019-01-08 at 16:19 +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.
As discussed off-list, this approach doesn't work well when multiple
planes and sub-sampling are involved, causing over-allocation.
For instance, NV12 requires at least width * height * 1.5.
The bpp for the second plane is 16 but with four times less pixels due
to sub-sampling. Here, this would amount to width * height * 3 at
least, so over-allocating.
I think dividing each plane's bpp by the sub-sampling before adding it
should work (with a round-up division to be on the safe side for cases
where bpp cannot be exactly dividided by hsub * vsub in the future).
What do you think?
Cheers,
Paul
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
> lib/igt_fb.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 2c33a899bc04..96d56d0e63be 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -529,6 +529,7 @@ static void clear_yuv_buffer(struct igt_fb *fb)
> static int create_bo_for_fb(struct igt_fb *fb)
> {
> uint64_t size = calc_fb_size(fb);
> + unsigned int plane, bpp;
> int fd = fb->fd;
>
> /* respect the size requested by the caller */
> @@ -557,10 +558,12 @@ static int create_bo_for_fb(struct igt_fb *fb)
> }
> }
>
> + for (plane = 0; plane < fb->num_planes; plane++)
> + bpp += fb->plane_bpp[plane];
> +
> 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;
> }
--
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
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 07/13] igt: fb: Account for all planes bpp
2019-01-10 10:21 ` Paul Kocialkowski
@ 2019-01-21 13:10 ` Maxime Ripard
0 siblings, 0 replies; 36+ messages in thread
From: Maxime Ripard @ 2019-01-21 13:10 UTC (permalink / raw)
To: Paul Kocialkowski; +Cc: Petri Latvala, eben, igt-dev, Thomas Petazzoni
[-- Attachment #1.1: Type: text/plain, Size: 1119 bytes --]
On Thu, Jan 10, 2019 at 11:21:24AM +0100, Paul Kocialkowski wrote:
> On Tue, 2019-01-08 at 16:19 +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.
>
> As discussed off-list, this approach doesn't work well when multiple
> planes and sub-sampling are involved, causing over-allocation.
>
> For instance, NV12 requires at least width * height * 1.5.
> The bpp for the second plane is 16 but with four times less pixels due
> to sub-sampling. Here, this would amount to width * height * 3 at
> least, so over-allocating.
>
> I think dividing each plane's bpp by the sub-sampling before adding it
> should work (with a round-up division to be on the safe side for cases
> where bpp cannot be exactly dividided by hsub * vsub in the future).
>
> What do you think?
It sounds great, thanks!
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [igt-dev] [PATCH i-g-t v2 08/13] igt: fb: Clear YUV dumb buffers
2019-01-08 15:19 [igt-dev] [PATCH i-g-t v2 00/13] igt: chamelium: Test YUV buffers using the Chamelium Maxime Ripard
` (6 preceding siblings ...)
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 07/13] igt: fb: Account for all planes bpp Maxime Ripard
@ 2019-01-08 15:19 ` Maxime Ripard
2019-01-10 10:27 ` Paul Kocialkowski
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 09/13] igt: fb: Rework YUV i915 allocation path Maxime Ripard
` (7 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2019-01-08 15:19 UTC (permalink / raw)
To: igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
YUV dumb buffers, just like i915 GEM buffers also need to be cleared once
allocated. Make sure it happens.
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 96d56d0e63be..263a889dbaa3 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -565,6 +565,9 @@ static int create_bo_for_fb(struct igt_fb *fb)
fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
bpp, &fb->strides[0], &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
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 08/13] igt: fb: Clear YUV dumb buffers
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 08/13] igt: fb: Clear YUV dumb buffers Maxime Ripard
@ 2019-01-10 10:27 ` Paul Kocialkowski
0 siblings, 0 replies; 36+ messages in thread
From: Paul Kocialkowski @ 2019-01-10 10:27 UTC (permalink / raw)
To: Maxime Ripard, igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
Hi,
On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> YUV dumb buffers, just like i915 GEM buffers also need to be cleared once
> allocated. Make sure it happens.
As I mentionned in the patch refactoring the function, I don't see why
it's a good thing to clear YUV buffers. Can't we just keep that for
the i915 path (assuming it had a reason to be there in the first
place)?
Cheers,
Paul
> 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 96d56d0e63be..263a889dbaa3 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -565,6 +565,9 @@ static int create_bo_for_fb(struct igt_fb *fb)
> fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
> bpp, &fb->strides[0], &fb->size);
>
> + if (igt_format_is_yuv(fb->drm_format))
> + clear_yuv_buffer(fb);
> +
> return fb->gem_handle;
> }
>
--
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [igt-dev] [PATCH i-g-t v2 09/13] igt: fb: Rework YUV i915 allocation path
2019-01-08 15:19 [igt-dev] [PATCH i-g-t v2 00/13] igt: chamelium: Test YUV buffers using the Chamelium Maxime Ripard
` (7 preceding siblings ...)
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 08/13] igt: fb: Clear YUV dumb buffers Maxime Ripard
@ 2019-01-08 15:19 ` Maxime Ripard
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 10/13] igt: fb: Add a bunch of new YUV formats Maxime Ripard
` (6 subsequent siblings)
15 siblings, 0 replies; 36+ messages in thread
From: Maxime Ripard @ 2019-01-08 15:19 UTC (permalink / raw)
To: igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
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.
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
lib/igt_fb.c | 26 ++++++++------------------
1 file changed, 8 insertions(+), 18 deletions(-)
diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 263a889dbaa3..386908b72782 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -536,26 +536,15 @@ static int create_bo_for_fb(struct igt_fb *fb)
if (fb->size == 0)
fb->size = size;
- 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))) {
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]);
- clear_yuv_buffer(fd);
-
- return fb->gem_handle;
- } else {
- bool driver_has_gem_api = false;
-
- igt_require(driver_has_gem_api);
- return -EINVAL;
- }
+ goto out;
}
for (plane = 0; plane < fb->num_planes; plane++)
@@ -565,6 +554,7 @@ static int create_bo_for_fb(struct igt_fb *fb)
fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
bpp, &fb->strides[0], &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
^ permalink raw reply related [flat|nested] 36+ messages in thread* [igt-dev] [PATCH i-g-t v2 10/13] igt: fb: Add a bunch of new YUV formats
2019-01-08 15:19 [igt-dev] [PATCH i-g-t v2 00/13] igt: chamelium: Test YUV buffers using the Chamelium Maxime Ripard
` (8 preceding siblings ...)
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 09/13] igt: fb: Rework YUV i915 allocation path Maxime Ripard
@ 2019-01-08 15:19 ` Maxime Ripard
2019-01-10 10:45 ` Paul Kocialkowski
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 11/13] igt: tests: chamelium: Start to unify tests Maxime Ripard
` (5 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2019-01-08 15:19 UTC (permalink / raw)
To: igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
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.
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 386908b72782..9ceeb824fff4 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++)
@@ -1590,10 +1625,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:
@@ -1610,6 +1656,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;
@@ -1626,11 +1679,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;
@@ -1857,9 +1932,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;
@@ -1868,10 +1950,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;
}
@@ -2218,6 +2307,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
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 10/13] igt: fb: Add a bunch of new YUV formats
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 10/13] igt: fb: Add a bunch of new YUV formats Maxime Ripard
@ 2019-01-10 10:45 ` Paul Kocialkowski
0 siblings, 0 replies; 36+ messages in thread
From: Paul Kocialkowski @ 2019-01-10 10:45 UTC (permalink / raw)
To: Maxime Ripard, igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> 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.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@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 386908b72782..9ceeb824fff4 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++)
> @@ -1590,10 +1625,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:
> @@ -1610,6 +1656,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;
> @@ -1626,11 +1679,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;
> @@ -1857,9 +1932,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;
> @@ -1868,10 +1950,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;
> }
> @@ -2218,6 +2307,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:
--
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [igt-dev] [PATCH i-g-t v2 11/13] igt: tests: chamelium: Start to unify tests
2019-01-08 15:19 [igt-dev] [PATCH i-g-t v2 00/13] igt: chamelium: Test YUV buffers using the Chamelium Maxime Ripard
` (9 preceding siblings ...)
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 10/13] igt: fb: Add a bunch of new YUV formats Maxime Ripard
@ 2019-01-08 15:19 ` Maxime Ripard
2019-01-10 10:50 ` Paul Kocialkowski
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 12/13] igt: tests: chamelium: Convert VGA tests to do_test_display Maxime Ripard
` (4 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2019-01-08 15:19 UTC (permalink / raw)
To: igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
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 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
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 11/13] igt: tests: chamelium: Start to unify tests
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 11/13] igt: tests: chamelium: Start to unify tests Maxime Ripard
@ 2019-01-10 10:50 ` Paul Kocialkowski
2019-01-21 13:17 ` Maxime Ripard
0 siblings, 1 reply; 36+ messages in thread
From: Paul Kocialkowski @ 2019-01-10 10:50 UTC (permalink / raw)
To: Maxime Ripard, igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
Hi,
On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> 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.
I think we can go even further and unify the assert_foo_or_dump
functions, which would imply moving enum chamelium_check to
igt_chamelium. It's probably best to do it in a separate patch though
and I might have a go at it in the next revision of my plane testing
series.
What do you think?
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@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);
--
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
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 11/13] igt: tests: chamelium: Start to unify tests
2019-01-10 10:50 ` Paul Kocialkowski
@ 2019-01-21 13:17 ` Maxime Ripard
0 siblings, 0 replies; 36+ messages in thread
From: Maxime Ripard @ 2019-01-21 13:17 UTC (permalink / raw)
To: Paul Kocialkowski; +Cc: Petri Latvala, eben, igt-dev, Thomas Petazzoni
[-- Attachment #1.1: Type: text/plain, Size: 1305 bytes --]
Hi,
On Thu, Jan 10, 2019 at 11:50:35AM +0100, Paul Kocialkowski wrote:
> On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> > 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.
>
> I think we can go even further and unify the assert_foo_or_dump
> functions, which would imply moving enum chamelium_check to
> igt_chamelium. It's probably best to do it in a separate patch though
> and I might have a go at it in the next revision of my plane testing
> series.
>
> What do you think?
I don't have a strong opinion on this, so feel free to send that patch
:)
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
>
> Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Thanks!
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [igt-dev] [PATCH i-g-t v2 12/13] igt: tests: chamelium: Convert VGA tests to do_test_display
2019-01-08 15:19 [igt-dev] [PATCH i-g-t v2 00/13] igt: chamelium: Test YUV buffers using the Chamelium Maxime Ripard
` (10 preceding siblings ...)
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 11/13] igt: tests: chamelium: Start to unify tests Maxime Ripard
@ 2019-01-08 15:19 ` Maxime Ripard
2019-01-10 10:50 ` Paul Kocialkowski
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 13/13] igt: tests: chamelium: Add YUV formats tests Maxime Ripard
` (3 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2019-01-08 15:19 UTC (permalink / raw)
To: igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
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 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
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 12/13] igt: tests: chamelium: Convert VGA tests to do_test_display
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 12/13] igt: tests: chamelium: Convert VGA tests to do_test_display Maxime Ripard
@ 2019-01-10 10:50 ` Paul Kocialkowski
0 siblings, 0 replies; 36+ messages in thread
From: Paul Kocialkowski @ 2019-01-10 10:50 UTC (permalink / raw)
To: Maxime Ripard, igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> 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>
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@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 {
--
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [igt-dev] [PATCH i-g-t v2 13/13] igt: tests: chamelium: Add YUV formats tests
2019-01-08 15:19 [igt-dev] [PATCH i-g-t v2 00/13] igt: chamelium: Test YUV buffers using the Chamelium Maxime Ripard
` (11 preceding siblings ...)
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 12/13] igt: tests: chamelium: Convert VGA tests to do_test_display Maxime Ripard
@ 2019-01-08 15:19 ` Maxime Ripard
2019-01-10 10:54 ` Paul Kocialkowski
2019-01-08 15:31 ` [igt-dev] ✗ Fi.CI.BAT: failure for igt: chamelium: Test YUV buffers using the Chamelium (rev3) Patchwork
` (2 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Maxime Ripard @ 2019-01-08 15:19 UTC (permalink / raw)
To: igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
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.
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
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 13/13] igt: tests: chamelium: Add YUV formats tests
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 13/13] igt: tests: chamelium: Add YUV formats tests Maxime Ripard
@ 2019-01-10 10:54 ` Paul Kocialkowski
0 siblings, 0 replies; 36+ messages in thread
From: Paul Kocialkowski @ 2019-01-10 10:54 UTC (permalink / raw)
To: Maxime Ripard, igt-dev; +Cc: Petri Latvala, eben, Thomas Petazzoni
On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> 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.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
After numerous rounds of testing, I found that the analog test has a
strong tendency to return false positives on digital interfaces.
I'll send out a more reliable testing mechanism soon and will switch
these tests over to using it.
Since it's not available yet and the analog tests are the next best
thing for fuzzy comparison:
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@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);
> }
--
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [igt-dev] ✗ Fi.CI.BAT: failure for igt: chamelium: Test YUV buffers using the Chamelium (rev3)
2019-01-08 15:19 [igt-dev] [PATCH i-g-t v2 00/13] igt: chamelium: Test YUV buffers using the Chamelium Maxime Ripard
` (12 preceding siblings ...)
2019-01-08 15:19 ` [igt-dev] [PATCH i-g-t v2 13/13] igt: tests: chamelium: Add YUV formats tests Maxime Ripard
@ 2019-01-08 15:31 ` Patchwork
2019-01-08 17:59 ` [igt-dev] ✓ Fi.CI.BAT: success for igt: chamelium: Test YUV buffers using the Chamelium (rev4) Patchwork
2019-01-09 3:11 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
15 siblings, 0 replies; 36+ messages in thread
From: Patchwork @ 2019-01-08 15:31 UTC (permalink / raw)
To: Maxime Ripard; +Cc: igt-dev
== Series Details ==
Series: igt: chamelium: Test YUV buffers using the Chamelium (rev3)
URL : https://patchwork.freedesktop.org/series/53468/
State : failure
== Summary ==
Patch is empty.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 36+ messages in thread* [igt-dev] ✓ Fi.CI.BAT: success for igt: chamelium: Test YUV buffers using the Chamelium (rev4)
2019-01-08 15:19 [igt-dev] [PATCH i-g-t v2 00/13] igt: chamelium: Test YUV buffers using the Chamelium Maxime Ripard
` (13 preceding siblings ...)
2019-01-08 15:31 ` [igt-dev] ✗ Fi.CI.BAT: failure for igt: chamelium: Test YUV buffers using the Chamelium (rev3) Patchwork
@ 2019-01-08 17:59 ` Patchwork
2019-01-09 3:11 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
15 siblings, 0 replies; 36+ messages in thread
From: Patchwork @ 2019-01-08 17:59 UTC (permalink / raw)
To: Maxime Ripard; +Cc: igt-dev
== Series Details ==
Series: igt: chamelium: Test YUV buffers using the Chamelium (rev4)
URL : https://patchwork.freedesktop.org/series/53468/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5378 -> IGTPW_2203
====================================================
Summary
-------
**WARNING**
Minor unknown changes coming with IGTPW_2203 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in IGTPW_2203, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/53468/revisions/4/
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in IGTPW_2203:
### IGT changes ###
#### Warnings ####
* igt@kms_busy@basic-flip-a:
- fi-kbl-7567u: PASS -> SKIP +2
Known issues
------------
Here are the changes found in IGTPW_2203 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@amdgpu/amd_basic@cs-compute:
- fi-kbl-8809g: NOTRUN -> FAIL [fdo#108094]
* igt@amdgpu/amd_prime@amd-to-i915:
- fi-kbl-8809g: NOTRUN -> FAIL [fdo#107341]
* igt@i915_selftest@live_hangcheck:
- fi-skl-iommu: PASS -> INCOMPLETE [fdo#108602] / [fdo#108744]
* igt@kms_flip@basic-flip-vs-modeset:
- fi-skl-6700hq: PASS -> DMESG-WARN [fdo#105998]
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
- fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362]
#### Possible fixes ####
* igt@amdgpu/amd_basic@userptr:
- fi-kbl-8809g: DMESG-WARN [fdo#108965] -> PASS
* igt@gem_exec_suspend@basic-s4-devices:
- fi-bsw-kefka: DMESG-WARN -> PASS
- fi-blb-e6850: INCOMPLETE [fdo#107718] -> PASS
* igt@i915_selftest@live_hangcheck:
- fi-apl-guc: DMESG-FAIL [fdo#109228] -> PASS
* igt@kms_frontbuffer_tracking@basic:
- fi-byt-clapper: FAIL [fdo#103167] -> PASS
* igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
- fi-byt-clapper: FAIL [fdo#103191] / [fdo#107362] -> PASS
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
- fi-byt-clapper: INCOMPLETE [fdo#102657] -> PASS
* igt@kms_psr@sprite_plane_onoff:
- fi-skl-6700hq: FAIL [fdo#107383] -> PASS +3
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102657]: https://bugs.freedesktop.org/show_bug.cgi?id=102657
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#105998]: https://bugs.freedesktop.org/show_bug.cgi?id=105998
[fdo#107341]: https://bugs.freedesktop.org/show_bug.cgi?id=107341
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108094]: https://bugs.freedesktop.org/show_bug.cgi?id=108094
[fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
[fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
[fdo#108915]: https://bugs.freedesktop.org/show_bug.cgi?id=108915
[fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
[fdo#109228]: https://bugs.freedesktop.org/show_bug.cgi?id=109228
[fdo#109241]: https://bugs.freedesktop.org/show_bug.cgi?id=109241
Participating hosts (48 -> 46)
------------------------------
Additional (1): fi-icl-y
Missing (3): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
Build changes
-------------
* IGT: IGT_4756 -> IGTPW_2203
CI_DRM_5378: 96b07848e43c024bd6a5a44970371c4866140a1c @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_2203: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2203/
IGT_4756: 75081c6bfb9998bd7cbf35a7ac0578c683fe55a8 @ 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_2203/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 36+ messages in thread* [igt-dev] ✓ Fi.CI.IGT: success for igt: chamelium: Test YUV buffers using the Chamelium (rev4)
2019-01-08 15:19 [igt-dev] [PATCH i-g-t v2 00/13] igt: chamelium: Test YUV buffers using the Chamelium Maxime Ripard
` (14 preceding siblings ...)
2019-01-08 17:59 ` [igt-dev] ✓ Fi.CI.BAT: success for igt: chamelium: Test YUV buffers using the Chamelium (rev4) Patchwork
@ 2019-01-09 3:11 ` Patchwork
15 siblings, 0 replies; 36+ messages in thread
From: Patchwork @ 2019-01-09 3:11 UTC (permalink / raw)
To: Maxime Ripard; +Cc: igt-dev
== Series Details ==
Series: igt: chamelium: Test YUV buffers using the Chamelium (rev4)
URL : https://patchwork.freedesktop.org/series/53468/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5378_full -> IGTPW_2203_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/53468/revisions/4/
Known issues
------------
Here are the changes found in IGTPW_2203_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@kms_available_modes_crc@available_mode_test_crc:
- shard-snb: NOTRUN -> FAIL [fdo#106641]
* igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
- shard-kbl: NOTRUN -> DMESG-WARN [fdo#107956]
* igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
- shard-kbl: NOTRUN -> FAIL [fdo#107725] / [fdo#108145]
* igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
- shard-glk: PASS -> FAIL [fdo#108145]
* igt@kms_color@pipe-a-legacy-gamma:
- shard-apl: PASS -> FAIL [fdo#104782] / [fdo#108145]
* igt@kms_color@pipe-c-legacy-gamma:
- shard-kbl: PASS -> FAIL [fdo#104782]
* igt@kms_cursor_crc@cursor-64x21-random:
- shard-apl: PASS -> FAIL [fdo#103232] +3
* igt@kms_cursor_crc@cursor-64x64-sliding:
- shard-glk: PASS -> FAIL [fdo#103232] +3
* igt@kms_flip@2x-flip-vs-expired-vblank:
- shard-glk: PASS -> FAIL [fdo#105363]
* igt@kms_flip@flip-vs-expired-vblank:
- shard-apl: PASS -> FAIL [fdo#102887] / [fdo#105363]
* igt@kms_flip@modeset-vs-vblank-race-interruptible:
- shard-glk: PASS -> FAIL [fdo#103060]
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
- shard-apl: PASS -> FAIL [fdo#103167] +2
- shard-kbl: PASS -> FAIL [fdo#103167]
* igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
- shard-glk: PASS -> FAIL [fdo#103167] +6
* igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
- shard-glk: PASS -> FAIL [fdo#103166] +4
* igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
- shard-apl: PASS -> FAIL [fdo#103166] +2
- shard-kbl: PASS -> FAIL [fdo#103166]
* igt@kms_setmode@basic:
- shard-kbl: NOTRUN -> FAIL [fdo#99912]
* igt@pm_rpm@system-suspend-execbuf:
- shard-kbl: PASS -> INCOMPLETE [fdo#103665] / [fdo#107807]
#### Possible fixes ####
* igt@kms_cursor_crc@cursor-128x128-suspend:
- shard-apl: FAIL [fdo#103191] / [fdo#103232] -> PASS
* igt@kms_cursor_crc@cursor-256x85-random:
- shard-apl: FAIL [fdo#103232] -> PASS +2
* igt@kms_cursor_crc@cursor-64x21-onscreen:
- shard-glk: FAIL [fdo#103232] -> PASS +1
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
- shard-apl: FAIL [fdo#103167] -> PASS
* igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-wc:
- shard-glk: FAIL [fdo#103167] -> PASS +2
* igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
- shard-glk: FAIL [fdo#108145] -> PASS
* igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
- shard-apl: FAIL [fdo#108145] -> PASS +1
* igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
- shard-glk: FAIL [fdo#103166] -> PASS +3
* igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
- shard-apl: FAIL [fdo#103166] -> PASS +1
- shard-kbl: FAIL [fdo#103166] -> PASS
* igt@kms_rotation_crc@multiplane-rotation-cropping-top:
- shard-kbl: DMESG-FAIL [fdo#108950] -> PASS
* igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
- shard-kbl: INCOMPLETE [fdo#103665] -> PASS
* igt@pm_rps@min-max-config-loaded:
- shard-apl: FAIL [fdo#102250] -> PASS
- shard-glk: FAIL [fdo#102250] -> PASS
#### Warnings ####
* igt@i915_suspend@shrink:
- shard-hsw: DMESG-WARN [fdo#109244] -> INCOMPLETE [fdo#103540] / [fdo#106886]
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102250]: https://bugs.freedesktop.org/show_bug.cgi?id=102250
[fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
[fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
[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#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
[fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
[fdo#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
[fdo#107469]: https://bugs.freedesktop.org/show_bug.cgi?id=107469
[fdo#107725]: https://bugs.freedesktop.org/show_bug.cgi?id=107725
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108784]: https://bugs.freedesktop.org/show_bug.cgi?id=108784
[fdo#108950]: https://bugs.freedesktop.org/show_bug.cgi?id=108950
[fdo#109241]: https://bugs.freedesktop.org/show_bug.cgi?id=109241
[fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244
[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_4756 -> IGTPW_2203
* Piglit: piglit_4509 -> None
CI_DRM_5378: 96b07848e43c024bd6a5a44970371c4866140a1c @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_2203: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2203/
IGT_4756: 75081c6bfb9998bd7cbf35a7ac0578c683fe55a8 @ 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_2203/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 36+ messages in thread