Hi Kristen,
Fengguang developped two patches for audio driver to help fixing bug BOO
#6988.
alsa-pointer-trace.patch adds the trace functionality to help show
how the app_ptr and hw_ptr goes. This will help users understand what's
going on under the hood.
alsa-mrst-smooth-hw-pointer.patch, as shown in the changelog, smoothes
the hardware pointer reported from audio firmware, so that it can help
reduce the problem of jitter and drift.
We've tested these two patches locally and it works fine. Could you
please review it and accept it in MRST patchset?
Thanks!
--
guanqun
Subject: ALSA: intel sst - smooth hardware pointer
Date: Mon Jun 27 15:45:33 CST 2011
The raw hardware pointer has jitters (5ms for current firmware and
~100ms for new firmware to be released soon).
This provides smooth hardware pointer to the upper layer based on the
raw hardware pointers reported at interrupt times. It's based on jiffies,
so can provide 1ms smoothness on a HZ=1000 system.
Signed-off-by: Wu Fengguang <fengguang...@intel.com>
---
drivers/staging/intel_sst/intel_sst.h | 3 +
drivers/staging/intel_sst/intelmid.c | 63 +++++++++++++++++++++++-
2 files changed, 64 insertions(+), 2 deletions(-)
--- linux-2.6.37.orig/drivers/staging/intel_sst/intelmid.c 2011-06-28
16:38:12.676000012 +0800
+++ linux-2.6.37/drivers/staging/intel_sst/intelmid.c 2011-07-01
16:57:20.748000009 +0800
@@ -125,6 +125,7 @@ static int snd_intelmad_pcm_trigger(stru
WARN_ON(!intelmaddata->sstdrv_ops->scard_ops);
sst_ops = intelmaddata->sstdrv_ops->pcm_control;
str_id = stream->stream_info.str_id;
+ stream->stream_info.base_ptr_jiffies = 0;
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
@@ -134,6 +135,8 @@ static int snd_intelmad_pcm_trigger(stru
return ret_val;
stream->stream_status = RUNNING;
stream->substream = substream;
+ stream->stream_info.last_ptr = ~0UL;
+ stream->stream_info.base_ptr = ~0UL;
break;
case SNDRV_PCM_TRIGGER_STOP:
pr_debug("sst: in stop\n");
@@ -232,6 +235,29 @@ static int snd_intelmad_hw_free(struct s
return snd_pcm_lib_free_pages(substream);
}
+#define DEUBUG_POINTER_TRACE 0
+static snd_pcm_uframes_t smooth_pointer(struct snd_pcm_runtime *runtime,
+ struct pcm_stream_info *info)
+{
+ unsigned long jdelta;
+ unsigned long fdelta;
+
+ jdelta = jiffies - info->base_ptr_jiffies;
+ fdelta = jdelta * runtime->rate / HZ;
+
+ info->last_ptr = info->base_ptr + fdelta;
+ info->last_ptr %= runtime->buffer_size;
+
+ if (DEUBUG_POINTER_TRACE)
+ trace_printk("buffer_ptr=%llu base_ptr=%lu delta=%lu ret=%lu\n",
+ info->buffer_ptr,
+ info->base_ptr,
+ jdelta * 1000 / HZ,
+ info->last_ptr);
+
+ return info->last_ptr;
+}
+
/**
* snd_intelmad_pcm_pointer- to send the current buffer pointer processed by hw
*
@@ -243,9 +269,12 @@ static int snd_intelmad_hw_free(struct s
static snd_pcm_uframes_t snd_intelmad_pcm_pointer
(struct snd_pcm_substream *substream)
{
- /* struct snd_pcm_runtime *runtime = substream->runtime; */
+ struct snd_pcm_runtime *runtime = substream->runtime;
struct mad_stream_pvt *stream;
struct snd_intelmad *intelmaddata;
+ struct pcm_stream_info *info;
+ unsigned long last;
+ unsigned long ptr;
int ret_val;
WARN_ON(!substream);
@@ -254,6 +283,7 @@ static snd_pcm_uframes_t snd_intelmad_pc
stream = substream->runtime->private_data;
if (stream->stream_status == INIT)
return 0;
+ info = &stream->stream_info;
ret_val = intelmaddata->sstdrv_ops->pcm_control->device_control(
SST_SND_BUFFER_POINTER, &stream->stream_info);
@@ -267,7 +297,36 @@ static snd_pcm_uframes_t snd_intelmad_pc
(int)substream->runtime->frame_bits,
(int)substream->runtime->period_size);
- return stream->stream_info.buffer_ptr;
+ ptr = info->buffer_ptr;
+
+ if (!info->base_ptr_jiffies)
+ goto out;
+
+ /* pointers collected at non-periodic time are inaccurate */
+ if (!in_interrupt())
+ return smooth_pointer(runtime, info);
+
+ /* the pointer may never drop */
+ last = info->last_ptr;
+ if (last < ptr)
+ last += runtime->buffer_size;
+ if (last - ptr < runtime->buffer_size / 4)
+ return smooth_pointer(runtime, info);
+
+out:
+ if (DEUBUG_POINTER_TRACE)
+ trace_printk("buffer_ptr=%llu delta=%lu\n",
+ info->buffer_ptr,
+ (jiffies - info->base_ptr_jiffies) * 1000 / HZ);
+
+ /* only trust periodic pointers */
+ if (in_interrupt()) {
+ info->base_ptr = ptr;
+ info->base_ptr_jiffies = jiffies;
+ }
+ info->last_ptr = ptr;
+
+ return ptr;
}
--- linux-2.6.37.orig/drivers/staging/intel_sst/intel_sst.h 2011-06-28
16:38:12.688000015 +0800
+++ linux-2.6.37/drivers/staging/intel_sst/intel_sst.h 2011-07-01
16:34:05.831999980 +0800
@@ -76,6 +76,9 @@ struct pcm_stream_info {
void (*period_elapsed) (void *mad_substream);
unsigned long long buffer_ptr;
int sfreq;
+ unsigned long base_ptr_jiffies;
+ unsigned long base_ptr;
+ unsigned long last_ptr;
};
struct snd_pmic_ops {
Subject: ALSA: trace application/hardware pointers
Date: Wed Jun 15 16:25:58 CST 2011
Usage:
echo 0 > /debug/tracing/trace
echo 1 > /debug/tracing/events/sound/enable
# do audio tests
echo 0 > /debug/tracing/events/sound/enable
# copy and send /debug/tracing/trace
Signed-off-by: Wu Fengguang <fengguang...@intel.com>
---
include/trace/events/sound.h | 198 +++++++++++++++++++++++++++++++++
sound/core/pcm_lib.c | 18 ++-
sound/core/pcm_native.c | 38 +++++-
3 files changed, 250 insertions(+), 4 deletions(-)
--- linux-2.6.37.orig/sound/core/pcm_native.c 2011-06-27 11:28:09.639999989
+0800
+++ linux-2.6.37/sound/core/pcm_native.c 2011-06-27 11:28:15.867999996
+0800
@@ -38,6 +38,9 @@
#include <dma-coherence.h>
#endif
+#define CREATE_TRACE_POINTS
+#include <trace/events/sound.h>
+
/*
* Compatibility
*/
@@ -1273,6 +1276,9 @@ static void snd_pcm_post_reset(struct sn
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
runtime->silence_size > 0)
snd_pcm_playback_silence(substream, ULONG_MAX);
+ trace_snd_pointer(substream, __func__,
+ __builtin_return_address(0),
+ __builtin_return_address(1));
}
static struct action_ops snd_pcm_action_reset = {
@@ -1317,6 +1323,9 @@ static void snd_pcm_post_prepare(struct
struct snd_pcm_runtime *runtime = substream->runtime;
runtime->control->appl_ptr = runtime->status->hw_ptr;
runtime->status->state = SNDRV_PCM_STATE_PREPARED;
+ trace_snd_pointer(substream, __func__,
+ __builtin_return_address(0),
+ __builtin_return_address(1));
}
static struct action_ops snd_pcm_action_prepare = {
@@ -1425,6 +1434,9 @@ static int snd_pcm_drain(struct snd_pcm_
if (runtime->status->state == SNDRV_PCM_STATE_OPEN)
return -EBADFD;
+ trace_snd_pointer(substream, __func__,
+ __builtin_return_address(0),
+ __builtin_return_address(1));
snd_power_lock(card);
if (runtime->status->state == SNDRV_PCM_STATE_SUSPENDED) {
result = snd_power_wait(card, SNDRV_CTL_POWER_D0);
@@ -1503,7 +1515,9 @@ static int snd_pcm_drain(struct snd_pcm_
snd_pcm_stream_unlock_irq(substream);
up_read(&snd_pcm_link_rwsem);
snd_power_unlock(card);
-
+ trace_snd_pointer(substream, __func__,
+ __builtin_return_address(0),
+ __builtin_return_address(1));
return result;
}
@@ -1536,6 +1550,9 @@ static int snd_pcm_drop(struct snd_pcm_s
snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP);
/* runtime->control->appl_ptr = runtime->status->hw_ptr; */
snd_pcm_stream_unlock_irq(substream);
+ trace_snd_pointer(substream, __func__,
+ __builtin_return_address(0),
+ __builtin_return_address(1));
return result;
}
@@ -2219,6 +2236,9 @@ static snd_pcm_sframes_t snd_pcm_playbac
if (appl_ptr < 0)
appl_ptr += runtime->boundary;
runtime->control->appl_ptr = appl_ptr;
+ trace_snd_pointer(substream, __func__,
+ __builtin_return_address(0),
+ __builtin_return_address(1));
ret = frames;
__end:
snd_pcm_stream_unlock_irq(substream);
@@ -2316,6 +2336,9 @@ static snd_pcm_sframes_t snd_pcm_playbac
if (appl_ptr >= (snd_pcm_sframes_t)runtime->boundary)
appl_ptr -= runtime->boundary;
runtime->control->appl_ptr = appl_ptr;
+ trace_snd_pointer(substream, __func__,
+ __builtin_return_address(0),
+ __builtin_return_address(1));
ret = frames;
__end:
snd_pcm_stream_unlock_irq(substream);
@@ -2463,9 +2486,12 @@ static int snd_pcm_sync_ptr(struct snd_p
return err;
}
snd_pcm_stream_lock_irq(substream);
- if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_APPL))
+ if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_APPL)) {
control->appl_ptr = sync_ptr.c.control.appl_ptr;
- else
+ trace_snd_pointer(substream, __func__,
+ __builtin_return_address(0),
+ __builtin_return_address(1));
+ } else
sync_ptr.c.control.appl_ptr = control->appl_ptr;
if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_AVAIL_MIN))
control->avail_min = sync_ptr.c.control.avail_min;
@@ -2572,6 +2598,7 @@ static int snd_pcm_playback_ioctl1(struc
return -ENXIO;
if (snd_BUG_ON(substream->stream != SNDRV_PCM_STREAM_PLAYBACK))
return -EINVAL;
+ trace_snd_pcm_playback_ioctl(substream, cmd);
switch (cmd) {
case SNDRV_PCM_IOCTL_WRITEI_FRAMES:
{
@@ -2652,6 +2679,7 @@ static int snd_pcm_capture_ioctl1(struct
return -ENXIO;
if (snd_BUG_ON(substream->stream != SNDRV_PCM_STREAM_CAPTURE))
return -EINVAL;
+ trace_snd_pcm_capture_ioctl(substream, cmd);
switch (cmd) {
case SNDRV_PCM_IOCTL_READI_FRAMES:
{
@@ -2911,6 +2939,9 @@ static unsigned int snd_pcm_playback_pol
return -ENXIO;
runtime = substream->runtime;
+ trace_snd_pointer(substream, __func__,
+ __builtin_return_address(0),
+ __builtin_return_address(1));
poll_wait(file, &runtime->sleep, wait);
snd_pcm_stream_lock_irq(substream);
@@ -2932,6 +2963,7 @@ static unsigned int snd_pcm_playback_pol
break;
}
snd_pcm_stream_unlock_irq(substream);
+ trace_snd_poll(substream, avail, mask);
return mask;
}
--- linux-2.6.37.orig/sound/core/pcm_lib.c 2011-06-27 11:28:09.651999992
+0800
+++ linux-2.6.37/sound/core/pcm_lib.c 2011-06-27 11:28:18.548000016 +0800
@@ -30,6 +30,8 @@
#include <sound/pcm_params.h>
#include <sound/timer.h>
+#include <trace/events/sound.h>
+
/*
* fill ring buffer with silence
* runtime->silence_start: starting pointer to silence area
@@ -44,6 +46,9 @@ void snd_pcm_playback_silence(struct snd
struct snd_pcm_runtime *runtime = substream->runtime;
snd_pcm_uframes_t frames, ofs, transfer;
+ trace_snd_pointer(substream, __func__,
+ __builtin_return_address(0),
+ __builtin_return_address(1));
if (runtime->silence_size < runtime->boundary) {
snd_pcm_sframes_t noise_dist, n;
if (runtime->silence_start != runtime->control->appl_ptr) {
@@ -442,8 +447,12 @@ static int snd_pcm_update_hw_ptr0(struct
(long)old_hw_ptr);
}
- if (runtime->status->hw_ptr == new_hw_ptr)
+ if (runtime->status->hw_ptr == new_hw_ptr) {
+ trace_snd_pointer(substream, __func__,
+ __builtin_return_address(0),
+ __builtin_return_address(1));
return 0;
+ }
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
runtime->silence_size > 0)
@@ -464,6 +473,10 @@ static int snd_pcm_update_hw_ptr0(struct
if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE)
snd_pcm_gettime(runtime, (struct timespec
*)&runtime->status->tstamp);
+ trace_snd_pointer(substream, __func__,
+ __builtin_return_address(0),
+ __builtin_return_address(1));
+ trace_snd_sample(substream);
return snd_pcm_update_state(substream, runtime);
}
@@ -1882,6 +1895,9 @@ static snd_pcm_sframes_t snd_pcm_lib_wri
if (appl_ptr >= runtime->boundary)
appl_ptr -= runtime->boundary;
runtime->control->appl_ptr = appl_ptr;
+ trace_snd_pointer(substream, __func__,
+ __builtin_return_address(0),
+ __builtin_return_address(1));
if (substream->ops->ack)
substream->ops->ack(substream);
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.37/include/trace/events/sound.h 2011-06-27 11:28:15.871999997
+0800
@@ -0,0 +1,198 @@
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM sound
+
+#if !defined(_TRACE_SOUND_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_SOUND_H
+
+#include <sound/asound.h>
+#include <linux/tracepoint.h>
+
+#define snd_stream_id(substream) \
+ ((substream->pcm->card->number << 24) | \
+ (substream->pcm->device << 16) | \
+ (substream->number << 8) | \
+ substream->stream)
+
+#define snd_ioctl_name(ioctl) { ioctl, #ioctl }
+#define show_snd_pcm_ioctl(cmd) \
+ __print_symbolic(cmd, \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_PVERSION), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_INFO), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_TSTAMP), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_TTSTAMP), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_HW_REFINE), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_HW_PARAMS), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_HW_FREE), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_SW_PARAMS), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_STATUS), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_DELAY), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_HWSYNC), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_SYNC_PTR), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_CHANNEL_INFO), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_PREPARE), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_RESET), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_START), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_DROP), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_DRAIN), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_PAUSE), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_REWIND), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_RESUME), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_XRUN), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_FORWARD), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_WRITEI_FRAMES), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_READI_FRAMES), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_WRITEN_FRAMES), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_READN_FRAMES), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_LINK), \
+ snd_ioctl_name(SNDRV_PCM_IOCTL_UNLINK))
+
+DECLARE_EVENT_CLASS(snd_pcm_ioctl_class,
+ TP_PROTO(struct snd_pcm_substream *substream, int cmd),
+ TP_ARGS(substream, cmd),
+ TP_STRUCT__entry(
+ __field(unsigned int, cmd)
+ ),
+ TP_fast_assign(
+ __entry->cmd = cmd;
+ ),
+ TP_printk("%s",
+ show_snd_pcm_ioctl(__entry->cmd)
+ )
+);
+#define DEFINE_SND_PCM_IOCTL_EVENT(name) \
+DEFINE_EVENT(snd_pcm_ioctl_class, name, \
+ TP_PROTO(struct snd_pcm_substream *substream, int cmd), \
+ TP_ARGS(substream, cmd))
+
+DEFINE_SND_PCM_IOCTL_EVENT(snd_pcm_playback_ioctl);
+DEFINE_SND_PCM_IOCTL_EVENT(snd_pcm_capture_ioctl);
+
+TRACE_EVENT(snd_pointer,
+ TP_PROTO(struct snd_pcm_substream *substream,
+ const char *func,
+ void *caller0,
+ void *caller1),
+ TP_ARGS(substream, func, caller0, caller1),
+ TP_STRUCT__entry(
+ __field(unsigned int, stream_id)
+ __field(unsigned long, appl_ptr)
+ __field(unsigned long, hw_ptr)
+ __field(unsigned long, hw_end)
+ __field(long, delay)
+ __array(char, func, 32)
+ __field(void *, caller0)
+ __field(void *, caller1)
+ ),
+ TP_fast_assign(
+ struct snd_pcm_runtime *runtime = substream->runtime;
+ snd_pcm_sframes_t n;
+
+ __entry->stream_id = snd_stream_id(substream);
+ __entry->appl_ptr = runtime->control->appl_ptr;
+ __entry->hw_ptr = runtime->status->hw_ptr;
+ __entry->hw_end = runtime->status->hw_ptr +
+ runtime->buffer_size;
+
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+ n = snd_pcm_playback_hw_avail(runtime);
+ else
+ n = snd_pcm_capture_avail(runtime);
+ n += runtime->delay;
+ __entry->delay = n * 1000 / (long)runtime->rate;
+
+ strlcpy(__entry->func, func, 32);
+ __entry->caller0 = caller0;
+ __entry->caller1 = caller1;
+ ),
+ TP_printk("%s-%x: appl_ptr=%lu hw_ptr=%lu hw_end=%lu delay=%ld"
+ "\t%-32s %pS\t%pS",
+ (__entry->stream_id & 0xff) ? "capture" : "playback",
+ __entry->stream_id,
+ __entry->appl_ptr,
+ __entry->hw_ptr,
+ __entry->hw_end, /* last writable position */
+ __entry->delay,
+ __entry->func,
+ __entry->caller0,
+ __entry->caller1)
+);
+
+#define SAMPLE_BYTES 32
+#define __print_samples(s) ({ \
+ int i; \
+ const char *ret = p->buffer + p->len; \
+ \
+ for (i = 0; i < SAMPLE_BYTES; i += 4) \
+ trace_seq_printf(p, " %02x%02x%02x%02x", \
+ s[i], s[i+1], s[i+2], s[i+3]); \
+ trace_seq_printf(p, "%c", 0); \
+ ret; \
+ })
+
+TRACE_EVENT(snd_sample,
+ TP_PROTO(struct snd_pcm_substream *substream),
+ TP_ARGS(substream),
+ TP_STRUCT__entry(
+ __array(uint8_t, samples, SAMPLE_BYTES)
+ __field(unsigned int, stream_id)
+ ),
+ TP_fast_assign(
+ struct snd_pcm_runtime *runtime = substream->runtime;
+ unsigned long size;
+ unsigned long offset;
+ uint8_t *ptr;
+ int i;
+
+ __entry->stream_id = snd_stream_id(substream);
+
+ size = frames_to_bytes(runtime, runtime->buffer_size);
+ offset = frames_to_bytes(runtime, runtime->status->hw_ptr);
+
+ for (i = 0; i < SAMPLE_BYTES; i++) {
+ ptr = substream->runtime->dma_area + (offset % size);
+ __entry->samples[i] = *ptr;
+ offset++;
+ }
+ ),
+ TP_printk("%s-%x: %s",
+ (__entry->stream_id & 0xff) ? "capture" : "playback",
+ __entry->stream_id,
+ __print_samples(__entry->samples)
+ )
+);
+
+TRACE_EVENT(snd_poll,
+ TP_PROTO(struct snd_pcm_substream *substream,
+ unsigned long avail,
+ unsigned int mask),
+ TP_ARGS(substream, avail, mask),
+ TP_STRUCT__entry(
+ __field(unsigned int, stream_id)
+ __field(unsigned long, avail)
+ __field(unsigned long, avail_min)
+ __field(long, time)
+ __field(unsigned int, mask)
+ ),
+ TP_fast_assign(
+ __entry->stream_id = snd_stream_id(substream);
+ __entry->mask = mask;
+ __entry->avail = avail;
+ __entry->avail_min = substream->runtime->control->avail_min;
+ __entry->time = (__entry->avail_min - avail) * 1000 /
+ substream->runtime->rate;
+ ),
+ TP_printk("%s-%x: avail=%lu avail_min=%lu time=%ldms mask=%#x",
+ (__entry->stream_id & 0xff) ? "capture" : "playback",
+ __entry->stream_id,
+ __entry->avail,
+ __entry->avail_min,
+ __entry->time,
+ __entry->mask
+ )
+);
+
+#endif /* _TRACE_SOUND_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
_______________________________________________
MeeGo-kernel mailing list
MeeGo-kernel@lists.meego.com
http://lists.meego.com/listinfo/meego-kernel
