From: Randy Li <[email protected]>
This module is still at a early stage, I wrote this for showing what
APIs we need here.
Let me explain why we need such a module here.
If you won't allocate buffers from a V4L2 M2M device, this module
may not be very useful. I am sure the most of users won't know a
device would require them allocate buffers from a DMA-Heap then
import those buffers into a V4L2's queue.
Then the question goes back to why DMA-Heap. From the Android's
description, we know it is about the copyright's DRM.
When we allocate a buffer in a DMA-Heap, it may register that buffer
in the trusted execution environment so the firmware which is running
or could only be acccesed from there could use that buffer later.
The answer above leads to another thing which is not done in this
version, the DMA mapping. Although in some platforms, a DMA-Heap
responses a IOMMU device as well. For the genernal purpose, we would
be better assuming the device mapping should be done for each device
itself. The problem here we only know alloc_devs in those DMAbuf
methods, which are DMA-heaps in my design, the device from the queue
is not enough, a plane may requests another IOMMU device or table
for mapping.
Signed-off-by: Randy Li <[email protected]>
---
drivers/media/common/videobuf2/Kconfig | 6 +
drivers/media/common/videobuf2/Makefile | 1 +
.../common/videobuf2/videobuf2-dma-heap.c | 350 ++++++++++++++++++
include/media/videobuf2-dma-heap.h | 30 ++
4 files changed, 387 insertions(+)
create mode 100644 drivers/media/common/videobuf2/videobuf2-dma-heap.c
create mode 100644 include/media/videobuf2-dma-heap.h
diff --git a/drivers/media/common/videobuf2/Kconfig b/drivers/media/common/videobuf2/Kconfig
index d2223a12c95f..02235077f07e 100644
--- a/drivers/media/common/videobuf2/Kconfig
+++ b/drivers/media/common/videobuf2/Kconfig
@@ -30,3 +30,9 @@ config VIDEOBUF2_DMA_SG
config VIDEOBUF2_DVB
tristate
select VIDEOBUF2_CORE
+
+config VIDEOBUF2_DMA_HEAP
+ tristate
+ select VIDEOBUF2_CORE
+ select VIDEOBUF2_MEMOPS
+ select DMABUF_HEAPS
diff --git a/drivers/media/common/videobuf2/Makefile b/drivers/media/common/videobuf2/Makefile
index a6fe3f304685..7fe65f93117f 100644
--- a/drivers/media/common/videobuf2/Makefile
+++ b/drivers/media/common/videobuf2/Makefile
@@ -10,6 +10,7 @@ endif
# (e. g. LC_ALL=C sort Makefile)
obj-$(CONFIG_VIDEOBUF2_CORE) += videobuf2-common.o
obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
+obj-$(CONFIG_VIDEOBUF2_DMA_HEAP) += videobuf2-dma-heap.o
obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
obj-$(CONFIG_VIDEOBUF2_DVB) += videobuf2-dvb.o
obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-heap.c b/drivers/media/common/videobuf2/videobuf2-dma-heap.c
new file mode 100644
index 000000000000..377b82ab8f5a
--- /dev/null
+++ b/drivers/media/common/videobuf2/videobuf2-dma-heap.c
@@ -0,0 +1,350 @@
+/*
+ * Copyright (C) 2022 Randy Li <[email protected]>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/dma-buf.h>
+#include <linux/dma-heap.h>
+#include <linux/refcount.h>
+#include <linux/scatterlist.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/dma-mapping.h>
+
+#include <media/videobuf2-v4l2.h>
+#include <media/videobuf2-memops.h>
+#include <media/videobuf2-dma-heap.h>
+
+struct vb2_dmaheap_buf {
+ struct device *dev;
+ void *vaddr;
+ unsigned long size;
+ struct dma_buf *dmabuf;
+ dma_addr_t dma_addr;
+ unsigned long attrs;
+ enum dma_data_direction dma_dir;
+ struct sg_table *dma_sgt;
+
+ /* MMAP related */
+ struct vb2_vmarea_handler handler;
+ refcount_t refcount;
+
+ /* DMABUF related */
+ struct dma_buf_attachment *db_attach;
+};
+
+/*********************************************/
+/* callbacks for all buffers */
+/*********************************************/
+
+void *vb2_dmaheap_cookie(struct vb2_buffer *vb, void *buf_priv)
+{
+ struct vb2_dmaheap_buf *buf = buf_priv;
+
+ return &buf->dma_addr;
+}
+
+static void *vb2_dmaheap_vaddr(struct vb2_buffer *vb, void *buf_priv)
+{
+ struct vb2_dmaheap_buf *buf = buf_priv;
+ struct iosys_map map;
+
+ if (buf->vaddr)
+ return buf->vaddr;
+
+ if (buf->db_attach) {
+ if (!dma_buf_vmap(buf->db_attach->dmabuf, &map))
+ buf->vaddr = map.vaddr;
+ }
+
+ return buf->vaddr;
+}
+
+static unsigned int vb2_dmaheap_num_users(void *buf_priv)
+{
+ struct vb2_dmaheap_buf *buf = buf_priv;
+
+ return refcount_read(&buf->refcount);
+}
+
+static void vb2_dmaheap_prepare(void *buf_priv)
+{
+ struct vb2_dmaheap_buf *buf = buf_priv;
+
+ /* TODO: DMABUF exporter will flush the cache for us */
+ if (buf->db_attach)
+ return;
+
+ dma_buf_end_cpu_access(buf->dmabuf, buf->dma_dir);
+}
+
+static void vb2_dmaheap_finish(void *buf_priv)
+{
+ struct vb2_dmaheap_buf *buf = buf_priv;
+
+ /* TODO: DMABUF exporter will flush the cache for us */
+ if (buf->db_attach)
+ return;
+
+ dma_buf_begin_cpu_access(buf->dmabuf, buf->dma_dir);
+}
+
+/*********************************************/
+/* callbacks for MMAP buffers */
+/*********************************************/
+
+void vb2_dmaheap_put(void *buf_priv)
+{
+ struct vb2_dmaheap_buf *buf = buf_priv;
+
+ if (!refcount_dec_and_test(&buf->refcount))
+ return;
+
+ dma_buf_put(buf->dmabuf);
+
+ put_device(buf->dev);
+ kfree(buf);
+}
+
+static void *vb2_dmaheap_alloc(struct vb2_buffer *vb,
+ struct device *dev,
+ unsigned long size)
+{
+ struct vb2_queue *q = vb->vb2_queue;
+ struct dma_heap *heap;
+ struct vb2_dmaheap_buf *buf;
+ const char *heap_name;
+ int ret;
+
+ if (WARN_ON(!dev))
+ return ERR_PTR(-EINVAL);
+
+ heap_name = dev_name(dev);
+ if (!heap_name)
+ return ERR_PTR(-EINVAL);
+
+ heap = dma_heap_find(heap_name);
+ if (!heap) {
+ dev_err(dev, "is not a DMA-heap device\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ buf = kzalloc(sizeof *buf, GFP_KERNEL);
+ if (!buf)
+ return ERR_PTR(-ENOMEM);
+
+ /* Prevent the device from being released while the buffer is used */
+ buf->dev = get_device(dev);
+ buf->attrs = vb->vb2_queue->dma_attrs;
+ buf->dma_dir = vb->vb2_queue->dma_dir;
+
+ /* TODO: heap flags */
+ ret = dma_heap_buffer_alloc(heap, size, 0, 0);
+ if (ret < 0) {
+ dev_err(dev, "is not a DMA-heap device\n");
+ put_device(buf->dev);
+ kfree(buf);
+ return ERR_PTR(ret);
+ }
+ buf->dmabuf = dma_buf_get(ret);
+
+ /* FIXME */
+ buf->dma_addr = 0;
+
+ if ((q->dma_attrs & DMA_ATTR_NO_KERNEL_MAPPING) == 0)
+ buf->vaddr = buf->dmabuf;
+
+ buf->handler.refcount = &buf->refcount;
+ buf->handler.put = vb2_dmaheap_put;
+ buf->handler.arg = buf;
+
+ refcount_set(&buf->refcount, 1);
+
+ return buf;
+}
+
+static int vb2_dmaheap_mmap(void *buf_priv, struct vm_area_struct *vma)
+{
+ struct vb2_dmaheap_buf *buf = buf_priv;
+ int ret;
+
+ if (!buf) {
+ printk(KERN_ERR "No buffer to map\n");
+ return -EINVAL;
+ }
+
+ vma->vm_flags &= ~VM_PFNMAP;
+
+ ret = dma_buf_mmap(buf->dmabuf, vma, 0);
+ if (ret) {
+ pr_err("Remapping memory failed, error: %d\n", ret);
+ return ret;
+ }
+ vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
+ vma->vm_private_data = &buf->handler;
+ vma->vm_ops = &vb2_common_vm_ops;
+
+ vma->vm_ops->open(vma);
+
+ pr_debug("%s: mapped memid 0x%08lx at 0x%08lx, size %ld\n",
+ __func__, (unsigned long)buf->dma_addr, vma->vm_start,
+ buf->size);
+
+ return 0;
+}
+
+/*********************************************/
+/* DMABUF ops for exporters */
+/*********************************************/
+
+static struct dma_buf *vb2_dmaheap_get_dmabuf(struct vb2_buffer *vb,
+ void *buf_priv,
+ unsigned long flags)
+{
+ struct vb2_dmaheap_buf *buf = buf_priv;
+ struct dma_buf *dbuf;
+
+ dbuf = buf->dmabuf;
+
+ return dbuf;
+}
+
+/*********************************************/
+/* callbacks for DMABUF buffers */
+/*********************************************/
+
+static int vb2_dmaheap_map_dmabuf(void *mem_priv)
+{
+ struct vb2_dmaheap_buf *buf = mem_priv;
+ struct sg_table *sgt;
+
+ if (WARN_ON(!buf->db_attach)) {
+ pr_err("trying to pin a non attached buffer\n");
+ return -EINVAL;
+ }
+
+ if (WARN_ON(buf->dma_sgt)) {
+ pr_err("dmabuf buffer is already pinned\n");
+ return 0;
+ }
+
+ /* get the associated scatterlist for this buffer */
+ sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir);
+ if (IS_ERR(sgt)) {
+ pr_err("Error getting dmabuf scatterlist\n");
+ return -EINVAL;
+ }
+
+ buf->dma_addr = sg_dma_address(sgt->sgl);
+ buf->dma_sgt = sgt;
+ buf->vaddr = NULL;
+
+ return 0;
+}
+
+static void vb2_dmaheap_unmap_dmabuf(void *mem_priv)
+{
+ struct vb2_dmaheap_buf *buf = mem_priv;
+ struct sg_table *sgt = buf->dma_sgt;
+ struct iosys_map map = IOSYS_MAP_INIT_VADDR(buf->vaddr);
+
+ if (WARN_ON(!buf->db_attach)) {
+ pr_err("trying to unpin a not attached buffer\n");
+ return;
+ }
+
+ if (WARN_ON(!sgt)) {
+ pr_err("dmabuf buffer is already unpinned\n");
+ return;
+ }
+
+ if (buf->vaddr) {
+ dma_buf_vunmap(buf->db_attach->dmabuf, &map);
+ buf->vaddr = NULL;
+ }
+ dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
+
+ buf->dma_addr = 0;
+ buf->dma_sgt = NULL;
+}
+
+static void vb2_dmaheap_detach_dmabuf(void *mem_priv)
+{
+ struct vb2_dmaheap_buf *buf = mem_priv;
+
+ /* if vb2 works correctly you should never detach mapped buffer */
+ if (WARN_ON(buf->dma_addr))
+ vb2_dmaheap_unmap_dmabuf(buf);
+
+ /* detach this attachment */
+ dma_buf_detach(buf->db_attach->dmabuf, buf->db_attach);
+ kfree(buf);
+}
+
+static void *vb2_dmaheap_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
+ struct dma_buf *dbuf, unsigned long size)
+{
+ struct vb2_dmaheap_buf *buf;
+ struct dma_buf_attachment *dba;
+
+ if (dbuf->size < size)
+ return ERR_PTR(-EFAULT);
+
+ if (WARN_ON(!dev))
+ return ERR_PTR(-EINVAL);
+ /*
+ * TODO: A better way to check whether the buffer is coming
+ * from this heap or this heap could accept this buffer
+ */
+ if (strcmp(dbuf->exp_name, dev_name(dev)))
+ return ERR_PTR(-EINVAL);
+
+ buf = kzalloc(sizeof(*buf), GFP_KERNEL);
+ if (!buf)
+ return ERR_PTR(-ENOMEM);
+
+ buf->dev = dev;
+ /* create attachment for the dmabuf with the user device */
+ dba = dma_buf_attach(dbuf, buf->dev);
+ if (IS_ERR(dba)) {
+ pr_err("failed to attach dmabuf\n");
+ kfree(buf);
+ return dba;
+ }
+
+ buf->dma_dir = vb->vb2_queue->dma_dir;
+ buf->size = size;
+ buf->db_attach = dba;
+
+ return buf;
+}
+
+const struct vb2_mem_ops vb2_dmaheap_memops = {
+ .alloc = vb2_dmaheap_alloc,
+ .put = vb2_dmaheap_put,
+ .get_dmabuf = vb2_dmaheap_get_dmabuf,
+ .cookie = vb2_dmaheap_cookie,
+ .vaddr = vb2_dmaheap_vaddr,
+ .prepare = vb2_dmaheap_prepare,
+ .finish = vb2_dmaheap_finish,
+ .map_dmabuf = vb2_dmaheap_map_dmabuf,
+ .unmap_dmabuf = vb2_dmaheap_unmap_dmabuf,
+ .attach_dmabuf = vb2_dmaheap_attach_dmabuf,
+ .detach_dmabuf = vb2_dmaheap_detach_dmabuf,
+ .num_users = vb2_dmaheap_num_users,
+ .mmap = vb2_dmaheap_mmap,
+};
+
+MODULE_DESCRIPTION("DMA-Heap memory handling routines for videobuf2");
+MODULE_AUTHOR("Randy Li <[email protected]>");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(DMA_BUF);
diff --git a/include/media/videobuf2-dma-heap.h b/include/media/videobuf2-dma-heap.h
new file mode 100644
index 000000000000..fa057f67d6e9
--- /dev/null
+++ b/include/media/videobuf2-dma-heap.h
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2022 Randy Li <[email protected]>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef _MEDIA_VIDEOBUF2_DMA_HEAP_H
+#define _MEDIA_VIDEOBUF2_DMA_HEAP_H
+
+#include <media/videobuf2-v4l2.h>
+#include <linux/dma-mapping.h>
+
+static inline dma_addr_t
+vb2_dmaheap_plane_dma_addr(struct vb2_buffer *vb, unsigned int plane_no)
+{
+ dma_addr_t *addr = vb2_plane_cookie(vb, plane_no);
+
+ return *addr;
+}
+
+extern const struct vb2_mem_ops vb2_dmaheap_memops;
+#endif
--
2.17.1
?Hi Randy,
On Mon, Aug 1, 2022 at 5:21 AM <[email protected]> wrote:
>
> From: Randy Li <[email protected]>
>
> This module is still at a early stage, I wrote this for showing what
> APIs we need here.
>
> Let me explain why we need such a module here.
> If you won't allocate buffers from a V4L2 M2M device, this module
> may not be very useful. I am sure the most of users won't know a
> device would require them allocate buffers from a DMA-Heap then
> import those buffers into a V4L2's queue.
>
> Then the question goes back to why DMA-Heap. From the Android's
> description, we know it is about the copyright's DRM.
> When we allocate a buffer in a DMA-Heap, it may register that buffer
> in the trusted execution environment so the firmware which is running
> or could only be acccesed from there could use that buffer later.
>
> The answer above leads to another thing which is not done in this
> version, the DMA mapping. Although in some platforms, a DMA-Heap
> responses a IOMMU device as well. For the genernal purpose, we would
> be better assuming the device mapping should be done for each device
> itself. The problem here we only know alloc_devs in those DMAbuf
> methods, which are DMA-heaps in my design, the device from the queue
> is not enough, a plane may requests another IOMMU device or table
> for mapping.
>
> Signed-off-by: Randy Li <[email protected]>
> ---
> drivers/media/common/videobuf2/Kconfig | 6 +
> drivers/media/common/videobuf2/Makefile | 1 +
> .../common/videobuf2/videobuf2-dma-heap.c | 350 ++++++++++++++++++
> include/media/videobuf2-dma-heap.h | 30 ++
> 4 files changed, 387 insertions(+)
> create mode 100644 drivers/media/common/videobuf2/videobuf2-dma-heap.c
> create mode 100644 include/media/videobuf2-dma-heap.h
>
First of all, thanks for the series.
Possibly a stupid question, but why not just allocate the DMA-bufs
directly from the DMA-buf heap device in the userspace and just import
the buffers to the V4L2 device using V4L2_MEMORY_DMABUF?
Best regards,
Tomasz
> diff --git a/drivers/media/common/videobuf2/Kconfig b/drivers/media/common/videobuf2/Kconfig
> index d2223a12c95f..02235077f07e 100644
> --- a/drivers/media/common/videobuf2/Kconfig
> +++ b/drivers/media/common/videobuf2/Kconfig
> @@ -30,3 +30,9 @@ config VIDEOBUF2_DMA_SG
> config VIDEOBUF2_DVB
> tristate
> select VIDEOBUF2_CORE
> +
> +config VIDEOBUF2_DMA_HEAP
> + tristate
> + select VIDEOBUF2_CORE
> + select VIDEOBUF2_MEMOPS
> + select DMABUF_HEAPS
> diff --git a/drivers/media/common/videobuf2/Makefile b/drivers/media/common/videobuf2/Makefile
> index a6fe3f304685..7fe65f93117f 100644
> --- a/drivers/media/common/videobuf2/Makefile
> +++ b/drivers/media/common/videobuf2/Makefile
> @@ -10,6 +10,7 @@ endif
> # (e. g. LC_ALL=C sort Makefile)
> obj-$(CONFIG_VIDEOBUF2_CORE) += videobuf2-common.o
> obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
> +obj-$(CONFIG_VIDEOBUF2_DMA_HEAP) += videobuf2-dma-heap.o
> obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
> obj-$(CONFIG_VIDEOBUF2_DVB) += videobuf2-dvb.o
> obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-heap.c b/drivers/media/common/videobuf2/videobuf2-dma-heap.c
> new file mode 100644
> index 000000000000..377b82ab8f5a
> --- /dev/null
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-heap.c
> @@ -0,0 +1,350 @@
> +/*
> + * Copyright (C) 2022 Randy Li <[email protected]>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/dma-buf.h>
> +#include <linux/dma-heap.h>
> +#include <linux/refcount.h>
> +#include <linux/scatterlist.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/dma-mapping.h>
> +
> +#include <media/videobuf2-v4l2.h>
> +#include <media/videobuf2-memops.h>
> +#include <media/videobuf2-dma-heap.h>
> +
> +struct vb2_dmaheap_buf {
> + struct device *dev;
> + void *vaddr;
> + unsigned long size;
> + struct dma_buf *dmabuf;
> + dma_addr_t dma_addr;
> + unsigned long attrs;
> + enum dma_data_direction dma_dir;
> + struct sg_table *dma_sgt;
> +
> + /* MMAP related */
> + struct vb2_vmarea_handler handler;
> + refcount_t refcount;
> +
> + /* DMABUF related */
> + struct dma_buf_attachment *db_attach;
> +};
> +
> +/*********************************************/
> +/* callbacks for all buffers */
> +/*********************************************/
> +
> +void *vb2_dmaheap_cookie(struct vb2_buffer *vb, void *buf_priv)
> +{
> + struct vb2_dmaheap_buf *buf = buf_priv;
> +
> + return &buf->dma_addr;
> +}
> +
> +static void *vb2_dmaheap_vaddr(struct vb2_buffer *vb, void *buf_priv)
> +{
> + struct vb2_dmaheap_buf *buf = buf_priv;
> + struct iosys_map map;
> +
> + if (buf->vaddr)
> + return buf->vaddr;
> +
> + if (buf->db_attach) {
> + if (!dma_buf_vmap(buf->db_attach->dmabuf, &map))
> + buf->vaddr = map.vaddr;
> + }
> +
> + return buf->vaddr;
> +}
> +
> +static unsigned int vb2_dmaheap_num_users(void *buf_priv)
> +{
> + struct vb2_dmaheap_buf *buf = buf_priv;
> +
> + return refcount_read(&buf->refcount);
> +}
> +
> +static void vb2_dmaheap_prepare(void *buf_priv)
> +{
> + struct vb2_dmaheap_buf *buf = buf_priv;
> +
> + /* TODO: DMABUF exporter will flush the cache for us */
> + if (buf->db_attach)
> + return;
> +
> + dma_buf_end_cpu_access(buf->dmabuf, buf->dma_dir);
> +}
> +
> +static void vb2_dmaheap_finish(void *buf_priv)
> +{
> + struct vb2_dmaheap_buf *buf = buf_priv;
> +
> + /* TODO: DMABUF exporter will flush the cache for us */
> + if (buf->db_attach)
> + return;
> +
> + dma_buf_begin_cpu_access(buf->dmabuf, buf->dma_dir);
> +}
> +
> +/*********************************************/
> +/* callbacks for MMAP buffers */
> +/*********************************************/
> +
> +void vb2_dmaheap_put(void *buf_priv)
> +{
> + struct vb2_dmaheap_buf *buf = buf_priv;
> +
> + if (!refcount_dec_and_test(&buf->refcount))
> + return;
> +
> + dma_buf_put(buf->dmabuf);
> +
> + put_device(buf->dev);
> + kfree(buf);
> +}
> +
> +static void *vb2_dmaheap_alloc(struct vb2_buffer *vb,
> + struct device *dev,
> + unsigned long size)
> +{
> + struct vb2_queue *q = vb->vb2_queue;
> + struct dma_heap *heap;
> + struct vb2_dmaheap_buf *buf;
> + const char *heap_name;
> + int ret;
> +
> + if (WARN_ON(!dev))
> + return ERR_PTR(-EINVAL);
> +
> + heap_name = dev_name(dev);
> + if (!heap_name)
> + return ERR_PTR(-EINVAL);
> +
> + heap = dma_heap_find(heap_name);
> + if (!heap) {
> + dev_err(dev, "is not a DMA-heap device\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + buf = kzalloc(sizeof *buf, GFP_KERNEL);
> + if (!buf)
> + return ERR_PTR(-ENOMEM);
> +
> + /* Prevent the device from being released while the buffer is used */
> + buf->dev = get_device(dev);
> + buf->attrs = vb->vb2_queue->dma_attrs;
> + buf->dma_dir = vb->vb2_queue->dma_dir;
> +
> + /* TODO: heap flags */
> + ret = dma_heap_buffer_alloc(heap, size, 0, 0);
> + if (ret < 0) {
> + dev_err(dev, "is not a DMA-heap device\n");
> + put_device(buf->dev);
> + kfree(buf);
> + return ERR_PTR(ret);
> + }
> + buf->dmabuf = dma_buf_get(ret);
> +
> + /* FIXME */
> + buf->dma_addr = 0;
> +
> + if ((q->dma_attrs & DMA_ATTR_NO_KERNEL_MAPPING) == 0)
> + buf->vaddr = buf->dmabuf;
> +
> + buf->handler.refcount = &buf->refcount;
> + buf->handler.put = vb2_dmaheap_put;
> + buf->handler.arg = buf;
> +
> + refcount_set(&buf->refcount, 1);
> +
> + return buf;
> +}
> +
> +static int vb2_dmaheap_mmap(void *buf_priv, struct vm_area_struct *vma)
> +{
> + struct vb2_dmaheap_buf *buf = buf_priv;
> + int ret;
> +
> + if (!buf) {
> + printk(KERN_ERR "No buffer to map\n");
> + return -EINVAL;
> + }
> +
> + vma->vm_flags &= ~VM_PFNMAP;
> +
> + ret = dma_buf_mmap(buf->dmabuf, vma, 0);
> + if (ret) {
> + pr_err("Remapping memory failed, error: %d\n", ret);
> + return ret;
> + }
> + vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
> + vma->vm_private_data = &buf->handler;
> + vma->vm_ops = &vb2_common_vm_ops;
> +
> + vma->vm_ops->open(vma);
> +
> + pr_debug("%s: mapped memid 0x%08lx at 0x%08lx, size %ld\n",
> + __func__, (unsigned long)buf->dma_addr, vma->vm_start,
> + buf->size);
> +
> + return 0;
> +}
> +
> +/*********************************************/
> +/* DMABUF ops for exporters */
> +/*********************************************/
> +
> +static struct dma_buf *vb2_dmaheap_get_dmabuf(struct vb2_buffer *vb,
> + void *buf_priv,
> + unsigned long flags)
> +{
> + struct vb2_dmaheap_buf *buf = buf_priv;
> + struct dma_buf *dbuf;
> +
> + dbuf = buf->dmabuf;
> +
> + return dbuf;
> +}
> +
> +/*********************************************/
> +/* callbacks for DMABUF buffers */
> +/*********************************************/
> +
> +static int vb2_dmaheap_map_dmabuf(void *mem_priv)
> +{
> + struct vb2_dmaheap_buf *buf = mem_priv;
> + struct sg_table *sgt;
> +
> + if (WARN_ON(!buf->db_attach)) {
> + pr_err("trying to pin a non attached buffer\n");
> + return -EINVAL;
> + }
> +
> + if (WARN_ON(buf->dma_sgt)) {
> + pr_err("dmabuf buffer is already pinned\n");
> + return 0;
> + }
> +
> + /* get the associated scatterlist for this buffer */
> + sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir);
> + if (IS_ERR(sgt)) {
> + pr_err("Error getting dmabuf scatterlist\n");
> + return -EINVAL;
> + }
> +
> + buf->dma_addr = sg_dma_address(sgt->sgl);
> + buf->dma_sgt = sgt;
> + buf->vaddr = NULL;
> +
> + return 0;
> +}
> +
> +static void vb2_dmaheap_unmap_dmabuf(void *mem_priv)
> +{
> + struct vb2_dmaheap_buf *buf = mem_priv;
> + struct sg_table *sgt = buf->dma_sgt;
> + struct iosys_map map = IOSYS_MAP_INIT_VADDR(buf->vaddr);
> +
> + if (WARN_ON(!buf->db_attach)) {
> + pr_err("trying to unpin a not attached buffer\n");
> + return;
> + }
> +
> + if (WARN_ON(!sgt)) {
> + pr_err("dmabuf buffer is already unpinned\n");
> + return;
> + }
> +
> + if (buf->vaddr) {
> + dma_buf_vunmap(buf->db_attach->dmabuf, &map);
> + buf->vaddr = NULL;
> + }
> + dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
> +
> + buf->dma_addr = 0;
> + buf->dma_sgt = NULL;
> +}
> +
> +static void vb2_dmaheap_detach_dmabuf(void *mem_priv)
> +{
> + struct vb2_dmaheap_buf *buf = mem_priv;
> +
> + /* if vb2 works correctly you should never detach mapped buffer */
> + if (WARN_ON(buf->dma_addr))
> + vb2_dmaheap_unmap_dmabuf(buf);
> +
> + /* detach this attachment */
> + dma_buf_detach(buf->db_attach->dmabuf, buf->db_attach);
> + kfree(buf);
> +}
> +
> +static void *vb2_dmaheap_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
> + struct dma_buf *dbuf, unsigned long size)
> +{
> + struct vb2_dmaheap_buf *buf;
> + struct dma_buf_attachment *dba;
> +
> + if (dbuf->size < size)
> + return ERR_PTR(-EFAULT);
> +
> + if (WARN_ON(!dev))
> + return ERR_PTR(-EINVAL);
> + /*
> + * TODO: A better way to check whether the buffer is coming
> + * from this heap or this heap could accept this buffer
> + */
> + if (strcmp(dbuf->exp_name, dev_name(dev)))
> + return ERR_PTR(-EINVAL);
> +
> + buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> + if (!buf)
> + return ERR_PTR(-ENOMEM);
> +
> + buf->dev = dev;
> + /* create attachment for the dmabuf with the user device */
> + dba = dma_buf_attach(dbuf, buf->dev);
> + if (IS_ERR(dba)) {
> + pr_err("failed to attach dmabuf\n");
> + kfree(buf);
> + return dba;
> + }
> +
> + buf->dma_dir = vb->vb2_queue->dma_dir;
> + buf->size = size;
> + buf->db_attach = dba;
> +
> + return buf;
> +}
> +
> +const struct vb2_mem_ops vb2_dmaheap_memops = {
> + .alloc = vb2_dmaheap_alloc,
> + .put = vb2_dmaheap_put,
> + .get_dmabuf = vb2_dmaheap_get_dmabuf,
> + .cookie = vb2_dmaheap_cookie,
> + .vaddr = vb2_dmaheap_vaddr,
> + .prepare = vb2_dmaheap_prepare,
> + .finish = vb2_dmaheap_finish,
> + .map_dmabuf = vb2_dmaheap_map_dmabuf,
> + .unmap_dmabuf = vb2_dmaheap_unmap_dmabuf,
> + .attach_dmabuf = vb2_dmaheap_attach_dmabuf,
> + .detach_dmabuf = vb2_dmaheap_detach_dmabuf,
> + .num_users = vb2_dmaheap_num_users,
> + .mmap = vb2_dmaheap_mmap,
> +};
> +
> +MODULE_DESCRIPTION("DMA-Heap memory handling routines for videobuf2");
> +MODULE_AUTHOR("Randy Li <[email protected]>");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(DMA_BUF);
> diff --git a/include/media/videobuf2-dma-heap.h b/include/media/videobuf2-dma-heap.h
> new file mode 100644
> index 000000000000..fa057f67d6e9
> --- /dev/null
> +++ b/include/media/videobuf2-dma-heap.h
> @@ -0,0 +1,30 @@
> +/*
> + * Copyright (C) 2022 Randy Li <[email protected]>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#ifndef _MEDIA_VIDEOBUF2_DMA_HEAP_H
> +#define _MEDIA_VIDEOBUF2_DMA_HEAP_H
> +
> +#include <media/videobuf2-v4l2.h>
> +#include <linux/dma-mapping.h>
> +
> +static inline dma_addr_t
> +vb2_dmaheap_plane_dma_addr(struct vb2_buffer *vb, unsigned int plane_no)
> +{
> + dma_addr_t *addr = vb2_plane_cookie(vb, plane_no);
> +
> + return *addr;
> +}
> +
> +extern const struct vb2_mem_ops vb2_dmaheap_memops;
> +#endif
> --
> 2.17.1
>
On 8/1/22 14:19, Tomasz Figa wrote:
>
Hello Tomasz
>
> ?Hi Randy,
>
> On Mon, Aug 1, 2022 at 5:21 AM <[email protected]> wrote:
>>
>> From: Randy Li <[email protected]>
>>
>> This module is still at a early stage, I wrote this for showing what
>> APIs we need here.
>>
>> Let me explain why we need such a module here.
>> If you won't allocate buffers from a V4L2 M2M device, this module
>> may not be very useful. I am sure the most of users won't know a
>> device would require them allocate buffers from a DMA-Heap then
>> import those buffers into a V4L2's queue.
>>
>> Then the question goes back to why DMA-Heap. From the Android's
>> description, we know it is about the copyright's DRM.
>> When we allocate a buffer in a DMA-Heap, it may register that buffer
>> in the trusted execution environment so the firmware which is running
>> or could only be acccesed from there could use that buffer later.
>>
>> The answer above leads to another thing which is not done in this
>> version, the DMA mapping. Although in some platforms, a DMA-Heap
>> responses a IOMMU device as well. For the genernal purpose, we would
>> be better assuming the device mapping should be done for each device
>> itself. The problem here we only know alloc_devs in those DMAbuf
>> methods, which are DMA-heaps in my design, the device from the queue
>> is not enough, a plane may requests another IOMMU device or table
>> for mapping.
>>
>> Signed-off-by: Randy Li <[email protected]>
>> ---
>> drivers/media/common/videobuf2/Kconfig | 6 +
>> drivers/media/common/videobuf2/Makefile | 1 +
>> .../common/videobuf2/videobuf2-dma-heap.c | 350 ++++++++++++++++++
>> include/media/videobuf2-dma-heap.h | 30 ++
>> 4 files changed, 387 insertions(+)
>> create mode 100644 drivers/media/common/videobuf2/videobuf2-dma-heap.c
>> create mode 100644 include/media/videobuf2-dma-heap.h
>>
>
> First of all, thanks for the series.
>
> Possibly a stupid question, but why not just allocate the DMA-bufs
> directly from the DMA-buf heap device in the userspace and just import
> the buffers to the V4L2 device using V4L2_MEMORY_DMABUF?
Sometimes the allocation policy could be very complex, let's suppose a
multiple planes pixel format enabling with frame buffer compression.
Its luma, chroma data could be allocated from a pool which is delegated
for large buffers while its metadata would come from a pool which many
users could take some few slices from it(likes system pool).
Then when we have a new users knowing nothing about this platform, if we
just configure the alloc_devs in each queues well. The user won't need
to know those complex rules.
The real situation could be more complex, Samsung MFC's left and right
banks could be regarded as two pools, many devices would benefit from
this either from the allocation times or the security buffers policy.
In our design, when we need to do some security decoding(DRM video),
codecs2 would allocate buffers from the pool delegated for that. While
the non-DRM video, users could not care about this.
>
> Best regards,
> Tomasz
>
>> diff --git a/drivers/media/common/videobuf2/Kconfig b/drivers/media/common/videobuf2/Kconfig
>> index d2223a12c95f..02235077f07e 100644
>> --- a/drivers/media/common/videobuf2/Kconfig
>> +++ b/drivers/media/common/videobuf2/Kconfig
>> @@ -30,3 +30,9 @@ config VIDEOBUF2_DMA_SG
>> config VIDEOBUF2_DVB
>> tristate
>> select VIDEOBUF2_CORE
>> +
>> +config VIDEOBUF2_DMA_HEAP
>> + tristate
>> + select VIDEOBUF2_CORE
>> + select VIDEOBUF2_MEMOPS
>> + select DMABUF_HEAPS
>> diff --git a/drivers/media/common/videobuf2/Makefile b/drivers/media/common/videobuf2/Makefile
>> index a6fe3f304685..7fe65f93117f 100644
>> --- a/drivers/media/common/videobuf2/Makefile
>> +++ b/drivers/media/common/videobuf2/Makefile
>> @@ -10,6 +10,7 @@ endif
>> # (e. g. LC_ALL=C sort Makefile)
>> obj-$(CONFIG_VIDEOBUF2_CORE) += videobuf2-common.o
>> obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
>> +obj-$(CONFIG_VIDEOBUF2_DMA_HEAP) += videobuf2-dma-heap.o
>> obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
>> obj-$(CONFIG_VIDEOBUF2_DVB) += videobuf2-dvb.o
>> obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
>> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-heap.c b/drivers/media/common/videobuf2/videobuf2-dma-heap.c
>> new file mode 100644
>> index 000000000000..377b82ab8f5a
>> --- /dev/null
>> +++ b/drivers/media/common/videobuf2/videobuf2-dma-heap.c
>> @@ -0,0 +1,350 @@
>> +/*
>> + * Copyright (C) 2022 Randy Li <[email protected]>
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/dma-buf.h>
>> +#include <linux/dma-heap.h>
>> +#include <linux/refcount.h>
>> +#include <linux/scatterlist.h>
>> +#include <linux/sched.h>
>> +#include <linux/slab.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#include <media/videobuf2-v4l2.h>
>> +#include <media/videobuf2-memops.h>
>> +#include <media/videobuf2-dma-heap.h>
>> +
>> +struct vb2_dmaheap_buf {
>> + struct device *dev;
>> + void *vaddr;
>> + unsigned long size;
>> + struct dma_buf *dmabuf;
>> + dma_addr_t dma_addr;
>> + unsigned long attrs;
>> + enum dma_data_direction dma_dir;
>> + struct sg_table *dma_sgt;
>> +
>> + /* MMAP related */
>> + struct vb2_vmarea_handler handler;
>> + refcount_t refcount;
>> +
>> + /* DMABUF related */
>> + struct dma_buf_attachment *db_attach;
>> +};
>> +
>> +/*********************************************/
>> +/* callbacks for all buffers */
>> +/*********************************************/
>> +
>> +void *vb2_dmaheap_cookie(struct vb2_buffer *vb, void *buf_priv)
>> +{
>> + struct vb2_dmaheap_buf *buf = buf_priv;
>> +
>> + return &buf->dma_addr;
>> +}
>> +
>> +static void *vb2_dmaheap_vaddr(struct vb2_buffer *vb, void *buf_priv)
>> +{
>> + struct vb2_dmaheap_buf *buf = buf_priv;
>> + struct iosys_map map;
>> +
>> + if (buf->vaddr)
>> + return buf->vaddr;
>> +
>> + if (buf->db_attach) {
>> + if (!dma_buf_vmap(buf->db_attach->dmabuf, &map))
>> + buf->vaddr = map.vaddr;
>> + }
>> +
>> + return buf->vaddr;
>> +}
>> +
>> +static unsigned int vb2_dmaheap_num_users(void *buf_priv)
>> +{
>> + struct vb2_dmaheap_buf *buf = buf_priv;
>> +
>> + return refcount_read(&buf->refcount);
>> +}
>> +
>> +static void vb2_dmaheap_prepare(void *buf_priv)
>> +{
>> + struct vb2_dmaheap_buf *buf = buf_priv;
>> +
>> + /* TODO: DMABUF exporter will flush the cache for us */
>> + if (buf->db_attach)
>> + return;
>> +
>> + dma_buf_end_cpu_access(buf->dmabuf, buf->dma_dir);
>> +}
>> +
>> +static void vb2_dmaheap_finish(void *buf_priv)
>> +{
>> + struct vb2_dmaheap_buf *buf = buf_priv;
>> +
>> + /* TODO: DMABUF exporter will flush the cache for us */
>> + if (buf->db_attach)
>> + return;
>> +
>> + dma_buf_begin_cpu_access(buf->dmabuf, buf->dma_dir);
>> +}
>> +
>> +/*********************************************/
>> +/* callbacks for MMAP buffers */
>> +/*********************************************/
>> +
>> +void vb2_dmaheap_put(void *buf_priv)
>> +{
>> + struct vb2_dmaheap_buf *buf = buf_priv;
>> +
>> + if (!refcount_dec_and_test(&buf->refcount))
>> + return;
>> +
>> + dma_buf_put(buf->dmabuf);
>> +
>> + put_device(buf->dev);
>> + kfree(buf);
>> +}
>> +
>> +static void *vb2_dmaheap_alloc(struct vb2_buffer *vb,
>> + struct device *dev,
>> + unsigned long size)
>> +{
>> + struct vb2_queue *q = vb->vb2_queue;
>> + struct dma_heap *heap;
>> + struct vb2_dmaheap_buf *buf;
>> + const char *heap_name;
>> + int ret;
>> +
>> + if (WARN_ON(!dev))
>> + return ERR_PTR(-EINVAL);
>> +
>> + heap_name = dev_name(dev);
>> + if (!heap_name)
>> + return ERR_PTR(-EINVAL);
>> +
>> + heap = dma_heap_find(heap_name);
>> + if (!heap) {
>> + dev_err(dev, "is not a DMA-heap device\n");
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + buf = kzalloc(sizeof *buf, GFP_KERNEL);
>> + if (!buf)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + /* Prevent the device from being released while the buffer is used */
>> + buf->dev = get_device(dev);
>> + buf->attrs = vb->vb2_queue->dma_attrs;
>> + buf->dma_dir = vb->vb2_queue->dma_dir;
>> +
>> + /* TODO: heap flags */
>> + ret = dma_heap_buffer_alloc(heap, size, 0, 0);
>> + if (ret < 0) {
>> + dev_err(dev, "is not a DMA-heap device\n");
>> + put_device(buf->dev);
>> + kfree(buf);
>> + return ERR_PTR(ret);
>> + }
>> + buf->dmabuf = dma_buf_get(ret);
>> +
>> + /* FIXME */
>> + buf->dma_addr = 0;
>> +
>> + if ((q->dma_attrs & DMA_ATTR_NO_KERNEL_MAPPING) == 0)
>> + buf->vaddr = buf->dmabuf;
>> +
>> + buf->handler.refcount = &buf->refcount;
>> + buf->handler.put = vb2_dmaheap_put;
>> + buf->handler.arg = buf;
>> +
>> + refcount_set(&buf->refcount, 1);
>> +
>> + return buf;
>> +}
>> +
>> +static int vb2_dmaheap_mmap(void *buf_priv, struct vm_area_struct *vma)
>> +{
>> + struct vb2_dmaheap_buf *buf = buf_priv;
>> + int ret;
>> +
>> + if (!buf) {
>> + printk(KERN_ERR "No buffer to map\n");
>> + return -EINVAL;
>> + }
>> +
>> + vma->vm_flags &= ~VM_PFNMAP;
>> +
>> + ret = dma_buf_mmap(buf->dmabuf, vma, 0);
>> + if (ret) {
>> + pr_err("Remapping memory failed, error: %d\n", ret);
>> + return ret;
>> + }
>> + vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
>> + vma->vm_private_data = &buf->handler;
>> + vma->vm_ops = &vb2_common_vm_ops;
>> +
>> + vma->vm_ops->open(vma);
>> +
>> + pr_debug("%s: mapped memid 0x%08lx at 0x%08lx, size %ld\n",
>> + __func__, (unsigned long)buf->dma_addr, vma->vm_start,
>> + buf->size);
>> +
>> + return 0;
>> +}
>> +
>> +/*********************************************/
>> +/* DMABUF ops for exporters */
>> +/*********************************************/
>> +
>> +static struct dma_buf *vb2_dmaheap_get_dmabuf(struct vb2_buffer *vb,
>> + void *buf_priv,
>> + unsigned long flags)
>> +{
>> + struct vb2_dmaheap_buf *buf = buf_priv;
>> + struct dma_buf *dbuf;
>> +
>> + dbuf = buf->dmabuf;
>> +
>> + return dbuf;
>> +}
>> +
>> +/*********************************************/
>> +/* callbacks for DMABUF buffers */
>> +/*********************************************/
>> +
>> +static int vb2_dmaheap_map_dmabuf(void *mem_priv)
>> +{
>> + struct vb2_dmaheap_buf *buf = mem_priv;
>> + struct sg_table *sgt;
>> +
>> + if (WARN_ON(!buf->db_attach)) {
>> + pr_err("trying to pin a non attached buffer\n");
>> + return -EINVAL;
>> + }
>> +
>> + if (WARN_ON(buf->dma_sgt)) {
>> + pr_err("dmabuf buffer is already pinned\n");
>> + return 0;
>> + }
>> +
>> + /* get the associated scatterlist for this buffer */
>> + sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir);
>> + if (IS_ERR(sgt)) {
>> + pr_err("Error getting dmabuf scatterlist\n");
>> + return -EINVAL;
>> + }
>> +
>> + buf->dma_addr = sg_dma_address(sgt->sgl);
>> + buf->dma_sgt = sgt;
>> + buf->vaddr = NULL;
>> +
>> + return 0;
>> +}
>> +
>> +static void vb2_dmaheap_unmap_dmabuf(void *mem_priv)
>> +{
>> + struct vb2_dmaheap_buf *buf = mem_priv;
>> + struct sg_table *sgt = buf->dma_sgt;
>> + struct iosys_map map = IOSYS_MAP_INIT_VADDR(buf->vaddr);
>> +
>> + if (WARN_ON(!buf->db_attach)) {
>> + pr_err("trying to unpin a not attached buffer\n");
>> + return;
>> + }
>> +
>> + if (WARN_ON(!sgt)) {
>> + pr_err("dmabuf buffer is already unpinned\n");
>> + return;
>> + }
>> +
>> + if (buf->vaddr) {
>> + dma_buf_vunmap(buf->db_attach->dmabuf, &map);
>> + buf->vaddr = NULL;
>> + }
>> + dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
>> +
>> + buf->dma_addr = 0;
>> + buf->dma_sgt = NULL;
>> +}
>> +
>> +static void vb2_dmaheap_detach_dmabuf(void *mem_priv)
>> +{
>> + struct vb2_dmaheap_buf *buf = mem_priv;
>> +
>> + /* if vb2 works correctly you should never detach mapped buffer */
>> + if (WARN_ON(buf->dma_addr))
>> + vb2_dmaheap_unmap_dmabuf(buf);
>> +
>> + /* detach this attachment */
>> + dma_buf_detach(buf->db_attach->dmabuf, buf->db_attach);
>> + kfree(buf);
>> +}
>> +
>> +static void *vb2_dmaheap_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
>> + struct dma_buf *dbuf, unsigned long size)
>> +{
>> + struct vb2_dmaheap_buf *buf;
>> + struct dma_buf_attachment *dba;
>> +
>> + if (dbuf->size < size)
>> + return ERR_PTR(-EFAULT);
>> +
>> + if (WARN_ON(!dev))
>> + return ERR_PTR(-EINVAL);
>> + /*
>> + * TODO: A better way to check whether the buffer is coming
>> + * from this heap or this heap could accept this buffer
>> + */
>> + if (strcmp(dbuf->exp_name, dev_name(dev)))
>> + return ERR_PTR(-EINVAL);
>> +
>> + buf = kzalloc(sizeof(*buf), GFP_KERNEL);
>> + if (!buf)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + buf->dev = dev;
>> + /* create attachment for the dmabuf with the user device */
>> + dba = dma_buf_attach(dbuf, buf->dev);
>> + if (IS_ERR(dba)) {
>> + pr_err("failed to attach dmabuf\n");
>> + kfree(buf);
>> + return dba;
>> + }
>> +
>> + buf->dma_dir = vb->vb2_queue->dma_dir;
>> + buf->size = size;
>> + buf->db_attach = dba;
>> +
>> + return buf;
>> +}
>> +
>> +const struct vb2_mem_ops vb2_dmaheap_memops = {
>> + .alloc = vb2_dmaheap_alloc,
>> + .put = vb2_dmaheap_put,
>> + .get_dmabuf = vb2_dmaheap_get_dmabuf,
>> + .cookie = vb2_dmaheap_cookie,
>> + .vaddr = vb2_dmaheap_vaddr,
>> + .prepare = vb2_dmaheap_prepare,
>> + .finish = vb2_dmaheap_finish,
>> + .map_dmabuf = vb2_dmaheap_map_dmabuf,
>> + .unmap_dmabuf = vb2_dmaheap_unmap_dmabuf,
>> + .attach_dmabuf = vb2_dmaheap_attach_dmabuf,
>> + .detach_dmabuf = vb2_dmaheap_detach_dmabuf,
>> + .num_users = vb2_dmaheap_num_users,
>> + .mmap = vb2_dmaheap_mmap,
>> +};
>> +
>> +MODULE_DESCRIPTION("DMA-Heap memory handling routines for videobuf2");
>> +MODULE_AUTHOR("Randy Li <[email protected]>");
>> +MODULE_LICENSE("GPL");
>> +MODULE_IMPORT_NS(DMA_BUF);
>> diff --git a/include/media/videobuf2-dma-heap.h b/include/media/videobuf2-dma-heap.h
>> new file mode 100644
>> index 000000000000..fa057f67d6e9
>> --- /dev/null
>> +++ b/include/media/videobuf2-dma-heap.h
>> @@ -0,0 +1,30 @@
>> +/*
>> + * Copyright (C) 2022 Randy Li <[email protected]>
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#ifndef _MEDIA_VIDEOBUF2_DMA_HEAP_H
>> +#define _MEDIA_VIDEOBUF2_DMA_HEAP_H
>> +
>> +#include <media/videobuf2-v4l2.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +static inline dma_addr_t
>> +vb2_dmaheap_plane_dma_addr(struct vb2_buffer *vb, unsigned int plane_no)
>> +{
>> + dma_addr_t *addr = vb2_plane_cookie(vb, plane_no);
>> +
>> + return *addr;
>> +}
>> +
>> +extern const struct vb2_mem_ops vb2_dmaheap_memops;
>> +#endif
>> --
>> 2.17.1
>>
--
Hsia-Jun(Randy) Li
On Mon, Aug 1, 2022 at 3:44 PM Hsia-Jun Li <[email protected]> wrote:
>
>
>
> On 8/1/22 14:19, Tomasz Figa wrote:
> >
> Hello Tomasz
> >
> > ?Hi Randy,
> >
> > On Mon, Aug 1, 2022 at 5:21 AM <[email protected]> wrote:
> >>
> >> From: Randy Li <[email protected]>
> >>
> >> This module is still at a early stage, I wrote this for showing what
> >> APIs we need here.
> >>
> >> Let me explain why we need such a module here.
> >> If you won't allocate buffers from a V4L2 M2M device, this module
> >> may not be very useful. I am sure the most of users won't know a
> >> device would require them allocate buffers from a DMA-Heap then
> >> import those buffers into a V4L2's queue.
> >>
> >> Then the question goes back to why DMA-Heap. From the Android's
> >> description, we know it is about the copyright's DRM.
> >> When we allocate a buffer in a DMA-Heap, it may register that buffer
> >> in the trusted execution environment so the firmware which is running
> >> or could only be acccesed from there could use that buffer later.
> >>
> >> The answer above leads to another thing which is not done in this
> >> version, the DMA mapping. Although in some platforms, a DMA-Heap
> >> responses a IOMMU device as well. For the genernal purpose, we would
> >> be better assuming the device mapping should be done for each device
> >> itself. The problem here we only know alloc_devs in those DMAbuf
> >> methods, which are DMA-heaps in my design, the device from the queue
> >> is not enough, a plane may requests another IOMMU device or table
> >> for mapping.
> >>
> >> Signed-off-by: Randy Li <[email protected]>
> >> ---
> >> drivers/media/common/videobuf2/Kconfig | 6 +
> >> drivers/media/common/videobuf2/Makefile | 1 +
> >> .../common/videobuf2/videobuf2-dma-heap.c | 350 ++++++++++++++++++
> >> include/media/videobuf2-dma-heap.h | 30 ++
> >> 4 files changed, 387 insertions(+)
> >> create mode 100644 drivers/media/common/videobuf2/videobuf2-dma-heap.c
> >> create mode 100644 include/media/videobuf2-dma-heap.h
> >>
> >
> > First of all, thanks for the series.
> >
> > Possibly a stupid question, but why not just allocate the DMA-bufs
> > directly from the DMA-buf heap device in the userspace and just import
> > the buffers to the V4L2 device using V4L2_MEMORY_DMABUF?
> Sometimes the allocation policy could be very complex, let's suppose a
> multiple planes pixel format enabling with frame buffer compression.
> Its luma, chroma data could be allocated from a pool which is delegated
> for large buffers while its metadata would come from a pool which many
> users could take some few slices from it(likes system pool).
>
> Then when we have a new users knowing nothing about this platform, if we
> just configure the alloc_devs in each queues well. The user won't need
> to know those complex rules.
>
> The real situation could be more complex, Samsung MFC's left and right
> banks could be regarded as two pools, many devices would benefit from
> this either from the allocation times or the security buffers policy.
>
> In our design, when we need to do some security decoding(DRM video),
> codecs2 would allocate buffers from the pool delegated for that. While
> the non-DRM video, users could not care about this.
I'm a little bit surprised about this, because on Android all the
graphics buffers are allocated from the system IAllocator and imported
to the specific devices.
Would it make sense to instead extend the UAPI to expose enough
information about the allocation requirements to the userspace, so it
can allocate correctly?
My reasoning here is that it's not a driver's decision to allocate
from a DMA-buf heap (and which one) or not. It's the userspace which
knows that, based on the specific use case that it wants to fulfill.
Also, FWIW, dma_heap_ioctl_allocate() is a static function not exposed
to other kernel modules:
https://elixir.bootlin.com/linux/v5.19/source/drivers/dma-buf/dma-heap.c#L52
By the way, the MFC left/right port requirement was gone long ago, it
was only one of the earliest Exynos SoCs which required that.
Best regards,
Tomasz
> >
> > Best regards,
> > Tomasz
> >
> >> diff --git a/drivers/media/common/videobuf2/Kconfig b/drivers/media/common/videobuf2/Kconfig
> >> index d2223a12c95f..02235077f07e 100644
> >> --- a/drivers/media/common/videobuf2/Kconfig
> >> +++ b/drivers/media/common/videobuf2/Kconfig
> >> @@ -30,3 +30,9 @@ config VIDEOBUF2_DMA_SG
> >> config VIDEOBUF2_DVB
> >> tristate
> >> select VIDEOBUF2_CORE
> >> +
> >> +config VIDEOBUF2_DMA_HEAP
> >> + tristate
> >> + select VIDEOBUF2_CORE
> >> + select VIDEOBUF2_MEMOPS
> >> + select DMABUF_HEAPS
> >> diff --git a/drivers/media/common/videobuf2/Makefile b/drivers/media/common/videobuf2/Makefile
> >> index a6fe3f304685..7fe65f93117f 100644
> >> --- a/drivers/media/common/videobuf2/Makefile
> >> +++ b/drivers/media/common/videobuf2/Makefile
> >> @@ -10,6 +10,7 @@ endif
> >> # (e. g. LC_ALL=C sort Makefile)
> >> obj-$(CONFIG_VIDEOBUF2_CORE) += videobuf2-common.o
> >> obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
> >> +obj-$(CONFIG_VIDEOBUF2_DMA_HEAP) += videobuf2-dma-heap.o
> >> obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
> >> obj-$(CONFIG_VIDEOBUF2_DVB) += videobuf2-dvb.o
> >> obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
> >> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-heap.c b/drivers/media/common/videobuf2/videobuf2-dma-heap.c
> >> new file mode 100644
> >> index 000000000000..377b82ab8f5a
> >> --- /dev/null
> >> +++ b/drivers/media/common/videobuf2/videobuf2-dma-heap.c
> >> @@ -0,0 +1,350 @@
> >> +/*
> >> + * Copyright (C) 2022 Randy Li <[email protected]>
> >> + *
> >> + * This software is licensed under the terms of the GNU General Public
> >> + * License version 2, as published by the Free Software Foundation, and
> >> + * may be copied, distributed, and modified under those terms.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> + * GNU General Public License for more details.
> >> + *
> >> + */
> >> +
> >> +#include <linux/dma-buf.h>
> >> +#include <linux/dma-heap.h>
> >> +#include <linux/refcount.h>
> >> +#include <linux/scatterlist.h>
> >> +#include <linux/sched.h>
> >> +#include <linux/slab.h>
> >> +#include <linux/dma-mapping.h>
> >> +
> >> +#include <media/videobuf2-v4l2.h>
> >> +#include <media/videobuf2-memops.h>
> >> +#include <media/videobuf2-dma-heap.h>
> >> +
> >> +struct vb2_dmaheap_buf {
> >> + struct device *dev;
> >> + void *vaddr;
> >> + unsigned long size;
> >> + struct dma_buf *dmabuf;
> >> + dma_addr_t dma_addr;
> >> + unsigned long attrs;
> >> + enum dma_data_direction dma_dir;
> >> + struct sg_table *dma_sgt;
> >> +
> >> + /* MMAP related */
> >> + struct vb2_vmarea_handler handler;
> >> + refcount_t refcount;
> >> +
> >> + /* DMABUF related */
> >> + struct dma_buf_attachment *db_attach;
> >> +};
> >> +
> >> +/*********************************************/
> >> +/* callbacks for all buffers */
> >> +/*********************************************/
> >> +
> >> +void *vb2_dmaheap_cookie(struct vb2_buffer *vb, void *buf_priv)
> >> +{
> >> + struct vb2_dmaheap_buf *buf = buf_priv;
> >> +
> >> + return &buf->dma_addr;
> >> +}
> >> +
> >> +static void *vb2_dmaheap_vaddr(struct vb2_buffer *vb, void *buf_priv)
> >> +{
> >> + struct vb2_dmaheap_buf *buf = buf_priv;
> >> + struct iosys_map map;
> >> +
> >> + if (buf->vaddr)
> >> + return buf->vaddr;
> >> +
> >> + if (buf->db_attach) {
> >> + if (!dma_buf_vmap(buf->db_attach->dmabuf, &map))
> >> + buf->vaddr = map.vaddr;
> >> + }
> >> +
> >> + return buf->vaddr;
> >> +}
> >> +
> >> +static unsigned int vb2_dmaheap_num_users(void *buf_priv)
> >> +{
> >> + struct vb2_dmaheap_buf *buf = buf_priv;
> >> +
> >> + return refcount_read(&buf->refcount);
> >> +}
> >> +
> >> +static void vb2_dmaheap_prepare(void *buf_priv)
> >> +{
> >> + struct vb2_dmaheap_buf *buf = buf_priv;
> >> +
> >> + /* TODO: DMABUF exporter will flush the cache for us */
> >> + if (buf->db_attach)
> >> + return;
> >> +
> >> + dma_buf_end_cpu_access(buf->dmabuf, buf->dma_dir);
> >> +}
> >> +
> >> +static void vb2_dmaheap_finish(void *buf_priv)
> >> +{
> >> + struct vb2_dmaheap_buf *buf = buf_priv;
> >> +
> >> + /* TODO: DMABUF exporter will flush the cache for us */
> >> + if (buf->db_attach)
> >> + return;
> >> +
> >> + dma_buf_begin_cpu_access(buf->dmabuf, buf->dma_dir);
> >> +}
> >> +
> >> +/*********************************************/
> >> +/* callbacks for MMAP buffers */
> >> +/*********************************************/
> >> +
> >> +void vb2_dmaheap_put(void *buf_priv)
> >> +{
> >> + struct vb2_dmaheap_buf *buf = buf_priv;
> >> +
> >> + if (!refcount_dec_and_test(&buf->refcount))
> >> + return;
> >> +
> >> + dma_buf_put(buf->dmabuf);
> >> +
> >> + put_device(buf->dev);
> >> + kfree(buf);
> >> +}
> >> +
> >> +static void *vb2_dmaheap_alloc(struct vb2_buffer *vb,
> >> + struct device *dev,
> >> + unsigned long size)
> >> +{
> >> + struct vb2_queue *q = vb->vb2_queue;
> >> + struct dma_heap *heap;
> >> + struct vb2_dmaheap_buf *buf;
> >> + const char *heap_name;
> >> + int ret;
> >> +
> >> + if (WARN_ON(!dev))
> >> + return ERR_PTR(-EINVAL);
> >> +
> >> + heap_name = dev_name(dev);
> >> + if (!heap_name)
> >> + return ERR_PTR(-EINVAL);
> >> +
> >> + heap = dma_heap_find(heap_name);
> >> + if (!heap) {
> >> + dev_err(dev, "is not a DMA-heap device\n");
> >> + return ERR_PTR(-EINVAL);
> >> + }
> >> +
> >> + buf = kzalloc(sizeof *buf, GFP_KERNEL);
> >> + if (!buf)
> >> + return ERR_PTR(-ENOMEM);
> >> +
> >> + /* Prevent the device from being released while the buffer is used */
> >> + buf->dev = get_device(dev);
> >> + buf->attrs = vb->vb2_queue->dma_attrs;
> >> + buf->dma_dir = vb->vb2_queue->dma_dir;
> >> +
> >> + /* TODO: heap flags */
> >> + ret = dma_heap_buffer_alloc(heap, size, 0, 0);
> >> + if (ret < 0) {
> >> + dev_err(dev, "is not a DMA-heap device\n");
> >> + put_device(buf->dev);
> >> + kfree(buf);
> >> + return ERR_PTR(ret);
> >> + }
> >> + buf->dmabuf = dma_buf_get(ret);
> >> +
> >> + /* FIXME */
> >> + buf->dma_addr = 0;
> >> +
> >> + if ((q->dma_attrs & DMA_ATTR_NO_KERNEL_MAPPING) == 0)
> >> + buf->vaddr = buf->dmabuf;
> >> +
> >> + buf->handler.refcount = &buf->refcount;
> >> + buf->handler.put = vb2_dmaheap_put;
> >> + buf->handler.arg = buf;
> >> +
> >> + refcount_set(&buf->refcount, 1);
> >> +
> >> + return buf;
> >> +}
> >> +
> >> +static int vb2_dmaheap_mmap(void *buf_priv, struct vm_area_struct *vma)
> >> +{
> >> + struct vb2_dmaheap_buf *buf = buf_priv;
> >> + int ret;
> >> +
> >> + if (!buf) {
> >> + printk(KERN_ERR "No buffer to map\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + vma->vm_flags &= ~VM_PFNMAP;
> >> +
> >> + ret = dma_buf_mmap(buf->dmabuf, vma, 0);
> >> + if (ret) {
> >> + pr_err("Remapping memory failed, error: %d\n", ret);
> >> + return ret;
> >> + }
> >> + vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
> >> + vma->vm_private_data = &buf->handler;
> >> + vma->vm_ops = &vb2_common_vm_ops;
> >> +
> >> + vma->vm_ops->open(vma);
> >> +
> >> + pr_debug("%s: mapped memid 0x%08lx at 0x%08lx, size %ld\n",
> >> + __func__, (unsigned long)buf->dma_addr, vma->vm_start,
> >> + buf->size);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/*********************************************/
> >> +/* DMABUF ops for exporters */
> >> +/*********************************************/
> >> +
> >> +static struct dma_buf *vb2_dmaheap_get_dmabuf(struct vb2_buffer *vb,
> >> + void *buf_priv,
> >> + unsigned long flags)
> >> +{
> >> + struct vb2_dmaheap_buf *buf = buf_priv;
> >> + struct dma_buf *dbuf;
> >> +
> >> + dbuf = buf->dmabuf;
> >> +
> >> + return dbuf;
> >> +}
> >> +
> >> +/*********************************************/
> >> +/* callbacks for DMABUF buffers */
> >> +/*********************************************/
> >> +
> >> +static int vb2_dmaheap_map_dmabuf(void *mem_priv)
> >> +{
> >> + struct vb2_dmaheap_buf *buf = mem_priv;
> >> + struct sg_table *sgt;
> >> +
> >> + if (WARN_ON(!buf->db_attach)) {
> >> + pr_err("trying to pin a non attached buffer\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + if (WARN_ON(buf->dma_sgt)) {
> >> + pr_err("dmabuf buffer is already pinned\n");
> >> + return 0;
> >> + }
> >> +
> >> + /* get the associated scatterlist for this buffer */
> >> + sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir);
> >> + if (IS_ERR(sgt)) {
> >> + pr_err("Error getting dmabuf scatterlist\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + buf->dma_addr = sg_dma_address(sgt->sgl);
> >> + buf->dma_sgt = sgt;
> >> + buf->vaddr = NULL;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static void vb2_dmaheap_unmap_dmabuf(void *mem_priv)
> >> +{
> >> + struct vb2_dmaheap_buf *buf = mem_priv;
> >> + struct sg_table *sgt = buf->dma_sgt;
> >> + struct iosys_map map = IOSYS_MAP_INIT_VADDR(buf->vaddr);
> >> +
> >> + if (WARN_ON(!buf->db_attach)) {
> >> + pr_err("trying to unpin a not attached buffer\n");
> >> + return;
> >> + }
> >> +
> >> + if (WARN_ON(!sgt)) {
> >> + pr_err("dmabuf buffer is already unpinned\n");
> >> + return;
> >> + }
> >> +
> >> + if (buf->vaddr) {
> >> + dma_buf_vunmap(buf->db_attach->dmabuf, &map);
> >> + buf->vaddr = NULL;
> >> + }
> >> + dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
> >> +
> >> + buf->dma_addr = 0;
> >> + buf->dma_sgt = NULL;
> >> +}
> >> +
> >> +static void vb2_dmaheap_detach_dmabuf(void *mem_priv)
> >> +{
> >> + struct vb2_dmaheap_buf *buf = mem_priv;
> >> +
> >> + /* if vb2 works correctly you should never detach mapped buffer */
> >> + if (WARN_ON(buf->dma_addr))
> >> + vb2_dmaheap_unmap_dmabuf(buf);
> >> +
> >> + /* detach this attachment */
> >> + dma_buf_detach(buf->db_attach->dmabuf, buf->db_attach);
> >> + kfree(buf);
> >> +}
> >> +
> >> +static void *vb2_dmaheap_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
> >> + struct dma_buf *dbuf, unsigned long size)
> >> +{
> >> + struct vb2_dmaheap_buf *buf;
> >> + struct dma_buf_attachment *dba;
> >> +
> >> + if (dbuf->size < size)
> >> + return ERR_PTR(-EFAULT);
> >> +
> >> + if (WARN_ON(!dev))
> >> + return ERR_PTR(-EINVAL);
> >> + /*
> >> + * TODO: A better way to check whether the buffer is coming
> >> + * from this heap or this heap could accept this buffer
> >> + */
> >> + if (strcmp(dbuf->exp_name, dev_name(dev)))
> >> + return ERR_PTR(-EINVAL);
> >> +
> >> + buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> >> + if (!buf)
> >> + return ERR_PTR(-ENOMEM);
> >> +
> >> + buf->dev = dev;
> >> + /* create attachment for the dmabuf with the user device */
> >> + dba = dma_buf_attach(dbuf, buf->dev);
> >> + if (IS_ERR(dba)) {
> >> + pr_err("failed to attach dmabuf\n");
> >> + kfree(buf);
> >> + return dba;
> >> + }
> >> +
> >> + buf->dma_dir = vb->vb2_queue->dma_dir;
> >> + buf->size = size;
> >> + buf->db_attach = dba;
> >> +
> >> + return buf;
> >> +}
> >> +
> >> +const struct vb2_mem_ops vb2_dmaheap_memops = {
> >> + .alloc = vb2_dmaheap_alloc,
> >> + .put = vb2_dmaheap_put,
> >> + .get_dmabuf = vb2_dmaheap_get_dmabuf,
> >> + .cookie = vb2_dmaheap_cookie,
> >> + .vaddr = vb2_dmaheap_vaddr,
> >> + .prepare = vb2_dmaheap_prepare,
> >> + .finish = vb2_dmaheap_finish,
> >> + .map_dmabuf = vb2_dmaheap_map_dmabuf,
> >> + .unmap_dmabuf = vb2_dmaheap_unmap_dmabuf,
> >> + .attach_dmabuf = vb2_dmaheap_attach_dmabuf,
> >> + .detach_dmabuf = vb2_dmaheap_detach_dmabuf,
> >> + .num_users = vb2_dmaheap_num_users,
> >> + .mmap = vb2_dmaheap_mmap,
> >> +};
> >> +
> >> +MODULE_DESCRIPTION("DMA-Heap memory handling routines for videobuf2");
> >> +MODULE_AUTHOR("Randy Li <[email protected]>");
> >> +MODULE_LICENSE("GPL");
> >> +MODULE_IMPORT_NS(DMA_BUF);
> >> diff --git a/include/media/videobuf2-dma-heap.h b/include/media/videobuf2-dma-heap.h
> >> new file mode 100644
> >> index 000000000000..fa057f67d6e9
> >> --- /dev/null
> >> +++ b/include/media/videobuf2-dma-heap.h
> >> @@ -0,0 +1,30 @@
> >> +/*
> >> + * Copyright (C) 2022 Randy Li <[email protected]>
> >> + *
> >> + * This software is licensed under the terms of the GNU General Public
> >> + * License version 2, as published by the Free Software Foundation, and
> >> + * may be copied, distributed, and modified under those terms.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> + * GNU General Public License for more details.
> >> + *
> >> + */
> >> +
> >> +#ifndef _MEDIA_VIDEOBUF2_DMA_HEAP_H
> >> +#define _MEDIA_VIDEOBUF2_DMA_HEAP_H
> >> +
> >> +#include <media/videobuf2-v4l2.h>
> >> +#include <linux/dma-mapping.h>
> >> +
> >> +static inline dma_addr_t
> >> +vb2_dmaheap_plane_dma_addr(struct vb2_buffer *vb, unsigned int plane_no)
> >> +{
> >> + dma_addr_t *addr = vb2_plane_cookie(vb, plane_no);
> >> +
> >> + return *addr;
> >> +}
> >> +
> >> +extern const struct vb2_mem_ops vb2_dmaheap_memops;
> >> +#endif
> >> --
> >> 2.17.1
> >>
>
> --
> Hsia-Jun(Randy) Li
Sent from my iPad
> On Aug 1, 2022, at 5:46 PM, Tomasz Figa <[email protected]> wrote:
>
> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
>> On Mon, Aug 1, 2022 at 3:44 PM Hsia-Jun Li <[email protected]> wrote:
>>
>>
>>
>>> On 8/1/22 14:19, Tomasz Figa wrote:
>>>
>> Hello Tomasz
>>>
>>> ?Hi Randy,
>>>
>>> On Mon, Aug 1, 2022 at 5:21 AM <[email protected]> wrote:
>>>>
>>>> From: Randy Li <[email protected]>
>>>>
>>>> This module is still at a early stage, I wrote this for showing what
>>>> APIs we need here.
>>>>
>>>> Let me explain why we need such a module here.
>>>> If you won't allocate buffers from a V4L2 M2M device, this module
>>>> may not be very useful. I am sure the most of users won't know a
>>>> device would require them allocate buffers from a DMA-Heap then
>>>> import those buffers into a V4L2's queue.
>>>>
>>>> Then the question goes back to why DMA-Heap. From the Android's
>>>> description, we know it is about the copyright's DRM.
>>>> When we allocate a buffer in a DMA-Heap, it may register that buffer
>>>> in the trusted execution environment so the firmware which is running
>>>> or could only be acccesed from there could use that buffer later.
>>>>
>>>> The answer above leads to another thing which is not done in this
>>>> version, the DMA mapping. Although in some platforms, a DMA-Heap
>>>> responses a IOMMU device as well. For the genernal purpose, we would
>>>> be better assuming the device mapping should be done for each device
>>>> itself. The problem here we only know alloc_devs in those DMAbuf
>>>> methods, which are DMA-heaps in my design, the device from the queue
>>>> is not enough, a plane may requests another IOMMU device or table
>>>> for mapping.
>>>>
>>>> Signed-off-by: Randy Li <[email protected]>
>>>> ---
>>>> drivers/media/common/videobuf2/Kconfig | 6 +
>>>> drivers/media/common/videobuf2/Makefile | 1 +
>>>> .../common/videobuf2/videobuf2-dma-heap.c | 350 ++++++++++++++++++
>>>> include/media/videobuf2-dma-heap.h | 30 ++
>>>> 4 files changed, 387 insertions(+)
>>>> create mode 100644 drivers/media/common/videobuf2/videobuf2-dma-heap.c
>>>> create mode 100644 include/media/videobuf2-dma-heap.h
>>>>
>>>
>>> First of all, thanks for the series.
>>>
>>> Possibly a stupid question, but why not just allocate the DMA-bufs
>>> directly from the DMA-buf heap device in the userspace and just import
>>> the buffers to the V4L2 device using V4L2_MEMORY_DMABUF?
>> Sometimes the allocation policy could be very complex, let's suppose a
>> multiple planes pixel format enabling with frame buffer compression.
>> Its luma, chroma data could be allocated from a pool which is delegated
>> for large buffers while its metadata would come from a pool which many
>> users could take some few slices from it(likes system pool).
>>
>> Then when we have a new users knowing nothing about this platform, if we
>> just configure the alloc_devs in each queues well. The user won't need
>> to know those complex rules.
>>
>> The real situation could be more complex, Samsung MFC's left and right
>> banks could be regarded as two pools, many devices would benefit from
>> this either from the allocation times or the security buffers policy.
>>
>> In our design, when we need to do some security decoding(DRM video),
>> codecs2 would allocate buffers from the pool delegated for that. While
>> the non-DRM video, users could not care about this.
>
> I'm a little bit surprised about this, because on Android all the
> graphics buffers are allocated from the system IAllocator and imported
> to the specific devices.
>
In the non-tunnel mode, yes it is. While the tunnel mode is completely vendor defined. Neither HWC nor codec2 cares about where the buffers coming from, you could do what ever you want.
Besides there are DRM video in GNU Linux platform, I heard the webkit has made huge effort here and Playready is one could work in non-Android Linux.
> Would it make sense to instead extend the UAPI to expose enough
> information about the allocation requirements to the userspace, so it
> can allocate correctly?
Yes, it could. But as I said it would need the users to do more works.
> My reasoning here is that it's not a driver's decision to allocate
> from a DMA-buf heap (and which one) or not. It's the userspace which
> knows that, based on the specific use case that it wants to fulfill.
>
Although I would like to let the users decide that, users just can’t do that which would violate the security rules in some platforms.
For example, video codec and display device could only access a region of memory, any other device or trusted apps can’t access it. Users have to allocate the buffer from the pool the vendor decided.
So why not we offer a quick way that users don’t need to try and error.
> Also, FWIW, dma_heap_ioctl_allocate() is a static function not exposed
> to other kernel modules:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__elixir.bootlin.com_linux_v5.19_source_drivers_dma-2Dbuf_dma-2Dheap.c-23L52&d=DwIBaQ&c=7dfBJ8cXbWjhc0BhImu8wVIoUFmBzj1s88r8EGyM0UY&r=P4xb2_7biqBxD4LGGPrSV6j-jf3C3xlR7PXU-mLTeZE&m=HFC3KS_ZU9m61rWQgCO99xSAwnfR3nT8M6h9aW2JYG4-Sy_Uog4xkR8awUOw65Fe&s=TPQwWeG-DdLcgJtaA1cIuQhl3zsHLITkWFV1UNLSFWs&e=
>
I may forget to mention that you need two extra patches from Linaro that export those API(original version is actually out of time). Besides Android kernel did have the two kAPI I need here.
Actually I need more APIs from DMA-heap to archive those things in TODO list.
> By the way, the MFC left/right port requirement was gone long ago, it
> was only one of the earliest Exynos SoCs which required that.
>
Yes, MFCv5 or v6 right. I just want mention that the world has any possible, vendor always has its own reason.
> Best regards,
> Tomasz
>
>>>
>>> Best regards,
>>> Tomasz
>>>
>>>> diff --git a/drivers/media/common/videobuf2/Kconfig b/drivers/media/common/videobuf2/Kconfig
>>>> index d2223a12c95f..02235077f07e 100644
>>>> --- a/drivers/media/common/videobuf2/Kconfig
>>>> +++ b/drivers/media/common/videobuf2/Kconfig
>>>> @@ -30,3 +30,9 @@ config VIDEOBUF2_DMA_SG
>>>> config VIDEOBUF2_DVB
>>>> tristate
>>>> select VIDEOBUF2_CORE
>>>> +
>>>> +config VIDEOBUF2_DMA_HEAP
>>>> + tristate
>>>> + select VIDEOBUF2_CORE
>>>> + select VIDEOBUF2_MEMOPS
>>>> + select DMABUF_HEAPS
>>>> diff --git a/drivers/media/common/videobuf2/Makefile b/drivers/media/common/videobuf2/Makefile
>>>> index a6fe3f304685..7fe65f93117f 100644
>>>> --- a/drivers/media/common/videobuf2/Makefile
>>>> +++ b/drivers/media/common/videobuf2/Makefile
>>>> @@ -10,6 +10,7 @@ endif
>>>> # (e. g. LC_ALL=C sort Makefile)
>>>> obj-$(CONFIG_VIDEOBUF2_CORE) += videobuf2-common.o
>>>> obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
>>>> +obj-$(CONFIG_VIDEOBUF2_DMA_HEAP) += videobuf2-dma-heap.o
>>>> obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
>>>> obj-$(CONFIG_VIDEOBUF2_DVB) += videobuf2-dvb.o
>>>> obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
>>>> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-heap.c b/drivers/media/common/videobuf2/videobuf2-dma-heap.c
>>>> new file mode 100644
>>>> index 000000000000..377b82ab8f5a
>>>> --- /dev/null
>>>> +++ b/drivers/media/common/videobuf2/videobuf2-dma-heap.c
>>>> @@ -0,0 +1,350 @@
>>>> +/*
>>>> + * Copyright (C) 2022 Randy Li <[email protected]>
>>>> + *
>>>> + * This software is licensed under the terms of the GNU General Public
>>>> + * License version 2, as published by the Free Software Foundation, and
>>>> + * may be copied, distributed, and modified under those terms.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>> + * GNU General Public License for more details.
>>>> + *
>>>> + */
>>>> +
>>>> +#include <linux/dma-buf.h>
>>>> +#include <linux/dma-heap.h>
>>>> +#include <linux/refcount.h>
>>>> +#include <linux/scatterlist.h>
>>>> +#include <linux/sched.h>
>>>> +#include <linux/slab.h>
>>>> +#include <linux/dma-mapping.h>
>>>> +
>>>> +#include <media/videobuf2-v4l2.h>
>>>> +#include <media/videobuf2-memops.h>
>>>> +#include <media/videobuf2-dma-heap.h>
>>>> +
>>>> +struct vb2_dmaheap_buf {
>>>> + struct device *dev;
>>>> + void *vaddr;
>>>> + unsigned long size;
>>>> + struct dma_buf *dmabuf;
>>>> + dma_addr_t dma_addr;
>>>> + unsigned long attrs;
>>>> + enum dma_data_direction dma_dir;
>>>> + struct sg_table *dma_sgt;
>>>> +
>>>> + /* MMAP related */
>>>> + struct vb2_vmarea_handler handler;
>>>> + refcount_t refcount;
>>>> +
>>>> + /* DMABUF related */
>>>> + struct dma_buf_attachment *db_attach;
>>>> +};
>>>> +
>>>> +/*********************************************/
>>>> +/* callbacks for all buffers */
>>>> +/*********************************************/
>>>> +
>>>> +void *vb2_dmaheap_cookie(struct vb2_buffer *vb, void *buf_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = buf_priv;
>>>> +
>>>> + return &buf->dma_addr;
>>>> +}
>>>> +
>>>> +static void *vb2_dmaheap_vaddr(struct vb2_buffer *vb, void *buf_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = buf_priv;
>>>> + struct iosys_map map;
>>>> +
>>>> + if (buf->vaddr)
>>>> + return buf->vaddr;
>>>> +
>>>> + if (buf->db_attach) {
>>>> + if (!dma_buf_vmap(buf->db_attach->dmabuf, &map))
>>>> + buf->vaddr = map.vaddr;
>>>> + }
>>>> +
>>>> + return buf->vaddr;
>>>> +}
>>>> +
>>>> +static unsigned int vb2_dmaheap_num_users(void *buf_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = buf_priv;
>>>> +
>>>> + return refcount_read(&buf->refcount);
>>>> +}
>>>> +
>>>> +static void vb2_dmaheap_prepare(void *buf_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = buf_priv;
>>>> +
>>>> + /* TODO: DMABUF exporter will flush the cache for us */
>>>> + if (buf->db_attach)
>>>> + return;
>>>> +
>>>> + dma_buf_end_cpu_access(buf->dmabuf, buf->dma_dir);
>>>> +}
>>>> +
>>>> +static void vb2_dmaheap_finish(void *buf_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = buf_priv;
>>>> +
>>>> + /* TODO: DMABUF exporter will flush the cache for us */
>>>> + if (buf->db_attach)
>>>> + return;
>>>> +
>>>> + dma_buf_begin_cpu_access(buf->dmabuf, buf->dma_dir);
>>>> +}
>>>> +
>>>> +/*********************************************/
>>>> +/* callbacks for MMAP buffers */
>>>> +/*********************************************/
>>>> +
>>>> +void vb2_dmaheap_put(void *buf_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = buf_priv;
>>>> +
>>>> + if (!refcount_dec_and_test(&buf->refcount))
>>>> + return;
>>>> +
>>>> + dma_buf_put(buf->dmabuf);
>>>> +
>>>> + put_device(buf->dev);
>>>> + kfree(buf);
>>>> +}
>>>> +
>>>> +static void *vb2_dmaheap_alloc(struct vb2_buffer *vb,
>>>> + struct device *dev,
>>>> + unsigned long size)
>>>> +{
>>>> + struct vb2_queue *q = vb->vb2_queue;
>>>> + struct dma_heap *heap;
>>>> + struct vb2_dmaheap_buf *buf;
>>>> + const char *heap_name;
>>>> + int ret;
>>>> +
>>>> + if (WARN_ON(!dev))
>>>> + return ERR_PTR(-EINVAL);
>>>> +
>>>> + heap_name = dev_name(dev);
>>>> + if (!heap_name)
>>>> + return ERR_PTR(-EINVAL);
>>>> +
>>>> + heap = dma_heap_find(heap_name);
>>>> + if (!heap) {
>>>> + dev_err(dev, "is not a DMA-heap device\n");
>>>> + return ERR_PTR(-EINVAL);
>>>> + }
>>>> +
>>>> + buf = kzalloc(sizeof *buf, GFP_KERNEL);
>>>> + if (!buf)
>>>> + return ERR_PTR(-ENOMEM);
>>>> +
>>>> + /* Prevent the device from being released while the buffer is used */
>>>> + buf->dev = get_device(dev);
>>>> + buf->attrs = vb->vb2_queue->dma_attrs;
>>>> + buf->dma_dir = vb->vb2_queue->dma_dir;
>>>> +
>>>> + /* TODO: heap flags */
>>>> + ret = dma_heap_buffer_alloc(heap, size, 0, 0);
>>>> + if (ret < 0) {
>>>> + dev_err(dev, "is not a DMA-heap device\n");
>>>> + put_device(buf->dev);
>>>> + kfree(buf);
>>>> + return ERR_PTR(ret);
>>>> + }
>>>> + buf->dmabuf = dma_buf_get(ret);
>>>> +
>>>> + /* FIXME */
>>>> + buf->dma_addr = 0;
>>>> +
>>>> + if ((q->dma_attrs & DMA_ATTR_NO_KERNEL_MAPPING) == 0)
>>>> + buf->vaddr = buf->dmabuf;
>>>> +
>>>> + buf->handler.refcount = &buf->refcount;
>>>> + buf->handler.put = vb2_dmaheap_put;
>>>> + buf->handler.arg = buf;
>>>> +
>>>> + refcount_set(&buf->refcount, 1);
>>>> +
>>>> + return buf;
>>>> +}
>>>> +
>>>> +static int vb2_dmaheap_mmap(void *buf_priv, struct vm_area_struct *vma)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = buf_priv;
>>>> + int ret;
>>>> +
>>>> + if (!buf) {
>>>> + printk(KERN_ERR "No buffer to map\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + vma->vm_flags &= ~VM_PFNMAP;
>>>> +
>>>> + ret = dma_buf_mmap(buf->dmabuf, vma, 0);
>>>> + if (ret) {
>>>> + pr_err("Remapping memory failed, error: %d\n", ret);
>>>> + return ret;
>>>> + }
>>>> + vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
>>>> + vma->vm_private_data = &buf->handler;
>>>> + vma->vm_ops = &vb2_common_vm_ops;
>>>> +
>>>> + vma->vm_ops->open(vma);
>>>> +
>>>> + pr_debug("%s: mapped memid 0x%08lx at 0x%08lx, size %ld\n",
>>>> + __func__, (unsigned long)buf->dma_addr, vma->vm_start,
>>>> + buf->size);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +/*********************************************/
>>>> +/* DMABUF ops for exporters */
>>>> +/*********************************************/
>>>> +
>>>> +static struct dma_buf *vb2_dmaheap_get_dmabuf(struct vb2_buffer *vb,
>>>> + void *buf_priv,
>>>> + unsigned long flags)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = buf_priv;
>>>> + struct dma_buf *dbuf;
>>>> +
>>>> + dbuf = buf->dmabuf;
>>>> +
>>>> + return dbuf;
>>>> +}
>>>> +
>>>> +/*********************************************/
>>>> +/* callbacks for DMABUF buffers */
>>>> +/*********************************************/
>>>> +
>>>> +static int vb2_dmaheap_map_dmabuf(void *mem_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = mem_priv;
>>>> + struct sg_table *sgt;
>>>> +
>>>> + if (WARN_ON(!buf->db_attach)) {
>>>> + pr_err("trying to pin a non attached buffer\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + if (WARN_ON(buf->dma_sgt)) {
>>>> + pr_err("dmabuf buffer is already pinned\n");
>>>> + return 0;
>>>> + }
>>>> +
>>>> + /* get the associated scatterlist for this buffer */
>>>> + sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir);
>>>> + if (IS_ERR(sgt)) {
>>>> + pr_err("Error getting dmabuf scatterlist\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + buf->dma_addr = sg_dma_address(sgt->sgl);
>>>> + buf->dma_sgt = sgt;
>>>> + buf->vaddr = NULL;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void vb2_dmaheap_unmap_dmabuf(void *mem_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = mem_priv;
>>>> + struct sg_table *sgt = buf->dma_sgt;
>>>> + struct iosys_map map = IOSYS_MAP_INIT_VADDR(buf->vaddr);
>>>> +
>>>> + if (WARN_ON(!buf->db_attach)) {
>>>> + pr_err("trying to unpin a not attached buffer\n");
>>>> + return;
>>>> + }
>>>> +
>>>> + if (WARN_ON(!sgt)) {
>>>> + pr_err("dmabuf buffer is already unpinned\n");
>>>> + return;
>>>> + }
>>>> +
>>>> + if (buf->vaddr) {
>>>> + dma_buf_vunmap(buf->db_attach->dmabuf, &map);
>>>> + buf->vaddr = NULL;
>>>> + }
>>>> + dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
>>>> +
>>>> + buf->dma_addr = 0;
>>>> + buf->dma_sgt = NULL;
>>>> +}
>>>> +
>>>> +static void vb2_dmaheap_detach_dmabuf(void *mem_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = mem_priv;
>>>> +
>>>> + /* if vb2 works correctly you should never detach mapped buffer */
>>>> + if (WARN_ON(buf->dma_addr))
>>>> + vb2_dmaheap_unmap_dmabuf(buf);
>>>> +
>>>> + /* detach this attachment */
>>>> + dma_buf_detach(buf->db_attach->dmabuf, buf->db_attach);
>>>> + kfree(buf);
>>>> +}
>>>> +
>>>> +static void *vb2_dmaheap_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
>>>> + struct dma_buf *dbuf, unsigned long size)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf;
>>>> + struct dma_buf_attachment *dba;
>>>> +
>>>> + if (dbuf->size < size)
>>>> + return ERR_PTR(-EFAULT);
>>>> +
>>>> + if (WARN_ON(!dev))
>>>> + return ERR_PTR(-EINVAL);
>>>> + /*
>>>> + * TODO: A better way to check whether the buffer is coming
>>>> + * from this heap or this heap could accept this buffer
>>>> + */
>>>> + if (strcmp(dbuf->exp_name, dev_name(dev)))
>>>> + return ERR_PTR(-EINVAL);
>>>> +
>>>> + buf = kzalloc(sizeof(*buf), GFP_KERNEL);
>>>> + if (!buf)
>>>> + return ERR_PTR(-ENOMEM);
>>>> +
>>>> + buf->dev = dev;
>>>> + /* create attachment for the dmabuf with the user device */
>>>> + dba = dma_buf_attach(dbuf, buf->dev);
>>>> + if (IS_ERR(dba)) {
>>>> + pr_err("failed to attach dmabuf\n");
>>>> + kfree(buf);
>>>> + return dba;
>>>> + }
>>>> +
>>>> + buf->dma_dir = vb->vb2_queue->dma_dir;
>>>> + buf->size = size;
>>>> + buf->db_attach = dba;
>>>> +
>>>> + return buf;
>>>> +}
>>>> +
>>>> +const struct vb2_mem_ops vb2_dmaheap_memops = {
>>>> + .alloc = vb2_dmaheap_alloc,
>>>> + .put = vb2_dmaheap_put,
>>>> + .get_dmabuf = vb2_dmaheap_get_dmabuf,
>>>> + .cookie = vb2_dmaheap_cookie,
>>>> + .vaddr = vb2_dmaheap_vaddr,
>>>> + .prepare = vb2_dmaheap_prepare,
>>>> + .finish = vb2_dmaheap_finish,
>>>> + .map_dmabuf = vb2_dmaheap_map_dmabuf,
>>>> + .unmap_dmabuf = vb2_dmaheap_unmap_dmabuf,
>>>> + .attach_dmabuf = vb2_dmaheap_attach_dmabuf,
>>>> + .detach_dmabuf = vb2_dmaheap_detach_dmabuf,
>>>> + .num_users = vb2_dmaheap_num_users,
>>>> + .mmap = vb2_dmaheap_mmap,
>>>> +};
>>>> +
>>>> +MODULE_DESCRIPTION("DMA-Heap memory handling routines for videobuf2");
>>>> +MODULE_AUTHOR("Randy Li <[email protected]>");
>>>> +MODULE_LICENSE("GPL");
>>>> +MODULE_IMPORT_NS(DMA_BUF);
>>>> diff --git a/include/media/videobuf2-dma-heap.h b/include/media/videobuf2-dma-heap.h
>>>> new file mode 100644
>>>> index 000000000000..fa057f67d6e9
>>>> --- /dev/null
>>>> +++ b/include/media/videobuf2-dma-heap.h
>>>> @@ -0,0 +1,30 @@
>>>> +/*
>>>> + * Copyright (C) 2022 Randy Li <[email protected]>
>>>> + *
>>>> + * This software is licensed under the terms of the GNU General Public
>>>> + * License version 2, as published by the Free Software Foundation, and
>>>> + * may be copied, distributed, and modified under those terms.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>> + * GNU General Public License for more details.
>>>> + *
>>>> + */
>>>> +
>>>> +#ifndef _MEDIA_VIDEOBUF2_DMA_HEAP_H
>>>> +#define _MEDIA_VIDEOBUF2_DMA_HEAP_H
>>>> +
>>>> +#include <media/videobuf2-v4l2.h>
>>>> +#include <linux/dma-mapping.h>
>>>> +
>>>> +static inline dma_addr_t
>>>> +vb2_dmaheap_plane_dma_addr(struct vb2_buffer *vb, unsigned int plane_no)
>>>> +{
>>>> + dma_addr_t *addr = vb2_plane_cookie(vb, plane_no);
>>>> +
>>>> + return *addr;
>>>> +}
>>>> +
>>>> +extern const struct vb2_mem_ops vb2_dmaheap_memops;
>>>> +#endif
>>>> --
>>>> 2.17.1
>>>>
>>
>> --
>> Hsia-Jun(Randy) Li
Sent from my iPad
> On Aug 1, 2022, at 5:46 PM, Tomasz Figa <[email protected]> wrote:
>
> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
>> On Mon, Aug 1, 2022 at 3:44 PM Hsia-Jun Li <[email protected]> wrote:
>>> On 8/1/22 14:19, Tomasz Figa wrote:
>> Hello Tomasz
>>> ?Hi Randy,
>>> On Mon, Aug 1, 2022 at 5:21 AM <[email protected]> wrote:
>>>> From: Randy Li <[email protected]>
>>>> This module is still at a early stage, I wrote this for showing what
>>>> APIs we need here.
>>>> Let me explain why we need such a module here.
>>>> If you won't allocate buffers from a V4L2 M2M device, this module
>>>> may not be very useful. I am sure the most of users won't know a
>>>> device would require them allocate buffers from a DMA-Heap then
>>>> import those buffers into a V4L2's queue.
>>>> Then the question goes back to why DMA-Heap. From the Android's
>>>> description, we know it is about the copyright's DRM.
>>>> When we allocate a buffer in a DMA-Heap, it may register that buffer
>>>> in the trusted execution environment so the firmware which is running
>>>> or could only be acccesed from there could use that buffer later.
>>>> The answer above leads to another thing which is not done in this
>>>> version, the DMA mapping. Although in some platforms, a DMA-Heap
>>>> responses a IOMMU device as well. For the genernal purpose, we would
>>>> be better assuming the device mapping should be done for each device
>>>> itself. The problem here we only know alloc_devs in those DMAbuf
>>>> methods, which are DMA-heaps in my design, the device from the queue
>>>> is not enough, a plane may requests another IOMMU device or table
>>>> for mapping.
>>>> Signed-off-by: Randy Li <[email protected]>
>>>> ---
>>>> drivers/media/common/videobuf2/Kconfig | 6 +
>>>> drivers/media/common/videobuf2/Makefile | 1 +
>>>> .../common/videobuf2/videobuf2-dma-heap.c | 350 ++++++++++++++++++
>>>> include/media/videobuf2-dma-heap.h | 30 ++
>>>> 4 files changed, 387 insertions(+)
>>>> create mode 100644 drivers/media/common/videobuf2/videobuf2-dma-heap.c
>>>> create mode 100644 include/media/videobuf2-dma-heap.h
>>> First of all, thanks for the series.
>>> Possibly a stupid question, but why not just allocate the DMA-bufs
>>> directly from the DMA-buf heap device in the userspace and just import
>>> the buffers to the V4L2 device using V4L2_MEMORY_DMABUF?
>> Sometimes the allocation policy could be very complex, let's suppose a
>> multiple planes pixel format enabling with frame buffer compression.
>> Its luma, chroma data could be allocated from a pool which is delegated
>> for large buffers while its metadata would come from a pool which many
>> users could take some few slices from it(likes system pool).
>> Then when we have a new users knowing nothing about this platform, if we
>> just configure the alloc_devs in each queues well. The user won't need
>> to know those complex rules.
>> The real situation could be more complex, Samsung MFC's left and right
>> banks could be regarded as two pools, many devices would benefit from
>> this either from the allocation times or the security buffers policy.
>> In our design, when we need to do some security decoding(DRM video),
>> codecs2 would allocate buffers from the pool delegated for that. While
>> the non-DRM video, users could not care about this.
>
> I'm a little bit surprised about this, because on Android all the
> graphics buffers are allocated from the system IAllocator and imported
> to the specific devices.
In the non-tunnel mode, yes it is. While the tunnel mode is completely vendor defined. Neither HWC nor codec2 cares about where the buffers coming from, you could do what ever you want.
Besides there are DRM video in GNU Linux platform, I heard the webkit has made huge effort here and Playready is one could work in non-Android Linux.
> Would it make sense to instead extend the UAPI to expose enough
> information about the allocation requirements to the userspace, so it
> can allocate correctly?
Yes, it could. But as I said it would need the users to do more works.
> My reasoning here is that it's not a driver's decision to allocate
> from a DMA-buf heap (and which one) or not. It's the userspace which
> knows that, based on the specific use case that it wants to fulfill.
Although I would like to let the users decide that, users just can’t do that which would violate the security rules in some platforms.
For example, video codec and display device could only access a region of memory, any other device or trusted apps can’t access it. Users have to allocate the buffer from the pool the vendor decided.
So why not we offer a quick way that users don’t need to try and error.
> Also, FWIW, dma_heap_ioctl_allocate() is a static function not exposed
> to other kernel modules:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__elixir.bootlin.com_linux_v5.19_source_drivers_dma-2Dbuf_dma-2Dheap.c-23L52&d=DwIBaQ&c=7dfBJ8cXbWjhc0BhImu8wVIoUFmBzj1s88r8EGyM0UY&r=P4xb2_7biqBxD4LGGPrSV6j-jf3C3xlR7PXU-mLTeZE&m=HFC3KS_ZU9m61rWQgCO99xSAwnfR3nT8M6h9aW2JYG4-Sy_Uog4xkR8awUOw65Fe&s=TPQwWeG-DdLcgJtaA1cIuQhl3zsHLITkWFV1UNLSFWs&e=
I may forget to mention that you need two extra patches from Linaro that export those API(original version is actually out of time). Besides Android kernel did have the two kAPI I need here.
Actually I need more APIs from DMA-heap to archive those things in TODO list.
> By the way, the MFC left/right port requirement was gone long ago, it
> was only one of the earliest Exynos SoCs which required that.
Yes, MFCv5 or v6 right. I just want mention that the world has any possible, vendor always has its own reason.
> Best regards,
> Tomasz
>
>>> Best regards,
>>> Tomasz
>>>> diff --git a/drivers/media/common/videobuf2/Kconfig b/drivers/media/common/videobuf2/Kconfig
>>>> index d2223a12c95f..02235077f07e 100644
>>>> --- a/drivers/media/common/videobuf2/Kconfig
>>>> +++ b/drivers/media/common/videobuf2/Kconfig
>>>> @@ -30,3 +30,9 @@ config VIDEOBUF2_DMA_SG
>>>> config VIDEOBUF2_DVB
>>>> tristate
>>>> select VIDEOBUF2_CORE
>>>> +
>>>> +config VIDEOBUF2_DMA_HEAP
>>>> + tristate
>>>> + select VIDEOBUF2_CORE
>>>> + select VIDEOBUF2_MEMOPS
>>>> + select DMABUF_HEAPS
>>>> diff --git a/drivers/media/common/videobuf2/Makefile b/drivers/media/common/videobuf2/Makefile
>>>> index a6fe3f304685..7fe65f93117f 100644
>>>> --- a/drivers/media/common/videobuf2/Makefile
>>>> +++ b/drivers/media/common/videobuf2/Makefile
>>>> @@ -10,6 +10,7 @@ endif
>>>> # (e. g. LC_ALL=C sort Makefile)
>>>> obj-$(CONFIG_VIDEOBUF2_CORE) += videobuf2-common.o
>>>> obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
>>>> +obj-$(CONFIG_VIDEOBUF2_DMA_HEAP) += videobuf2-dma-heap.o
>>>> obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
>>>> obj-$(CONFIG_VIDEOBUF2_DVB) += videobuf2-dvb.o
>>>> obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
>>>> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-heap.c b/drivers/media/common/videobuf2/videobuf2-dma-heap.c
>>>> new file mode 100644
>>>> index 000000000000..377b82ab8f5a
>>>> --- /dev/null
>>>> +++ b/drivers/media/common/videobuf2/videobuf2-dma-heap.c
>>>> @@ -0,0 +1,350 @@
>>>> +/*
>>>> + * Copyright (C) 2022 Randy Li <[email protected]>
>>>> + *
>>>> + * This software is licensed under the terms of the GNU General Public
>>>> + * License version 2, as published by the Free Software Foundation, and
>>>> + * may be copied, distributed, and modified under those terms.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>> + * GNU General Public License for more details.
>>>> + *
>>>> + */
>>>> +
>>>> +#include <linux/dma-buf.h>
>>>> +#include <linux/dma-heap.h>
>>>> +#include <linux/refcount.h>
>>>> +#include <linux/scatterlist.h>
>>>> +#include <linux/sched.h>
>>>> +#include <linux/slab.h>
>>>> +#include <linux/dma-mapping.h>
>>>> +
>>>> +#include <media/videobuf2-v4l2.h>
>>>> +#include <media/videobuf2-memops.h>
>>>> +#include <media/videobuf2-dma-heap.h>
>>>> +
>>>> +struct vb2_dmaheap_buf {
>>>> + struct device *dev;
>>>> + void *vaddr;
>>>> + unsigned long size;
>>>> + struct dma_buf *dmabuf;
>>>> + dma_addr_t dma_addr;
>>>> + unsigned long attrs;
>>>> + enum dma_data_direction dma_dir;
>>>> + struct sg_table *dma_sgt;
>>>> +
>>>> + /* MMAP related */
>>>> + struct vb2_vmarea_handler handler;
>>>> + refcount_t refcount;
>>>> +
>>>> + /* DMABUF related */
>>>> + struct dma_buf_attachment *db_attach;
>>>> +};
>>>> +
>>>> +/*********************************************/
>>>> +/* callbacks for all buffers */
>>>> +/*********************************************/
>>>> +
>>>> +void *vb2_dmaheap_cookie(struct vb2_buffer *vb, void *buf_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = buf_priv;
>>>> +
>>>> + return &buf->dma_addr;
>>>> +}
>>>> +
>>>> +static void *vb2_dmaheap_vaddr(struct vb2_buffer *vb, void *buf_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = buf_priv;
>>>> + struct iosys_map map;
>>>> +
>>>> + if (buf->vaddr)
>>>> + return buf->vaddr;
>>>> +
>>>> + if (buf->db_attach) {
>>>> + if (!dma_buf_vmap(buf->db_attach->dmabuf, &map))
>>>> + buf->vaddr = map.vaddr;
>>>> + }
>>>> +
>>>> + return buf->vaddr;
>>>> +}
>>>> +
>>>> +static unsigned int vb2_dmaheap_num_users(void *buf_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = buf_priv;
>>>> +
>>>> + return refcount_read(&buf->refcount);
>>>> +}
>>>> +
>>>> +static void vb2_dmaheap_prepare(void *buf_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = buf_priv;
>>>> +
>>>> + /* TODO: DMABUF exporter will flush the cache for us */
>>>> + if (buf->db_attach)
>>>> + return;
>>>> +
>>>> + dma_buf_end_cpu_access(buf->dmabuf, buf->dma_dir);
>>>> +}
>>>> +
>>>> +static void vb2_dmaheap_finish(void *buf_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = buf_priv;
>>>> +
>>>> + /* TODO: DMABUF exporter will flush the cache for us */
>>>> + if (buf->db_attach)
>>>> + return;
>>>> +
>>>> + dma_buf_begin_cpu_access(buf->dmabuf, buf->dma_dir);
>>>> +}
>>>> +
>>>> +/*********************************************/
>>>> +/* callbacks for MMAP buffers */
>>>> +/*********************************************/
>>>> +
>>>> +void vb2_dmaheap_put(void *buf_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = buf_priv;
>>>> +
>>>> + if (!refcount_dec_and_test(&buf->refcount))
>>>> + return;
>>>> +
>>>> + dma_buf_put(buf->dmabuf);
>>>> +
>>>> + put_device(buf->dev);
>>>> + kfree(buf);
>>>> +}
>>>> +
>>>> +static void *vb2_dmaheap_alloc(struct vb2_buffer *vb,
>>>> + struct device *dev,
>>>> + unsigned long size)
>>>> +{
>>>> + struct vb2_queue *q = vb->vb2_queue;
>>>> + struct dma_heap *heap;
>>>> + struct vb2_dmaheap_buf *buf;
>>>> + const char *heap_name;
>>>> + int ret;
>>>> +
>>>> + if (WARN_ON(!dev))
>>>> + return ERR_PTR(-EINVAL);
>>>> +
>>>> + heap_name = dev_name(dev);
>>>> + if (!heap_name)
>>>> + return ERR_PTR(-EINVAL);
>>>> +
>>>> + heap = dma_heap_find(heap_name);
>>>> + if (!heap) {
>>>> + dev_err(dev, "is not a DMA-heap device\n");
>>>> + return ERR_PTR(-EINVAL);
>>>> + }
>>>> +
>>>> + buf = kzalloc(sizeof *buf, GFP_KERNEL);
>>>> + if (!buf)
>>>> + return ERR_PTR(-ENOMEM);
>>>> +
>>>> + /* Prevent the device from being released while the buffer is used */
>>>> + buf->dev = get_device(dev);
>>>> + buf->attrs = vb->vb2_queue->dma_attrs;
>>>> + buf->dma_dir = vb->vb2_queue->dma_dir;
>>>> +
>>>> + /* TODO: heap flags */
>>>> + ret = dma_heap_buffer_alloc(heap, size, 0, 0);
>>>> + if (ret < 0) {
>>>> + dev_err(dev, "is not a DMA-heap device\n");
>>>> + put_device(buf->dev);
>>>> + kfree(buf);
>>>> + return ERR_PTR(ret);
>>>> + }
>>>> + buf->dmabuf = dma_buf_get(ret);
>>>> +
>>>> + /* FIXME */
>>>> + buf->dma_addr = 0;
>>>> +
>>>> + if ((q->dma_attrs & DMA_ATTR_NO_KERNEL_MAPPING) == 0)
>>>> + buf->vaddr = buf->dmabuf;
>>>> +
>>>> + buf->handler.refcount = &buf->refcount;
>>>> + buf->handler.put = vb2_dmaheap_put;
>>>> + buf->handler.arg = buf;
>>>> +
>>>> + refcount_set(&buf->refcount, 1);
>>>> +
>>>> + return buf;
>>>> +}
>>>> +
>>>> +static int vb2_dmaheap_mmap(void *buf_priv, struct vm_area_struct *vma)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = buf_priv;
>>>> + int ret;
>>>> +
>>>> + if (!buf) {
>>>> + printk(KERN_ERR "No buffer to map\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + vma->vm_flags &= ~VM_PFNMAP;
>>>> +
>>>> + ret = dma_buf_mmap(buf->dmabuf, vma, 0);
>>>> + if (ret) {
>>>> + pr_err("Remapping memory failed, error: %d\n", ret);
>>>> + return ret;
>>>> + }
>>>> + vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
>>>> + vma->vm_private_data = &buf->handler;
>>>> + vma->vm_ops = &vb2_common_vm_ops;
>>>> +
>>>> + vma->vm_ops->open(vma);
>>>> +
>>>> + pr_debug("%s: mapped memid 0x%08lx at 0x%08lx, size %ld\n",
>>>> + __func__, (unsigned long)buf->dma_addr, vma->vm_start,
>>>> + buf->size);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +/*********************************************/
>>>> +/* DMABUF ops for exporters */
>>>> +/*********************************************/
>>>> +
>>>> +static struct dma_buf *vb2_dmaheap_get_dmabuf(struct vb2_buffer *vb,
>>>> + void *buf_priv,
>>>> + unsigned long flags)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = buf_priv;
>>>> + struct dma_buf *dbuf;
>>>> +
>>>> + dbuf = buf->dmabuf;
>>>> +
>>>> + return dbuf;
>>>> +}
>>>> +
>>>> +/*********************************************/
>>>> +/* callbacks for DMABUF buffers */
>>>> +/*********************************************/
>>>> +
>>>> +static int vb2_dmaheap_map_dmabuf(void *mem_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = mem_priv;
>>>> + struct sg_table *sgt;
>>>> +
>>>> + if (WARN_ON(!buf->db_attach)) {
>>>> + pr_err("trying to pin a non attached buffer\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + if (WARN_ON(buf->dma_sgt)) {
>>>> + pr_err("dmabuf buffer is already pinned\n");
>>>> + return 0;
>>>> + }
>>>> +
>>>> + /* get the associated scatterlist for this buffer */
>>>> + sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir);
>>>> + if (IS_ERR(sgt)) {
>>>> + pr_err("Error getting dmabuf scatterlist\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + buf->dma_addr = sg_dma_address(sgt->sgl);
>>>> + buf->dma_sgt = sgt;
>>>> + buf->vaddr = NULL;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void vb2_dmaheap_unmap_dmabuf(void *mem_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = mem_priv;
>>>> + struct sg_table *sgt = buf->dma_sgt;
>>>> + struct iosys_map map = IOSYS_MAP_INIT_VADDR(buf->vaddr);
>>>> +
>>>> + if (WARN_ON(!buf->db_attach)) {
>>>> + pr_err("trying to unpin a not attached buffer\n");
>>>> + return;
>>>> + }
>>>> +
>>>> + if (WARN_ON(!sgt)) {
>>>> + pr_err("dmabuf buffer is already unpinned\n");
>>>> + return;
>>>> + }
>>>> +
>>>> + if (buf->vaddr) {
>>>> + dma_buf_vunmap(buf->db_attach->dmabuf, &map);
>>>> + buf->vaddr = NULL;
>>>> + }
>>>> + dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
>>>> +
>>>> + buf->dma_addr = 0;
>>>> + buf->dma_sgt = NULL;
>>>> +}
>>>> +
>>>> +static void vb2_dmaheap_detach_dmabuf(void *mem_priv)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf = mem_priv;
>>>> +
>>>> + /* if vb2 works correctly you should never detach mapped buffer */
>>>> + if (WARN_ON(buf->dma_addr))
>>>> + vb2_dmaheap_unmap_dmabuf(buf);
>>>> +
>>>> + /* detach this attachment */
>>>> + dma_buf_detach(buf->db_attach->dmabuf, buf->db_attach);
>>>> + kfree(buf);
>>>> +}
>>>> +
>>>> +static void *vb2_dmaheap_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
>>>> + struct dma_buf *dbuf, unsigned long size)
>>>> +{
>>>> + struct vb2_dmaheap_buf *buf;
>>>> + struct dma_buf_attachment *dba;
>>>> +
>>>> + if (dbuf->size < size)
>>>> + return ERR_PTR(-EFAULT);
>>>> +
>>>> + if (WARN_ON(!dev))
>>>> + return ERR_PTR(-EINVAL);
>>>> + /*
>>>> + * TODO: A better way to check whether the buffer is coming
>>>> + * from this heap or this heap could accept this buffer
>>>> + */
>>>> + if (strcmp(dbuf->exp_name, dev_name(dev)))
>>>> + return ERR_PTR(-EINVAL);
>>>> +
>>>> + buf = kzalloc(sizeof(*buf), GFP_KERNEL);
>>>> + if (!buf)
>>>> + return ERR_PTR(-ENOMEM);
>>>> +
>>>> + buf->dev = dev;
>>>> + /* create attachment for the dmabuf with the user device */
>>>> + dba = dma_buf_attach(dbuf, buf->dev);
>>>> + if (IS_ERR(dba)) {
>>>> + pr_err("failed to attach dmabuf\n");
>>>> + kfree(buf);
>>>> + return dba;
>>>> + }
>>>> +
>>>> + buf->dma_dir = vb->vb2_queue->dma_dir;
>>>> + buf->size = size;
>>>> + buf->db_attach = dba;
>>>> +
>>>> + return buf;
>>>> +}
>>>> +
>>>> +const struct vb2_mem_ops vb2_dmaheap_memops = {
>>>> + .alloc = vb2_dmaheap_alloc,
>>>> + .put = vb2_dmaheap_put,
>>>> + .get_dmabuf = vb2_dmaheap_get_dmabuf,
>>>> + .cookie = vb2_dmaheap_cookie,
>>>> + .vaddr = vb2_dmaheap_vaddr,
>>>> + .prepare = vb2_dmaheap_prepare,
>>>> + .finish = vb2_dmaheap_finish,
>>>> + .map_dmabuf = vb2_dmaheap_map_dmabuf,
>>>> + .unmap_dmabuf = vb2_dmaheap_unmap_dmabuf,
>>>> + .attach_dmabuf = vb2_dmaheap_attach_dmabuf,
>>>> + .detach_dmabuf = vb2_dmaheap_detach_dmabuf,
>>>> + .num_users = vb2_dmaheap_num_users,
>>>> + .mmap = vb2_dmaheap_mmap,
>>>> +};
>>>> +
>>>> +MODULE_DESCRIPTION("DMA-Heap memory handling routines for videobuf2");
>>>> +MODULE_AUTHOR("Randy Li <[email protected]>");
>>>> +MODULE_LICENSE("GPL");
>>>> +MODULE_IMPORT_NS(DMA_BUF);
>>>> diff --git a/include/media/videobuf2-dma-heap.h b/include/media/videobuf2-dma-heap.h
>>>> new file mode 100644
>>>> index 000000000000..fa057f67d6e9
>>>> --- /dev/null
>>>> +++ b/include/media/videobuf2-dma-heap.h
>>>> @@ -0,0 +1,30 @@
>>>> +/*
>>>> + * Copyright (C) 2022 Randy Li <[email protected]>
>>>> + *
>>>> + * This software is licensed under the terms of the GNU General Public
>>>> + * License version 2, as published by the Free Software Foundation, and
>>>> + * may be copied, distributed, and modified under those terms.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>> + * GNU General Public License for more details.
>>>> + *
>>>> + */
>>>> +
>>>> +#ifndef _MEDIA_VIDEOBUF2_DMA_HEAP_H
>>>> +#define _MEDIA_VIDEOBUF2_DMA_HEAP_H
>>>> +
>>>> +#include <media/videobuf2-v4l2.h>
>>>> +#include <linux/dma-mapping.h>
>>>> +
>>>> +static inline dma_addr_t
>>>> +vb2_dmaheap_plane_dma_addr(struct vb2_buffer *vb, unsigned int plane_no)
>>>> +{
>>>> + dma_addr_t *addr = vb2_plane_cookie(vb, plane_no);
>>>> +
>>>> + return *addr;
>>>> +}
>>>> +
>>>> +extern const struct vb2_mem_ops vb2_dmaheap_memops;
>>>> +#endif
>>>> --
>>>> 2.17.1
>> --
>> Hsia-Jun(Randy) Li
On Mon, Aug 1, 2022 at 8:43 PM ayaka <[email protected]> wrote:
>
>
>
> Sent from my iPad
>
> > On Aug 1, 2022, at 5:46 PM, Tomasz Figa <[email protected]> wrote:
> >
> > CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
> >
> >
> >> On Mon, Aug 1, 2022 at 3:44 PM Hsia-Jun Li <[email protected]> wrote:
> >>> On 8/1/22 14:19, Tomasz Figa wrote:
> >> Hello Tomasz
> >>> ?Hi Randy,
> >>> On Mon, Aug 1, 2022 at 5:21 AM <[email protected]> wrote:
> >>>> From: Randy Li <[email protected]>
> >>>> This module is still at a early stage, I wrote this for showing what
> >>>> APIs we need here.
> >>>> Let me explain why we need such a module here.
> >>>> If you won't allocate buffers from a V4L2 M2M device, this module
> >>>> may not be very useful. I am sure the most of users won't know a
> >>>> device would require them allocate buffers from a DMA-Heap then
> >>>> import those buffers into a V4L2's queue.
> >>>> Then the question goes back to why DMA-Heap. From the Android's
> >>>> description, we know it is about the copyright's DRM.
> >>>> When we allocate a buffer in a DMA-Heap, it may register that buffer
> >>>> in the trusted execution environment so the firmware which is running
> >>>> or could only be acccesed from there could use that buffer later.
> >>>> The answer above leads to another thing which is not done in this
> >>>> version, the DMA mapping. Although in some platforms, a DMA-Heap
> >>>> responses a IOMMU device as well. For the genernal purpose, we would
> >>>> be better assuming the device mapping should be done for each device
> >>>> itself. The problem here we only know alloc_devs in those DMAbuf
> >>>> methods, which are DMA-heaps in my design, the device from the queue
> >>>> is not enough, a plane may requests another IOMMU device or table
> >>>> for mapping.
> >>>> Signed-off-by: Randy Li <[email protected]>
> >>>> ---
> >>>> drivers/media/common/videobuf2/Kconfig | 6 +
> >>>> drivers/media/common/videobuf2/Makefile | 1 +
> >>>> .../common/videobuf2/videobuf2-dma-heap.c | 350 ++++++++++++++++++
> >>>> include/media/videobuf2-dma-heap.h | 30 ++
> >>>> 4 files changed, 387 insertions(+)
> >>>> create mode 100644 drivers/media/common/videobuf2/videobuf2-dma-heap.c
> >>>> create mode 100644 include/media/videobuf2-dma-heap.h
> >>> First of all, thanks for the series.
> >>> Possibly a stupid question, but why not just allocate the DMA-bufs
> >>> directly from the DMA-buf heap device in the userspace and just import
> >>> the buffers to the V4L2 device using V4L2_MEMORY_DMABUF?
> >> Sometimes the allocation policy could be very complex, let's suppose a
> >> multiple planes pixel format enabling with frame buffer compression.
> >> Its luma, chroma data could be allocated from a pool which is delegated
> >> for large buffers while its metadata would come from a pool which many
> >> users could take some few slices from it(likes system pool).
> >> Then when we have a new users knowing nothing about this platform, if we
> >> just configure the alloc_devs in each queues well. The user won't need
> >> to know those complex rules.
> >> The real situation could be more complex, Samsung MFC's left and right
> >> banks could be regarded as two pools, many devices would benefit from
> >> this either from the allocation times or the security buffers policy.
> >> In our design, when we need to do some security decoding(DRM video),
> >> codecs2 would allocate buffers from the pool delegated for that. While
> >> the non-DRM video, users could not care about this.
> >
> > I'm a little bit surprised about this, because on Android all the
> > graphics buffers are allocated from the system IAllocator and imported
> > to the specific devices.
> In the non-tunnel mode, yes it is. While the tunnel mode is completely vendor defined. Neither HWC nor codec2 cares about where the buffers coming from, you could do what ever you want.
>
> Besides there are DRM video in GNU Linux platform, I heard the webkit has made huge effort here and Playready is one could work in non-Android Linux.
> > Would it make sense to instead extend the UAPI to expose enough
> > information about the allocation requirements to the userspace, so it
> > can allocate correctly?
> Yes, it could. But as I said it would need the users to do more works.
> > My reasoning here is that it's not a driver's decision to allocate
> > from a DMA-buf heap (and which one) or not. It's the userspace which
> > knows that, based on the specific use case that it wants to fulfill.
> Although I would like to let the users decide that, users just can’t do that which would violate the security rules in some platforms.
> For example, video codec and display device could only access a region of memory, any other device or trusted apps can’t access it. Users have to allocate the buffer from the pool the vendor decided.
>
> So why not we offer a quick way that users don’t need to try and error.
In principle, I'm not against integrating DMA-buf heap with vb2,
however I see some problems I mentioned before:
1) How would the driver know if it should allocate from a DMA-buf heap or not?
2) How would the driver know which heap to allocate from?
3) How would the heap know how to allocate properly for the device?
Best regards,
Tomasz
Sorry, the previous one contains html data.
> On Aug 2, 2022, at 3:33 PM, Tomasz Figa <[email protected]> wrote:
>
> On Mon, Aug 1, 2022 at 8:43 PM ayaka <[email protected]> wrote:
>> Sent from my iPad
>>>> On Aug 1, 2022, at 5:46 PM, Tomasz Figa <[email protected]> wrote:
>>> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
>>>> On Mon, Aug 1, 2022 at 3:44 PM Hsia-Jun Li <[email protected]> wrote:
>>>>> On 8/1/22 14:19, Tomasz Figa wrote:
>>>> Hello Tomasz
>>>>> ?Hi Randy,
>>>>> On Mon, Aug 1, 2022 at 5:21 AM <[email protected]> wrote:
>>>>>> From: Randy Li <[email protected]>
>>>>>> This module is still at a early stage, I wrote this for showing what
>>>>>> APIs we need here.
>>>>>> Let me explain why we need such a module here.
>>>>>> If you won't allocate buffers from a V4L2 M2M device, this module
>>>>>> may not be very useful. I am sure the most of users won't know a
>>>>>> device would require them allocate buffers from a DMA-Heap then
>>>>>> import those buffers into a V4L2's queue.
>>>>>> Then the question goes back to why DMA-Heap. From the Android's
>>>>>> description, we know it is about the copyright's DRM.
>>>>>> When we allocate a buffer in a DMA-Heap, it may register that buffer
>>>>>> in the trusted execution environment so the firmware which is running
>>>>>> or could only be acccesed from there could use that buffer later.
>>>>>> The answer above leads to another thing which is not done in this
>>>>>> version, the DMA mapping. Although in some platforms, a DMA-Heap
>>>>>> responses a IOMMU device as well. For the genernal purpose, we would
>>>>>> be better assuming the device mapping should be done for each device
>>>>>> itself. The problem here we only know alloc_devs in those DMAbuf
>>>>>> methods, which are DMA-heaps in my design, the device from the queue
>>>>>> is not enough, a plane may requests another IOMMU device or table
>>>>>> for mapping.
>>>>>> Signed-off-by: Randy Li <[email protected]>
>>>>>> ---
>>>>>> drivers/media/common/videobuf2/Kconfig | 6 +
>>>>>> drivers/media/common/videobuf2/Makefile | 1 +
>>>>>> .../common/videobuf2/videobuf2-dma-heap.c | 350 ++++++++++++++++++
>>>>>> include/media/videobuf2-dma-heap.h | 30 ++
>>>>>> 4 files changed, 387 insertions(+)
>>>>>> create mode 100644 drivers/media/common/videobuf2/videobuf2-dma-heap.c
>>>>>> create mode 100644 include/media/videobuf2-dma-heap.h
>>>>> First of all, thanks for the series.
>>>>> Possibly a stupid question, but why not just allocate the DMA-bufs
>>>>> directly from the DMA-buf heap device in the userspace and just import
>>>>> the buffers to the V4L2 device using V4L2_MEMORY_DMABUF?
>>>> Sometimes the allocation policy could be very complex, let's suppose a
>>>> multiple planes pixel format enabling with frame buffer compression.
>>>> Its luma, chroma data could be allocated from a pool which is delegated
>>>> for large buffers while its metadata would come from a pool which many
>>>> users could take some few slices from it(likes system pool).
>>>> Then when we have a new users knowing nothing about this platform, if we
>>>> just configure the alloc_devs in each queues well. The user won't need
>>>> to know those complex rules.
>>>> The real situation could be more complex, Samsung MFC's left and right
>>>> banks could be regarded as two pools, many devices would benefit from
>>>> this either from the allocation times or the security buffers policy.
>>>> In our design, when we need to do some security decoding(DRM video),
>>>> codecs2 would allocate buffers from the pool delegated for that. While
>>>> the non-DRM video, users could not care about this.
>>> I'm a little bit surprised about this, because on Android all the
>>> graphics buffers are allocated from the system IAllocator and imported
>>> to the specific devices.
>> In the non-tunnel mode, yes it is. While the tunnel mode is completely vendor defined. Neither HWC nor codec2 cares about where the buffers coming from, you could do what ever you want.
>> Besides there are DRM video in GNU Linux platform, I heard the webkit has made huge effort here and Playready is one could work in non-Android Linux.
>>> Would it make sense to instead extend the UAPI to expose enough
>>> information about the allocation requirements to the userspace, so it
>>> can allocate correctly?
>> Yes, it could. But as I said it would need the users to do more works.
>>> My reasoning here is that it's not a driver's decision to allocate
>>> from a DMA-buf heap (and which one) or not. It's the userspace which
>>> knows that, based on the specific use case that it wants to fulfill.
>> Although I would like to let the users decide that, users just can’t do that which would violate the security rules in some platforms.
>> For example, video codec and display device could only access a region of memory, any other device or trusted apps can’t access it. Users have to allocate the buffer from the pool the vendor decided.
>> So why not we offer a quick way that users don’t need to try and error.
>
> In principle, I'm not against integrating DMA-buf heap with vb2,
> however I see some problems I mentioned before:
>
> 1) How would the driver know if it should allocate from a DMA-buf heap or not?
struct vb2_queue.mem_ops
int (*queue_setup)(struct vb2_queue *q,unsigned int *num_buffers, unsigned int *num_planes, unsigned int sizes[], struct device *alloc_devs[]);
> 2) How would the driver know which heap to allocate from?
From vb2_queue.alloc_devs
What I did now is likes what MFC does, create some mem_alloc_devs.
It would be better that we could retrieve the DMA-heaps’ devices from kernel, but that is not enough, we need a place to store the heap flags although none of them are defined yet.
From Android documents, I think it is unlikely we would have heap flags.
“Standardization: The DMA-BUF heaps framework offers a well-defined UAPI. ION allowed custom flags and heap IDs that prevented developing a common testing framework because each device’s ION implementation could behave differently.”
> 3) How would the heap know how to allocate properly for the device?
>
Because “each DMA-BUF heap is a separate character device”.
But as I said in the first draft I am not sure about the DMA mapping part. alloc_devs responds for the heap, we have a device variable in the queue that mapping function could access, but that may not be enough. A plane may apply a different mapping policy or IOMMU here.
Would it be better that I create a interface here that creating a memdev with DMA-heap description ?
> Best regards,
> Tomasz
On Tue, Aug 2, 2022 at 9:21 PM ayaka <[email protected]> wrote:
>
> Sorry, the previous one contains html data.
>
> > On Aug 2, 2022, at 3:33 PM, Tomasz Figa <[email protected]> wrote:
> >
> > On Mon, Aug 1, 2022 at 8:43 PM ayaka <[email protected]> wrote:
> >> Sent from my iPad
> >>>> On Aug 1, 2022, at 5:46 PM, Tomasz Figa <[email protected]> wrote:
> >>> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
> >>>> On Mon, Aug 1, 2022 at 3:44 PM Hsia-Jun Li <[email protected]> wrote:
> >>>>> On 8/1/22 14:19, Tomasz Figa wrote:
> >>>> Hello Tomasz
> >>>>> ?Hi Randy,
> >>>>> On Mon, Aug 1, 2022 at 5:21 AM <[email protected]> wrote:
> >>>>>> From: Randy Li <[email protected]>
> >>>>>> This module is still at a early stage, I wrote this for showing what
> >>>>>> APIs we need here.
> >>>>>> Let me explain why we need such a module here.
> >>>>>> If you won't allocate buffers from a V4L2 M2M device, this module
> >>>>>> may not be very useful. I am sure the most of users won't know a
> >>>>>> device would require them allocate buffers from a DMA-Heap then
> >>>>>> import those buffers into a V4L2's queue.
> >>>>>> Then the question goes back to why DMA-Heap. From the Android's
> >>>>>> description, we know it is about the copyright's DRM.
> >>>>>> When we allocate a buffer in a DMA-Heap, it may register that buffer
> >>>>>> in the trusted execution environment so the firmware which is running
> >>>>>> or could only be acccesed from there could use that buffer later.
> >>>>>> The answer above leads to another thing which is not done in this
> >>>>>> version, the DMA mapping. Although in some platforms, a DMA-Heap
> >>>>>> responses a IOMMU device as well. For the genernal purpose, we would
> >>>>>> be better assuming the device mapping should be done for each device
> >>>>>> itself. The problem here we only know alloc_devs in those DMAbuf
> >>>>>> methods, which are DMA-heaps in my design, the device from the queue
> >>>>>> is not enough, a plane may requests another IOMMU device or table
> >>>>>> for mapping.
> >>>>>> Signed-off-by: Randy Li <[email protected]>
> >>>>>> ---
> >>>>>> drivers/media/common/videobuf2/Kconfig | 6 +
> >>>>>> drivers/media/common/videobuf2/Makefile | 1 +
> >>>>>> .../common/videobuf2/videobuf2-dma-heap.c | 350 ++++++++++++++++++
> >>>>>> include/media/videobuf2-dma-heap.h | 30 ++
> >>>>>> 4 files changed, 387 insertions(+)
> >>>>>> create mode 100644 drivers/media/common/videobuf2/videobuf2-dma-heap.c
> >>>>>> create mode 100644 include/media/videobuf2-dma-heap.h
> >>>>> First of all, thanks for the series.
> >>>>> Possibly a stupid question, but why not just allocate the DMA-bufs
> >>>>> directly from the DMA-buf heap device in the userspace and just import
> >>>>> the buffers to the V4L2 device using V4L2_MEMORY_DMABUF?
> >>>> Sometimes the allocation policy could be very complex, let's suppose a
> >>>> multiple planes pixel format enabling with frame buffer compression.
> >>>> Its luma, chroma data could be allocated from a pool which is delegated
> >>>> for large buffers while its metadata would come from a pool which many
> >>>> users could take some few slices from it(likes system pool).
> >>>> Then when we have a new users knowing nothing about this platform, if we
> >>>> just configure the alloc_devs in each queues well. The user won't need
> >>>> to know those complex rules.
> >>>> The real situation could be more complex, Samsung MFC's left and right
> >>>> banks could be regarded as two pools, many devices would benefit from
> >>>> this either from the allocation times or the security buffers policy.
> >>>> In our design, when we need to do some security decoding(DRM video),
> >>>> codecs2 would allocate buffers from the pool delegated for that. While
> >>>> the non-DRM video, users could not care about this.
> >>> I'm a little bit surprised about this, because on Android all the
> >>> graphics buffers are allocated from the system IAllocator and imported
> >>> to the specific devices.
> >> In the non-tunnel mode, yes it is. While the tunnel mode is completely vendor defined. Neither HWC nor codec2 cares about where the buffers coming from, you could do what ever you want.
> >> Besides there are DRM video in GNU Linux platform, I heard the webkit has made huge effort here and Playready is one could work in non-Android Linux.
> >>> Would it make sense to instead extend the UAPI to expose enough
> >>> information about the allocation requirements to the userspace, so it
> >>> can allocate correctly?
> >> Yes, it could. But as I said it would need the users to do more works.
> >>> My reasoning here is that it's not a driver's decision to allocate
> >>> from a DMA-buf heap (and which one) or not. It's the userspace which
> >>> knows that, based on the specific use case that it wants to fulfill.
> >> Although I would like to let the users decide that, users just can’t do that which would violate the security rules in some platforms.
> >> For example, video codec and display device could only access a region of memory, any other device or trusted apps can’t access it. Users have to allocate the buffer from the pool the vendor decided.
> >> So why not we offer a quick way that users don’t need to try and error.
> >
> > In principle, I'm not against integrating DMA-buf heap with vb2,
> > however I see some problems I mentioned before:
> >
> > 1) How would the driver know if it should allocate from a DMA-buf heap or not?
>
> struct vb2_queue.mem_ops
>
> int (*queue_setup)(struct vb2_queue *q,unsigned int *num_buffers, unsigned int *num_planes, unsigned int sizes[], struct device *alloc_devs[]);
Sorry, I don't understand what you mean here.
Just to make sure we're on the same page - what I'm referring to is
that whether DMA-buf heap is used or not is specific to a given use
case, which is controlled by the userspace. So the userspace must be
able to control whether the driver allocates from a DMA-buf heap or
the regular way.
>
> > 2) How would the driver know which heap to allocate from?
>
> From vb2_queue.alloc_devs
>
> What I did now is likes what MFC does, create some mem_alloc_devs.
> It would be better that we could retrieve the DMA-heaps’ devices from kernel, but that is not enough, we need a place to store the heap flags although none of them are defined yet.
>
> From Android documents, I think it is unlikely we would have heap flags.
> “Standardization: The DMA-BUF heaps framework offers a well-defined UAPI. ION allowed custom flags and heap IDs that prevented developing a common testing framework because each device’s ION implementation could behave differently.”
>
alloc_devs is something that the driver sets and it's a struct device
for which the DMA API can be called to manage the DMA buffers for this
video device. It's not a way to select a use case-dependent allocation
method.
> > 3) How would the heap know how to allocate properly for the device?
> >
> Because “each DMA-BUF heap is a separate character device”.
Could you elaborate? Sorry, I'm not sure how this answers my question.
> But as I said in the first draft I am not sure about the DMA mapping part. alloc_devs responds for the heap, we have a device variable in the queue that mapping function could access, but that may not be enough. A plane may apply a different mapping policy or IOMMU here.
>
> Would it be better that I create a interface here that creating a memdev with DMA-heap description ?
My intuition still tells me that it would be universally better to
just let the userspace allocate the buffers independently (like with
gralloc/Ion) and import to V4L2 using V4L2_MEM_DMABUF. It was possible
to do things this way nicely with regular Android graphics buffers, so
could you explain what difference of your use case makes it
impossible?
Best regards,
Tomasz
On 8/5/22 18:09, Tomasz Figa wrote:
> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> On Tue, Aug 2, 2022 at 9:21 PM ayaka <[email protected]> wrote:
>>
>> Sorry, the previous one contains html data.
>>
>>> On Aug 2, 2022, at 3:33 PM, Tomasz Figa <[email protected]> wrote:
>>>
>>> On Mon, Aug 1, 2022 at 8:43 PM ayaka <[email protected]> wrote:
>>>> Sent from my iPad
>>>>>> On Aug 1, 2022, at 5:46 PM, Tomasz Figa <[email protected]> wrote:
>>>>> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
>>>>>> On Mon, Aug 1, 2022 at 3:44 PM Hsia-Jun Li <[email protected]> wrote:
>>>>>>> On 8/1/22 14:19, Tomasz Figa wrote:
>>>>>> Hello Tomasz
>>>>>>> ?Hi Randy,
>>>>>>> On Mon, Aug 1, 2022 at 5:21 AM <[email protected]> wrote:
>>>>>>>> From: Randy Li <[email protected]>
>>>>>>>> This module is still at a early stage, I wrote this for showing what
>>>>>>>> APIs we need here.
>>>>>>>> Let me explain why we need such a module here.
>>>>>>>> If you won't allocate buffers from a V4L2 M2M device, this module
>>>>>>>> may not be very useful. I am sure the most of users won't know a
>>>>>>>> device would require them allocate buffers from a DMA-Heap then
>>>>>>>> import those buffers into a V4L2's queue.
>>>>>>>> Then the question goes back to why DMA-Heap. From the Android's
>>>>>>>> description, we know it is about the copyright's DRM.
>>>>>>>> When we allocate a buffer in a DMA-Heap, it may register that buffer
>>>>>>>> in the trusted execution environment so the firmware which is running
>>>>>>>> or could only be acccesed from there could use that buffer later.
>>>>>>>> The answer above leads to another thing which is not done in this
>>>>>>>> version, the DMA mapping. Although in some platforms, a DMA-Heap
>>>>>>>> responses a IOMMU device as well. For the genernal purpose, we would
>>>>>>>> be better assuming the device mapping should be done for each device
>>>>>>>> itself. The problem here we only know alloc_devs in those DMAbuf
>>>>>>>> methods, which are DMA-heaps in my design, the device from the queue
>>>>>>>> is not enough, a plane may requests another IOMMU device or table
>>>>>>>> for mapping.
>>>>>>>> Signed-off-by: Randy Li <[email protected]>
>>>>>>>> ---
>>>>>>>> drivers/media/common/videobuf2/Kconfig | 6 +
>>>>>>>> drivers/media/common/videobuf2/Makefile | 1 +
>>>>>>>> .../common/videobuf2/videobuf2-dma-heap.c | 350 ++++++++++++++++++
>>>>>>>> include/media/videobuf2-dma-heap.h | 30 ++
>>>>>>>> 4 files changed, 387 insertions(+)
>>>>>>>> create mode 100644 drivers/media/common/videobuf2/videobuf2-dma-heap.c
>>>>>>>> create mode 100644 include/media/videobuf2-dma-heap.h
>>>>>>> First of all, thanks for the series.
>>>>>>> Possibly a stupid question, but why not just allocate the DMA-bufs
>>>>>>> directly from the DMA-buf heap device in the userspace and just import
>>>>>>> the buffers to the V4L2 device using V4L2_MEMORY_DMABUF?
>>>>>> Sometimes the allocation policy could be very complex, let's suppose a
>>>>>> multiple planes pixel format enabling with frame buffer compression.
>>>>>> Its luma, chroma data could be allocated from a pool which is delegated
>>>>>> for large buffers while its metadata would come from a pool which many
>>>>>> users could take some few slices from it(likes system pool).
>>>>>> Then when we have a new users knowing nothing about this platform, if we
>>>>>> just configure the alloc_devs in each queues well. The user won't need
>>>>>> to know those complex rules.
>>>>>> The real situation could be more complex, Samsung MFC's left and right
>>>>>> banks could be regarded as two pools, many devices would benefit from
>>>>>> this either from the allocation times or the security buffers policy.
>>>>>> In our design, when we need to do some security decoding(DRM video),
>>>>>> codecs2 would allocate buffers from the pool delegated for that. While
>>>>>> the non-DRM video, users could not care about this.
>>>>> I'm a little bit surprised about this, because on Android all the
>>>>> graphics buffers are allocated from the system IAllocator and imported
>>>>> to the specific devices.
>>>> In the non-tunnel mode, yes it is. While the tunnel mode is completely vendor defined. Neither HWC nor codec2 cares about where the buffers coming from, you could do what ever you want.
>>>> Besides there are DRM video in GNU Linux platform, I heard the webkit has made huge effort here and Playready is one could work in non-Android Linux.
>>>>> Would it make sense to instead extend the UAPI to expose enough
>>>>> information about the allocation requirements to the userspace, so it
>>>>> can allocate correctly?
>>>> Yes, it could. But as I said it would need the users to do more works.
>>>>> My reasoning here is that it's not a driver's decision to allocate
>>>>> from a DMA-buf heap (and which one) or not. It's the userspace which
>>>>> knows that, based on the specific use case that it wants to fulfill.
>>>> Although I would like to let the users decide that, users just can’t do that which would violate the security rules in some platforms.
>>>> For example, video codec and display device could only access a region of memory, any other device or trusted apps can’t access it. Users have to allocate the buffer from the pool the vendor decided.
>>>> So why not we offer a quick way that users don’t need to try and error.
>>>
>>> In principle, I'm not against integrating DMA-buf heap with vb2,
>>> however I see some problems I mentioned before:
>>>
>>> 1) How would the driver know if it should allocate from a DMA-buf heap or not?
>>
>> struct vb2_queue.mem_ops
>>
>> int (*queue_setup)(struct vb2_queue *q,unsigned int *num_buffers, unsigned int *num_planes, unsigned int sizes[], struct device *alloc_devs[]);
>
> Sorry, I don't understand what you mean here.
>
> Just to make sure we're on the same page - what I'm referring to is
> that whether DMA-buf heap is used or not is specific to a given use
> case, which is controlled by the userspace. So the userspace must be
> able to control whether the driver allocates from a DMA-buf heap or
> the regular way.
No, it does not depend on the use case here. We don't accept any buffers
beyond the region we decided. Even for the non-DRM, non-security video,
our codec devices are running under the secure mode.
You MUST allocate the buffer for a device from the DMA-heap we(SYNA)
decided.
I believe some other devices may have much limitation for not the
security reason, for example, it can't access the memory above 4 GiB or
for the performance's reason.
>
>>
>>> 2) How would the driver know which heap to allocate from?
>>
>> From vb2_queue.alloc_devs
>>
>> What I did now is likes what MFC does, create some mem_alloc_devs.
>> It would be better that we could retrieve the DMA-heaps’ devices from kernel, but that is not enough, we need a place to store the heap flags although none of them are defined yet.
>>
>> From Android documents, I think it is unlikely we would have heap flags.
>> “Standardization: The DMA-BUF heaps framework offers a well-defined UAPI. ION allowed custom flags and heap IDs that prevented developing a common testing framework because each device’s ION implementation could behave differently.”
>>
>
> alloc_devs is something that the driver sets and it's a struct device
> for which the DMA API can be called to manage the DMA buffers for this
> video device. It's not a way to select a use case-dependent allocation
> method.
>
I see, then move to the last question, we need to expand the V4L2
framework's structure.
>>> 3) How would the heap know how to allocate properly for the device?
>>>
>> Because “each DMA-BUF heap is a separate character device”.
>
> Could you elaborate? Sorry, I'm not sure how this answers my question.
Because a DMA-heap, a heap device itself is enough here, may plus HEAP
flag when there is.
I don't know what else would be need to do here.
If you allocate a buffer from a heap which is delegated for security
memory of that device, the heap driver itself would inform the TEE the
pages occupied by it or the memory allocated from the pool which is in a
region of memory reserved for this purpose.
>
>> But as I said in the first draft I am not sure about the DMA mapping part. alloc_devs responds for the heap, we have a device variable in the queue that mapping function could access, but that may not be enough. A plane may apply a different mapping policy or IOMMU here.
>>
>> Would it be better that I create a interface here that creating a memdev with DMA-heap description ?
>
> My intuition still tells me that it would be universally better to
> just let the userspace allocate the buffers independently (like with
> gralloc/Ion) and import to V4L2 using V4L2_MEM_DMABUF. It was possible
> to do things this way nicely with regular Android graphics buffers, so
> could you explain what difference of your use case makes it
> impossible?
Without keeping the backward compatibility, it won't have any problem IF
we could tell the users the acceptable DMA-heap for each of planes and
DMA-heap's heap flags.
We don't have an ioctl for this yet, the most possible for the decoder
is doing that at GET_FMT ioctl()?.
>
> Best regards,
> Tomasz
P.S. we would disclose our pixel formats soon, it could offer more
information here.
---
Sincerely
Hsia-Jun(Randy) Li
On 8/18/22 13:50, Tomasz Figa wrote:
> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> Hi Randy,
>
> Sorry for the late reply, I went on vacation last week.
>
> On Sun, Aug 7, 2022 at 12:23 AM Hsia-Jun Li <[email protected]> wrote:
>>
>>
>>
>> On 8/5/22 18:09, Tomasz Figa wrote:
>>> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
>>>
>>>
>>> On Tue, Aug 2, 2022 at 9:21 PM ayaka <[email protected]> wrote:
>>>>
>>>> Sorry, the previous one contains html data.
>>>>
>>>>> On Aug 2, 2022, at 3:33 PM, Tomasz Figa <[email protected]> wrote:
>>>>>
>>>>> On Mon, Aug 1, 2022 at 8:43 PM ayaka <[email protected]> wrote:
>>>>>> Sent from my iPad
>>>>>>>> On Aug 1, 2022, at 5:46 PM, Tomasz Figa <[email protected]> wrote:
>>>>>>> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
>>>>>>>> On Mon, Aug 1, 2022 at 3:44 PM Hsia-Jun Li <[email protected]> wrote:
>>>>>>>>> On 8/1/22 14:19, Tomasz Figa wrote:
>>>>>>>> Hello Tomasz
>>>>>>>>> ?Hi Randy,
>>>>>>>>> On Mon, Aug 1, 2022 at 5:21 AM <[email protected]> wrote:
>>>>>>>>>> From: Randy Li <[email protected]>
>>>>>>>>>> This module is still at a early stage, I wrote this for showing what
>>>>>>>>>> APIs we need here.
>>>>>>>>>> Let me explain why we need such a module here.
>>>>>>>>>> If you won't allocate buffers from a V4L2 M2M device, this module
>>>>>>>>>> may not be very useful. I am sure the most of users won't know a
>>>>>>>>>> device would require them allocate buffers from a DMA-Heap then
>>>>>>>>>> import those buffers into a V4L2's queue.
>>>>>>>>>> Then the question goes back to why DMA-Heap. From the Android's
>>>>>>>>>> description, we know it is about the copyright's DRM.
>>>>>>>>>> When we allocate a buffer in a DMA-Heap, it may register that buffer
>>>>>>>>>> in the trusted execution environment so the firmware which is running
>>>>>>>>>> or could only be acccesed from there could use that buffer later.
>>>>>>>>>> The answer above leads to another thing which is not done in this
>>>>>>>>>> version, the DMA mapping. Although in some platforms, a DMA-Heap
>>>>>>>>>> responses a IOMMU device as well. For the genernal purpose, we would
>>>>>>>>>> be better assuming the device mapping should be done for each device
>>>>>>>>>> itself. The problem here we only know alloc_devs in those DMAbuf
>>>>>>>>>> methods, which are DMA-heaps in my design, the device from the queue
>>>>>>>>>> is not enough, a plane may requests another IOMMU device or table
>>>>>>>>>> for mapping.
>>>>>>>>>> Signed-off-by: Randy Li <[email protected]>
>>>>>>>>>> ---
>>>>>>>>>> drivers/media/common/videobuf2/Kconfig | 6 +
>>>>>>>>>> drivers/media/common/videobuf2/Makefile | 1 +
>>>>>>>>>> .../common/videobuf2/videobuf2-dma-heap.c | 350 ++++++++++++++++++
>>>>>>>>>> include/media/videobuf2-dma-heap.h | 30 ++
>>>>>>>>>> 4 files changed, 387 insertions(+)
>>>>>>>>>> create mode 100644 drivers/media/common/videobuf2/videobuf2-dma-heap.c
>>>>>>>>>> create mode 100644 include/media/videobuf2-dma-heap.h
>>>>>>>>> First of all, thanks for the series.
>>>>>>>>> Possibly a stupid question, but why not just allocate the DMA-bufs
>>>>>>>>> directly from the DMA-buf heap device in the userspace and just import
>>>>>>>>> the buffers to the V4L2 device using V4L2_MEMORY_DMABUF?
>>>>>>>> Sometimes the allocation policy could be very complex, let's suppose a
>>>>>>>> multiple planes pixel format enabling with frame buffer compression.
>>>>>>>> Its luma, chroma data could be allocated from a pool which is delegated
>>>>>>>> for large buffers while its metadata would come from a pool which many
>>>>>>>> users could take some few slices from it(likes system pool).
>>>>>>>> Then when we have a new users knowing nothing about this platform, if we
>>>>>>>> just configure the alloc_devs in each queues well. The user won't need
>>>>>>>> to know those complex rules.
>>>>>>>> The real situation could be more complex, Samsung MFC's left and right
>>>>>>>> banks could be regarded as two pools, many devices would benefit from
>>>>>>>> this either from the allocation times or the security buffers policy.
>>>>>>>> In our design, when we need to do some security decoding(DRM video),
>>>>>>>> codecs2 would allocate buffers from the pool delegated for that. While
>>>>>>>> the non-DRM video, users could not care about this.
>>>>>>> I'm a little bit surprised about this, because on Android all the
>>>>>>> graphics buffers are allocated from the system IAllocator and imported
>>>>>>> to the specific devices.
>>>>>> In the non-tunnel mode, yes it is. While the tunnel mode is completely vendor defined. Neither HWC nor codec2 cares about where the buffers coming from, you could do what ever you want.
>>>>>> Besides there are DRM video in GNU Linux platform, I heard the webkit has made huge effort here and Playready is one could work in non-Android Linux.
>>>>>>> Would it make sense to instead extend the UAPI to expose enough
>>>>>>> information about the allocation requirements to the userspace, so it
>>>>>>> can allocate correctly?
>>>>>> Yes, it could. But as I said it would need the users to do more works.
>>>>>>> My reasoning here is that it's not a driver's decision to allocate
>>>>>>> from a DMA-buf heap (and which one) or not. It's the userspace which
>>>>>>> knows that, based on the specific use case that it wants to fulfill.
>>>>>> Although I would like to let the users decide that, users just can’t do that which would violate the security rules in some platforms.
>>>>>> For example, video codec and display device could only access a region of memory, any other device or trusted apps can’t access it. Users have to allocate the buffer from the pool the vendor decided.
>>>>>> So why not we offer a quick way that users don’t need to try and error.
>>>>>
>>>>> In principle, I'm not against integrating DMA-buf heap with vb2,
>>>>> however I see some problems I mentioned before:
>>>>>
>>>>> 1) How would the driver know if it should allocate from a DMA-buf heap or not?
>>>>
>>>> struct vb2_queue.mem_ops
>>>>
>>>> int (*queue_setup)(struct vb2_queue *q,unsigned int *num_buffers, unsigned int *num_planes, unsigned int sizes[], struct device *alloc_devs[]);
>>>
>>> Sorry, I don't understand what you mean here.
>>>
>>> Just to make sure we're on the same page - what I'm referring to is
>>> that whether DMA-buf heap is used or not is specific to a given use
>>> case, which is controlled by the userspace. So the userspace must be
>>> able to control whether the driver allocates from a DMA-buf heap or
>>> the regular way.
>> No, it does not depend on the use case here. We don't accept any buffers
>> beyond the region we decided. Even for the non-DRM, non-security video,
>> our codec devices are running under the secure mode.
>>
>> You MUST allocate the buffer for a device from the DMA-heap we(SYNA)
>> decided.
>
> That's your use case, but there could be use cases which work
> differently. In fact, in ChromeOS we only use the secure allocation
> path for protected content, because it imposes some overhead.
>
>>
>> I believe some other devices may have much limitation for not the
>> security reason, for example, it can't access the memory above 4 GiB or
>> for the performance's reason.
>
> For such limitations, there is the shared DMA pool or restricted DMA
> pool functionality, which can be given to a device in DT and then the
> DMA mapping API would just allocate within that area for that device.
> Maybe that's what you need here?
>
For Synaptics VideoSmart devices, it is simple that we want to limit the
memory region a IP device could access. I just try to find out some
reasons here.
>>>
>>>>
>>>>> 2) How would the driver know which heap to allocate from?
>>>>
>>>> From vb2_queue.alloc_devs
>>>>
>>>> What I did now is likes what MFC does, create some mem_alloc_devs.
>>>> It would be better that we could retrieve the DMA-heaps’ devices from kernel, but that is not enough, we need a place to store the heap flags although none of them are defined yet.
>>>>
>>>> From Android documents, I think it is unlikely we would have heap flags.
>>>> “Standardization: The DMA-BUF heaps framework offers a well-defined UAPI. ION allowed custom flags and heap IDs that prevented developing a common testing framework because each device’s ION implementation could behave differently.”
>>>>
>>>
>>> alloc_devs is something that the driver sets and it's a struct device
>>> for which the DMA API can be called to manage the DMA buffers for this
>>> video device. It's not a way to select a use case-dependent allocation
>>> method.
>>>
>> I see, then move to the last question, we need to expand the V4L2
>> framework's structure.
>>>>> 3) How would the heap know how to allocate properly for the device?
>>>>>
>>>> Because “each DMA-BUF heap is a separate character device”.
>>>
>>> Could you elaborate? Sorry, I'm not sure how this answers my question.
>> Because a DMA-heap, a heap device itself is enough here, may plus HEAP
>> flag when there is.
>>
>> I don't know what else would be need to do here.
>> If you allocate a buffer from a heap which is delegated for security
>> memory of that device, the heap driver itself would inform the TEE the
>> pages occupied by it or the memory allocated from the pool which is in a
>> region of memory reserved for this purpose.
>
> So the heap is only for the video device?
>
dma-heaps?
There are heaps for dolby audio, NPU and DRM IPs.
Even the GPU shader or AI model in SPIR-V could be protected in the
synaptics's platform.
>>>
>>>> But as I said in the first draft I am not sure about the DMA mapping part. alloc_devs responds for the heap, we have a device variable in the queue that mapping function could access, but that may not be enough. A plane may apply a different mapping policy or IOMMU here.
>>>>
>>>> Would it be better that I create a interface here that creating a memdev with DMA-heap description ?
>>>
>>> My intuition still tells me that it would be universally better to
>>> just let the userspace allocate the buffers independently (like with
>>> gralloc/Ion) and import to V4L2 using V4L2_MEM_DMABUF. It was possible
>>> to do things this way nicely with regular Android graphics buffers, so
>>> could you explain what difference of your use case makes it
>>> impossible?
>> Without keeping the backward compatibility, it won't have any problem IF
>> we could tell the users the acceptable DMA-heap for each of planes and
>> DMA-heap's heap flags.
>>
>> We don't have an ioctl for this yet, the most possible for the decoder
>> is doing that at GET_FMT ioctl()?.
>
> Do we need the kernel to tell the userspace which heap to use? As you
> mentioned above, the heap would be specific for the video device and
> the userspace would also be specific for your use case, so why
> couldn't it just find the right heap on its own (e.g. by name)?
Yes, as long as you won't mind me expanding the v4l2 allocator APIs.
As I would mention this in the synaptics pxiel formats' email, mvtp may
(in the most of case) use a different heap with the one luma and chroma
data.
>
> As for heap flags, could you elaborate on what kind of flags you
> imagine could be decided by a V4L2 driver?
I don't much idea here acutally, I don't even know any pretension need
here. But if I could expand the APIs, I would just leave a windows for
the furture usage.
>
> Best regards,
> Tomasz
--
Hsia-Jun(Randy) Li
Hi Randy,
Sorry for the late reply, I went on vacation last week.
On Sun, Aug 7, 2022 at 12:23 AM Hsia-Jun Li <[email protected]> wrote:
>
>
>
> On 8/5/22 18:09, Tomasz Figa wrote:
> > CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
> >
> >
> > On Tue, Aug 2, 2022 at 9:21 PM ayaka <[email protected]> wrote:
> >>
> >> Sorry, the previous one contains html data.
> >>
> >>> On Aug 2, 2022, at 3:33 PM, Tomasz Figa <[email protected]> wrote:
> >>>
> >>> On Mon, Aug 1, 2022 at 8:43 PM ayaka <[email protected]> wrote:
> >>>> Sent from my iPad
> >>>>>> On Aug 1, 2022, at 5:46 PM, Tomasz Figa <[email protected]> wrote:
> >>>>> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
> >>>>>> On Mon, Aug 1, 2022 at 3:44 PM Hsia-Jun Li <[email protected]> wrote:
> >>>>>>> On 8/1/22 14:19, Tomasz Figa wrote:
> >>>>>> Hello Tomasz
> >>>>>>> ?Hi Randy,
> >>>>>>> On Mon, Aug 1, 2022 at 5:21 AM <[email protected]> wrote:
> >>>>>>>> From: Randy Li <[email protected]>
> >>>>>>>> This module is still at a early stage, I wrote this for showing what
> >>>>>>>> APIs we need here.
> >>>>>>>> Let me explain why we need such a module here.
> >>>>>>>> If you won't allocate buffers from a V4L2 M2M device, this module
> >>>>>>>> may not be very useful. I am sure the most of users won't know a
> >>>>>>>> device would require them allocate buffers from a DMA-Heap then
> >>>>>>>> import those buffers into a V4L2's queue.
> >>>>>>>> Then the question goes back to why DMA-Heap. From the Android's
> >>>>>>>> description, we know it is about the copyright's DRM.
> >>>>>>>> When we allocate a buffer in a DMA-Heap, it may register that buffer
> >>>>>>>> in the trusted execution environment so the firmware which is running
> >>>>>>>> or could only be acccesed from there could use that buffer later.
> >>>>>>>> The answer above leads to another thing which is not done in this
> >>>>>>>> version, the DMA mapping. Although in some platforms, a DMA-Heap
> >>>>>>>> responses a IOMMU device as well. For the genernal purpose, we would
> >>>>>>>> be better assuming the device mapping should be done for each device
> >>>>>>>> itself. The problem here we only know alloc_devs in those DMAbuf
> >>>>>>>> methods, which are DMA-heaps in my design, the device from the queue
> >>>>>>>> is not enough, a plane may requests another IOMMU device or table
> >>>>>>>> for mapping.
> >>>>>>>> Signed-off-by: Randy Li <[email protected]>
> >>>>>>>> ---
> >>>>>>>> drivers/media/common/videobuf2/Kconfig | 6 +
> >>>>>>>> drivers/media/common/videobuf2/Makefile | 1 +
> >>>>>>>> .../common/videobuf2/videobuf2-dma-heap.c | 350 ++++++++++++++++++
> >>>>>>>> include/media/videobuf2-dma-heap.h | 30 ++
> >>>>>>>> 4 files changed, 387 insertions(+)
> >>>>>>>> create mode 100644 drivers/media/common/videobuf2/videobuf2-dma-heap.c
> >>>>>>>> create mode 100644 include/media/videobuf2-dma-heap.h
> >>>>>>> First of all, thanks for the series.
> >>>>>>> Possibly a stupid question, but why not just allocate the DMA-bufs
> >>>>>>> directly from the DMA-buf heap device in the userspace and just import
> >>>>>>> the buffers to the V4L2 device using V4L2_MEMORY_DMABUF?
> >>>>>> Sometimes the allocation policy could be very complex, let's suppose a
> >>>>>> multiple planes pixel format enabling with frame buffer compression.
> >>>>>> Its luma, chroma data could be allocated from a pool which is delegated
> >>>>>> for large buffers while its metadata would come from a pool which many
> >>>>>> users could take some few slices from it(likes system pool).
> >>>>>> Then when we have a new users knowing nothing about this platform, if we
> >>>>>> just configure the alloc_devs in each queues well. The user won't need
> >>>>>> to know those complex rules.
> >>>>>> The real situation could be more complex, Samsung MFC's left and right
> >>>>>> banks could be regarded as two pools, many devices would benefit from
> >>>>>> this either from the allocation times or the security buffers policy.
> >>>>>> In our design, when we need to do some security decoding(DRM video),
> >>>>>> codecs2 would allocate buffers from the pool delegated for that. While
> >>>>>> the non-DRM video, users could not care about this.
> >>>>> I'm a little bit surprised about this, because on Android all the
> >>>>> graphics buffers are allocated from the system IAllocator and imported
> >>>>> to the specific devices.
> >>>> In the non-tunnel mode, yes it is. While the tunnel mode is completely vendor defined. Neither HWC nor codec2 cares about where the buffers coming from, you could do what ever you want.
> >>>> Besides there are DRM video in GNU Linux platform, I heard the webkit has made huge effort here and Playready is one could work in non-Android Linux.
> >>>>> Would it make sense to instead extend the UAPI to expose enough
> >>>>> information about the allocation requirements to the userspace, so it
> >>>>> can allocate correctly?
> >>>> Yes, it could. But as I said it would need the users to do more works.
> >>>>> My reasoning here is that it's not a driver's decision to allocate
> >>>>> from a DMA-buf heap (and which one) or not. It's the userspace which
> >>>>> knows that, based on the specific use case that it wants to fulfill.
> >>>> Although I would like to let the users decide that, users just can’t do that which would violate the security rules in some platforms.
> >>>> For example, video codec and display device could only access a region of memory, any other device or trusted apps can’t access it. Users have to allocate the buffer from the pool the vendor decided.
> >>>> So why not we offer a quick way that users don’t need to try and error.
> >>>
> >>> In principle, I'm not against integrating DMA-buf heap with vb2,
> >>> however I see some problems I mentioned before:
> >>>
> >>> 1) How would the driver know if it should allocate from a DMA-buf heap or not?
> >>
> >> struct vb2_queue.mem_ops
> >>
> >> int (*queue_setup)(struct vb2_queue *q,unsigned int *num_buffers, unsigned int *num_planes, unsigned int sizes[], struct device *alloc_devs[]);
> >
> > Sorry, I don't understand what you mean here.
> >
> > Just to make sure we're on the same page - what I'm referring to is
> > that whether DMA-buf heap is used or not is specific to a given use
> > case, which is controlled by the userspace. So the userspace must be
> > able to control whether the driver allocates from a DMA-buf heap or
> > the regular way.
> No, it does not depend on the use case here. We don't accept any buffers
> beyond the region we decided. Even for the non-DRM, non-security video,
> our codec devices are running under the secure mode.
>
> You MUST allocate the buffer for a device from the DMA-heap we(SYNA)
> decided.
That's your use case, but there could be use cases which work
differently. In fact, in ChromeOS we only use the secure allocation
path for protected content, because it imposes some overhead.
>
> I believe some other devices may have much limitation for not the
> security reason, for example, it can't access the memory above 4 GiB or
> for the performance's reason.
For such limitations, there is the shared DMA pool or restricted DMA
pool functionality, which can be given to a device in DT and then the
DMA mapping API would just allocate within that area for that device.
Maybe that's what you need here?
> >
> >>
> >>> 2) How would the driver know which heap to allocate from?
> >>
> >> From vb2_queue.alloc_devs
> >>
> >> What I did now is likes what MFC does, create some mem_alloc_devs.
> >> It would be better that we could retrieve the DMA-heaps’ devices from kernel, but that is not enough, we need a place to store the heap flags although none of them are defined yet.
> >>
> >> From Android documents, I think it is unlikely we would have heap flags.
> >> “Standardization: The DMA-BUF heaps framework offers a well-defined UAPI. ION allowed custom flags and heap IDs that prevented developing a common testing framework because each device’s ION implementation could behave differently.”
> >>
> >
> > alloc_devs is something that the driver sets and it's a struct device
> > for which the DMA API can be called to manage the DMA buffers for this
> > video device. It's not a way to select a use case-dependent allocation
> > method.
> >
> I see, then move to the last question, we need to expand the V4L2
> framework's structure.
> >>> 3) How would the heap know how to allocate properly for the device?
> >>>
> >> Because “each DMA-BUF heap is a separate character device”.
> >
> > Could you elaborate? Sorry, I'm not sure how this answers my question.
> Because a DMA-heap, a heap device itself is enough here, may plus HEAP
> flag when there is.
>
> I don't know what else would be need to do here.
> If you allocate a buffer from a heap which is delegated for security
> memory of that device, the heap driver itself would inform the TEE the
> pages occupied by it or the memory allocated from the pool which is in a
> region of memory reserved for this purpose.
So the heap is only for the video device?
> >
> >> But as I said in the first draft I am not sure about the DMA mapping part. alloc_devs responds for the heap, we have a device variable in the queue that mapping function could access, but that may not be enough. A plane may apply a different mapping policy or IOMMU here.
> >>
> >> Would it be better that I create a interface here that creating a memdev with DMA-heap description ?
> >
> > My intuition still tells me that it would be universally better to
> > just let the userspace allocate the buffers independently (like with
> > gralloc/Ion) and import to V4L2 using V4L2_MEM_DMABUF. It was possible
> > to do things this way nicely with regular Android graphics buffers, so
> > could you explain what difference of your use case makes it
> > impossible?
> Without keeping the backward compatibility, it won't have any problem IF
> we could tell the users the acceptable DMA-heap for each of planes and
> DMA-heap's heap flags.
>
> We don't have an ioctl for this yet, the most possible for the decoder
> is doing that at GET_FMT ioctl()?.
Do we need the kernel to tell the userspace which heap to use? As you
mentioned above, the heap would be specific for the video device and
the userspace would also be specific for your use case, so why
couldn't it just find the right heap on its own (e.g. by name)?
As for heap flags, could you elaborate on what kind of flags you
imagine could be decided by a V4L2 driver?
Best regards,
Tomasz