Re: [igt-dev] [PATCH i-g-t] tests/kms_yuv: Add yuv specific tests, v6.

Maarten Lankhorst
2019-06-13T13:14:27Z

Op 12-06-2019 om 22:18 schreef Juha-Pekka Heikkila:
> Sorry for delay Maarten, I've been reading this on several evenings but always was interrupted. I think this look all good.
>
> Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>
> On 13.5.2019 19.53, Maarten Lankhorst wrote:
>> Add tests excercising switching between various scaled/unscaled
>> transitions to and from various nv12 formats, since some of the
>> workarounds mention this may fail.
>>
>> We also add NV12 specific clipping/scaling tests, to make sure
>> that YUV src coordinates are always programmed as a multiple of 2
>> correctly, and verify scaling/clipping works with CRC tests.
>>
>> crop-scale-bug shows a corruption when scaling and cropping at the
>> same time.
>>
>> The subpixel clip test rotates and clips a rectangle on each side,
>> which should produce the same CRC as when that part of the rectangle
>> is hidden by a cursor and a whole 2x2 pixel is hidden.
>>
>> Changes since v1:
>> - Reset plane position to 0,0 at the start of yuv_valid_width_plane().
>> - Handle Swati's feedback.
>> Changes since v2:
>> - Skip 90°/270° rotation in tests when formats don't support it.
>> - Add all new HDR formats to test.
>> - Use igt_plane_has_format_mod.
>> - Support tests on !i915 by checking if X/Y tiling modifiers are supported.
>> - Do not enable untested planes to prevent exhausing memory bandwidth
>>    and available NV12 Y planes in the rgb-switch-scaled test.
>> Changes since v3:
>> - Add a sanity test to sanity test YUV conversion paths, on all planes and pipes.
>> - Dynamically generate list of YUV tests through igt_format_array_fill.
>> Changes since v4:
>> - Decrease size to 64x64 to reduce test execution time even more,
>>    some YUV conversion routines are slow.
>> - Don't clear degamma/ctm, this is done in core now.
>> Changes since v5:
>> - Fix minor compile error.
>> - Add missing igt_subtest_group
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>   tests/Makefile.sources |   1 +
>>   tests/kms_yuv.c        | 937 +++++++++++++++++++++++++++++++++++++++++
>>   tests/meson.build      |   1 +
>>   3 files changed, 939 insertions(+)
>>   create mode 100644 tests/kms_yuv.c
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>> index 7f921f6c5988..574861b2e30f 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -75,6 +75,7 @@ TESTS_progs = \
>>       kms_universal_plane \
>>       kms_vblank \
>>       kms_vrr \
>> +    kms_yuv \
>>       meta_test \
>>       perf \
>>       perf_pmu \
>> diff --git a/tests/kms_yuv.c b/tests/kms_yuv.c
>> new file mode 100644
>> index 000000000000..6cf69b0504f8
>> --- /dev/null
>> +++ b/tests/kms_yuv.c
>> @@ -0,0 +1,937 @@
>> +/*
>> + * Copyright © 2019 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
>> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + * Authors:
>> + *    Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> + */
>> +#include "config.h"
>> +
>> +#include "igt.h"
>> +#include <cairo.h>
>> +#include <errno.h>
>> +#include <stdint.h>
>> +#include <unistd.h>
>> +#include <sys/time.h>
>> +
>> +typedef struct {
>> +    igt_display_t display;
>> +
>> +    igt_pipe_crc_t *pipe_crc;
>> +    struct igt_fb fb[6];
>> +} data_t;
>> +
>> +static uint64_t x_modifier = LOCAL_DRM_FORMAT_MOD_NONE;
>> +static uint64_t y_modifier = LOCAL_DRM_FORMAT_MOD_NONE;
>> +static uint32_t *formats, count_formats;
>> +
>> +static bool pipe_supports_format(igt_display_t *display, enum pipe pipe,
>> +                 uint32_t format, uint64_t tiling)
>> +{
>> +    igt_plane_t *plane;
>> +
>> +    for_each_plane_on_pipe(display, pipe, plane)
>> +        if (igt_plane_has_format_mod(plane, format, tiling))
>> +            return true;
>> +
>> +    return false;
>> +}
>> +
>> +
>> +static void remove_fbs(data_t *data)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < ARRAY_SIZE(data->fb); i++)
>> +        igt_remove_fb(data->display.drm_fd, &data->fb[i]);
>> +}
>> +
>> +static void prepare_crtc(data_t *data, enum pipe pipe, igt_output_t *output)
>> +{
>> +    igt_display_t *display = &data->display;
>> +
>> +    remove_fbs(data);
>> +    igt_display_reset(display);
>> +    igt_output_set_pipe(output, pipe);
>> +    igt_display_commit2(display, COMMIT_ATOMIC);
>> +
>> +    igt_pipe_crc_free(data->pipe_crc);
>> +    data->pipe_crc = igt_pipe_crc_new(display->drm_fd, pipe,
>> +                      INTEL_PIPE_CRC_SOURCE_AUTO);
>> +}
>> +
>> +static void fudge_lut(data_t *data, enum pipe pipe, uint16_t mask)
>> +{
>> +    igt_pipe_t *pipe_obj = &data->display.pipes[pipe];
>> +    uint16_t *lut;
>> +    int i, lut_size;
>> +
>> +    if (!igt_pipe_obj_has_prop(pipe_obj, IGT_CRTC_GAMMA_LUT_SIZE))
>> +        return;
>> +
>> +    lut_size = igt_pipe_obj_get_prop(pipe_obj, IGT_CRTC_GAMMA_LUT_SIZE);
>> +
>> +    lut = malloc(sizeof(uint16_t) * lut_size);
>> +    for (i = 0; i < lut_size; i++)
>> +        lut[i] = (i * 0xffff / (lut_size - 1)) & mask;
>> +
>> +    igt_pipe_obj_replace_prop_blob(pipe_obj, IGT_CRTC_GAMMA_LUT, lut, lut_size * sizeof(uint16_t));
>> +    free(lut);
>> +}
>> +
>> +static void unfudge_lut(data_t *data, enum pipe pipe)
>> +{
>> +    igt_pipe_t *pipe_obj = &data->display.pipes[pipe];
>> +
>> +    if (igt_pipe_obj_has_prop(pipe_obj, IGT_CRTC_GAMMA_LUT))
>> +        igt_pipe_obj_replace_prop_blob(pipe_obj, IGT_CRTC_GAMMA_LUT, NULL, 0);
>> +}
>> +
>> +/*
>> + * Test that we initialize a YUV fb as black correctly,
>> + * that when we write a black FB it will get the same CRC, and that
>> + * all colors we test will give the same CRC on all formats.
>> + */
>> +static void yuv_sanity(data_t *data, enum pipe pipe, igt_output_t *output)
>> +{
>> +    igt_display_t *display = &data->display;
>> +    igt_plane_t *plane;
>> +    igt_crc_t black_crc, color_crc, crc;
>> +    bool first = true;
>> +
>> +    prepare_crtc(data, pipe, output);
>> +
>> +    for_each_plane_on_pipe(display, pipe, plane) {
>> +        for (int i = 0; i < count_formats; i++) {
>> +            struct igt_fb *fb = &data->fb[0];
>> +            cairo_t *cr;
>> +
>> +            if (!igt_format_is_yuv(formats[i]) ||
>> +                !igt_plane_has_format_mod(plane, formats[i], x_modifier))
>> +                continue;
>> +
>> +            igt_debug("Testing format %.4s on plane %i\n",
>> +                 (char *)&formats[i], plane->index);
>> +            igt_create_fb(display->drm_fd, 64, 64, formats[i],
>> +                      x_modifier, fb);
>> +
>> +            igt_plane_set_fb(plane, fb);
>> +            igt_display_commit2(display, COMMIT_ATOMIC);
>> +            if (!first) {
>> +                igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
>> +                igt_assert_crc_equal(&black_crc, &crc);
>> +            } else {
>> +                igt_pipe_crc_collect_crc(data->pipe_crc, &black_crc);
>> +            }
>> +
>> +            /* Compare initial black fb with reference painted black color */
>> +            cr = igt_get_cairo_ctx(display->drm_fd, fb);
>> +            igt_paint_color(cr, 0, 0, fb->width, fb->height, 0., 0., 0.);
>> +            igt_put_cairo_ctx(display->drm_fd, fb, cr);
>> +            igt_dirty_fb(display->drm_fd, fb);
>> +            igt_wait_for_vblank(display->drm_fd, pipe);
>> +            igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
>> +            igt_assert_crc_equal(&black_crc, &crc);
>> +
>> +
>> +            /* Write a color fb */
>> +            cr = igt_get_cairo_ctx(display->drm_fd, fb);
>> +            igt_paint_color(cr, 0, 0, fb->width, fb->height, 1., 1., 0.);
>> +            igt_put_cairo_ctx(display->drm_fd, fb, cr);
>> +            fudge_lut(data, pipe, 0xfc00);
>> +            igt_plane_set_fb(plane, fb);
>> +            igt_display_commit2(display, COMMIT_ATOMIC);
>> +
>> +            if (!first) {
>> +                igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
>> +                igt_assert_crc_equal(&color_crc, &crc);
>> +            } else {
>> +                igt_pipe_crc_collect_crc(data->pipe_crc, &color_crc);
>> +                first = false;
>> +            }
>> +
>> +            /* Convert from YUV, back to YUV, to expose bugs in conversion routines */
>> +            cr = igt_get_cairo_ctx(display->drm_fd, fb);
>> +            igt_put_cairo_ctx(display->drm_fd, fb, cr);
>> +            igt_dirty_fb(display->drm_fd, fb);
>> +            igt_wait_for_vblank(display->drm_fd, pipe);
>> +            igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
>> +            igt_assert_crc_equal(&color_crc, &crc);
>> +
>> +            unfudge_lut(data, pipe);
>> +            igt_plane_set_fb(plane, NULL);
>> +            igt_remove_fb(display->drm_fd, &data->fb[0]);
>> +        }
>> +    }
>> +}
>> +
>> +static void set_fb(igt_plane_t *plane, struct igt_fb *fb,
>> +           bool scaled, igt_rotation_t rot)
>> +{
>> +    igt_plane_set_fb(plane, fb);
>> +
>> +    if (scaled && fb)
>> +        igt_fb_set_size(fb, plane, fb->width, 16);
>> +
>> +    igt_plane_set_rotation(plane, rot);
>> +    if (fb && (rot == IGT_ROTATION_90 || rot == IGT_ROTATION_270))
>> +        igt_plane_set_size(plane, fb->height, fb->width);
>> +}
>> +
>> +static void yuv_rgb_switch(data_t *data, enum pipe pipe,
>> +               igt_output_t *output, bool scaled, unsigned format)
>> +{
>> +    drmModeModeInfo *mode = igt_output_get_mode(output);
>> +    igt_display_t *display = &data->display;
>> +    igt_plane_t *plane;
>> +    int i;
>> +    igt_crc_t ref_crc[4], crc;
>> +    bool valid[4] = {};
>> +
>> +    prepare_crtc(data, pipe, output);
>> +
>> +    igt_create_pattern_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
>> +                  format, x_modifier, &data->fb[0]);
>> +
>> +    igt_create_pattern_fb(display->drm_fd, mode->vdisplay, mode->hdisplay,
>> +                  format, y_modifier, &data->fb[1]);
>> +
>> +    igt_create_pattern_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
>> +                  DRM_FORMAT_XRGB8888, x_modifier, &data->fb[2]);
>> +
>> +    igt_create_pattern_fb(display->drm_fd, mode->vdisplay, mode->hdisplay,
>> +                  DRM_FORMAT_XRGB8888, y_modifier, &data->fb[3]);
>> +
>> +    for_each_plane_on_pipe(display, pipe, plane) {
>> +        const int seq[] = {
>> +            2, 0, 2, 1, 3, 1, 3
>> +        };
>> +
>> +        if (!igt_plane_has_format_mod(plane, format, data->fb[0].modifier))
>> +            continue;
>> +
>> +        /* Collect reference crc with toggle in between. */
>> +        for (i = 0; i < ARRAY_SIZE(ref_crc); i++) {
>> +            igt_rotation_t rot = (i == 1 || i == 3) ?
>> +                IGT_ROTATION_90 : IGT_ROTATION_0;
>> +
>> +            set_fb(plane, &data->fb[i], scaled, rot);
>> +
>> +            igt_display_commit2(display, COMMIT_ATOMIC);
>> +            igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc[i]);
>> +            valid[i] = true;
>> +
>> +            set_fb(plane, NULL, scaled, rot);
>> +            igt_display_commit2(display, COMMIT_ATOMIC);
>> +        }
>> +
>> +        for (i = 0; i < ARRAY_SIZE(seq); i++) {
>> +            int j = seq[i];
>> +            igt_rotation_t rot = (j == 1 || j == 3) ?
>> +                IGT_ROTATION_90 : IGT_ROTATION_0;
>> +
>> +            if (!valid[j])
>> +                continue;
>> +
>> +            set_fb(plane, &data->fb[j], scaled, rot);
>> +            igt_display_commit2(display, COMMIT_ATOMIC);
>> +
>> +            igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
>> +            igt_assert_crc_equal(&ref_crc[j], &crc);
>> +        }
>> +
>> +        /* We only have few scalers, don't use 1 for unused planes */
>> +        igt_plane_set_fb(plane, NULL);
>> +    }
>> +}
>> +
>> +#define assert_collected_crc_equal(pipe_crc, crc, ref_crc) \
>> +    do { \
>> +        igt_pipe_crc_get_current(display->drm_fd, pipe_crc, crc);    \
>> +        igt_assert_crc_equal(ref_crc, crc);    \
>> +    } while (0)
>> +
>> +static void set_src_coords(igt_plane_t *plane, struct igt_fb *fb,
>> +               igt_rotation_t rot, int vis, int hidden)
>> +{
>> +    switch (rot) {
>> +    case IGT_ROTATION_0:
>> +        igt_fb_set_position(fb, plane, fb->width / 2 - vis, fb->height / 2 - vis);
>> +        break;
>> +    case IGT_ROTATION_90:
>> +        igt_fb_set_position(fb, plane, fb->width / 2 - hidden, fb->height / 2 - vis);
>> +        break;
>> +    case IGT_ROTATION_180:
>> +        igt_fb_set_position(fb, plane, fb->width / 2 - hidden, fb->height / 2 - hidden);
>> +        break;
>> +    case IGT_ROTATION_270:
>> +        igt_fb_set_position(fb, plane, fb->width / 2 - vis, fb->height / 2 - hidden);
>> +        break;
>> +    default: igt_assert(0);
> ^^
> This was the only nit I noticed.

What's wrong there? Or do you mean putting it on a newline?

~Maarten