[igt-dev] [PATCH i-g-t 1/2] tests/kms_plane: optimize pixel format tests
Ville Syrjälä
ville.syrjala at linux.intel.com
Fri Nov 27 15:26:27 UTC 2020
On Wed, Nov 25, 2020 at 09:50:50PM +0200, Juha-Pekka Heikkila wrote:
> On packed formats there's no need to show separate colors on separate fbs.
> Here augmented path with packed color handling which check colors just on
> one fb.
>
> This cannot be directly applied to planar yuv formats because of scaler
> use with those formats.
Not entirely sure we can use this for any subsampled format. I guess
the fact that you used horizontal lines does make it work for 4:2:2
formats now, but aren't there potentially (on other hw) packed
YCbCr formats with vertical subsampling?
>
> On my ICL this drop test execution time from 44.91s to 21.98s
>
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> ---
> tests/kms_plane.c | 88 ++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 71 insertions(+), 17 deletions(-)
>
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index 4ce859fba..749a75eee 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -466,7 +466,8 @@ static void prepare_format_color(data_t *data, enum pipe pipe,
> int width, int height,
> enum igt_color_encoding color_encoding,
> enum igt_color_range color_range,
> - const color_t *c, struct igt_fb *fb)
> + const color_t *c, struct igt_fb *fb,
> + bool singlefbtest)
> {
> cairo_t *cr;
>
> @@ -480,6 +481,15 @@ static void prepare_format_color(data_t *data, enum pipe pipe,
> igt_paint_color(cr, 0, 0, width, height,
> c->red, c->green, c->blue);
>
> + if (singlefbtest) {
> + for (int n = 0; n < data->num_colors; n++) {
> + igt_paint_color(cr, 0, n * 4, fb->width, 2,
> + data->colors[n].red,
> + data->colors[n].green,
> + data->colors[n].blue);
> + }
> + }
> +
> igt_put_cairo_ctx(cr);
> } else {
> igt_create_fb_with_bo_size(data->drm_fd,
> @@ -505,6 +515,16 @@ static void prepare_format_color(data_t *data, enum pipe pipe,
> width, height,
> c->red, c->green, c->blue);
>
> + if (singlefbtest) {
> + int inc = format != DRM_FORMAT_XRGB8888 ? data->crop : 0;
> + for (int n = 0; n < data->num_colors; n++) {
> + igt_paint_color(cr, 0, inc + n * 4, fb->width, 2,
> + data->colors[n].red,
> + data->colors[n].green,
> + data->colors[n].blue);
Seems like a good candidate for a new helper function
And I think I would just make it use extended_colors unconditionally,
and probably render the lines a bit thicker (eg. just
fb->height/num_ext_colors) to make it possible to actually see the
individual lines on high res displays.
> + }
> + }
> +
> igt_put_cairo_ctx(cr);
> }
>
> @@ -548,13 +568,35 @@ static void capture_crc(data_t *data, unsigned int vblank, igt_crc_t *crc)
> crc->frame, vblank);
> }
>
> -static void capture_format_crcs(data_t *data, enum pipe pipe,
> - igt_plane_t *plane,
> - uint32_t format, uint64_t modifier,
> - int width, int height,
> - enum igt_color_encoding encoding,
> - enum igt_color_range range,
> - igt_crc_t crc[], struct igt_fb *fb)
> +static void capture_format_crcs_packed(data_t *data, enum pipe pipe,
> + igt_plane_t *plane,
> + uint32_t format, uint64_t modifier,
> + int width, int height,
> + enum igt_color_encoding encoding,
> + enum igt_color_range range,
> + igt_crc_t crc[], struct igt_fb *fb)
> +{
> + struct igt_fb old_fb = *fb;
> + const color_t black = { 0.0f, 0.0f, 0.0f };
> +
> + prepare_format_color(data, pipe, plane, format, modifier,
> + width, height, encoding, range, &black, fb, true);
> +
> + igt_display_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET,
> + NULL);
> +
> + igt_remove_fb(data->drm_fd, &old_fb);
> +
> + igt_pipe_crc_get_current(data->drm_fd, data->pipe_crc, &crc[0]);
> +}
> +
> +static void capture_format_crcs_planar(data_t *data, enum pipe pipe,
> + igt_plane_t *plane,
> + uint32_t format, uint64_t modifier,
> + int width, int height,
> + enum igt_color_encoding encoding,
> + enum igt_color_range range,
> + igt_crc_t crc[], struct igt_fb *fb)
> {
> unsigned int vblank[ARRAY_SIZE(colors_extended)];
> struct drm_event_vblank ev;
> @@ -567,7 +609,7 @@ restart_round:
> int ret;
>
> prepare_format_color(data, pipe, plane, format, modifier,
> - width, height, encoding, range, c, fb);
> + width, height, encoding, range, c, fb, false);
>
> if (data->display.is_atomic && i >= 1) {
> igt_assert(read(data->drm_fd, &ev, sizeof(ev)) == sizeof(ev));
> @@ -684,12 +726,20 @@ static bool test_format_plane_colors(data_t *data, enum pipe pipe,
> int crc_mismatch_count = 0;
> bool result = true;
> int i;
> + bool planar = igt_format_is_yuv_semiplanar(format);
>
> - capture_format_crcs(data, pipe, plane, format, modifier,
> - width, height, encoding, range, crc, fb);
> + if (planar)
> + capture_format_crcs_planar(data, pipe, plane, format, modifier,
> + width, height, encoding, range, crc,
> + fb);
> + else
> + capture_format_crcs_packed(data, pipe, plane, format, modifier,
> + width, height, encoding, range, crc,
> + fb);
>
> - for (i = 0; i < data->num_colors; i++) {
> - if (!igt_check_crc_equal(&crc[i], &ref_crc[i])) {
> + for (i = 0; i < (planar ? data->num_colors : 1); i++) {
Can't we just make num_colors correct? Ie. make it 1 for packed formats.
> +
> + if (!igt_check_crc_equal(&crc[i], &ref_crc[i + (planar ? 1 : 0)])) {
This ref_crc indexing is rather ugly. I guess I would add a small helper
for it to at least hide it better.
> crc_mismatch_count++;
> crc_mismatch_mask |= (1 << i);
> result = false;
> @@ -784,7 +834,7 @@ static bool test_format_plane(data_t *data, enum pipe pipe,
> uint32_t format, ref_format;
> uint64_t modifier, ref_modifier;
> uint64_t width, height;
> - igt_crc_t ref_crc[ARRAY_SIZE(colors_extended)];
> + igt_crc_t ref_crc[ARRAY_SIZE(colors_extended) + 1];
> bool result = true;
>
> /*
> @@ -843,9 +893,13 @@ static bool test_format_plane(data_t *data, enum pipe pipe,
> igt_remove_fb(data->drm_fd, &test_fb);
> }
>
> - capture_format_crcs(data, pipe, plane, format, modifier,
> - width, height, IGT_COLOR_YCBCR_BT709,
> - IGT_COLOR_YCBCR_LIMITED_RANGE, ref_crc, &fb);
> + capture_format_crcs_packed(data, pipe, plane, format, modifier,
> + width, height, IGT_COLOR_YCBCR_BT709,
> + IGT_COLOR_YCBCR_LIMITED_RANGE, ref_crc, &fb);
> +
> + capture_format_crcs_planar(data, pipe, plane, format, modifier,
> + width, height, IGT_COLOR_YCBCR_BT709,
> + IGT_COLOR_YCBCR_LIMITED_RANGE, &ref_crc[1], &fb);
>
> /*
> * Make sure we have some difference between the colors. This
> --
> 2.28.0
>
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
--
Ville Syrjälä
Intel
More information about the igt-dev
mailing list