Commit 982b604bc56a3da874e489051fc7adb49b1eba65

Authored by Russell King
Committed by Mark Brown
1 parent 2424d45810

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>

Showing 1 changed file with 38 additions and 29 deletions Side-by-side Diff

sound/soc/kirkwood/kirkwood-i2s.c
... ... @@ -180,67 +180,76 @@
180 180 int cmd, struct snd_soc_dai *dai)
181 181 {
182 182 struct kirkwood_dma_data *priv = snd_soc_dai_get_drvdata(dai);
183   - unsigned long value;
  183 + uint32_t ctl, value;
184 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);
  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--);
190 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 + }
  205 +
191 206 switch (cmd) {
192 207 case SNDRV_PCM_TRIGGER_START:
193 208 /* 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);
197 211  
198 212 value = readl(priv->io + KIRKWOOD_INT_MASK);
199 213 value |= KIRKWOOD_INT_CAUSE_PLAY_BYTES;
200 214 writel(value, priv->io + KIRKWOOD_INT_MASK);
201 215  
202 216 /* 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
206 219 | KIRKWOOD_PLAYCTL_SPDIF_EN);
207 220  
208 221 if (priv->burst == 32)
209   - value |= KIRKWOOD_PLAYCTL_BURST_32;
  222 + ctl |= KIRKWOOD_PLAYCTL_BURST_32;
210 223 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);
214 227 break;
215 228  
216 229 case SNDRV_PCM_TRIGGER_STOP:
217 230 /* 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);
221 233  
222 234 value = readl(priv->io + KIRKWOOD_INT_MASK);
223 235 value &= ~KIRKWOOD_INT_CAUSE_PLAY_BYTES;
224 236 writel(value, priv->io + KIRKWOOD_INT_MASK);
225 237  
226 238 /* 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);
230 241 break;
231 242  
232 243 case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
233 244 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);
237 247 break;
238 248  
239 249 case SNDRV_PCM_TRIGGER_RESUME:
240 250 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);
244 253 break;
245 254  
246 255 default: