On 09/02/16(Tue) 21:06, Christian Weisgerber wrote: > On 2016-02-09, Martin Pieuchot <m...@openbsd.org> wrote: > > > Since brightness support has been added to acpithinkpad(4) I can easily > > trigger a regression on my x220: > > > > - Switching to the first virtual console just after xdm starts using the > > Ctrl+Alt+F1 key combination turns my screen dark. > > - Killing X just after xdm start, by using Ctrl+Alt+Del, also turns my > > screen Dark. > > Yes, I also see this on my X230. > > > The problem seems to be a race related to multiple events, diff below > > fixes that for me. > > With that diff the brightness is stuck at 100%.
The diff is broken. I couldn't spot a way to fix this. Maybe it's a bug in the firmware? For what I could see the intel driver (xf86-video-intel) switches the brightness, called backlight, twice when switching to a console with Ctrl+Alt+Fn. It does the equivalent of: # wsconsctl display.brightness=0 # wsconsctl display.brightness=100 Here are the relevant Xorg.0.log lines: [ 152.114] sna_output_dpms: saving current backlight 15 [ 152.114] sna_output_backlight_set(LVDS1) level=0, max=15 [ 152.387] sna_output_backlight_set(LVDS1) level=15, max=15 Which trigger the following output in dmesg with the diff below: thinkpad_set_brightness: 0 thinkpad_get_brightness: 3840 thinkpad_set_brightness: 15 thinkpad_get_brightness: 3855 At this point the screen is still dark but the system *and* the ACPI notification reports it to be a 100% brightness. Pressing the "brighness up key" produces the following message and the brightness now really is at 100%: thinkpad_get_brightness: 3855 I don't have any more idea about how to solve this regression but it is really annoying. Index: acpithinkpad.c =================================================================== RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v retrieving revision 1.50 diff -u -p -r1.50 acpithinkpad.c --- acpithinkpad.c 17 Dec 2015 12:17:14 -0000 1.50 +++ acpithinkpad.c 10 Feb 2016 18:15:41 -0000 @@ -407,6 +407,7 @@ thinkpad_hotkey(struct aml_node *node, i break; case THINKPAD_BACKLIGHT_CHANGED: thinkpad_get_brightness(sc); + handled = 1; break; case THINKPAD_ADAPTIVE_BACK: case THINKPAD_ADAPTIVE_GESTURES: @@ -653,6 +654,7 @@ thinkpad_get_brightness(struct acpithink { aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG", 0, NULL, &sc->sc_brightness); + printf("%s: %lld\n", __func__, sc->sc_brightness); } void @@ -663,9 +665,11 @@ thinkpad_set_brightness(void *arg0, int 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); + arg.v_integer = arg1; + if (aml_evalname(sc->sc_acpi, sc->sc_devnode, + "PBLS", 1, &arg, NULL)) + return; + printf("%s: %d\n", __func__, arg1); } int @@ -702,9 +706,8 @@ thinkpad_set_param(struct wsdisplay_para 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_addtask(sc->sc_acpi, thinkpad_set_brightness, sc, + dp->curval); acpi_wakeup(sc->sc_acpi); return 0; default: