diff mbox

disable _BQC/_BCQ if it returns invalid values

Message ID 4A54FE96.2030407@marcansoft.com (mailing list archive)
State RFC, archived
Delegated to: Zhang Rui
Headers show

Commit Message

Hector Martin July 8, 2009, 8:16 p.m. UTC
Validate the result of the _BQC or _BCQ method when querying the current
backlight level. If the value is out of range, the support for this
method is disabled. This does not break functionality as long as the
hardware doesn't touch the backlight level on it own, since the driver
will just use the last cached value.

Acer Aspire 8930G with BIOS v1.10 (latest) has this issue.

Signed-off-by: Hector Martin <hector@marcansoft.com>

Comments

Zhang Rui July 9, 2009, 12:57 a.m. UTC | #1
On Thu, 2009-07-09 at 04:16 +0800, Hector Martin wrote:
> Validate the result of the _BQC or _BCQ method when querying the current
> backlight level. If the value is out of range, the support for this
> method is disabled. This does not break functionality as long as the
> hardware doesn't touch the backlight level on it own, since the driver
> will just use the last cached value.
> 
> Acer Aspire 8930G with BIOS v1.10 (latest) has this issue.
> 
> Signed-off-by: Hector Martin <hector@marcansoft.com>
> --- linux-2.6.30/drivers/acpi/video.c	2009-07-08 22:04:54.000000000 +0200
> +++ linux-2.6.30-mod/drivers/acpi/video.c	2009-07-08 22:12:19.000000000 +0200
> @@ -602,15 +605,29 @@ acpi_video_device_lcd_get_level_current(
>  						NULL, level);
>  		if (ACPI_SUCCESS(status)) {
>  			if (device->brightness->flags._BQC_use_index) {
> +				if (*level >= (device->brightness->count - 2)) {
> +					ACPI_WARNING((AE_INFO,
> +						"%s is broken (returned %llu)",
> +						buf, *level));
> +					device->cap._BQC = device->cap._BCQ = 0;
> +					*level = device->brightness->curr;
> +					return 0;
> +				}
>  				if (device->brightness->flags._BCL_reversed)
>  					*level = device->brightness->count
>  								 - 3 - (*level);
>  				*level = device->brightness->levels[*level + 2];
>  
>  			}
> -			*level += bqc_offset_aml_bug_workaround;
> -			device->brightness->curr = *level;
> -			return 0;
> +			if (*level > 100) {
> +				ACPI_WARNING((AE_INFO, "%s is broken "
> +					"(returned %llu)", buf, *level));
_BQC returns a value larger than 100?
is this true on your laptop?

thanks,
rui

> +				device->cap._BQC = device->cap._BCQ = 0;
> +			} else {
> +				*level += bqc_offset_aml_bug_workaround;
> +				device->brightness->curr = *level;
> +				return 0;
> +			}
>  		} else {
>  			/* Fixme:
>  			 * should we return an error or ignore this failure?
> 
> 

--
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
Hector Martin July 9, 2009, 1:27 a.m. UTC | #2
Zhang Rui wrote:
> _BQC returns a value larger than 100?
> is this true on your laptop?

Yes, it returns random garbage. It is completely broken. There's an if()
in the ASL that disables the entire method (including no valid return
value) when a certain EC SRAM register is 0. The backlight code also
constantly clears this register. I suppose the EC should theoretically
set it at some point, but it doesn't (at least not reliably). In any
case, _BQC isn't needed here since the hardware doesn't change the
screen brightness autonomously, and disabling the method as soon as it
misbehaves works just fine.
diff mbox

Patch

--- linux-2.6.30/drivers/acpi/video.c	2009-07-08 22:04:54.000000000 +0200
+++ linux-2.6.30-mod/drivers/acpi/video.c	2009-07-08 22:12:19.000000000 +0200
@@ -602,15 +605,29 @@  acpi_video_device_lcd_get_level_current(
 						NULL, level);
 		if (ACPI_SUCCESS(status)) {
 			if (device->brightness->flags._BQC_use_index) {
+				if (*level >= (device->brightness->count - 2)) {
+					ACPI_WARNING((AE_INFO,
+						"%s is broken (returned %llu)",
+						buf, *level));
+					device->cap._BQC = device->cap._BCQ = 0;
+					*level = device->brightness->curr;
+					return 0;
+				}
 				if (device->brightness->flags._BCL_reversed)
 					*level = device->brightness->count
 								 - 3 - (*level);
 				*level = device->brightness->levels[*level + 2];
 
 			}
-			*level += bqc_offset_aml_bug_workaround;
-			device->brightness->curr = *level;
-			return 0;
+			if (*level > 100) {
+				ACPI_WARNING((AE_INFO, "%s is broken "
+					"(returned %llu)", buf, *level));
+				device->cap._BQC = device->cap._BCQ = 0;
+			} else {
+				*level += bqc_offset_aml_bug_workaround;
+				device->brightness->curr = *level;
+				return 0;
+			}
 		} else {
 			/* Fixme:
 			 * should we return an error or ignore this failure?