Re: [PATCH] [v2] ARM: sa1100/assabet: move dmabounce hack to ohci driver

[Arnd Bergmann]

On Thu, Feb 3, 2022 at 9:47 AM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> On Thu, 3 Feb 2022 at 09:38, Arnd Bergmann <arnd@xxxxxxxxxx> wrote:
>
> > There are two main downsides:
> >
> >  - rather than using a dynamically sized pool, this buffer needs
> >    to be allocated at probe time using a fixed size. Without
> >    having any idea of what it should be, I picked a size of
> >    64KB, which is between what the other two OHCI front-ends use
> >    in their SRAM. If anyone has a better idea what that size
> >    is reasonable, this can be trivially changed.
> >
>
> I suppose this is a problem if the driver falls back to ordinary DRAM
> once the allocation runs out?

>From what I can tell, there is no such fallback. If the localmem_pool
runs out, the allocation fails, which may cause other problems, but
it never falls back to the wrong DMA address.

> >  - Previously, only USB transfers to unaddressable memory needed
> >    to go through the bounce buffer, now all of them do, which may
> >    impact runtime performance for USB endpoints that do a lot of
> >    transfers.
> >
> > On the upside, the local_mem support uses write-combining buffers,
> > which should be a bit faster for transfers to the device compared to
> > normal uncached coherent memory as used in dmabounce.
> >
>
> Talking from past experience using this trick on a NXP ARM9 SoC ~10
> years ago, using on-chip SRAM for USB DMA likely results in a
> significant performance boost, even without write combining, although
> the exact scenario obviously matters.

Right, that makes sense, but it won't help here because there is
no SRAM. One detail  I noticed is that the localmem pool normally
gets mapped as WC, which is what I did in the new code as well, but
dma_alloc_flags(..., DMA_ATTR_WRITE_COMBINE) does not always
honor this flag. I think it will do it here because a GFP_KERNEL
allocation should be served by the remap_allocator, while
GFP_ATOMIC allocations would be served by pool_allocator_alloc(),
which ignores the flag.

        Arnd