Re: Longstanding bug in ac97/intel8x0 resume/init

From: Takashi Iwai
Date: Wed Jul 09 2008 - 10:40:35 EST


At Mon, 07 Jul 2008 01:17:38 +0200,
Johannes Weiner wrote:
>
> Hi Takashi,
>
> Takashi Iwai <tiwai@xxxxxxx> writes:
>
> > At Tue, 01 Jul 2008 17:12:02 +0200,
> > Johannes Weiner wrote:
> >>
> >> Hi,
> >>
> >> Takashi Iwai <tiwai@xxxxxxx> writes:
> >>
> >> > At Tue, 01 Jul 2008 16:37:42 +0200,
> >> > Johannes Weiner wrote:
> >> >>
> >> >> Hi,
> >> >>
> >> >> Takashi Iwai <tiwai@xxxxxxx> writes:
> >> >>
> >> >> > At 30 Jun 2008 20:58:03 +0200,
> >> >> > Mathieu Chouquet-Stringer wrote:
> >> >> >>
> >> >> >> Hey there,
> >> >> >>
> >> >> >> hannes@xxxxxxxxxxxx (Johannes Weiner) writes:
> >> >> >> > Johannes Weiner <hannes@xxxxxxxxxxxx> writes:
> >> >> >> > > my laptop has muted sound after resuming the soundcard (by
> >> >> >> > > s2ram/hibernation). The problem seems to be that the cached register
> >> >> >> > > values are not written back to the device properly.
> >> >> >>
> >> >> >> I've got the same exact issue on a Thinkpad T30:
> >> >> >>
> >> >> >> 0 [I82801CAICH3 ]: ICH - Intel 82801CA-ICH3
> >> >> >> Intel 82801CA-ICH3 with AD1881A at irq 5
> >> >> >>
> >> >> >> 00:1f.5 Multimedia audio controller: Intel Corporation 82801CA/CAM AC'97 Audio Controller (rev 02)
> >> >> >
> >> >> > Does this happen for both hibernation and S2RAM?
> >> >> > And, resetting the mixer repairs the mute state, right?
> >> >> > If yes, the problem appears independently from the codec chip. Hmm...
> >> >>
> >> >> Yes, happens in both cases here.
> >> >>
> >> >> The alsamixer shows the state of the channels before the suspension(!).
> >> >
> >> > Yes. The driver returns the cached values.
> >>
> >> Okay.
> >>
> >> >> If I change the channel state, the sound works again. No complete reset
> >> >> needed at all, I just have to increase/decrease the value a bit (for
> >> >> each affected channel).
> >> >
> >> > Just touching one mixer element?
> >>
> >> What means `element' here? I have to touch MASTER and PCM in order to
> >> get some output again, at least ;)
> >
> > Well, for example, some laptops with maestro3 have a similar problem,
> > but in that case, you just need to touch one mixer element
> > (e.g. Master), and you don't have to re-adjust PCM volume.
> >
> >> >> >From my experiments with the code, I figured that the cached register
> >> >> values are not written back properly on resume. The cache is in the
> >> >> correct state but the hardware is not. This also explains the behaviour
> >> >> when changing the channels with alsamixer; the register cache is touched
> >> >> and written back (and this time, the value really gets through to the
> >> >> hardware).
> >> >
> >> > Right.
> >> >
> >> > snd_ac97_resume() has a check whether the write to MASTER register
> >> > succeeds, but its timeout is 100ms. Could you check whether this
> >> > check passes at resume or failed? I remember that some device
> >> > actually passed the test but didn't update the real hardware state.
> >> > If it failed on yours, we may simply extend the timeout, or make it
> >> > pending somehow. If the hardware fools us, however, it'd be toucher.
> >>
> >> By experimentation I found that the writeback works with a two seconds
> >> delay before writeback. I can't remember if it was before or after the
> >> check. Another approach was to hammer down the value by writing and
> >> reading back in a loop until the hardware responded with the correct
> >> value.
> >>
> >> I will redo the tests later and report back to you what helped.
> >
> > Yeah, that'll be appreciated.
>
> Okay, I redid the test with something (pretty stupid) like this:
>
> --- a/sound/pci/ac97/ac97_codec.c
> +++ b/sound/pci/ac97/ac97_codec.c
> @@ -28,6 +28,7 @@
> #include <linux/pci.h>
> #include <linux/moduleparam.h>
> #include <linux/mutex.h>
> +#include <linux/kallsyms.h>
> #include <sound/core.h>
> #include <sound/pcm.h>
> #include <sound/tlv.h>
> @@ -2456,8 +2457,23 @@ static void snd_ac97_restore_status(struct snd_ac97 *ac97)
> * are accessed..!
> */
> if (test_bit(i, ac97->reg_accessed)) {
> + printk("restoring register %d\n", i);
> snd_ac97_write(ac97, i, ac97->regs[i]);
> - snd_ac97_read(ac97, i);
> + msleep(800);
> + if (snd_ac97_read(ac97, i) != ac97->regs[i]) {
> + printk("double write register %d\n", i);
> + snd_ac97_write(ac97, i, ac97->regs[i]);
> + }
> + msleep(800);
> + if (snd_ac97_read(ac97, i) != ac97->regs[i]) {
> + printk("triple write register %d\n", i);
> + snd_ac97_write(ac97, i, ac97->regs[i]);
> + }
> + msleep(800);
> + if (snd_ac97_read(ac97, i) != ac97->regs[i]) {
> + printk("quadruple write register %d\n", i);
> + snd_ac97_write(ac97, i, ac97->regs[i]);
> + }
> }
> }
> }
>
> This makes the device resume properly, but the delays are insanely long
> and still sometimes it comes to the third write!
>
> I suspect that this issue is not a problem in the writeback code but in
> the init/exit code of the driver (either intel8x0 or ac97 itself, no
> idea).
>
> Because the following behaviour can be seen:
>
> 1. modprobe snd-intel8x0: everything fine.
> 2. rmmod snd-intel8x0: everything fine.
> 3. modprobe snd-intel8x0:
> ACPI: PCI Interrupt 0000:00:1f.5[B] -> Link [LNKB] -> GSI 11 (level, low) -> IRQ 11
> PCI: Setting latency timer of device 0000:00:1f.5 to 64
> ALSA sound/pci/ac97/ac97_codec.c:2054: AC'97 0 does not respond - RESET
> ACPI: PCI interrupt for device 0000:00:1f.5 disabled
> Intel ICH: probe of 0000:00:1f.5 failed with error -13
> 2. rmmod snd-intel8x0: everything fine.
> 3. modprobe snd-intel8x0: everything fine
>
> So I suspect that the device is not shut down properly on
> deactivation/suspension.
>
> Therefor this module reloading fails and the resume tries to writeback
> registers on the not-properly-initialized hardware. The delays appear
> way too long for me to be expectable from this hardware if it is
> properly initialized, no?
>
> May this be a possible?

Yes, possible.

BTW, is CONFIG_SND_AC97_POWER_SAVE enabled? The reset sequence is a
bit dependent on POWER_SAVE kconfig. When it's set, the driver always
tries a cold reset. What happens if you turn it on/off?

Also, any relation with the ac97 modem (i.e. snd-intel8x0m driver)?


thanks,

Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/