On Sat, Aug 1, 2009 at 11:04 AM, John Bonesio<bones@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> 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@xxxxxxxxxxxx>
> ---
> 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 Bonesio<bones@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
>> > ---
>> >
>> > 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))
>> >
>> >
>>
>>
>>
>
--
Jon Smirl
jonsmirl@xxxxxxxxx
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
