Re: [PATCH v3 2/6] drm/modes: Support modes names on the command line
Den 18.04.2019 14.41, skrev Maxime Ripard:
> From: Maxime Ripard
>
> The drm subsystem also uses the video= kernel parameter, and in the
> documentation refers to the fbdev documentation for that parameter.
>
> However, that documentation also says that instead of giving the mode using
> its resolution we can also give a name. However, DRM doesn't handle that
> case at the moment. Even though in most case it shouldn't make any
> difference, it might be useful for analog modes, where different standards
> might have the same resolution, but still have a few different parameters
> that are not encoded in the modes (NTSC vs NTSC-J vs PAL-M for example).
>
> Signed-off-by: Maxime Ripard
> ---
> drivers/gpu/drm/drm_client_modeset.c | 4 ++-
> drivers/gpu/drm/drm_connector.c | 3 +-
> drivers/gpu/drm/drm_modes.c | 52 -
> include/drm/drm_connector.h | 1 +-
> 4 files changed, 44 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c
> b/drivers/gpu/drm/drm_client_modeset.c
> index 2a428ac00930..f2869c82510c 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -149,6 +149,10 @@ drm_connector_pick_cmdline_mode(struct drm_connector
> *connector)
> prefer_non_interlace = !cmdline_mode->interlace;
> again:
> list_for_each_entry(mode, &connector->modes, head) {
> + /* Check (optional) mode name first */
> + if (!strcmp(mode->name, cmdline_mode->name))
> + return mode;
> +
> /* check width/height */
> if (mode->hdisplay != cmdline_mode->xres ||
> mode->vdisplay != cmdline_mode->yres)
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 2355124849db..e33814f5940e 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -139,8 +139,9 @@ static void drm_connector_get_cmdline_mode(struct
> drm_connector *connector)
> connector->force = mode->force;
> }
>
> - DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",
> + DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
> connector->name,
> + mode->name ? mode->name : "",
> mode->xres, mode->yres,
> mode->refresh_specified ? mode->refresh : 60,
> mode->rb ? " reduced blanking" : "",
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 3f89198f0891..9613c1a28487 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1557,7 +1557,7 @@ bool drm_mode_parse_command_line_for_connector(const
> char *mode_option,
> struct drm_cmdline_mode *mode)
> {
> const char *name;
> - bool parse_extras = false;
> + bool named_mode = false, parse_extras = false;
You don't need to assign a value here, or if you do, you can drop the
else further down.
Reviewed-by: Noralf Trønnes
> unsigned int bpp_off = 0, refresh_off = 0;
> unsigned int mode_end = 0;
> char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
> @@ -1576,8 +1576,14 @@ bool drm_mode_parse_command_line_for_connector(const
> char *mode_option,
>
> name = mode_option;
>
> + /*
> + * If the first character is not a digit, then it means that
> + * we have a named mode.
> + */
> if (!isdigit(name[0]))
> - return false;
> + named_mode = true;
> + else
> + named_mode = false;
>
> /* Try to locate the bpp and refresh specifiers, if any */
> bpp_ptr = strchr(name, '-');
> @@ -1588,6 +1594,9 @@ bool drm_mode_parse_command_line_for_connector(const
> char *mode_option,
>
> refresh_ptr = strchr(name, '@');
> if (refresh_ptr) {
> + if (named_mode)
> + return false;
> +
> refresh_off = refresh_ptr - name;
> mode->refresh_specified = true;
> }
> @@ -1604,12 +1613,16 @@ bool drm_mode_parse_command_line_for_connector(const
> char *mode_option,
> parse_extras = true;
> }
>
> - ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
> - parse_extras,
> - connector,
> - mode);
> - if (ret)
> - return false;
> + if (named_mode) {
> + strncpy(mode->name, name, mode_end);
> + } else {
> + ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
> + parse_extras,
> + connector,
> + mode);
> + if (ret)
> + return
[PATCH v3 2/6] drm/modes: Support modes names on the command line
From: Maxime Ripard
The drm subsystem also uses the video= kernel parameter, and in the
documentation refers to the fbdev documentation for that parameter.
However, that documentation also says that instead of giving the mode using
its resolution we can also give a name. However, DRM doesn't handle that
case at the moment. Even though in most case it shouldn't make any
difference, it might be useful for analog modes, where different standards
might have the same resolution, but still have a few different parameters
that are not encoded in the modes (NTSC vs NTSC-J vs PAL-M for example).
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_client_modeset.c | 4 ++-
drivers/gpu/drm/drm_connector.c | 3 +-
drivers/gpu/drm/drm_modes.c | 52 -
include/drm/drm_connector.h | 1 +-
4 files changed, 44 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index 2a428ac00930..f2869c82510c 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -149,6 +149,10 @@ drm_connector_pick_cmdline_mode(struct drm_connector
*connector)
prefer_non_interlace = !cmdline_mode->interlace;
again:
list_for_each_entry(mode, &connector->modes, head) {
+ /* Check (optional) mode name first */
+ if (!strcmp(mode->name, cmdline_mode->name))
+ return mode;
+
/* check width/height */
if (mode->hdisplay != cmdline_mode->xres ||
mode->vdisplay != cmdline_mode->yres)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2355124849db..e33814f5940e 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -139,8 +139,9 @@ static void drm_connector_get_cmdline_mode(struct
drm_connector *connector)
connector->force = mode->force;
}
- DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",
+ DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
connector->name,
+ mode->name ? mode->name : "",
mode->xres, mode->yres,
mode->refresh_specified ? mode->refresh : 60,
mode->rb ? " reduced blanking" : "",
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 3f89198f0891..9613c1a28487 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1557,7 +1557,7 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
struct drm_cmdline_mode *mode)
{
const char *name;
- bool parse_extras = false;
+ bool named_mode = false, parse_extras = false;
unsigned int bpp_off = 0, refresh_off = 0;
unsigned int mode_end = 0;
char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
@@ -1576,8 +1576,14 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
name = mode_option;
+ /*
+* If the first character is not a digit, then it means that
+* we have a named mode.
+*/
if (!isdigit(name[0]))
- return false;
+ named_mode = true;
+ else
+ named_mode = false;
/* Try to locate the bpp and refresh specifiers, if any */
bpp_ptr = strchr(name, '-');
@@ -1588,6 +1594,9 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
refresh_ptr = strchr(name, '@');
if (refresh_ptr) {
+ if (named_mode)
+ return false;
+
refresh_off = refresh_ptr - name;
mode->refresh_specified = true;
}
@@ -1604,12 +1613,16 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
parse_extras = true;
}
- ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
- parse_extras,
- connector,
- mode);
- if (ret)
- return false;
+ if (named_mode) {
+ strncpy(mode->name, name, mode_end);
+ } else {
+ ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
+ parse_extras,
+ connector,
+ mode);
+ if (ret)
+ return false;
+ }
mode->specified = true;
if (bpp_ptr) {
@@ -1637,14 +1650,23 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
extra_ptr = refresh_end_ptr;
if (extra_ptr) {
- int remaining =
