ACPI / Video: Fix initial brightness problem on Acer Ferrari One

PATCH In review Rafael J. Wysocki 2 participants 12 messages Apr 1, 2010 – Apr 7, 2010 ACPI

lore

Share & tools

lore

Rafael J. Wysocki

author

2010-04-01 01:16 UTC #0 lore raw

PATCH 1/1 ACPI / Video: Fix initial brightness problem on Acer Ferrari One

From: Rafael J. Wysocki <[email protected]>

On Acer Ferrari One, when _BQC is invoked for the first time, the minimum brightness is returned, so the ACPI video drivers sets the minimum brightness on boot. This is not desirable, so use the following rule:
If _BQC initially returns an invalid value or the minimum brightness, use either the brightness level supposed to be used on AC power, or the brightness level supposed to be used on battery, depending on whether or not the system is on AC power. If these values are not exported by the BIOS, use the maximum brightness level.

Signed-off-by Rafael J. Wysocki <[email protected]>

---
 drivers/acpi/video.c |   26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

drivers/acpi/video.c +13 -3

Index: linux-2.6/drivers/acpi/video.c
===================================================================
@@ -44,6 +44,7 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <linux/suspend.h>
+#include <linux/power_supply.h>
 
 #define PREFIX "ACPI: "

@@ -920,11 +928,20 @@ acpi_video_init_brightness(struct acpi_v
 		 * Set the backlight to the initial state.
 		 * On some buggy laptops, _BQC returns an uninitialized value
 		 * when invoked for the first time, i.e. level_old is invalid.
-		 * set the backlight to max_level in this case
+		 * On some other systems _BQC returns the minimum brightness
+		 * when invoked for the first time, which also usually is
+		 * undesirable.  In that cases use the "AC power" value if on AC
+		 * power or the "battery" value otherwise.  If these vaules are
+		 * not exported, use max_level.
 		 */
-		for (i = 2; i < br->count; i++)
-			if (level_old == br->levels[i])
+		for (i = 3; i < br->count; i++)
+			if (level_old == br->levels[i]) {
 				level = level_old;
+				goto set_level;
+			}
+		if (!br->flags._BCL_no_ac_battery_levels)
+			level = power_supply_is_system_supplied() ?
+						br->levels[0] : br->levels[1];
 		goto set_level;
 	}

Zhang Rui

2010-04-01 01:33 UTC #1 lore raw

Quoted 42 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>
On Acer Ferrari One, when _BQC is invoked for the first time, the minimum brightness is returned, so the ACPI video drivers sets the minimum brightness on boot. This is not desirable, so use the following rule:
If _BQC initially returns an invalid value or the minimum brightness, use either the brightness level supposed to be used on AC power, or the brightness level supposed to be used on battery, depending on whether or not the system is on AC power. If these values are not exported by the BIOS, use the maximum brightness level.
Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/acpi/video.c |   26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)
Index: linux-2.6/drivers/acpi/video.c
===================================================================
drivers/acpi/video.c +6 -2
@@ -44,6 +44,7 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <linux/suspend.h>
+#include <linux/power_supply.h>
 
 #define PREFIX "ACPI: "
 
@@ -920,11 +928,20 @@ acpi_video_init_brightness(struct acpi_v
 		 * Set the backlight to the initial state.
 		 * On some buggy laptops, _BQC returns an uninitialized value
 		 * when invoked for the first time, i.e. level_old is invalid.
-		 * set the backlight to max_level in this case
+		 * On some other systems _BQC returns the minimum brightness
+		 * when invoked for the first time, which also usually is
+		 * undesirable.  In that cases use the "AC power" value if on AC
+		 * power or the "battery" value otherwise.  If these vaules are
+		 * not exported, use max_level.
 		 */
-		for (i = 2; i < br->count; i++)

drivers/acpi/video.c acpi_video_init_brightness(struct acpi_v · line 945

Quoted 10 lines

drivers/acpi/video.c +7 -1

-			if (level_old == br->levels[i])
+		for (i = 3; i < br->count; i++)
+			if (level_old == br->levels[i]) {
 				level = level_old;
+				goto set_level;
+			}
+		if (!br->flags._BCL_no_ac_battery_levels)
+			level = power_supply_is_system_supplied() ?
+
 						br->levels[0] : br->levels[1];

this doesn't work currently.
br->levels[0] and br->levels[1] doesn't equal the "AC power" value and "Battery" value, because the "AC power" and "Battery" value is not exported by many BIOS at all.
But we can make it work if we fake the AC and Battery value for these laptops, which is not done yet. For example, set the AC power value the maximum brightness level, if it's not exported by BIOS.

thanks,
rui

Quoted 7 lines

drivers/acpi/video.c +0 -1
 		goto set_level;
 	}
 
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Zhang Rui

2010-04-01 07:17 UTC #2 lore raw

Quoted 50 lines · Zhang Rui

Quoted 49 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
On Acer Ferrari One, when _BQC is invoked for the first time, the minimum brightness is returned, so the ACPI video drivers sets the minimum brightness on boot. This is not desirable, so use the following rule:
If _BQC initially returns an invalid value or the minimum brightness, use either the brightness level supposed to be used on AC power, or the brightness level supposed to be used on battery, depending on whether or not the system is on AC power. If these values are not exported by the BIOS, use the maximum brightness level.
Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/acpi/video.c |   26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)
Index: linux-2.6/drivers/acpi/video.c
===================================================================
drivers/acpi/video.c +11 -3
@@ -44,6 +44,7 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <linux/suspend.h>
+#include <linux/power_supply.h>
 
 #define PREFIX "ACPI: "
 
@@ -920,11 +928,20 @@ acpi_video_init_brightness(struct acpi_v
 		 * Set the backlight to the initial state.
 		 * On some buggy laptops, _BQC returns an uninitialized value
 		 * when invoked for the first time, i.e. level_old is invalid.
-		 * set the backlight to max_level in this case
+		 * On some other systems _BQC returns the minimum brightness
+		 * when invoked for the first time, which also usually is
+		 * undesirable.  In that cases use the "AC power" value if on AC
+		 * power or the "battery" value otherwise.  If these vaules are
+		 * not exported, use max_level.
 		 */
-		for (i = 2; i < br->count; i++)
-			if (level_old == br->levels[i])
+		for (i = 3; i < br->count; i++)
+			if (level_old == br->levels[i]) {
 				level = level_old;
+				goto set_level;
+			}
+		if (!br->flags._BCL_no_ac_battery_levels)

Quoted 8 lines

drivers/acpi/video.c acpi_video_init_brightness(struct acpi_v · line 945
Quoted 3 lines
drivers/acpi/video.c +2 -0
+			level = power_supply_is_system_supplied() ?
+
 						br->levels[0] : br->levels[1];
this doesn't work currently.
br->levels[0] and br->levels[1] doesn't equal the "AC power" value and "Battery" value, because the "AC power" and "Battery" value is not exported by many BIOS at all.

I mean, for these laptops, br->levels[0] and br->levels[1] equals the first and second elements in _BCL package.
But we can not use them as the "AC power" and "Battery" value.

Quoted 22 lines

But we can make it work if we fake the AC and Battery value for these laptops, which is not done yet. For example, set the AC power value the maximum brightness level, if it's not exported by BIOS.
thanks,
rui
Quoted 7 lines
drivers/acpi/video.c +0 -1
 		goto set_level;
 	}
 
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Rafael J. Wysocki

author

2010-04-01 19:42 UTC #3 lore raw

Quoted 51 lines · Zhang Rui

Quoted 50 lines · Zhang Rui
Quoted 49 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
On Acer Ferrari One, when _BQC is invoked for the first time, the minimum brightness is returned, so the ACPI video drivers sets the minimum brightness on boot. This is not desirable, so use the following rule:
If _BQC initially returns an invalid value or the minimum brightness, use either the brightness level supposed to be used on AC power, or the brightness level supposed to be used on battery, depending on whether or not the system is on AC power. If these values are not exported by the BIOS, use the maximum brightness level.
Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/acpi/video.c |   26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)
Index: linux-2.6/drivers/acpi/video.c
===================================================================
drivers/acpi/video.c +11 -3
@@ -44,6 +44,7 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <linux/suspend.h>
+#include <linux/power_supply.h>
 
 #define PREFIX "ACPI: "
 
@@ -920,11 +928,20 @@ acpi_video_init_brightness(struct acpi_v
 		 * Set the backlight to the initial state.
 		 * On some buggy laptops, _BQC returns an uninitialized value
 		 * when invoked for the first time, i.e. level_old is invalid.
-		 * set the backlight to max_level in this case
+		 * On some other systems _BQC returns the minimum brightness
+		 * when invoked for the first time, which also usually is
+		 * undesirable.  In that cases use the "AC power" value if on AC
+		 * power or the "battery" value otherwise.  If these vaules are
+		 * not exported, use max_level.
 		 */
-		for (i = 2; i < br->count; i++)
-			if (level_old == br->levels[i])
+		for (i = 3; i < br->count; i++)
+			if (level_old == br->levels[i]) {
 				level = level_old;
+				goto set_level;
+			}
+		if (!br->flags._BCL_no_ac_battery_levels)

Quoted 8 lines

Quoted 8 lines
drivers/acpi/video.c acpi_video_init_brightness(struct acpi_v · line 945
Quoted 3 lines
drivers/acpi/video.c +2 -0
+			level = power_supply_is_system_supplied() ?
+
 						br->levels[0] : br->levels[1];
this doesn't work currently.
br->levels[0] and br->levels[1] doesn't equal the "AC power" value and "Battery" value, because the "AC power" and "Battery" value is not exported by many BIOS at all.

But if the BIOS doesn't export these values, br->flags._BCL_no_ac_battery_levels will be true, won't it?

Quoted 3 lines

I mean, for these laptops, br->levels[0] and br->levels[1] equals the first and second elements in _BCL package.
But we can not use them as the "AC power" and "Battery" value.

In that cases the if (!br->flags._BCL_no_ac_battery_levels) condition won't be satisfied.

On a second thought, though, something like this also works and is simpler.

Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: ACPI / Video: Fix initial brightness problem on Acer Ferrari One (v. 2)

On Acer Ferrari One, when _BQC is invoked for the first time, the minimum brightness is returned, so the ACPI video drivers sets the minimum brightness on boot. This is not desirable, so use the following rule:
Set brightness to either the level supposed to be used on AC power, or the brightness level supposed to be used on battery, depending on whether or not the system is on AC power. If these values are not exported by the BIOS, use the brightness level initially returned by _BQC. If that value is invalid, use the maximum brightness level.

Signed-off-by Rafael J. Wysocki <[email protected]>

---
 drivers/acpi/video.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

drivers/acpi/video.c +15 -1

Index: linux-2.6/drivers/acpi/video.c
===================================================================
@@ -44,6 +44,7 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <linux/suspend.h>
+#include <linux/power_supply.h>
 
 #define PREFIX "ACPI: "

@@ -915,6 +916,18 @@ acpi_video_init_brightness(struct acpi_v
 
 	br->flags._BQC_use_index = (level == max_level ? 0 : 1);
 
+	/*
+	 * Set brightness to the "AC power" value if on AC power or to the
+	 * "battery" value otherwise.  If these vaules are not exported, try
+	 * to use the value returned by the initial _BQC and fall back to
+	 * max_level.
+	 */
+	if (!br->flags._BCL_no_ac_battery_levels) {
+		level = power_supply_is_system_supplied() ?
+					br->levels[0] : br->levels[1];
+		goto set_level;
+	}
+
 	if (!br->flags._BQC_use_index) {
 		/*
 		 * Set the backlight to the initial state.

@@ -930,7 +943,8 @@ acpi_video_init_brightness(struct acpi_v
 
 	if (br->flags._BCL_reversed)
 		level_old = (br->count - 1) - level_old;
-	level = br->levels[level_old];
+	level = level_old >= 0 && level_old < br->count ?
+				br->levels[level_old] : max_level;
 
 set_level:
 	result = acpi_video_device_lcd_set_level(device, level);

Rafael J. Wysocki

author

2010-04-01 19:56 UTC #4 lore raw

Quoted 4 lines · Rafael J. Wysocki

Quoted 3 lines · Zhang Rui
Quoted 2 lines · Zhang Rui
Quoted 1 line · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>

...

Quoted 13 lines

Quoted 9 lines
Quoted 9 lines
Quoted 4 lines
(quoted diff) +3 -0
+		if (!br->flags._BCL_no_ac_battery_levels)
+			level = power_supply_is_system_supplied() ?
+
						br->levels[0] : br->levels[1];
this doesn't work currently.
br->levels[0] and br->levels[1] doesn't equal the "AC power" value and "Battery" value, because the "AC power" and "Battery" value is not exported by many BIOS at all.
But if the BIOS doesn't export these values, br->flags._BCL_no_ac_battery_levels will be true, won't it?

Quoted 8 lines

Quoted 3 lines
I mean, for these laptops, br->levels[0] and br->levels[1] equals the first and second elements in _BCL package.
But we can not use them as the "AC power" and "Battery" value.
In that cases the if (!br->flags._BCL_no_ac_battery_levels) condition won't be satisfied.
On a second thought, though, something like this also works and is simpler.

OK, Matthew wanted me to make the second expression using the ternary operator more readable.

Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: ACPI / Video: Fix initial brightness problem on Acer Ferrari One (v. 2)

On Acer Ferrari One, when _BQC is invoked for the first time, the minimum brightness is returned, so the ACPI video drivers sets the minimum brightness on boot. This is not desirable, so use the following rule:
Set brightness to either the level supposed to be used on AC power, or the brightness level supposed to be used on battery, depending on whether or not the system is on AC power. If these values are not exported by the BIOS, use the brightness level initially returned by _BQC. If that value is invalid, use the maximum brightness level.

Signed-off-by Rafael J. Wysocki <[email protected]>

---
 drivers/acpi/video.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

drivers/acpi/video.c +15 -1

Index: linux-2.6/drivers/acpi/video.c
===================================================================
@@ -44,6 +44,7 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <linux/suspend.h>
+#include <linux/power_supply.h>
 
 #define PREFIX "ACPI: "

@@ -915,6 +916,18 @@ acpi_video_init_brightness(struct acpi_v
 
 	br->flags._BQC_use_index = (level == max_level ? 0 : 1);
 
+	/*
+	 * Set brightness to the "AC power" value if on AC power or to the
+	 * "battery" value otherwise.  If these vaules are not exported, try
+	 * to use the value returned by the initial _BQC and fall back to
+	 * max_level.
+	 */
+	if (!br->flags._BCL_no_ac_battery_levels) {
+		level = power_supply_is_system_supplied() ?
+					br->levels[0] : br->levels[1];
+		goto set_level;
+	}
+
 	if (!br->flags._BQC_use_index) {
 		/*
 		 * Set the backlight to the initial state.

@@ -930,7 +943,8 @@ acpi_video_init_brightness(struct acpi_v
 
 	if (br->flags._BCL_reversed)
 		level_old = (br->count - 1) - level_old;
-	level = br->levels[level_old];
+	level = (level_old >= 0 && level_old < br->count) ?
+				br->levels[level_old] : max_level;
 
 set_level:
 	result = acpi_video_device_lcd_set_level(device, level);

Zhang Rui

2010-04-02 01:14 UTC #5 lore raw

In reply to Rafael J. Wysocki

Quoted 61 lines · Rafael J. Wysocki

Quoted 51 lines · Zhang Rui
Quoted 50 lines · Zhang Rui
Quoted 49 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
On Acer Ferrari One, when _BQC is invoked for the first time, the minimum brightness is returned, so the ACPI video drivers sets the minimum brightness on boot. This is not desirable, so use the following rule:
If _BQC initially returns an invalid value or the minimum brightness, use either the brightness level supposed to be used on AC power, or the brightness level supposed to be used on battery, depending on whether or not the system is on AC power. If these values are not exported by the BIOS, use the maximum brightness level.
Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/acpi/video.c |   26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)
Index: linux-2.6/drivers/acpi/video.c
===================================================================
drivers/acpi/video.c +11 -3
@@ -44,6 +44,7 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <linux/suspend.h>
+#include <linux/power_supply.h>
 
 #define PREFIX "ACPI: "
 
@@ -920,11 +928,20 @@ acpi_video_init_brightness(struct acpi_v
 		 * Set the backlight to the initial state.
 		 * On some buggy laptops, _BQC returns an uninitialized value
 		 * when invoked for the first time, i.e. level_old is invalid.
-		 * set the backlight to max_level in this case
+		 * On some other systems _BQC returns the minimum brightness
+		 * when invoked for the first time, which also usually is
+		 * undesirable.  In that cases use the "AC power" value if on AC
+		 * power or the "battery" value otherwise.  If these vaules are
+		 * not exported, use max_level.
 		 */
-		for (i = 2; i < br->count; i++)
-			if (level_old == br->levels[i])
+		for (i = 3; i < br->count; i++)
+			if (level_old == br->levels[i]) {
 				level = level_old;
+				goto set_level;
+			}
+		if (!br->flags._BCL_no_ac_battery_levels)
Quoted 8 lines
Quoted 8 lines
drivers/acpi/video.c acpi_video_init_brightness(struct acpi_v
Quoted 3 lines
drivers/acpi/video.c +2 -0
+			level = power_supply_is_system_supplied() ?
+
 						br->levels[0] : br->levels[1];
this doesn't work currently.
br->levels[0] and br->levels[1] doesn't equal the "AC power" value and "Battery" value, because the "AC power" and "Battery" value is not exported by many BIOS at all.

Quoted 3 lines

But if the BIOS doesn't export these values, br->flags._BCL_no_ac_battery_levels will be true, won't it?

no.
for example, I have seen a _BCL package like this: {100, 0, 10, 20, ..., 90, 100}.
Only one value, not sure the "AC power" or "Battery" value, is returned, the br->flags._BCL_no_ac_battery_levels is also set in this case.

Quoted 44 lines

Quoted 3 lines
I mean, for these laptops, br->levels[0] and br->levels[1] equals the first and second elements in _BCL package.
But we can not use them as the "AC power" and "Battery" value.
In that cases the if (!br->flags._BCL_no_ac_battery_levels) condition won't be satisfied.
On a second thought, though, something like this also works and is simpler.
Rafael
---
From: Rafael J. Wysocki <[email protected]>
Subject: ACPI / Video: Fix initial brightness problem on Acer Ferrari One (v. 2)
On Acer Ferrari One, when _BQC is invoked for the first time, the minimum brightness is returned, so the ACPI video drivers sets the minimum brightness on boot. This is not desirable, so use the following rule:
Set brightness to either the level supposed to be used on AC power, or the brightness level supposed to be used on battery, depending on whether or not the system is on AC power. If these values are not exported by the BIOS, use the brightness level initially returned by _BQC. If that value is invalid, use the maximum brightness level.
Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/acpi/video.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
Index: linux-2.6/drivers/acpi/video.c
===================================================================
drivers/acpi/video.c +1 -0
@@ -44,6 +44,7 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <linux/suspend.h>
+#include <linux/power_supply.h>
 
 #define PREFIX "ACPI: "
 

drivers/acpi/video.c acpi_video_init_brightness(struct acpi_v · line 918

Quoted 3 lines

drivers/acpi/video.c +0 -0

@@ -915,6 +916,18 @@ acpi_video_init_brightness(struct acpi_v
 
 	br->flags._BQC_use_index = (level == max_level ? 0 : 1);

I think the patch will work if we add some code here:

	if (!br->flags._BCL_no_ac_battery_levels) {
		/* the "AC power" and "Battery" brightness value is not reliable */
		br->levels[0] = ...; /* the maximum value? */
		br->levels[1] = ...; /* br->levels[maximum -1 ]? */
	}

thanks,
rui

Quoted 29 lines

drivers/acpi/video.c +14 -2

+	/*
+	 * Set brightness to the "AC power" value if on AC power or to the
+	 * "battery" value otherwise.  If these vaules are not exported, try
+	 * to use the value returned by the initial _BQC and fall back to
+	 * max_level.
+	 */
+	if (!br->flags._BCL_no_ac_battery_levels) {
+		level = power_supply_is_system_supplied() ?
+					br->levels[0] : br->levels[1];
+		goto set_level;
+	}
+
 	if (!br->flags._BQC_use_index) {
 		/*
 		 * Set the backlight to the initial state.

@@ -930,7 +943,8 @@ acpi_video_init_brightness(struct acpi_v
 
 	if (br->flags._BCL_reversed)
 		level_old = (br->count - 1) - level_old;
-	level = br->levels[level_old];
+	level = level_old >= 0 && level_old < br->count ?
+				br->levels[level_old] : max_level;
 
 set_level:
 	result = acpi_video_device_lcd_set_level(device, level);
--

To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Rafael J. Wysocki

author

2010-04-02 17:22 UTC #6 lore raw

Quoted 62 lines · Zhang Rui

Quoted 61 lines · Rafael J. Wysocki
Quoted 59 lines · Zhang Rui
Quoted 58 lines · Zhang Rui
Quoted 42 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
On Acer Ferrari One, when _BQC is invoked for the first time, the minimum brightness is returned, so the ACPI video drivers sets the minimum brightness on boot. This is not desirable, so use the following rule:
If _BQC initially returns an invalid value or the minimum brightness, use either the brightness level supposed to be used on AC power, or the brightness level supposed to be used on battery, depending on whether or not the system is on AC power. If these values are not exported by the BIOS, use the maximum brightness level.
Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/acpi/video.c |   26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)
Index: linux-2.6/drivers/acpi/video.c
===================================================================
drivers/acpi/video.c +6 -2
@@ -44,6 +44,7 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <linux/suspend.h>
+#include <linux/power_supply.h>
 
 #define PREFIX "ACPI: "
 
@@ -920,11 +928,20 @@ acpi_video_init_brightness(struct acpi_v
 		 * Set the backlight to the initial state.
 		 * On some buggy laptops, _BQC returns an uninitialized value
 		 * when invoked for the first time, i.e. level_old is invalid.
-		 * set the backlight to max_level in this case
+		 * On some other systems _BQC returns the minimum brightness
+		 * when invoked for the first time, which also usually is
+		 * undesirable.  In that cases use the "AC power" value if on AC
+		 * power or the "battery" value otherwise.  If these vaules are
+		 * not exported, use max_level.
 		 */
-		for (i = 2; i < br->count; i++)
drivers/acpi/video.c acpi_video_init_brightness(struct acpi_v · line 945
Quoted 10 lines
drivers/acpi/video.c +7 -1
-			if (level_old == br->levels[i])
+		for (i = 3; i < br->count; i++)
+			if (level_old == br->levels[i]) {
 				level = level_old;
+				goto set_level;
+			}
+		if (!br->flags._BCL_no_ac_battery_levels)
+			level = power_supply_is_system_supplied() ?
+
 						br->levels[0] : br->levels[1];
this doesn't work currently.
br->levels[0] and br->levels[1] doesn't equal the "AC power" value and "Battery" value, because the "AC power" and "Battery" value is not exported by many BIOS at all.

Quoted 8 lines

Quoted 3 lines
But if the BIOS doesn't export these values, br->flags._BCL_no_ac_battery_levels will be true, won't it?
no.
for example, I have seen a _BCL package like this: {100, 0, 10, 20, ..., 90, 100}.
Only one value, not sure the "AC power" or "Battery" value, is returned, the br->flags._BCL_no_ac_battery_levels is also set in this case.

Yes, it is, and that's fine.

The code added by the patch won't be executed in that case.

Quoted 38 lines

Quoted 33 lines
---
From: Rafael J. Wysocki <[email protected]>
Subject: ACPI / Video: Fix initial brightness problem on Acer Ferrari One (v. 2)
On Acer Ferrari One, when _BQC is invoked for the first time, the minimum brightness is returned, so the ACPI video drivers sets the minimum brightness on boot. This is not desirable, so use the following rule:
Set brightness to either the level supposed to be used on AC power, or the brightness level supposed to be used on battery, depending on whether or not the system is on AC power. If these values are not exported by the BIOS, use the brightness level initially returned by _BQC. If that value is invalid, use the maximum brightness level.
Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/acpi/video.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
Index: linux-2.6/drivers/acpi/video.c
===================================================================
drivers/acpi/video.c +1 -0
@@ -44,6 +44,7 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <linux/suspend.h>
+#include <linux/power_supply.h>
 
 #define PREFIX "ACPI: "
 
drivers/acpi/video.c acpi_video_init_brightness(struct acpi_v · line 918
Quoted 3 lines
drivers/acpi/video.c +0 -0
@@ -915,6 +916,18 @@ acpi_video_init_brightness(struct acpi_v
 
 	br->flags._BQC_use_index = (level == max_level ? 0 : 1);
 
I think the patch will work if we add some code here:

Quoted 5 lines

	if (!br->flags._BCL_no_ac_battery_levels) {
		/* the "AC power" and "Battery" brightness value is not reliable */
		br->levels[0] = ...; /* the maximum value? */
		br->levels[1] = ...; /* br->levels[maximum -1 ]? */
	}

Well, the code that checks the "AC power" and "battery" levels does this:

        for (i = 2; i < count; i++) {
                if (br->levels[i] == br->levels[0])
                        level_ac_battery++;
                if (br->levels[i] == br->levels[1])
                        level_ac_battery++;
        }

        if (level_ac_battery < 2) {
                level_ac_battery = 2 - level_ac_battery;
                br->flags._BCL_no_ac_battery_levels = 1;

so in your example case the code below won't use the "AC power" and "battery" values, which I believe is correct. [We'll try to use the initial brightness and fall back to max_level in that case, as described in the changelog.]

Quoted 17 lines

drivers/acpi/video.c +12 -0

+	/*
+	 * Set brightness to the "AC power" value if on AC power or to the
+	 * "battery" value otherwise.  If these vaules are not exported, try
+	 * to use the value returned by the initial _BQC and fall back to
+	 * max_level.
+	 */
+	if (!br->flags._BCL_no_ac_battery_levels) {
+		level = power_supply_is_system_supplied() ?
+					br->levels[0] : br->levels[1];
+		goto set_level;
+	}
+
 	if (!br->flags._BQC_use_index) {
 		/*
 		 * Set the backlight to the initial state.

@@ -930,7 +943,8 @@ acpi_video_init_brightness(struct acpi_v

Quoted 8 lines

drivers/acpi/video.c acpi_video_init_brightness(struct acpi_v · line 950

Quoted 8 lines

drivers/acpi/video.c +2 -1

 	if (br->flags._BCL_reversed)
 		level_old = (br->count - 1) - level_old;
-	level = br->levels[level_old];
+	level = level_old >= 0 && level_old < br->count ?
+				br->levels[level_old] : max_level;
 
 set_level:
 	result = acpi_video_device_lcd_set_level(device, level);

Thanks,
Rafael

Rafael J. Wysocki

author

2010-04-04 23:16 UTC #7 lore raw

Quoted 5 lines · Rafael J. Wysocki

Quoted 4 lines · Zhang Rui
Quoted 3 lines · Rafael J. Wysocki
Quoted 2 lines · Zhang Rui
Quoted 1 line · Zhang Rui
On Thu, 2010-04-01 at 09:19 +0800, Rafael J. Wysocki wrote:

This patch may not be necessary after all.

I'll repost it if I find it's still needed.

Rafael

Zhang Rui

2010-04-06 06:03 UTC #8 lore raw

Quoted 11 lines · Rafael J. Wysocki

Quoted 5 lines · Rafael J. Wysocki
Quoted 4 lines · Zhang Rui
Quoted 3 lines · Rafael J. Wysocki
Quoted 2 lines · Zhang Rui
Quoted 1 line · Zhang Rui
On Thu, 2010-04-01 at 09:19 +0800, Rafael J. Wysocki wrote:
This patch may not be necessary after all.
I'll repost it if I find it's still needed.

okay.
I re-read the code and find out that you're right. sorry for the noise.

please feel free to add an Acked-by from me next time you send it to the list.

thanks,
rui

Quoted 5 lines

Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Rafael J. Wysocki

author

2010-04-06 18:48 UTC #9 lore raw

Quoted 10 lines · Zhang Rui

Quoted 9 lines · Rafael J. Wysocki
Quoted 5 lines · Rafael J. Wysocki
Quoted 4 lines · Zhang Rui
Quoted 3 lines · Rafael J. Wysocki
Quoted 2 lines · Zhang Rui
Quoted 1 line · Zhang Rui
On Thu, 2010-04-01 at 09:19 +0800, Rafael J. Wysocki wrote:
This patch may not be necessary after all.

Quoted 7 lines

Quoted 2 lines
I'll repost it if I find it's still needed.
okay.
I re-read the code and find out that you're right. sorry for the noise.
please feel free to add an Acked-by from me next time you send it to the list.

Thanks, but it looks like only one box is affected and because its BIOS returns the minimal brightness value in the initial _BQC. I'm not sure if it's worth changing the current behavior because of that.

Rafael

Zhang Rui

2010-04-07 05:46 UTC #10 lore raw

Quoted 16 lines · Rafael J. Wysocki

Quoted 15 lines · Zhang Rui
Quoted 11 lines · Rafael J. Wysocki
Quoted 5 lines · Rafael J. Wysocki
Quoted 4 lines · Zhang Rui
Quoted 3 lines · Rafael J. Wysocki
Quoted 2 lines · Zhang Rui
Quoted 1 line · Zhang Rui
On Thu, 2010-04-01 at 09:19 +0800, Rafael J. Wysocki wrote:
This patch may not be necessary after all.
I'll repost it if I find it's still needed.
okay.
I re-read the code and find out that you're right. sorry for the noise.

Quoted 9 lines

Quoted 2 lines
please feel free to add an Acked-by from me next time you send it to the list.
Thanks, but it looks like only one box is affected and because its BIOS returns the minimal brightness value in the initial _BQC.
I'm not sure if it's worth
changing the current behavior because of that.

well. in the ACPI video driver, we assume that BIOS sets a PROPER brightness level when loading OS.
so we use the _BQC return value as the default backlight, but apparently this doesn't work well in this BIOS.
I would rather say this is a BIOS problem, which I'm not sure if we should fix it in Linux kernel or not...

thanks,
rui

Quoted 5 lines

Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Rafael J. Wysocki

author

2010-04-07 19:10 UTC #11 lore raw

Quoted 27 lines · Zhang Rui

Quoted 16 lines · Rafael J. Wysocki
Quoted 15 lines · Zhang Rui
Quoted 11 lines · Rafael J. Wysocki
Quoted 5 lines · Rafael J. Wysocki
Quoted 4 lines · Zhang Rui
Quoted 3 lines · Rafael J. Wysocki
Quoted 2 lines · Zhang Rui
Quoted 1 line · Zhang Rui
On Thu, 2010-04-01 at 09:19 +0800, Rafael J. Wysocki wrote:
This patch may not be necessary after all.
I'll repost it if I find it's still needed.
okay.
I re-read the code and find out that you're right. sorry for the noise.
Quoted 9 lines
Quoted 2 lines
please feel free to add an Acked-by from me next time you send it to the list.
Thanks, but it looks like only one box is affected and because its BIOS returns the minimal brightness value in the initial _BQC.
I'm not sure if it's worth
changing the current behavior because of that.

Quoted 6 lines

well. in the ACPI video driver, we assume that BIOS sets a PROPER brightness level when loading OS.
so we use the _BQC return value as the default backlight, but apparently this doesn't work well in this BIOS.
I would rather say this is a BIOS problem, which I'm not sure if we should fix it in Linux kernel or not...

Yes, exactly.

If more systems were affected, I'd say it would be worth considering, but not if there's only one.

Thanks,
Rafael