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 >>>>> burst >>>>> of data, hence it can report transfer's residual with more fidelity >>>>> which >>>>> 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 >>>>> tested >>>>> on Tegra20 and Tegra30 devices. >>>>> >>>>> [1] >>>>> https://lore.kernel.org/lkml/20190424162348.23692-1-ben.dooks@xxxxxxxxxxxxxxx/ >>>>> >>>>> >>>>> 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 >>>>> b/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 int residual) >>>>> +{ >>>>> + 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) >>>>> + wcount = tdc_read(tdc, TEGRA_APBDMA_CHAN_WORD_TRANSFER); >>>>> + >>>>> + status = tdc_read(tdc, TEGRA_APBDMA_CHAN_STATUS); >>>>> + >>>>> + if (!tdc->tdma->chip_data->support_separate_wcount_reg) >>>>> + wcount = status; >>>>> + >>>>> + if (status & TEGRA_APBDMA_STATUS_ISE_EOC) >>>>> + 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 >>>> months. >>> >>> 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 ... https://nv-tegra.nvidia.com/gitweb/ Jon -- nvpublic
