Re: [PATCH] drm: document that user-space should avoid parsing EDIDs

[Pekka Paalanen <ppaalanen@gmail.com>]

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

On Fri, 9 Oct 2020 11:48:44 +0200
Thomas Zimmermann <tzimmermann@suse.de> wrote:

> Hi
> 
> Am 09.10.20 um 11:24 schrieb Simon Ser:
> > User-space should avoid parsing EDIDs for metadata already exposed via
> > other KMS interfaces and properties. For instance, user-space should not
> > try to extract a list of modes from the EDID: the kernel might mutate
> > the mode list (because of link capabilities or quirks for instance).
> > 
> > Other metadata not exposed by KMS can be parsed by user-space. This
> > includes for instance monitor identification (make/model/serial) and
> > supported color-spaces.
> > 
> > Signed-off-by: Simon Ser <contact@emersion.fr>
> > Suggested-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>

Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>

> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_connector.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index 717c4e7271b0..00e58fd2d292 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -960,6 +960,10 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
> >   * 	drm_connector_update_edid_property(), usually after having parsed
> >   * 	the EDID using drm_add_edid_modes(). Userspace cannot change this
> >   * 	property.
> > + *
> > + * 	User-space should not parse the EDID to obtain information exposed via
> > + * 	other KMS properties. For instance, user-space should not try to parse
> > + * 	mode lists from the EDID.  
> 
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> But this makes me wonder why the kernel exposes raw EDID in the first
> place. Wouldn't it be better to expose meaningful fields (display model,
> vendor, etc) instead?

There are many EDID bits of information that the kernel has no use
for. EDID specifications and extensions also continually evolve.

If the kernel set out to expose all information EDID may encode, what
should the UAPI look like? How do you keep the UAPI maintainable and
extendable?

Why should the kernel parse information it has no use for itself at
all? For example: RGB and white-point chromaticities, or maximum HDR
luminance.

That seems quite a lot of continuous work for a benefit I'm not sure I
see when compared to just handing the EDID blob to userspace and let
userspace parse the things the kernel does not expose already.

What we do need is a userspace EDID parsing library. This was discussed
in #dri-devel IRC today as well.


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel