[RFT PATCH] dmaengine: Fix choppy sound because of unimplemented resume

[RFT PATCH] dmaengine: Fix choppy sound because of unimplemented resume

[Krzysztof Kozlowski]

Some drivers implement only pause operation (no resuming). Example is
pl330 where pause is needed for getting residuum. pl330 does not support
resume operation, transfer must be stopped after pause.

However for slaves this is exposed always as "pause and resume" which
introduces subtle errors on Odroid U3 board (Exynos4412 with pl330).
After adding pause function to pl330 driver the audio playback
(utilizing DMA) gets choppy after some time (approximately 24 hours).

Fix this by exposing "cmd_pause" if and only if pause and resume are
implemented.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reported-by: gabriel@unseen.is
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: <stable@vger.kernel.org>
Fixes: 88987d2c7534 ("dmaengine: pl330: add DMA_PAUSE feature")

---

Thanks for Marek Szyprowski for nailing the issue.

Few drivers implement only pause (pl330, nbpfaxi, ipu_idmac). The fix
was not extensively tested.

I tested this only on pl330 with dmatest which actually does not pause
the transfer.  I am kindly asking for testing for other use cases,
especially UART transfer and sound playback. Testing on these two other
devices (nbpfaxi and ipu_idmac) could be useful as well.
---
 drivers/dma/dmaengine.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 855dd6ccf09d..8208abe4e1d3 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -494,7 +494,11 @@ int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps)
 	caps->directions = device->directions;
 	caps->residue_granularity = device->residue_granularity;
 
-	caps->cmd_pause = !!device->device_pause;
+	/*
+	 * Some devices implement only pause (e.g. to get residuum) but no
+	 * resume. However cmd_pause is advertised as pause AND resume.
+	 */
+	caps->cmd_pause = !!(device->device_pause && device->device_resume);
 	caps->cmd_terminate = !!device->device_terminate_all;
 
 	return 0;
-- 
1.9.1

Re: [RFT PATCH] dmaengine: Fix choppy sound because of unimplemented resume

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]


On Wed, Jun 10, 2015 at 05:17:07PM +0900, Krzysztof Kozlowski wrote:
> Some drivers implement only pause operation (no resuming). Example is
> pl330 where pause is needed for getting residuum. pl330 does not support
> resume operation, transfer must be stopped after pause.
> 
> However for slaves this is exposed always as "pause and resume" which
> introduces subtle errors on Odroid U3 board (Exynos4412 with pl330).
> After adding pause function to pl330 driver the audio playback
> (utilizing DMA) gets choppy after some time (approximately 24 hours).
> 
> Fix this by exposing "cmd_pause" if and only if pause and resume are
> implemented.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Reported-by: gabriel@unseen.is
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: <stable@vger.kernel.org>
> Fixes: 88987d2c7534 ("dmaengine: pl330: add DMA_PAUSE feature")

That looks reasonable.

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks! (and sorry for that)
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFT PATCH] dmaengine: Fix choppy sound because of unimplemented resume

[Vinod Koul]

On Wed, Jun 10, 2015 at 05:17:07PM +0900, Krzysztof Kozlowski wrote:
> Some drivers implement only pause operation (no resuming). Example is
> pl330 where pause is needed for getting residuum. pl330 does not support
> resume operation, transfer must be stopped after pause.
> 
> However for slaves this is exposed always as "pause and resume" which
> introduces subtle errors on Odroid U3 board (Exynos4412 with pl330).
> After adding pause function to pl330 driver the audio playback
> (utilizing DMA) gets choppy after some time (approximately 24 hours).
> 
> Fix this by exposing "cmd_pause" if and only if pause and resume are
> implemented.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Reported-by: gabriel@unseen.is
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: <stable@vger.kernel.org>
> Fixes: 88987d2c7534 ("dmaengine: pl330: add DMA_PAUSE feature")

Applied, thanks
-- 
~Vinod