All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ALSA: hda - Fix 'Beep Playback Switch' with no underlying mute switch
@ 2012-08-13 15:30 David Henningsson
  2012-08-13 15:39 ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: David Henningsson @ 2012-08-13 15:30 UTC (permalink / raw)
  To: tiwai, alsa-devel; +Cc: David Henningsson

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 :-)

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.

Example alsa info: codecs/canonical/cx20590-lenovo-thinkpad-t420-ccert-201102-7230

 sound/pci/hda/hda_beep.c       |    6 ++++--
 sound/pci/hda/patch_conexant.c |    8 +++++++-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/sound/pci/hda/hda_beep.c b/sound/pci/hda/hda_beep.c
index d26ae65..3a72543 100644
--- a/sound/pci/hda/hda_beep.c
+++ b/sound/pci/hda/hda_beep.c
@@ -237,9 +237,9 @@ int snd_hda_mixer_amp_switch_get_beep(struct snd_kcontrol *kcontrol,
 {
 	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
 	struct hda_beep *beep = codec->beep;
-	if (beep && !beep->enabled) {
+	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;
 		return 0;
 	}
 	return snd_hda_mixer_amp_switch_get(kcontrol, ucontrol);
@@ -263,6 +263,8 @@ int snd_hda_mixer_amp_switch_put_beep(struct snd_kcontrol *kcontrol,
 			enable |= *valp;
 		snd_hda_enable_beep_device(codec, enable);
 	}
+	if (!get_amp_nid(kcontrol))
+		return 0;
 	return snd_hda_mixer_amp_switch_put(kcontrol, ucontrol);
 }
 EXPORT_SYMBOL_HDA(snd_hda_mixer_amp_switch_put_beep);
diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
index 5e22a8f..fa36c5e 100644
--- a/sound/pci/hda/patch_conexant.c
+++ b/sound/pci/hda/patch_conexant.c
@@ -537,12 +537,18 @@ static int conexant_build_controls(struct hda_codec *codec)
 	/* create beep controls if needed */
 	if (spec->beep_amp) {
 		const struct snd_kcontrol_new *knew;
+		unsigned int caps;
+		caps = snd_hda_param_read(codec, spec->beep_amp, AC_PAR_AMP_OUT_CAP);
+
 		for (knew = cxt_beep_mixer; knew->name; knew++) {
 			struct snd_kcontrol *kctl;
 			kctl = snd_ctl_new1(knew, codec);
 			if (!kctl)
 				return -ENOMEM;
-			kctl->private_value = spec->beep_amp;
+			if (knew->info == snd_hda_mixer_amp_switch_info && !(caps & AC_AMPCAP_MUTE))
+				kctl->private_value = 0x10000; /* Mono channel, no nid */
+			else
+				kctl->private_value = spec->beep_amp;
 			err = snd_hda_ctl_add(codec, 0, kctl);
 			if (err < 0)
 				return err;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ALSA: hda - Fix 'Beep Playback Switch' with no underlying mute switch
  2012-08-13 15:30 [PATCH] ALSA: hda - Fix 'Beep Playback Switch' with no underlying mute switch David Henningsson
@ 2012-08-13 15:39 ` Takashi Iwai
  2012-08-14  7:02   ` David Henningsson
  0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2012-08-13 15:39 UTC (permalink / raw)
  To: David Henningsson; +Cc: alsa-devel

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

> 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.


thanks,

Takashi

> 
> Example alsa info: codecs/canonical/cx20590-lenovo-thinkpad-t420-ccert-201102-7230
> 
>  sound/pci/hda/hda_beep.c       |    6 ++++--
>  sound/pci/hda/patch_conexant.c |    8 +++++++-
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_beep.c b/sound/pci/hda/hda_beep.c
> index d26ae65..3a72543 100644
> --- a/sound/pci/hda/hda_beep.c
> +++ b/sound/pci/hda/hda_beep.c
> @@ -237,9 +237,9 @@ int snd_hda_mixer_amp_switch_get_beep(struct snd_kcontrol *kcontrol,
>  {
>  	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
>  	struct hda_beep *beep = codec->beep;
> -	if (beep && !beep->enabled) {
> +	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;
>  		return 0;
>  	}
>  	return snd_hda_mixer_amp_switch_get(kcontrol, ucontrol);
> @@ -263,6 +263,8 @@ int snd_hda_mixer_amp_switch_put_beep(struct snd_kcontrol *kcontrol,
>  			enable |= *valp;
>  		snd_hda_enable_beep_device(codec, enable);
>  	}
> +	if (!get_amp_nid(kcontrol))
> +		return 0;
>  	return snd_hda_mixer_amp_switch_put(kcontrol, ucontrol);
>  }
>  EXPORT_SYMBOL_HDA(snd_hda_mixer_amp_switch_put_beep);
> diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> index 5e22a8f..fa36c5e 100644
> --- a/sound/pci/hda/patch_conexant.c
> +++ b/sound/pci/hda/patch_conexant.c
> @@ -537,12 +537,18 @@ static int conexant_build_controls(struct hda_codec *codec)
>  	/* create beep controls if needed */
>  	if (spec->beep_amp) {
>  		const struct snd_kcontrol_new *knew;
> +		unsigned int caps;
> +		caps = snd_hda_param_read(codec, spec->beep_amp, AC_PAR_AMP_OUT_CAP);
> +
>  		for (knew = cxt_beep_mixer; knew->name; knew++) {
>  			struct snd_kcontrol *kctl;
>  			kctl = snd_ctl_new1(knew, codec);
>  			if (!kctl)
>  				return -ENOMEM;
> -			kctl->private_value = spec->beep_amp;
> +			if (knew->info == snd_hda_mixer_amp_switch_info && !(caps & AC_AMPCAP_MUTE))
> +				kctl->private_value = 0x10000; /* Mono channel, no nid */
> +			else
> +				kctl->private_value = spec->beep_amp;
>  			err = snd_hda_ctl_add(codec, 0, kctl);
>  			if (err < 0)
>  				return err;
> -- 
> 1.7.9.5
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ALSA: hda - Fix 'Beep Playback Switch' with no underlying mute switch
  2012-08-13 15:39 ` Takashi Iwai
@ 2012-08-14  7:02   ` David Henningsson
  2012-08-14  7:44     ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: David Henningsson @ 2012-08-14  7:02 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

[-- Attachment #1: Type: text/plain, Size: 1047 bytes --]

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?


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

[-- Attachment #2: 0001-ALSA-hda-Fix-Beep-Playback-Switch-with-no-underlying.patch --]
[-- Type: text/x-patch, Size: 2968 bytes --]

>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;
 		return 0;
 	}
 	return snd_hda_mixer_amp_switch_get(kcontrol, ucontrol);
@@ -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;
 	return snd_hda_mixer_amp_switch_put(kcontrol, ucontrol);
 }
 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


[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ALSA: hda - Fix 'Beep Playback Switch' with no underlying mute switch
  2012-08-14  7:02   ` David Henningsson
@ 2012-08-14  7:44     ` Takashi Iwai
  2012-08-14  8:04       ` David Henningsson
  0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2012-08-14  7:44 UTC (permalink / raw)
  To: David Henningsson; +Cc: alsa-devel

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;
>  		return 0;
>  	}
>  	return snd_hda_mixer_amp_switch_get(kcontrol, ucontrol);
> @@ -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;
>  	return snd_hda_mixer_amp_switch_put(kcontrol, ucontrol);
>  }
>  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
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ALSA: hda - Fix 'Beep Playback Switch' with no underlying mute switch
  2012-08-14  7:44     ` Takashi Iwai
@ 2012-08-14  8:04       ` David Henningsson
  2012-08-14  8:23         ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: David Henningsson @ 2012-08-14  8:04 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

[-- Attachment #1: Type: text/plain, Size: 637 bytes --]

On 08/14/2012 09:44 AM, Takashi Iwai wrote:
>>>> 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:

Ok, third version attached.



-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

[-- Attachment #2: 0001-ALSA-hda-Fix-Beep-Playback-Switch-with-no-underlying.patch --]
[-- Type: text/x-patch, Size: 2007 bytes --]

>From 885d5e0f099b089ebec7b61d6caeb7e751d1f6e7 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 |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/sound/pci/hda/hda_beep.c b/sound/pci/hda/hda_beep.c
index d26ae65..e22413e 100644
--- a/sound/pci/hda/hda_beep.c
+++ b/sound/pci/hda/hda_beep.c
@@ -231,15 +231,22 @@ void snd_hda_detach_beep_device(struct hda_codec *codec)
 }
 EXPORT_SYMBOL_HDA(snd_hda_detach_beep_device);
 
+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;
+}
+
 /* 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 && (!ctl_has_mute(kcontrol) || !beep->enabled)) {
 		ucontrol->value.integer.value[0] =
-			ucontrol->value.integer.value[1] = 0;
+			ucontrol->value.integer.value[1] = beep->enabled;
 		return 0;
 	}
 	return snd_hda_mixer_amp_switch_get(kcontrol, ucontrol);
@@ -263,6 +270,8 @@ int snd_hda_mixer_amp_switch_put_beep(struct snd_kcontrol *kcontrol,
 			enable |= *valp;
 		snd_hda_enable_beep_device(codec, enable);
 	}
+	if (!ctl_has_mute(kcontrol))
+		return 0;
 	return snd_hda_mixer_amp_switch_put(kcontrol, ucontrol);
 }
 EXPORT_SYMBOL_HDA(snd_hda_mixer_amp_switch_put_beep);
-- 
1.7.9.5


[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ALSA: hda - Fix 'Beep Playback Switch' with no underlying mute switch
  2012-08-14  8:04       ` David Henningsson
@ 2012-08-14  8:23         ` Takashi Iwai
  0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2012-08-14  8:23 UTC (permalink / raw)
  To: David Henningsson; +Cc: alsa-devel

At Tue, 14 Aug 2012 10:04:05 +0200,
David Henningsson wrote:
> 
> On 08/14/2012 09:44 AM, Takashi Iwai wrote:
> >>>> 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:
> 
> Ok, third version attached.

Thanks, applied now (with a minor optimization).


Takashi

> 
> 
> 
> -- 
> David Henningsson, Canonical Ltd.
> https://launchpad.net/~diwic

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-08-14  7:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-13 15:30 [PATCH] ALSA: hda - Fix 'Beep Playback Switch' with no underlying mute switch David Henningsson
2012-08-13 15:39 ` Takashi Iwai
2012-08-14  7:02   ` David Henningsson
2012-08-14  7:44     ` Takashi Iwai
2012-08-14  8:04       ` David Henningsson
2012-08-14  8:23         ` Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.