On Wed, Feb 10, 2016 at 07:10:41PM GMT, Martin Pieuchot wrote:
> 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.
Hi Martin,
Are you sure that this is Thinkpad-specific?
The reason why I'm asking is because I can see something similar on my
Sony Vaio VGN-Z11WN_B laptop when, after logging onto console, starting
X using startx, then quitting X (cwm in this case) gets me back to the
console with brightness set to 64% - subsequently brightness stays at
64% after I log onto X again. There, if I run `xbacklight -set 100`,
brightness goes back to 100% *but* if I quit X again, the screen goes
dark again (presumably to said 64%) *and* `wsconstl display.brightness`
reports 100% (sic!). I have to run `wsconstl display.brightness=100` to
bring it back to 100% brightness - this happens *every* time I quit X.
Also, a very strange behaviour: the brightness up key combination,
doesn't do anything but, when pressing the brightness down
key, the screen goes bright (to more like 99% since `wsconstl
display.brightness=100` still makes it a bit brighter) on first key
press, and then down (as one might expect) on subsequent key presses.
If it is just a coincidence, and you are looking at a different bug,
then I'll report a new one, otherwise, I can provide more information
(full dmesg, etc.) - simply don't want to hijack the thread (might have
done so already - sorry).
Running the newest snapshot, BTW:
OpenBSD 5.9 (GENERIC.MP) #1868: Mon Feb 1 20:02:36 MST 2016
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
Regards,
Raf
> 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:
>
