Hi Jiri,
On Monday 28 February 2011 16:07:43 Jiri Slaby wrote:
> On 02/28/2011 11:53 AM, Laurent Pinchart wrote:
> > On Monday 28 February 2011 10:37:02 Jiri Slaby wrote:
> >> mem->dma_handle is a dma address obtained by dma_alloc_coherent which
> >> needn't be a physical address in presence of IOMMU. So ensure we are
> >> remapping (remap_pfn_range) the right page in __videobuf_mmap_mapper
> >> by using virt_to_phys(mem->vaddr) and not mem->dma_handle.
> >
> > Quoting arch/arm/include/asm/memory.h,
> >
> > /*
> >
> > * These are *only* valid on the kernel direct mapped RAM memory.
>
> Which the DMA allocation shall be.
>
> > * Note: Drivers should NOT use these.
>
> This is weird.
>
> > They are the wrong
> >
> > * translation for translating DMA addresses. Use the driver
> > * DMA support - see dma-mapping.h.
>
> Yes, ACK, and vice versa. DMA addresses cannot be used as physical ones.
>
> > */
> >
> > static inline unsigned long virt_to_phys(const volatile void *x)
> > {
> >
> > return __virt_to_phys((unsigned long)(x));
> >
> > }
> >
> > Why would you use physically contiguous memory if you have an IOMMU
> > anyway ?
>
> Sorry, what? When IOMMU is used, dma_alloc_* functions may return "tags"
> as a DMA address, not a physical address. So using these DMA "addresses"
> directly (e.g. in remap_pfn_range) is a bug.
What I mean is that videobuf-dma-contig is meant to be used by drivers that
require physically contiguous memory. If the system has an IOMMU, why would
drivers need that ?
> Maybe there is a better way to convert a kernel address of the DMA
> mapping into a physical frame, but I'm not aware of any...
>
> >> While at it, use PFN_DOWN instead of explicit shift.
> >>
> >> Signed-off-by: Jiri Slaby <jsl...@suse.cz>
> >> Cc: Mauro Carvalho Chehab <mche...@infradead.org>
> >> Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
> >> ---
> >>
> >> drivers/media/video/videobuf-dma-contig.c | 2 +-
> >> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/media/video/videobuf-dma-contig.c
> >> b/drivers/media/video/videobuf-dma-contig.c index c969111..19d3e4a
> >> 100644 --- a/drivers/media/video/videobuf-dma-contig.c
> >> +++ b/drivers/media/video/videobuf-dma-contig.c
> >> @@ -300,7 +300,7 @@ static int __videobuf_mmap_mapper(struct
> >> videobuf_queue *q,
> >>
> >> vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> >> retval = remap_pfn_range(vma, vma->vm_start,
> >>
> >> - mem->dma_handle >> PAGE_SHIFT,
> >> + PFN_DOWN(virt_to_phys(mem->vaddr))
> >>
> >> size, vma->vm_page_prot);
> >>
> >> if (retval) {
> >>
> >> dev_err(q->dev, "mmap: remap failed with error %d. ", retval);
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
