| Message ID | 20220728-rpi-analog-tv-properties-v7-14-7072a478c6b3@cerno.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Analog TV Improvements | expand |
Den 07.11.2022 15.16, skrev Maxime Ripard: > The framework will get the drm_display_mode from the drm_cmdline_mode it > got by parsing the video command line argument by calling > drm_connector_pick_cmdline_mode(). > > The heavy lifting will then be done by the drm_mode_create_from_cmdline_mode() > function. > > In the case of the named modes though, there's no real code to make that > translation and we rely on the drivers to guess which actual display mode > we meant. > > Let's modify drm_mode_create_from_cmdline_mode() to properly generate the > drm_display_mode we mean when passing a named mode. > > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > > --- > Changes in v7: > - Use tv_mode_specified in drm_mode_parse_command_line_for_connector > > Changes in v6: > - Fix get_modes to return 0 instead of an error code > - Rename the tests to follow the DRM test naming convention > > Changes in v5: > - Switched to KUNIT_ASSERT_NOT_NULL > --- > drivers/gpu/drm/drm_modes.c | 34 ++++++++++- > drivers/gpu/drm/tests/drm_client_modeset_test.c | 77 ++++++++++++++++++++++++- > 2 files changed, 109 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index dc037f7ceb37..49441cabdd9d 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -2497,6 +2497,36 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, > } > EXPORT_SYMBOL(drm_mode_parse_command_line_for_connector); > > +static struct drm_display_mode *drm_named_mode(struct drm_device *dev, > + struct drm_cmdline_mode *cmd) > +{ > + struct drm_display_mode *mode; > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(drm_named_modes); i++) { > + const struct drm_named_mode *named_mode = &drm_named_modes[i]; > + > + if (strcmp(cmd->name, named_mode->name)) > + continue; > + > + if (!cmd->tv_mode_specified) > + continue; Only a named mode will set cmd->name, so is this check necessary? > + > + mode = drm_analog_tv_mode(dev, > + named_mode->tv_mode, > + named_mode->pixel_clock_khz * 1000, > + named_mode->xres, > + named_mode->yres, > + named_mode->flags & DRM_MODE_FLAG_INTERLACE); > + if (!mode) > + return NULL; > + > + return mode; You can just return the result from drm_analog_tv_mode() directly. With those considered: Reviewed-by: Noralf Trønnes <noralf@tronnes.org> > + } > + > + return NULL; > +} > + > /** > * drm_mode_create_from_cmdline_mode - convert a command line modeline into a DRM display mode > * @dev: DRM device to create the new mode for > @@ -2514,7 +2544,9 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev, > if (cmd->xres == 0 || cmd->yres == 0) > return NULL; > > - if (cmd->cvt) > + if (strlen(cmd->name)) > + mode = drm_named_mode(dev, cmd); > + else if (cmd->cvt) > mode = drm_cvt_mode(dev, > cmd->xres, cmd->yres, > cmd->refresh_specified ? cmd->refresh : 60, > diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c b/drivers/gpu/drm/tests/drm_client_modeset_test.c > index 3aa1acfe75df..fdfe9e20702e 100644 > --- a/drivers/gpu/drm/tests/drm_client_modeset_test.c > +++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c > @@ -21,7 +21,26 @@ struct drm_client_modeset_test_priv { > > static int drm_client_modeset_connector_get_modes(struct drm_connector *connector) > { > - return drm_add_modes_noedid(connector, 1920, 1200); > + struct drm_display_mode *mode; > + int count; > + > + count = drm_add_modes_noedid(connector, 1920, 1200); > + > + mode = drm_mode_analog_ntsc_480i(connector->dev); > + if (!mode) > + return count; > + > + drm_mode_probed_add(connector, mode); > + count += 1; > + > + mode = drm_mode_analog_pal_576i(connector->dev); > + if (!mode) > + return count; > + > + drm_mode_probed_add(connector, mode); > + count += 1; > + > + return count; > } > > static const struct drm_connector_helper_funcs drm_client_modeset_connector_helper_funcs = { > @@ -52,6 +71,9 @@ static int drm_client_modeset_test_init(struct kunit *test) > > drm_connector_helper_add(&priv->connector, &drm_client_modeset_connector_helper_funcs); > > + priv->connector.interlace_allowed = true; > + priv->connector.doublescan_allowed = true; > + > return 0; > > } > @@ -85,9 +107,62 @@ static void drm_test_pick_cmdline_res_1920_1080_60(struct kunit *test) > KUNIT_EXPECT_TRUE(test, drm_mode_equal(expected_mode, mode)); > } > > +static void drm_test_pick_cmdline_named_ntsc(struct kunit *test) > +{ > + struct drm_client_modeset_test_priv *priv = test->priv; > + struct drm_device *drm = priv->drm; > + struct drm_connector *connector = &priv->connector; > + struct drm_cmdline_mode *cmdline_mode = &connector->cmdline_mode; > + struct drm_display_mode *mode; > + const char *cmdline = "NTSC"; > + int ret; > + > + KUNIT_ASSERT_TRUE(test, > + drm_mode_parse_command_line_for_connector(cmdline, > + connector, > + cmdline_mode)); > + > + mutex_lock(&drm->mode_config.mutex); > + ret = drm_helper_probe_single_connector_modes(connector, 1920, 1080); > + mutex_unlock(&drm->mode_config.mutex); > + KUNIT_ASSERT_GT(test, ret, 0); > + > + mode = drm_connector_pick_cmdline_mode(connector); > + KUNIT_ASSERT_NOT_NULL(test, mode); > + > + KUNIT_EXPECT_TRUE(test, drm_mode_equal(drm_mode_analog_ntsc_480i(drm), mode)); > +} > + > +static void drm_test_pick_cmdline_named_pal(struct kunit *test) > +{ > + struct drm_client_modeset_test_priv *priv = test->priv; > + struct drm_device *drm = priv->drm; > + struct drm_connector *connector = &priv->connector; > + struct drm_cmdline_mode *cmdline_mode = &connector->cmdline_mode; > + struct drm_display_mode *mode; > + const char *cmdline = "PAL"; > + int ret; > + > + KUNIT_ASSERT_TRUE(test, > + drm_mode_parse_command_line_for_connector(cmdline, > + connector, > + cmdline_mode)); > + > + mutex_lock(&drm->mode_config.mutex); > + ret = drm_helper_probe_single_connector_modes(connector, 1920, 1080); > + mutex_unlock(&drm->mode_config.mutex); > + KUNIT_ASSERT_GT(test, ret, 0); > + > + mode = drm_connector_pick_cmdline_mode(connector); > + KUNIT_ASSERT_NOT_NULL(test, mode); > + > + KUNIT_EXPECT_TRUE(test, drm_mode_equal(drm_mode_analog_pal_576i(drm), mode)); > +} > > static struct kunit_case drm_test_pick_cmdline_tests[] = { > KUNIT_CASE(drm_test_pick_cmdline_res_1920_1080_60), > + KUNIT_CASE(drm_test_pick_cmdline_named_ntsc), > + KUNIT_CASE(drm_test_pick_cmdline_named_pal), > {} > }; > >
Den 07.11.2022 18.49, skrev Noralf Trønnes: > > > Den 07.11.2022 15.16, skrev Maxime Ripard: >> The framework will get the drm_display_mode from the drm_cmdline_mode it >> got by parsing the video command line argument by calling >> drm_connector_pick_cmdline_mode(). >> >> The heavy lifting will then be done by the drm_mode_create_from_cmdline_mode() >> function. >> >> In the case of the named modes though, there's no real code to make that >> translation and we rely on the drivers to guess which actual display mode >> we meant. >> >> Let's modify drm_mode_create_from_cmdline_mode() to properly generate the >> drm_display_mode we mean when passing a named mode. >> >> Signed-off-by: Maxime Ripard <maxime@cerno.tech> >> >> --- >> Changes in v7: >> - Use tv_mode_specified in drm_mode_parse_command_line_for_connector >> >> Changes in v6: >> - Fix get_modes to return 0 instead of an error code >> - Rename the tests to follow the DRM test naming convention >> >> Changes in v5: >> - Switched to KUNIT_ASSERT_NOT_NULL >> --- >> drivers/gpu/drm/drm_modes.c | 34 ++++++++++- >> drivers/gpu/drm/tests/drm_client_modeset_test.c | 77 ++++++++++++++++++++++++- >> 2 files changed, 109 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c >> index dc037f7ceb37..49441cabdd9d 100644 >> --- a/drivers/gpu/drm/drm_modes.c >> +++ b/drivers/gpu/drm/drm_modes.c >> @@ -2497,6 +2497,36 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, >> } >> EXPORT_SYMBOL(drm_mode_parse_command_line_for_connector); >> >> +static struct drm_display_mode *drm_named_mode(struct drm_device *dev, >> + struct drm_cmdline_mode *cmd) >> +{ >> + struct drm_display_mode *mode; >> + unsigned int i; >> + >> + for (i = 0; i < ARRAY_SIZE(drm_named_modes); i++) { >> + const struct drm_named_mode *named_mode = &drm_named_modes[i]; >> + >> + if (strcmp(cmd->name, named_mode->name)) >> + continue; >> + >> + if (!cmd->tv_mode_specified) >> + continue; > > Only a named mode will set cmd->name, so is this check necessary? > >> + >> + mode = drm_analog_tv_mode(dev, >> + named_mode->tv_mode, >> + named_mode->pixel_clock_khz * 1000, >> + named_mode->xres, >> + named_mode->yres, >> + named_mode->flags & DRM_MODE_FLAG_INTERLACE); >> + if (!mode) >> + return NULL; >> + >> + return mode; > > You can just return the result from drm_analog_tv_mode() directly. > > With those considered: > > Reviewed-by: Noralf Trønnes <noralf@tronnes.org> > I forgot one thing, shouldn't the named mode test in drm_connector_pick_cmdline_mode() be removed now that we have proper modes? Noralf. >> + } >> + >> + return NULL; >> +} >> + >> /** >> * drm_mode_create_from_cmdline_mode - convert a command line modeline into a DRM display mode >> * @dev: DRM device to create the new mode for >> @@ -2514,7 +2544,9 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev, >> if (cmd->xres == 0 || cmd->yres == 0) >> return NULL; >> >> - if (cmd->cvt) >> + if (strlen(cmd->name)) >> + mode = drm_named_mode(dev, cmd); >> + else if (cmd->cvt) >> mode = drm_cvt_mode(dev, >> cmd->xres, cmd->yres, >> cmd->refresh_specified ? cmd->refresh : 60, >> diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c b/drivers/gpu/drm/tests/drm_client_modeset_test.c >> index 3aa1acfe75df..fdfe9e20702e 100644 >> --- a/drivers/gpu/drm/tests/drm_client_modeset_test.c >> +++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c >> @@ -21,7 +21,26 @@ struct drm_client_modeset_test_priv { >> >> static int drm_client_modeset_connector_get_modes(struct drm_connector *connector) >> { >> - return drm_add_modes_noedid(connector, 1920, 1200); >> + struct drm_display_mode *mode; >> + int count; >> + >> + count = drm_add_modes_noedid(connector, 1920, 1200); >> + >> + mode = drm_mode_analog_ntsc_480i(connector->dev); >> + if (!mode) >> + return count; >> + >> + drm_mode_probed_add(connector, mode); >> + count += 1; >> + >> + mode = drm_mode_analog_pal_576i(connector->dev); >> + if (!mode) >> + return count; >> + >> + drm_mode_probed_add(connector, mode); >> + count += 1; >> + >> + return count; >> } >> >> static const struct drm_connector_helper_funcs drm_client_modeset_connector_helper_funcs = { >> @@ -52,6 +71,9 @@ static int drm_client_modeset_test_init(struct kunit *test) >> >> drm_connector_helper_add(&priv->connector, &drm_client_modeset_connector_helper_funcs); >> >> + priv->connector.interlace_allowed = true; >> + priv->connector.doublescan_allowed = true; >> + >> return 0; >> >> } >> @@ -85,9 +107,62 @@ static void drm_test_pick_cmdline_res_1920_1080_60(struct kunit *test) >> KUNIT_EXPECT_TRUE(test, drm_mode_equal(expected_mode, mode)); >> } >> >> +static void drm_test_pick_cmdline_named_ntsc(struct kunit *test) >> +{ >> + struct drm_client_modeset_test_priv *priv = test->priv; >> + struct drm_device *drm = priv->drm; >> + struct drm_connector *connector = &priv->connector; >> + struct drm_cmdline_mode *cmdline_mode = &connector->cmdline_mode; >> + struct drm_display_mode *mode; >> + const char *cmdline = "NTSC"; >> + int ret; >> + >> + KUNIT_ASSERT_TRUE(test, >> + drm_mode_parse_command_line_for_connector(cmdline, >> + connector, >> + cmdline_mode)); >> + >> + mutex_lock(&drm->mode_config.mutex); >> + ret = drm_helper_probe_single_connector_modes(connector, 1920, 1080); >> + mutex_unlock(&drm->mode_config.mutex); >> + KUNIT_ASSERT_GT(test, ret, 0); >> + >> + mode = drm_connector_pick_cmdline_mode(connector); >> + KUNIT_ASSERT_NOT_NULL(test, mode); >> + >> + KUNIT_EXPECT_TRUE(test, drm_mode_equal(drm_mode_analog_ntsc_480i(drm), mode)); >> +} >> + >> +static void drm_test_pick_cmdline_named_pal(struct kunit *test) >> +{ >> + struct drm_client_modeset_test_priv *priv = test->priv; >> + struct drm_device *drm = priv->drm; >> + struct drm_connector *connector = &priv->connector; >> + struct drm_cmdline_mode *cmdline_mode = &connector->cmdline_mode; >> + struct drm_display_mode *mode; >> + const char *cmdline = "PAL"; >> + int ret; >> + >> + KUNIT_ASSERT_TRUE(test, >> + drm_mode_parse_command_line_for_connector(cmdline, >> + connector, >> + cmdline_mode)); >> + >> + mutex_lock(&drm->mode_config.mutex); >> + ret = drm_helper_probe_single_connector_modes(connector, 1920, 1080); >> + mutex_unlock(&drm->mode_config.mutex); >> + KUNIT_ASSERT_GT(test, ret, 0); >> + >> + mode = drm_connector_pick_cmdline_mode(connector); >> + KUNIT_ASSERT_NOT_NULL(test, mode); >> + >> + KUNIT_EXPECT_TRUE(test, drm_mode_equal(drm_mode_analog_pal_576i(drm), mode)); >> +} >> >> static struct kunit_case drm_test_pick_cmdline_tests[] = { >> KUNIT_CASE(drm_test_pick_cmdline_res_1920_1080_60), >> + KUNIT_CASE(drm_test_pick_cmdline_named_ntsc), >> + KUNIT_CASE(drm_test_pick_cmdline_named_pal), >> {} >> }; >> >>
Hi, On Mon, Nov 07, 2022 at 06:49:57PM +0100, Noralf Trønnes wrote: > Den 07.11.2022 15.16, skrev Maxime Ripard: > > The framework will get the drm_display_mode from the drm_cmdline_mode it > > got by parsing the video command line argument by calling > > drm_connector_pick_cmdline_mode(). > > > > The heavy lifting will then be done by the drm_mode_create_from_cmdline_mode() > > function. > > > > In the case of the named modes though, there's no real code to make that > > translation and we rely on the drivers to guess which actual display mode > > we meant. > > > > Let's modify drm_mode_create_from_cmdline_mode() to properly generate the > > drm_display_mode we mean when passing a named mode. > > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > > > > --- > > Changes in v7: > > - Use tv_mode_specified in drm_mode_parse_command_line_for_connector > > > > Changes in v6: > > - Fix get_modes to return 0 instead of an error code > > - Rename the tests to follow the DRM test naming convention > > > > Changes in v5: > > - Switched to KUNIT_ASSERT_NOT_NULL > > --- > > drivers/gpu/drm/drm_modes.c | 34 ++++++++++- > > drivers/gpu/drm/tests/drm_client_modeset_test.c | 77 ++++++++++++++++++++++++- > > 2 files changed, 109 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > > index dc037f7ceb37..49441cabdd9d 100644 > > --- a/drivers/gpu/drm/drm_modes.c > > +++ b/drivers/gpu/drm/drm_modes.c > > @@ -2497,6 +2497,36 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, > > } > > EXPORT_SYMBOL(drm_mode_parse_command_line_for_connector); > > > > +static struct drm_display_mode *drm_named_mode(struct drm_device *dev, > > + struct drm_cmdline_mode *cmd) > > +{ > > + struct drm_display_mode *mode; > > + unsigned int i; > > + > > + for (i = 0; i < ARRAY_SIZE(drm_named_modes); i++) { > > + const struct drm_named_mode *named_mode = &drm_named_modes[i]; > > + > > + if (strcmp(cmd->name, named_mode->name)) > > + continue; > > + > > + if (!cmd->tv_mode_specified) > > + continue; > > Only a named mode will set cmd->name, so is this check necessary? Yeah, but (and even though it's not the case at the moment) there's no implication that a named mode will be about TV. We could use it for VGA/XGA/etc just as well, in which case we wouldn't have tv_mode_specified. Maxime
On Tue, Nov 08, 2022 at 10:40:07AM +0100, Noralf Trønnes wrote: > > > Den 07.11.2022 18.49, skrev Noralf Trønnes: > > > > > > Den 07.11.2022 15.16, skrev Maxime Ripard: > >> The framework will get the drm_display_mode from the drm_cmdline_mode it > >> got by parsing the video command line argument by calling > >> drm_connector_pick_cmdline_mode(). > >> > >> The heavy lifting will then be done by the drm_mode_create_from_cmdline_mode() > >> function. > >> > >> In the case of the named modes though, there's no real code to make that > >> translation and we rely on the drivers to guess which actual display mode > >> we meant. > >> > >> Let's modify drm_mode_create_from_cmdline_mode() to properly generate the > >> drm_display_mode we mean when passing a named mode. > >> > >> Signed-off-by: Maxime Ripard <maxime@cerno.tech> > >> > >> --- > >> Changes in v7: > >> - Use tv_mode_specified in drm_mode_parse_command_line_for_connector > >> > >> Changes in v6: > >> - Fix get_modes to return 0 instead of an error code > >> - Rename the tests to follow the DRM test naming convention > >> > >> Changes in v5: > >> - Switched to KUNIT_ASSERT_NOT_NULL > >> --- > >> drivers/gpu/drm/drm_modes.c | 34 ++++++++++- > >> drivers/gpu/drm/tests/drm_client_modeset_test.c | 77 ++++++++++++++++++++++++- > >> 2 files changed, 109 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > >> index dc037f7ceb37..49441cabdd9d 100644 > >> --- a/drivers/gpu/drm/drm_modes.c > >> +++ b/drivers/gpu/drm/drm_modes.c > >> @@ -2497,6 +2497,36 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, > >> } > >> EXPORT_SYMBOL(drm_mode_parse_command_line_for_connector); > >> > >> +static struct drm_display_mode *drm_named_mode(struct drm_device *dev, > >> + struct drm_cmdline_mode *cmd) > >> +{ > >> + struct drm_display_mode *mode; > >> + unsigned int i; > >> + > >> + for (i = 0; i < ARRAY_SIZE(drm_named_modes); i++) { > >> + const struct drm_named_mode *named_mode = &drm_named_modes[i]; > >> + > >> + if (strcmp(cmd->name, named_mode->name)) > >> + continue; > >> + > >> + if (!cmd->tv_mode_specified) > >> + continue; > > > > Only a named mode will set cmd->name, so is this check necessary? > > > >> + > >> + mode = drm_analog_tv_mode(dev, > >> + named_mode->tv_mode, > >> + named_mode->pixel_clock_khz * 1000, > >> + named_mode->xres, > >> + named_mode->yres, > >> + named_mode->flags & DRM_MODE_FLAG_INTERLACE); > >> + if (!mode) > >> + return NULL; > >> + > >> + return mode; > > > > You can just return the result from drm_analog_tv_mode() directly. > > > > With those considered: > > > > Reviewed-by: Noralf Trønnes <noralf@tronnes.org> > > > > I forgot one thing, shouldn't the named mode test in > drm_connector_pick_cmdline_mode() be removed now that we have proper modes? Good catch, I've fixed it Thanks! Maxime
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index dc037f7ceb37..49441cabdd9d 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -2497,6 +2497,36 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, } EXPORT_SYMBOL(drm_mode_parse_command_line_for_connector); +static struct drm_display_mode *drm_named_mode(struct drm_device *dev, + struct drm_cmdline_mode *cmd) +{ + struct drm_display_mode *mode; + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(drm_named_modes); i++) { + const struct drm_named_mode *named_mode = &drm_named_modes[i]; + + if (strcmp(cmd->name, named_mode->name)) + continue; + + if (!cmd->tv_mode_specified) + continue; + + mode = drm_analog_tv_mode(dev, + named_mode->tv_mode, + named_mode->pixel_clock_khz * 1000, + named_mode->xres, + named_mode->yres, + named_mode->flags & DRM_MODE_FLAG_INTERLACE); + if (!mode) + return NULL; + + return mode; + } + + return NULL; +} + /** * drm_mode_create_from_cmdline_mode - convert a command line modeline into a DRM display mode * @dev: DRM device to create the new mode for @@ -2514,7 +2544,9 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev, if (cmd->xres == 0 || cmd->yres == 0) return NULL; - if (cmd->cvt) + if (strlen(cmd->name)) + mode = drm_named_mode(dev, cmd); + else if (cmd->cvt) mode = drm_cvt_mode(dev, cmd->xres, cmd->yres, cmd->refresh_specified ? cmd->refresh : 60, diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c b/drivers/gpu/drm/tests/drm_client_modeset_test.c index 3aa1acfe75df..fdfe9e20702e 100644 --- a/drivers/gpu/drm/tests/drm_client_modeset_test.c +++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c @@ -21,7 +21,26 @@ struct drm_client_modeset_test_priv { static int drm_client_modeset_connector_get_modes(struct drm_connector *connector) { - return drm_add_modes_noedid(connector, 1920, 1200); + struct drm_display_mode *mode; + int count; + + count = drm_add_modes_noedid(connector, 1920, 1200); + + mode = drm_mode_analog_ntsc_480i(connector->dev); + if (!mode) + return count; + + drm_mode_probed_add(connector, mode); + count += 1; + + mode = drm_mode_analog_pal_576i(connector->dev); + if (!mode) + return count; + + drm_mode_probed_add(connector, mode); + count += 1; + + return count; } static const struct drm_connector_helper_funcs drm_client_modeset_connector_helper_funcs = { @@ -52,6 +71,9 @@ static int drm_client_modeset_test_init(struct kunit *test) drm_connector_helper_add(&priv->connector, &drm_client_modeset_connector_helper_funcs); + priv->connector.interlace_allowed = true; + priv->connector.doublescan_allowed = true; + return 0; } @@ -85,9 +107,62 @@ static void drm_test_pick_cmdline_res_1920_1080_60(struct kunit *test) KUNIT_EXPECT_TRUE(test, drm_mode_equal(expected_mode, mode)); } +static void drm_test_pick_cmdline_named_ntsc(struct kunit *test) +{ + struct drm_client_modeset_test_priv *priv = test->priv; + struct drm_device *drm = priv->drm; + struct drm_connector *connector = &priv->connector; + struct drm_cmdline_mode *cmdline_mode = &connector->cmdline_mode; + struct drm_display_mode *mode; + const char *cmdline = "NTSC"; + int ret; + + KUNIT_ASSERT_TRUE(test, + drm_mode_parse_command_line_for_connector(cmdline, + connector, + cmdline_mode)); + + mutex_lock(&drm->mode_config.mutex); + ret = drm_helper_probe_single_connector_modes(connector, 1920, 1080); + mutex_unlock(&drm->mode_config.mutex); + KUNIT_ASSERT_GT(test, ret, 0); + + mode = drm_connector_pick_cmdline_mode(connector); + KUNIT_ASSERT_NOT_NULL(test, mode); + + KUNIT_EXPECT_TRUE(test, drm_mode_equal(drm_mode_analog_ntsc_480i(drm), mode)); +} + +static void drm_test_pick_cmdline_named_pal(struct kunit *test) +{ + struct drm_client_modeset_test_priv *priv = test->priv; + struct drm_device *drm = priv->drm; + struct drm_connector *connector = &priv->connector; + struct drm_cmdline_mode *cmdline_mode = &connector->cmdline_mode; + struct drm_display_mode *mode; + const char *cmdline = "PAL"; + int ret; + + KUNIT_ASSERT_TRUE(test, + drm_mode_parse_command_line_for_connector(cmdline, + connector, + cmdline_mode)); + + mutex_lock(&drm->mode_config.mutex); + ret = drm_helper_probe_single_connector_modes(connector, 1920, 1080); + mutex_unlock(&drm->mode_config.mutex); + KUNIT_ASSERT_GT(test, ret, 0); + + mode = drm_connector_pick_cmdline_mode(connector); + KUNIT_ASSERT_NOT_NULL(test, mode); + + KUNIT_EXPECT_TRUE(test, drm_mode_equal(drm_mode_analog_pal_576i(drm), mode)); +} static struct kunit_case drm_test_pick_cmdline_tests[] = { KUNIT_CASE(drm_test_pick_cmdline_res_1920_1080_60), + KUNIT_CASE(drm_test_pick_cmdline_named_ntsc), + KUNIT_CASE(drm_test_pick_cmdline_named_pal), {} };
The framework will get the drm_display_mode from the drm_cmdline_mode it got by parsing the video command line argument by calling drm_connector_pick_cmdline_mode(). The heavy lifting will then be done by the drm_mode_create_from_cmdline_mode() function. In the case of the named modes though, there's no real code to make that translation and we rely on the drivers to guess which actual display mode we meant. Let's modify drm_mode_create_from_cmdline_mode() to properly generate the drm_display_mode we mean when passing a named mode. Signed-off-by: Maxime Ripard <maxime@cerno.tech> --- Changes in v7: - Use tv_mode_specified in drm_mode_parse_command_line_for_connector Changes in v6: - Fix get_modes to return 0 instead of an error code - Rename the tests to follow the DRM test naming convention Changes in v5: - Switched to KUNIT_ASSERT_NOT_NULL --- drivers/gpu/drm/drm_modes.c | 34 ++++++++++- drivers/gpu/drm/tests/drm_client_modeset_test.c | 77 ++++++++++++++++++++++++- 2 files changed, 109 insertions(+), 2 deletions(-)