RE: [RFC PATCH v2 4/8] media: videobuf2: introduce VIDEOBUF2_PAGE memops

2011-12-22 Thread Marek Szyprowski
Hello,

On Wednesday, December 14, 2011 3:00 PM Ming Lei wrote:

 DMA contig memory resource is very limited and precious, also
 accessing to it from CPU is very slow on some platform.
 
 For some cases(such as the comming face detection driver), DMA Streaming
 buffer is enough, so introduce VIDEOBUF2_PAGE to allocate continuous
 physical memory but letting video device driver to handle DMA buffer mapping
 and unmapping things.
 
 Signed-off-by: Ming Lei ming@canonical.com

Could you elaborate a bit why do you think that DMA contig memory resource
is so limited? If dma_alloc_coherent fails because of the memory fragmentation,
the alloc_pages() call with order  0 will also fail.

I understand that there might be some speed issues with coherent (uncached)
userspace mappings, but I would solve it in completely different way. The 
interface
for both coherent/uncached and non-coherent/cached contig allocator should be 
the
same, so exchanging them is easy and will not require changes in the driver.
I'm planning to introduce some design changes in memory allocator api and 
introduce
prepare and finish callbacks in allocator ops. I hope to post the rfc after
Christmas. For your face detection driver using standard dma-contig allocator
shouldn't be a big issue.

Your current implementation also abuses the design and api of videobuf2 memory
allocators. If the allocator needs to return a custom structure to the driver
you should use cookie method. vaddr is intended to provide only a pointer to
kernel virtual mapping, but you pass a struct page * there.

(snipped)

Best regards
-- 
Marek Szyprowski
Samsung Poland RD Center



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC PATCH v2 4/8] media: videobuf2: introduce VIDEOBUF2_PAGE memops

2011-12-14 Thread Ming Lei
DMA contig memory resource is very limited and precious, also
accessing to it from CPU is very slow on some platform.

For some cases(such as the comming face detection driver), DMA Streaming
buffer is enough, so introduce VIDEOBUF2_PAGE to allocate continuous
physical memory but letting video device driver to handle DMA buffer mapping
and unmapping things.

Signed-off-by: Ming Lei ming@canonical.com
---
 drivers/media/video/Kconfig  |4 +
 drivers/media/video/Makefile |1 +
 drivers/media/video/videobuf2-page.c |  117 ++
 include/media/videobuf2-page.h   |   20 ++
 4 files changed, 142 insertions(+), 0 deletions(-)
 create mode 100644 drivers/media/video/videobuf2-page.c
 create mode 100644 include/media/videobuf2-page.h

diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 4e8a0c4..5684a00 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -60,6 +60,10 @@ config VIDEOBUF2_VMALLOC
select VIDEOBUF2_MEMOPS
tristate
 
+config VIDEOBUF2_PAGE
+   select VIDEOBUF2_CORE
+   select VIDEOBUF2_MEMOPS
+   tristate
 
 config VIDEOBUF2_DMA_SG
#depends on HAS_DMA
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index ddeaa6c..bc797f2 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -125,6 +125,7 @@ obj-$(CONFIG_VIDEO_BTCX)  += btcx-risc.o
 obj-$(CONFIG_VIDEOBUF2_CORE)   += videobuf2-core.o
 obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
 obj-$(CONFIG_VIDEOBUF2_VMALLOC)+= videobuf2-vmalloc.o
+obj-$(CONFIG_VIDEOBUF2_PAGE)   += videobuf2-page.o
 obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
 obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
 
diff --git a/drivers/media/video/videobuf2-page.c 
b/drivers/media/video/videobuf2-page.c
new file mode 100644
index 000..6a24a34
--- /dev/null
+++ b/drivers/media/video/videobuf2-page.c
@@ -0,0 +1,117 @@
+/*
+ * videobuf2-page.c - page memory allocator for videobuf2
+ *
+ * Copyright (C) 2011 Canonical Ltd.
+ *
+ * Author: Ming Lei ming@canonical.com
+ *
+ * This file is based on videobuf2-vmalloc.c
+ *
+ * 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 linux/module.h
+#include linux/mm.h
+#include linux/slab.h
+
+#include media/videobuf2-core.h
+#include media/videobuf2-memops.h
+
+struct vb2_page_buf {
+   void*vaddr;
+   unsigned long   size;
+   atomic_trefcount;
+   struct vb2_vmarea_handler   handler;
+};
+
+static void vb2_page_put(void *buf_priv);
+
+static void *vb2_page_alloc(void *alloc_ctx, unsigned long size)
+{
+   struct vb2_page_buf *buf;
+
+   buf = kzalloc(sizeof *buf, GFP_KERNEL);
+   if (!buf)
+   return NULL;
+
+   buf-size = size;
+   buf-vaddr = (void *)__get_free_pages(GFP_KERNEL,
+   get_order(buf-size));
+   buf-handler.refcount = buf-refcount;
+   buf-handler.put = vb2_page_put;
+   buf-handler.arg = buf;
+
+   if (!buf-vaddr) {
+   printk(KERN_ERR page of size %ld failed\n, buf-size);
+   kfree(buf);
+   return NULL;
+   }
+
+   atomic_inc(buf-refcount);
+   printk(KERN_DEBUG Allocated page buffer of size %ld at vaddr=%p\n,
+   buf-size, buf-vaddr);
+
+   return buf;
+}
+
+static void vb2_page_put(void *buf_priv)
+{
+   struct vb2_page_buf *buf = buf_priv;
+
+   if (atomic_dec_and_test(buf-refcount)) {
+   printk(KERN_DEBUG %s: Freeing page mem at vaddr=%p\n,
+   __func__, buf-vaddr);
+   free_pages((unsigned long)buf-vaddr, get_order(buf-size));
+   kfree(buf);
+   }
+}
+
+static void *vb2_page_vaddr(void *buf_priv)
+{
+   struct vb2_page_buf *buf = buf_priv;
+
+   BUG_ON(!buf);
+
+   if (!buf-vaddr) {
+   printk(KERN_ERR Address of an unallocated plane requested\n);
+   return NULL;
+   }
+
+   return buf-vaddr;
+}
+
+static unsigned int vb2_page_num_users(void *buf_priv)
+{
+   struct vb2_page_buf *buf = buf_priv;
+   return atomic_read(buf-refcount);
+}
+
+static int vb2_page_mmap(void *buf_priv, struct vm_area_struct *vma)
+{
+   struct vb2_page_buf *buf = buf_priv;
+
+   if (!buf) {
+   printk(KERN_ERR No memory to map\n);
+   return -EINVAL;
+   }
+
+   vma-vm_page_prot = vm_get_page_prot(vma-vm_flags);
+   return vb2_mmap_pfn_range(vma, virt_to_phys(buf-vaddr),
+   buf-size, vb2_common_vm_ops,
+   buf-handler);
+}
+
+const struct vb2_mem_ops vb2_page_memops = {
+   .alloc  =