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

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

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

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 <rjw@sisk.pl>
---
 drivers/acpi/video.c |   26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

Index: linux-2.6/drivers/acpi/video.c
===================================================================
--- linux-2.6.orig/drivers/acpi/video.c
+++ 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;
 	}
 

Re: [PATCH] ACPI / Video: Fix initial brightness problem on Acer Ferrari One

[Zhang Rui]

On Thu, 2010-04-01 at 09:19 +0800, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> 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 <rjw@sisk.pl>
> ---
>  drivers/acpi/video.c |   26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6/drivers/acpi/video.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/video.c
> +++ 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];

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



>  		goto set_level;
>  	}
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI / Video: Fix initial brightness problem on Acer Ferrari One

[Zhang Rui]

On Thu, 2010-04-01 at 09:34 +0800, Zhang Rui wrote:
> On Thu, 2010-04-01 at 09:19 +0800, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > 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 <rjw@sisk.pl>
> > ---
> >  drivers/acpi/video.c |   26 +++++++++++++++++++++++---
> >  1 file changed, 23 insertions(+), 3 deletions(-)
> > 
> > Index: linux-2.6/drivers/acpi/video.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/acpi/video.c
> > +++ 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];
> 
> 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.

> 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
> 
> 
> 
> >  		goto set_level;
> >  	}
> >  
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI / Video: Fix initial brightness problem on Acer Ferrari One

[Rafael J. Wysocki]

On Thursday 01 April 2010, Zhang Rui wrote:
> On Thu, 2010-04-01 at 09:34 +0800, Zhang Rui wrote:
> > On Thu, 2010-04-01 at 09:19 +0800, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > 
> > > 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 <rjw@sisk.pl>
> > > ---
> > >  drivers/acpi/video.c |   26 +++++++++++++++++++++++---
> > >  1 file changed, 23 insertions(+), 3 deletions(-)
> > > 
> > > Index: linux-2.6/drivers/acpi/video.c
> > > ===================================================================
> > > --- linux-2.6.orig/drivers/acpi/video.c
> > > +++ 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];
> > 
> > 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?

> 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 <rjw@sisk.pl>
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 <rjw@sisk.pl>
---
 drivers/acpi/video.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/acpi/video.c
===================================================================
--- linux-2.6.orig/drivers/acpi/video.c
+++ 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);

Re: [PATCH] ACPI / Video: Fix initial brightness problem on Acer Ferrari One

[Rafael J. Wysocki]

On Thursday 01 April 2010, Rafael J. Wysocki wrote:
> On Thursday 01 April 2010, Zhang Rui wrote:
> > On Thu, 2010-04-01 at 09:34 +0800, Zhang Rui wrote:
> > > On Thu, 2010-04-01 at 09:19 +0800, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <rjw@sisk.pl>
...
> > > > +		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?
> 
> > 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 <rjw@sisk.pl>
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 <rjw@sisk.pl>
---
 drivers/acpi/video.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/acpi/video.c
===================================================================
--- linux-2.6.orig/drivers/acpi/video.c
+++ 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);

Re: [PATCH] ACPI / Video: Fix initial brightness problem on Acer Ferrari One

[Zhang Rui]

On Fri, 2010-04-02 at 03:45 +0800, Rafael J. Wysocki wrote:
> On Thursday 01 April 2010, Zhang Rui wrote:
> > On Thu, 2010-04-01 at 09:34 +0800, Zhang Rui wrote:
> > > On Thu, 2010-04-01 at 09:19 +0800, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > > 
> > > > 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 <rjw@sisk.pl>
> > > > ---
> > > >  drivers/acpi/video.c |   26 +++++++++++++++++++++++---
> > > >  1 file changed, 23 insertions(+), 3 deletions(-)
> > > > 
> > > > Index: linux-2.6/drivers/acpi/video.c
> > > > ===================================================================
> > > > --- linux-2.6.orig/drivers/acpi/video.c
> > > > +++ 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];
> > > 
> > > 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?
> 
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.


> > 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 <rjw@sisk.pl>
> 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 <rjw@sisk.pl>
> ---
>  drivers/acpi/video.c |   16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6/drivers/acpi/video.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/video.c
> +++ 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);
>  
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
> +	/*
> +	 * 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI / Video: Fix initial brightness problem on Acer Ferrari One

[Rafael J. Wysocki]

On Friday 02 April 2010, Zhang Rui wrote:
> On Fri, 2010-04-02 at 03:45 +0800, Rafael J. Wysocki wrote:
> > On Thursday 01 April 2010, Zhang Rui wrote:
> > > On Thu, 2010-04-01 at 09:34 +0800, Zhang Rui wrote:
> > > > On Thu, 2010-04-01 at 09:19 +0800, Rafael J. Wysocki wrote:
> > > > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > > > 
> > > > > 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 <rjw@sisk.pl>
> > > > > ---
> > > > >  drivers/acpi/video.c |   26 +++++++++++++++++++++++---
> > > > >  1 file changed, 23 insertions(+), 3 deletions(-)
> > > > > 
> > > > > Index: linux-2.6/drivers/acpi/video.c
> > > > > ===================================================================
> > > > > --- linux-2.6.orig/drivers/acpi/video.c
> > > > > +++ 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];
> > > > 
> > > > 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?
> > 
> 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.

> > ---
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 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 <rjw@sisk.pl>
> > ---
> >  drivers/acpi/video.c |   16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> > 
> > Index: linux-2.6/drivers/acpi/video.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/acpi/video.c
> > +++ 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);
> >  
> 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 ]? */
> 	}

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.]

> > +	/*
> > +	 * 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);

Thanks,
Rafael

Re: [PATCH] ACPI / Video: Fix initial brightness problem on Acer Ferrari One

[Rafael J. Wysocki]

On Friday 02 April 2010, Rafael J. Wysocki wrote:
> On Friday 02 April 2010, Zhang Rui wrote:
> > On Fri, 2010-04-02 at 03:45 +0800, Rafael J. Wysocki wrote:
> > > On Thursday 01 April 2010, Zhang Rui wrote:
> > > > On Thu, 2010-04-01 at 09:34 +0800, Zhang Rui wrote:
> > > > > 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

Re: [PATCH] ACPI / Video: Fix initial brightness problem on Acer Ferrari One

[Zhang Rui]

On Mon, 2010-04-05 at 07:19 +0800, Rafael J. Wysocki wrote:
> On Friday 02 April 2010, Rafael J. Wysocki wrote:
> > On Friday 02 April 2010, Zhang Rui wrote:
> > > On Fri, 2010-04-02 at 03:45 +0800, Rafael J. Wysocki wrote:
> > > > On Thursday 01 April 2010, Zhang Rui wrote:
> > > > > On Thu, 2010-04-01 at 09:34 +0800, Zhang Rui wrote:
> > > > > > 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

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

Re: [PATCH] ACPI / Video: Fix initial brightness problem on Acer Ferrari One

[Rafael J. Wysocki]

On Tuesday 06 April 2010, Zhang Rui wrote:
> On Mon, 2010-04-05 at 07:19 +0800, Rafael J. Wysocki wrote:
> > On Friday 02 April 2010, Rafael J. Wysocki wrote:
> > > On Friday 02 April 2010, Zhang Rui wrote:
> > > > On Fri, 2010-04-02 at 03:45 +0800, Rafael J. Wysocki wrote:
> > > > > On Thursday 01 April 2010, Zhang Rui wrote:
> > > > > > On Thu, 2010-04-01 at 09:34 +0800, Zhang Rui wrote:
> > > > > > > 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, 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

Re: [PATCH] ACPI / Video: Fix initial brightness problem on Acer Ferrari One

[Zhang Rui]

On Wed, 2010-04-07 at 02:51 +0800, Rafael J. Wysocki wrote:
> On Tuesday 06 April 2010, Zhang Rui wrote:
> > On Mon, 2010-04-05 at 07:19 +0800, Rafael J. Wysocki wrote:
> > > On Friday 02 April 2010, Rafael J. Wysocki wrote:
> > > > On Friday 02 April 2010, Zhang Rui wrote:
> > > > > On Fri, 2010-04-02 at 03:45 +0800, Rafael J. Wysocki wrote:
> > > > > > On Thursday 01 April 2010, Zhang Rui wrote:
> > > > > > > On Thu, 2010-04-01 at 09:34 +0800, Zhang Rui wrote:
> > > > > > > > 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, 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

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

Re: [PATCH] ACPI / Video: Fix initial brightness problem on Acer Ferrari One

[Rafael J. Wysocki]

On Wednesday 07 April 2010, Zhang Rui wrote:
> On Wed, 2010-04-07 at 02:51 +0800, Rafael J. Wysocki wrote:
> > On Tuesday 06 April 2010, Zhang Rui wrote:
> > > On Mon, 2010-04-05 at 07:19 +0800, Rafael J. Wysocki wrote:
> > > > On Friday 02 April 2010, Rafael J. Wysocki wrote:
> > > > > On Friday 02 April 2010, Zhang Rui wrote:
> > > > > > On Fri, 2010-04-02 at 03:45 +0800, Rafael J. Wysocki wrote:
> > > > > > > On Thursday 01 April 2010, Zhang Rui wrote:
> > > > > > > > On Thu, 2010-04-01 at 09:34 +0800, Zhang Rui wrote:
> > > > > > > > > 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, 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...

Yes, exactly.

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

Thanks,
Rafael