Re: [PATCHv10 23/26] v4l: vb2-dma-contig: align buffer size to PAGE_SIZE

[Tomasz Stanislawski <t.stanislaws@samsung.com>]

Hi Laurent,

On 10/11/2012 11:31 PM, Laurent Pinchart wrote:
> Hi Tomasz,
> 
> On Wednesday 10 October 2012 16:46:42 Tomasz Stanislawski wrote:
>> Most operations on DMA and DMABUF framework need page
>> aligned buffers.
> 
> The comment is a bit misleading, the buffer is already page-aligned (unless 
> I'm mistaken dma_alloc_coherent() returns a page-aligned buffer) but its size 
> isn't a multiple of the page size.

Ok. I will update the commit message that only buffer size is going to be page aligned.

> 
> Do we really need a page size multiple ? Isn't it enough to make the size a 
> multiple of the cache line size ?
> 

Frankly, I strongly oppose forcing a size of a DMA buffer to be rounded up.

However, I discovered a problem while testing mmap() interface in dma-buf.
The test in dma_buf_mmap() will fail if the size is not a multiple of 4k.

Maybe the value from dma-buf.c:456 should be changed from:

dmabuf->size >> PAGE_SHIFT

to

PAGE_ALIGN(dmabuf->size) >> PAGE_SHIFT

However, I preferred to avoid any changes outside of the media tree
hoping that the patchset gets merged. Rounding the buffer size to
a page size was quick workaround for the issue with DMABUF mmap().

Regards,
Tomasz Stanislawski

>> This fix guarantees this requirement
>> for vb2-dma-contig buffers.
>>
>> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
>> ---
>>  drivers/media/v4l2-core/videobuf2-dma-contig.c |    3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
>> b/drivers/media/v4l2-core/videobuf2-dma-contig.c index 571a919..002ee50
>> 100644
>> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
>> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
>> @@ -162,6 +162,9 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long
>> size) if (!buf)
>>  		return ERR_PTR(-ENOMEM);
>>
>> +	/* align image size to PAGE_SIZE */
>> +	size = PAGE_ALIGN(size);
>> +
>>  	buf->vaddr = dma_alloc_coherent(dev, size, &buf->dma_addr, GFP_KERNEL);
>>  	if (!buf->vaddr) {
>>  		dev_err(dev, "dma_alloc_coherent of size %ld failed\n", size);