[ALSA] pcm-midlevel: Add more strict buffer position checks based on jiffies

Some drivers like Intel8x0 or Intel HDA are broken for some hardware variants. This patch adds more strict buffer position checks based on jiffies when internal hw_ptr is updated. Enable xrun_debug to see mangling of wrong positions. As a side effect, the hw_ptr interrupt update routine might do slightly better job when many interrupts are lost. Signed-off-by: Jaroslav Kysela <perex@perex.cz>

[ALSA] pcm-midlevel: Add more strict buffer position checks based on jiffies
Some drivers like Intel8x0 or Intel HDA are broken for some hardware variants. This patch adds more strict buffer position checks based on jiffies when internal hw_ptr is updated. Enable xrun_debug to see mangling of wrong positions. As a side effect, the hw_ptr interrupt update routine might do slightly better job when many interrupts are lost. Signed-off-by: Jaroslav Kysela <perex@perex.cz>
Jaroslav Kysela
1 parent fa00e046b4
Showing 2 changed files with 41 additions and 9 deletions Side-by-side Diff
include/sound/pcm.h
sound/core/pcm_lib.c
@@ -268,7 +268,8 @@
 	int overrange;
 	snd_pcm_uframes_t avail_max;
 	snd_pcm_uframes_t hw_ptr_base;	/* Position at buffer restart */
-	snd_pcm_uframes_t hw_ptr_interrupt; /* Position at interrupt time*/
+	snd_pcm_uframes_t hw_ptr_interrupt; /* Position at interrupt time */
+	unsigned long hw_ptr_jiffies;	/* Time when hw_ptr is updated */
  
 	/* -- HW params -- */
 	snd_pcm_access_t access;	/* access mode */
@@ -209,9 +209,11 @@
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	snd_pcm_uframes_t pos;
-	snd_pcm_uframes_t new_hw_ptr, hw_ptr_interrupt, hw_base;
-	snd_pcm_sframes_t delta;
+	snd_pcm_uframes_t old_hw_ptr, new_hw_ptr, hw_ptr_interrupt, hw_base;
+	snd_pcm_sframes_t hdelta, delta;
+	unsigned long jdelta;
  
+	old_hw_ptr = runtime->status->hw_ptr;
 	pos = snd_pcm_update_hw_ptr_pos(substream, runtime);
 	if (pos == SNDRV_PCM_POS_XRUN) {
 		xrun(substream);
  
@@ -247,8 +249,31 @@
 			new_hw_ptr = hw_base + pos;
 		}
 	}
-	if (delta > runtime->period_size) {
+	hdelta = new_hw_ptr - old_hw_ptr;
+	jdelta = jiffies - runtime->hw_ptr_jiffies;
+	if (((hdelta * HZ) / runtime->rate) > jdelta + HZ/100) {
+		delta = jdelta /
+			(((runtime->period_size * HZ) / runtime->rate)
+								+ HZ/100);
 		hw_ptr_error(substream,
+			     "hw_ptr skipping! [Q] "
+			     "(pos=%ld, delta=%ld, period=%ld, "
+			     "jdelta=%lu/%lu/%lu)\n",
+			     (long)pos, (long)hdelta,
+			     (long)runtime->period_size, jdelta,
+			     ((hdelta * HZ) / runtime->rate), delta);
+		hw_ptr_interrupt = runtime->hw_ptr_interrupt +
+				   runtime->period_size * delta;
+		if (hw_ptr_interrupt >= runtime->boundary)
+			hw_ptr_interrupt -= runtime->boundary;
+		/* rebase to interrupt position */
+		hw_base = new_hw_ptr = hw_ptr_interrupt;
+		/* align hw_base to buffer_size */
+		hw_base -= hw_base % runtime->buffer_size;
+		delta = 0;
+	}
+	if (delta > runtime->period_size + runtime->period_size / 2) {
+		hw_ptr_error(substream,
 			     "Lost interrupts? "
 			     "(stream=%i, delta=%ld, intr_ptr=%ld)\n",
 			     substream->stream, (long)delta,
@@ -263,6 +288,7 @@
  
 	runtime->hw_ptr_base = hw_base;
 	runtime->status->hw_ptr = new_hw_ptr;
+	runtime->hw_ptr_jiffies = jiffies;
 	runtime->hw_ptr_interrupt = hw_ptr_interrupt;
  
 	return snd_pcm_update_hw_ptr_post(substream, runtime);
@@ -275,6 +301,7 @@
 	snd_pcm_uframes_t pos;
 	snd_pcm_uframes_t old_hw_ptr, new_hw_ptr, hw_base;
 	snd_pcm_sframes_t delta;
+	unsigned long jdelta;
  
 	old_hw_ptr = runtime->status->hw_ptr;
 	pos = snd_pcm_update_hw_ptr_pos(substream, runtime);
  
  
@@ -286,14 +313,15 @@
 	new_hw_ptr = hw_base + pos;
  
 	delta = new_hw_ptr - old_hw_ptr;
+	jdelta = jiffies - runtime->hw_ptr_jiffies;
 	if (delta < 0) {
 		delta += runtime->buffer_size;
 		if (delta < 0) {
 			hw_ptr_error(substream, 
 				     "Unexpected hw_pointer value [2] "
-				     "(stream=%i, pos=%ld, old_ptr=%ld)\n",
+				     "(stream=%i, pos=%ld, old_ptr=%ld, jdelta=%li)\n",
 				     substream->stream, (long)pos,
-				     (long)old_hw_ptr);
+				     (long)old_hw_ptr, jdelta);
 			return 0;
 		}
 		hw_base += runtime->buffer_size;
  
  
@@ -301,12 +329,13 @@
 			hw_base = 0;
 		new_hw_ptr = hw_base + pos;
 	}
-	if (delta > runtime->period_size && runtime->periods > 1) {
+	if (((delta * HZ) / runtime->rate) > jdelta + HZ/100) {
 		hw_ptr_error(substream,
 			     "hw_ptr skipping! "
-			     "(pos=%ld, delta=%ld, period=%ld)\n",
+			     "(pos=%ld, delta=%ld, period=%ld, jdelta=%lu/%lu)\n",
 			     (long)pos, (long)delta,
-			     (long)runtime->period_size);
+			     (long)runtime->period_size, jdelta,
+			     ((delta * HZ) / runtime->rate));
 		return 0;
 	}
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
@@ -315,6 +344,7 @@
  
 	runtime->hw_ptr_base = hw_base;
 	runtime->status->hw_ptr = new_hw_ptr;
+	runtime->hw_ptr_jiffies = jiffies;
  
 	return snd_pcm_update_hw_ptr_post(substream, runtime);
 }
@@ -1441,6 +1471,7 @@
 		runtime->status->hw_ptr %= runtime->buffer_size;
 	else
 		runtime->status->hw_ptr = 0;
+	runtime->hw_ptr_jiffies = jiffies;
 	snd_pcm_stream_unlock_irqrestore(substream, flags);
 	return 0;
 }
...	...	@@ -268,7 +268,8 @@
268	268	int overrange;
269	269	snd_pcm_uframes_t avail_max;
270	270	snd_pcm_uframes_t hw_ptr_base; /* Position at buffer restart */
271		- snd_pcm_uframes_t hw_ptr_interrupt; /* Position at interrupt time*/
	271	+ snd_pcm_uframes_t hw_ptr_interrupt; /* Position at interrupt time */
	272	+ unsigned long hw_ptr_jiffies; /* Time when hw_ptr is updated */
272	273
273	274	/* -- HW params -- */
274	275	snd_pcm_access_t access; /* access mode */
...	...	@@ -209,9 +209,11 @@
209	209	{
210	210	struct snd_pcm_runtime *runtime = substream->runtime;
211	211	snd_pcm_uframes_t pos;
212		- snd_pcm_uframes_t new_hw_ptr, hw_ptr_interrupt, hw_base;
213		- snd_pcm_sframes_t delta;
	212	+ snd_pcm_uframes_t old_hw_ptr, new_hw_ptr, hw_ptr_interrupt, hw_base;
	213	+ snd_pcm_sframes_t hdelta, delta;
	214	+ unsigned long jdelta;
214	215
	216	+ old_hw_ptr = runtime->status->hw_ptr;
215	217	pos = snd_pcm_update_hw_ptr_pos(substream, runtime);
216	218	if (pos == SNDRV_PCM_POS_XRUN) {
217	219	xrun(substream);
218	220
...	...	@@ -247,8 +249,31 @@
247	249	new_hw_ptr = hw_base + pos;
248	250	}
249	251	}
250		- if (delta > runtime->period_size) {
	252	+ hdelta = new_hw_ptr - old_hw_ptr;
	253	+ jdelta = jiffies - runtime->hw_ptr_jiffies;
	254	+ if (((hdelta * HZ) / runtime->rate) > jdelta + HZ/100) {
	255	+ delta = jdelta /
	256	+ (((runtime->period_size * HZ) / runtime->rate)
	257	+ + HZ/100);
251	258	hw_ptr_error(substream,
	259	+ "hw_ptr skipping! [Q] "
	260	+ "(pos=%ld, delta=%ld, period=%ld, "
	261	+ "jdelta=%lu/%lu/%lu)\n",
	262	+ (long)pos, (long)hdelta,
	263	+ (long)runtime->period_size, jdelta,
	264	+ ((hdelta * HZ) / runtime->rate), delta);
	265	+ hw_ptr_interrupt = runtime->hw_ptr_interrupt +
	266	+ runtime->period_size * delta;
	267	+ if (hw_ptr_interrupt >= runtime->boundary)
	268	+ hw_ptr_interrupt -= runtime->boundary;
	269	+ /* rebase to interrupt position */
	270	+ hw_base = new_hw_ptr = hw_ptr_interrupt;
	271	+ /* align hw_base to buffer_size */
	272	+ hw_base -= hw_base % runtime->buffer_size;
	273	+ delta = 0;
	274	+ }
	275	+ if (delta > runtime->period_size + runtime->period_size / 2) {
	276	+ hw_ptr_error(substream,
252	277	"Lost interrupts? "
253	278	"(stream=%i, delta=%ld, intr_ptr=%ld)\n",
254	279	substream->stream, (long)delta,
...	...	@@ -263,6 +288,7 @@
263	288
264	289	runtime->hw_ptr_base = hw_base;
265	290	runtime->status->hw_ptr = new_hw_ptr;
	291	+ runtime->hw_ptr_jiffies = jiffies;
266	292	runtime->hw_ptr_interrupt = hw_ptr_interrupt;
267	293
268	294	return snd_pcm_update_hw_ptr_post(substream, runtime);
...	...	@@ -275,6 +301,7 @@
275	301	snd_pcm_uframes_t pos;
276	302	snd_pcm_uframes_t old_hw_ptr, new_hw_ptr, hw_base;
277	303	snd_pcm_sframes_t delta;
	304	+ unsigned long jdelta;
278	305
279	306	old_hw_ptr = runtime->status->hw_ptr;
280	307	pos = snd_pcm_update_hw_ptr_pos(substream, runtime);
281	308
282	309
...	...	@@ -286,14 +313,15 @@
286	313	new_hw_ptr = hw_base + pos;
287	314
288	315	delta = new_hw_ptr - old_hw_ptr;
	316	+ jdelta = jiffies - runtime->hw_ptr_jiffies;
289	317	if (delta < 0) {
290	318	delta += runtime->buffer_size;
291	319	if (delta < 0) {
292	320	hw_ptr_error(substream,
293	321	"Unexpected hw_pointer value [2] "
294		- "(stream=%i, pos=%ld, old_ptr=%ld)\n",
	322	+ "(stream=%i, pos=%ld, old_ptr=%ld, jdelta=%li)\n",
295	323	substream->stream, (long)pos,
296		- (long)old_hw_ptr);
	324	+ (long)old_hw_ptr, jdelta);
297	325	return 0;
298	326	}
299	327	hw_base += runtime->buffer_size;
300	328
301	329
...	...	@@ -301,12 +329,13 @@
301	329	hw_base = 0;
302	330	new_hw_ptr = hw_base + pos;
303	331	}
304		- if (delta > runtime->period_size && runtime->periods > 1) {
	332	+ if (((delta * HZ) / runtime->rate) > jdelta + HZ/100) {
305	333	hw_ptr_error(substream,
306	334	"hw_ptr skipping! "
307		- "(pos=%ld, delta=%ld, period=%ld)\n",
	335	+ "(pos=%ld, delta=%ld, period=%ld, jdelta=%lu/%lu)\n",
308	336	(long)pos, (long)delta,
309		- (long)runtime->period_size);
	337	+ (long)runtime->period_size, jdelta,
	338	+ ((delta * HZ) / runtime->rate));
310	339	return 0;
311	340	}
312	341	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
...	...	@@ -315,6 +344,7 @@
315	344
316	345	runtime->hw_ptr_base = hw_base;
317	346	runtime->status->hw_ptr = new_hw_ptr;
	347	+ runtime->hw_ptr_jiffies = jiffies;
318	348
319	349	return snd_pcm_update_hw_ptr_post(substream, runtime);
320	350	}
...	...	@@ -1441,6 +1471,7 @@
1441	1471	runtime->status->hw_ptr %= runtime->buffer_size;
1442	1472	else
1443	1473	runtime->status->hw_ptr = 0;
	1474	+ runtime->hw_ptr_jiffies = jiffies;
1444	1475	snd_pcm_stream_unlock_irqrestore(substream, flags);
1445	1476	return 0;
1446	1477	}