acpi_system76: Add backlight(9) support for keyboard
ClosedPublic
Actions

Authored by pouria on Sat, Mar 7, 11:03 PM.

Details

Reviewers

wulf
imp

Commits

rG105869a2c78d: acpi_system76: Add backlight(9) support for keyboard

Summary

Add backlight(9) support for keyboard brightness.

Test Plan

To set keyboard backlight via backlight(8)

% backlight -f /dev/backlight/backlight1 84
% sysctl hw.acpi.s76.keyboard_backlight
hw.acpi.s76.keyboard_backlight: 224
% backlight -f /dev/backlight/backlight1 0
% sysctl hw.acpi.s76.keyboard_backlight
hw.acpi.s76.keyboard_backlight: 0
% backlight -f /dev/backlight/backlight1 incr
% backlight -f /dev/backlight/backlight1
brightness: 6
levels: 0 6 12 18 24 30 36 42 48 54 60 66 72 78 84 100
% backlight -f /dev/backlight/backlight1 decr
% backlight -f /dev/backlight/backlight1
brightness: 0
levels: 0 6 12 18 24 30 36 42 48 54 60 66 72 78 84 100

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

pouria requested review of this revision.Sat, Mar 7, 11:03 PM

pouria created this revision.

Harbormaster completed remote builds in B71250: Diff 173319.Sat, Mar 7, 11:03 PM

pouria added a parent revision: D55710: acpi_system76: Add support for battary charge thresholds.Sat, Mar 7, 11:05 PM

wulf added inline comments.Sun, Mar 8, 7:28 AM

sys/dev/acpi_support/acpi_system76.c
470	That is not enough to store only hardware value of brightness. You should keep users i.e. obtained via backlight(9) value too and convert one to another every time it has changed. Imagine that user's software increases brightness in 2 pct steps through backlight(9) interface. It wont work with current code as minimal step is 6 pct.

pouria added inline comments.Sun, Mar 8, 11:14 AM

sys/dev/acpi_support/acpi_system76.c
470	You're right. Just a question. The System76 keyboard uses a PWM mechanism to power its keyboard backlight. It really supports 0 to 255 different levels. I don't like to manually fill the acpi_system76_backlight_levels with values from 0 to 100 since that will fill the stack too so using a macro won't help much neigher. What do you recommend in that regard?

wulf added inline comments.Sun, Mar 8, 11:21 AM

sys/dev/acpi_support/acpi_system76.c
470	IIRC setting of number of levels to zero enables all of them.

Address @wulf comments.
Test:

% backlight -f /dev/backlight/backlight1
brightness: 0
levels: 0 6 12 18 24 30 36 42 48 54 60 66 72 78 84 100
% backlight -f /dev/backlight/backlight1 + 5
% backlight -qf /dev/backlight/backlight1
brightness: 5
% sysctl hw.acpi.s76.keyboard_backlight
hw.acpi.s76.keyboard_backlight: 0
% backlight -f /dev/backlight/backlight1 + 1
% sysctl hw.acpi.s76.keyboard_backlight
hw.acpi.s76.keyboard_backlight: 16
% backlight -f /dev/backlight/backlight1 44
% backlight -qf /dev/backlight/backlight1
brightness: 44
% sysctl hw.acpi.s76.keyboard_backlight
hw.acpi.s76.keyboard_backlight: 112
% sysctl hw.acpi.s76.keyboard_backlight=81
hw.acpi.s76.keyboard_backlight: 112 -> 81
% backlight -qf /dev/backlight/backlight1
brightness: 30

Also add suspend, resume, shutdown method.
suspend and resume depends on this revision.
shutdown is added to avoid panics AcpiNotify after kldunload.

Harbormaster completed remote builds in B71255: Diff 173354.Sun, Mar 8, 1:22 PM

Rebase on D55710

Harbormaster completed remote builds in B71257: Diff 173361.Sun, Mar 8, 1:34 PM

wulf accepted this revision.Sun, Mar 8, 8:29 PM

This revision is now accepted and ready to land.Sun, Mar 8, 8:29 PM

Closed by commit rG105869a2c78d: acpi_system76: Add backlight(9) support for keyboard (authored by pouria). · Explain WhyMon, Mar 9, 9:22 AM

This revision was automatically updated to reflect the committed changes.

pouria added a commit: rG105869a2c78d: acpi_system76: Add backlight(9) support for keyboard.