Re: [PATCH 2/2 v4] asus-wmi: store backlight power status for AIO machine

[AceLan Kao <acelan.kao@xxxxxxxxxxxxx>]

Dear Corentin,

It looks good and works on the ET2012 machines.
Thanks.

Best regards,
AceLan Kao.

2012/3/20 Corentin Chary <corentin.chary@xxxxxxxxx>:
> On Mon, Mar 19, 2012 at 1:22 PM, AceLan Kao <acelan.kao@xxxxxxxxxxxxx> wrote:
>> Dear Corentin,
>>
>> 2012/3/19 Corentin Chary <corentin.chary@xxxxxxxxx>:
>>> On Mon, Mar 19, 2012 at 10:46 AM, AceLan Kao <acelan.kao@xxxxxxxxxxxxx> wrote:
>>>> Due to some implementation reasons, ASUS ET2012 All-in-One machines
>>>> can't report the correct backlight power status, it will always return
>>>> 1. To track the backlight power status correctly, we have to store the
>>>> status by ourselves.
>>>>
>>>> BTW, by the BIOS design, the backlight power will be turn on/off
>>>> sequently, no matter what the value of the parameter will be.
>>>> More over, the brightness adjustment command will turn on the backlight
>>>> power. Those behaviors will make us fail to track the backlight power
>>>> status.
>>>> For example, While we are trying to turn on the backlight power,
>>>> we will send out the brightness adjustment command and then trying to
>>>> figure out if we have to turn on the backlight power, then send out
>>>> the command. But, the real case is that, the backlight power turns on
>>>> while sending the brightness adjustment command, and then we send out
>>>> the command to turn on the backlight power, it actually will turn off
>>>> the backlight power and the backlight power status we recorded becomes
>>>> wrong. So, we have to seperate these two commands by a if statement.
>>>>
>>>> Signed-off-by: AceLan Kao <acelan.kao@xxxxxxxxxxxxx>
>>>> ---
>>>>  drivers/platform/x86/asus-wmi.c  |   33 ++++++++++++++++++++-------------
>>>>  drivers/platform/x86/asus-wmi.h  |    2 ++
>>>>  drivers/platform/x86/eeepc-wmi.c |   15 ++++++++++++---
>>>>  3 files changed, 34 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
>>>> index ea80fbe..9cd459a 100644
>>>> --- a/drivers/platform/x86/asus-wmi.c
>>>> +++ b/drivers/platform/x86/asus-wmi.c
>>>> @@ -1076,7 +1076,12 @@ static int asus_wmi_hwmon_init(struct asus_wmi *asus)
>>>>  */
>>>>  static int read_backlight_power(struct asus_wmi *asus)
>>>>  {
>>>> -       int ret = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_BACKLIGHT);
>>>> +       int ret;
>>>> +       if (asus->driver->quirks->store_backlight_power)
>>>> +               ret = !asus->driver->panel_power;
>>>> +       else
>>>> +               ret = asus_wmi_get_devstate_simple(asus,
>>>> +                                                  ASUS_WMI_DEVID_BACKLIGHT);
>>>>
>>>>        if (ret < 0)
>>>>                return ret;
>>>> @@ -1138,24 +1143,23 @@ static int update_bl_status(struct backlight_device *bd)
>>>>  {
>>>>        struct asus_wmi *asus = bl_get_data(bd);
>>>>        u32 ctrl_param;
>>>> -       int power, err;
>>>> -
>>>> -       if (asus->driver->quirks->scalar_panel_brightness)
>>>> -               ctrl_param = get_scalar_command(bd);
>>>> -       else
>>>> -               ctrl_param = bd->props.brightness;
>>>> -
>>>> -       err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BRIGHTNESS,
>>>> -                                   ctrl_param, NULL);
>>>> -
>>>> -       if (err < 0)
>>>> -               return err;
>>>> +       int power, err = 0;
>>>>
>>>>        power = read_backlight_power(asus);
>>>>        if (power != -ENODEV && bd->props.power != power) {
>>>>                ctrl_param = !!(bd->props.power == FB_BLANK_UNBLANK);
>>>>                err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT,
>>>>                                            ctrl_param, NULL);
>>>> +               if (asus->driver->quirks->store_backlight_power)
>>>> +                       asus->driver->panel_power = bd->props.power;
>>>> +       } else {
>>>> +               if (asus->driver->quirks->scalar_panel_brightness)
>>>> +                       ctrl_param = get_scalar_command(bd);
>>>> +               else
>>>> +                       ctrl_param = bd->props.brightness;
>>>> +
>>>> +               err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BRIGHTNESS,
>>>> +                                           ctrl_param, NULL);
>>>>        }
>>>
>>> Something I didn't notice, why is this moved in an "else" ?
>>> Does this break something on your hardware ?
>> Yes, in the original sequence, it will send out a brightness command through
>> WMI anyway, even if we hit this function because of changing the
>> backlight power.
>>
>> And in the ET2012 series machines, it will turn on the backlight power
>> automatically
>> when you send out the brightness command.
>> And the backlight power command actually is a backlight toggle command
>> in ET2012.
>> That is no matter what the parameter is while sending the backlight
>> power command,
>> it will turn on and off the backlight power in a row.
>> So, when we're trying to turn on the backlight power, the panel will
>> be turned on
>> by the brightness command and the turn on backlight command will actually
>> turn off the backlight.
>>
>> To avoid this, I use an if else statement to distinguish these 2 cases,
>> brightness or backlight power, and this should do no harm to normal machines,
>> hope I don't miss anything.
>
> Ok I see. I pushed some patches on top of these two, they are
> available at http://git.iksaif.net/?p=acpi4asus-dkms.git;a=summary
> These are mostly cleanups, fix for asus-nb-wmi, and I restored the
> ability to update both brightness and power (still disabled for
> et2012).
> Could you test these patches ?
> Thanks,
>
> --
> 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



-- 
Chia-Lin Kao(AceLan)
http://blog.acelan.idv.tw/
E-Mail: acelan.kaoATcanonical.com (s/AT/@/)
--
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