Hi Prabhakar, (CC'ing Marek) On Tuesday 16 April 2013 16:24:30 Prabhakar lad wrote: > From: Lad, Prabhakar <prabhakar.csengg@xxxxxxxxx> > > with recent commit with id 068a0df76023926af958a336a78bef60468d2033 > which adds add length check for mmap, the application were failing to > mmap the buffers. > > This patch aligns the the buffer size to page size boundary for both > capture and display driver so the it pass the check. > > Signed-off-by: Lad, Prabhakar <prabhakar.csengg@xxxxxxxxx> > Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > Cc: Hans Verkuil <hans.verkuil@xxxxxxxxx> > Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx> > --- > Changes for v2: > 1: Fixed a typo in commit message. > > drivers/media/platform/davinci/vpif_capture.c | 1 + > drivers/media/platform/davinci/vpif_display.c | 1 + > 2 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/drivers/media/platform/davinci/vpif_capture.c > b/drivers/media/platform/davinci/vpif_capture.c index 5f98df1..25981d6 > 100644 > --- a/drivers/media/platform/davinci/vpif_capture.c > +++ b/drivers/media/platform/davinci/vpif_capture.c > @@ -183,6 +183,7 @@ static int vpif_buffer_queue_setup(struct vb2_queue *vq, > *nbuffers = config_params.min_numbuffers; > > *nplanes = 1; > + size = PAGE_ALIGN(size); I wonder if that's the best fix. The queue_setup operation is supposed to return the size required by the driver for each plane. Depending on the hardware requirements, that size might not be a multiple of the page size. As we can't mmap() a fraction of a page, the allocated plane size needs to be rounded up to the next page boundary to allow mmap() support. The dma-contig and dma-sg allocators already do so in their alloc operation, but the vmalloc allocator doesn't. The recent "media: vb2: add length check for mmap" patch verifies that the mmap() size requested by userspace doesn't exceed the buffer size. As the mmap() size is rounded up to the next page boundary the check will fail for buffer sizes that are not multiple of the page size. Your fix will not result in overallocation (as the allocator already rounds the size up), but will prevent the driver from importing a buffer large enough for the hardware but not rounded up to the page size. A better fix might be to round up the buffer size in the buffer size check at mmap() time, and fix the vmalloc allocator to round up the size. That the allocator, not drivers, is responsible for buffer size alignment should be documented in videobuf2-core.h. > sizes[0] = size; > alloc_ctxs[0] = common->alloc_ctx; > > diff --git a/drivers/media/platform/davinci/vpif_display.c > b/drivers/media/platform/davinci/vpif_display.c index 1b3fb5c..3414715 > 100644 > --- a/drivers/media/platform/davinci/vpif_display.c > +++ b/drivers/media/platform/davinci/vpif_display.c > @@ -162,6 +162,7 @@ static int vpif_buffer_queue_setup(struct vb2_queue *vq, > *nbuffers = config_params.min_numbuffers; > > *nplanes = 1; > + size = PAGE_ALIGN(size); > sizes[0] = size; > alloc_ctxs[0] = common->alloc_ctx; > return 0; -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
