[PATCH 1/7] protocol: add linux_dmabuf extension RFCv1

[Pekka Paalanen]

On Tue, 20 Jan 2015 07:41:52 +0100
Daniel Vetter <daniel at ffwll.ch> wrote:

> On Thu, Jan 08, 2015 at 02:26:00PM +0200, Pekka Paalanen wrote:
> > On Mon, 5 Jan 2015 11:44:49 +0100
> > Daniel Vetter <daniel at ffwll.ch> wrote:
> > 

> > > But as long as you don't do crazy things the drm dma-buf import will
> > > notice that it's the same dma-buf and dtrt. Crazy = actively trying to
> > > reimport buffers while some other thread is dropping the last drm
> > > use-count for the same.
> > 
> > I'm not sure how one could even attempt such things.
> 
> You need your dma-buf to come from some other device (e.g. v2l pipeline),
> otherwise it's impossible. And if it ever becomes an issue userspace
> shouldn't ever care since the problem is in the kernel really.
> 
> > > > I assumed it could be a problem, so I designed a way that maintains the
> > > > numerical identities between the fds.
> > > > 
> > > > Also, we don't have a mechanism to send "not a valid fd" for an fd
> > > > field, in case there are less than 3 (4?) planes, so we need a separate
> > > > request for each valid fd.
> > > 
> > > Maybe we could share the drm helpers to compute the number of planes
> > > somehow and make it part of the proto? tbh no idea whether that would help
> > > you (I have no clue about wl protos after all ...).
> > 
> > Do you mean compute the number of planes from a DRM fourcc code? I
> > can't see how that would help protocol-wise. We have no such thing
> > as variable arguments. Every argument listed in the XML
> > specification of a message must be there exactly. If an argument is
> > specified to be an fd, it must also be a valid fd at runtime AFAIK.
> > 
> > To send 1-4 fds, one needs to either pick a message type (request)
> > that specifies exactly the right number of fds, or use a single
> > message type to send each fd one by one. The difference in IPC
> > efficiency is negligible, I believe.
> > 
> > I suppose one could use a single message type with 4 fds always,
> > just repeating the unwanted arguments as one of the wanted fds, but
> > it feels... ugly and inefficient (extra system calls to transmit
> > extra fds), especially if we usually need just one fd.
> > 
> > At least we could simplify the dmabuf_batch interface by leaving
> > just one offset/stride argument pair in the "add" request. That
> > would make it a lot more clear.
> > 
> > And we should bump the limit to 4 dmabufs per wl_buffer, like you
> > pointed out from AddFB2. The above simplification makes this very
> > easy. In fact, we wouldn't even need to specify any max number in
> > the protocol at all which is nice.
> 
> Well I was more thinking about the validation the wayland server is
> supposed to do, not necessarily whether the protocol allows it. I.e. match
> the input validation to what the drm core does (checks for number of
> planes, that kind of stuff). Otoh I guess you could just do the addfb2
> synchronously and reuse the checking the kernel does by forwarding the
> -EINVAL.
> 
> I just thinking that some spec language along "the kernel's drm subsystem
> is the authoritative sources for how these fourcc codes work" would be
> good.

Ah. Foremost the compositor needs to guarantee that the wl_buffer
(a set of dmabufs and metadata) is usable in GL, so I would expect
the import checks done in EGL should be enough. If AddFB2 has
additional restrictions, that's fine: the compositor just can't
scan out the wl_buffer, but it can still composite it.

In the Wayland protocol extension, we would probably just say
something like "if the compositor cannot use this buffer, you get a
failure back instead of creating a wl_buffer object" or such.
Practically the checking would be done by the EGL implementation.

Of course, using a DRM fourcc code with a wrong number of planes
could be a fatal protocol error, just like using invalid offset or
stride is an indication of a bug rather than just an unsupported
format. That's a worthy idea to think about indeed.

How would we get this kind of centralized collection of DRM fourcc
format descriptions to run checks with?


Thanks,
pq