dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* KMS enums and bitfields UAPI
@ 2020-04-03 10:15 Simon Ser
  2020-04-03 12:24 ` Pekka Paalanen
  2020-04-13 22:38 ` Simon Ser
  0 siblings, 2 replies; 15+ messages in thread
From: Simon Ser @ 2020-04-03 10:15 UTC (permalink / raw)
  To: DRI Development

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.

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?

Thanks,

Simon

[1]: https://github.com/emersion/libliftoff
[2]: https://github.com/emersion/libliftoff/issues/39
[3]: https://github.com/swaywm/wlroots/pull/1335
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: KMS enums and bitfields UAPI
  2020-04-03 10:15 KMS enums and bitfields UAPI Simon Ser
@ 2020-04-03 12:24 ` Pekka Paalanen
  2020-04-03 12:38   ` Simon Ser
                     ` (2 more replies)
  2020-04-13 22:38 ` Simon Ser
  1 sibling, 3 replies; 15+ messages in thread
From: Pekka Paalanen @ 2020-04-03 12:24 UTC (permalink / raw)
  To: Simon Ser; +Cc: DRI Development


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

On Fri, 03 Apr 2020 10:15:21 +0000
Simon Ser <contact@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


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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: KMS enums and bitfields UAPI
  2020-04-03 12:24 ` Pekka Paalanen
@ 2020-04-03 12:38   ` Simon Ser
  2020-04-03 14:23   ` Adam Jackson
  2020-04-07  8:18   ` Daniel Stone
  2 siblings, 0 replies; 15+ messages in thread
From: Simon Ser @ 2020-04-03 12:38 UTC (permalink / raw)
  To: Pekka Paalanen; +Cc: DRI Development

On Friday, April 3, 2020 2:24 PM, Pekka Paalanen <ppaalanen@gmail.com> wrote:

> On Fri, 03 Apr 2020 10:15:21 +0000
> Simon Ser contact@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.)

Well, it depends on the user-space policy for dependencies. Weston
chooses to only bump its dependencies version when strictly necessary,
so Weston could choose to continue to use the hardcoded strings. OTOH,
other projects like wlroots have a different policy and require the
latest versions of its dependencies, and can use the constants from the
headers.

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

What's unclear right now is that half the enum properties have
constants in the UAPI headers (maybe more than half actually?). So
looking at the UAPI headers it's not clear whether this is just an
oversight (someone forgot to add enum values), or whether the policy
has changed. I only know about the policy because I asked, it's not
documented anywhere.

> But I also don't have anything against changing that policy, if kernel
> maintainers agree.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: KMS enums and bitfields UAPI
  2020-04-03 12:24 ` Pekka Paalanen
  2020-04-03 12:38   ` Simon Ser
@ 2020-04-03 14:23   ` Adam Jackson
  2020-04-06 13:55     ` Simon Ser
  2020-04-07  8:18   ` Daniel Stone
  2 siblings, 1 reply; 15+ messages in thread
From: Adam Jackson @ 2020-04-03 14:23 UTC (permalink / raw)
  To: Pekka Paalanen, Simon Ser; +Cc: DRI Development

On Fri, 2020-04-03 at 15:24 +0300, Pekka Paalanen wrote:
> On Fri, 03 Apr 2020 10:15:21 +0000 Simon Ser <contact@emersion.fr> wrote:
> 
> > 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.)

Why not put this in some external registry and regularly sync it into
drm-next? This seems like an xorgproto kind of problem to me.

- ajax

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: KMS enums and bitfields UAPI
  2020-04-03 14:23   ` Adam Jackson
@ 2020-04-06 13:55     ` Simon Ser
  0 siblings, 0 replies; 15+ messages in thread
From: Simon Ser @ 2020-04-06 13:55 UTC (permalink / raw)
  To: Adam Jackson; +Cc: DRI Development

On Friday, April 3, 2020 4:23 PM, Adam Jackson <ajax@redhat.com> wrote:

> On Fri, 2020-04-03 at 15:24 +0300, Pekka Paalanen wrote:
>
> > On Fri, 03 Apr 2020 10:15:21 +0000 Simon Ser contact@emersion.fr wrote:
> >
> > > 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.)
>
> Why not put this in some external registry and regularly sync it into
> drm-next? This seems like an xorgproto kind of problem to me.

How would that fix the issue of KMS clients which don't want to depend
on too recent dependencies?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: KMS enums and bitfields UAPI
  2020-04-03 12:24 ` Pekka Paalanen
  2020-04-03 12:38   ` Simon Ser
  2020-04-03 14:23   ` Adam Jackson
@ 2020-04-07  8:18   ` Daniel Stone
  2020-04-07  9:26     ` Daniel Vetter
  2 siblings, 1 reply; 15+ messages in thread
From: Daniel Stone @ 2020-04-07  8:18 UTC (permalink / raw)
  To: Pekka Paalanen; +Cc: DRI Development

Hi,

On Fri, 3 Apr 2020 at 13:24, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> On Fri, 03 Apr 2020 10:15:21 +0000 Simon Ser <contact@emersion.fr> wrote:
> > 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.

I'm in the same boat. The existing policy (runtime enum name lookups
are the only correct thing, and the presence of anything else in
headers is merely accidental) seems pretty clear to me. But I'd be
totally fine with changing it too, though it might require a cap to
say that this kernel version lets you use the stable integer
enumerations, and anything else requires runtime lookup.

I had a quick look to see how drivers used properties, and was
pleasantly surprised to see that only the (very old) Intel driver,
VMware and QXL drivers have custom properties. So maybe we don't have
to really worry about vendor-extended properties too much ... though
someone will definitely try to use it on some kind of nightmare vendor
BSP and have to fork libliftoff for it at some point.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: KMS enums and bitfields UAPI
  2020-04-07  8:18   ` Daniel Stone
@ 2020-04-07  9:26     ` Daniel Vetter
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2020-04-07  9:26 UTC (permalink / raw)
  To: Daniel Stone; +Cc: DRI Development

On Tue, Apr 7, 2020 at 10:19 AM Daniel Stone <daniel@fooishbar.org> wrote:
>
> Hi,
>
> On Fri, 3 Apr 2020 at 13:24, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> > On Fri, 03 Apr 2020 10:15:21 +0000 Simon Ser <contact@emersion.fr> wrote:
> > > 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.
>
> I'm in the same boat. The existing policy (runtime enum name lookups
> are the only correct thing, and the presence of anything else in
> headers is merely accidental) seems pretty clear to me. But I'd be
> totally fine with changing it too, though it might require a cap to
> say that this kernel version lets you use the stable integer
> enumerations, and anything else requires runtime lookup.
>
> I had a quick look to see how drivers used properties, and was
> pleasantly surprised to see that only the (very old) Intel driver,
> VMware and QXL drivers have custom properties. So maybe we don't have
> to really worry about vendor-extended properties too much ... though
> someone will definitely try to use it on some kind of nightmare vendor
> BSP and have to fork libliftoff for it at some point.

Not sure what you grepped, but I'm also seeing amdgpu, armada, i915
and gma500, nouveau, radeon, rcar and sti create custom properties.

But yeah we've definitely become a lot better at this, with the
cracked down rules on drm properties and what you all need to supply
before merging is ok.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: KMS enums and bitfields UAPI
  2020-04-03 10:15 KMS enums and bitfields UAPI Simon Ser
  2020-04-03 12:24 ` Pekka Paalanen
@ 2020-04-13 22:38 ` Simon Ser
  2020-04-14 12:24   ` Daniel Vetter
  1 sibling, 1 reply; 15+ messages in thread
From: Simon Ser @ 2020-04-13 22:38 UTC (permalink / raw)
  To: DRI Development

Daniel Vetter, Ville, any thoughts about this?

Thanks,

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: KMS enums and bitfields UAPI
  2020-04-13 22:38 ` Simon Ser
@ 2020-04-14 12:24   ` Daniel Vetter
  2020-04-14 12:34     ` Simon Ser
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2020-04-14 12:24 UTC (permalink / raw)
  To: Simon Ser; +Cc: DRI Development

On Mon, Apr 13, 2020 at 10:38:37PM +0000, Simon Ser wrote:
> Daniel Vetter, Ville, any thoughts about this?

Magic 8ball says "unclear", and I feel like I keep flip-flopping around on
this.

I think best-case outcome here is that we're a) consistent across
compositors and b) document that consensus in the kernel's uapi section
(for lack of better places).

I'm not hung up on what exactly that consensus should be, as long as it's
a consistent across projects. If you folks can't figure this out I'll do a
live youtube sessions and throw a dice :-P

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: KMS enums and bitfields UAPI
  2020-04-14 12:24   ` Daniel Vetter
@ 2020-04-14 12:34     ` Simon Ser
  2020-04-14 12:39       ` Daniel Vetter
  0 siblings, 1 reply; 15+ messages in thread
From: Simon Ser @ 2020-04-14 12:34 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: DRI Development

On Tuesday, April 14, 2020 2:24 PM, Daniel Vetter <daniel@ffwll.ch> wrote:

> On Mon, Apr 13, 2020 at 10:38:37PM +0000, Simon Ser wrote:
>
> > Daniel Vetter, Ville, any thoughts about this?
>
> Magic 8ball says "unclear", and I feel like I keep flip-flopping around on
> this.
>
> I think best-case outcome here is that we're a) consistent across
> compositors and b) document that consensus in the kernel's uapi section
> (for lack of better places).

Agreed.

> I'm not hung up on what exactly that consensus should be, as long as it's
> a consistent across projects. If you folks can't figure this out I'll do a
> live youtube sessions and throw a dice :-P

It seems like everyone's fine with whatever decision we make as long as
we make one. :P

I guess I'll summarize again my main point here: requiring user-space
to use the KMS API to get enum values just makes it more difficult for
user-space to use KMS. I can't think of any reason why the kernel would
want to use different enum values for a standard property.

Does anybody remember if there was such a use-case when this UAPI was
introduced?

Thanks,

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: KMS enums and bitfields UAPI
  2020-04-14 12:34     ` Simon Ser
@ 2020-04-14 12:39       ` Daniel Vetter
  2020-04-14 13:25         ` Simon Ser
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2020-04-14 12:39 UTC (permalink / raw)
  To: Simon Ser; +Cc: DRI Development

On Tue, Apr 14, 2020 at 12:34:17PM +0000, Simon Ser wrote:
> On Tuesday, April 14, 2020 2:24 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Mon, Apr 13, 2020 at 10:38:37PM +0000, Simon Ser wrote:
> >
> > > Daniel Vetter, Ville, any thoughts about this?
> >
> > Magic 8ball says "unclear", and I feel like I keep flip-flopping around on
> > this.
> >
> > I think best-case outcome here is that we're a) consistent across
> > compositors and b) document that consensus in the kernel's uapi section
> > (for lack of better places).
> 
> Agreed.
> 
> > I'm not hung up on what exactly that consensus should be, as long as it's
> > a consistent across projects. If you folks can't figure this out I'll do a
> > live youtube sessions and throw a dice :-P
> 
> It seems like everyone's fine with whatever decision we make as long as
> we make one. :P
> 
> I guess I'll summarize again my main point here: requiring user-space
> to use the KMS API to get enum values just makes it more difficult for
> user-space to use KMS. I can't think of any reason why the kernel would
> want to use different enum values for a standard property.
> 
> Does anybody remember if there was such a use-case when this UAPI was
> introduced?

I just rang across one, and boy does it suck.

So we're trying to standardize across drivers as much as possible. Within
the kernel we do that by decoding standardized properties directly into
state structures (including any backwards compat hacks), and outside of
the kernel by requiring igts so compliance across drivers can be tested.

But we still have a pile of legacy properties, and there's pure wild west
out there. Some have mispelled version of the same stuff, some have same
naming but different values. If userspace hardcodes values then we're more
screwed than if we have some indirection here to remap to standardized
properties. And legacy userspace did do that full remapping dance, because
that's how the first X property decoder for connectors was coded.

So given that I think everyone should do the symbolic decoding, so that we
can more seamlessly upgrade when we standardize props.

Like I said, I'm flip-flopping on this all the time, but since I just ran
over an example of trying to standardize another one of the old horrors,
maybe better to make that slightly easier going forward. Userspace should
be able to just stuff this all into a library and be done.

Volunteered to write the kernel patch?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: KMS enums and bitfields UAPI
  2020-04-14 12:39       ` Daniel Vetter
@ 2020-04-14 13:25         ` Simon Ser
  2020-04-14 13:33           ` Daniel Vetter
  0 siblings, 1 reply; 15+ messages in thread
From: Simon Ser @ 2020-04-14 13:25 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: DRI Development

On Tuesday, April 14, 2020 2:39 PM, Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Apr 14, 2020 at 12:34:17PM +0000, Simon Ser wrote:
>
> > On Tuesday, April 14, 2020 2:24 PM, Daniel Vetter daniel@ffwll.ch wrote:
> >
> > > On Mon, Apr 13, 2020 at 10:38:37PM +0000, Simon Ser wrote:
> > >
> > > > Daniel Vetter, Ville, any thoughts about this?
> > >
> > > Magic 8ball says "unclear", and I feel like I keep flip-flopping around on
> > > this.
> > > I think best-case outcome here is that we're a) consistent across
> > > compositors and b) document that consensus in the kernel's uapi section
> > > (for lack of better places).
> >
> > Agreed.
> >
> > > I'm not hung up on what exactly that consensus should be, as long as it's
> > > a consistent across projects. If you folks can't figure this out I'll do a
> > > live youtube sessions and throw a dice :-P
> >
> > It seems like everyone's fine with whatever decision we make as long as
> > we make one. :P
> > I guess I'll summarize again my main point here: requiring user-space
> > to use the KMS API to get enum values just makes it more difficult for
> > user-space to use KMS. I can't think of any reason why the kernel would
> > want to use different enum values for a standard property.
> > Does anybody remember if there was such a use-case when this UAPI was
> > introduced?
>
> I just rang across one, and boy does it suck.
>
> So we're trying to standardize across drivers as much as possible. Within
> the kernel we do that by decoding standardized properties directly into
> state structures (including any backwards compat hacks), and outside of
> the kernel by requiring igts so compliance across drivers can be tested.
>
> But we still have a pile of legacy properties, and there's pure wild west
> out there. Some have mispelled version of the same stuff, some have same
> naming but different values. If userspace hardcodes values then we're more
> screwed than if we have some indirection here to remap to standardized
> properties. And legacy userspace did do that full remapping dance, because
> that's how the first X property decoder for connectors was coded.
>
> So given that I think everyone should do the symbolic decoding, so that we
> can more seamlessly upgrade when we standardize props.
>
> Like I said, I'm flip-flopping on this all the time, but since I just ran
> over an example of trying to standardize another one of the old horrors,
> maybe better to make that slightly easier going forward. Userspace should
> be able to just stuff this all into a library and be done.

What I'm suggesting isn't to make all enum values UAPI. I'm suggesting
to add standard enum values as #defines in the UAPI headers to make
these values UAPI. Non-standard properties wouldn't be in the UAPI
headers, so user-space would need to query values from KMS just like
they do now.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: KMS enums and bitfields UAPI
  2020-04-14 13:25         ` Simon Ser
@ 2020-04-14 13:33           ` Daniel Vetter
  2020-04-15 10:40             ` Simon Ser
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2020-04-14 13:33 UTC (permalink / raw)
  To: Simon Ser; +Cc: DRI Development

On Tue, Apr 14, 2020 at 3:25 PM Simon Ser <contact@emersion.fr> wrote:
>
> On Tuesday, April 14, 2020 2:39 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> > On Tue, Apr 14, 2020 at 12:34:17PM +0000, Simon Ser wrote:
> >
> > > On Tuesday, April 14, 2020 2:24 PM, Daniel Vetter daniel@ffwll.ch wrote:
> > >
> > > > On Mon, Apr 13, 2020 at 10:38:37PM +0000, Simon Ser wrote:
> > > >
> > > > > Daniel Vetter, Ville, any thoughts about this?
> > > >
> > > > Magic 8ball says "unclear", and I feel like I keep flip-flopping around on
> > > > this.
> > > > I think best-case outcome here is that we're a) consistent across
> > > > compositors and b) document that consensus in the kernel's uapi section
> > > > (for lack of better places).
> > >
> > > Agreed.
> > >
> > > > I'm not hung up on what exactly that consensus should be, as long as it's
> > > > a consistent across projects. If you folks can't figure this out I'll do a
> > > > live youtube sessions and throw a dice :-P
> > >
> > > It seems like everyone's fine with whatever decision we make as long as
> > > we make one. :P
> > > I guess I'll summarize again my main point here: requiring user-space
> > > to use the KMS API to get enum values just makes it more difficult for
> > > user-space to use KMS. I can't think of any reason why the kernel would
> > > want to use different enum values for a standard property.
> > > Does anybody remember if there was such a use-case when this UAPI was
> > > introduced?
> >
> > I just rang across one, and boy does it suck.
> >
> > So we're trying to standardize across drivers as much as possible. Within
> > the kernel we do that by decoding standardized properties directly into
> > state structures (including any backwards compat hacks), and outside of
> > the kernel by requiring igts so compliance across drivers can be tested.
> >
> > But we still have a pile of legacy properties, and there's pure wild west
> > out there. Some have mispelled version of the same stuff, some have same
> > naming but different values. If userspace hardcodes values then we're more
> > screwed than if we have some indirection here to remap to standardized
> > properties. And legacy userspace did do that full remapping dance, because
> > that's how the first X property decoder for connectors was coded.
> >
> > So given that I think everyone should do the symbolic decoding, so that we
> > can more seamlessly upgrade when we standardize props.
> >
> > Like I said, I'm flip-flopping on this all the time, but since I just ran
> > over an example of trying to standardize another one of the old horrors,
> > maybe better to make that slightly easier going forward. Userspace should
> > be able to just stuff this all into a library and be done.
>
> What I'm suggesting isn't to make all enum values UAPI. I'm suggesting
> to add standard enum values as #defines in the UAPI headers to make
> these values UAPI. Non-standard properties wouldn't be in the UAPI
> headers, so user-space would need to query values from KMS just like
> they do now.

Hm that sounds like the half-way that wont work. Because then some
compositors will only use the hard-coded versions, and if they don't
have them, nag us to add them. And then be really disappointed if we
don't (or we screw up and add them where we shouldn't). That's the
status quo "let's have it both ways" that I think is the worst of all
options we have. So I guess from that pov the "userspace needs to
decode from symbolic values, always" as the only consistent one.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: KMS enums and bitfields UAPI
  2020-04-14 13:33           ` Daniel Vetter
@ 2020-04-15 10:40             ` Simon Ser
  2020-04-16  8:51               ` UAPI doc (Re: KMS enums and bitfields UAPI) Pekka Paalanen
  0 siblings, 1 reply; 15+ messages in thread
From: Simon Ser @ 2020-04-15 10:40 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: DRI Development

On Tuesday, April 14, 2020 3:33 PM, Daniel Vetter <daniel@ffwll.ch> wrote:

> > What I'm suggesting isn't to make all enum values UAPI. I'm suggesting
> > to add standard enum values as #defines in the UAPI headers to make
> > these values UAPI. Non-standard properties wouldn't be in the UAPI
> > headers, so user-space would need to query values from KMS just like
> > they do now.
>
> Hm that sounds like the half-way that wont work. Because then some
> compositors will only use the hard-coded versions, and if they don't
> have them, nag us to add them. And then be really disappointed if we
> don't (or we screw up and add them where we shouldn't). That's the
> status quo "let's have it both ways" that I think is the worst of all
> options we have. So I guess from that pov the "userspace needs to
> decode from symbolic values, always" as the only consistent one.

Fair enough. Let's just continue using symbolic names then.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* UAPI doc (Re: KMS enums and bitfields UAPI)
  2020-04-15 10:40             ` Simon Ser
@ 2020-04-16  8:51               ` Pekka Paalanen
  0 siblings, 0 replies; 15+ messages in thread
From: Pekka Paalanen @ 2020-04-16  8:51 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: DRI Development


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

On Wed, 15 Apr 2020 10:40:36 +0000
Simon Ser <contact@emersion.fr> wrote:

> On Tuesday, April 14, 2020 3:33 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > > What I'm suggesting isn't to make all enum values UAPI. I'm suggesting
> > > to add standard enum values as #defines in the UAPI headers to make
> > > these values UAPI. Non-standard properties wouldn't be in the UAPI
> > > headers, so user-space would need to query values from KMS just like
> > > they do now.  
> >
> > Hm that sounds like the half-way that wont work. Because then some
> > compositors will only use the hard-coded versions, and if they don't
> > have them, nag us to add them. And then be really disappointed if we
> > don't (or we screw up and add them where we shouldn't). That's the
> > status quo "let's have it both ways" that I think is the worst of all
> > options we have. So I guess from that pov the "userspace needs to
> > decode from symbolic values, always" as the only consistent one.  
> 
> Fair enough. Let's just continue using symbolic names then.

Hi,

I'm happy to see a decision made that is clear and simple, even if it
makes userspace need a little more code.

We as userspace programmers have a huge responsibility of figuring out
exactly how KMS UAPI is supposed to be used, and use it correctly,
because the UAPI is largely undocumented and whatever documentation
exists is not easily discoverable to a non-kernel developer.

Libdrm has a few man-pages that seem to have been left to rot. If I
could, bringing them up-to-date and extending to cover everything about
driver-agnostic KMS UAPI would seem like a nice idea. It would be the
obvious place for userspace oriented docs. After all, UAPI docs cannot
become incorrect over time (the stable UAPI promise), so the
maintenance effort would be just adding the new stuff and if necessary,
clarifying old stuff. No confusion with any kernel internal API that is
subject to change at any time.

https://www.kernel.org/doc/html/latest/ is really hard to find anything
in it when you need it. Often the terms you need to search for are not
the ones used in the UAPI or have far too many hits, assuming you know
what UAPI you would want to use in the first place.

Another disconnect is that libdrm API is not the UAPI exactly. So
documenting libdrm API with the kernel UAPI in mind would be the best.
People use libdrm more than kernel UAPI.


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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-04-16  8:51 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-03 10:15 KMS enums and bitfields UAPI Simon Ser
2020-04-03 12:24 ` Pekka Paalanen
2020-04-03 12:38   ` Simon Ser
2020-04-03 14:23   ` Adam Jackson
2020-04-06 13:55     ` Simon Ser
2020-04-07  8:18   ` Daniel Stone
2020-04-07  9:26     ` Daniel Vetter
2020-04-13 22:38 ` Simon Ser
2020-04-14 12:24   ` Daniel Vetter
2020-04-14 12:34     ` Simon Ser
2020-04-14 12:39       ` Daniel Vetter
2020-04-14 13:25         ` Simon Ser
2020-04-14 13:33           ` Daniel Vetter
2020-04-15 10:40             ` Simon Ser
2020-04-16  8:51               ` UAPI doc (Re: KMS enums and bitfields UAPI) Pekka Paalanen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).