| Message ID | 20220728-rpi-analog-tv-properties-v1-34-3d53ae722097@cerno.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Analog TV Improvements | expand |
Hi Maxime, On Fri, Jul 29, 2022 at 6:37 PM Maxime Ripard <maxime@cerno.tech> 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@cerno.tech> Thanks for your patch! > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -1677,6 +1677,80 @@ static int drm_mode_parse_panel_orientation(const char *delim, > return 0; > } > > +#define TV_OPTION_EQUAL(value, len, option) \ > + ((strlen(option) == len) && !strncmp(value, option, len)) > + > +static int drm_mode_parse_tv_mode(const char *delim, > + struct drm_cmdline_mode *mode) > +{ > + const char *value; > + unsigned int len; > + > + if (*delim != '=') > + return -EINVAL; > + > + value = delim + 1; > + delim = strchr(value, ','); > + if (!delim) > + delim = value + strlen(value); > + > + len = delim - value; > + if (TV_OPTION_EQUAL(value, len, "NTSC-443")) > + mode->tv_mode = DRM_MODE_TV_NORM_NTSC_443; > + else if (TV_OPTION_EQUAL(value, len, "NTSC-J")) > + mode->tv_mode = DRM_MODE_TV_NORM_NTSC_J; > + else if (TV_OPTION_EQUAL(value, len, "NTSC-M")) > + mode->tv_mode = DRM_MODE_TV_NORM_NTSC_M; [...] You already have the array tv_norm_values[] from "[PATCH v1 05/35] drm/connector: Add TV standard property". Can't you export that, and loop over that array instead? > + else if (TV_OPTION_EQUAL(value, len, "HD480I")) > + mode->tv_mode = DRM_MODE_TV_NORM_HD480I; > + else if (TV_OPTION_EQUAL(value, len, "HD480P")) > + mode->tv_mode = DRM_MODE_TV_NORM_HD480P; > + else if (TV_OPTION_EQUAL(value, len, "HD576I")) > + mode->tv_mode = DRM_MODE_TV_NORM_HD576I; > + else if (TV_OPTION_EQUAL(value, len, "HD576P")) > + mode->tv_mode = DRM_MODE_TV_NORM_HD576P; > + else if (TV_OPTION_EQUAL(value, len, "HD720P")) > + mode->tv_mode = DRM_MODE_TV_NORM_HD720P; > + else if (TV_OPTION_EQUAL(value, len, "HD1080I")) > + mode->tv_mode = DRM_MODE_TV_NORM_HD1080I; The names in tv_norm_values[] use lower-case, while you use upper-case here. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Fri, Aug 12, 2022 at 03:31:19PM +0200, Geert Uytterhoeven wrote: > Hi Maxime, > > On Fri, Jul 29, 2022 at 6:37 PM Maxime Ripard <maxime@cerno.tech> 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@cerno.tech> > > Thanks for your patch! > > > --- a/drivers/gpu/drm/drm_modes.c > > +++ b/drivers/gpu/drm/drm_modes.c > > @@ -1677,6 +1677,80 @@ static int drm_mode_parse_panel_orientation(const char *delim, > > return 0; > > } > > > > +#define TV_OPTION_EQUAL(value, len, option) \ > > + ((strlen(option) == len) && !strncmp(value, option, len)) > > + > > +static int drm_mode_parse_tv_mode(const char *delim, > > + struct drm_cmdline_mode *mode) > > +{ > > + const char *value; > > + unsigned int len; > > + > > + if (*delim != '=') > > + return -EINVAL; > > + > > + value = delim + 1; > > + delim = strchr(value, ','); > > + if (!delim) > > + delim = value + strlen(value); > > + > > + len = delim - value; > > + if (TV_OPTION_EQUAL(value, len, "NTSC-443")) > > + mode->tv_mode = DRM_MODE_TV_NORM_NTSC_443; > > + else if (TV_OPTION_EQUAL(value, len, "NTSC-J")) > > + mode->tv_mode = DRM_MODE_TV_NORM_NTSC_J; > > + else if (TV_OPTION_EQUAL(value, len, "NTSC-M")) > > + mode->tv_mode = DRM_MODE_TV_NORM_NTSC_M; > > [...] > > You already have the array tv_norm_values[] from "[PATCH v1 05/35] > drm/connector: Add TV standard property". Can't you export that, and > loop over that array instead? I'm not sure, the command line doesn't follow the same conventions than the property names for a number of conventions, but at the same time we should try to keep it as consistent as possible... Then again, Jani and Thomas didn't seem too fond about exposing data as part of the API, so I'm not sure how we could expose that properly. > > + else if (TV_OPTION_EQUAL(value, len, "HD480I")) > > + mode->tv_mode = DRM_MODE_TV_NORM_HD480I; > > + else if (TV_OPTION_EQUAL(value, len, "HD480P")) > > + mode->tv_mode = DRM_MODE_TV_NORM_HD480P; > > + else if (TV_OPTION_EQUAL(value, len, "HD576I")) > > + mode->tv_mode = DRM_MODE_TV_NORM_HD576I; > > + else if (TV_OPTION_EQUAL(value, len, "HD576P")) > > + mode->tv_mode = DRM_MODE_TV_NORM_HD576P; > > + else if (TV_OPTION_EQUAL(value, len, "HD720P")) > > + mode->tv_mode = DRM_MODE_TV_NORM_HD720P; > > + else if (TV_OPTION_EQUAL(value, len, "HD1080I")) > > + mode->tv_mode = DRM_MODE_TV_NORM_HD1080I; > > The names in tv_norm_values[] use lower-case, while you use upper-case > here. Indeed, I'll fix it, thanks! Maxime
Den 29.07.2022 18.35, skrev Maxime Ripard: > 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@cerno.tech> > In the subject it says "tv_mode property", but the property is called "tv norm", so the option should be tv_norm? Noralf. > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c > index fce5569bd66a..49da8d8236f5 100644 > --- a/drivers/gpu/drm/drm_atomic_state_helper.c > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c > @@ -490,6 +490,7 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_tv_margins_reset); > void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector) > { > struct drm_device *dev = connector->dev; > + struct drm_cmdline_mode *cmdline = &connector->cmdline_mode; > struct drm_connector_state *state = connector->state; > struct drm_property *prop; > uint64_t val; > @@ -500,6 +501,9 @@ void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector) > prop, &val)) > state->tv.norm = val; > > + if (cmdline->tv_mode) > + state->tv.norm = cmdline->tv_mode; > + > prop = dev->mode_config.tv_select_subconnector_property; > if (prop) > if (!drm_object_property_get_default_value(&connector->base, > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index ecb2e83cf860..3634ac9f787d 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -1677,6 +1677,80 @@ static int drm_mode_parse_panel_orientation(const char *delim, > return 0; > } > > +#define TV_OPTION_EQUAL(value, len, option) \ > + ((strlen(option) == len) && !strncmp(value, option, len)) > + > +static int drm_mode_parse_tv_mode(const char *delim, > + struct drm_cmdline_mode *mode) > +{ > + const char *value; > + unsigned int len; > + > + if (*delim != '=') > + return -EINVAL; > + > + value = delim + 1; > + delim = strchr(value, ','); > + if (!delim) > + delim = value + strlen(value); > + > + len = delim - value; > + if (TV_OPTION_EQUAL(value, len, "NTSC-443")) > + mode->tv_mode = DRM_MODE_TV_NORM_NTSC_443; > + else if (TV_OPTION_EQUAL(value, len, "NTSC-J")) > + mode->tv_mode = DRM_MODE_TV_NORM_NTSC_J; > + else if (TV_OPTION_EQUAL(value, len, "NTSC-M")) > + mode->tv_mode = DRM_MODE_TV_NORM_NTSC_M; > + else if (TV_OPTION_EQUAL(value, len, "PAL-60")) > + mode->tv_mode = DRM_MODE_TV_NORM_PAL_60; > + else if (TV_OPTION_EQUAL(value, len, "PAL-B")) > + mode->tv_mode = DRM_MODE_TV_NORM_PAL_B; > + else if (TV_OPTION_EQUAL(value, len, "PAL-D")) > + mode->tv_mode = DRM_MODE_TV_NORM_PAL_D; > + else if (TV_OPTION_EQUAL(value, len, "PAL-G")) > + mode->tv_mode = DRM_MODE_TV_NORM_PAL_G; > + else if (TV_OPTION_EQUAL(value, len, "PAL-H")) > + mode->tv_mode = DRM_MODE_TV_NORM_PAL_H; > + else if (TV_OPTION_EQUAL(value, len, "PAL-I")) > + mode->tv_mode = DRM_MODE_TV_NORM_PAL_I; > + else if (TV_OPTION_EQUAL(value, len, "PAL-M")) > + mode->tv_mode = DRM_MODE_TV_NORM_PAL_M; > + else if (TV_OPTION_EQUAL(value, len, "PAL-N")) > + mode->tv_mode = DRM_MODE_TV_NORM_PAL_N; > + else if (TV_OPTION_EQUAL(value, len, "PAL-NC")) > + mode->tv_mode = DRM_MODE_TV_NORM_PAL_NC; > + else if (TV_OPTION_EQUAL(value, len, "SECAM-60")) > + mode->tv_mode = DRM_MODE_TV_NORM_SECAM_60; > + else if (TV_OPTION_EQUAL(value, len, "SECAM-B")) > + mode->tv_mode = DRM_MODE_TV_NORM_SECAM_B; > + else if (TV_OPTION_EQUAL(value, len, "SECAM-D")) > + mode->tv_mode = DRM_MODE_TV_NORM_SECAM_D; > + else if (TV_OPTION_EQUAL(value, len, "SECAM-G")) > + mode->tv_mode = DRM_MODE_TV_NORM_SECAM_G; > + else if (TV_OPTION_EQUAL(value, len, "SECAM-K")) > + mode->tv_mode = DRM_MODE_TV_NORM_SECAM_K; > + else if (TV_OPTION_EQUAL(value, len, "SECAM-K1")) > + mode->tv_mode = DRM_MODE_TV_NORM_SECAM_K1; > + else if (TV_OPTION_EQUAL(value, len, "SECAM-L")) > + mode->tv_mode = DRM_MODE_TV_NORM_SECAM_L; > + else if (TV_OPTION_EQUAL(value, len, "HD480I")) > + mode->tv_mode = DRM_MODE_TV_NORM_HD480I; > + else if (TV_OPTION_EQUAL(value, len, "HD480P")) > + mode->tv_mode = DRM_MODE_TV_NORM_HD480P; > + else if (TV_OPTION_EQUAL(value, len, "HD576I")) > + mode->tv_mode = DRM_MODE_TV_NORM_HD576I; > + else if (TV_OPTION_EQUAL(value, len, "HD576P")) > + mode->tv_mode = DRM_MODE_TV_NORM_HD576P; > + else if (TV_OPTION_EQUAL(value, len, "HD720P")) > + mode->tv_mode = DRM_MODE_TV_NORM_HD720P; > + else if (TV_OPTION_EQUAL(value, len, "HD1080I")) > + mode->tv_mode = DRM_MODE_TV_NORM_HD1080I; > + else > + return -EINVAL; > + > + return 0; > +} > + > static int drm_mode_parse_cmdline_options(const char *str, > bool freestanding, > const struct drm_connector *connector, > @@ -1746,6 +1820,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; > } > @@ -1771,11 +1848,12 @@ static int drm_mode_parse_cmdline_options(const char *str, > struct drm_named_mode { > const char *name; > const struct drm_display_mode *mode; > + unsigned int tv_mode; > }; > > static const struct drm_named_mode drm_named_modes[] = { > - { "NTSC", &drm_mode_480i, }, > - { "PAL", &drm_mode_576i, }, > + { "NTSC", &drm_mode_480i, DRM_MODE_TV_NORM_NTSC_M, }, > + { "PAL", &drm_mode_576i, DRM_MODE_TV_NORM_PAL_B, }, > }; > > static bool drm_mode_parse_cmdline_named_mode(const char *name, > @@ -1796,6 +1874,7 @@ static bool drm_mode_parse_cmdline_named_mode(const char *name, > cmdline_mode->xres = mode->mode->hdisplay; > cmdline_mode->yres = mode->mode->vdisplay; > cmdline_mode->interlace = !!(mode->mode->flags & DRM_MODE_FLAG_INTERLACE); > + cmdline_mode->tv_mode = mode->tv_mode; > cmdline_mode->specified = true; > > return true; > diff --git a/drivers/gpu/drm/tests/drm_cmdline_parser_test.c b/drivers/gpu/drm/tests/drm_cmdline_parser_test.c > index 59b29cdfdd35..d198717c8375 100644 > --- a/drivers/gpu/drm/tests/drm_cmdline_parser_test.c > +++ b/drivers/gpu/drm/tests/drm_cmdline_parser_test.c > @@ -885,6 +885,274 @@ 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_hd480i(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x480i,tv_mode=HD480I", > + &drm_mode_480i, > + DRM_MODE_TV_NORM_HD480I); > +} > + > +static const struct drm_display_mode drm_mode_480p = { > + DRM_MODE("720x480", DRM_MODE_TYPE_DRIVER, 27000, > + 720, 735, 743, 858, 0, 480, 490, 494, 525, 0, > + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC) > +}; > + > +static void drm_cmdline_test_tv_option_hd480p(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x480,tv_mode=HD480P", > + &drm_mode_480p, > + DRM_MODE_TV_NORM_HD480P); > +} > + > +static void drm_cmdline_test_tv_option_hd576i(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x576i,tv_mode=HD576I", > + &drm_mode_576i, > + DRM_MODE_TV_NORM_HD576I); > +} > + > +static const struct drm_display_mode drm_mode_576p = { > + DRM_MODE("720x576", DRM_MODE_TYPE_DRIVER, 27000, > + 720, 730, 738, 864, 0, 576, 581, 585, 625, 0, > + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC) > +}; > + > +static void drm_cmdline_test_tv_option_hd576p(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "720x576,tv_mode=HD576P", > + &drm_mode_576p, > + DRM_MODE_TV_NORM_HD576P); > +} > + > +static const struct drm_display_mode drm_mode_720p = { > + DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, > + 1280, 1349, 1357, 1650, 0, 720, 725, 730, 750, 0, > + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC) > +}; > + > +static void drm_cmdline_test_tv_option_hd720p(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "1280x720,tv_mode=HD720P", > + &drm_mode_720p, > + DRM_MODE_TV_NORM_HD720P); > +} > + > +static const struct drm_display_mode drm_mode_1080i = { > + DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 74250, > + 1920, 1961, 2049, 2200, 0, 1080, 1084, 1088, 1125, 0, > + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC > + | DRM_MODE_FLAG_INTERLACE) > +}; > + > +static void drm_cmdline_test_tv_option_hd1080i(struct kunit *test) > +{ > + drm_cmdline_test_tv_options(test, > + "1920x1080i,tv_mode=HD1080I", > + &drm_mode_1080i, > + DRM_MODE_TV_NORM_HD1080I); > +} > + > +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_480i, > + DRM_MODE_TV_NORM_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_480i, > + DRM_MODE_TV_NORM_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_480i, > + DRM_MODE_TV_NORM_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_576i, > + DRM_MODE_TV_NORM_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_576i, > + DRM_MODE_TV_NORM_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_576i, > + DRM_MODE_TV_NORM_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_576i, > + DRM_MODE_TV_NORM_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_576i, > + DRM_MODE_TV_NORM_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_576i, > + DRM_MODE_TV_NORM_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_480i, > + DRM_MODE_TV_NORM_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_576i, > + DRM_MODE_TV_NORM_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_576i, > + DRM_MODE_TV_NORM_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_576i, > + DRM_MODE_TV_NORM_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_576i, > + DRM_MODE_TV_NORM_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_576i, > + DRM_MODE_TV_NORM_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_576i, > + DRM_MODE_TV_NORM_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_576i, > + DRM_MODE_TV_NORM_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_576i, > + DRM_MODE_TV_NORM_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_576i, > + DRM_MODE_TV_NORM_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 +1315,33 @@ 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_hd480i), > + KUNIT_CASE(drm_cmdline_test_tv_option_hd480p), > + KUNIT_CASE(drm_cmdline_test_tv_option_hd576i), > + KUNIT_CASE(drm_cmdline_test_tv_option_hd576p), > + KUNIT_CASE(drm_cmdline_test_tv_option_hd720p), > + KUNIT_CASE(drm_cmdline_test_tv_option_hd1080i), > + 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 e535dca6376e..26a779e85785 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -1305,6 +1305,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_NORM_*. > + */ > + unsigned int tv_mode; > }; > > /** >
Hi Noralf, On Sat, Aug 20, 2022 at 10:18:47PM +0200, Noralf Trønnes wrote: > Den 29.07.2022 18.35, skrev Maxime Ripard: > > 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@cerno.tech> > > > > In the subject it says "tv_mode property", but the property is called > "tv norm", so the option should be tv_norm? Yeah... I don't know. mode is taken but it's obviously the best name. So I went with norm to avoid the (internal) conflict but I left mode for the user facing property. I'm not sure what's best here, or maybe we can pick another name entirely? Maxime
Hi Maxime & Noralf, W dniu 24.08.2022 o 17:45, Maxime Ripard pisze: > Hi Noralf, > > On Sat, Aug 20, 2022 at 10:18:47PM +0200, Noralf Trønnes wrote: >> Den 29.07.2022 18.35, skrev Maxime Ripard: >>> 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@cerno.tech> >>> >> >> In the subject it says "tv_mode property", but the property is called >> "tv norm", so the option should be tv_norm? > > Yeah... I don't know. mode is taken but it's obviously the best name. So > I went with norm to avoid the (internal) conflict but I left mode for > the user facing property. > > I'm not sure what's best here, or maybe we can pick another name entirely? I think "standard" and "system" are also fairly common names for this property. I once had an old multi-standard Sharp CRT TV that allowed manually selecting the expected color encoding (PAL/SECAM/NTSC/NTSC-443), and the relevant button on the remote was labeled "SYSTEM". V4L2 calls this "standard", see include/uapi/linux/videodev2.h, compare with v4l2_std_id, V4L2_STD_* etc. BTW, maybe reusing those V4L2 constants, or aligning the numerical values, would make some sense? > > Maxime Best regards, Mateusz Kwiatkowski
Den 24.08.2022 17.45, skrev Maxime Ripard: > Hi Noralf, > > On Sat, Aug 20, 2022 at 10:18:47PM +0200, Noralf Trønnes wrote: >> Den 29.07.2022 18.35, skrev Maxime Ripard: >>> 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@cerno.tech> >>> >> >> In the subject it says "tv_mode property", but the property is called >> "tv norm", so the option should be tv_norm? > > Yeah... I don't know. mode is taken but it's obviously the best name. So > I went with norm to avoid the (internal) conflict but I left mode for > the user facing property. > > I'm not sure what's best here, or maybe we can pick another name entirely? > Why not just call it "tv mode" or even better "TV Mode". The state member can be called tv_mode, but the mode_config member will need a temporary name until the "mode" property is removed. tv_tv_mode or maybe connector_tv_mode? But IMO, if there are no users of this property, adding it should have a drm maintainers ack. Noralf.
On Thu, Aug 25, 2022 at 02:41:27PM +0200, Noralf Trønnes wrote: > Den 24.08.2022 17.45, skrev Maxime Ripard: > > Hi Noralf, > > > > On Sat, Aug 20, 2022 at 10:18:47PM +0200, Noralf Trønnes wrote: > >> Den 29.07.2022 18.35, skrev Maxime Ripard: > >>> 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@cerno.tech> > >>> > >> > >> In the subject it says "tv_mode property", but the property is called > >> "tv norm", so the option should be tv_norm? > > > > Yeah... I don't know. mode is taken but it's obviously the best name. So > > I went with norm to avoid the (internal) conflict but I left mode for > > the user facing property. > > > > I'm not sure what's best here, or maybe we can pick another name entirely? > > > > Why not just call it "tv mode" or even better "TV Mode". The state > member can be called tv_mode, but the mode_config member will need a > temporary name until the "mode" property is removed. tv_tv_mode or maybe > connector_tv_mode? Yeah, that seems like a good idea. Would legacy_tv_mode work for you? > But IMO, if there are no users of this property, adding it should have a > drm maintainers ack. Yep, of course :) Maxime
Den 26.08.2022 08.46, skrev Maxime Ripard: > On Thu, Aug 25, 2022 at 02:41:27PM +0200, Noralf Trønnes wrote: >> Den 24.08.2022 17.45, skrev Maxime Ripard: >>> Hi Noralf, >>> >>> On Sat, Aug 20, 2022 at 10:18:47PM +0200, Noralf Trønnes wrote: >>>> Den 29.07.2022 18.35, skrev Maxime Ripard: >>>>> 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@cerno.tech> >>>>> >>>> >>>> In the subject it says "tv_mode property", but the property is called >>>> "tv norm", so the option should be tv_norm? >>> >>> Yeah... I don't know. mode is taken but it's obviously the best name. So >>> I went with norm to avoid the (internal) conflict but I left mode for >>> the user facing property. >>> >>> I'm not sure what's best here, or maybe we can pick another name entirely? >>> >> >> Why not just call it "tv mode" or even better "TV Mode". The state >> member can be called tv_mode, but the mode_config member will need a >> temporary name until the "mode" property is removed. tv_tv_mode or maybe >> connector_tv_mode? > > Yeah, that seems like a good idea. Would legacy_tv_mode work for you? > Oh yeah, renaming the "mode" property, that works fine. Noralf.
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c index fce5569bd66a..49da8d8236f5 100644 --- a/drivers/gpu/drm/drm_atomic_state_helper.c +++ b/drivers/gpu/drm/drm_atomic_state_helper.c @@ -490,6 +490,7 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_tv_margins_reset); void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector) { struct drm_device *dev = connector->dev; + struct drm_cmdline_mode *cmdline = &connector->cmdline_mode; struct drm_connector_state *state = connector->state; struct drm_property *prop; uint64_t val; @@ -500,6 +501,9 @@ void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector) prop, &val)) state->tv.norm = val; + if (cmdline->tv_mode) + state->tv.norm = cmdline->tv_mode; + prop = dev->mode_config.tv_select_subconnector_property; if (prop) if (!drm_object_property_get_default_value(&connector->base, diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index ecb2e83cf860..3634ac9f787d 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1677,6 +1677,80 @@ static int drm_mode_parse_panel_orientation(const char *delim, return 0; } +#define TV_OPTION_EQUAL(value, len, option) \ + ((strlen(option) == len) && !strncmp(value, option, len)) + +static int drm_mode_parse_tv_mode(const char *delim, + struct drm_cmdline_mode *mode) +{ + const char *value; + unsigned int len; + + if (*delim != '=') + return -EINVAL; + + value = delim + 1; + delim = strchr(value, ','); + if (!delim) + delim = value + strlen(value); + + len = delim - value; + if (TV_OPTION_EQUAL(value, len, "NTSC-443")) + mode->tv_mode = DRM_MODE_TV_NORM_NTSC_443; + else if (TV_OPTION_EQUAL(value, len, "NTSC-J")) + mode->tv_mode = DRM_MODE_TV_NORM_NTSC_J; + else if (TV_OPTION_EQUAL(value, len, "NTSC-M")) + mode->tv_mode = DRM_MODE_TV_NORM_NTSC_M; + else if (TV_OPTION_EQUAL(value, len, "PAL-60")) + mode->tv_mode = DRM_MODE_TV_NORM_PAL_60; + else if (TV_OPTION_EQUAL(value, len, "PAL-B")) + mode->tv_mode = DRM_MODE_TV_NORM_PAL_B; + else if (TV_OPTION_EQUAL(value, len, "PAL-D")) + mode->tv_mode = DRM_MODE_TV_NORM_PAL_D; + else if (TV_OPTION_EQUAL(value, len, "PAL-G")) + mode->tv_mode = DRM_MODE_TV_NORM_PAL_G; + else if (TV_OPTION_EQUAL(value, len, "PAL-H")) + mode->tv_mode = DRM_MODE_TV_NORM_PAL_H; + else if (TV_OPTION_EQUAL(value, len, "PAL-I")) + mode->tv_mode = DRM_MODE_TV_NORM_PAL_I; + else if (TV_OPTION_EQUAL(value, len, "PAL-M")) + mode->tv_mode = DRM_MODE_TV_NORM_PAL_M; + else if (TV_OPTION_EQUAL(value, len, "PAL-N")) + mode->tv_mode = DRM_MODE_TV_NORM_PAL_N; + else if (TV_OPTION_EQUAL(value, len, "PAL-NC")) + mode->tv_mode = DRM_MODE_TV_NORM_PAL_NC; + else if (TV_OPTION_EQUAL(value, len, "SECAM-60")) + mode->tv_mode = DRM_MODE_TV_NORM_SECAM_60; + else if (TV_OPTION_EQUAL(value, len, "SECAM-B")) + mode->tv_mode = DRM_MODE_TV_NORM_SECAM_B; + else if (TV_OPTION_EQUAL(value, len, "SECAM-D")) + mode->tv_mode = DRM_MODE_TV_NORM_SECAM_D; + else if (TV_OPTION_EQUAL(value, len, "SECAM-G")) + mode->tv_mode = DRM_MODE_TV_NORM_SECAM_G; + else if (TV_OPTION_EQUAL(value, len, "SECAM-K")) + mode->tv_mode = DRM_MODE_TV_NORM_SECAM_K; + else if (TV_OPTION_EQUAL(value, len, "SECAM-K1")) + mode->tv_mode = DRM_MODE_TV_NORM_SECAM_K1; + else if (TV_OPTION_EQUAL(value, len, "SECAM-L")) + mode->tv_mode = DRM_MODE_TV_NORM_SECAM_L; + else if (TV_OPTION_EQUAL(value, len, "HD480I")) + mode->tv_mode = DRM_MODE_TV_NORM_HD480I; + else if (TV_OPTION_EQUAL(value, len, "HD480P")) + mode->tv_mode = DRM_MODE_TV_NORM_HD480P; + else if (TV_OPTION_EQUAL(value, len, "HD576I")) + mode->tv_mode = DRM_MODE_TV_NORM_HD576I; + else if (TV_OPTION_EQUAL(value, len, "HD576P")) + mode->tv_mode = DRM_MODE_TV_NORM_HD576P; + else if (TV_OPTION_EQUAL(value, len, "HD720P")) + mode->tv_mode = DRM_MODE_TV_NORM_HD720P; + else if (TV_OPTION_EQUAL(value, len, "HD1080I")) + mode->tv_mode = DRM_MODE_TV_NORM_HD1080I; + else + return -EINVAL; + + return 0; +} + static int drm_mode_parse_cmdline_options(const char *str, bool freestanding, const struct drm_connector *connector, @@ -1746,6 +1820,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; } @@ -1771,11 +1848,12 @@ static int drm_mode_parse_cmdline_options(const char *str, struct drm_named_mode { const char *name; const struct drm_display_mode *mode; + unsigned int tv_mode; }; static const struct drm_named_mode drm_named_modes[] = { - { "NTSC", &drm_mode_480i, }, - { "PAL", &drm_mode_576i, }, + { "NTSC", &drm_mode_480i, DRM_MODE_TV_NORM_NTSC_M, }, + { "PAL", &drm_mode_576i, DRM_MODE_TV_NORM_PAL_B, }, }; static bool drm_mode_parse_cmdline_named_mode(const char *name, @@ -1796,6 +1874,7 @@ static bool drm_mode_parse_cmdline_named_mode(const char *name, cmdline_mode->xres = mode->mode->hdisplay; cmdline_mode->yres = mode->mode->vdisplay; cmdline_mode->interlace = !!(mode->mode->flags & DRM_MODE_FLAG_INTERLACE); + cmdline_mode->tv_mode = mode->tv_mode; cmdline_mode->specified = true; return true; diff --git a/drivers/gpu/drm/tests/drm_cmdline_parser_test.c b/drivers/gpu/drm/tests/drm_cmdline_parser_test.c index 59b29cdfdd35..d198717c8375 100644 --- a/drivers/gpu/drm/tests/drm_cmdline_parser_test.c +++ b/drivers/gpu/drm/tests/drm_cmdline_parser_test.c @@ -885,6 +885,274 @@ 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_hd480i(struct kunit *test) +{ + drm_cmdline_test_tv_options(test, + "720x480i,tv_mode=HD480I", + &drm_mode_480i, + DRM_MODE_TV_NORM_HD480I); +} + +static const struct drm_display_mode drm_mode_480p = { + DRM_MODE("720x480", DRM_MODE_TYPE_DRIVER, 27000, + 720, 735, 743, 858, 0, 480, 490, 494, 525, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC) +}; + +static void drm_cmdline_test_tv_option_hd480p(struct kunit *test) +{ + drm_cmdline_test_tv_options(test, + "720x480,tv_mode=HD480P", + &drm_mode_480p, + DRM_MODE_TV_NORM_HD480P); +} + +static void drm_cmdline_test_tv_option_hd576i(struct kunit *test) +{ + drm_cmdline_test_tv_options(test, + "720x576i,tv_mode=HD576I", + &drm_mode_576i, + DRM_MODE_TV_NORM_HD576I); +} + +static const struct drm_display_mode drm_mode_576p = { + DRM_MODE("720x576", DRM_MODE_TYPE_DRIVER, 27000, + 720, 730, 738, 864, 0, 576, 581, 585, 625, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC) +}; + +static void drm_cmdline_test_tv_option_hd576p(struct kunit *test) +{ + drm_cmdline_test_tv_options(test, + "720x576,tv_mode=HD576P", + &drm_mode_576p, + DRM_MODE_TV_NORM_HD576P); +} + +static const struct drm_display_mode drm_mode_720p = { + DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, + 1280, 1349, 1357, 1650, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC) +}; + +static void drm_cmdline_test_tv_option_hd720p(struct kunit *test) +{ + drm_cmdline_test_tv_options(test, + "1280x720,tv_mode=HD720P", + &drm_mode_720p, + DRM_MODE_TV_NORM_HD720P); +} + +static const struct drm_display_mode drm_mode_1080i = { + DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 74250, + 1920, 1961, 2049, 2200, 0, 1080, 1084, 1088, 1125, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC + | DRM_MODE_FLAG_INTERLACE) +}; + +static void drm_cmdline_test_tv_option_hd1080i(struct kunit *test) +{ + drm_cmdline_test_tv_options(test, + "1920x1080i,tv_mode=HD1080I", + &drm_mode_1080i, + DRM_MODE_TV_NORM_HD1080I); +} + +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_480i, + DRM_MODE_TV_NORM_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_480i, + DRM_MODE_TV_NORM_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_480i, + DRM_MODE_TV_NORM_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_576i, + DRM_MODE_TV_NORM_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_576i, + DRM_MODE_TV_NORM_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_576i, + DRM_MODE_TV_NORM_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_576i, + DRM_MODE_TV_NORM_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_576i, + DRM_MODE_TV_NORM_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_576i, + DRM_MODE_TV_NORM_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_480i, + DRM_MODE_TV_NORM_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_576i, + DRM_MODE_TV_NORM_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_576i, + DRM_MODE_TV_NORM_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_576i, + DRM_MODE_TV_NORM_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_576i, + DRM_MODE_TV_NORM_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_576i, + DRM_MODE_TV_NORM_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_576i, + DRM_MODE_TV_NORM_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_576i, + DRM_MODE_TV_NORM_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_576i, + DRM_MODE_TV_NORM_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_576i, + DRM_MODE_TV_NORM_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 +1315,33 @@ 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_hd480i), + KUNIT_CASE(drm_cmdline_test_tv_option_hd480p), + KUNIT_CASE(drm_cmdline_test_tv_option_hd576i), + KUNIT_CASE(drm_cmdline_test_tv_option_hd576p), + KUNIT_CASE(drm_cmdline_test_tv_option_hd720p), + KUNIT_CASE(drm_cmdline_test_tv_option_hd1080i), + 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 e535dca6376e..26a779e85785 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1305,6 +1305,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_NORM_*. + */ + unsigned int tv_mode; }; /**
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@cerno.tech>