| Message ID | 20231124150923.257687-2-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | dmaengine: xilinx: Better cyclic transfers | expand |
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. Lizhi > > if (desc->cyclic) { > ret = regmap_read(xdev->rmap, xchan->base + XDMA_CHAN_STATUS,
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
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; if (desc->cyclic) { ret = regmap_read(xdev->rmap, xchan->base + XDMA_CHAN_STATUS,
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(-)