diff mbox series

[RFC,4/5,v4] dma-buf: heaps: Add CMA heap to dmabuf heaps

Message ID 20190513183727.15755-5-john.stultz@linaro.org (mailing list archive)
State New, archived
Headers show
Series DMA-BUF Heaps (destaging ION) | expand

Commit Message

John Stultz May 13, 2019, 6:37 p.m. UTC
This adds a CMA heap, which allows userspace to allocate
a dma-buf of contiguous memory out of a CMA region.

This code is an evolution of the Android ION implementation, so
thanks to its original author and maintainters:
  Benjamin Gaignard, Laura Abbott, and others!

Cc: Laura Abbott <labbott@redhat.com>
Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Liam Mark <lmark@codeaurora.org>
Cc: Pratik Patel <pratikp@codeaurora.org>
Cc: Brian Starkey <Brian.Starkey@arm.com>
Cc: Vincent Donnefort <Vincent.Donnefort@arm.com>
Cc: Sudipto Paul <Sudipto.Paul@arm.com>
Cc: Andrew F. Davis <afd@ti.com>
Cc: Xu YiPing <xuyiping@hisilicon.com>
Cc: "Chenfeng (puck)" <puck.chen@hisilicon.com>
Cc: butao <butao@hisilicon.com>
Cc: "Xiaqing (A)" <saberlily.xia@hisilicon.com>
Cc: Yudongbin <yudongbin@hisilicon.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Chenbo Feng <fengc@google.com>
Cc: Alistair Strachan <astrachan@google.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v2:
* Switch allocate to return dmabuf fd
* Simplify init code
* Checkpatch fixups
v3:
* Switch to inline function for to_cma_heap()
* Minor cleanups suggested by Brian
* Fold in new registration style from Andrew
* Folded in changes from Andrew to use simplified page list
  from the heap helpers
* Use the fd_flags when creating dmabuf fd (Suggested by
  Benjamin)
* Use precalculated pagecount (Suggested by Andrew)
---
 drivers/dma-buf/heaps/Kconfig    |   8 ++
 drivers/dma-buf/heaps/Makefile   |   1 +
 drivers/dma-buf/heaps/cma_heap.c | 169 +++++++++++++++++++++++++++++++
 3 files changed, 178 insertions(+)
 create mode 100644 drivers/dma-buf/heaps/cma_heap.c

Comments

Xiaqing (A) May 14, 2019, 10:39 a.m. UTC | #1
On 2019/5/14 2:37, John Stultz wrote:
> This adds a CMA heap, which allows userspace to allocate
> a dma-buf of contiguous memory out of a CMA region.
> 
> This code is an evolution of the Android ION implementation, so
> thanks to its original author and maintainters:
>    Benjamin Gaignard, Laura Abbott, and others!
> 
> Cc: Laura Abbott <labbott@redhat.com>
> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: Liam Mark <lmark@codeaurora.org>
> Cc: Pratik Patel <pratikp@codeaurora.org>
> Cc: Brian Starkey <Brian.Starkey@arm.com>
> Cc: Vincent Donnefort <Vincent.Donnefort@arm.com>
> Cc: Sudipto Paul <Sudipto.Paul@arm.com>
> Cc: Andrew F. Davis <afd@ti.com>
> Cc: Xu YiPing <xuyiping@hisilicon.com>
> Cc: "Chenfeng (puck)" <puck.chen@hisilicon.com>
> Cc: butao <butao@hisilicon.com>
> Cc: "Xiaqing (A)" <saberlily.xia@hisilicon.com>
> Cc: Yudongbin <yudongbin@hisilicon.com>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Chenbo Feng <fengc@google.com>
> Cc: Alistair Strachan <astrachan@google.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> v2:
> * Switch allocate to return dmabuf fd
> * Simplify init code
> * Checkpatch fixups
> v3:
> * Switch to inline function for to_cma_heap()
> * Minor cleanups suggested by Brian
> * Fold in new registration style from Andrew
> * Folded in changes from Andrew to use simplified page list
>    from the heap helpers
> * Use the fd_flags when creating dmabuf fd (Suggested by
>    Benjamin)
> * Use precalculated pagecount (Suggested by Andrew)
> ---
>   drivers/dma-buf/heaps/Kconfig    |   8 ++
>   drivers/dma-buf/heaps/Makefile   |   1 +
>   drivers/dma-buf/heaps/cma_heap.c | 169 +++++++++++++++++++++++++++++++
>   3 files changed, 178 insertions(+)
>   create mode 100644 drivers/dma-buf/heaps/cma_heap.c
> 
> diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> index 205052744169..a5eef06c4226 100644
> --- a/drivers/dma-buf/heaps/Kconfig
> +++ b/drivers/dma-buf/heaps/Kconfig
> @@ -4,3 +4,11 @@ config DMABUF_HEAPS_SYSTEM
>   	help
>   	  Choose this option to enable the system dmabuf heap. The system heap
>   	  is backed by pages from the buddy allocator. If in doubt, say Y.
> +
> +config DMABUF_HEAPS_CMA
> +	bool "DMA-BUF CMA Heap"
> +	depends on DMABUF_HEAPS && DMA_CMA
> +	help
> +	  Choose this option to enable dma-buf CMA heap. This heap is backed
> +	  by the Contiguous Memory Allocator (CMA). If your system has these
> +	  regions, you should say Y here.
> diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
> index d1808eca2581..6e54cdec3da0 100644
> --- a/drivers/dma-buf/heaps/Makefile
> +++ b/drivers/dma-buf/heaps/Makefile
> @@ -1,3 +1,4 @@
>   # SPDX-License-Identifier: GPL-2.0
>   obj-y					+= heap-helpers.o
>   obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)	+= system_heap.o
> +obj-$(CONFIG_DMABUF_HEAPS_CMA)		+= cma_heap.o
> diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
> new file mode 100644
> index 000000000000..3d0ffbbd0a34
> --- /dev/null
> +++ b/drivers/dma-buf/heaps/cma_heap.c
> @@ -0,0 +1,169 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DMABUF CMA heap exporter
> + *
> + * Copyright (C) 2012, 2019 Linaro Ltd.
> + * Author: <benjamin.gaignard@linaro.org> for ST-Ericsson.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/dma-buf.h>
> +#include <linux/dma-heap.h>
> +#include <linux/slab.h>
> +#include <linux/errno.h>
> +#include <linux/err.h>
> +#include <linux/cma.h>
> +#include <linux/scatterlist.h>
> +#include <linux/highmem.h>
> +
> +#include "heap-helpers.h"
> +
> +struct cma_heap {
> +	struct dma_heap *heap;
> +	struct cma *cma;
> +};
> +
> +static void cma_heap_free(struct heap_helper_buffer *buffer)
> +{
> +	struct cma_heap *cma_heap = dma_heap_get_data(buffer->heap_buffer.heap);
> +	unsigned long nr_pages = buffer->pagecount;
> +	struct page *pages = buffer->priv_virt;
> +
> +	/* free page list */
> +	kfree(buffer->pages);
> +	/* release memory */
> +	cma_release(cma_heap->cma, pages, nr_pages);
> +	kfree(buffer);
> +}
> +
> +/* dmabuf heap CMA operations functions */
> +static int cma_heap_allocate(struct dma_heap *heap,
> +				unsigned long len,
> +				unsigned long fd_flags,
> +				unsigned long heap_flags)
> +{
> +	struct cma_heap *cma_heap = dma_heap_get_data(heap);
> +	struct heap_helper_buffer *helper_buffer;
> +	struct page *pages;
> +	size_t size = PAGE_ALIGN(len);
> +	unsigned long nr_pages = size >> PAGE_SHIFT;
> +	unsigned long align = get_order(size);
> +	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> +	struct dma_buf *dmabuf;
> +	int ret = -ENOMEM;
> +	pgoff_t pg;
> +
> +	if (align > CONFIG_CMA_ALIGNMENT)
> +		align = CONFIG_CMA_ALIGNMENT;
> +
> +	helper_buffer = kzalloc(sizeof(*helper_buffer), GFP_KERNEL);
> +	if (!helper_buffer)
> +		return -ENOMEM;
> +
> +	INIT_HEAP_HELPER_BUFFER(helper_buffer, cma_heap_free);
> +	helper_buffer->heap_buffer.flags = heap_flags;
> +	helper_buffer->heap_buffer.heap = heap;
> +	helper_buffer->heap_buffer.size = len;
> +
> +	pages = cma_alloc(cma_heap->cma, nr_pages, align, false);
> +	if (!pages)
> +		goto free_buf;
> +
> +	if (PageHighMem(pages)) {
> +		unsigned long nr_clear_pages = nr_pages;
> +		struct page *page = pages;
> +
> +		while (nr_clear_pages > 0) {
> +			void *vaddr = kmap_atomic(page);
> +
> +			memset(vaddr, 0, PAGE_SIZE);
> +			kunmap_atomic(vaddr);
> +			page++;
> +			nr_clear_pages--;
> +		}
> +	} else {
> +		memset(page_address(pages), 0, size);
> +	}
> +
> +	helper_buffer->pagecount = nr_pages;
> +	helper_buffer->pages = kmalloc_array(helper_buffer->pagecount,
> +					     sizeof(*helper_buffer->pages),
> +					     GFP_KERNEL);
> +	if (!helper_buffer->pages) {
> +		ret = -ENOMEM;
> +		goto free_cma;
> +	}
> +
> +	for (pg = 0; pg < helper_buffer->pagecount; pg++) {
> +		helper_buffer->pages[pg] = &pages[pg];
> +		if (!helper_buffer->pages[pg])
> +			goto free_pages;
> +	}
> +
> +	/* create the dmabuf */
> +	exp_info.ops = &heap_helper_ops;
> +	exp_info.size = len;
> +	exp_info.flags = fd_flags;
> +	exp_info.priv = &helper_buffer->heap_buffer;
> +	dmabuf = dma_buf_export(&exp_info);
> +	if (IS_ERR(dmabuf)) {
> +		ret = PTR_ERR(dmabuf);
> +		goto free_pages;
> +	}
Can the dmabuf be created in the framework layer?
each heap needs to add the same code here, which is not very good.

> +
> +	helper_buffer->heap_buffer.dmabuf = dmabuf;
> +	helper_buffer->priv_virt = pages;
> +
> +	ret = dma_buf_fd(dmabuf, fd_flags);
> +	if (ret < 0) {
> +		dma_buf_put(dmabuf);
> +		/* just return, as put will call release and that will free */
> +		return ret;
> +	}
> +
> +	return ret;
> +
> +free_pages:
> +	kfree(helper_buffer->pages);
> +free_cma:
> +	cma_release(cma_heap->cma, pages, nr_pages);
> +free_buf:
> +	kfree(helper_buffer);
> +	return ret;
> +}
> +
> +static struct dma_heap_ops cma_heap_ops = {
> +	.allocate = cma_heap_allocate,
> +};
> +
> +static int __add_cma_heap(struct cma *cma, void *data)
> +{
> +	struct cma_heap *cma_heap;
> +	struct dma_heap_export_info exp_info;
> +
> +	cma_heap = kzalloc(sizeof(*cma_heap), GFP_KERNEL);
> +	if (!cma_heap)
> +		return -ENOMEM;
> +	cma_heap->cma = cma;
> +
> +	exp_info.name = cma_get_name(cma);
> +	exp_info.ops = &cma_heap_ops;
> +	exp_info.priv = cma_heap;
> +
> +	cma_heap->heap = dma_heap_add(&exp_info);
> +	if (IS_ERR(cma_heap->heap)) {
> +		int ret = PTR_ERR(cma_heap->heap);
> +
> +		kfree(cma_heap);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int add_cma_heaps(void)
> +{
> +	cma_for_each_area(__add_cma_heap, NULL);
> +	return 0;
> +}
> +device_initcall(add_cma_heaps);
>
Benjamin Gaignard May 14, 2019, 11:55 a.m. UTC | #2
Le mar. 14 mai 2019 à 12:40, Xiaqing (A) <saberlily.xia@hisilicon.com> a écrit :
>
>
>
> On 2019/5/14 2:37, John Stultz wrote:
> > This adds a CMA heap, which allows userspace to allocate
> > a dma-buf of contiguous memory out of a CMA region.
> >
> > This code is an evolution of the Android ION implementation, so
> > thanks to its original author and maintainters:
> >    Benjamin Gaignard, Laura Abbott, and others!
> >
> > Cc: Laura Abbott <labbott@redhat.com>
> > Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> > Cc: Sumit Semwal <sumit.semwal@linaro.org>
> > Cc: Liam Mark <lmark@codeaurora.org>
> > Cc: Pratik Patel <pratikp@codeaurora.org>
> > Cc: Brian Starkey <Brian.Starkey@arm.com>
> > Cc: Vincent Donnefort <Vincent.Donnefort@arm.com>
> > Cc: Sudipto Paul <Sudipto.Paul@arm.com>
> > Cc: Andrew F. Davis <afd@ti.com>
> > Cc: Xu YiPing <xuyiping@hisilicon.com>
> > Cc: "Chenfeng (puck)" <puck.chen@hisilicon.com>
> > Cc: butao <butao@hisilicon.com>
> > Cc: "Xiaqing (A)" <saberlily.xia@hisilicon.com>
> > Cc: Yudongbin <yudongbin@hisilicon.com>
> > Cc: Christoph Hellwig <hch@infradead.org>
> > Cc: Chenbo Feng <fengc@google.com>
> > Cc: Alistair Strachan <astrachan@google.com>
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > ---
> > v2:
> > * Switch allocate to return dmabuf fd
> > * Simplify init code
> > * Checkpatch fixups
> > v3:
> > * Switch to inline function for to_cma_heap()
> > * Minor cleanups suggested by Brian
> > * Fold in new registration style from Andrew
> > * Folded in changes from Andrew to use simplified page list
> >    from the heap helpers
> > * Use the fd_flags when creating dmabuf fd (Suggested by
> >    Benjamin)
> > * Use precalculated pagecount (Suggested by Andrew)
> > ---
> >   drivers/dma-buf/heaps/Kconfig    |   8 ++
> >   drivers/dma-buf/heaps/Makefile   |   1 +
> >   drivers/dma-buf/heaps/cma_heap.c | 169 +++++++++++++++++++++++++++++++
> >   3 files changed, 178 insertions(+)
> >   create mode 100644 drivers/dma-buf/heaps/cma_heap.c
> >
> > diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> > index 205052744169..a5eef06c4226 100644
> > --- a/drivers/dma-buf/heaps/Kconfig
> > +++ b/drivers/dma-buf/heaps/Kconfig
> > @@ -4,3 +4,11 @@ config DMABUF_HEAPS_SYSTEM
> >       help
> >         Choose this option to enable the system dmabuf heap. The system heap
> >         is backed by pages from the buddy allocator. If in doubt, say Y.
> > +
> > +config DMABUF_HEAPS_CMA
> > +     bool "DMA-BUF CMA Heap"
> > +     depends on DMABUF_HEAPS && DMA_CMA
> > +     help
> > +       Choose this option to enable dma-buf CMA heap. This heap is backed
> > +       by the Contiguous Memory Allocator (CMA). If your system has these
> > +       regions, you should say Y here.
> > diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
> > index d1808eca2581..6e54cdec3da0 100644
> > --- a/drivers/dma-buf/heaps/Makefile
> > +++ b/drivers/dma-buf/heaps/Makefile
> > @@ -1,3 +1,4 @@
> >   # SPDX-License-Identifier: GPL-2.0
> >   obj-y                                       += heap-helpers.o
> >   obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)   += system_heap.o
> > +obj-$(CONFIG_DMABUF_HEAPS_CMA)               += cma_heap.o
> > diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
> > new file mode 100644
> > index 000000000000..3d0ffbbd0a34
> > --- /dev/null
> > +++ b/drivers/dma-buf/heaps/cma_heap.c
> > @@ -0,0 +1,169 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * DMABUF CMA heap exporter
> > + *
> > + * Copyright (C) 2012, 2019 Linaro Ltd.
> > + * Author: <benjamin.gaignard@linaro.org> for ST-Ericsson.
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/dma-buf.h>
> > +#include <linux/dma-heap.h>
> > +#include <linux/slab.h>
> > +#include <linux/errno.h>
> > +#include <linux/err.h>
> > +#include <linux/cma.h>
> > +#include <linux/scatterlist.h>
> > +#include <linux/highmem.h>
> > +
> > +#include "heap-helpers.h"
> > +
> > +struct cma_heap {
> > +     struct dma_heap *heap;
> > +     struct cma *cma;
> > +};
> > +
> > +static void cma_heap_free(struct heap_helper_buffer *buffer)
> > +{
> > +     struct cma_heap *cma_heap = dma_heap_get_data(buffer->heap_buffer.heap);
> > +     unsigned long nr_pages = buffer->pagecount;
> > +     struct page *pages = buffer->priv_virt;
> > +
> > +     /* free page list */
> > +     kfree(buffer->pages);
> > +     /* release memory */
> > +     cma_release(cma_heap->cma, pages, nr_pages);
> > +     kfree(buffer);
> > +}
> > +
> > +/* dmabuf heap CMA operations functions */
> > +static int cma_heap_allocate(struct dma_heap *heap,
> > +                             unsigned long len,
> > +                             unsigned long fd_flags,
> > +                             unsigned long heap_flags)
> > +{
> > +     struct cma_heap *cma_heap = dma_heap_get_data(heap);
> > +     struct heap_helper_buffer *helper_buffer;
> > +     struct page *pages;
> > +     size_t size = PAGE_ALIGN(len);
> > +     unsigned long nr_pages = size >> PAGE_SHIFT;
> > +     unsigned long align = get_order(size);
> > +     DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> > +     struct dma_buf *dmabuf;
> > +     int ret = -ENOMEM;
> > +     pgoff_t pg;
> > +
> > +     if (align > CONFIG_CMA_ALIGNMENT)
> > +             align = CONFIG_CMA_ALIGNMENT;
> > +
> > +     helper_buffer = kzalloc(sizeof(*helper_buffer), GFP_KERNEL);
> > +     if (!helper_buffer)
> > +             return -ENOMEM;
> > +
> > +     INIT_HEAP_HELPER_BUFFER(helper_buffer, cma_heap_free);
> > +     helper_buffer->heap_buffer.flags = heap_flags;
> > +     helper_buffer->heap_buffer.heap = heap;
> > +     helper_buffer->heap_buffer.size = len;
> > +
> > +     pages = cma_alloc(cma_heap->cma, nr_pages, align, false);
> > +     if (!pages)
> > +             goto free_buf;
> > +
> > +     if (PageHighMem(pages)) {
> > +             unsigned long nr_clear_pages = nr_pages;
> > +             struct page *page = pages;
> > +
> > +             while (nr_clear_pages > 0) {
> > +                     void *vaddr = kmap_atomic(page);
> > +
> > +                     memset(vaddr, 0, PAGE_SIZE);
> > +                     kunmap_atomic(vaddr);
> > +                     page++;
> > +                     nr_clear_pages--;
> > +             }
> > +     } else {
> > +             memset(page_address(pages), 0, size);
> > +     }
> > +
> > +     helper_buffer->pagecount = nr_pages;
> > +     helper_buffer->pages = kmalloc_array(helper_buffer->pagecount,
> > +                                          sizeof(*helper_buffer->pages),
> > +                                          GFP_KERNEL);
> > +     if (!helper_buffer->pages) {
> > +             ret = -ENOMEM;
> > +             goto free_cma;
> > +     }
> > +
> > +     for (pg = 0; pg < helper_buffer->pagecount; pg++) {
> > +             helper_buffer->pages[pg] = &pages[pg];
> > +             if (!helper_buffer->pages[pg])
> > +                     goto free_pages;
> > +     }
> > +
> > +     /* create the dmabuf */
> > +     exp_info.ops = &heap_helper_ops;
> > +     exp_info.size = len;
> > +     exp_info.flags = fd_flags;
> > +     exp_info.priv = &helper_buffer->heap_buffer;
> > +     dmabuf = dma_buf_export(&exp_info);
> > +     if (IS_ERR(dmabuf)) {
> > +             ret = PTR_ERR(dmabuf);
> > +             goto free_pages;
> > +     }
> Can the dmabuf be created in the framework layer?
> each heap needs to add the same code here, which is not very good.

No that would make errors cases more difficult to handle because
you will need to call a new heap's function to clean everything.

>
> > +
> > +     helper_buffer->heap_buffer.dmabuf = dmabuf;
> > +     helper_buffer->priv_virt = pages;
> > +
> > +     ret = dma_buf_fd(dmabuf, fd_flags);
> > +     if (ret < 0) {
> > +             dma_buf_put(dmabuf);
> > +             /* just return, as put will call release and that will free */
> > +             return ret;
> > +     }
> > +
> > +     return ret;
> > +
> > +free_pages:
> > +     kfree(helper_buffer->pages);
> > +free_cma:
> > +     cma_release(cma_heap->cma, pages, nr_pages);
> > +free_buf:
> > +     kfree(helper_buffer);
> > +     return ret;
> > +}
> > +
> > +static struct dma_heap_ops cma_heap_ops = {
> > +     .allocate = cma_heap_allocate,
> > +};
> > +
> > +static int __add_cma_heap(struct cma *cma, void *data)
> > +{
> > +     struct cma_heap *cma_heap;
> > +     struct dma_heap_export_info exp_info;
> > +
> > +     cma_heap = kzalloc(sizeof(*cma_heap), GFP_KERNEL);
> > +     if (!cma_heap)
> > +             return -ENOMEM;
> > +     cma_heap->cma = cma;
> > +
> > +     exp_info.name = cma_get_name(cma);
> > +     exp_info.ops = &cma_heap_ops;
> > +     exp_info.priv = cma_heap;
> > +
> > +     cma_heap->heap = dma_heap_add(&exp_info);
> > +     if (IS_ERR(cma_heap->heap)) {
> > +             int ret = PTR_ERR(cma_heap->heap);
> > +
> > +             kfree(cma_heap);
> > +             return ret;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int add_cma_heaps(void)
> > +{
> > +     cma_for_each_area(__add_cma_heap, NULL);
> > +     return 0;
> > +}
> > +device_initcall(add_cma_heaps);
> >
>
John Stultz May 16, 2019, 10:05 p.m. UTC | #3
On Tue, May 14, 2019 at 3:40 AM Xiaqing (A) <saberlily.xia@hisilicon.com> wrote:
>
>
>
> On 2019/5/14 2:37, John Stultz wrote:
> > This adds a CMA heap, which allows userspace to allocate
> > a dma-buf of contiguous memory out of a CMA region.
> >
> > This code is an evolution of the Android ION implementation, so
> > thanks to its original author and maintainters:
> >    Benjamin Gaignard, Laura Abbott, and others!
> >
> > Cc: Laura Abbott <labbott@redhat.com>
> > Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> > Cc: Sumit Semwal <sumit.semwal@linaro.org>
> > Cc: Liam Mark <lmark@codeaurora.org>
> > Cc: Pratik Patel <pratikp@codeaurora.org>
> > Cc: Brian Starkey <Brian.Starkey@arm.com>
> > Cc: Vincent Donnefort <Vincent.Donnefort@arm.com>
> > Cc: Sudipto Paul <Sudipto.Paul@arm.com>
> > Cc: Andrew F. Davis <afd@ti.com>
> > Cc: Xu YiPing <xuyiping@hisilicon.com>
> > Cc: "Chenfeng (puck)" <puck.chen@hisilicon.com>
> > Cc: butao <butao@hisilicon.com>
> > Cc: "Xiaqing (A)" <saberlily.xia@hisilicon.com>
> > Cc: Yudongbin <yudongbin@hisilicon.com>
> > Cc: Christoph Hellwig <hch@infradead.org>
> > Cc: Chenbo Feng <fengc@google.com>
> > Cc: Alistair Strachan <astrachan@google.com>
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > ---
> > v2:
> > * Switch allocate to return dmabuf fd
> > * Simplify init code
> > * Checkpatch fixups
> > v3:
> > * Switch to inline function for to_cma_heap()
> > * Minor cleanups suggested by Brian
> > * Fold in new registration style from Andrew
> > * Folded in changes from Andrew to use simplified page list
> >    from the heap helpers
> > * Use the fd_flags when creating dmabuf fd (Suggested by
> >    Benjamin)
> > * Use precalculated pagecount (Suggested by Andrew)
> > ---
> >   drivers/dma-buf/heaps/Kconfig    |   8 ++
> >   drivers/dma-buf/heaps/Makefile   |   1 +
> >   drivers/dma-buf/heaps/cma_heap.c | 169 +++++++++++++++++++++++++++++++
> >   3 files changed, 178 insertions(+)
> >   create mode 100644 drivers/dma-buf/heaps/cma_heap.c
> >
> > diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> > index 205052744169..a5eef06c4226 100644
> > --- a/drivers/dma-buf/heaps/Kconfig
> > +++ b/drivers/dma-buf/heaps/Kconfig
> > @@ -4,3 +4,11 @@ config DMABUF_HEAPS_SYSTEM
> >       help
> >         Choose this option to enable the system dmabuf heap. The system heap
> >         is backed by pages from the buddy allocator. If in doubt, say Y.
> > +
> > +config DMABUF_HEAPS_CMA
> > +     bool "DMA-BUF CMA Heap"
> > +     depends on DMABUF_HEAPS && DMA_CMA
> > +     help
> > +       Choose this option to enable dma-buf CMA heap. This heap is backed
> > +       by the Contiguous Memory Allocator (CMA). If your system has these
> > +       regions, you should say Y here.
> > diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
> > index d1808eca2581..6e54cdec3da0 100644
> > --- a/drivers/dma-buf/heaps/Makefile
> > +++ b/drivers/dma-buf/heaps/Makefile
> > @@ -1,3 +1,4 @@
> >   # SPDX-License-Identifier: GPL-2.0
> >   obj-y                                       += heap-helpers.o
> >   obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)   += system_heap.o
> > +obj-$(CONFIG_DMABUF_HEAPS_CMA)               += cma_heap.o
> > diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
> > new file mode 100644
> > index 000000000000..3d0ffbbd0a34
> > --- /dev/null
> > +++ b/drivers/dma-buf/heaps/cma_heap.c
> > @@ -0,0 +1,169 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * DMABUF CMA heap exporter
> > + *
> > + * Copyright (C) 2012, 2019 Linaro Ltd.
> > + * Author: <benjamin.gaignard@linaro.org> for ST-Ericsson.
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/dma-buf.h>
> > +#include <linux/dma-heap.h>
> > +#include <linux/slab.h>
> > +#include <linux/errno.h>
> > +#include <linux/err.h>
> > +#include <linux/cma.h>
> > +#include <linux/scatterlist.h>
> > +#include <linux/highmem.h>
> > +
> > +#include "heap-helpers.h"
> > +
> > +struct cma_heap {
> > +     struct dma_heap *heap;
> > +     struct cma *cma;
> > +};
> > +
> > +static void cma_heap_free(struct heap_helper_buffer *buffer)
> > +{
> > +     struct cma_heap *cma_heap = dma_heap_get_data(buffer->heap_buffer.heap);
> > +     unsigned long nr_pages = buffer->pagecount;
> > +     struct page *pages = buffer->priv_virt;
> > +
> > +     /* free page list */
> > +     kfree(buffer->pages);
> > +     /* release memory */
> > +     cma_release(cma_heap->cma, pages, nr_pages);
> > +     kfree(buffer);
> > +}
> > +
> > +/* dmabuf heap CMA operations functions */
> > +static int cma_heap_allocate(struct dma_heap *heap,
> > +                             unsigned long len,
> > +                             unsigned long fd_flags,
> > +                             unsigned long heap_flags)
> > +{
> > +     struct cma_heap *cma_heap = dma_heap_get_data(heap);
> > +     struct heap_helper_buffer *helper_buffer;
> > +     struct page *pages;
> > +     size_t size = PAGE_ALIGN(len);
> > +     unsigned long nr_pages = size >> PAGE_SHIFT;
> > +     unsigned long align = get_order(size);
> > +     DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> > +     struct dma_buf *dmabuf;
> > +     int ret = -ENOMEM;
> > +     pgoff_t pg;
> > +
> > +     if (align > CONFIG_CMA_ALIGNMENT)
> > +             align = CONFIG_CMA_ALIGNMENT;
> > +
> > +     helper_buffer = kzalloc(sizeof(*helper_buffer), GFP_KERNEL);
> > +     if (!helper_buffer)
> > +             return -ENOMEM;
> > +
> > +     INIT_HEAP_HELPER_BUFFER(helper_buffer, cma_heap_free);
> > +     helper_buffer->heap_buffer.flags = heap_flags;
> > +     helper_buffer->heap_buffer.heap = heap;
> > +     helper_buffer->heap_buffer.size = len;
> > +
> > +     pages = cma_alloc(cma_heap->cma, nr_pages, align, false);
> > +     if (!pages)
> > +             goto free_buf;
> > +
> > +     if (PageHighMem(pages)) {
> > +             unsigned long nr_clear_pages = nr_pages;
> > +             struct page *page = pages;
> > +
> > +             while (nr_clear_pages > 0) {
> > +                     void *vaddr = kmap_atomic(page);
> > +
> > +                     memset(vaddr, 0, PAGE_SIZE);
> > +                     kunmap_atomic(vaddr);
> > +                     page++;
> > +                     nr_clear_pages--;
> > +             }
> > +     } else {
> > +             memset(page_address(pages), 0, size);
> > +     }
> > +
> > +     helper_buffer->pagecount = nr_pages;
> > +     helper_buffer->pages = kmalloc_array(helper_buffer->pagecount,
> > +                                          sizeof(*helper_buffer->pages),
> > +                                          GFP_KERNEL);
> > +     if (!helper_buffer->pages) {
> > +             ret = -ENOMEM;
> > +             goto free_cma;
> > +     }
> > +
> > +     for (pg = 0; pg < helper_buffer->pagecount; pg++) {
> > +             helper_buffer->pages[pg] = &pages[pg];
> > +             if (!helper_buffer->pages[pg])
> > +                     goto free_pages;
> > +     }
> > +
> > +     /* create the dmabuf */
> > +     exp_info.ops = &heap_helper_ops;
> > +     exp_info.size = len;
> > +     exp_info.flags = fd_flags;
> > +     exp_info.priv = &helper_buffer->heap_buffer;
> > +     dmabuf = dma_buf_export(&exp_info);
> > +     if (IS_ERR(dmabuf)) {
> > +             ret = PTR_ERR(dmabuf);
> > +             goto free_pages;
> > +     }
> Can the dmabuf be created in the framework layer?
> each heap needs to add the same code here, which is not very good.

Benjamin's point is true, that it might not be best to try to handle
it in the framework layer, but there is a fair amount of boilerplate
that I'll see if I can refactor into some helper functions.

thanks for the feedback!
-john
diff mbox series

Patch

diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
index 205052744169..a5eef06c4226 100644
--- a/drivers/dma-buf/heaps/Kconfig
+++ b/drivers/dma-buf/heaps/Kconfig
@@ -4,3 +4,11 @@  config DMABUF_HEAPS_SYSTEM
 	help
 	  Choose this option to enable the system dmabuf heap. The system heap
 	  is backed by pages from the buddy allocator. If in doubt, say Y.
+
+config DMABUF_HEAPS_CMA
+	bool "DMA-BUF CMA Heap"
+	depends on DMABUF_HEAPS && DMA_CMA
+	help
+	  Choose this option to enable dma-buf CMA heap. This heap is backed
+	  by the Contiguous Memory Allocator (CMA). If your system has these
+	  regions, you should say Y here.
diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
index d1808eca2581..6e54cdec3da0 100644
--- a/drivers/dma-buf/heaps/Makefile
+++ b/drivers/dma-buf/heaps/Makefile
@@ -1,3 +1,4 @@ 
 # SPDX-License-Identifier: GPL-2.0
 obj-y					+= heap-helpers.o
 obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)	+= system_heap.o
+obj-$(CONFIG_DMABUF_HEAPS_CMA)		+= cma_heap.o
diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
new file mode 100644
index 000000000000..3d0ffbbd0a34
--- /dev/null
+++ b/drivers/dma-buf/heaps/cma_heap.c
@@ -0,0 +1,169 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DMABUF CMA heap exporter
+ *
+ * Copyright (C) 2012, 2019 Linaro Ltd.
+ * Author: <benjamin.gaignard@linaro.org> for ST-Ericsson.
+ */
+
+#include <linux/device.h>
+#include <linux/dma-buf.h>
+#include <linux/dma-heap.h>
+#include <linux/slab.h>
+#include <linux/errno.h>
+#include <linux/err.h>
+#include <linux/cma.h>
+#include <linux/scatterlist.h>
+#include <linux/highmem.h>
+
+#include "heap-helpers.h"
+
+struct cma_heap {
+	struct dma_heap *heap;
+	struct cma *cma;
+};
+
+static void cma_heap_free(struct heap_helper_buffer *buffer)
+{
+	struct cma_heap *cma_heap = dma_heap_get_data(buffer->heap_buffer.heap);
+	unsigned long nr_pages = buffer->pagecount;
+	struct page *pages = buffer->priv_virt;
+
+	/* free page list */
+	kfree(buffer->pages);
+	/* release memory */
+	cma_release(cma_heap->cma, pages, nr_pages);
+	kfree(buffer);
+}
+
+/* dmabuf heap CMA operations functions */
+static int cma_heap_allocate(struct dma_heap *heap,
+				unsigned long len,
+				unsigned long fd_flags,
+				unsigned long heap_flags)
+{
+	struct cma_heap *cma_heap = dma_heap_get_data(heap);
+	struct heap_helper_buffer *helper_buffer;
+	struct page *pages;
+	size_t size = PAGE_ALIGN(len);
+	unsigned long nr_pages = size >> PAGE_SHIFT;
+	unsigned long align = get_order(size);
+	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
+	struct dma_buf *dmabuf;
+	int ret = -ENOMEM;
+	pgoff_t pg;
+
+	if (align > CONFIG_CMA_ALIGNMENT)
+		align = CONFIG_CMA_ALIGNMENT;
+
+	helper_buffer = kzalloc(sizeof(*helper_buffer), GFP_KERNEL);
+	if (!helper_buffer)
+		return -ENOMEM;
+
+	INIT_HEAP_HELPER_BUFFER(helper_buffer, cma_heap_free);
+	helper_buffer->heap_buffer.flags = heap_flags;
+	helper_buffer->heap_buffer.heap = heap;
+	helper_buffer->heap_buffer.size = len;
+
+	pages = cma_alloc(cma_heap->cma, nr_pages, align, false);
+	if (!pages)
+		goto free_buf;
+
+	if (PageHighMem(pages)) {
+		unsigned long nr_clear_pages = nr_pages;
+		struct page *page = pages;
+
+		while (nr_clear_pages > 0) {
+			void *vaddr = kmap_atomic(page);
+
+			memset(vaddr, 0, PAGE_SIZE);
+			kunmap_atomic(vaddr);
+			page++;
+			nr_clear_pages--;
+		}
+	} else {
+		memset(page_address(pages), 0, size);
+	}
+
+	helper_buffer->pagecount = nr_pages;
+	helper_buffer->pages = kmalloc_array(helper_buffer->pagecount,
+					     sizeof(*helper_buffer->pages),
+					     GFP_KERNEL);
+	if (!helper_buffer->pages) {
+		ret = -ENOMEM;
+		goto free_cma;
+	}
+
+	for (pg = 0; pg < helper_buffer->pagecount; pg++) {
+		helper_buffer->pages[pg] = &pages[pg];
+		if (!helper_buffer->pages[pg])
+			goto free_pages;
+	}
+
+	/* create the dmabuf */
+	exp_info.ops = &heap_helper_ops;
+	exp_info.size = len;
+	exp_info.flags = fd_flags;
+	exp_info.priv = &helper_buffer->heap_buffer;
+	dmabuf = dma_buf_export(&exp_info);
+	if (IS_ERR(dmabuf)) {
+		ret = PTR_ERR(dmabuf);
+		goto free_pages;
+	}
+
+	helper_buffer->heap_buffer.dmabuf = dmabuf;
+	helper_buffer->priv_virt = pages;
+
+	ret = dma_buf_fd(dmabuf, fd_flags);
+	if (ret < 0) {
+		dma_buf_put(dmabuf);
+		/* just return, as put will call release and that will free */
+		return ret;
+	}
+
+	return ret;
+
+free_pages:
+	kfree(helper_buffer->pages);
+free_cma:
+	cma_release(cma_heap->cma, pages, nr_pages);
+free_buf:
+	kfree(helper_buffer);
+	return ret;
+}
+
+static struct dma_heap_ops cma_heap_ops = {
+	.allocate = cma_heap_allocate,
+};
+
+static int __add_cma_heap(struct cma *cma, void *data)
+{
+	struct cma_heap *cma_heap;
+	struct dma_heap_export_info exp_info;
+
+	cma_heap = kzalloc(sizeof(*cma_heap), GFP_KERNEL);
+	if (!cma_heap)
+		return -ENOMEM;
+	cma_heap->cma = cma;
+
+	exp_info.name = cma_get_name(cma);
+	exp_info.ops = &cma_heap_ops;
+	exp_info.priv = cma_heap;
+
+	cma_heap->heap = dma_heap_add(&exp_info);
+	if (IS_ERR(cma_heap->heap)) {
+		int ret = PTR_ERR(cma_heap->heap);
+
+		kfree(cma_heap);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int add_cma_heaps(void)
+{
+	cma_for_each_area(__add_cma_heap, NULL);
+	return 0;
+}
+device_initcall(add_cma_heaps);