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

[Dmitry Osipenko]

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.