KMS enums and bitfields UAPI

[Pekka Paalanen]

On Fri, 03 Apr 2020 10:15:21 +0000
Simon Ser <contact at emersion.fr> wrote:

> Hi all,
> 
> I've been working on a library called libliftoff [1]. This library allows users
> to set KMS properties on a hardware planes abstraction called layers.
> Basically, library users create as many layers as they want with the KMS
> properties they want, and libliftoff will map layers to actual hardware planes
> when possible. The layer-to-plane mapping is dynamic. In other words, when
> setting a layer's properties, the library user can't guess which KMS plane will
> be used (if any).
> 
> This works fine for many properties, but doesn't work for enums and bitfields [2].
> The KMS UAPI expects clients to retrieve the list of enum entries for each
> object via drmModeGetProperty. The KMS UAPI allows a lot of freedom here: each
> driver and even each plane can assign a different meaning to a given enum
> value. For instance, in theory a plane could expose a "rotation" property where
> 0x01 means "rotate-180", while another plane could expose a "rotation" property
> where 0x01 means "rotate-90".
> 
> This makes things complicated for all KMS clients, not only for libliftoff. All
> clients need to have an internal enum for e.g. "rotation", then when applying
> properties to a plane needs to convert their internal value enum to the
> per-plane enum value.
> 
> However, this isn't true for all properties. A bunch of properties have
> hardcoded values in the UAPI headers. Looking at my copy of drm_mode.h I can
> see DRM_MODE_SCALE_*, DRM_MODE_DITHERING, DRM_MODE_LINK_STATUS_*,
> DRM_MODE_ROTATE_*, DRM_MODE_REFLECT_* and DRM_MODE_CONTENT_PROTECTION_*.
> (That's why I said "in theory" when I referred to the "rotation" property
> above.)
> 
> I understand the intent is to allow adding new KMS properties without having to
> update the UAPI headers. However having a nice KMS UAPI for dynamically listing
> all enum entries for a property doesn't forbid us from also exposing the values
> in the UAPI header to make things easier and simpler for user-space.
> 
> Additionally, I've heard Pekka saying that it would be nice to have constants
> for property names in the UAPI headers. Indeed, this would prevent
> hard-to-debug typo issues. I have a very good example of such a typo issue that
> took literally hours to debug, with X11 atoms which also use free-form strings
> like KMS properties [3].
> 
> If we have constants for property names in UAPI headers, it wouldn't be a big
> hurdle to also have constants for enum values alongside.

To clarify, the property names would be of the form

#define DRM_KMS_PROPERTY_KERBLAH "KerBlah"

while enum values would be integers, i.e. the raw values.

Daniel Stone did have a good counter-argument to defining property
names: userspace would be full of

#ifndef DRM_KMS_PROPERTY_KERBLAH
#define DRM_KMS_PROPERTY_KERBLAH "KerBleh"
#endif

anyway as long as they cannot rely on the headers to be recent enough.
(The typo is intentional.)

> Are there any use-cases for defining a standard property which uses different
> enum values depending on the driver/device/object (ie. the same enum value
> can have a different meaning depending on the driver/device/object)?
> 
> At the very least, having a clear policy for both kernel public headers and
> user-space would help a lot. Right now it's unclear for both parties what to do
> regarding enum values.
> 
> What do you think?

I do not think it is unclear at all. You have to query the kernel for
value by string names. Maybe it's not clearly communicated though?

But I also don't have anything against changing that policy, if kernel
maintainers agree.


Thanks,
pq

> 
> Thanks,
> 
> Simon
> 
> [1]: https://github.com/emersion/libliftoff
> [2]: https://github.com/emersion/libliftoff/issues/39
> [3]: https://github.com/swaywm/wlroots/pull/1335

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20200403/81cbeb47/attachment.sig>