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

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 3091 bytes --]

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@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?

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

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