[PATCH v2 1/3] drm/tests: Add tests for the new Monochrome value of tv_mode

[Maxime Ripard]

Hi,

Thanks for working on the tests

On Wed, Jun 19, 2024 at 04:39:11PM GMT, Dave Stevenson wrote:
> Adds test for the cmdline parser, connector property, and
> drm_analog_tv_mode to ensure the behaviour of the new value is
> correct.
> 
> Signed-off-by: Dave Stevenson <dave.stevenson at raspberrypi.com>
> ---
>  .../gpu/drm/tests/drm_cmdline_parser_test.c   | 20 ++++++------
>  drivers/gpu/drm/tests/drm_connector_test.c    |  1 +
>  drivers/gpu/drm/tests/drm_modes_test.c        | 31 +++++++++++++++++++
>  3 files changed, 43 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tests/drm_cmdline_parser_test.c b/drivers/gpu/drm/tests/drm_cmdline_parser_test.c
> index 6f1457bd21d9..95fb379c69eb 100644
> --- a/drivers/gpu/drm/tests/drm_cmdline_parser_test.c
> +++ b/drivers/gpu/drm/tests/drm_cmdline_parser_test.c
> @@ -976,22 +976,24 @@ static void drm_test_cmdline_tv_options(struct kunit *test)
>  	KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
>  }
>  
> -#define TV_OPT_TEST(_opt, _cmdline, _mode_fn)		\
> +#define TV_OPT_TEST(_opt, _cmdline, _mode_fn, _tvmode)		\
>  	{						\
>  		.name = #_opt,				\
>  		.cmdline = _cmdline,			\
>  		.mode_fn = _mode_fn,			\
> -		.tv_mode = DRM_MODE_TV_MODE_ ## _opt,	\
> +		.tv_mode = DRM_MODE_TV_MODE_ ## _tvmode,	\
>  	}
>  
>  static const struct drm_cmdline_tv_option_test drm_cmdline_tv_option_tests[] = {
> -	TV_OPT_TEST(NTSC, "720x480i,tv_mode=NTSC", drm_mode_analog_ntsc_480i),
> -	TV_OPT_TEST(NTSC_443, "720x480i,tv_mode=NTSC-443", drm_mode_analog_ntsc_480i),
> -	TV_OPT_TEST(NTSC_J, "720x480i,tv_mode=NTSC-J", drm_mode_analog_ntsc_480i),
> -	TV_OPT_TEST(PAL, "720x576i,tv_mode=PAL", drm_mode_analog_pal_576i),
> -	TV_OPT_TEST(PAL_M, "720x480i,tv_mode=PAL-M", drm_mode_analog_ntsc_480i),
> -	TV_OPT_TEST(PAL_N, "720x576i,tv_mode=PAL-N", drm_mode_analog_pal_576i),
> -	TV_OPT_TEST(SECAM, "720x576i,tv_mode=SECAM", drm_mode_analog_pal_576i),
> +	TV_OPT_TEST(NTSC, "720x480i,tv_mode=NTSC", drm_mode_analog_ntsc_480i, NTSC),
> +	TV_OPT_TEST(NTSC_443, "720x480i,tv_mode=NTSC-443", drm_mode_analog_ntsc_480i, NTSC_443),
> +	TV_OPT_TEST(NTSC_J, "720x480i,tv_mode=NTSC-J", drm_mode_analog_ntsc_480i, NTSC_J),
> +	TV_OPT_TEST(PAL, "720x576i,tv_mode=PAL", drm_mode_analog_pal_576i, PAL),
> +	TV_OPT_TEST(PAL_M, "720x480i,tv_mode=PAL-M", drm_mode_analog_ntsc_480i, PAL_M),
> +	TV_OPT_TEST(PAL_N, "720x576i,tv_mode=PAL-N", drm_mode_analog_pal_576i, PAL_N),
> +	TV_OPT_TEST(SECAM, "720x576i,tv_mode=SECAM", drm_mode_analog_pal_576i, SECAM),
> +	TV_OPT_TEST(MONO_525, "720x480i,tv_mode=Mono", drm_mode_analog_ntsc_480i, MONOCHROME),
> +	TV_OPT_TEST(MONO_625, "720x576i,tv_mode=Mono", drm_mode_analog_pal_576i, MONOCHROME),
>  };

I assume you did that because, otherwise, the name for both mono
variants would have been the same and the test generation wouldn't work
anymore?

If so, I think I'd prefer to not use the macro in this case but define
the struct by hand. It keeps the general case clean, while allowing to
deal with our odd case still.

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20240620/5a4f177b/attachment.sig>