[diff] lpbl(4): driver for TI LP8550 backlight controller (found in e.g. MacBook Air 6,2)

Discussion:

Sviatoslav Chagaev

2016-01-10 02:59:22 UTC

Hi,

I'm running -current on Apple MacBook Air 6,2. I installed using Jasper's
instructions [1], OpenBSD is the only OS and boots via EFI.

I'm experiencing a problem with LCD backlight: on wakeup from suspend, the
backlight brightness is not restored and becomes unadjustable. If adjusted down,
it turns off, if adjusted up, it goes to 100%.

An Intel developer claims [2] that it's not a bug in Intel KMS driver and people
claim [3] the problem goes away when booting in legacy BIOS mode.

It turns out [4], a numbers of MacBook models [5], including mine, have a Texas
Instruments LP8550 LED backlight controller on I2C bus. By default, it's
controlled by external PWM. But it can be configured to be controlled by it's
own internal register instead.

So below is a driver for TI LP8550.

With this driver, after wake up from suspend, the brightness level is restored
and can be freely adjusted.

The catch is that Intel KMS driver seems to control the chip's ``EN'' pin
(basically an on/off pin). So whenever Intel KMS toggles the backlight, namely
on power management (e.g. `xset dpms force off`) and on Xorg <--> VT switch,
the chip's brightness control register gets reset and you need to
`wsconsctl display.backlight=<non_zero>` to turn the backlight back on.

But it beats rebooting.

If there's any chance for it to be commited, please tell me what I need to
fix/improve to get it commited (so I don't have to reapply it every time I
upgrade).

Next step could be to somehow integrate it with Intel KMS.

[1] https://blog.jasper.la/openbsd-uefi-bootloader-howto/
[2] https://bugs.freedesktop.org/show_bug.cgi?id=67454#c103
[3] https://bugs.freedesktop.org/show_bug.cgi?id=67454#c58
[4] https://bugs.freedesktop.org/show_bug.cgi?id=67454#c78
[5] http://marc.info/?l=openbsd-misc&m=144988400031788&w=2

Index: share/man/man4/iic.4
===================================================================
RCS file: /cvs/src/share/man/man4/iic.4,v
retrieving revision 1.84
diff -u -p -u -p -r1.84 iic.4
--- share/man/man4/iic.4 27 Dec 2014 16:08:03 -0000 1.84
+++ share/man/man4/iic.4 10 Jan 2016 01:52:55 -0000
@@ -153,6 +153,8 @@ National Semiconductor LM87 temperature,
National Semiconductor LM93 temperature, voltage, and fan sensor
.It Xr lmtemp 4
National Semiconductor LM75/LM76/LM77 temperature sensor
+.It Xr lpbl 4
+Texas Instruments LP8550 LED backlight controller
.It Xr maxds 4
Maxim DS1624/DS1631/DS1721 temperature sensor
.It Xr maxtmp 4
Index: share/man/man4/lpbl.4
===================================================================
RCS file: share/man/man4/lpbl.4
diff -N share/man/man4/lpbl.4
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ share/man/man4/lpbl.4 10 Jan 2016 01:52:55 -0000
@@ -0,0 +1,48 @@
+.\"
+.\" Copyright (c) 2016 Sviatoslav Chagaev <***@gmail.com>
+.\"
+.\" Permission to use, copy, modify, and distribute this software for any
+.\" purpose with or without fee is hereby granted, provided that the above
+.\" copyright notice and this permission notice appear in all copies.
+.\"
+.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+.\"
+.Dd $Mdocdate: January 7 2016 $
+.Dt LPBL 4
+.Os
+.Sh NAME
+.Nm lpbl
+.Nd Texas Instruments LP8550 LED backlight controller
+.Sh SYNOPSIS
+.Cd "lpbl0 at iic?"
+.Sh DESCRIPTION
+The
+.Nm
+driver provides support for the LP8550 LED backlight controller, which can be
+found in various models of Intel based Apple laptops.
+.Pp
+The driver allows to control the brightness of LCD backlight through
+.Xr wsconsctl 8
+variable
+.Va display.backlight .
+.Sh SEE ALSO
+.Xr intro 4 ,
+.Xr iic 4 ,
+.Xr wsconsctl 8
+.Sh HISTORY
+The
+.Nm
+driver first appeared in
+.Ox 5.9 .
+.Sh AUTHORS
+.An -nosplit
+The
+.Nm
+driver was written by
+.An Sviatoslav Chagaev Aq Mt ***@gmail.com .
Index: sys/dev/i2c/files.i2c
===================================================================
RCS file: /cvs/src/sys/dev/i2c/files.i2c,v
retrieving revision 1.51
diff -u -p -u -p -r1.51 files.i2c
--- sys/dev/i2c/files.i2c 31 Mar 2013 13:30:24 -0000 1.51
+++ sys/dev/i2c/files.i2c 10 Jan 2016 01:52:58 -0000
@@ -123,6 +123,11 @@ device glenv
attach glenv at i2c
file dev/i2c/gl518sm.c glenv

+# Texas Instruments LP8550 LED backlight controller
+device lpbl
+attach lpbl at i2c
+file dev/i2c/lp8550.c lpbl
+
# RICOH RS5C372[AB] Real Time Clock
device ricohrtc
attach ricohrtc at i2c
Index: sys/dev/i2c/i2c_scan.c
===================================================================
RCS file: /cvs/src/sys/dev/i2c/i2c_scan.c,v
retrieving revision 1.145
diff -u -p -u -p -r1.145 i2c_scan.c
--- sys/dev/i2c/i2c_scan.c 29 May 2015 00:37:10 -0000 1.145
+++ sys/dev/i2c/i2c_scan.c 10 Jan 2016 01:52:58 -0000
@@ -874,6 +874,8 @@ iic_probe_sensor(struct device *self, u_
} else if ((addr == 0x2c || addr == 0x2d || addr == 0x2e) &&
iicprobe(0x16) == 0x41 && ((iicprobe(0x17) & 0xf0) == 0x40)) {
name = "adm1026";
+ } else if (addr == 0x2c && ((iicprobe(0x03) >> 3) & 0xf) == 0xf) {
+ name = "lp8550";
} else if ((addr & 0x78) == 0x18 && iicprobew(0x06) == 0x1131 &&
(iicprobew(0x07) & 0xfffc) == 0xa200) {
name = "se97"; /* or se97b */
Index: sys/dev/i2c/lp8550.c
===================================================================
RCS file: sys/dev/i2c/lp8550.c
diff -N sys/dev/i2c/lp8550.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ sys/dev/i2c/lp8550.c 10 Jan 2016 01:52:58 -0000
@@ -0,0 +1,347 @@
+/*
+ * Copyright (c) 2015 Sviatoslav Chagaev <***@gmail.com>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <sys/param.h>
+#include <sys/systm.h>
+#include <sys/device.h>
+
+#include <dev/i2c/i2cvar.h>
+#include <dev/wscons/wsconsio.h>
+
+#ifdef LPBL_DEBUG
+#define DUMP_REGS(sc) lpbl_dump_registers(sc)
+#define DPRINTF(...) printf(__VA_ARGS__)
+#else
+#define DUMP_REGS(sc)
+#define DPRINTF(...)
+#endif
+
+#define LPBL_WRITE(reg, data) \
+ iic_smbus_write_byte(sc->sc_tag, sc->sc_addr, reg, data, 0)
+#define LPBL_READ(reg, data) \
+ iic_smbus_read_byte(sc->sc_tag, sc->sc_addr, reg, data, 0)
+
+/* Registers */
+#define LP8550_BRIGHTNESS_CTL 0x00
+#define LP8550_MIN_BRIGHTNESS 0
+#define LP8550_MAX_BRIGHTNESS 0xff
+#define LP8550_DEVICE_CTL 0x01
+#define LP8550_BRT_MODE_REG 0x4 /* Control brightness using register. */
+#define LP8550_BL_CTL_ENABLE 0x1 /* Enable backlight. */
+#define LP8550_FAULT 0x02
+#define LP8550_ID 0x03
+#define LP8550_DIRECT_CTL 0x04
+#define LP8550_TEMP_MSB 0x05
+#define LP8550_TEMP_LSB 0x06
+#define LP8550_EEPROM_CTL 0x72
+
+struct lpbl_softc {
+ struct device sc_dev;
+
+ i2c_tag_t sc_tag;
+ i2c_addr_t sc_addr;
+
+ u_int8_t brightness; /* Used to restore brightness level on wake up. */
+};
+
+int lpbl_match(struct device *, void *, void *);
+void lpbl_attach(struct device *, struct device *, void *);
+int lpbl_detach(struct device *, int);
+int lpbl_activate(struct device *, int);
+int lpbl_reset(struct lpbl_softc *);
+struct lpbl_softc *lpbl_get_softc(void);
+int lpbl_set(struct lpbl_softc *, u_int8_t);
+int lpbl_get(struct lpbl_softc *, u_int8_t *);
+#ifdef LPBL_DEBUG
+int lpbl_dump_registers(struct lpbl_softc *);
+#endif
+
+/* wsconsole hook functions */
+int lpbl_get_wsparam(struct wsdisplay_param *p);
+int lpbl_set_wsparam(struct wsdisplay_param *p);
+extern int (*ws_get_param)(struct wsdisplay_param *);
+extern int (*ws_set_param)(struct wsdisplay_param *);
+
+struct cfattach lpbl_ca = {
+ sizeof(struct lpbl_softc),
+ lpbl_match,
+ lpbl_attach,
+ lpbl_detach,
+ lpbl_activate
+};
+
+struct cfdriver lpbl_cd = {
+ NULL, "lpbl", DV_DULL
+};
+
+int
+lpbl_match(struct device *parent, void *match, void *aux)
+{
+ struct i2c_attach_args *ia = aux;
+
+ if (strcmp(ia->ia_name, "lp8550") == 0)
+ return 1;
+ return 0;
+}
+
+void
+lpbl_attach(struct device *parent, struct device *self, void *aux)
+{
+ struct lpbl_softc *sc = (struct lpbl_softc *)self;
+ struct i2c_attach_args *ia = aux;
+ u_int8_t data;
+
+ sc->sc_tag = ia->ia_tag;
+ sc->sc_addr = ia->ia_addr;
+
+ if (iic_acquire_bus(sc->sc_tag, 0) != 0) {
+ printf(": cannot acquire bus\n");
+ return;
+ }
+ if (LPBL_READ(LP8550_ID, &data) != 0) {
+ iic_release_bus(sc->sc_tag, 0);
+ printf(": cannot read id register\n");
+ return;
+ }
+ iic_release_bus(sc->sc_tag, 0);
+
+ printf(": LP8550 rev 0x%02x\n", data & 0x7);
+
+ DUMP_REGS(sc);
+
+ if (lpbl_set(sc, LP8550_MAX_BRIGHTNESS) != 0)
+ return;
+
+ /*
+ * XXX
+ * Other drivers, e.g. acpivout(4), set these too, but earlier.
+ */
+ ws_get_param = lpbl_get_wsparam;
+ ws_set_param = lpbl_set_wsparam;
+}
+
+/*
+ * Restore registers we touch to their default values.
+ */
+int
+lpbl_reset(struct lpbl_softc *sc)
+{
+ if (iic_acquire_bus(sc->sc_tag, 0) != 0) {
+ printf("%s: cannot acquire I2C bus\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+
+ if (LPBL_WRITE(LP8550_DIRECT_CTL, 0) != 0) {
+ iic_release_bus(sc->sc_tag, 0);
+ printf("%s: cannot write direct ctl register\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+ if (LPBL_WRITE(LP8550_DEVICE_CTL, 0) != 0) {
+ iic_release_bus(sc->sc_tag, 0);
+ printf("%s: cannot write device ctl register\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+ if (LPBL_WRITE(LP8550_BRIGHTNESS_CTL, 0)) {
+ iic_release_bus(sc->sc_tag, 0);
+ printf("%s: cannot write brightness ctl register\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+
+ iic_release_bus(sc->sc_tag, 0);
+
+ return 0;
+}
+
+int
+lpbl_detach(struct device *self, int flags)
+{
+ struct lpbl_softc *sc = (struct lpbl_softc *)self;
+
+ lpbl_reset(sc);
+
+ return 0;
+}
+
+int
+lpbl_activate(struct device *self, int act)
+{
+ struct lpbl_softc *sc = (struct lpbl_softc *)self;
+
+ DPRINTF("%s: %s(): act=%d\n", sc->sc_dev.dv_xname, __func__, act);
+
+ switch (act) {
+ case DVACT_WAKEUP:
+ (void)lpbl_set(sc, sc->brightness);
+ break;
+ case DVACT_POWERDOWN:
+ (void)lpbl_reset(sc);
+ break;
+ }
+
+ return 0;
+}
+
+struct lpbl_softc *
+lpbl_first_softc(void)
+{
+ struct lpbl_softc *sc = NULL;
+ int i;
+
+ for (i = 0; i < lpbl_cd.cd_ndevs && sc == NULL; i++) {
+ if (lpbl_cd.cd_devs[i] == NULL)
+ continue;
+ sc = (struct lpbl_softc *)lpbl_cd.cd_devs[i];
+ }
+ return sc;
+}
+
+int
+lpbl_get_wsparam(struct wsdisplay_param *p)
+{
+ struct lpbl_softc *sc = NULL;
+ u_int8_t value;
+
+ sc = lpbl_first_softc();
+ if (sc == NULL)
+ return -1;
+
+ if (p->param != WSDISPLAYIO_PARAM_BACKLIGHT)
+ return -1;
+
+ DPRINTF("%s: %s()\n", sc->sc_dev.dv_xname, __func__);
+
+ if (lpbl_get(sc, &value) != 0)
+ return -1;
+ p->min = LP8550_MIN_BRIGHTNESS;
+ p->max = LP8550_MAX_BRIGHTNESS;
+ p->curval = value;
+ return 0;
+}
+
+int
+lpbl_set_wsparam(struct wsdisplay_param *p)
+{
+ struct lpbl_softc *sc = NULL;
+
+ sc = lpbl_first_softc();
+ if (sc == NULL)
+ return -1;
+
+ if (p->param != WSDISPLAYIO_PARAM_BACKLIGHT)
+ return -1;
+
+ DPRINTF("%s: %s(): p->curval=%d\n",
+ sc->sc_dev.dv_xname, __func__, p->curval);
+
+ return lpbl_set(sc, (u_int8_t)p->curval);
+}
+
+/*
+ * Set backlight brightness level.
+ */
+int
+lpbl_set(struct lpbl_softc *sc, u_int8_t value)
+{
+ if (iic_acquire_bus(sc->sc_tag, 0) != 0) {
+ printf("%s: cannot acquire I2C bus\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+
+ if (LPBL_WRITE(LP8550_BRIGHTNESS_CTL, value) != 0) {
+ iic_release_bus(sc->sc_tag, 0);
+ printf("%s: cannot write brightness ctl register\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+ if (LPBL_WRITE(LP8550_DIRECT_CTL, 0) != 0) {
+ iic_release_bus(sc->sc_tag, 0);
+ printf("%s: cannot write direct ctl register\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+ if (LPBL_WRITE(LP8550_DEVICE_CTL,
+ LP8550_BRT_MODE_REG | LP8550_BL_CTL_ENABLE) != 0) {
+ iic_release_bus(sc->sc_tag, 0);
+ printf("%s: cannot write device ctl register\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+
+ sc->brightness = value;
+
+ iic_release_bus(sc->sc_tag, 0);
+
+ return 0;
+}
+
+/*
+ * Get backlight brightness level.
+ */
+int
+lpbl_get(struct lpbl_softc *sc, u_int8_t *valuep)
+{
+ *valuep = sc->brightness;
+
+ return 0;
+}
+
+#ifdef LPBL_DEBUG
+int
+lpbl_dump_registers(struct lpbl_softc *sc)
+{
+ u_int8_t value;
+
+ printf("%s: register dump:\n",
+ sc->sc_dev.dv_xname);
+
+ if (iic_acquire_bus(sc->sc_tag, 0) != 0) {
+ printf("%s: cannot acquire I2C bus\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+
+#define Y(register) \
+ if (LPBL_READ(register, &value) != 0) { \
+ iic_release_bus(sc->sc_tag, 0); \
+ printf("%s: cannot read " #register " register\n", \
+ sc->sc_dev.dv_xname); \
+ return -1; \
+ } else { \
+ printf("%s: " #register ": 0x%02x\n", \
+ sc->sc_dev.dv_xname, value); \
+ }
+
+ Y(LP8550_BRIGHTNESS_CTL);
+ Y(LP8550_DEVICE_CTL);
+ Y(LP8550_FAULT);
+ Y(LP8550_ID);
+ Y(LP8550_DIRECT_CTL);
+ Y(LP8550_TEMP_MSB);
+ Y(LP8550_TEMP_LSB);
+ Y(LP8550_EEPROM_CTL);
+
+#undef Y
+
+ iic_release_bus(sc->sc_tag, 0);
+
+ return 0;
+}
+#endif /* LPBL_DEBUG */
Index: sys/arch/amd64/conf/GENERIC
===================================================================
RCS file: /cvs/src/sys/arch/amd64/conf/GENERIC,v
retrieving revision 1.408
diff -u -p -u -p -r1.408 GENERIC
--- sys/arch/amd64/conf/GENERIC 8 Jan 2016 09:48:41 -0000 1.408
+++ sys/arch/amd64/conf/GENERIC 10 Jan 2016 01:52:59 -0000
@@ -147,6 +147,7 @@ spdmem* at iic? # SPD memory eeproms
sdtemp* at iic? # SO-DIMM (JC-42.4) temperature
wbng* at iic? # Winbond W83793G
nvt* at iic? # Novoton W83795G
+lpbl0 at iic? # Texas Instruments LP8550 LED backlight controller

skgpio0 at isa? port 0x680 # Soekris net6501 GPIO and LEDs
gpio* at skgpio?

Joerg Jung

2016-01-10 20:28:31 UTC

Permalink

Post by Sviatoslav Chagaev
Hi,
I'm running -current on Apple MacBook Air 6,2. I installed using Jasper's
instructions [1], OpenBSD is the only OS and boots via EFI.
I'm experiencing a problem with LCD backlight: on wakeup from suspend, the
backlight brightness is not restored and becomes unadjustable. If adjusted down,
it turns off, if adjusted up, it goes to 100%.

I also own a MacBook Air 6,2 and wakeup does not really work at all for
me, as the screen just stays black. Basically I see the same as
reported in [5] for the MacBook Pro.

Post by Sviatoslav Chagaev
An Intel developer claims [2] that it's not a bug in Intel KMS driver and people
claim [3] the problem goes away when booting in legacy BIOS mode.

That makes no sense to me. If the problem goes aways when booting
legacy BIOS, so it must be a bug when not, right?

Post by Sviatoslav Chagaev
It turns out [4], a numbers of MacBook models [5], including mine, have a Texas
Instruments LP8550 LED backlight controller on I2C bus. By default, it's
controlled by external PWM. But it can be configured to be controlled by it's
own internal register instead.
So below is a driver for TI LP8550.
With this driver, after wake up from suspend, the brightness level is restored
and can be freely adjusted.

I've tested the driver and while it compiles and attaches fine it does
not work for me. After switching to console the screen stays black.

Post by Sviatoslav Chagaev
The catch is that Intel KMS driver seems to control the chip's ``EN'' pin
(basically an on/off pin). So whenever Intel KMS toggles the backlight, namely
on power management (e.g. `xset dpms force off`) and on Xorg <--> VT switch,
the chip's brightness control register gets reset and you need to
`wsconsctl display.backlight=<non_zero>` to turn the backlight back on.
But it beats rebooting.

Not for me, with my machine -current works better without your patch,
because I can switch from X to console and back and brightness is
correctly adjusted.

Post by Sviatoslav Chagaev
If there's any chance for it to be commited, please tell me what I need to
fix/improve to get it commited (so I don't have to reapply it every time I
upgrade).

IMHO, instead of adding another driver to the mix, I would prefer this
to be handled through the associated asmc(4) keys instead of accessing
the chip directly. The SMC is supposed to be the central point for such
manipulations. Unfortunately, the keys are not documented and need some
non-trivial effort to be figured out.

If this is not possible through asmc(4) and if your new driver improves
the situation then I'm fine with importing it, but for now it is just
worse.

Post by Sviatoslav Chagaev
Next step could be to somehow integrate it with Intel KMS.

Yes, for me this is must-have before it can be committed.

Some minor style comments on the code inline below.

Post by Sviatoslav Chagaev
[1] https://blog.jasper.la/openbsd-uefi-bootloader-howto/
[2] https://bugs.freedesktop.org/show_bug.cgi?id=67454#c103
[3] https://bugs.freedesktop.org/show_bug.cgi?id=67454#c58
[4] https://bugs.freedesktop.org/show_bug.cgi?id=67454#c78
[5] http://marc.info/?l=openbsd-misc&m=144988400031788&w=2
Index: share/man/man4/iic.4
===================================================================
RCS file: /cvs/src/share/man/man4/iic.4,v
retrieving revision 1.84
diff -u -p -u -p -r1.84 iic.4
--- share/man/man4/iic.4 27 Dec 2014 16:08:03 -0000 1.84
+++ share/man/man4/iic.4 10 Jan 2016 01:52:55 -0000
@@ -153,6 +153,8 @@ National Semiconductor LM87 temperature,
National Semiconductor LM93 temperature, voltage, and fan sensor
.It Xr lmtemp 4
National Semiconductor LM75/LM76/LM77 temperature sensor
+.It Xr lpbl 4
+Texas Instruments LP8550 LED backlight controller
.It Xr maxds 4
Maxim DS1624/DS1631/DS1721 temperature sensor
.It Xr maxtmp 4
Index: share/man/man4/lpbl.4
===================================================================
RCS file: share/man/man4/lpbl.4
diff -N share/man/man4/lpbl.4
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ share/man/man4/lpbl.4 10 Jan 2016 01:52:55 -0000
@@ -0,0 +1,48 @@
+.\"
+.\"
+.\" Permission to use, copy, modify, and distribute this software for any
+.\" purpose with or without fee is hereby granted, provided that the above
+.\" copyright notice and this permission notice appear in all copies.
+.\"
+.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+.\"
+.Dd $Mdocdate: January 7 2016 $
+.Dt LPBL 4
+.Os
+.Sh NAME
+.Nm lpbl
+.Nd Texas Instruments LP8550 LED backlight controller
+.Sh SYNOPSIS
+.Cd "lpbl0 at iic?"
+.Sh DESCRIPTION
+The
+.Nm
+driver provides support for the LP8550 LED backlight controller, which can be
+found in various models of Intel based Apple laptops.
+.Pp
+The driver allows to control the brightness of LCD backlight through
+.Xr wsconsctl 8
+variable
+.Va display.backlight .
+.Sh SEE ALSO
+.Xr intro 4 ,
+.Xr iic 4 ,

^ AFAIK, these should be sorted.

Post by Sviatoslav Chagaev
+.Xr wsconsctl 8
+.Sh HISTORY
+The
+.Nm
+driver first appeared in
+.Ox 5.9 .
+.Sh AUTHORS
+.An -nosplit
+The
+.Nm
+driver was written by
Index: sys/dev/i2c/files.i2c
===================================================================
RCS file: /cvs/src/sys/dev/i2c/files.i2c,v
retrieving revision 1.51
diff -u -p -u -p -r1.51 files.i2c
--- sys/dev/i2c/files.i2c 31 Mar 2013 13:30:24 -0000 1.51
+++ sys/dev/i2c/files.i2c 10 Jan 2016 01:52:58 -0000
@@ -123,6 +123,11 @@ device glenv
attach glenv at i2c
file dev/i2c/gl518sm.c glenv
+# Texas Instruments LP8550 LED backlight controller
+device lpbl
+attach lpbl at i2c
+file dev/i2c/lp8550.c lpbl
+
# RICOH RS5C372[AB] Real Time Clock
device ricohrtc
attach ricohrtc at i2c
Index: sys/dev/i2c/i2c_scan.c
===================================================================
RCS file: /cvs/src/sys/dev/i2c/i2c_scan.c,v
retrieving revision 1.145
diff -u -p -u -p -r1.145 i2c_scan.c
--- sys/dev/i2c/i2c_scan.c 29 May 2015 00:37:10 -0000 1.145
+++ sys/dev/i2c/i2c_scan.c 10 Jan 2016 01:52:58 -0000
@@ -874,6 +874,8 @@ iic_probe_sensor(struct device *self, u_
} else if ((addr == 0x2c || addr == 0x2d || addr == 0x2e) &&
iicprobe(0x16) == 0x41 && ((iicprobe(0x17) & 0xf0) == 0x40)) {
name = "adm1026";
+ } else if (addr == 0x2c && ((iicprobe(0x03) >> 3) & 0xf) == 0xf) {
+ name = "lp8550";
} else if ((addr & 0x78) == 0x18 && iicprobew(0x06) == 0x1131 &&
(iicprobew(0x07) & 0xfffc) == 0xa200) {
name = "se97"; /* or se97b */
Index: sys/dev/i2c/lp8550.c
===================================================================
RCS file: sys/dev/i2c/lp8550.c
diff -N sys/dev/i2c/lp8550.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ sys/dev/i2c/lp8550.c 10 Jan 2016 01:52:58 -0000
@@ -0,0 +1,347 @@
+/*
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */

You may want to start with some comment describing the purpose of
file/driver.

Post by Sviatoslav Chagaev
+#include <sys/param.h>
+#include <sys/systm.h>
+#include <sys/device.h>
+
+#include <dev/i2c/i2cvar.h>
+#include <dev/wscons/wsconsio.h>
+
+#ifdef LPBL_DEBUG
+#define DUMP_REGS(sc) lpbl_dump_registers(sc)
+#define DPRINTF(...) printf(__VA_ARGS__)
+#else
+#define DUMP_REGS(sc)
+#define DPRINTF(...)
+#endif
+
+#define LPBL_WRITE(reg, data) \
+ iic_smbus_write_byte(sc->sc_tag, sc->sc_addr, reg, data, 0)
+#define LPBL_READ(reg, data) \
+ iic_smbus_read_byte(sc->sc_tag, sc->sc_addr, reg, data, 0)
+
+/* Registers */
+#define LP8550_BRIGHTNESS_CTL 0x00
+#define LP8550_MIN_BRIGHTNESS 0
+#define LP8550_MAX_BRIGHTNESS 0xff
+#define LP8550_DEVICE_CTL 0x01
+#define LP8550_BRT_MODE_REG 0x4 /* Control brightness using register. */
+#define LP8550_BL_CTL_ENABLE 0x1 /* Enable backlight. */
+#define LP8550_FAULT 0x02
+#define LP8550_ID 0x03
+#define LP8550_DIRECT_CTL 0x04
+#define LP8550_TEMP_MSB 0x05
+#define LP8550_TEMP_LSB 0x06
+#define LP8550_EEPROM_CTL 0x72
+
+struct lpbl_softc {
+ struct device sc_dev;
+
+ i2c_tag_t sc_tag;
+ i2c_addr_t sc_addr;
+
+ u_int8_t brightness; /* Used to restore brightness level on wake up. */

I would use uint8_t here as it is supposed to be more modern style.

Post by Sviatoslav Chagaev
+};
+
+int lpbl_match(struct device *, void *, void *);
+void lpbl_attach(struct device *, struct device *, void *);
+int lpbl_detach(struct device *, int);
+int lpbl_activate(struct device *, int);
+int lpbl_reset(struct lpbl_softc *);
+struct lpbl_softc *lpbl_get_softc(void);
+int lpbl_set(struct lpbl_softc *, u_int8_t);
+int lpbl_get(struct lpbl_softc *, u_int8_t *);
+#ifdef LPBL_DEBUG
+int lpbl_dump_registers(struct lpbl_softc *);
+#endif
+
+/* wsconsole hook functions */
+int lpbl_get_wsparam(struct wsdisplay_param *p);
+int lpbl_set_wsparam(struct wsdisplay_param *p);
+extern int (*ws_get_param)(struct wsdisplay_param *);
+extern int (*ws_set_param)(struct wsdisplay_param *);
+
+struct cfattach lpbl_ca = {
+ sizeof(struct lpbl_softc),
+ lpbl_match,
+ lpbl_attach,
+ lpbl_detach,
+ lpbl_activate
+};

^ AFAIK this struct should be static.

Post by Sviatoslav Chagaev
+struct cfdriver lpbl_cd = {
+ NULL, "lpbl", DV_DULL
+};
+
+int
+lpbl_match(struct device *parent, void *match, void *aux)
+{
+ struct i2c_attach_args *ia = aux;
+
+ if (strcmp(ia->ia_name, "lp8550") == 0)
+ return 1;
+ return 0;
+}
+
+void
+lpbl_attach(struct device *parent, struct device *self, void *aux)
+{
+ struct lpbl_softc *sc = (struct lpbl_softc *)self;
+ struct i2c_attach_args *ia = aux;
+ u_int8_t data;
+
+ sc->sc_tag = ia->ia_tag;
+ sc->sc_addr = ia->ia_addr;
+
+ if (iic_acquire_bus(sc->sc_tag, 0) != 0) {
+ printf(": cannot acquire bus\n");
+ return;
+ }
+ if (LPBL_READ(LP8550_ID, &data) != 0) {
+ iic_release_bus(sc->sc_tag, 0);
+ printf(": cannot read id register\n");
+ return;
+ }
+ iic_release_bus(sc->sc_tag, 0);
+
+ printf(": LP8550 rev 0x%02x\n", data & 0x7);
+
+ DUMP_REGS(sc);
+
+ if (lpbl_set(sc, LP8550_MAX_BRIGHTNESS) != 0)
+ return;
+
+ /*
+ * XXX
+ * Other drivers, e.g. acpivout(4), set these too, but earlier.
+ */
+ ws_get_param = lpbl_get_wsparam;
+ ws_set_param = lpbl_set_wsparam;
+}
+
+/*
+ * Restore registers we touch to their default values.
+ */
+int
+lpbl_reset(struct lpbl_softc *sc)
+{
+ if (iic_acquire_bus(sc->sc_tag, 0) != 0) {
+ printf("%s: cannot acquire I2C bus\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+
+ if (LPBL_WRITE(LP8550_DIRECT_CTL, 0) != 0) {
+ iic_release_bus(sc->sc_tag, 0);
+ printf("%s: cannot write direct ctl register\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+ if (LPBL_WRITE(LP8550_DEVICE_CTL, 0) != 0) {
+ iic_release_bus(sc->sc_tag, 0);
+ printf("%s: cannot write device ctl register\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+ if (LPBL_WRITE(LP8550_BRIGHTNESS_CTL, 0)) {
+ iic_release_bus(sc->sc_tag, 0);
+ printf("%s: cannot write brightness ctl register\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+
+ iic_release_bus(sc->sc_tag, 0);
+
+ return 0;
+}
+
+int
+lpbl_detach(struct device *self, int flags)
+{
+ struct lpbl_softc *sc = (struct lpbl_softc *)self;
+
+ lpbl_reset(sc);
+
+ return 0;
+}
+
+int
+lpbl_activate(struct device *self, int act)
+{
+ struct lpbl_softc *sc = (struct lpbl_softc *)self;
+
+ DPRINTF("%s: %s(): act=%d\n", sc->sc_dev.dv_xname, __func__, act);
+
+ switch (act) {
+ (void)lpbl_set(sc, sc->brightness);
+ break;
+ (void)lpbl_reset(sc);
+ break;
+ }
+
+ return 0;
+}
+
+struct lpbl_softc *
+lpbl_first_softc(void)
+{
+ struct lpbl_softc *sc = NULL;
+ int i;
+
+ for (i = 0; i < lpbl_cd.cd_ndevs && sc == NULL; i++) {
+ if (lpbl_cd.cd_devs[i] == NULL)
+ continue;
+ sc = (struct lpbl_softc *)lpbl_cd.cd_devs[i];
+ }
+ return sc;
+}
+
+int
+lpbl_get_wsparam(struct wsdisplay_param *p)
+{
+ struct lpbl_softc *sc = NULL;
+ u_int8_t value;
+
+ sc = lpbl_first_softc();
+ if (sc == NULL)
+ return -1;
+
+ if (p->param != WSDISPLAYIO_PARAM_BACKLIGHT)
+ return -1;
+
+ DPRINTF("%s: %s()\n", sc->sc_dev.dv_xname, __func__);
+
+ if (lpbl_get(sc, &value) != 0)
+ return -1;
+ p->min = LP8550_MIN_BRIGHTNESS;
+ p->max = LP8550_MAX_BRIGHTNESS;
+ p->curval = value;
+ return 0;
+}
+
+int
+lpbl_set_wsparam(struct wsdisplay_param *p)
+{
+ struct lpbl_softc *sc = NULL;
+
+ sc = lpbl_first_softc();
+ if (sc == NULL)
+ return -1;
+
+ if (p->param != WSDISPLAYIO_PARAM_BACKLIGHT)
+ return -1;
+
+ DPRINTF("%s: %s(): p->curval=%d\n",
+ sc->sc_dev.dv_xname, __func__, p->curval);
+
+ return lpbl_set(sc, (u_int8_t)p->curval);
+}
+
+/*
+ * Set backlight brightness level.
+ */
+int
+lpbl_set(struct lpbl_softc *sc, u_int8_t value)
+{
+ if (iic_acquire_bus(sc->sc_tag, 0) != 0) {
+ printf("%s: cannot acquire I2C bus\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+
+ if (LPBL_WRITE(LP8550_BRIGHTNESS_CTL, value) != 0) {
+ iic_release_bus(sc->sc_tag, 0);
+ printf("%s: cannot write brightness ctl register\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+ if (LPBL_WRITE(LP8550_DIRECT_CTL, 0) != 0) {
+ iic_release_bus(sc->sc_tag, 0);
+ printf("%s: cannot write direct ctl register\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+ if (LPBL_WRITE(LP8550_DEVICE_CTL,
+ LP8550_BRT_MODE_REG | LP8550_BL_CTL_ENABLE) != 0) {
+ iic_release_bus(sc->sc_tag, 0);
+ printf("%s: cannot write device ctl register\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+
+ sc->brightness = value;
+
+ iic_release_bus(sc->sc_tag, 0);
+
+ return 0;
+}
+
+/*
+ * Get backlight brightness level.
+ */
+int
+lpbl_get(struct lpbl_softc *sc, u_int8_t *valuep)
+{
+ *valuep = sc->brightness;
+
+ return 0;
+}
+
+#ifdef LPBL_DEBUG
+int
+lpbl_dump_registers(struct lpbl_softc *sc)
+{
+ u_int8_t value;
+
+ printf("%s: register dump:\n",
+ sc->sc_dev.dv_xname);
+
+ if (iic_acquire_bus(sc->sc_tag, 0) != 0) {
+ printf("%s: cannot acquire I2C bus\n",
+ sc->sc_dev.dv_xname);
+ return -1;
+ }
+
+#define Y(register) \
+ if (LPBL_READ(register, &value) != 0) { \
+ iic_release_bus(sc->sc_tag, 0); \
+ printf("%s: cannot read " #register " register\n", \
+ sc->sc_dev.dv_xname); \
+ return -1; \
+ } else { \
+ printf("%s: " #register ": 0x%02x\n", \
+ sc->sc_dev.dv_xname, value); \
+ }
+
+ Y(LP8550_BRIGHTNESS_CTL);
+ Y(LP8550_DEVICE_CTL);
+ Y(LP8550_FAULT);
+ Y(LP8550_ID);
+ Y(LP8550_DIRECT_CTL);
+ Y(LP8550_TEMP_MSB);
+ Y(LP8550_TEMP_LSB);
+ Y(LP8550_EEPROM_CTL);
+
+#undef Y
+
+ iic_release_bus(sc->sc_tag, 0);
+
+ return 0;
+}
+#endif /* LPBL_DEBUG */
Index: sys/arch/amd64/conf/GENERIC
===================================================================
RCS file: /cvs/src/sys/arch/amd64/conf/GENERIC,v
retrieving revision 1.408
diff -u -p -u -p -r1.408 GENERIC
--- sys/arch/amd64/conf/GENERIC 8 Jan 2016 09:48:41 -0000 1.408
+++ sys/arch/amd64/conf/GENERIC 10 Jan 2016 01:52:59 -0000
@@ -147,6 +147,7 @@ spdmem* at iic? # SPD memory eeproms
sdtemp* at iic? # SO-DIMM (JC-42.4) temperature
wbng* at iic? # Winbond W83793G
nvt* at iic? # Novoton W83795G
+lpbl0 at iic? # Texas Instruments LP8550 LED backlight controller
skgpio0 at isa? port 0x680 # Soekris net6501 GPIO and LEDs
gpio* at skgpio?

Mark Kettenis

2016-01-10 20:47:10 UTC

Permalink

Date: Sun, 10 Jan 2016 21:28:31 +0100

Post by Sviatoslav Chagaev
Hi,
I'm running -current on Apple MacBook Air 6,2. I installed using Jasper's
instructions [1], OpenBSD is the only OS and boots via EFI.
I'm experiencing a problem with LCD backlight: on wakeup from
suspend, the backlight brightness is not restored and becomes
unadjustable. If adjusted down, it turns off, if adjusted up, it
goes to 100%.

I also own a MacBook Air 6,2 and wakeup does not really work at all for
me, as the screen just stays black. Basically I see the same as
reported in [5] for the MacBook Pro.

Post by Sviatoslav Chagaev
An Intel developer claims [2] that it's not a bug in Intel KMS
driver and people claim [3] the problem goes away when booting in
legacy BIOS mode.

That makes no sense to me. If the problem goes aways when booting
legacy BIOS, so it must be a bug when not, right?

Post by Sviatoslav Chagaev
It turns out [4], a numbers of MacBook models [5], including mine,
have a Texas Instruments LP8550 LED backlight controller on I2C
bus. By default, it's controlled by external PWM. But it can be
configured to be controlled by it's own internal register instead.
So below is a driver for TI LP8550.
With this driver, after wake up from suspend, the brightness level
is restored and can be freely adjusted.

I've tested the driver and while it compiles and attaches fine it
does not work for me. After switching to console the screen stays
black.

Post by Sviatoslav Chagaev
The catch is that Intel KMS driver seems to control the chip's
``EN'' pin (basically an on/off pin). So whenever Intel KMS
toggles the backlight, namely on power management (e.g. `xset dpms
force off`) and on Xorg <--> VT switch, the chip's brightness
control register gets reset and you need to `wsconsctl
display.backlight=<non_zero>` to turn the backlight back on.
But it beats rebooting.

Not for me, with my machine -current works better without your
patch, because I can switch from X to console and back and
brightness is correctly adjusted.

Post by Sviatoslav Chagaev
If there's any chance for it to be commited, please tell me what I need to
fix/improve to get it commited (so I don't have to reapply it every time I
upgrade).

IMHO, instead of adding another driver to the mix, I would prefer
this to be handled through the associated asmc(4) keys instead of
accessing the chip directly. The SMC is supposed to be the central
point for such manipulations. Unfortunately, the keys are not
documented and need some non-trivial effort to be figured out.
If this is not possible through asmc(4) and if your new driver
improves the situation then I'm fine with importing it, but for now
it is just worse.

Also, I'dlike to see the acpidump output for this machine before we
decide things are indeed broken.

Sviatoslav Chagaev

2016-01-11 22:45:13 UTC

Permalink

On Sun, 10 Jan 2016 21:47:10 +0100 (CET)

Post by Mark Kettenis
Also, I'dlike to see the acpidump output for this machine before we
decide things are indeed broken.

https://drive.google.com/file/d/0B0d5PPQ95AzWMGxxZGlZZ3RsSjA/view?usp=sharing

Unrelated observation: when I boot with Apple Ethernet adapter [1]
connected to Thunderbolt port, ACPI S3 state disappears:

https://drive.google.com/file/d/0B0d5PPQ95AzWZFpNMXQycUpkMUE/view?usp=sharing

[1] http://www.apple.com/shop/product/MD463LL/A/thunderbolt-to-gigabit-ethernet-adapter

Sviatoslav Chagaev

2016-01-11 23:37:39 UTC

Permalink

On Sun, 10 Jan 2016 21:28:31 +0100

Post by Joerg Jung

I also own a MacBook Air 6,2 and wakeup does not really work at all for
me, as the screen just stays black. Basically I see the same as
reported in [5] for the MacBook Pro.

Try the following:

1) Upgrade to latest snapshot.
2) Bind `wsconsctl display.brightness+=5` to some key combination in
your X window manager.
3) On wake up, start adjusting the brightness up.

At some magical point (something like 87% for me IIRC) you should
observe the backlight turning on and going to 100% brightness. From
this point, if you adjust down, the backlight turns off, if you adjust
up, the backlight goes to 100%.

If you don't observe this, we might be experiencing different issues.

Post by Joerg Jung

Post by Sviatoslav Chagaev
An Intel developer claims [2] that it's not a bug in Intel KMS driver and people
claim [3] the problem goes away when booting in legacy BIOS mode.

That makes no sense to me. If the problem goes aways when booting
legacy BIOS, so it must be a bug when not, right?

Judging by the code at e.g. sys/dev/pci/drm/i915/intel_panel.c:984, it
seems Intel KMS driver expects BIOS or EFI to configure backlight,
then reads these values and uses them itself.

I found documentation for Intel Haswell [1] and glanced over
descriptions of relevant registers (BLC_PWM_CTL, BLC_PWM2_CTL,
BLC_PWM_DATA, BLC_MISC_CTL, BLM_HIST_CTL, BLM_HIST_BIN, BLM_HIST_GUARD,
UTIL_PIN_CTL, SBLC_PWM_CTL1, SBLC_PWM_CTL2), they are confusing...

Based on these, I agree, it might be a bug in Intel KMS.

I'll add debug prints and play with these registers, maybe I'll spot
some incorrect values.

[1] https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-hsw-commandreference-registers_0_0.pdf

Post by Joerg Jung

I've tested the driver and while it compiles and attaches fine it does
not work for me. After switching to console the screen stays black.

Yep, it needs to be integrated with Intel KMS to not break anything...

Post by Joerg Jung

Not for me, with my machine -current works better without your patch,
because I can switch from X to console and back and brightness is
correctly adjusted.

Post by Sviatoslav Chagaev
If there's any chance for it to be commited, please tell me what I need to
fix/improve to get it commited (so I don't have to reapply it every time I
upgrade).

IMHO, instead of adding another driver to the mix, I would prefer this
to be handled through the associated asmc(4) keys instead of accessing
the chip directly. The SMC is supposed to be the central point for such
manipulations. Unfortunately, the keys are not documented and need some
non-trivial effort to be figured out.
If this is not possible through asmc(4) and if your new driver improves
the situation then I'm fine with importing it, but for now it is just
worse.

Post by Sviatoslav Chagaev
Next step could be to somehow integrate it with Intel KMS.

Yes, for me this is must-have before it can be committed.
Some minor style comments on the code inline below.

Thanks, if there will be a next version of this diff, I'll fix them.

Sviatoslav Chagaev

2016-01-13 01:40:38 UTC

Permalink

On Tue, 12 Jan 2016 01:37:39 +0200

Post by Sviatoslav Chagaev
On Sun, 10 Jan 2016 21:28:31 +0100

Post by Joerg Jung

Post by Sviatoslav Chagaev
An Intel developer claims [2] that it's not a bug in Intel KMS driver and people
claim [3] the problem goes away when booting in legacy BIOS mode.

That makes no sense to me. If the problem goes aways when booting
legacy BIOS, so it must be a bug when not, right?

Judging by the code at e.g. sys/dev/pci/drm/i915/intel_panel.c:984, it
seems Intel KMS driver expects BIOS or EFI to configure backlight,
then reads these values and uses them itself.
I found documentation for Intel Haswell [1] and glanced over
descriptions of relevant registers (BLC_PWM_CTL, BLC_PWM2_CTL,
BLC_PWM_DATA, BLC_MISC_CTL, BLM_HIST_CTL, BLM_HIST_BIN, BLM_HIST_GUARD,
UTIL_PIN_CTL, SBLC_PWM_CTL1, SBLC_PWM_CTL2), they are confusing...
Based on these, I agree, it might be a bug in Intel KMS.
I'll add debug prints and play with these registers, maybe I'll spot
some incorrect values.
[1] https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-hsw-commandreference-registers_0_0.pdf

Had no luck with Intel KMS:

Added register dump to every backlight operation, see first diff.

State of registers I get from EFI:

pch_dump_backlight_registers(): pch_setup_backlight
MY_BLC_PWM_CTL = 0x00000000; MY_BLC_PWM_DATA = 0x00000000
MY_BLC_PWM_DATA2 = 0x00000000; MY_BLM_HIST_CTL = 0x00000000
MY_BLM_HIST_BIN = 0x00000000; MY_BLM_HIST_GUARD = 0x00000000
MY_BLC_PWM2_CTL = 0x60000000; MY_BLC_MISC_CTL = 0x00000000
MY_UTIL_PIN_CTL = 0x00000000; MY_SBLC_PWM_CTL1 = 0xc0000000
MY_SBLC_PWM_CTL2 = 0x0ad901c3;

State of registers after Intel KMS configures them:

pch_dump_backlight_registers(): pch_set_backlight
MY_BLC_PWM_CTL = 0xe0000000; MY_BLC_PWM_DATA = 0x00000ad9
MY_BLC_PWM_DATA2 = 0x00000000; MY_BLM_HIST_CTL = 0x00000006
MY_BLM_HIST_BIN = 0x00000000; MY_BLM_HIST_GUARD = 0x00000000
MY_BLC_PWM2_CTL = 0x60000000; MY_BLC_MISC_CTL = 0x00000000
MY_UTIL_PIN_CTL = 0x00000000; MY_SBLC_PWM_CTL1 = 0x80000000
MY_SBLC_PWM_CTL2 = 0x0ad90000;

BLC_PWM_CTL change seems strange.
Modified pch_*_backlight() functions so they do not change the
configuration set by EFI, see second diff, but observed no change in
behaviour.

Index: sys/dev/pci/drm/i915/i915_reg.h
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_reg.h,v
retrieving revision 1.11
diff -u -p -r1.11 i915_reg.h
--- sys/dev/pci/drm/i915/i915_reg.h 25 Sep 2015 16:15:19 -0000 1.11
+++ sys/dev/pci/drm/i915/i915_reg.h 12 Jan 2016 02:54:49 -0000
@@ -2493,6 +2493,18 @@
#define BACKLIGHT_DUTY_CYCLE_MASK_PNV (0xfffe)
#define BLM_POLARITY_PNV (1 << 0) /* pnv only */

+#define MY_BLC_PWM_CTL 0x48250
+#define MY_BLC_PWM_DATA 0x48254
+#define MY_BLC_PWM_DATA2 0x48354
+#define MY_BLM_HIST_CTL 0x48260
+#define MY_BLM_HIST_BIN 0x48264
+#define MY_BLM_HIST_GUARD 0x48268
+#define MY_BLC_PWM2_CTL 0x48350
+#define MY_BLC_MISC_CTL 0x48360
+#define MY_UTIL_PIN_CTL 0x48400
+#define MY_SBLC_PWM_CTL1 0xc8250
+#define MY_SBLC_PWM_CTL2 0xc8254
+
#define BLC_HIST_CTL (dev_priv->info->display_mmio_offset + 0x61260)

/* New registers for PCH-split platforms. Safe where new bits show up, the
Index: sys/dev/pci/drm/i915/intel_panel.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_panel.c,v
retrieving revision 1.11
diff -u -p -r1.11 intel_panel.c
--- sys/dev/pci/drm/i915/intel_panel.c 23 Sep 2015 23:12:12 -0000 1.11
+++ sys/dev/pci/drm/i915/intel_panel.c 12 Jan 2016 02:54:49 -0000
@@ -36,6 +36,38 @@

#define PCI_LBPC 0xf4 /* legacy/combination backlight modes */

+void pch_dump_backlight_registers(struct intel_connector *connector, const char *msg)
+{
+ struct drm_device *dev = connector->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 i = 0;
+ u32 value;
+
+ printf("%s(): %s\n", __func__, msg);
+
+#define Y(register) \
+ value = I915_READ(register); \
+ printf("%-17s = 0x%08x%s", \
+ #register, value, (i++ & 1) ? "\n" : "; ");
+
+ Y(MY_BLC_PWM_CTL);
+ Y(MY_BLC_PWM_DATA);
+ Y(MY_BLC_PWM_DATA2);
+ Y(MY_BLM_HIST_CTL);
+ Y(MY_BLM_HIST_BIN);
+ Y(MY_BLM_HIST_GUARD);
+ Y(MY_BLC_PWM2_CTL);
+ Y(MY_BLC_MISC_CTL);
+ Y(MY_UTIL_PIN_CTL);
+ Y(MY_SBLC_PWM_CTL1);
+ Y(MY_SBLC_PWM_CTL2);
+
+ if (i & 1)
+ printf("\n");
+
+#undef Y
+}
+
void
intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
struct drm_display_mode *adjusted_mode)
@@ -366,6 +398,8 @@ static u32 pch_get_backlight(struct inte
struct drm_device *dev = connector->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;

+ pch_dump_backlight_registers(connector, __func__);
+
return I915_READ(BLC_PWM_CPU_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
}

@@ -439,6 +473,8 @@ static void pch_set_backlight(struct int
struct drm_i915_private *dev_priv = dev->dev_private;
u32 tmp;

+ pch_dump_backlight_registers(connector, __func__);
+
tmp = I915_READ(BLC_PWM_CPU_CTL) & ~BACKLIGHT_DUTY_CYCLE_MASK;
I915_WRITE(BLC_PWM_CPU_CTL, tmp | level);
}
@@ -535,6 +571,8 @@ static void pch_disable_backlight(struct
struct drm_i915_private *dev_priv = dev->dev_private;
u32 tmp;

+ pch_dump_backlight_registers(connector, __func__);
+
intel_panel_actually_set_backlight(connector, 0);

tmp = I915_READ(BLC_PWM_CPU_CTL2);
@@ -648,6 +686,8 @@ static void pch_enable_backlight(struct
intel_pipe_to_cpu_transcoder(dev_priv, pipe);
u32 cpu_ctl2, pch_ctl1, pch_ctl2;

+ pch_dump_backlight_registers(connector, __func__);
+
cpu_ctl2 = I915_READ(BLC_PWM_CPU_CTL2);
if (cpu_ctl2 & BLM_PWM_ENABLE) {
DRM_DEBUG_KMS("cpu backlight already enabled\n");
@@ -980,6 +1020,8 @@ static int pch_setup_backlight(struct in
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_panel *panel = &connector->panel;
u32 cpu_ctl2, pch_ctl1, pch_ctl2, val;
+
+ pch_dump_backlight_registers(connector, __func__);

pch_ctl1 = I915_READ(BLC_PWM_PCH_CTL1);
panel->backlight.active_low_pwm = pch_ctl1 & BLM_PCH_POLARITY;

Index: sys/dev/pci/drm/i915/i915_reg.h
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_reg.h,v
retrieving revision 1.11
diff -u -p -r1.11 i915_reg.h
--- sys/dev/pci/drm/i915/i915_reg.h 25 Sep 2015 16:15:19 -0000 1.11
+++ sys/dev/pci/drm/i915/i915_reg.h 12 Jan 2016 23:07:04 -0000
@@ -2493,6 +2493,18 @@
#define BACKLIGHT_DUTY_CYCLE_MASK_PNV (0xfffe)
#define BLM_POLARITY_PNV (1 << 0) /* pnv only */

+#define MY_BLC_PWM_CTL 0x48250
+#define MY_BLC_PWM_DATA 0x48254
+#define MY_BLC_PWM_DATA2 0x48354
+#define MY_BLM_HIST_CTL 0x48260
+#define MY_BLM_HIST_BIN 0x48264
+#define MY_BLM_HIST_GUARD 0x48268
+#define MY_BLC_PWM2_CTL 0x48350
+#define MY_BLC_MISC_CTL 0x48360
+#define MY_UTIL_PIN_CTL 0x48400
+#define MY_SBLC_PWM_CTL1 0xc8250
+#define MY_SBLC_PWM_CTL2 0xc8254
+
#define BLC_HIST_CTL (dev_priv->info->display_mmio_offset + 0x61260)

/* New registers for PCH-split platforms. Safe where new bits show up, the
Index: sys/dev/pci/drm/i915/intel_panel.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_panel.c,v
retrieving revision 1.11
diff -u -p -r1.11 intel_panel.c
--- sys/dev/pci/drm/i915/intel_panel.c 23 Sep 2015 23:12:12 -0000 1.11
+++ sys/dev/pci/drm/i915/intel_panel.c 12 Jan 2016 23:07:05 -0000
@@ -36,6 +36,38 @@

#define PCI_LBPC 0xf4 /* legacy/combination backlight modes */

+void pch_dump_backlight_registers(struct intel_connector *connector, const char *msg)
+{
+ struct drm_device *dev = connector->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 i = 0;
+ u32 value;
+
+ printf("%s(): %s\n", __func__, msg);
+
+#define Y(register) \
+ value = I915_READ(register); \
+ printf("%-17s = 0x%08x%s", \
+ #register, value, (i++ & 1) ? "\n" : "; ");
+
+ Y(MY_BLC_PWM_CTL);
+ Y(MY_BLC_PWM_DATA);
+ Y(MY_BLC_PWM_DATA2);
+ Y(MY_BLM_HIST_CTL);
+ Y(MY_BLM_HIST_BIN);
+ Y(MY_BLM_HIST_GUARD);
+ Y(MY_BLC_PWM2_CTL);
+ Y(MY_BLC_MISC_CTL);
+ Y(MY_UTIL_PIN_CTL);
+ Y(MY_SBLC_PWM_CTL1);
+ Y(MY_SBLC_PWM_CTL2);
+
+ if (i & 1)
+ printf("\n");
+
+#undef Y
+}
+
void
intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
struct drm_display_mode *adjusted_mode)
@@ -366,7 +398,9 @@ static u32 pch_get_backlight(struct inte
struct drm_device *dev = connector->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;

- return I915_READ(BLC_PWM_CPU_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
+ pch_dump_backlight_registers(connector, __func__);
+
+ return I915_READ(MY_SBLC_PWM_CTL2) & BACKLIGHT_DUTY_CYCLE_MASK;
}

static u32 i9xx_get_backlight(struct intel_connector *connector)
@@ -439,8 +473,10 @@ static void pch_set_backlight(struct int
struct drm_i915_private *dev_priv = dev->dev_private;
u32 tmp;

- tmp = I915_READ(BLC_PWM_CPU_CTL) & ~BACKLIGHT_DUTY_CYCLE_MASK;
- I915_WRITE(BLC_PWM_CPU_CTL, tmp | level);
+ pch_dump_backlight_registers(connector, __func__);
+
+ tmp = I915_READ(MY_SBLC_PWM_CTL2) & ~BACKLIGHT_DUTY_CYCLE_MASK;
+ I915_WRITE(MY_SBLC_PWM_CTL2, tmp | level);
}

static void i9xx_set_backlight(struct intel_connector *connector, u32 level)
@@ -535,13 +571,10 @@ static void pch_disable_backlight(struct
struct drm_i915_private *dev_priv = dev->dev_private;
u32 tmp;

- intel_panel_actually_set_backlight(connector, 0);
-
- tmp = I915_READ(BLC_PWM_CPU_CTL2);
- I915_WRITE(BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE);
+ pch_dump_backlight_registers(connector, __func__);

- tmp = I915_READ(BLC_PWM_PCH_CTL1);
- I915_WRITE(BLC_PWM_PCH_CTL1, tmp & ~BLM_PCH_PWM_ENABLE);
+ tmp = I915_READ(MY_SBLC_PWM_CTL2) & ~BACKLIGHT_DUTY_CYCLE_MASK;
+ I915_WRITE(MY_SBLC_PWM_CTL2, tmp);
}

static void i9xx_disable_backlight(struct intel_connector *connector)
@@ -642,51 +675,23 @@ static void pch_enable_backlight(struct
{
struct drm_device *dev = connector->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_panel *panel = &connector->panel;
- enum pipe pipe = intel_get_pipe_from_connector(connector);
- enum transcoder cpu_transcoder =
- intel_pipe_to_cpu_transcoder(dev_priv, pipe);
- u32 cpu_ctl2, pch_ctl1, pch_ctl2;
-
- cpu_ctl2 = I915_READ(BLC_PWM_CPU_CTL2);
- if (cpu_ctl2 & BLM_PWM_ENABLE) {
- DRM_DEBUG_KMS("cpu backlight already enabled\n");
- cpu_ctl2 &= ~BLM_PWM_ENABLE;
- I915_WRITE(BLC_PWM_CPU_CTL2, cpu_ctl2);
- }
-
- pch_ctl1 = I915_READ(BLC_PWM_PCH_CTL1);
- if (pch_ctl1 & BLM_PCH_PWM_ENABLE) {
- DRM_DEBUG_KMS("pch backlight already enabled\n");
- pch_ctl1 &= ~BLM_PCH_PWM_ENABLE;
- I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1);
- }
+ u32 tmp;

- if (cpu_transcoder == TRANSCODER_EDP)
- cpu_ctl2 = BLM_TRANSCODER_EDP;
- else
- cpu_ctl2 = BLM_PIPE(cpu_transcoder);
- I915_WRITE(BLC_PWM_CPU_CTL2, cpu_ctl2);
- POSTING_READ(BLC_PWM_CPU_CTL2);
- I915_WRITE(BLC_PWM_CPU_CTL2, cpu_ctl2 | BLM_PWM_ENABLE);
+ pch_dump_backlight_registers(connector, __func__);

- /* This won't stick until the above enable. */
- intel_panel_actually_set_backlight(connector, panel->backlight.level);
+ if (I915_READ(MY_BLC_PWM_CTL) != 0)
+ I915_WRITE(MY_BLC_PWM_CTL, 0);

- pch_ctl2 = panel->backlight.max << 16;
- I915_WRITE(BLC_PWM_PCH_CTL2, pch_ctl2);
-
- /* XXX: transitional */
- if (dev_priv->quirks & QUIRK_NO_PCH_PWM_ENABLE)
- return;
+ if (I915_READ(MY_SBLC_PWM_CTL2) == 0)
+ I915_WRITE(MY_SBLC_PWM_CTL2, 0x0ad90ad9);

- pch_ctl1 = 0;
- if (panel->backlight.active_low_pwm)
- pch_ctl1 |= BLM_PCH_POLARITY;
+ /* SBLC_PWM_CTL1 must be programmed *after* SBLC_PWM_CTL2 */
+ if (I915_READ(MY_SBLC_PWM_CTL1) != 0xc0000000)
+ I915_WRITE(MY_SBLC_PWM_CTL1, 0xc0000000);

- I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1);
- POSTING_READ(BLC_PWM_PCH_CTL1);
- I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_PWM_ENABLE);
+ tmp = I915_READ(MY_SBLC_PWM_CTL2);
+ tmp |= tmp >> 16;
+ I915_WRITE(MY_SBLC_PWM_CTL2, tmp);
}

static void i9xx_enable_backlight(struct intel_connector *connector)
@@ -979,22 +984,23 @@ static int pch_setup_backlight(struct in
struct drm_device *dev = connector->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_panel *panel = &connector->panel;
- u32 cpu_ctl2, pch_ctl1, pch_ctl2, val;

- pch_ctl1 = I915_READ(BLC_PWM_PCH_CTL1);
- panel->backlight.active_low_pwm = pch_ctl1 & BLM_PCH_POLARITY;
+ pch_dump_backlight_registers(connector, __func__);

- pch_ctl2 = I915_READ(BLC_PWM_PCH_CTL2);
- panel->backlight.max = pch_ctl2 >> 16;
- if (!panel->backlight.max)
- return -ENODEV;
+ if (I915_READ(MY_BLC_PWM_CTL) != 0)
+ I915_WRITE(MY_BLC_PWM_CTL, 0);

- val = pch_get_backlight(connector);
- panel->backlight.level = intel_panel_compute_brightness(connector, val);
+ if (I915_READ(MY_SBLC_PWM_CTL2) == 0)
+ I915_WRITE(MY_SBLC_PWM_CTL2, 0x0ad90ad9);

- cpu_ctl2 = I915_READ(BLC_PWM_CPU_CTL2);
- panel->backlight.enabled = (cpu_ctl2 & BLM_PWM_ENABLE) &&
- (pch_ctl1 & BLM_PCH_PWM_ENABLE) && panel->backlight.level != 0;
+ /* SBLC_PWM_CTL1 must be programmed *after* SBLC_PWM_CTL2 */
+ if (I915_READ(MY_SBLC_PWM_CTL1) != 0xc0000000)
+ I915_WRITE(MY_SBLC_PWM_CTL1, 0xc0000000);
+
+ panel->backlight.active_low_pwm = false;
+ panel->backlight.max = I915_READ(MY_SBLC_PWM_CTL2) >> 16;
+ panel->backlight.level = intel_panel_compute_brightness(connector,
+ I915_READ(MY_SBLC_PWM_CTL2) & BACKLIGHT_DUTY_CYCLE_MASK);

return 0;
}

Sviatoslav Chagaev

2016-01-27 03:20:59 UTC

Permalink

Post by Joerg Jung

Post by Sviatoslav Chagaev
So below is a driver for TI LP8550.

...

Post by Joerg Jung

Post by Sviatoslav Chagaev
If there's any chance for it to be commited, please tell me what I need to
fix/improve to get it commited (so I don't have to reapply it every time I
upgrade).

IMHO, instead of adding another driver to the mix, I would prefer this
to be handled through the associated asmc(4) keys instead of accessing
the chip directly. The SMC is supposed to be the central point for such
manipulations. Unfortunately, the keys are not documented and need some
non-trivial effort to be figured out.
If this is not possible through asmc(4) and if your new driver improves
the situation then I'm fine with importing it, but for now it is just
worse.

Using knowledge acquired from [1] and [2] made a diff, posted below for
reference, which adds:
* ioctl interface to asmc(4) to allow reading/writing of ``keys'' from
userspace;
* companion smcprobe(1) which reads/writes ``keys'' using the ioctl;
* smcdumpkeys(1): scans an SMC textual firmware file and dumps the ``key''
table that it contains.

Downloaded SMC FW [3], unpacked it using Google Drive and used smcdumpkeys(1) +
smcprobe(1) to create the table of keys and their current values before suspend
[4] and after wake up [5]. Diffed them [6], filtering out keys which change over
time as they are probably sensors (some might have gotten through).

Tried modifying all keys whose names start with 'B': no effect.
Tried modifying keys which are different after wake up: no effect.

[1] https://reverse.put.as/wp-content/uploads/2015/12/D1_02_Alex_Ninjas_and_Harry_Potter.pdf
[2] https://dev.inversepath.com/download/public/embedded_systems_exploitation.pdf
[3] https://support.apple.com/kb/DL1748?locale=en_US
[4] https://gist.github.com/S010/26145f4446fcc0b8a0d6#file-before_suspend-txt
[5] https://gist.github.com/S010/26145f4446fcc0b8a0d6#file-after_wakeup-txt
[6] https://gist.github.com/S010/26145f4446fcc0b8a0d6#file-suspend_wakeup_key-diff

Index: sys/dev/isa/asmc.c
===================================================================
RCS file: /cvs/src/sys/dev/isa/asmc.c,v
retrieving revision 1.28
diff -u -p -r1.28 asmc.c
--- sys/dev/isa/asmc.c 27 Dec 2015 20:54:53 -0000 1.28
+++ sys/dev/isa/asmc.c 27 Jan 2016 02:43:15 -0000
@@ -26,6 +26,8 @@
#include <sys/rwlock.h>
#include <sys/task.h>
#include <sys/sensors.h>
+#include <sys/ioctl.h>
+#include <dev/isa/asmc.h>

#include <machine/bus.h>

@@ -705,4 +707,47 @@ asmc_update(void *arg)
if (!(sc->sc_sensor_motion[i].flags & SENSOR_FINVALID))
asmc_motion(sc, i, 0);
#endif
+}
+
+int
+asmcopen(dev_t dev, int flag, int mode, struct proc *p)
+{
+ /* FIXME */
+ return 0;
+}
+
+int
+asmcclose(dev_t dev, int flag, int mode, struct proc *p)
+{
+ /* FIXME */
+ return 0;
+}
+
+/*
+ * An ioctl interface to poke SMC ``keys'' from userspace.
+ */
+int
+asmcioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, struct proc *p)
+{
+ struct asmc_softc *sc;
+ struct asmc_key_buf *buf = (struct asmc_key_buf *)addr;
+ int error;
+
+ sc = asmc_cd.cd_devs[minor(dev)];
+
+ switch (cmd) {
+ case ASMC_READ_KEY:
+ error = asmc_try(sc, ASMC_READ, buf->key, buf->data, buf->len);
+ break;
+ case ASMC_WRITE_KEY:
+ error = asmc_try(sc, ASMC_WRITE, buf->key, buf->data, buf->len);
+ break;
+ default:
+ return ENOTTY;
+ }
+
+ if (error)
+ return EIO;
+ else
+ return 0;
}
Index: sys/dev/isa/asmc.h
===================================================================
RCS file: sys/dev/isa/asmc.h
diff -N sys/dev/isa/asmc.h
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ sys/dev/isa/asmc.h 27 Jan 2016 02:43:15 -0000
@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) 2016 Sviatoslav Chagaev <***@gmail.com>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#ifndef _ASMC_H_
+#define _ASMC_H_
+
+#include <sys/types.h>
+#include <sys/ioctl.h>
+
+struct asmc_key_buf {
+ char key[4];
+ size_t len;
+ uint8_t data[32];
+};
+
+#define ASMC_READ_KEY _IOWR('a', 1, struct asmc_key_buf)
+#define ASMC_WRITE_KEY _IOWR('a', 2, struct asmc_key_buf)
+
+#endif
Index: etc/etc.amd64/MAKEDEV
===================================================================
RCS file: /cvs/src/etc/etc.amd64/MAKEDEV,v
retrieving revision 1.103
diff -u -p -r1.103 MAKEDEV
--- etc/etc.amd64/MAKEDEV 21 Dec 2015 22:23:24 -0000 1.103
+++ etc/etc.amd64/MAKEDEV 27 Jan 2016 02:43:16 -0000
@@ -94,6 +94,7 @@
# video* Video V4L2 devices
# vmm Virtual Machine Monitor
# vscsi* Virtual SCSI controller
+# asmc* Apple SMC
PATH=/sbin:/usr/sbin:/bin:/usr/bin
T=$0

@@ -242,6 +243,10 @@ std)
ttyc*)
M ttyc$U c 38 $U 660 dialer uucp
M cuac$U c 38 $(($U+128)) 660 dialer uucp
+ ;;
+
+asmc)
+ M asmc$U c 95 $U
;;

vscsi*)
Index: etc/etc.amd64/MAKEDEV.md
===================================================================
RCS file: /cvs/src/etc/etc.amd64/MAKEDEV.md,v
retrieving revision 1.62
diff -u -p -r1.62 MAKEDEV.md
--- etc/etc.amd64/MAKEDEV.md 21 Dec 2015 22:15:53 -0000 1.62
+++ etc/etc.amd64/MAKEDEV.md 27 Jan 2016 02:43:16 -0000
@@ -20,7 +20,9 @@ dnl
dnl
__devitem(apm, apm, Power Management Interface)dnl
__devitem(nvram, nvram, NVRAM access)dnl
+__devitem(asmc, asmc*, Apple SMC)dnl
_mkdev(nvram, nvram, {-M nvram c major_nvram_c 0 440 kmem-})dnl
+_mkdev(asmc, asmc, {-M asmc$U c major_asmc_c $U-})dnl
_TITLE(make)
_DEV(all)
_DEV(ramdisk)
@@ -92,6 +94,7 @@ _DEV(uk, 20)
_DEV(vi, 44)
_DEV(vmm, 10)
_DEV(vscsi, 89)
+_DEV(asmc, 95)
dnl
divert(__mddivert)dnl
dnl
Index: usr.sbin/smcdumpkeys/Makefile
===================================================================
RCS file: usr.sbin/smcdumpkeys/Makefile
diff -N usr.sbin/smcdumpkeys/Makefile
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ usr.sbin/smcdumpkeys/Makefile 27 Jan 2016 02:43:22 -0000
@@ -0,0 +1,5 @@
+PROG= smcdumpkeys
+MAN=
+CFLAGS+= -Wall
+
+.include <bsd.prog.mk>
Index: usr.sbin/smcdumpkeys/smcdumpkeys.c
===================================================================
RCS file: usr.sbin/smcdumpkeys/smcdumpkeys.c
diff -N usr.sbin/smcdumpkeys/smcdumpkeys.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ usr.sbin/smcdumpkeys/smcdumpkeys.c 27 Jan 2016 02:43:22 -0000
@@ -0,0 +1,203 @@
+/*
+ * Copyright (c) 2016 Sviatoslav Chagaev <***@gmail.com>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+/*
+ * Dump the ``key'' table in an Apple System Management Controller (SMC)
+ * firmware to standard output.
+ */
+
+#include <unistd.h>
+#include <err.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <ctype.h>
+#include <stdint.h>
+#include <stdbool.h>
+
+#define _STR(x) #x
+#define STR(x) _STR(x)
+
+#define FIRST_KEY "#KEY"
+
+struct key_record {
+ uint8_t name[4];
+ uint8_t unknown1[4];
+ uint8_t type[4];
+ uint8_t unknown2[4];
+};
+#define FIELD_LEN 4
+#define KEY_NAME_FMT "%-" STR(FIELD_LEN) "." STR(FIELD_LEN) "s"
+#define KEY_TYPE_FMT KEY_NAME_FMT
+
+void smcdumpkeys(const char *);
+void parse(const char *, uint8_t **, size_t *);
+void *xrealloc(void *, size_t);
+bool is_key(const uint8_t *, const uint8_t *);
+
+int
+main(int argc, char **argv)
+{
+ while (++argv, --argc)
+ smcdumpkeys(*argv);
+
+ return 0;
+}
+
+void *
+find_first_key_record(uint8_t *p, uint8_t *end)
+{
+ /* Find the first key's record. */
+ while (p < end) {
+ p = memchr(p, FIRST_KEY[0], end - p);
+ if (p == NULL || end - p < FIELD_LEN)
+ errx(1, "first key record not found");
+ if (memcmp(p, FIRST_KEY, FIELD_LEN) == 0)
+ break;
+ ++p;
+ }
+
+ return p;
+}
+
+bool
+is_key_record(struct key_record *r)
+{
+ unsigned i;
+
+ for (i = 0; i < FIELD_LEN; i++)
+ if (!isprint(r->name[i]))
+ return false;
+ return true;
+}
+
+void
+print_unknown_field(uint8_t *p)
+{
+ unsigned i;
+
+ for (i = 0; i < FIELD_LEN; i++)
+ printf("%02x", (unsigned)*p++);
+}
+
+void
+smcdumpkeys(const char *path)
+{
+ uint8_t *fwdata = NULL;
+ size_t fwdata_size = 0;
+ uint8_t *p;
+ size_t n_records;
+ struct key_record *r;
+ struct key_record *end;
+
+ /* Parse SMC firmware file into binary form. */
+ parse(path, &fwdata, &fwdata_size);
+
+ p = find_first_key_record(fwdata, fwdata + fwdata_size);
+ n_records = (p - fwdata) / sizeof(*r);
+
+ r = (struct key_record *)p;
+ end = r + n_records;
+
+ /* Keep dumping records as long as they look like key records. */
+ while (r < end && is_key_record(r)) {
+ printf(KEY_NAME_FMT " ", r->name);
+ print_unknown_field(r->unknown1);
+ printf(" " KEY_TYPE_FMT " ", r->type);
+ print_unknown_field(r->unknown2);
+ printf("\n");
+ r++;
+ }
+}
+
+void
+parse(const char *path, uint8_t **fwdatap, size_t *fwdata_sizep)
+{
+ FILE *fp;
+ char line[1024];
+ char *linep;
+ unsigned line_no = 0;
+ unsigned byte;
+ uint8_t *fwdata = NULL;
+ size_t fwdata_size = 0;
+
+ fp = fopen(path, "r");
+ if (fp == NULL)
+ err(1, "%s: failed to open", path);
+
+ while (fgets(line, sizeof line, fp) != NULL) {
+ line_no++;
+
+ if (line[0] != 'D' && line[0] != '+')
+ continue;
+
+ if (strchr(line, '\n') == NULL
+ && strlen(line) == sizeof(line) - 1) {
+ warnx("%s: line %u: line possibly truncated",
+ path, line_no);
+ }
+
+ linep = strrchr(line, ':');
+ if (linep == NULL) {
+ errx(1,
+ "%s: line %u: second ``:'' data separator not found",
+ path, line_no);
+ }
+ *linep = '\0';
+
+ linep = strrchr(line, ':');
+ if (linep == NULL) {
+ errx(1,
+ "%s: line %u: first ``:'' data separator not found",
+ path, line_no);
+ }
+ ++linep;
+
+
+ if (strlen(linep) % 2 != 0) {
+ errx(1, "%s: line %u: data string is not of even length",
+ path, line_no);
+ }
+
+ while (*linep != '\0') {
+ if (sscanf(linep, "%2X", &byte) != 1) {
+ errx(1, "%s: line %u: non-hex characters",
+ path, line_no);
+ }
+ fwdata = xrealloc(fwdata, fwdata_size + 1);
+ fwdata[fwdata_size++] = byte;
+ linep += 2;
+ }
+ }
+ if (!feof(fp))
+ err(1, "%s: read error", path);
+
+ *fwdatap = fwdata;
+ *fwdata_sizep = fwdata_size;
+
+ fclose(fp);
+}
+
+void *
+xrealloc(void *ptr, size_t size)
+{
+ void *tmp;
+
+ tmp = realloc(ptr, size);
+ if (tmp == NULL)
+ err(1, "failed to allocate %lu bytes", (unsigned long)size);
+ return tmp;
+}
Index: usr.sbin/smcprobe/Makefile
===================================================================
RCS file: usr.sbin/smcprobe/Makefile
diff -N usr.sbin/smcprobe/Makefile
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ usr.sbin/smcprobe/Makefile 27 Jan 2016 02:43:22 -0000
@@ -0,0 +1,5 @@
+PROG= smcprobe
+MAN=
+CFLAGS+= -Wall
+
+.include <bsd.prog.mk>
Index: usr.sbin/smcprobe/smcprobe.c
===================================================================
RCS file: usr.sbin/smcprobe/smcprobe.c
diff -N usr.sbin/smcprobe/smcprobe.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ usr.sbin/smcprobe/smcprobe.c 27 Jan 2016 02:43:22 -0000
@@ -0,0 +1,75 @@
+/*
+ * Copyright (c) 2016 Sviatoslav Chagaev <***@gmail.com>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+/*
+ * Read/write Apple System Management Cotroller (SMC) ``keys''.
+ */
+
+#include <dev/isa/asmc.h>
+#include <unistd.h>
+#include <err.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+int
+main(int argc, char **argv)
+{
+ struct asmc_key_buf buf;
+ unsigned i;
+ unsigned len;
+ unsigned byte;
+ int fd;
+
+ if (argc < 4) {
+ printf("usage: smcprobe <dev> <key> <len> [<value>]\n");
+ return 0;
+ }
+
+ fd = open(argv[1], O_RDWR);
+ if (fd < 0)
+ err(1, "%s", argv[1]);
+
+ strncpy(buf.key, argv[2], sizeof buf.key);
+ sscanf(argv[3], "%u", &len);
+ buf.len = len;
+
+ if (argc > 4) {
+ for (i = 0; i < len; i++) {
+ if (sscanf(argv[4] + i * 2, "%2x", &byte) != 1)
+ errx(1, "failed to parse value");
+ buf.data[i] = byte;
+ }
+ if (ioctl(fd, ASMC_WRITE_KEY, &buf) < 0)
+ err(1, "ioctl ASMC_WRITE_KEY");
+ } else {
+ if (ioctl(fd, ASMC_READ_KEY, &buf) < 0)
+ err(1, "ioctl ASMC_READ_KEY");
+
+ if (buf.len == 0)
+ printf("buf empty\n");
+ else {
+ for (len = 0; len < buf.len; len++)
+ printf("%02x ", buf.data[len]);
+ printf("\n");
+ }
+ }
+
+ close(fd);
+
+ return 0;
+}
Index: sys/arch/amd64/amd64/conf.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/amd64/conf.c,v
retrieving revision 1.54
diff -u -p -r1.54 conf.c
--- sys/arch/amd64/amd64/conf.c 8 Jan 2016 11:20:58 -0000 1.54
+++ sys/arch/amd64/amd64/conf.c 27 Jan 2016 02:43:23 -0000
@@ -113,6 +113,21 @@ int nblkdev = nitems(bdevsw);
(dev_type_mmap((*))) enodev }

+int asmcopen(dev_t, int, int, struct proc *);
+int asmcclose(dev_t, int, int, struct proc *);
+int asmcioctl(dev_t, u_long, caddr_t, int, struct proc *);
+
+/* open, close, ioctl */
+#define cdev_asmc_init(c,n) { \
+ dev_init(c,n,open), \
+ dev_init(c,n,close), \
+ (dev_type_read((*))) enodev, \
+ (dev_type_write((*))) enodev, \
+ dev_init(c,n,ioctl), \
+ (dev_type_stop((*))) enodev, 0, seltrue, \
+ (dev_type_mmap((*))) enodev }
+
+
#define mmread mmrw
#define mmwrite mmrw
cdev_decl(mm);
@@ -292,6 +307,7 @@ struct cdevsw cdevsw[] =
cdev_fuse_init(NFUSE,fuse), /* 92: fuse */
cdev_tun_init(NTUN,tap), /* 93: Ethernet network tunnel */
cdev_tty_init(NVIOCON,viocon), /* 94: virtio console */
+ cdev_asmc_init(1,asmc), /* 95: Apple SMC */
};
int nchrdev = nitems(cdevsw);

Joerg Jung

2016-01-30 13:01:50 UTC

Permalink

Post by Sviatoslav Chagaev

Post by Joerg Jung

Post by Sviatoslav Chagaev
So below is a driver for TI LP8550.

...

Post by Joerg Jung

Post by Sviatoslav Chagaev
If there's any chance for it to be commited, please tell me what I need to
fix/improve to get it commited (so I don't have to reapply it every time I
upgrade).

IMHO, instead of adding another driver to the mix, I would prefer this
to be handled through the associated asmc(4) keys instead of accessing
the chip directly. The SMC is supposed to be the central point for such
manipulations. Unfortunately, the keys are not documented and need some
non-trivial effort to be figured out.
If this is not possible through asmc(4) and if your new driver improves
the situation then I'm fine with importing it, but for now it is just
worse.

Using knowledge acquired from [1]

[1] contains some "wrong" (obsolete?) information, e.g. the Errors list.

Post by Sviatoslav Chagaev
and [2] made a diff, posted below for
* ioctl interface to asmc(4) to allow reading/writing of ``keys'' from
userspace;
* companion smcprobe(1) which reads/writes ``keys'' using the ioctl;

Wow..., much effort for simple key dumping. Please, find below an
(older/may not apply) diff which just dumps the keys directly from
kernel into dmesg.

Are you aware that keys can be all kind of types, e.g. structs? Have
you seen the ASMC_INFO (0x13) command which might be used to determine
the type? The key flags (read/write/atomic...) are mostly documented
somewhere.

Post by Sviatoslav Chagaev
* smcdumpkeys(1): scans an SMC textual firmware file and dumps the ``key''
table that it contains.

Are you aware of the ASMC_INDEX (0x12) command, which can easily be used
to just iterate ALL keys based on index?

Post by Sviatoslav Chagaev
Downloaded SMC FW [3], unpacked it using Google Drive and used smcdumpkeys(1) +
smcprobe(1) to create the table of keys and their current values before suspend
[4] and after wake up [5]. Diffed them [6],

The diff approach is interesting... :) From your diff output I would try
to play with IPBF, MSXk, SAPN, and DM0T.

Post by Sviatoslav Chagaev
filtering out keys which change over
time as they are probably sensors (some might have gotten through).

If they start with:

'A'... usually ambient light sensor related,
'F'... fan sensors,
'T'... temperature sensors,
'P'... power related,
'V'... voltage sensors.

Post by Sviatoslav Chagaev
Tried modifying all keys whose names start with 'B': no effect.

'B'... is likely not "backlight" but "battery".

Post by Sviatoslav Chagaev
Tried modifying keys which are different after wake up: no effect.

Please find a more exhaustive/descriptive list of keys here:
http://web.archive.org/web/20151103144947/http://www.parhelia.ch/blog/statics/k3_keys.html

Have you seen this: https://github.com/gcsgithub/smc_util it contains
more details for the various structs.

You may also want to have a look into the existing FreeBSD and Linux
drivers.

Post by Sviatoslav Chagaev
[1] https://reverse.put.as/wp-content/uploads/2015/12/D1_02_Alex_Ninjas_and_Harry_Potter.pdf
[2] https://dev.inversepath.com/download/public/embedded_systems_exploitation.pdf
[3] https://support.apple.com/kb/DL1748?locale=en_US
[4] https://gist.github.com/S010/26145f4446fcc0b8a0d6#file-before_suspend-txt
[5] https://gist.github.com/S010/26145f4446fcc0b8a0d6#file-after_wakeup-txt
[6] https://gist.github.com/S010/26145f4446fcc0b8a0d6#file-suspend_wakeup_key-diff

Index: asmc.c
===================================================================
RCS file: /cvs/src/sys/dev/isa/asmc.c,v
retrieving revision 1.17
diff -u -p -r1.17 asmc.c
--- asmc.c 12 Dec 2015 12:34:05 -0000 1.17
+++ asmc.c 12 Dec 2015 13:30:12 -0000
@@ -32,6 +32,8 @@
#include <dev/isa/isavar.h>
#include <dev/wscons/wsconsio.h>

+#define ASMC_DEBUG 1
+
#define ASMC_BASE 0x300 /* SMC base address */
#define ASMC_IOSIZE 32 /* I/O region size 0x300-0x31f */

@@ -42,6 +44,7 @@

#define ASMC_READ 0x10 /* SMC read command */
#define ASMC_WRITE 0x11 /* SMC write command */
+#define ASMC_INDEX 0x12 /* SMC index command */
#define ASMC_INFO 0x13 /* SMC info/type command */

#define ASMC_RETRY 10
@@ -85,6 +88,9 @@ struct asmc_softc {
};

int asmc_try(struct asmc_softc *, int, const char *, uint8_t *, uint8_t);
+#ifdef ASMC_DEBUG
+void asmc_dump(struct asmc_softc *, uint32_t);
+#endif

void asmc_init(void *);
void asmc_refresh(void *);
@@ -292,7 +298,9 @@ asmc_attach(struct device *parent, struc
}
printf(", %u key%s\n", ntohl(*(uint32_t *)buf),
(ntohl(*(uint32_t *)buf) == 1) ? "" : "s");
-
+#ifdef ASMC_DEBUG
+ asmc_dump(sc, ntohl(*(uint32_t *)buf));
+#endif
/* keyboard backlight led is optional */
sc->sc_kbdled = buf[0] = 127, buf[1] = 0;
if ((r = asmc_try(sc, ASMC_WRITE, "LKSB", buf, 2))) {
@@ -479,25 +487,25 @@ asmc_command(struct asmc_softc *sc, int
if (len > ASMC_MAXLEN)
return 1;
if (asmc_write(sc, ASMC_COMMAND, cmd))
- return 1;
+ return 2;
for (i = 0; i < 4; i++)
if (asmc_write(sc, ASMC_DATA, key[i]))
- return 1;
+ return 3;
if (asmc_write(sc, ASMC_DATA, len))
- return 1;
- if (cmd == ASMC_READ || cmd == ASMC_INFO) {
+ return 4;
+ if (cmd == ASMC_READ || cmd == ASMC_INDEX || cmd == ASMC_INFO) {
for (i = 0; i < len; i++)
if (asmc_read(sc, ASMC_DATA, &buf[i]))
- return 1;
+ return 5;
for (i = 0; i < ASMC_MAXLEN; i++) /* sanity flush */
if (asmc_read(sc, ASMC_DATA, &n))
break;
} else if (cmd == ASMC_WRITE) {
for (i = 0; i < len; i++)
if (asmc_write(sc, ASMC_DATA, buf[i]))
- return 1;
+ return 6;
} else
- return 1;
+ return 7;
return 0;
}

@@ -750,3 +758,40 @@ asmc_update(void *arg)
if (sc->sc_init)
task_add(sc->sc_taskq, &sc->sc_task_refresh);
}
+
+#ifdef ASMC_DEBUG
+void
+asmc_dump(struct asmc_softc *sc, uint32_t len)
+{
+ uint8_t buf[4], key[4];
+ uint32_t i;
+ int r, e;
+
+ for (i = 0; i < len; i++, e = 0) {
+ *(uint32_t *)buf = htonl(i);
+ printf("%s: dump key[%u]", sc->sc_dev.dv_xname, i);
+retry:
+ if ((r = asmc_try(sc, ASMC_INDEX, buf, key, 4))) {
+ printf(" failed (0x%x)\n", r);
+ continue;
+ }
+ if (!key[0] || !strncmp(key, "Pz0F", 4)) {
+ if (++e < ASMC_RETRY) {
+ delay(2 << 16);
+ goto retry;
+ }
+ printf(" *");
+ }
+ printf(" '%c%c%c%c'", key[0], key[1], key[2], key[3]);
+ if (key[0] == 'T' || key[0] == 't') {
+ delay(2 << 16);
+ if (!(r = asmc_try(sc, ASMC_READ, key, buf, 2)))
+ printf(" t: %hd",
+ (((int16_t)ntohs(*(uint16_t *)buf)) >> 8));
+ printf(" (0x%x)", r);
+ }
+ printf("\n");
+ delay(2 << 16);
+ }
+}
+#endif