[EARLY RFC][PATCH 0/4] dmabuf pools infrastructure (destaging ION)

[Andrew F. Davis]

On 2/21/19 1:40 AM, John Stultz wrote:
> Here is a very early peek at my dmabuf pools patchset, which
> tries to destage a fair chunk of ION functionality.
> 
> This build and boots, but I've not gotten to testing the actual
> pool devices yet (need to write some kselftests)! I just wanted
> some early feedback on the overall direction.
> 
> The patchset implements per-pool devices (extending my ion
> per-heap devices patchset from last week), which can be opened
> directly and then an ioctl is used to allocate a dmabuf from the
> pool.
> 
> The interface is similar, but simpler then IONs, only providing
> an ALLOC ioctl.
> 
> Also, I've only destaged the system/system-contig and cma pools,
> since the ION carveout and chunk heaps depended on out of tree
> board files to initialize those heaps. I'll leave that to folks
> who are actually using those heaps.
> 
> Let me know what you think!
> 

+1

Was this source not pulled from -next, I have some fixes in next that I
don't see in this code, so I won't review the code itself just yet (it
is and early RFC after all). For the concept itself I have a couple
small suggestions:

I'm not sure I like the name. "Pool" in the context of DMA-BUF feels
like it means something else, like some new feature of DMA-BUFs
exporters/importers can use for making buffer pools. How about just keep
the "heap" terminology to prevent too much re-wording. Maybe just call
this dma-buf/heaps/ ?

Although the differed free stuff is nice and should be available, I
don't think it needs to be part of the first set of de-staged features.
It is a bolt-on feature that can be added later, making this first
patchset more simple.

In the same way I would like to see the changes suggested in one of the
other threads implemented. Basically let the heaps(pools?) provide their
own struct dma_buf_ops. If this is to be an extension of dma-buf then it
shouldn't be making forcing the use of its own dma_buf_ops like ION did.
Instead it should just handle the userspace exporting API only. We can
always provide helpers for the basic dma_buf_ops for consistency and
code-reuse, but the heaps themselves should have full control if/when to
use them.

It might be easier to show this all by example, I'll put together my own
rough RFC over the weekend if that is okay with you (not trying to walk
over your work here or anything, just want to show what I'm thinking if
any of the above doesn't make sense) :)

Thanks,
Andrew

> thanks
> -john
> 
> Cc: Laura Abbott <labbott at redhat.com>
> Cc: Benjamin Gaignard <benjamin.gaignard at linaro.org>
> Cc: Sumit Semwal <sumit.semwal at linaro.org>
> Cc: Liam Mark <lmark at codeaurora.org>
> Cc: Brian Starkey <Brian.Starkey at arm.com>
> Cc: Andrew F. Davis <afd at ti.com>
> Cc: Chenbo Feng <fengc at google.com>
> Cc: Alistair Strachan <astrachan at google.com>
> Cc: dri-devel at lists.freedesktop.org
> 
> John Stultz (4):
>   dma-buf: Add dma-buf pools framework
>   dma-buf: pools: Add page-pool for dma-buf pools
>   dma-buf: pools: Add system/system-contig pools to dmabuf pools
>   dma-buf: pools: Add CMA pool to dmabuf pools
> 
>  MAINTAINERS                          |  13 +
>  drivers/dma-buf/Kconfig              |   2 +
>  drivers/dma-buf/Makefile             |   1 +
>  drivers/dma-buf/pools/Kconfig        |  25 ++
>  drivers/dma-buf/pools/Makefile       |   4 +
>  drivers/dma-buf/pools/cma_pool.c     | 143 ++++++++
>  drivers/dma-buf/pools/dmabuf-pools.c | 670 +++++++++++++++++++++++++++++++++++
>  drivers/dma-buf/pools/dmabuf-pools.h | 295 +++++++++++++++
>  drivers/dma-buf/pools/page_pool.c    | 157 ++++++++
>  drivers/dma-buf/pools/pool-helpers.c | 317 +++++++++++++++++
>  drivers/dma-buf/pools/pool-ioctl.c   |  94 +++++
>  drivers/dma-buf/pools/system_pool.c  | 374 +++++++++++++++++++
>  include/uapi/linux/dmabuf-pools.h    |  59 +++
>  13 files changed, 2154 insertions(+)
>  create mode 100644 drivers/dma-buf/pools/Kconfig
>  create mode 100644 drivers/dma-buf/pools/Makefile
>  create mode 100644 drivers/dma-buf/pools/cma_pool.c
>  create mode 100644 drivers/dma-buf/pools/dmabuf-pools.c
>  create mode 100644 drivers/dma-buf/pools/dmabuf-pools.h
>  create mode 100644 drivers/dma-buf/pools/page_pool.c
>  create mode 100644 drivers/dma-buf/pools/pool-helpers.c
>  create mode 100644 drivers/dma-buf/pools/pool-ioctl.c
>  create mode 100644 drivers/dma-buf/pools/system_pool.c
>  create mode 100644 include/uapi/linux/dmabuf-pools.h
>