[Linaro-mm-sig] [RFC] dma-shared-buf: Add buffer sharing framework

[Daniel Vetter]

On Thu, Aug 11, 2011 at 05:02:21AM -0500, Clark, Rob wrote:
> On Wed, Aug 10, 2011 at 7:11 AM, Sumit Semwal <sumit.semwal at ti.com> wrote:
> >
> > +/**
> > + * struct dma_buf_ops - operations possible on struct dmabuf
> > + * @get_scatterlist: returns list of scatter pages allocated, increases
> > + *                  usecount of the buffer
> > + * @put_scatterlist: decreases usecount of buffer, might deallocate scatter
> > + *                  pages
> > + * @mmap: map this buffer
> > + * @read: read from this buffer
> > + * @write: write to this buffer
> > + * @release: release this buffer; to be called after the last dma_buf_put
> > + * @sync_sg_for_cpu: sync the sg list for cpu
> > + * @sync_sg_for_device: synch the sg list for device
> > + */
> > +struct dma_buf_ops {
> > +       /* allow buffer to not be pinned when DMA is not happening */
> > +       struct scatterlist * (*get_scatterlist)(struct dma_buf *);
> > +       void (*put_scatterlist)(struct dma_buf *, struct scatterlist *);
> 
> One thought, {get,put}_scatterlist() gives the allocating driver half
> of what it needs to implement a sync object.. ie. if receiving driver
> does a "get" before DMA, and "put" on completion, this tells the
> allocating driver that the buffer is in use.  If we added an 'op' to
> get_scatterlist which was READ &/| WRITE this would even tell the
> allocator the nature of the DMA which could later be used to implement
> a wait(op).
> 
> Another thought.. at the risk of re-opening a can of worms..  an
> 'attrib' flag/mask as a param to get_scatterlist() could allow the
> allocator to defer backing the buffer w/ real memory until the first
> get_scatterlist() call, for now attrib flag could be discontig,
> contig, and weird (as Hans suggested) and we can try to define more
> precisely "weird" later but for now can have platform specific meaning
> perhaps.  This gives a sort of "poor mans negotiation" which might
> help on some platforms for now..

You've probably discussed this all at the recent summit I've totally
missed, but why not add a struct device * pointer argument to
get_scatterlist? This way we could shovel all the platform specific
complexity into platform specific code.

For devices that have different dma requirements depending upon function
(i.e. contigous for scanout, iommu-mapped with large pages for overlay
image) we could use a bunch of subdevices to map all the different
requirements.

Another similar idea could also solve the backing storage negotiation
problem: A dma_buf object is at first just an abstract handle (with a
fixed size), then you can attach users with dma_buf_attach_device.
Then, after all users have themselves attached, the first get_scatterlist
call actually allocates the backing storage for the buffer. A subsequent
attach_device would then return -EBUSY.

This way we don't have a special device that allocates a buffer but
instead all the knowledge about dma buffer handling sits in the platform
specific dma code. With that design get_scatterlist should imo then also
return an already mapped sg list, i.e. what dma_map_sg would return.
Probably rename them to dma_map_buffer/dma_unmap_buffer.

In short, I imagine an api like this (neglecting the actual buffer sharing
part and reference counting boiler plate):

struct dma_buf * dma_buf_create(size_t size);
int dma_buf_attach_device(struct dma_buf *buf, struct device *device);
	/* might return -EBUSY to signal that the backing storage is
	 * already allocated and incompatible with the device's
	 * requirements */
struct scatterlist *dma_buf_map(struct dma_buf *buf, struct device *device, int *ents);
	/* not allowed without a preceeding attach_device */
void dma_buf_unmap(struct dma_buf *buf, struct device *device, 
		   struct scatterlist *sg, int ents);

Yours, Daniel
-- 
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48