At Tue, 14 Aug 2012 09:02:38 +0200, David Henningsson wrote:
On 08/13/2012 05:39 PM, Takashi Iwai wrote:
At Mon, 13 Aug 2012 17:30:13 +0200, David Henningsson wrote:
Some conexant devices has no mute capability on their Beep widgets. This patch makes sure we don't try setting mutes on those widgets.
Signed-off-by: David Henningsson david.henningsson@canonical.com
Another Beep fix. It's not that I'm suddenly obsessed with beeping; it's my automated test tool that finds them :-)
Good that you can demonstrate the results ;)
Yes, I'm still working on the test tool. Got to have something to show at Plumber's :-)
I admit "knew->info == snd_hda_mixer_amp_switch_info" and "kctl->private_value = 0x10000" looks a bit hacky, feel free to suggest something more elegant if you wish.
Checking the amp out caps in snd_hda_mixer_amp_switch_*_beep() would be better, IMO. It's not necessarily limited to patch_conexant.c.
Ok, here comes a second version of the patch. What do you think?
Better to use query_amp_caps(). It's cached, so the succeeding call is cheap. For example:
static bool ctl_has_mute(struct snd_kcontrol *kcontrol) { struct hda_codec *codec = snd_kcontrol_chip(kcontrol); return query_amp_caps(codec, get_amp_nid(kcontrol), get_amp_direction(kcontrol)) & AC_AMPCAP_MUTE; }
.... int snd_hda_mixer_amp_switch_get_beep(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { .... if (beep && (!beep->enabled || !ctl_has_mute(kcontrol)) { ucontrol->value.integer.value[0] = ucontrol->value.integer.value[1] = beep->enabled; return 0; } .... }
thanks,
Takashi
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic [2 0001-ALSA-hda-Fix-Beep-Playback-Switch-with-no-underlying.patch <text/x-patch (7bit)>]
From 82710da74aa51e3fa1c19f1d575cbe124327e501 Mon Sep 17 00:00:00 2001
From: David Henningsson david.henningsson@canonical.com Date: Mon, 13 Aug 2012 17:10:46 +0200 Subject: [PATCH] ALSA: hda - Fix 'Beep Playback Switch' with no underlying mute switch
Some Conexant devices (e g CX20590) have no mute capability on their Beep widgets. This patch makes sure we don't try setting mutes on those widgets.
Signed-off-by: David Henningsson david.henningsson@canonical.com
sound/pci/hda/hda_beep.c | 26 ++++++++++++++++++++++++-- sound/pci/hda/hda_beep.h | 1 + 2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/hda_beep.c b/sound/pci/hda/hda_beep.c index d26ae65..815dc88 100644 --- a/sound/pci/hda/hda_beep.c +++ b/sound/pci/hda/hda_beep.c @@ -231,15 +231,33 @@ void snd_hda_detach_beep_device(struct hda_codec *codec) } EXPORT_SYMBOL_HDA(snd_hda_detach_beep_device);
+static void verify_amp_has_mute(struct snd_kcontrol *kcontrol) +{
- unsigned int caps;
- int dir;
- struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
- struct hda_beep *beep = codec->beep;
- if (snd_BUG_ON(!beep))
return;- dir = get_amp_direction(kcontrol) == HDA_INPUT ? AC_PAR_AMP_IN_CAP : AC_PAR_AMP_OUT_CAP;
- caps = snd_hda_param_read(codec, get_amp_nid(kcontrol), dir);
- if (!(caps & AC_AMPCAP_MUTE))
kcontrol->private_value &= 0xffff0000; /* No NID to set */- beep->amp_verified = 1;
+}
/* get/put callbacks for beep mute mixer switches */ int snd_hda_mixer_amp_switch_get_beep(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { struct hda_codec *codec = snd_kcontrol_chip(kcontrol); struct hda_beep *beep = codec->beep;
- if (beep && !beep->enabled) {
- if (beep && !beep->amp_verified)
verify_amp_has_mute(kcontrol);- if (beep && (!get_amp_nid(kcontrol) || !beep->enabled)) { ucontrol->value.integer.value[0] =
ucontrol->value.integer.value[1] = 0;
ucontrol->value.integer.value[1] = beep->enabled;@@ -262,7 +280,11 @@ int snd_hda_mixer_amp_switch_put_beep(struct snd_kcontrol *kcontrol, if (chs & 2) enable |= *valp; snd_hda_enable_beep_device(codec, enable);
if (!beep->amp_verified)verify_amp_has_mute(kcontrol);- if (!get_amp_nid(kcontrol))
return 0;} EXPORT_SYMBOL_HDA(snd_hda_mixer_amp_switch_put_beep); diff --git a/sound/pci/hda/hda_beep.h b/sound/pci/hda/hda_beep.h index 4dc6933..923e91a 100644 --- a/sound/pci/hda/hda_beep.h +++ b/sound/pci/hda/hda_beep.h @@ -36,6 +36,7 @@ struct hda_beep { hda_nid_t nid; unsigned int enabled:1; unsigned int linear_tone:1; /* linear tone for IDT/STAC codec */
- unsigned int amp_verified:1; /* Have we verified that the node has a mute switch? */ struct work_struct beep_work; /* scheduled task for beep event */ struct mutex mutex;
};
1.7.9.5