Re: [PATCH v1] dmaengine: tegra-apb: Support per-burst residue granularity

From: Ben Dooks
Date: Wed Jun 19 2019 - 06:21:53 EST

On 19/06/2019 11:13, Jon Hunter wrote:

On 19/06/2019 11:08, Ben Dooks wrote:
On 19/06/2019 11:04, Jon Hunter wrote:

On 19/06/2019 00:27, Dmitry Osipenko wrote:
19.06.2019 1:22, Ben Dooks ÐÐÑÐÑ:
On 13/06/2019 22:08, Dmitry Osipenko wrote:
Tegra's APB DMA engine updates words counter after each transferred
of data, hence it can report transfer's residual with more fidelity
may be required in cases like audio playback. In particular this fixes
audio stuttering during playback in a chromiuim web browser. The
patch is
based on the original work that was made by Ben Dooks [1]. It was
on Tegra20 and Tegra30 devices.


Inspired-by: Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx>
Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx>
ÂÂ drivers/dma/tegra20-apb-dma.c | 35
ÂÂ 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/tegra20-apb-dma.c
index 79e9593815f1..c5af8f703548 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -797,12 +797,36 @@ static int tegra_dma_terminate_all(struct
dma_chan *dc)
ÂÂÂÂÂÂ return 0;
ÂÂ }
ÂÂ +static unsigned int tegra_dma_update_residual(struct
tegra_dma_channel *tdc,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct tegra_dma_sg_req *sg_req,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct tegra_dma_desc *dma_desc,
+ÂÂÂ unsigned long status, wcount = 0;
+ÂÂÂ if (!list_is_first(&sg_req->node, &tdc->pending_sg_req))
+ÂÂÂÂÂÂÂ return residual;
+ÂÂÂ if (tdc->tdma->chip_data->support_separate_wcount_reg)
+ÂÂÂ status = tdc_read(tdc, TEGRA_APBDMA_CHAN_STATUS);
+ÂÂÂ if (!tdc->tdma->chip_data->support_separate_wcount_reg)
+ÂÂÂÂÂÂÂ wcount = status;
+ÂÂÂÂÂÂÂ return residual - sg_req->req_len;
+ÂÂÂ return residual - get_current_xferred_count(tdc, sg_req, wcount);

I am unfortunately nowhere near my notes, so can't completely
review this. I think the complexity of my patch series is due
to an issue with the count being updated before the EOC IRQ
is actually flagged (and most definetly before it gets to the
CPU IRQ handler).

The test system I was using, which i've not really got any
access to at the moment would show these internal inconsistent
states every few hours, however it was moving 48kHz 8ch 16bit
TDM data.

Thanks for looking into this, I am not sure if I am going to
get any time to look into this within the next couple of

I'll try to add some debug checks to try to catch the case where
count is updated before EOC
is set. Thank you very much for the clarification of the problem. So
far I haven't spotted
anything going wrong.

Jon / Laxman, are you aware about the possibility to get such
inconsistency of words count
vs EOC? Assuming the cyclic transfer mode.

I can't say that I am. However, for the case of cyclic transfer, given
that the next transfer is always programmed into the registers before
the last one completes, I could see that by the time the interrupt is
serviced that the DMA has moved on to the next transfer (which I assume
would reset the count).

Interestingly, our downstream kernel implemented a change to avoid the
count appearing to move backwards. I am curious if this also works,
which would be a lot simpler that what Ben has implemented and may
mitigate that race condition that Ben is describing.

That might be the same thing we saw. IIRC it looks like the DMA has
moved on, but the count gets re-set before the EOC? I can't see that
git site so can't comment.

That's odd, that should be a public site AFAIK. Does the following not
work ...


Getting ssl errors which i think is causing a firewall issue here.

I can try again when back at the hotel instead of client site.

Ben Dooks
Senior Engineer Codethink - Providing Genius