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; }