Re: [PATCH 4/4] eeepc-wmi: add backlight support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Apr 10, 2010 at 2:42 AM, Yong Wang <yong.y.wang@xxxxxxxxxxxxxxx> wrote:
> On Fri, Apr 09, 2010 at 08:40:08PM +0200, Corentin Chary wrote:
>> On Fri, Apr 9, 2010 at 5:07 PM, Yong Wang <yong.y.wang@xxxxxxxxxxxxxxx> wrote:
>> > +static int read_brightness(struct backlight_device *bd)
>> > +{
>> > + ? ? ? static u32 ctrl_param;
>> > + ? ? ? acpi_status status;
>> > +
>> > + ? ? ? status = eeepc_wmi_get_devstate(EEEPC_WMI_DEVID_BACKLIGHT, &ctrl_param);
>> > +
>> > + ? ? ? if (ACPI_FAILURE(status))
>> > + ? ? ? ? ? ? ? return -1;
>> > + ? ? ? else
>> > + ? ? ? ? ? ? ? return ctrl_param & 0x000000FF;
>> > +}
>>
>> ctrl_param & 0xFF seems simpler
>>
>
> OK.
>
>> > +static int eeepc_wmi_backlight_notify(struct eeepc_wmi *eeepc_wmi, bool increase)
>> > +{
>> > + ? ? ? struct backlight_device *bd = eeepc_wmi->backlight_dev;
>> > + ? ? ? int old = bd->props.brightness;
>> > + ? ? ? int new;
>> > +
>> > + ? ? ? if (increase)
>> > + ? ? ? ? ? ? ? new = old + 1;
>> > + ? ? ? else
>> > + ? ? ? ? ? ? ? new = old - 1;
>> > +
>> > + ? ? ? if (new > bd->props.max_brightness)
>> > + ? ? ? ? ? ? ? new = bd->props.max_brightness;
>> > + ? ? ? else if (new < 0)
>> > + ? ? ? ? ? ? ? new = 0;
>> > +
>> > + ? ? ? bd->props.brightness = new;
>> > + ? ? ? backlight_update_status(bd);
>>
>> Do you really need to do that ? Can't you just use read_backlight()
>> and do nothing (just call backlight_force_update) ?
>> If you really need to do that (but I don't think so), can't you just
>> do "new = event -   NOTIFY_BRN_MIN" ?
>>
>
> Backlight does not change when pressing the brightness hotkeys.
> Therefore read_backlight returns the old brightness value.

Ok didn't now that.

> Unlike the netbooks eeepc-laptop is written for, the new ones based on
> WMI could generate two different notification codes depending on which
> brightness hotkey is pressed.
>
> #define NOTIFY_BRNUP_MIN        0x11
> #define NOTIFY_BRNUP_MAX        0x1f
> #define NOTIFY_BRNDOWN_MIN      0x20
> #define NOTIFY_BRNDOWN_MAX      0x2e

Would something like that work ?

if (event between BRN_UP_MIN and BRN_UP_MAX)
new = event - BRNUP_MIN
if (event between BRNDOWN_MIN and BRNDOWN_MAX)
new = event - BRNDOWN_MIN

It would be more coherent if the bios can change the brightness by
itself (light sensor, AC plug/unplug, low battery, ...)



-- 
Corentin Chary
http://xf.iksaif.net
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux