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>

diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c
index 37ddbed8e167318d4cc30607c3f834f4bb71928f..5613e0882b5c67ee1b3d40df9b6f4dcf1fb34962 100644 (file)
--- a/drivers/media/video/videobuf-core.c
+++ b/drivers/media/video/videobuf-core.c
@@ -91,20 +91,6 @@ int videobuf_iolock(struct videobuf_queue *q, struct videobuf_buffer *vb,
        MAGIC_CHECK(vb->magic, MAGIC_BUFFER);
        MAGIC_CHECK(q->int_ops->magic, MAGIC_QTYPE_OPS);
 
-       /* This is required to avoid OOPS on some cases,
-          since mmap_mapper() method should be called before _iolock.
-          On some cases, the mmap_mapper() is called only after scheduling.
-        */
-       if (vb->memory == V4L2_MEMORY_MMAP) {
-               wait_event_timeout(vb->done, q->is_mmapped,
-                                  msecs_to_jiffies(100));
-               if (!q->is_mmapped) {
-                       printk(KERN_ERR
-                              "Error: mmap_mapper() never called!\n");
-                       return -EINVAL;
-               }
-       }
-
        return CALL(q, iolock, q, vb, fbuf);
 }
 

diff --git a/drivers/media/video/videobuf-vmalloc.c b/drivers/media/video/videobuf-vmalloc.c
index 9eb7982988a6f2f918291e0e0e692aa760283309..075acdf6b7c71ec4b670c565c46cbc9a9aca6d10 100644 (file)
--- 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;
 }