videobuf-dma-contig sync question

videobuf-dma-contig sync question

[Matthieu CASTET]

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:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: videobuf-dma-contig sync question

[Magnus Damm]

Hi Matthieu,

[CC Paul, Paulius]

On Wed, Apr 22, 2009 at 6:50 PM, Matthieu CASTET
<matthieu.castet@parrot.com> wrote:
> 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.

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?

> 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 ?

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:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: videobuf-dma-contig sync question

[Paul Mundt]

On Wed, Apr 22, 2009 at 08:31:28PM +0900, Magnus Damm wrote:
> On Wed, Apr 22, 2009 at 6:50 PM, Matthieu CASTET
> <matthieu.castet@parrot.com> wrote:
> > 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.
> 
> 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?
> 
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.

> 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:
> 
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:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list