rc: add a backlight service to save/restore backlight levels
ClosedPublic
Actions

Authored by kevans on Tue, Sep 1, 3:41 AM.

Details

Reviewers

bapt
manu
ivy
ziaee

Commits

rGc45bdb39fe98: rc: add a backlight service to save/restore backlight levels

Summary

The default on my laptop is annoyingly bright, and this is a useful feature to
mitigate that. The backlight script is largely a copy of the mixer
service which provides the same value for mixers, but this one is
specifically dependant on kld to allow DRM drivers a chance to attach.

Relnotes: maybe

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 76374
Build 73257: arc lint + arc unit

Event Timeline

kevans created this revision.Tue, Sep 1, 3:41 AM

Herald added a subscriber: imp. · View Herald TranscriptTue, Sep 1, 3:41 AM

kevans requested review of this revision.Tue, Sep 1, 3:41 AM

Harbormaster completed remote builds in B76365: Diff 185531.Tue, Sep 1, 3:41 AM

kevans edited the summary of this revision. (Show Details)Tue, Sep 1, 4:28 AM

bapt added inline comments.Tue, Sep 1, 6:58 AM

libexec/rc/rc.d/backlight
4	this is weird, I bet this comes from copy/paste from rc.d/mixer, but you can probably right your own here (and use SPDX)
31	You probably want required kld not provide kld

arrowd added a subscriber: arrowd.Tue, Sep 1, 9:53 AM

arrowd added inline comments.

libexec/rc/rc.conf
45	Enabling this by default will clash with DEs doing their own backlight management. But unless user call `save` manually, I think it should really get in the way.
libexec/rc/rc.d/backlight
73	I'd prefer `/var/db/backlight/...` or `/var/db/backlight-${1}-state`

kevans added inline comments.Tue, Sep 1, 12:56 PM

libexec/rc/rc.conf
45	I'm cool with off-by-default, I'll just want to amend backlight(8) to mention that the cpability exists for th rest of us.
libexec/rc/rc.d/backlight
31	sigh EDOOFUS at the last minute
73	The naming here forces them to be `/var/db/backlightN-state` because I wanted to explicitly ignore aliases up in `list_backlights`. That said, I have no objection to `/var/db/backlight` and I've somewhat wished that mixer did the same.

Review feedback:

Scope the backlight settings off into /var/db/backlight
Turn it off by default, mention it in the manpage
Condense the license/copyright header down and fix the PROVIDE/REQUIRE
Use my preferred quoting style while we're here

Herald added subscribers: ivy, ziaee. · View Herald TranscriptTue, Sep 1, 1:47 PM

Harbormaster completed remote builds in B76374: Diff 185558.Tue, Sep 1, 1:47 PM

Thanks, my annoying 30% default backlight level will soon be history.

This revision is now accepted and ready to land.Tue, Sep 1, 1:49 PM

looks fine from the pkgbase side.

bapt accepted this revision.Tue, Sep 1, 1:53 PM

Can you put this in rc.conf.5 too?

In D59296#1360620, @ziaee wrote:

Can you put this in rc.conf.5 too?

How about:

diff --git a/share/man/man5/rc.conf.5 b/share/man/man5/rc.conf.5
index 586e33980fd8..4db1586f2fcc 100644
--- a/share/man/man5/rc.conf.5
+++ b/share/man/man5/rc.conf.5
@@ -22,7 +22,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd August 30, 2026
+.Dd September 1, 2026
 .Dt RC.CONF 5
 .Os
 .Sh NAME
@@ -4433,6 +4433,11 @@ For example:
 autobridge_interfaces="bridge0"
 autobridge_bridge0="tap* dc0 vlan[345]"
 .Ed
+.It Va backlight_enable
+.Pq Vt bool
+If set to
+.Dq Li YES ,
+enable saving and restoring backlight levels at shutdown and boot time.
 .It Va mixer_enable
 .Pq Vt bool
 If set to