From 968ced78a53509a996708a14e8b9269d1dc6a61c Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Sun, 13 Apr 2008 14:58:21 -0300 Subject: [PATCH] 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 --- drivers/media/video/videobuf-core.c | 14 --- drivers/media/video/videobuf-vmalloc.c | 163 ++++++++++++++++--------- 2 files changed, 105 insertions(+), 72 deletions(-) diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c index 37ddbed8e1..5613e0882b 100644 --- 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 9eb7982988..075acdf6b7 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; } -- 2.39.5