On Thu, Jun 7, 2012 at 8:52 AM, Erik Gilling konkers@android.com wrote:
On Wed, Jun 6, 2012 at 2:14 PM, Clark, Rob rob@ti.com wrote:
If the sync primitives are backed by file descriptors (or attached to dma_bufs) you don't need an extra char dev either.
Sure.. and just fwiw, I'm partially playing devil's advocate here (because whatever kernel<->user APIs get added, we have to maintain compatibility for a long time.. so best to start w/ fewer interfaces to userspace when possible, since it is always easier to add new interfaces than to take one away).
That has to be balanced with having enough functionality to be useful in a real system or it won't get used.
sure, and I don't expect android to sit around and wait multiple merge window cycles for things to get to a point of having enough functionality to be used, but android doesn't really have the same backwards compatibility guarantees
And btw, why not eventfd? (Ok, I looked at possibly an earlier version of syncpoints and only just briefly at what you sent to the list earlier today, so if that has changed pls disregard)
I haven't looked at eventfd before and just did a quick read-through of it. The only place I could see using it is as the userspace notification mechanism which is a pretty small part of the whole equation. I'd be interested to hear if you think it could be used in a broader way.
I was just thinking for the notification to userspace part.. so it is only a small part but seems like it could be useful for that small part
The bigger issue is the previous point about how to deal with cases where the CPU doesn't really need to get involved as an intermediary.
CPU fallback access to the buffer is the only legit case where we need a standardized API to userspace (since CPU access isn't already associated w/ some other kernel device file where some extra ioctl can be added)
The CPU case will still need to wait on an arbitrarily backed sync primitive. It shouldn't need to know if it's backed by the gpu, camera, or dsp.
Right, this is the one place we definitely need something.. some userspace code would just get passed a dmabuf file descriptor and want to mmap it and do something, without really knowing where it came from. I *guess* we'll have to add some ioctl's to the dmabuf fd.
I personally favor having sync primitives have their own anon inode vs. strictly coupling them with dma_buf.
or possible it could be an ioctl in dmabuf to return an sync fd? I do think we want a way to at least optionally stuff a sync obj onto a dmabuf, because pre-existing non android userspace is expecting implicit synchronization in a number of places, and we can't really go breaking all those interfaces. But I don't think that should prevent also being able to do explicit synchronization.
BR, -R
Although I suppose it doesn't have to be part of the first patchset, it would be easy enough to add later if we had to.
This is certainly a case where, without this functionality, we won't use what ever the solution is.
Cheers, Erik