This new export type exposes to userspace the SRAM area as a DMA-Heap, this allows for allocations as DMA-BUFs that can be consumed by various DMA-BUF supporting devices.
Signed-off-by: Andrew F. Davis afd@ti.com --- .../devicetree/bindings/sram/sram.yaml | 8 + drivers/misc/Kconfig | 7 + drivers/misc/Makefile | 1 + drivers/misc/sram-dma-heap.c | 243 ++++++++++++++++++ drivers/misc/sram.c | 20 +- drivers/misc/sram.h | 17 ++ 6 files changed, 292 insertions(+), 4 deletions(-) create mode 100644 drivers/misc/sram-dma-heap.c
diff --git a/Documentation/devicetree/bindings/sram/sram.yaml b/Documentation/devicetree/bindings/sram/sram.yaml index 7b83cc6c9bfa..b8e33c8d205d 100644 --- a/Documentation/devicetree/bindings/sram/sram.yaml +++ b/Documentation/devicetree/bindings/sram/sram.yaml @@ -105,6 +105,14 @@ patternProperties: manipulation of the page attributes. type: boolean
+ dma-heap-export: + description: + Similar to 'pool' and 'export' this region will be exported for use + by drivers, devices, and userspace using the DMA-Heaps framework. + NOTE: This region must be page aligned on start and end in order to + properly allow manipulation of the page attributes. + type: boolean + label: description: The name for the reserved partition, if omitted, the label is taken diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 99e151475d8f..10a9aed531c4 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -423,6 +423,13 @@ config SRAM config SRAM_EXEC bool
+config SRAM_DMA_HEAP + bool "Export on-chip SRAM pools using DMA-Heaps" + depends on DMABUF_HEAPS && SRAM + help + This driver allows the export of on-chip SRAM marked as exportable + to userspace using the DMA-Heaps interface. + config VEXPRESS_SYSCFG bool "Versatile Express System Configuration driver" depends on VEXPRESS_CONFIG diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 9abf2923d831..c5b5db26ebb2 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -46,6 +46,7 @@ obj-$(CONFIG_VMWARE_VMCI) += vmw_vmci/ obj-$(CONFIG_LATTICE_ECP3_CONFIG) += lattice-ecp3-config.o obj-$(CONFIG_SRAM) += sram.o obj-$(CONFIG_SRAM_EXEC) += sram-exec.o +obj-$(CONFIG_SRAM_DMA_HEAP) += sram-dma-heap.o obj-y += mic/ obj-$(CONFIG_GENWQE) += genwqe/ obj-$(CONFIG_ECHO) += echo/ diff --git a/drivers/misc/sram-dma-heap.c b/drivers/misc/sram-dma-heap.c new file mode 100644 index 000000000000..38df0397f294 --- /dev/null +++ b/drivers/misc/sram-dma-heap.c @@ -0,0 +1,243 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * SRAM DMA-Heap userspace exporter + * + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ + * Andrew F. Davis afd@ti.com + */ + +#include <linux/dma-mapping.h> +#include <linux/err.h> +#include <linux/genalloc.h> +#include <linux/io.h> +#include <linux/mm.h> +#include <linux/scatterlist.h> +#include <linux/slab.h> +#include <linux/dma-buf.h> +#include <linux/dma-heap.h> + +#include "sram.h" + +struct sram_dma_heap { + struct dma_heap *heap; + struct gen_pool *pool; +}; + +struct sram_dma_heap_buffer { + struct gen_pool *pool; + struct list_head attachments; + struct mutex attachments_lock; + unsigned long len; + void *vaddr; + phys_addr_t paddr; +}; + +struct dma_heap_attachment { + struct device *dev; + struct sg_table *table; + struct list_head list; +}; + +static int dma_heap_attach(struct dma_buf *dmabuf, + struct dma_buf_attachment *attachment) +{ + struct sram_dma_heap_buffer *buffer = dmabuf->priv; + struct dma_heap_attachment *a; + struct sg_table *table; + + a = kzalloc(sizeof(*a), GFP_KERNEL); + if (!a) + return -ENOMEM; + + table = kmalloc(sizeof(*table), GFP_KERNEL); + if (!table) { + kfree(a); + return -ENOMEM; + } + if (sg_alloc_table(table, 1, GFP_KERNEL)) { + kfree(table); + kfree(a); + return -ENOMEM; + } + sg_set_page(table->sgl, pfn_to_page(PFN_DOWN(buffer->paddr)), buffer->len, 0); + + a->table = table; + a->dev = attachment->dev; + INIT_LIST_HEAD(&a->list); + + attachment->priv = a; + + mutex_lock(&buffer->attachments_lock); + list_add(&a->list, &buffer->attachments); + mutex_unlock(&buffer->attachments_lock); + + return 0; +} + +static void dma_heap_detatch(struct dma_buf *dmabuf, + struct dma_buf_attachment *attachment) +{ + struct sram_dma_heap_buffer *buffer = dmabuf->priv; + struct dma_heap_attachment *a = attachment->priv; + + mutex_lock(&buffer->attachments_lock); + list_del(&a->list); + mutex_unlock(&buffer->attachments_lock); + + sg_free_table(a->table); + kfree(a->table); + kfree(a); +} + +static struct sg_table *dma_heap_map_dma_buf(struct dma_buf_attachment *attachment, + enum dma_data_direction direction) +{ + struct dma_heap_attachment *a = attachment->priv; + struct sg_table *table = a->table; + + if (!dma_map_sg_attrs(attachment->dev, table->sgl, table->nents, + direction, DMA_ATTR_SKIP_CPU_SYNC)) + return ERR_PTR(-ENOMEM); + + return table; +} + +static void dma_heap_unmap_dma_buf(struct dma_buf_attachment *attachment, + struct sg_table *table, + enum dma_data_direction direction) +{ + dma_unmap_sg_attrs(attachment->dev, table->sgl, table->nents, + direction, DMA_ATTR_SKIP_CPU_SYNC); +} + +static void dma_heap_dma_buf_release(struct dma_buf *dmabuf) +{ + struct sram_dma_heap_buffer *buffer = dmabuf->priv; + + gen_pool_free(buffer->pool, (unsigned long)buffer->vaddr, buffer->len); + kfree(buffer); +} + +static int dma_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma) +{ + struct sram_dma_heap_buffer *buffer = dmabuf->priv; + int ret; + + /* SRAM mappings are not cached */ + vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot); + + ret = vm_iomap_memory(vma, buffer->paddr, buffer->len); + if (ret) + pr_err("Could not map buffer to userspace\n"); + + return ret; +} + +static void *dma_heap_vmap(struct dma_buf *dmabuf) +{ + struct sram_dma_heap_buffer *buffer = dmabuf->priv; + + return buffer->vaddr; +} + +const struct dma_buf_ops sram_dma_heap_buf_ops = { + .attach = dma_heap_attach, + .detach = dma_heap_detatch, + .map_dma_buf = dma_heap_map_dma_buf, + .unmap_dma_buf = dma_heap_unmap_dma_buf, + .release = dma_heap_dma_buf_release, + .mmap = dma_heap_mmap, + .vmap = dma_heap_vmap, +}; + +static int sram_dma_heap_allocate(struct dma_heap *heap, + unsigned long len, + unsigned long fd_flags, + unsigned long heap_flags) +{ + struct sram_dma_heap *sram_dma_heap = dma_heap_get_drvdata(heap); + struct sram_dma_heap_buffer *buffer; + + DEFINE_DMA_BUF_EXPORT_INFO(exp_info); + struct dma_buf *dmabuf; + int ret; + + buffer = kzalloc(sizeof(*buffer), GFP_KERNEL); + if (!buffer) + return -ENOMEM; + buffer->pool = sram_dma_heap->pool; + INIT_LIST_HEAD(&buffer->attachments); + mutex_init(&buffer->attachments_lock); + buffer->len = len; + + buffer->vaddr = (void *)gen_pool_alloc(buffer->pool, buffer->len); + if (!buffer->vaddr) { + ret = -ENOMEM; + goto free_buffer; + } + + buffer->paddr = gen_pool_virt_to_phys(buffer->pool, (unsigned long)buffer->vaddr); + if (buffer->paddr == -1) { + ret = -ENOMEM; + goto free_pool; + } + + /* create the dmabuf */ + exp_info.ops = &sram_dma_heap_buf_ops; + exp_info.size = buffer->len; + exp_info.flags = fd_flags; + exp_info.priv = buffer; + dmabuf = dma_buf_export(&exp_info); + if (IS_ERR(dmabuf)) { + ret = PTR_ERR(dmabuf); + goto free_pool; + } + + 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_pool: + gen_pool_free(buffer->pool, (unsigned long)buffer->vaddr, buffer->len); +free_buffer: + kfree(buffer); + + return ret; +} + +static struct dma_heap_ops sram_dma_heap_ops = { + .allocate = sram_dma_heap_allocate, +}; + +int sram_dma_heap_export(struct sram_dev *sram, + struct sram_reserve *block, + phys_addr_t start, + struct sram_partition *part) +{ + struct sram_dma_heap *sram_dma_heap; + struct dma_heap_export_info exp_info; + + dev_info(sram->dev, "Exporting SRAM pool '%s'\n", block->label); + + sram_dma_heap = kzalloc(sizeof(*sram_dma_heap), GFP_KERNEL); + if (!sram_dma_heap) + return -ENOMEM; + sram_dma_heap->pool = part->pool; + + exp_info.name = kasprintf(GFP_KERNEL, "sram_%s", block->label); + exp_info.ops = &sram_dma_heap_ops; + exp_info.priv = sram_dma_heap; + sram_dma_heap->heap = dma_heap_add(&exp_info); + if (IS_ERR(sram_dma_heap->heap)) { + int ret = PTR_ERR(sram_dma_heap->heap); + kfree(sram_dma_heap); + return ret; + } + + return 0; +} diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c index 6c1a23cb3e8c..0337e271cfe2 100644 --- a/drivers/misc/sram.c +++ b/drivers/misc/sram.c @@ -109,6 +109,15 @@ static int sram_add_partition(struct sram_dev *sram, struct sram_reserve *block, if (ret) return ret; } + if (block->dma_heap_export) { + ret = sram_add_pool(sram, block, start, part); + if (ret) + return ret; + + ret = sram_dma_heap_export(sram, block, start, part); + if (ret) + return ret; + } if (block->protect_exec) { ret = sram_check_protect_exec(sram, block, part); if (ret) @@ -209,8 +218,11 @@ static int sram_reserve_regions(struct sram_dev *sram, struct resource *res) if (of_find_property(child, "protect-exec", NULL)) block->protect_exec = true;
- if ((block->export || block->pool || block->protect_exec) && - block->size) { + if (of_find_property(child, "dma-heap-export", NULL)) + block->dma_heap_export = true; + + if ((block->export || block->pool || block->protect_exec || + block->dma_heap_export) && block->size) { exports++;
label = NULL; @@ -272,8 +284,8 @@ static int sram_reserve_regions(struct sram_dev *sram, struct resource *res) goto err_chunks; }
- if ((block->export || block->pool || block->protect_exec) && - block->size) { + if ((block->export || block->pool || block->protect_exec || + block->dma_heap_export) && block->size) { ret = sram_add_partition(sram, block, res->start + block->start); if (ret) { diff --git a/drivers/misc/sram.h b/drivers/misc/sram.h index 9c1d21ff7347..e60ab13e8e6b 100644 --- a/drivers/misc/sram.h +++ b/drivers/misc/sram.h @@ -32,6 +32,7 @@ struct sram_reserve { bool export; bool pool; bool protect_exec; + bool dma_heap_export; const char *label; };
@@ -52,4 +53,20 @@ static inline int sram_add_protect_exec(struct sram_partition *part) return -ENODEV; } #endif /* CONFIG_SRAM_EXEC */ + +#ifdef CONFIG_SRAM_DMA_HEAP +int sram_dma_heap_export(struct sram_dev *sram, + struct sram_reserve *block, + phys_addr_t start, + struct sram_partition *part); +#else +static inline int sram_dma_heap_export(struct sram_dev *sram, + struct sram_reserve *block, + phys_addr_t start, + struct sram_partition *part) +{ + return 0; +} +#endif /* CONFIG_SRAM_DMA_HEAP */ + #endif /* __SRAM_H */
On Fri, Apr 24, 2020 at 3:27 PM Andrew F. Davis afd@ti.com wrote:
This new export type exposes to userspace the SRAM area as a DMA-Heap, this allows for allocations as DMA-BUFs that can be consumed by various DMA-BUF supporting devices.
Signed-off-by: Andrew F. Davis afd@ti.com
Nice! Very excited to have the first new heap (that didn't come with the initial patchset)!
Overall looks good! I don't have any comment on the SRAM side of things, but a few minor questions/nits below.
diff --git a/drivers/misc/sram-dma-heap.c b/drivers/misc/sram-dma-heap.c new file mode 100644 index 000000000000..38df0397f294 --- /dev/null +++ b/drivers/misc/sram-dma-heap.c @@ -0,0 +1,243 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- SRAM DMA-Heap userspace exporter
- Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
Andrew F. Davis <afd@ti.com>- */
+#include <linux/dma-mapping.h> +#include <linux/err.h> +#include <linux/genalloc.h> +#include <linux/io.h> +#include <linux/mm.h> +#include <linux/scatterlist.h> +#include <linux/slab.h> +#include <linux/dma-buf.h> +#include <linux/dma-heap.h>
+#include "sram.h"
+struct sram_dma_heap {
struct dma_heap *heap;struct gen_pool *pool;+};
+struct sram_dma_heap_buffer {
struct gen_pool *pool;struct list_head attachments;struct mutex attachments_lock;unsigned long len;void *vaddr;phys_addr_t paddr;+};
+struct dma_heap_attachment {
struct device *dev;struct sg_table *table;struct list_head list;+};
+static int dma_heap_attach(struct dma_buf *dmabuf,
struct dma_buf_attachment *attachment)+{
struct sram_dma_heap_buffer *buffer = dmabuf->priv;struct dma_heap_attachment *a;struct sg_table *table;a = kzalloc(sizeof(*a), GFP_KERNEL);if (!a)return -ENOMEM;table = kmalloc(sizeof(*table), GFP_KERNEL);if (!table) {kfree(a);return -ENOMEM;}if (sg_alloc_table(table, 1, GFP_KERNEL)) {kfree(table);kfree(a);return -ENOMEM;}sg_set_page(table->sgl, pfn_to_page(PFN_DOWN(buffer->paddr)), buffer->len, 0);a->table = table;a->dev = attachment->dev;INIT_LIST_HEAD(&a->list);attachment->priv = a;mutex_lock(&buffer->attachments_lock);list_add(&a->list, &buffer->attachments);mutex_unlock(&buffer->attachments_lock);return 0;+}
+static void dma_heap_detatch(struct dma_buf *dmabuf,
struct dma_buf_attachment *attachment)+{
struct sram_dma_heap_buffer *buffer = dmabuf->priv;struct dma_heap_attachment *a = attachment->priv;mutex_lock(&buffer->attachments_lock);list_del(&a->list);mutex_unlock(&buffer->attachments_lock);sg_free_table(a->table);kfree(a->table);kfree(a);+}
+static struct sg_table *dma_heap_map_dma_buf(struct dma_buf_attachment *attachment,
enum dma_data_direction direction)+{
struct dma_heap_attachment *a = attachment->priv;struct sg_table *table = a->table;if (!dma_map_sg_attrs(attachment->dev, table->sgl, table->nents,direction, DMA_ATTR_SKIP_CPU_SYNC))
Might be nice to have a comment as to why you're using SKIP_CPU_SYNC and why it's safe.
return ERR_PTR(-ENOMEM);return table;+}
+static void dma_heap_unmap_dma_buf(struct dma_buf_attachment *attachment,
struct sg_table *table,enum dma_data_direction direction)+{
dma_unmap_sg_attrs(attachment->dev, table->sgl, table->nents,direction, DMA_ATTR_SKIP_CPU_SYNC);+}
+static void dma_heap_dma_buf_release(struct dma_buf *dmabuf) +{
struct sram_dma_heap_buffer *buffer = dmabuf->priv;gen_pool_free(buffer->pool, (unsigned long)buffer->vaddr, buffer->len);kfree(buffer);+}
+static int dma_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma) +{
struct sram_dma_heap_buffer *buffer = dmabuf->priv;int ret;/* SRAM mappings are not cached */vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);ret = vm_iomap_memory(vma, buffer->paddr, buffer->len);if (ret)pr_err("Could not map buffer to userspace\n");return ret;+}
+static void *dma_heap_vmap(struct dma_buf *dmabuf) +{
struct sram_dma_heap_buffer *buffer = dmabuf->priv;return buffer->vaddr;+}
+const struct dma_buf_ops sram_dma_heap_buf_ops = {
.attach = dma_heap_attach,.detach = dma_heap_detatch,.map_dma_buf = dma_heap_map_dma_buf,.unmap_dma_buf = dma_heap_unmap_dma_buf,.release = dma_heap_dma_buf_release,.mmap = dma_heap_mmap,.vmap = dma_heap_vmap,+};
No begin/end_cpu_access functions here? I'm guessing it's because you're always using SKIP_CPU_SYNC so it wouldn't do anything? A small comment in the code might help.
+static int sram_dma_heap_allocate(struct dma_heap *heap,
unsigned long len,unsigned long fd_flags,unsigned long heap_flags)+{
struct sram_dma_heap *sram_dma_heap = dma_heap_get_drvdata(heap);struct sram_dma_heap_buffer *buffer;DEFINE_DMA_BUF_EXPORT_INFO(exp_info);struct dma_buf *dmabuf;int ret;buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);if (!buffer)return -ENOMEM;buffer->pool = sram_dma_heap->pool;INIT_LIST_HEAD(&buffer->attachments);mutex_init(&buffer->attachments_lock);buffer->len = len;buffer->vaddr = (void *)gen_pool_alloc(buffer->pool, buffer->len);if (!buffer->vaddr) {ret = -ENOMEM;goto free_buffer;}buffer->paddr = gen_pool_virt_to_phys(buffer->pool, (unsigned long)buffer->vaddr);if (buffer->paddr == -1) {ret = -ENOMEM;goto free_pool;}/* create the dmabuf */exp_info.ops = &sram_dma_heap_buf_ops;exp_info.size = buffer->len;exp_info.flags = fd_flags;exp_info.priv = buffer;dmabuf = dma_buf_export(&exp_info);if (IS_ERR(dmabuf)) {ret = PTR_ERR(dmabuf);goto free_pool;}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_pool:
gen_pool_free(buffer->pool, (unsigned long)buffer->vaddr, buffer->len);+free_buffer:
kfree(buffer);return ret;+}
+static struct dma_heap_ops sram_dma_heap_ops = {
.allocate = sram_dma_heap_allocate,+};
+int sram_dma_heap_export(struct sram_dev *sram,
This is totally a bikeshed thing (feel free to ignore), but maybe sram_dma_heap_create() or _add() would be a better name to avoid folks mixing it up with the dmabuf exporter?
struct sram_reserve *block,phys_addr_t start,struct sram_partition *part)+{
struct sram_dma_heap *sram_dma_heap;struct dma_heap_export_info exp_info;dev_info(sram->dev, "Exporting SRAM pool '%s'\n", block->label);
Again, shed issue: but for terminology consistency (at least in the dmabuf heaps space), maybe heap instead of pool?
Thanks so much again for submitting this! -john
On 4/24/20 8:44 PM, John Stultz wrote:
On Fri, Apr 24, 2020 at 3:27 PM Andrew F. Davis afd@ti.com wrote:
This new export type exposes to userspace the SRAM area as a DMA-Heap, this allows for allocations as DMA-BUFs that can be consumed by various DMA-BUF supporting devices.
Signed-off-by: Andrew F. Davis afd@ti.com
Nice! Very excited to have the first new heap (that didn't come with the initial patchset)!
Overall looks good! I don't have any comment on the SRAM side of things, but a few minor questions/nits below.
diff --git a/drivers/misc/sram-dma-heap.c b/drivers/misc/sram-dma-heap.c new file mode 100644 index 000000000000..38df0397f294 --- /dev/null +++ b/drivers/misc/sram-dma-heap.c @@ -0,0 +1,243 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- SRAM DMA-Heap userspace exporter
- Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
Andrew F. Davis <afd@ti.com>- */
+#include <linux/dma-mapping.h> +#include <linux/err.h> +#include <linux/genalloc.h> +#include <linux/io.h> +#include <linux/mm.h> +#include <linux/scatterlist.h> +#include <linux/slab.h> +#include <linux/dma-buf.h> +#include <linux/dma-heap.h>
+#include "sram.h"
+struct sram_dma_heap {
struct dma_heap *heap;struct gen_pool *pool;+};
+struct sram_dma_heap_buffer {
struct gen_pool *pool;struct list_head attachments;struct mutex attachments_lock;unsigned long len;void *vaddr;phys_addr_t paddr;+};
+struct dma_heap_attachment {
struct device *dev;struct sg_table *table;struct list_head list;+};
+static int dma_heap_attach(struct dma_buf *dmabuf,
struct dma_buf_attachment *attachment)+{
struct sram_dma_heap_buffer *buffer = dmabuf->priv;struct dma_heap_attachment *a;struct sg_table *table;a = kzalloc(sizeof(*a), GFP_KERNEL);if (!a)return -ENOMEM;table = kmalloc(sizeof(*table), GFP_KERNEL);if (!table) {kfree(a);return -ENOMEM;}if (sg_alloc_table(table, 1, GFP_KERNEL)) {kfree(table);kfree(a);return -ENOMEM;}sg_set_page(table->sgl, pfn_to_page(PFN_DOWN(buffer->paddr)), buffer->len, 0);a->table = table;a->dev = attachment->dev;INIT_LIST_HEAD(&a->list);attachment->priv = a;mutex_lock(&buffer->attachments_lock);list_add(&a->list, &buffer->attachments);mutex_unlock(&buffer->attachments_lock);return 0;+}
+static void dma_heap_detatch(struct dma_buf *dmabuf,
struct dma_buf_attachment *attachment)+{
struct sram_dma_heap_buffer *buffer = dmabuf->priv;struct dma_heap_attachment *a = attachment->priv;mutex_lock(&buffer->attachments_lock);list_del(&a->list);mutex_unlock(&buffer->attachments_lock);sg_free_table(a->table);kfree(a->table);kfree(a);+}
+static struct sg_table *dma_heap_map_dma_buf(struct dma_buf_attachment *attachment,
enum dma_data_direction direction)+{
struct dma_heap_attachment *a = attachment->priv;struct sg_table *table = a->table;if (!dma_map_sg_attrs(attachment->dev, table->sgl, table->nents,direction, DMA_ATTR_SKIP_CPU_SYNC))Might be nice to have a comment as to why you're using SKIP_CPU_SYNC and why it's safe.
Ack, should be simple enough to explain that SRAM is non-cached and so this sync is not needed (and may not work either given the SRAM region does not have valid page structures assdociated).
return ERR_PTR(-ENOMEM);return table;+}
+static void dma_heap_unmap_dma_buf(struct dma_buf_attachment *attachment,
struct sg_table *table,enum dma_data_direction direction)+{
dma_unmap_sg_attrs(attachment->dev, table->sgl, table->nents,direction, DMA_ATTR_SKIP_CPU_SYNC);+}
+static void dma_heap_dma_buf_release(struct dma_buf *dmabuf) +{
struct sram_dma_heap_buffer *buffer = dmabuf->priv;gen_pool_free(buffer->pool, (unsigned long)buffer->vaddr, buffer->len);kfree(buffer);+}
+static int dma_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma) +{
struct sram_dma_heap_buffer *buffer = dmabuf->priv;int ret;/* SRAM mappings are not cached */vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);ret = vm_iomap_memory(vma, buffer->paddr, buffer->len);if (ret)pr_err("Could not map buffer to userspace\n");return ret;+}
+static void *dma_heap_vmap(struct dma_buf *dmabuf) +{
struct sram_dma_heap_buffer *buffer = dmabuf->priv;return buffer->vaddr;+}
+const struct dma_buf_ops sram_dma_heap_buf_ops = {
.attach = dma_heap_attach,.detach = dma_heap_detatch,.map_dma_buf = dma_heap_map_dma_buf,.unmap_dma_buf = dma_heap_unmap_dma_buf,.release = dma_heap_dma_buf_release,.mmap = dma_heap_mmap,.vmap = dma_heap_vmap,+};
No begin/end_cpu_access functions here? I'm guessing it's because you're always using SKIP_CPU_SYNC so it wouldn't do anything? A small comment in the code might help.
Yes, same idea, non-cached/coherent means the access does not need to be bracketed by *_cpu_access functions. Will comment.
+static int sram_dma_heap_allocate(struct dma_heap *heap,
unsigned long len,unsigned long fd_flags,unsigned long heap_flags)+{
struct sram_dma_heap *sram_dma_heap = dma_heap_get_drvdata(heap);struct sram_dma_heap_buffer *buffer;DEFINE_DMA_BUF_EXPORT_INFO(exp_info);struct dma_buf *dmabuf;int ret;buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);if (!buffer)return -ENOMEM;buffer->pool = sram_dma_heap->pool;INIT_LIST_HEAD(&buffer->attachments);mutex_init(&buffer->attachments_lock);buffer->len = len;buffer->vaddr = (void *)gen_pool_alloc(buffer->pool, buffer->len);if (!buffer->vaddr) {ret = -ENOMEM;goto free_buffer;}buffer->paddr = gen_pool_virt_to_phys(buffer->pool, (unsigned long)buffer->vaddr);if (buffer->paddr == -1) {ret = -ENOMEM;goto free_pool;}/* create the dmabuf */exp_info.ops = &sram_dma_heap_buf_ops;exp_info.size = buffer->len;exp_info.flags = fd_flags;exp_info.priv = buffer;dmabuf = dma_buf_export(&exp_info);if (IS_ERR(dmabuf)) {ret = PTR_ERR(dmabuf);goto free_pool;}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_pool:
gen_pool_free(buffer->pool, (unsigned long)buffer->vaddr, buffer->len);+free_buffer:
kfree(buffer);return ret;+}
+static struct dma_heap_ops sram_dma_heap_ops = {
.allocate = sram_dma_heap_allocate,+};
+int sram_dma_heap_export(struct sram_dev *sram,
This is totally a bikeshed thing (feel free to ignore), but maybe sram_dma_heap_create() or _add() would be a better name to avoid folks mixing it up with the dmabuf exporter?
Maybe sram_add_dma_heap() which will match up better with the other SRAM functions, will change.
struct sram_reserve *block,phys_addr_t start,struct sram_partition *part)+{
struct sram_dma_heap *sram_dma_heap;struct dma_heap_export_info exp_info;dev_info(sram->dev, "Exporting SRAM pool '%s'\n", block->label);Again, shed issue: but for terminology consistency (at least in the dmabuf heaps space), maybe heap instead of pool?
Ack, s/pool/heap.
Thanks so much again for submitting this!
Thanks for the review, Andrew
-john
On Fri, Apr 24, 2020 at 5:27 PM Andrew F. Davis afd@ti.com wrote:
This new export type exposes to userspace the SRAM area as a DMA-Heap, this allows for allocations as DMA-BUFs that can be consumed by various DMA-BUF supporting devices.
Signed-off-by: Andrew F. Davis afd@ti.com
.../devicetree/bindings/sram/sram.yaml | 8 +
Separate patch and needs to go to DT list...
drivers/misc/Kconfig | 7 + drivers/misc/Makefile | 1 + drivers/misc/sram-dma-heap.c | 243 ++++++++++++++++++ drivers/misc/sram.c | 20 +- drivers/misc/sram.h | 17 ++ 6 files changed, 292 insertions(+), 4 deletions(-) create mode 100644 drivers/misc/sram-dma-heap.c
diff --git a/Documentation/devicetree/bindings/sram/sram.yaml b/Documentation/devicetree/bindings/sram/sram.yaml index 7b83cc6c9bfa..b8e33c8d205d 100644 --- a/Documentation/devicetree/bindings/sram/sram.yaml +++ b/Documentation/devicetree/bindings/sram/sram.yaml @@ -105,6 +105,14 @@ patternProperties: manipulation of the page attributes. type: boolean
dma-heap-export:description:Similar to 'pool' and 'export' this region will be exported for useby drivers, devices, and userspace using the DMA-Heaps framework.NOTE: This region must be page aligned on start and end in order toproperly allow manipulation of the page attributes.type: boolean
Though I'm not sure this should be in DT. You have to change your firmware to enable a new kernel feature? We also already have 'export' which sounds like the same function. Or 'pool' though reading the description, I don't really understand it's use.
What combination of all 3 of these options would be valid?
Rob
On 4/27/20 11:17 AM, Rob Herring wrote:
On Fri, Apr 24, 2020 at 5:27 PM Andrew F. Davis afd@ti.com wrote:
This new export type exposes to userspace the SRAM area as a DMA-Heap, this allows for allocations as DMA-BUFs that can be consumed by various DMA-BUF supporting devices.
Signed-off-by: Andrew F. Davis afd@ti.com
.../devicetree/bindings/sram/sram.yaml | 8 +
Separate patch and needs to go to DT list...
Okay, will split for v2.
drivers/misc/Kconfig | 7 + drivers/misc/Makefile | 1 + drivers/misc/sram-dma-heap.c | 243 ++++++++++++++++++ drivers/misc/sram.c | 20 +- drivers/misc/sram.h | 17 ++ 6 files changed, 292 insertions(+), 4 deletions(-) create mode 100644 drivers/misc/sram-dma-heap.c
diff --git a/Documentation/devicetree/bindings/sram/sram.yaml b/Documentation/devicetree/bindings/sram/sram.yaml index 7b83cc6c9bfa..b8e33c8d205d 100644 --- a/Documentation/devicetree/bindings/sram/sram.yaml +++ b/Documentation/devicetree/bindings/sram/sram.yaml @@ -105,6 +105,14 @@ patternProperties: manipulation of the page attributes. type: boolean
dma-heap-export:description:Similar to 'pool' and 'export' this region will be exported for useby drivers, devices, and userspace using the DMA-Heaps framework.NOTE: This region must be page aligned on start and end in order toproperly allow manipulation of the page attributes.type: booleanThough I'm not sure this should be in DT. You have to change your firmware to enable a new kernel feature? We also already have 'export' which sounds like the same function. Or 'pool' though reading the description, I don't really understand it's use.
Maybe I could just re-use 'export', right now that property causes the SRAM region to be exported to userspace as a file one can read/write. Exporting via dma-heaps/dma-buf allows more flexibility as one can pass the exported regions to other devices or mmap them directly in userspace.
It's up to the SRAM driver maintainer if re-purposing that property in the driver is acceptable.
'Pool' doesn't make much sense to me either, it creates a gen-pool out of the region, but I cant find any users not in combination with the other properties.
What combination of all 3 of these options would be valid?
From looking at the SRAM driver, it seems each of the option must be
used exclusively per region, combining will cause issues. I can update the documentation for the same after we settle on a strategy for this new property.
Andrew
Rob
dri-devel@lists.freedesktop.org
-
Andrew F. Davis -
John Stultz -
Rob Herring