Hi Maxime,

On Tue, Aug 16, 2022 at 3:46 PM Maxime Ripard [off-list ref] wrote:

On Fri, Aug 12, 2022 at 03:27:17PM +0200, Geert Uytterhoeven wrote:

quoted

On Fri, Jul 29, 2022 at 6:36 PM Maxime Ripard [off-list ref] wrote:

quoted

The current construction of the named mode parsing doesn't allow to extend
it easily. Let's move it to a separate function so we can add more
parameters and modes.

Signed-off-by: Maxime Ripard <redacted>

Thanks for your patch, which looks similar to my "[PATCH v2 2/5]
drm/modes: Extract drm_mode_parse_cmdline_named_mode()"
(https://lore.kernel.org/dri-devel/1371554419ae63cb54c2a377db0c1016fcf200bb.1657788997.git.geert@linux-m68k.org (local)
;-)

Indeed, I forgot about that one, sorry :/

I think I'd still prefer to have the check for refresh + named mode
outside of the function, since I see them as an "integration" issue, not
a parsing one.

It's not the named mode parsing that fails, but the fact that we both
have a valid refresh and a valid named mode.

quoted

quoted

--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c

@@ -1773,6 +1773,28 @@ static const char * const drm_named_modes_whitelist[] = {
        "PAL",
 };

+static bool drm_mode_parse_cmdline_named_mode(const char *name,
+                                             unsigned int name_end,
+                                             struct drm_cmdline_mode *cmdline_mode)
+{
+       unsigned int i;
+
+       for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) {
+               int ret;
+
+               ret = str_has_prefix(name, drm_named_modes_whitelist[i]);
+               if (ret != name_end)
+                       continue;
+
+               strcpy(cmdline_mode->name, drm_named_modes_whitelist[i]);
+               cmdline_mode->specified = true;
+
+               return true;
+       }
+
+       return false;

What's the point in returning a value, if it is never checked?
Just make this function return void?

Yeah, it's something I went back and forth to between the dev, and it's
an artifact.

I'll drop that patch, take your version and move the refresh check to
drm_mode_parse_command_line_for_connector if that's alright for you?

Fine for me.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel