On Tue, Jan 20, 2015 at 10:31:52AM +0200, Pekka Paalanen wrote: > On Tue, 20 Jan 2015 07:41:52 +0100 > Daniel Vetter <dan...@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 <dan...@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?
Copypaste from kernel maybe, perhaps after some refactoring to extract the fourcc code checking into a mostly standalone drm_fourcc.c (functions like drm_format_num_planes or anything else used by the input check code for the addfb2 ioctl). Not a pretty approach, but I don't have a better idea really. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel