Re: [PATCH 2/2] media: usb: pwc: Don't use coherent DMA buffers for ISO transfer

[Tomasz Figa]

On Wed, Jul 18, 2018 at 5:51 AM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, 17 Jul 2018, Ezequiel Garcia wrote:
>
> > Hi Matwey,
> >
> > First of all, sorry for the delay.
> >
> > Adding Alan and Hans. Guys, do you have any feedback here?
>
> ...
>
> > > > So, what is the benefit of using consistent
> > > > for these URBs, as opposed to streaming?
> > >
> > > I don't know, I think there is no real benefit and all we see is a
> > > consequence of copy-pasta when some webcam drivers were inspired by
> > > others and development priparily was going at x86 platforms.
> >
> > You are probably right about the copy-pasta.
> >
> > >  It would
> > > be great if somebody corrected me here. DMA Coherence is quite strong
> > > property and I cannot figure out how can it help when streaming video.
> > > The CPU host always reads from the buffer and never writes to.
> > > Hardware perepherial always writes to and never reads from. Moreover,
> > > buffer access is mutually exclusive and separated in time by Interrupt
> > > fireing and URB starting (when we reuse existing URB for new request).
> > > Only single one memory barrier is really required here.
> > >
> >
> > Yeah, and not setting URB_NO_TRANSFER_DMA_MAP makes the USB core
> > create DMA mappings and use the streaming API. Which makes more
> > sense in hardware without hardware coherency.
>
> As far as I know, the _only_ advantage to using coherent DMA in this
> situation is that you then do not have to pay the overhead of
> constantly setting up and tearing down the streaming mappings.  So it
> depends very much on the platform: If coherent buffers are cached then
> it's a slight win and otherwise it's a big lose.

Isn't it about usb_alloc_coherent() being backed by DMA coherent API
(dma_alloc_coherent/attrs()) and ending up allocating uncached (or
write-combine) memory for devices with non-coherent DMAs? I'm not sure
how this memory is used by this driver, but if it reads from it using
CPU, uncached mapping might be significantly slower than cached
mapping of memory allocated with kmalloc().

Best regards,
Tomasz