On Fri, Jan 24, 2020 at 01:04:41AM +0100, Patrick Wildt wrote:
> On Thu, Jan 23, 2020 at 11:25:51PM +0100, Mark Kettenis wrote:
> > Since acpi_set_brightness() can be called from anywhere, it should really
> > use the acpi task queue to do its thing instead of calling AML directly.
>
> I didn't know there's an acpi task queue, so that's awesome! I just saw
> that acpithinkpad(4) also uses that. Changing this diff to do it that
> way was also quite easy, so this should read much better. Feel free to
> give it a go.
>
> ok?
>
> Patrick
Looking for some more testing on this. Successful test results would
help to get this diff in, so I can start sending the next diffs to
finally enable X395 backlight keys support.
Expected behaviour: Backlight keys continue to change the backlight
as well as they did before.
Machines: All those where acpivout(4) attaches.
Patrick
> diff --git a/sys/dev/acpi/acpivout.c b/sys/dev/acpi/acpivout.c
> index 58e8e3d431c..89922f4723b 100644
> --- a/sys/dev/acpi/acpivout.c
> +++ b/sys/dev/acpi/acpivout.c
> @@ -60,14 +60,17 @@ struct acpivout_softc {
>
> int *sc_bcl;
> size_t sc_bcl_len;
> +
> + int sc_brightness;
> };
>
> void acpivout_brightness_cycle(struct acpivout_softc *);
> void acpivout_brightness_step(struct acpivout_softc *, int);
> void acpivout_brightness_zero(struct acpivout_softc *);
> int acpivout_get_brightness(struct acpivout_softc *);
> +int acpivout_select_brightness(struct acpivout_softc *, int);
> int acpivout_find_brightness(struct acpivout_softc *, int);
> -void acpivout_set_brightness(struct acpivout_softc *, int);
> +void acpivout_set_brightness(void *, int);
> void acpivout_get_bcl(struct acpivout_softc *);
>
> /* wconsole hook functions */
> @@ -117,6 +120,8 @@ acpivout_attach(struct device *parent, struct device
> *self, void *aux)
> ws_set_param = acpivout_set_param;
>
> acpivout_get_bcl(sc);
> +
> + sc->sc_brightness = acpivout_get_brightness(sc);
> }
>
> int
> @@ -124,6 +129,9 @@ acpivout_notify(struct aml_node *node, int notify, void
> *arg)
> {
> struct acpivout_softc *sc = arg;
>
> + if (ws_get_param == NULL || ws_set_param == NULL)
> + return (0);
> +
> switch (notify) {
> case NOTIFY_BRIGHTNESS_CYCLE:
> acpivout_brightness_cycle(sc);
> @@ -151,12 +159,13 @@ acpivout_notify(struct aml_node *node, int notify, void
> *arg)
> void
> acpivout_brightness_cycle(struct acpivout_softc *sc)
> {
> - int cur_level;
> + struct wsdisplay_param dp;
>
> - if (sc->sc_bcl_len == 0)
> + dp.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
> + if (ws_get_param(&dp))
> return;
> - cur_level = acpivout_get_brightness(sc);
> - if (cur_level == sc->sc_bcl[sc->sc_bcl_len - 1])
> +
> + if (dp.curval == dp.max)
> acpivout_brightness_zero(sc);
> else
> acpivout_brightness_step(sc, 1);
> @@ -165,33 +174,45 @@ acpivout_brightness_cycle(struct acpivout_softc *sc)
> void
> acpivout_brightness_step(struct acpivout_softc *sc, int dir)
> {
> - int level, nindex;
> + struct wsdisplay_param dp;
> + int delta, new;
>
> - if (sc->sc_bcl_len == 0)
> - return;
> - level = acpivout_get_brightness(sc);
> - if (level == -1)
> + dp.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
> + if (ws_get_param(&dp))
> return;
>
> - nindex = acpivout_find_brightness(sc, level + (dir * BRIGHTNESS_STEP));
> - if (sc->sc_bcl[nindex] == level) {
> - if (dir == 1 && (nindex + 1 < sc->sc_bcl_len))
> - nindex++;
> - else if (dir == -1 && (nindex - 1 >= 0))
> - nindex--;
> + new = dp.curval;
> + delta = ((dp.max - dp.min) * BRIGHTNESS_STEP) / 100;
> + if (dir > 0) {
> + if (delta > dp.max - dp.curval)
> + new = dp.max;
> + else
> + new += delta;
> + } else if (dir < 0) {
> + if (delta > dp.curval - dp.min)
> + new = dp.min;
> + else
> + new -= delta;
> }
> - if (sc->sc_bcl[nindex] == level)
> +
> + if (dp.curval == new)
> return;
>
> - acpivout_set_brightness(sc, sc->sc_bcl[nindex]);
> + dp.curval = new;
> + ws_set_param(&dp);
> }
>
> void
> acpivout_brightness_zero(struct acpivout_softc *sc)
> {
> - if (sc->sc_bcl_len == 0)
> + struct wsdisplay_param dp;
> +
> + dp.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
> + if (ws_get_param(&dp))
> return;
> - acpivout_set_brightness(sc, sc->sc_bcl[0]);
> +
> + dp.curval = dp.min;
> + ws_set_param(&dp);
> }
>
> int
> @@ -211,6 +232,26 @@ acpivout_get_brightness(struct acpivout_softc *sc)
> return (level);
> }
>
> +int
> +acpivout_select_brightness(struct acpivout_softc *sc, int nlevel)
> +{
> + int nindex, level;
> +
> + level = sc->sc_brightness;
> + if (level == -1)
> + return level;
> +
> + nindex = acpivout_find_brightness(sc, nlevel);
> + if (sc->sc_bcl[nindex] == level) {
> + if (nlevel > level && (nindex + 1 < sc->sc_bcl_len))
> + nindex++;
> + else if (nlevel < level && (nindex - 1 >= 0))
> + nindex--;
> + }
> +
> + return nindex;
> +}
> +
> int
> acpivout_find_brightness(struct acpivout_softc *sc, int level)
> {
> @@ -230,15 +271,16 @@ acpivout_find_brightness(struct acpivout_softc *sc, int
> level)
> }
>
> void
> -acpivout_set_brightness(struct acpivout_softc *sc, int level)
> +acpivout_set_brightness(void *arg0, int arg1)
> {
> + struct acpivout_softc *sc = arg0;
> struct aml_value args, res;
>
> memset(&args, 0, sizeof(args));
> - args.v_integer = level;
> + args.v_integer = sc->sc_brightness;
> args.type = AML_OBJTYPE_INTEGER;
>
> - DPRINTF(("%s: BCM = %d\n", DEVNAME(sc), level));
> + DPRINTF(("%s: BCM = %d\n", DEVNAME(sc), sc->sc_brightness));
> aml_evalname(sc->sc_acpi, sc->sc_devnode, "_BCM", 1, &args, &res);
>
> aml_freevalue(&res);
> @@ -304,12 +346,9 @@ acpivout_get_param(struct wsdisplay_param *dp)
> }
> if (sc != NULL && sc->sc_bcl_len != 0) {
> dp->min = 0;
> - dp->max = sc->sc_bcl[sc->sc_bcl_len - 1];
> - rw_enter_write(&sc->sc_acpi->sc_lck);
> - dp->curval = acpivout_get_brightness(sc);
> - rw_exit_write(&sc->sc_acpi->sc_lck);
> - if (dp->curval != -1)
> - return 0;
> + dp->max = sc->sc_bcl[sc->sc_bcl_len - 1];
> + dp->curval = sc->sc_brightness;
> + return 0;
> }
> return -1;
> default:
> @@ -334,11 +373,14 @@ acpivout_set_param(struct wsdisplay_param *dp)
> break;
> }
> if (sc != NULL && sc->sc_bcl_len != 0) {
> - rw_enter_write(&sc->sc_acpi->sc_lck);
> - nindex = acpivout_find_brightness(sc, dp->curval);
> - acpivout_set_brightness(sc, sc->sc_bcl[nindex]);
> - rw_exit_write(&sc->sc_acpi->sc_lck);
> - return 0;
> + nindex = acpivout_select_brightness(sc, dp->curval);
> + if (nindex != -1) {
> + sc->sc_brightness = sc->sc_bcl[nindex];
> + acpi_addtask(sc->sc_acpi,
> + acpivout_set_brightness, sc, 0);
> + acpi_wakeup(sc->sc_acpi);
> + return 0;
> + }
> }
> return -1;
> default:
