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 at 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);
