[PATCH 02/10] drm: selftest: refactor drm_cmdline_parser

Shuah Khan skhan at linuxfoundation.org
Fri Jun 17 22:58:23 UTC 2022


On 6/15/22 7:58 AM, Maíra Canal wrote:
> From: Arthur Grillo <arthur.grillo at usp.br>
> 
> Refactor the tests by modularizing the functions to avoid code repetition.
> 

Tell me more about the refactor and how does it help. This patch seems
to combine refactor with some other formatting changes that aren't
necessary and making the code not easy to read.

Lot of code changes with no expalination on how and why this is being
refactored. Are these just refractor or are there any new tests being
added?

Please don't cobine formatting changes with refactoring. Also don't
break up the code into small chunks unless there is a good reason to
do so.

> Co-developed-by: Maíra Canal <maira.canal at usp.br>
> Signed-off-by: Arthur Grillo <arthur.grillo at usp.br>
> Signed-off-by: Maíra Canal <maira.canal at 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 at usp.br>,
> + * Copyright (c) 2021 Arthur Grillo <arthur.grillo at 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));

This change for example.

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

These should be lined up with the first argument.

>   {
> -	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);
>   

Whst are these constants for - please add defines for them with meaningful
names so it cna be maintained easily.

> -	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);

Same here

> +
> +	return 0;
> +}
> +
> +static int drm_cmdline_test_force_e_only(void *ignored)
> +{
> +	drm_cmdline_test_force_only(ignored, "e", &no_connector, DRM_FORCE_ON);
> +
Get rid of the extra line.

> +	return 0;

Same comment here on a new routine. Let's not add new routines
> +}
> +
> +static int drm_cmdline_test_force_D_only_not_digital(void *ignored)
> +{
> +	drm_cmdline_test_force_only(ignored, "D", &no_connector, DRM_FORCE_ON);
>   
same here. What is the need to add a whole new routine for this.
It you really want to make this a marco.


>   	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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 60D",
> -							   &connector,
> -							   &mode));
> +							   &connector, &mode));

Why combine the lines here - the code was just fine earlier.

>   	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 at 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 at 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));

Same here about changing the format.

> +
>   	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;
>   }
> 

thanks,
-- Shuah


More information about the dri-devel mailing list