[3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
Commit Message
Signed-off-by: Federico Vaga <federico.vaga@gmail.com>
---
drivers/media/v4l2-core/Kconfig | 5 +
drivers/media/v4l2-core/Makefile | 1 +
drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 ++++++++++++++++++++++
include/media/videobuf2-dma-streaming.h | 24 +++
4 file modificati, 235 inserzioni(+)
create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c
create mode 100644 include/media/videobuf2-dma-streaming.h
Comments
Hello,
On Thursday, September 13, 2012 3:53 PM Federico Vaga wrote:
> Signed-off-by: Federico Vaga <federico.vaga@gmail.com>
A few words explaining why this memory handling module is required or beneficial will
definitely improve the commit :)
> ---
> drivers/media/v4l2-core/Kconfig | 5 +
> drivers/media/v4l2-core/Makefile | 1 +
> drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 ++++++++++++++++++++++
> include/media/videobuf2-dma-streaming.h | 24 +++
> 4 file modificati, 235 inserzioni(+)
> create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c
> create mode 100644 include/media/videobuf2-dma-streaming.h
>
> diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
> index 0c54e19..60548a7 100644
> --- a/drivers/media/v4l2-core/Kconfig
> +++ b/drivers/media/v4l2-core/Kconfig
> @@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG
> #depends on HAS_DMA
> select VIDEOBUF2_CORE
> select VIDEOBUF2_MEMOPS
> +
> +config VIDEOBUF2_DMA_STREAMING
> + select VIDEOBUF2_CORE
> + select VIDEOBUF2_MEMOPS
> + tristate
> diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
> index c2d61d4..0b2756f 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
> obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o
> obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
> obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
> +obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o
>
> ccflags-y += -I$(srctree)/drivers/media/dvb-core
> ccflags-y += -I$(srctree)/drivers/media/dvb-frontends
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-streaming.c b/drivers/media/v4l2-
> core/videobuf2-dma-streaming.c
> new file mode 100644
> index 0000000..23475a6
> --- /dev/null
> +++ b/drivers/media/v4l2-core/videobuf2-dma-streaming.c
> @@ -0,0 +1,205 @@
> +/*
> + * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2
> + *
> + * Copyright (C) 2012 Federico Vaga <federico.vaga@gmail.com>
> + * *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/pagemap.h>
> +#include <linux/dma-mapping.h>
> +
> +#include <media/videobuf2-core.h>
> +#include <media/videobuf2-dma-streaming.h>
> +#include <media/videobuf2-memops.h>
> +
> +struct vb2_streaming_conf {
> + struct device *dev;
> +};
> +struct vb2_streaming_buf {
> + struct vb2_streaming_conf *conf;
> + void *vaddr;
> +
> + dma_addr_t dma_handle;
> +
> + unsigned long size;
> + struct vm_area_struct *vma;
> +
> + atomic_t refcount;
> + struct vb2_vmarea_handler handler;
> +};
> +
> +static void vb2_dma_streaming_put(void *buf_priv)
> +{
> + struct vb2_streaming_buf *buf = buf_priv;
> +
> + if (atomic_dec_and_test(&buf->refcount)) {
> + dma_unmap_single(buf->conf->dev, buf->dma_handle, buf->size,
> + DMA_FROM_DEVICE);
> + free_pages_exact(buf->vaddr, buf->size);
> + kfree(buf);
> + }
> +
> +}
> +
> +static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size)
> +{
> + struct vb2_streaming_conf *conf = alloc_ctx;
> + struct vb2_streaming_buf *buf;
> + int err;
> +
> + buf = kzalloc(sizeof *buf, GFP_KERNEL);
> + if (!buf)
> + return ERR_PTR(-ENOMEM);
> + buf->vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA);
> + if (!buf->vaddr) {
> + err = -ENOMEM;
> + goto out;
> + }
> + buf->dma_handle = dma_map_single(conf->dev, buf->vaddr, size,
> + DMA_FROM_DEVICE);
> + err = dma_mapping_error(conf->dev, buf->dma_handle);
> + if (err) {
> + dev_err(conf->dev, "dma_map_single failed\n");
> +
> + free_pages_exact(buf->vaddr, size);
> + buf->vaddr = NULL;
> + goto out_pages;
> + }
> + buf->conf = conf;
> + buf->size = size;
> + buf->handler.refcount = &buf->refcount;
> + buf->handler.put = vb2_dma_streaming_put;
> + buf->handler.arg = buf;
> +
> + atomic_inc(&buf->refcount);
> + return buf;
> +
> +out_pages:
> + free_pages_exact(buf->vaddr, buf->size);
> +out:
> + kfree(buf);
> + return ERR_PTR(err);
> +}
> +
> +static void *vb2_dma_streaming_cookie(void *buf_priv)
> +{
> + struct vb2_streaming_buf *buf = buf_priv;
> +
> + return (void *)buf->dma_handle;
> +}
Please change this function to:
static void *vb2_dma_streaming_cookie(void *buf_priv)
{
struct vb2_streaming_buf *buf = buf_priv;
return &buf->dma_handle;
}
and add a following static inline to include/media/videobuf2-dma-streaming.h:
static inline dma_addr_t
vb2_dma_streaming_plane_paddr(struct vb2_buffer *vb, unsigned int plane_no)
{
dma_addr_t *dma_addr = vb2_plane_cookie(vb, plane_no);
return *dma_addr;
}
Do not use 'cookie' callback directly in the driver, the driver should use
the above proxy.
The &buf->dma_handle workaround is required for some possible configurations
with 64bit dma addresses, see commit 472af2b05bdefc.
> +
> +static void *vb2_dma_streaming_vaddr(void *buf_priv)
> +{
> + struct vb2_streaming_buf *buf = buf_priv;
> +
> + if (!buf)
> + return NULL;
> + return buf->vaddr;
> +}
> +
> +static unsigned int vb2_dma_streaming_num_users(void *buf_priv)
> +{
> + struct vb2_streaming_buf *buf = buf_priv;
> +
> + return atomic_read(&buf->refcount);
> +}
> +
> +static int vb2_dma_streaming_mmap(void *buf_priv, struct vm_area_struct *vma)
> +{
> + struct vb2_streaming_buf *buf = buf_priv;
> + unsigned long pos, start = vma->vm_start;
> + unsigned long size;
> + struct page *page;
> + int err;
> +
> + /* Try to remap memory */
> + size = vma->vm_end - vma->vm_start;
> + size = (size < buf->size) ? size : buf->size;
> + pos = (unsigned long)buf->vaddr;
> +
> + while (size > 0) {
> + page = virt_to_page((void *)pos);
> + if (!page) {
> + dev_err(buf->conf->dev, "mmap: virt_to_page failed\n");
> + return -ENOMEM;
> + }
> + err = vm_insert_page(vma, start, page);
> + if (err) {
> + dev_err(buf->conf->dev, "mmap: insert failed %d\n", err);
> + return -ENOMEM;
> + }
> + start += PAGE_SIZE;
> + pos += PAGE_SIZE;
> +
> + if (size > PAGE_SIZE)
> + size -= PAGE_SIZE;
> + else
> + size = 0;
> + }
> +
> +
> + vma->vm_ops = &vb2_common_vm_ops;
> + vma->vm_flags |= VM_DONTEXPAND;
> + vma->vm_private_data = &buf->handler;
> +
> + vma->vm_ops->open(vma);
> +
> + return 0;
> +}
> +
> +static void vb2_dma_streaming_prepare(void *buf_priv)
> +{
> + struct vb2_streaming_buf *buf = buf_priv;
> +
> + dma_sync_single_for_device(buf->conf->dev, buf->dma_handle,
> + buf->size, DMA_FROM_DEVICE);
> +}
> +
> +static void vb2_dma_streaming_finish(void *buf_priv)
> +{
> + struct vb2_streaming_buf *buf = buf_priv;
> +
> + dma_sync_single_for_cpu(buf->conf->dev, buf->dma_handle,
> + buf->size, DMA_FROM_DEVICE);
> +}
> +
> +const struct vb2_mem_ops vb2_dma_streaming_memops = {
> + .alloc = vb2_dma_streaming_alloc,
> + .put = vb2_dma_streaming_put,
> + .cookie = vb2_dma_streaming_cookie,
> + .vaddr = vb2_dma_streaming_vaddr,
> + .mmap = vb2_dma_streaming_mmap,
> + .num_users = vb2_dma_streaming_num_users,
> + .prepare = vb2_dma_streaming_prepare,
> + .finish = vb2_dma_streaming_finish,
> +};
> +EXPORT_SYMBOL_GPL(vb2_dma_streaming_memops);
> +
> +void *vb2_dma_streaming_init_ctx(struct device *dev)
> +{
> + struct vb2_streaming_conf *conf;
> +
> + conf = kmalloc(sizeof *conf, GFP_KERNEL);
> + if (!conf)
> + return ERR_PTR(-ENOMEM);
> +
> + conf->dev = dev;
> +
> + return conf;
> +}
> +EXPORT_SYMBOL_GPL(vb2_dma_streaming_init_ctx);
> +
> +void vb2_dma_streaming_cleanup_ctx(void *alloc_ctx)
> +{
> + kfree(alloc_ctx);
> +}
> +EXPORT_SYMBOL_GPL(vb2_dma_streaming_cleanup_ctx);
> +
> +MODULE_DESCRIPTION("DMA-streaming memory allocator for videobuf2");
> +MODULE_AUTHOR("Federico Vaga <federico.vaga@gmail.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/media/videobuf2-dma-streaming.h b/include/media/videobuf2-dma-streaming.h
> new file mode 100644
> index 0000000..89cbd06
> --- /dev/null
> +++ b/include/media/videobuf2-dma-streaming.h
> @@ -0,0 +1,24 @@
> +/*
> + * videobuf2-dma-streaming.h - DMA steaming memory allocator for videobuf2
> + *
> + * Copyright (C) 2012 Federico Vaga
> + *
> + * Author: Federico Vaga <federico.vaga@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation.
> + */
> +
> +#ifndef _MEDIA_VIDEOBUF2_DMA_STREAMING_H
> +#define _MEDIA_VIDEOBUF2_DMA_STREAMING_H
> +
> +#include <media/videobuf2-core.h>
> +#include <linux/dma-mapping.h>
> +
> +void *vb2_dma_streaming_init_ctx(struct device *dev);
> +void vb2_dma_streaming_cleanup_ctx(void *alloc_ctx);
> +
> +extern const struct vb2_mem_ops vb2_dma_streaming_memops;
> +
> +#endif
> --
> 1.7.11.4
Best regards
On Thu 13 September 2012 15:52:47 Federico Vaga wrote:
> Signed-off-by: Federico Vaga <federico.vaga@gmail.com>
> ---
> drivers/media/v4l2-core/Kconfig | 5 +
> drivers/media/v4l2-core/Makefile | 1 +
> drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 ++++++++++++++++++++++
> include/media/videobuf2-dma-streaming.h | 24 +++
> 4 file modificati, 235 inserzioni(+)
> create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c
> create mode 100644 include/media/videobuf2-dma-streaming.h
>
> diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
> index 0c54e19..60548a7 100644
> --- a/drivers/media/v4l2-core/Kconfig
> +++ b/drivers/media/v4l2-core/Kconfig
> @@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG
> #depends on HAS_DMA
> select VIDEOBUF2_CORE
> select VIDEOBUF2_MEMOPS
> +
> +config VIDEOBUF2_DMA_STREAMING
> + select VIDEOBUF2_CORE
> + select VIDEOBUF2_MEMOPS
> + tristate
> diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
> index c2d61d4..0b2756f 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
> obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o
> obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
> obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
> +obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o
>
> ccflags-y += -I$(srctree)/drivers/media/dvb-core
> ccflags-y += -I$(srctree)/drivers/media/dvb-frontends
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-streaming.c b/drivers/media/v4l2-core/videobuf2-dma-streaming.c
> new file mode 100644
> index 0000000..23475a6
> --- /dev/null
> +++ b/drivers/media/v4l2-core/videobuf2-dma-streaming.c
> @@ -0,0 +1,205 @@
> +/*
> + * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2
> + *
> + * Copyright (C) 2012 Federico Vaga <federico.vaga@gmail.com>
> + * *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/pagemap.h>
> +#include <linux/dma-mapping.h>
> +
> +#include <media/videobuf2-core.h>
> +#include <media/videobuf2-dma-streaming.h>
> +#include <media/videobuf2-memops.h>
> +
> +struct vb2_streaming_conf {
> + struct device *dev;
> +};
> +struct vb2_streaming_buf {
> + struct vb2_streaming_conf *conf;
> + void *vaddr;
> +
> + dma_addr_t dma_handle;
> +
> + unsigned long size;
> + struct vm_area_struct *vma;
> +
> + atomic_t refcount;
> + struct vb2_vmarea_handler handler;
> +};
> +
> +static void vb2_dma_streaming_put(void *buf_priv)
> +{
> + struct vb2_streaming_buf *buf = buf_priv;
> +
> + if (atomic_dec_and_test(&buf->refcount)) {
> + dma_unmap_single(buf->conf->dev, buf->dma_handle, buf->size,
> + DMA_FROM_DEVICE);
> + free_pages_exact(buf->vaddr, buf->size);
> + kfree(buf);
> + }
> +
> +}
> +
> +static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size)
> +{
> + struct vb2_streaming_conf *conf = alloc_ctx;
> + struct vb2_streaming_buf *buf;
> + int err;
> +
> + buf = kzalloc(sizeof *buf, GFP_KERNEL);
> + if (!buf)
> + return ERR_PTR(-ENOMEM);
> + buf->vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA);
> + if (!buf->vaddr) {
> + err = -ENOMEM;
> + goto out;
> + }
> + buf->dma_handle = dma_map_single(conf->dev, buf->vaddr, size,
> + DMA_FROM_DEVICE);
> + err = dma_mapping_error(conf->dev, buf->dma_handle);
> + if (err) {
> + dev_err(conf->dev, "dma_map_single failed\n");
> +
> + free_pages_exact(buf->vaddr, size);
> + buf->vaddr = NULL;
> + goto out_pages;
> + }
> + buf->conf = conf;
> + buf->size = size;
> + buf->handler.refcount = &buf->refcount;
> + buf->handler.put = vb2_dma_streaming_put;
> + buf->handler.arg = buf;
> +
> + atomic_inc(&buf->refcount);
> + return buf;
> +
> +out_pages:
> + free_pages_exact(buf->vaddr, buf->size);
> +out:
> + kfree(buf);
> + return ERR_PTR(err);
> +}
> +
> +static void *vb2_dma_streaming_cookie(void *buf_priv)
> +{
> + struct vb2_streaming_buf *buf = buf_priv;
> +
> + return (void *)buf->dma_handle;
> +}
> +
> +static void *vb2_dma_streaming_vaddr(void *buf_priv)
> +{
> + struct vb2_streaming_buf *buf = buf_priv;
> +
> + if (!buf)
> + return NULL;
> + return buf->vaddr;
> +}
> +
> +static unsigned int vb2_dma_streaming_num_users(void *buf_priv)
> +{
> + struct vb2_streaming_buf *buf = buf_priv;
> +
> + return atomic_read(&buf->refcount);
> +}
> +
> +static int vb2_dma_streaming_mmap(void *buf_priv, struct vm_area_struct *vma)
> +{
> + struct vb2_streaming_buf *buf = buf_priv;
> + unsigned long pos, start = vma->vm_start;
> + unsigned long size;
> + struct page *page;
> + int err;
> +
> + /* Try to remap memory */
> + size = vma->vm_end - vma->vm_start;
> + size = (size < buf->size) ? size : buf->size;
> + pos = (unsigned long)buf->vaddr;
> +
> + while (size > 0) {
> + page = virt_to_page((void *)pos);
> + if (!page) {
> + dev_err(buf->conf->dev, "mmap: virt_to_page failed\n");
> + return -ENOMEM;
> + }
> + err = vm_insert_page(vma, start, page);
> + if (err) {
> + dev_err(buf->conf->dev, "mmap: insert failed %d\n", err);
> + return -ENOMEM;
> + }
> + start += PAGE_SIZE;
> + pos += PAGE_SIZE;
> +
> + if (size > PAGE_SIZE)
> + size -= PAGE_SIZE;
> + else
> + size = 0;
> + }
> +
> +
> + vma->vm_ops = &vb2_common_vm_ops;
> + vma->vm_flags |= VM_DONTEXPAND;
> + vma->vm_private_data = &buf->handler;
> +
> + vma->vm_ops->open(vma);
> +
> + return 0;
> +}
> +
> +static void vb2_dma_streaming_prepare(void *buf_priv)
> +{
> + struct vb2_streaming_buf *buf = buf_priv;
> +
> + dma_sync_single_for_device(buf->conf->dev, buf->dma_handle,
> + buf->size, DMA_FROM_DEVICE);
> +}
> +
> +static void vb2_dma_streaming_finish(void *buf_priv)
> +{
> + struct vb2_streaming_buf *buf = buf_priv;
> +
> + dma_sync_single_for_cpu(buf->conf->dev, buf->dma_handle,
> + buf->size, DMA_FROM_DEVICE);
> +}
> +
> +const struct vb2_mem_ops vb2_dma_streaming_memops = {
> + .alloc = vb2_dma_streaming_alloc,
> + .put = vb2_dma_streaming_put,
> + .cookie = vb2_dma_streaming_cookie,
> + .vaddr = vb2_dma_streaming_vaddr,
> + .mmap = vb2_dma_streaming_mmap,
> + .num_users = vb2_dma_streaming_num_users,
> + .prepare = vb2_dma_streaming_prepare,
> + .finish = vb2_dma_streaming_finish,
> +};
> +EXPORT_SYMBOL_GPL(vb2_dma_streaming_memops);
> +
> +void *vb2_dma_streaming_init_ctx(struct device *dev)
> +{
> + struct vb2_streaming_conf *conf;
> +
> + conf = kmalloc(sizeof *conf, GFP_KERNEL);
> + if (!conf)
> + return ERR_PTR(-ENOMEM);
> +
> + conf->dev = dev;
> +
> + return conf;
> +}
> +EXPORT_SYMBOL_GPL(vb2_dma_streaming_init_ctx);
> +
> +void vb2_dma_streaming_cleanup_ctx(void *alloc_ctx)
> +{
> + kfree(alloc_ctx);
> +}
> +EXPORT_SYMBOL_GPL(vb2_dma_streaming_cleanup_ctx);
> +
> +MODULE_DESCRIPTION("DMA-streaming memory allocator for videobuf2");
> +MODULE_AUTHOR("Federico Vaga <federico.vaga@gmail.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/media/videobuf2-dma-streaming.h b/include/media/videobuf2-dma-streaming.h
> new file mode 100644
> index 0000000..89cbd06
> --- /dev/null
> +++ b/include/media/videobuf2-dma-streaming.h
> @@ -0,0 +1,24 @@
> +/*
> + * videobuf2-dma-streaming.h - DMA steaming memory allocator for videobuf2
typo: steaming -> streaming :-)
The header and esp. the source could really do with more documentation. It is
not at all clear from the code what the dma-streaming allocator does and how
it differs from other allocators.
Regards,
Hans
> + *
> + * Copyright (C) 2012 Federico Vaga
> + *
> + * Author: Federico Vaga <federico.vaga@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation.
> + */
> +
> +#ifndef _MEDIA_VIDEOBUF2_DMA_STREAMING_H
> +#define _MEDIA_VIDEOBUF2_DMA_STREAMING_H
> +
> +#include <media/videobuf2-core.h>
> +#include <linux/dma-mapping.h>
> +
> +void *vb2_dma_streaming_init_ctx(struct device *dev);
> +void vb2_dma_streaming_cleanup_ctx(void *alloc_ctx);
> +
> +extern const struct vb2_mem_ops vb2_dma_streaming_memops;
> +
> +#endif
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
> typo: steaming -> streaming :-)
fixed
> The header and esp. the source could really do with more
> documentation. It is not at all clear from the code what the
> dma-streaming allocator does and how it differs from other
> allocators.
The other allocators are not documented and to understand them I read
the code. All the memory allocators reflect a kind of DMA interface: SG
for scatter/gather, contig for choerent and (now) streaming for
streaming. So, I'm not sure to understand what do you think is the
correct way to document a memory allocator; I can write one or two lines
of comment to summarize each function. what do you think?
> On Thursday, September 13, 2012 3:53 PM Federico Vaga wrote:
> > Signed-off-by: Federico Vaga <federico.vaga@gmail.com>
>
> A few words explaining why this memory handling module is required or
> beneficial will definitely improve the commit :)
ok, I will write some lines
> > +static void *vb2_dma_streaming_cookie(void *buf_priv)
> > +{
> > + struct vb2_streaming_buf *buf = buf_priv;
> > +
> > + return (void *)buf->dma_handle;
> > +}
>
> Please change this function to:
>
> static void *vb2_dma_streaming_cookie(void *buf_priv)
> {
> struct vb2_streaming_buf *buf = buf_priv;
> return &buf->dma_handle;
> }
>
> and add a following static inline to
> include/media/videobuf2-dma-streaming.h:
>
> static inline dma_addr_t
> vb2_dma_streaming_plane_paddr(struct vb2_buffer *vb, unsigned int
> plane_no) {
> dma_addr_t *dma_addr = vb2_plane_cookie(vb, plane_no);
> return *dma_addr;
> }
>
> Do not use 'cookie' callback directly in the driver, the driver should
> use the above proxy.
>
> The &buf->dma_handle workaround is required for some possible
> configurations with 64bit dma addresses, see commit 472af2b05bdefc.
ACK.
On Thu, 13 Sep 2012 17:46:32 +0200
Federico Vaga <federico.vaga@gmail.com> wrote:
> > A few words explaining why this memory handling module is required or
> > beneficial will definitely improve the commit :)
>
> ok, I will write some lines
In general, all of these patches need *much* better changelogs (i.e. they
need changelogs). What are you doing, and *why* are you doing it? The
future will want to know.
I'm going to try to look at the actual code, but time isn't something I
have a lot of, still...
Thanks,
jon
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 13 Sep 2012 17:42:33 +0200
Federico Vaga <federico.vaga@gmail.com> wrote:
> > The header and esp. the source could really do with more
> > documentation. It is not at all clear from the code what the
> > dma-streaming allocator does and how it differs from other
> > allocators.
>
> The other allocators are not documented and to understand them I read
> the code. All the memory allocators reflect a kind of DMA interface: SG
> for scatter/gather, contig for choerent and (now) streaming for
> streaming. So, I'm not sure to understand what do you think is the
> correct way to document a memory allocator; I can write one or two lines
> of comment to summarize each function. what do you think?
Well, there is some documentation here:
https://lwn.net/Articles/447435/
This reminds me that I've always meant to turn it into something to put
into Documentation/. I'll try to get to that soon.
In general, the fact that a subsystem is insufficiently documented is not
a good reason to add more poorly-documented code. We are, after all,
trying to make the kernel better over time...
Thanks,
jon
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
> Well, there is some documentation here:
>
> https://lwn.net/Articles/447435/
I know this, I learned from this page :)
What I'm saying is that I don't know what to write inside the code to
make it clearer than now. I think is clear, because if you know the
videobuf2, you know what I'm doing in each vb2_mem_ops. I suppose that
this is the reason why there are no comments inside the other memory
allocator. Maybe I am wrong.
In data giovedì 13 settembre 2012 11:45:31, Jonathan Corbet ha scritto:
> On Thu, 13 Sep 2012 17:46:32 +0200
>
> Federico Vaga <federico.vaga@gmail.com> wrote:
> > > A few words explaining why this memory handling module is required
> > > or
> > > beneficial will definitely improve the commit :)
> >
> > ok, I will write some lines
>
> In general, all of these patches need *much* better changelogs (i.e.
> they need changelogs). What are you doing, and *why* are you doing
> it? The future will want to know.
I can improve the comment about the ADV7180: it is not clear why I'm
removing that functions. (and the memory allocator commit is also in the
todo list).
The STA2X11_VIP commit, I think is clear, I convert it to use videobu2
and control framework. I can add some extra lines to explain why:
because videobuf is obsolete
> I'm going to try to look at the actual code, but time isn't something
> I have a lot of, still...
The actual code is the same of the previous one with yours (plural)
suggestions from the RFC submission (few week ago). I did not write
anything else.
Hi Jon,
On Thu, Sep 13, 2012 at 2:54 PM, Jonathan Corbet <corbet@lwn.net> wrote:
>
> Well, there is some documentation here:
>
> https://lwn.net/Articles/447435/
>
I thank you for this. It really helped me getting started on videobuf2.
> This reminds me that I've always meant to turn it into something to put
> into Documentation/. I'll try to get to that soon.
>
Yes, that's something many of us will appreciate A LOT.
Perhaps we could start with some skeleton documentation and then build
from there.
I maybe naive, but I think that way other developers might add little
contributions.
Regards,
Ezequiel.
PS: I wish I had some time to do it myself.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
@@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG
#depends on HAS_DMA
select VIDEOBUF2_CORE
select VIDEOBUF2_MEMOPS
+
+config VIDEOBUF2_DMA_STREAMING
+ select VIDEOBUF2_CORE
+ select VIDEOBUF2_MEMOPS
+ tristate
@@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o
obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
+obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o
ccflags-y += -I$(srctree)/drivers/media/dvb-core
ccflags-y += -I$(srctree)/drivers/media/dvb-frontends
new file mode 100644
@@ -0,0 +1,205 @@
+/*
+ * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2
+ *
+ * Copyright (C) 2012 Federico Vaga <federico.vaga@gmail.com>
+ * *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/pagemap.h>
+#include <linux/dma-mapping.h>
+
+#include <media/videobuf2-core.h>
+#include <media/videobuf2-dma-streaming.h>
+#include <media/videobuf2-memops.h>
+
+struct vb2_streaming_conf {
+ struct device *dev;
+};
+struct vb2_streaming_buf {
+ struct vb2_streaming_conf *conf;
+ void *vaddr;
+
+ dma_addr_t dma_handle;
+
+ unsigned long size;
+ struct vm_area_struct *vma;
+
+ atomic_t refcount;
+ struct vb2_vmarea_handler handler;
+};
+
+static void vb2_dma_streaming_put(void *buf_priv)
+{
+ struct vb2_streaming_buf *buf = buf_priv;
+
+ if (atomic_dec_and_test(&buf->refcount)) {
+ dma_unmap_single(buf->conf->dev, buf->dma_handle, buf->size,
+ DMA_FROM_DEVICE);
+ free_pages_exact(buf->vaddr, buf->size);
+ kfree(buf);
+ }
+
+}
+
+static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size)
+{
+ struct vb2_streaming_conf *conf = alloc_ctx;
+ struct vb2_streaming_buf *buf;
+ int err;
+
+ buf = kzalloc(sizeof *buf, GFP_KERNEL);
+ if (!buf)
+ return ERR_PTR(-ENOMEM);
+ buf->vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA);
+ if (!buf->vaddr) {
+ err = -ENOMEM;
+ goto out;
+ }
+ buf->dma_handle = dma_map_single(conf->dev, buf->vaddr, size,
+ DMA_FROM_DEVICE);
+ err = dma_mapping_error(conf->dev, buf->dma_handle);
+ if (err) {
+ dev_err(conf->dev, "dma_map_single failed\n");
+
+ free_pages_exact(buf->vaddr, size);
+ buf->vaddr = NULL;
+ goto out_pages;
+ }
+ buf->conf = conf;
+ buf->size = size;
+ buf->handler.refcount = &buf->refcount;
+ buf->handler.put = vb2_dma_streaming_put;
+ buf->handler.arg = buf;
+
+ atomic_inc(&buf->refcount);
+ return buf;
+
+out_pages:
+ free_pages_exact(buf->vaddr, buf->size);
+out:
+ kfree(buf);
+ return ERR_PTR(err);
+}
+
+static void *vb2_dma_streaming_cookie(void *buf_priv)
+{
+ struct vb2_streaming_buf *buf = buf_priv;
+
+ return (void *)buf->dma_handle;
+}
+
+static void *vb2_dma_streaming_vaddr(void *buf_priv)
+{
+ struct vb2_streaming_buf *buf = buf_priv;
+
+ if (!buf)
+ return NULL;
+ return buf->vaddr;
+}
+
+static unsigned int vb2_dma_streaming_num_users(void *buf_priv)
+{
+ struct vb2_streaming_buf *buf = buf_priv;
+
+ return atomic_read(&buf->refcount);
+}
+
+static int vb2_dma_streaming_mmap(void *buf_priv, struct vm_area_struct *vma)
+{
+ struct vb2_streaming_buf *buf = buf_priv;
+ unsigned long pos, start = vma->vm_start;
+ unsigned long size;
+ struct page *page;
+ int err;
+
+ /* Try to remap memory */
+ size = vma->vm_end - vma->vm_start;
+ size = (size < buf->size) ? size : buf->size;
+ pos = (unsigned long)buf->vaddr;
+
+ while (size > 0) {
+ page = virt_to_page((void *)pos);
+ if (!page) {
+ dev_err(buf->conf->dev, "mmap: virt_to_page failed\n");
+ return -ENOMEM;
+ }
+ err = vm_insert_page(vma, start, page);
+ if (err) {
+ dev_err(buf->conf->dev, "mmap: insert failed %d\n", err);
+ return -ENOMEM;
+ }
+ start += PAGE_SIZE;
+ pos += PAGE_SIZE;
+
+ if (size > PAGE_SIZE)
+ size -= PAGE_SIZE;
+ else
+ size = 0;
+ }
+
+
+ vma->vm_ops = &vb2_common_vm_ops;
+ vma->vm_flags |= VM_DONTEXPAND;
+ vma->vm_private_data = &buf->handler;
+
+ vma->vm_ops->open(vma);
+
+ return 0;
+}
+
+static void vb2_dma_streaming_prepare(void *buf_priv)
+{
+ struct vb2_streaming_buf *buf = buf_priv;
+
+ dma_sync_single_for_device(buf->conf->dev, buf->dma_handle,
+ buf->size, DMA_FROM_DEVICE);
+}
+
+static void vb2_dma_streaming_finish(void *buf_priv)
+{
+ struct vb2_streaming_buf *buf = buf_priv;
+
+ dma_sync_single_for_cpu(buf->conf->dev, buf->dma_handle,
+ buf->size, DMA_FROM_DEVICE);
+}
+
+const struct vb2_mem_ops vb2_dma_streaming_memops = {
+ .alloc = vb2_dma_streaming_alloc,
+ .put = vb2_dma_streaming_put,
+ .cookie = vb2_dma_streaming_cookie,
+ .vaddr = vb2_dma_streaming_vaddr,
+ .mmap = vb2_dma_streaming_mmap,
+ .num_users = vb2_dma_streaming_num_users,
+ .prepare = vb2_dma_streaming_prepare,
+ .finish = vb2_dma_streaming_finish,
+};
+EXPORT_SYMBOL_GPL(vb2_dma_streaming_memops);
+
+void *vb2_dma_streaming_init_ctx(struct device *dev)
+{
+ struct vb2_streaming_conf *conf;
+
+ conf = kmalloc(sizeof *conf, GFP_KERNEL);
+ if (!conf)
+ return ERR_PTR(-ENOMEM);
+
+ conf->dev = dev;
+
+ return conf;
+}
+EXPORT_SYMBOL_GPL(vb2_dma_streaming_init_ctx);
+
+void vb2_dma_streaming_cleanup_ctx(void *alloc_ctx)
+{
+ kfree(alloc_ctx);
+}
+EXPORT_SYMBOL_GPL(vb2_dma_streaming_cleanup_ctx);
+
+MODULE_DESCRIPTION("DMA-streaming memory allocator for videobuf2");
+MODULE_AUTHOR("Federico Vaga <federico.vaga@gmail.com>");
+MODULE_LICENSE("GPL v2");
new file mode 100644
@@ -0,0 +1,24 @@
+/*
+ * videobuf2-dma-streaming.h - DMA steaming memory allocator for videobuf2
+ *
+ * Copyright (C) 2012 Federico Vaga
+ *
+ * Author: Federico Vaga <federico.vaga@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ */
+
+#ifndef _MEDIA_VIDEOBUF2_DMA_STREAMING_H
+#define _MEDIA_VIDEOBUF2_DMA_STREAMING_H
+
+#include <media/videobuf2-core.h>
+#include <linux/dma-mapping.h>
+
+void *vb2_dma_streaming_init_ctx(struct device *dev);
+void vb2_dma_streaming_cleanup_ctx(void *alloc_ctx);
+
+extern const struct vb2_mem_ops vb2_dma_streaming_memops;
+
+#endif