Message ID | e32cd4009153b184103554009135c7bf7c9975d7.1560783090.git-series.maxime.ripard@bootlin.com |
---|---|
State | Accepted |
Commit | e08ab74bd4c7a5fe311bc05f32dbb4f1e7fa3428 |
Headers | show |
Series | None | expand |
17.06.2019 17:51, Maxime Ripard пишет: > From: Maxime Ripard <maxime.ripard@free-electrons.com> > > Rewrite the command line parser in order to get away from the state machine > parsing the video mode lines. > > Hopefully, this will allow to extend it more easily to support named modes > and / or properties set directly on the command line. > > Reviewed-by: Noralf Trønnes <noralf@tronnes.org> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > --- > drivers/gpu/drm/drm_modes.c | 325 +++++++++++++++++++++++-------------- > 1 file changed, 210 insertions(+), 115 deletions(-) Hello, I have a Tegra device that uses a stock android bootloader which passes "video=tegrafb" in the kernels cmdline. That wasn't a problem before this patch, but now Tegra DRM driver fails to probe because the mode is 0x0:0 and hence framebuffer allocation fails. Is it a legit regression that should be fixed in upstream?
Hi, On Fri, Jul 05, 2019 at 07:54:47PM +0300, Dmitry Osipenko wrote: > 17.06.2019 17:51, Maxime Ripard пишет: > > From: Maxime Ripard <maxime.ripard@free-electrons.com> > > > > Rewrite the command line parser in order to get away from the state machine > > parsing the video mode lines. > > > > Hopefully, this will allow to extend it more easily to support named modes > > and / or properties set directly on the command line. > > > > Reviewed-by: Noralf Trønnes <noralf@tronnes.org> > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > --- > > drivers/gpu/drm/drm_modes.c | 325 +++++++++++++++++++++++-------------- > > 1 file changed, 210 insertions(+), 115 deletions(-) > > I have a Tegra device that uses a stock android bootloader which > passes "video=tegrafb" in the kernels cmdline. That wasn't a problem > before this patch, but now Tegra DRM driver fails to probe because > the mode is 0x0:0 and hence framebuffer allocation fails. Is it a > legit regression that should be fixed in upstream? Thierry indeed reported that issue, but the discussion pretty much stalled since then. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
09.07.2019 15:45, Maxime Ripard пишет: > Hi, > > On Fri, Jul 05, 2019 at 07:54:47PM +0300, Dmitry Osipenko wrote: >> 17.06.2019 17:51, Maxime Ripard пишет: >>> From: Maxime Ripard <maxime.ripard@free-electrons.com> >>> >>> Rewrite the command line parser in order to get away from the state machine >>> parsing the video mode lines. >>> >>> Hopefully, this will allow to extend it more easily to support named modes >>> and / or properties set directly on the command line. >>> >>> Reviewed-by: Noralf Trønnes <noralf@tronnes.org> >>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> >>> --- >>> drivers/gpu/drm/drm_modes.c | 325 +++++++++++++++++++++++-------------- >>> 1 file changed, 210 insertions(+), 115 deletions(-) >> >> I have a Tegra device that uses a stock android bootloader which >> passes "video=tegrafb" in the kernels cmdline. That wasn't a problem >> before this patch, but now Tegra DRM driver fails to probe because >> the mode is 0x0:0 and hence framebuffer allocation fails. Is it a >> legit regression that should be fixed in upstream? > > Thierry indeed reported that issue, but the discussion pretty much > stalled since then. Sorry, this doesn't answer my question. Where it was reported? If it's a valid regression (my device is broken), then the patch should either be fixed or reverted.
On 09/07/2019 13:52, Dmitry Osipenko wrote: > 09.07.2019 15:45, Maxime Ripard пишет: >> Hi, >> >> On Fri, Jul 05, 2019 at 07:54:47PM +0300, Dmitry Osipenko wrote: >>> 17.06.2019 17:51, Maxime Ripard пишет: >>>> From: Maxime Ripard <maxime.ripard@free-electrons.com> >>>> >>>> Rewrite the command line parser in order to get away from the state machine >>>> parsing the video mode lines. >>>> >>>> Hopefully, this will allow to extend it more easily to support named modes >>>> and / or properties set directly on the command line. >>>> >>>> Reviewed-by: Noralf Trønnes <noralf@tronnes.org> >>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> >>>> --- >>>> drivers/gpu/drm/drm_modes.c | 325 +++++++++++++++++++++++-------------- >>>> 1 file changed, 210 insertions(+), 115 deletions(-) >>> >>> I have a Tegra device that uses a stock android bootloader which >>> passes "video=tegrafb" in the kernels cmdline. That wasn't a problem >>> before this patch, but now Tegra DRM driver fails to probe because >>> the mode is 0x0:0 and hence framebuffer allocation fails. Is it a >>> legit regression that should be fixed in upstream? >> >> Thierry indeed reported that issue, but the discussion pretty much >> stalled since then. Yes Thierry is currently out and hence has not responded. I had been planning to look at this this week and responded. > Sorry, this doesn't answer my question. Where it was reported? Same thread [0]. Dmitry, are you able to respond to Maxime's response to Thierry? Cheers Jon [0] https://patchwork.kernel.org/patch/10999393/
On 09/07/2019 14:26, Jon Hunter wrote: > > On 09/07/2019 13:52, Dmitry Osipenko wrote: >> 09.07.2019 15:45, Maxime Ripard пишет: >>> Hi, >>> >>> On Fri, Jul 05, 2019 at 07:54:47PM +0300, Dmitry Osipenko wrote: >>>> 17.06.2019 17:51, Maxime Ripard пишет: >>>>> From: Maxime Ripard <maxime.ripard@free-electrons.com> >>>>> >>>>> Rewrite the command line parser in order to get away from the state machine >>>>> parsing the video mode lines. >>>>> >>>>> Hopefully, this will allow to extend it more easily to support named modes >>>>> and / or properties set directly on the command line. >>>>> >>>>> Reviewed-by: Noralf Trønnes <noralf@tronnes.org> >>>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> >>>>> --- >>>>> drivers/gpu/drm/drm_modes.c | 325 +++++++++++++++++++++++-------------- >>>>> 1 file changed, 210 insertions(+), 115 deletions(-) >>>> >>>> I have a Tegra device that uses a stock android bootloader which >>>> passes "video=tegrafb" in the kernels cmdline. That wasn't a problem >>>> before this patch, but now Tegra DRM driver fails to probe because >>>> the mode is 0x0:0 and hence framebuffer allocation fails. Is it a >>>> legit regression that should be fixed in upstream? >>> >>> Thierry indeed reported that issue, but the discussion pretty much >>> stalled since then. > > Yes Thierry is currently out and hence has not responded. I had been > planning to look at this this week and responded. > >> Sorry, this doesn't answer my question. Where it was reported? > > Same thread [0]. Correction, it was on patch 6/12 and not this one. Jon
09.07.2019 16:28, Jon Hunter пишет: > > On 09/07/2019 14:26, Jon Hunter wrote: >> >> On 09/07/2019 13:52, Dmitry Osipenko wrote: >>> 09.07.2019 15:45, Maxime Ripard пишет: >>>> Hi, >>>> >>>> On Fri, Jul 05, 2019 at 07:54:47PM +0300, Dmitry Osipenko wrote: >>>>> 17.06.2019 17:51, Maxime Ripard пишет: >>>>>> From: Maxime Ripard <maxime.ripard@free-electrons.com> >>>>>> >>>>>> Rewrite the command line parser in order to get away from the state machine >>>>>> parsing the video mode lines. >>>>>> >>>>>> Hopefully, this will allow to extend it more easily to support named modes >>>>>> and / or properties set directly on the command line. >>>>>> >>>>>> Reviewed-by: Noralf Trønnes <noralf@tronnes.org> >>>>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> >>>>>> --- >>>>>> drivers/gpu/drm/drm_modes.c | 325 +++++++++++++++++++++++-------------- >>>>>> 1 file changed, 210 insertions(+), 115 deletions(-) >>>>> >>>>> I have a Tegra device that uses a stock android bootloader which >>>>> passes "video=tegrafb" in the kernels cmdline. That wasn't a problem >>>>> before this patch, but now Tegra DRM driver fails to probe because >>>>> the mode is 0x0:0 and hence framebuffer allocation fails. Is it a >>>>> legit regression that should be fixed in upstream? >>>> >>>> Thierry indeed reported that issue, but the discussion pretty much >>>> stalled since then. >> >> Yes Thierry is currently out and hence has not responded. I had been >> planning to look at this this week and responded. >> >>> Sorry, this doesn't answer my question. Where it was reported? >> >> Same thread [0]. > > Correction, it was on patch 6/12 and not this one. Thank you very much, Jon! So looks like this patch is simply broken because it silently copies the "mode's name" and then drm_connector_get_cmdline_mode() doesn't even try to validate the mode.
+CC: Thomas Graichen Dne ponedeljek, 17. junij 2019 ob 16:51:32 CEST je Maxime Ripard napisal(a): > From: Maxime Ripard <maxime.ripard@free-electrons.com> > > Rewrite the command line parser in order to get away from the state machine > parsing the video mode lines. > > Hopefully, this will allow to extend it more easily to support named modes > and / or properties set directly on the command line. > > Reviewed-by: Noralf Trønnes <noralf@tronnes.org> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> Thomas reported to me that this patch breaks "video=CONNECTOR:e" kernel parameter which he currently uses as a workaround for H6 HDMI monitor detection issue on one STB. I suppose this is the same issue that Dmitry noticed. Thomas Graichen (in CC) can provide more information if needed. Best regards, Jernej
On Mon, Aug 19, 2019 at 8:54 PM Jernej Škrabec <jernej.skrabec@gmail.com> wrote: > > +CC: Thomas Graichen > > Dne ponedeljek, 17. junij 2019 ob 16:51:32 CEST je Maxime Ripard napisal(a): > > From: Maxime Ripard <maxime.ripard@free-electrons.com> > > > > Rewrite the command line parser in order to get away from the state machine > > parsing the video mode lines. > > > > Hopefully, this will allow to extend it more easily to support named modes > > and / or properties set directly on the command line. > > > > Reviewed-by: Noralf Trønnes <noralf@tronnes.org> > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > Thomas reported to me that this patch breaks "video=CONNECTOR:e" kernel > parameter which he currently uses as a workaround for H6 HDMI monitor > detection issue on one STB. > > I suppose this is the same issue that Dmitry noticed. > > Thomas Graichen (in CC) can provide more information if needed. > > Best regards, > Jernej as jernej already mentioned i am currently having to use the kernel cmdline option video=HDMI-A-1:e to get a working hdmi output on an eachlink h6 mini tv box and was wondering that i did not get any hdmi output even with this option when switching from the https://github.com/megous/linux oprange-pi-5.2 to the orange-pi-5.3 branch which seems to contain this patch. as i had no idea what might have caused the breakage of the hdmi output and did a full bisect of the kernel between those two versions, which ended reliably at exactly this patch - so i guess there is a regression at least with the video=CONNECTOR:e option (maybe others too?) with this patches code which makes it not working anymore. best wishes - thomas
Hi, On Mon, Aug 19, 2019 at 09:20:00PM +0200, Thomas Graichen wrote: > On Mon, Aug 19, 2019 at 8:54 PM Jernej Škrabec <jernej.skrabec@gmail.com> wrote: > > > > +CC: Thomas Graichen > > > > Dne ponedeljek, 17. junij 2019 ob 16:51:32 CEST je Maxime Ripard napisal(a): > > > From: Maxime Ripard <maxime.ripard@free-electrons.com> > > > > > > Rewrite the command line parser in order to get away from the state machine > > > parsing the video mode lines. > > > > > > Hopefully, this will allow to extend it more easily to support named modes > > > and / or properties set directly on the command line. > > > > > > Reviewed-by: Noralf Trønnes <noralf@tronnes.org> > > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > > > Thomas reported to me that this patch breaks "video=CONNECTOR:e" kernel > > parameter which he currently uses as a workaround for H6 HDMI monitor > > detection issue on one STB. > > > > I suppose this is the same issue that Dmitry noticed. > > > > Thomas Graichen (in CC) can provide more information if needed. > > as jernej already mentioned i am currently having to use the kernel > cmdline option video=HDMI-A-1:e to get a working hdmi output on an > eachlink h6 mini tv box and was wondering that i did not get any hdmi > output even with this option when switching from the > https://github.com/megous/linux oprange-pi-5.2 to the orange-pi-5.3 > branch which seems to contain this patch. Which kernel version is that based on? > as i had no idea what might have caused the breakage of the hdmi > output and did a full bisect of the kernel between those two > versions, which ended reliably at exactly this patch - so i guess > there is a regression at least with the video=CONNECTOR:e option > (maybe others too?) with this patches code which makes it not > working anymore. I'm not sure I'll have the time to look into it this week (or the next, unfortunately). However, the e parameter is supposed to be parsed by drm_mode_parse_cmdline_extra, which in turn is supposed to be called there: https://elixir.bootlin.com/linux/v5.3-rc5/source/drivers/gpu/drm/drm_modes.c#L1810 If you can test that, having an idea of if that function is called, which return code it returns, and if it isn't if why would be super helpful. Thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
hi maxim, On Tue, Aug 20, 2019 at 5:00 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > Hi, > > On Mon, Aug 19, 2019 at 09:20:00PM +0200, Thomas Graichen wrote: > > On Mon, Aug 19, 2019 at 8:54 PM Jernej Škrabec <jernej.skrabec@gmail.com> wrote: > > > > > > +CC: Thomas Graichen > > > > > > Dne ponedeljek, 17. junij 2019 ob 16:51:32 CEST je Maxime Ripard napisal(a): > > > > From: Maxime Ripard <maxime.ripard@free-electrons.com> > > > > > > > > Rewrite the command line parser in order to get away from the state machine > > > > parsing the video mode lines. > > > > > > > > Hopefully, this will allow to extend it more easily to support named modes > > > > and / or properties set directly on the command line. > > > > > > > > Reviewed-by: Noralf Trønnes <noralf@tronnes.org> > > > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > > > > > Thomas reported to me that this patch breaks "video=CONNECTOR:e" kernel > > > parameter which he currently uses as a workaround for H6 HDMI monitor > > > detection issue on one STB. > > > > > > I suppose this is the same issue that Dmitry noticed. > > > > > > Thomas Graichen (in CC) can provide more information if needed. > > > > as jernej already mentioned i am currently having to use the kernel > > cmdline option video=HDMI-A-1:e to get a working hdmi output on an > > eachlink h6 mini tv box and was wondering that i did not get any hdmi > > output even with this option when switching from the > > https://github.com/megous/linux oprange-pi-5.2 to the orange-pi-5.3 > > branch which seems to contain this patch. > > Which kernel version is that based on? 5.3-rc3 > > as i had no idea what might have caused the breakage of the hdmi > > output and did a full bisect of the kernel between those two > > versions, which ended reliably at exactly this patch - so i guess > > there is a regression at least with the video=CONNECTOR:e option > > (maybe others too?) with this patches code which makes it not > > working anymore. > > I'm not sure I'll have the time to look into it this week (or the > next, unfortunately). However, the e parameter is supposed to be > parsed by drm_mode_parse_cmdline_extra, which in turn is supposed to > be called there: > https://elixir.bootlin.com/linux/v5.3-rc5/source/drivers/gpu/drm/drm_modes.c#L1810 > > If you can test that, having an idea of if that function is called, > which return code it returns, and if it isn't if why would be super > helpful. i just added a printk and it looks like it is not getting called: diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index b0369e690f36..4c58fdb1d7be 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1813,6 +1813,7 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, ret = drm_mode_parse_cmdline_extra(extra_ptr, len, connector, mode); + printk(KERN_WARNING "DEBUG - drm_mode_parse_cmdline_extra %d", ret); if (ret) return false; } no output from it in dmesg (my loglevel=8 and on the kernel cmdline and in /proc/cmdline i have "video=HDMI-A-1:e") - so looks like it really gets lost somewhere along the way ... best wishes - thomas
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 5a07a28fec6d..6debbd6c1763 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -30,6 +30,7 @@ * authorization from the copyright holder(s) and author(s). */ +#include <linux/ctype.h> #include <linux/list.h> #include <linux/list_sort.h> #include <linux/export.h> @@ -1408,6 +1409,151 @@ void drm_connector_list_update(struct drm_connector *connector) } EXPORT_SYMBOL(drm_connector_list_update); +static int drm_mode_parse_cmdline_bpp(const char *str, char **end_ptr, + struct drm_cmdline_mode *mode) +{ + unsigned int bpp; + + if (str[0] != '-') + return -EINVAL; + + str++; + bpp = simple_strtol(str, end_ptr, 10); + if (*end_ptr == str) + return -EINVAL; + + mode->bpp = bpp; + mode->bpp_specified = true; + + return 0; +} + +static int drm_mode_parse_cmdline_refresh(const char *str, char **end_ptr, + struct drm_cmdline_mode *mode) +{ + unsigned int refresh; + + if (str[0] != '@') + return -EINVAL; + + str++; + refresh = simple_strtol(str, end_ptr, 10); + if (*end_ptr == str) + return -EINVAL; + + mode->refresh = refresh; + mode->refresh_specified = true; + + return 0; +} + +static int drm_mode_parse_cmdline_extra(const char *str, int length, + struct drm_connector *connector, + struct drm_cmdline_mode *mode) +{ + int i; + + for (i = 0; i < length; i++) { + switch (str[i]) { + case 'i': + mode->interlace = true; + break; + case 'm': + mode->margins = true; + break; + case 'D': + if (mode->force != DRM_FORCE_UNSPECIFIED) + return -EINVAL; + + if ((connector->connector_type != DRM_MODE_CONNECTOR_DVII) && + (connector->connector_type != DRM_MODE_CONNECTOR_HDMIB)) + mode->force = DRM_FORCE_ON; + else + mode->force = DRM_FORCE_ON_DIGITAL; + break; + case 'd': + if (mode->force != DRM_FORCE_UNSPECIFIED) + return -EINVAL; + + mode->force = DRM_FORCE_OFF; + break; + case 'e': + if (mode->force != DRM_FORCE_UNSPECIFIED) + return -EINVAL; + + mode->force = DRM_FORCE_ON; + break; + default: + return -EINVAL; + } + } + + return 0; +} + +static int drm_mode_parse_cmdline_res_mode(const char *str, unsigned int length, + bool extras, + struct drm_connector *connector, + struct drm_cmdline_mode *mode) +{ + const char *str_start = str; + bool rb = false, cvt = false; + int xres = 0, yres = 0; + int remaining, i; + char *end_ptr; + + xres = simple_strtol(str, &end_ptr, 10); + if (end_ptr == str) + return -EINVAL; + + if (end_ptr[0] != 'x') + return -EINVAL; + end_ptr++; + + str = end_ptr; + yres = simple_strtol(str, &end_ptr, 10); + if (end_ptr == str) + return -EINVAL; + + remaining = length - (end_ptr - str_start); + if (remaining < 0) + return -EINVAL; + + for (i = 0; i < remaining; i++) { + switch (end_ptr[i]) { + case 'M': + cvt = true; + break; + case 'R': + rb = true; + break; + default: + /* + * Try to pass that to our extras parsing + * function to handle the case where the + * extras are directly after the resolution + */ + if (extras) { + int ret = drm_mode_parse_cmdline_extra(end_ptr + i, + 1, + connector, + mode); + if (ret) + return ret; + } else { + return -EINVAL; + } + } + } + + mode->xres = xres; + mode->yres = yres; + mode->cvt = cvt; + mode->rb = rb; + + return 0; +} + /** * drm_mode_parse_command_line_for_connector - parse command line modeline for connector * @mode_option: optional per connector mode option @@ -1434,13 +1580,12 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, struct drm_cmdline_mode *mode) { const char *name; - unsigned int namelen; - bool res_specified = false, bpp_specified = false, refresh_specified = false; - unsigned int xres = 0, yres = 0, bpp = 32, refresh = 0; - bool yres_specified = false, cvt = false, rb = false; - bool interlace = false, margins = false, was_digit = false; - int i; - enum drm_connector_force force = DRM_FORCE_UNSPECIFIED; + bool 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; + char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL; + int ret; #ifdef CONFIG_FB if (!mode_option) @@ -1453,127 +1598,77 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, } name = mode_option; - namelen = strlen(name); - for (i = namelen-1; i >= 0; i--) { - switch (name[i]) { - case '@': - if (!refresh_specified && !bpp_specified && - !yres_specified && !cvt && !rb && was_digit) { - refresh = simple_strtol(&name[i+1], NULL, 10); - refresh_specified = true; - was_digit = false; - } else - goto done; - break; - case '-': - if (!bpp_specified && !yres_specified && !cvt && - !rb && was_digit) { - bpp = simple_strtol(&name[i+1], NULL, 10); - bpp_specified = true; - was_digit = false; - } else - goto done; - break; - case 'x': - if (!yres_specified && was_digit) { - yres = simple_strtol(&name[i+1], NULL, 10); - yres_specified = true; - was_digit = false; - } else - goto done; - break; - case '0' ... '9': - was_digit = true; - break; - case 'M': - if (yres_specified || cvt || was_digit) - goto done; - cvt = true; - break; - case 'R': - if (yres_specified || cvt || rb || was_digit) - goto done; - rb = true; - break; - case 'm': - if (cvt || yres_specified || was_digit) - goto done; - margins = true; - break; - case 'i': - if (cvt || yres_specified || was_digit) - goto done; - interlace = true; - break; - case 'e': - if (yres_specified || bpp_specified || refresh_specified || - was_digit || (force != DRM_FORCE_UNSPECIFIED)) - goto done; - force = DRM_FORCE_ON; - break; - case 'D': - if (yres_specified || bpp_specified || refresh_specified || - was_digit || (force != DRM_FORCE_UNSPECIFIED)) - goto done; + if (!isdigit(name[0])) + return false; - if ((connector->connector_type != DRM_MODE_CONNECTOR_DVII) && - (connector->connector_type != DRM_MODE_CONNECTOR_HDMIB)) - force = DRM_FORCE_ON; - else - force = DRM_FORCE_ON_DIGITAL; - break; - case 'd': - if (yres_specified || bpp_specified || refresh_specified || - was_digit || (force != DRM_FORCE_UNSPECIFIED)) - goto done; + /* Try to locate the bpp and refresh specifiers, if any */ + bpp_ptr = strchr(name, '-'); + if (bpp_ptr) { + bpp_off = bpp_ptr - name; + mode->bpp_specified = true; + } - force = DRM_FORCE_OFF; - break; - default: - goto done; - } + refresh_ptr = strchr(name, '@'); + if (refresh_ptr) { + refresh_off = refresh_ptr - name; + mode->refresh_specified = true; } - if (i < 0 && yres_specified) { - char *ch; - xres = simple_strtol(name, &ch, 10); - if ((ch != NULL) && (*ch == 'x')) - res_specified = true; - else - i = ch - name; - } else if (!yres_specified && was_digit) { - /* catch mode that begins with digits but has no 'x' */ - i = 0; + /* Locate the end of the name / resolution, and parse it */ + if (bpp_ptr && refresh_ptr) { + mode_end = min(bpp_off, refresh_off); + } else if (bpp_ptr) { + mode_end = bpp_off; + } else if (refresh_ptr) { + mode_end = refresh_off; + } else { + mode_end = strlen(name); + parse_extras = true; } -done: - if (i >= 0) { - pr_warn("[drm] parse error at position %i in video mode '%s'\n", - i, name); - mode->specified = false; + + ret = drm_mode_parse_cmdline_res_mode(name, mode_end, + parse_extras, + connector, + mode); + if (ret) return false; - } + mode->specified = true; - if (res_specified) { - mode->specified = true; - mode->xres = xres; - mode->yres = yres; + if (bpp_ptr) { + ret = drm_mode_parse_cmdline_bpp(bpp_ptr, &bpp_end_ptr, mode); + if (ret) + return false; } - if (refresh_specified) { - mode->refresh_specified = true; - mode->refresh = refresh; + if (refresh_ptr) { + ret = drm_mode_parse_cmdline_refresh(refresh_ptr, + &refresh_end_ptr, mode); + if (ret) + return false; } - if (bpp_specified) { - mode->bpp_specified = true; - mode->bpp = bpp; + /* + * Locate the end of the bpp / refresh, and parse the extras + * if relevant + */ + if (bpp_ptr && refresh_ptr) + extra_ptr = max(bpp_end_ptr, refresh_end_ptr); + else if (bpp_ptr) + extra_ptr = bpp_end_ptr; + else if (refresh_ptr) + extra_ptr = refresh_end_ptr; + + if (extra_ptr) { + int remaining = strlen(name) - (extra_ptr - name); + + /* + * We still have characters to process, while + * we shouldn't have any + */ + if (remaining > 0) + return false; } - mode->rb = rb; - mode->cvt = cvt; - mode->interlace = interlace; - mode->margins = margins; - mode->force = force; return true; }