Re: [PATCH 4/5] drm/modes: Add support for driver-specific named modes

From: Thomas Zimmermann
Date: Mon Jul 11 2022 - 07:27:39 EST


Hi

Am 11.07.22 um 11:35 schrieb Geert Uytterhoeven:
Hi Thomas,

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

Yeah, I saw his reply already.

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.
I don't really have a strong opinion, but having all modes in one global list is quite user-friendly. It's all there for everyone. Otherwise users would somehow have to know which hardware supports which modes. That's actually the job of each driver's mode_valid and atomic_check functions.

Best regards
Thomas


Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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

--
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: OpenPGP_signature
Description: OpenPGP digital signature