Re: [igt-dev] [PATCH i-g-t v3 21/21] chamelium: Add a display test for randomized planes

[Lyude Paul <lyude@redhat.com>]

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@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

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