| Message ID | 20190827115850.25731-2-mripard@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] drm/modes: Add a switch to differentiate free standing options | expand |
hi maxime, On Tue, Aug 27, 2019 at 1:59 PM Maxime Ripard <mripard@kernel.org> wrote: > > From: Maxime Ripard <maxime.ripard@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@gmail.com> > Reported-by: Thomas Graichen <thomas.graichen@googlemail.com> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> Tested-by: thomas graichen <thomas.graichen@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 >
Hi! Dne torek, 27. avgust 2019 ob 13:58:48 CEST je Maxime Ripard napisal(a): > From: Maxime Ripard <maxime.ripard@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@gmail.com> > Reported-by: Thomas Graichen <thomas.graichen@googlemail.com> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net> Best regards, Jernej
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, '-');