Paths

Table of Contentst

acpi_system76: Support for acpi-controlled buttons on System76
ClosedPublic
Actions

Authored by pouria on Mar 6 2026, 7:18 PM.

Details

Reviewers

jkim
olce
imp
glebius
markj

Commits

rGfa4f625ed854: acpi_system76: unbreak LINT
rGcdad55809ef5: acpi_system76: Support for acpi-controlled buttons on System76

Summary

Add acpi_system76 for handling acpi-controlled buttons
on System76 Laptops.

This is my first attempt to break from network only codes :)
So please review carefully since it's my first ACPI driver.
I've tested it on my laptop and checked the dsdt.dsl of:
https://github.com/system76/firmware-open

I may add battary thresholds in future. But that needs custom ACPI
objects so I prefer to keep this simple for now.

Not sure who to add in reviewers list.

Test Plan

I have serw13 and it works perfectly:

% sysctl hw.acpi.s76.keyboard_backlight=0
hw.acpi.s76.keyboard_backlight: 255 -> 0
% sysctl hw.acpi.s76.keyboard_backlight=255
hw.acpi.s76.keyboard_backlight: 0 -> 255
% sysctl hw.acpi.s76.keyboard_color=0
hw.acpi.s76.keyboard_color: 65535 -> 0
% sysctl hw.acpi.s76.keyboard_color=123123
hw.acpi.s76.keyboard_color: 0 -> 123123

input validation

sysctl: hw.acpi.s76.keyboard_backlight=256: Invalid argument
% sysctl hw.acpi.s76.keyboard_backlight=-1
hw.acpi.s76.keyboard_backlight: 255
sysctl: hw.acpi.s76.keyboard_backlight=-1: Invalid argument

change color + backlight using keyboard and check if sysctl is updated.

% sysctl hw.acpi.s76
hw.acpi.s76.keyboard_color: 16776960
hw.acpi.s76.keyboard_backlight: 48

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

pouria requested review of this revision.Mar 6 2026, 7:18 PM

pouria created this revision.

Harbormaster completed remote builds in B71233: Diff 173276.Mar 6 2026, 7:18 PM

pouria edited the test plan for this revision. (Show Details)Mar 6 2026, 7:24 PM

Fix minor style.9 issues

Harbormaster completed remote builds in B71234: Diff 173279.Mar 6 2026, 7:48 PM

We have backlight(9) interface since FreeBSD 13

In D55694#1274781, @wulf wrote:

We have backlight(9) interface since FreeBSD 13

I think there's a misunderstanding here.
The identifier named keyboard_backlight in my code refers to one of the System76's ACPI-exposed controls, it's not related to the backlight(9) interface.
I can rename it if that's clearer.

This implementation targets System76 ACPI (as noted in the revision title). System76 exposes several controls in their ACPI tree (e.g., DGPU/IGPU switching, charge thresholds, a vendor-specific airplane mode).
See their ACPI EC code here:
https://github.com/system76/coreboot/blob/05dda8aa05f988f3d38720b46804c9cf2590855a/src/ec/system76/ec/acpi/

In my current patch I handle keyboard color (which is not the job of backlight(9)) and keyboard brightness which is not registered under /dev/backlight on my laptop.
I probably didn't make that clear in the summary.
could you take another look at the code?

Driver looks fairly straightforward. One question, but looking at other drivers after I wrote it, maybe the answer is 'that's just the pattern' which is fine.

sys/dev/acpi_support/acpi_system76.c
248	This is held for a lot longer than I'd think it would need to be. I see that it's likely because it makes the error path simpler. Will this cause problems?

Address @imp comment.

Harbormaster completed remote builds in B71241: Diff 173306.Mar 7 2026, 2:57 PM

Just one question. I'm not familiar with the usual number of reviewer approval in ACPI driver changes here.
Can I commit this after your approval?
This driver simply works on my laptop and I want to go further and implement other functionalities provided by "\\_SB.S76D".

sys/dev/acpi_support/acpi_system76.c
248	No, but you're right. I was refactoring this function multiple times and forgot to reduce the locking duration. Also I removed the unnecessary goto below.

pouria marked an inline comment as done.Mar 7 2026, 3:02 PM

imp accepted this revision.Mar 7 2026, 3:19 PM

This revision is now accepted and ready to land.Mar 7 2026, 3:19 PM

Closed by commit rGcdad55809ef5: acpi_system76: Support for acpi-controlled buttons on System76 (authored by pouria). · Explain WhyMar 7 2026, 3:28 PM

This revision was automatically updated to reflect the committed changes.

pouria added a commit: rGcdad55809ef5: acpi_system76: Support for acpi-controlled buttons on System76.

In D55694#1274782, @pouria wrote:

could you take another look at the code?

backlight(9) interface is replacement or addition to sysctl interface. It has no relation to ACPI as it is user interface rather than hardware one. See e.g. sys/dev/acpi_support/acpi_asus_wmi.c

Adding of EVDEV interface to notyfy userland software would be good idea too. See sys/dev/acpi_support/acpi_asus_wmi.c again

In D55694#1274835, @wulf wrote:

In D55694#1274782, @pouria wrote:

could you take another look at the code?

backlight(9) interface is replacement or addition to sysctl interface. It has no relation to ACPI as it is user interface rather than hardware one. See e.g. sys/dev/acpi_support/acpi_asus_wmi.c

So you think I should use backlight for keyboard_backlight?
how about keyboard_color?
The keyboard_color has full RGB coverage and accept 24-bit value as its input, so I don't think registering it in backlight be a good idea.

In D55694#1274836, @wulf wrote:

Adding of EVDEV interface to notyfy userland software would be good idea too. See sys/dev/acpi_support/acpi_asus_wmi.c again

Thank you, I'll look into that.

In D55694#1274837, @pouria wrote:

The keyboard_color has full RGB coverage and accept 24-bit value as its input, so I don't think registering it in backlight be a good idea.

No one requires you to drop sysctl interface for the sake of backlight. Just expose hw.acpi.s76.keyboard_backlight sysctl through backlight(9) as well. This greatly helps userland software to find right backlight with scandir of /dev/backlight

It would be good to add RGB color support to backlight(9) itself, but that is other story.

In D55694#1274838, @wulf wrote:

In D55694#1274837, @pouria wrote:

The keyboard_color has full RGB coverage and accept 24-bit value as its input, so I don't think registering it in backlight be a good idea.

No one requires you to drop sysctl interface for the sake of backlight. Just expose hw.acpi.s76.keyboard_backlight sysctl through backlight(9) as well. This greatly helps userland software to find right backlight with scandir of /dev/backlight

It would be good to add RGB color support to backlight(9) itself, but that is other story.