V4L/DVB (7552): videbuf-vmalloc: Corrects mmap code

There were some bugs on videobuf-vmalloc.

Basically, remap were called with a wrong parameter. Due to that, a later remap
were needed, generating the need of some hacks on videobuf-vmalloc and
videobuf-core.

This patch fixes the remap and removes the hacks.

TODO:

- V4L2_MEMORY_USERPTR is not implemented yet. This method should be
  properly implemented, in order to work with a few userspace applications.

- The driver also doesn't implement V4L2_MEMORY_OVERLAY. This method is used
  only by a few applications, and are becaming obsolete, due to the increment
  of cpu performance. So, most apps prefer to retrieve data to an internal
  buffer, doing some processing like de-interlacing.

Signed-off-by: Mauro Carvalho Chehab <mchehab@infradead.org>

1 files changed, 105 insertions, 58 deletions

diff --git a/drivers/media/video/videobuf-vmalloc.c b/drivers/media/video/videobuf-vmalloc.c
index 9eb7982988a6..075acdf6b7c7 100644
--- a/drivers/media/video/videobuf-vmalloc.c
+++ b/drivers/media/video/videobuf-vmalloc.c
@@ -124,45 +124,75 @@ static int __videobuf_iolock (struct videobuf_queue* q,
                               struct videobuf_buffer *vb,
                               struct v4l2_framebuffer *fbuf)
 {
-        int pages;
-        struct videobuf_vmalloc_memory *mem=vb->priv;
+        struct videobuf_vmalloc_memory *mem = vb->priv;
 
         BUG_ON(!mem);
 
-        MAGIC_CHECK(mem->magic,MAGIC_VMAL_MEM);
+        MAGIC_CHECK(mem->magic, MAGIC_VMAL_MEM);
 
-        pages = PAGE_ALIGN(vb->size) >> PAGE_SHIFT;
+        switch (vb->memory) {
+        case V4L2_MEMORY_MMAP:
+                dprintk(1, "%s memory method MMAP\n", __func__);
 
-        /* Currently, doesn't support V4L2_MEMORY_OVERLAY */
-        if ((vb->memory != V4L2_MEMORY_MMAP) &&
-                                (vb->memory != V4L2_MEMORY_USERPTR) ) {
-                printk(KERN_ERR "Method currently unsupported.\n");
-                return -EINVAL;
-        }
+                /* All handling should be done by __videobuf_mmap_mapper() */
+                if (!mem->vmalloc) {
+                        printk(KERN_ERR "memory is not alloced/mmapped.\n");
+                        return -EINVAL;
+                }
+                break;
+        case V4L2_MEMORY_USERPTR:
+        {
+                int pages = PAGE_ALIGN(vb->size);
 
-        /* FIXME: should be tested with kernel mmap mem */
-        mem->vmalloc=vmalloc_user (PAGE_ALIGN(vb->size));
-        if (NULL == mem->vmalloc) {
-                printk(KERN_ERR "vmalloc (%d pages) failed\n",pages);
-                return -ENOMEM;
-        }
+                dprintk(1, "%s memory method USERPTR\n", __func__);
+
+#if 1
+                if (vb->baddr) {
+                        printk(KERN_ERR "USERPTR is currently not supported\n");
+                        return -EINVAL;
+                }
+#endif
 
-        dprintk(1,"vmalloc is at addr 0x%08lx, size=%d\n",
-                                (unsigned long)mem->vmalloc,
-                                pages << PAGE_SHIFT);
-
-        /* It seems that some kernel versions need to do remap *after*
-           the mmap() call
-         */
-        if (mem->vma) {
-                int retval=remap_vmalloc_range(mem->vma, mem->vmalloc,0);
-                kfree(mem->vma);
-                mem->vma=NULL;
-                if (retval<0) {
-                        dprintk(1,"mmap app bug: remap_vmalloc_range area %p error %d\n",
-                                mem->vmalloc,retval);
-                        return retval;
+                /* The only USERPTR currently supported is the one needed for
+                   read() method.
+                 */
+
+                mem->vmalloc = vmalloc_user(pages);
+                if (!mem->vmalloc) {
+                        printk(KERN_ERR "vmalloc (%d pages) failed\n", pages);
+                        return -ENOMEM;
                 }
+                dprintk(1, "vmalloc is at addr %p (%d pages)\n",
+                        mem->vmalloc, pages);
+
+#if 0
+                int rc;
+                /* Kernel userptr is used also by read() method. In this case,
+                   there's no need to remap, since data will be copied to user
+                 */
+                if (!vb->baddr)
+                        return 0;
+
+                /* FIXME: to properly support USERPTR, remap should occur.
+                   The code bellow won't work, since mem->vma = NULL
+                 */
+                /* Try to remap memory */
+                rc = remap_vmalloc_range(mem->vma, (void *)vb->baddr, 0);
+                if (rc < 0) {
+                        printk(KERN_ERR "mmap: remap failed with error %d. ", rc);
+                        return -ENOMEM;
+                }
+#endif
+
+                break;
+        }
+        case V4L2_MEMORY_OVERLAY:
+        default:
+                dprintk(1, "%s memory method OVERLAY/unknown\n", __func__);
+
+                /* Currently, doesn't support V4L2_MEMORY_OVERLAY */
+                printk(KERN_ERR "Memory method currently unsupported.\n");
+                return -EINVAL;
         }
 
         return 0;
@@ -178,6 +208,7 @@ static int __videobuf_mmap_free(struct videobuf_queue *q)
 {
         unsigned int i;
 
+        dprintk(1, "%s\n", __func__);
         for (i = 0; i < VIDEO_MAX_FRAME; i++) {
                 if (q->bufs[i]) {
                         if (q->bufs[i]->map)
@@ -194,10 +225,11 @@ static int __videobuf_mmap_mapper(struct videobuf_queue *q,
         struct videobuf_vmalloc_memory *mem;
         struct videobuf_mapping *map;
         unsigned int first;
-        int retval;
+        int retval, pages;
         unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
 
-        if (! (vma->vm_flags & VM_WRITE) || ! (vma->vm_flags & VM_SHARED))
+        dprintk(1, "%s\n", __func__);
+        if (!(vma->vm_flags & VM_WRITE) || !(vma->vm_flags & VM_SHARED))
                 return -EINVAL;
 
         /* look for first buffer to map */
@@ -217,46 +249,55 @@ static int __videobuf_mmap_mapper(struct videobuf_queue *q,
         }
 
         /* create mapping + update buffer list */
-        map = q->bufs[first]->map = kzalloc(sizeof(struct videobuf_mapping),GFP_KERNEL);
+        map = kzalloc(sizeof(struct videobuf_mapping), GFP_KERNEL);
         if (NULL == map)
                 return -ENOMEM;
 
+        q->bufs[first]->map = map;
         map->start = vma->vm_start;
         map->end   = vma->vm_end;
         map->q     = q;
 
         q->bufs[first]->baddr = vma->vm_start;
 
-        vma->vm_ops          = &videobuf_vm_ops;
-        vma->vm_flags       |= VM_DONTEXPAND | VM_RESERVED;
-        vma->vm_private_data = map;
+        mem = q->bufs[first]->priv;
+        BUG_ON(!mem);
+        MAGIC_CHECK(mem->magic, MAGIC_VMAL_MEM);
 
-        mem=q->bufs[first]->priv;
-        BUG_ON (!mem);
-        MAGIC_CHECK(mem->magic,MAGIC_VMAL_MEM);
+        pages = PAGE_ALIGN(vma->vm_end - vma->vm_start);
+        mem->vmalloc = vmalloc_user(pages);
+        if (!mem->vmalloc) {
+                printk(KERN_ERR "vmalloc (%d pages) failed\n", pages);
+                goto error;
+        }
+        dprintk(1, "vmalloc is at addr %p (%d pages)\n",
+                mem->vmalloc, pages);
 
         /* Try to remap memory */
-        retval=remap_vmalloc_range(vma, mem->vmalloc,0);
-        if (retval<0) {
-                dprintk(1,"mmap: postponing remap_vmalloc_range\n");
-
-                mem->vma=kmalloc(sizeof(*vma),GFP_KERNEL);
-                if (!mem->vma) {
-                        kfree(map);
-                        q->bufs[first]->map=NULL;
-                        return -ENOMEM;
-                }
-                memcpy(mem->vma,vma,sizeof(*vma));
+        retval = remap_vmalloc_range(vma, mem->vmalloc, 0);
+        if (retval < 0) {
+                printk(KERN_ERR "mmap: remap failed with error %d. ", retval);
+                vfree(mem->vmalloc);
+                goto error;
         }
 
+        vma->vm_ops          = &videobuf_vm_ops;
+        vma->vm_flags       |= VM_DONTEXPAND | VM_RESERVED;
+        vma->vm_private_data = map;
+
         dprintk(1,"mmap %p: q=%p %08lx-%08lx (%lx) pgoff %08lx buf %d\n",
-                map,q,vma->vm_start,vma->vm_end,
+                map, q, vma->vm_start, vma->vm_end,
                 (long int) q->bufs[first]->bsize,
-                vma->vm_pgoff,first);
+                vma->vm_pgoff, first);
 
         videobuf_vm_open(vma);
 
-        return (0);
+        return 0;
+
+error:
+        mem = NULL;
+        kfree(map);
+        return -ENOMEM;
 }
 
 static int __videobuf_copy_to_user ( struct videobuf_queue *q,
@@ -347,13 +388,19 @@ EXPORT_SYMBOL_GPL(videobuf_to_vmalloc);
 
 void videobuf_vmalloc_free (struct videobuf_buffer *buf)
 {
-        struct videobuf_vmalloc_memory *mem=buf->priv;
-        BUG_ON (!mem);
+        struct videobuf_vmalloc_memory *mem = buf->priv;
 
-        MAGIC_CHECK(mem->magic,MAGIC_VMAL_MEM);
+        if (!mem)
+                return;
+
+        MAGIC_CHECK(mem->magic, MAGIC_VMAL_MEM);
 
         vfree(mem->vmalloc);
-        mem->vmalloc=NULL;
+        mem->vmalloc = NULL;
+
+
+
+        /* FIXME: need to do buf->priv = NULL? */
 
         return;
 }