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

[Ard Biesheuvel]

On Tue, 8 Feb 2022 at 13:49, Arnd Bergmann <arnd@xxxxxxxxxx> wrote:
>
> 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.
>

OK that is the least bad outcome I suppose.

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

Ah yes, ignore me. For some reason, I thought this was about on-chip SRAM.