Re: [EARLY RFC][PATCH] dma-buf: Add dma-buf heaps framework

["Andrew F. Davis" <afd@ti.com>]

On 2/26/19 12:20 AM, John Stultz wrote:
> On Mon, Feb 25, 2019 at 6:36 AM Andrew F. Davis <afd@ti.com> wrote:
>>
>> This framework allows a unified userspace interface for dma-buf
>> exporters, allowing userland to allocate specific types of memory
>> for use in dma-buf sharing.
>>
>> Each heap is given its own device node, which a user can allocate
>> a dma-buf fd from using the DMA_HEAP_IOC_ALLOC.
>>
>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>> ---
>>
>> Hello all,
>>
>> I had a little less time over the weekend than I thought I would to
>> clean this up more and finish the first set of user heaps, but wanted
>> to get this out anyway.
>>
>> ION in its current form assumes a lot about the memory it exports and
>> these assumptions lead to restrictions on the full range of operations
>> dma-buf's can produce. Due to this if we are to add this as an extension
>> of the core dma-buf then it should only be the user-space advertisement
>> and allocation front-end. All dma-buf exporting and operation need to be
>> placed in the control of the exporting heap. The core becomes rather
>> small, just a few hundred lines you see here. This is not to say we
>> should not provide helpers to make the heap exporters more simple, but
>> they should only be helpers and not enforced by the core framework.
>>
>> Basic use model here is an exporter (dedicated heap memory driver, CMA,
>> System, etc.) registers with the framework by providing a struct
>> dma_heap_info which gives us the needed info to export this heap to
>> userspace as a device node. Next a user will request an allocation,
>> the IOCTL is parsed and the request made to a heap provided alloc() op.
>> The heap should return the filled out struct dma_heap_buffer, including
>> exporting the buffer as a dma-buf. This dma-buf we then return to the
>> user as a fd. Buffer free is still a bit open as we need to update some
>> stats and free some memory, but the release operation is controlled by
>> the heap exporter, so some hook will have to be created.
>>
>> It all needs a bit more work, and I'm sure I'll find missing parts when
>> I add some more heaps, but hopefully this framework is simple enough that
>> it does not impede the implementation of all functionality once provided
>> by ION (shrinker, delayed free), nor any new functionality needed for
>> future heap exporting devices.
> 
> I took your patch here, made some small reworks as I mentioned
> earlier, and tried to add some generic helpers and the system and cma
> heaps integrated.
> 
> You can find the changes added on top of your patch here:
> https://git.linaro.org/people/john.stultz/android-dev.git/log/?h=dev/dma-buf-heap
> 
> I propose we should squish down the first three changes down into the
> core patch if you agree.
> 

All looks good to me. One thing I'm still unsure of is moving struct
dma_heap out into the public interface header. That struct contains
framework internal only info (device nodes, stats, etc.) that should not
be exposed to the outside world. I think of them as private members of
our class (too much C++ work lately), if we need to expose any members
of that struct then we can add accessors as needed.

> The helper functions need some work and cleanup, right now I'm only
> exposing the dmabuf ops and realistically folks will probably want to
> fill part of the ops with custom things.
> 

Yes, the current set of helper dma-buf-ops work well for pre-allocated
page-struct-backed memory buffers (basically sg list). For buffers
allocated outside of normal RAM, secure (unmapped) heaps, and attach
time allocated heaps all kinda break down with the current helpers. But
that's the best part of helpers, you don't need to use them if you don't
want :)

> It boots w/ AOSP, and allocations seem to work, but there's something
> wrong with the dmabuf mmaping which casues the homescreen to crash
> over and over.
> (userland patches updated here:
> https://android-review.googlesource.com/c/device/linaro/hikey/+/909436)
> 

Interesting, I wonder if the caching stuff is not right here, I'll see
if I can get this working on my side (AOSP on Beagle x15).

Thanks,
Andrew

> But that's about as far as I can do tonight, so I'm crashing and will
> see what you think tomorrow. :)
> 
> thanks
> -john
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel