Skip to contentSkip to conversation

hda: STAC927x power down inactive DACs

PATCH In review Matthew Ranostay 2 participants 4 messages Jan 30, 2008 – Jan 31, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 hda: STAC927x power down inactive DACs

On several laptops that have STAC9228 codecs have unused DACs, this powers them down to a D3 state.

---

pci/hda/patch_sigmatel.c +13 -1
diff -r 86c8c0a82bb9 pci/hda/patch_sigmatel.c
@@ -288,6 +288,10 @@ static hda_nid_t stac927x_adc_nids[3] = 
 
 static hda_nid_t stac927x_mux_nids[3] = {
         0x15, 0x16, 0x17
+};
+
+static hda_nid_t stac927x_dac_nids[5] = {
+	0x02, 0x03, 0x04, 0x05, 0x06
 };
 
 static hda_nid_t stac927x_dmux_nids[1] = {
@@ -3537,7 +3541,7 @@ static int patch_stac927x(struct hda_cod
 static int patch_stac927x(struct hda_codec *codec)
 {
 	struct sigmatel_spec *spec;
-	int err;
+	int err, i;
 
 	spec  = kzalloc(sizeof(*spec), GFP_KERNEL);
 	if (spec == NULL)
@@ -3628,6 +3632,14 @@ static int patch_stac927x(struct hda_cod
 		stac92xx_free(codec);
 		return err;
 	}
+
+	/* on some 3-stack systems not all the DACs are used
+ 	 * this powers down ones that aren't used
+ 	 */
+	for (i = 0; i < ARRAY_SIZE(stac927x_dac_nids); i++)
+		if (!is_in_dac_nids(spec, stac927x_dac_nids[i]))
+			snd_hda_codec_write_cache(codec, stac927x_dac_nids[i],
+				0, AC_VERB_SET_POWER_STATE, AC_PWRST_D3);
 
 	codec->patch_ops = stac92xx_patch_ops;
#1 lore raw

At Wed, 30 Jan 2008 09:14:02 -0500,

Quoted 4 lines · Matthew Ranostay

On several laptops that have STAC9228 codecs have unused DACs, this powers them down to a D3 state.

I think they will be eventually powered up again in hda_set_power_state() in hda_codec.c. As a temporary solution, we can have a sort of blacklist in struct hda_codec for always turning the power off for such widgets.

As a long-term solution, the automatic detection of such widgets would be nice...

Takashi

Quoted 39 lines

---
diff -r 86c8c0a82bb9 pci/hda/patch_sigmatel.c

pci/hda/patch_sigmatel.c +13 -1
@@ -288,6 +288,10 @@ static hda_nid_t stac927x_adc_nids[3] = 
 
 static hda_nid_t stac927x_mux_nids[3] = {
         0x15, 0x16, 0x17
+};
+
+static hda_nid_t stac927x_dac_nids[5] = {
+	0x02, 0x03, 0x04, 0x05, 0x06
 };
 
 static hda_nid_t stac927x_dmux_nids[1] = {
@@ -3537,7 +3541,7 @@ static int patch_stac927x(struct hda_cod
 static int patch_stac927x(struct hda_codec *codec)
 {
 	struct sigmatel_spec *spec;
-	int err;
+	int err, i;
 
 	spec  = kzalloc(sizeof(*spec), GFP_KERNEL);
 	if (spec == NULL)
@@ -3628,6 +3632,14 @@ static int patch_stac927x(struct hda_cod
 		stac92xx_free(codec);
 		return err;
 	}
+
+	/* on some 3-stack systems not all the DACs are used
+ 	 * this powers down ones that aren't used
+ 	 */
+	for (i = 0; i < ARRAY_SIZE(stac927x_dac_nids); i++)
+		if (!is_in_dac_nids(spec, stac927x_dac_nids[i]))
+			snd_hda_codec_write_cache(codec, stac927x_dac_nids[i],
+				0, AC_VERB_SET_POWER_STATE, AC_PWRST_D3);
 
 	codec->patch_ops = stac92xx_patch_ops;
 
#2 lore raw
Quoted 55 lines · Takashi Iwai

At Wed, 30 Jan 2008 09:14:02 -0500,

Quoted 3 lines · Matthew Ranostay

On several laptops that have STAC9228 codecs have unused DACs, this powers them down to a D3 state.

I think they will be eventually powered up again in hda_set_power_state() in hda_codec.c. As a temporary solution, we can have a sort of blacklist in struct hda_codec for always turning the power off for such widgets.

As a long-term solution, the automatic detection of such widgets would be nice...

Takashi

Quoted 37 lines

---
diff -r 86c8c0a82bb9 pci/hda/patch_sigmatel.c

pci/hda/patch_sigmatel.c +13 -1
@@ -288,6 +288,10 @@ static hda_nid_t stac927x_adc_nids[3] = 
 
 static hda_nid_t stac927x_mux_nids[3] = {
         0x15, 0x16, 0x17
+};
+
+static hda_nid_t stac927x_dac_nids[5] = {
+	0x02, 0x03, 0x04, 0x05, 0x06
 };
 
 static hda_nid_t stac927x_dmux_nids[1] = {
@@ -3537,7 +3541,7 @@ static int patch_stac927x(struct hda_cod
 static int patch_stac927x(struct hda_codec *codec)
 {
 	struct sigmatel_spec *spec;
-	int err;
+	int err, i;
 
 	spec  = kzalloc(sizeof(*spec), GFP_KERNEL);
 	if (spec == NULL)
@@ -3628,6 +3632,14 @@ static int patch_stac927x(struct hda_cod
 		stac92xx_free(codec);
 		return err;
 	}
+
+	/* on some 3-stack systems not all the DACs are used
+ 	 * this powers down ones that aren't used
+ 	 */
+	for (i = 0; i < ARRAY_SIZE(stac927x_dac_nids); i++)
+		if (!is_in_dac_nids(spec, stac927x_dac_nids[i]))
+			snd_hda_codec_write_cache(codec, stac927x_dac_nids[i],
+				0, AC_VERB_SET_POWER_STATE, AC_PWRST_D3);
 
Quoted 3 lines

pci/hda/patch_sigmatel.c static int patch_stac927x(struct hda_cod · line 3645

Quoted 2 lines
pci/hda/patch_sigmatel.c +0 -0
 	codec->patch_ops = stac92xx_patch_ops;
 

I think this what you had in mind.
Also is putting the function call in stac92xx_resume correct?

pci/hda/patch_sigmatel.c +23 -1
diff -r 86c8c0a82bb9 pci/hda/patch_sigmatel.c
@@ -135,6 +135,7 @@ struct sigmatel_spec {
 	/* power management */
 	unsigned int num_pwrs;
 	hda_nid_t *pwr_nids;
+	hda_nid_t *dac_list;
 
 	/* playback */
 	struct hda_input_mux *mono_mux;
@@ -288,6 +289,10 @@ static hda_nid_t stac927x_adc_nids[3] = 
 
 static hda_nid_t stac927x_mux_nids[3] = {
         0x15, 0x16, 0x17
+};
+
+static hda_nid_t stac927x_dac_nids[6] = {
+	0x02, 0x03, 0x04, 0x05, 0x06, 0
 };
 
 static hda_nid_t stac927x_dmux_nids[1] = {
@@ -2858,6 +2863,18 @@ static int is_nid_hp_pin(struct auto_pin
 	return 0; /* nid is not a HP-Out */
 };
 
+static void stac92xx_power_down(struct hda_codec *codec)
+{
+	struct sigmatel_spec *spec = codec->spec;
+
+	/* power down inactive DACs */
+	hda_nid_t *dac;
+	for (dac = spec->dac_list; *dac; dac++)
+		if (!is_in_dac_nids(spec, *dac))
+			snd_hda_codec_write_cache(codec, *dac, 0,
+					AC_VERB_SET_POWER_STATE, AC_PWRST_D3);
+}
+
 static int stac92xx_init(struct hda_codec *codec)
 {
 	struct sigmatel_spec *spec = codec->spec;
@@ -2910,7 +2927,8 @@ static int stac92xx_init(struct hda_code
 		enable_pin_detect(codec, spec->pwr_nids[i], event | i);
 		codec->patch_ops.unsol_event(codec, (event | i) << 26);
 	}
-
+	if (spec->dac_list)
+		stac92xx_power_down(codec);
 	if (cfg->dig_out_pin)
 		stac92xx_auto_set_pinctl(codec, cfg->dig_out_pin,
 					 AC_PINCTL_OUT_EN);
@@ -3083,6 +3101,9 @@ static int stac92xx_resume(struct hda_co
 		spec->gpio_dir, spec->gpio_data);
 	snd_hda_codec_resume_amp(codec);
 	snd_hda_codec_resume_cache(codec);
+	/* power down inactive DACs */
+	if (spec->dac_list)
+		stac92xx_power_down(codec);
 	/* invoke unsolicited event to reset the HP state */
 	if (spec->hp_detect)
 		codec->patch_ops.unsol_event(codec, STAC_HP_EVENT << 26);
@@ -3569,6 +3590,7 @@ static int patch_stac927x(struct hda_cod
 	spec->num_adcs = ARRAY_SIZE(stac927x_adc_nids);
 	spec->mux_nids = stac927x_mux_nids;
 	spec->num_muxes = ARRAY_SIZE(stac927x_mux_nids);
+	spec->dac_list = stac927x_dac_nids;
 	spec->multiout.dac_nids = spec->dac_nids;
 
 	switch (spec->board_config) {
#3 lore raw

At Wed, 30 Jan 2008 17:33:57 -0500,

Quoted 57 lines · Matthew Ranostay
Quoted 55 lines · Takashi Iwai

At Wed, 30 Jan 2008 09:14:02 -0500,

Quoted 3 lines · Matthew Ranostay

On several laptops that have STAC9228 codecs have unused DACs, this powers them down to a D3 state.

I think they will be eventually powered up again in hda_set_power_state() in hda_codec.c. As a temporary solution, we can have a sort of blacklist in struct hda_codec for always turning the power off for such widgets.

As a long-term solution, the automatic detection of such widgets would be nice...

Takashi

Quoted 37 lines

---
diff -r 86c8c0a82bb9 pci/hda/patch_sigmatel.c

pci/hda/patch_sigmatel.c +13 -1
@@ -288,6 +288,10 @@ static hda_nid_t stac927x_adc_nids[3] = 
 
 static hda_nid_t stac927x_mux_nids[3] = {
         0x15, 0x16, 0x17
+};
+
+static hda_nid_t stac927x_dac_nids[5] = {
+	0x02, 0x03, 0x04, 0x05, 0x06
 };
 
 static hda_nid_t stac927x_dmux_nids[1] = {
@@ -3537,7 +3541,7 @@ static int patch_stac927x(struct hda_cod
 static int patch_stac927x(struct hda_codec *codec)
 {
 	struct sigmatel_spec *spec;
-	int err;
+	int err, i;
 
 	spec  = kzalloc(sizeof(*spec), GFP_KERNEL);
 	if (spec == NULL)
@@ -3628,6 +3632,14 @@ static int patch_stac927x(struct hda_cod
 		stac92xx_free(codec);
 		return err;
 	}
+
+	/* on some 3-stack systems not all the DACs are used
+ 	 * this powers down ones that aren't used
+ 	 */
+	for (i = 0; i < ARRAY_SIZE(stac927x_dac_nids); i++)
+		if (!is_in_dac_nids(spec, stac927x_dac_nids[i]))
+			snd_hda_codec_write_cache(codec, stac927x_dac_nids[i],
+				0, AC_VERB_SET_POWER_STATE, AC_PWRST_D3);
 
Quoted 6 lines
Quoted 3 lines

pci/hda/patch_sigmatel.c static int patch_stac927x(struct hda_cod · line 3645

Quoted 2 lines
pci/hda/patch_sigmatel.c +0 -0
 	codec->patch_ops = stac92xx_patch_ops;
 

I think this what you had in mind.
Also is putting the function call in stac92xx_resume correct?

Yeah, thanks, I forgot it.
I applied it to my tree.

Takashi