Date: Tue, 30 Mar 2010 16:12:18 -0400 From: John Baldwin <jhb@freebsd.org> To: Dan Lukes <dan@obluda.cz> Cc: freebsd-acpi@freebsd.org Subject: Re: Brightness change on notebook that follow ACPI specification (par. B.7) Message-ID: <201003301612.18045.jhb@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 3:30:37 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'm not sure if there was a PR. I do recall a thread on the acpi@ mailing list when the patch was first submitted. > 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. 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201003301612.18045.jhb>