[alsa-devel] [RFC] [PATCH 0/5] Add vmaster hook for HD-audio mute-LED controls
This is yet another hack for HD-audio. Currently the mute-LED is controlled via abusing the powerstatus check callback for power-saving feature. Thus the mute-LED won't be enabled when user didn't build the driver with CONFIG_SND_HDA_POWER_SAVE=y. With this patchset, the driver will refer to only Master switch to follow the mute-LED status.
The first patch is to add a hook to vmaster control. It's simple and small. The rest are implementations of hooks and replacements of the existing codes with the vmaster hook. The last patch is the addition of the mute-EAPD support on Conexant codec. This was one of the major reason I wanted to implement before 3.4, since it's the biggest missing piece in Conexant auto-parser.
I've checked quickly on a few machines. The patches are found in my sound-unstable tree topic/cxt-fix branch, too. git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-unstable.git topic/cxt-fix
Let me know if you see any problems with this patchset.
thanks,
Takashi
This patch adds a hook to vmaster control to be called at each time when the master value is changed. It'd be handy for an additional mute LED control following the Master switch, for example.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/control.h | 5 +++++ sound/core/vmaster.c | 46 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/include/sound/control.h b/include/sound/control.h index b2796e8..eff96dc 100644 --- a/include/sound/control.h +++ b/include/sound/control.h @@ -227,6 +227,11 @@ snd_ctl_add_slave_uncached(struct snd_kcontrol *master, return _snd_ctl_add_slave(master, slave, SND_CTL_SLAVE_NEED_UPDATE); }
+int snd_ctl_add_vmaster_hook(struct snd_kcontrol *kctl, + void (*hook)(void *private_data, int), + void *private_data); +void snd_ctl_sync_vmaster_hook(struct snd_kcontrol *kctl); + /* * Helper functions for jack-detection controls */ diff --git a/sound/core/vmaster.c b/sound/core/vmaster.c index 130cfe6..14a286a 100644 --- a/sound/core/vmaster.c +++ b/sound/core/vmaster.c @@ -37,6 +37,8 @@ struct link_master { struct link_ctl_info info; int val; /* the master value */ unsigned int tlv[4]; + void (*hook)(void *private_data, int); + void *hook_private_data; };
/* @@ -126,7 +128,9 @@ static int master_init(struct link_master *master) master->info.count = 1; /* always mono */ /* set full volume as default (= no attenuation) */ master->val = master->info.max_val; - return 0; + if (master->hook) + master->hook(master->hook_private_data, master->val); + return 1; } return -ENOENT; } @@ -329,6 +333,8 @@ static int master_put(struct snd_kcontrol *kcontrol, slave_put_val(slave, uval); } kfree(uval); + if (master->hook && !err) + master->hook(master->hook_private_data, master->val); return 1; }
@@ -408,3 +414,41 @@ struct snd_kcontrol *snd_ctl_make_virtual_master(char *name, return kctl; } EXPORT_SYMBOL(snd_ctl_make_virtual_master); + +/** + * snd_ctl_add_vmaster_hook - Add a hook to a vmaster control + * @kcontrol: vmaster kctl element + * @hook: the hook function + * + * Adds the given hook to the vmaster control element so that it's called + * at each time when the value is changed. + */ +int snd_ctl_add_vmaster_hook(struct snd_kcontrol *kcontrol, + void (*hook)(void *private_data, int), + void *private_data) +{ + struct link_master *master = snd_kcontrol_chip(kcontrol); + master->hook = hook; + master->hook_private_data = private_data; + return 0; +} +EXPORT_SYMBOL_GPL(snd_ctl_add_vmaster_hook); + +/** + * snd_ctl_sync_vmaster_hook - Sync the vmaster hook + * @kcontrol: vmaster kctl element + * + * Call the hook function to synchronize with the current value of the given + * vmaster element. NOP when NULL is passed to @kcontrol or the hook doesn't + * exist. + */ +void snd_ctl_sync_vmaster_hook(struct snd_kcontrol *kcontrol) +{ + struct link_master *master; + if (!kcontrol) + return; + master = snd_kcontrol_chip(kcontrol); + if (master->hook) + master->hook(master->hook_private_data, master->val); +} +EXPORT_SYMBOL_GPL(snd_ctl_sync_vmaster_hook);
It'll be used for adding hooks in later patches.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_codec.c | 9 ++++++++- sound/pci/hda/hda_local.h | 7 ++++--- sound/pci/hda/patch_analog.c | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 0c0ac0e..b79ee34 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -2399,6 +2399,7 @@ static int init_slave_unmute(void *data, struct snd_kcontrol *slave) * @slaves: slave control names (optional) * @suffix: suffix string to each slave name (optional) * @init_slave_vol: initialize slaves to unmute/0dB + * @ctl_ret: store the vmaster kcontrol in return * * Create a virtual master control with the given name. The TLV data * must be either NULL or a valid data. @@ -2411,11 +2412,15 @@ static int init_slave_unmute(void *data, struct snd_kcontrol *slave) */ int __snd_hda_add_vmaster(struct hda_codec *codec, char *name, unsigned int *tlv, const char * const *slaves, - const char *suffix, bool init_slave_vol) + const char *suffix, bool init_slave_vol, + struct snd_kcontrol **ctl_ret) { struct snd_kcontrol *kctl; int err;
+ if (ctl_ret) + *ctl_ret = NULL; + err = map_slaves(codec, slaves, suffix, check_slave_present, NULL); if (err != 1) { snd_printdd("No slave found for %s\n", name); @@ -2439,6 +2444,8 @@ int __snd_hda_add_vmaster(struct hda_codec *codec, char *name, map_slaves(codec, slaves, suffix, tlv ? init_slave_0dB : init_slave_unmute, kctl);
+ if (ctl_ret) + *ctl_ret = kctl; return 0; } EXPORT_SYMBOL_HDA(__snd_hda_add_vmaster); diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index caa6468..c3ee4ed 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -140,10 +140,11 @@ void snd_hda_set_vmaster_tlv(struct hda_codec *codec, hda_nid_t nid, int dir, struct snd_kcontrol *snd_hda_find_mixer_ctl(struct hda_codec *codec, const char *name); int __snd_hda_add_vmaster(struct hda_codec *codec, char *name, - unsigned int *tlv, const char * const *slaves, - const char *suffix, bool init_slave_vol); + unsigned int *tlv, const char * const *slaves, + const char *suffix, bool init_slave_vol, + struct snd_kcontrol **ctl_ret); #define snd_hda_add_vmaster(codec, name, tlv, slaves, suffix) \ - __snd_hda_add_vmaster(codec, name, tlv, slaves, suffix, true) + __snd_hda_add_vmaster(codec, name, tlv, slaves, suffix, true, NULL) int snd_hda_codec_reset(struct hda_codec *codec);
/* amp value bits */ diff --git a/sound/pci/hda/patch_analog.c b/sound/pci/hda/patch_analog.c index fa97a0c..7143393 100644 --- a/sound/pci/hda/patch_analog.c +++ b/sound/pci/hda/patch_analog.c @@ -229,7 +229,7 @@ static int ad198x_build_controls(struct hda_codec *codec) (spec->slave_vols ? spec->slave_vols : ad_slave_pfxs), "Playback Volume", - !spec->avoid_init_slave_vol); + !spec->avoid_init_slave_vol, NULL); if (err < 0) return err; }
The mute-LED is controlled in patch_sigmatel.c by (ab-)using the powersave hook. This can be now rewritten with the vmaster hook instead, which is much simpler and can work even without CONFIG_SND_HDA_POWER_SAVE kconfig.
A drawback is that the mute-LED corresponds _only_ to the Master mixer switch instead of checking the whole DACs. But usually this shouldn't be a big problem as PA enables the mixer elements accordingly.
Also, this patch changes the code to create vmaster always even on STAC9200 and STAC925x. The former "Master" on these chips are renamed as "PCM" now.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_sigmatel.c | 136 +++++++++++++++------------------------- 1 file changed, 49 insertions(+), 87 deletions(-)
diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c index 5988dbd..6e92649 100644 --- a/sound/pci/hda/patch_sigmatel.c +++ b/sound/pci/hda/patch_sigmatel.c @@ -310,6 +310,8 @@ struct sigmatel_spec { unsigned long auto_capvols[MAX_ADCS_NUM]; unsigned auto_dmic_cnt; hda_nid_t auto_dmic_nids[MAX_DMICS_NUM]; + + struct snd_kcontrol *vmaster_sw_kctl; };
static const hda_nid_t stac9200_adc_nids[1] = { @@ -1007,8 +1009,8 @@ static const struct hda_verb stac9205_core_init[] = { }
static const struct snd_kcontrol_new stac9200_mixer[] = { - HDA_CODEC_VOLUME_MIN_MUTE("Master Playback Volume", 0xb, 0, HDA_OUTPUT), - HDA_CODEC_MUTE("Master Playback Switch", 0xb, 0, HDA_OUTPUT), + HDA_CODEC_VOLUME_MIN_MUTE("PCM Playback Volume", 0xb, 0, HDA_OUTPUT), + HDA_CODEC_MUTE("PCM Playback Switch", 0xb, 0, HDA_OUTPUT), HDA_CODEC_VOLUME("Capture Volume", 0x0a, 0, HDA_OUTPUT), HDA_CODEC_MUTE("Capture Switch", 0x0a, 0, HDA_OUTPUT), { } /* end */ @@ -1035,8 +1037,8 @@ static const struct snd_kcontrol_new stac92hd71bxx_loopback[] = { };
static const struct snd_kcontrol_new stac925x_mixer[] = { - HDA_CODEC_VOLUME_MIN_MUTE("Master Playback Volume", 0xe, 0, HDA_OUTPUT), - HDA_CODEC_MUTE("Master Playback Switch", 0x0e, 0, HDA_OUTPUT), + HDA_CODEC_VOLUME_MIN_MUTE("PCM Playback Volume", 0xe, 0, HDA_OUTPUT), + HDA_CODEC_MUTE("PCM Playback Switch", 0x0e, 0, HDA_OUTPUT), { } /* end */ };
@@ -1074,11 +1076,19 @@ static const char * const slave_pfxs[] = { NULL };
+static void stac92xx_update_led_status(struct hda_codec *codec, int enabled); + +static void stac92xx_vmaster_hook(void *private_data, int val) +{ + stac92xx_update_led_status(private_data, val); +} + static void stac92xx_free_kctls(struct hda_codec *codec);
static int stac92xx_build_controls(struct hda_codec *codec) { struct sigmatel_spec *spec = codec->spec; + unsigned int vmaster_tlv[4]; int err; int i;
@@ -1135,26 +1145,29 @@ static int stac92xx_build_controls(struct hda_codec *codec) }
/* 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->multiout.dac_nids[0], - HDA_OUTPUT, vmaster_tlv); - /* correct volume offset */ - vmaster_tlv[2] += vmaster_tlv[3] * spec->volume_offset; - /* minimum value is actually mute */ - vmaster_tlv[3] |= TLV_DB_SCALE_MUTE; - err = snd_hda_add_vmaster(codec, "Master Playback Volume", - vmaster_tlv, slave_pfxs, - "Playback Volume"); - if (err < 0) - return err; - } - if (!snd_hda_find_mixer_ctl(codec, "Master Playback Switch")) { - err = snd_hda_add_vmaster(codec, "Master Playback Switch", - NULL, slave_pfxs, - "Playback Switch"); - if (err < 0) - return err; + snd_hda_set_vmaster_tlv(codec, spec->multiout.dac_nids[0], + HDA_OUTPUT, vmaster_tlv); + /* correct volume offset */ + vmaster_tlv[2] += vmaster_tlv[3] * spec->volume_offset; + /* minimum value is actually mute */ + vmaster_tlv[3] |= TLV_DB_SCALE_MUTE; + err = snd_hda_add_vmaster(codec, "Master Playback Volume", + vmaster_tlv, slave_pfxs, + "Playback Volume"); + if (err < 0) + return err; + + err = __snd_hda_add_vmaster(codec, "Master Playback Switch", + NULL, slave_pfxs, + "Playback Switch", true, + &spec->vmaster_sw_kctl); + if (err < 0) + return err; + + if (spec->gpio_led) { + snd_ctl_add_vmaster_hook(spec->vmaster_sw_kctl, + stac92xx_vmaster_hook, codec); + snd_ctl_sync_vmaster_hook(spec->vmaster_sw_kctl); }
if (spec->aloopback_ctl && @@ -4419,8 +4432,7 @@ static int stac92xx_init(struct hda_codec *codec) snd_hda_jack_report_sync(codec);
/* sync mute LED */ - if (spec->gpio_led) - hda_call_check_power_status(codec, 0x01); + snd_ctl_sync_vmaster_hook(spec->vmaster_sw_kctl); if (spec->dac_list) stac92xx_power_down(codec); return 0; @@ -5033,83 +5045,37 @@ static void stac92xx_set_power_state(struct hda_codec *codec, hda_nid_t fg, afg_power_state); snd_hda_codec_set_power_to_all(codec, fg, power_state, true); } +#endif /* CONFIG_SND_HDA_POWER_SAVE */ +#endif /* CONFIG_PM */
-/* - * For this feature CONFIG_SND_HDA_POWER_SAVE is needed - * as mute LED state is updated in check_power_status hook - */ -static int stac92xx_update_led_status(struct hda_codec *codec) +/* update mute-LED accoring to the master switch */ +static void stac92xx_update_led_status(struct hda_codec *codec, int enabled) { struct sigmatel_spec *spec = codec->spec; - int i, num_ext_dacs, muted = 1; - unsigned int muted_lvl, notmtd_lvl; - hda_nid_t nid; + int muted = !enabled;
if (!spec->gpio_led) - return 0; + return; + + /* LED state is inverted on these systems */ + if (spec->gpio_led_polarity) + muted = !muted;
- for (i = 0; i < spec->multiout.num_dacs; i++) { - nid = spec->multiout.dac_nids[i]; - if (!(snd_hda_codec_amp_read(codec, nid, 0, HDA_OUTPUT, 0) & - HDA_AMP_MUTE)) { - muted = 0; /* something heard */ - break; - } - } - if (muted && spec->multiout.hp_nid) - if (!(snd_hda_codec_amp_read(codec, - spec->multiout.hp_nid, 0, HDA_OUTPUT, 0) & - HDA_AMP_MUTE)) { - muted = 0; /* HP is not muted */ - } - num_ext_dacs = ARRAY_SIZE(spec->multiout.extra_out_nid); - for (i = 0; muted && i < num_ext_dacs; i++) { - nid = spec->multiout.extra_out_nid[i]; - if (nid == 0) - break; - if (!(snd_hda_codec_amp_read(codec, nid, 0, HDA_OUTPUT, 0) & - HDA_AMP_MUTE)) { - muted = 0; /* extra output is not muted */ - } - } /*polarity defines *not* muted state level*/ if (!spec->vref_mute_led_nid) { if (muted) spec->gpio_data &= ~spec->gpio_led; /* orange */ else spec->gpio_data |= spec->gpio_led; /* white */ - - if (!spec->gpio_led_polarity) { - /* LED state is inverted on these systems */ - spec->gpio_data ^= spec->gpio_led; - } stac_gpio_set(codec, spec->gpio_mask, spec->gpio_dir, spec->gpio_data); } else { - notmtd_lvl = spec->gpio_led_polarity ? - AC_PINCTL_VREF_50 : AC_PINCTL_VREF_GRD; - muted_lvl = spec->gpio_led_polarity ? - AC_PINCTL_VREF_GRD : AC_PINCTL_VREF_50; - spec->vref_led = muted ? muted_lvl : notmtd_lvl; + spec->vref_led = muted ? AC_PINCTL_VREF_50 : AC_PINCTL_VREF_GRD; stac_vrefout_set(codec, spec->vref_mute_led_nid, spec->vref_led); } - return 0; }
-/* - * use power check for controlling mute led of HP notebooks - */ -static int stac92xx_check_power_status(struct hda_codec *codec, - hda_nid_t nid) -{ - stac92xx_update_led_status(codec); - - return 0; -} -#endif /* CONFIG_SND_HDA_POWER_SAVE */ -#endif /* CONFIG_PM */ - static const struct hda_codec_ops stac92xx_patch_ops = { .build_controls = stac92xx_build_controls, .build_pcms = stac92xx_build_pcms, @@ -5627,8 +5593,6 @@ again: stac92xx_set_power_state; } codec->patch_ops.pre_resume = stac92xx_pre_resume; - codec->patch_ops.check_power_status = - stac92xx_check_power_status; } #endif
@@ -5938,8 +5902,6 @@ again: stac92xx_set_power_state; } codec->patch_ops.pre_resume = stac92xx_pre_resume; - codec->patch_ops.check_power_status = - stac92xx_check_power_status; } #endif
We've had ugly static handling of the mute-LED with a powersave hook for ALC269 HP laptops just like done in patch_sigmatel.c. This is now rewritten with the new vmaster hook and a fixup code.
For that, the new fixup action, ALC_FIXUP_ACT_BUILD, is introduced. It's called after build_controls is called. The reason of this new action is that vmaster hook must be added at this stage (not in init or probe).
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_realtek.c | 82 ++++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 38 deletions(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 1de0c16..9015472 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -198,6 +198,7 @@ struct alc_spec {
/* for virtual master */ hda_nid_t vmaster_nid; + struct snd_kcontrol *vmaster_sw_kctl; #ifdef CONFIG_SND_HDA_POWER_SAVE struct hda_loopback_check loopback; int num_loopbacks; @@ -1441,6 +1442,7 @@ enum { ALC_FIXUP_ACT_PRE_PROBE, ALC_FIXUP_ACT_PROBE, ALC_FIXUP_ACT_INIT, + ALC_FIXUP_ACT_BUILD, };
static void alc_apply_fixup(struct hda_codec *codec, int action) @@ -1955,9 +1957,10 @@ static int __alc_build_controls(struct hda_codec *codec) } if (!spec->no_analog && !snd_hda_find_mixer_ctl(codec, "Master Playback Switch")) { - err = snd_hda_add_vmaster(codec, "Master Playback Switch", - NULL, alc_slave_pfxs, - "Playback Switch"); + err = __snd_hda_add_vmaster(codec, "Master Playback Switch", + NULL, alc_slave_pfxs, + "Playback Switch", + true, &spec->vmaster_sw_kctl); if (err < 0) return err; } @@ -2042,7 +2045,11 @@ static int alc_build_controls(struct hda_codec *codec) int err = __alc_build_controls(codec); if (err < 0) return err; - return snd_hda_jack_add_kctls(codec, &spec->autocfg); + err = snd_hda_jack_add_kctls(codec, &spec->autocfg); + if (err < 0) + return err; + alc_apply_fixup(codec, ALC_FIXUP_ACT_BUILD); + return 0; }
@@ -5721,35 +5728,6 @@ static const struct hda_pcm_stream alc269_44k_pcm_analog_capture = { /* NID is set in alc_build_pcms */ };
-#ifdef CONFIG_SND_HDA_POWER_SAVE -static int alc269_mic2_for_mute_led(struct hda_codec *codec) -{ - switch (codec->subsystem_id) { - case 0x103c1586: - return 1; - } - return 0; -} - -static int alc269_mic2_mute_check_ps(struct hda_codec *codec, hda_nid_t nid) -{ - /* update mute-LED according to the speaker mute state */ - if (nid == 0x01 || nid == 0x14) { - int pinval; - if (snd_hda_codec_amp_read(codec, 0x14, 0, HDA_OUTPUT, 0) & - HDA_AMP_MUTE) - pinval = 0x24; - else - pinval = 0x20; - /* mic2 vref pin is used for mute LED control */ - snd_hda_codec_update_cache(codec, 0x19, 0, - AC_VERB_SET_PIN_WIDGET_CONTROL, - pinval); - } - return alc_check_power_status(codec, nid); -} -#endif /* CONFIG_SND_HDA_POWER_SAVE */ - /* different alc269-variants */ enum { ALC269_TYPE_ALC269VA, @@ -5900,6 +5878,33 @@ static void alc269_fixup_quanta_mute(struct hda_codec *codec, spec->automute_hook = alc269_quanta_automute; }
+/* update mute-LED according to the speaker mute state via mic2 VREF pin */ +static void alc269_fixup_mic2_mute_hook(void *private_data, int enabled) +{ + struct hda_codec *codec = private_data; + unsigned int pinval = enabled ? 0x20 : 0x24; + snd_hda_codec_update_cache(codec, 0x19, 0, + AC_VERB_SET_PIN_WIDGET_CONTROL, + pinval); +} + +static void alc269_fixup_mic2_mute(struct hda_codec *codec, + const struct alc_fixup *fix, int action) +{ + struct alc_spec *spec = codec->spec; + switch (action) { + case ALC_FIXUP_ACT_BUILD: + if (!spec->vmaster_sw_kctl) + return; + snd_ctl_add_vmaster_hook(spec->vmaster_sw_kctl, + alc269_fixup_mic2_mute_hook, codec); + /* fallthru */ + case ALC_FIXUP_ACT_INIT: + snd_ctl_sync_vmaster_hook(spec->vmaster_sw_kctl); + break; + } +} + enum { ALC269_FIXUP_SONY_VAIO, ALC275_FIXUP_SONY_VAIO_GPIO2, @@ -5917,6 +5922,7 @@ enum { ALC269_FIXUP_DMIC, ALC269VB_FIXUP_AMIC, ALC269VB_FIXUP_DMIC, + ALC269_FIXUP_MIC2_MUTE_LED, };
static const struct alc_fixup alc269_fixups[] = { @@ -6037,9 +6043,14 @@ static const struct alc_fixup alc269_fixups[] = { { } }, }, + [ALC269_FIXUP_MIC2_MUTE_LED] = { + .type = ALC_FIXUP_FUNC, + .v.func = alc269_fixup_mic2_mute, + }, };
static const struct snd_pci_quirk alc269_fixup_tbl[] = { + SND_PCI_QUIRK(0x103c, 0x1586, "HP", ALC269_FIXUP_MIC2_MUTE_LED), SND_PCI_QUIRK(0x1043, 0x1a13, "Asus G73Jw", ALC269_FIXUP_ASUS_G73JW), SND_PCI_QUIRK(0x1043, 0x16e3, "ASUS UX50", ALC269_FIXUP_STEREO_DMIC), SND_PCI_QUIRK(0x1043, 0x831a, "ASUS P901", ALC269_FIXUP_STEREO_DMIC), @@ -6231,11 +6242,6 @@ static int patch_alc269(struct hda_codec *codec) #endif spec->shutup = alc269_shutup;
-#ifdef CONFIG_SND_HDA_POWER_SAVE - if (alc269_mic2_for_mute_led(codec)) - codec->patch_ops.check_power_status = alc269_mic2_mute_check_ps; -#endif - alc_apply_fixup(codec, ALC_FIXUP_ACT_PROBE);
return 0;
Added the vmaster hook for controlling EAPD dynamically to Conexant auto-parser. When the Master is muted, EAPDs are turned off as well. This will fix the missing mute-LED control on some machines in addition to the more power-saving in the auto-parser mode.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_conexant.c | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c index 5a56fda..f1c9aed 100644 --- a/sound/pci/hda/patch_conexant.c +++ b/sound/pci/hda/patch_conexant.c @@ -70,6 +70,8 @@ struct conexant_spec { const struct snd_kcontrol_new *mixers[5]; int num_mixers; hda_nid_t vmaster_nid; + struct snd_kcontrol *vmaster_sw_kctl; + void (*vmaster_hook)(struct snd_kcontrol *, int);
const struct hda_verb *init_verbs[5]; /* initialization verbs * don't forget NULL @@ -513,9 +515,10 @@ static int conexant_build_controls(struct hda_codec *codec) } if (spec->vmaster_nid && !snd_hda_find_mixer_ctl(codec, "Master Playback Switch")) { - err = snd_hda_add_vmaster(codec, "Master Playback Switch", - NULL, slave_pfxs, - "Playback Switch"); + err = __snd_hda_add_vmaster(codec, "Master Playback Switch", + NULL, slave_pfxs, + "Playback Switch", true, + &spec->vmaster_sw_kctl); if (err < 0) return err; } @@ -3975,6 +3978,19 @@ static void clear_unsol_on_unused_pins(struct hda_codec *codec) } }
+/* turn on/off EAPD according to Master switch */ +static void cx_auto_vmaster_hook(void *private_data, int enabled) +{ + struct hda_codec *codec = private_data; + struct conexant_spec *spec = codec->spec; + + if (enabled && spec->pin_eapd_ctrls) { + cx_auto_update_speakers(codec); + return; + } + cx_auto_turn_eapd(codec, spec->num_eapds, spec->eapds, enabled); +} + static void cx_auto_init_output(struct hda_codec *codec) { struct conexant_spec *spec = codec->spec; @@ -4079,11 +4095,13 @@ static void cx_auto_init_digital(struct hda_codec *codec)
static int cx_auto_init(struct hda_codec *codec) { + struct conexant_spec *spec = codec->spec; /*snd_hda_sequence_write(codec, cx_auto_init_verbs);*/ cx_auto_init_output(codec); cx_auto_init_input(codec); cx_auto_init_digital(codec); snd_hda_jack_report_sync(codec); + snd_ctl_sync_vmaster_hook(spec->vmaster_sw_kctl); return 0; }
@@ -4329,6 +4347,11 @@ static int cx_auto_build_controls(struct hda_codec *codec) err = snd_hda_jack_add_kctls(codec, &spec->autocfg); if (err < 0) return err; + if (spec->vmaster_hook && spec->vmaster_sw_kctl) { + snd_ctl_add_vmaster_hook(spec->vmaster_sw_kctl, + spec->vmaster_hook, codec); + snd_ctl_sync_vmaster_hook(spec->vmaster_sw_kctl); + } return 0; }
@@ -4353,7 +4376,6 @@ static int cx_auto_search_adcs(struct hda_codec *codec) return 0; }
- static const struct hda_codec_ops cx_auto_patch_ops = { .build_controls = cx_auto_build_controls, .build_pcms = conexant_build_pcms, @@ -4455,6 +4477,12 @@ static int patch_conexant_auto(struct hda_codec *codec)
apply_pin_fixup(codec, cxt_fixups, cxt_pincfg_tbl);
+ /* add EAPD vmaster hook to all HP machines */ + /* NOTE: this should be applied via fixup once when the generic + * fixup code is merged to hda_codec.c + */ + spec->vmaster_hook = cx_auto_vmaster_hook; + err = cx_auto_search_adcs(codec); if (err < 0) return err;
On 03/12/2012 03:28 PM, Takashi Iwai wrote:
This is yet another hack for HD-audio. Currently the mute-LED is controlled via abusing the powerstatus check callback for power-saving feature. Thus the mute-LED won't be enabled when user didn't build the driver with CONFIG_SND_HDA_POWER_SAVE=y. With this patchset, the driver will refer to only Master switch to follow the mute-LED status.
The first patch is to add a hook to vmaster control. It's simple and small. The rest are implementations of hooks and replacements of the existing codes with the vmaster hook. The last patch is the addition of the mute-EAPD support on Conexant codec. This was one of the major reason I wanted to implement before 3.4, since it's the biggest missing piece in Conexant auto-parser.
I've checked quickly on a few machines. The patches are found in my sound-unstable tree topic/cxt-fix branch, too. git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-unstable.git topic/cxt-fix
Let me know if you see any problems with this patchset.
I'm not really happy with it. Or rather, it does not solve my problem. I've heard people say "hmm, when I mute the internal speakers or headphones, the mute LED is lit, but not when I mute HDMI, what's the logic in that?". (And since HDMI might very well be on a separate card, checking all different codecs won't help.)
I believe you need to make the mute LED (and mic mute LED, if present) controllable by userspace. If you like it the way it is, I'm okay with making a kcontrol that has "On", "Off" and "Follow master mute" with the last one being the default.
At Mon, 12 Mar 2012 15:57:35 +0100, David Henningsson wrote:
On 03/12/2012 03:28 PM, Takashi Iwai wrote:
This is yet another hack for HD-audio. Currently the mute-LED is controlled via abusing the powerstatus check callback for power-saving feature. Thus the mute-LED won't be enabled when user didn't build the driver with CONFIG_SND_HDA_POWER_SAVE=y. With this patchset, the driver will refer to only Master switch to follow the mute-LED status.
The first patch is to add a hook to vmaster control. It's simple and small. The rest are implementations of hooks and replacements of the existing codes with the vmaster hook. The last patch is the addition of the mute-EAPD support on Conexant codec. This was one of the major reason I wanted to implement before 3.4, since it's the biggest missing piece in Conexant auto-parser.
I've checked quickly on a few machines. The patches are found in my sound-unstable tree topic/cxt-fix branch, too. git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-unstable.git topic/cxt-fix
Let me know if you see any problems with this patchset.
I'm not really happy with it. Or rather, it does not solve my problem. I've heard people say "hmm, when I mute the internal speakers or headphones, the mute LED is lit, but not when I mute HDMI, what's the logic in that?". (And since HDMI might very well be on a separate card, checking all different codecs won't help.)
I believe you need to make the mute LED (and mic mute LED, if present) controllable by userspace. If you like it the way it is, I'm okay with making a kcontrol that has "On", "Off" and "Follow master mute" with the last one being the default.
It's a good point. Note that the patch isn't meant to solve such problems at all. It's rather a clean-up and improvement of the current code. In the current code, Conexant auto-paresr doesn't provide _any_ mute LED control. With this patch, it provides, at least, the compatible mute LED control like other static quirks. In other words, this is a regression fix.
About the user-space mute-LED: I'm fine to add an enum to change the mute-LED behavior. But also remember that this isn't always easy because the mute-LED and EAPD might be bound together in the hardware level (at least for Conexant). Thus it'd be possible that the speaker is suddenly turned on when you connect an HDMI if the mute-LED is controlled individually.
thanks,
Takashi
At Mon, 12 Mar 2012 16:08:14 +0100, Takashi Iwai wrote:
At Mon, 12 Mar 2012 15:57:35 +0100, David Henningsson wrote:
On 03/12/2012 03:28 PM, Takashi Iwai wrote:
This is yet another hack for HD-audio. Currently the mute-LED is controlled via abusing the powerstatus check callback for power-saving feature. Thus the mute-LED won't be enabled when user didn't build the driver with CONFIG_SND_HDA_POWER_SAVE=y. With this patchset, the driver will refer to only Master switch to follow the mute-LED status.
The first patch is to add a hook to vmaster control. It's simple and small. The rest are implementations of hooks and replacements of the existing codes with the vmaster hook. The last patch is the addition of the mute-EAPD support on Conexant codec. This was one of the major reason I wanted to implement before 3.4, since it's the biggest missing piece in Conexant auto-parser.
I've checked quickly on a few machines. The patches are found in my sound-unstable tree topic/cxt-fix branch, too. git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-unstable.git topic/cxt-fix
Let me know if you see any problems with this patchset.
I'm not really happy with it. Or rather, it does not solve my problem. I've heard people say "hmm, when I mute the internal speakers or headphones, the mute LED is lit, but not when I mute HDMI, what's the logic in that?". (And since HDMI might very well be on a separate card, checking all different codecs won't help.)
I believe you need to make the mute LED (and mic mute LED, if present) controllable by userspace. If you like it the way it is, I'm okay with making a kcontrol that has "On", "Off" and "Follow master mute" with the last one being the default.
It's a good point. Note that the patch isn't meant to solve such problems at all. It's rather a clean-up and improvement of the current code. In the current code, Conexant auto-paresr doesn't provide _any_ mute LED control. With this patch, it provides, at least, the compatible mute LED control like other static quirks. In other words, this is a regression fix.
About the user-space mute-LED: I'm fine to add an enum to change the mute-LED behavior. But also remember that this isn't always easy because the mute-LED and EAPD might be bound together in the hardware level (at least for Conexant). Thus it'd be possible that the speaker is suddenly turned on when you connect an HDMI if the mute-LED is controlled individually.
Below is a quick implementation. Does it look OK for you? It's applied on the top of the previous series.
thanks,
Takashi
--- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Add "Mute-LED Mode" enum control
Create snd_hda_add_vmaster_hook() and snd_hda_sync_vmaster_hook() helper functions to handle the mute-LED in vmaster hook more commonly. In the former function, a new enum control "Mute-LED Mode" is added. This provides user to choose whether the mute-LED should be turned on/off explicitly or to follow the master-mute status.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_codec.c | 94 ++++++++++++++++++++++++++++++++++++++++ sound/pci/hda/hda_local.h | 21 +++++++++ sound/pci/hda/patch_conexant.c | 17 ++++---- sound/pci/hda/patch_realtek.c | 12 +++-- sound/pci/hda/patch_sigmatel.c | 13 +++--- 5 files changed, 135 insertions(+), 22 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index b79ee34..b981ea9 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -2450,6 +2450,100 @@ int __snd_hda_add_vmaster(struct hda_codec *codec, char *name, } EXPORT_SYMBOL_HDA(__snd_hda_add_vmaster);
+/* + * mute-LED control using vmaster + */ +static int vmaster_mute_mode_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + static const char * const texts[] = { + "Off", "On", "Follow Master" + }; + unsigned int index; + + uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED; + uinfo->count = 1; + uinfo->value.enumerated.items = 3; + index = uinfo->value.enumerated.item; + if (index >= 3) + index = 2; + strcpy(uinfo->value.enumerated.name, texts[index]); + return 0; +} + +static int vmaster_mute_mode_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct hda_vmaster_mute_hook *hook = snd_kcontrol_chip(kcontrol); + ucontrol->value.enumerated.item[0] = hook->mute_mode; + return 0; +} + +static int vmaster_mute_mode_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct hda_vmaster_mute_hook *hook = snd_kcontrol_chip(kcontrol); + unsigned int old_mode = hook->mute_mode; + + hook->mute_mode = ucontrol->value.enumerated.item[0]; + if (hook->mute_mode > HDA_VMUTE_FOLLOW_MASTER) + hook->mute_mode = HDA_VMUTE_FOLLOW_MASTER; + if (old_mode == hook->mute_mode) + return 0; + snd_hda_sync_vmaster_hook(hook); + return 1; +} + +static struct snd_kcontrol_new vmaster_mute_mode = { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Mute-LED Mode", + .info = vmaster_mute_mode_info, + .get = vmaster_mute_mode_get, + .put = vmaster_mute_mode_put, +}; + +/* + * Add a mute-LED hook with the given vmaster switch kctl + * "Mute-LED Mode" control is automatically created and associated with + * the given hook. + */ +int snd_hda_add_vmaster_hook(struct hda_codec *codec, + struct hda_vmaster_mute_hook *hook) +{ + struct snd_kcontrol *kctl; + + if (!hook->hook || !hook->sw_kctl) + return 0; + snd_ctl_add_vmaster_hook(hook->sw_kctl, hook->hook, codec); + hook->codec = codec; + hook->mute_mode = HDA_VMUTE_FOLLOW_MASTER; + kctl = snd_ctl_new1(&vmaster_mute_mode, hook); + if (!kctl) + return -ENOMEM; + return snd_hda_ctl_add(codec, 0, kctl); +} +EXPORT_SYMBOL_HDA(snd_hda_add_vmaster_hook); + +/* + * Call the hook with the current value for synchronization + * Should be called in init callback + */ +void snd_hda_sync_vmaster_hook(struct hda_vmaster_mute_hook *hook) +{ + if (!hook->hook || !hook->codec) + return; + switch (hook->mute_mode) { + case HDA_VMUTE_FOLLOW_MASTER: + snd_ctl_sync_vmaster_hook(hook->sw_kctl); + break; + default: + hook->hook(hook->codec, hook->mute_mode); + break; + } +} +EXPORT_SYMBOL_HDA(snd_hda_sync_vmaster_hook); + + /** * snd_hda_mixer_amp_switch_info - Info callback for a standard AMP mixer switch * diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index c3ee4ed..3f82ab6 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -147,6 +147,27 @@ int __snd_hda_add_vmaster(struct hda_codec *codec, char *name, __snd_hda_add_vmaster(codec, name, tlv, slaves, suffix, true, NULL) int snd_hda_codec_reset(struct hda_codec *codec);
+enum { + HDA_VMUTE_OFF, + HDA_VMUTE_ON, + HDA_VMUTE_FOLLOW_MASTER, +}; + +struct hda_vmaster_mute_hook { + /* below two fields must be filled by the caller of + * snd_hda_add_vmaster_hook() beforehand + */ + struct snd_kcontrol *sw_kctl; + void (*hook)(void *, int); + /* below are initialized automatically */ + unsigned int mute_mode; /* HDA_VMUTE_XXX */ + struct hda_codec *codec; +}; + +int snd_hda_add_vmaster_hook(struct hda_codec *codec, + struct hda_vmaster_mute_hook *hook); +void snd_hda_sync_vmaster_hook(struct hda_vmaster_mute_hook *hook); + /* amp value bits */ #define HDA_AMP_MUTE 0x80 #define HDA_AMP_UNMUTE 0x00 diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c index f1c9aed..a21a485 100644 --- a/sound/pci/hda/patch_conexant.c +++ b/sound/pci/hda/patch_conexant.c @@ -70,8 +70,7 @@ struct conexant_spec { const struct snd_kcontrol_new *mixers[5]; int num_mixers; hda_nid_t vmaster_nid; - struct snd_kcontrol *vmaster_sw_kctl; - void (*vmaster_hook)(struct snd_kcontrol *, int); + struct hda_vmaster_mute_hook vmaster_mute;
const struct hda_verb *init_verbs[5]; /* initialization verbs * don't forget NULL @@ -518,7 +517,7 @@ static int conexant_build_controls(struct hda_codec *codec) err = __snd_hda_add_vmaster(codec, "Master Playback Switch", NULL, slave_pfxs, "Playback Switch", true, - &spec->vmaster_sw_kctl); + &spec->vmaster_mute.sw_kctl); if (err < 0) return err; } @@ -4101,7 +4100,7 @@ static int cx_auto_init(struct hda_codec *codec) cx_auto_init_input(codec); cx_auto_init_digital(codec); snd_hda_jack_report_sync(codec); - snd_ctl_sync_vmaster_hook(spec->vmaster_sw_kctl); + snd_hda_sync_vmaster_hook(&spec->vmaster_mute); return 0; }
@@ -4347,10 +4346,10 @@ static int cx_auto_build_controls(struct hda_codec *codec) err = snd_hda_jack_add_kctls(codec, &spec->autocfg); if (err < 0) return err; - if (spec->vmaster_hook && spec->vmaster_sw_kctl) { - snd_ctl_add_vmaster_hook(spec->vmaster_sw_kctl, - spec->vmaster_hook, codec); - snd_ctl_sync_vmaster_hook(spec->vmaster_sw_kctl); + if (spec->vmaster_mute.hook && spec->vmaster_mute.sw_kctl) { + err = snd_hda_add_vmaster_hook(codec, &spec->vmaster_mute); + if (err < 0) + return err; } return 0; } @@ -4481,7 +4480,7 @@ static int patch_conexant_auto(struct hda_codec *codec) /* NOTE: this should be applied via fixup once when the generic * fixup code is merged to hda_codec.c */ - spec->vmaster_hook = cx_auto_vmaster_hook; + spec->vmaster_mute.hook = cx_auto_vmaster_hook;
err = cx_auto_search_adcs(codec); if (err < 0) diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 9015472..b69d2fe 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -198,7 +198,7 @@ struct alc_spec {
/* for virtual master */ hda_nid_t vmaster_nid; - struct snd_kcontrol *vmaster_sw_kctl; + struct hda_vmaster_mute_hook vmaster_mute; #ifdef CONFIG_SND_HDA_POWER_SAVE struct hda_loopback_check loopback; int num_loopbacks; @@ -1960,7 +1960,7 @@ static int __alc_build_controls(struct hda_codec *codec) err = __snd_hda_add_vmaster(codec, "Master Playback Switch", NULL, alc_slave_pfxs, "Playback Switch", - true, &spec->vmaster_sw_kctl); + true, &spec->vmaster_mute.sw_kctl); if (err < 0) return err; } @@ -5894,13 +5894,11 @@ static void alc269_fixup_mic2_mute(struct hda_codec *codec, struct alc_spec *spec = codec->spec; switch (action) { case ALC_FIXUP_ACT_BUILD: - if (!spec->vmaster_sw_kctl) - return; - snd_ctl_add_vmaster_hook(spec->vmaster_sw_kctl, - alc269_fixup_mic2_mute_hook, codec); + spec->vmaster_mute.hook = alc269_fixup_mic2_mute_hook; + snd_hda_add_vmaster_hook(codec, &spec->vmaster_mute); /* fallthru */ case ALC_FIXUP_ACT_INIT: - snd_ctl_sync_vmaster_hook(spec->vmaster_sw_kctl); + snd_hda_sync_vmaster_hook(&spec->vmaster_mute); break; } } diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c index 6e92649..cd04e29 100644 --- a/sound/pci/hda/patch_sigmatel.c +++ b/sound/pci/hda/patch_sigmatel.c @@ -311,7 +311,7 @@ struct sigmatel_spec { unsigned auto_dmic_cnt; hda_nid_t auto_dmic_nids[MAX_DMICS_NUM];
- struct snd_kcontrol *vmaster_sw_kctl; + struct hda_vmaster_mute_hook vmaster_mute; };
static const hda_nid_t stac9200_adc_nids[1] = { @@ -1160,14 +1160,15 @@ static int stac92xx_build_controls(struct hda_codec *codec) err = __snd_hda_add_vmaster(codec, "Master Playback Switch", NULL, slave_pfxs, "Playback Switch", true, - &spec->vmaster_sw_kctl); + &spec->vmaster_mute.sw_kctl); if (err < 0) return err;
if (spec->gpio_led) { - snd_ctl_add_vmaster_hook(spec->vmaster_sw_kctl, - stac92xx_vmaster_hook, codec); - snd_ctl_sync_vmaster_hook(spec->vmaster_sw_kctl); + spec->vmaster_mute.hook = stac92xx_vmaster_hook; + err = snd_hda_add_vmaster_hook(codec, &spec->vmaster_mute); + if (err < 0) + return err; }
if (spec->aloopback_ctl && @@ -4432,7 +4433,7 @@ static int stac92xx_init(struct hda_codec *codec) snd_hda_jack_report_sync(codec);
/* sync mute LED */ - snd_ctl_sync_vmaster_hook(spec->vmaster_sw_kctl); + snd_hda_sync_vmaster_hook(&spec->vmaster_mute); if (spec->dac_list) stac92xx_power_down(codec); return 0;
On 03/12/2012 05:03 PM, Takashi Iwai wrote:
At Mon, 12 Mar 2012 16:08:14 +0100, Takashi Iwai wrote:
At Mon, 12 Mar 2012 15:57:35 +0100, David Henningsson wrote:
On 03/12/2012 03:28 PM, Takashi Iwai wrote:
This is yet another hack for HD-audio. Currently the mute-LED is controlled via abusing the powerstatus check callback for power-saving feature. Thus the mute-LED won't be enabled when user didn't build the driver with CONFIG_SND_HDA_POWER_SAVE=y. With this patchset, the driver will refer to only Master switch to follow the mute-LED status.
The first patch is to add a hook to vmaster control. It's simple and small. The rest are implementations of hooks and replacements of the existing codes with the vmaster hook. The last patch is the addition of the mute-EAPD support on Conexant codec. This was one of the major reason I wanted to implement before 3.4, since it's the biggest missing piece in Conexant auto-parser.
I've checked quickly on a few machines. The patches are found in my sound-unstable tree topic/cxt-fix branch, too. git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-unstable.git topic/cxt-fix
Let me know if you see any problems with this patchset.
I'm not really happy with it. Or rather, it does not solve my problem. I've heard people say "hmm, when I mute the internal speakers or headphones, the mute LED is lit, but not when I mute HDMI, what's the logic in that?". (And since HDMI might very well be on a separate card, checking all different codecs won't help.)
I believe you need to make the mute LED (and mic mute LED, if present) controllable by userspace. If you like it the way it is, I'm okay with making a kcontrol that has "On", "Off" and "Follow master mute" with the last one being the default.
It's a good point. Note that the patch isn't meant to solve such problems at all. It's rather a clean-up and improvement of the current code. In the current code, Conexant auto-paresr doesn't provide _any_ mute LED control. With this patch, it provides, at least, the compatible mute LED control like other static quirks. In other words, this is a regression fix.
About the user-space mute-LED: I'm fine to add an enum to change the mute-LED behavior. But also remember that this isn't always easy because the mute-LED and EAPD might be bound together in the hardware level (at least for Conexant). Thus it'd be possible that the speaker is suddenly turned on when you connect an HDMI if the mute-LED is controlled individually.
Below is a quick implementation. Does it look OK for you? It's applied on the top of the previous series.
It looks very good to me, thanks! I guess we'll need mic mute LED's also, but that can come later. Maybe it's just those Thinkpads who have mic mute LEDs now anyway, and maybe they do that through ACPI...
thanks,
Takashi
From: Takashi Iwaitiwai@suse.de Subject: [PATCH] ALSA: hda - Add "Mute-LED Mode" enum control
Create snd_hda_add_vmaster_hook() and snd_hda_sync_vmaster_hook() helper functions to handle the mute-LED in vmaster hook more commonly. In the former function, a new enum control "Mute-LED Mode" is added. This provides user to choose whether the mute-LED should be turned on/off explicitly or to follow the master-mute status.
Signed-off-by: Takashi Iwaitiwai@suse.de
Reviewed-by: David Henningsson david.henningsson@canonical.com
sound/pci/hda/hda_codec.c | 94 ++++++++++++++++++++++++++++++++++++++++ sound/pci/hda/hda_local.h | 21 +++++++++ sound/pci/hda/patch_conexant.c | 17 ++++---- sound/pci/hda/patch_realtek.c | 12 +++-- sound/pci/hda/patch_sigmatel.c | 13 +++--- 5 files changed, 135 insertions(+), 22 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index b79ee34..b981ea9 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -2450,6 +2450,100 @@ int __snd_hda_add_vmaster(struct hda_codec *codec, char *name, } EXPORT_SYMBOL_HDA(__snd_hda_add_vmaster);
+/*
- mute-LED control using vmaster
- */
+static int vmaster_mute_mode_info(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_info *uinfo)+{
- static const char * const texts[] = {
"Off", "On", "Follow Master"- };
- unsigned int index;
- uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED;
- uinfo->count = 1;
- uinfo->value.enumerated.items = 3;
- index = uinfo->value.enumerated.item;
- if (index>= 3)
index = 2;- strcpy(uinfo->value.enumerated.name, texts[index]);
- return 0;
+}
+static int vmaster_mute_mode_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct hda_vmaster_mute_hook *hook = snd_kcontrol_chip(kcontrol);
- ucontrol->value.enumerated.item[0] = hook->mute_mode;
- return 0;
+}
+static int vmaster_mute_mode_put(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct hda_vmaster_mute_hook *hook = snd_kcontrol_chip(kcontrol);
- unsigned int old_mode = hook->mute_mode;
- hook->mute_mode = ucontrol->value.enumerated.item[0];
- if (hook->mute_mode> HDA_VMUTE_FOLLOW_MASTER)
hook->mute_mode = HDA_VMUTE_FOLLOW_MASTER;- if (old_mode == hook->mute_mode)
return 0;- snd_hda_sync_vmaster_hook(hook);
- return 1;
+}
+static struct snd_kcontrol_new vmaster_mute_mode = {
- .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
- .name = "Mute-LED Mode",
- .info = vmaster_mute_mode_info,
- .get = vmaster_mute_mode_get,
- .put = vmaster_mute_mode_put,
+};
+/*
- Add a mute-LED hook with the given vmaster switch kctl
- "Mute-LED Mode" control is automatically created and associated with
- the given hook.
- */
+int snd_hda_add_vmaster_hook(struct hda_codec *codec,
struct hda_vmaster_mute_hook *hook)+{
- struct snd_kcontrol *kctl;
- if (!hook->hook || !hook->sw_kctl)
return 0;- snd_ctl_add_vmaster_hook(hook->sw_kctl, hook->hook, codec);
- hook->codec = codec;
- hook->mute_mode = HDA_VMUTE_FOLLOW_MASTER;
- kctl = snd_ctl_new1(&vmaster_mute_mode, hook);
- if (!kctl)
return -ENOMEM;- return snd_hda_ctl_add(codec, 0, kctl);
+} +EXPORT_SYMBOL_HDA(snd_hda_add_vmaster_hook);
+/*
- Call the hook with the current value for synchronization
- Should be called in init callback
- */
+void snd_hda_sync_vmaster_hook(struct hda_vmaster_mute_hook *hook) +{
- if (!hook->hook || !hook->codec)
return;- switch (hook->mute_mode) {
- case HDA_VMUTE_FOLLOW_MASTER:
snd_ctl_sync_vmaster_hook(hook->sw_kctl);break;- default:
hook->hook(hook->codec, hook->mute_mode);break;- }
+} +EXPORT_SYMBOL_HDA(snd_hda_sync_vmaster_hook);
- /**
- snd_hda_mixer_amp_switch_info - Info callback for a standard AMP mixer switch
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index c3ee4ed..3f82ab6 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -147,6 +147,27 @@ int __snd_hda_add_vmaster(struct hda_codec *codec, char *name, __snd_hda_add_vmaster(codec, name, tlv, slaves, suffix, true, NULL) int snd_hda_codec_reset(struct hda_codec *codec);
+enum {
- HDA_VMUTE_OFF,
- HDA_VMUTE_ON,
- HDA_VMUTE_FOLLOW_MASTER,
+};
+struct hda_vmaster_mute_hook {
- /* below two fields must be filled by the caller of
* snd_hda_add_vmaster_hook() beforehand*/- struct snd_kcontrol *sw_kctl;
- void (*hook)(void *, int);
- /* below are initialized automatically */
- unsigned int mute_mode; /* HDA_VMUTE_XXX */
- struct hda_codec *codec;
+};
+int snd_hda_add_vmaster_hook(struct hda_codec *codec,
struct hda_vmaster_mute_hook *hook);+void snd_hda_sync_vmaster_hook(struct hda_vmaster_mute_hook *hook);
- /* amp value bits */ #define HDA_AMP_MUTE 0x80 #define HDA_AMP_UNMUTE 0x00
diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c index f1c9aed..a21a485 100644 --- a/sound/pci/hda/patch_conexant.c +++ b/sound/pci/hda/patch_conexant.c @@ -70,8 +70,7 @@ struct conexant_spec { const struct snd_kcontrol_new *mixers[5]; int num_mixers; hda_nid_t vmaster_nid;
- struct snd_kcontrol *vmaster_sw_kctl;
- void (*vmaster_hook)(struct snd_kcontrol *, int);
struct hda_vmaster_mute_hook vmaster_mute;
const struct hda_verb *init_verbs[5]; /* initialization verbs * don't forget NULL
@@ -518,7 +517,7 @@ static int conexant_build_controls(struct hda_codec *codec) err = __snd_hda_add_vmaster(codec, "Master Playback Switch", NULL, slave_pfxs, "Playback Switch", true,
&spec->vmaster_sw_kctl);
&spec->vmaster_mute.sw_kctl);@@ -4101,7 +4100,7 @@ static int cx_auto_init(struct hda_codec *codec) cx_auto_init_input(codec); cx_auto_init_digital(codec); snd_hda_jack_report_sync(codec);
- snd_ctl_sync_vmaster_hook(spec->vmaster_sw_kctl);
- snd_hda_sync_vmaster_hook(&spec->vmaster_mute); return 0; }
@@ -4347,10 +4346,10 @@ static int cx_auto_build_controls(struct hda_codec *codec) err = snd_hda_jack_add_kctls(codec,&spec->autocfg); if (err< 0) return err;
- if (spec->vmaster_hook&& spec->vmaster_sw_kctl) {
snd_ctl_add_vmaster_hook(spec->vmaster_sw_kctl,spec->vmaster_hook, codec);snd_ctl_sync_vmaster_hook(spec->vmaster_sw_kctl);
- if (spec->vmaster_mute.hook&& spec->vmaster_mute.sw_kctl) {
err = snd_hda_add_vmaster_hook(codec,&spec->vmaster_mute);if (err< 0)return err;@@ -4481,7 +4480,7 @@ static int patch_conexant_auto(struct hda_codec *codec) /* NOTE: this should be applied via fixup once when the generic * fixup code is merged to hda_codec.c */
- spec->vmaster_hook = cx_auto_vmaster_hook;
spec->vmaster_mute.hook = cx_auto_vmaster_hook;
err = cx_auto_search_adcs(codec); if (err< 0)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 9015472..b69d2fe 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -198,7 +198,7 @@ struct alc_spec {
/* for virtual master */ hda_nid_t vmaster_nid;
- struct snd_kcontrol *vmaster_sw_kctl;
- struct hda_vmaster_mute_hook vmaster_mute; #ifdef CONFIG_SND_HDA_POWER_SAVE struct hda_loopback_check loopback; int num_loopbacks;
@@ -1960,7 +1960,7 @@ static int __alc_build_controls(struct hda_codec *codec) err = __snd_hda_add_vmaster(codec, "Master Playback Switch", NULL, alc_slave_pfxs, "Playback Switch",
true,&spec->vmaster_sw_kctl);
true,&spec->vmaster_mute.sw_kctl);@@ -5894,13 +5894,11 @@ static void alc269_fixup_mic2_mute(struct hda_codec *codec, struct alc_spec *spec = codec->spec; switch (action) { case ALC_FIXUP_ACT_BUILD:
if (!spec->vmaster_sw_kctl)return;snd_ctl_add_vmaster_hook(spec->vmaster_sw_kctl,alc269_fixup_mic2_mute_hook, codec);
spec->vmaster_mute.hook = alc269_fixup_mic2_mute_hook;snd_hda_add_vmaster_hook(codec,&spec->vmaster_mute);
snd_ctl_sync_vmaster_hook(spec->vmaster_sw_kctl);
snd_hda_sync_vmaster_hook(&spec->vmaster_mute);diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c index 6e92649..cd04e29 100644 --- a/sound/pci/hda/patch_sigmatel.c +++ b/sound/pci/hda/patch_sigmatel.c @@ -311,7 +311,7 @@ struct sigmatel_spec { unsigned auto_dmic_cnt; hda_nid_t auto_dmic_nids[MAX_DMICS_NUM];
- struct snd_kcontrol *vmaster_sw_kctl;
struct hda_vmaster_mute_hook vmaster_mute; };
static const hda_nid_t stac9200_adc_nids[1] = {
@@ -1160,14 +1160,15 @@ static int stac92xx_build_controls(struct hda_codec *codec) err = __snd_hda_add_vmaster(codec, "Master Playback Switch", NULL, slave_pfxs, "Playback Switch", true,
&spec->vmaster_sw_kctl);
&spec->vmaster_mute.sw_kctl);if (err< 0) return err;
if (spec->gpio_led) {
snd_ctl_add_vmaster_hook(spec->vmaster_sw_kctl,stac92xx_vmaster_hook, codec);snd_ctl_sync_vmaster_hook(spec->vmaster_sw_kctl);
spec->vmaster_mute.hook = stac92xx_vmaster_hook;err = snd_hda_add_vmaster_hook(codec,&spec->vmaster_mute);if (err< 0)return err;}
if (spec->aloopback_ctl&&
@@ -4432,7 +4433,7 @@ static int stac92xx_init(struct hda_codec *codec) snd_hda_jack_report_sync(codec);
/* sync mute LED */
- snd_ctl_sync_vmaster_hook(spec->vmaster_sw_kctl);
- snd_hda_sync_vmaster_hook(&spec->vmaster_mute); if (spec->dac_list) stac92xx_power_down(codec); return 0;
At Mon, 12 Mar 2012 23:12:12 +0100, David Henningsson wrote:
On 03/12/2012 05:03 PM, Takashi Iwai wrote:
At Mon, 12 Mar 2012 16:08:14 +0100, Takashi Iwai wrote:
At Mon, 12 Mar 2012 15:57:35 +0100, David Henningsson wrote:
On 03/12/2012 03:28 PM, Takashi Iwai wrote:
This is yet another hack for HD-audio. Currently the mute-LED is controlled via abusing the powerstatus check callback for power-saving feature. Thus the mute-LED won't be enabled when user didn't build the driver with CONFIG_SND_HDA_POWER_SAVE=y. With this patchset, the driver will refer to only Master switch to follow the mute-LED status.
The first patch is to add a hook to vmaster control. It's simple and small. The rest are implementations of hooks and replacements of the existing codes with the vmaster hook. The last patch is the addition of the mute-EAPD support on Conexant codec. This was one of the major reason I wanted to implement before 3.4, since it's the biggest missing piece in Conexant auto-parser.
I've checked quickly on a few machines. The patches are found in my sound-unstable tree topic/cxt-fix branch, too. git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-unstable.git topic/cxt-fix
Let me know if you see any problems with this patchset.
I'm not really happy with it. Or rather, it does not solve my problem. I've heard people say "hmm, when I mute the internal speakers or headphones, the mute LED is lit, but not when I mute HDMI, what's the logic in that?". (And since HDMI might very well be on a separate card, checking all different codecs won't help.)
I believe you need to make the mute LED (and mic mute LED, if present) controllable by userspace. If you like it the way it is, I'm okay with making a kcontrol that has "On", "Off" and "Follow master mute" with the last one being the default.
It's a good point. Note that the patch isn't meant to solve such problems at all. It's rather a clean-up and improvement of the current code. In the current code, Conexant auto-paresr doesn't provide _any_ mute LED control. With this patch, it provides, at least, the compatible mute LED control like other static quirks. In other words, this is a regression fix.
About the user-space mute-LED: I'm fine to add an enum to change the mute-LED behavior. But also remember that this isn't always easy because the mute-LED and EAPD might be bound together in the hardware level (at least for Conexant). Thus it'd be possible that the speaker is suddenly turned on when you connect an HDMI if the mute-LED is controlled individually.
Below is a quick implementation. Does it look OK for you? It's applied on the top of the previous series.
It looks very good to me, thanks! I guess we'll need mic mute LED's also, but that can come later.
Which machines? I know of OLPC, but we haven't implemented it for others, no?
Maybe it's just those Thinkpads who have mic mute LEDs now anyway, and maybe they do that through ACPI...
Possible.
BTW, on the second thought, I think it's better not to expose the mute-LED control blindly for Conexant. At least, some machines are known to be using EAPD really as EAPD. And some, at least HP machines, are known to use EAPD only as the mute LED control.
So, I added a flag to the function not to expose the mute-LED at creation time. The additional patch is below.
thanks,
Takashi
--- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Add expose_enum_ctl flag to snd_hda_add_vmaster_hook()
Since it's not always safe to assume that the vmaster hook is purely the mute-LED control, add the flag indicating whether to expose the mute-LED enum control or not. Currently, conexant codec sets this off for non-HP laptops where EAPD may be used really as EAPD.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_codec.c | 5 ++++- sound/pci/hda/hda_local.h | 3 ++- sound/pci/hda/patch_conexant.c | 21 +++++++++++++++------ sound/pci/hda/patch_realtek.c | 2 +- sound/pci/hda/patch_sigmatel.c | 2 +- 5 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index b981ea9..7a8fcc4 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -2508,7 +2508,8 @@ static struct snd_kcontrol_new vmaster_mute_mode = { * the given hook. */ int snd_hda_add_vmaster_hook(struct hda_codec *codec, - struct hda_vmaster_mute_hook *hook) + struct hda_vmaster_mute_hook *hook, + bool expose_enum_ctl) { struct snd_kcontrol *kctl;
@@ -2517,6 +2518,8 @@ int snd_hda_add_vmaster_hook(struct hda_codec *codec, snd_ctl_add_vmaster_hook(hook->sw_kctl, hook->hook, codec); hook->codec = codec; hook->mute_mode = HDA_VMUTE_FOLLOW_MASTER; + if (!expose_enum_ctl) + return 0; kctl = snd_ctl_new1(&vmaster_mute_mode, hook); if (!kctl) return -ENOMEM; diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index 3f82ab6..0ec9248 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -165,7 +165,8 @@ struct hda_vmaster_mute_hook { };
int snd_hda_add_vmaster_hook(struct hda_codec *codec, - struct hda_vmaster_mute_hook *hook); + struct hda_vmaster_mute_hook *hook, + bool expose_enum_ctl); void snd_hda_sync_vmaster_hook(struct hda_vmaster_mute_hook *hook);
/* amp value bits */ diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c index a21a485..e6eafb1 100644 --- a/sound/pci/hda/patch_conexant.c +++ b/sound/pci/hda/patch_conexant.c @@ -71,6 +71,7 @@ struct conexant_spec { int num_mixers; hda_nid_t vmaster_nid; struct hda_vmaster_mute_hook vmaster_mute; + bool vmaster_mute_led;
const struct hda_verb *init_verbs[5]; /* initialization verbs * don't forget NULL @@ -4346,8 +4347,10 @@ static int cx_auto_build_controls(struct hda_codec *codec) err = snd_hda_jack_add_kctls(codec, &spec->autocfg); if (err < 0) return err; - if (spec->vmaster_mute.hook && spec->vmaster_mute.sw_kctl) { - err = snd_hda_add_vmaster_hook(codec, &spec->vmaster_mute); + if (spec->vmaster_mute.sw_kctl) { + spec->vmaster_mute.hook = cx_auto_vmaster_hook; + err = snd_hda_add_vmaster_hook(codec, &spec->vmaster_mute, + spec->vmaster_mute_led); if (err < 0) return err; } @@ -4476,11 +4479,17 @@ static int patch_conexant_auto(struct hda_codec *codec)
apply_pin_fixup(codec, cxt_fixups, cxt_pincfg_tbl);
- /* add EAPD vmaster hook to all HP machines */ - /* NOTE: this should be applied via fixup once when the generic - * fixup code is merged to hda_codec.c + /* Show mute-led control only on HP laptops + * This is a sort of white-list: on HP laptops, EAPD corresponds + * only to the mute-LED without actualy amp function. Meanwhile, + * others may use EAPD really as an amp switch, so it might be + * not good to expose it blindly. */ - spec->vmaster_mute.hook = cx_auto_vmaster_hook; + switch (codec->subsystem_id >> 16) { + case 0x103c: + spec->vmaster_mute_led = 1; + break; + }
err = cx_auto_search_adcs(codec); if (err < 0) diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index b69d2fe..8ea2fd6 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -5895,7 +5895,7 @@ static void alc269_fixup_mic2_mute(struct hda_codec *codec, switch (action) { case ALC_FIXUP_ACT_BUILD: spec->vmaster_mute.hook = alc269_fixup_mic2_mute_hook; - snd_hda_add_vmaster_hook(codec, &spec->vmaster_mute); + snd_hda_add_vmaster_hook(codec, &spec->vmaster_mute, true); /* fallthru */ case ALC_FIXUP_ACT_INIT: snd_hda_sync_vmaster_hook(&spec->vmaster_mute); diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c index cd04e29..153b9ae 100644 --- a/sound/pci/hda/patch_sigmatel.c +++ b/sound/pci/hda/patch_sigmatel.c @@ -1166,7 +1166,7 @@ static int stac92xx_build_controls(struct hda_codec *codec)
if (spec->gpio_led) { spec->vmaster_mute.hook = stac92xx_vmaster_hook; - err = snd_hda_add_vmaster_hook(codec, &spec->vmaster_mute); + err = snd_hda_add_vmaster_hook(codec, &spec->vmaster_mute, true); if (err < 0) return err; }
On 03/13/2012 08:05 AM, Takashi Iwai wrote:
At Mon, 12 Mar 2012 23:12:12 +0100,
It looks very good to me, thanks! I guess we'll need mic mute LED's also, but that can come later.
Which machines? I know of OLPC, but we haven't implemented it for others, no?
I have asked around a little and so far it seems to be only Thinkpads, and they are generally controlled through ACPI.
Maybe the collective audience of this mailinglist knows of more machines...
participants (2)
-
David Henningsson -
Takashi Iwai