acpi: video: no automatic brightness changes by firmware

PATCH In review Aaron Lu 3 participants 9 messages Jun 17, 2013 – Jul 17, 2013 2 messages not in the archive ACPI

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Aaron Lu

author

2013-06-17 01:00 UTC #1 lore raw

PATCH 1/1 acpi: video: no automatic brightness changes by firmware

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 <[email protected]>

Reported-and-tested-by Dan Garton <[email protected]>

Reported-and-tested-by Bob Ziuchkovski <[email protected]>

Signed-off-by Aaron Lu <[email protected]>

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

drivers/acpi/video.c +2 -2

index b8299f2..f8fa541 100644
@@ -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)

Signature · 1 line

1.8.3.3.gfada522

Rafael J. Wysocki

2013-06-17 11:18 UTC #2 lore raw

Quoted 16 lines · 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.

Quoted 6 lines

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 <[email protected]>
Reported-and-tested-by Dan Garton <[email protected]>
Reported-and-tested-by Bob Ziuchkovski <[email protected]>
Signed-off-by Aaron Lu <[email protected]>

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

Rafael

Quoted 24 lines

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

drivers/acpi/video.c +2 -2

index b8299f2..f8fa541 100644
@@ -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)

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Aaron Lu

author

2013-06-17 15:01 UTC #3 lore raw

Quoted 17 lines · Rafael J. Wysocki

Quoted 16 lines · 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.

Quoted 8 lines

Quoted 6 lines
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 <[email protected]>
Reported-and-tested-by Dan Garton <[email protected]>
Reported-and-tested-by Bob Ziuchkovski <[email protected]>
Signed-off-by Aaron Lu <[email protected]>
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

Quoted 28 lines

Rafael

Quoted 24 lines

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

drivers/acpi/video.c +2 -2

index b8299f2..f8fa541 100644
@@ -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)

Rafael J. Wysocki

2013-06-17 22:09 UTC #4 lore raw

Quoted 25 lines · Aaron Lu

Quoted 24 lines · Rafael J. Wysocki
Quoted 22 lines · 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 <[email protected]>
Reported-and-tested-by Dan Garton <[email protected]>
Reported-and-tested-by Bob Ziuchkovski <[email protected]>
Signed-off-by Aaron Lu <[email protected]>

Quoted 4 lines

Quoted 1 line
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

Quoted 25 lines

Quoted 24 lines

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

drivers/acpi/video.c +2 -2

index b8299f2..f8fa541 100644
@@ -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)

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Matthew Garrett

2013-06-17 22:40 UTC #5 lore raw

In reply to Aaron Lu

Quoted 6 lines · 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.

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.

Signature · 1 line

Matthew Garrett | [email protected]

Aaron Lu

author

2013-06-18 00:13 UTC #6 lore raw

Quoted 11 lines · Matthew Garrett

Quoted 6 lines · 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.
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

Aaron Lu

author

2013-07-05 01:15 UTC #7 lore raw

Quoted 9 lines · Aaron Lu

Quoted 8 lines · Matthew Garrett
Quoted 6 lines · 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.

Quoted 8 lines

Quoted 3 lines
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

Aaron Lu

author

2013-07-16 05:07 UTC #8 lore raw

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 <[email protected]>

Reported-by Dan Garton <[email protected]>

Reported-by Bob Ziuchkovski <[email protected]>

Signed-off-by Aaron Lu <[email protected]>

---
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(-)

drivers/acpi/video.c +8 -2

index 01b1a25..bc2954b 100644
@@ -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

A message that is not in the archive (sent off-list or not yet received): CAKw44fYBePZ-Qy5VsUUyJ5ob5SA34LNNCmBtqyCz5D_hCbYgUQ@mail.gmail.com

Aaron Lu

author

2013-07-17 06:26 UTC #10 lore raw

Quoted 1 line · Micael

Working fine here.

Thanks for your testing!

-Aaron

Quoted 72 lines · Aaron Lu

Quoted 66 lines
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 <[email protected]>
Reported-by Dan Garton <[email protected]>
Reported-by Bob Ziuchkovski <[email protected]>
Signed-off-by Aaron Lu <[email protected]>
---
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(-)
drivers/acpi/video.c +8 -2
index 01b1a25..bc2954b 100644
@@ -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