[igt-dev] [PATCH i-g-t v3 21/21] chamelium: Add a display test for randomized planes
Lyude Paul
lyude at redhat.com
Tue Jan 15 22:56:10 UTC 2019
On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> This introduces a new test for the Chamelium, that sets up planes
> with randomized properties such as the format, dimensions, position,
> in-framebuffer offsets and stride. The Chamelium capture is checked
> against the reference generated by cairo with either a CRC or the
> checkerboard-specific comparison method.
>
> This test also includes testing of the VC4-specific T-tiled and
> SAND-tiled modes, in all formats supported by the hardware.
>
> Since this test does not share much with previous Chamelium display
> tests (especially regarding KMS configuration), most of the code is
> not shared with other tests.
>
> This test can be derived with reproducible properties for regression
> testing in the future. For now, it serves as a fuzzing test
>
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski at bootlin.com>
> ---
> tests/kms_chamelium.c | 446 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 446 insertions(+)
>
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 2779d74f55b6..5963964878ba 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -26,6 +26,7 @@
>
> #include "config.h"
> #include "igt.h"
> +#include "igt_vc4.h"
>
> #include <fcntl.h>
> #include <string.h>
> @@ -702,6 +703,443 @@ test_display_frame_dump(data_t *data, struct
> chamelium_port *port)
> drmModeFreeConnector(connector);
> }
>
> +static void randomize_plane_format_stride(igt_plane_t *plane, uint32_t
> width,
> + uint32_t *format, size_t *stride,
> + bool *tiled, bool allow_yuv)
> +{
> + size_t stride_min;
> + uint32_t *formats_array;
> + unsigned int formats_count;
> + unsigned int count = 0;
> + unsigned int i;
> + int index;
> +
> + igt_format_array_fill(&formats_array, &formats_count, allow_yuv);
> +
> + /* First pass to count the supported formats. */
> + for (i = 0; i < formats_count; i++)
> + if (igt_plane_has_format_mod(plane, formats_array[i],
> + DRM_FORMAT_MOD_LINEAR))
> + count++;
> +
> + igt_assert(count > 0);
> +
> + index = rand() % count;
> +
> + /* Second pass to get the index-th supported format. */
> + for (i = 0; i < formats_count; i++) {
> + if (!igt_plane_has_format_mod(plane, formats_array[i],
> + DRM_FORMAT_MOD_LINEAR))
> + continue;
> +
> + if (!index--) {
> + *format = formats_array[i];
> + break;
> + }
> + }
> +
> + free(formats_array);
> +
> + igt_assert(index < 0);
> +
> + stride_min = width * igt_format_plane_bpp(*format, 0) / 8;
> +
> + /* Randomize the stride with at most twice the minimum. */
> + *stride = (rand() % stride_min) + stride_min;
Maybe need to change the comment or fix this line? Given
rand() = 9999999
stride_min = 10
This would come out to 19, e.g. less then twice the maximum. Maybe you meant
*stride = (rand() % (stride_min + 1)) + stride_min;
> +
> + /* Pixman requires the stride to be aligned to 32-byte words. */
> + *stride = ALIGN(*stride, sizeof(uint32_t));
> +
> + /* Randomize the use of a tiled mode with a 1/4 probability. */
> + *tiled = ((rand() % 4) == 0);
> +}
> +
> +static void randomize_plane_dimensions(drmModeModeInfo *mode,
> + uint32_t *width, uint32_t *height,
> + uint32_t *src_w, uint32_t *src_h,
> + uint32_t *src_x, uint32_t *src_y,
> + uint32_t *crtc_w, uint32_t *crtc_h,
> + int32_t *crtc_x, int32_t *crtc_y,
> + bool allow_scaling)
> +{
> + double ratio;
> +
> + /* Randomize width and height in the mode dimensions range. */
> + *width = (rand() % mode->hdisplay) + 1;
> + *height = (rand() % mode->vdisplay) + 1;
> +
> + /*
> + * Randomize source offset, but keep at least half of the
> + * original size.
> + */
> + *src_x = rand() % (*width / 2);
> + *src_y = rand() % (*height / 2);
Given a width of 1920, this would make
*src_x >= 0 && *src_x < 960 (e.g. src_x is between 0 and 959)
So I think you're missing a +1 here
> +
> + /* The source size only includes the active source area. */
> + *src_w = *width - *src_x;
> + *src_h = *height - *src_y;
> +
> + if (allow_scaling) {
> + *crtc_w = (rand() % mode->hdisplay) + 1;
> + *crtc_h = (rand() % mode->vdisplay) + 1;
> +
> + /*
> + * Don't bother with scaling if dimensions are quite close in
> + * order to get non-scaling cases more frequently. Also limit
> + * scaling to 3x to avoid agressive filtering that makes
> + * comparison less reliable.
> + */
> +
> + ratio = ((double) *crtc_w / *src_w);
> + if (ratio > 0.8 && ratio < 1.2)
> + *crtc_w = *src_w;
> + else if (ratio > 3.0)
> + *crtc_w = *src_w * 3;
> +
> + ratio = ((double) *crtc_h / *src_h);
> + if (ratio > 0.8 && ratio < 1.2)
> + *crtc_h = *src_h;
> + else if (ratio > 3.0)
> + *crtc_h = *src_h * 3;
> + } else {
> + *crtc_w = *src_w;
> + *crtc_h = *src_h;
> + }
> +
> + if (*crtc_w != *src_w || *crtc_h != *src_h) {
> + /*
> + * When scaling is involved, make sure to not go off-bounds or
> + * scaled clipping may result in decimal dimensions, that most
> + * drivers don't support.
> + */
> + *crtc_x = rand() % (mode->hdisplay - *crtc_w);
> + *crtc_y = rand() % (mode->vdisplay - *crtc_h);
> + } else {
> + /*
> + * Randomize the on-crtc position and allow the plane to go
> + * off-display by up to half of its on-crtc width and height.
> + */
> + *crtc_x = (rand() % mode->hdisplay) - *crtc_w / 2;
> + *crtc_y = (rand() % mode->vdisplay) - *crtc_h / 2;
I think you're missing +1s here as well (should be rand() % mode->hdisplay+1,
etc.)
> + }
> +}
> +
> +static void blit_plane_cairo(data_t *data, cairo_surface_t *result,
> + uint32_t src_w, uint32_t src_h,
> + uint32_t src_x, uint32_t src_y,
> + uint32_t crtc_w, uint32_t crtc_h,
> + int32_t crtc_x, int32_t crtc_y,
> + struct igt_fb *fb)
> +{
> + cairo_surface_t *surface;
> + cairo_surface_t *clipped_surface;
> + cairo_t *cr;
> +
> + surface = igt_get_cairo_surface(data->drm_fd, fb);
> +
> + if (src_x || src_y) {
> + clipped_surface =
> cairo_image_surface_create(CAIRO_FORMAT_RGB24,
> + src_w, src_h);
> +
> + cr = cairo_create(clipped_surface);
> +
> + cairo_translate(cr, -1. * src_x, -1. * src_y);
> +
> + cairo_set_source_surface(cr, surface, 0, 0);
> +
> + cairo_paint(cr);
> + cairo_surface_flush(clipped_surface);
> +
> + cairo_destroy(cr);
> + } else {
> + clipped_surface = surface;
> + }
> +
> + cr = cairo_create(result);
> +
> + cairo_translate(cr, crtc_x, crtc_y);
> +
> + if (src_w != crtc_w || src_h != crtc_h) {
> + cairo_scale(cr, (double) crtc_w / src_w,
> + (double) crtc_h / src_h);
> + }
> +
> + cairo_set_source_surface(cr, clipped_surface, 0, 0);
> + cairo_surface_destroy(clipped_surface);
> +
> + if (src_w != crtc_w || src_h != crtc_h) {
> + cairo_pattern_set_filter(cairo_get_source(cr),
> + CAIRO_FILTER_BILINEAR);
> + cairo_pattern_set_extend(cairo_get_source(cr),
> + CAIRO_EXTEND_NONE);
> + }
> +
> + cairo_paint(cr);
> + cairo_surface_flush(result);
> +
> + cairo_destroy(cr);
> +}
> +
> +static void configure_plane(igt_plane_t *plane, uint32_t src_w, uint32_t
> src_h,
> + uint32_t src_x, uint32_t src_y, uint32_t crtc_w,
> + uint32_t crtc_h, int32_t crtc_x, int32_t crtc_y,
> + struct igt_fb *fb)
> +{
> + igt_plane_set_fb(plane, fb);
> +
> + igt_plane_set_position(plane, crtc_x, crtc_y);
> + igt_plane_set_size(plane, crtc_w, crtc_h);
> +
> + igt_fb_set_position(fb, plane, src_x, src_y);
> + igt_fb_set_size(fb, plane, src_w, src_h);
> +}
> +
> +static void prepare_tiled_plane(data_t *data, igt_plane_t *plane,
> + unsigned int index, uint32_t format,
> + struct igt_fb *overlay_fb)
> +{
> + struct igt_fb tiled_fb;
> + unsigned int fb_id;
> +
> + if (is_vc4_device(data->drm_fd) &&
> + igt_vc4_plane_supports_t_tiling(plane, format)) {
> + igt_debug("Plane %d: using VC4 T-tiling\n", index);
> +
> + fb_id = igt_vc4_fb_t_tiled_convert(&tiled_fb, overlay_fb);
> + } else if (is_vc4_device(data->drm_fd) &&
> + igt_vc4_plane_supports_sand_tiling(plane, format, 256)) {
> + /* Randomize the column height with at most twice the height.
> */
> + size_t column_height = (rand() % overlay_fb->height) +
> + overlay_fb->height;
> +
> + igt_debug("Plane %d: using VC4 SAND256 tiling with column
> height %ld\n",
> + index, column_height);
> +
> + fb_id = vc4_fb_sand_tiled_convert(&tiled_fb, overlay_fb, 256,
> + column_height);
> + } else {
> + return;
> + }
Bikeshed: Why not remove the two is_vc4_device(data->drm_fd) checks and just
put this at the top of prepare_tiled_plane():
if (!is_vc4_device(data->drm_fd))
return;
Would make these conditionals a bit easier to read
> +
> + igt_assert(fb_id > 0);
> +
> + /* Make the tiled framebuffer the overlay. */
> + igt_remove_fb(data->drm_fd, overlay_fb);
> + *overlay_fb = tiled_fb;
> +}
> +
> +static void prepare_randomized_plane(data_t *data,
> + drmModeModeInfo *mode,
> + igt_plane_t *plane,
> + struct igt_fb *overlay_fb,
> + unsigned int index,
> + cairo_surface_t *result_surface,
> + bool allow_scaling, bool allow_yuv)
> +{
> + struct igt_fb pattern_fb;
> + uint32_t overlay_fb_w, overlay_fb_h;
> + uint32_t overlay_src_w, overlay_src_h;
> + uint32_t overlay_src_x, overlay_src_y;
> + int32_t overlay_crtc_x, overlay_crtc_y;
> + uint32_t overlay_crtc_w, overlay_crtc_h;
> + uint32_t format;
> + size_t stride;
> + bool tiled;
> + int fb_id;
> +
> + randomize_plane_dimensions(mode, &overlay_fb_w, &overlay_fb_h,
> + &overlay_src_w, &overlay_src_h,
> + &overlay_src_x, &overlay_src_y,
> + &overlay_crtc_w, &overlay_crtc_h,
> + &overlay_crtc_x, &overlay_crtc_y,
> + allow_scaling);
> +
> + igt_debug("Plane %d: framebuffer size %dx%d\n", index,
> + overlay_fb_w, overlay_fb_h);
> + igt_debug("Plane %d: on-crtc size %dx%d\n", index,
> + overlay_crtc_w, overlay_crtc_h);
> + igt_debug("Plane %d: on-crtc position %dx%d\n", index,
> + overlay_crtc_x, overlay_crtc_y);
> + igt_debug("Plane %d: in-framebuffer size %dx%d\n", index,
> + overlay_src_w, overlay_src_h);
> + igt_debug("Plane %d: in-framebuffer position %dx%d\n", index,
> + overlay_src_x, overlay_src_y);
> +
> + /* Get a pattern framebuffer for the overlay plane. */
> + fb_id = chamelium_get_pattern_fb(data, overlay_fb_w, overlay_fb_h,
> + DRM_FORMAT_XRGB8888, 32,
> &pattern_fb);
> + igt_assert(fb_id > 0);
> +
> + randomize_plane_format_stride(plane, overlay_fb_w, &format, &stride,
> + &tiled, allow_yuv);
> +
> + igt_debug("Plane %d: %s format with stride %ld\n", index,
> + igt_format_str(format), stride);
> +
> + fb_id = igt_fb_convert_with_stride(overlay_fb, &pattern_fb, format,
> + stride);
> + igt_assert(fb_id > 0);
> +
> + if (tiled)
> + prepare_tiled_plane(data, plane, index, format, overlay_fb);
> +
> + blit_plane_cairo(data, result_surface, overlay_src_w, overlay_src_h,
> + overlay_src_x, overlay_src_y,
> + overlay_crtc_w, overlay_crtc_h,
> + overlay_crtc_x, overlay_crtc_y, &pattern_fb);
> +
> + configure_plane(plane, overlay_src_w, overlay_src_h,
> + overlay_src_x, overlay_src_y,
> + overlay_crtc_w, overlay_crtc_h,
> + overlay_crtc_x, overlay_crtc_y, overlay_fb);
> +
> + /* Remove the original pattern framebuffer. */
> + igt_remove_fb(data->drm_fd, &pattern_fb);
> +}
> +
> +static void test_display_planes_random(data_t *data,
> + struct chamelium_port *port,
> + enum chamelium_check check)
> +{
> + igt_output_t *output;
> + drmModeModeInfo *mode;
> + igt_plane_t *primary_plane;
> + struct igt_fb primary_fb;
> + struct igt_fb result_fb;
> + struct igt_fb *overlay_fbs;
> + igt_crc_t *crc;
> + igt_crc_t *expected_crc;
> + struct chamelium_fb_crc_async_data *fb_crc;
> + unsigned int overlay_planes_max = 0;
> + unsigned int overlay_planes_count;
> + cairo_surface_t *result_surface;
> + int captured_frame_count;
> + bool allow_scaling;
> + bool allow_yuv;
> + unsigned int i;
> + unsigned int fb_id;
> +
> + switch (check) {
> + case CHAMELIUM_CHECK_CRC:
> + allow_scaling = false;
> + allow_yuv = false;
> + break;
> + case CHAMELIUM_CHECK_CHECKERBOARD:
> + allow_scaling = true;
> + allow_yuv = true;
> + break;
> + default:
> + igt_assert(false);
> + }
> +
> + srand(time(NULL));
> +
> + reset_state(data, port);
> +
> + /* Find the connector and pipe. */
> + output = prepare_output(data, port);
> +
> + mode = igt_output_get_mode(output);
> +
> + /* Get a framebuffer for the primary plane. */
> + primary_plane = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
> + igt_assert(primary_plane);
> +
> + fb_id = chamelium_get_pattern_fb(data, mode->hdisplay, mode->vdisplay,
> + DRM_FORMAT_XRGB8888, 64,
> &primary_fb);
> + igt_assert(fb_id > 0);
> +
> + /* Get a framebuffer for the cairo composition result. */
> + fb_id = igt_create_fb(data->drm_fd, mode->hdisplay,
> + mode->vdisplay, DRM_FORMAT_XRGB8888,
> + LOCAL_DRM_FORMAT_MOD_NONE, &result_fb);
> + igt_assert(fb_id > 0);
> +
> + result_surface = igt_get_cairo_surface(data->drm_fd, &result_fb);
> +
> + /* Paint the primary framebuffer on the result surface. */
> + blit_plane_cairo(data, result_surface, 0, 0, 0, 0, 0, 0, 0, 0,
> + &primary_fb);
> +
> + /* Configure the primary plane. */
> + igt_plane_set_fb(primary_plane, &primary_fb);
> +
> + overlay_planes_max =
> + igt_output_count_plane_type(output, DRM_PLANE_TYPE_OVERLAY);
> +
> + /* Limit the number of planes to a reasonable scene. */
> + if (overlay_planes_max > 4)
> + overlay_planes_max = 4;
Why not just use max() here?
> +
> + overlay_planes_count = (rand() % overlay_planes_max) + 1;
> + igt_debug("Using %d overlay planes\n", overlay_planes_count);
> +
> + overlay_fbs = calloc(sizeof(struct igt_fb), overlay_planes_count);
igt_assert(overlay_fbs);
Other then that, looks really good!
> +
> + for (i = 0; i < overlay_planes_count; i++) {
> + struct igt_fb *overlay_fb = &overlay_fbs[i];
> + igt_plane_t *plane =
> + igt_output_get_plane_type_index(output,
> + DRM_PLANE_TYPE_OVERLAY
> ,
> + i);
> + igt_assert(plane);
> +
> + prepare_randomized_plane(data, mode, plane, overlay_fb, i,
> + result_surface, allow_scaling,
> + allow_yuv);
> + }
> +
> + cairo_surface_destroy(result_surface);
> +
> + if (check == CHAMELIUM_CHECK_CRC)
> + fb_crc = chamelium_calculate_fb_crc_async_start(data->drm_fd,
> + &result_fb);
> +
> + igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +
> + if (check == CHAMELIUM_CHECK_CRC) {
> + chamelium_capture(data->chamelium, port, 0, 0, 0, 0, 1);
> + crc = chamelium_read_captured_crcs(data->chamelium,
> + &captured_frame_count);
> +
> + igt_assert(captured_frame_count == 1);
> +
> + expected_crc =
> chamelium_calculate_fb_crc_async_finish(fb_crc);
> +
> + chamelium_assert_crc_eq_or_dump(data->chamelium,
> + expected_crc, crc,
> + &result_fb, 0);
> +
> + free(expected_crc);
> + free(crc);
> + } else if (check == CHAMELIUM_CHECK_CHECKERBOARD) {
> + struct chamelium_frame_dump *dump;
> +
> + dump = chamelium_port_dump_pixels(data->chamelium, port, 0, 0,
> + 0, 0);
> + chamelium_assert_frame_match_or_dump(data->chamelium, port,
> + dump, &result_fb, check);
> + chamelium_destroy_frame_dump(dump);
> + }
> +
> + for (i = 0; i < overlay_planes_count; i++) {
> + struct igt_fb *overlay_fb = &overlay_fbs[i];
> + igt_plane_t *plane;
> +
> + plane = igt_output_get_plane_type_index(output,
> + DRM_PLANE_TYPE_OVERLAY
> ,
> + i);
> + igt_assert(plane);
> +
> + igt_remove_fb(data->drm_fd, overlay_fb);
> + }
> +
> + free(overlay_fbs);
> +
> + igt_remove_fb(data->drm_fd, &primary_fb);
> + igt_remove_fb(data->drm_fd, &result_fb);
> +}
> +
> static void
> test_hpd_without_ddc(data_t *data, struct chamelium_port *port)
> {
> @@ -976,6 +1414,10 @@ igt_main
> test_display_one_mode(&data, port,
> DRM_FORMAT_XRGB1555,
> CHAMELIUM_CHECK_CRC, 1);
>
> + connector_subtest("hdmi-crc-planes-random", HDMIA)
> + test_display_planes_random(&data, port,
> + CHAMELIUM_CHECK_CRC);
> +
> connector_subtest("hdmi-cmp-nv12", HDMIA)
> test_display_one_mode(&data, port, DRM_FORMAT_NV12,
> CHAMELIUM_CHECK_CHECKERBOARD,
> 1);
> @@ -1008,6 +1450,10 @@ igt_main
> test_display_one_mode(&data, port, DRM_FORMAT_YVU422,
> CHAMELIUM_CHECK_CHECKERBOARD,
> 1);
>
> + connector_subtest("hdmi-cmp-planes-random", HDMIA)
> + test_display_planes_random(&data, port,
> + CHAMELIUM_CHECK_CHECKERBOAR
> D);
> +
> connector_subtest("hdmi-frame-dump", HDMIA)
> test_display_frame_dump(&data, port);
> }
--
Cheers,
Lyude Paul
More information about the igt-dev
mailing list