All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] ALSA: hda beep clean up
@ 2012-07-03 16:03 Takashi Iwai
  2012-07-03 16:03 ` [PATCH 1/4] ALSA: hda - Remove beep_mode=2 Takashi Iwai
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Takashi Iwai @ 2012-07-03 16:03 UTC (permalink / raw)
  To: alsa-devel

Hi,

this is a series of small cleanups of HD-audio beep code.
Since the keyboard driver already handles the simultaneous outputs
from the multiple input beep instances, there is no merit to have
beep_mode=2 option to switch the beep input device.


Takashi

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

* [PATCH 1/4] ALSA: hda - Remove beep_mode=2
  2012-07-03 16:03 [PATCH 0/4] ALSA: hda beep clean up Takashi Iwai
@ 2012-07-03 16:03 ` Takashi Iwai
  2012-07-03 16:03 ` [PATCH 2/4] ALSA: hda - Move beep helper functions to hda_beep.c Takashi Iwai
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2012-07-03 16:03 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai

The beep_mode=2 option was introduced to make the beep mixer
controlling the beep input allocation/deallocation dynamically, so
that a user can switch between HD-audio codec digital beep and the
system beep only via mixer API.  This was necessary because the
keyboard driver took only the first input beep instance at that time.

However, the recent keyboard driver already processes the multiple
input instances, thus there is no point to keep this mode.

Let's remove it.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 Documentation/sound/alsa/ALSA-Configuration.txt |    3 +-
 sound/pci/hda/Kconfig                           |    7 ++---
 sound/pci/hda/hda_beep.c                        |   37 +----------------------
 sound/pci/hda/hda_beep.h                        |    4 ---
 sound/pci/hda/hda_intel.c                       |    6 ++--
 5 files changed, 8 insertions(+), 49 deletions(-)

diff --git a/Documentation/sound/alsa/ALSA-Configuration.txt b/Documentation/sound/alsa/ALSA-Configuration.txt
index 221b810..4e4d0bc 100644
--- a/Documentation/sound/alsa/ALSA-Configuration.txt
+++ b/Documentation/sound/alsa/ALSA-Configuration.txt
@@ -875,8 +875,7 @@ Prior to version 0.9.0rc4 options had a 'snd_' prefix. This was removed.
     		setup before initializing the codecs.  This option is
 		available only when CONFIG_SND_HDA_PATCH_LOADER=y is set.
 		See HD-Audio.txt for details.
-    beep_mode	- Selects the beep registration mode (0=off, 1=on, 2=
-		dynamic registration via mute switch on/off); the default
+    beep_mode	- Selects the beep registration mode (0=off, 1=on); default
 		value is set via CONFIG_SND_HDA_INPUT_BEEP_MODE kconfig.
     
     [Single (global) options]
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
index d030797..194d625 100644
--- a/sound/pci/hda/Kconfig
+++ b/sound/pci/hda/Kconfig
@@ -53,15 +53,14 @@ config SND_HDA_INPUT_BEEP
 	  driver. This interface is used to generate digital beeps.
 
 config SND_HDA_INPUT_BEEP_MODE
-	int "Digital beep registration mode (0=off, 1=on, 2=mute sw on/off)"
+	int "Digital beep registration mode (0=off, 1=on)"
 	depends on SND_HDA_INPUT_BEEP=y
 	default "1"
-	range 0 2
+	range 0 1
 	help
 	  Set 0 to disable the digital beep interface for HD-audio by default.
 	  Set 1 to always enable the digital beep interface for HD-audio by
-	  default. Set 2 to control the beep device registration to input
-	  layer using a "Beep Switch" in mixer applications.
+	  default.
 
 config SND_HDA_INPUT_JACK
 	bool "Support jack plugging notification via input layer"
diff --git a/sound/pci/hda/hda_beep.c b/sound/pci/hda/hda_beep.c
index 60738e5..662de6e 100644
--- a/sound/pci/hda/hda_beep.c
+++ b/sound/pci/hda/hda_beep.c
@@ -162,28 +162,6 @@ static int snd_hda_do_attach(struct hda_beep *beep)
 	return 0;
 }
 
-static void snd_hda_do_register(struct work_struct *work)
-{
-	struct hda_beep *beep =
-		container_of(work, struct hda_beep, register_work);
-
-	mutex_lock(&beep->mutex);
-	if (beep->enabled && !beep->dev)
-		snd_hda_do_attach(beep);
-	mutex_unlock(&beep->mutex);
-}
-
-static void snd_hda_do_unregister(struct work_struct *work)
-{
-	struct hda_beep *beep =
-		container_of(work, struct hda_beep, unregister_work.work);
-
-	mutex_lock(&beep->mutex);
-	if (!beep->enabled && beep->dev)
-		snd_hda_do_detach(beep);
-	mutex_unlock(&beep->mutex);
-}
-
 int snd_hda_enable_beep_device(struct hda_codec *codec, int enable)
 {
 	struct hda_beep *beep = codec->beep;
@@ -197,15 +175,6 @@ int snd_hda_enable_beep_device(struct hda_codec *codec, int enable)
 			snd_hda_codec_write(beep->codec, beep->nid, 0,
 						  AC_VERB_SET_BEEP_CONTROL, 0);
 		}
-		if (beep->mode == HDA_BEEP_MODE_SWREG) {
-			if (enable) {
-				cancel_delayed_work(&beep->unregister_work);
-				schedule_work(&beep->register_work);
-			} else {
-				schedule_delayed_work(&beep->unregister_work,
-									   HZ);
-			}
-		}
 		return 1;
 	}
 	return 0;
@@ -235,12 +204,10 @@ int snd_hda_attach_beep_device(struct hda_codec *codec, int nid)
 	beep->mode = codec->beep_mode;
 	codec->beep = beep;
 
-	INIT_WORK(&beep->register_work, &snd_hda_do_register);
-	INIT_DELAYED_WORK(&beep->unregister_work, &snd_hda_do_unregister);
 	INIT_WORK(&beep->beep_work, &snd_hda_generate_beep);
 	mutex_init(&beep->mutex);
 
-	if (beep->mode == HDA_BEEP_MODE_ON) {
+	if (beep->mode) {
 		int err = snd_hda_do_attach(beep);
 		if (err < 0) {
 			kfree(beep);
@@ -257,8 +224,6 @@ void snd_hda_detach_beep_device(struct hda_codec *codec)
 {
 	struct hda_beep *beep = codec->beep;
 	if (beep) {
-		cancel_work_sync(&beep->register_work);
-		cancel_delayed_work(&beep->unregister_work);
 		if (beep->dev)
 			snd_hda_do_detach(beep);
 		codec->beep = NULL;
diff --git a/sound/pci/hda/hda_beep.h b/sound/pci/hda/hda_beep.h
index 55f0647..30e79d1 100644
--- a/sound/pci/hda/hda_beep.h
+++ b/sound/pci/hda/hda_beep.h
@@ -26,7 +26,6 @@
 
 #define HDA_BEEP_MODE_OFF	0
 #define HDA_BEEP_MODE_ON	1
-#define HDA_BEEP_MODE_SWREG	2
 
 /* beep information */
 struct hda_beep {
@@ -37,10 +36,7 @@ struct hda_beep {
 	int tone;
 	hda_nid_t nid;
 	unsigned int enabled:1;
-	unsigned int request_enable:1;
 	unsigned int linear_tone:1;	/* linear tone for IDT/STAC codec */
-	struct work_struct register_work; /* registration work */
-	struct delayed_work unregister_work; /* unregistration work */
 	struct work_struct beep_work; /* scheduled task for beep event */
 	struct mutex mutex;
 };
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 7757536..334c0ba 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -72,7 +72,7 @@ static int enable_msi = -1;
 static char *patch[SNDRV_CARDS];
 #endif
 #ifdef CONFIG_SND_HDA_INPUT_BEEP
-static int beep_mode[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)] =
+static bool beep_mode[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)] =
 					CONFIG_SND_HDA_INPUT_BEEP_MODE};
 #endif
 
@@ -103,9 +103,9 @@ module_param_array(patch, charp, NULL, 0444);
 MODULE_PARM_DESC(patch, "Patch file for Intel HD audio interface.");
 #endif
 #ifdef CONFIG_SND_HDA_INPUT_BEEP
-module_param_array(beep_mode, int, NULL, 0444);
+module_param_array(beep_mode, bool, NULL, 0444);
 MODULE_PARM_DESC(beep_mode, "Select HDA Beep registration mode "
-			    "(0=off, 1=on, 2=mute switch on/off) (default=1).");
+			    "(0=off, 1=on) (default=1).");
 #endif
 
 #ifdef CONFIG_SND_HDA_POWER_SAVE
-- 
1.7.10.4

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

* [PATCH 2/4] ALSA: hda - Move beep helper functions to hda_beep.c
  2012-07-03 16:03 [PATCH 0/4] ALSA: hda beep clean up Takashi Iwai
  2012-07-03 16:03 ` [PATCH 1/4] ALSA: hda - Remove beep_mode=2 Takashi Iwai
@ 2012-07-03 16:03 ` Takashi Iwai
  2012-07-03 16:03 ` [PATCH 3/4] ALSA: hda - Get rid of superfluous beep->mode field Takashi Iwai
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2012-07-03 16:03 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai

Move snd_hda_mixer_amp_switch_put_beep() to hda_beep.c as a clean up
to remove one more ifdef.

Also add the corresponding get callback to return consistently the
digital beep state independently from the mixer amp value.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/hda_beep.c  |   28 ++++++++++++++++++++++++++++
 sound/pci/hda/hda_codec.c |   19 -------------------
 sound/pci/hda/hda_local.h |    4 +++-
 3 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/sound/pci/hda/hda_beep.c b/sound/pci/hda/hda_beep.c
index 662de6e..336b4b3 100644
--- a/sound/pci/hda/hda_beep.c
+++ b/sound/pci/hda/hda_beep.c
@@ -231,3 +231,31 @@ void snd_hda_detach_beep_device(struct hda_codec *codec)
 	}
 }
 EXPORT_SYMBOL_HDA(snd_hda_detach_beep_device);
+
+/* 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) {
+		ucontrol->value.integer.value[0] =
+			ucontrol->value.integer.value[1] =
+			beep->enabled;
+		return 0;
+	}
+	return snd_hda_mixer_amp_switch_get(kcontrol, ucontrol);
+}
+EXPORT_SYMBOL_HDA(snd_hda_mixer_amp_switch_get_beep);
+
+int snd_hda_mixer_amp_switch_put_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)
+		snd_hda_enable_beep_device(codec,
+					   *ucontrol->value.integer.value);
+	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_codec.c b/sound/pci/hda/hda_codec.c
index 51cb2a2..ddac428 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -2676,25 +2676,6 @@ int snd_hda_mixer_amp_switch_put(struct snd_kcontrol *kcontrol,
 }
 EXPORT_SYMBOL_HDA(snd_hda_mixer_amp_switch_put);
 
-#ifdef CONFIG_SND_HDA_INPUT_BEEP
-/**
- * snd_hda_mixer_amp_switch_put_beep - Put callback for a beep AMP switch
- *
- * This function calls snd_hda_enable_beep_device(), which behaves differently
- * depending on beep_mode option.
- */
-int snd_hda_mixer_amp_switch_put_beep(struct snd_kcontrol *kcontrol,
-				      struct snd_ctl_elem_value *ucontrol)
-{
-	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
-	long *valp = ucontrol->value.integer.value;
-
-	snd_hda_enable_beep_device(codec, *valp);
-	return snd_hda_mixer_amp_switch_put(kcontrol, ucontrol);
-}
-EXPORT_SYMBOL_HDA(snd_hda_mixer_amp_switch_put_beep);
-#endif /* CONFIG_SND_HDA_INPUT_BEEP */
-
 /*
  * bound volume controls
  *
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
index 9a096a8..1b4c129 100644
--- a/sound/pci/hda/hda_local.h
+++ b/sound/pci/hda/hda_local.h
@@ -89,7 +89,7 @@
 	{ .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, .index = xcidx, \
 	  .subdevice = HDA_SUBDEV_AMP_FLAG, \
 	  .info = snd_hda_mixer_amp_switch_info, \
-	  .get = snd_hda_mixer_amp_switch_get, \
+	  .get = snd_hda_mixer_amp_switch_get_beep, \
 	  .put = snd_hda_mixer_amp_switch_put_beep, \
 	  .private_value = HDA_COMPOSE_AMP_VAL(nid, channel, xindex, direction) }
 #else
@@ -121,6 +121,8 @@ int snd_hda_mixer_amp_switch_get(struct snd_kcontrol *kcontrol,
 int snd_hda_mixer_amp_switch_put(struct snd_kcontrol *kcontrol,
 				 struct snd_ctl_elem_value *ucontrol);
 #ifdef CONFIG_SND_HDA_INPUT_BEEP
+int snd_hda_mixer_amp_switch_get_beep(struct snd_kcontrol *kcontrol,
+				      struct snd_ctl_elem_value *ucontrol);
 int snd_hda_mixer_amp_switch_put_beep(struct snd_kcontrol *kcontrol,
 				      struct snd_ctl_elem_value *ucontrol);
 #endif
-- 
1.7.10.4

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

* [PATCH 3/4] ALSA: hda - Get rid of superfluous beep->mode field
  2012-07-03 16:03 [PATCH 0/4] ALSA: hda beep clean up Takashi Iwai
  2012-07-03 16:03 ` [PATCH 1/4] ALSA: hda - Remove beep_mode=2 Takashi Iwai
  2012-07-03 16:03 ` [PATCH 2/4] ALSA: hda - Move beep helper functions to hda_beep.c Takashi Iwai
@ 2012-07-03 16:03 ` Takashi Iwai
  2012-07-03 16:03 ` [PATCH 4/4] ALSA: hda - Avoid possible race of beep on/off Takashi Iwai
  2012-07-03 16:31 ` [PATCH 0/4] ALSA: hda beep clean up Jaroslav Kysela
  4 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2012-07-03 16:03 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai

It's no longer necessary since beep_mode=2 option was dropped.
It can be checked simply via codec->beep != NULL.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/hda_beep.c |   14 ++++++--------
 sound/pci/hda/hda_beep.h |    1 -
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/sound/pci/hda/hda_beep.c b/sound/pci/hda/hda_beep.c
index 336b4b3..e6cf2a2 100644
--- a/sound/pci/hda/hda_beep.c
+++ b/sound/pci/hda/hda_beep.c
@@ -184,6 +184,7 @@ EXPORT_SYMBOL_HDA(snd_hda_enable_beep_device);
 int snd_hda_attach_beep_device(struct hda_codec *codec, int nid)
 {
 	struct hda_beep *beep;
+	int err;
 
 	if (!snd_hda_get_bool_hint(codec, "beep"))
 		return 0; /* disabled explicitly by hints */
@@ -201,19 +202,16 @@ int snd_hda_attach_beep_device(struct hda_codec *codec, int nid)
 
 	beep->nid = nid;
 	beep->codec = codec;
-	beep->mode = codec->beep_mode;
 	codec->beep = beep;
 
 	INIT_WORK(&beep->beep_work, &snd_hda_generate_beep);
 	mutex_init(&beep->mutex);
 
-	if (beep->mode) {
-		int err = snd_hda_do_attach(beep);
-		if (err < 0) {
-			kfree(beep);
-			codec->beep = NULL;
-			return err;
-		}
+	err = snd_hda_do_attach(beep);
+	if (err < 0) {
+		kfree(beep);
+		codec->beep = NULL;
+		return err;
 	}
 
 	return 0;
diff --git a/sound/pci/hda/hda_beep.h b/sound/pci/hda/hda_beep.h
index 30e79d1..4dc6933 100644
--- a/sound/pci/hda/hda_beep.h
+++ b/sound/pci/hda/hda_beep.h
@@ -31,7 +31,6 @@
 struct hda_beep {
 	struct input_dev *dev;
 	struct hda_codec *codec;
-	unsigned int mode;
 	char phys[32];
 	int tone;
 	hda_nid_t nid;
-- 
1.7.10.4

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

* [PATCH 4/4] ALSA: hda - Avoid possible race of beep on/off
  2012-07-03 16:03 [PATCH 0/4] ALSA: hda beep clean up Takashi Iwai
                   ` (2 preceding siblings ...)
  2012-07-03 16:03 ` [PATCH 3/4] ALSA: hda - Get rid of superfluous beep->mode field Takashi Iwai
@ 2012-07-03 16:03 ` Takashi Iwai
  2012-07-03 16:31 ` [PATCH 0/4] ALSA: hda beep clean up Jaroslav Kysela
  4 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2012-07-03 16:03 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai

Call cancel_work_sync() when turning off the beep switch so that the
mute call is executed in a proper order.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/hda_beep.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/pci/hda/hda_beep.c b/sound/pci/hda/hda_beep.c
index e6cf2a2..0bc2315 100644
--- a/sound/pci/hda/hda_beep.c
+++ b/sound/pci/hda/hda_beep.c
@@ -165,12 +165,13 @@ static int snd_hda_do_attach(struct hda_beep *beep)
 int snd_hda_enable_beep_device(struct hda_codec *codec, int enable)
 {
 	struct hda_beep *beep = codec->beep;
-	enable = !!enable;
-	if (beep == NULL)
+	if (!beep)
 		return 0;
+	enable = !!enable;
 	if (beep->enabled != enable) {
 		beep->enabled = enable;
 		if (!enable) {
+			cancel_work_sync(&beep->beep_work);
 			/* turn off beep */
 			snd_hda_codec_write(beep->codec, beep->nid, 0,
 						  AC_VERB_SET_BEEP_CONTROL, 0);
-- 
1.7.10.4

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

* Re: [PATCH 0/4] ALSA: hda beep clean up
  2012-07-03 16:03 [PATCH 0/4] ALSA: hda beep clean up Takashi Iwai
                   ` (3 preceding siblings ...)
  2012-07-03 16:03 ` [PATCH 4/4] ALSA: hda - Avoid possible race of beep on/off Takashi Iwai
@ 2012-07-03 16:31 ` Jaroslav Kysela
  2012-07-03 20:01   ` Jaroslav Kysela
  4 siblings, 1 reply; 10+ messages in thread
From: Jaroslav Kysela @ 2012-07-03 16:31 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

Date 3.7.2012 18:03, Takashi Iwai wrote:
> Hi,
> 
> this is a series of small cleanups of HD-audio beep code.
> Since the keyboard driver already handles the simultaneous outputs
> from the multiple input beep instances, there is no merit to have
> beep_mode=2 option to switch the beep input device.

Acked-by: Jaroslav Kysela <perex@perex.cz>

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project; Red Hat, Inc.

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

* Re: [PATCH 0/4] ALSA: hda beep clean up
  2012-07-03 16:31 ` [PATCH 0/4] ALSA: hda beep clean up Jaroslav Kysela
@ 2012-07-03 20:01   ` Jaroslav Kysela
  2012-07-04  5:16     ` Takashi Iwai
  0 siblings, 1 reply; 10+ messages in thread
From: Jaroslav Kysela @ 2012-07-03 20:01 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai

Date 3.7.2012 18:31, Jaroslav Kysela wrote:
> Date 3.7.2012 18:03, Takashi Iwai wrote:
>> Hi,
>>
>> this is a series of small cleanups of HD-audio beep code.
>> Since the keyboard driver already handles the simultaneous outputs
>> from the multiple input beep instances, there is no merit to have
>> beep_mode=2 option to switch the beep input device.
> 
> Acked-by: Jaroslav Kysela <perex@perex.cz>

While I acked this, I vaguely remember now, that the event duplication
(simultaneous outputs) caused some issues on some Lenovo notebooks.

When the pcspkr and HDA beep generator is connected to the same output
(integrated speakers), it may cause some issues (bad beep quality -
frequency etc.).

I looked to the input code, and the kd_mksound() calls
input_handler_for_each_handle(), thus the event is duplicated to all
beep generators, right?

The beep_mode=2 was introduced to let users to help with this issue with
a comfortable way.

						Jaroslav


-- 
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project; Red Hat, Inc.

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

* Re: [PATCH 0/4] ALSA: hda beep clean up
  2012-07-03 20:01   ` Jaroslav Kysela
@ 2012-07-04  5:16     ` Takashi Iwai
  2012-07-04  7:43       ` Jaroslav Kysela
  0 siblings, 1 reply; 10+ messages in thread
From: Takashi Iwai @ 2012-07-04  5:16 UTC (permalink / raw)
  To: Jaroslav Kysela; +Cc: alsa-devel

At Tue, 03 Jul 2012 22:01:45 +0200,
Jaroslav Kysela wrote:
> 
> Date 3.7.2012 18:31, Jaroslav Kysela wrote:
> > Date 3.7.2012 18:03, Takashi Iwai wrote:
> >> Hi,
> >>
> >> this is a series of small cleanups of HD-audio beep code.
> >> Since the keyboard driver already handles the simultaneous outputs
> >> from the multiple input beep instances, there is no merit to have
> >> beep_mode=2 option to switch the beep input device.
> > 
> > Acked-by: Jaroslav Kysela <perex@perex.cz>
> 
> While I acked this, I vaguely remember now, that the event duplication
> (simultaneous outputs) caused some issues on some Lenovo notebooks.
> 
> When the pcspkr and HDA beep generator is connected to the same output
> (integrated speakers), it may cause some issues (bad beep quality -
> frequency etc.).
> 
> I looked to the input code, and the kd_mksound() calls
> input_handler_for_each_handle(), thus the event is duplicated to all
> beep generators, right?

Right.

> The beep_mode=2 was introduced to let users to help with this issue with
> a comfortable way.

Not really.  At the time this was introduced, the keyboard driver took
only a single input handle exclusively, namely only the last
registered one.  Thus playing on both had never happened until
recently.


Takashi

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

* Re: [PATCH 0/4] ALSA: hda beep clean up
  2012-07-04  5:16     ` Takashi Iwai
@ 2012-07-04  7:43       ` Jaroslav Kysela
  2012-07-04  7:46         ` Takashi Iwai
  0 siblings, 1 reply; 10+ messages in thread
From: Jaroslav Kysela @ 2012-07-04  7:43 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

Date 4.7.2012 07:16, Takashi Iwai wrote:
> At Tue, 03 Jul 2012 22:01:45 +0200,
> Jaroslav Kysela wrote:
>>
>> Date 3.7.2012 18:31, Jaroslav Kysela wrote:
>>> Date 3.7.2012 18:03, Takashi Iwai wrote:
>>>> Hi,
>>>>
>>>> this is a series of small cleanups of HD-audio beep code.
>>>> Since the keyboard driver already handles the simultaneous outputs
>>>> from the multiple input beep instances, there is no merit to have
>>>> beep_mode=2 option to switch the beep input device.
>>>
>>> Acked-by: Jaroslav Kysela <perex@perex.cz>
>>
>> While I acked this, I vaguely remember now, that the event duplication
>> (simultaneous outputs) caused some issues on some Lenovo notebooks.
>>
>> When the pcspkr and HDA beep generator is connected to the same output
>> (integrated speakers), it may cause some issues (bad beep quality -
>> frequency etc.).
>>
>> I looked to the input code, and the kd_mksound() calls
>> input_handler_for_each_handle(), thus the event is duplicated to all
>> beep generators, right?
> 
> Right.
> 
>> The beep_mode=2 was introduced to let users to help with this issue with
>> a comfortable way.
> 
> Not really.  At the time this was introduced, the keyboard driver took
> only a single input handle exclusively, namely only the last
> registered one.  Thus playing on both had never happened until
> recently.

OK, thanks for the clarification. The question is, if it's the desired
behaviour. Multiple outs can cause some harmonic effects. It would be
nice, if the input layer can turn on/off (filter) the beep events at
runtime separately for all registered drivers. The setup can be handled
using sysfs or so..

					Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project; Red Hat, Inc.

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

* Re: [PATCH 0/4] ALSA: hda beep clean up
  2012-07-04  7:43       ` Jaroslav Kysela
@ 2012-07-04  7:46         ` Takashi Iwai
  0 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2012-07-04  7:46 UTC (permalink / raw)
  To: Jaroslav Kysela; +Cc: alsa-devel

At Wed, 04 Jul 2012 09:43:23 +0200,
Jaroslav Kysela wrote:
> 
> Date 4.7.2012 07:16, Takashi Iwai wrote:
> > At Tue, 03 Jul 2012 22:01:45 +0200,
> > Jaroslav Kysela wrote:
> >>
> >> Date 3.7.2012 18:31, Jaroslav Kysela wrote:
> >>> Date 3.7.2012 18:03, Takashi Iwai wrote:
> >>>> Hi,
> >>>>
> >>>> this is a series of small cleanups of HD-audio beep code.
> >>>> Since the keyboard driver already handles the simultaneous outputs
> >>>> from the multiple input beep instances, there is no merit to have
> >>>> beep_mode=2 option to switch the beep input device.
> >>>
> >>> Acked-by: Jaroslav Kysela <perex@perex.cz>
> >>
> >> While I acked this, I vaguely remember now, that the event duplication
> >> (simultaneous outputs) caused some issues on some Lenovo notebooks.
> >>
> >> When the pcspkr and HDA beep generator is connected to the same output
> >> (integrated speakers), it may cause some issues (bad beep quality -
> >> frequency etc.).
> >>
> >> I looked to the input code, and the kd_mksound() calls
> >> input_handler_for_each_handle(), thus the event is duplicated to all
> >> beep generators, right?
> > 
> > Right.
> > 
> >> The beep_mode=2 was introduced to let users to help with this issue with
> >> a comfortable way.
> > 
> > Not really.  At the time this was introduced, the keyboard driver took
> > only a single input handle exclusively, namely only the last
> > registered one.  Thus playing on both had never happened until
> > recently.
> 
> OK, thanks for the clarification. The question is, if it's the desired
> behaviour. Multiple outs can cause some harmonic effects. It would be
> nice, if the input layer can turn on/off (filter) the beep events at
> runtime separately for all registered drivers. The setup can be handled
> using sysfs or so..

Yeah, it's an issue in the input layer, or maybe specific to keyboard
driver.  It should be relatively easy to implement.

Meanwhile, you can simply mute "Beep" volume.  That is effectively as
same as deregistering the HD-audio input handle.  The beep code in
HD-audio driver doesn't emit the verb any longer when it's muted.

Getting rid of the system beep is another question.  But it could be
done via setterm or whatever, too.



Takashi

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

end of thread, other threads:[~2012-07-04  7:46 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-03 16:03 [PATCH 0/4] ALSA: hda beep clean up Takashi Iwai
2012-07-03 16:03 ` [PATCH 1/4] ALSA: hda - Remove beep_mode=2 Takashi Iwai
2012-07-03 16:03 ` [PATCH 2/4] ALSA: hda - Move beep helper functions to hda_beep.c Takashi Iwai
2012-07-03 16:03 ` [PATCH 3/4] ALSA: hda - Get rid of superfluous beep->mode field Takashi Iwai
2012-07-03 16:03 ` [PATCH 4/4] ALSA: hda - Avoid possible race of beep on/off Takashi Iwai
2012-07-03 16:31 ` [PATCH 0/4] ALSA: hda beep clean up Jaroslav Kysela
2012-07-03 20:01   ` Jaroslav Kysela
2012-07-04  5:16     ` Takashi Iwai
2012-07-04  7:43       ` Jaroslav Kysela
2012-07-04  7:46         ` 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.