> From: Dave Voutila <d...@sisu.io>
> Date: Fri, 17 Feb 2023 16:30:46 -0500
> 
> joshua stein <j...@jcs.org> writes:
> 
> > Revision 1.67 made acpithinkpad not take over screen backlight
> > control and let inteldrm or some other driver handle it.  This was
> > done to support fine-grained levels of backlight control rather than
> > the dozen steps that the native EC interface supports.
> >
> > Unfortunately this has the drawback of the EC getting confused about
> > what the backlight level actually is and it still tries to change
> > the backlight automatically in response to Fn+F# keys or plug/unplug
> > events.  Notably if you have the laptop plugged in with a backlight
> > value of something high, then unplug it and later lower it with
> > xbacklight, when you plug it back in, the EC automatically adjusts
> > the backlight up to that high value.
> >
> > I don't know how to tell the EC to stop adjusting the backlight
> > automatically on power changes, so I would like to revert this
> > change and go back to only having native EC-controlled backlight
> > with xbacklight or wsconsctl.
> >
> 
> Odd...this backout makes my X1 Carbon gen10 lost the ability to dim the
> screen to the point of being off. xbacklight and wsconsctl also don't
> stay in sync.
> 
> Without the backout (i.e. current tree), it behaves perfectly fine with
> both xbacklight and wsconsctl staying in sync and being able to go fully
> off at 0.0%.
> 
> This machine reports v2.0 btw:
> 
>  acpithinkpad0 at acpi0: version 2.0

Yes, the firmware-based backlight control stuff is broken on many of
the more recent ThinkPad models.  And I believe it is no longer used
by modern versions Windows.  The limited number of levels supported by
the firmware-based method certainly wasn't the most important reason
to stop using it.

I don't think this backout should go in.

> > patrick@ confirmed that he can still change backlight values on the
> > ThinkPad X395 with this backed out, even though part of the reason
> > for this initial change was to help that machine.
> >
> >
> > Index: acpithinkpad.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
> > retrieving revision 1.70
> > diff -u -p -u -p -r1.70 acpithinkpad.c
> > --- acpithinkpad.c  6 Apr 2022 18:59:27 -0000       1.70
> > +++ acpithinkpad.c  17 Feb 2023 14:41:26 -0000
> > @@ -321,10 +321,8 @@ thinkpad_attach(struct device *parent, s
> >             wskbd_set_backlight = thinkpad_set_kbd_backlight;
> >     }
> >
> > -   /* On version 2 and newer, let *drm or acpivout control brightness */
> > -   if (sc->sc_hkey_version == THINKPAD_HKEY_VERSION1 &&
> > -       (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG",
> > -       0, NULL, &sc->sc_brightness) == 0)) {
> > +   if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG",
> > +       0, NULL, &sc->sc_brightness) == 0) {
> >             ws_get_param = thinkpad_get_param;
> >             ws_set_param = thinkpad_set_param;
> >     }
> 
> 

Reply via email to