Re: [alsa-devel] [PATCH] ALSA: hda - Add digital BEEP generator support for Realtek codecs.
Sorry for the late reply.
On Mon, Nov 17, 2008 at 17:53:34PM +0100, Takashi Iwai wrote:
Thanks for the patch. Do these beep widgets have the mute amp bits? If not, we'd likely need to add mute switches as well (otherwise it gets too annoying) controlling on software. Check patch_sigmatel.c in the latest master branch or topic/fix/hda branch of sound git tree: git://git.kernel.org/pub/scm/linux/kernel/tiwai/sound-2.6.git
Every codec can mute beep.
Another concern is that I don't want to add the digital beep control unconditionally. On many devices, the beep is implemented in the old good analog way. We'd need to add the check of analog/digital beep in a new spec field and in the preset table not to duplicate beep controls on these.
I don't think such a check is needed. I presume that analog beep in is disabled while digital beep generetor is being used. Furthermore, keyboard bell rings only one device even if both HDA Digital PCBeep and PC Speaker have been registered.
At Tue, 3 Feb 2009 23:28:51 +0900, Kusanagi Kouichi wrote:
Sorry for the late reply.
On Mon, Nov 17, 2008 at 17:53:34PM +0100, Takashi Iwai wrote:
Thanks for the patch. Do these beep widgets have the mute amp bits? If not, we'd likely need to add mute switches as well (otherwise it gets too annoying) controlling on software. Check patch_sigmatel.c in the latest master branch or topic/fix/hda branch of sound git tree: git://git.kernel.org/pub/scm/linux/kernel/tiwai/sound-2.6.git
Every codec can mute beep.
Then it's fine.
Another concern is that I don't want to add the digital beep control unconditionally. On many devices, the beep is implemented in the old good analog way. We'd need to add the check of analog/digital beep in a new spec field and in the preset table not to duplicate beep controls on these.
I don't think such a check is needed. I presume that analog beep in is disabled while digital beep generetor is being used. Furthermore, keyboard bell rings only one device even if both HDA Digital PCBeep and PC Speaker have been registered.
However it's just annoying to have multiple controls for the very same purpose. If the digital beep works more reliably overall, let's get rid of all existing analog beep stuff. Having both makes no sense.
(Well, I don't remember exactly your patch so I suppose your patch doesn't remove the analog beep stuff... Anyway reposting the patch and checking it's still applicable to the latest driver code would be helpful :)
thanks,
Takashi
On Tue, Feb 03, 2009 at 03:36:20PM +0100, Takashi Iwai wrote:
Another concern is that I don't want to add the digital beep control unconditionally. On many devices, the beep is implemented in the old good analog way. We'd need to add the check of analog/digital beep in a new spec field and in the preset table not to duplicate beep controls on these.
I don't think such a check is needed. I presume that analog beep in is disabled while digital beep generetor is being used. Furthermore, keyboard bell rings only one device even if both HDA Digital PCBeep and PC Speaker have been registered.
However it's just annoying to have multiple controls for the very same purpose. If the digital beep works more reliably overall, let's get rid of all existing analog beep stuff. Having both makes no sense.
(Well, I don't remember exactly your patch so I suppose your patch doesn't remove the analog beep stuff... Anyway reposting the patch and checking it's still applicable to the latest driver code would be helpful :)
Ah! I see the point. There is no duplicated control because I didn't add any new controls. Existing beep control controls digital beep, too. Analog beep and digital beep look to be routed through the same path.
Updated patch is attached.
At Wed, 4 Feb 2009 17:37:51 +0900, Kusanagi Kouichi wrote:
On Tue, Feb 03, 2009 at 03:36:20PM +0100, Takashi Iwai wrote:
Another concern is that I don't want to add the digital beep control unconditionally. On many devices, the beep is implemented in the old good analog way. We'd need to add the check of analog/digital beep in a new spec field and in the preset table not to duplicate beep controls on these.
I don't think such a check is needed. I presume that analog beep in is disabled while digital beep generetor is being used. Furthermore, keyboard bell rings only one device even if both HDA Digital PCBeep and PC Speaker have been registered.
However it's just annoying to have multiple controls for the very same purpose. If the digital beep works more reliably overall, let's get rid of all existing analog beep stuff. Having both makes no sense.
(Well, I don't remember exactly your patch so I suppose your patch doesn't remove the analog beep stuff... Anyway reposting the patch and checking it's still applicable to the latest driver code would be helpful :)
Ah! I see the point. There is no duplicated control because I didn't add any new controls. Existing beep control controls digital beep, too. Analog beep and digital beep look to be routed through the same path.
Fair enough.
Though, there is still one problem -- not all models enabled the analog beep path. So, the next step is to create beep controls in all possible situations.
But I'm going to apply your patch as now. Could you repost the patch with a proper summary, a changelog text and your sign-off so that I can apply it as is?
thanks,
Takashi
On Wed, Feb 04, 2009 at 12:17:14PM +0100, Takashi Iwai wrote:
Fair enough.
Though, there is still one problem -- not all models enabled the analog beep path. So, the next step is to create beep controls in all possible situations.
I'll try later.
But I'm going to apply your patch as now. Could you repost the patch with a proper summary, a changelog text and your sign-off so that I can apply it as is?
Sure. I'll send the patch. Thank you.
At Wed, 4 Feb 2009 23:20:12 +0900, Kusanagi Kouichi wrote:
On Wed, Feb 04, 2009 at 12:17:14PM +0100, Takashi Iwai wrote:
Fair enough.
Though, there is still one problem -- not all models enabled the analog beep path. So, the next step is to create beep controls in all possible situations.
I'll try later.
This can be a separate (additional) patch. Let's get the first patch into the main tree so that one can work on further issues.
thanks,
Takashi
At Wed, 04 Feb 2009 15:44:12 +0100, I wrote:
At Wed, 4 Feb 2009 23:20:12 +0900, Kusanagi Kouichi wrote:
On Wed, Feb 04, 2009 at 12:17:14PM +0100, Takashi Iwai wrote:
Fair enough.
Though, there is still one problem -- not all models enabled the analog beep path. So, the next step is to create beep controls in all possible situations.
I'll try later.
This can be a separate (additional) patch. Let's get the first patch into the main tree so that one can work on further issues.
... and now fixed on sound git tree.
Takashi
On Fri, Feb 06, 2009 at 05:25:13PM +0100, Takashi Iwai wrote:
At Wed, 04 Feb 2009 15:44:12 +0100, I wrote:
At Wed, 4 Feb 2009 23:20:12 +0900, Kusanagi Kouichi wrote:
On Wed, Feb 04, 2009 at 12:17:14PM +0100, Takashi Iwai wrote:
Fair enough.
Though, there is still one problem -- not all models enabled the analog beep path. So, the next step is to create beep controls in all possible situations.
I'll try later.
This can be a separate (additional) patch. Let's get the first patch into the main tree so that one can work on further issues.
... and now fixed on sound git tree.
Some models for alc268 still seems to lack a beep control. Is it OK to add a beep control unconditionally?
At Sat, 7 Feb 2009 19:40:52 +0900, Kusanagi Kouichi wrote:
On Fri, Feb 06, 2009 at 05:25:13PM +0100, Takashi Iwai wrote:
At Wed, 04 Feb 2009 15:44:12 +0100, I wrote:
At Wed, 4 Feb 2009 23:20:12 +0900, Kusanagi Kouichi wrote:
On Wed, Feb 04, 2009 at 12:17:14PM +0100, Takashi Iwai wrote:
Fair enough.
Though, there is still one problem -- not all models enabled the analog beep path. So, the next step is to create beep controls in all possible situations.
I'll try later.
This can be a separate (additional) patch. Let's get the first patch into the main tree so that one can work on further issues.
... and now fixed on sound git tree.
Some models for alc268 still seems to lack a beep control. Is it OK to add a beep control unconditionally?
The question is whether it works. In your previous patch, patch_alc268() is untouched and has no digital beep hook yet.
I'll check a machine with ALC268 later.
thanks,
Takashi
[2 0001-hda-Add-mising-beep-controls-for-realtek-alc268-cod.patch <text/plain; us-ascii (7bit)>]
From 184d1a1b1c3976a8a76b2f3b40ad02992b44db48 Mon Sep 17 00:00:00 2001
From: Kusanagi Kouichi slash@ma.neweb.ne.jp Date: Sat, 7 Feb 2009 19:30:05 +0900 Subject: [PATCH] hda: Add mising beep controls for realtek alc268 codec.
So far, some models didn't add a beep control since they don't use analog beep in.
Signed-off-by: Kusanagi Kouichi slash@ma.neweb.ne.jp
sound/pci/hda/patch_realtek.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index f594a09..2fa26f9 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -11887,7 +11887,7 @@ static struct snd_pci_quirk alc268_cfg_tbl[] = {
static struct alc_config_preset alc268_presets[] = { [ALC267_QUANTA_IL1] = {
.mixers = { alc267_quanta_il1_mixer },
.mixers = { alc267_quanta_il1_mixer, alc268_beep_mixer },@@ -11969,7 +11969,7 @@ static struct alc_config_preset alc268_presets[] = { }, [ALC268_ACER_ASPIRE_ONE] = { .mixers = { alc268_acer_aspire_one_mixer,
alc268_capture_alt_mixer },
alc268_capture_alt_mixer, alc268_beep_mixer },-- 1.5.6.5
[3 <text/plain; us-ascii (7bit)>] _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
At Sat, 07 Feb 2009 12:54:25 +0100, I wrote:
At Sat, 7 Feb 2009 19:40:52 +0900, Kusanagi Kouichi wrote:
On Fri, Feb 06, 2009 at 05:25:13PM +0100, Takashi Iwai wrote:
At Wed, 04 Feb 2009 15:44:12 +0100, I wrote:
At Wed, 4 Feb 2009 23:20:12 +0900, Kusanagi Kouichi wrote:
On Wed, Feb 04, 2009 at 12:17:14PM +0100, Takashi Iwai wrote:
Fair enough.
Though, there is still one problem -- not all models enabled the analog beep path. So, the next step is to create beep controls in all possible situations.
I'll try later.
This can be a separate (additional) patch. Let's get the first patch into the main tree so that one can work on further issues.
... and now fixed on sound git tree.
Some models for alc268 still seems to lack a beep control. Is it OK to add a beep control unconditionally?
The question is whether it works. In your previous patch, patch_alc268() is untouched and has no digital beep hook yet.
I'll check a machine with ALC268 later.
As it seems working, I fixed the issue on sound git tree now.
Takashi
participants (2)
-
Kusanagi Kouichi -
Takashi Iwai