[PATCH v3 2/6] drm/modes: Support modes names on the command line

[Noralf Trønnes]

Den 18.04.2019 14.41, skrev Maxime Ripard:
> From: Maxime Ripard <maxime.ripard at free-electrons.com>
> 
> 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 <maxime.ripard at free-electrons.com>
> ---
>  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 <noralf at tronnes.org>

>  	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 = strlen(name) - (extra_ptr - name);
> +		if (!named_mode) {
> +			int len = strlen(name) - (extra_ptr - name);
>  
> -		/*
> -		 * We still have characters to process, while
> -		 * we shouldn't have any
> -		 */
> -		if (remaining > 0)
> -			return false;
> +			ret = drm_mode_parse_cmdline_extra(extra_ptr, len,
> +							   connector, mode);
> +			if (ret)
> +				return false;
> +		} else {
> +			int remaining = strlen(name) - (extra_ptr - name);
> +
> +			/*
> +			 * We still have characters to process, while
> +			 * we shouldn't have any
> +			 */
> +			if (remaining > 0)
> +				return false;
> +		}
>  	}
>  
>  	return true;
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 02a131202add..06aa3b9cb920 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -896,6 +896,7 @@ struct drm_connector_funcs {
>  
>  /* mode specified on the command line */
>  struct drm_cmdline_mode {
> +	char name[DRM_DISPLAY_MODE_LEN];
>  	bool specified;
>  	bool refresh_specified;
>  	bool bpp_specified;
>