Message ID | 20220615135824.15522-1-maira.canal@usp.br (mailing list archive) |
---|---|
Headers | show |
Series | drm: selftest: Convert to KUnit | expand |
On Wed, Jun 15, 2022 at 9:59 PM Maíra Canal <maira.canal@usp.br> wrote: > > KUnit unifies the test structure and provides helper tools that simplify > the development of tests. The basic use case allows running tests as regular > processes, which makes it easier to run unit tests on a development machine > and to integrate the tests into a CI system. > > That said, the conversion of selftests for DRM to KUnit tests is beneficial > as it unifies the testing API by using the KUnit API. > > KUnit is beneficial for developers as it eases the process to run unit tests. > It is possible to run the tests by using the kunit-tool on userspace with the > following command: > > ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/tests --arch=x86_64 > > For CI system, it is possible to execute during the build. But, we also think > about IGT: we are developing a patch to introduce KUnit to IGT. > > These patches were developed during a KUnit hackathon [0] last October. Now, > we believe that both the IGT side and the Kernel side are in good shape for > submission. > > If you are willing to check the output, here is the Pastebin with the output > and execution times [1]. > > [0] https://groups.google.com/g/kunit-dev/c/YqFR1q2uZvk/m/IbvItSfHBAAJ > [1] https://pastebin.com/FJjLPKsC > > - Arthur Grillo, Isabella Basso, and Maíra Canal Great to see these going upstream! I've tested them on my machine, both with x86_64 qemu and with UML using: ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/tests/.kunitconfig \ --kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y \ --kconfig_add CONFIG_VIRTIO_UML=y And all 114 tests pass, and everything looks good. My only minor notes (from a quick look at the results, rather than a detailed review of the code) are that the test names have a few small oddities: - The suites all end in _tests (or _test, in the case of drm_plane_helper_test). This is a bit redundant (and while there is only one drm_plane_helper_test, the inconsistency with the others is a bit awkward), so removing the suffix may be cleaner. (Or at least being optimistic, and making drm_plane_helper_test plural.) - The drm_cmdline_parser_tests suite's tests have some inconsistencies name-wise: they're the only ones to start with drm_, not igt_, and they have a few capital letters in some of the 'drm_cmdline_test_force_D_' tests. (It's also technically redundant to start all of the test names with drm_cmdline_test, given the suite name.) Of course, if you're trying to keep compatibility with existing tests or tooling, or there's some deeper reason they're named like this, it's definitely not a dealbreaker. Either way, this whole series is: Tested-by: David Gow <davidgow@google.com> Cheers, -- David
On 6/16/22 16:55, David Gow wrote: > On Wed, Jun 15, 2022 at 9:59 PM Maíra Canal <maira.canal@usp.br> wrote: >> >> KUnit unifies the test structure and provides helper tools that simplify >> the development of tests. The basic use case allows running tests as regular >> processes, which makes it easier to run unit tests on a development machine >> and to integrate the tests into a CI system. >> >> That said, the conversion of selftests for DRM to KUnit tests is beneficial >> as it unifies the testing API by using the KUnit API. >> >> KUnit is beneficial for developers as it eases the process to run unit tests. >> It is possible to run the tests by using the kunit-tool on userspace with the >> following command: >> >> ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/tests --arch=x86_64 >> >> For CI system, it is possible to execute during the build. But, we also think >> about IGT: we are developing a patch to introduce KUnit to IGT. >> >> These patches were developed during a KUnit hackathon [0] last October. Now, >> we believe that both the IGT side and the Kernel side are in good shape for >> submission. >> >> If you are willing to check the output, here is the Pastebin with the output >> and execution times [1]. >> >> [0] https://groups.google.com/g/kunit-dev/c/YqFR1q2uZvk/m/IbvItSfHBAAJ >> [1] https://pastebin.com/FJjLPKsC >> >> - Arthur Grillo, Isabella Basso, and Maíra Canal > > Great to see these going upstream! > Indeed, this is pretty awesome! I haven't reviewed the patches yet but just have a meta comment. There's a TODO entry for this [0] in Documentation/gpu/todo.rst, so I think that you could add a patch removing that as a part of this series. [0]: https://cgit.freedesktop.org/drm/drm/tree/Documentation/gpu/todo.rst#n620
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