[igt-dev] [PATCH i-g-t 2/2] tests/kms_chamelium: Fix *-cmp-random and *-crc-random tests.

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

The random tests allowed potentially invalid things:
- 1x1 fb's to be created. Force a minimum of 32 so a scaled
  plane will be at least 16x16. This will fix scaled/planar format
  support in i915 and avoid a div by zero when calculating a value
  modulo h/2 and w/2.
- Downscaling to any amount, restrict it to 2x to make the test pass.
- Some hw may not allow scaling, in those cases we should fallback
  to no scaling at all.
- Attempting to configure a minimum of 4 planes, instead of a maximum.
  This fails with a null pointer deref if the hw doesn't have 4
  configurable overlay planes.

Cc: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 tests/kms_chamelium.c | 195 ++++++++++++++++++++++++++++--------------
 1 file changed, 131 insertions(+), 64 deletions(-)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 2dc1049d2dda..5e5c68ccf879 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -792,26 +792,53 @@ static void randomize_plane_format_stride(igt_plane_t *plane,
 }
 
 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)
+				       uint32_t *width, uint32_t *height)
 {
-	double ratio;
+	/*
+	 * Randomize width and height in the mode dimensions range.
+	 *
+	 * Restrict to a min of 32, this way src_w/h are always at least 16,
+	 * the minimum for scaled planar YUV on i915.
+	 */
+	*width = max((rand() % mode->hdisplay) + 1, 32);
+	*height = max((rand() % mode->vdisplay) + 1, 32);
+}
+
+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);
 
-	/* Randomize width and height in the mode dimensions range. */
-	*width = (rand() % mode->hdisplay) + 1;
-	*height = (rand() % mode->vdisplay) + 1;
+	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 randomize_plane_coordinates(data_t *data, igt_plane_t *plane,
+					drmModeModeInfo *mode,
+					struct igt_fb *fb,
+					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)
+{
+	bool is_yuv = igt_format_is_yuv(fb->drm_format);
+	uint32_t width = fb->width, height = fb->height;
+	double ratio;
+	int ret;
 
 	/* Randomize source offset in the first half of the original size. */
-	*src_x = rand() % (*width / 2);
-	*src_y = rand() % (*height / 2);
+	*src_x = rand() % (width / 2);
+	*src_y = rand() % (height / 2);
 
 	/* The source size only includes the active source area. */
-	*src_w = *width - *src_x;
-	*src_h = *height - *src_y;
+	*src_w = width - *src_x;
+	*src_h = height - *src_y;
 
 	if (allow_scaling) {
 		*crtc_w = (rand() % mode->hdisplay) + 1;
@@ -821,17 +848,22 @@ static void randomize_plane_dimensions(drmModeModeInfo *mode,
 		 * 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.
+		 * comparison less reliable, and don't go above 2x downsampling
+		 * to avoid possible hw limitations.
 		 */
 
 		ratio = ((double) *crtc_w / *src_w);
-		if (ratio > 0.8 && ratio < 1.2)
+		if (ratio < 0.5)
+			*src_w = *crtc_w * 2;
+		else 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)
+		if (ratio < 0.5)
+			*src_h = *crtc_h * 2;
+		else if (ratio > 0.8 && ratio < 1.2)
 			*crtc_h = *src_h;
 		else if (ratio > 3.0)
 			*crtc_h = *src_h * 3;
@@ -846,8 +878,15 @@ static void randomize_plane_dimensions(drmModeModeInfo *mode,
 		 * 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);
+		if (*crtc_w < mode->hdisplay)
+			*crtc_x = rand() % (mode->hdisplay - *crtc_w);
+		else
+			*crtc_x = 0;
+
+		if (*crtc_h < mode->vdisplay)
+			*crtc_y = rand() % (mode->vdisplay - *crtc_h);
+		else
+			*crtc_y = 0;
 	} else {
 		/*
 		 * Randomize the on-crtc position and allow the plane to go
@@ -856,6 +895,50 @@ static void randomize_plane_dimensions(drmModeModeInfo *mode,
 		*crtc_x = (rand() % mode->hdisplay) - *crtc_w / 2;
 		*crtc_y = (rand() % mode->vdisplay) - *crtc_h / 2;
 	}
+
+	configure_plane(plane, *src_w, *src_h, *src_x, *src_y,
+			*crtc_w, *crtc_h, *crtc_x, *crtc_y, fb);
+	ret = igt_display_try_commit_atomic(&data->display,
+					    DRM_MODE_ATOMIC_TEST_ONLY |
+					    DRM_MODE_ATOMIC_ALLOW_MODESET,
+					    NULL);
+	if (!ret)
+		return;
+
+	/* Coordinates are logged in the dumped debug log, so only report w/h on failure here */
+	igt_assert_f(ret != -ENOSPC, "Failure in testcase, invalid coordinates on a %ux%u fb\n", width, height);
+
+	/* Make YUV coordinates a multiple of 2 and retry the math.. */
+	if (is_yuv) {
+		*src_x &= ~1;
+		*src_y &= ~1;
+		*src_w &= ~1;
+		*src_h &= ~1;
+		/* To handle 1:1 scaling, clear crtc_w/h too */
+		*crtc_w &= ~1;
+		*crtc_h &= ~1;
+		configure_plane(plane, *src_w, *src_h, *src_x, *src_y, *crtc_w,
+				*crtc_h, *crtc_x, *crtc_y, fb);
+		ret = igt_display_try_commit_atomic(&data->display,
+						DRM_MODE_ATOMIC_TEST_ONLY |
+						DRM_MODE_ATOMIC_ALLOW_MODESET,
+						NULL);
+		if (!ret)
+			return;
+	}
+
+	igt_assert(!ret || allow_scaling);
+	igt_info("Scaling ratio %g / %g failed, trying without scaling.\n",
+		  ((double) *crtc_w / *src_w), ((double) *crtc_h / *src_h));
+
+	*crtc_w = *src_w;
+	*crtc_h = *src_h;
+
+	configure_plane(plane, *src_w, *src_h, *src_x, *src_y, *crtc_w,
+			*crtc_h, *crtc_x, *crtc_y, fb);
+	igt_display_commit_atomic(&data->display,
+				  DRM_MODE_ATOMIC_TEST_ONLY |
+				  DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
 }
 
 static void blit_plane_cairo(data_t *data, cairo_surface_t *result,
@@ -914,20 +997,6 @@ static void blit_plane_cairo(data_t *data, cairo_surface_t *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_randomized_plane(data_t *data,
 				     drmModeModeInfo *mode,
 				     igt_plane_t *plane,
@@ -947,24 +1016,12 @@ static void prepare_randomized_plane(data_t *data,
 	size_t stride;
 	bool tiled;
 	int fb_id;
+	uint32_t tile_w, tile_h;
 
-	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);
+	randomize_plane_dimensions(mode, &overlay_fb_w, &overlay_fb_h);
 
 	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,
@@ -976,23 +1033,42 @@ static void prepare_randomized_plane(data_t *data,
 
 	tiled = (modifier != LOCAL_DRM_FORMAT_MOD_NONE);
 
+	igt_get_fb_tile_size(data->display.drm_fd, modifier,
+			     igt_drm_format_to_bpp(format),
+			     &tile_w, &tile_h);
+
+	/* Make sure stride is a multiple of tile size */
+	stride = DIV_ROUND_UP(stride, tile_w) * tile_w;
+
 	igt_debug("Plane %d: %s format (%s) with stride %ld\n", index,
 		  igt_format_str(format), tiled ? "tiled" : "linear", stride);
 
+
 	fb_id = igt_fb_convert_with_stride(overlay_fb, &pattern_fb, format,
 					   modifier, stride);
 	igt_assert(fb_id > 0);
 
+	randomize_plane_coordinates(data, plane, mode, overlay_fb,
+				    &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: 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);
+	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);
+
 	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);
 }
@@ -1068,7 +1144,7 @@ static void test_display_planes_random(data_t *data,
 		igt_output_count_plane_type(output, DRM_PLANE_TYPE_OVERLAY);
 
 	/* Limit the number of planes to a reasonable scene. */
-	overlay_planes_max = max(overlay_planes_max, 4);
+	overlay_planes_max = min(overlay_planes_max, 4);
 
 	overlay_planes_count = (rand() % overlay_planes_max) + 1;
 	igt_debug("Using %d overlay planes\n", overlay_planes_count);
@@ -1121,17 +1197,8 @@ static void test_display_planes_random(data_t *data,
 		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);
-	}
+	for (i = 0; i < overlay_planes_count; i++)
+		igt_remove_fb(data->drm_fd, &overlay_fbs[i]);
 
 	free(overlay_fbs);
 
-- 
2.20.1

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