Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 30 Mar 2010 19:32:09 -0400
From:      Jung-uk Kim <jkim@FreeBSD.org>
To:        freebsd-acpi@FreeBSD.org
Subject:   Re: Brightness change on notebook that follow ACPI specification (par. B.7)
Message-ID:  <201003301932.11258.jkim@FreeBSD.org>
In-Reply-To: <4BB2515D.1060808@obluda.cz>
References:  <4BB170E7.3030407@obluda.cz> <201003301011.40215.jhb@freebsd.org> <4BB2515D.1060808@obluda.cz>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tuesday 30 March 2010 03:30 pm, Dan Lukes wrote:
> On 03/30/10 16:11, John Baldwin:
> >> Most notebooks have special keys (mostly Fn+something) to change
> >> brightness of LCD display.
> >>
> >> Some of them (my notebok, for example) follows the ACPI
> >> specification (paragraph B.7) how to announce the user request
> >> for brightness change to OS.
> >>
> >> I implemented such handling as part of acpi-video module.
> >>
> >> It's verified to work on my HP Mini 5101&  FreeBSD 8.0.
> >>
> >> No test on other notebook done as I have no other notebook.
> >
> > A patch to implement the brightness controls was already
> > committed to HEAD a while ago and have already been merged to
> > 8-stable and 7-stable.  Can you test the a kernel from 8-stable
> > to ensure it works ok on your machine?  The patches in HEAD work
> > fine on an HP Mini 2140 that I have here.
>
> I'm sure there has been a regular PR before commitment assigned to
> ACPI group for review, but I missed them. It resulted in waste of
> time. My fault.

I wrote it based on Daniel Walter's initial work.

> I will test it later (I have no 8-STABLE here), I'm almost sure it
> will work (based on code review).
>
> There are several notices related to coding of the committed patch:
>
>   --- 1 ---------------
> /* events */
> #define	VID_NOTIFY_SWITCHED	0x80
> #define	VID_NOTIFY_REPROBE	0x81
> #define	VID_NOTIFY_CYCLE_BRN	0x85
> #define	VID_NOTIFY_INC_BRN	0x86
> #define	VID_NOTIFY_DEC_BRN	0x87
> #define	VID_NOTIFY_ZERO_BRN	0x88
>
> The events 0x80 and 0x81 are "Display Devices" device specific
> notifications (according ACPI specification B.5), but events
> 0x85-0x88 are "Output devices" device specific notification (ACPI
> spec. B.7). The common name VID_NOTIFY_* for values that come from
> different domains is confusing. Note that future versions of ACPI
> specification may overlaps (there may be defined 0x80 event for
> Output device or event 0x85 event for Display device).

I didn't think it was necessary because ACPI will not define 
overlapping events, AFAIK.

>   --- 2 ---------------
> The code of acpi_video_vo_notify_handler():
>
> Handling of event 0x85 refuse do anything when there are less than
> four levels. I see no reason for such limitation - we can safely
> cycle trougth three or even trougth two levels as well.

B.6.2 _BCL (Query List of Brightness Control Levels Supported)

The first number in the package is the level of the panel when full
power is connected to the machine. The second number in the package is
the level of the panel when the machine is on batteries. All other
numbers are treated as a list of levels OSPM will cycle through when
the user toggles (via a keystroke) the brightness level of the 
display.

Please note the last sentence.  I know a lot of BIOSes do not follow 
the rule but I wanted to implement it as close as possible.

> It advance, the 0x85 handling assume the _BCL list is sorted and
> ignores vo_levels[0] and vo_levels[1] (note that handling of
> 0x86/0x87 just few lines bellow doesn't assume sorted levels nor
> ignore levels [0] and [1]).

It was intentional, i.e., I didn't want to implement increase/decrease 
via cycle up/down or to miss the first two levels.  The spec. was 
little vague about those cases and I abused it. :-)

> The 0x88 handling is duplicate implementation of
> acpi_video_vo_check_level instead just calling the already
> implemented function.

Again, it was intentional, i.e., to mimic the style of other cases in 
the switch statement and to avoid an unnecessary assertion.

>   ------------------------
>
> Such notices should not be considered offence against the autor nor
> committer in any way. Just my $0.02 related to the code.

No offense taken.

Thanks,

Jung-uk Kim



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201003301932.11258.jkim>