~lkcamp/patches

1

[PATCH v3 1/2] drm: selftests: refactor drm_cmdline_parser

Leandro Ribeiro <leandrohr@riseup.net>
Details
Message ID
<b34fdf15-76ef-58cf-f7d4-1d02d97a283e@riseup.net>
DKIM signature
missing
Download raw message


-------- Forwarded Message --------
Subject: Fwd: [PATCH v3 1/2] drm: selftests: refactor drm_cmdline_parser
Date: Wed, 2 Feb 2022 20:55:25 -0300
From: Maíra Canal <maira.canal@usp.br>
To: leandrohr@riseup.net




-------- Forwarded Message --------
Subject: [PATCH v3 1/2] drm: selftests: refactor drm_cmdline_parser
Date: Sat, 29 Jan 2022 10:56:30 -0300
From: Maíra Canal <maira.canal@usp.br>
To: ~lkcamp/patches@lists.sr.ht
CC: arthur.grillo@usp.br, Maíra Canal <maira.canal@usp.br>

From: Arthur Grillo <arthur.grillo@usp.br>

Refactor the tests by modularizing the functions to avoid code repetition.

Co-developed-by: Maíra Canal <maira.canal@usp.br>
Signed-off-by: Maíra Canal <maira.canal@usp.br>
Signed-off-by: Arthur Grillo <arthur.grillo@usp.br>
---
 .../drm/selftests/test-drm_cmdline_parser.c   | 579 +++++-------------
 1 file changed, 156 insertions(+), 423 deletions(-)

diff --git a/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
index d96cd890def6..57a229c5fc35 100644
--- a/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
+++ b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
  * Copyright (c) 2019 Bootlin
+ * Copyright (c) 2021 Ma�ra Canal <maira.canal@usp.br>,
+ * Copyright (c) 2021 Arthur Grillo <arthur.grillo@usp.br>
  */
  #define pr_fmt(fmt) "drm_cmdline: " fmt
@@ -17,13 +19,25 @@
  static const struct drm_connector no_connector = {};
 -static int drm_cmdline_test_force_e_only(void *ignored)
+static int drm_cmdline_test_properties(void *ignored,
+		struct drm_cmdline_mode *mode, enum drm_connector_force force)
+{
+	FAIL_ON(mode->rb);
+	FAIL_ON(mode->cvt);
+	FAIL_ON(mode->interlace);
+	FAIL_ON(mode->margins);
+	FAIL_ON(mode->force != force);
+
+	return 0;
+}
+
+static int drm_cmdline_test_force_only(void *ignored, char *cmdline,
+		const struct drm_connector *connector, enum drm_connector_force force)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("e",
-							   &no_connector,
-							   &mode));
+	FAIL_ON(!drm_mode_parse_command_line_for_connector(cmdline,
+							   connector, &mode));
 	FAIL_ON(mode.specified);
 	FAIL_ON(mode.refresh_specified);
 	FAIL_ON(mode.bpp_specified);
@@ -32,95 +46,101 @@ static int drm_cmdline_test_force_e_only(void *ignored)
 	FAIL_ON(mode.cvt);
 	FAIL_ON(mode.interlace);
 	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_ON);
+	FAIL_ON(mode.force != force);
  	return 0;
 }
 -static int drm_cmdline_test_force_D_only_not_digital(void *ignored)
+static int drm_cmdline_test_freestanding(void *ignored,
+		struct drm_cmdline_mode *mode, char *cmdline,
+		const struct drm_connector *connector)
 {
-	struct drm_cmdline_mode mode = { };
+	FAIL_ON(!drm_mode_parse_command_line_for_connector(cmdline,
+							   connector, mode));
+	FAIL_ON(mode->specified);
+	FAIL_ON(mode->refresh_specified);
+	FAIL_ON(mode->bpp_specified);
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("D",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(mode.specified);
-	FAIL_ON(mode.refresh_specified);
-	FAIL_ON(mode.bpp_specified);
+	FAIL_ON(mode->tv_margins.right != 14);
+	FAIL_ON(mode->tv_margins.left != 24);
+	FAIL_ON(mode->tv_margins.bottom != 36);
+	FAIL_ON(mode->tv_margins.top != 42);
 -	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_ON);
+	return 0;
+}
+
+static int drm_cmdline_test_res_init(void *ignored,
+		struct drm_cmdline_mode *mode, char *cmdline)
+{
+	FAIL_ON(!drm_mode_parse_command_line_for_connector(cmdline,
+							   &no_connector, mode));
+	FAIL_ON(!mode->specified);
+	FAIL_ON(mode->xres != 720);
+	FAIL_ON(mode->yres != 480);
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_bpp_init(void *ignored,
+		struct drm_cmdline_mode *mode, char *cmdline)
+{
+	FAIL_ON(!drm_mode_parse_command_line_for_connector(cmdline,
+							   &no_connector, mode));
+	FAIL_ON(!mode->specified);
+	FAIL_ON(mode->xres != 720);
+	FAIL_ON(mode->yres != 480);
+
+	FAIL_ON(!mode->refresh_specified);
+	FAIL_ON(mode->refresh != 60);
+	FAIL_ON(!mode->bpp_specified);
+	FAIL_ON(mode->bpp != 24);
+
+	return 0;
+}
+
+static int drm_cmdline_test_force_e_only(void *ignored)
+{
+	drm_cmdline_test_force_only(ignored, "e", &no_connector, DRM_FORCE_ON);
+
+	return 0;
+}
+
+static int drm_cmdline_test_force_D_only_not_digital(void *ignored)
+{
+	drm_cmdline_test_force_only(ignored, "D", &no_connector, DRM_FORCE_ON);
  	return 0;
 }
  static const struct drm_connector connector_hdmi = {
 	.connector_type	= DRM_MODE_CONNECTOR_HDMIB,
+
 };
  static int drm_cmdline_test_force_D_only_hdmi(void *ignored)
 {
-	struct drm_cmdline_mode mode = { };
-
-	FAIL_ON(!drm_mode_parse_command_line_for_connector("D",
-							   &connector_hdmi,
-							   &mode));
-	FAIL_ON(mode.specified);
-	FAIL_ON(mode.refresh_specified);
-	FAIL_ON(mode.bpp_specified);
-
-	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_ON_DIGITAL);
+	drm_cmdline_test_force_only(ignored, "D", &connector_hdmi,
+			DRM_FORCE_ON_DIGITAL);
  	return 0;
 }
  static const struct drm_connector connector_dvi = {
 	.connector_type	= DRM_MODE_CONNECTOR_DVII,
+
 };
  static int drm_cmdline_test_force_D_only_dvi(void *ignored)
 {
-	struct drm_cmdline_mode mode = { };
-
-	FAIL_ON(!drm_mode_parse_command_line_for_connector("D",
-							   &connector_dvi,
-							   &mode));
-	FAIL_ON(mode.specified);
-	FAIL_ON(mode.refresh_specified);
-	FAIL_ON(mode.bpp_specified);
-
-	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_ON_DIGITAL);
+	drm_cmdline_test_force_only(ignored, "D", &connector_dvi,
+			DRM_FORCE_ON_DIGITAL);
  	return 0;
 }
  static int drm_cmdline_test_force_d_only(void *ignored)
 {
-	struct drm_cmdline_mode mode = { };
-
-	FAIL_ON(!drm_mode_parse_command_line_for_connector("d",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(mode.specified);
-	FAIL_ON(mode.refresh_specified);
-	FAIL_ON(mode.bpp_specified);
-
-	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_OFF);
+	drm_cmdline_test_force_only(ignored, "d", &no_connector, DRM_FORCE_OFF);
  	return 0;
 }
@@ -151,15 +171,9 @@ static int drm_cmdline_test_res(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
+	drm_cmdline_test_res_init(ignored, &mode, "720x480");
  	FAIL_ON(mode.refresh_specified);
-
 	FAIL_ON(mode.bpp_specified);
  	FAIL_ON(mode.rb);
@@ -219,15 +233,9 @@ static int drm_cmdline_test_res_vesa(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480M",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
+	drm_cmdline_test_res_init(ignored, &mode, "720x480M");
  	FAIL_ON(mode.refresh_specified);
-
 	FAIL_ON(mode.bpp_specified);
  	FAIL_ON(mode.rb);
@@ -243,15 +251,9 @@ static int drm_cmdline_test_res_vesa_rblank(void
*ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480MR",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
+	drm_cmdline_test_res_init(ignored, &mode, "720x480MR");
  	FAIL_ON(mode.refresh_specified);
-
 	FAIL_ON(mode.bpp_specified);
  	FAIL_ON(!mode.rb);
@@ -267,15 +269,9 @@ static int drm_cmdline_test_res_rblank(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480R",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
+	drm_cmdline_test_res_init(ignored, &mode, "720x480R");
  	FAIL_ON(mode.refresh_specified);
-
 	FAIL_ON(mode.bpp_specified);
  	FAIL_ON(!mode.rb);
@@ -291,23 +287,13 @@ static int drm_cmdline_test_res_bpp(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
+	drm_cmdline_test_res_init(ignored, &mode, "720x480-24");
  	FAIL_ON(mode.refresh_specified);
-
 	FAIL_ON(!mode.bpp_specified);
 	FAIL_ON(mode.bpp != 24);
 -	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
  	return 0;
 }
@@ -327,23 +313,13 @@ static int drm_cmdline_test_res_refresh(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480@60",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
+	drm_cmdline_test_res_init(ignored, &mode, "720x480@60");
  	FAIL_ON(!mode.refresh_specified);
 	FAIL_ON(mode.refresh != 60);
-
 	FAIL_ON(mode.bpp_specified);
 -	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
  	return 0;
 }
@@ -363,24 +339,8 @@ static int drm_cmdline_test_res_bpp_refresh(void
*ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
-
-	FAIL_ON(!mode.refresh_specified);
-	FAIL_ON(mode.refresh != 60);
-
-	FAIL_ON(!mode.bpp_specified);
-	FAIL_ON(mode.bpp != 24);
-
-	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+	drm_cmdline_test_res_bpp_init(ignored, &mode, "720x480-24@60");
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
  	return 0;
 }
@@ -389,18 +349,7 @@ static int
drm_cmdline_test_res_bpp_refresh_interlaced(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60i",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
-
-	FAIL_ON(!mode.refresh_specified);
-	FAIL_ON(mode.refresh != 60);
-
-	FAIL_ON(!mode.bpp_specified);
-	FAIL_ON(mode.bpp != 24);
+	drm_cmdline_test_res_bpp_init(ignored, &mode, "720x480-24@60i");
  	FAIL_ON(mode.rb);
 	FAIL_ON(mode.cvt);
@@ -415,18 +364,7 @@ static int
drm_cmdline_test_res_bpp_refresh_margins(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60m",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
-
-	FAIL_ON(!mode.refresh_specified);
-	FAIL_ON(mode.refresh != 60);
-
-	FAIL_ON(!mode.bpp_specified);
-	FAIL_ON(mode.bpp != 24);
+	drm_cmdline_test_res_bpp_init(ignored, &mode, "720x480-24@60m");
  	FAIL_ON(mode.rb);
 	FAIL_ON(mode.cvt);
@@ -441,24 +379,8 @@ static int
drm_cmdline_test_res_bpp_refresh_force_off(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60d",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
-
-	FAIL_ON(!mode.refresh_specified);
-	FAIL_ON(mode.refresh != 60);
-
-	FAIL_ON(!mode.bpp_specified);
-	FAIL_ON(mode.bpp != 24);
-
-	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_OFF);
+	drm_cmdline_test_res_bpp_init(ignored, &mode, "720x480-24@60d");
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_OFF);
  	return 0;
 }
@@ -478,24 +400,8 @@ static int
drm_cmdline_test_res_bpp_refresh_force_on(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60e",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
-
-	FAIL_ON(!mode.refresh_specified);
-	FAIL_ON(mode.refresh != 60);
-
-	FAIL_ON(!mode.bpp_specified);
-	FAIL_ON(mode.bpp != 24);
-
-	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_ON);
+	drm_cmdline_test_res_bpp_init(ignored, &mode, "720x480-24@60e");
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_ON);
  	return 0;
 }
@@ -504,24 +410,8 @@ static int
drm_cmdline_test_res_bpp_refresh_force_on_analog(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60D",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
-
-	FAIL_ON(!mode.refresh_specified);
-	FAIL_ON(mode.refresh != 60);
-
-	FAIL_ON(!mode.bpp_specified);
-	FAIL_ON(mode.bpp != 24);
-
-	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_ON);
+	drm_cmdline_test_res_bpp_init(ignored, &mode, "720x480-24@60D");
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_ON);
  	return 0;
 }
@@ -534,8 +424,7 @@ static int
drm_cmdline_test_res_bpp_refresh_force_on_digital(void *ignored)
 	};
  	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60D",
-							   &connector,
-							   &mode));
+							   &connector, &mode));
 	FAIL_ON(!mode.specified);
 	FAIL_ON(mode.xres != 720);
 	FAIL_ON(mode.yres != 480);
@@ -546,11 +435,7 @@ static int
drm_cmdline_test_res_bpp_refresh_force_on_digital(void *ignored)
 	FAIL_ON(!mode.bpp_specified);
 	FAIL_ON(mode.bpp != 24);
 -	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_ON_DIGITAL);
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_ON_DIGITAL);
  	return 0;
 }
@@ -559,18 +444,7 @@ static int
drm_cmdline_test_res_bpp_refresh_interlaced_margins_force_on(void *ig
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60ime",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
-
-	FAIL_ON(!mode.refresh_specified);
-	FAIL_ON(mode.refresh != 60);
-
-	FAIL_ON(!mode.bpp_specified);
-	FAIL_ON(mode.bpp != 24);
+	drm_cmdline_test_res_bpp_init(ignored, &mode, "720x480-24@60ime");
  	FAIL_ON(mode.rb);
 	FAIL_ON(mode.cvt);
@@ -585,15 +459,9 @@ static int
drm_cmdline_test_res_margins_force_on(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480me",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
+	drm_cmdline_test_res_init(ignored, &mode, "720x480me");
  	FAIL_ON(mode.refresh_specified);
-
 	FAIL_ON(mode.bpp_specified);
  	FAIL_ON(mode.rb);
@@ -609,15 +477,9 @@ static int drm_cmdline_test_res_vesa_margins(void
*ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480Mm",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
+	drm_cmdline_test_res_init(ignored, &mode, "720x480Mm");
  	FAIL_ON(mode.refresh_specified);
-
 	FAIL_ON(mode.bpp_specified);
  	FAIL_ON(mode.rb);
@@ -673,10 +535,9 @@ static int drm_cmdline_test_name_bpp(void *ignored)
 							   &no_connector,
 							   &mode));
 	FAIL_ON(strcmp(mode.name, "NTSC"));
-
 	FAIL_ON(mode.refresh_specified);
-
 	FAIL_ON(!mode.bpp_specified);
+
 	FAIL_ON(mode.bpp != 24);
  	return 0;
@@ -760,23 +621,13 @@ static int drm_cmdline_test_rotate_0(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,rotate=0",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
-	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_0);
+	drm_cmdline_test_res_init(ignored, &mode, "720x480,rotate=0");
 +	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_0);
 	FAIL_ON(mode.refresh_specified);
-
 	FAIL_ON(mode.bpp_specified);
 -	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
  	return 0;
 }
@@ -785,23 +636,13 @@ static int drm_cmdline_test_rotate_90(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,rotate=90",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
-	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_90);
+	drm_cmdline_test_res_init(ignored, &mode, "720x480,rotate=90");
 +	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_90);
 	FAIL_ON(mode.refresh_specified);
-
 	FAIL_ON(mode.bpp_specified);
 -	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
  	return 0;
 }
@@ -810,23 +651,13 @@ static int drm_cmdline_test_rotate_180(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,rotate=180",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
-	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_180);
+	drm_cmdline_test_res_init(ignored, &mode, "720x480,rotate=180");
 +	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_180);
 	FAIL_ON(mode.refresh_specified);
-
 	FAIL_ON(mode.bpp_specified);
 -	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
  	return 0;
 }
@@ -835,23 +666,13 @@ static int drm_cmdline_test_rotate_270(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,rotate=270",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
-	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_270);
+	drm_cmdline_test_res_init(ignored, &mode, "720x480,rotate=270");
 +	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_270);
 	FAIL_ON(mode.refresh_specified);
-
 	FAIL_ON(mode.bpp_specified);
 -	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
  	return 0;
 }
@@ -860,9 +681,8 @@ static int drm_cmdline_test_rotate_multiple(void
*ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -
FAIL_ON(drm_mode_parse_command_line_for_connector("720x480,rotate=0,rotate=90",
-							  &no_connector,
-							  &mode));
+	FAIL_ON(drm_mode_parse_command_line_for_connector(
+				"720x480,rotate=0,rotate=90", &no_connector, &mode));
  	return 0;
 }
@@ -871,9 +691,8 @@ static int drm_cmdline_test_rotate_invalid_val(void
*ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(drm_mode_parse_command_line_for_connector("720x480,rotate=42",
-							  &no_connector,
-							  &mode));
+	FAIL_ON(drm_mode_parse_command_line_for_connector(
+				"720x480,rotate=42", &no_connector, &mode));
  	return 0;
 }
@@ -882,9 +701,8 @@ static int drm_cmdline_test_rotate_truncated(void
*ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(drm_mode_parse_command_line_for_connector("720x480,rotate=",
-							  &no_connector,
-							  &mode));
+	FAIL_ON(drm_mode_parse_command_line_for_connector(
+				"720x480,rotate=", &no_connector, &mode));
  	return 0;
 }
@@ -893,23 +711,13 @@ static int drm_cmdline_test_hmirror(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,reflect_x",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
-	FAIL_ON(mode.rotation_reflection != (DRM_MODE_ROTATE_0 |
DRM_MODE_REFLECT_X));
+	drm_cmdline_test_res_init(ignored, &mode, "720x480,reflect_x");
 +	FAIL_ON(mode.rotation_reflection != (DRM_MODE_ROTATE_0 |
DRM_MODE_REFLECT_X));
 	FAIL_ON(mode.refresh_specified);
-
 	FAIL_ON(mode.bpp_specified);
 -	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
  	return 0;
 }
@@ -918,23 +726,13 @@ static int drm_cmdline_test_vmirror(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,reflect_y",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
-	FAIL_ON(mode.rotation_reflection != (DRM_MODE_ROTATE_0 |
DRM_MODE_REFLECT_Y));
+	drm_cmdline_test_res_init(ignored, &mode, "720x480,reflect_y");
 +	FAIL_ON(mode.rotation_reflection != (DRM_MODE_ROTATE_0 |
DRM_MODE_REFLECT_Y));
 	FAIL_ON(mode.refresh_specified);
-
 	FAIL_ON(mode.bpp_specified);
 -	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
  	return 0;
 }
@@ -943,26 +741,18 @@ static int drm_cmdline_test_margin_options(void
*ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -
FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,margin_right=14,margin_left=24,margin_bottom=36,margin_top=42",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
+	drm_cmdline_test_res_init(ignored, &mode,
+		
"720x480,margin_right=14,margin_left=24,margin_bottom=36,margin_top=42");
+
 	FAIL_ON(mode.tv_margins.right != 14);
 	FAIL_ON(mode.tv_margins.left != 24);
 	FAIL_ON(mode.tv_margins.bottom != 36);
 	FAIL_ON(mode.tv_margins.top != 42);
  	FAIL_ON(mode.refresh_specified);
-
 	FAIL_ON(mode.bpp_specified);
 -	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
  	return 0;
 }
@@ -971,23 +761,13 @@ static int drm_cmdline_test_multiple_options(void
*ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -
FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,rotate=270,reflect_x",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
-	FAIL_ON(mode.rotation_reflection != (DRM_MODE_ROTATE_270 |
DRM_MODE_REFLECT_X));
+	drm_cmdline_test_res_init(ignored, &mode, "720x480,rotate=270,reflect_x");
 +	FAIL_ON(mode.rotation_reflection != (DRM_MODE_ROTATE_270 |
DRM_MODE_REFLECT_X));
 	FAIL_ON(mode.refresh_specified);
-
 	FAIL_ON(mode.bpp_specified);
 -	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
  	return 0;
 }
@@ -996,9 +776,8 @@ static int drm_cmdline_test_invalid_option(void
*ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(drm_mode_parse_command_line_for_connector("720x480,test=42",
-							  &no_connector,
-							  &mode));
+	FAIL_ON(drm_mode_parse_command_line_for_connector(
+				"720x480,test=42", &no_connector, &mode));
  	return 0;
 }
@@ -1007,24 +786,14 @@ static int
drm_cmdline_test_bpp_extra_and_option(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -
FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24e,rotate=180",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
-	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_180);
+	drm_cmdline_test_res_init(ignored, &mode, "720x480-24e,rotate=180");
 +	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_180);
 	FAIL_ON(mode.refresh_specified);
-
 	FAIL_ON(!mode.bpp_specified);
 	FAIL_ON(mode.bpp != 24);
 -	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_ON);
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_ON);
  	return 0;
 }
@@ -1033,22 +802,13 @@ static int drm_cmdline_test_extra_and_option(void
*ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480e,rotate=180",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(!mode.specified);
-	FAIL_ON(mode.xres != 720);
-	FAIL_ON(mode.yres != 480);
-	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_180);
+	drm_cmdline_test_res_init(ignored, &mode, "720x480e,rotate=180");
 +	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_180);
 	FAIL_ON(mode.refresh_specified);
 	FAIL_ON(mode.bpp_specified);
 -	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_ON);
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_ON);
  	return 0;
 }
@@ -1057,23 +817,11 @@ static int
drm_cmdline_test_freestanding_options(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -
FAIL_ON(!drm_mode_parse_command_line_for_connector("margin_right=14,margin_left=24,margin_bottom=36,margin_top=42",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(mode.specified);
-	FAIL_ON(mode.refresh_specified);
-	FAIL_ON(mode.bpp_specified);
+	drm_cmdline_test_freestanding(ignored, &mode,
+			"margin_right=14,margin_left=24,margin_bottom=36,margin_top=42",
+			&no_connector);
 -	FAIL_ON(mode.tv_margins.right != 14);
-	FAIL_ON(mode.tv_margins.left != 24);
-	FAIL_ON(mode.tv_margins.bottom != 36);
-	FAIL_ON(mode.tv_margins.top != 42);
-
-	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
  	return 0;
 }
@@ -1082,23 +830,11 @@ static int
drm_cmdline_test_freestanding_force_e_and_options(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -
FAIL_ON(!drm_mode_parse_command_line_for_connector("e,margin_right=14,margin_left=24,margin_bottom=36,margin_top=42",
-							   &no_connector,
-							   &mode));
-	FAIL_ON(mode.specified);
-	FAIL_ON(mode.refresh_specified);
-	FAIL_ON(mode.bpp_specified);
+	drm_cmdline_test_freestanding(ignored, &mode,
+			"e,margin_right=14,margin_left=24,margin_bottom=36,margin_top=42",
+			&no_connector);
 -	FAIL_ON(mode.tv_margins.right != 14);
-	FAIL_ON(mode.tv_margins.left != 24);
-	FAIL_ON(mode.tv_margins.bottom != 36);
-	FAIL_ON(mode.tv_margins.top != 42);
-
-	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_ON);
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_ON);
  	return 0;
 }
@@ -1107,20 +843,17 @@ static int
drm_cmdline_test_panel_orientation(void *ignored)
 {
 	struct drm_cmdline_mode mode = { };
 -
FAIL_ON(!drm_mode_parse_command_line_for_connector("panel_orientation=upside_down",
-							   &no_connector,
-							   &mode));
+	FAIL_ON(!drm_mode_parse_command_line_for_connector(
+				"panel_orientation=upside_down", &no_connector, &mode));
+
 	FAIL_ON(mode.specified);
 	FAIL_ON(mode.refresh_specified);
 	FAIL_ON(mode.bpp_specified);
 +
 	FAIL_ON(mode.panel_orientation != DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP);
 -	FAIL_ON(mode.rb);
-	FAIL_ON(mode.cvt);
-	FAIL_ON(mode.interlace);
-	FAIL_ON(mode.margins);
-	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
  	return 0;
 }
-- 
2.34.1
Details
Message ID
<20220206015120.xdlgazobpfgy34hr@notapiano>
In-Reply-To
<b34fdf15-76ef-58cf-f7d4-1d02d97a283e@riseup.net> (view parent)
DKIM signature
missing
Download raw message
Hi Maira,

thanks for the patch.

To be quite honest with you, I have a feeling that this change won't be accepted
upstream. I know that seeing how much duplicated code there are in these tests
gives an itch to create helper functions that de-duplicate it. I feel it as
well.

However, it seems to me that these helper functions end up just making the test
harder to follow. Here you have written the helper functions with very few
parameters, which makes them even shorter (less typing and code duplication),
however it makes a lot of assumptions, and for this reason they aren't very
re-usable.

My inline comments below have been with the aim of adding a few more parameters
to the functions, and in this way making them more generic. However this will
create a bit more duplication, and, to me at least, the end result is still less
readable than the original (the code before this patch). Mainly because instead
of having to guess what each function parameter is ("does xres or yres come
first?"), in the original code you have all the checks staring there in your
face. And I think in the particular case of unit tests, it's more important to
make it as simple and easy to follow as possible, than to make the code shorter.

This all said, I'm not the maintainer. And since you already did the work, might
as well send to the mailing list and see what the real maintainers think about
it. They might have a different perspective and think the code de-duplication is
worth it and merge it.

So please check out my inline comments below. Feel free to take or not my
suggestions based on what you prefer. But do remove the superfluous blank lines,
since those only make reviews harder. Then send a v4 to our mailing list and
we'll get the patches sent to the real mailing lists :).

(I hope the wall of text in this email doesn't disencourage you, I'm writing
this much with the intent of explaining my thoughts. Hopefully they'll be useful
to you :) ).

On Wed, Feb 02, 2022 at 09:02:47PM -0300, Leandro Ribeiro wrote:
> 
> 
> 
> -------- Forwarded Message --------
> Subject: Fwd: [PATCH v3 1/2] drm: selftests: refactor drm_cmdline_parser
> Date: Wed, 2 Feb 2022 20:55:25 -0300
> From: Maíra Canal <maira.canal@usp.br>
> To: leandrohr@riseup.net
> 
> 
> 
> 
> -------- Forwarded Message --------
> Subject: [PATCH v3 1/2] drm: selftests: refactor drm_cmdline_parser
> Date: Sat, 29 Jan 2022 10:56:30 -0300
> From: Maíra Canal <maira.canal@usp.br>
> To: ~lkcamp/patches@lists.sr.ht
> CC: arthur.grillo@usp.br, Maíra Canal <maira.canal@usp.br>
> 
> From: Arthur Grillo <arthur.grillo@usp.br>
> 
> Refactor the tests by modularizing the functions to avoid code repetition.
> 
> Co-developed-by: Maíra Canal <maira.canal@usp.br>
> Signed-off-by: Maíra Canal <maira.canal@usp.br>
> Signed-off-by: Arthur Grillo <arthur.grillo@usp.br>
> ---
>  .../drm/selftests/test-drm_cmdline_parser.c   | 579 +++++-------------
>  1 file changed, 156 insertions(+), 423 deletions(-)
> 
> diff --git a/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
> b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
> index d96cd890def6..57a229c5fc35 100644
> --- a/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
> +++ b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
> @@ -1,6 +1,8 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
>   * Copyright (c) 2019 Bootlin
> + * Copyright (c) 2021 Ma�ra Canal <maira.canal@usp.br>,
> + * Copyright (c) 2021 Arthur Grillo <arthur.grillo@usp.br>
>   */
>   #define pr_fmt(fmt) "drm_cmdline: " fmt
> @@ -17,13 +19,25 @@
>   static const struct drm_connector no_connector = {};
>  -static int drm_cmdline_test_force_e_only(void *ignored)
> +static int drm_cmdline_test_properties(void *ignored,
> +		struct drm_cmdline_mode *mode, enum drm_connector_force force)
> +{
> +	FAIL_ON(mode->rb);
> +	FAIL_ON(mode->cvt);
> +	FAIL_ON(mode->interlace);
> +	FAIL_ON(mode->margins);
> +	FAIL_ON(mode->force != force);
> +
> +	return 0;
> +}

This is the function that makes me double think this patch the most. It has
hardcoded checks for rb, cvt, interlace, margins, always checking that all of
them are false. However, this is just one case. There are functions like
drm_cmdline_test_res_vesa() that has slightly different checks:

	FAIL_ON(mode.rb);
	FAIL_ON(!mode.cvt);
	FAIL_ON(mode.interlace);
	FAIL_ON(mode.margins);

Since your drm_cmdline_test_properties() helper function is hardcoded to check
for false in all properties, it can't be used here, just because this scenario
has non-null cvt. So the way your helper is structured, makes it not very
reusable.

One thought is to have each one of those parametrized as booleans. Like 

	static int drm_cmdline_test_properties(void *ignored,
			struct drm_cmdline_mode *mode, enum drm_connector_force force,
			bool rb, bool cvt, bool interlace, bool margins)

	...

	if (rb)
		FAIL_ON(!mode.rb);
	else
		FAIL_ON(mode.rb);

	... and same thing for the others

This would allow you to use this helper in all cases. But the thing is, this is
how it would look in drm_cmdline_test_res_vesa():

	drm_cmdline_test_properties(ignored, "720x480R", DRM_FORCE_UNSPECIFIED,
				true, false, true, true)

Is this more readable than the original?

	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480R",
							   &no_connector,
							   &mode));

	FAIL_ON(mode.rb);
	FAIL_ON(!mode.cvt);
	FAIL_ON(mode.interlace);
	FAIL_ON(mode.margins);
	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);

Honestly to me it's not, because you have to guess the argument order for the
booleans. (And actually in this particular case, you couldn't even use this
helper since you already used drm_cmdline_test_res_init()). So the options here
are to either 
1. not create the helper; or
2. create it just for the all null case like you did; or
3. make it have parametrized booleans like I showed here.
Each has its pros and cons, so pick the one you prefer. The maintainers are the
ones that will give the final word anyway :).

> +
> +static int drm_cmdline_test_force_only(void *ignored, char *cmdline,
> +		const struct drm_connector *connector, enum drm_connector_force force)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("e",
> -							   &no_connector,
> -							   &mode));
> +	FAIL_ON(!drm_mode_parse_command_line_for_connector(cmdline,
> +							   connector, &mode));
>  	FAIL_ON(mode.specified);
>  	FAIL_ON(mode.refresh_specified);
>  	FAIL_ON(mode.bpp_specified);
> @@ -32,95 +46,101 @@ static int drm_cmdline_test_force_e_only(void *ignored)
>  	FAIL_ON(mode.cvt);
>  	FAIL_ON(mode.interlace);
>  	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_ON);
> +	FAIL_ON(mode.force != force);
>   	return 0;
>  }
>  -static int drm_cmdline_test_force_D_only_not_digital(void *ignored)
> +static int drm_cmdline_test_freestanding(void *ignored,
> +		struct drm_cmdline_mode *mode, char *cmdline,
> +		const struct drm_connector *connector)
>  {
> -	struct drm_cmdline_mode mode = { };
> +	FAIL_ON(!drm_mode_parse_command_line_for_connector(cmdline,
> +							   connector, mode));
> +	FAIL_ON(mode->specified);
> +	FAIL_ON(mode->refresh_specified);
> +	FAIL_ON(mode->bpp_specified);
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("D",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(mode.specified);
> -	FAIL_ON(mode.refresh_specified);
> -	FAIL_ON(mode.bpp_specified);
> +	FAIL_ON(mode->tv_margins.right != 14);
> +	FAIL_ON(mode->tv_margins.left != 24);
> +	FAIL_ON(mode->tv_margins.bottom != 36);
> +	FAIL_ON(mode->tv_margins.top != 42);
>  -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_ON);
> +	return 0;
> +}

This function shouldn't have hardcoded checks for the margins. The 14, 24, 36
and 42 compared to the margins here are directly related to the cmdline used,
and that's a parameter to this function:
"margin_right=14,margin_left=24,margin_bottom=36,margin_top=42".
So these four values should also be passed as parameters to the function.

> +
> +static int drm_cmdline_test_res_init(void *ignored,
> +		struct drm_cmdline_mode *mode, char *cmdline)
> +{
> +	FAIL_ON(!drm_mode_parse_command_line_for_connector(cmdline,
> +							   &no_connector, mode));
> +	FAIL_ON(!mode->specified);
> +	FAIL_ON(mode->xres != 720);
> +	FAIL_ON(mode->yres != 480);
> +
> +	return 0;
> +}

Same thing here for xres and yres. 720 and 480 are used because the cmdline is 
"720x480". And just because the cmdline always uses this resolution, hardcoding
those checks feels weird. So I'd also make these two function parameters.

> +
> +static int drm_cmdline_test_res_bpp_init(void *ignored,
> +		struct drm_cmdline_mode *mode, char *cmdline)
> +{
> +	FAIL_ON(!drm_mode_parse_command_line_for_connector(cmdline,
> +							   &no_connector, mode));
> +	FAIL_ON(!mode->specified);
> +	FAIL_ON(mode->xres != 720);
> +	FAIL_ON(mode->yres != 480);
> +
> +	FAIL_ON(!mode->refresh_specified);
> +	FAIL_ON(mode->refresh != 60);
> +	FAIL_ON(!mode->bpp_specified);
> +	FAIL_ON(mode->bpp != 24);
> +
> +	return 0;
> +}

And again here. 60 as the refresh rate is a consequence of the "@60" in the
cmdline, and the 24 bpp relates to "-24". So if the cmdline is a function
parameter, so should these two values (in addition to xres and yres).

> +
> +static int drm_cmdline_test_force_e_only(void *ignored)
> +{
> +	drm_cmdline_test_force_only(ignored, "e", &no_connector, DRM_FORCE_ON);
> +
> +	return 0;
> +}
> +
> +static int drm_cmdline_test_force_D_only_not_digital(void *ignored)
> +{
> +	drm_cmdline_test_force_only(ignored, "D", &no_connector, DRM_FORCE_ON);
>   	return 0;
>  }
>   static const struct drm_connector connector_hdmi = {
>  	.connector_type	= DRM_MODE_CONNECTOR_HDMIB,
> +

This extra blank line is not needed. There are some other like this one below.
Please always double-check that your text editor is not inserting these kind of
things if it does auto-formating, as changes like these add noise to the review.

>  };
>   static int drm_cmdline_test_force_D_only_hdmi(void *ignored)
>  {
> -	struct drm_cmdline_mode mode = { };
> -
> -	FAIL_ON(!drm_mode_parse_command_line_for_connector("D",
> -							   &connector_hdmi,
> -							   &mode));
> -	FAIL_ON(mode.specified);
> -	FAIL_ON(mode.refresh_specified);
> -	FAIL_ON(mode.bpp_specified);
> -
> -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_ON_DIGITAL);
> +	drm_cmdline_test_force_only(ignored, "D", &connector_hdmi,
> +			DRM_FORCE_ON_DIGITAL);
>   	return 0;
>  }
>   static const struct drm_connector connector_dvi = {
>  	.connector_type	= DRM_MODE_CONNECTOR_DVII,
> +
>  };
>   static int drm_cmdline_test_force_D_only_dvi(void *ignored)
>  {
> -	struct drm_cmdline_mode mode = { };
> -
> -	FAIL_ON(!drm_mode_parse_command_line_for_connector("D",
> -							   &connector_dvi,
> -							   &mode));
> -	FAIL_ON(mode.specified);
> -	FAIL_ON(mode.refresh_specified);
> -	FAIL_ON(mode.bpp_specified);
> -
> -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_ON_DIGITAL);
> +	drm_cmdline_test_force_only(ignored, "D", &connector_dvi,
> +			DRM_FORCE_ON_DIGITAL);
>   	return 0;
>  }
>   static int drm_cmdline_test_force_d_only(void *ignored)
>  {
> -	struct drm_cmdline_mode mode = { };
> -
> -	FAIL_ON(!drm_mode_parse_command_line_for_connector("d",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(mode.specified);
> -	FAIL_ON(mode.refresh_specified);
> -	FAIL_ON(mode.bpp_specified);
> -
> -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_OFF);
> +	drm_cmdline_test_force_only(ignored, "d", &no_connector, DRM_FORCE_OFF);
>   	return 0;
>  }
> @@ -151,15 +171,9 @@ static int drm_cmdline_test_res(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> +	drm_cmdline_test_res_init(ignored, &mode, "720x480");
>   	FAIL_ON(mode.refresh_specified);
> -
>  	FAIL_ON(mode.bpp_specified);
>   	FAIL_ON(mode.rb);
> @@ -219,15 +233,9 @@ static int drm_cmdline_test_res_vesa(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480M",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> +	drm_cmdline_test_res_init(ignored, &mode, "720x480M");
>   	FAIL_ON(mode.refresh_specified);
> -
>  	FAIL_ON(mode.bpp_specified);
>   	FAIL_ON(mode.rb);
> @@ -243,15 +251,9 @@ static int drm_cmdline_test_res_vesa_rblank(void
> *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480MR",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> +	drm_cmdline_test_res_init(ignored, &mode, "720x480MR");
>   	FAIL_ON(mode.refresh_specified);
> -
>  	FAIL_ON(mode.bpp_specified);
>   	FAIL_ON(!mode.rb);
> @@ -267,15 +269,9 @@ static int drm_cmdline_test_res_rblank(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480R",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> +	drm_cmdline_test_res_init(ignored, &mode, "720x480R");
>   	FAIL_ON(mode.refresh_specified);
> -
>  	FAIL_ON(mode.bpp_specified);
>   	FAIL_ON(!mode.rb);
> @@ -291,23 +287,13 @@ static int drm_cmdline_test_res_bpp(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> +	drm_cmdline_test_res_init(ignored, &mode, "720x480-24");
>   	FAIL_ON(mode.refresh_specified);
> -
>  	FAIL_ON(!mode.bpp_specified);
>  	FAIL_ON(mode.bpp != 24);
>  -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
>   	return 0;
>  }
> @@ -327,23 +313,13 @@ static int drm_cmdline_test_res_refresh(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480@60",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> +	drm_cmdline_test_res_init(ignored, &mode, "720x480@60");
>   	FAIL_ON(!mode.refresh_specified);
>  	FAIL_ON(mode.refresh != 60);
> -
>  	FAIL_ON(mode.bpp_specified);
>  -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
>   	return 0;
>  }
> @@ -363,24 +339,8 @@ static int drm_cmdline_test_res_bpp_refresh(void
> *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> -
> -	FAIL_ON(!mode.refresh_specified);
> -	FAIL_ON(mode.refresh != 60);
> -
> -	FAIL_ON(!mode.bpp_specified);
> -	FAIL_ON(mode.bpp != 24);
> -
> -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
> +	drm_cmdline_test_res_bpp_init(ignored, &mode, "720x480-24@60");
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
>   	return 0;
>  }
> @@ -389,18 +349,7 @@ static int
> drm_cmdline_test_res_bpp_refresh_interlaced(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60i",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> -
> -	FAIL_ON(!mode.refresh_specified);
> -	FAIL_ON(mode.refresh != 60);
> -
> -	FAIL_ON(!mode.bpp_specified);
> -	FAIL_ON(mode.bpp != 24);
> +	drm_cmdline_test_res_bpp_init(ignored, &mode, "720x480-24@60i");
>   	FAIL_ON(mode.rb);
>  	FAIL_ON(mode.cvt);
> @@ -415,18 +364,7 @@ static int
> drm_cmdline_test_res_bpp_refresh_margins(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60m",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> -
> -	FAIL_ON(!mode.refresh_specified);
> -	FAIL_ON(mode.refresh != 60);
> -
> -	FAIL_ON(!mode.bpp_specified);
> -	FAIL_ON(mode.bpp != 24);
> +	drm_cmdline_test_res_bpp_init(ignored, &mode, "720x480-24@60m");
>   	FAIL_ON(mode.rb);
>  	FAIL_ON(mode.cvt);
> @@ -441,24 +379,8 @@ static int
> drm_cmdline_test_res_bpp_refresh_force_off(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60d",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> -
> -	FAIL_ON(!mode.refresh_specified);
> -	FAIL_ON(mode.refresh != 60);
> -
> -	FAIL_ON(!mode.bpp_specified);
> -	FAIL_ON(mode.bpp != 24);
> -
> -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_OFF);
> +	drm_cmdline_test_res_bpp_init(ignored, &mode, "720x480-24@60d");
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_OFF);
>   	return 0;
>  }
> @@ -478,24 +400,8 @@ static int
> drm_cmdline_test_res_bpp_refresh_force_on(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60e",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> -
> -	FAIL_ON(!mode.refresh_specified);
> -	FAIL_ON(mode.refresh != 60);
> -
> -	FAIL_ON(!mode.bpp_specified);
> -	FAIL_ON(mode.bpp != 24);
> -
> -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_ON);
> +	drm_cmdline_test_res_bpp_init(ignored, &mode, "720x480-24@60e");
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_ON);
>   	return 0;
>  }
> @@ -504,24 +410,8 @@ static int
> drm_cmdline_test_res_bpp_refresh_force_on_analog(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60D",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> -
> -	FAIL_ON(!mode.refresh_specified);
> -	FAIL_ON(mode.refresh != 60);
> -
> -	FAIL_ON(!mode.bpp_specified);
> -	FAIL_ON(mode.bpp != 24);
> -
> -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_ON);
> +	drm_cmdline_test_res_bpp_init(ignored, &mode, "720x480-24@60D");
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_ON);
>   	return 0;
>  }
> @@ -534,8 +424,7 @@ static int
> drm_cmdline_test_res_bpp_refresh_force_on_digital(void *ignored)
>  	};
>   	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60D",
> -							   &connector,
> -							   &mode));
> +							   &connector, &mode));
>  	FAIL_ON(!mode.specified);
>  	FAIL_ON(mode.xres != 720);
>  	FAIL_ON(mode.yres != 480);
> @@ -546,11 +435,7 @@ static int
> drm_cmdline_test_res_bpp_refresh_force_on_digital(void *ignored)
>  	FAIL_ON(!mode.bpp_specified);
>  	FAIL_ON(mode.bpp != 24);
>  -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_ON_DIGITAL);
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_ON_DIGITAL);
>   	return 0;
>  }
> @@ -559,18 +444,7 @@ static int
> drm_cmdline_test_res_bpp_refresh_interlaced_margins_force_on(void *ig
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60ime",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> -
> -	FAIL_ON(!mode.refresh_specified);
> -	FAIL_ON(mode.refresh != 60);
> -
> -	FAIL_ON(!mode.bpp_specified);
> -	FAIL_ON(mode.bpp != 24);
> +	drm_cmdline_test_res_bpp_init(ignored, &mode, "720x480-24@60ime");
>   	FAIL_ON(mode.rb);
>  	FAIL_ON(mode.cvt);
> @@ -585,15 +459,9 @@ static int
> drm_cmdline_test_res_margins_force_on(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480me",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> +	drm_cmdline_test_res_init(ignored, &mode, "720x480me");
>   	FAIL_ON(mode.refresh_specified);
> -
>  	FAIL_ON(mode.bpp_specified);
>   	FAIL_ON(mode.rb);
> @@ -609,15 +477,9 @@ static int drm_cmdline_test_res_vesa_margins(void
> *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480Mm",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> +	drm_cmdline_test_res_init(ignored, &mode, "720x480Mm");
>   	FAIL_ON(mode.refresh_specified);
> -
>  	FAIL_ON(mode.bpp_specified);
>   	FAIL_ON(mode.rb);
> @@ -673,10 +535,9 @@ static int drm_cmdline_test_name_bpp(void *ignored)
>  							   &no_connector,
>  							   &mode));
>  	FAIL_ON(strcmp(mode.name, "NTSC"));
> -
>  	FAIL_ON(mode.refresh_specified);
> -
>  	FAIL_ON(!mode.bpp_specified);
> +
>  	FAIL_ON(mode.bpp != 24);
>   	return 0;
> @@ -760,23 +621,13 @@ static int drm_cmdline_test_rotate_0(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,rotate=0",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> -	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_0);
> +	drm_cmdline_test_res_init(ignored, &mode, "720x480,rotate=0");
>  +	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_0);
>  	FAIL_ON(mode.refresh_specified);
> -
>  	FAIL_ON(mode.bpp_specified);
>  -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
>   	return 0;
>  }
> @@ -785,23 +636,13 @@ static int drm_cmdline_test_rotate_90(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,rotate=90",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> -	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_90);
> +	drm_cmdline_test_res_init(ignored, &mode, "720x480,rotate=90");
>  +	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_90);
>  	FAIL_ON(mode.refresh_specified);
> -
>  	FAIL_ON(mode.bpp_specified);
>  -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
>   	return 0;
>  }
> @@ -810,23 +651,13 @@ static int drm_cmdline_test_rotate_180(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,rotate=180",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> -	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_180);
> +	drm_cmdline_test_res_init(ignored, &mode, "720x480,rotate=180");
>  +	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_180);
>  	FAIL_ON(mode.refresh_specified);
> -
>  	FAIL_ON(mode.bpp_specified);
>  -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
>   	return 0;
>  }
> @@ -835,23 +666,13 @@ static int drm_cmdline_test_rotate_270(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,rotate=270",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> -	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_270);
> +	drm_cmdline_test_res_init(ignored, &mode, "720x480,rotate=270");
>  +	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_270);
>  	FAIL_ON(mode.refresh_specified);
> -
>  	FAIL_ON(mode.bpp_specified);
>  -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
>   	return 0;
>  }
> @@ -860,9 +681,8 @@ static int drm_cmdline_test_rotate_multiple(void
> *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -
> FAIL_ON(drm_mode_parse_command_line_for_connector("720x480,rotate=0,rotate=90",
> -							  &no_connector,
> -							  &mode));
> +	FAIL_ON(drm_mode_parse_command_line_for_connector(
> +				"720x480,rotate=0,rotate=90", &no_connector, &mode));
>   	return 0;
>  }
> @@ -871,9 +691,8 @@ static int drm_cmdline_test_rotate_invalid_val(void
> *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(drm_mode_parse_command_line_for_connector("720x480,rotate=42",
> -							  &no_connector,
> -							  &mode));
> +	FAIL_ON(drm_mode_parse_command_line_for_connector(
> +				"720x480,rotate=42", &no_connector, &mode));
>   	return 0;
>  }
> @@ -882,9 +701,8 @@ static int drm_cmdline_test_rotate_truncated(void
> *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(drm_mode_parse_command_line_for_connector("720x480,rotate=",
> -							  &no_connector,
> -							  &mode));
> +	FAIL_ON(drm_mode_parse_command_line_for_connector(
> +				"720x480,rotate=", &no_connector, &mode));
>   	return 0;
>  }
> @@ -893,23 +711,13 @@ static int drm_cmdline_test_hmirror(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,reflect_x",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> -	FAIL_ON(mode.rotation_reflection != (DRM_MODE_ROTATE_0 |
> DRM_MODE_REFLECT_X));
> +	drm_cmdline_test_res_init(ignored, &mode, "720x480,reflect_x");
>  +	FAIL_ON(mode.rotation_reflection != (DRM_MODE_ROTATE_0 |
> DRM_MODE_REFLECT_X));
>  	FAIL_ON(mode.refresh_specified);
> -
>  	FAIL_ON(mode.bpp_specified);
>  -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
>   	return 0;
>  }
> @@ -918,23 +726,13 @@ static int drm_cmdline_test_vmirror(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,reflect_y",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> -	FAIL_ON(mode.rotation_reflection != (DRM_MODE_ROTATE_0 |
> DRM_MODE_REFLECT_Y));
> +	drm_cmdline_test_res_init(ignored, &mode, "720x480,reflect_y");
>  +	FAIL_ON(mode.rotation_reflection != (DRM_MODE_ROTATE_0 |
> DRM_MODE_REFLECT_Y));
>  	FAIL_ON(mode.refresh_specified);
> -
>  	FAIL_ON(mode.bpp_specified);
>  -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
>   	return 0;
>  }
> @@ -943,26 +741,18 @@ static int drm_cmdline_test_margin_options(void
> *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -
> FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,margin_right=14,margin_left=24,margin_bottom=36,margin_top=42",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> +	drm_cmdline_test_res_init(ignored, &mode,
> +		
> "720x480,margin_right=14,margin_left=24,margin_bottom=36,margin_top=42");
> +
>  	FAIL_ON(mode.tv_margins.right != 14);
>  	FAIL_ON(mode.tv_margins.left != 24);
>  	FAIL_ON(mode.tv_margins.bottom != 36);
>  	FAIL_ON(mode.tv_margins.top != 42);
>   	FAIL_ON(mode.refresh_specified);
> -
>  	FAIL_ON(mode.bpp_specified);
>  -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
>   	return 0;
>  }
> @@ -971,23 +761,13 @@ static int drm_cmdline_test_multiple_options(void
> *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -
> FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,rotate=270,reflect_x",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> -	FAIL_ON(mode.rotation_reflection != (DRM_MODE_ROTATE_270 |
> DRM_MODE_REFLECT_X));
> +	drm_cmdline_test_res_init(ignored, &mode, "720x480,rotate=270,reflect_x");
>  +	FAIL_ON(mode.rotation_reflection != (DRM_MODE_ROTATE_270 |
> DRM_MODE_REFLECT_X));
>  	FAIL_ON(mode.refresh_specified);
> -
>  	FAIL_ON(mode.bpp_specified);
>  -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
>   	return 0;
>  }
> @@ -996,9 +776,8 @@ static int drm_cmdline_test_invalid_option(void
> *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(drm_mode_parse_command_line_for_connector("720x480,test=42",
> -							  &no_connector,
> -							  &mode));
> +	FAIL_ON(drm_mode_parse_command_line_for_connector(
> +				"720x480,test=42", &no_connector, &mode));
>   	return 0;
>  }
> @@ -1007,24 +786,14 @@ static int
> drm_cmdline_test_bpp_extra_and_option(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -
> FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24e,rotate=180",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> -	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_180);
> +	drm_cmdline_test_res_init(ignored, &mode, "720x480-24e,rotate=180");
>  +	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_180);
>  	FAIL_ON(mode.refresh_specified);
> -
>  	FAIL_ON(!mode.bpp_specified);
>  	FAIL_ON(mode.bpp != 24);
>  -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_ON);
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_ON);
>   	return 0;
>  }
> @@ -1033,22 +802,13 @@ static int drm_cmdline_test_extra_and_option(void
> *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480e,rotate=180",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(!mode.specified);
> -	FAIL_ON(mode.xres != 720);
> -	FAIL_ON(mode.yres != 480);
> -	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_180);
> +	drm_cmdline_test_res_init(ignored, &mode, "720x480e,rotate=180");
>  +	FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_180);
>  	FAIL_ON(mode.refresh_specified);
>  	FAIL_ON(mode.bpp_specified);
>  -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_ON);
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_ON);
>   	return 0;
>  }
> @@ -1057,23 +817,11 @@ static int
> drm_cmdline_test_freestanding_options(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -
> FAIL_ON(!drm_mode_parse_command_line_for_connector("margin_right=14,margin_left=24,margin_bottom=36,margin_top=42",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(mode.specified);
> -	FAIL_ON(mode.refresh_specified);
> -	FAIL_ON(mode.bpp_specified);
> +	drm_cmdline_test_freestanding(ignored, &mode,
> +			"margin_right=14,margin_left=24,margin_bottom=36,margin_top=42",
> +			&no_connector);
>  -	FAIL_ON(mode.tv_margins.right != 14);
> -	FAIL_ON(mode.tv_margins.left != 24);
> -	FAIL_ON(mode.tv_margins.bottom != 36);
> -	FAIL_ON(mode.tv_margins.top != 42);
> -
> -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
>   	return 0;
>  }
> @@ -1082,23 +830,11 @@ static int
> drm_cmdline_test_freestanding_force_e_and_options(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -
> FAIL_ON(!drm_mode_parse_command_line_for_connector("e,margin_right=14,margin_left=24,margin_bottom=36,margin_top=42",
> -							   &no_connector,
> -							   &mode));
> -	FAIL_ON(mode.specified);
> -	FAIL_ON(mode.refresh_specified);
> -	FAIL_ON(mode.bpp_specified);
> +	drm_cmdline_test_freestanding(ignored, &mode,
> +			"e,margin_right=14,margin_left=24,margin_bottom=36,margin_top=42",
> +			&no_connector);
>  -	FAIL_ON(mode.tv_margins.right != 14);
> -	FAIL_ON(mode.tv_margins.left != 24);
> -	FAIL_ON(mode.tv_margins.bottom != 36);
> -	FAIL_ON(mode.tv_margins.top != 42);
> -
> -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_ON);
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_ON);
>   	return 0;
>  }
> @@ -1107,20 +843,17 @@ static int
> drm_cmdline_test_panel_orientation(void *ignored)
>  {
>  	struct drm_cmdline_mode mode = { };
>  -
> FAIL_ON(!drm_mode_parse_command_line_for_connector("panel_orientation=upside_down",
> -							   &no_connector,
> -							   &mode));
> +	FAIL_ON(!drm_mode_parse_command_line_for_connector(
> +				"panel_orientation=upside_down", &no_connector, &mode));
> +
>  	FAIL_ON(mode.specified);
>  	FAIL_ON(mode.refresh_specified);
>  	FAIL_ON(mode.bpp_specified);
>  +
>  	FAIL_ON(mode.panel_orientation != DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP);
>  -	FAIL_ON(mode.rb);
> -	FAIL_ON(mode.cvt);
> -	FAIL_ON(mode.interlace);
> -	FAIL_ON(mode.margins);
> -	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
> +	drm_cmdline_test_properties(ignored, &mode, DRM_FORCE_UNSPECIFIED);
>   	return 0;
>  }
> -- 
> 2.34.1
> 
Reply to thread Export thread (mbox)