Most, if not all, somewhat recent Thinkpads have some subtle issues
with display brightness control.  For example,if you change the
display brightness using wsconsctl(8) or cbacklight(1), and later use
the brightness control buttons on the keyboard, you're likely to see a
big jump in brightness.  or if you plug or unplug the power, the
display brightness will suddenly change.  The problem here is that on
these machines we us the display brightness interface provided by
inteldrm(4), which doesn't coordinate changes with the firmware.

In principle these machines support the standard acpi brightness
control interface.  However that interface is quite badly broken if
the OS claims to be Windows 8.  Unfortunately that is what we do on
OpenBSD because otherwise some other machines don't work properly.

Fortunately Thinkpads provide an alternative acpi interface through
the "hotkey" device that is handled by acpithinkpad(4).  The diff
below implements that interface.

The downside of this diff is that number of levels is limited to 16
whereas we currently have much finer granularity.  But I think that is
acceptable.  The levels are probably better calibrated and we now have
proper coordination between the OS and the firmware when it comes to
brightness changes.  On top of that, this diff will almost certainly
result in working brightness control on machines that don't have Intel
graphics.

ok?


Index: acpithinkpad.c
===================================================================
RCS file: /home/cvs/src/sys/dev/acpi/acpithinkpad.c,v
retrieving revision 1.49
diff -u -p -r1.49 acpithinkpad.c
--- acpithinkpad.c      16 Dec 2015 15:43:14 -0000      1.49
+++ acpithinkpad.c      16 Dec 2015 18:00:03 -0000
@@ -122,6 +122,8 @@ struct acpithinkpad_softc {
        uint64_t                 sc_thinklight;
        const char              *sc_thinklight_get;
        const char              *sc_thinklight_set;
+
+       uint64_t                 sc_brightness;
 };
 
 extern void acpiec_read(struct acpiec_softc *, u_int8_t, int, u_int8_t *);
@@ -141,13 +143,19 @@ int       thinkpad_brightness_down(struct acpi
 int    thinkpad_adaptive_change(struct acpithinkpad_softc *);
 int    thinkpad_activate(struct device *, int);
 
-/* wskbd hook functions */
+/* 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 *);
 extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
 extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
+void   thinkpad_get_brightness(struct acpithinkpad_softc *);
+void   thinkpad_set_brightness(void *, int);
+int    thinkpad_get_param(struct wsdisplay_param *);
+int    thinkpad_set_param(struct wsdisplay_param *);
+extern int (*ws_get_param)(struct wsdisplay_param *);
+extern int (*ws_set_param)(struct wsdisplay_param *);
 
 void    thinkpad_sensor_attach(struct acpithinkpad_softc *sc);
 void    thinkpad_sensor_refresh(void *);
@@ -267,6 +275,12 @@ thinkpad_attach(struct device *parent, s
                wskbd_set_backlight = thinkpad_set_backlight;
        }
 
+       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;
+       }
+
        /* Run thinkpad_hotkey on button presses */
        aml_register_notify(sc->sc_devnode, aa->aaa_dev,
            thinkpad_hotkey, sc, ACPIDEV_POLL);
@@ -391,6 +405,9 @@ thinkpad_hotkey(struct aml_node *node, i
                        thinkpad_adaptive_change(sc);
                        handled = 1;
                        break;
+               case THINKPAD_BACKLIGHT_CHANGED:
+                       thinkpad_get_brightness(sc);
+                       break;
                case THINKPAD_ADAPTIVE_BACK:
                case THINKPAD_ADAPTIVE_GESTURES:
                case THINKPAD_ADAPTIVE_REFRESH:
@@ -398,7 +415,6 @@ thinkpad_hotkey(struct aml_node *node, i
                case THINKPAD_ADAPTIVE_SNIP:
                case THINKPAD_ADAPTIVE_TAB:
                case THINKPAD_ADAPTIVE_VOICE:
-               case THINKPAD_BACKLIGHT_CHANGED:
                case THINKPAD_KEYLIGHT_CHANGED:
                case THINKPAD_BRIGHTNESS_CHANGED:
                case THINKPAD_BUTTON_BATTERY_INFO:
@@ -630,4 +646,63 @@ thinkpad_set_backlight(struct wskbd_back
        acpi_addtask(sc->sc_acpi, thinkpad_set_thinklight, sc, 0);
        acpi_wakeup(sc->sc_acpi);
        return 0;
+}
+
+void
+thinkpad_get_brightness(struct acpithinkpad_softc *sc)
+{
+       aml_evalinteger(sc->sc_acpi, sc->sc_devnode,
+           "PBLG", 0, NULL, &sc->sc_brightness);
+}
+
+void
+thinkpad_set_brightness(void *arg0, int arg1)
+{
+       struct acpithinkpad_softc *sc = arg0;
+       struct aml_value arg;
+
+       memset(&arg, 0, sizeof(arg));
+       arg.type = AML_OBJTYPE_INTEGER;
+       arg.v_integer = sc->sc_brightness & 0xff;
+       aml_evalname(sc->sc_acpi, sc->sc_devnode,
+           "PBLS", 1, &arg, NULL);
+}
+
+int
+thinkpad_get_param(struct wsdisplay_param *dp)
+{
+       struct acpithinkpad_softc *sc = acpithinkpad_cd.cd_devs[0];
+
+       if (sc == NULL)
+               return -1;
+
+       dp->min = 0;
+       dp->max = (sc->sc_brightness >> 8) & 0xff;
+       dp->curval = sc->sc_brightness & 0xff;
+       return 0;
+}
+
+int
+thinkpad_set_param(struct wsdisplay_param *dp)
+{
+       struct acpithinkpad_softc *sc = acpithinkpad_cd.cd_devs[0];
+       int maxval = (sc->sc_brightness >> 8) & 0xff;
+
+       if (sc == NULL)
+               return -1;
+
+       switch (dp->param) {
+       case WSDISPLAYIO_PARAM_BRIGHTNESS:
+               if (dp->curval < 0)
+                       dp->curval = 0;
+               if (dp->curval > maxval)
+                       dp->curval = maxval;
+               sc->sc_brightness &= ~0xff;
+               sc->sc_brightness |= dp->curval;
+               acpi_addtask(sc->sc_acpi, thinkpad_set_brightness, sc, 0);
+               acpi_wakeup(sc->sc_acpi);
+               return 0;
+       default:
+               return -1;
+       }
 }

Reply via email to