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:
