Re: [PATCH v2 2/4] drm/uAPI: Add "force color format" drm property as setting for userspace

[Pekka Paalanen]

On Wed, 17 Jan 2024 12:58:15 +0000
Andri Yngvason <andri@xxxxxxxxxxx> wrote:

> mið., 17. jan. 2024 kl. 09:21 skrifaði Pekka Paalanen <ppaalanen@xxxxxxxxx>:

..

> > EDID and DisplayID standards also evolve. The kernel could be behind
> > userspace in chasing them, which was the reason why the kernel does not
> > validate HDR_OUTPUT_METADATA against EDID.
> >
> > The design of today with HDR_OUTPUT_METADATA and whatnot is
> > that userspace is responsible for checking sink capabilities, and
> > atomic check is responsible for driver and display controller
> > capabilities.  
> 
> I'm not really sure where you're going with this. Are you for or
> against userspace parsing EDID instead of getting the information from
> the kernel?

In that specific email, neither. I attempted to describe the current
situation without any bias towards whether I think that is a good or
not design. There is an existing behaviour, and if you want to deviate
from that, you need more justification than for following it.

Even the video modes list that I mentioned as the major example of
things that userspace should not parse from EDID itself is not
exhaustive nor exclusive. Userspace can still craft an arbitrary video
mode and set it. If the driver and display controller can do it, it
passes I believe, even if it would literally destroy the sink (in the
CRT era, you could burn the flyback transistor of an unfortunate
monitor).

If you want me to take a stance, I think the kernel not gating settings
based on EDID for these things is a good idea for these reasons:

- EDID can easily be wrong, and it is easier to test sink "unsupported"
  configurations if you do not need to craft a modified EDID and
  (reboot to?) load it in the kernel first.

- EDID spec gets occasionally extended. If the kernel gated settings,
  you would not be able to test new features without getting an updated
  kernel that allows them to pass. This mostly applies to blob
  properties, and not enums, because you cannot set arbitrary values to
  enum properties.

Finally, as to why userspace parsing EDID at all is a good idea:

- The kernel is not interested in most of the stuff contained in EDIDs,
  so it has no inherent reason to parse everything.

- EDID is a fairly wide "API" and gets occasionally extended.
  Replicating all that in a kernel UAPI is a lot of work that won't
  benefit the kernel itself. There does not seem to be benefit in
  reinventing EDID information encoding in fine-grained UAPI
  structures, but there certainly is risk, because UAPI is written in
  stone once published. Userspace can get the equivalent from libraries
  like libdisplay-info which are much easier to develop and replace
  than UAPI.


Thanks,
pq

Attachment: pgpDdOBIBVcwY.pgp
Description: OpenPGP digital signature