From: Maxime Ripard maxime.ripard@bootlin.com
Some extra command line options can be either specified without anything else on the command line (basically all the force connection options), but some other are only relevant when matched with a resolution (margin and interlace).
Let's add a switch to restrict if needed the available option set.
Fixes: e08ab74bd4c7 ("drm/modes: Rewrite the command line parser") Signed-off-by: Maxime Ripard maxime.ripard@bootlin.com --- drivers/gpu/drm/drm_modes.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 11acc9581977..e5997f35b779 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1454,6 +1454,7 @@ static int drm_mode_parse_cmdline_refresh(const char *str, char **end_ptr, }
static int drm_mode_parse_cmdline_extra(const char *str, int length, + bool freestanding, const struct drm_connector *connector, struct drm_cmdline_mode *mode) { @@ -1462,9 +1463,15 @@ static int drm_mode_parse_cmdline_extra(const char *str, int length, for (i = 0; i < length; i++) { switch (str[i]) { case 'i': + if (freestanding) + return -EINVAL; + mode->interlace = true; break; case 'm': + if (freestanding) + return -EINVAL; + mode->margins = true; break; case 'D': @@ -1542,6 +1549,7 @@ static int drm_mode_parse_cmdline_res_mode(const char *str, unsigned int length, if (extras) { int ret = drm_mode_parse_cmdline_extra(end_ptr + i, 1, + false, connector, mode); if (ret) @@ -1811,7 +1819,7 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, extra_ptr != options_ptr) { int len = strlen(name) - (extra_ptr - name);
- ret = drm_mode_parse_cmdline_extra(extra_ptr, len, + ret = drm_mode_parse_cmdline_extra(extra_ptr, len, false, connector, mode); if (ret) return false;
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 --- 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, '-');
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
From: Maxime Ripard maxime.ripard@bootlin.com
The named modes support has introduced a number of glitches that were in part due to the fact that the parser will take any string as a named mode.
Since we shouldn't have a lot of options there (and they should be pretty standard), let's introduce a whitelist of the available named modes so that the kernel can differentiate between a poorly formed command line and a named mode.
Signed-off-by: Maxime Ripard maxime.ripard@bootlin.com --- drivers/gpu/drm/drm_modes.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index ea7e6c8c8318..988797d8080a 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1677,6 +1677,22 @@ static int drm_mode_parse_cmdline_options(char *str, size_t len, return 0; }
+const char *drm_named_modes_whitelist[] = { + "NTSC", + "PAL", +}; + +static bool drm_named_mode_is_in_whitelist(const char *mode, unsigned int size) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) + if (!strncmp(mode, drm_named_modes_whitelist[i], size)) + return true; + + return false; +} + /** * drm_mode_parse_command_line_for_connector - parse command line modeline for connector * @mode_option: optional per connector mode option @@ -1794,6 +1810,10 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, if (named_mode) { if (mode_end + 1 > DRM_DISPLAY_MODE_LEN) return false; + + if (!drm_named_mode_is_in_whitelist(name, mode_end)) + return false; + strscpy(mode->name, name, mode_end + 1); } else { ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
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 named modes support has introduced a number of glitches that were in part due to the fact that the parser will take any string as a named mode.
Since we shouldn't have a lot of options there (and they should be pretty standard), let's introduce a whitelist of the available named modes so that the kernel can differentiate between a poorly formed command line and a named mode.
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 | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index ea7e6c8c8318..988797d8080a 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1677,6 +1677,22 @@ static int drm_mode_parse_cmdline_options(char *str, size_t len, return 0; }
+const char *drm_named_modes_whitelist[] = {
"NTSC","PAL",+};
+static bool drm_named_mode_is_in_whitelist(const char *mode, unsigned int size) +{
int i;for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++)if (!strncmp(mode, drm_named_modes_whitelist[i], size))return true;return false;+}
/**
- drm_mode_parse_command_line_for_connector - parse command line modeline for connector
- @mode_option: optional per connector mode option
@@ -1794,6 +1810,10 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, if (named_mode) { if (mode_end + 1 > DRM_DISPLAY_MODE_LEN) return false;
if (!drm_named_mode_is_in_whitelist(name, mode_end))return false;strscpy(mode->name, name, mode_end + 1); } else { ret = drm_mode_parse_cmdline_res_mode(name, mode_end,-- 2.21.0
Hi!
Dne torek, 27. avgust 2019 ob 13:58:49 CEST je Maxime Ripard napisal(a):
From: Maxime Ripard maxime.ripard@bootlin.com
The named modes support has introduced a number of glitches that were in part due to the fact that the parser will take any string as a named mode.
Since we shouldn't have a lot of options there (and they should be pretty standard), let's introduce a whitelist of the available named modes so that the kernel can differentiate between a poorly formed command line and a named mode.
Signed-off-by: Maxime Ripard maxime.ripard@bootlin.com
drivers/gpu/drm/drm_modes.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index ea7e6c8c8318..988797d8080a 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1677,6 +1677,22 @@ static int drm_mode_parse_cmdline_options(char *str, size_t len, return 0; }
+const char *drm_named_modes_whitelist[] = {
- "NTSC",
- "PAL",
+};
That array should be static. With that fixed:
Reviewed-by: Jernej Skrabec jernej.skrabec@siol.net
Best regards, Jernej
+static bool drm_named_mode_is_in_whitelist(const char *mode, unsigned int size) +{
- int i;
- for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++)
if (!strncmp(mode, drm_named_modes_whitelist[i], size))return true;- return false;
+}
/**
- drm_mode_parse_command_line_for_connector - parse command line modeline
for connector * @mode_option: optional per connector mode option @@ -1794,6 +1810,10 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, if (named_mode) { if (mode_end + 1 > DRM_DISPLAY_MODE_LEN) return false;
if (!drm_named_mode_is_in_whitelist(name, mode_end))return false;- strscpy(mode->name, name, mode_end + 1); } else { ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
On Thu, 29 Aug 2019, Jernej Škrabec jernej.skrabec@gmail.com wrote:
Hi!
Dne torek, 27. avgust 2019 ob 13:58:49 CEST je Maxime Ripard napisal(a):
From: Maxime Ripard maxime.ripard@bootlin.com
The named modes support has introduced a number of glitches that were in part due to the fact that the parser will take any string as a named mode.
Since we shouldn't have a lot of options there (and they should be pretty standard), let's introduce a whitelist of the available named modes so that the kernel can differentiate between a poorly formed command line and a named mode.
Signed-off-by: Maxime Ripard maxime.ripard@bootlin.com
drivers/gpu/drm/drm_modes.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index ea7e6c8c8318..988797d8080a 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1677,6 +1677,22 @@ static int drm_mode_parse_cmdline_options(char *str, size_t len, return 0; }
+const char *drm_named_modes_whitelist[] = {
- "NTSC",
- "PAL",
+};
That array should be static. With that fixed:
And add more const for good measure:
static const char * const drm_named_modes_whitelist[] = {
BR, Jani.
Reviewed-by: Jernej Skrabec jernej.skrabec@siol.net
Best regards, Jernej
+static bool drm_named_mode_is_in_whitelist(const char *mode, unsigned int size) +{
- int i;
- for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++)
if (!strncmp(mode, drm_named_modes_whitelist[i], size))return true;- return false;
+}
/**
- drm_mode_parse_command_line_for_connector - parse command line modeline
for connector * @mode_option: optional per connector mode option @@ -1794,6 +1810,10 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, if (named_mode) { if (mode_end + 1 > DRM_DISPLAY_MODE_LEN) return false;
if (!drm_named_mode_is_in_whitelist(name, mode_end))return false;- strscpy(mode->name, name, mode_end + 1); } else { ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
From: Maxime Ripard maxime.ripard@bootlin.com
Let's add some unit tests for the recent bugs we just fixed.
Signed-off-by: Maxime Ripard maxime.ripard@bootlin.com --- .../gpu/drm/selftests/drm_cmdline_selftests.h | 7 + .../drm/selftests/test-drm_cmdline_parser.c | 130 ++++++++++++++++++ 2 files changed, 137 insertions(+)
diff --git a/drivers/gpu/drm/selftests/drm_cmdline_selftests.h b/drivers/gpu/drm/selftests/drm_cmdline_selftests.h index b45824ec7c8f..6d61a0eb5d64 100644 --- a/drivers/gpu/drm/selftests/drm_cmdline_selftests.h +++ b/drivers/gpu/drm/selftests/drm_cmdline_selftests.h @@ -9,6 +9,13 @@
#define cmdline_test(test) selftest(test, test)
+cmdline_test(drm_cmdline_test_force_d_only) +cmdline_test(drm_cmdline_test_force_D_only_dvi) +cmdline_test(drm_cmdline_test_force_D_only_hdmi) +cmdline_test(drm_cmdline_test_force_D_only_not_digital) +cmdline_test(drm_cmdline_test_force_e_only) +cmdline_test(drm_cmdline_test_margin_only) +cmdline_test(drm_cmdline_test_interlace_only) cmdline_test(drm_cmdline_test_res) cmdline_test(drm_cmdline_test_res_missing_x) cmdline_test(drm_cmdline_test_res_missing_y) diff --git a/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c index 14c96edb13df..013de9d27c35 100644 --- a/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c +++ b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c @@ -17,6 +17,136 @@
static const struct drm_connector no_connector = {};
+static int drm_cmdline_test_force_e_only(void *ignored) +{ + struct drm_cmdline_mode mode = { }; + + FAIL_ON(!drm_mode_parse_command_line_for_connector("e", + &no_connector, + &mode)); + FAIL_ON(mode.specified); + FAIL_ON(mode.refresh_specified); + FAIL_ON(mode.bpp_specified); + + FAIL_ON(mode.rb); + FAIL_ON(mode.cvt); + FAIL_ON(mode.interlace); + FAIL_ON(mode.margins); + FAIL_ON(mode.force != DRM_FORCE_ON); + + return 0; +} + +static int drm_cmdline_test_force_D_only_not_digital(void *ignored) +{ + struct drm_cmdline_mode mode = { }; + + FAIL_ON(!drm_mode_parse_command_line_for_connector("D", + &no_connector, + &mode)); + FAIL_ON(mode.specified); + FAIL_ON(mode.refresh_specified); + FAIL_ON(mode.bpp_specified); + + FAIL_ON(mode.rb); + FAIL_ON(mode.cvt); + FAIL_ON(mode.interlace); + FAIL_ON(mode.margins); + FAIL_ON(mode.force != DRM_FORCE_ON); + + return 0; +} + +static const struct drm_connector connector_hdmi = { + .connector_type = DRM_MODE_CONNECTOR_HDMIB, +}; + +static int drm_cmdline_test_force_D_only_hdmi(void *ignored) +{ + struct drm_cmdline_mode mode = { }; + + FAIL_ON(!drm_mode_parse_command_line_for_connector("D", + &connector_hdmi, + &mode)); + FAIL_ON(mode.specified); + FAIL_ON(mode.refresh_specified); + FAIL_ON(mode.bpp_specified); + + FAIL_ON(mode.rb); + FAIL_ON(mode.cvt); + FAIL_ON(mode.interlace); + FAIL_ON(mode.margins); + FAIL_ON(mode.force != DRM_FORCE_ON_DIGITAL); + + return 0; +} + +static const struct drm_connector connector_dvi = { + .connector_type = DRM_MODE_CONNECTOR_DVII, +}; + +static int drm_cmdline_test_force_D_only_dvi(void *ignored) +{ + struct drm_cmdline_mode mode = { }; + + FAIL_ON(!drm_mode_parse_command_line_for_connector("D", + &connector_dvi, + &mode)); + FAIL_ON(mode.specified); + FAIL_ON(mode.refresh_specified); + FAIL_ON(mode.bpp_specified); + + FAIL_ON(mode.rb); + FAIL_ON(mode.cvt); + FAIL_ON(mode.interlace); + FAIL_ON(mode.margins); + FAIL_ON(mode.force != DRM_FORCE_ON_DIGITAL); + + return 0; +} + +static int drm_cmdline_test_force_d_only(void *ignored) +{ + struct drm_cmdline_mode mode = { }; + + FAIL_ON(!drm_mode_parse_command_line_for_connector("d", + &no_connector, + &mode)); + FAIL_ON(mode.specified); + FAIL_ON(mode.refresh_specified); + FAIL_ON(mode.bpp_specified); + + FAIL_ON(mode.rb); + FAIL_ON(mode.cvt); + FAIL_ON(mode.interlace); + FAIL_ON(mode.margins); + FAIL_ON(mode.force != DRM_FORCE_OFF); + + return 0; +} + +static int drm_cmdline_test_margin_only(void *ignored) +{ + struct drm_cmdline_mode mode = { }; + + FAIL_ON(drm_mode_parse_command_line_for_connector("m", + &no_connector, + &mode)); + + return 0; +} + +static int drm_cmdline_test_interlace_only(void *ignored) +{ + struct drm_cmdline_mode mode = { }; + + FAIL_ON(drm_mode_parse_command_line_for_connector("i", + &no_connector, + &mode)); + + return 0; +} + static int drm_cmdline_test_res(void *ignored) { struct drm_cmdline_mode mode = { };
hi maxime,
On Tue, Aug 27, 2019 at 1:59 PM Maxime Ripard mripard@kernel.org wrote:
From: Maxime Ripard maxime.ripard@bootlin.com
Let's add some unit tests for the recent bugs we just fixed.
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
.../gpu/drm/selftests/drm_cmdline_selftests.h | 7 + .../drm/selftests/test-drm_cmdline_parser.c | 130 ++++++++++++++++++ 2 files changed, 137 insertions(+)
diff --git a/drivers/gpu/drm/selftests/drm_cmdline_selftests.h b/drivers/gpu/drm/selftests/drm_cmdline_selftests.h index b45824ec7c8f..6d61a0eb5d64 100644 --- a/drivers/gpu/drm/selftests/drm_cmdline_selftests.h +++ b/drivers/gpu/drm/selftests/drm_cmdline_selftests.h @@ -9,6 +9,13 @@
#define cmdline_test(test) selftest(test, test)
+cmdline_test(drm_cmdline_test_force_d_only) +cmdline_test(drm_cmdline_test_force_D_only_dvi) +cmdline_test(drm_cmdline_test_force_D_only_hdmi) +cmdline_test(drm_cmdline_test_force_D_only_not_digital) +cmdline_test(drm_cmdline_test_force_e_only) +cmdline_test(drm_cmdline_test_margin_only) +cmdline_test(drm_cmdline_test_interlace_only) cmdline_test(drm_cmdline_test_res) cmdline_test(drm_cmdline_test_res_missing_x) cmdline_test(drm_cmdline_test_res_missing_y) diff --git a/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c index 14c96edb13df..013de9d27c35 100644 --- a/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c +++ b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c @@ -17,6 +17,136 @@
static const struct drm_connector no_connector = {};
+static int drm_cmdline_test_force_e_only(void *ignored) +{
struct drm_cmdline_mode mode = { };FAIL_ON(!drm_mode_parse_command_line_for_connector("e",&no_connector,&mode));FAIL_ON(mode.specified);FAIL_ON(mode.refresh_specified);FAIL_ON(mode.bpp_specified);FAIL_ON(mode.rb);FAIL_ON(mode.cvt);FAIL_ON(mode.interlace);FAIL_ON(mode.margins);FAIL_ON(mode.force != DRM_FORCE_ON);return 0;+}
+static int drm_cmdline_test_force_D_only_not_digital(void *ignored) +{
struct drm_cmdline_mode mode = { };FAIL_ON(!drm_mode_parse_command_line_for_connector("D",&no_connector,&mode));FAIL_ON(mode.specified);FAIL_ON(mode.refresh_specified);FAIL_ON(mode.bpp_specified);FAIL_ON(mode.rb);FAIL_ON(mode.cvt);FAIL_ON(mode.interlace);FAIL_ON(mode.margins);FAIL_ON(mode.force != DRM_FORCE_ON);return 0;+}
+static const struct drm_connector connector_hdmi = {
.connector_type = DRM_MODE_CONNECTOR_HDMIB,+};
+static int drm_cmdline_test_force_D_only_hdmi(void *ignored) +{
struct drm_cmdline_mode mode = { };FAIL_ON(!drm_mode_parse_command_line_for_connector("D",&connector_hdmi,&mode));FAIL_ON(mode.specified);FAIL_ON(mode.refresh_specified);FAIL_ON(mode.bpp_specified);FAIL_ON(mode.rb);FAIL_ON(mode.cvt);FAIL_ON(mode.interlace);FAIL_ON(mode.margins);FAIL_ON(mode.force != DRM_FORCE_ON_DIGITAL);return 0;+}
+static const struct drm_connector connector_dvi = {
.connector_type = DRM_MODE_CONNECTOR_DVII,+};
+static int drm_cmdline_test_force_D_only_dvi(void *ignored) +{
struct drm_cmdline_mode mode = { };FAIL_ON(!drm_mode_parse_command_line_for_connector("D",&connector_dvi,&mode));FAIL_ON(mode.specified);FAIL_ON(mode.refresh_specified);FAIL_ON(mode.bpp_specified);FAIL_ON(mode.rb);FAIL_ON(mode.cvt);FAIL_ON(mode.interlace);FAIL_ON(mode.margins);FAIL_ON(mode.force != DRM_FORCE_ON_DIGITAL);return 0;+}
+static int drm_cmdline_test_force_d_only(void *ignored) +{
struct drm_cmdline_mode mode = { };FAIL_ON(!drm_mode_parse_command_line_for_connector("d",&no_connector,&mode));FAIL_ON(mode.specified);FAIL_ON(mode.refresh_specified);FAIL_ON(mode.bpp_specified);FAIL_ON(mode.rb);FAIL_ON(mode.cvt);FAIL_ON(mode.interlace);FAIL_ON(mode.margins);FAIL_ON(mode.force != DRM_FORCE_OFF);return 0;+}
+static int drm_cmdline_test_margin_only(void *ignored) +{
struct drm_cmdline_mode mode = { };FAIL_ON(drm_mode_parse_command_line_for_connector("m",&no_connector,&mode));return 0;+}
+static int drm_cmdline_test_interlace_only(void *ignored) +{
struct drm_cmdline_mode mode = { };FAIL_ON(drm_mode_parse_command_line_for_connector("i",&no_connector,&mode));return 0;+}
static int drm_cmdline_test_res(void *ignored) { struct drm_cmdline_mode mode = { }; -- 2.21.0
Hi!
Dne torek, 27. avgust 2019 ob 13:58:50 CEST je Maxime Ripard napisal(a):
From: Maxime Ripard maxime.ripard@bootlin.com
Let's add some unit tests for the recent bugs we just fixed.
Signed-off-by: Maxime Ripard maxime.ripard@bootlin.com
.../gpu/drm/selftests/drm_cmdline_selftests.h | 7 + .../drm/selftests/test-drm_cmdline_parser.c | 130 ++++++++++++++++++ 2 files changed, 137 insertions(+)
diff --git a/drivers/gpu/drm/selftests/drm_cmdline_selftests.h b/drivers/gpu/drm/selftests/drm_cmdline_selftests.h index
Reviewed-by: Jernej Skrabec jernej.skrabec@siol.net
Best regards, Jernej
hi maxime,
On Tue, Aug 27, 2019 at 1:58 PM Maxime Ripard mripard@kernel.org wrote:
From: Maxime Ripard maxime.ripard@bootlin.com
Some extra command line options can be either specified without anything else on the command line (basically all the force connection options), but some other are only relevant when matched with a resolution (margin and interlace).
Let's add a switch to restrict if needed the available option set.
Fixes: e08ab74bd4c7 ("drm/modes: Rewrite the command line parser") 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 | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 11acc9581977..e5997f35b779 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1454,6 +1454,7 @@ static int drm_mode_parse_cmdline_refresh(const char *str, char **end_ptr, }
static int drm_mode_parse_cmdline_extra(const char *str, int length,
bool freestanding, const struct drm_connector *connector, struct drm_cmdline_mode *mode){ @@ -1462,9 +1463,15 @@ static int drm_mode_parse_cmdline_extra(const char *str, int length, for (i = 0; i < length; i++) { switch (str[i]) { case 'i':
if (freestanding)return -EINVAL;mode->interlace = true; break; case 'm':if (freestanding)return -EINVAL;mode->margins = true; break; case 'D':@@ -1542,6 +1549,7 @@ static int drm_mode_parse_cmdline_res_mode(const char *str, unsigned int length, if (extras) { int ret = drm_mode_parse_cmdline_extra(end_ptr + i, 1,
false, connector, mode); if (ret)@@ -1811,7 +1819,7 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, extra_ptr != options_ptr) { int len = strlen(name) - (extra_ptr - name);
ret = drm_mode_parse_cmdline_extra(extra_ptr, len,
ret = drm_mode_parse_cmdline_extra(extra_ptr, len, false, connector, mode); if (ret) return false;-- 2.21.0
Hi!
Dne torek, 27. avgust 2019 ob 13:58:47 CEST je Maxime Ripard napisal(a):
From: Maxime Ripard maxime.ripard@bootlin.com
Some extra command line options can be either specified without anything else on the command line (basically all the force connection options), but some other are only relevant when matched with a resolution (margin and interlace).
Let's add a switch to restrict if needed the available option set.
Fixes: e08ab74bd4c7 ("drm/modes: Rewrite the command line parser") Signed-off-by: Maxime Ripard maxime.ripard@bootlin.com
Reviewed-by: Jernej Skrabec jernej.skrabec@siol.net
Best regards, Jernej
dri-devel@lists.freedesktop.org
-
Jani Nikula -
Jernej Škrabec -
Maxime Ripard -
Thomas Graichen