[alsa-devel] possible fix for buffer underruns on msm7k alsa driver
hi, I'm still rebasing the codeaurora alsa driver for msm soc like the htcdream on the android msm 2.6.32 kernel. I looked the commits for the other socs like msm7kv2 and qsd8k(snapdragon)
and I found that commit: commit ac97ef246a754b9a2b4236ca08c6ade389dc206f Author: Pradeep Jilagam pjilagam@qualcomm.com Date: Wed Apr 7 17:38:20 2010 +0530
alsa: 8k: Limit period size for sample rates greater than 8KHz
The DSP interrupts are non-periodic. ALSA expects periodic acknowledgement for the buffers sent. Hence, the alsa driver uses timer as alternate interrupt source. The min resolution is 10ms which results in distorted audio for some sample rates. Hence restricting the minimum period size for sample rates greater than 8KHz to 512 frames.
Change-Id: Ic8c4180fbee7242fa3139230ddcf6a9f98e4c7ab CRs-fixed: 231784 Signed-off-by: Pradeep Jilagam pjilagam@qualcomm.com
I tried a big buck bunny at 44100 Hz: lots(too much) of buffer underrun => too bad to be usable. Then I tried it with 8000 Hz and found nearly no buffer underruns => usable.
So I've great expectations for that fix.
here's the source code involved in the fix:
static int hw_rule_periodsize_by_rate(struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule) { struct snd_interval *ps = hw_param_interval(params, SNDRV_PCM_HW_PARAM_PERIOD_SIZE); struct snd_interval *r = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE); struct snd_interval ch;
if (!ps || !r){ pr_debug("REMOVE_THAT: exiting from !ps||!r\n"); return 0; }else{ pr_debug("REMOVE_THAT: not exiting\n"); } snd_interval_any(&ch);
if (r->min > 8000) { ch.min = 512; pr_debug("Minimum period size is adjusted to 512\n"); return snd_interval_refine(ps, &ch); } return 0; }
static int msm_pcm_open(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime = substream->runtime; struct msm_audio *prtd; int ret = 0;
prtd = kzalloc(sizeof(struct msm_audio), GFP_KERNEL); if (prtd == NULL) { ret = -ENOMEM; return ret; } if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { msm_vol_ctl.update = 1; /* Update Volume, with Cached value */ runtime->hw = msm_pcm_playback_hardware; prtd->dir = SNDRV_PCM_STREAM_PLAYBACK; prtd->playback_substream = substream; prtd->eos_ack = 0; } else if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) { runtime->hw = msm_pcm_capture_hardware; prtd->dir = SNDRV_PCM_STREAM_CAPTURE; prtd->capture_substream = substream; } ret = snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &constraints_sample_rates); if (ret < 0) goto out; /* Ensure that buffer size is a multiple of period size */ ret = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS); if (ret < 0) goto out;
ret = snd_pcm_hw_rule_add(substream->runtime, 0, SNDRV_PCM_HW_PARAM_PERIOD_SIZE, hw_rule_periodsize_by_rate, substream, SNDRV_PCM_HW_PARAM_RATE, -1);
if (ret < 0) goto out;
prtd->ops = &snd_msm_audio_ops; prtd->out[0].used = BUF_INVALID_LEN; prtd->out_head = 1; /* point to second buffer on startup */ runtime->private_data = prtd;
ret = alsa_adsp_configure(prtd); if (ret) goto out; copy_count = 0; return 0;
out: kfree(prtd); return ret; }
Unfortunately it doesn't seem to print the REMOVE_THAT debug messages in dmesg,after playing something with mplayer, like a 44100 Hz song in mp3 format with mplayer.
I wonder why the hw_rule_periodsize_by_rate function is not executed.
Background: The htcdream is the first android smartphone and has by default a dsp interface that is not alsa nor oss nor something that was created before. Codeaurora(qualcomm) had some alsa driver,I rebased it but it was buggy,so I tried to debug it and succedded at making mplayer run,but it had many buffer underruns...
Denis.
On Sat, Aug 14, 2010 at 08:29:44PM +0200, Denis 'GNUtoo' Carikli wrote:
hi, I'm still rebasing the codeaurora alsa driver for msm soc like the htcdream on the android msm 2.6.32 kernel. I looked the commits for the other socs like msm7kv2 and qsd8k(snapdragon)
Your mail is very lengthy and something seems to have stripped all the indentation from the source code you've pasted. This all makes it extremely hard to read, and the fact that it's discussing drivers which have never been sent upstream doesn't help either.
The last time I looked at MSM audio drivers I noticed several abuses of APIs in there which make me concerned about the robustness of any changes that one might make to the code. I've no idea if this was the same set of drivers as you are looking at, it's really unfortunate that work on these drivers appears so fragmented.
Your mail is very lengthy and something seems to have stripped all the indentation from the source code you've pasted. This all makes it extremely hard to read, and the fact that it's discussing drivers which have never been sent upstream doesn't help either.
The last time I looked at MSM audio drivers I noticed several abuses of APIs in there which make me concerned about the robustness of any changes that one might make to the code. I've no idea if this was the same set of drivers as you are looking at, it's really unfortunate that work on these drivers appears so fragmented.
The reference code source is at: http://gitorious.org/htc-msm-2-6-32/leviathan-incoming/trees/android-msm-2.6...
And the audio patch that I rebased were attached The most important one is that one: [PATCH 2/4] sound: soc: msm7k: improve buffer underrun issue
There are only 2 patches.
The mmap interface is a patch rebased from this msm7kv2 alsa driver patch: commit c25698fee4fbd82a325394263752cb8b843fcfdc Author: Asish Bhattacharya asishb@qualcomm.com Date: Thu Jan 21 14:38:41 2010 +0530
alsa: soc: MMAP capability to playback stream
The current code supports playback non-mmap(copy) mode playback. Adding support to do mmap-mode playback.
Signed-off-by: Asish Bhattacharya asishb@qualcomm.com It is comming from git://codeaurora.org/kernel/msm.git
Note that this is still not usable,because the buffer underrun are a lot worse than with non-mmaped interface. Signed-off-by: Denis 'GNUtoo' Carikli GNUtoo@no-log.org --- sound/soc/msm/TODO | 4 ++ sound/soc/msm/msm-pcm.c | 8 ++- sound/soc/msm/msm-pcm.h | 7 ++- sound/soc/msm/msm7k-pcm.c | 110 ++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 117 insertions(+), 12 deletions(-) create mode 100644 sound/soc/msm/TODO
diff --git a/sound/soc/msm/TODO b/sound/soc/msm/TODO new file mode 100644 index 0000000..87084ea --- /dev/null +++ b/sound/soc/msm/TODO @@ -0,0 +1,4 @@ +ad4974853c00ac1b6ac388a2915d86c06a04447f +87a86ea6bd2868d9e9b3113f366e7ca440ef6199 +35240ee6cc224db5c916b345ae7a23fbe9b1ade6 +3d02aeb5607c558dec70054a234b2f36007ea362 diff --git a/sound/soc/msm/msm-pcm.c b/sound/soc/msm/msm-pcm.c index f90c778..23c2c55 100644 --- a/sound/soc/msm/msm-pcm.c +++ b/sound/soc/msm/msm-pcm.c @@ -47,8 +47,6 @@ msm_adsp_write(prtd->audrec, QDSP_uPAudRecCmdQueue, cmd, len)
int intcnt; -static int audio_dsp_send_buffer(struct msm_audio *prtd, - unsigned idx, unsigned len);
struct audio_frame { uint16_t count_low; @@ -141,6 +139,9 @@ void alsa_dsp_event(void *data, unsigned id, uint16_t *msg) if (prtd->ops->playback) prtd->ops->playback(prtd);
+ if (prtd->mmap_flag) + break; + spin_lock_irqsave(&the_locks.write_dsp_lock, flag); if (prtd->running) { prtd->out[idx].used = 0; @@ -631,7 +632,7 @@ int alsa_buffer_read(struct msm_audio *prtd, void __user *buf, } EXPORT_SYMBOL(alsa_buffer_read);
-static int audio_dsp_send_buffer(struct msm_audio *prtd, +int audio_dsp_send_buffer(struct msm_audio *prtd, unsigned idx, unsigned len) { audpp_cmd_pcm_intf_send_buffer cmd; @@ -645,6 +646,7 @@ static int audio_dsp_send_buffer(struct msm_audio *prtd, return audpp_send_queue2(&cmd, sizeof(cmd)); }
+ int alsa_rec_dsp_enable(struct msm_audio *prtd, int enable) { audrec_cmd_cfg cmd; diff --git a/sound/soc/msm/msm-pcm.h b/sound/soc/msm/msm-pcm.h index 9551bb0..62651a7 100644 --- a/sound/soc/msm/msm-pcm.h +++ b/sound/soc/msm/msm-pcm.h @@ -52,8 +52,7 @@ (SNDRV_PCM_RATE_8000_48000 | SNDRV_PCM_RATE_KNOT) #define USE_RATE_MIN 8000 #define USE_RATE_MAX 48000 -#define MAX_BUFFER_PLAYBACK_SIZE \ - (4800*4) +#define MAX_BUFFER_PLAYBACK_SIZE PLAYBACK_DMASZ /* 2048 frames (Mono), 1024 frames (Stereo) */ #define CAPTURE_SIZE 4096 #define MAX_BUFFER_CAPTURE_SIZE (4096*4) @@ -174,6 +173,8 @@ struct msm_audio { int running; int stopped; /* set when stopped, cleared on flush */ int eos_ack; + int mmap_flag; + int period; };
@@ -193,6 +194,8 @@ extern int alsa_audio_disable(struct msm_audio *prtd); extern int alsa_adsp_configure(struct msm_audio *prtd); extern int alsa_buffer_read(struct msm_audio *prtd, void __user *buf, size_t count, loff_t *pos); +extern int audio_dsp_send_buffer(struct msm_audio *prtd, + unsigned idx, unsigned len); ssize_t alsa_send_buffer(struct msm_audio *prtd, const char __user *buf, size_t count, loff_t *pos, int copy_count); int msm_audio_volume_update(unsigned id, diff --git a/sound/soc/msm/msm7k-pcm.c b/sound/soc/msm/msm7k-pcm.c index c366902..9561e91 100644 --- a/sound/soc/msm/msm7k-pcm.c +++ b/sound/soc/msm/msm7k-pcm.c @@ -108,7 +108,9 @@ static unsigned convert_samp_rate(unsigned hz) }
static struct snd_pcm_hardware msm_pcm_playback_hardware = { - .info = SNDRV_PCM_INFO_INTERLEAVED, + .info = SNDRV_PCM_INFO_MMAP | + SNDRV_PCM_INFO_MMAP_VALID | + SNDRV_PCM_INFO_INTERLEAVED, .formats = USE_FORMATS, .rates = USE_RATE, .rate_min = USE_RATE_MIN, @@ -150,10 +152,35 @@ static struct snd_pcm_hw_constraint_list constraints_sample_rates = { .mask = 0, };
+static void msm_pcm_enqueue_data(struct snd_pcm_substream *substream) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + struct msm_audio *prtd = runtime->private_data; + unsigned int period_size; + + period_size = snd_pcm_lib_period_bytes(substream); + audio_dsp_send_buffer(prtd, prtd->out_tail, period_size); + prtd->out_tail ^= 1; + ++copy_count; + prtd->period++; + if (unlikely(prtd->period >= runtime->periods)) + prtd->period = 0; + +} + + static void playback_event_handler(void *data) { struct msm_audio *prtd = data; snd_pcm_period_elapsed(prtd->playback_substream); + if (prtd->mmap_flag) { + if (prtd->dir == SNDRV_PCM_STREAM_CAPTURE) + return; + if (!prtd->stopped) + msm_pcm_enqueue_data(prtd->playback_substream); + else + prtd->out_needed++; + } }
static void capture_event_handler(void *data) @@ -171,10 +198,30 @@ static int msm_pcm_playback_prepare(struct snd_pcm_substream *substream) prtd->pcm_count = snd_pcm_lib_period_bytes(substream); prtd->pcm_irq_pos = 0; prtd->pcm_buf_pos = 0; + if (prtd->enabled) + return 0;
/* rate and channels are sent to audio driver */ prtd->out_sample_rate = runtime->rate; prtd->out_channel_mode = runtime->channels; + /* prtd->data = prtd->substream->dma_buffer.area; */ + /* prtd->phys = prtd->substream->dma_buffer.addr; */ + /* prtd->out[0].data = prtd->data + 0; */ + /* prtd->out[0].addr = prtd->phys + 0; */ + /* prtd->out[0].size = BUFSZ; */ + /* prtd->out[1].data = prtd->data + BUFSZ; */ + /* prtd->out[1].addr = prtd->phys + BUFSZ; */ + /* prtd->out[1].size = BUFSZ; */ + + if (prtd->enabled | !(prtd->mmap_flag)) + return 0; + + prtd->out[0].used = prtd->pcm_count; + prtd->out[1].used = prtd->pcm_count; + + mutex_lock(&the_locks.lock); + alsa_audio_configure(prtd); + mutex_unlock(&the_locks.lock);
return 0; } @@ -230,19 +277,56 @@ static int msm_pcm_capture_prepare(struct snd_pcm_substream *substream)
static int msm_pcm_trigger(struct snd_pcm_substream *substream, int cmd) { + struct snd_pcm_runtime *runtime = substream->runtime; + struct msm_audio *prtd = runtime->private_data; + unsigned long flag = 0; int ret = 0;
switch (cmd) { case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: + if ((substream->stream == SNDRV_PCM_STREAM_CAPTURE) + || !prtd->mmap_flag) + break; + if (!prtd->out_needed) { + prtd->stopped = 0; + break; + } + spin_lock_irqsave(&the_locks.write_dsp_lock, flag); + if (prtd->running == 1) { + if (prtd->stopped == 1) { + prtd->stopped = 0; + prtd->period = 0; + if (prtd->pcm_irq_pos == 0) { + prtd->out_tail = 0; + msm_pcm_enqueue_data(prtd->playback_substream); + prtd->out_needed--; + } else { + prtd->out_tail = 1; + msm_pcm_enqueue_data(prtd->playback_substream); + prtd->out_needed--; + } + if (prtd->out_needed) { + prtd->out_tail ^= 1; + msm_pcm_enqueue_data(prtd->playback_substream); + prtd->out_needed--; + } + } + } + spin_unlock_irqrestore(&the_locks.write_dsp_lock, flag); break; case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_PAUSE_PUSH: + if ((substream->stream == SNDRV_PCM_STREAM_CAPTURE) + || !prtd->mmap_flag) + break; + prtd->stopped = 1; break; default: ret = -EINVAL; + break; }
return ret; @@ -463,16 +547,27 @@ static snd_pcm_uframes_t msm_pcm_pointer(struct snd_pcm_substream *substream) int msm_pcm_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { - struct snd_pcm_runtime *runtime = substream->runtime; - - if (substream->pcm->device & 1) { - runtime->hw.info &= ~SNDRV_PCM_INFO_INTERLEAVED; - runtime->hw.info |= SNDRV_PCM_INFO_NONINTERLEAVED; - } + snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer); return 0;
}
+int msm_pcm_mmap(struct snd_pcm_substream *substream, + struct vm_area_struct *vma) + { + struct snd_pcm_runtime *runtime = substream->runtime; + struct msm_audio *prtd = runtime->private_data; + + prtd->out_head = 0; /* point to First buffer on startup */ + prtd->mmap_flag = 1; + runtime->dma_bytes = snd_pcm_lib_period_bytes(substream)*2; + dma_mmap_coherent(substream->pcm->card->dev, vma, + runtime->dma_area, + runtime->dma_addr, + runtime->dma_bytes); + return 0; +} + static struct snd_pcm_ops msm_pcm_ops = { .open = msm_pcm_open, .copy = msm_pcm_copy, @@ -483,6 +578,7 @@ static struct snd_pcm_ops msm_pcm_ops = { .prepare = msm_pcm_prepare, .trigger = msm_pcm_trigger, .pointer = msm_pcm_pointer, + .mmap = msm_pcm_mmap, };
This commit is a rebase of the following patch: commit ac97ef246a754b9a2b4236ca08c6ade389dc206f Author: Pradeep Jilagam pjilagam@qualcomm.com Date: Wed Apr 7 17:38:20 2010 +0530
alsa: 8k: Limit period size for sample rates greater than 8KHz
The DSP interrupts are non-periodic. ALSA expects periodic acknowledgement for the buffers sent. Hence, the alsa driver uses timer as alternate interrupt source. The min resolution is 10ms which results in distorted audio for some sample rates. Hence restricting the minimum period size for sample rates greater than 8KHz to 512 frames.
Change-Id: Ic8c4180fbee7242fa3139230ddcf6a9f98e4c7ab CRs-fixed: 231784 Signed-off-by: Pradeep Jilagam pjilagam@qualcomm.com
On Sun, Aug 15, 2010 at 07:58:01PM +0200, Denis 'GNUtoo' Carikli wrote:
The last time I looked at MSM audio drivers I noticed several abuses of APIs in there which make me concerned about the robustness of any changes that one might make to the code. I've no idea if this was the same set of drivers as you are looking at, it's really unfortunate that work on these drivers appears so fragmented.
The reference code source is at: http://gitorious.org/htc-msm-2-6-32/leviathan-incoming/trees/android-msm-2.6...
That looks roughly like the problematic code I mentioned above.
And the audio patch that I rebased were attached The most important one is that one: [PATCH 2/4] sound: soc: msm7k: improve buffer underrun issue
There are only 2 patches.
Sorry, I'm a bit lost as to what the actual question is here?
participants (2)
-
Denis 'GNUtoo' Carikli -
Mark Brown