Skip to content

Commit 982b604

Browse files
Russell Kingbroonie
authored andcommitted
ASoC: kirkwood-i2s: fix DMA underruns
Stress testing the driver with multiple start/stop events causes kirkwood-dma to report underrun errors (which used to cause the kernel to lock up solidly). This is because kirkwood-i2s is not respecting the restrictions imposed on clearing the 'pause' bit. Follow what the spec says; the busy bit must be read as being clear twice before the pause bit can be released. This solves the underruns. However, it has been noticed that the busy bit occasionally does not clear itself, hence the waiting is bounded to 5ms maximum to avoid a new reason for the kernel to lockup. Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
1 parent 2424d45 commit 982b604

File tree

1 file changed

+38
-29
lines changed

1 file changed

+38
-29
lines changed

sound/soc/kirkwood/kirkwood-i2s.c

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -180,67 +180,76 @@ static int kirkwood_i2s_play_trigger(struct snd_pcm_substream *substream,
180180
int cmd, struct snd_soc_dai *dai)
181181
{
182182
struct kirkwood_dma_data *priv = snd_soc_dai_get_drvdata(dai);
183-
unsigned long value;
184-
185-
/*
186-
* specs says KIRKWOOD_PLAYCTL must be read 2 times before
187-
* changing it. So read 1 time here and 1 later.
188-
*/
189-
value = readl(priv->io + KIRKWOOD_PLAYCTL);
183+
uint32_t ctl, value;
184+
185+
ctl = readl(priv->io + KIRKWOOD_PLAYCTL);
186+
if (ctl & KIRKWOOD_PLAYCTL_PAUSE) {
187+
unsigned timeout = 5000;
188+
/*
189+
* The Armada510 spec says that if we enter pause mode, the
190+
* busy bit must be read back as clear _twice_. Make sure
191+
* we respect that otherwise we get DMA underruns.
192+
*/
193+
do {
194+
value = ctl;
195+
ctl = readl(priv->io + KIRKWOOD_PLAYCTL);
196+
if (!((ctl | value) & KIRKWOOD_PLAYCTL_PLAY_BUSY))
197+
break;
198+
udelay(1);
199+
} while (timeout--);
200+
201+
if ((ctl | value) & KIRKWOOD_PLAYCTL_PLAY_BUSY)
202+
dev_notice(dai->dev, "timed out waiting for busy to deassert: %08x\n",
203+
ctl);
204+
}
190205

191206
switch (cmd) {
192207
case SNDRV_PCM_TRIGGER_START:
193208
/* stop audio, enable interrupts */
194-
value = readl(priv->io + KIRKWOOD_PLAYCTL);
195-
value |= KIRKWOOD_PLAYCTL_PAUSE;
196-
writel(value, priv->io + KIRKWOOD_PLAYCTL);
209+
ctl |= KIRKWOOD_PLAYCTL_PAUSE;
210+
writel(ctl, priv->io + KIRKWOOD_PLAYCTL);
197211

198212
value = readl(priv->io + KIRKWOOD_INT_MASK);
199213
value |= KIRKWOOD_INT_CAUSE_PLAY_BYTES;
200214
writel(value, priv->io + KIRKWOOD_INT_MASK);
201215

202216
/* configure audio & enable i2s playback */
203-
value = readl(priv->io + KIRKWOOD_PLAYCTL);
204-
value &= ~KIRKWOOD_PLAYCTL_BURST_MASK;
205-
value &= ~(KIRKWOOD_PLAYCTL_PAUSE | KIRKWOOD_PLAYCTL_I2S_MUTE
217+
ctl &= ~KIRKWOOD_PLAYCTL_BURST_MASK;
218+
ctl &= ~(KIRKWOOD_PLAYCTL_PAUSE | KIRKWOOD_PLAYCTL_I2S_MUTE
206219
| KIRKWOOD_PLAYCTL_SPDIF_EN);
207220

208221
if (priv->burst == 32)
209-
value |= KIRKWOOD_PLAYCTL_BURST_32;
222+
ctl |= KIRKWOOD_PLAYCTL_BURST_32;
210223
else
211-
value |= KIRKWOOD_PLAYCTL_BURST_128;
212-
value |= KIRKWOOD_PLAYCTL_I2S_EN;
213-
writel(value, priv->io + KIRKWOOD_PLAYCTL);
224+
ctl |= KIRKWOOD_PLAYCTL_BURST_128;
225+
ctl |= KIRKWOOD_PLAYCTL_I2S_EN;
226+
writel(ctl, priv->io + KIRKWOOD_PLAYCTL);
214227
break;
215228

216229
case SNDRV_PCM_TRIGGER_STOP:
217230
/* stop audio, disable interrupts */
218-
value = readl(priv->io + KIRKWOOD_PLAYCTL);
219-
value |= KIRKWOOD_PLAYCTL_PAUSE | KIRKWOOD_PLAYCTL_I2S_MUTE;
220-
writel(value, priv->io + KIRKWOOD_PLAYCTL);
231+
ctl |= KIRKWOOD_PLAYCTL_PAUSE | KIRKWOOD_PLAYCTL_I2S_MUTE;
232+
writel(ctl, priv->io + KIRKWOOD_PLAYCTL);
221233

222234
value = readl(priv->io + KIRKWOOD_INT_MASK);
223235
value &= ~KIRKWOOD_INT_CAUSE_PLAY_BYTES;
224236
writel(value, priv->io + KIRKWOOD_INT_MASK);
225237

226238
/* disable all playbacks */
227-
value = readl(priv->io + KIRKWOOD_PLAYCTL);
228-
value &= ~(KIRKWOOD_PLAYCTL_I2S_EN | KIRKWOOD_PLAYCTL_SPDIF_EN);
229-
writel(value, priv->io + KIRKWOOD_PLAYCTL);
239+
ctl &= ~(KIRKWOOD_PLAYCTL_I2S_EN | KIRKWOOD_PLAYCTL_SPDIF_EN);
240+
writel(ctl, priv->io + KIRKWOOD_PLAYCTL);
230241
break;
231242

232243
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
233244
case SNDRV_PCM_TRIGGER_SUSPEND:
234-
value = readl(priv->io + KIRKWOOD_PLAYCTL);
235-
value |= KIRKWOOD_PLAYCTL_PAUSE | KIRKWOOD_PLAYCTL_I2S_MUTE;
236-
writel(value, priv->io + KIRKWOOD_PLAYCTL);
245+
ctl |= KIRKWOOD_PLAYCTL_PAUSE | KIRKWOOD_PLAYCTL_I2S_MUTE;
246+
writel(ctl, priv->io + KIRKWOOD_PLAYCTL);
237247
break;
238248

239249
case SNDRV_PCM_TRIGGER_RESUME:
240250
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
241-
value = readl(priv->io + KIRKWOOD_PLAYCTL);
242-
value &= ~(KIRKWOOD_PLAYCTL_PAUSE | KIRKWOOD_PLAYCTL_I2S_MUTE);
243-
writel(value, priv->io + KIRKWOOD_PLAYCTL);
251+
ctl &= ~(KIRKWOOD_PLAYCTL_PAUSE | KIRKWOOD_PLAYCTL_I2S_MUTE);
252+
writel(ctl, priv->io + KIRKWOOD_PLAYCTL);
244253
break;
245254

246255
default:

0 commit comments

Comments
 (0)