[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMuHMdV9wVgHFfwHoqtBoYzJDnjDmKTfaZkAKvTVKh1Y-2x1pA@mail.gmail.com>
Date: Tue, 30 Aug 2022 12:06:55 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Maxime Ripard <maxime@...no.tech>
Cc: Maxime Ripard <mripard@...nel.org>,
Ben Skeggs <bskeggs@...hat.com>,
David Airlie <airlied@...ux.ie>, Chen-Yu Tsai <wens@...e.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
Jani Nikula <jani.nikula@...ux.intel.com>,
Lyude Paul <lyude@...hat.com>,
Philipp Zabel <p.zabel@...gutronix.de>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Rodrigo Vivi <rodrigo.vivi@...el.com>,
Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>,
Jernej Skrabec <jernej.skrabec@...il.com>,
Samuel Holland <samuel@...lland.org>,
Karol Herbst <kherbst@...hat.com>,
Noralf Trønnes <noralf@...nnes.org>,
Emma Anholt <emma@...olt.net>, Daniel Vetter <daniel@...ll.ch>,
Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
Hans de Goede <hdegoede@...hat.com>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
Phil Elwell <phil@...pberrypi.com>,
Intel Graphics Development <intel-gfx@...ts.freedesktop.org>,
Dave Stevenson <dave.stevenson@...pberrypi.com>,
DRI Development <dri-devel@...ts.freedesktop.org>,
Dom Cobley <dom@...pberrypi.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Nouveau Dev <nouveau@...ts.freedesktop.org>,
linux-sunxi@...ts.linux.dev,
Mateusz Kwiatkowski <kfyatek+publicgit@...il.com>
Subject: Re: [PATCH v2 14/41] drm/modes: Move named modes parsing to a
separate function
Hi Maxime,
On Mon, Aug 29, 2022 at 3:13 PM Maxime Ripard <maxime@...no.tech> wrote:
> 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 <maxime@...no.tech>
Thanks for your patch!
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1909,6 +1909,9 @@ void drm_connector_list_update(struct drm_connector *connector)
> }
> EXPORT_SYMBOL(drm_connector_list_update);
>
> +#define STR_STRICT_EQ(str, len, cmp) \
> + ((strlen(cmp) == len) && !strncmp(str, cmp, len))
This is not part of the move, but newly added.
> +
> static int drm_mode_parse_cmdline_bpp(const char *str, char **end_ptr,
> struct drm_cmdline_mode *mode)
> {
> @@ -2208,6 +2211,52 @@ static const char * const drm_named_modes_whitelist[] = {
> "PAL",
> };
>
> +static int drm_mode_parse_cmdline_named_mode(const char *name,
> + unsigned int name_end,
> + struct drm_cmdline_mode *cmdline_mode)
> +{
> + unsigned int i;
> +
> + if (!name_end)
> + return 0;
This is already checked by the caller.
> +
> + /* If the name starts with a digit, it's not a named mode */
> + if (isdigit(name[0]))
> + return 0;
> +
> + /*
> + * If there's an equal sign in the name, the command-line
> + * contains only an option and no mode.
> + */
> + if (strnchr(name, name_end, '='))
> + return 0;
> +
> + /* The connection status extras can be set without a mode. */
> + if (STR_STRICT_EQ(name, name_end, "d") ||
> + STR_STRICT_EQ(name, name_end, "D") ||
> + STR_STRICT_EQ(name, name_end, "e"))
> + return 0;
These checks are not part of the move, and should probably be added
in a separate patch.
> +
> + /*
> + * We're sure we're a named mode at that point, iterate over the
> + * list of modes we're aware of.
> + */
> + 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 1;
> + }
> +
> + return -EINVAL;
> +}
> +
> /**
> * drm_mode_parse_command_line_for_connector - parse command line modeline for connector
> * @mode_option: optional per connector mode option
> @@ -2244,7 +2293,7 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
> const char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
> const char *options_ptr = NULL;
> char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL;
> - int i, len, ret;
> + int len, ret;
>
> memset(mode, 0, sizeof(*mode));
> mode->panel_orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> @@ -2285,17 +2334,19 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
> parse_extras = true;
> }
>
> - /* First check for a named mode */
> - for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) {
> - ret = str_has_prefix(name, drm_named_modes_whitelist[i]);
> - if (ret == mode_end) {
> - if (refresh_ptr)
> - return false; /* named + refresh is invalid */
>
> - strcpy(mode->name, drm_named_modes_whitelist[i]);
> - mode->specified = true;
> - break;
> - }
> + if (mode_end) {
> + ret = drm_mode_parse_cmdline_named_mode(name, mode_end, mode);
> + if (ret < 0)
> + return false;
> +
> + /*
> + * Having a mode that starts by a letter (and thus is named)
> + * and an at-sign (used to specify a refresh rate) is
> + * disallowed.
> + */
> + if (ret && refresh_ptr)
> + return false;
> }
>
> /* No named mode? Check for a normal mode argument, e.g. 1024x768 */
>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-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
Powered by blists - more mailing lists