Re: acpithinkpad: fix brightness keys, keyboard backlight value
On Mon, Mar 11, 2019 at 4:33 AM Edd Barrett wrote: > On Wed, Mar 06, 2019 at 09:37:52PM +0100, Juan Francisco Cantero Hurtado > wrote: > > The brightness keys on the X61s still work fine. > > I've just built today's kernel on my X1 5th gen, and the backlight keys > now function. > > Many thanks jcs@! > > -- > Best Regards > Edd Barrett > > http://www.theunixzoo.co.uk > > Brightness key now work of Thinkpad L460, thanks!
Re: acpithinkpad: fix brightness keys, keyboard backlight value
On Wed, Mar 06, 2019 at 09:37:52PM +0100, Juan Francisco Cantero Hurtado wrote: > The brightness keys on the X61s still work fine. I've just built today's kernel on my X1 5th gen, and the backlight keys now function. Many thanks jcs@! -- Best Regards Edd Barrett http://www.theunixzoo.co.uk
Re: acpithinkpad: fix brightness keys, keyboard backlight value
On Tue, Mar 05, 2019 at 02:03:13PM -0600, joshua stein wrote: > Here we go again... > > On at least the ThinkPad X1C6, the screen brightness keys (F5 and > F6) do not work and "wsconsctl keyboard.backlight" doesn't report > the correct value when the keyboard backlight is adjusted with > Fn+Space. > > These are both caused by the default event mask not including these > events, so explicitly enable them. > > But then acpithinkpad has to actually do something for the screen > brightness keys, but it tries the very old CMOS method which doesn't > work on these newer machines[0]. So make it use the ACPI method. > > I renamed thinkpad_[gs]et_backlight to thinkpad_[gs]et_kbd_backlight > because it was confusing that they have nothing to do with screen > backlight. > > > 0. "newer machines" being those with MHKV reporting version 2. If > this diff breaks on older "newer machines", this metric will have to > be changed to something else. The brightness keys on the X61s still work fine. -- Juan Francisco Cantero Hurtado http://juanfra.info
Re: acpithinkpad: fix brightness keys, keyboard backlight value
On Tue, Mar 05, 2019 at 02:03:13PM -0600, joshua stein wrote: > 0. "newer machines" being those with MHKV reporting version 2. If > this diff breaks on older "newer machines", this metric will have to > be changed to something else. Works fine here on x201 and x250.
Re: acpithinkpad: fix brightness keys, keyboard backlight value
On Tue, Mar 05, 2019 at 02:03:13PM -0600, joshua stein wrote:
> Here we go again...
>
> On at least the ThinkPad X1C6, the screen brightness keys (F5 and
> F6) do not work and "wsconsctl keyboard.backlight" doesn't report
> the correct value when the keyboard backlight is adjusted with
> Fn+Space.
>
> These are both caused by the default event mask not including these
> events, so explicitly enable them.
>
> But then acpithinkpad has to actually do something for the screen
> brightness keys, but it tries the very old CMOS method which doesn't
> work on these newer machines[0]. So make it use the ACPI method.
>
> I renamed thinkpad_[gs]et_backlight to thinkpad_[gs]et_kbd_backlight
> because it was confusing that they have nothing to do with screen
> backlight.
>
>
> 0. "newer machines" being those with MHKV reporting version 2. If
> this diff breaks on older "newer machines", this metric will have to
> be changed to something else.
>
Tested on an x240 and x270. Both work fine (actually fixes the x270).
Diff reads ok to me. OK claudio@
> Index: sys/dev/acpi/acpithinkpad.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
> retrieving revision 1.61
> diff -u -p -u -p -r1.61 acpithinkpad.c
> --- sys/dev/acpi/acpithinkpad.c 1 Jul 2018 19:40:49 - 1.61
> +++ sys/dev/acpi/acpithinkpad.c 5 Mar 2019 20:00:23 -
> @@ -124,6 +124,10 @@
> #define THINKPAD_ADAPTIVE_MODE_HOME 1
> #define THINKPAD_ADAPTIVE_MODE_FUNCTION 3
>
> +#define THINKPAD_MASK_BRIGHTNESS_UP (1 << 15)
> +#define THINKPAD_MASK_BRIGHTNESS_DOWN(1 << 16)
> +#define THINKPAD_MASK_KBD_BACKLIGHT (1 << 17)
> +
> struct acpithinkpad_softc {
> struct devicesc_dev;
>
> @@ -134,6 +138,8 @@ struct acpithinkpad_softc {
> struct ksensor sc_sens[THINKPAD_NSENSORS];
> struct ksensordevsc_sensdev;
>
> + uint64_t sc_hkey_version;
> +
> uint64_t sc_thinklight;
> const char *sc_thinklight_get;
> const char *sc_thinklight_set;
> @@ -161,8 +167,8 @@ int thinkpad_activate(struct device *, i
> /* wscons hook functions */
> void thinkpad_get_thinklight(struct acpithinkpad_softc *);
> void thinkpad_set_thinklight(void *, int);
> -int thinkpad_get_backlight(struct wskbd_backlight *);
> -int thinkpad_set_backlight(struct wskbd_backlight *);
> +int thinkpad_get_kbd_backlight(struct wskbd_backlight *);
> +int thinkpad_set_kbd_backlight(struct wskbd_backlight *);
> extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
> extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
> void thinkpad_get_brightness(struct acpithinkpad_softc *);
> @@ -284,6 +290,10 @@ thinkpad_attach(struct device *parent, s
>
> printf("\n");
>
> + if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MHKV", 0, NULL,
> + >sc_hkey_version))
> + sc->sc_hkey_version = THINKPAD_HKEY_VERSION1;
> +
> #if NAUDIO > 0 && NWSKBD > 0
> /* Defer speaker mute */
> if (thinkpad_get_volume_mute(sc) == 1)
> @@ -299,14 +309,14 @@ thinkpad_attach(struct device *parent, s
> 0, NULL, >sc_thinklight) == 0) {
> sc->sc_thinklight_get = "KLCG";
> sc->sc_thinklight_set = "KLCS";
> - wskbd_get_backlight = thinkpad_get_backlight;
> - wskbd_set_backlight = thinkpad_set_backlight;
> + wskbd_get_backlight = thinkpad_get_kbd_backlight;
> + wskbd_set_backlight = thinkpad_set_kbd_backlight;
> } else if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MLCG",
> 0, NULL, >sc_thinklight) == 0) {
> sc->sc_thinklight_get = "MLCG";
> sc->sc_thinklight_set = "MLCS";
> - wskbd_get_backlight = thinkpad_get_backlight;
> - wskbd_set_backlight = thinkpad_set_backlight;
> + wskbd_get_backlight = thinkpad_get_kbd_backlight;
> + wskbd_set_backlight = thinkpad_set_kbd_backlight;
> }
>
> if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG",
> @@ -327,13 +337,19 @@ thinkpad_enable_events(struct acpithinkp
> int64_t mask;
> int i;
>
> - /* Get the supported event mask */
> + /* Get the default event mask */
> if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MHKA",
> 0, NULL, )) {
> printf("%s: no MHKA\n", DEVNAME(sc));
> return (1);
> }
>
> + /* Enable events we need to know about */
> + mask |= (THINKPAD_MASK_BRIGHTNESS_UP | THINKPAD_MASK_BRIGHTNESS_DOWN |
> + THINKPAD_MASK_KBD_BACKLIGHT);
> +
> + DPRINTF(("%s: setting event mask to 0x%llx\n", DEVNAME(sc), mask));
> +
> /* Update hotkey mask */
> bzero(args, sizeof(args));
> args[0].type = args[1].type = AML_OBJTYPE_INTEGER;
> @@ -380,6 +396,8 @@
Re: acpithinkpad: fix brightness keys, keyboard backlight value
Hi Joshua,
I just tried your patch on T470p and X230.
On T470p, display brightness keys are now working and keyboard
backlight is reported correctly by wsconsctl.
On X230, display brightness keys are still working fine.
Regards,
On Tue, Mar 05 2019, joshua stein wrote:
Here we go again...
On at least the ThinkPad X1C6, the screen brightness keys (F5
and
F6) do not work and "wsconsctl keyboard.backlight" doesn't
report
the correct value when the keyboard backlight is adjusted with
Fn+Space.
These are both caused by the default event mask not including
these
events, so explicitly enable them.
But then acpithinkpad has to actually do something for the
screen
brightness keys, but it tries the very old CMOS method which
doesn't
work on these newer machines[0]. So make it use the ACPI
method.
I renamed thinkpad_[gs]et_backlight to
thinkpad_[gs]et_kbd_backlight
because it was confusing that they have nothing to do with
screen
backlight.
0. "newer machines" being those with MHKV reporting version 2.
If
this diff breaks on older "newer machines", this metric will
have to
be changed to something else.
Index: sys/dev/acpi/acpithinkpad.c
===
RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
retrieving revision 1.61
diff -u -p -u -p -r1.61 acpithinkpad.c
--- sys/dev/acpi/acpithinkpad.c 1 Jul 2018 19:40:49 -
1.61
+++ sys/dev/acpi/acpithinkpad.c 5 Mar 2019 20:00:23 -
@@ -124,6 +124,10 @@
#defineTHINKPAD_ADAPTIVE_MODE_HOME 1
#defineTHINKPAD_ADAPTIVE_MODE_FUNCTION 3
+#define THINKPAD_MASK_BRIGHTNESS_UP (1 << 15)
+#define THINKPAD_MASK_BRIGHTNESS_DOWN (1 << 16)
+#define THINKPAD_MASK_KBD_BACKLIGHT(1 << 17)
+
struct acpithinkpad_softc {
struct devicesc_dev;
@@ -134,6 +138,8 @@ struct acpithinkpad_softc {
struct ksensor sc_sens[THINKPAD_NSENSORS];
struct ksensordevsc_sensdev;
+ uint64_t sc_hkey_version;
+
uint64_t sc_thinklight;
const char *sc_thinklight_get;
const char *sc_thinklight_set;
@@ -161,8 +167,8 @@ int thinkpad_activate(struct device *,
i
/* wscons hook functions */
void thinkpad_get_thinklight(struct acpithinkpad_softc *);
void thinkpad_set_thinklight(void *, int);
-intthinkpad_get_backlight(struct wskbd_backlight *);
-intthinkpad_set_backlight(struct wskbd_backlight *);
+intthinkpad_get_kbd_backlight(struct wskbd_backlight *);
+intthinkpad_set_kbd_backlight(struct wskbd_backlight *);
extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
void thinkpad_get_brightness(struct acpithinkpad_softc *);
@@ -284,6 +290,10 @@ thinkpad_attach(struct device *parent, s
printf("\n");
+ if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MHKV",
0, NULL,
+ >sc_hkey_version))
+ sc->sc_hkey_version = THINKPAD_HKEY_VERSION1;
+
#if NAUDIO > 0 && NWSKBD > 0
/* Defer speaker mute */
if (thinkpad_get_volume_mute(sc) == 1)
@@ -299,14 +309,14 @@ thinkpad_attach(struct device *parent, s
0, NULL, >sc_thinklight) == 0) {
sc->sc_thinklight_get = "KLCG";
sc->sc_thinklight_set = "KLCS";
- wskbd_get_backlight = thinkpad_get_backlight;
- wskbd_set_backlight = thinkpad_set_backlight;
+ wskbd_get_backlight = thinkpad_get_kbd_backlight;
+ wskbd_set_backlight = thinkpad_set_kbd_backlight;
} else if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode,
"MLCG",
0, NULL, >sc_thinklight) == 0) {
sc->sc_thinklight_get = "MLCG";
sc->sc_thinklight_set = "MLCS";
- wskbd_get_backlight = thinkpad_get_backlight;
- wskbd_set_backlight = thinkpad_set_backlight;
+ wskbd_get_backlight = thinkpad_get_kbd_backlight;
+ wskbd_set_backlight = thinkpad_set_kbd_backlight;
}
if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG",
@@ -327,13 +337,19 @@ thinkpad_enable_events(struct acpithinkp
int64_t mask;
int i;
- /* Get the supported event mask */
+ /* Get the default event mask */
if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MHKA",
0, NULL, )) {
printf("%s: no MHKA\n", DEVNAME(sc));
return (1);
}
+ /* Enable events we need to know about */
+ mask |= (THINKPAD_MASK_BRIGHTNESS_UP |
THINKPAD_MASK_BRIGHTNESS_DOWN |
+ THINKPAD_MASK_KBD_BACKLIGHT);
+
+ DPRINTF(("%s: setting event mask to 0x%llx\n",
DEVNAME(sc), mask));
+
/* Update hotkey mask */
bzero(args, sizeof(args));
args[0].type = args[1].type = AML_OBJTYPE_INTEGER;
@@ -380,6 +396,8 @@ thinkpad_hotkey(struct aml_node *node, i
if (aml_evalinteger(sc->sc_acpi,
Re: acpithinkpad: fix brightness keys, keyboard backlight value
On Tue, Mar 05, 2019 at 02:03:13PM -0600, joshua stein wrote:
> Here we go again...
>
> On at least the ThinkPad X1C6, the screen brightness keys (F5 and
> F6) do not work and "wsconsctl keyboard.backlight" doesn't report
> the correct value when the keyboard backlight is adjusted with
> Fn+Space.
>
> These are both caused by the default event mask not including these
> events, so explicitly enable them.
>
> But then acpithinkpad has to actually do something for the screen
> brightness keys, but it tries the very old CMOS method which doesn't
> work on these newer machines[0]. So make it use the ACPI method.
>
> I renamed thinkpad_[gs]et_backlight to thinkpad_[gs]et_kbd_backlight
> because it was confusing that they have nothing to do with screen
> backlight.
>
>
> 0. "newer machines" being those with MHKV reporting version 2. If
> this diff breaks on older "newer machines", this metric will have to
> be changed to something else.
>
>
This patch fixes the backlight buttons and keyboard brightness reading on the
T740s. Awesome!
Thanks,
Tracey
> Index: sys/dev/acpi/acpithinkpad.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
> retrieving revision 1.61
> diff -u -p -u -p -r1.61 acpithinkpad.c
> --- sys/dev/acpi/acpithinkpad.c 1 Jul 2018 19:40:49 - 1.61
> +++ sys/dev/acpi/acpithinkpad.c 5 Mar 2019 20:00:23 -
> @@ -124,6 +124,10 @@
> #define THINKPAD_ADAPTIVE_MODE_HOME 1
> #define THINKPAD_ADAPTIVE_MODE_FUNCTION 3
>
> +#define THINKPAD_MASK_BRIGHTNESS_UP (1 << 15)
> +#define THINKPAD_MASK_BRIGHTNESS_DOWN(1 << 16)
> +#define THINKPAD_MASK_KBD_BACKLIGHT (1 << 17)
> +
> struct acpithinkpad_softc {
> struct devicesc_dev;
>
> @@ -134,6 +138,8 @@ struct acpithinkpad_softc {
> struct ksensor sc_sens[THINKPAD_NSENSORS];
> struct ksensordevsc_sensdev;
>
> + uint64_t sc_hkey_version;
> +
> uint64_t sc_thinklight;
> const char *sc_thinklight_get;
> const char *sc_thinklight_set;
> @@ -161,8 +167,8 @@ int thinkpad_activate(struct device *, i
> /* wscons hook functions */
> void thinkpad_get_thinklight(struct acpithinkpad_softc *);
> void thinkpad_set_thinklight(void *, int);
> -int thinkpad_get_backlight(struct wskbd_backlight *);
> -int thinkpad_set_backlight(struct wskbd_backlight *);
> +int thinkpad_get_kbd_backlight(struct wskbd_backlight *);
> +int thinkpad_set_kbd_backlight(struct wskbd_backlight *);
> extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
> extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
> void thinkpad_get_brightness(struct acpithinkpad_softc *);
> @@ -284,6 +290,10 @@ thinkpad_attach(struct device *parent, s
>
> printf("\n");
>
> + if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MHKV", 0, NULL,
> + >sc_hkey_version))
> + sc->sc_hkey_version = THINKPAD_HKEY_VERSION1;
> +
> #if NAUDIO > 0 && NWSKBD > 0
> /* Defer speaker mute */
> if (thinkpad_get_volume_mute(sc) == 1)
> @@ -299,14 +309,14 @@ thinkpad_attach(struct device *parent, s
> 0, NULL, >sc_thinklight) == 0) {
> sc->sc_thinklight_get = "KLCG";
> sc->sc_thinklight_set = "KLCS";
> - wskbd_get_backlight = thinkpad_get_backlight;
> - wskbd_set_backlight = thinkpad_set_backlight;
> + wskbd_get_backlight = thinkpad_get_kbd_backlight;
> + wskbd_set_backlight = thinkpad_set_kbd_backlight;
> } else if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MLCG",
> 0, NULL, >sc_thinklight) == 0) {
> sc->sc_thinklight_get = "MLCG";
> sc->sc_thinklight_set = "MLCS";
> - wskbd_get_backlight = thinkpad_get_backlight;
> - wskbd_set_backlight = thinkpad_set_backlight;
> + wskbd_get_backlight = thinkpad_get_kbd_backlight;
> + wskbd_set_backlight = thinkpad_set_kbd_backlight;
> }
>
> if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG",
> @@ -327,13 +337,19 @@ thinkpad_enable_events(struct acpithinkp
> int64_t mask;
> int i;
>
> - /* Get the supported event mask */
> + /* Get the default event mask */
> if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MHKA",
> 0, NULL, )) {
> printf("%s: no MHKA\n", DEVNAME(sc));
> return (1);
> }
>
> + /* Enable events we need to know about */
> + mask |= (THINKPAD_MASK_BRIGHTNESS_UP | THINKPAD_MASK_BRIGHTNESS_DOWN |
> + THINKPAD_MASK_KBD_BACKLIGHT);
> +
> + DPRINTF(("%s: setting event mask to 0x%llx\n", DEVNAME(sc), mask));
> +
> /* Update hotkey mask */
> bzero(args, sizeof(args));
> args[0].type = args[1].type = AML_OBJTYPE_INTEGER;
> @@ -380,6 +396,8 @@
Re: acpithinkpad: fix brightness keys, keyboard backlight value
joshua stein(j...@openbsd.org) on 2019.03.05 14:03:13 -0600:
> Here we go again...
>
> On at least the ThinkPad X1C6, the screen brightness keys (F5 and
> F6) do not work and "wsconsctl keyboard.backlight" doesn't report
> the correct value when the keyboard backlight is adjusted with
> Fn+Space.
>
> These are both caused by the default event mask not including these
> events, so explicitly enable them.
>
> But then acpithinkpad has to actually do something for the screen
> brightness keys, but it tries the very old CMOS method which doesn't
> work on these newer machines[0]. So make it use the ACPI method.
>
> I renamed thinkpad_[gs]et_backlight to thinkpad_[gs]et_kbd_backlight
> because it was confusing that they have nothing to do with screen
> backlight.
>
>
> 0. "newer machines" being those with MHKV reporting version 2. If
> this diff breaks on older "newer machines", this metric will have to
> be changed to something else.
with this the Fn-F5/F6 buttons work on the x270, and it makes
wsconsctl keyboard.backlight show the current setting and wsconsctl
keyboard.backlight=0/50/100 set the keyboard light.
On the x230, the F8/9 brightness keys still work (and the keyboard led is
different of course)
/Benno
> Index: sys/dev/acpi/acpithinkpad.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
> retrieving revision 1.61
> diff -u -p -u -p -r1.61 acpithinkpad.c
> --- sys/dev/acpi/acpithinkpad.c 1 Jul 2018 19:40:49 - 1.61
> +++ sys/dev/acpi/acpithinkpad.c 5 Mar 2019 20:00:23 -
> @@ -124,6 +124,10 @@
> #define THINKPAD_ADAPTIVE_MODE_HOME 1
> #define THINKPAD_ADAPTIVE_MODE_FUNCTION 3
>
> +#define THINKPAD_MASK_BRIGHTNESS_UP (1 << 15)
> +#define THINKPAD_MASK_BRIGHTNESS_DOWN(1 << 16)
> +#define THINKPAD_MASK_KBD_BACKLIGHT (1 << 17)
> +
> struct acpithinkpad_softc {
> struct devicesc_dev;
>
> @@ -134,6 +138,8 @@ struct acpithinkpad_softc {
> struct ksensor sc_sens[THINKPAD_NSENSORS];
> struct ksensordevsc_sensdev;
>
> + uint64_t sc_hkey_version;
> +
> uint64_t sc_thinklight;
> const char *sc_thinklight_get;
> const char *sc_thinklight_set;
> @@ -161,8 +167,8 @@ int thinkpad_activate(struct device *, i
> /* wscons hook functions */
> void thinkpad_get_thinklight(struct acpithinkpad_softc *);
> void thinkpad_set_thinklight(void *, int);
> -int thinkpad_get_backlight(struct wskbd_backlight *);
> -int thinkpad_set_backlight(struct wskbd_backlight *);
> +int thinkpad_get_kbd_backlight(struct wskbd_backlight *);
> +int thinkpad_set_kbd_backlight(struct wskbd_backlight *);
> extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
> extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
> void thinkpad_get_brightness(struct acpithinkpad_softc *);
> @@ -284,6 +290,10 @@ thinkpad_attach(struct device *parent, s
>
> printf("\n");
>
> + if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MHKV", 0, NULL,
> + >sc_hkey_version))
> + sc->sc_hkey_version = THINKPAD_HKEY_VERSION1;
> +
> #if NAUDIO > 0 && NWSKBD > 0
> /* Defer speaker mute */
> if (thinkpad_get_volume_mute(sc) == 1)
> @@ -299,14 +309,14 @@ thinkpad_attach(struct device *parent, s
> 0, NULL, >sc_thinklight) == 0) {
> sc->sc_thinklight_get = "KLCG";
> sc->sc_thinklight_set = "KLCS";
> - wskbd_get_backlight = thinkpad_get_backlight;
> - wskbd_set_backlight = thinkpad_set_backlight;
> + wskbd_get_backlight = thinkpad_get_kbd_backlight;
> + wskbd_set_backlight = thinkpad_set_kbd_backlight;
> } else if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MLCG",
> 0, NULL, >sc_thinklight) == 0) {
> sc->sc_thinklight_get = "MLCG";
> sc->sc_thinklight_set = "MLCS";
> - wskbd_get_backlight = thinkpad_get_backlight;
> - wskbd_set_backlight = thinkpad_set_backlight;
> + wskbd_get_backlight = thinkpad_get_kbd_backlight;
> + wskbd_set_backlight = thinkpad_set_kbd_backlight;
> }
>
> if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG",
> @@ -327,13 +337,19 @@ thinkpad_enable_events(struct acpithinkp
> int64_t mask;
> int i;
>
> - /* Get the supported event mask */
> + /* Get the default event mask */
> if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MHKA",
> 0, NULL, )) {
> printf("%s: no MHKA\n", DEVNAME(sc));
> return (1);
> }
>
> + /* Enable events we need to know about */
> + mask |= (THINKPAD_MASK_BRIGHTNESS_UP | THINKPAD_MASK_BRIGHTNESS_DOWN |
> + THINKPAD_MASK_KBD_BACKLIGHT);
> +
> + DPRINTF(("%s: setting event mask to 0x%llx\n", DEVNAME(sc),
Re: acpithinkpad: fix brightness keys, keyboard backlight value
On Tue, Mar 05, 2019 at 02:03:13PM -0600, joshua stein wrote:
> Here we go again...
>
> On at least the ThinkPad X1C6, the screen brightness keys (F5 and
> F6) do not work and "wsconsctl keyboard.backlight" doesn't report
> the correct value when the keyboard backlight is adjusted with
> Fn+Space.
>
> These are both caused by the default event mask not including these
> events, so explicitly enable them.
>
> But then acpithinkpad has to actually do something for the screen
> brightness keys, but it tries the very old CMOS method which doesn't
> work on these newer machines[0]. So make it use the ACPI method.
>
> I renamed thinkpad_[gs]et_backlight to thinkpad_[gs]et_kbd_backlight
> because it was confusing that they have nothing to do with screen
> backlight.
>
>
> 0. "newer machines" being those with MHKV reporting version 2. If
> this diff breaks on older "newer machines", this metric will have to
> be changed to something else.
>
This patch fixes both the brightness buttons and the backlight keyboard
reporting in wsconsctl on my x280.
If this doesn't break older models ok jturner@.
>
> Index: sys/dev/acpi/acpithinkpad.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
> retrieving revision 1.61
> diff -u -p -u -p -r1.61 acpithinkpad.c
> --- sys/dev/acpi/acpithinkpad.c 1 Jul 2018 19:40:49 - 1.61
> +++ sys/dev/acpi/acpithinkpad.c 5 Mar 2019 20:00:23 -
> @@ -124,6 +124,10 @@
> #define THINKPAD_ADAPTIVE_MODE_HOME 1
> #define THINKPAD_ADAPTIVE_MODE_FUNCTION 3
>
> +#define THINKPAD_MASK_BRIGHTNESS_UP (1 << 15)
> +#define THINKPAD_MASK_BRIGHTNESS_DOWN(1 << 16)
> +#define THINKPAD_MASK_KBD_BACKLIGHT (1 << 17)
> +
> struct acpithinkpad_softc {
> struct devicesc_dev;
>
> @@ -134,6 +138,8 @@ struct acpithinkpad_softc {
> struct ksensor sc_sens[THINKPAD_NSENSORS];
> struct ksensordevsc_sensdev;
>
> + uint64_t sc_hkey_version;
> +
> uint64_t sc_thinklight;
> const char *sc_thinklight_get;
> const char *sc_thinklight_set;
> @@ -161,8 +167,8 @@ int thinkpad_activate(struct device *, i
> /* wscons hook functions */
> void thinkpad_get_thinklight(struct acpithinkpad_softc *);
> void thinkpad_set_thinklight(void *, int);
> -int thinkpad_get_backlight(struct wskbd_backlight *);
> -int thinkpad_set_backlight(struct wskbd_backlight *);
> +int thinkpad_get_kbd_backlight(struct wskbd_backlight *);
> +int thinkpad_set_kbd_backlight(struct wskbd_backlight *);
> extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
> extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
> void thinkpad_get_brightness(struct acpithinkpad_softc *);
> @@ -284,6 +290,10 @@ thinkpad_attach(struct device *parent, s
>
> printf("\n");
>
> + if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MHKV", 0, NULL,
> + >sc_hkey_version))
> + sc->sc_hkey_version = THINKPAD_HKEY_VERSION1;
> +
> #if NAUDIO > 0 && NWSKBD > 0
> /* Defer speaker mute */
> if (thinkpad_get_volume_mute(sc) == 1)
> @@ -299,14 +309,14 @@ thinkpad_attach(struct device *parent, s
> 0, NULL, >sc_thinklight) == 0) {
> sc->sc_thinklight_get = "KLCG";
> sc->sc_thinklight_set = "KLCS";
> - wskbd_get_backlight = thinkpad_get_backlight;
> - wskbd_set_backlight = thinkpad_set_backlight;
> + wskbd_get_backlight = thinkpad_get_kbd_backlight;
> + wskbd_set_backlight = thinkpad_set_kbd_backlight;
> } else if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MLCG",
> 0, NULL, >sc_thinklight) == 0) {
> sc->sc_thinklight_get = "MLCG";
> sc->sc_thinklight_set = "MLCS";
> - wskbd_get_backlight = thinkpad_get_backlight;
> - wskbd_set_backlight = thinkpad_set_backlight;
> + wskbd_get_backlight = thinkpad_get_kbd_backlight;
> + wskbd_set_backlight = thinkpad_set_kbd_backlight;
> }
>
> if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG",
> @@ -327,13 +337,19 @@ thinkpad_enable_events(struct acpithinkp
> int64_t mask;
> int i;
>
> - /* Get the supported event mask */
> + /* Get the default event mask */
> if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MHKA",
> 0, NULL, )) {
> printf("%s: no MHKA\n", DEVNAME(sc));
> return (1);
> }
>
> + /* Enable events we need to know about */
> + mask |= (THINKPAD_MASK_BRIGHTNESS_UP | THINKPAD_MASK_BRIGHTNESS_DOWN |
> + THINKPAD_MASK_KBD_BACKLIGHT);
> +
> + DPRINTF(("%s: setting event mask to 0x%llx\n", DEVNAME(sc), mask));
> +
> /* Update hotkey mask */
> bzero(args, sizeof(args));
> args[0].type = args[1].type =
acpithinkpad: fix brightness keys, keyboard backlight value
Here we go again...
On at least the ThinkPad X1C6, the screen brightness keys (F5 and
F6) do not work and "wsconsctl keyboard.backlight" doesn't report
the correct value when the keyboard backlight is adjusted with
Fn+Space.
These are both caused by the default event mask not including these
events, so explicitly enable them.
But then acpithinkpad has to actually do something for the screen
brightness keys, but it tries the very old CMOS method which doesn't
work on these newer machines[0]. So make it use the ACPI method.
I renamed thinkpad_[gs]et_backlight to thinkpad_[gs]et_kbd_backlight
because it was confusing that they have nothing to do with screen
backlight.
0. "newer machines" being those with MHKV reporting version 2. If
this diff breaks on older "newer machines", this metric will have to
be changed to something else.
Index: sys/dev/acpi/acpithinkpad.c
===
RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
retrieving revision 1.61
diff -u -p -u -p -r1.61 acpithinkpad.c
--- sys/dev/acpi/acpithinkpad.c 1 Jul 2018 19:40:49 - 1.61
+++ sys/dev/acpi/acpithinkpad.c 5 Mar 2019 20:00:23 -
@@ -124,6 +124,10 @@
#defineTHINKPAD_ADAPTIVE_MODE_HOME 1
#defineTHINKPAD_ADAPTIVE_MODE_FUNCTION 3
+#define THINKPAD_MASK_BRIGHTNESS_UP(1 << 15)
+#define THINKPAD_MASK_BRIGHTNESS_DOWN (1 << 16)
+#define THINKPAD_MASK_KBD_BACKLIGHT(1 << 17)
+
struct acpithinkpad_softc {
struct devicesc_dev;
@@ -134,6 +138,8 @@ struct acpithinkpad_softc {
struct ksensor sc_sens[THINKPAD_NSENSORS];
struct ksensordevsc_sensdev;
+ uint64_t sc_hkey_version;
+
uint64_t sc_thinklight;
const char *sc_thinklight_get;
const char *sc_thinklight_set;
@@ -161,8 +167,8 @@ int thinkpad_activate(struct device *, i
/* wscons hook functions */
void thinkpad_get_thinklight(struct acpithinkpad_softc *);
void thinkpad_set_thinklight(void *, int);
-intthinkpad_get_backlight(struct wskbd_backlight *);
-intthinkpad_set_backlight(struct wskbd_backlight *);
+intthinkpad_get_kbd_backlight(struct wskbd_backlight *);
+intthinkpad_set_kbd_backlight(struct wskbd_backlight *);
extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
void thinkpad_get_brightness(struct acpithinkpad_softc *);
@@ -284,6 +290,10 @@ thinkpad_attach(struct device *parent, s
printf("\n");
+ if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MHKV", 0, NULL,
+ >sc_hkey_version))
+ sc->sc_hkey_version = THINKPAD_HKEY_VERSION1;
+
#if NAUDIO > 0 && NWSKBD > 0
/* Defer speaker mute */
if (thinkpad_get_volume_mute(sc) == 1)
@@ -299,14 +309,14 @@ thinkpad_attach(struct device *parent, s
0, NULL, >sc_thinklight) == 0) {
sc->sc_thinklight_get = "KLCG";
sc->sc_thinklight_set = "KLCS";
- wskbd_get_backlight = thinkpad_get_backlight;
- wskbd_set_backlight = thinkpad_set_backlight;
+ wskbd_get_backlight = thinkpad_get_kbd_backlight;
+ wskbd_set_backlight = thinkpad_set_kbd_backlight;
} else if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MLCG",
0, NULL, >sc_thinklight) == 0) {
sc->sc_thinklight_get = "MLCG";
sc->sc_thinklight_set = "MLCS";
- wskbd_get_backlight = thinkpad_get_backlight;
- wskbd_set_backlight = thinkpad_set_backlight;
+ wskbd_get_backlight = thinkpad_get_kbd_backlight;
+ wskbd_set_backlight = thinkpad_set_kbd_backlight;
}
if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG",
@@ -327,13 +337,19 @@ thinkpad_enable_events(struct acpithinkp
int64_t mask;
int i;
- /* Get the supported event mask */
+ /* Get the default event mask */
if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MHKA",
0, NULL, )) {
printf("%s: no MHKA\n", DEVNAME(sc));
return (1);
}
+ /* Enable events we need to know about */
+ mask |= (THINKPAD_MASK_BRIGHTNESS_UP | THINKPAD_MASK_BRIGHTNESS_DOWN |
+ THINKPAD_MASK_KBD_BACKLIGHT);
+
+ DPRINTF(("%s: setting event mask to 0x%llx\n", DEVNAME(sc), mask));
+
/* Update hotkey mask */
bzero(args, sizeof(args));
args[0].type = args[1].type = AML_OBJTYPE_INTEGER;
@@ -380,6 +396,8 @@ thinkpad_hotkey(struct aml_node *node, i
if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "MHKP",
0, NULL, ))
break;
+
+ DPRINTF(("%s: event 0x%03llx\n", DEVNAME(sc), event));
if (event == 0)
break;
@@
