| Message ID | 7461f695b4daed80f2fc4b1463ead47f04f9ad05.1739741254.git.mail@maciej.szmigiero.name (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] ALSA: hda/realtek: Enable PC beep passthrough for HP EliteBook 855 G7 | expand |
On Sun, 16 Feb 2025 22:31:03 +0100, Maciej S. Szmigiero wrote: > > PC speaker works well on this platform in BIOS and in Linux until sound > card drivers are loaded. Then it stops working. > > There seems to be a beep generator node at 0x1a in this CODEC > (ALC269_TYPE_ALC215) but it seems to be only connected to capture mixers > at nodes 0x22 and 0x23. > If I unmute the mixer input for 0x1a at node 0x23 and start recording > from its "ALC285 Analog" capture device I can clearly hear beeps in that > recording. > > So the beep generator is indeed working properly, however I wasn't able to > figure out any way to connect it to speakers. > > However, the bits in the "Passthrough Control" register (0x36) seems to > work at least partially: by zeroing "B" and "h" and setting "S" I can at > least make the PIT PC speaker output appear either in this laptop speakers > or headphones (depending on whether they are connected or not). > > > There are some caveats, however: > * If the CODEC gets runtime-suspended the beeps stop so it needs HDA beep > device for keeping it awake during beeping. > > * If the beep generator node is generating any beep the PC beep passthrough > seems to be temporarily inhibited, so the HDA beep device has to be > prevented from using the actual beep generator node - but the beep device > is still necessary due to the previous point. > > * In contrast with other platforms here beep amplification has to be > disabled otherwise the beeps output are WAY louder than they were on pure > BIOS setup. > > > Unless someone (from Realtek probably) knows how to make the beep generator > node output appear in speakers / headphones using PC beep passthrough seems > to be the only way to make PC speaker beeping actually work on this > platform. > > Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name> > --- > > Since more than 6 weeks has now passed since the previous version of this > patch was posted, yet no better or other solution was provided I'm > re-submitting an updated version of the original patch. > > This solution has been working fine for me on this platform all that time, > without any obvious issues. > > Changes from v1: > * Correct the typo in !IS_ENABLED(CONFIG_INPUT_PCSPKR) code that the > kernel test robot found. > > * Change codec_warn() into dev_warn_once(hda_codec_dev(codec)) > to avoid spamming the kernel log at runtime PM CODEC re-init. This is really a thing to be checked by Realtek people at first, as it's very vendor-specific thing. Kailang, please check this. And, except for that, I'm still concerned by the behavior change. Also the caveat you mentioned about the runtime PM raises some doubt, too. thanks, Takashi > > include/sound/hda_codec.h | 1 + > sound/pci/hda/hda_beep.c | 15 +++++++++------ > sound/pci/hda/patch_realtek.c | 34 +++++++++++++++++++++++++++++++++- > 3 files changed, 43 insertions(+), 7 deletions(-) > > diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h > index 575e55aa08ca..c1fe6290d04d 100644 > --- a/include/sound/hda_codec.h > +++ b/include/sound/hda_codec.h > @@ -195,6 +195,7 @@ struct hda_codec { > /* beep device */ > struct hda_beep *beep; > unsigned int beep_mode; > + bool beep_just_power_on; > > /* widget capabilities cache */ > u32 *wcaps; > diff --git a/sound/pci/hda/hda_beep.c b/sound/pci/hda/hda_beep.c > index e51d47572557..13a7d92e8d8d 100644 > --- a/sound/pci/hda/hda_beep.c > +++ b/sound/pci/hda/hda_beep.c > @@ -31,8 +31,9 @@ static void generate_tone(struct hda_beep *beep, int tone) > beep->power_hook(beep, true); > beep->playing = 1; > } > - snd_hda_codec_write(codec, beep->nid, 0, > - AC_VERB_SET_BEEP_CONTROL, tone); > + if (!codec->beep_just_power_on) > + snd_hda_codec_write(codec, beep->nid, 0, > + AC_VERB_SET_BEEP_CONTROL, tone); > if (!tone && beep->playing) { > beep->playing = 0; > if (beep->power_hook) > @@ -212,10 +213,12 @@ 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 */ > - if (codec->beep_mode == HDA_BEEP_MODE_OFF) > - return 0; /* disabled by module option */ > + if (!codec->beep_just_power_on) { > + if (!snd_hda_get_bool_hint(codec, "beep")) > + return 0; /* disabled explicitly by hints */ > + if (codec->beep_mode == HDA_BEEP_MODE_OFF) > + return 0; /* disabled by module option */ > + } > > beep = kzalloc(sizeof(*beep), GFP_KERNEL); > if (beep == NULL) > diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c > index 224616fbec4f..fff6e7c1c265 100644 > --- a/sound/pci/hda/patch_realtek.c > +++ b/sound/pci/hda/patch_realtek.c > @@ -28,6 +28,7 @@ > #include <sound/hda_codec.h> > #include "hda_local.h" > #include "hda_auto_parser.h" > +#include "hda_beep.h" > #include "hda_jack.h" > #include "hda_generic.h" > #include "hda_component.h" > @@ -6923,6 +6924,30 @@ static void alc285_fixup_hp_envy_x360(struct hda_codec *codec, > } > } > > +static void alc285_fixup_hp_beep(struct hda_codec *codec, > + const struct hda_fixup *fix, int action) > +{ > + if (action == HDA_FIXUP_ACT_PRE_PROBE) { > + codec->beep_just_power_on = true; > + } else if (action == HDA_FIXUP_ACT_INIT) { > +#ifdef CONFIG_SND_HDA_INPUT_BEEP > + /* > + * Just enable loopback to internal speaker and headphone jack. > + * Disable amplification to get about the same beep volume as > + * was on pure BIOS setup before loading the driver. > + */ > + alc_update_coef_idx(codec, 0x36, 0x7070, BIT(13)); > + > + snd_hda_enable_beep_device(codec, 1); > + > +#if !IS_ENABLED(CONFIG_INPUT_PCSPKR) > + dev_warn_once(hda_codec_dev(codec), > + "enable CONFIG_INPUT_PCSPKR to get PC beeps\n"); > +#endif > +#endif > + } > +} > + > /* for hda_fixup_thinkpad_acpi() */ > #include "thinkpad_helper.c" > > @@ -7707,6 +7732,7 @@ enum { > ALC285_FIXUP_HP_GPIO_LED, > ALC285_FIXUP_HP_MUTE_LED, > ALC285_FIXUP_HP_SPECTRE_X360_MUTE_LED, > + ALC285_FIXUP_HP_BEEP_MICMUTE_LED, > ALC236_FIXUP_HP_MUTE_LED_COEFBIT2, > ALC236_FIXUP_HP_GPIO_LED, > ALC236_FIXUP_HP_MUTE_LED, > @@ -9303,6 +9329,12 @@ static const struct hda_fixup alc269_fixups[] = { > .type = HDA_FIXUP_FUNC, > .v.func = alc285_fixup_hp_spectre_x360_mute_led, > }, > + [ALC285_FIXUP_HP_BEEP_MICMUTE_LED] = { > + .type = HDA_FIXUP_FUNC, > + .v.func = alc285_fixup_hp_beep, > + .chained = true, > + .chain_id = ALC285_FIXUP_HP_MUTE_LED, > + }, > [ALC236_FIXUP_HP_MUTE_LED_COEFBIT2] = { > .type = HDA_FIXUP_FUNC, > .v.func = alc236_fixup_hp_mute_led_coefbit2, > @@ -10392,7 +10424,7 @@ static const struct hda_quirk alc269_fixup_tbl[] = { > SND_PCI_QUIRK(0x103c, 0x8730, "HP ProBook 445 G7", ALC236_FIXUP_HP_MUTE_LED_MICMUTE_VREF), > SND_PCI_QUIRK(0x103c, 0x8735, "HP ProBook 435 G7", ALC236_FIXUP_HP_MUTE_LED_MICMUTE_VREF), > SND_PCI_QUIRK(0x103c, 0x8736, "HP", ALC285_FIXUP_HP_GPIO_AMP_INIT), > - SND_PCI_QUIRK(0x103c, 0x8760, "HP", ALC285_FIXUP_HP_MUTE_LED), > + SND_PCI_QUIRK(0x103c, 0x8760, "HP EliteBook 8{4,5}5 G7", ALC285_FIXUP_HP_BEEP_MICMUTE_LED), > SND_PCI_QUIRK(0x103c, 0x876e, "HP ENVY x360 Convertible 13-ay0xxx", ALC245_FIXUP_HP_X360_MUTE_LEDS), > SND_PCI_QUIRK(0x103c, 0x877a, "HP", ALC285_FIXUP_HP_MUTE_LED), > SND_PCI_QUIRK(0x103c, 0x877d, "HP", ALC236_FIXUP_HP_MUTE_LED),
On 17. 02. 25 11:02, Takashi Iwai wrote: > On Sun, 16 Feb 2025 22:31:03 +0100, > Maciej S. Szmigiero wrote: >> >> PC speaker works well on this platform in BIOS and in Linux until sound >> card drivers are loaded. Then it stops working. >> >> There seems to be a beep generator node at 0x1a in this CODEC >> (ALC269_TYPE_ALC215) but it seems to be only connected to capture mixers >> at nodes 0x22 and 0x23. >> If I unmute the mixer input for 0x1a at node 0x23 and start recording >> from its "ALC285 Analog" capture device I can clearly hear beeps in that >> recording. >> >> So the beep generator is indeed working properly, however I wasn't able to >> figure out any way to connect it to speakers. >> >> However, the bits in the "Passthrough Control" register (0x36) seems to >> work at least partially: by zeroing "B" and "h" and setting "S" I can at >> least make the PIT PC speaker output appear either in this laptop speakers >> or headphones (depending on whether they are connected or not). >> >> >> There are some caveats, however: >> * If the CODEC gets runtime-suspended the beeps stop so it needs HDA beep >> device for keeping it awake during beeping. >> >> * If the beep generator node is generating any beep the PC beep passthrough >> seems to be temporarily inhibited, so the HDA beep device has to be >> prevented from using the actual beep generator node - but the beep device >> is still necessary due to the previous point. >> >> * In contrast with other platforms here beep amplification has to be >> disabled otherwise the beeps output are WAY louder than they were on pure >> BIOS setup. >> >> >> Unless someone (from Realtek probably) knows how to make the beep generator >> node output appear in speakers / headphones using PC beep passthrough seems >> to be the only way to make PC speaker beeping actually work on this >> platform. >> >> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name> >> --- >> >> Since more than 6 weeks has now passed since the previous version of this >> patch was posted, yet no better or other solution was provided I'm >> re-submitting an updated version of the original patch. >> >> This solution has been working fine for me on this platform all that time, >> without any obvious issues. >> >> Changes from v1: >> * Correct the typo in !IS_ENABLED(CONFIG_INPUT_PCSPKR) code that the >> kernel test robot found. >> >> * Change codec_warn() into dev_warn_once(hda_codec_dev(codec)) >> to avoid spamming the kernel log at runtime PM CODEC re-init. > > This is really a thing to be checked by Realtek people at first, as > it's very vendor-specific thing. > > Kailang, please check this. The primary issue seems to be with the beep signal routing to the analog outputs. The hidden codec registers accessed through the coefficient nodes may play a role for this setup. Only Realtek can give a solid answer for this (lack of public codec documentation). Jaroslav
On 17.02.2025 11:02, Takashi Iwai wrote: > On Sun, 16 Feb 2025 22:31:03 +0100, > Maciej S. Szmigiero wrote: >> >> PC speaker works well on this platform in BIOS and in Linux until sound >> card drivers are loaded. Then it stops working. >> >> There seems to be a beep generator node at 0x1a in this CODEC >> (ALC269_TYPE_ALC215) but it seems to be only connected to capture mixers >> at nodes 0x22 and 0x23. >> If I unmute the mixer input for 0x1a at node 0x23 and start recording >> from its "ALC285 Analog" capture device I can clearly hear beeps in that >> recording. >> >> So the beep generator is indeed working properly, however I wasn't able to >> figure out any way to connect it to speakers. >> >> However, the bits in the "Passthrough Control" register (0x36) seems to >> work at least partially: by zeroing "B" and "h" and setting "S" I can at >> least make the PIT PC speaker output appear either in this laptop speakers >> or headphones (depending on whether they are connected or not). >> >> >> There are some caveats, however: >> * If the CODEC gets runtime-suspended the beeps stop so it needs HDA beep >> device for keeping it awake during beeping. >> >> * If the beep generator node is generating any beep the PC beep passthrough >> seems to be temporarily inhibited, so the HDA beep device has to be >> prevented from using the actual beep generator node - but the beep device >> is still necessary due to the previous point. >> >> * In contrast with other platforms here beep amplification has to be >> disabled otherwise the beeps output are WAY louder than they were on pure >> BIOS setup. >> >> >> Unless someone (from Realtek probably) knows how to make the beep generator >> node output appear in speakers / headphones using PC beep passthrough seems >> to be the only way to make PC speaker beeping actually work on this >> platform. >> >> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name> >> --- >> >> Since more than 6 weeks has now passed since the previous version of this >> patch was posted, yet no better or other solution was provided I'm >> re-submitting an updated version of the original patch. >> >> This solution has been working fine for me on this platform all that time, >> without any obvious issues. >> >> Changes from v1: >> * Correct the typo in !IS_ENABLED(CONFIG_INPUT_PCSPKR) code that the >> kernel test robot found. >> >> * Change codec_warn() into dev_warn_once(hda_codec_dev(codec)) >> to avoid spamming the kernel log at runtime PM CODEC re-init. > > This is really a thing to be checked by Realtek people at first, as > it's very vendor-specific thing. > > Kailang, please check this. Realtek people has been asked to comment/provide alternative solution 3 times in last 6 weeks: On the original v1 submission, by your message from Jan 12, by my message on Feb 2. Looking at https://lore.kernel.org/linux-sound/?q=f%3Arealtek it seems Kailang sent two e-mails about unrelated cases to linux-sound during that time. To be honest, I don't understand why Realtek people don't comment on this since I would think that's a rather simple matter without any truly confidential aspects but on the other hand this fact should *not* permanently block fixing PC beep on this platform. So I think there should be some deadline here for the vendor response - like 1 month more or so? > And, except for that, I'm still concerned by the behavior change. AFAIK most sound card drivers in ALSA were developed without any docs, and the register that's being changed is unofficially documented in ALSA: https://docs.kernel.org/sound/hd-audio/realtek-pc-beep.html Also, the behavior change is clearly limited to this single laptop platform (HP EliteBook 855 G7) so the "blast radius" is very limited. > Also the caveat you mentioned about the runtime PM raises some doubt, > too. I think it's simply because the CODEC get re-initialized when it comes out of runtime PM sleep so if we print a message there then it would be printed each time the CODEC resumed from runtime PM sleep. That's why I changed to print this hint about CONFIG_INPUT_PCSPKR just once per CODEC device. > > thanks, > > Takashi > Thanks, Maciej
On Mon, 17 Feb 2025 11:17:31 +0100, Maciej S. Szmigiero wrote: > > On 17.02.2025 11:02, Takashi Iwai wrote: > > On Sun, 16 Feb 2025 22:31:03 +0100, > > Maciej S. Szmigiero wrote: > >> > >> PC speaker works well on this platform in BIOS and in Linux until sound > >> card drivers are loaded. Then it stops working. > >> > >> There seems to be a beep generator node at 0x1a in this CODEC > >> (ALC269_TYPE_ALC215) but it seems to be only connected to capture mixers > >> at nodes 0x22 and 0x23. > >> If I unmute the mixer input for 0x1a at node 0x23 and start recording > >> from its "ALC285 Analog" capture device I can clearly hear beeps in that > >> recording. > >> > >> So the beep generator is indeed working properly, however I wasn't able to > >> figure out any way to connect it to speakers. > >> > >> However, the bits in the "Passthrough Control" register (0x36) seems to > >> work at least partially: by zeroing "B" and "h" and setting "S" I can at > >> least make the PIT PC speaker output appear either in this laptop speakers > >> or headphones (depending on whether they are connected or not). > >> > >> > >> There are some caveats, however: > >> * If the CODEC gets runtime-suspended the beeps stop so it needs HDA beep > >> device for keeping it awake during beeping. > >> > >> * If the beep generator node is generating any beep the PC beep passthrough > >> seems to be temporarily inhibited, so the HDA beep device has to be > >> prevented from using the actual beep generator node - but the beep device > >> is still necessary due to the previous point. > >> > >> * In contrast with other platforms here beep amplification has to be > >> disabled otherwise the beeps output are WAY louder than they were on pure > >> BIOS setup. > >> > >> > >> Unless someone (from Realtek probably) knows how to make the beep generator > >> node output appear in speakers / headphones using PC beep passthrough seems > >> to be the only way to make PC speaker beeping actually work on this > >> platform. > >> > >> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name> > >> --- > >> > >> Since more than 6 weeks has now passed since the previous version of this > >> patch was posted, yet no better or other solution was provided I'm > >> re-submitting an updated version of the original patch. > >> This solution has been working fine for me on this platform > >> all that time, > >> without any obvious issues. > >> Changes from v1: > >> * Correct the typo in !IS_ENABLED(CONFIG_INPUT_PCSPKR) code that the > >> kernel test robot found. > >> * Change codec_warn() into dev_warn_once(hda_codec_dev(codec)) > >> to avoid spamming the kernel log at runtime PM CODEC re-init. > > > > This is really a thing to be checked by Realtek people at first, as > > it's very vendor-specific thing. > > > > Kailang, please check this. > > Realtek people has been asked to comment/provide alternative solution > 3 times in last 6 weeks: > On the original v1 submission, by your message from Jan 12, by my > message on Feb 2. > > Looking at https://lore.kernel.org/linux-sound/?q=f%3Arealtek > it seems Kailang sent two e-mails about unrelated cases to linux-sound > during that time. > > To be honest, I don't understand why Realtek people don't comment > on this since I would think that's a rather simple matter without any > truly confidential aspects but on the other hand this fact should *not* > permanently block fixing PC beep on this platform. > > So I think there should be some deadline here for the vendor response - > like 1 month more or so? Sorry, I don't like that kind of attitude. If the patch were perfectly fine, I'd have already taken. But there is a thing that can't be validated without the confirmation from the vendor, and that's not what we can accept because it's supposed to work on your machine -- that's only one special use case, and it doesn't qualify to prove the safety. In general, Kailang (and Realtek) has been helpful over decades for HD-audio stuff development, but they might be busy sometimes. Let's keep asking until catching their attention, at first. > > And, except for that, I'm still concerned by the behavior change. > > AFAIK most sound card drivers in ALSA were developed without any docs, > and the register that's being changed is unofficially documented in ALSA: > https://docs.kernel.org/sound/hd-audio/realtek-pc-beep.html > > Also, the behavior change is clearly limited to this single laptop > platform (HP EliteBook 855 G7) so the "blast radius" is very limited. If you were the only user of this laptop in the world, I wouldn't be concerned, sure :) But certainly that's not the case. > > Also the caveat you mentioned about the runtime PM raises some doubt, > > too. > > I think it's simply because the CODEC get re-initialized when it comes > out of runtime PM sleep so if we print a message there then it would > be printed each time the CODEC resumed from runtime PM sleep. > > That's why I changed to print this hint about CONFIG_INPUT_PCSPKR > just once per CODEC device. Well, but this runtime PM has to be turned off manually, otherwise the beep gets broken, right? This is already some trade-off, so it's not super trivial to apply the suggested change blindly. I thought that the input beep infrastructure may work with multiple input beep devices, and it usually triggers the all beep devices? If so, you can still keep the HD-audio beep (even though it doesn't do actually output) so that it can manage the runtime PM of HD-audio device at beeping, too. thanks, Takashi
On 17.02.2025 11:52, Takashi Iwai wrote: > On Mon, 17 Feb 2025 11:17:31 +0100, > Maciej S. Szmigiero wrote: >> >> On 17.02.2025 11:02, Takashi Iwai wrote: >>> On Sun, 16 Feb 2025 22:31:03 +0100, >>> Maciej S. Szmigiero wrote: >>>> >>>> PC speaker works well on this platform in BIOS and in Linux until sound >>>> card drivers are loaded. Then it stops working. >>>> >>>> There seems to be a beep generator node at 0x1a in this CODEC >>>> (ALC269_TYPE_ALC215) but it seems to be only connected to capture mixers >>>> at nodes 0x22 and 0x23. >>>> If I unmute the mixer input for 0x1a at node 0x23 and start recording >>>> from its "ALC285 Analog" capture device I can clearly hear beeps in that >>>> recording. >>>> >>>> So the beep generator is indeed working properly, however I wasn't able to >>>> figure out any way to connect it to speakers. >>>> >>>> However, the bits in the "Passthrough Control" register (0x36) seems to >>>> work at least partially: by zeroing "B" and "h" and setting "S" I can at >>>> least make the PIT PC speaker output appear either in this laptop speakers >>>> or headphones (depending on whether they are connected or not). >>>> >>>> >>>> There are some caveats, however: >>>> * If the CODEC gets runtime-suspended the beeps stop so it needs HDA beep >>>> device for keeping it awake during beeping. >>>> >>>> * If the beep generator node is generating any beep the PC beep passthrough >>>> seems to be temporarily inhibited, so the HDA beep device has to be >>>> prevented from using the actual beep generator node - but the beep device >>>> is still necessary due to the previous point. >>>> >>>> * In contrast with other platforms here beep amplification has to be >>>> disabled otherwise the beeps output are WAY louder than they were on pure >>>> BIOS setup. >>>> >>>> >>>> Unless someone (from Realtek probably) knows how to make the beep generator >>>> node output appear in speakers / headphones using PC beep passthrough seems >>>> to be the only way to make PC speaker beeping actually work on this >>>> platform. >>>> >>>> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name> >>>> --- >>>> >>>> Since more than 6 weeks has now passed since the previous version of this >>>> patch was posted, yet no better or other solution was provided I'm >>>> re-submitting an updated version of the original patch. >>>> This solution has been working fine for me on this platform >>>> all that time, >>>> without any obvious issues. >>>> Changes from v1: >>>> * Correct the typo in !IS_ENABLED(CONFIG_INPUT_PCSPKR) code that the >>>> kernel test robot found. >>>> * Change codec_warn() into dev_warn_once(hda_codec_dev(codec)) >>>> to avoid spamming the kernel log at runtime PM CODEC re-init. >>> >>> This is really a thing to be checked by Realtek people at first, as >>> it's very vendor-specific thing. >>> >>> Kailang, please check this. >> >> Realtek people has been asked to comment/provide alternative solution >> 3 times in last 6 weeks: >> On the original v1 submission, by your message from Jan 12, by my >> message on Feb 2. >> >> Looking at https://lore.kernel.org/linux-sound/?q=f%3Arealtek >> it seems Kailang sent two e-mails about unrelated cases to linux-sound >> during that time. >> >> To be honest, I don't understand why Realtek people don't comment >> on this since I would think that's a rather simple matter without any >> truly confidential aspects but on the other hand this fact should *not* >> permanently block fixing PC beep on this platform. >> >> So I think there should be some deadline here for the vendor response - >> like 1 month more or so? > > Sorry, I don't like that kind of attitude. > > If the patch were perfectly fine, I'd have already taken. But there > is a thing that can't be validated without the confirmation from the > vendor, and that's not what we can accept because it's supposed to > work on your machine -- that's only one special use case, and it > doesn't qualify to prove the safety. I don't want to pressure you as the maintainer to take this patch, sorry if it sounded that way. Just wanted to make sure that if the vendor does not respond within a reasonable time (I think 2.5 months total would be such time) then there's some alternate way. But let's wait then. > In general, Kailang (and Realtek) has been helpful over decades for > HD-audio stuff development, but they might be busy sometimes. Let's > keep asking until catching their attention, at first. Nice, I hope Realtek will finally provide a good answer at some point. Also, that's just PC beep passthrough, not CPU voltage adjustment :) So the risk is rather limited here. >>> And, except for that, I'm still concerned by the behavior change. >> >> AFAIK most sound card drivers in ALSA were developed without any docs, >> and the register that's being changed is unofficially documented in ALSA: >> https://docs.kernel.org/sound/hd-audio/realtek-pc-beep.html >> >> Also, the behavior change is clearly limited to this single laptop >> platform (HP EliteBook 855 G7) so the "blast radius" is very limited. > > If you were the only user of this laptop in the world, I wouldn't be > concerned, sure :) But certainly that's not the case. I don't think my particular laptop is a special specimen - it's running totally stock HP firmware, and AFAIK nothing has been changed there. >>> Also the caveat you mentioned about the runtime PM raises some doubt, >>> too. >> >> I think it's simply because the CODEC get re-initialized when it comes >> out of runtime PM sleep so if we print a message there then it would >> be printed each time the CODEC resumed from runtime PM sleep. >> >> That's why I changed to print this hint about CONFIG_INPUT_PCSPKR >> just once per CODEC device. > > Well, but this runtime PM has to be turned off manually, otherwise the > beep gets broken, right? This is already some trade-off, so it's not > super trivial to apply the suggested change blindly. No, runtime PM does *not* have to be turned off manually - in fact, I'm running with CONFIG_SND_HDA_POWER_SAVE_DEFAULT=10 and when testing that patch I could see the CODEC being runtime suspended and resumed automatically. But for the beep passthrough or speakers to be working the CODEC has to be awake during that beep duration. That's already the case for the existing hda_beep.c code - that's why its runtime PM wakeup code was reused in this patch. > I thought that the input beep infrastructure may work with multiple > input beep devices, and it usually triggers the all beep devices? That's true and in fact *necessary* for this patch to work since it needs *both* the temporary runtime PM wakeup provided by hda_beep.c AND the actual beep provided by PIT (or CONFIG_INPUT_PCSPKR). > If so, you can still keep the HD-audio beep (even though it doesn't do > actually output) so that it can manage the runtime PM of HD-audio > device at beeping, too. The beep widget is *not* just not generating beep, it seems to also inhibit the passthrough path, that's why I wrote in the patch message that: > If the beep generator node is generating any beep the PC beep passthrough > seems to be temporarily inhibited, so the HDA beep device has to be > prevented from using the actual beep generator node - but the beep device > is still necessary due to the previous point. > > thanks, > > Takashi Thanks, Maciej
Please let me check with our AE team. > -----Original Message----- > From: Takashi Iwai <tiwai@suse.de> > Sent: Monday, February 17, 2025 6:02 PM > To: Maciej S. Szmigiero <mail@maciej.szmigiero.name> > Cc: Jaroslav Kysela <perex@perex.cz>; Takashi Iwai <tiwai@suse.com>; Kailang > <kailang@realtek.com>; Oder Chiou <oder_chiou@realtek.com>; Shuming [θ > ζΈι] <shumingf@realtek.com>; Qiu Wenbo <qiuwenbo@kylinos.com.cn>; > linux-sound@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v2] ALSA: hda/realtek: Enable PC beep passthrough for HP > EliteBook 855 G7 > > > External mail : This email originated from outside the organization. Do not > reply, click links, or open attachments unless you recognize the sender and > know the content is safe. > > > > On Sun, 16 Feb 2025 22:31:03 +0100, > Maciej S. Szmigiero wrote: > > > > PC speaker works well on this platform in BIOS and in Linux until > > sound card drivers are loaded. Then it stops working. > > > > There seems to be a beep generator node at 0x1a in this CODEC > > (ALC269_TYPE_ALC215) but it seems to be only connected to capture > > mixers at nodes 0x22 and 0x23. > > If I unmute the mixer input for 0x1a at node 0x23 and start recording > > from its "ALC285 Analog" capture device I can clearly hear beeps in > > that recording. > > > > So the beep generator is indeed working properly, however I wasn't > > able to figure out any way to connect it to speakers. > > > > However, the bits in the "Passthrough Control" register (0x36) seems > > to work at least partially: by zeroing "B" and "h" and setting "S" I > > can at least make the PIT PC speaker output appear either in this > > laptop speakers or headphones (depending on whether they are connected > or not). > > > > > > There are some caveats, however: > > * If the CODEC gets runtime-suspended the beeps stop so it needs HDA > > beep device for keeping it awake during beeping. > > > > * If the beep generator node is generating any beep the PC beep > > passthrough seems to be temporarily inhibited, so the HDA beep device > > has to be prevented from using the actual beep generator node - but > > the beep device is still necessary due to the previous point. > > > > * In contrast with other platforms here beep amplification has to be > > disabled otherwise the beeps output are WAY louder than they were on > > pure BIOS setup. > > > > > > Unless someone (from Realtek probably) knows how to make the beep > > generator node output appear in speakers / headphones using PC beep > > passthrough seems to be the only way to make PC speaker beeping > > actually work on this platform. > > > > Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name> > > --- > > > > Since more than 6 weeks has now passed since the previous version of > > this patch was posted, yet no better or other solution was provided > > I'm re-submitting an updated version of the original patch. > > > > This solution has been working fine for me on this platform all that > > time, without any obvious issues. > > > > Changes from v1: > > * Correct the typo in !IS_ENABLED(CONFIG_INPUT_PCSPKR) code that the > > kernel test robot found. > > > > * Change codec_warn() into dev_warn_once(hda_codec_dev(codec)) > > to avoid spamming the kernel log at runtime PM CODEC re-init. > > This is really a thing to be checked by Realtek people at first, as it's very > vendor-specific thing. > > Kailang, please check this. > > And, except for that, I'm still concerned by the behavior change. > Also the caveat you mentioned about the runtime PM raises some doubt, too. > > > thanks, > > Takashi > > > > > include/sound/hda_codec.h | 1 + > > sound/pci/hda/hda_beep.c | 15 +++++++++------ > > sound/pci/hda/patch_realtek.c | 34 > +++++++++++++++++++++++++++++++++- > > 3 files changed, 43 insertions(+), 7 deletions(-) > > > > diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h > > index 575e55aa08ca..c1fe6290d04d 100644 > > --- a/include/sound/hda_codec.h > > +++ b/include/sound/hda_codec.h > > @@ -195,6 +195,7 @@ struct hda_codec { > > /* beep device */ > > struct hda_beep *beep; > > unsigned int beep_mode; > > + bool beep_just_power_on; > > > > /* widget capabilities cache */ > > u32 *wcaps; > > diff --git a/sound/pci/hda/hda_beep.c b/sound/pci/hda/hda_beep.c index > > e51d47572557..13a7d92e8d8d 100644 > > --- a/sound/pci/hda/hda_beep.c > > +++ b/sound/pci/hda/hda_beep.c > > @@ -31,8 +31,9 @@ static void generate_tone(struct hda_beep *beep, int > tone) > > beep->power_hook(beep, true); > > beep->playing = 1; > > } > > - snd_hda_codec_write(codec, beep->nid, 0, > > - AC_VERB_SET_BEEP_CONTROL, tone); > > + if (!codec->beep_just_power_on) > > + snd_hda_codec_write(codec, beep->nid, 0, > > + AC_VERB_SET_BEEP_CONTROL, > tone); > > if (!tone && beep->playing) { > > beep->playing = 0; > > if (beep->power_hook) > > @@ -212,10 +213,12 @@ 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 */ > > - if (codec->beep_mode == HDA_BEEP_MODE_OFF) > > - return 0; /* disabled by module option */ > > + if (!codec->beep_just_power_on) { > > + if (!snd_hda_get_bool_hint(codec, "beep")) > > + return 0; /* disabled explicitly by hints */ > > + if (codec->beep_mode == HDA_BEEP_MODE_OFF) > > + return 0; /* disabled by module option */ > > + } > > > > beep = kzalloc(sizeof(*beep), GFP_KERNEL); > > if (beep == NULL) > > diff --git a/sound/pci/hda/patch_realtek.c > > b/sound/pci/hda/patch_realtek.c index 224616fbec4f..fff6e7c1c265 > > 100644 > > --- a/sound/pci/hda/patch_realtek.c > > +++ b/sound/pci/hda/patch_realtek.c > > @@ -28,6 +28,7 @@ > > #include <sound/hda_codec.h> > > #include "hda_local.h" > > #include "hda_auto_parser.h" > > +#include "hda_beep.h" > > #include "hda_jack.h" > > #include "hda_generic.h" > > #include "hda_component.h" > > @@ -6923,6 +6924,30 @@ static void alc285_fixup_hp_envy_x360(struct > hda_codec *codec, > > } > > } > > > > +static void alc285_fixup_hp_beep(struct hda_codec *codec, > > + const struct hda_fixup *fix, int > > +action) { > > + if (action == HDA_FIXUP_ACT_PRE_PROBE) { > > + codec->beep_just_power_on = true; > > + } else if (action == HDA_FIXUP_ACT_INIT) { #ifdef > > +CONFIG_SND_HDA_INPUT_BEEP > > + /* > > + * Just enable loopback to internal speaker and headphone > jack. > > + * Disable amplification to get about the same beep > volume as > > + * was on pure BIOS setup before loading the driver. > > + */ > > + alc_update_coef_idx(codec, 0x36, 0x7070, BIT(13)); > > + > > + snd_hda_enable_beep_device(codec, 1); > > + > > +#if !IS_ENABLED(CONFIG_INPUT_PCSPKR) > > + dev_warn_once(hda_codec_dev(codec), > > + "enable CONFIG_INPUT_PCSPKR to get PC > > +beeps\n"); #endif #endif > > + } > > +} > > + > > /* for hda_fixup_thinkpad_acpi() */ > > #include "thinkpad_helper.c" > > > > @@ -7707,6 +7732,7 @@ enum { > > ALC285_FIXUP_HP_GPIO_LED, > > ALC285_FIXUP_HP_MUTE_LED, > > ALC285_FIXUP_HP_SPECTRE_X360_MUTE_LED, > > + ALC285_FIXUP_HP_BEEP_MICMUTE_LED, > > ALC236_FIXUP_HP_MUTE_LED_COEFBIT2, > > ALC236_FIXUP_HP_GPIO_LED, > > ALC236_FIXUP_HP_MUTE_LED, > > @@ -9303,6 +9329,12 @@ static const struct hda_fixup alc269_fixups[] = { > > .type = HDA_FIXUP_FUNC, > > .v.func = alc285_fixup_hp_spectre_x360_mute_led, > > }, > > + [ALC285_FIXUP_HP_BEEP_MICMUTE_LED] = { > > + .type = HDA_FIXUP_FUNC, > > + .v.func = alc285_fixup_hp_beep, > > + .chained = true, > > + .chain_id = ALC285_FIXUP_HP_MUTE_LED, > > + }, > > [ALC236_FIXUP_HP_MUTE_LED_COEFBIT2] = { > > .type = HDA_FIXUP_FUNC, > > .v.func = alc236_fixup_hp_mute_led_coefbit2, > > @@ -10392,7 +10424,7 @@ static const struct hda_quirk alc269_fixup_tbl[] > = { > > SND_PCI_QUIRK(0x103c, 0x8730, "HP ProBook 445 G7", > ALC236_FIXUP_HP_MUTE_LED_MICMUTE_VREF), > > SND_PCI_QUIRK(0x103c, 0x8735, "HP ProBook 435 G7", > ALC236_FIXUP_HP_MUTE_LED_MICMUTE_VREF), > > SND_PCI_QUIRK(0x103c, 0x8736, "HP", > ALC285_FIXUP_HP_GPIO_AMP_INIT), > > - SND_PCI_QUIRK(0x103c, 0x8760, "HP", > ALC285_FIXUP_HP_MUTE_LED), > > + SND_PCI_QUIRK(0x103c, 0x8760, "HP EliteBook 8{4,5}5 G7", > > + ALC285_FIXUP_HP_BEEP_MICMUTE_LED), > > SND_PCI_QUIRK(0x103c, 0x876e, "HP ENVY x360 Convertible > 13-ay0xxx", ALC245_FIXUP_HP_X360_MUTE_LEDS), > > SND_PCI_QUIRK(0x103c, 0x877a, "HP", > ALC285_FIXUP_HP_MUTE_LED), > > SND_PCI_QUIRK(0x103c, 0x877d, "HP", > ALC236_FIXUP_HP_MUTE_LED),
Hi Maciej S, Could you test plug headphone then speaker will mute or not? Below was why I close the beep mode. https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?id=fcf c9f711d1e2fc7876ac12b1b16c509404b9625 Hi Takashi, COEF 0x36 bit 13 was the enable bit for pcbeep passthrough for 0x14. If this patch no PM issues and speaker can mute by plug headphone or headset, I think this work for me.
Paste link again. https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?id=fcfc9f711d1e2fc7876ac12b1b16c509404b9625
Hi Kailang, On 19.02.2025 09:32, Kailang wrote: > > Hi Maciej S, > > Could you test plug headphone then speaker will mute or not? > > Below was why I close the beep mode. > https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?id=fcf c9f711d1e2fc7876ac12b1b16c509404b9625 When I plug headphones then both normal sound playback (ALSA) and PC beep move to headphones. The main laptop speaker gets muted indeed. I'm testing on kernel 6.12 so the Dell patch above is included. > Hi Takashi, > > COEF 0x36 bit 13 was the enable bit for pcbeep passthrough for 0x14. > If this patch no PM issues and speaker can mute by plug headphone or headset, > I think this work for me. > Nice, thanks for clarifying. Thanks, Maciej
diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h index 575e55aa08ca..c1fe6290d04d 100644 --- a/include/sound/hda_codec.h +++ b/include/sound/hda_codec.h @@ -195,6 +195,7 @@ struct hda_codec { /* beep device */ struct hda_beep *beep; unsigned int beep_mode; + bool beep_just_power_on; /* widget capabilities cache */ u32 *wcaps; diff --git a/sound/pci/hda/hda_beep.c b/sound/pci/hda/hda_beep.c index e51d47572557..13a7d92e8d8d 100644 --- a/sound/pci/hda/hda_beep.c +++ b/sound/pci/hda/hda_beep.c @@ -31,8 +31,9 @@ static void generate_tone(struct hda_beep *beep, int tone) beep->power_hook(beep, true); beep->playing = 1; } - snd_hda_codec_write(codec, beep->nid, 0, - AC_VERB_SET_BEEP_CONTROL, tone); + if (!codec->beep_just_power_on) + snd_hda_codec_write(codec, beep->nid, 0, + AC_VERB_SET_BEEP_CONTROL, tone); if (!tone && beep->playing) { beep->playing = 0; if (beep->power_hook) @@ -212,10 +213,12 @@ 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 */ - if (codec->beep_mode == HDA_BEEP_MODE_OFF) - return 0; /* disabled by module option */ + if (!codec->beep_just_power_on) { + if (!snd_hda_get_bool_hint(codec, "beep")) + return 0; /* disabled explicitly by hints */ + if (codec->beep_mode == HDA_BEEP_MODE_OFF) + return 0; /* disabled by module option */ + } beep = kzalloc(sizeof(*beep), GFP_KERNEL); if (beep == NULL) diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 224616fbec4f..fff6e7c1c265 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -28,6 +28,7 @@ #include <sound/hda_codec.h> #include "hda_local.h" #include "hda_auto_parser.h" +#include "hda_beep.h" #include "hda_jack.h" #include "hda_generic.h" #include "hda_component.h" @@ -6923,6 +6924,30 @@ static void alc285_fixup_hp_envy_x360(struct hda_codec *codec, } } +static void alc285_fixup_hp_beep(struct hda_codec *codec, + const struct hda_fixup *fix, int action) +{ + if (action == HDA_FIXUP_ACT_PRE_PROBE) { + codec->beep_just_power_on = true; + } else if (action == HDA_FIXUP_ACT_INIT) { +#ifdef CONFIG_SND_HDA_INPUT_BEEP + /* + * Just enable loopback to internal speaker and headphone jack. + * Disable amplification to get about the same beep volume as + * was on pure BIOS setup before loading the driver. + */ + alc_update_coef_idx(codec, 0x36, 0x7070, BIT(13)); + + snd_hda_enable_beep_device(codec, 1); + +#if !IS_ENABLED(CONFIG_INPUT_PCSPKR) + dev_warn_once(hda_codec_dev(codec), + "enable CONFIG_INPUT_PCSPKR to get PC beeps\n"); +#endif +#endif + } +} + /* for hda_fixup_thinkpad_acpi() */ #include "thinkpad_helper.c" @@ -7707,6 +7732,7 @@ enum { ALC285_FIXUP_HP_GPIO_LED, ALC285_FIXUP_HP_MUTE_LED, ALC285_FIXUP_HP_SPECTRE_X360_MUTE_LED, + ALC285_FIXUP_HP_BEEP_MICMUTE_LED, ALC236_FIXUP_HP_MUTE_LED_COEFBIT2, ALC236_FIXUP_HP_GPIO_LED, ALC236_FIXUP_HP_MUTE_LED, @@ -9303,6 +9329,12 @@ static const struct hda_fixup alc269_fixups[] = { .type = HDA_FIXUP_FUNC, .v.func = alc285_fixup_hp_spectre_x360_mute_led, }, + [ALC285_FIXUP_HP_BEEP_MICMUTE_LED] = { + .type = HDA_FIXUP_FUNC, + .v.func = alc285_fixup_hp_beep, + .chained = true, + .chain_id = ALC285_FIXUP_HP_MUTE_LED, + }, [ALC236_FIXUP_HP_MUTE_LED_COEFBIT2] = { .type = HDA_FIXUP_FUNC, .v.func = alc236_fixup_hp_mute_led_coefbit2, @@ -10392,7 +10424,7 @@ static const struct hda_quirk alc269_fixup_tbl[] = { SND_PCI_QUIRK(0x103c, 0x8730, "HP ProBook 445 G7", ALC236_FIXUP_HP_MUTE_LED_MICMUTE_VREF), SND_PCI_QUIRK(0x103c, 0x8735, "HP ProBook 435 G7", ALC236_FIXUP_HP_MUTE_LED_MICMUTE_VREF), SND_PCI_QUIRK(0x103c, 0x8736, "HP", ALC285_FIXUP_HP_GPIO_AMP_INIT), - SND_PCI_QUIRK(0x103c, 0x8760, "HP", ALC285_FIXUP_HP_MUTE_LED), + SND_PCI_QUIRK(0x103c, 0x8760, "HP EliteBook 8{4,5}5 G7", ALC285_FIXUP_HP_BEEP_MICMUTE_LED), SND_PCI_QUIRK(0x103c, 0x876e, "HP ENVY x360 Convertible 13-ay0xxx", ALC245_FIXUP_HP_X360_MUTE_LEDS), SND_PCI_QUIRK(0x103c, 0x877a, "HP", ALC285_FIXUP_HP_MUTE_LED), SND_PCI_QUIRK(0x103c, 0x877d, "HP", ALC236_FIXUP_HP_MUTE_LED),
PC speaker works well on this platform in BIOS and in Linux until sound card drivers are loaded. Then it stops working. There seems to be a beep generator node at 0x1a in this CODEC (ALC269_TYPE_ALC215) but it seems to be only connected to capture mixers at nodes 0x22 and 0x23. If I unmute the mixer input for 0x1a at node 0x23 and start recording from its "ALC285 Analog" capture device I can clearly hear beeps in that recording. So the beep generator is indeed working properly, however I wasn't able to figure out any way to connect it to speakers. However, the bits in the "Passthrough Control" register (0x36) seems to work at least partially: by zeroing "B" and "h" and setting "S" I can at least make the PIT PC speaker output appear either in this laptop speakers or headphones (depending on whether they are connected or not). There are some caveats, however: * If the CODEC gets runtime-suspended the beeps stop so it needs HDA beep device for keeping it awake during beeping. * If the beep generator node is generating any beep the PC beep passthrough seems to be temporarily inhibited, so the HDA beep device has to be prevented from using the actual beep generator node - but the beep device is still necessary due to the previous point. * In contrast with other platforms here beep amplification has to be disabled otherwise the beeps output are WAY louder than they were on pure BIOS setup. Unless someone (from Realtek probably) knows how to make the beep generator node output appear in speakers / headphones using PC beep passthrough seems to be the only way to make PC speaker beeping actually work on this platform. Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name> --- Since more than 6 weeks has now passed since the previous version of this patch was posted, yet no better or other solution was provided I'm re-submitting an updated version of the original patch. This solution has been working fine for me on this platform all that time, without any obvious issues. Changes from v1: * Correct the typo in !IS_ENABLED(CONFIG_INPUT_PCSPKR) code that the kernel test robot found. * Change codec_warn() into dev_warn_once(hda_codec_dev(codec)) to avoid spamming the kernel log at runtime PM CODEC re-init. include/sound/hda_codec.h | 1 + sound/pci/hda/hda_beep.c | 15 +++++++++------ sound/pci/hda/patch_realtek.c | 34 +++++++++++++++++++++++++++++++++- 3 files changed, 43 insertions(+), 7 deletions(-)