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

[Hans Verkuil]

On Wednesday, September 28, 2011 12:52:58 Daniel Vetter wrote:
> On Wed, Sep 28, 2011 at 10:51:08AM +0200, Hans Verkuil wrote:
> > On Tuesday, September 27, 2011 16:19:56 Daniel Vetter wrote:
> > > Hi Hans,
> > > 
> > > I'll try to explain a bit, after all I've been pushing this attachment
> > > buisness quite a bit.
> > > 
> > > On Tue, Sep 27, 2011 at 03:24:24PM +0200, Hans Verkuil wrote:
> > > > OK, it is not clear to me what the purpose is of the attachments.
> > > > 
> > > > If I understand the discussion from this list correctly, then the idea is
> > > > that each device that wants to use this buffer first attaches itself by
> > > > calling dma_buf_attach().
> > > > 
> > > > Then at some point the application asks some driver to export the buffer.
> > > > So the driver calls dma_buf_export() and passes its own dma_buf_ops. In
> > > > other words, this becomes the driver that 'controls' the memory, right?
> > > 
> > > Actually, the ordering is the other way round. First, some driver calls
> > > dam_buf_export, userspace then passes around the fd to all other drivers,
> > > they do an import and call attach. While all this happens, the driver that
> > > exported the dma_buf does not (yet) allocate any backing storage.
> > 
> > Ah. This really should be documented better :-)
> > 
> > > > Another driver that receives the fd will call dma_buf_get() and can then
> > > > call e.g. get_scatterlist from dma_buf->ops. (As an aside: I would make
> > > > inline functions that take a dma_buf pointer and call the corresponding
> > > > op, rather than requiring drivers to go through ops directly)
> > > 
> > > Well, drivers should only call get_scatterlist when they actually need to
> > > access the memory. This way the originating driver can go through the list
> > > of all attached devices and decide where to allocate backing storage on
> > > the first get_scatterlist.
> > 
> > Unless I am mistaken there is nothing there in the attachment datastructure
> > at this moment to help determine the best device to use for the scatterlist.
> > Right? It's just a list of devices together with an opaque pointer.
> 
> Well, I want a struct device pointer in there, not something opaque. But
> yes, an attachment is just bookkeeping.
> > 
> > Another thing I don't get (sorry, I must be really dense today) is that the
> > get_scatterlist op is set by the dma_buf_export call. I would expect that
> > that op is part of the attachment data since I would expect that to be
> > device specific.
> 
> There's a dma_buf_attachment parameter so you can get at the device the
> mapping is for.
> 
> > At least, my assumption is that the actual memory is allocated by the first
> > call to get_scatterlist?
> 
> Yes.
> 
> > Actually, I think this might be key: who allocates the actual memory and
> > when? There is no documentation whatsoever on this crucial topic.
> 
> Ok, let me elaborate a bit on how I see this. The memory will be allocated
> by whichever driver exported the dma_buf originally. We probably end up
> with a bunch of helper fucntion for the common case, but this approach
> allows crazy stuff like sharing special memory (e.g. TILER on omap) which
> can only be accessed by certain devices. I think this is easieast with a
> few examples:
> 
> - On a sane hw, e.g. buffer exported by i915 on x86: attach is a noop (and
>   hence should be optional), in get_scatterlist I'll just create a sg_list
>   out of the struct page * array I get from the gem backing storage and
>   the call map_sg and return that sg_list (now mapped into the device
>   address space).

OK.

> - On insane hw with strange dma requirements (like dram bank placement,
>   contigious, hugepagesize for puny tlbs, ...): Attach could check whether
>   the buffer is already fixed or not (e.g. the kernel fb console probably
>   can't be moved) and if the fixed buffer could be mapped by the device.
>   That's we attach needs a return value. If the buffer is not pinned or
>   not yet allocated, do nothing.

OK.

>   On get_scatterlist walk the attachment list and grab the dma
>   requirements of all devices (via some platform/dma specific stuff
>   attached to struct device). Then walk through a list of dma allocators
>   your platform provides (page_alloc, cma, ...) until you have one that
>   satisfies the requirements and alloc the backing storage.
> 
>   Then create an sg_list out of hit and map_sg it, like above.

Hmm. Here is where I run into problems. Right now the get_scatterlist op
is set by the driver that exports the buffer. So each driver supporting
dma_buf would have it's own get_scatterlist implementation, each able to
walk the attachment list. This feels wrong to me. get_scatterlist is doing
too much and this functionality should probably be split up somehow.

There is also the problem of how to determine whether all requirements
are satisfied: that's currently left undefined.

>   For arm we probably need some helper functions to make this easier.
> 
> - Sharing of a special buffer object, like a tiled buffer remapping into
>   omap's TILER. attach would check whether the device can actually access
>   that special io area (iirc on omap only certain devices can access the
>   tiler). get_scatterlist would just create a 1-entry sg_list that points
>   directly to the pyhsical address (in device space) of the remapped
>   buffer. This is way I want get_scatterlist to return a mapped buffer
>   (and for api cleanliness).

OK.

I think I would like to see a demo implementation and proper documentation.
Both help a lot to bring the API into focus and identify what works and what
doesn't. If it's hard to explain or to implement, then we probably should
improve that part :-)

Regards,

	Hans