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

[John Stultz]

On Tue, Feb 26, 2019 at 6:47 AM Andrew F. Davis <afd at ti.com> wrote:
> On 2/26/19 12:20 AM, John Stultz wrote:
> > On Mon, Feb 25, 2019 at 6:36 AM Andrew F. Davis <afd at ti.com> wrote:
> >> 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.

Hrm. The trouble is that the heaps need some way to index back to
their own per-heap data (similar to how the heaps also need their own
per-buffer data) like the cma struct for the specific cma region, for
instance. Maybe this doesn't address your concern, but one way I've
dealt with "subsystem internal" structures is to have local .h files,
so only the heap implementations can see it.  I may be missing a
different approach that your thinking of, so please let me know. :)

> > 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).
>

Let me know if you figure anything out, I'll also be looking at this today.

I also realized some of Benajmin's error path improvements are going
to be hard to fix w/ my current code, specifically having the allocate
op do the allocation of the dma_heap_buffer (since we don't have a
free op, if something fails creating the dmabuf fd in the core, we
don't have a hook to release the dma_heap_buffer and heap private
data).

thanks
-john