ALSA: Rewrite hw_ptr updaters

Clean up and improve snd_pcm_update_hw_ptr*() functions.

snd_pcm_update_hw_ptr() tries to detect the unexpected hwptr jumps
more strictly to avoid the position mess-up, which often results in
the bad quality I/O with pulseaudio.

The hw-ptr skip error messages are printed when xrun proc is set to
non-zero.

Signed-off-by: Takashi Iwai <tiwai@suse.de>

1 files changed, 83 insertions, 45 deletions

diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index 921691080f35..86ac9ae9460e 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -125,19 +125,27 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram
         }
 }
 
+#ifdef CONFIG_SND_PCM_XRUN_DEBUG
+#define xrun_debug(substream)   ((substream)->pstr->xrun_debug)
+#else
+#define xrun_debug(substream)   0
+#endif
+
+#define dump_stack_on_xrun(substream) do {      \
+                if (xrun_debug(substream) > 1)  \
+                        dump_stack();           \
+        } while (0)
+
 static void xrun(struct snd_pcm_substream *substream)
 {
         snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
-#ifdef CONFIG_SND_PCM_XRUN_DEBUG
-        if (substream->pstr->xrun_debug) {
+        if (xrun_debug(substream)) {
                 snd_printd(KERN_DEBUG "XRUN: pcmC%dD%d%c\n",
                            substream->pcm->card->number,
                            substream->pcm->device,
                            substream->stream ? 'c' : 'p');
-                if (substream->pstr->xrun_debug > 1)
-                        dump_stack();
+                dump_stack_on_xrun(substream);
         }
-#endif
 }
 
 static inline snd_pcm_uframes_t snd_pcm_update_hw_ptr_pos(struct snd_pcm_substream *substream,
@@ -182,11 +190,21 @@ static inline int snd_pcm_update_hw_ptr_post(struct snd_pcm_substream *substream
         return 0;
 }
 
+#define hw_ptr_error(substream, fmt, args...)                           \
+        do {                                                            \
+                if (xrun_debug(substream)) {                            \
+                        if (printk_ratelimit()) {                       \
+                                snd_printd("hda_codec: " fmt, ##args);  \
+                        }                                               \
+                        dump_stack_on_xrun(substream);                  \
+                }                                                       \
+        } while (0)
+
 static inline int snd_pcm_update_hw_ptr_interrupt(struct snd_pcm_substream *substream)
 {
         struct snd_pcm_runtime *runtime = substream->runtime;
         snd_pcm_uframes_t pos;
-        snd_pcm_uframes_t new_hw_ptr, hw_ptr_interrupt;
+        snd_pcm_uframes_t new_hw_ptr, hw_ptr_interrupt, hw_base;
         snd_pcm_sframes_t delta;
 
         pos = snd_pcm_update_hw_ptr_pos(substream, runtime);
@@ -194,36 +212,47 @@ static inline int snd_pcm_update_hw_ptr_interrupt(struct snd_pcm_substream *subs
                 xrun(substream);
                 return -EPIPE;
         }
-        if (runtime->period_size == runtime->buffer_size)
-                goto __next_buf;
-        new_hw_ptr = runtime->hw_ptr_base + pos;
+        hw_base = runtime->hw_ptr_base;
+        new_hw_ptr = hw_base + pos;
         hw_ptr_interrupt = runtime->hw_ptr_interrupt + runtime->period_size;
-
-        delta = hw_ptr_interrupt - new_hw_ptr;
-        if (delta > 0) {
-                if ((snd_pcm_uframes_t)delta < runtime->buffer_size / 2) {
-#ifdef CONFIG_SND_PCM_XRUN_DEBUG
-                        if (runtime->periods > 1 && substream->pstr->xrun_debug) {
-                                snd_printd(KERN_ERR "Unexpected hw_pointer value [1] (stream = %i, delta: -%ld, max jitter = %ld): wrong interrupt acknowledge?\n", substream->stream, (long) delta, runtime->buffer_size / 2);
-                                if (substream->pstr->xrun_debug > 1)
-                                        dump_stack();
-                        }
-#endif
-                        return 0;
+        delta = new_hw_ptr - hw_ptr_interrupt;
+        if (hw_ptr_interrupt == runtime->boundary)
+                hw_ptr_interrupt = 0;
+        if (delta < 0) {
+                delta += runtime->buffer_size;
+                if (delta < 0) {
+                        hw_ptr_error(substream, 
+                                     "Unexpected hw_pointer value "
+                                     "(stream=%i, pos=%ld, intr_ptr=%ld)\n",
+                                     substream->stream, (long)pos,
+                                     (long)hw_ptr_interrupt);
+                        /* rebase to interrupt position */
+                        hw_base = new_hw_ptr = hw_ptr_interrupt;
+                        delta = 0;
+                } else {
+                        hw_base += runtime->buffer_size;
+                        if (hw_base == runtime->boundary)
+                                hw_base = 0;
+                        new_hw_ptr = hw_base + pos;
                 }
-              __next_buf:
-                runtime->hw_ptr_base += runtime->buffer_size;
-                if (runtime->hw_ptr_base == runtime->boundary)
-                        runtime->hw_ptr_base = 0;
-                new_hw_ptr = runtime->hw_ptr_base + pos;
         }
-
+        if (delta > runtime->period_size) {
+                hw_ptr_error(substream,
+                             "Lost interrupts? "
+                             "(stream=%i, delta=%ld, intr_ptr=%ld)\n",
+                             substream->stream, (long)delta,
+                             (long)hw_ptr_interrupt);
+                /* rebase hw_ptr_interrupt */
+                hw_ptr_interrupt =
+                        new_hw_ptr - new_hw_ptr % runtime->period_size;
+        }
         if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
             runtime->silence_size > 0)
                 snd_pcm_playback_silence(substream, new_hw_ptr);
 
+        runtime->hw_ptr_base = hw_base;
         runtime->status->hw_ptr = new_hw_ptr;
-        runtime->hw_ptr_interrupt = new_hw_ptr - new_hw_ptr % runtime->period_size;
+        runtime->hw_ptr_interrupt = hw_ptr_interrupt;
 
         return snd_pcm_update_hw_ptr_post(substream, runtime);
 }
@@ -233,7 +262,7 @@ int snd_pcm_update_hw_ptr(struct snd_pcm_substream *substream)
 {
         struct snd_pcm_runtime *runtime = substream->runtime;
         snd_pcm_uframes_t pos;
-        snd_pcm_uframes_t old_hw_ptr, new_hw_ptr;
+        snd_pcm_uframes_t old_hw_ptr, new_hw_ptr, hw_base;
         snd_pcm_sframes_t delta;
 
         old_hw_ptr = runtime->status->hw_ptr;
@@ -242,29 +271,38 @@ int snd_pcm_update_hw_ptr(struct snd_pcm_substream *substream)
                 xrun(substream);
                 return -EPIPE;
         }
-        new_hw_ptr = runtime->hw_ptr_base + pos;
-
-        delta = old_hw_ptr - new_hw_ptr;
-        if (delta > 0) {
-                if ((snd_pcm_uframes_t)delta < runtime->buffer_size / 2) {
-#ifdef CONFIG_SND_PCM_XRUN_DEBUG
-                        if (runtime->periods > 2 && substream->pstr->xrun_debug) {
-                                snd_printd(KERN_ERR "Unexpected hw_pointer value [2] (stream = %i, delta: -%ld, max jitter = %ld): wrong interrupt acknowledge?\n", substream->stream, (long) delta, runtime->buffer_size / 2);
-                                if (substream->pstr->xrun_debug > 1)
-                                        dump_stack();
-                        }
-#endif
+        hw_base = runtime->hw_ptr_base;
+        new_hw_ptr = hw_base + pos;
+
+        delta = new_hw_ptr - old_hw_ptr;
+        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",
+                                     substream->stream, (long)pos,
+                                     (long)old_hw_ptr);
                         return 0;
                 }
-                runtime->hw_ptr_base += runtime->buffer_size;
-                if (runtime->hw_ptr_base == runtime->boundary)
-                        runtime->hw_ptr_base = 0;
-                new_hw_ptr = runtime->hw_ptr_base + pos;
+                hw_base += runtime->buffer_size;
+                if (hw_base == runtime->boundary)
+                        hw_base = 0;
+                new_hw_ptr = hw_base + pos;
+        }
+        if (delta > runtime->period_size && runtime->periods > 1) {
+                hw_ptr_error(substream,
+                             "hw_ptr skipping! "
+                             "(pos=%ld, delta=%ld, period=%ld)\n",
+                             (long)pos, (long)delta,
+                             (long)runtime->period_size);
+                return 0;
         }
         if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
             runtime->silence_size > 0)
                 snd_pcm_playback_silence(substream, new_hw_ptr);
 
+        runtime->hw_ptr_base = hw_base;
         runtime->status->hw_ptr = new_hw_ptr;
 
         return snd_pcm_update_hw_ptr_post(substream, runtime);