Re: [PATCH 1/3] dmaengine: xilinx: xdma: Fix the count of elapsed periods in cyclic mode

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Lizhi,

lizhi.hou@amd.com wrote on Tue, 28 Nov 2023 14:44:35 -0800:

> On 11/24/23 07:09, Miquel Raynal wrote:
> > Xilinx DMA engine is capable of keeping track of the number of elapsed
> > periods and this is an increasing 32-bit counter which is only reset
> > when turning off the engine. No need to add this value to our local
> > counter.
> >
> > Fixes: cd8c732ce1a5 ("dmaengine: xilinx: xdma: Support cyclic transfers")
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >
> > Hello, so far all my testing was performed by looping the playback
> > output to the recording input and comparing the files using
> > FFTs. Unfortunately, when the DMA engine suffers from the same issue on
> > both sides, some issues may appear un-noticed, which is likely what
> > happened here as the tooling did not report any issue while analyzing
> > the output until I actually listened to real audio now that I have in my
> > hands the relevant hardware/connectors to do so.
> > ---
> >   drivers/dma/xilinx/xdma.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
> > index 84a88029226f..75533e787414 100644
> > --- a/drivers/dma/xilinx/xdma.c
> > +++ b/drivers/dma/xilinx/xdma.c
> > @@ -754,7 +754,7 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id)
> >   	if (ret)
> >   		goto out;  
> >   > -	desc->completed_desc_num += complete_desc_num;  
> > +	desc->completed_desc_num = complete_desc_num;  
> 
> Based on PG195, completed descriptor count will be reset to 0 on rising edge of Control register Run bit. That means it resets to zero for each transaction.
> 
> This change breaks our long sg list test.

Ok, so we need act differently depending on the type of transfer (sg or
cyclic). Thanks for the feedback!

> 
> >   >   	if (desc->cyclic) {  
> >   		ret = regmap_read(xdev->rmap, xchan->base + XDMA_CHAN_STATUS,  

Miquèl