At Sun, 27 Mar 2011 03:27:28 +0100 (BST), Chris Wilson wrote:

Hi Takashi,

On Sat, 26 Mar 2011, Takashi Iwai wrote:

...

I attached below. This is no final version and we are working on it. But it'd be helpful if you can test it whether it solves the issue.

Thanks for sending the patch :) I think it makes some progress towards solving the issue. On suspend I now get only one pop instead of two, and none on resume. The pop that I still get on suspend is the one that causes my amplifier to trip out (which was the second one previously).

Good to hear of improvements.

I cleaned up the patch a bit. Could you try the patch below instead? I removed the second msleep() in the resume callback, supposedly it's irrelevant. If you get a noise again in the resume path, try to add msleep(150) after codec->patch_ops.init() in alc_resume().

The remaining noise in the suspend path is still unclear. We have one place that we do NOT turn down the power to D3 -- in hda_set_power_state() in hda_codec.c, D3-transition is skipped on a widget with EAPD control. It's interesting whether any widgets hit this.

Otherwise, we may need to check which call actually gives the noise. You can put a printk() and a long timeout at each line, and watch the kernel message....

thanks,

Takashi

--- diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 12c6f45..29eb397 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -375,6 +375,7 @@ struct alc_spec { #ifdef CONFIG_SND_HDA_POWER_SAVE void (*power_hook)(struct hda_codec *codec); #endif + void (*shutup)(struct hda_codec *codec);

/* for pin sensing */ unsigned int sense_updated: 1; @@ -1236,6 +1237,15 @@ static void set_eapd(struct hda_codec *codec, hda_nid_t nid, int on) on ? 2 : 0); }

+static void alc_eapd_shutup(struct hda_codec *codec) +{ + struct alc_spec *spec = codec->spec; + + set_eapd(codec, 0x14, 0); + set_eapd(codec, 0x15, 0); + msleep(200); +} + static void alc_auto_init_amp(struct hda_codec *codec, int type) { unsigned int tmp; @@ -4193,6 +4203,10 @@ static int alc_build_pcms(struct hda_codec *codec)

static inline void alc_shutup(struct hda_codec *codec) { + struct alc_spec *spec = codec->spec; + + if (spec && spec->shutup) + spec->shutup(codec); snd_hda_shutup_pins(codec); }

@@ -4263,6 +4277,7 @@ static int alc_suspend(struct hda_codec *codec, pm_message_t state) #ifdef SND_HDA_NEEDS_RESUME static int alc_resume(struct hda_codec *codec) { + msleep(150); /* to avoid pop noise */ codec->patch_ops.init(codec); snd_hda_codec_resume_amp(codec); snd_hda_codec_resume_cache(codec); @@ -13018,6 +13033,7 @@ static int patch_alc262(struct hda_codec *codec) codec->patch_ops = alc_patch_ops; if (board_config == ALC262_AUTO) spec->init_hook = alc262_auto_init; + spec->shutup = alc_eapd_shutup;

alc_init_jacks(codec); #ifdef CONFIG_SND_HDA_POWER_SAVE @@ -14092,6 +14108,7 @@ static int patch_alc268(struct hda_codec *codec) codec->patch_ops = alc_patch_ops; if (board_config == ALC268_AUTO) spec->init_hook = alc268_auto_init; + spec->shutup = alc_eapd_shutup;

alc_init_jacks(codec);

@@ -17410,6 +17418,8 @@ static int patch_alc861vd(struct hda_codec *codec)

if (board_config == ALC861VD_AUTO) spec->init_hook = alc861vd_auto_init; + spec->shutup = alc_eapd_shutup; + #ifdef CONFIG_SND_HDA_POWER_SAVE if (!spec->loopback.amplist) spec->loopback.amplist = alc861vd_loopbacks; @@ -19611,6 +19621,7 @@ static int patch_alc662(struct hda_codec *codec) codec->patch_ops = alc_patch_ops; if (board_config == ALC662_AUTO) spec->init_hook = alc662_auto_init; + spec->shutup = alc_eapd_shutup;

alc_init_jacks(codec);