From: Shuah Khan <skhan@linuxfoundation.org>
To: "Maíra Canal" <maira.canal@usp.br>,
"Isabella Basso" <isabbasso@riseup.net>,
magalilemes00@gmail.com, tales.aparecida@gmail.com,
mwen@igalia.com, andrealmeid@riseup.net,
"Trevor Woerner" <twoerner@gmail.com>,
leandro.ribeiro@collabora.com, n@nfraprado.net,
"Daniel Vetter" <daniel@ffwll.ch>,
"David Airlie" <airlied@linux.ie>,
"Maxime Ripard" <mripard@kernel.org>,
"Thomas Zimmermann" <tzimmermann@suse.de>,
michal.winiarski@intel.com,
"Javier Martinez Canillas" <javierm@redhat.com>,
"José Expósito" <jose.exposito89@gmail.com>,
"David Gow" <davidgow@google.com>,
"Daniel Latypov" <dlatypov@google.com>,
brendanhiggins@google.com
Cc: Arthur Grillo <arthur.grillo@usp.br>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
linux-kselftest@vger.kernel.org,
Shuah Khan <skhan@linuxfoundation.org>,
kunit-dev@googlegroups.com
Subject: Re: [PATCH 02/10] drm: selftest: refactor drm_cmdline_parser
Date: Fri, 17 Jun 2022 16:58:23 -0600 [thread overview]
Message-ID: <350cf07e-297d-0578-f139-35bf0a9c7a96@linuxfoundation.org> (raw)
In-Reply-To: <20220615135824.15522-3-maira.canal@usp.br>
On 6/15/22 7:58 AM, Maíra Canal wrote:
> From: Arthur Grillo <arthur.grillo@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@usp.br>
> Signed-off-by: Arthur Grillo <arthur.grillo@usp.br>
> Signed-off-by: Maíra Canal <maira.canal@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));
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@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));
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@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));
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
next prev parent reply other threads:[~2022-06-17 22:58 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-15 13:58 [PATCH 00/10] drm: selftest: Convert to KUnit Maíra Canal
2022-06-15 13:58 ` [PATCH 01/10] drm: selftest: convert drm_damage_helper selftest " Maíra Canal
2022-06-15 14:16 ` [PATCH 02/10] drm: selftest: refactor drm_cmdline_parser Maíra Canal
2022-06-15 13:58 ` Maíra Canal
2022-06-17 22:58 ` Shuah Khan [this message]
2022-06-15 13:58 ` [PATCH 03/10] drm: selftest: convert drm_cmdline_parser selftest to KUnit Maíra Canal
2022-06-15 13:58 ` [PATCH 04/10] drm: selftest: convert drm_rect " Maíra Canal
2022-06-15 13:58 ` [PATCH 05/10] drm: selftest: convert drm_format " Maíra Canal
2022-06-15 18:03 ` kernel test robot
2022-06-15 13:58 ` [PATCH 06/10] drm: selftest: convert drm_plane_helper " Maíra Canal
2022-06-15 13:58 ` [PATCH 07/10] drm: selftest: convert drm_dp_mst_helper " Maíra Canal
2022-06-18 13:38 ` kernel test robot
2022-06-15 13:58 ` [PATCH 08/10] drm: selftest: convert drm_framebuffer " Maíra Canal
2022-06-19 15:40 ` kernel test robot
2022-06-15 13:58 ` [PATCH 09/10] drm: selftest: convert drm_buddy " Maíra Canal
2022-06-15 14:27 ` [PATCH 10/10] drm: selftest: convert drm_mm " Maíra Canal
2022-06-16 14:55 ` [PATCH 00/10] drm: selftest: Convert " David Gow
2022-06-16 16:43 ` Javier Martinez Canillas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=350cf07e-297d-0578-f139-35bf0a9c7a96@linuxfoundation.org \
--to=skhan@linuxfoundation.org \
--cc=airlied@linux.ie \
--cc=andrealmeid@riseup.net \
--cc=arthur.grillo@usp.br \
--cc=brendanhiggins@google.com \
--cc=daniel@ffwll.ch \
--cc=davidgow@google.com \
--cc=dlatypov@google.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=isabbasso@riseup.net \
--cc=javierm@redhat.com \
--cc=jose.exposito89@gmail.com \
--cc=kunit-dev@googlegroups.com \
--cc=leandro.ribeiro@collabora.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=magalilemes00@gmail.com \
--cc=maira.canal@usp.br \
--cc=michal.winiarski@intel.com \
--cc=mripard@kernel.org \
--cc=mwen@igalia.com \
--cc=n@nfraprado.net \
--cc=tales.aparecida@gmail.com \
--cc=twoerner@gmail.com \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).