Hi,
On 08/24/2015 07:56 PM, Michael Spang wrote:
On Mon, Aug 24, 2015 at 1:42 PM, Thomas Hellstrom thellstrom@vmware.com wrote:
On 08/24/2015 07:12 PM, Daniel Stone wrote:
Hi,
On 24 August 2015 at 18:10, Thomas Hellstrom thellstrom@vmware.com wrote:
On 08/24/2015 07:04 PM, Daniel Stone wrote:
On 24 August 2015 at 17:56, Thomas Hellstrom thellstrom@vmware.com wrote:
On 08/24/2015 05:52 PM, Daniel Stone wrote: > I still don't think this ameliorates the need for batching: consider > the case where you update two disjoint screen regions and want them > both flushed. Either you issue two separate sync calls (which can be > disadvantageous performance-wise on some hardware / setups), or you > accumulate the regions and only flush later. So either two ioctls (one > in the style of dirtyfb and one to perform the sync/flush; you can > shortcut to assume the full buffer was damaged if the former is > missing), or one like this: > > struct dma_buf_sync_2d { > enum dma_buf_sync_flags flags; > > __u64 stride_bytes; > __u32 bytes_per_pixel; > __u32 num_regions; > > struct { > __u64 x; > __u64 y; > __u64 width; > __u64 height; > } regions[]; > }; Fine with me, although perhaps bytes_per_pixel is a bit redundant?
Redundant how? It's not implicit in stride.
For flushing purposes, isn't it possible to cover all cases by assuming bytes_per_pixel=1? Not that it matters much.
Sure, though in that case best to replace x with line_byte_offset or something, because otherwise I guarantee you everyone will get it wrong, and it'll be a pain to track down. Like how I managed to misread it now. :)
OK, yeah you have a point. IMO let's go for your proposal.
Tiago, is this OK with you?
Is there any obstacle to making the above API a new syscall?
I think that whether this API can be accepted as a syscall or not is really up to Dave and in the end Linus. What speaks against is that it's not applicable to all file types, only dma-bufs and also that we might want to extend it in the future.
The reason we have issues with ioctls is because it's not possible to whitelist them properly with seccomp BPF - there's no single namespace for ioctls.
If this API is merged as a ioctl, we may not be able to actually use it. Which is a bit unfortunate when it has been proposed with the chrome renderer use case in mind.
I don't think mmap() on dma-bufs can be accepted as a _generic_ API without a mandatory sync mechanism, and I think there is a need to specify a sync api regardless of who is the first user.
Thanks, Thomas
Michael