Brightness change on notebook that follow ACPI specification (par. B.7)
Dan Lukes
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.
No test on other OS version, but verified that patched module can be
compiled on 6.4-R, 7.2-R, 8.0-R. I assume (not verified) it's compilable
on all 6.x-8.x. I assume (not verified) it should work on all of these
versions.
If someone want to test it then the patch is here:
http://www.freebsd.cz/~dan/patch-acpi-video
installation:
cd /usr/src ; patch < patch-acpi-video
then compile the module:
http://www.freebsd.org/doc/handbook/acpi-debug.html#ACPI-DEBUGOUTPUT
then kldload acpi_video
then try keys for brightness change.
I'm interested to know on which notebook and FreeBSD version is verified
to be (not)working.
If it doesn't work it may be either:
1. bug in my patch
2. your notebook doesn't follow ACPI specification (par. B.7)
If you are skilled user and you want to know more (and you have compiled
ACPI modules with ACPI_DEBUG) you can set
debug.acpi.level="ACPI_LV_ALL_EXCEPTIONS"
then look on console messages what happens when you press the special keys.
Dan
P.S. Brief info how it works:
Specification claim the ACPI will send notification of the type
0x85-0x88 (brightness cycle/inc/dec/zero) to the output device handler.
It's catched by my code (in acpi_video module) so I know what key user
pressed. With such knowledge and current brightness (obtained from _BCQ
method evaluation) and list of allowed brightnesses (obtained from _BCL
method evaluation), I can calculate new brightness. It's set using _BCM
method. No more fun here ...
Dan Lukes
>> 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.
> I see nothing wrong with this patch and I think it can be committed, but I would like others to take a look.
Thanks. Code review needs to be done also, but it would be nice to have
more "yes, it works with my NB and OS ?.??" reports at this phase ...
Did you tried it ? Which notebook and FreeBSD version ?
Dan
Rui Paulo
On 30 Mar 2010, at 04:32, Dan Lukes wrote:
>
> 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.
>
> No test on other OS version, but verified that patched module can be compiled on 6.4-R, 7.2-R, 8.0-R. I assume (not verified) it's compilable on all 6.x-8.x. I assume (not verified) it should work on all of these versions.
>
> If someone want to test it then the patch is here:
> http://www.freebsd.cz/~dan/patch-acpi-video
>
> installation:
> cd /usr/src ; patch < patch-acpi-video
> then compile the module:
> http://www.freebsd.org/doc/handbook/acpi-debug.html#ACPI-DEBUGOUTPUT
> then kldload acpi_video
> then try keys for brightness change.
>
> I'm interested to know on which notebook and FreeBSD version is verified to be (not)working.
>
> If it doesn't work it may be either:
> 1. bug in my patch
> 2. your notebook doesn't follow ACPI specification (par. B.7)
>
> If you are skilled user and you want to know more (and you have compiled ACPI modules with ACPI_DEBUG) you can set
> debug.acpi.level="ACPI_LV_ALL_EXCEPTIONS"
> then look on console messages what happens when you press the special keys.
>
> Dan
I see nothing wrong with this patch and I think it can be committed, but I would like others to take a look.
--
Rui Paulo
Rui Paulo
I haven't tried it.
--
Rui Paulo
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.
--
John Baldwin
Dan Lukes
>> 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.
>>
>>
>> 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 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).
--- 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.
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]).
The 0x88 handling is duplicate implementation of
acpi_video_vo_check_level instead just calling the already implemented
function.
------------------------
Such notices should not be considered offence against the autor nor
committer in any way. Just my $0.02 related to the code.
Dan
John Baldwin
> 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'm not sure if there was a PR. I do recall a thread on the acpi@ mailing
list when the patch was first submitted.
These all sound like good fixes to me. If you can code up a patch that
implements them I would be happy to test it. (My netbook only has keys
for up and down, so I can't test changes for 0x85 and 0x88.)
--
John Baldwin
Dan Lukes
>>>> It's verified to work on my HP Mini 5101& FreeBSD 8.0.
>> --- 1 ---------------
>> --- 2 ---------------
> These all sound like good fixes to me. If you can code up a patch
Sure, but not today nor tomorrow. At the first I need to install
8-STABLE and before them I need sleep sometimes ... ;-)
Dan
Jung-uk Kim
> 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
Dan Lukes
>> --- 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.
Of course. Language problem. I'm speaking about the meaning of the
event. We are speaking about range of "device specific" events and they
are different meaning for every type of devices.
>> 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.
There is no "only" word in mentioned sequence. They are list of levels
the OSPM will cycle through when ..., but sentence doesn't say that the
levels come from such list only. It's sounds like your over-assumption
of text rather than "close as possible" implementation. Or, in the world
of mathematics symbols, '=>' operator is not '<=>'.
I interpret whole _BCL list as list of levels. Yes, first two levels
have additional meaning, but it doesn't mean they lost the basic attribute.
You should look into description of event, e.g. table B-7 which sounds
very clear to me. It doesn't distinquish between first two levels in the
list and other levels in the list.
But I'm not native english speaker, so it's hard to dispute about exact
meaning of the specification text and meaning of wording in the
sentences for me.
In the fact, I can say for sure only I see nothing in the specification,
that can be interpret as "with only three levels in _BCL you can't cycle
trough it".
>> 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. :-)
I'm sayed the actual implementation looks to me like not to be optimal
approach. Nothing more...
>> 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.
So you implemented the functionality you already have just few lines
above because it looks more uniform when printed on paper ? Well, it's
not my way, but it's your code and it works, so it's between you and
comitter - and because you are comitter, then it's between you and you ;-)
Dan
P.S. Don't forget the english is not my native language. It doesn't
affect my interpertation of the specification text only but such
disputation as well.