> Date: Sun, 18 Sep 2022 01:53:52 +0200 > From: Tobias Heider <[email protected]> > > Hi, > > below is a diff for a new driver to control the screen backlight on > Apple Silicon laptops via 'gpio-backlight'. This allows disabling the > screen via wsconsctl or hotkeys once they are hooked up. > The task is needed because aplsmc(4) sleeps in aplsmc_set_pin() which > crashes when called via hotkey. > > ok?
Hi Tobias, The backlight GPIO only allows us to turn the backlight on and off. Therefore I think it makes more sense to hook this up to the "burner". That way we can make the display turn off automatically after the machine has not been used for a while if you use wsconsctl(4) to set display.screen_off to a non-zero value. This should also make screen blanking in X work. A few more notes below... > diff --git sys/arch/arm64/conf/GENERIC sys/arch/arm64/conf/GENERIC > index bc88d2c9a59..c4a80a84de6 100644 > --- sys/arch/arm64/conf/GENERIC > +++ sys/arch/arm64/conf/GENERIC > @@ -138,6 +138,7 @@ amdgpu* at pci? > drm* at amdgpu? > wsdisplay* at amdgpu? > > +gpiobl* at fdt? > gpiocharger* at fdt? > gpiokeys* at fdt? > gpioleds* at fdt? > diff --git sys/dev/fdt/files.fdt sys/dev/fdt/files.fdt > index 6a721edfa3e..3bf3ca4f26e 100644 > --- sys/dev/fdt/files.fdt > +++ sys/dev/fdt/files.fdt > @@ -601,6 +601,10 @@ device dapmic > attach dapmic at i2c > file dev/fdt/dapmic.c dapmic > > +device gpiobl > +attach gpiobl at fdt > +file dev/fdt/gpiobl.c gpiobl > + > device gpiocharger > attach gpiocharger at fdt > file dev/fdt/gpiocharger.c gpiocharger > diff --git sys/dev/fdt/gpiobl.c sys/dev/fdt/gpiobl.c > new file mode 100644 > index 00000000000..f9774df20f2 > --- /dev/null > +++ sys/dev/fdt/gpiobl.c > @@ -0,0 +1,127 @@ > +/* $OpenBSD$ */ > +/* > + * Copyright (c) 2022 Tobias Heider <[email protected]> > + * > + * 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 <sys/malloc.h> > +#include <sys/task.h> > + > +#include <machine/fdt.h> > +#include <machine/bus.h> > + > +#include <dev/ofw/openfirm.h> > +#include <dev/ofw/ofw_gpio.h> > +#include <dev/ofw/ofw_misc.h> > + > +#include <dev/wscons/wsconsio.h> > +#include <dev/wscons/wsdisplayvar.h> > + > +struct gpiobl_softc { > + struct device sc_dev; > + int sc_on; > + uint32_t sc_gpio[3]; > + struct task sc_task; > +}; > + > +struct gpiobl_softc *sc_gpiobl; > + > +int gpiobl_match(struct device *, void *, void *); > +void gpiobl_attach(struct device *, struct device *, void *); > + > +const struct cfattach gpiobl_ca = { > + sizeof(struct gpiobl_softc), gpiobl_match, gpiobl_attach > +}; > + > +struct cfdriver gpiobl_cd = { > + NULL, "gpiobl", DV_DULL > +}; > + > +void gpiobl_task(void *); > +int gpiobl_get_param(struct wsdisplay_param *); > +int gpiobl_set_param(struct wsdisplay_param *); > + > +int > +gpiobl_match(struct device *parent, void *match, void *aux) > +{ > + struct fdt_attach_args *faa = aux; > + > + return OF_is_compatible(faa->fa_node, "gpio-backlight"); > +} > + > + > +void > +gpiobl_attach(struct device *parent, struct device *self, void *aux) > +{ > + struct gpiobl_softc *sc = (struct gpiobl_softc *)self; > + struct fdt_attach_args *faa = aux; > + size_t len; > + > + len = OF_getproplen(faa->fa_node, "gpios"); > + if (len <= 0) > + return; > + OF_getpropintarray(faa->fa_node, "gpios", sc->sc_gpio, len); Need to be careful here that we don't overflow the buffer. Using sizeof(sc->sc_gpio) is probably better. Since we support platforms that use 4 cells for gpio decscriptors, make the array 4 elements instead of 3. > + gpio_controller_config_pin(sc->sc_gpio, GPIO_CONFIG_OUTPUT); > + > + sc->sc_on = OF_getpropbool(faa->fa_node, "default-on"); > + sc_gpiobl = sc; Reading Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml in the linux tree, it seems that "default-on" means we should *enable* the backlight at boot. Although that doesn't seem to be what the Linux driver does... Not sure what to do here. Maybe leave this as you have it now until we have other hardware that uses this driver. > + > + task_set(&sc->sc_task, gpiobl_task, sc); > + ws_get_param = gpiobl_get_param; > + ws_set_param = gpiobl_set_param; > + printf("\n"); > +} > + > +void > +gpiobl_task(void *args) > +{ > + struct gpiobl_softc *sc = args; > + gpio_controller_set_pin(&sc->sc_gpio[0], sc->sc_on); > +} > + > +int > +gpiobl_get_param(struct wsdisplay_param *dp) > +{ > + struct gpiobl_softc *sc = (struct gpiobl_softc *)sc_gpiobl; > + > + switch (dp->param) { > + case WSDISPLAYIO_PARAM_BRIGHTNESS: > + dp->min = 0; > + dp->max = 1; > + dp->curval = sc->sc_on; > + return 0; > + default: > + return -1; > + } > +} > + > +int > +gpiobl_set_param(struct wsdisplay_param *dp) > +{ > + struct gpiobl_softc *sc = (struct gpiobl_softc *)sc_gpiobl; > + > + switch (dp->param) { > + case WSDISPLAYIO_PARAM_BRIGHTNESS: > + if (dp->curval == sc->sc_on) > + return 0; > + sc->sc_on = !sc->sc_on; > + task_add(systq, &sc->sc_task); > + return 0; > + default: > + return -1; > + } > +} >
