Hi Maxime, On 8/29/22 10:11, Maxime Ripard wrote: > Our new tv mode option allows to specify the TV mode from a property. > However, it can still be useful, for example to avoid any boot time > artifact, to set that property directly from the kernel command line. > > Let's add some code to allow it, and some unit tests to exercise that code. > > Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx> > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index 73d01e755496..a759a4ba0036 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -2115,6 +2115,30 @@ static int drm_mode_parse_panel_orientation(const char *delim, > return 0; > } > > +static int drm_mode_parse_tv_mode(const char *delim, > + struct drm_cmdline_mode *mode) > +{ > + const char *value; > + unsigned int len; Looks like this variable len is not being used and is producing the following warning: ../drivers/gpu/drm/drm_modes.c:2122:15: warning: unused variable 'len' [-Wunused-variable] unsigned int len; ^ Best Regards, - Maíra Canal > + int ret; > + > + if (*delim != '=') > + return -EINVAL; > + > + value = delim + 1; > + delim = strchr(value, ','); > + if (!delim) > + delim = value + strlen(value); > + > + ret = drm_get_tv_mode_from_name(value, delim - value); > + if (ret < 0) > + return ret; > + > + mode->tv_mode = ret; > + > + return 0; > +} > + > static int drm_mode_parse_cmdline_options(const char *str, > bool freestanding, > const struct drm_connector *connector, > @@ -2184,6 +2208,9 @@ static int drm_mode_parse_cmdline_options(const char *str, > } else if (!strncmp(option, "panel_orientation", delim - option)) { > if (drm_mode_parse_panel_orientation(delim, mode)) > return -EINVAL; > + } else if (!strncmp(option, "tv_mode", delim - option)) { > + if (drm_mode_parse_tv_mode(delim, mode)) > + return -EINVAL; > } else { > return -EINVAL; > } > @@ -2212,20 +2239,22 @@ struct drm_named_mode { > unsigned int xres; > unsigned int yres; > unsigned int flags; > + unsigned int tv_mode; > }; > > -#define NAMED_MODE(_name, _pclk, _x, _y, _flags) \ > +#define NAMED_MODE(_name, _pclk, _x, _y, _flags, _mode) \ > { \ > .name = _name, \ > .pixel_clock_khz = _pclk, \ > .xres = _x, \ > .yres = _y, \ > .flags = _flags, \ > + .tv_mode = _mode, \ > } > > static const struct drm_named_mode drm_named_modes[] = { > - NAMED_MODE("NTSC", 13500, 720, 480, DRM_MODE_FLAG_INTERLACE), > - NAMED_MODE("PAL", 13500, 720, 576, DRM_MODE_FLAG_INTERLACE), > + NAMED_MODE("NTSC", 13500, 720, 480, DRM_MODE_FLAG_INTERLACE, DRM_MODE_TV_MODE_NTSC_M), > + NAMED_MODE("PAL", 13500, 720, 576, DRM_MODE_FLAG_INTERLACE, DRM_MODE_TV_MODE_PAL_B), > }; > > static int drm_mode_parse_cmdline_named_mode(const char *name, > @@ -2271,6 +2300,7 @@ static int drm_mode_parse_cmdline_named_mode(const char *name, > cmdline_mode->xres = mode->xres; > cmdline_mode->yres = mode->yres; > cmdline_mode->interlace = !!(mode->flags & DRM_MODE_FLAG_INTERLACE); > + cmdline_mode->tv_mode = mode->tv_mode; > cmdline_mode->specified = true; > > return 1; > diff --git a/drivers/gpu/drm/tests/drm_cmdline_parser_test.c b/drivers/gpu/drm/tests/drm_cmdline_parser_test.c > index 59b29cdfdd35..f1e73ed65be0 100644 > --- a/drivers/gpu/drm/tests/drm_cmdline_parser_test.c > +++ b/drivers/gpu/drm/tests/drm_cmdline_parser_test.c > @@ -885,6 +885,201 @@ static void drm_cmdline_test_multiple_options(struct kunit *test) > KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED); > } > > +static void drm_cmdline_test_tv_options(struct kunit *test, > + const char *cmdline, > + const struct drm_display_mode *expected_mode, > + unsigned int expected_tv_mode) > +{ > + struct drm_cmdline_mode mode = { }; > + > + KUNIT_EXPECT_TRUE(test, drm_mode_parse_command_line_for_connector(cmdline, > + &no_connector, &mode)); > + KUNIT_EXPECT_TRUE(test, mode.specified); > + KUNIT_EXPECT_EQ(test, mode.xres, expected_mode->hdisplay); > + KUNIT_EXPECT_EQ(test, mode.yres, expected_mode->vdisplay); > + KUNIT_EXPECT_EQ(test, mode.tv_mode, expected_tv_mode); > + > + KUNIT_EXPECT_FALSE(test, mode.refresh_specified); > + > + KUNIT_EXPECT_FALSE(test, mode.bpp_specified); > + > + KUNIT_EXPECT_FALSE(test, mode.rb); > + KUNIT_EXPECT_FALSE(test, mode.cvt); > + KUNIT_EXPECT_EQ(test, mode.interlace, !!(expected_mode->flags & DRM_MODE_FLAG_INTERLACE)); > + KUNIT_EXPECT_FALSE(test, mode.margins); > + KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED); > +} > + > +static void drm_cmdline_test_tv_option_ntsc_443(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x480i,tv_mode=NTSC-443", > + drm_mode_analog_ntsc_480i(NULL), > + DRM_MODE_TV_MODE_NTSC_443); > +} > + > +static void drm_cmdline_test_tv_option_ntsc_j(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x480i,tv_mode=NTSC-J", > + drm_mode_analog_ntsc_480i(NULL), > + DRM_MODE_TV_MODE_NTSC_J); > +} > + > +static void drm_cmdline_test_tv_option_ntsc_m(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x480i,tv_mode=NTSC-M", > + drm_mode_analog_ntsc_480i(NULL), > + DRM_MODE_TV_MODE_NTSC_M); > +} > + > +static void drm_cmdline_test_tv_option_pal_60(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x576i,tv_mode=PAL-60", > + drm_mode_analog_pal_576i(NULL), > + DRM_MODE_TV_MODE_PAL_60); > +} > + > +static void drm_cmdline_test_tv_option_pal_b(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x576i,tv_mode=PAL-B", > + drm_mode_analog_pal_576i(NULL), > + DRM_MODE_TV_MODE_PAL_B); > +} > + > +static void drm_cmdline_test_tv_option_pal_d(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x576i,tv_mode=PAL-D", > + drm_mode_analog_pal_576i(NULL), > + DRM_MODE_TV_MODE_PAL_D); > +} > + > +static void drm_cmdline_test_tv_option_pal_g(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x576i,tv_mode=PAL-G", > + drm_mode_analog_pal_576i(NULL), > + DRM_MODE_TV_MODE_PAL_G); > +} > + > +static void drm_cmdline_test_tv_option_pal_h(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x576i,tv_mode=PAL-H", > + drm_mode_analog_pal_576i(NULL), > + DRM_MODE_TV_MODE_PAL_H); > +} > + > +static void drm_cmdline_test_tv_option_pal_i(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x576i,tv_mode=PAL-I", > + drm_mode_analog_pal_576i(NULL), > + DRM_MODE_TV_MODE_PAL_I); > +} > + > +static void drm_cmdline_test_tv_option_pal_m(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x480i,tv_mode=PAL-M", > + drm_mode_analog_ntsc_480i(NULL), > + DRM_MODE_TV_MODE_PAL_M); > +} > + > +static void drm_cmdline_test_tv_option_pal_n(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x576i,tv_mode=PAL-N", > + drm_mode_analog_pal_576i(NULL), > + DRM_MODE_TV_MODE_PAL_N); > +} > + > +static void drm_cmdline_test_tv_option_pal_nc(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x576i,tv_mode=PAL-Nc", > + drm_mode_analog_pal_576i(NULL), > + DRM_MODE_TV_MODE_PAL_NC); > +} > + > +static void drm_cmdline_test_tv_option_secam_60(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x576i,tv_mode=SECAM-60", > + drm_mode_analog_pal_576i(NULL), > + DRM_MODE_TV_MODE_SECAM_60); > +} > + > +static void drm_cmdline_test_tv_option_secam_b(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x576i,tv_mode=SECAM-B", > + drm_mode_analog_pal_576i(NULL), > + DRM_MODE_TV_MODE_SECAM_B); > +} > + > +static void drm_cmdline_test_tv_option_secam_d(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x576i,tv_mode=SECAM-D", > + drm_mode_analog_pal_576i(NULL), > + DRM_MODE_TV_MODE_SECAM_D); > +} > + > +static void drm_cmdline_test_tv_option_secam_g(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x576i,tv_mode=SECAM-G", > + drm_mode_analog_pal_576i(NULL), > + DRM_MODE_TV_MODE_SECAM_G); > +} > + > +static void drm_cmdline_test_tv_option_secam_k(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x576i,tv_mode=SECAM-K", > + drm_mode_analog_pal_576i(NULL), > + DRM_MODE_TV_MODE_SECAM_K); > +} > + > +static void drm_cmdline_test_tv_option_secam_k1(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x576i,tv_mode=SECAM-K1", > + drm_mode_analog_pal_576i(NULL), > + DRM_MODE_TV_MODE_SECAM_K1); > +} > + > +static void drm_cmdline_test_tv_option_secam_l(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x576i,tv_mode=SECAM-L", > + drm_mode_analog_pal_576i(NULL), > + DRM_MODE_TV_MODE_SECAM_L); > +} > + > +static void drm_cmdline_test_tv_option_invalid(struct kunit *test) > +{ > + struct drm_cmdline_mode mode = { }; > + const char *cmdline = "720x480i,tv_mode=invalid"; > + > + KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline, > + &no_connector, &mode)); > +} > + > +static void drm_cmdline_test_tv_option_truncated(struct kunit *test) > +{ > + struct drm_cmdline_mode mode = { }; > + const char *cmdline = "720x480i,tv_mode=NTSC"; > + > + KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline, > + &no_connector, &mode)); > +} > + > static void drm_cmdline_test_invalid_option(struct kunit *test) > { > struct drm_cmdline_mode mode = { }; > @@ -1047,6 +1242,27 @@ static struct kunit_case drm_cmdline_parser_tests[] = { > KUNIT_CASE(drm_cmdline_test_name_refresh_invalid_mode), > KUNIT_CASE(drm_cmdline_test_name_option), > KUNIT_CASE(drm_cmdline_test_name_bpp_option), > + KUNIT_CASE(drm_cmdline_test_tv_option_ntsc_443), > + KUNIT_CASE(drm_cmdline_test_tv_option_ntsc_j), > + KUNIT_CASE(drm_cmdline_test_tv_option_ntsc_m), > + KUNIT_CASE(drm_cmdline_test_tv_option_pal_60), > + KUNIT_CASE(drm_cmdline_test_tv_option_pal_b), > + KUNIT_CASE(drm_cmdline_test_tv_option_pal_d), > + KUNIT_CASE(drm_cmdline_test_tv_option_pal_g), > + KUNIT_CASE(drm_cmdline_test_tv_option_pal_h), > + KUNIT_CASE(drm_cmdline_test_tv_option_pal_i), > + KUNIT_CASE(drm_cmdline_test_tv_option_pal_m), > + KUNIT_CASE(drm_cmdline_test_tv_option_pal_n), > + KUNIT_CASE(drm_cmdline_test_tv_option_pal_nc), > + KUNIT_CASE(drm_cmdline_test_tv_option_secam_60), > + KUNIT_CASE(drm_cmdline_test_tv_option_secam_b), > + KUNIT_CASE(drm_cmdline_test_tv_option_secam_d), > + KUNIT_CASE(drm_cmdline_test_tv_option_secam_g), > + KUNIT_CASE(drm_cmdline_test_tv_option_secam_k), > + KUNIT_CASE(drm_cmdline_test_tv_option_secam_k1), > + KUNIT_CASE(drm_cmdline_test_tv_option_secam_l), > + KUNIT_CASE(drm_cmdline_test_tv_option_invalid), > + KUNIT_CASE(drm_cmdline_test_tv_option_truncated), > KUNIT_CASE(drm_cmdline_test_rotate_0), > KUNIT_CASE(drm_cmdline_test_rotate_90), > KUNIT_CASE(drm_cmdline_test_rotate_180), > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 49d261977d4e..9589108ba202 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -1447,6 +1447,11 @@ struct drm_cmdline_mode { > * @tv_margins: TV margins to apply to the mode. > */ > struct drm_connector_tv_margins tv_margins; > + > + /** > + * @tv_mode: TV mode standard. See DRM_MODE_TV_MODE_*. > + */ > + enum drm_connector_tv_mode tv_mode; > }; > > /** >