From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brian Starkey Subject: Re: [RFC][PATCH 4/5 v2] dma-buf: heaps: Add CMA heap to dmabuf heapss Date: Tue, 19 Mar 2019 14:53:41 +0000 Message-ID: <20190319145340.nbunzlwguyotthvw@DESKTOP-E1NTVVP.localdomain> References: <1551819273-640-1-git-send-email-john.stultz@linaro.org> <1551819273-640-5-git-send-email-john.stultz@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <1551819273-640-5-git-send-email-john.stultz@linaro.org> Content-Language: en-US Content-ID: Sender: linux-kernel-owner@vger.kernel.org To: John Stultz Cc: lkml , Laura Abbott , Benjamin Gaignard , Greg KH , Sumit Semwal , Liam Mark , "Andrew F . Davis" , Chenbo Feng , Alistair Strachan , "dri-devel@lists.freedesktop.org" , nd List-Id: dri-devel@lists.freedesktop.org On Tue, Mar 05, 2019 at 12:54:32PM -0800, John Stultz wrote: > This adds a CMA heap, which allows userspace to allocate > a dma-buf of contiguous memory out of a CMA region. >=20 > This code is an evolution of the Android ION implementation, so > thanks to its original author and maintainters: > Benjamin Gaignard, Laura Abbott, and others! >=20 > Cc: Laura Abbott > Cc: Benjamin Gaignard > Cc: Greg KH > Cc: Sumit Semwal > Cc: Liam Mark > Cc: Brian Starkey > Cc: Andrew F. Davis > Cc: Chenbo Feng > Cc: Alistair Strachan > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: John Stultz > --- > v2: > * Switch allocate to return dmabuf fd > * Simplify init code > * Checkpatch fixups > --- > drivers/dma-buf/heaps/Kconfig | 8 ++ > drivers/dma-buf/heaps/Makefile | 1 + > drivers/dma-buf/heaps/cma_heap.c | 164 +++++++++++++++++++++++++++++++++= ++++++ > 3 files changed, 173 insertions(+) > create mode 100644 drivers/dma-buf/heaps/cma_heap.c >=20 > diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfi= g > index 2050527..a5eef06 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/Makef= ile > index d1808ec..6e54cde 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 +=3D heap-helpers.o > obj-$(CONFIG_DMABUF_HEAPS_SYSTEM) +=3D system_heap.o > +obj-$(CONFIG_DMABUF_HEAPS_CMA) +=3D 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 0000000..33c18ec > --- /dev/null > +++ b/drivers/dma-buf/heaps/cma_heap.c > @@ -0,0 +1,164 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * DMABUF CMA heap exporter > + * > + * Copyright (C) 2012, 2019 Linaro Ltd. > + * Author: for ST-Ericsson. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "heap-helpers.h" > + > +struct cma_heap { > + struct dma_heap heap; > + struct cma *cma; > +}; > + > + extra line > +#define to_cma_heap(x) container_of(x, struct cma_heap, heap) > + > + extra line > +static void cma_heap_free(struct heap_helper_buffer *buffer) > +{ > + struct cma_heap *cma_heap =3D to_cma_heap(buffer->heap_buffer.heap); > + struct page *pages =3D buffer->priv_virt; > + unsigned long nr_pages; > + > + nr_pages =3D PAGE_ALIGN(buffer->heap_buffer.size) >> PAGE_SHIFT; As you align at alloc time, I don't think the PAGE_ALIGN is really necessary here. > + > + /* release memory */ > + cma_release(cma_heap->cma, pages, nr_pages); > + /* release sg table */ > + sg_free_table(buffer->sg_table); > + kfree(buffer->sg_table); > + kfree(buffer); > +} > + > +/* dmabuf heap CMA operations functions */ > +static int cma_heap_allocate(struct dma_heap *heap, > + unsigned long len, > + unsigned long flags) > +{ > + struct cma_heap *cma_heap =3D to_cma_heap(heap); > + struct heap_helper_buffer *helper_buffer; > + struct sg_table *table; > + struct page *pages; > + size_t size =3D PAGE_ALIGN(len); > + unsigned long nr_pages =3D size >> PAGE_SHIFT; > + unsigned long align =3D get_order(size); > + DEFINE_DMA_BUF_EXPORT_INFO(exp_info); > + struct dma_buf *dmabuf; > + int ret =3D -ENOMEM; > + > + if (align > CONFIG_CMA_ALIGNMENT) > + align =3D CONFIG_CMA_ALIGNMENT; > + > + helper_buffer =3D 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 =3D flags; > + helper_buffer->heap_buffer.heap =3D heap; > + helper_buffer->heap_buffer.size =3D len; > + > + extra line > + pages =3D cma_alloc(cma_heap->cma, nr_pages, align, false); > + if (!pages) > + goto free_buf; > + > + if (PageHighMem(pages)) { > + unsigned long nr_clear_pages =3D nr_pages; > + struct page *page =3D pages; > + > + while (nr_clear_pages > 0) { > + void *vaddr =3D kmap_atomic(page); > + > + memset(vaddr, 0, PAGE_SIZE); > + kunmap_atomic(vaddr); > + page++; > + nr_clear_pages--; > + } > + } else { > + memset(page_address(pages), 0, size); > + } > + > + table =3D kmalloc(sizeof(*table), GFP_KERNEL); > + if (!table) > + goto free_cma; > + > + ret =3D sg_alloc_table(table, 1, GFP_KERNEL); > + if (ret) > + goto free_table; > + > + sg_set_page(table->sgl, pages, size, 0); > + > + /* create the dmabuf */ > + exp_info.ops =3D &heap_helper_ops; > + exp_info.size =3D len; > + exp_info.flags =3D O_RDWR; > + exp_info.priv =3D &helper_buffer->heap_buffer; > + dmabuf =3D dma_buf_export(&exp_info); > + if (IS_ERR(dmabuf)) { > + ret =3D PTR_ERR(dmabuf); > + goto free_table; > + } > + > + helper_buffer->heap_buffer.dmabuf =3D dmabuf; > + helper_buffer->priv_virt =3D pages; > + helper_buffer->sg_table =3D table; > + > + ret =3D dma_buf_fd(dmabuf, O_CLOEXEC); > + if (ret < 0) { > + dma_buf_put(dmabuf); > + /* just return, as put will call release and that will free */ > + return ret; > + } > + > + return ret; > +free_table: > + kfree(table); > +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 =3D { > + .allocate =3D cma_heap_allocate, > +}; > + > +static int __add_cma_heaps(struct cma *cma, void *data) nit: __add_cma_heap (not plural) seems more accurate. Whatever you decide for the above, you can add my r-b. Thanks, -Brian > +{ > + struct cma_heap *cma_heap; > + > + cma_heap =3D kzalloc(sizeof(*cma_heap), GFP_KERNEL); > + > + if (!cma_heap) > + return -ENOMEM; > + > + cma_heap->heap.name =3D cma_get_name(cma); > + cma_heap->heap.ops =3D &cma_heap_ops; > + cma_heap->cma =3D cma; > + > + dma_heap_add(&cma_heap->heap); > + > + return 0; > +} > + > +static int add_cma_heaps(void) > +{ > + cma_for_each_area(__add_cma_heaps, NULL); > + return 0; > +} > +device_initcall(add_cma_heaps); > --=20 > 2.7.4 >=20