[PATCH 2/4] drm/modes: Fix the command line parser to take force options into account

[Thomas Graichen]

hi maxime,

On Tue, Aug 27, 2019 at 1:59 PM Maxime Ripard <mripard at kernel.org> wrote:
>
> From: Maxime Ripard <maxime.ripard at bootlin.com>
>
> The command line parser when it has been rewritten introduced a regression
> when the only thing on the command line is an option to force the detection
> of a connector (such as video=HDMI-A-1:d), which are completely valid.
>
> It's been further broken by the support for the named modes which take
> anything that is not a resolution as a named mode.
>
> Let's fix this by running the extra command line option parser on the named
> modes if they only take a single character.
>
> Fixes: e08ab74bd4c7 ("drm/modes: Rewrite the command line parser")
> Reported-by: Jernej Škrabec <jernej.skrabec at gmail.com>
> Reported-by: Thomas Graichen <thomas.graichen at googlemail.com>
> Signed-off-by: Maxime Ripard <maxime.ripard at bootlin.com>

Tested-by: thomas graichen <thomas.graichen at gmail.com>

BEFORE (only "[PATCH v5 05/12] drm/modes: Rewrite the command line
parser " applied):
my eachlink h6 mini tv box (which requires the video=HDMI-A-1:e
cmdline option to give any output on hdmi) did not show any hdmi
output any more (in 5.2 it still worked)

AFTER (the above patch plus this patch set here applied):
my eachlink h6 mini tv box gives output on hdmi again. i also did
check it the other way around: if i remove the video=HDMI-A-1:e option
i no longer get any hdmi output as expected. as a result this patch
series fixes the problem/regression for me.

best wishes - thomas

> ---
>  drivers/gpu/drm/drm_modes.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index e5997f35b779..ea7e6c8c8318 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1733,16 +1733,30 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
>          * bunch of things:
>          *   - We need to make sure that the first character (which
>          *     would be our resolution in X) is a digit.
> -        *   - However, if the X resolution is missing, then we end up
> -        *     with something like x<yres>, with our first character
> -        *     being an alpha-numerical character, which would be
> -        *     considered a named mode.
> +        *   - If not, then it's either a named mode or a force on/off.
> +        *     To distinguish between the two, we need to run the
> +        *     extra parsing function, and if not, then we consider it
> +        *     a named mode.
>          *
>          * If this isn't enough, we should add more heuristics here,
>          * and matching unit-tests.
>          */
> -       if (!isdigit(name[0]) && name[0] != 'x')
> +       if (!isdigit(name[0]) && name[0] != 'x') {
> +               unsigned int namelen = strlen(name);
> +
> +               /*
> +                * Only the force on/off options can be in that case,
> +                * and they all take a single character.
> +                */
> +               if (namelen == 1) {
> +                       ret = drm_mode_parse_cmdline_extra(name, namelen, true,
> +                                                          connector, mode);
> +                       if (!ret)
> +                               return true;
> +               }
> +
>                 named_mode = true;
> +       }
>
>         /* Try to locate the bpp and refresh specifiers, if any */
>         bpp_ptr = strchr(name, '-');
> --
> 2.21.0
>