From owner-freebsd-acpi@FreeBSD.ORG Tue Mar 30 20:12:30 2010 Return-Path: Delivered-To: freebsd-acpi@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id CA50B1065672 for ; Tue, 30 Mar 2010 20:12:30 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 885CB8FC16 for ; Tue, 30 Mar 2010 20:12:30 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 21AFD46B2D; Tue, 30 Mar 2010 16:12:30 -0400 (EDT) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPA id 52A0F8A01F; Tue, 30 Mar 2010 16:12:29 -0400 (EDT) From: John Baldwin To: Dan Lukes Date: Tue, 30 Mar 2010 16:12:18 -0400 User-Agent: KMail/1.12.1 (FreeBSD/7.3-CBSD-20100217; KDE/4.3.1; amd64; ; ) References: <4BB170E7.3030407@obluda.cz> <201003301011.40215.jhb@freebsd.org> <4BB2515D.1060808@obluda.cz> In-Reply-To: <4BB2515D.1060808@obluda.cz> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201003301612.18045.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Tue, 30 Mar 2010 16:12:29 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-1.8 required=4.2 tests=AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: freebsd-acpi@freebsd.org Subject: Re: Brightness change on notebook that follow ACPI specification (par. B.7) X-BeenThere: freebsd-acpi@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: ACPI and power management development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 Mar 2010 20:12:31 -0000 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