At Wed, 14 Apr 2010 14:42:44 +0200, I wrote:
At Tue, 13 Apr 2010 15:38:53 -0300, Herton Ronaldo Krzesinski wrote:
On VIA VT1812/VT2002P, the "Master Front Playback Switch" doesn't mute first line-out. This is an issue, for example I saw a laptop with VT1812 and only one line-out (main speaker) that doesn't mute main speaker, because the master switch doesn't act on first line-out, and first line-out nid in array is assigned to speaker as expected by autoconfig code.
But there is one more issue also: main switches and automute code deal with mute on same Amp-Outs, which can cause conflicts, thus if you mute "Master Front Playback Switch" and remove headphone for example, it will not respect the mixer setting. To solve it, we can change the pin type instead of muting Amp-Out, which is done here.
The problem is rather that "Master Front" is a misleading name. "Master Front" volume control exists for other VIA codecs because there is another "Front" volume that is assigned to a pin widget. So, this "Master" isn't really a master control.
IMO, a better way would be to rename this "Master Front" to either a pin-specific one ("Speaker", etc), and make a vmaster control to bind both this and headphone controls, like other codecs.
I'm going to try to implement it...
Something like below.
BTW, I fixed another bugs in patch_via.c, which should go to 2.6.34. They are included in the latest sound git tree or alsa-driver snapshot.
Give it a try.
thanks,
Takashi
--- diff --git a/sound/pci/hda/patch_via.c b/sound/pci/hda/patch_via.c index 7345381..0f1d221 100644 --- a/sound/pci/hda/patch_via.c +++ b/sound/pci/hda/patch_via.c @@ -157,6 +157,11 @@ struct via_spec { #ifdef CONFIG_SND_HDA_POWER_SAVE struct hda_loopback_check loopback; #endif + + /* for virtual master */ + hda_nid_t vmaster_nid; + const char **slave_vols; + const char **slave_sws; };
static struct via_spec * via_new_spec(struct hda_codec *codec) @@ -1881,11 +1886,35 @@ static struct hda_pcm_stream vt1708_pcm_digital_capture = { .channels_max = 2, };
+/* + * slave controls for virtual master + */ +static const char *via_slave_vols[] = { + "Front Playback Volume", + "Surround Playback Volume", + "Center Playback Volume", + "LFE Playback Volume", + "Side Playback Volume", + "Headphone Playback Volume", + NULL, +}; + +static const char *via_slave_sws[] = { + "Front Playback Switch", + "Surround Playback Switch", + "Center Playback Switch", + "LFE Playback Switch", + "Side Playback Switch", + "Headphone Playback Switch", + NULL, +}; + static int via_build_controls(struct hda_codec *codec) { struct via_spec *spec = codec->spec; struct snd_kcontrol *kctl; struct snd_kcontrol_new *knew; + const char **slave; int err, i;
for (i = 0; i < spec->num_mixers; i++) { @@ -1911,6 +1940,31 @@ static int via_build_controls(struct hda_codec *codec) return err; }
+ /* if we have no master control, let's create it */ + if (!snd_hda_find_mixer_ctl(codec, "Master Playback Volume")) { + unsigned int vmaster_tlv[4]; + snd_hda_set_vmaster_tlv(codec, spec->vmaster_nid, + HDA_OUTPUT, vmaster_tlv); + if (spec->slave_vols) + slave = spec->slave_vols; + else + slave = via_slave_vols; + err = snd_hda_add_vmaster(codec, "Master Playback Volume", + vmaster_tlv, slave); + if (err < 0) + return err; + } + if (!snd_hda_find_mixer_ctl(codec, "Master Playback Switch")) { + if (spec->slave_vols) + slave = spec->slave_sws; + else + slave = via_slave_sws; + err = snd_hda_add_vmaster(codec, "Master Playback Switch", + NULL, slave); + if (err < 0) + return err; + } + /* assign Capture Source enums to NID */ kctl = snd_hda_find_mixer_ctl(codec, "Input Source"); for (i = 0; kctl && i < kctl->count; i++) { @@ -2268,6 +2322,19 @@ static int vt1708_auto_fill_dac_nids(struct via_spec *spec, return 0; }
+static const char *get_line_out_label(const struct auto_pin_cfg *cfg, + int can_be_master) +{ + if (can_be_master && !cfg->speaker_pins[0] && !cfg->hp_pins[0]) + return "Master"; + else if (cfg->line_out_type == AUTO_PIN_SPEAKER_OUT) + return "Speaker"; + else if (cfg->line_out_type == AUTO_PIN_HP_OUT) + return "Headphone"; + else + return "Line-Out"; +} + /* add playback controls from the parsed DAC table */ static int vt1708_auto_create_multi_out_ctls(struct via_spec *spec, const struct auto_pin_cfg *cfg) @@ -2311,44 +2378,36 @@ static int vt1708_auto_create_multi_out_ctls(struct via_spec *spec, HDA_OUTPUT)); if (err < 0) return err; - } else if (i == AUTO_SEQ_FRONT) { - /* add control to mixer index 0 */ - err = via_add_control(spec, VIA_CTL_WIDGET_VOL, - "Master Front Playback Volume", - HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, - HDA_INPUT)); - if (err < 0) - return err; - err = via_add_control(spec, VIA_CTL_WIDGET_MUTE, - "Master Front Playback Switch", - HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, - HDA_INPUT)); - if (err < 0) - return err; - - /* add control to PW3 */ + } else { + if (!spec->vmaster_nid) + spec->vmaster_nid = nid_vol; sprintf(name, "%s Playback Volume", chname[i]); err = via_add_control(spec, VIA_CTL_WIDGET_VOL, name, - HDA_COMPOSE_AMP_VAL(nid, 3, 0, + HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, HDA_OUTPUT)); if (err < 0) return err; sprintf(name, "%s Playback Switch", chname[i]); err = via_add_control(spec, VIA_CTL_WIDGET_MUTE, name, - HDA_COMPOSE_AMP_VAL(nid, 3, 0, + HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, HDA_OUTPUT)); if (err < 0) return err; - } else { - sprintf(name, "%s Playback Volume", chname[i]); + if (i != AUTO_SEQ_FRONT) + continue; + + /* add control to PW3 */ + sprintf(name, "%s Playback Volume", + get_line_out_label(cfg, 0)); err = via_add_control(spec, VIA_CTL_WIDGET_VOL, name, - HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, + HDA_COMPOSE_AMP_VAL(nid, 3, 0, HDA_OUTPUT)); if (err < 0) return err; - sprintf(name, "%s Playback Switch", chname[i]); + sprintf(name, "%s Playback Switch", + get_line_out_label(cfg, 0)); err = via_add_control(spec, VIA_CTL_WIDGET_MUTE, name, - HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, + HDA_COMPOSE_AMP_VAL(nid, 3, 0, HDA_OUTPUT)); if (err < 0) return err; @@ -2923,35 +2982,9 @@ static int vt1709_auto_create_multi_out_ctls(struct via_spec *spec, HDA_OUTPUT)); if (err < 0) return err; - } else if (i == AUTO_SEQ_FRONT) { - /* ADD control to mixer index 0 */ - err = via_add_control(spec, VIA_CTL_WIDGET_VOL, - "Master Front Playback Volume", - HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, - HDA_INPUT)); - if (err < 0) - return err; - err = via_add_control(spec, VIA_CTL_WIDGET_MUTE, - "Master Front Playback Switch", - HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, - HDA_INPUT)); - if (err < 0) - return err; - - /* add control to PW3 */ - sprintf(name, "%s Playback Volume", chname[i]); - err = via_add_control(spec, VIA_CTL_WIDGET_VOL, name, - HDA_COMPOSE_AMP_VAL(nid, 3, 0, - HDA_OUTPUT)); - if (err < 0) - return err; - sprintf(name, "%s Playback Switch", chname[i]); - err = via_add_control(spec, VIA_CTL_WIDGET_MUTE, name, - HDA_COMPOSE_AMP_VAL(nid, 3, 0, - HDA_OUTPUT)); - if (err < 0) - return err; - } else if (i == AUTO_SEQ_SURROUND) { + } else { + if (!spec->vmaster_nid) + spec->vmaster_nid = nid_vol; sprintf(name, "%s Playback Volume", chname[i]); err = via_add_control(spec, VIA_CTL_WIDGET_VOL, name, HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, @@ -2964,16 +2997,21 @@ static int vt1709_auto_create_multi_out_ctls(struct via_spec *spec, HDA_OUTPUT)); if (err < 0) return err; - } else if (i == AUTO_SEQ_SIDE) { - sprintf(name, "%s Playback Volume", chname[i]); + if (i != AUTO_SEQ_FRONT) + continue; + + /* add control to PW3 */ + sprintf(name, "%s Playback Volume", + get_line_out_label(cfg, 0)); err = via_add_control(spec, VIA_CTL_WIDGET_VOL, name, - HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, + HDA_COMPOSE_AMP_VAL(nid, 3, 0, HDA_OUTPUT)); if (err < 0) return err; - sprintf(name, "%s Playback Switch", chname[i]); + sprintf(name, "%s Playback Volume", + get_line_out_label(cfg, 0)); err = via_add_control(spec, VIA_CTL_WIDGET_MUTE, name, - HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, + HDA_COMPOSE_AMP_VAL(nid, 3, 0, HDA_OUTPUT)); if (err < 0) return err; @@ -3504,44 +3542,36 @@ static int vt1708B_auto_create_multi_out_ctls(struct via_spec *spec, HDA_OUTPUT)); if (err < 0) return err; - } else if (i == AUTO_SEQ_FRONT) { - /* add control to mixer index 0 */ - err = via_add_control(spec, VIA_CTL_WIDGET_VOL, - "Master Front Playback Volume", - HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, - HDA_INPUT)); - if (err < 0) - return err; - err = via_add_control(spec, VIA_CTL_WIDGET_MUTE, - "Master Front Playback Switch", - HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, - HDA_INPUT)); - if (err < 0) - return err; - - /* add control to PW3 */ + } else { + if (!spec->vmaster_nid) + spec->vmaster_nid = nid_vol; sprintf(name, "%s Playback Volume", chname[i]); err = via_add_control(spec, VIA_CTL_WIDGET_VOL, name, - HDA_COMPOSE_AMP_VAL(nid, 3, 0, + HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, HDA_OUTPUT)); if (err < 0) return err; sprintf(name, "%s Playback Switch", chname[i]); err = via_add_control(spec, VIA_CTL_WIDGET_MUTE, name, - HDA_COMPOSE_AMP_VAL(nid, 3, 0, + HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, HDA_OUTPUT)); if (err < 0) return err; - } else { - sprintf(name, "%s Playback Volume", chname[i]); + if (i != AUTO_SEQ_FRONT) + continue; + + /* add control to PW3 */ + sprintf(name, "%s Playback Volume", + get_line_out_label(cfg, 0)); err = via_add_control(spec, VIA_CTL_WIDGET_VOL, name, - HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, + HDA_COMPOSE_AMP_VAL(nid, 3, 0, HDA_OUTPUT)); if (err < 0) return err; - sprintf(name, "%s Playback Switch", chname[i]); + sprintf(name, "%s Playback Volume", + get_line_out_label(cfg, 0)); err = via_add_control(spec, VIA_CTL_WIDGET_MUTE, name, - HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, + HDA_COMPOSE_AMP_VAL(nid, 3, 0, HDA_OUTPUT)); if (err < 0) return err; @@ -3966,22 +3996,9 @@ static int vt1708S_auto_create_multi_out_ctls(struct via_spec *spec, HDA_OUTPUT)); if (err < 0) return err; - } else if (i == AUTO_SEQ_FRONT) { - /* add control to mixer index 0 */ - err = via_add_control(spec, VIA_CTL_WIDGET_VOL, - "Master Front Playback Volume", - HDA_COMPOSE_AMP_VAL(0x16, 3, 0, - HDA_INPUT)); - if (err < 0) - return err; - err = via_add_control(spec, VIA_CTL_WIDGET_MUTE, - "Master Front Playback Switch", - HDA_COMPOSE_AMP_VAL(0x16, 3, 0, - HDA_INPUT)); - if (err < 0) - return err; - - /* Front */ + } else { + if (!spec->vmaster_nid) + spec->vmaster_nid = nid_vol; sprintf(name, "%s Playback Volume", chname[i]); err = via_add_control(spec, VIA_CTL_WIDGET_VOL, name, HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, @@ -3995,14 +4012,18 @@ static int vt1708S_auto_create_multi_out_ctls(struct via_spec *spec, HDA_OUTPUT)); if (err < 0) return err; - } else { - sprintf(name, "%s Playback Volume", chname[i]); + if (i != AUTO_SEQ_FRONT) + continue; + + sprintf(name, "%s Playback Volume", + get_line_out_label(cfg, 0)); err = via_add_control(spec, VIA_CTL_WIDGET_VOL, name, HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, HDA_OUTPUT)); if (err < 0) return err; - sprintf(name, "%s Playback Switch", chname[i]); + sprintf(name, "%s Playback Volume", + get_line_out_label(cfg, 0)); err = via_add_control(spec, VIA_CTL_WIDGET_MUTE, name, HDA_COMPOSE_AMP_VAL(nid_mute, 3, 0, @@ -4366,31 +4387,33 @@ static int vt1702_auto_fill_dac_nids(struct via_spec *spec, static int vt1702_auto_create_line_out_ctls(struct via_spec *spec, const struct auto_pin_cfg *cfg) { + char name[32]; int err;
if (!cfg->line_out_pins[0]) return -1;
+ spec->vmaster_nid = 0x1a; /* add control to mixer index 0 */ err = via_add_control(spec, VIA_CTL_WIDGET_VOL, - "Master Front Playback Volume", + "Front Playback Volume", HDA_COMPOSE_AMP_VAL(0x1A, 3, 0, HDA_INPUT)); if (err < 0) return err; err = via_add_control(spec, VIA_CTL_WIDGET_MUTE, - "Master Front Playback Switch", + "Front Playback Switch", HDA_COMPOSE_AMP_VAL(0x1A, 3, 0, HDA_INPUT)); if (err < 0) return err;
/* Front */ - err = via_add_control(spec, VIA_CTL_WIDGET_VOL, - "Front Playback Volume", + sprintf(name, "%s Playback Volume", get_line_out_label(cfg, 1)); + err = via_add_control(spec, VIA_CTL_WIDGET_VOL, name, HDA_COMPOSE_AMP_VAL(0x10, 3, 0, HDA_OUTPUT)); if (err < 0) return err; - err = via_add_control(spec, VIA_CTL_WIDGET_MUTE, - "Front Playback Switch", + sprintf(name, "%s Playback Switch", get_line_out_label(cfg, 1)); + err = via_add_control(spec, VIA_CTL_WIDGET_MUTE, name, HDA_COMPOSE_AMP_VAL(0x16, 3, 0, HDA_OUTPUT)); if (err < 0) return err; @@ -4793,22 +4816,9 @@ static int vt1718S_auto_create_multi_out_ctls(struct via_spec *spec, HDA_OUTPUT)); if (err < 0) return err; - } else if (i == AUTO_SEQ_FRONT) { - /* Front */ - sprintf(name, "%s Playback Volume", chname[i]); - err = via_add_control( - spec, VIA_CTL_WIDGET_VOL, name, - HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, HDA_OUTPUT)); - if (err < 0) - return err; - sprintf(name, "%s Playback Switch", chname[i]); - err = via_add_control( - spec, VIA_CTL_WIDGET_MUTE, name, - HDA_COMPOSE_AMP_VAL(nid_mute, 3, 0, - HDA_OUTPUT)); - if (err < 0) - return err; } else { + if (!spec->vmaster_nid) + spec->vmaster_nid = nid_vol; sprintf(name, "%s Playback Volume", chname[i]); err = via_add_control( spec, VIA_CTL_WIDGET_VOL, name, @@ -5279,21 +5289,9 @@ static int vt1716S_auto_create_multi_out_ctls(struct via_spec *spec, HDA_OUTPUT)); if (err < 0) return err; - } else if (i == AUTO_SEQ_FRONT) { - - err = via_add_control( - spec, VIA_CTL_WIDGET_VOL, - "Master Front Playback Volume", - HDA_COMPOSE_AMP_VAL(0x16, 3, 0, HDA_INPUT)); - if (err < 0) - return err; - err = via_add_control( - spec, VIA_CTL_WIDGET_MUTE, - "Master Front Playback Switch", - HDA_COMPOSE_AMP_VAL(0x16, 3, 0, HDA_INPUT)); - if (err < 0) - return err; - + } else { + if (!spec->vmaster_nid) + spec->vmaster_nid = nid_vol; sprintf(name, "%s Playback Volume", chname[i]); err = via_add_control( spec, VIA_CTL_WIDGET_VOL, name, @@ -5307,18 +5305,19 @@ static int vt1716S_auto_create_multi_out_ctls(struct via_spec *spec, HDA_OUTPUT)); if (err < 0) return err; - } else { - sprintf(name, "%s Playback Volume", chname[i]); - err = via_add_control( - spec, VIA_CTL_WIDGET_VOL, name, - HDA_COMPOSE_AMP_VAL(nid_vol, 3, 0, HDA_OUTPUT)); + if (i != AUTO_SEQ_FRONT) + continue; + + sprintf(name, "%s Playback Volume", + get_line_out_label(cfg, 0)); + err = via_add_control(spec, VIA_CTL_WIDGET_VOL, name, + HDA_COMPOSE_AMP_VAL(0x16, 3, 0, HDA_INPUT)); if (err < 0) return err; - sprintf(name, "%s Playback Switch", chname[i]); - err = via_add_control( - spec, VIA_CTL_WIDGET_MUTE, name, - HDA_COMPOSE_AMP_VAL(nid_mute, 3, 0, - HDA_OUTPUT)); + sprintf(name, "%s Playback Switch", + get_line_out_label(cfg, 0)); + err = via_add_control(spec, VIA_CTL_WIDGET_MUTE, name, + HDA_COMPOSE_AMP_VAL(0x16, 3, 0, HDA_INPUT)); if (err < 0) return err; } @@ -5646,24 +5645,41 @@ static int vt2002P_auto_fill_dac_nids(struct via_spec *spec, return 0; }
+static const char *vt2002_slave_vols[] = { + "Headphone Playback Volume", + "Speaker Playback Volume", + "Line-Out Playback Volume", + NULL, +}; + +static const char *vt2002_slave_sws[] = { + "Headphone Playback Switch", + "Speaker Playback Switch", + "Line-Out Playback Switch", + NULL, +}; + /* add playback controls from the parsed DAC table */ static int vt2002P_auto_create_multi_out_ctls(struct via_spec *spec, const struct auto_pin_cfg *cfg) { + char name[32]; int err;
if (!cfg->line_out_pins[0]) return -1;
- + spec->vmaster_nid = 0x08; + spec->slave_vols = vt2002_slave_vols; + spec->slave_sws = vt2002_slave_sws; /* Line-Out: PortE */ - err = via_add_control(spec, VIA_CTL_WIDGET_VOL, - "Master Front Playback Volume", + sprintf(name, "%s Playback Volume", get_line_out_label(cfg, 1)); + err = via_add_control(spec, VIA_CTL_WIDGET_VOL, name, HDA_COMPOSE_AMP_VAL(0x8, 3, 0, HDA_OUTPUT)); if (err < 0) return err; - err = via_add_control(spec, VIA_CTL_WIDGET_BIND_PIN_MUTE, - "Master Front Playback Switch", + sprintf(name, "%s Playback Switch", get_line_out_label(cfg, 1)); + err = via_add_control(spec, VIA_CTL_WIDGET_BIND_PIN_MUTE, name, HDA_COMPOSE_AMP_VAL(0x26, 3, 0, HDA_OUTPUT)); if (err < 0) return err; @@ -6000,19 +6016,23 @@ static int vt1812_auto_fill_dac_nids(struct via_spec *spec, static int vt1812_auto_create_multi_out_ctls(struct via_spec *spec, const struct auto_pin_cfg *cfg) { + char name[32]; int err;
if (!cfg->line_out_pins[0]) return -1;
+ spec->vmaster_nid = 0x08; + spec->slave_vols = vt2002_slave_vols; + spec->slave_sws = vt2002_slave_sws; /* Line-Out: PortE */ - err = via_add_control(spec, VIA_CTL_WIDGET_VOL, - "Front Playback Volume", + sprintf(name, "%s Playback Volume", get_line_out_label(cfg, 1)); + err = via_add_control(spec, VIA_CTL_WIDGET_VOL, name, HDA_COMPOSE_AMP_VAL(0x8, 3, 0, HDA_OUTPUT)); if (err < 0) return err; - err = via_add_control(spec, VIA_CTL_WIDGET_BIND_PIN_MUTE, - "Front Playback Switch", + sprintf(name, "%s Playback Switch", get_line_out_label(cfg, 1)); + err = via_add_control(spec, VIA_CTL_WIDGET_BIND_PIN_MUTE, name, HDA_COMPOSE_AMP_VAL(0x28, 3, 0, HDA_OUTPUT)); if (err < 0) return err;