[PATCH wayland-protocols] Add the wl_drm protocol

Tue Nov 14 11:23:22 UTC 2017

On 14 November 2017 at 08:18, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> On Mon, 13 Nov 2017 16:27:24 +0000
> Emil Velikov <emil.l.velikov at gmail.com> wrote:
>
>> On 13 November 2017 at 14:52, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> > On 13 November 2017 at 14:21, Daniel Stone <daniel at fooishbar.org> wrote:
>> >> Hi Emil,
>> >>
>> >> On 2 November 2017 at 17:09, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> >>> Import latest version (v2) of the protocol from Mesa.
>> >>>
>> >>> From the README:
>> >>>
>> >>> Warning!
>> >>> The goal is to share the protocol file across Mesa and other low-level
>> >>> components graphics stack such as libva and Xwayland. File is moved to
>> >>> wayland-protocols for sharing purposes _only_.
>> >>>
>> >>> The protocol is _not_ for public use. Furthermore Mesa and others consider
>> >>> the protocol as deprecated over wl_dmabuf.
>> >>
>> >> I suspect the sheer fact that we've recently added it as a stable
>> >> published protocol would be enough for people to ignore that
>> >> objection.
>> >>
>> >> I have to say that I'm still against this one, for a few reasons.
>> >>
>> >> Firstly, yes, VA-API got it wrong: there's no way it should be using
>> >> unpublished private protocol. Luckily, newer versions of VA-API and
>> >> the GStreamer integration now have both proper dmabuf integration as
>> >> well as the notion of simply exporting buffers rather than trying to
>> >> be a presentation _and_ decoding layer in one, so using a VA-API
>> >> decoder piped to waylandsink is enough to get it working without
>> >> wl_drm ever being used. Happy days. (Xwayland is in the same boat:
>> >> lfrb's patches mean it can use dmabuf instead.)
>> >>
>> >> Secondly, apart from VA-API, it's not used generically. It gets
>> >> published as a side effect of Mesa calling eglBindWaylandDisplayWL(),
>> >> and the Mesa client code (both in libEGL.so, though the two versions
>> >> can indeed get out of sync) uses it. Compositors don't explicitly
>> >> advertise it, and again for the most part clients don't use it.
>> >>
>> >> What I worry is that, if we publish it, clients _will_ come to use it
>> >> despite the warnings, and Mesa will never be able to stop advertising
>> >> it. Having it be private/unpublished protocol is an implementation
>> >> detail, but with a published protocol doc, the fact Mesa advertises it
>> >> will come to be seen as ABI instead. I really, really, don't want to
>> >> paint ourselves into that corner: wl_drm has always been an awkward
>> >> special case, but I'd much prefer to kill it off than enshrine its
>> >> position.
>> >>
>> >> There is one gap which zwp_linux_dmabuf_v1 doesn't fill, which is
>> >> passing a device handle for the composition GPU, as well as maybe
>> >> another for the scanout device. That would give the allocator enough
>> >> information to determine correct buffer placement and so on. But if we
>> >> ignore placement as we do today, just having format + modifiers would
>> >> be enough for both EGL/Vulkan and VA-API clients which were aware of
>> >> modifiers to work properly. Drivers without modifiers would still need
>> >> the GPU device passed down so they can decide which magic tiling mode
>> >> to use, but that'll go as they get converted over.
>> >>
>> >> I'd much prefer to see wl_drm go completely unused where possible on
>> >> the client side first, then work out a transition plan for Mesa
>> >> deprecating it, then we can say with a straight face that we're not
>> >> publishing it because we expect it to disappear.
>> >>
>> > Thanks for the elaborate answer Dan.
>> >
>> > Being the more paranoid person (me thinks), I doubt anyone outside of
>> > existing users will use wl_drm.
>> > I do see your concern though - let me see if I can figure out a way to
>> > make it harder to misuse.
>> >
>> > What I am planning here is the de-facto deprecation of wl_drm. It's
>> > somewhat evil, so I kept is half-hidden.
>> > Here it is, in all its [evil] glory:
>> >  - step 1: move wl_drm to wayland-protocols, throw as many obstacles
>> > needed to prevent average Joe from using it
>> >  - 2: switch users {Mesa...} to it
>> >  - 3: in parallel with #2, propose/shout at people that wl_drm will be
>> > gone soon (tm)
>> >  - 4: after users have working wl_dmabuf implementation, kill off
>> > wl_drm from wayland-protocols
>> >  - 5: roll new wayland-protocols, bump the requirement in users
>> >  - 6: drop 'dead' code from users
>> >  - ...
>> >  - Profit!
>> >
>> > How does that sound? I could add that to the commit message if you prefer.
>> > FTR Pekka was asking why Mesa doesn't ship/install the file - because
>> > it leads to a circular dependency.
>> > Plus people are more likely to misuse it, in said case.
>> >
>> Some more ideas that come to mind:
>>
>> * by moving the XML we do not make it stable, the protocol already is
>>
>> * users already depend on wayland-protocols ... to a degree
>>
>> * moving wl_drm serves as a lovely reminder, same as the version bump later on
>> "We really want to update $user _away_ from use wl_drm"
>> "We want to test the !wl_drm codepaths more extensively"
>
> Hi Emil,
>
> sorry, I cannot see it doing that. What I would see from moving wl_drm
> into wayland-protocols is "It is actually ok to start using wl_drm in
> arbitrary programs, so we are making the protocol definition easily
> accessible for everyone." Because that is the whole point of
> wayland-protocols: promote extensions that everyone should use or at
> least consider instead of making up custom ones. The protocols marked
> as stable especially are that.
>
The keywords we seems to be missing are public vs private.
How about we add those notions and associate wl_drm with the latter?

> Another thing I would imagine user projects to think is: "Oh good, I
> can stop carrying the copy I had." If they do that, and then you
> remove, rename, or depracate wl_drm in wayland-protocols, the projects
> will just get annoyed by the useless motion they were tricked into and
> move back to having an internal copy of wl_drm.
>
> You can cause a lot of code churn by renaming wl_drm interfaces, but it
> is superfluous. The same could be achieved by simply making Mesa not
> advertise wl_drm at runtime.
>
I'm not proposing any renaming, because as you mentioned - it doesn't help much.

Can you throw a quick plan, vaguely like the one I did earlier?
One that covers deprecation and removal* of wl_drm.

>>
>> * adding a single toggle in wayland-protocols instead of
>> --disable-wl-drm in each of three projects
>> --hack-hack-I-want-wl-drm-but-I-use-if-only-for-mesa-vaapu-xwayland
>>
>
> As explained above, that would be a useless side-step. We need such a
> switch for Mesa runtime, not wayland-protocols build time.
>
Why runtime? You mention it a few times throughout, but I'm failing to
see any arguments behind it.
There are a few other gotchas mentioned throughout tagged with *.

> The projects using wl_drm as a client must already deal with the fact
> that wl_drm might not be advertised by the server at runtime. Then it
> is client project's choice if they just seize to work at all or use
> something else.
>
That is correct - clients _should_ know how to deal with it, sadly
they do _not_.
Even if we fix the current instances we cannot retroactively fit older
releases ;-(

Now combine that with the fact that wayland client/server libs (+
headers) will gladly deref NULL pointers and things get extra 'fun'.

Looking from the POV of the average Joe - the above breakages are hard
to debug and will lead to a lot of noise/bugreports.
By using wayland-protocols, builders/packagers will effectively pair
components that can work together.

Additionally they will ensure an extra baseline testing, before
shipping to users.
With the runtime toggle:
 - the pairing will be missing - trying to use A with (or w/o) wl_foo with crash
 - different parties will test different codepaths ... not ideal

> What we need is to stop Mesa from advertising wl_drm on the Wayland
> server side, with perhaps an opt-in "yes, I as a human user really do
> use and need programs that rely on it" for the transition period.
>
This proposal might work but we're looking at deprecation period a lot
larger* than with my proposal.

>> * explicit and annoying enable ^^ ensures
>> a) people are less likely to misuse
>> b) people use wl_dmabuf by default
>
> If the enable was at Mesa runtime, not wayland-protocols build time,
> yes. Otherwise no.
>
Please elaborate*.

>> * combinatoric explosion
>> With separate toggles we have - mesa with wl_drm & vaapi w/o wl_drm etc
>> Bump wayland-protocols req. across the board - everything is
>> consistently built w/o wl_drm ;-)
>
> I can't see why build options matter, when Wayland already requires all
> clients to deal with the availability of an extension at runtime. A
> project cannot use wl_drm, if a Wayland server does not advertise it.
>
Original assumption was that each user will have their own toggle -
from that POV it does not matter if it's a build or runtime one.
Even if the client correctly handles wl_drm presence* we end up with
combinatoric explosion. Which tends to be harder and slower to debug.

HTH
Emil