From: Jonathan Matthew Subject: Re: acpithinkpad: don't unmask brightness events on version 2 machines To: tech@openbsd.org Date: Sun, 1 Jun 2025 16:53:08 +1000 On Mon, May 19, 2025 at 06:43:23PM +1000, Jonathan Matthew wrote: > On my X13 gen 1, the screen brightness down button sets the screen to > minimum brightness, which is not very pleasant. This turns out to be > because both acpithinkpad(4) and acpivout(4) are processing these events, > even though this is a 'version 2' machine where acpithinkpad declines to > take control of screen brightness. > > The diff below resolves this by only unmasking the brightness up/down > events on version 1 machines. Without these events unmasked, only > acpivout is trying to apply screen brightness changes, and everything > works properly. > > Has anyone seen this problem on other thinkpads? Could this change > cause problems anywhere else? So this has been tested on version 1 machines (X240, T530), version 2 machines with native backlight control (X13 gen 1 and 2), and a version 2 machine with ACPI backlight control (P51). It doesn't break anything on the version 1 machines, and it improves backlight control using the brightness up/down hotkeys on the version 2 machines. As this only changes event masking for these hotkeys, it won't affect userspace driven backlight control, which will already be going through acpivout or *drm native support. This also brings us more in line with how linux handles the thinkpad brightness events, at least as I understand it, as seen here: https://github.com/torvalds/linux/blob/7d4e49a77d9930c69751b9192448fda6ff9100f1/drivers/platform/x86/thinkpad_acpi.c#L3508 It involves userspace stuff on Linux, but the idea is the same - you don't want two events for the same button press. ok? > > Index: acpithinkpad.c > =================================================================== > RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v > diff -u -p -r1.74 acpithinkpad.c > --- acpithinkpad.c 7 Jul 2023 07:37:59 -0000 1.74 > +++ acpithinkpad.c 19 May 2025 08:26:54 -0000 > @@ -428,10 +428,11 @@ thinkpad_enable_events(struct acpithinkp > } > > /* Enable events we need to know about */ > - mask |= (THINKPAD_MASK_MIC_MUTE | > - THINKPAD_MASK_BRIGHTNESS_UP | > - THINKPAD_MASK_BRIGHTNESS_DOWN | > - THINKPAD_MASK_KBD_BACKLIGHT); > + mask |= THINKPAD_MASK_MIC_MUTE | > + THINKPAD_MASK_KBD_BACKLIGHT; > + if (sc->sc_hkey_version == THINKPAD_HKEY_VERSION1) > + mask |= THINKPAD_MASK_BRIGHTNESS_UP | > + THINKPAD_MASK_BRIGHTNESS_DOWN; > > DPRINTF(("%s: setting event mask to 0x%llx\n", DEVNAME(sc), mask)); > >