Re: [PATCH v2 1/2] dmaengine: sun6i: fix non-atomic read of DMA position registers

On Fri, Aug 14, 2026 at 04:21:10PM +0200, Christian Lugnberg wrote:
>
> sun6i_get_chan_size() reads DMA_CHAN_LLI_ADDR and DMA_CHAN_CUR_CNT in two
> separate readl() calls with no synchronisation between them:
>
>     pos   = readl(pchan->base + DMA_CHAN_LLI_ADDR);
>     bytes = readl(pchan->base + DMA_CHAN_CUR_CNT);
>
> DMA_CHAN_LLI_ADDR holds the physical address of the *next* descriptor the
> engine will load once the current one completes. DMA_CHAN_CUR_CNT holds the
> remaining byte count for the *current* descriptor. If the DMA engine
> advances to the next LLI entry between the two reads, pos becomes stale: it
> still points to what was the next descriptor at the time of the first read,
> but that descriptor is now the current one and CUR_CNT reflects its initial
> (full) byte count. The subsequent virtual-chain walk starts one entry too
> early and accumulates an extra full period's worth of bytes into the
> residue estimate.

Thanks for this fix. This common problem, above already clean enough.

please cut below debug/test proccess. and keep

Fix this by re-reading DMA_CHAN_LLI_ADDR after DMA_CHAN_CUR_CNT and
retrying if the value changed. This double-read pattern guarantees that
both registers were sampled during the same descriptor interval.

Frank

>
> For ALSA cyclic buffers the over-counted residue can reach the full buffer
> size, causing the computed playback position to appear to jump backward to
> near zero. The ALSA PCM core treats such a backward discontinuity in hw_ptr
> as evidence that the buffer has underrun and declares an xrun.
>
> On the Barix IPAM400 (Allwinner H3, kernel 6.12) this manifests as audible
> glitches accompanied by spurious xrun log entries, confirmed by two
> independent observations:
>
> First, the ALSA buffer in the affected configuration is 2 seconds deep with
> a 500 ms refill period (the interval at which the player software wakes up
> to top up the buffer). For a real underrun to occur the player would have
> to stall for the full 2 seconds without writing any audio — effectively
> impossible under normal scheduling conditions. Yet xruns are observed
> regularly.
>
> Second, the underrun duration reported by the kernel at xrun time is
> ~30 µs, roughly one audio sample at 44100 Hz. A genuine drain of a 2
> second buffer cannot resolve in 30 µs; only a phantom position jump
> caused by a register read race can produce such a number.
>
> Observed on a 44100 Hz stereo S16_LE stream:
>
>   $ cat /proc/asound/Codec/pcm0p/sub0/status
>   state: XRUN
>   delay: 0
>   avail: 88200
>   avail_max: 22514
>
> The avail_max of 22514 frames (511 ms) matches exactly one ALSA period —
> the amount added by starting the LLI chain walk one entry too early.
>
> The race window itself is narrow. Each DMA descriptor covers approximately
> 88 samples (~2 ms at 44100 Hz), so the engine advances to a new descriptor
> roughly every 2 ms. The two readl() calls must straddle that exact boundary
> for the corruption to occur, which explains why the bug is intermittent.
>
> The bug is further confirmed by the xrun_debug bit 2 toggle (jiffies
> position validation). With it enabled xruns cease immediately and do not
> return; clearing it causes xruns to reappear within minutes. This on/off
> reproducibility isolates the fault to the hw_ptr position reporting path;
> the DMA engine itself is functioning correctly, as evidenced by hw_ptr
> advancing at a steady 44100 frames/sec between events:
>
>   $ echo 4 > /proc/asound/Codec/pcm0p/xrun_debug  # xruns stop
>   $ echo 0 > /proc/asound/Codec/pcm0p/xrun_debug  # xruns return
>


> Fix this by re-reading DMA_CHAN_LLI_ADDR after DMA_CHAN_CUR_CNT and
> retrying if the value changed. This double-read pattern guarantees that
> both registers were sampled during the same descriptor interval. The cost
> is at most one extra readl() pair per call in the racy case, which occurs
> only at descriptor boundaries (~every 2 ms) and is negligible.
>
> Fixes: a90e173f3faf ("dmaengine: sun6i: Add cyclic capability")
> Cc: [email protected]
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-off-by: Christian Lugnberg <[email protected]>
> ---
>  drivers/dma/sun6i-dma.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index f47a326dd7ff..04fe1f5042e9 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -354,8 +354,10 @@ static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)
>         size_t bytes;
>         dma_addr_t pos;
>
> -       pos = readl(pchan->base + DMA_CHAN_LLI_ADDR);
> -       bytes = readl(pchan->base + DMA_CHAN_CUR_CNT);
> +       do {
> +               pos = readl(pchan->base + DMA_CHAN_LLI_ADDR);
> +               bytes = readl(pchan->base + DMA_CHAN_CUR_CNT);
> +       } while (pos != readl(pchan->base + DMA_CHAN_LLI_ADDR));
>
>         if (pos == LLI_LAST_ITEM)
>                 return bytes;
> --
> 2.54.0 (Apple Git-156)
>