When I try to beep the PC speaker with "echo -e '\a'" or beep command, I hear only clicking sound coming from the PC speaker. It happens after changing motherboard to MSI PRO B760M-A WiFi. Both modules pcspkr and snd_pcsp give the same result. There is no problems with the hardware because speaker beeps fine in: - BIOS when enabled beep on boot; - GRUB with 'play' command; - DOS. Reproducible: Always Steps to Reproduce: 1. modprobe pcspkr (or "modprobe snd_pcsp") 2. echo -e '\a' (or "beep") Actual Results: Clicking sound coming from the PC speaker. Expected Results: Beep sound coming from the PC speaker. Current kernel version: 6.8.10-300.fc40.x86_64
Is this reproducible in the Linux console? E.g. Ctrl + Alt + F2 -> login -> echo -e '\a' (Press Alt + F1 to return back to your Xorg/Wayland session).
Yes, this can be reproduced in both the Linux console and Xorg.
Please try this: sudo rmmod snd_pcsp echo -e '\a'
Oh, and make sure pcspkr is loaded: sudo lsmod | grep pcspkr
> Please try this: > sudo rmmod snd_pcsp > echo -e '\a' Predictably, no sound at all. > Oh, and make sure pcspkr is loaded: > sudo lsmod | grep pcspkr With pcspkr there is the same clicking sound as with snd_pcsp.
I'd make same additional tests. 1. I wrote a program for Linux and DOS to start infinite 1 kHz beep by directly programming of PIT2 ports (0x43, 0x42, 0x61). It makes infinite beep under DOS. But it makes only one click on start under Linux. 2. One more program for Linux and DOS reads current PIT2 value (port 0x42). It gets random values from 0 to 1193 under DOS, that is a correct behavior during playing 1 kHz sound. But under Linux every time it returns a value 8 less than the previous one. So, PIT2 doesn't work possibly due to the lack of a reference frequency at the timer input. 3. Finally, I wrote grub-efi module that starts infinite 1 kHz beep. When loading a kernel, the sound is turned off even if the kernel is loaded without initrd and modules. Something early in the kernel boot process disrupts the normal operation of the 8254 timer. Please advise how to determine the part of the kernel that is causing the problem.
Created attachment 306415 [details] grub module source
Created attachment 306416 [details] Compiled grub-efi module
how about adding a boot dmesg and a sudo lspci -vvv ? (and I assume cat /proc/ioports as root shows timer0 at 0040-0043 ?)
Just a few notes: 1. IIRC the "beep" code of snd-pcsp is a direct copy/paste from pcspkr.c, Therefore no differences are expected. 2. There is an "nforce_wa" module parameter of snd-pcsp. Enabling it unbinds the driver from PIT2 entirely, so worth a try. But that only works for digital sound, because of (1). I.e. beep code remains PIT2-based. 3. Since we don't use PIT0 and its IRQ, its really surprising that something in an early kernel boot could "break" PIT2 (which is the only PIT used in both drivers). The only theory I can come up with (with a very good portion of imagination), is that PIT2 is SMM-assisted on your HW, and linux does some (likely ACPI-based) hand-off procedure and that SMM module goes away. This is not so impossible, knowing that nforce chipsets were lacking most of PIT modes 20 years ago already. Maybe now the modes were re-added via some SMM helper?
That said, non-acpi boot might be worth a try.
Created attachment 306457 [details] dmesg
Created attachment 306458 [details] lspci -vvv
(In reply to Dr. David Alan Gilbert from comment #9) > how about adding a boot dmesg and a sudo lspci -vvv ? > > (and I assume cat /proc/ioports as root shows timer0 at 0040-0043 ?) Yes, timer0 is present. 0000-0cf7 : PCI Bus 0000:00 0000-001f : dma1 0020-0021 : pic1 0040-0043 : timer0 0050-0053 : timer1 0060-0060 : keyboard 0064-0064 : keyboard 0070-0071 : rtc_cmos 0070-0071 : rtc0 0080-008f : dma page reg 00a0-00a1 : pic2 00c0-00df : dma2 00f0-00ff : fpu 0400-041f : iTCO_wdt 0400-041f : iTCO_wdt 0680-069f : pnp 00:01 0a00-0a0f : pnp 00:00 0a10-0a1f : pnp 00:00 0a20-0a2f : pnp 00:00 0a24-0a27 : nct6683 0a24-0a27 : nct6683 0cf8-0cff : PCI conf1 0d00-ffff : PCI Bus 0000:00 164e-164f : pnp 00:01 1800-1803 : ACPI PM1a_EVT_BLK 1804-1805 : ACPI PM1a_CNT_BLK 1808-180b : ACPI PM_TMR 1850-1850 : ACPI PM2_CNT_BLK 1854-1857 : pnp 00:02 1860-187f : ACPI GPE0_BLK 2000-20fe : pnp 00:04 3000-3fff : PCI Bus 0000:03 3000-30ff : 0000:03:00.0 4000-4fff : PCI Bus 0000:02 5000-5fff : PCI Bus 0000:01 5000-507f : 0000:01:00.0 6020-603f : 0000:00:17.0 6020-603f : ahci 6040-6043 : 0000:00:17.0 6040-6043 : ahci 6050-6057 : 0000:00:17.0 6050-6057 : ahci efa0-efbf : 0000:00:1f.4 efa0-efbf : i801_smbus
(In reply to Stas Sergeev from comment #11) > That said, non-acpi boot might be worth a try. When I boot kernel with acpi=off, the PC speaker works fine. But other problems appear: xorg session does not start, the computer freezes when rebooting or shutting down.
Then you need to re-assign this to acpi and reach out to some acpi experts.
I suggest (besides reaching the ACPI experts via an ML, as this bugzilla is usually unhelpful) to change the beep code in snd-pcsp to use hrtimer instead of PIT2, just as it is done for "nforce_wa". This is a 20-minutes work that gives you a functional setup at least with 1 driver of 2.
(In reply to Stas Sergeev from comment #17) > This is a 20-minutes work that > gives you a functional setup at > least with 1 driver of 2. Hi Stas, I have the same issue as the OP. I looked into the snd_pcsp code, and realised that the 20 minutes of work for you would take hours or days for someone like me with no kernel or sound driver development experience. Could we possibly ask a big favor of 20 minutes of your time to produce a patch that can be tested? It would be really appreciated, I miss being able to beep my laptop freely.
All you need is to flip bit 1 of port 0x61 with the desired frequency. This can be done by using the hrtimer, see pcsp_lib.c:pcsp_do_timer() as an example of the hrtimer use. Please ask github copilot to write that code, if you think you can't try doing that yourself. When he does, put it into pcsp_input.c instead of pcspkr_do_sound().
Thank you for the information Stas. With the amount of context I have, I honestly wouldn't know if what Copilot produces is correct or not. However, I understand if you are too busy to spend any time on this. If I find a spare day I will have a go myself. Best, Max -------- Original Message -------- On 5/1/25 03:48, <bugzilla-daemon@kernel.org> wrote: > https://bugzilla.kernel.org/show_bug.cgi?id=218918 > > Stas Sergeev (stsp@users.sourceforge.net) changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > CC| |stsp@users.sourceforge.net > > --- Comment #19 from Stas Sergeev (stsp@users.sourceforge.net) --- > All you need is to flip bit > 1 of port 0x61 with the desired > frequency. This can be done > by using the hrtimer, see > pcsp_lib.c:pcsp_do_timer() > as an example of the hrtimer > use. > Please ask github copilot > to write that code, if you > think you can't try doing > that yourself. When he does, > put it into pcsp_input.c > instead of pcspkr_do_sound(). > > -- > You may reply to this email to add a comment. > > You are receiving this mail because: > You are on the CC list for the bug.
OK I asked the copilot for you, and here's the patch that he suggests: diff --git a/sound/drivers/pcsp/pcsp_input.c b/sound/drivers/pcsp/pcsp_input.c index 4f79eaa2ceac..abcd1234abcd 100644 --- a/sound/drivers/pcsp/pcsp_input.c +++ b/sound/drivers/pcsp/pcsp_input.c @@ -10,6 +10,8 @@ #include <linux/io.h> #include "pcsp.h" #include "pcsp_input.h" +#include <linux/hrtimer.h> +#include <linux/ktime.h> static struct hrtimer pcsp_hrtimer; static ktime_t pcsp_hrtimer_interval; +static unsigned char pcsp_last_port_61_value = 0; /* Tracks last written value to port 0x61 */ static void pcspkr_do_sound(unsigned int count) { @@ -16,46 +18,90 @@ static void pcspkr_do_sound(unsigned int count) unsigned long flags; raw_spin_lock_irqsave(&i8253_lock, flags); - - if (count) { - /* set command for counter 2, 2 byte write */ - outb_p(0xB6, 0x43); - /* select desired HZ */ - outb_p(count & 0xff, 0x42); - outb((count >> 8) & 0xff, 0x42); - /* enable counter 2 */ - outb_p(inb_p(0x61) | 3, 0x61); - } else { - /* disable counter 2 */ - outb(inb_p(0x61) & 0xFC, 0x61); - } + if (count) { + /* Toggle bit 1 of the stored value for port 0x61 */ + pcsp_last_port_61_value ^= 0x02; /* Toggle bit 1 */ + outb(pcsp_last_port_61_value, 0x61); /* Write updated value to port */ + } else { + /* Reset bit 1 to 0 when count is zero */ + pcsp_last_port_61_value &= ~0x02; /* Clear bit 1 */ + outb(pcsp_last_port_61_value, 0x61); /* Write updated value to port */ + } raw_spin_unlock_irqrestore(&i8253_lock, flags); } void pcspkr_stop_sound(void) { - pcspkr_do_sound(0); + hrtimer_cancel(&pcsp_hrtimer); + pcspkr_do_sound(0); /* Ensure bit 1 is cleared when stopping sound */ } static enum hrtimer_restart pcsp_hrtimer_callback(struct hrtimer *timer) { + /* Toggle bit 1 of port 0x61 using the stored value */ + unsigned long flags; + raw_spin_lock_irqsave(&i8253_lock, flags); + pcsp_last_port_61_value ^= 0x02; /* Toggle bit 1 */ + outb(pcsp_last_port_61_value, 0x61); /* Write updated value to port */ + raw_spin_unlock_irqrestore(&i8253_lock, flags); + /* Restart the timer */ hrtimer_forward_now(timer, pcsp_hrtimer_interval); return HRTIMER_RESTART; } static void pcspkr_do_sound(unsigned int count) { + if (count) { + pcsp_hrtimer_interval = ktime_set(0, (NSEC_PER_SEC / count)); + hrtimer_start(&pcsp_hrtimer, pcsp_hrtimer_interval, HRTIMER_MODE_REL); + } else { + hrtimer_cancel(&pcsp_hrtimer); + } } int pcspkr_input_init(struct input_dev **rdev, struct device *dev) { int err; struct input_dev *input_dev = devm_input_allocate_device(dev); if (!input_dev) return -ENOMEM; + /* Initialize the hrtimer */ + hrtimer_init(&pcsp_hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); + pcsp_hrtimer.function = pcsp_hrtimer_callback; + + /* Initialize the last written value for port 0x61 */ + pcsp_last_port_61_value = inb(0x61) & 0xFC; /* Read initial value, clear bits 0 and 1 */ + input_dev->name = "PC Speaker"; input_dev->phys = "isa0061/input0"; input_dev->id.bustype = BUS_ISA;
The above diff was wrong. I asked him to rediff, here's the better result: ``` diff --git a/sound/drivers/pcsp/pcsp_input.c b/sound/drivers/pcsp/pcsp_input.c index 4f79eaa2ceac..abcd1234abcd 100644 --- a/sound/drivers/pcsp/pcsp_input.c +++ b/sound/drivers/pcsp/pcsp_input.c @@ -10,6 +10,8 @@ #include <linux/io.h> #include "pcsp.h" #include "pcsp_input.h" +#include <linux/hrtimer.h> +#include <linux/ktime.h> +static struct hrtimer pcsp_hrtimer; +static ktime_t pcsp_hrtimer_interval; +static unsigned char pcsp_last_port_61_value = 0; /* Tracks last written value to port 0x61 */ static void pcspkr_do_sound(unsigned int count) { @@ -16,46 +20,76 @@ static void pcspkr_do_sound(unsigned int count) unsigned long flags; raw_spin_lock_irqsave(&i8253_lock, flags); - - if (count) { - /* set command for counter 2, 2 byte write */ - outb_p(0xB6, 0x43); - /* select desired HZ */ - outb_p(count & 0xff, 0x42); - outb((count >> 8) & 0xff, 0x42); - /* enable counter 2 */ - outb_p(inb_p(0x61) | 3, 0x61); - } else { - /* disable counter 2 */ - outb(inb_p(0x61) & 0xFC, 0x61); - } + if (count) { + pcsp_last_port_61_value ^= 0x02; /* Toggle bit 1 */ + outb(pcsp_last_port_61_value, 0x61); /* Write updated value to port */ + } else { + pcsp_last_port_61_value &= ~0x02; /* Clear bit 1 */ + outb(pcsp_last_port_61_value, 0x61); /* Write updated value to port */ + } raw_spin_unlock_irqrestore(&i8253_lock, flags); } void pcspkr_stop_sound(void) { - pcspkr_do_sound(0); + hrtimer_cancel(&pcsp_hrtimer); + pcspkr_do_sound(0); /* Ensure bit 1 is cleared when stopping sound */ } +static enum hrtimer_restart pcsp_hrtimer_callback(struct hrtimer *timer) +{ + /* Toggle bit 1 of port 0x61 using the stored value */ + unsigned long flags; + raw_spin_lock_irqsave(&i8253_lock, flags); + pcsp_last_port_61_value ^= 0x02; /* Toggle bit 1 */ + outb(pcsp_last_port_61_value, 0x61); /* Write updated value to port */ + raw_spin_unlock_irqrestore(&i8253_lock, flags); + + /* Restart the timer */ + hrtimer_forward_now(timer, pcsp_hrtimer_interval); + return HRTIMER_RESTART; +} + +static void pcspkr_do_sound(unsigned int count) +{ + if (count) { + pcsp_hrtimer_interval = ktime_set(0, (NSEC_PER_SEC / count)); + hrtimer_start(&pcsp_hrtimer, pcsp_hrtimer_interval, HRTIMER_MODE_REL); + } else { + hrtimer_cancel(&pcsp_hrtimer); + } +} + int pcspkr_input_init(struct input_dev **rdev, struct device *dev) { int err; struct input_dev *input_dev = devm_input_allocate_device(dev); if (!input_dev) return -ENOMEM; + /* Initialize the hrtimer */ + hrtimer_init(&pcsp_hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); + pcsp_hrtimer.function = pcsp_hrtimer_callback; + + /* Initialize the last written value for port 0x61 */ + pcsp_last_port_61_value = inb(0x61) & 0xFC; /* Read initial value, clear bits 0 and 1 */ + input_dev->name = "PC Speaker"; input_dev->phys = "isa0061/input0"; input_dev->id.bustype = BUS_ISA; ```
Ok, he is full of bugs... After another hour of chatting, he came up with this, which looks at least somewhat viable: ``` diff --git a/sound/drivers/pcsp/pcsp_input.c b/sound/drivers/pcsp/pcsp_input.c index 4f79eaa2ceac..abcd1234abcd 100644 --- a/sound/drivers/pcsp/pcsp_input.c +++ b/sound/drivers/pcsp/pcsp_input.c @@ -10,13 +10,15 @@ #include <linux/init.h> #include <linux/input.h> #include <linux/io.h> +#include <linux/hrtimer.h> +#include <linux/ktime.h> #include "pcsp.h" #include "pcsp_input.h" -static void pcspkr_do_sound(unsigned int count) -{ - unsigned long flags; - - raw_spin_lock_irqsave(&i8253_lock, flags); - - if (count) { - /* set command for counter 2, 2 byte write */ - outb_p(0xB6, 0x43); - /* select desired HZ */ - outb_p(count & 0xff, 0x42); - outb((count >> 8) & 0xff, 0x42); - /* enable counter 2 */ - outb_p(inb_p(0x61) | 3, 0x61); - } else { - /* disable counter 2 */ - outb(inb_p(0x61) & 0xFC, 0x61); - } - - raw_spin_unlock_irqrestore(&i8253_lock, flags); -} - +/* hrtimer-related variables for precise sound control */ +static struct hrtimer pcsp_hrtimer; +static ktime_t pcsp_hrtimer_interval; +static unsigned char pcsp_last_port_61_value = 0; /* Tracks last written value to port 0x61 */ + +static enum hrtimer_restart pcspkr_hrtimer_callback(struct hrtimer *timer) +{ + /* Toggle bit 1 of port 0x61 */ + pcsp_last_port_61_value ^= 0x02; + outb(pcsp_last_port_61_value, 0x61); /* Write updated value to port */ + + /* Restart the timer */ + hrtimer_forward_now(timer, pcsp_hrtimer_interval); + return HRTIMER_RESTART; +} + +void pcspkr_stop_sound(void) +{ + /* Cancel the hrtimer */ + hrtimer_cancel(&pcsp_hrtimer); + + /* Clear bit 1 of port 0x61 to stop sound */ + pcsp_last_port_61_value &= ~0x02; + outb(pcsp_last_port_61_value, 0x61); /* Write updated value to port */ +} static int pcspkr_input_event(struct input_dev *dev, unsigned int type, unsigned int code, int value) { unsigned int count = 0; if (atomic_read(&pcsp_chip.timer_active) || !pcsp_chip.pcspkr) return 0; switch (type) { case EV_SND: switch (code) { case SND_BELL: if (value) value = 1000; break; case SND_TONE: break; default: return -1; } break; default: return -1; } if (value > 20 && value < 32767) { count = PIT_TICK_RATE / value; + pcsp_hrtimer_interval = ktime_set(0, (NSEC_PER_SEC / count)); + hrtimer_start(&pcsp_hrtimer, pcsp_hrtimer_interval, HRTIMER_MODE_REL); } else { + pcspkr_stop_sound(); } return 0; } int pcspkr_input_init(struct input_dev **rdev, struct device *dev) { int err; struct input_dev *input_dev = devm_input_allocate_device(dev); if (!input_dev) return -ENOMEM; input_dev->name = "PC Speaker"; input_dev->phys = "isa0061/input0"; input_dev->id.bustype = BUS_ISA; input_dev->id.vendor = 0x001f; input_dev->id.product = 0x0001; input_dev->id.version = 0x0100; input_dev->dev.parent = dev; input_dev->evbit[0] = BIT(EV_SND); input_dev->sndbit[0] = BIT(SND_BELL) | BIT(SND_TONE); input_dev->event = pcspkr_input_event; + /* Initialize the hrtimer */ + hrtimer_init(&pcsp_hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); + pcsp_hrtimer.function = pcspkr_hrtimer_callback; + + /* Initialize the last written value for port 0x61 */ + pcsp_last_port_61_value = inb(0x61) & 0xFC; /* Read initial value, clear bits 0 and 1 */ + err = input_register_device(input_dev); if (err) return err; ```
Hi Stas, Thank you so much for your work. I would have been pretty surprised if copilot could get kernel code right on the first go. I will compile and test your patch on my machine when I can find the time. (This is also my first time compiling a module myself!)