[PATCH] acpi: video: no automatic brightness changes by firmware

[PATCH] acpi: video: no automatic brightness changes by firmware

[Aaron Lu]

Starting from win8, MS backlight control driver will set bit 2 of the
parameter of control method _DOS, to inform firmware it should not
perform any automatic brightness changes. This mostly affects hotkey
notification deliver - if we do not set this bit, on hotkey press,
firmware may choose to adjust brightness level instead of sending out
notification and doing nothing.

So this patch sets bit 2 when calling _DOS so that GUIs can show the
notification window on hotkey press.

The MS document on win8 backlight control is here:
http://msdn.microsoft.com/en-US/library/windows/hardware/jj159305

This patch solves problem for ASUS N56VZ and ASUS N56VJ, both for
notification delivery and for brightness control.

Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=52951
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=56711
Reported-and-tested-by: Micael Dias <kam1kaz3@gmail.com>
Reported-and-tested-by: Dan Garton <dan.garton@gmail.com>
Reported-and-tested-by: Bob Ziuchkovski <bob.ziuchkovski@gmail.com>
Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
 drivers/acpi/video.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index b8299f2..f8fa541 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1543,12 +1543,12 @@ static int acpi_video_bus_put_devices(struct acpi_video_bus *video)
 
 static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
 {
-	return acpi_video_bus_DOS(video, 0, 0);
+	return acpi_video_bus_DOS(video, 0, 1);
 }
 
 static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
 {
-	return acpi_video_bus_DOS(video, 0, 1);
+	return acpi_video_bus_DOS(video, 0, 0);
 }
 
 static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
-- 
1.8.3.3.gfada522

Re: [PATCH] acpi: video: no automatic brightness changes by firmware

[Rafael J. Wysocki]

On Monday, June 17, 2013 09:01:10 AM Aaron Lu wrote:
> Starting from win8, MS backlight control driver will set bit 2 of the
> parameter of control method _DOS, to inform firmware it should not
> perform any automatic brightness changes. This mostly affects hotkey
> notification deliver - if we do not set this bit, on hotkey press,
> firmware may choose to adjust brightness level instead of sending out
> notification and doing nothing.
> 
> So this patch sets bit 2 when calling _DOS so that GUIs can show the
> notification window on hotkey press.
> 
> The MS document on win8 backlight control is here:
> http://msdn.microsoft.com/en-US/library/windows/hardware/jj159305
> 
> This patch solves problem for ASUS N56VZ and ASUS N56VJ, both for
> notification delivery and for brightness control.
> 
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=52951
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=56711
> Reported-and-tested-by: Micael Dias <kam1kaz3@gmail.com>
> Reported-and-tested-by: Dan Garton <dan.garton@gmail.com>
> Reported-and-tested-by: Bob Ziuchkovski <bob.ziuchkovski@gmail.com>
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>

OK, do we need that in -stable and if so, which ones?

Rafael


> ---
>  drivers/acpi/video.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index b8299f2..f8fa541 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -1543,12 +1543,12 @@ static int acpi_video_bus_put_devices(struct acpi_video_bus *video)
>  
>  static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
>  {
> -	return acpi_video_bus_DOS(video, 0, 0);
> +	return acpi_video_bus_DOS(video, 0, 1);
>  }
>  
>  static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
>  {
> -	return acpi_video_bus_DOS(video, 0, 1);
> +	return acpi_video_bus_DOS(video, 0, 0);
>  }
>  
>  static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] acpi: video: no automatic brightness changes by firmware

[Aaron Lu]

On 06/17/2013 07:27 PM, Rafael J. Wysocki wrote:
> On Monday, June 17, 2013 09:01:10 AM Aaron Lu wrote:
>> Starting from win8, MS backlight control driver will set bit 2 of the
>> parameter of control method _DOS, to inform firmware it should not
>> perform any automatic brightness changes. This mostly affects hotkey
>> notification deliver - if we do not set this bit, on hotkey press,
>> firmware may choose to adjust brightness level instead of sending out
>> notification and doing nothing.
>>
>> So this patch sets bit 2 when calling _DOS so that GUIs can show the
>> notification window on hotkey press.
>>
>> The MS document on win8 backlight control is here:
>> http://msdn.microsoft.com/en-US/library/windows/hardware/jj159305
>>
>> This patch solves problem for ASUS N56VZ and ASUS N56VJ, both for
>> notification delivery and for brightness control.
>>
>> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=52951
>> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=56711
>> Reported-and-tested-by: Micael Dias <kam1kaz3@gmail.com>
>> Reported-and-tested-by: Dan Garton <dan.garton@gmail.com>
>> Reported-and-tested-by: Bob Ziuchkovski <bob.ziuchkovski@gmail.com>
>> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> 
> OK, do we need that in -stable and if so, which ones?

Perhaps only for new kernels, since 3.7 and 3.8 are EOL-ed, so just
for 3.9. I don't think older kernels need change the behavior.

Thanks,
Aaron

> 
> Rafael
> 
> 
>> ---
>>  drivers/acpi/video.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
>> index b8299f2..f8fa541 100644
>> --- a/drivers/acpi/video.c
>> +++ b/drivers/acpi/video.c
>> @@ -1543,12 +1543,12 @@ static int acpi_video_bus_put_devices(struct acpi_video_bus *video)
>>  
>>  static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
>>  {
>> -	return acpi_video_bus_DOS(video, 0, 0);
>> +	return acpi_video_bus_DOS(video, 0, 1);
>>  }
>>  
>>  static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
>>  {
>> -	return acpi_video_bus_DOS(video, 0, 1);
>> +	return acpi_video_bus_DOS(video, 0, 0);
>>  }
>>  
>>  static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
>>

Re: [PATCH] acpi: video: no automatic brightness changes by firmware

[Rafael J. Wysocki]

On Monday, June 17, 2013 11:01:50 PM Aaron Lu wrote:
> On 06/17/2013 07:27 PM, Rafael J. Wysocki wrote:
> > On Monday, June 17, 2013 09:01:10 AM Aaron Lu wrote:
> >> Starting from win8, MS backlight control driver will set bit 2 of the
> >> parameter of control method _DOS, to inform firmware it should not
> >> perform any automatic brightness changes. This mostly affects hotkey
> >> notification deliver - if we do not set this bit, on hotkey press,
> >> firmware may choose to adjust brightness level instead of sending out
> >> notification and doing nothing.
> >>
> >> So this patch sets bit 2 when calling _DOS so that GUIs can show the
> >> notification window on hotkey press.
> >>
> >> The MS document on win8 backlight control is here:
> >> http://msdn.microsoft.com/en-US/library/windows/hardware/jj159305
> >>
> >> This patch solves problem for ASUS N56VZ and ASUS N56VJ, both for
> >> notification delivery and for brightness control.
> >>
> >> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=52951
> >> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=56711
> >> Reported-and-tested-by: Micael Dias <kam1kaz3@gmail.com>
> >> Reported-and-tested-by: Dan Garton <dan.garton@gmail.com>
> >> Reported-and-tested-by: Bob Ziuchkovski <bob.ziuchkovski@gmail.com>
> >> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> > 
> > OK, do we need that in -stable and if so, which ones?
> 
> Perhaps only for new kernels, since 3.7 and 3.8 are EOL-ed, so just
> for 3.9. I don't think older kernels need change the behavior.

Well, 3.8 has become a long-term at least ...

Thanks,
Rafael


> >> ---
> >>  drivers/acpi/video.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> >> index b8299f2..f8fa541 100644
> >> --- a/drivers/acpi/video.c
> >> +++ b/drivers/acpi/video.c
> >> @@ -1543,12 +1543,12 @@ static int acpi_video_bus_put_devices(struct acpi_video_bus *video)
> >>  
> >>  static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
> >>  {
> >> -	return acpi_video_bus_DOS(video, 0, 0);
> >> +	return acpi_video_bus_DOS(video, 0, 1);
> >>  }
> >>  
> >>  static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
> >>  {
> >> -	return acpi_video_bus_DOS(video, 0, 1);
> >> +	return acpi_video_bus_DOS(video, 0, 0);
> >>  }
> >>  
> >>  static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
> >>
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] acpi: video: no automatic brightness changes by firmware

[Matthew Garrett]

On Mon, Jun 17, 2013 at 09:01:10AM +0800, Aaron Lu wrote:
> Starting from win8, MS backlight control driver will set bit 2 of the
> parameter of control method _DOS, to inform firmware it should not
> perform any automatic brightness changes. This mostly affects hotkey
> notification deliver - if we do not set this bit, on hotkey press,
> firmware may choose to adjust brightness level instead of sending out
> notification and doing nothing.

It's worth noting that this is a change in behaviour, and there's at 
least one VIA-based netbook where making this change breaks backlight 
control.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] acpi: video: no automatic brightness changes by firmware

[Aaron Lu]

On 06/18/2013 06:40 AM, Matthew Garrett wrote:
> On Mon, Jun 17, 2013 at 09:01:10AM +0800, Aaron Lu wrote:
>> Starting from win8, MS backlight control driver will set bit 2 of the
>> parameter of control method _DOS, to inform firmware it should not
>> perform any automatic brightness changes. This mostly affects hotkey
>> notification deliver - if we do not set this bit, on hotkey press,
>> firmware may choose to adjust brightness level instead of sending out
>> notification and doing nothing.
> 
> It's worth noting that this is a change in behaviour, and there's at 
> least one VIA-based netbook where making this change breaks backlight 
> control.

Thanks for pointing this out.
Do you have any more information about this system, like its ACPI dump?

BTW, perhaps we should make this behavior change only for win8 systems?

Thanks,
Aaron

Re: [PATCH] acpi: video: no automatic brightness changes by firmware

[Aaron Lu]

On 06/18/2013 08:13 AM, Aaron Lu wrote:
> On 06/18/2013 06:40 AM, Matthew Garrett wrote:
>> On Mon, Jun 17, 2013 at 09:01:10AM +0800, Aaron Lu wrote:
>>> Starting from win8, MS backlight control driver will set bit 2 of the
>>> parameter of control method _DOS, to inform firmware it should not
>>> perform any automatic brightness changes. This mostly affects hotkey
>>> notification deliver - if we do not set this bit, on hotkey press,
>>> firmware may choose to adjust brightness level instead of sending out
>>> notification and doing nothing.
>>
>> It's worth noting that this is a change in behaviour, and there's at 
>> least one VIA-based netbook where making this change breaks backlight 
>> control.
> 
> Thanks for pointing this out.
> Do you have any more information about this system, like its ACPI dump?
> 
> BTW, perhaps we should make this behavior change only for win8 systems?

Just checked all affected systems:
https://bugzilla.kernel.org/show_bug.cgi?id=51051
https://bugzilla.kernel.org/show_bug.cgi?id=56711
https://bugzilla.kernel.org/show_bug.cgi?id=52951

All systems claim win8 in _OSI, so I suppose I'll make this behavior
change only for those systems with the help of the recently proposed
"Expose OSI version" patchset.

Please let me know if you disagree, thanks.

-Aaron

Re: [PATCH] acpi: video: no automatic brightness changes by firmware

[Aaron Lu]

Starting from win8, MS backlight control driver will set bit 2 of the
parameter of control method _DOS, to inform firmware it should not
perform any automatic brightness changes. This mostly affects hotkey
notification deliver - if we do not set this bit, on hotkey press,
firmware may choose to adjust brightness level instead of sending out
notification and doing nothing.

So this patch sets bit 2 when calling _DOS so that GUIs can show the
notification window on hotkey press. This behavior change is only
necessary for win8 systems.

The MS document on win8 backlight control is here:
http://msdn.microsoft.com/en-US/library/windows/hardware/jj159305

Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=52951
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=56711
Reported-by: Micael Dias <kam1kaz3@gmail.com>
Reported-by: Dan Garton <dan.garton@gmail.com>
Reported-by: Bob Ziuchkovski <bob.ziuchkovski@gmail.com>
Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
Apply on top of:
https://patchwork.kernel.org/patch/2812951/ (expose OSI version)
https://patchwork.kernel.org/patch/2827793/ (win8 video quirk)

Micael, Dan and Bob,
Please give the patch a test if possible, thanks.

v2: Make the behavior change only for win8 systems; use the newly added
    function acpi_video_backlight_quirks as the condition check for win8.

 drivers/acpi/video.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 01b1a25..bc2954b 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1534,14 +1534,20 @@ static int acpi_video_bus_put_devices(struct acpi_video_bus *video)
 
 /* acpi_video interface */
 
+/*
+ * Win8 requires setting bit2 of _DOS to let firmware know it shouldn't
+ * preform any automatic brightness change on receiving a notification.
+ */
 static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
 {
-	return acpi_video_bus_DOS(video, 0, 0);
+	return acpi_video_bus_DOS(video, 0,
+				  acpi_video_backlight_quirks() ? 1 : 0);
 }
 
 static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
 {
-	return acpi_video_bus_DOS(video, 0, 1);
+	return acpi_video_bus_DOS(video, 0,
+				  acpi_video_backlight_quirks() ? 0 : 1);
 }
 
 static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
--
1.8.3.2.10.g43d11f4 

Re: [PATCH] acpi: video: no automatic brightness changes by firmware

[Aaron Lu]

On 07/16/2013 10:39 PM, Micael wrote:
> Working fine here.

Thanks for your testing!

-Aaron

> 
> 
> On Tue, Jul 16, 2013 at 6:08 AM, Aaron Lu <aaron.lu@intel.com> wrote:
> 
>> Starting from win8, MS backlight control driver will set bit 2 of the
>> parameter of control method _DOS, to inform firmware it should not
>> perform any automatic brightness changes. This mostly affects hotkey
>> notification deliver - if we do not set this bit, on hotkey press,
>> firmware may choose to adjust brightness level instead of sending out
>> notification and doing nothing.
>>
>> So this patch sets bit 2 when calling _DOS so that GUIs can show the
>> notification window on hotkey press. This behavior change is only
>> necessary for win8 systems.
>>
>> The MS document on win8 backlight control is here:
>> http://msdn.microsoft.com/en-US/library/windows/hardware/jj159305
>>
>> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=52951
>> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=56711
>> Reported-by: Micael Dias <kam1kaz3@gmail.com>
>> Reported-by: Dan Garton <dan.garton@gmail.com>
>> Reported-by: Bob Ziuchkovski <bob.ziuchkovski@gmail.com>
>> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
>> ---
>> Apply on top of:
>> https://patchwork.kernel.org/patch/2812951/ (expose OSI version)
>> https://patchwork.kernel.org/patch/2827793/ (win8 video quirk)
>>
>> Micael, Dan and Bob,
>> Please give the patch a test if possible, thanks.
>>
>> v2: Make the behavior change only for win8 systems; use the newly added
>>     function acpi_video_backlight_quirks as the condition check for win8.
>>
>>  drivers/acpi/video.c | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
>> index 01b1a25..bc2954b 100644
>> --- a/drivers/acpi/video.c
>> +++ b/drivers/acpi/video.c
>> @@ -1534,14 +1534,20 @@ static int acpi_video_bus_put_devices(struct
>> acpi_video_bus *video)
>>
>>  /* acpi_video interface */
>>
>> +/*
>> + * Win8 requires setting bit2 of _DOS to let firmware know it shouldn't
>> + * preform any automatic brightness change on receiving a notification.
>> + */
>>  static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
>>  {
>> -       return acpi_video_bus_DOS(video, 0, 0);
>> +       return acpi_video_bus_DOS(video, 0,
>> +                                 acpi_video_backlight_quirks() ? 1 : 0);
>>  }
>>
>>  static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
>>  {
>> -       return acpi_video_bus_DOS(video, 0, 1);
>> +       return acpi_video_bus_DOS(video, 0,
>> +                                 acpi_video_backlight_quirks() ? 0 : 1);
>>  }
>>
>>  static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
>> --
>> 1.8.3.2.10.g43d11f4
>>
> 
> 
>