Re: [PATCH v1 7/7] v4l: videobuf2: add CMA allocator
Hello, On 09/15/2010 10:55 AM, han jonghun wrote: Hello, In vb2_cma_put if buf->refcount is 0, cma_free is called. But vb2_cma_put is usually called from munmap. In my opinion cma_free should be called from VIDIOC_REQBUFS(0) not munmap. cma_free has to be called from both, since we do not always call VIDIOC_REQBUFS(0) after finishing. If an application just closes the file descriptor (or even dies), we need a way to clean up the memory. -- Best regards, Pawel Osciak -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 7/7] v4l: videobuf2: add CMA allocator
Hello, In vb2_cma_put if buf->refcount is 0, cma_free is called. But vb2_cma_put is usually called from munmap. In my opinion cma_free should be called from VIDIOC_REQBUFS(0) not munmap. BRs, 2010/9/9 Pawel Osciak : > Add support for the CMA contiguous memory allocator to videobuf2. > > Signed-off-by: Pawel Osciak > Signed-off-by: Kyungmin Park > --- > drivers/media/video/Kconfig | 5 + > drivers/media/video/Makefile | 2 + > drivers/media/video/videobuf2-cma.c | 250 > +++ > include/media/videobuf2-cma.h | 25 > 4 files changed, 282 insertions(+), 0 deletions(-) > create mode 100644 drivers/media/video/videobuf2-cma.c > create mode 100644 include/media/videobuf2-cma.h > > diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig > index c2ea549..b63f377 100644 > --- a/drivers/media/video/Kconfig > +++ b/drivers/media/video/Kconfig > @@ -65,6 +65,11 @@ config VIDEOBUF2_VMALLOC > select VIDEOBUF2_GEN_MEMOPS > tristate > > +config VIDEOBUF2_CMA > + depends on CMA > + select VIDEOBUF2_CORE > + select VIDEOBUF2_GEN_MEMOPS > + tristate > > # > # Multimedia Video device configuration > diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile > index 20d359d..4146700 100644 > --- a/drivers/media/video/Makefile > +++ b/drivers/media/video/Makefile > @@ -128,6 +128,8 @@ obj-$(CONFIG_VIDEOBUF2_VMALLOC) += > videobuf2_vmalloc.o > videobuf2_vmalloc-y := videobuf2-vmalloc.o \ > videobuf2-memops.o > > +obj-$(CONFIG_VIDEOBUF2_CMA) += videobuf2_cma.o > +videobuf2_cma-y := videobuf2-cma.o > videobuf2-memops.o > > obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o > > diff --git a/drivers/media/video/videobuf2-cma.c > b/drivers/media/video/videobuf2-cma.c > new file mode 100644 > index 000..c51e5a8 > --- /dev/null > +++ b/drivers/media/video/videobuf2-cma.c > @@ -0,0 +1,250 @@ > +/* > + * videobuf2-cma.c - CMA memory allocator for videobuf2 > + * > + * Copyright (C) 2010 Samsung Electronics > + * > + * Author: Pawel Osciak > + * > + * 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. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > + > +struct vb2_cma_conf { > + struct vb2_alloc_ctx alloc_ctx; > + struct device *dev; > + const char *type; > + unsigned long alignment; > +}; > + > +struct vb2_cma_buf { > + struct vb2_cma_conf *conf; > + dma_addr_t paddr; > + unsigned long size; > + unsigned int refcount; > + struct vm_area_struct *vma; > +}; > + > +static void *vb2_cma_alloc(const struct vb2_alloc_ctx *alloc_ctx, > + unsigned long size) > +{ > + struct vb2_cma_conf *conf = > + container_of(alloc_ctx, struct vb2_cma_conf, alloc_ctx); > + struct vb2_cma_buf *buf; > + > + buf = kzalloc(sizeof *buf, GFP_KERNEL); > + if (!buf) > + return ERR_PTR(-ENOMEM); > + > + buf->paddr = cma_alloc(conf->dev, conf->type, size, conf->alignment); > + if (!buf->paddr) { > + printk(KERN_ERR "cma_alloc of size %ld failed\n", size); > + kfree(buf); > + return ERR_PTR(-ENOMEM); > + } > + > + buf->conf = conf; > + buf->size = size; > + buf->refcount++; > + > + printk(KERN_DEBUG "Allocated cma mem of size %ld at paddr=0x%08x\n", > + buf->size, buf->paddr); > + > + return buf; > +} > + > +static void vb2_cma_put(void *buf_priv) > +{ > + struct vb2_cma_buf *buf = buf_priv; > + > + buf->refcount--; > + > + if (0 == buf->refcount) { > + cma_free(buf->paddr); > + kfree(buf); > + } > +} > + > +static unsigned long vb2_cma_paddr(void *buf_priv) > +{ > + struct vb2_cma_buf *buf = buf_priv; > + > + return buf->paddr; > +} > + > +static unsigned int vb2_cma_num_users(void *buf_priv) > +{ > + struct vb2_cma_buf *buf = buf_priv; > + > + return buf->refcount; > +} > + > +static void vb2_cma_vm_open(struct vm_area_struct *vma) > +{ > + struct vb2_cma_buf *buf = vma->vm_private_data; > + > + printk(KERN_DEBUG "%s cma_priv: %p, refcount: %d, " > + "vma: %08lx-%08lx\n", __func__, buf, buf->refcount, > + vma->vm_start, vma->vm_end); > + > + buf->refcount++; > +} > + > +static void vb2_cma_vm_close(struct vm_area_struct *vma) > +{ > + struct vb2_cma_buf *buf = vma->vm_private_data; > + > + printk(KERN_DEBUG "%s cma_priv: %p, refcount: %d, " > +
[PATCH v1 7/7] v4l: videobuf2: add CMA allocator
Add support for the CMA contiguous memory allocator to videobuf2. Signed-off-by: Pawel Osciak Signed-off-by: Kyungmin Park --- drivers/media/video/Kconfig |5 + drivers/media/video/Makefile|2 + drivers/media/video/videobuf2-cma.c | 250 +++ include/media/videobuf2-cma.h | 25 4 files changed, 282 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/videobuf2-cma.c create mode 100644 include/media/videobuf2-cma.h diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index c2ea549..b63f377 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -65,6 +65,11 @@ config VIDEOBUF2_VMALLOC select VIDEOBUF2_GEN_MEMOPS tristate +config VIDEOBUF2_CMA + depends on CMA + select VIDEOBUF2_CORE + select VIDEOBUF2_GEN_MEMOPS + tristate # # Multimedia Video device configuration diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile index 20d359d..4146700 100644 --- a/drivers/media/video/Makefile +++ b/drivers/media/video/Makefile @@ -128,6 +128,8 @@ obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2_vmalloc.o videobuf2_vmalloc-y:= videobuf2-vmalloc.o \ videobuf2-memops.o +obj-$(CONFIG_VIDEOBUF2_CMA)+= videobuf2_cma.o +videobuf2_cma-y:= videobuf2-cma.o videobuf2-memops.o obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o diff --git a/drivers/media/video/videobuf2-cma.c b/drivers/media/video/videobuf2-cma.c new file mode 100644 index 000..c51e5a8 --- /dev/null +++ b/drivers/media/video/videobuf2-cma.c @@ -0,0 +1,250 @@ +/* + * videobuf2-cma.c - CMA memory allocator for videobuf2 + * + * Copyright (C) 2010 Samsung Electronics + * + * Author: Pawel Osciak + * + * 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. + */ + +#include +#include +#include +#include +#include +#include + +#include +#include + +struct vb2_cma_conf { + struct vb2_alloc_ctxalloc_ctx; + struct device *dev; + const char *type; + unsigned long alignment; +}; + +struct vb2_cma_buf { + struct vb2_cma_conf *conf; + dma_addr_t paddr; + unsigned long size; + unsigned intrefcount; + struct vm_area_struct *vma; +}; + +static void *vb2_cma_alloc(const struct vb2_alloc_ctx *alloc_ctx, + unsigned long size) +{ + struct vb2_cma_conf *conf = + container_of(alloc_ctx, struct vb2_cma_conf, alloc_ctx); + struct vb2_cma_buf *buf; + + buf = kzalloc(sizeof *buf, GFP_KERNEL); + if (!buf) + return ERR_PTR(-ENOMEM); + + buf->paddr = cma_alloc(conf->dev, conf->type, size, conf->alignment); + if (!buf->paddr) { + printk(KERN_ERR "cma_alloc of size %ld failed\n", size); + kfree(buf); + return ERR_PTR(-ENOMEM); + } + + buf->conf = conf; + buf->size = size; + buf->refcount++; + + printk(KERN_DEBUG "Allocated cma mem of size %ld at paddr=0x%08x\n", + buf->size, buf->paddr); + + return buf; +} + +static void vb2_cma_put(void *buf_priv) +{ + struct vb2_cma_buf *buf = buf_priv; + + buf->refcount--; + + if (0 == buf->refcount) { + cma_free(buf->paddr); + kfree(buf); + } +} + +static unsigned long vb2_cma_paddr(void *buf_priv) +{ + struct vb2_cma_buf *buf = buf_priv; + + return buf->paddr; +} + +static unsigned int vb2_cma_num_users(void *buf_priv) +{ + struct vb2_cma_buf *buf = buf_priv; + + return buf->refcount; +} + +static void vb2_cma_vm_open(struct vm_area_struct *vma) +{ + struct vb2_cma_buf *buf = vma->vm_private_data; + + printk(KERN_DEBUG "%s cma_priv: %p, refcount: %d, " + "vma: %08lx-%08lx\n", __func__, buf, buf->refcount, + vma->vm_start, vma->vm_end); + + buf->refcount++; +} + +static void vb2_cma_vm_close(struct vm_area_struct *vma) +{ + struct vb2_cma_buf *buf = vma->vm_private_data; + + printk(KERN_DEBUG "%s cma_priv: %p, refcount: %d, " + "vma: %08lx-%08lx\n", __func__, buf, buf->refcount, + vma->vm_start, vma->vm_end); + + vb2_cma_put(buf); +} + +static const struct vm_operations_struct vb2_cma_vm_ops = { + .open = vb2_cma_vm_open, + .close = vb2_cma_vm_close, +}; + +static int vb2_cma_mmap(void *buf_priv, struct vm_area_struct *vma) +{ + struct vb2_cma_buf *buf = buf_priv; + + if (!buf) { + printk(KERN_ERR "No memory to map\n"); + return -EINVAL; +