[PATCH] hda_codec: restore control element values during resume

[PATCH] hda_codec: restore control element values during resume

[Daniel Drake]

After resume from suspend-to-RAM, there is no sound output from my Intel HDA
hardware:

00:1b.0 Audio device: Intel Corporation 82801G (ICH7 Family) High Definition
Audio Controller (rev 01)
Found in a Dell Inspiron 640m, SigmaTel STAC9200 chip.

After resuming, sound immediately returns when you adjust the volume one notch
in either direction. I reported this on the ALSA bug tracker (#0002989).

Further investigation shows that the AC_VERB_SET_AMP_GAIN_MUTE verb is not
being executed during resume, which seems strange. Does other hardware really
store this value during a suspend/resume cycle? Or maybe the volume is
generally controlled through other means on other setups? I'm not entirely
sure what the difference is between kcontrol and kcontrol_new structures, but
I note that there is already some code to restore the values for kcontrol_new
elements.

I solved the issue with the following patch. Is it correct?

Signed-off-by: Daniel Drake <dsd@gentoo.org>

Index: linux-2.6/sound/pci/hda/hda_codec.c
===================================================================
--- linux-2.6.orig/sound/pci/hda/hda_codec.c
+++ linux-2.6/sound/pci/hda/hda_codec.c
@@ -2248,15 +2248,39 @@ EXPORT_SYMBOL(snd_hda_suspend);
 int snd_hda_resume(struct hda_bus *bus)
 {
 	struct list_head *p;
+	struct snd_ctl_elem_value *val = kmalloc(sizeof(*val), GFP_KERNEL);
+
+	if (!val)
+		return -ENOMEM;
 
 	list_for_each(p, &bus->codec_list) {
 		struct hda_codec *codec = list_entry(p, struct hda_codec, list);
+		struct snd_kcontrol *kctl;
+
+		codec->in_resume = 1;
+
 		hda_set_power_state(codec,
 				    codec->afg ? codec->afg : codec->mfg,
 				    AC_PWRST_D0);
+
+
+		list_for_each_entry(kctl, &bus->card->controls, list) {
+			memset(val, 0, sizeof(*val));
+			val->id = kctl->id;
+			/* Assume that get callback reads only from cache,
+			 * not accessing the real hardware
+			 */
+			if (snd_ctl_elem_read(bus->card, val) < 0)
+				continue;
+			snd_ctl_elem_write(bus->card, NULL, val);
+		}
+
 		if (codec->patch_ops.resume)
 			codec->patch_ops.resume(codec);
+
+		codec->in_resume = 0;
 	}
+	kfree(val);
 	return 0;
 }

Re: [PATCH] hda_codec: restore control element values during resume

[Takashi Iwai]

At Sun, 29 Apr 2007 17:14:21 +0100 (BST),
Daniel Drake wrote:
> 
> After resume from suspend-to-RAM, there is no sound output from my Intel HDA
> hardware:
> 
> 00:1b.0 Audio device: Intel Corporation 82801G (ICH7 Family) High Definition
> Audio Controller (rev 01)
> Found in a Dell Inspiron 640m, SigmaTel STAC9200 chip.
> 
> After resuming, sound immediately returns when you adjust the volume one notch
> in either direction. I reported this on the ALSA bug tracker (#0002989).
> 
> Further investigation shows that the AC_VERB_SET_AMP_GAIN_MUTE verb is not
> being executed during resume, which seems strange. Does other hardware really
> store this value during a suspend/resume cycle? Or maybe the volume is
> generally controlled through other means on other setups? I'm not entirely
> sure what the difference is between kcontrol and kcontrol_new structures, but
> I note that there is already some code to restore the values for kcontrol_new
> elements.
> 
> I solved the issue with the following patch. Is it correct?

The controls should have been restored by snd_hda_resume_ctls() call
in stac92xx_resume(), so basically your patch must be suprefluous.
Could you check whether this gets called?


Takashi

Re: [PATCH] hda_codec: restore control element values during resume

[Daniel Drake]

Takashi Iwai wrote:
> The controls should have been restored by snd_hda_resume_ctls() call
> in stac92xx_resume(), so basically your patch must be suprefluous.
> Could you check whether this gets called?

I did see the snd_hda_resume_ctls() code but noted it was resuming from 
snd_kcontrol_new structures rather than snd_kcontrol instances. I'm 
still not sure what the difference is here, are they supposed to 
'mirror' each other?

Anyway, it turns out that snd_hda_resume_ctls() doesn't get executed.

stac92xx_resume() has this code:

	for (i = 0; i < spec->num_mixers; i++)
		snd_hda_resume_ctls(codec, spec->mixers[i]);

On my system, spec->num_mixers is 0.

Daniel

Re: [PATCH] hda_codec: restore control element values during resume

[Takashi Iwai]

At Wed, 02 May 2007 11:57:00 -0400,
Daniel Drake wrote:
> 
> Takashi Iwai wrote:
> > The controls should have been restored by snd_hda_resume_ctls() call
> > in stac92xx_resume(), so basically your patch must be suprefluous.
> > Could you check whether this gets called?
> 
> I did see the snd_hda_resume_ctls() code but noted it was resuming from 
> snd_kcontrol_new structures rather than snd_kcontrol instances. I'm 
> still not sure what the difference is here, are they supposed to 
> 'mirror' each other?

Not really.  The snd_kcontrol_new struct is basically a template for
creating a control element.  The codec code uses this array to get the
control elements.

Tracking all kctls from the card instance is not suitable in this case
because it contains really _all_ control elements.  hda-intel driver
may have multiple codecs, so you cannot check whether the element is
what your codec created or not.

Another way would be to keep a list of created kctls, but I chose a
simpler way to use kcontrol_new because they are already there.

> Anyway, it turns out that snd_hda_resume_ctls() doesn't get executed.
> 
> stac92xx_resume() has this code:
> 
> 	for (i = 0; i < spec->num_mixers; i++)
> 		snd_hda_resume_ctls(codec, spec->mixers[i]);
> 
> On my system, spec->num_mixers is 0.

OK, it's a simple error.  The resume callback should call
snd_hda_resume_ctls() with spec->mixer argument.  The other codec
codes store this also onto spec->mixers[] array, so I overlooked.

Now fixed on HG tree.


thanks,

Takashi

Re: [PATCH] hda_codec: restore control element values during resume

[Daniel Drake]

Takashi Iwai wrote:
> OK, it's a simple error.  The resume callback should call
> snd_hda_resume_ctls() with spec->mixer argument.  The other codec
> codes store this also onto spec->mixers[] array, so I overlooked.

Thanks, this works splendidly.