Re: Revert "dmaengine: pl330: add DMA_PAUSE feature"

[Vinod Koul]

On 08-05-18, 10:36, Frank Mori Hess wrote:
> On Tue, May 8, 2018 at 5:04 AM, Marek Szyprowski
> <m.szyprowski@xxxxxxxxxxx> wrote:
> > Hi Frank and Vinod,
> >
> > On 2018-04-28 23:50, Frank Mori Hess wrote:
> >> This reverts commit 88987d2c7534a0269f567fb101e6d71a08f0f01d.
> >>
> >> The pl330.c pause implementation violates the dmaengine requirement
> >> for no data loss, since it relies on the DMAKILL
> >> instruction.  However, DMAKILL discards in-flight data from the
> >> dma controller's fifo.  This is documented in the dma-330 manual
> >> and I have observed it with hardware doing device-to-memory burst
> >> transfers.  The discarded data may or may not show up in the
> >> residue count, depending on timing (resulting in data corruption
> >> effectively).
> >>
> >> Signed-off-by: Frank Mori Hess <fmh6jj@xxxxxxxxx>
> >
> > This revert completely breaks serial driver operation on almost all Exynos
> > SoCs, because serial driver relies on having PAUSE feature and proper
> > residue reporting from dma engine. Please drop it if possible.

Hi Marek,

I would appreciate if you can review the pl330 changes as that clearly seems to
impact you. This was in review for quite a bit

> It will cause the serial driver to not use the pl330.c driver for dma,
> the serial driver will fall back on using the cpu.  This is
> unfortunate, but the dma hardware simply does not support pause.  The
> "nice" stop instruction DMAEND is not allowed to be inserted using the
> debug instruction register.  The only possibility for implementing
> pause would be to make the dma transfer do a DMAWFE (wait for event)
> before every transfer.  Then you would need to devote another dma
> thread to doing nothing but DMASEV (send event) to keep the transfer
> going.  The pause could then DMAKILL the event-generating thread
> rather than the transfer thread.  I don't know exactly what the
> performance impact would be, but it couldn't be good.
> 
> The serial driver could be modified to still use dma for TX, since it
> only needs pause for RX.  Also, if your serial hardware can report
> exactly how many bytes it has sitting in its rx fifo, the serial
> driver could be modified to use pause-less dma for RX.  This is
> actually what I did for the custom serial hardware I'm using with a
> dma-330, although our serial hardware has a very large rx fifo which
> makes this scheme worthwhile.

That makes sense to me. If dma doesnt support, then why should SW claim broken
support..

-- 
~Vinod