Hi Christoph,
On 13/11/2022 17:35, Christoph Hellwig wrote:
dma_alloc_coherent does not return a physical address, but a DMA address, which might be remapped or have an offset. Passing the DMA address to vm_iomap_memory is thus broken.
Use the proper dma_mmap_coherent helper instead, and stop passing __GFP_COMP to dma_alloc_coherent, as the memory management inside the DMA allocator is hidden from the callers and does not require it.
With this the gfp_t argument to __videobuf_dc_alloc can be removed and hard coded to GFP_KERNEL.
Fixes: a8f3c203e19b ("[media] videobuf-dma-contig: add cache support") Signed-off-by: Christoph Hellwig hch@lst.de
drivers/media/v4l2-core/videobuf-dma-contig.c | 22 +++++++------------
Very, very old code :-) Hopefully in the not-too-distant future we can kill off the old videobuf framework. But for now:
Acked-by: Hans Verkuil hverkuil-cisco@xs4all.nl
I assume you take this? If not, then let me know and I will pick it up for the media subsystem.
Regards,
Hans
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf-dma-contig.c b/drivers/media/v4l2-core/videobuf-dma-contig.c index 52312ce2ba056..f2c4393595574 100644 --- a/drivers/media/v4l2-core/videobuf-dma-contig.c +++ b/drivers/media/v4l2-core/videobuf-dma-contig.c @@ -36,12 +36,11 @@ struct videobuf_dma_contig_memory {
static int __videobuf_dc_alloc(struct device *dev, struct videobuf_dma_contig_memory *mem,
unsigned long size, gfp_t flags)
unsigned long size){ mem->size = size;
- mem->vaddr = dma_alloc_coherent(dev, mem->size,
&mem->dma_handle, flags);
- mem->vaddr = dma_alloc_coherent(dev, mem->size, &mem->dma_handle,
GFP_KERNEL);@@ -258,8 +257,7 @@ static int __videobuf_iolock(struct videobuf_queue *q, return videobuf_dma_contig_user_get(mem, vb);
/* allocate memory for the read() method */
if (__videobuf_dc_alloc(q->dev, mem, PAGE_ALIGN(vb->size),GFP_KERNEL))
if (__videobuf_dc_alloc(q->dev, mem, PAGE_ALIGN(vb->size))) return -ENOMEM;@@ -295,22 +293,18 @@ static int __videobuf_mmap_mapper(struct videobuf_queue *q, BUG_ON(!mem); MAGIC_CHECK(mem->magic, MAGIC_DC_MEM);
- if (__videobuf_dc_alloc(q->dev, mem, PAGE_ALIGN(buf->bsize),
GFP_KERNEL | __GFP_COMP))
- if (__videobuf_dc_alloc(q->dev, mem, PAGE_ALIGN(buf->bsize))) goto error;
- /* Try to remap memory */
- vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
- /* the "vm_pgoff" is just used in v4l2 to find the
- corresponding buffer data structure which is allocated
- earlier and it does not mean the offset from the physical
- buffer start address as usual. So set it to 0 to pass
* the sanity check in vm_iomap_memory().
* the sanity check in dma_mmap_coherent().
- retval = vm_iomap_memory(vma, mem->dma_handle, mem->size);
- retval = dma_mmap_coherent(q->dev, vma, mem->vaddr, mem->dma_handle,
mem->size);