From: Thomas Zimmermann <tzimmermann@suse.de> To: Geert Uytterhoeven <geert@linux-m68k.org>, Maarten Lankhorst <maarten.lankhorst@linux.intel.com>, Maxime Ripard <mripard@kernel.org>, David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>, Hans de Goede <hdegoede@redhat.com> Cc: dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org, linux-m68k@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 4/5] drm/modes: Add support for driver-specific named modes Date: Mon, 11 Jul 2022 11:03:38 +0200 [thread overview] Message-ID: <ef2aada2-96e4-c2e4-645f-39bc9094e93a@suse.de> (raw) In-Reply-To: <68923c8a129b6c2a70b570103679a1cf7876bbc2.1657301107.git.geert@linux-m68k.org> [-- Attachment #1.1: Type: text/plain, Size: 3979 bytes --] Hi Geert Am 08.07.22 um 20:21 schrieb Geert Uytterhoeven: > The mode parsing code recognizes named modes only if they are explicitly > listed in the internal whitelist, which is currently limited to "NTSC" > and "PAL". > > Provide a mechanism for drivers to override this list to support custom > mode names. > > Ideally, this list should just come from the driver's actual list of > modes, but connector->probed_modes is not yet populated at the time of > parsing. I've looked for code that uses these names, couldn't find any. How is this being used in practice? For example, if I say "PAL" on the command line, is there DRM code that fills in the PAL mode parameters? And another question I have is whether this whitelist belongs into the driver at all. Standard modes exist independent from drivers or hardware. Shouldn't there simply be a global list of all possible mode names? Drivers would filter out the unsupported modes anyway. Best regards Thomas > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > --- > drivers/gpu/drm/drm_modes.c | 15 +++++++++++---- > include/drm/drm_connector.h | 10 ++++++++++ > 2 files changed, 21 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index 9ce275fbda566b7c..7a00eb6df502e991 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -1748,25 +1748,31 @@ static int drm_mode_parse_cmdline_options(const char *str, > static const char * const drm_named_modes_whitelist[] = { > "NTSC", > "PAL", > + NULL > }; > > static int drm_mode_parse_cmdline_named_mode(const char *name, > unsigned int length, > bool refresh, > + const struct drm_connector *connector, > struct drm_cmdline_mode *mode) > { > + const char * const *named_modes_whitelist; > unsigned int i; > int ret; > > - for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) { > - ret = str_has_prefix(name, drm_named_modes_whitelist[i]); > + named_modes_whitelist = connector->named_modes_whitelist ? : > + drm_named_modes_whitelist; > + > + for (i = 0; named_modes_whitelist[i]; i++) { > + ret = str_has_prefix(name, named_modes_whitelist[i]); > if (!ret) > continue; > > if (refresh) > return -EINVAL; /* named + refresh is invalid */ > > - strcpy(mode->name, drm_named_modes_whitelist[i]); > + strcpy(mode->name, named_modes_whitelist[i]); > mode->specified = true; > return 0; > } > @@ -1850,7 +1856,8 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, > /* First check for a named mode */ > if (mode_end) { > ret = drm_mode_parse_cmdline_named_mode(name, mode_end, > - refresh_ptr, mode); > + refresh_ptr, connector, > + mode); > if (ret) > return false; > } > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 3ac4bf87f2571c4c..6361f8a596c01107 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -1659,6 +1659,16 @@ struct drm_connector { > > /** @hdr_sink_metadata: HDR Metadata Information read from sink */ > struct hdr_sink_metadata hdr_sink_metadata; > + > + /** > + * @named_modes_whitelist: > + * > + * Optional NULL-terminated array of names to be considered valid mode > + * names. This lets the command line option parser distinguish between > + * mode names and freestanding extras and/or options. > + * If not set, a set of defaults will be used. > + */ > + const char * const *named_modes_whitelist; > }; > > #define obj_to_connector(x) container_of(x, struct drm_connector, base) -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Thomas Zimmermann <tzimmermann@suse.de> To: Geert Uytterhoeven <geert@linux-m68k.org>, Maarten Lankhorst <maarten.lankhorst@linux.intel.com>, Maxime Ripard <mripard@kernel.org>, David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>, Hans de Goede <hdegoede@redhat.com> Cc: linux-fbdev@vger.kernel.org, linux-m68k@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 4/5] drm/modes: Add support for driver-specific named modes Date: Mon, 11 Jul 2022 11:03:38 +0200 [thread overview] Message-ID: <ef2aada2-96e4-c2e4-645f-39bc9094e93a@suse.de> (raw) In-Reply-To: <68923c8a129b6c2a70b570103679a1cf7876bbc2.1657301107.git.geert@linux-m68k.org> [-- Attachment #1.1: Type: text/plain, Size: 3979 bytes --] Hi Geert Am 08.07.22 um 20:21 schrieb Geert Uytterhoeven: > The mode parsing code recognizes named modes only if they are explicitly > listed in the internal whitelist, which is currently limited to "NTSC" > and "PAL". > > Provide a mechanism for drivers to override this list to support custom > mode names. > > Ideally, this list should just come from the driver's actual list of > modes, but connector->probed_modes is not yet populated at the time of > parsing. I've looked for code that uses these names, couldn't find any. How is this being used in practice? For example, if I say "PAL" on the command line, is there DRM code that fills in the PAL mode parameters? And another question I have is whether this whitelist belongs into the driver at all. Standard modes exist independent from drivers or hardware. Shouldn't there simply be a global list of all possible mode names? Drivers would filter out the unsupported modes anyway. Best regards Thomas > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > --- > drivers/gpu/drm/drm_modes.c | 15 +++++++++++---- > include/drm/drm_connector.h | 10 ++++++++++ > 2 files changed, 21 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index 9ce275fbda566b7c..7a00eb6df502e991 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -1748,25 +1748,31 @@ static int drm_mode_parse_cmdline_options(const char *str, > static const char * const drm_named_modes_whitelist[] = { > "NTSC", > "PAL", > + NULL > }; > > static int drm_mode_parse_cmdline_named_mode(const char *name, > unsigned int length, > bool refresh, > + const struct drm_connector *connector, > struct drm_cmdline_mode *mode) > { > + const char * const *named_modes_whitelist; > unsigned int i; > int ret; > > - for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) { > - ret = str_has_prefix(name, drm_named_modes_whitelist[i]); > + named_modes_whitelist = connector->named_modes_whitelist ? : > + drm_named_modes_whitelist; > + > + for (i = 0; named_modes_whitelist[i]; i++) { > + ret = str_has_prefix(name, named_modes_whitelist[i]); > if (!ret) > continue; > > if (refresh) > return -EINVAL; /* named + refresh is invalid */ > > - strcpy(mode->name, drm_named_modes_whitelist[i]); > + strcpy(mode->name, named_modes_whitelist[i]); > mode->specified = true; > return 0; > } > @@ -1850,7 +1856,8 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, > /* First check for a named mode */ > if (mode_end) { > ret = drm_mode_parse_cmdline_named_mode(name, mode_end, > - refresh_ptr, mode); > + refresh_ptr, connector, > + mode); > if (ret) > return false; > } > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 3ac4bf87f2571c4c..6361f8a596c01107 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -1659,6 +1659,16 @@ struct drm_connector { > > /** @hdr_sink_metadata: HDR Metadata Information read from sink */ > struct hdr_sink_metadata hdr_sink_metadata; > + > + /** > + * @named_modes_whitelist: > + * > + * Optional NULL-terminated array of names to be considered valid mode > + * names. This lets the command line option parser distinguish between > + * mode names and freestanding extras and/or options. > + * If not set, a set of defaults will be used. > + */ > + const char * const *named_modes_whitelist; > }; > > #define obj_to_connector(x) container_of(x, struct drm_connector, base) -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --]
next prev parent reply other threads:[~2022-07-11 9:03 UTC|newest] Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-07-08 18:21 [PATCH 0/5] drm/modes: Command line mode selection fixes and improvements Geert Uytterhoeven 2022-07-08 18:21 ` Geert Uytterhoeven 2022-07-08 18:21 ` [PATCH 1/5] drm/modes: parse_cmdline: Handle empty mode name part Geert Uytterhoeven 2022-07-08 18:21 ` Geert Uytterhoeven 2022-07-08 19:28 ` Hans de Goede 2022-07-08 19:28 ` Hans de Goede 2022-07-08 20:06 ` Geert Uytterhoeven 2022-07-08 20:06 ` Geert Uytterhoeven 2022-07-08 20:09 ` Geert Uytterhoeven 2022-07-08 20:09 ` Geert Uytterhoeven 2022-07-08 18:21 ` [PATCH 2/5] drm/modes: Extract drm_mode_parse_cmdline_named_mode() Geert Uytterhoeven 2022-07-08 18:21 ` Geert Uytterhoeven 2022-07-08 19:45 ` Hans de Goede 2022-07-08 19:45 ` Hans de Goede 2022-07-08 20:14 ` Geert Uytterhoeven 2022-07-08 20:14 ` Geert Uytterhoeven 2022-07-08 18:21 ` [PATCH 3/5] drm/modes: parse_cmdline: Make mode->*specified handling more uniform Geert Uytterhoeven 2022-07-08 18:21 ` Geert Uytterhoeven 2022-07-08 18:21 ` [PATCH 4/5] drm/modes: Add support for driver-specific named modes Geert Uytterhoeven 2022-07-08 18:21 ` Geert Uytterhoeven 2022-07-11 9:03 ` Thomas Zimmermann [this message] 2022-07-11 9:03 ` Thomas Zimmermann 2022-07-11 9:35 ` Maxime Ripard 2022-07-11 9:35 ` Maxime Ripard 2022-07-11 11:11 ` Thomas Zimmermann 2022-07-11 11:11 ` Thomas Zimmermann 2022-07-11 11:42 ` Maxime Ripard 2022-07-11 11:42 ` Maxime Ripard 2022-07-11 11:59 ` Geert Uytterhoeven 2022-07-11 11:59 ` Geert Uytterhoeven 2022-07-11 12:02 ` Maxime Ripard 2022-07-11 12:02 ` Maxime Ripard 2022-07-11 12:08 ` Geert Uytterhoeven 2022-07-11 12:08 ` Geert Uytterhoeven 2022-07-13 9:37 ` Maxime Ripard 2022-07-13 9:37 ` Maxime Ripard 2022-07-14 8:42 ` Geert Uytterhoeven 2022-07-14 8:42 ` Geert Uytterhoeven 2022-07-11 9:35 ` Geert Uytterhoeven 2022-07-11 9:35 ` Geert Uytterhoeven 2022-07-11 11:03 ` Thomas Zimmermann 2022-07-11 11:03 ` Thomas Zimmermann 2022-07-08 18:21 ` [PATCH 5/5] drm/modes: parse_cmdline: Add support for named modes containing dashes Geert Uytterhoeven 2022-07-08 18:21 ` Geert Uytterhoeven 2022-07-08 19:36 ` [PATCH 0/5] drm/modes: Command line mode selection fixes and improvements Hans de Goede 2022-07-08 19:36 ` Hans de Goede 2022-07-11 9:04 ` Thomas Zimmermann 2022-07-11 9:04 ` Thomas Zimmermann
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=ef2aada2-96e4-c2e4-645f-39bc9094e93a@suse.de \ --to=tzimmermann@suse.de \ --cc=airlied@linux.ie \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=geert@linux-m68k.org \ --cc=hdegoede@redhat.com \ --cc=linux-fbdev@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-m68k@vger.kernel.org \ --cc=maarten.lankhorst@linux.intel.com \ --cc=mripard@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.