Messages in this thread Patch in this message | | | From | Johannes Weiner <> | Subject | Re: Longstanding bug in ac97/intel8x0 resume/init | Date | Mon, 07 Jul 2008 01:17:38 +0200 |
| |
Hi Takashi,
Takashi Iwai <tiwai@suse.de> writes:
> At Tue, 01 Jul 2008 17:12:02 +0200, > Johannes Weiner wrote: >> >> Hi, >> >> Takashi Iwai <tiwai@suse.de> writes: >> >> > At Tue, 01 Jul 2008 16:37:42 +0200, >> > Johannes Weiner wrote: >> >> >> >> Hi, >> >> >> >> Takashi Iwai <tiwai@suse.de> writes: >> >> >> >> > At 30 Jun 2008 20:58:03 +0200, >> >> > Mathieu Chouquet-Stringer wrote: >> >> >> >> >> >> Hey there, >> >> >> >> >> >> hannes@saeurebad.de (Johannes Weiner) writes: >> >> >> > Johannes Weiner <hannes@saeurebad.de> 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?
If you need any more information, please say so. This bug is really annoying :/
Hannes
| |