Suspend/resume causes a popping sound

[wmb@…]

When the suspend-to-RAM code turns off power to the CPU, and again when resuming, a brief "pop" is heard from the speaker.

This pop is quite useful when debugging the suspend/resume code, but it will be very annoying when we do automatic short-interval suspend/resume via the OS.

The amplifier has a shutdown function that can be activated by a CODEC output bit (EAPD, net name AMP_SHDN). I tried enabling that (i.e. shutting down the amp) prior to suspending, but it made no difference. The pop was still present both when going down and coming back up.

The amplifier is powered from +5V (via AVDD_AMP). According to the voltage rail chart on page 2 of the schematics, that rail is off during S3 state, so I suppose the pops could be caused by the amp power supply going off and on. The data sheet for the amp claims that the part is supposed to suppress pops, but I think that is true only when the amp is shutdown via its SD# pin. Supposedly, the amp draws very little current (20 nA) in shutdown mode, so perhaps we could leave the amplifier powered during suspend and gate it on and off with SD#. The CODEC also has software-controlled shutdown capability, so perhaps it could be left powered too. If not, we would need to control the amp SD# via some pin other than EAPD from the CODEC.

[AlbertCahalan]

See the SSM2211 documentation, page 18, "START-UP POPPING NOISE", "(16)". Do the capacitors and resistors satisfy that inequality? By a wide enough margin?

[wad]

Re: AlbertCahalan The XO uses the SSM2302, not the SSM2211. The SSM2302 doesn't use an external bias decoupling cap, as the 2211 does. Supposedly, the 2302 doesn't pop/click...

[wad]

The problem with the pop seems to be that the inverter driving AMP_SHDN (Q43) is powered down 6 mS before the +5V supply is powered

down. (+3.3V is heavily loaded and turned off with a MOSFET, whereas +5V is lightly loaded and takes longer to drain) I suggest moving R335 to SUS_3.3V instead of AVDD. This would still leave the "pop" on power-off, but that is much less objectionable.

[wmb@…]

The following hardware modification eliminates the pops (with reference to the B2 schematic):

a) Change R337 from a pull-up to a pull-down (to AGND). b) Remove R335 c) Remove Q43 d) Put a 10K resistor across Q43 pads 2 and 3

This change results in the net elimination of one component, a transistor.

It requires that the audio driver must explicitly assert the EAPD signal in order to power on the amplifier. This is a change in the sense of that signal; asserting it used to power off the amplifier. I know of no way to avoid this inversion (other than moving the amplifier shutdown control to a completely different GPIO pin).

Existing boards can be reworked fairly easily:

1) Move R337 to the otherwise-unused pad for the (unstuffed) C508. 2) Remove Q43 3) Move R335 to pads 2,3 of Q43.

[wad]

As we have determined that the driver does allow the sense of the EAPD bit to be easily inverted, we are proposing to implement Mitch's ECR. A schematic of the proposed circuit for B3 is attached (Anti pop circuit.pdf) includes some 0 ohm resistors allowing several different solutions to be tried.

[cjb]

I've written a patch to do this in the driver (attached), and mailed Jaya and Andres for comments. The patch depends on being able to detect B3s in-kernel, which Andres is working on now.

[dilinger]

Does the quirk need to happen before the call to snd_ac97_mixer? If not, let's combine it with the other (post-snd_ac97_mixer-call) quirk.

Also, Mitch tells me that we need to clear the EAPD bit prior to suspend. It looks like ac97 already does this if AC97_SCAP_INV_EAPD-related things are set. It would be nice to piggyback off that functionality if we can..

[cjb]

Replying to dilinger:

Does the quirk need to happen before the call to snd_ac97_mixer? If not, let's combine it with the other (post-snd_ac97_mixer-call) quirk.

Yes, that's why we needed to create a new quirk function. This quirk needs to happen before mixer(), and the other quirks need to happen after it.

Also, Mitch tells me that we need to clear the EAPD bit prior to suspend. It looks like ac97 already does this if AC97_SCAP_INV_EAPD-related things are set. It would be nice to piggyback off that functionality if we can..

That sounds promising.

[cjb]

Replying to cjb:

Replying to dilinger:

Also, Mitch tells me that we need to clear the EAPD bit prior to suspend. It looks like ac97 already does this if AC97_SCAP_INV_EAPD-related things are set. It would be nice to piggyback off that functionality if we can..

That sounds promising.

Well, promising if it works and we don't need to have cleared it before mixer setup -- if that's the case, though, we should be able to put it straight into olpc_quirks()?

[cjb]

I see from IRC that we also need to have the "external amplifier" alsamixer setting changed by default in the build, from unmuted to muted? Is that correct?

[jg]

The codec/amplifier should be powered down whenever the device is closed. We should ensure that applications that use the audio device close the device when not using it...

[wad]

Replying to cjb:

I see from IRC that we also need to have the "external amplifier" alsamixer setting changed by default in the build, from unmuted to muted? Is that correct?

This should be fixed by the driver inverting the polarity of the "mute" signal. It should not need changes at the application layer. Manually inverting the sense of the alsamixer "mute" flag is a workaround until the driver recognizes our hardware.

[dilinger]

Ok, the invert-eapd patch has been pushed to master and stable.

[cjb]

This patch fixed the mic too, so that's working as well.