videobuf-dma-contig sync question

Hi,

I don't understand why __videobuf_sync in videobuf-dma-contig isn't a nop.

All the memory allocated by videobuf-dma-contig is coherent memory. And Documentation/DMA-API.txt seems to imply that this memory is coherent and doesn't need extra cache operation for synchronization.

Also calling dma_sync_single_for_cpu cause panic on arm for per-device coherent memory, because the memory isn't in the main memory[1].

Why __videobuf_sync need dma_sync_single_for_cpu ?

Regards,

Matthieu

[1]
void dma_cache_maint(const void *start, size_t size, int direction)

{
    const void *end = start + size;

    BUG_ON(!virt_addr_valid(start) || !virt_addr_valid(end - 1));

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Hi Matthieu,

[CC Paul, Paulius]

Sounds correct. With that in mind the sync doesn't make much sense. Fixing the videobuf-dma-contig code seems like a good idea to me. Or is it architecture code that needs to be fixed? Any thoughts Paul?

Initially in V1 of the patch the sync was just a nop, but in V2 the current behaviour was introduced. I think Paulius requested the sync implementation and I just blindly added it since it worked well for me on SuperH anyway:

http://osdir.com/ml/linux.ports.sh.devel/2008-07/msg00038.html

Paulius, do you really need the sync?

Cheers,

/ magnus

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

No, no sync points are necessary for dma_alloc_coherent() memory. In the case of dma_alloc_noncoherent() it is left up to the implementation to decide what to do and be moderately more intelligent in the sync code, but not in the dma_alloc_coherent() case where uncached coherent regions are handed back directly.

dma_alloc_coherent() is also the tie-in point for per-device coherent memory, which absolutely is not handled by the sync code and will rightly blow up.

This probably comes from a lack of understanding of the API and its subtleties. The sync ops are necessary if a platform can't gaurantee coherent memory (or the device needs more than it can gaurantee) and hands back a non-coherent allocation. The driver has to be aware of this and request it explicitly however, or you just fail on the allocation. Going down this road means that both the driver and the platform need to agree on how to manage potentially non-coherent memory, which most are ill-equipped to do. This also is not a concern for this driver given the use of dma_alloc_coherent() over dma_alloc_noncoherent().

Beyond that, the sync points are necessary for streaming DMA mappings, which are likewise unrelated to this driver. ie, videobuf-dma-sg would use sync points if it cared, videobuf-dma-contig would not, unless you renamed it to videobuf-dma-maybecontig and dealt with the potential for being handed back non-coherent memory.

The architecture code is quite right to blow up on this.

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list