On Sat, Aug 1, 2009 at 11:04 AM, John Bonesiobones@secretlab.ca wrote:
Hi All,
Woops! Yep, this is the wrong patch. I've included the correct one. I've also attached the patch in case email messes with its formatting. Sorry about the confusion.
Jon,
I put in a debug hook in the out_be32() routine. It would printk output anytime the AC97_HEADPHONE register was written to. This printk text would come out as expected when the Headphone mixer setting was muted/unmuted.
I was thinking a stale register write may have gotten into the mpc5200 AC97 hardware somehow and not been sent immediately to the codec. Something you do later causes it to be sent. I don't trust the mpc5200 AC97 register write hardware 100%, it was an after thought added in the mpc5200b.
Some logic analyzers have AC97 protocol decode capability. That would isolate easily isolate the problem.
When I umuted the PCM mixer setting, this printk text did not appear, yet when I read the mixer register setting for the headphone, it was set back to mute.
If there is code in software doing this, it's very subtle.
- John
----- correct patch -----
From 24b52cf2836f711118fd6d2c8895c91c944978cd Mon Sep 17 00:00:00 2001
From: John Bonesio bones@secretlab.ca Date: Fri, 31 Jul 2009 16:01:33 -0700 Subject: [PATCH] ASoC: WM9712 Codec: Workaround an unmute problem
When setting the PCM mixer (unuting), the main Headphone mixer setting gets set back to mute. At this time it appears this is occuring in hardware.
Signed-off-by: John Bonesio bones@secretlab.ca
sound/soc/codecs/wm9712.c | 21 +++++++++++++++++++-- 1 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/wm9712.c b/sound/soc/codecs/wm9712.c index b57c817..6f8d164 100644 --- a/sound/soc/codecs/wm9712.c +++ b/sound/soc/codecs/wm9712.c @@ -28,6 +28,7 @@ static unsigned int ac97_read(struct snd_soc_codec *codec, unsigned int reg); static int ac97_write(struct snd_soc_codec *codec, unsigned int reg, unsigned int val); +static int ac97_flush(struct snd_soc_codec *codec, unsigned int reg);
/* * WM9712 register cache @@ -177,9 +178,14 @@ static int mixer_event(struct snd_soc_dapm_widget *w, else ac97_write(w->codec, AC97_VIDEO, mic | 0x8000);
- if (l & 0x2 || r & 0x2)
- if (l & 0x2 || r & 0x2) {
ac97_write(w->codec, AC97_PCM, pcm & 0x7fff);
- else
- /*
- * Workaround an apparent bug where the headphone mute setting
- * is modified when the PCM mute setting is enabled.
- */
- ac97_flush(w->codec, AC97_HEADPHONE);
- } else
ac97_write(w->codec, AC97_PCM, pcm | 0x8000);
if (l & 0x4 || r & 0x4) @@ -472,6 +478,17 @@ static int ac97_write(struct snd_soc_codec *codec, unsigned int reg, return 0; }
+static int ac97_flush(struct snd_soc_codec *codec, unsigned int reg) +{
- unsigned int val;
- u16 *cache = codec->reg_cache;
- if ((reg >> 1) < (ARRAY_SIZE(wm9712_reg))) {
- val = cache[reg >> 1];
- soc_ac97_ops.write(codec->ac97, reg, val);
- }
+}
static int ac97_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { -- 1.6.0.4
On Sat, 2009-08-01 at 09:47 -0400, Jon Smirl wrote:
On Fri, Jul 31, 2009 at 7:08 PM, John Bonesiobones@secretlab.ca wrote:
We've encountered strange behavior in the alsamixer settings using the wm9712 codec. If we unmute the headphone output and then unmute the PCM output, the headphone output gets reset to mute in the hardware register. At this point the hardware register does not match the value in the register cache.
I've spent some time debugging this, and the headphone setting is set outside of any code path that would call the ac97_write() routine. As best as I can tell, there is something strange going on in hardware.
This could be something wrong in the mpc5200 code that writes the AC97 registers too. Maybe a register write command is getting generated when it wasn't supposed to.
You can set the mpc5200 to generate an interrupt everything it finishes writing a AC97 register. If you get more interrupts than you expected the problem is in the driver.
I've provided this patch that works around the problem.
Have any of you seen this before? Is this patch the right approach?
- John
The code in psc_dma_bcom_enqueue_tx() didn't account for the fact that s->runtime->control->appl_ptr can wrap around to the beginning of the buffer. This change fixes this problem.
Signed-off-by: John Bonesio bones@secretlab.ca
sound/soc/fsl/mpc5200_dma.c | 17 +++++++++++++++++ 1 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c index cfe0ea4..2551c58 100644 --- a/sound/soc/fsl/mpc5200_dma.c +++ b/sound/soc/fsl/mpc5200_dma.c @@ -70,6 +70,23 @@ static void psc_dma_bcom_enqueue_next_buffer(struct psc_dma_stream *s)
static void psc_dma_bcom_enqueue_tx(struct psc_dma_stream *s) {
- if (s->appl_ptr > s->runtime->control->appl_ptr) {
- /*
- * In this case s->runtime->control->appl_ptr has wrapped around.
- * Play the data to the end of the boundary, then wrap our own
- * appl_ptr back around.
- */
- while (s->appl_ptr < s->runtime->boundary) {
- if (bcom_queue_full(s->bcom_task))
- return;
- s->appl_ptr += s->period_size;
- psc_dma_bcom_enqueue_next_buffer(s);
- }
- s->appl_ptr -= s->runtime->boundary;
- }
while (s->appl_ptr < s->runtime->control->appl_ptr) {
if (bcom_queue_full(s->bcom_task))