dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/plane_helper: Print actual/expected values on failure
@ 2022-08-25 12:48 Michał Winiarski
  2022-08-25 12:48 ` [PATCH 2/2] drm/plane_helper: Split into parameterized test cases Michał Winiarski
  0 siblings, 1 reply; 4+ messages in thread
From: Michał Winiarski @ 2022-08-25 12:48 UTC (permalink / raw)
  To: dri-devel
  Cc: Michał Winiarski, Thomas Zimmermann, David Airlie,
	Daniel Latypov, Javier Martinez Canillas, Maíra Canal,
	Sam Ravnborg

Currently the values are printed with debug log level.
Adjust the log level and link the output with the test by using kunit_err.

Example output:
foo: dst: 20x20+10+10, expected: 10x10+0+0
foo: EXPECTATION FAILED at drivers/gpu/drm/tests/drm_plane_helper_test.c:85

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/tests/drm_plane_helper_test.c | 78 +++++++++++--------
 1 file changed, 44 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_plane_helper_test.c b/drivers/gpu/drm/tests/drm_plane_helper_test.c
index be6cff0020ed..0bbd42d2d37b 100644
--- a/drivers/gpu/drm/tests/drm_plane_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_plane_helper_test.c
@@ -10,6 +10,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_modes.h>
+#include <drm/drm_rect.h>
 
 static void set_src(struct drm_plane_state *plane_state,
 		    unsigned int src_x, unsigned int src_y,
@@ -21,26 +22,32 @@ static void set_src(struct drm_plane_state *plane_state,
 	plane_state->src_h = src_h;
 }
 
-static bool check_src_eq(struct drm_plane_state *plane_state,
+static bool check_src_eq(struct kunit *test, struct drm_plane_state *plane_state,
 			 unsigned int src_x, unsigned int src_y,
 			 unsigned int src_w, unsigned int src_h)
 {
+	struct drm_rect expected = DRM_RECT_INIT(src_x, src_y, src_w, src_h);
+
 	if (plane_state->src.x1 < 0) {
-		pr_err("src x coordinate %x should never be below 0.\n", plane_state->src.x1);
-		drm_rect_debug_print("src: ", &plane_state->src, true);
+		kunit_err(test,
+			  "src x coordinate %x should never be below 0, src: " DRM_RECT_FP_FMT,
+			  plane_state->src.x1, DRM_RECT_FP_ARG(&plane_state->src));
 		return false;
 	}
 	if (plane_state->src.y1 < 0) {
-		pr_err("src y coordinate %x should never be below 0.\n", plane_state->src.y1);
-		drm_rect_debug_print("src: ", &plane_state->src, true);
+		kunit_err(test,
+			  "src y coordinate %x should never be below 0, src: " DRM_RECT_FP_FMT,
+			  plane_state->src.y1, DRM_RECT_FP_ARG(&plane_state->src));
 		return false;
 	}
 
-	if (plane_state->src.x1 != src_x ||
-	    plane_state->src.y1 != src_y ||
-	    drm_rect_width(&plane_state->src) != src_w ||
-	    drm_rect_height(&plane_state->src) != src_h) {
-		drm_rect_debug_print("src: ", &plane_state->src, true);
+	if (plane_state->src.x1 != expected.x1 ||
+	    plane_state->src.y1 != expected.y1 ||
+	    drm_rect_width(&plane_state->src) != drm_rect_width(&expected) ||
+	    drm_rect_height(&plane_state->src) != drm_rect_height(&expected)) {
+		kunit_err(test, "src: " DRM_RECT_FP_FMT ", expected: " DRM_RECT_FP_FMT,
+			  DRM_RECT_FP_ARG(&plane_state->src), DRM_RECT_FP_ARG(&expected));
+
 		return false;
 	}
 
@@ -57,15 +64,18 @@ static void set_crtc(struct drm_plane_state *plane_state,
 	plane_state->crtc_h = crtc_h;
 }
 
-static bool check_crtc_eq(struct drm_plane_state *plane_state,
+static bool check_crtc_eq(struct kunit *test, struct drm_plane_state *plane_state,
 			  int crtc_x, int crtc_y,
 			  unsigned int crtc_w, unsigned int crtc_h)
 {
-	if (plane_state->dst.x1 != crtc_x ||
-	    plane_state->dst.y1 != crtc_y ||
-	    drm_rect_width(&plane_state->dst) != crtc_w ||
-	    drm_rect_height(&plane_state->dst) != crtc_h) {
-		drm_rect_debug_print("dst: ", &plane_state->dst, false);
+	struct drm_rect expected = DRM_RECT_INIT(crtc_x, crtc_y, crtc_w, crtc_h);
+
+	if (plane_state->dst.x1 != expected.x1 ||
+	    plane_state->dst.y1 != expected.y1 ||
+	    drm_rect_width(&plane_state->dst) != drm_rect_width(&expected) ||
+	    drm_rect_height(&plane_state->dst) != drm_rect_height(&expected)) {
+		kunit_err(test, "dst: " DRM_RECT_FMT ", expected: " DRM_RECT_FMT,
+			  DRM_RECT_ARG(&plane_state->dst), DRM_RECT_ARG(&expected));
 
 		return false;
 	}
@@ -109,8 +119,8 @@ static void igt_check_plane_state(struct kunit *test)
 						  false, false);
 	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Simple clipping check should pass\n");
 	KUNIT_EXPECT_TRUE(test, plane_state.visible);
-	KUNIT_EXPECT_TRUE(test, check_src_eq(&plane_state, 0, 0, 1024 << 16, 768 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(&plane_state, 0, 0, 1024, 768));
+	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 1024 << 16, 768 << 16));
+	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
 
 	/* Rotated clipping + reflection, no scaling. */
 	plane_state.rotation = DRM_MODE_ROTATE_90 | DRM_MODE_REFLECT_X;
@@ -120,8 +130,8 @@ static void igt_check_plane_state(struct kunit *test)
 						  false, false);
 	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Rotated clipping check should pass\n");
 	KUNIT_EXPECT_TRUE(test, plane_state.visible);
-	KUNIT_EXPECT_TRUE(test, check_src_eq(&plane_state, 0, 0, 768 << 16, 1024 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(&plane_state, 0, 0, 1024, 768));
+	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 768 << 16, 1024 << 16));
+	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
 	plane_state.rotation = DRM_MODE_ROTATE_0;
 
 	/* Check whether positioning works correctly. */
@@ -140,8 +150,8 @@ static void igt_check_plane_state(struct kunit *test)
 						  true, false);
 	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Simple positioning should work\n");
 	KUNIT_EXPECT_TRUE(test, plane_state.visible);
-	KUNIT_EXPECT_TRUE(test, check_src_eq(&plane_state, 0, 0, 1023 << 16, 767 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(&plane_state, 0, 0, 1023, 767));
+	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 1023 << 16, 767 << 16));
+	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1023, 767));
 
 	/* Simple scaling tests. */
 	set_src(&plane_state, 0, 0, 512 << 16, 384 << 16);
@@ -157,8 +167,8 @@ static void igt_check_plane_state(struct kunit *test)
 						  false, false);
 	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Upscaling exactly 2x should work\n");
 	KUNIT_EXPECT_TRUE(test, plane_state.visible);
-	KUNIT_EXPECT_TRUE(test, check_src_eq(&plane_state, 0, 0, 512 << 16, 384 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(&plane_state, 0, 0, 1024, 768));
+	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 512 << 16, 384 << 16));
+	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
 
 	set_src(&plane_state, 0, 0, 2048 << 16, 1536 << 16);
 	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
@@ -170,8 +180,8 @@ static void igt_check_plane_state(struct kunit *test)
 						  0x20000, false, false);
 	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed with exact scaling limit\n");
 	KUNIT_EXPECT_TRUE(test, plane_state.visible);
-	KUNIT_EXPECT_TRUE(test, check_src_eq(&plane_state, 0, 0, 2048 << 16, 1536 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(&plane_state, 0, 0, 1024, 768));
+	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 2048 << 16, 1536 << 16));
+	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
 
 	/* Testing rounding errors. */
 	set_src(&plane_state, 0, 0, 0x40001, 0x40001);
@@ -182,8 +192,8 @@ static void igt_check_plane_state(struct kunit *test)
 						  true, false);
 	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to exact multiple");
 	KUNIT_EXPECT_TRUE(test, plane_state.visible);
-	KUNIT_EXPECT_TRUE(test, check_src_eq(&plane_state, 0, 0, 2 << 16, 2 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(&plane_state, 1022, 766, 2, 2));
+	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 2 << 16, 2 << 16));
+	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 1022, 766, 2, 2));
 
 	set_src(&plane_state, 0x20001, 0x20001, 0x4040001, 0x3040001);
 	set_crtc(&plane_state, -2, -2, 1028, 772);
@@ -193,9 +203,9 @@ static void igt_check_plane_state(struct kunit *test)
 						  false, false);
 	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to exact multiple");
 	KUNIT_EXPECT_TRUE(test, plane_state.visible);
-	KUNIT_EXPECT_TRUE(test, check_src_eq(&plane_state, 0x40002, 0x40002,
+	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0x40002, 0x40002,
 					     1024 << 16, 768 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(&plane_state, 0, 0, 1024, 768));
+	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
 
 	set_src(&plane_state, 0, 0, 0x3ffff, 0x3ffff);
 	set_crtc(&plane_state, 1022, 766, 4, 4);
@@ -206,8 +216,8 @@ static void igt_check_plane_state(struct kunit *test)
 	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to exact multiple");
 	KUNIT_EXPECT_TRUE(test, plane_state.visible);
 	/* Should not be rounded to 0x20001, which would be upscaling. */
-	KUNIT_EXPECT_TRUE(test, check_src_eq(&plane_state, 0, 0, 2 << 16, 2 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(&plane_state, 1022, 766, 2, 2));
+	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 2 << 16, 2 << 16));
+	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 1022, 766, 2, 2));
 
 	set_src(&plane_state, 0x1ffff, 0x1ffff, 0x403ffff, 0x303ffff);
 	set_crtc(&plane_state, -2, -2, 1028, 772);
@@ -217,9 +227,9 @@ static void igt_check_plane_state(struct kunit *test)
 						  false, false);
 	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to exact multiple");
 	KUNIT_EXPECT_TRUE(test, plane_state.visible);
-	KUNIT_EXPECT_TRUE(test, check_src_eq(&plane_state, 0x3fffe, 0x3fffe,
+	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0x3fffe, 0x3fffe,
 					     1024 << 16, 768 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(&plane_state, 0, 0, 1024, 768));
+	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
 }
 
 static struct kunit_case drm_plane_helper_test[] = {
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] drm/plane_helper: Split into parameterized test cases
  2022-08-25 12:48 [PATCH 1/2] drm/plane_helper: Print actual/expected values on failure Michał Winiarski
@ 2022-08-25 12:48 ` Michał Winiarski
  2022-08-26 22:34   ` Maíra Canal
  0 siblings, 1 reply; 4+ messages in thread
From: Michał Winiarski @ 2022-08-25 12:48 UTC (permalink / raw)
  To: dri-devel
  Cc: Michał Winiarski, Thomas Zimmermann, David Airlie,
	Daniel Latypov, Javier Martinez Canillas, Maíra Canal,
	Sam Ravnborg

The test was constructed as a single function (test case) which checks
multiple conditions, calling the function that is tested multiple times
with different arguments.
This usually means that it can be easily converted into multiple test
cases.
Split igt_check_plane_state into two parameterized test cases,
drm_check_plane_state and drm_check_invalid_plane_state.

Passing output:
============================================================
============== drm_plane_helper (2 subtests) ===============
================== drm_check_plane_state ===================
[PASSED] clipping_simple
[PASSED] clipping_rotate_reflect
[PASSED] positioning_simple
[PASSED] upscaling
[PASSED] downscaling
[PASSED] rounding1
[PASSED] rounding2
[PASSED] rounding3
[PASSED] rounding4
============== [PASSED] drm_check_plane_state ==============
============== drm_check_invalid_plane_state ===============
[PASSED] positioning_invalid
[PASSED] upscaling_invalid
[PASSED] downscaling_invalid
========== [PASSED] drm_check_invalid_plane_state ==========
================ [PASSED] drm_plane_helper =================
============================================================
Testing complete. Ran 12 tests: passed: 12

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/tests/drm_plane_helper_test.c | 419 +++++++++++-------
 1 file changed, 255 insertions(+), 164 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_plane_helper_test.c b/drivers/gpu/drm/tests/drm_plane_helper_test.c
index 0bbd42d2d37b..cb8607e5c737 100644
--- a/drivers/gpu/drm/tests/drm_plane_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_plane_helper_test.c
@@ -12,14 +12,71 @@
 #include <drm/drm_modes.h>
 #include <drm/drm_rect.h>
 
-static void set_src(struct drm_plane_state *plane_state,
-		    unsigned int src_x, unsigned int src_y,
-		    unsigned int src_w, unsigned int src_h)
+static const struct drm_crtc_state crtc_state = {
+	.crtc = ZERO_SIZE_PTR,
+	.enable = true,
+	.active = true,
+	.mode = {
+		DRM_MODE("1024x768", 0, 65000, 1024, 1048,
+			 1184, 1344, 0, 768, 771, 777, 806, 0,
+			 DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC)
+	},
+};
+
+struct drm_check_plane_state_test {
+	const char *name;
+	const char *msg;
+	struct {
+		unsigned int x;
+		unsigned int y;
+		unsigned int w;
+		unsigned int h;
+	} src, src_expected;
+	struct {
+		int x;
+		int y;
+		unsigned int w;
+		unsigned int h;
+	} crtc, crtc_expected;
+	unsigned int rotation;
+	int min_scale;
+	int max_scale;
+	bool can_position;
+};
+
+static int drm_plane_helper_init(struct kunit *test)
 {
-	plane_state->src_x = src_x;
-	plane_state->src_y = src_y;
-	plane_state->src_w = src_w;
-	plane_state->src_h = src_h;
+	const struct drm_check_plane_state_test *params = test->param_value;
+	struct drm_plane *plane;
+	struct drm_framebuffer *fb;
+	struct drm_plane_state *mock;
+
+	plane = kunit_kzalloc(test, sizeof(*plane), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, plane);
+
+	fb = kunit_kzalloc(test, sizeof(*fb), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, fb);
+	fb->width = 2048;
+	fb->height = 2048;
+
+	mock = kunit_kzalloc(test, sizeof(*mock), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, mock);
+	mock->plane = plane;
+	mock->crtc = ZERO_SIZE_PTR;
+	mock->fb = fb;
+	mock->rotation = params->rotation;
+	mock->src_x = params->src.x;
+	mock->src_y = params->src.y;
+	mock->src_w = params->src.w;
+	mock->src_h = params->src.h;
+	mock->crtc_x = params->crtc.x;
+	mock->crtc_y = params->crtc.y;
+	mock->crtc_w = params->crtc.w;
+	mock->crtc_h = params->crtc.h;
+
+	test->priv = mock;
+
+	return 0;
 }
 
 static bool check_src_eq(struct kunit *test, struct drm_plane_state *plane_state,
@@ -54,16 +111,6 @@ static bool check_src_eq(struct kunit *test, struct drm_plane_state *plane_state
 	return true;
 }
 
-static void set_crtc(struct drm_plane_state *plane_state,
-		     int crtc_x, int crtc_y,
-		     unsigned int crtc_w, unsigned int crtc_h)
-{
-	plane_state->crtc_x = crtc_x;
-	plane_state->crtc_y = crtc_y;
-	plane_state->crtc_w = crtc_w;
-	plane_state->crtc_h = crtc_h;
-}
-
 static bool check_crtc_eq(struct kunit *test, struct drm_plane_state *plane_state,
 			  int crtc_x, int crtc_y,
 			  unsigned int crtc_w, unsigned int crtc_h)
@@ -83,162 +130,206 @@ static bool check_crtc_eq(struct kunit *test, struct drm_plane_state *plane_stat
 	return true;
 }
 
-static void igt_check_plane_state(struct kunit *test)
+static void drm_check_plane_state(struct kunit *test)
+{
+	const struct drm_check_plane_state_test *params = test->param_value;
+	struct drm_plane_state *plane_state = test->priv;
+
+	KUNIT_ASSERT_EQ_MSG(test,
+			    drm_atomic_helper_check_plane_state(plane_state, &crtc_state,
+								params->min_scale,
+								params->max_scale,
+								params->can_position, false),
+			    0, params->msg);
+	KUNIT_EXPECT_TRUE(test, plane_state->visible);
+	check_src_eq(test, plane_state, params->src_expected.x, params->src_expected.y,
+		     params->src_expected.w, params->src_expected.h);
+	check_crtc_eq(test, plane_state, params->crtc_expected.x, params->crtc_expected.y,
+		      params->crtc_expected.w, params->crtc_expected.h);
+}
+
+static void drm_check_plane_state_desc(const struct drm_check_plane_state_test *t,
+				       char *desc)
 {
-	int ret;
-
-	static const struct drm_crtc_state crtc_state = {
-		.crtc = ZERO_SIZE_PTR,
-		.enable = true,
-		.active = true,
-		.mode = {
-			DRM_MODE("1024x768", 0, 65000, 1024, 1048, 1184, 1344, 0, 768, 771,
-				 777, 806, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC)
-		},
-	};
-	static struct drm_plane plane = {
-		.dev = NULL
-	};
-	static struct drm_framebuffer fb = {
-		.width = 2048,
-		.height = 2048
-	};
-	static struct drm_plane_state plane_state = {
-		.plane = &plane,
-		.crtc = ZERO_SIZE_PTR,
-		.fb = &fb,
-		.rotation = DRM_MODE_ROTATE_0
-	};
-
-	/* Simple clipping, no scaling. */
-	set_src(&plane_state, 0, 0, fb.width << 16, fb.height << 16);
-	set_crtc(&plane_state, 0, 0, fb.width, fb.height);
-	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
-						  DRM_PLANE_NO_SCALING,
-						  DRM_PLANE_NO_SCALING,
-						  false, false);
-	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Simple clipping check should pass\n");
-	KUNIT_EXPECT_TRUE(test, plane_state.visible);
-	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 1024 << 16, 768 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
-
-	/* Rotated clipping + reflection, no scaling. */
-	plane_state.rotation = DRM_MODE_ROTATE_90 | DRM_MODE_REFLECT_X;
-	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
-						  DRM_PLANE_NO_SCALING,
-						  DRM_PLANE_NO_SCALING,
-						  false, false);
-	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Rotated clipping check should pass\n");
-	KUNIT_EXPECT_TRUE(test, plane_state.visible);
-	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 768 << 16, 1024 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
-	plane_state.rotation = DRM_MODE_ROTATE_0;
-
-	/* Check whether positioning works correctly. */
-	set_src(&plane_state, 0, 0, 1023 << 16, 767 << 16);
-	set_crtc(&plane_state, 0, 0, 1023, 767);
-	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
-						  DRM_PLANE_NO_SCALING,
-						  DRM_PLANE_NO_SCALING,
-						  false, false);
-	KUNIT_EXPECT_TRUE_MSG(test, ret,
-			      "Should not be able to position on the crtc with can_position=false\n");
-
-	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
-						  DRM_PLANE_NO_SCALING,
-						  DRM_PLANE_NO_SCALING,
-						  true, false);
-	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Simple positioning should work\n");
-	KUNIT_EXPECT_TRUE(test, plane_state.visible);
-	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 1023 << 16, 767 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1023, 767));
-
-	/* Simple scaling tests. */
-	set_src(&plane_state, 0, 0, 512 << 16, 384 << 16);
-	set_crtc(&plane_state, 0, 0, 1024, 768);
-	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
-						  0x8001,
-						  DRM_PLANE_NO_SCALING,
-						  false, false);
-	KUNIT_EXPECT_TRUE_MSG(test, ret, "Upscaling out of range should fail.\n");
-	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
-						  0x8000,
-						  DRM_PLANE_NO_SCALING,
-						  false, false);
-	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Upscaling exactly 2x should work\n");
-	KUNIT_EXPECT_TRUE(test, plane_state.visible);
-	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 512 << 16, 384 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
-
-	set_src(&plane_state, 0, 0, 2048 << 16, 1536 << 16);
-	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
-						  DRM_PLANE_NO_SCALING,
-						  0x1ffff, false, false);
-	KUNIT_EXPECT_TRUE_MSG(test, ret, "Downscaling out of range should fail.\n");
-	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
-						  DRM_PLANE_NO_SCALING,
-						  0x20000, false, false);
-	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed with exact scaling limit\n");
-	KUNIT_EXPECT_TRUE(test, plane_state.visible);
-	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 2048 << 16, 1536 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
-
-	/* Testing rounding errors. */
-	set_src(&plane_state, 0, 0, 0x40001, 0x40001);
-	set_crtc(&plane_state, 1022, 766, 4, 4);
-	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
-						  DRM_PLANE_NO_SCALING,
-						  0x10001,
-						  true, false);
-	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to exact multiple");
-	KUNIT_EXPECT_TRUE(test, plane_state.visible);
-	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 2 << 16, 2 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 1022, 766, 2, 2));
-
-	set_src(&plane_state, 0x20001, 0x20001, 0x4040001, 0x3040001);
-	set_crtc(&plane_state, -2, -2, 1028, 772);
-	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
-						  DRM_PLANE_NO_SCALING,
-						  0x10001,
-						  false, false);
-	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to exact multiple");
-	KUNIT_EXPECT_TRUE(test, plane_state.visible);
-	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0x40002, 0x40002,
-					     1024 << 16, 768 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
-
-	set_src(&plane_state, 0, 0, 0x3ffff, 0x3ffff);
-	set_crtc(&plane_state, 1022, 766, 4, 4);
-	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
-						  0xffff,
-						  DRM_PLANE_NO_SCALING,
-						  true, false);
-	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to exact multiple");
-	KUNIT_EXPECT_TRUE(test, plane_state.visible);
-	/* Should not be rounded to 0x20001, which would be upscaling. */
-	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 2 << 16, 2 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 1022, 766, 2, 2));
-
-	set_src(&plane_state, 0x1ffff, 0x1ffff, 0x403ffff, 0x303ffff);
-	set_crtc(&plane_state, -2, -2, 1028, 772);
-	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
-						  0xffff,
-						  DRM_PLANE_NO_SCALING,
-						  false, false);
-	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to exact multiple");
-	KUNIT_EXPECT_TRUE(test, plane_state.visible);
-	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0x3fffe, 0x3fffe,
-					     1024 << 16, 768 << 16));
-	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
+	sprintf(desc, "%s", t->name);
 }
 
+static const struct drm_check_plane_state_test drm_check_plane_state_tests[] = {
+	{
+		.name = "clipping_simple",
+		.msg = "Simple clipping check should pass",
+		.src = { 0, 0,
+			 2048 << 16,
+			 2048 << 16 },
+		.crtc = { 0, 0, 2048, 2048 },
+		.rotation = DRM_MODE_ROTATE_0,
+		.min_scale = DRM_PLANE_NO_SCALING,
+		.max_scale = DRM_PLANE_NO_SCALING,
+		.can_position = false,
+		.src_expected = { 0, 0, 1024 << 16, 768 << 16 },
+		.crtc_expected = { 0, 0, 1024, 768 },
+	},
+	{
+		.name = "clipping_rotate_reflect",
+		.msg = "Rotated clipping check should pass",
+		.src = { 0, 0,
+			 2048 << 16,
+			 2048 << 16 },
+		.crtc = { 0, 0, 2048, 2048 },
+		.rotation = DRM_MODE_ROTATE_90 | DRM_MODE_REFLECT_X,
+		.min_scale = DRM_PLANE_NO_SCALING,
+		.max_scale = DRM_PLANE_NO_SCALING,
+		.can_position = false,
+		.src_expected = { 0, 0, 768 << 16, 1024 << 16 },
+		.crtc_expected = { 0, 0, 1024, 768 },
+	},
+	{
+		.name = "positioning_simple",
+		.msg = "Simple positioning should work",
+		.src = { 0, 0, 1023 << 16, 767 << 16 },
+		.crtc = { 0, 0, 1023, 767 },
+		.rotation = DRM_MODE_ROTATE_0,
+		.min_scale = DRM_PLANE_NO_SCALING,
+		.max_scale = DRM_PLANE_NO_SCALING,
+		.can_position = true,
+		.src_expected = { 0, 0, 1023 << 16, 767 << 16 },
+		.crtc_expected = { 0, 0, 1023, 767 },
+	},
+	{
+		.name = "upscaling",
+		.msg = "Upscaling exactly 2x should work",
+		.src = { 0, 0, 512 << 16, 384 << 16 },
+		.crtc = { 0, 0, 1024, 768 },
+		.rotation = DRM_MODE_ROTATE_0,
+		.min_scale = 0x8000,
+		.max_scale = DRM_PLANE_NO_SCALING,
+		.can_position = false,
+		.src_expected = { 0, 0, 512 << 16, 384 << 16 },
+		.crtc_expected = { 0, 0, 1024, 768 },
+	},
+	{
+		.name = "downscaling",
+		.msg = "Should succeed with exact scaling limit",
+		.src = { 0, 0, 2048 << 16, 1536 << 16 },
+		.crtc = { 0, 0, 1024, 768 },
+		.rotation = DRM_MODE_ROTATE_0,
+		.min_scale = DRM_PLANE_NO_SCALING,
+		.max_scale = 0x20000,
+		.can_position = false,
+		.src_expected = { 0, 0, 2048 << 16, 1536 << 16 },
+		.crtc_expected = { 0, 0, 1024, 768 },
+	},
+	{
+		.name = "rounding1",
+		.msg = "Should succeed by clipping to exact multiple",
+		.src = { 0, 0, 0x40001, 0x40001 },
+		.crtc = { 1022, 766, 4, 4 },
+		.rotation = DRM_MODE_ROTATE_0,
+		.min_scale = DRM_PLANE_NO_SCALING,
+		.max_scale = 0x10001,
+		.can_position = true,
+		.src_expected = { 0, 0, 2 << 16, 2 << 16 },
+		.crtc_expected = { 1022, 766, 2, 2 },
+	},
+	{
+		.name = "rounding2",
+		.msg = "Should succeed by clipping to exact multiple",
+		.src = { 0x20001, 0x20001, 0x4040001, 0x3040001 },
+		.crtc = { -2, -2, 1028, 772 },
+		.rotation = DRM_MODE_ROTATE_0,
+		.min_scale = DRM_PLANE_NO_SCALING,
+		.max_scale = 0x10001,
+		.can_position = false,
+		.src_expected = { 0x40002, 0x40002, 1024 << 16, 768 << 16 },
+		.crtc_expected = { 0, 0, 1024, 768 },
+	},
+	{
+		.name = "rounding3",
+		.msg = "Should succeed by clipping to exact multiple",
+		.src = { 0, 0, 0x3ffff, 0x3ffff },
+		.crtc = { 1022, 766, 4, 4 },
+		.rotation = DRM_MODE_ROTATE_0,
+		.min_scale = 0xffff,
+		.max_scale = DRM_PLANE_NO_SCALING,
+		.can_position = true,
+		/* Should not be rounded to 0x20001, which would be upscaling. */
+		.src_expected = { 0, 0, 2 << 16, 2 << 16 },
+		.crtc_expected = { 1022, 766, 2, 2 },
+	},
+	{
+		.name = "rounding4",
+		.msg = "Should succeed by clipping to exact multiple",
+		.src = { 0x1ffff, 0x1ffff, 0x403ffff, 0x303ffff },
+		.crtc = { -2, -2, 1028, 772 },
+		.rotation = DRM_MODE_ROTATE_0,
+		.min_scale = 0xffff,
+		.max_scale = DRM_PLANE_NO_SCALING,
+		.can_position = false,
+		.src_expected = { 0x3fffe, 0x3fffe, 1024 << 16, 768 << 16 },
+		.crtc_expected = { 0, 0, 1024, 768 },
+	},
+};
+
+KUNIT_ARRAY_PARAM(drm_check_plane_state, drm_check_plane_state_tests, drm_check_plane_state_desc);
+
+static void drm_check_invalid_plane_state(struct kunit *test)
+{
+	const struct drm_check_plane_state_test *params = test->param_value;
+	struct drm_plane_state *plane_state = test->priv;
+
+	KUNIT_ASSERT_LT_MSG(test,
+			    drm_atomic_helper_check_plane_state(plane_state, &crtc_state,
+								params->min_scale,
+								params->max_scale,
+								params->can_position, false),
+			    0, params->msg);
+}
+
+static const struct drm_check_plane_state_test drm_check_invalid_plane_state_tests[] = {
+	{
+		.name = "positioning_invalid",
+		.msg = "Should not be able to position on the crtc with can_position=false",
+		.src = { 0, 0, 1023 << 16, 767 << 16 },
+		.crtc = { 0, 0, 1023, 767 },
+		.rotation = DRM_MODE_ROTATE_0,
+		.min_scale = DRM_PLANE_NO_SCALING,
+		.max_scale = DRM_PLANE_NO_SCALING,
+		.can_position = false,
+	},
+	{
+		.name = "upscaling_invalid",
+		.msg = "Upscaling out of range should fail",
+		.src = { 0, 0, 512 << 16, 384 << 16 },
+		.crtc = { 0, 0, 1024, 768 },
+		.rotation = DRM_MODE_ROTATE_0,
+		.min_scale = 0x8001,
+		.max_scale = DRM_PLANE_NO_SCALING,
+		.can_position = false,
+	},
+	{
+		.name = "downscaling_invalid",
+		.msg = "Downscaling out of range should fail",
+		.src = { 0, 0, 2048 << 16, 1536 << 16 },
+		.crtc = { 0, 0, 1024, 768 },
+		.rotation = DRM_MODE_ROTATE_0,
+		.min_scale = DRM_PLANE_NO_SCALING,
+		.max_scale = 0x1ffff,
+		.can_position = false,
+	},
+};
+
+KUNIT_ARRAY_PARAM(drm_check_invalid_plane_state, drm_check_invalid_plane_state_tests,
+		  drm_check_plane_state_desc);
+
 static struct kunit_case drm_plane_helper_test[] = {
-	KUNIT_CASE(igt_check_plane_state),
+	KUNIT_CASE_PARAM(drm_check_plane_state, drm_check_plane_state_gen_params),
+	KUNIT_CASE_PARAM(drm_check_invalid_plane_state, drm_check_invalid_plane_state_gen_params),
 	{}
 };
 
 static struct kunit_suite drm_plane_helper_test_suite = {
 	.name = "drm_plane_helper",
+	.init = drm_plane_helper_init,
 	.test_cases = drm_plane_helper_test,
 };
 
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] drm/plane_helper: Split into parameterized test cases
  2022-08-25 12:48 ` [PATCH 2/2] drm/plane_helper: Split into parameterized test cases Michał Winiarski
@ 2022-08-26 22:34   ` Maíra Canal
  2022-08-31 11:08     ` Michał Winiarski
  0 siblings, 1 reply; 4+ messages in thread
From: Maíra Canal @ 2022-08-26 22:34 UTC (permalink / raw)
  To: Michał Winiarski, dri-devel
  Cc: David Airlie, Daniel Latypov, Sam Ravnborg,
	Javier Martinez Canillas, Thomas Zimmermann

Hi Michał

Great patch! Just a few nits inline.

On 8/25/22 09:48, Michał Winiarski wrote:
> The test was constructed as a single function (test case) which checks
> multiple conditions, calling the function that is tested multiple times
> with different arguments.
> This usually means that it can be easily converted into multiple test
> cases.
> Split igt_check_plane_state into two parameterized test cases,
> drm_check_plane_state and drm_check_invalid_plane_state.
> 
> Passing output:
> ============================================================
> ============== drm_plane_helper (2 subtests) ===============
> ================== drm_check_plane_state ===================
> [PASSED] clipping_simple
> [PASSED] clipping_rotate_reflect
> [PASSED] positioning_simple
> [PASSED] upscaling
> [PASSED] downscaling
> [PASSED] rounding1
> [PASSED] rounding2
> [PASSED] rounding3
> [PASSED] rounding4
> ============== [PASSED] drm_check_plane_state ==============
> ============== drm_check_invalid_plane_state ===============
> [PASSED] positioning_invalid
> [PASSED] upscaling_invalid
> [PASSED] downscaling_invalid
> ========== [PASSED] drm_check_invalid_plane_state ==========
> ================ [PASSED] drm_plane_helper =================
> ============================================================
> Testing complete. Ran 12 tests: passed: 12
> 
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
>  drivers/gpu/drm/tests/drm_plane_helper_test.c | 419 +++++++++++-------
>  1 file changed, 255 insertions(+), 164 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tests/drm_plane_helper_test.c b/drivers/gpu/drm/tests/drm_plane_helper_test.c
> index 0bbd42d2d37b..cb8607e5c737 100644
> --- a/drivers/gpu/drm/tests/drm_plane_helper_test.c
> +++ b/drivers/gpu/drm/tests/drm_plane_helper_test.c
> @@ -12,14 +12,71 @@
>  #include <drm/drm_modes.h>
>  #include <drm/drm_rect.h>
>  
> -static void set_src(struct drm_plane_state *plane_state,
> -		    unsigned int src_x, unsigned int src_y,
> -		    unsigned int src_w, unsigned int src_h)
> +static const struct drm_crtc_state crtc_state = {
> +	.crtc = ZERO_SIZE_PTR,
> +	.enable = true,
> +	.active = true,
> +	.mode = {
> +		DRM_MODE("1024x768", 0, 65000, 1024, 1048,
> +			 1184, 1344, 0, 768, 771, 777, 806, 0,
> +			 DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC)
> +	},
> +};
> +
> +struct drm_check_plane_state_test {
> +	const char *name;
> +	const char *msg;
> +	struct {
> +		unsigned int x;
> +		unsigned int y;
> +		unsigned int w;
> +		unsigned int h;
> +	} src, src_expected;
> +	struct {
> +		int x;
> +		int y;
> +		unsigned int w;
> +		unsigned int h;
> +	} crtc, crtc_expected;
> +	unsigned int rotation;
> +	int min_scale;
> +	int max_scale;
> +	bool can_position;
> +};
> +
> +static int drm_plane_helper_init(struct kunit *test)
>  {
> -	plane_state->src_x = src_x;
> -	plane_state->src_y = src_y;
> -	plane_state->src_w = src_w;
> -	plane_state->src_h = src_h;
> +	const struct drm_check_plane_state_test *params = test->param_value;
> +	struct drm_plane *plane;
> +	struct drm_framebuffer *fb;
> +	struct drm_plane_state *mock;
> +
> +	plane = kunit_kzalloc(test, sizeof(*plane), GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_NULL(test, plane);
> +
> +	fb = kunit_kzalloc(test, sizeof(*fb), GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_NULL(test, fb);
> +	fb->width = 2048;
> +	fb->height = 2048;
> +
> +	mock = kunit_kzalloc(test, sizeof(*mock), GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_NULL(test, mock);
> +	mock->plane = plane;
> +	mock->crtc = ZERO_SIZE_PTR;
> +	mock->fb = fb;
> +	mock->rotation = params->rotation;
> +	mock->src_x = params->src.x;
> +	mock->src_y = params->src.y;
> +	mock->src_w = params->src.w;
> +	mock->src_h = params->src.h;
> +	mock->crtc_x = params->crtc.x;
> +	mock->crtc_y = params->crtc.y;
> +	mock->crtc_w = params->crtc.w;
> +	mock->crtc_h = params->crtc.h;
> +
> +	test->priv = mock;
> +
> +	return 0;
>  }
>  
>  static bool check_src_eq(struct kunit *test, struct drm_plane_state *plane_state,
> @@ -54,16 +111,6 @@ static bool check_src_eq(struct kunit *test, struct drm_plane_state *plane_state
>  	return true;
>  }
>  
> -static void set_crtc(struct drm_plane_state *plane_state,
> -		     int crtc_x, int crtc_y,
> -		     unsigned int crtc_w, unsigned int crtc_h)
> -{
> -	plane_state->crtc_x = crtc_x;
> -	plane_state->crtc_y = crtc_y;
> -	plane_state->crtc_w = crtc_w;
> -	plane_state->crtc_h = crtc_h;
> -}
> -
>  static bool check_crtc_eq(struct kunit *test, struct drm_plane_state *plane_state,
>  			  int crtc_x, int crtc_y,
>  			  unsigned int crtc_w, unsigned int crtc_h)
> @@ -83,162 +130,206 @@ static bool check_crtc_eq(struct kunit *test, struct drm_plane_state *plane_stat
>  	return true;
>  }
>  
> -static void igt_check_plane_state(struct kunit *test)
> +static void drm_check_plane_state(struct kunit *test)
> +{
> +	const struct drm_check_plane_state_test *params = test->param_value;
> +	struct drm_plane_state *plane_state = test->priv;
> +
> +	KUNIT_ASSERT_EQ_MSG(test,
> +			    drm_atomic_helper_check_plane_state(plane_state, &crtc_state,
> +								params->min_scale,
> +								params->max_scale,
> +								params->can_position, false),
> +			    0, params->msg);
> +	KUNIT_EXPECT_TRUE(test, plane_state->visible);
> +	check_src_eq(test, plane_state, params->src_expected.x, params->src_expected.y,
> +		     params->src_expected.w, params->src_expected.h);
> +	check_crtc_eq(test, plane_state, params->crtc_expected.x, params->crtc_expected.y,
> +		      params->crtc_expected.w, params->crtc_expected.h);

In order to preserve the same function of the tests, I believe that
check_src_eq and check_crtc_eq should be inside a KUNIT_EXPECT_TRUE. If
there is a reason for not using KUNIT_EXPECT_TRUE, then check_src_eq and
check_crtc_eq should not return a boolean.

Moreover, now that those functions are being called just once, you could
just add some expectations to this function, such as:

KUNIT_EXPECT_GE_MSG(test, plane_state->src.x1, 0,
	"src x coordinate %x should never be below 0, src: "
	DRM_RECT_FP_FMT, plane_state->src.x1,
	DRM_RECT_FP_ARG(&plane_state->src));

Best Regards,
- Maíra Canal

> +}
> +
> +static void drm_check_plane_state_desc(const struct drm_check_plane_state_test *t,
> +				       char *desc)
>  {
> -	int ret;
> -
> -	static const struct drm_crtc_state crtc_state = {
> -		.crtc = ZERO_SIZE_PTR,
> -		.enable = true,
> -		.active = true,
> -		.mode = {
> -			DRM_MODE("1024x768", 0, 65000, 1024, 1048, 1184, 1344, 0, 768, 771,
> -				 777, 806, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC)
> -		},
> -	};
> -	static struct drm_plane plane = {
> -		.dev = NULL
> -	};
> -	static struct drm_framebuffer fb = {
> -		.width = 2048,
> -		.height = 2048
> -	};
> -	static struct drm_plane_state plane_state = {
> -		.plane = &plane,
> -		.crtc = ZERO_SIZE_PTR,
> -		.fb = &fb,
> -		.rotation = DRM_MODE_ROTATE_0
> -	};
> -
> -	/* Simple clipping, no scaling. */
> -	set_src(&plane_state, 0, 0, fb.width << 16, fb.height << 16);
> -	set_crtc(&plane_state, 0, 0, fb.width, fb.height);
> -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> -						  DRM_PLANE_NO_SCALING,
> -						  DRM_PLANE_NO_SCALING,
> -						  false, false);
> -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Simple clipping check should pass\n");
> -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 1024 << 16, 768 << 16));
> -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
> -
> -	/* Rotated clipping + reflection, no scaling. */
> -	plane_state.rotation = DRM_MODE_ROTATE_90 | DRM_MODE_REFLECT_X;
> -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> -						  DRM_PLANE_NO_SCALING,
> -						  DRM_PLANE_NO_SCALING,
> -						  false, false);
> -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Rotated clipping check should pass\n");
> -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 768 << 16, 1024 << 16));
> -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
> -	plane_state.rotation = DRM_MODE_ROTATE_0;
> -
> -	/* Check whether positioning works correctly. */
> -	set_src(&plane_state, 0, 0, 1023 << 16, 767 << 16);
> -	set_crtc(&plane_state, 0, 0, 1023, 767);
> -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> -						  DRM_PLANE_NO_SCALING,
> -						  DRM_PLANE_NO_SCALING,
> -						  false, false);
> -	KUNIT_EXPECT_TRUE_MSG(test, ret,
> -			      "Should not be able to position on the crtc with can_position=false\n");
> -
> -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> -						  DRM_PLANE_NO_SCALING,
> -						  DRM_PLANE_NO_SCALING,
> -						  true, false);
> -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Simple positioning should work\n");
> -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 1023 << 16, 767 << 16));
> -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1023, 767));
> -
> -	/* Simple scaling tests. */
> -	set_src(&plane_state, 0, 0, 512 << 16, 384 << 16);
> -	set_crtc(&plane_state, 0, 0, 1024, 768);
> -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> -						  0x8001,
> -						  DRM_PLANE_NO_SCALING,
> -						  false, false);
> -	KUNIT_EXPECT_TRUE_MSG(test, ret, "Upscaling out of range should fail.\n");
> -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> -						  0x8000,
> -						  DRM_PLANE_NO_SCALING,
> -						  false, false);
> -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Upscaling exactly 2x should work\n");
> -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 512 << 16, 384 << 16));
> -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
> -
> -	set_src(&plane_state, 0, 0, 2048 << 16, 1536 << 16);
> -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> -						  DRM_PLANE_NO_SCALING,
> -						  0x1ffff, false, false);
> -	KUNIT_EXPECT_TRUE_MSG(test, ret, "Downscaling out of range should fail.\n");
> -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> -						  DRM_PLANE_NO_SCALING,
> -						  0x20000, false, false);
> -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed with exact scaling limit\n");
> -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 2048 << 16, 1536 << 16));
> -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
> -
> -	/* Testing rounding errors. */
> -	set_src(&plane_state, 0, 0, 0x40001, 0x40001);
> -	set_crtc(&plane_state, 1022, 766, 4, 4);
> -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> -						  DRM_PLANE_NO_SCALING,
> -						  0x10001,
> -						  true, false);
> -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to exact multiple");
> -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 2 << 16, 2 << 16));
> -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 1022, 766, 2, 2));
> -
> -	set_src(&plane_state, 0x20001, 0x20001, 0x4040001, 0x3040001);
> -	set_crtc(&plane_state, -2, -2, 1028, 772);
> -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> -						  DRM_PLANE_NO_SCALING,
> -						  0x10001,
> -						  false, false);
> -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to exact multiple");
> -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0x40002, 0x40002,
> -					     1024 << 16, 768 << 16));
> -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
> -
> -	set_src(&plane_state, 0, 0, 0x3ffff, 0x3ffff);
> -	set_crtc(&plane_state, 1022, 766, 4, 4);
> -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> -						  0xffff,
> -						  DRM_PLANE_NO_SCALING,
> -						  true, false);
> -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to exact multiple");
> -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> -	/* Should not be rounded to 0x20001, which would be upscaling. */
> -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 2 << 16, 2 << 16));
> -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 1022, 766, 2, 2));
> -
> -	set_src(&plane_state, 0x1ffff, 0x1ffff, 0x403ffff, 0x303ffff);
> -	set_crtc(&plane_state, -2, -2, 1028, 772);
> -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> -						  0xffff,
> -						  DRM_PLANE_NO_SCALING,
> -						  false, false);
> -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to exact multiple");
> -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0x3fffe, 0x3fffe,
> -					     1024 << 16, 768 << 16));
> -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
> +	sprintf(desc, "%s", t->name);
>  }
>  
> +static const struct drm_check_plane_state_test drm_check_plane_state_tests[] = {
> +	{
> +		.name = "clipping_simple",
> +		.msg = "Simple clipping check should pass",
> +		.src = { 0, 0,
> +			 2048 << 16,
> +			 2048 << 16 },
> +		.crtc = { 0, 0, 2048, 2048 },
> +		.rotation = DRM_MODE_ROTATE_0,
> +		.min_scale = DRM_PLANE_NO_SCALING,
> +		.max_scale = DRM_PLANE_NO_SCALING,
> +		.can_position = false,
> +		.src_expected = { 0, 0, 1024 << 16, 768 << 16 },
> +		.crtc_expected = { 0, 0, 1024, 768 },
> +	},
> +	{
> +		.name = "clipping_rotate_reflect",
> +		.msg = "Rotated clipping check should pass",
> +		.src = { 0, 0,
> +			 2048 << 16,
> +			 2048 << 16 },
> +		.crtc = { 0, 0, 2048, 2048 },
> +		.rotation = DRM_MODE_ROTATE_90 | DRM_MODE_REFLECT_X,
> +		.min_scale = DRM_PLANE_NO_SCALING,
> +		.max_scale = DRM_PLANE_NO_SCALING,
> +		.can_position = false,
> +		.src_expected = { 0, 0, 768 << 16, 1024 << 16 },
> +		.crtc_expected = { 0, 0, 1024, 768 },
> +	},
> +	{
> +		.name = "positioning_simple",
> +		.msg = "Simple positioning should work",
> +		.src = { 0, 0, 1023 << 16, 767 << 16 },
> +		.crtc = { 0, 0, 1023, 767 },
> +		.rotation = DRM_MODE_ROTATE_0,
> +		.min_scale = DRM_PLANE_NO_SCALING,
> +		.max_scale = DRM_PLANE_NO_SCALING,
> +		.can_position = true,
> +		.src_expected = { 0, 0, 1023 << 16, 767 << 16 },
> +		.crtc_expected = { 0, 0, 1023, 767 },
> +	},
> +	{
> +		.name = "upscaling",
> +		.msg = "Upscaling exactly 2x should work",
> +		.src = { 0, 0, 512 << 16, 384 << 16 },
> +		.crtc = { 0, 0, 1024, 768 },
> +		.rotation = DRM_MODE_ROTATE_0,
> +		.min_scale = 0x8000,
> +		.max_scale = DRM_PLANE_NO_SCALING,
> +		.can_position = false,
> +		.src_expected = { 0, 0, 512 << 16, 384 << 16 },
> +		.crtc_expected = { 0, 0, 1024, 768 },
> +	},
> +	{
> +		.name = "downscaling",
> +		.msg = "Should succeed with exact scaling limit",
> +		.src = { 0, 0, 2048 << 16, 1536 << 16 },
> +		.crtc = { 0, 0, 1024, 768 },
> +		.rotation = DRM_MODE_ROTATE_0,
> +		.min_scale = DRM_PLANE_NO_SCALING,
> +		.max_scale = 0x20000,
> +		.can_position = false,
> +		.src_expected = { 0, 0, 2048 << 16, 1536 << 16 },
> +		.crtc_expected = { 0, 0, 1024, 768 },
> +	},
> +	{
> +		.name = "rounding1",
> +		.msg = "Should succeed by clipping to exact multiple",
> +		.src = { 0, 0, 0x40001, 0x40001 },
> +		.crtc = { 1022, 766, 4, 4 },
> +		.rotation = DRM_MODE_ROTATE_0,
> +		.min_scale = DRM_PLANE_NO_SCALING,
> +		.max_scale = 0x10001,
> +		.can_position = true,
> +		.src_expected = { 0, 0, 2 << 16, 2 << 16 },
> +		.crtc_expected = { 1022, 766, 2, 2 },
> +	},
> +	{
> +		.name = "rounding2",
> +		.msg = "Should succeed by clipping to exact multiple",
> +		.src = { 0x20001, 0x20001, 0x4040001, 0x3040001 },
> +		.crtc = { -2, -2, 1028, 772 },
> +		.rotation = DRM_MODE_ROTATE_0,
> +		.min_scale = DRM_PLANE_NO_SCALING,
> +		.max_scale = 0x10001,
> +		.can_position = false,
> +		.src_expected = { 0x40002, 0x40002, 1024 << 16, 768 << 16 },
> +		.crtc_expected = { 0, 0, 1024, 768 },
> +	},
> +	{
> +		.name = "rounding3",
> +		.msg = "Should succeed by clipping to exact multiple",
> +		.src = { 0, 0, 0x3ffff, 0x3ffff },
> +		.crtc = { 1022, 766, 4, 4 },
> +		.rotation = DRM_MODE_ROTATE_0,
> +		.min_scale = 0xffff,
> +		.max_scale = DRM_PLANE_NO_SCALING,
> +		.can_position = true,
> +		/* Should not be rounded to 0x20001, which would be upscaling. */
> +		.src_expected = { 0, 0, 2 << 16, 2 << 16 },
> +		.crtc_expected = { 1022, 766, 2, 2 },
> +	},
> +	{
> +		.name = "rounding4",
> +		.msg = "Should succeed by clipping to exact multiple",
> +		.src = { 0x1ffff, 0x1ffff, 0x403ffff, 0x303ffff },
> +		.crtc = { -2, -2, 1028, 772 },
> +		.rotation = DRM_MODE_ROTATE_0,
> +		.min_scale = 0xffff,
> +		.max_scale = DRM_PLANE_NO_SCALING,
> +		.can_position = false,
> +		.src_expected = { 0x3fffe, 0x3fffe, 1024 << 16, 768 << 16 },
> +		.crtc_expected = { 0, 0, 1024, 768 },
> +	},
> +};
> +
> +KUNIT_ARRAY_PARAM(drm_check_plane_state, drm_check_plane_state_tests, drm_check_plane_state_desc);
> +
> +static void drm_check_invalid_plane_state(struct kunit *test)
> +{
> +	const struct drm_check_plane_state_test *params = test->param_value;
> +	struct drm_plane_state *plane_state = test->priv;
> +
> +	KUNIT_ASSERT_LT_MSG(test,
> +			    drm_atomic_helper_check_plane_state(plane_state, &crtc_state,
> +								params->min_scale,
> +								params->max_scale,
> +								params->can_position, false),
> +			    0, params->msg);
> +}
> +
> +static const struct drm_check_plane_state_test drm_check_invalid_plane_state_tests[] = {
> +	{
> +		.name = "positioning_invalid",
> +		.msg = "Should not be able to position on the crtc with can_position=false",
> +		.src = { 0, 0, 1023 << 16, 767 << 16 },
> +		.crtc = { 0, 0, 1023, 767 },
> +		.rotation = DRM_MODE_ROTATE_0,
> +		.min_scale = DRM_PLANE_NO_SCALING,
> +		.max_scale = DRM_PLANE_NO_SCALING,
> +		.can_position = false,
> +	},
> +	{
> +		.name = "upscaling_invalid",
> +		.msg = "Upscaling out of range should fail",
> +		.src = { 0, 0, 512 << 16, 384 << 16 },
> +		.crtc = { 0, 0, 1024, 768 },
> +		.rotation = DRM_MODE_ROTATE_0,
> +		.min_scale = 0x8001,
> +		.max_scale = DRM_PLANE_NO_SCALING,
> +		.can_position = false,
> +	},
> +	{
> +		.name = "downscaling_invalid",
> +		.msg = "Downscaling out of range should fail",
> +		.src = { 0, 0, 2048 << 16, 1536 << 16 },
> +		.crtc = { 0, 0, 1024, 768 },
> +		.rotation = DRM_MODE_ROTATE_0,
> +		.min_scale = DRM_PLANE_NO_SCALING,
> +		.max_scale = 0x1ffff,
> +		.can_position = false,
> +	},
> +};
> +
> +KUNIT_ARRAY_PARAM(drm_check_invalid_plane_state, drm_check_invalid_plane_state_tests,
> +		  drm_check_plane_state_desc);
> +
>  static struct kunit_case drm_plane_helper_test[] = {
> -	KUNIT_CASE(igt_check_plane_state),
> +	KUNIT_CASE_PARAM(drm_check_plane_state, drm_check_plane_state_gen_params),
> +	KUNIT_CASE_PARAM(drm_check_invalid_plane_state, drm_check_invalid_plane_state_gen_params),
>  	{}
>  };
>  
>  static struct kunit_suite drm_plane_helper_test_suite = {
>  	.name = "drm_plane_helper",
> +	.init = drm_plane_helper_init,
>  	.test_cases = drm_plane_helper_test,
>  };
>  

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] drm/plane_helper: Split into parameterized test cases
  2022-08-26 22:34   ` Maíra Canal
@ 2022-08-31 11:08     ` Michał Winiarski
  0 siblings, 0 replies; 4+ messages in thread
From: Michał Winiarski @ 2022-08-31 11:08 UTC (permalink / raw)
  To: Maíra Canal
  Cc: David Airlie, Daniel Latypov, Javier Martinez Canillas,
	dri-devel, Thomas Zimmermann, Sam Ravnborg

On Fri, Aug 26, 2022 at 07:34:11PM -0300, Maíra Canal wrote:
> Hi Michał
> 
> Great patch! Just a few nits inline.
> 
> On 8/25/22 09:48, Michał Winiarski wrote:
> > The test was constructed as a single function (test case) which checks
> > multiple conditions, calling the function that is tested multiple times
> > with different arguments.
> > This usually means that it can be easily converted into multiple test
> > cases.
> > Split igt_check_plane_state into two parameterized test cases,
> > drm_check_plane_state and drm_check_invalid_plane_state.
> > 
> > Passing output:
> > ============================================================
> > ============== drm_plane_helper (2 subtests) ===============
> > ================== drm_check_plane_state ===================
> > [PASSED] clipping_simple
> > [PASSED] clipping_rotate_reflect
> > [PASSED] positioning_simple
> > [PASSED] upscaling
> > [PASSED] downscaling
> > [PASSED] rounding1
> > [PASSED] rounding2
> > [PASSED] rounding3
> > [PASSED] rounding4
> > ============== [PASSED] drm_check_plane_state ==============
> > ============== drm_check_invalid_plane_state ===============
> > [PASSED] positioning_invalid
> > [PASSED] upscaling_invalid
> > [PASSED] downscaling_invalid
> > ========== [PASSED] drm_check_invalid_plane_state ==========
> > ================ [PASSED] drm_plane_helper =================
> > ============================================================
> > Testing complete. Ran 12 tests: passed: 12
> > 
> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > ---
> >  drivers/gpu/drm/tests/drm_plane_helper_test.c | 419 +++++++++++-------
> >  1 file changed, 255 insertions(+), 164 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/tests/drm_plane_helper_test.c b/drivers/gpu/drm/tests/drm_plane_helper_test.c
> > index 0bbd42d2d37b..cb8607e5c737 100644
> > --- a/drivers/gpu/drm/tests/drm_plane_helper_test.c
> > +++ b/drivers/gpu/drm/tests/drm_plane_helper_test.c
> > @@ -12,14 +12,71 @@
> >  #include <drm/drm_modes.h>
> >  #include <drm/drm_rect.h>
> >  
> > -static void set_src(struct drm_plane_state *plane_state,
> > -		    unsigned int src_x, unsigned int src_y,
> > -		    unsigned int src_w, unsigned int src_h)
> > +static const struct drm_crtc_state crtc_state = {
> > +	.crtc = ZERO_SIZE_PTR,
> > +	.enable = true,
> > +	.active = true,
> > +	.mode = {
> > +		DRM_MODE("1024x768", 0, 65000, 1024, 1048,
> > +			 1184, 1344, 0, 768, 771, 777, 806, 0,
> > +			 DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC)
> > +	},
> > +};
> > +
> > +struct drm_check_plane_state_test {
> > +	const char *name;
> > +	const char *msg;
> > +	struct {
> > +		unsigned int x;
> > +		unsigned int y;
> > +		unsigned int w;
> > +		unsigned int h;
> > +	} src, src_expected;
> > +	struct {
> > +		int x;
> > +		int y;
> > +		unsigned int w;
> > +		unsigned int h;
> > +	} crtc, crtc_expected;
> > +	unsigned int rotation;
> > +	int min_scale;
> > +	int max_scale;
> > +	bool can_position;
> > +};
> > +
> > +static int drm_plane_helper_init(struct kunit *test)
> >  {
> > -	plane_state->src_x = src_x;
> > -	plane_state->src_y = src_y;
> > -	plane_state->src_w = src_w;
> > -	plane_state->src_h = src_h;
> > +	const struct drm_check_plane_state_test *params = test->param_value;
> > +	struct drm_plane *plane;
> > +	struct drm_framebuffer *fb;
> > +	struct drm_plane_state *mock;
> > +
> > +	plane = kunit_kzalloc(test, sizeof(*plane), GFP_KERNEL);
> > +	KUNIT_ASSERT_NOT_NULL(test, plane);
> > +
> > +	fb = kunit_kzalloc(test, sizeof(*fb), GFP_KERNEL);
> > +	KUNIT_ASSERT_NOT_NULL(test, fb);
> > +	fb->width = 2048;
> > +	fb->height = 2048;
> > +
> > +	mock = kunit_kzalloc(test, sizeof(*mock), GFP_KERNEL);
> > +	KUNIT_ASSERT_NOT_NULL(test, mock);
> > +	mock->plane = plane;
> > +	mock->crtc = ZERO_SIZE_PTR;
> > +	mock->fb = fb;
> > +	mock->rotation = params->rotation;
> > +	mock->src_x = params->src.x;
> > +	mock->src_y = params->src.y;
> > +	mock->src_w = params->src.w;
> > +	mock->src_h = params->src.h;
> > +	mock->crtc_x = params->crtc.x;
> > +	mock->crtc_y = params->crtc.y;
> > +	mock->crtc_w = params->crtc.w;
> > +	mock->crtc_h = params->crtc.h;
> > +
> > +	test->priv = mock;
> > +
> > +	return 0;
> >  }
> >  
> >  static bool check_src_eq(struct kunit *test, struct drm_plane_state *plane_state,
> > @@ -54,16 +111,6 @@ static bool check_src_eq(struct kunit *test, struct drm_plane_state *plane_state
> >  	return true;
> >  }
> >  
> > -static void set_crtc(struct drm_plane_state *plane_state,
> > -		     int crtc_x, int crtc_y,
> > -		     unsigned int crtc_w, unsigned int crtc_h)
> > -{
> > -	plane_state->crtc_x = crtc_x;
> > -	plane_state->crtc_y = crtc_y;
> > -	plane_state->crtc_w = crtc_w;
> > -	plane_state->crtc_h = crtc_h;
> > -}
> > -
> >  static bool check_crtc_eq(struct kunit *test, struct drm_plane_state *plane_state,
> >  			  int crtc_x, int crtc_y,
> >  			  unsigned int crtc_w, unsigned int crtc_h)
> > @@ -83,162 +130,206 @@ static bool check_crtc_eq(struct kunit *test, struct drm_plane_state *plane_stat
> >  	return true;
> >  }
> >  
> > -static void igt_check_plane_state(struct kunit *test)
> > +static void drm_check_plane_state(struct kunit *test)
> > +{
> > +	const struct drm_check_plane_state_test *params = test->param_value;
> > +	struct drm_plane_state *plane_state = test->priv;
> > +
> > +	KUNIT_ASSERT_EQ_MSG(test,
> > +			    drm_atomic_helper_check_plane_state(plane_state, &crtc_state,
> > +								params->min_scale,
> > +								params->max_scale,
> > +								params->can_position, false),
> > +			    0, params->msg);
> > +	KUNIT_EXPECT_TRUE(test, plane_state->visible);
> > +	check_src_eq(test, plane_state, params->src_expected.x, params->src_expected.y,
> > +		     params->src_expected.w, params->src_expected.h);
> > +	check_crtc_eq(test, plane_state, params->crtc_expected.x, params->crtc_expected.y,
> > +		      params->crtc_expected.w, params->crtc_expected.h);
> 
> In order to preserve the same function of the tests, I believe that
> check_src_eq and check_crtc_eq should be inside a KUNIT_EXPECT_TRUE. If
> there is a reason for not using KUNIT_EXPECT_TRUE, then check_src_eq and
> check_crtc_eq should not return a boolean.
> 
> Moreover, now that those functions are being called just once, you could
> just add some expectations to this function, such as:
> 
> KUNIT_EXPECT_GE_MSG(test, plane_state->src.x1, 0,
> 	"src x coordinate %x should never be below 0, src: "
> 	DRM_RECT_FP_FMT, plane_state->src.x1,
> 	DRM_RECT_FP_ARG(&plane_state->src));

Yeah, good catch, sorry about that - I lost the assert when splitting the
series.
Let me send a v2 where check_*_eq functions are void and have asserts inside
(this is fine from test output readability, since after conversion there's a
single call for each of the check, so we can still pinpoint exactly what went
wrong)

Thanks!
-Michał

> 
> Best Regards,
> - Maíra Canal
> 
> > +}
> > +
> > +static void drm_check_plane_state_desc(const struct drm_check_plane_state_test *t,
> > +				       char *desc)
> >  {
> > -	int ret;
> > -
> > -	static const struct drm_crtc_state crtc_state = {
> > -		.crtc = ZERO_SIZE_PTR,
> > -		.enable = true,
> > -		.active = true,
> > -		.mode = {
> > -			DRM_MODE("1024x768", 0, 65000, 1024, 1048, 1184, 1344, 0, 768, 771,
> > -				 777, 806, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC)
> > -		},
> > -	};
> > -	static struct drm_plane plane = {
> > -		.dev = NULL
> > -	};
> > -	static struct drm_framebuffer fb = {
> > -		.width = 2048,
> > -		.height = 2048
> > -	};
> > -	static struct drm_plane_state plane_state = {
> > -		.plane = &plane,
> > -		.crtc = ZERO_SIZE_PTR,
> > -		.fb = &fb,
> > -		.rotation = DRM_MODE_ROTATE_0
> > -	};
> > -
> > -	/* Simple clipping, no scaling. */
> > -	set_src(&plane_state, 0, 0, fb.width << 16, fb.height << 16);
> > -	set_crtc(&plane_state, 0, 0, fb.width, fb.height);
> > -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> > -						  DRM_PLANE_NO_SCALING,
> > -						  DRM_PLANE_NO_SCALING,
> > -						  false, false);
> > -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Simple clipping check should pass\n");
> > -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> > -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 1024 << 16, 768 << 16));
> > -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
> > -
> > -	/* Rotated clipping + reflection, no scaling. */
> > -	plane_state.rotation = DRM_MODE_ROTATE_90 | DRM_MODE_REFLECT_X;
> > -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> > -						  DRM_PLANE_NO_SCALING,
> > -						  DRM_PLANE_NO_SCALING,
> > -						  false, false);
> > -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Rotated clipping check should pass\n");
> > -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> > -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 768 << 16, 1024 << 16));
> > -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
> > -	plane_state.rotation = DRM_MODE_ROTATE_0;
> > -
> > -	/* Check whether positioning works correctly. */
> > -	set_src(&plane_state, 0, 0, 1023 << 16, 767 << 16);
> > -	set_crtc(&plane_state, 0, 0, 1023, 767);
> > -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> > -						  DRM_PLANE_NO_SCALING,
> > -						  DRM_PLANE_NO_SCALING,
> > -						  false, false);
> > -	KUNIT_EXPECT_TRUE_MSG(test, ret,
> > -			      "Should not be able to position on the crtc with can_position=false\n");
> > -
> > -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> > -						  DRM_PLANE_NO_SCALING,
> > -						  DRM_PLANE_NO_SCALING,
> > -						  true, false);
> > -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Simple positioning should work\n");
> > -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> > -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 1023 << 16, 767 << 16));
> > -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1023, 767));
> > -
> > -	/* Simple scaling tests. */
> > -	set_src(&plane_state, 0, 0, 512 << 16, 384 << 16);
> > -	set_crtc(&plane_state, 0, 0, 1024, 768);
> > -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> > -						  0x8001,
> > -						  DRM_PLANE_NO_SCALING,
> > -						  false, false);
> > -	KUNIT_EXPECT_TRUE_MSG(test, ret, "Upscaling out of range should fail.\n");
> > -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> > -						  0x8000,
> > -						  DRM_PLANE_NO_SCALING,
> > -						  false, false);
> > -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Upscaling exactly 2x should work\n");
> > -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> > -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 512 << 16, 384 << 16));
> > -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
> > -
> > -	set_src(&plane_state, 0, 0, 2048 << 16, 1536 << 16);
> > -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> > -						  DRM_PLANE_NO_SCALING,
> > -						  0x1ffff, false, false);
> > -	KUNIT_EXPECT_TRUE_MSG(test, ret, "Downscaling out of range should fail.\n");
> > -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> > -						  DRM_PLANE_NO_SCALING,
> > -						  0x20000, false, false);
> > -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed with exact scaling limit\n");
> > -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> > -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 2048 << 16, 1536 << 16));
> > -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
> > -
> > -	/* Testing rounding errors. */
> > -	set_src(&plane_state, 0, 0, 0x40001, 0x40001);
> > -	set_crtc(&plane_state, 1022, 766, 4, 4);
> > -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> > -						  DRM_PLANE_NO_SCALING,
> > -						  0x10001,
> > -						  true, false);
> > -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to exact multiple");
> > -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> > -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 2 << 16, 2 << 16));
> > -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 1022, 766, 2, 2));
> > -
> > -	set_src(&plane_state, 0x20001, 0x20001, 0x4040001, 0x3040001);
> > -	set_crtc(&plane_state, -2, -2, 1028, 772);
> > -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> > -						  DRM_PLANE_NO_SCALING,
> > -						  0x10001,
> > -						  false, false);
> > -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to exact multiple");
> > -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> > -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0x40002, 0x40002,
> > -					     1024 << 16, 768 << 16));
> > -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
> > -
> > -	set_src(&plane_state, 0, 0, 0x3ffff, 0x3ffff);
> > -	set_crtc(&plane_state, 1022, 766, 4, 4);
> > -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> > -						  0xffff,
> > -						  DRM_PLANE_NO_SCALING,
> > -						  true, false);
> > -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to exact multiple");
> > -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> > -	/* Should not be rounded to 0x20001, which would be upscaling. */
> > -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0, 0, 2 << 16, 2 << 16));
> > -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 1022, 766, 2, 2));
> > -
> > -	set_src(&plane_state, 0x1ffff, 0x1ffff, 0x403ffff, 0x303ffff);
> > -	set_crtc(&plane_state, -2, -2, 1028, 772);
> > -	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> > -						  0xffff,
> > -						  DRM_PLANE_NO_SCALING,
> > -						  false, false);
> > -	KUNIT_EXPECT_FALSE_MSG(test, ret, 0, "Should succeed by clipping to exact multiple");
> > -	KUNIT_EXPECT_TRUE(test, plane_state.visible);
> > -	KUNIT_EXPECT_TRUE(test, check_src_eq(test, &plane_state, 0x3fffe, 0x3fffe,
> > -					     1024 << 16, 768 << 16));
> > -	KUNIT_EXPECT_TRUE(test, check_crtc_eq(test, &plane_state, 0, 0, 1024, 768));
> > +	sprintf(desc, "%s", t->name);
> >  }
> >  
> > +static const struct drm_check_plane_state_test drm_check_plane_state_tests[] = {
> > +	{
> > +		.name = "clipping_simple",
> > +		.msg = "Simple clipping check should pass",
> > +		.src = { 0, 0,
> > +			 2048 << 16,
> > +			 2048 << 16 },
> > +		.crtc = { 0, 0, 2048, 2048 },
> > +		.rotation = DRM_MODE_ROTATE_0,
> > +		.min_scale = DRM_PLANE_NO_SCALING,
> > +		.max_scale = DRM_PLANE_NO_SCALING,
> > +		.can_position = false,
> > +		.src_expected = { 0, 0, 1024 << 16, 768 << 16 },
> > +		.crtc_expected = { 0, 0, 1024, 768 },
> > +	},
> > +	{
> > +		.name = "clipping_rotate_reflect",
> > +		.msg = "Rotated clipping check should pass",
> > +		.src = { 0, 0,
> > +			 2048 << 16,
> > +			 2048 << 16 },
> > +		.crtc = { 0, 0, 2048, 2048 },
> > +		.rotation = DRM_MODE_ROTATE_90 | DRM_MODE_REFLECT_X,
> > +		.min_scale = DRM_PLANE_NO_SCALING,
> > +		.max_scale = DRM_PLANE_NO_SCALING,
> > +		.can_position = false,
> > +		.src_expected = { 0, 0, 768 << 16, 1024 << 16 },
> > +		.crtc_expected = { 0, 0, 1024, 768 },
> > +	},
> > +	{
> > +		.name = "positioning_simple",
> > +		.msg = "Simple positioning should work",
> > +		.src = { 0, 0, 1023 << 16, 767 << 16 },
> > +		.crtc = { 0, 0, 1023, 767 },
> > +		.rotation = DRM_MODE_ROTATE_0,
> > +		.min_scale = DRM_PLANE_NO_SCALING,
> > +		.max_scale = DRM_PLANE_NO_SCALING,
> > +		.can_position = true,
> > +		.src_expected = { 0, 0, 1023 << 16, 767 << 16 },
> > +		.crtc_expected = { 0, 0, 1023, 767 },
> > +	},
> > +	{
> > +		.name = "upscaling",
> > +		.msg = "Upscaling exactly 2x should work",
> > +		.src = { 0, 0, 512 << 16, 384 << 16 },
> > +		.crtc = { 0, 0, 1024, 768 },
> > +		.rotation = DRM_MODE_ROTATE_0,
> > +		.min_scale = 0x8000,
> > +		.max_scale = DRM_PLANE_NO_SCALING,
> > +		.can_position = false,
> > +		.src_expected = { 0, 0, 512 << 16, 384 << 16 },
> > +		.crtc_expected = { 0, 0, 1024, 768 },
> > +	},
> > +	{
> > +		.name = "downscaling",
> > +		.msg = "Should succeed with exact scaling limit",
> > +		.src = { 0, 0, 2048 << 16, 1536 << 16 },
> > +		.crtc = { 0, 0, 1024, 768 },
> > +		.rotation = DRM_MODE_ROTATE_0,
> > +		.min_scale = DRM_PLANE_NO_SCALING,
> > +		.max_scale = 0x20000,
> > +		.can_position = false,
> > +		.src_expected = { 0, 0, 2048 << 16, 1536 << 16 },
> > +		.crtc_expected = { 0, 0, 1024, 768 },
> > +	},
> > +	{
> > +		.name = "rounding1",
> > +		.msg = "Should succeed by clipping to exact multiple",
> > +		.src = { 0, 0, 0x40001, 0x40001 },
> > +		.crtc = { 1022, 766, 4, 4 },
> > +		.rotation = DRM_MODE_ROTATE_0,
> > +		.min_scale = DRM_PLANE_NO_SCALING,
> > +		.max_scale = 0x10001,
> > +		.can_position = true,
> > +		.src_expected = { 0, 0, 2 << 16, 2 << 16 },
> > +		.crtc_expected = { 1022, 766, 2, 2 },
> > +	},
> > +	{
> > +		.name = "rounding2",
> > +		.msg = "Should succeed by clipping to exact multiple",
> > +		.src = { 0x20001, 0x20001, 0x4040001, 0x3040001 },
> > +		.crtc = { -2, -2, 1028, 772 },
> > +		.rotation = DRM_MODE_ROTATE_0,
> > +		.min_scale = DRM_PLANE_NO_SCALING,
> > +		.max_scale = 0x10001,
> > +		.can_position = false,
> > +		.src_expected = { 0x40002, 0x40002, 1024 << 16, 768 << 16 },
> > +		.crtc_expected = { 0, 0, 1024, 768 },
> > +	},
> > +	{
> > +		.name = "rounding3",
> > +		.msg = "Should succeed by clipping to exact multiple",
> > +		.src = { 0, 0, 0x3ffff, 0x3ffff },
> > +		.crtc = { 1022, 766, 4, 4 },
> > +		.rotation = DRM_MODE_ROTATE_0,
> > +		.min_scale = 0xffff,
> > +		.max_scale = DRM_PLANE_NO_SCALING,
> > +		.can_position = true,
> > +		/* Should not be rounded to 0x20001, which would be upscaling. */
> > +		.src_expected = { 0, 0, 2 << 16, 2 << 16 },
> > +		.crtc_expected = { 1022, 766, 2, 2 },
> > +	},
> > +	{
> > +		.name = "rounding4",
> > +		.msg = "Should succeed by clipping to exact multiple",
> > +		.src = { 0x1ffff, 0x1ffff, 0x403ffff, 0x303ffff },
> > +		.crtc = { -2, -2, 1028, 772 },
> > +		.rotation = DRM_MODE_ROTATE_0,
> > +		.min_scale = 0xffff,
> > +		.max_scale = DRM_PLANE_NO_SCALING,
> > +		.can_position = false,
> > +		.src_expected = { 0x3fffe, 0x3fffe, 1024 << 16, 768 << 16 },
> > +		.crtc_expected = { 0, 0, 1024, 768 },
> > +	},
> > +};
> > +
> > +KUNIT_ARRAY_PARAM(drm_check_plane_state, drm_check_plane_state_tests, drm_check_plane_state_desc);
> > +
> > +static void drm_check_invalid_plane_state(struct kunit *test)
> > +{
> > +	const struct drm_check_plane_state_test *params = test->param_value;
> > +	struct drm_plane_state *plane_state = test->priv;
> > +
> > +	KUNIT_ASSERT_LT_MSG(test,
> > +			    drm_atomic_helper_check_plane_state(plane_state, &crtc_state,
> > +								params->min_scale,
> > +								params->max_scale,
> > +								params->can_position, false),
> > +			    0, params->msg);
> > +}
> > +
> > +static const struct drm_check_plane_state_test drm_check_invalid_plane_state_tests[] = {
> > +	{
> > +		.name = "positioning_invalid",
> > +		.msg = "Should not be able to position on the crtc with can_position=false",
> > +		.src = { 0, 0, 1023 << 16, 767 << 16 },
> > +		.crtc = { 0, 0, 1023, 767 },
> > +		.rotation = DRM_MODE_ROTATE_0,
> > +		.min_scale = DRM_PLANE_NO_SCALING,
> > +		.max_scale = DRM_PLANE_NO_SCALING,
> > +		.can_position = false,
> > +	},
> > +	{
> > +		.name = "upscaling_invalid",
> > +		.msg = "Upscaling out of range should fail",
> > +		.src = { 0, 0, 512 << 16, 384 << 16 },
> > +		.crtc = { 0, 0, 1024, 768 },
> > +		.rotation = DRM_MODE_ROTATE_0,
> > +		.min_scale = 0x8001,
> > +		.max_scale = DRM_PLANE_NO_SCALING,
> > +		.can_position = false,
> > +	},
> > +	{
> > +		.name = "downscaling_invalid",
> > +		.msg = "Downscaling out of range should fail",
> > +		.src = { 0, 0, 2048 << 16, 1536 << 16 },
> > +		.crtc = { 0, 0, 1024, 768 },
> > +		.rotation = DRM_MODE_ROTATE_0,
> > +		.min_scale = DRM_PLANE_NO_SCALING,
> > +		.max_scale = 0x1ffff,
> > +		.can_position = false,
> > +	},
> > +};
> > +
> > +KUNIT_ARRAY_PARAM(drm_check_invalid_plane_state, drm_check_invalid_plane_state_tests,
> > +		  drm_check_plane_state_desc);
> > +
> >  static struct kunit_case drm_plane_helper_test[] = {
> > -	KUNIT_CASE(igt_check_plane_state),
> > +	KUNIT_CASE_PARAM(drm_check_plane_state, drm_check_plane_state_gen_params),
> > +	KUNIT_CASE_PARAM(drm_check_invalid_plane_state, drm_check_invalid_plane_state_gen_params),
> >  	{}
> >  };
> >  
> >  static struct kunit_suite drm_plane_helper_test_suite = {
> >  	.name = "drm_plane_helper",
> > +	.init = drm_plane_helper_init,
> >  	.test_cases = drm_plane_helper_test,
> >  };
> >  

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-08-31 11:09 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-25 12:48 [PATCH 1/2] drm/plane_helper: Print actual/expected values on failure Michał Winiarski
2022-08-25 12:48 ` [PATCH 2/2] drm/plane_helper: Split into parameterized test cases Michał Winiarski
2022-08-26 22:34   ` Maíra Canal
2022-08-31 11:08     ` Michał Winiarski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).