linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: 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>,
	DRI Development <dri-devel@lists.freedesktop.org>,
	Linux Fbdev development list <linux-fbdev@vger.kernel.org>,
	"Linux/m68k" <linux-m68k@vger.kernel.org>,
	Linux Kernel Mailing List <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:35:51 +0200	[thread overview]
Message-ID: <CAMuHMdUqo-_5tyhmx_QqPJhqQdoRDE6_Q7b1AJWeBZc67RsBSA@mail.gmail.com> (raw)
In-Reply-To: <ef2aada2-96e4-c2e4-645f-39bc9094e93a@suse.de>

Hi Thomas,

On Mon, Jul 11, 2022 at 11:03 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> 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?

I guess Maxime knows, as he added the whitelist?
Reading the description of commit 3764137906a5acec ("drm/modes:
Introduce a whitelist for the named modes"), it looks like this is
more about preventing the parser from taking any string as a random
mode, than about adding support for "PAL" or "NTSC"?

Note that drivers/gpu/drm/i915/display/intel_tv.c defines an array of
tv_modes[], including "PAL", so perhaps these end up as named modes?

> 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.

For standard modes, I agree.  And these are usually specified by
resolution and refresh rate (e.g. "640x480@60", instead of "480p").

But legacy hardware may have very limited support for programmable
pixel clocks (e.g. Amiga is limited to pixel clocks of 7, 14, or 28
MHz), so the standard modes are a bad match, or may not work at all.
Hence drivers may need to provide their own modes, but it seems wrong
to me to make these non-standard modes global, and possibly pollute
the experience for everyone.

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

  parent reply	other threads:[~2022-07-11 10:19 UTC|newest]

Thread overview: 24+ 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 ` [PATCH 1/5] drm/modes: parse_cmdline: Handle empty mode name part Geert Uytterhoeven
2022-07-08 19:28   ` Hans de Goede
2022-07-08 20:06     ` 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 19:45   ` Hans de Goede
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 ` [PATCH 4/5] drm/modes: Add support for driver-specific named modes Geert Uytterhoeven
2022-07-11  9:03   ` Thomas Zimmermann
2022-07-11  9:35     ` Maxime Ripard
2022-07-11 11:11       ` Thomas Zimmermann
2022-07-11 11:42         ` Maxime Ripard
2022-07-11 11:59           ` Geert Uytterhoeven
2022-07-11 12:02             ` Maxime Ripard
2022-07-11 12:08               ` Geert Uytterhoeven
2022-07-13  9:37                 ` Maxime Ripard
2022-07-14  8:42                   ` Geert Uytterhoeven
2022-07-11  9:35     ` Geert Uytterhoeven [this message]
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 19:36 ` [PATCH 0/5] drm/modes: Command line mode selection fixes and improvements Hans de Goede
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=CAMuHMdUqo-_5tyhmx_QqPJhqQdoRDE6_Q7b1AJWeBZc67RsBSA@mail.gmail.com \
    --to=geert@linux-m68k.org \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.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 \
    --cc=tzimmermann@suse.de \
    /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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).