diff mbox series

ALSA: hda: Reset stream if DMA RUN bit not cleared

Message ID 20200128051508.26064-1-mkumard@nvidia.com
State New
Headers show
Series ALSA: hda: Reset stream if DMA RUN bit not cleared | expand

Commit Message

Mohan Kumar Jan. 28, 2020, 5:15 a.m. UTC
Tegra HDA has FIFO size which can hold upto 10 audio frames to support
DVFS. When HDA DMA RUN bit is set to 0 to stop the stream, the DMA RUN
bit will be cleared to 0 only after transferring all the remaining audio
frames queued up in the fifo. This is not in sync with spec which states
the controller will stop transmitting(output) in the beginning of the
next frame for the relevant stream.

The above behavior with Tegra HDA was resulting in machine check error
during the system suspend flow with active audio playback with below kernel
error logs.
[ 33.524583] mc-err: [mcerr] (hda) csr_hdar: EMEM address decode error
[ 33.531088] mc-err: [mcerr] status = 0x20000015; addr = 0x00000000
[ 33.537431] mc-err: [mcerr] secure: no, access-type: read, SMMU fault: none

This was due to the fifo has more than one audio frame when the DMA
RUN bit is set to 0 during system suspend flow and the timeout handling in
snd_hdac_stream_sync() was not designed to handle this scenario. So the
DMA will continue running even after timeout hit until all remaining
audio frames in the fifo are transferred, but the suspend flow will try
to reset the controller and turn off the hda clocks without the knowledge
of the DMA is still running and could result in mc-err.

The above issue can be resolved by doing stream reset with the help of
snd_hdac_stream_reset() which would ensure the DMA RUN bit is cleared
if the timeout was hit in snd_hdac_stream_sync().

Signed-off-by: Mohan Kumar <mkumard@nvidia.com>
---
 sound/hda/hdac_stream.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

Comments

Takashi Iwai Jan. 28, 2020, 9:50 a.m. UTC | #1
On Tue, 28 Jan 2020 06:15:08 +0100,
Mohan Kumar wrote:
> 
> Tegra HDA has FIFO size which can hold upto 10 audio frames to support
> DVFS. When HDA DMA RUN bit is set to 0 to stop the stream, the DMA RUN
> bit will be cleared to 0 only after transferring all the remaining audio
> frames queued up in the fifo. This is not in sync with spec which states
> the controller will stop transmitting(output) in the beginning of the
> next frame for the relevant stream.
> 
> The above behavior with Tegra HDA was resulting in machine check error
> during the system suspend flow with active audio playback with below kernel
> error logs.
> [ 33.524583] mc-err: [mcerr] (hda) csr_hdar: EMEM address decode error
> [ 33.531088] mc-err: [mcerr] status = 0x20000015; addr = 0x00000000
> [ 33.537431] mc-err: [mcerr] secure: no, access-type: read, SMMU fault: none
> 
> This was due to the fifo has more than one audio frame when the DMA
> RUN bit is set to 0 during system suspend flow and the timeout handling in
> snd_hdac_stream_sync() was not designed to handle this scenario. So the
> DMA will continue running even after timeout hit until all remaining
> audio frames in the fifo are transferred, but the suspend flow will try
> to reset the controller and turn off the hda clocks without the knowledge
> of the DMA is still running and could result in mc-err.
> 
> The above issue can be resolved by doing stream reset with the help of
> snd_hdac_stream_reset() which would ensure the DMA RUN bit is cleared
> if the timeout was hit in snd_hdac_stream_sync().
> 
> Signed-off-by: Mohan Kumar <mkumard@nvidia.com>

Applied now.  Thanks.


Takashi
diff mbox series

Patch

diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
index 682ed39f79b0..890ff1b7a878 100644
--- a/sound/hda/hdac_stream.c
+++ b/sound/hda/hdac_stream.c
@@ -629,20 +629,27 @@  void snd_hdac_stream_sync(struct hdac_stream *azx_dev, bool start,
 		nwait = 0;
 		i = 0;
 		list_for_each_entry(s, &bus->stream_list, list) {
-			if (streams & (1 << i)) {
-				if (start) {
-					/* check FIFO gets ready */
-					if (!(snd_hdac_stream_readb(s, SD_STS) &
-					      SD_STS_FIFO_READY))
-						nwait++;
-				} else {
-					/* check RUN bit is cleared */
-					if (snd_hdac_stream_readb(s, SD_CTL) &
-					    SD_CTL_DMA_START)
-						nwait++;
+			if (!(streams & (1 << i++)))
+				continue;
+
+			if (start) {
+				/* check FIFO gets ready */
+				if (!(snd_hdac_stream_readb(s, SD_STS) &
+				      SD_STS_FIFO_READY))
+					nwait++;
+			} else {
+				/* check RUN bit is cleared */
+				if (snd_hdac_stream_readb(s, SD_CTL) &
+				    SD_CTL_DMA_START) {
+					nwait++;
+					/*
+					 * Perform stream reset if DMA RUN
+					 * bit not cleared within given timeout
+					 */
+					if (timeout == 1)
+						snd_hdac_stream_reset(s);
 				}
 			}
-			i++;
 		}
 		if (!nwait)
 			break;