Bug 13511 (showard314)

Summary: Bad _BQC behavior on HP laptops
Product: ACPI Reporter: showard314
Component: Power-VideoAssignee: Zhang Rui (rui.zhang)
Status: CLOSED CODE_FIX    
Severity: normal CC: acpi-bugzilla, dmitriy.geels, lenb, phcoder, yakui.zhao
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 2.6.28 Subsystem:
Regression: No Bisected commit-id:
Attachments: Patch as provided by Vladimir 'phcoder' Serbinenko
My patch as requested
patch: create symbol link from backlight sys device to ACPI video device
patch: show the physical device node of the backlight class device
patch above, as applied to ACPI tree

Description showard314 2009-06-11 19:53:05 UTC 
When changing brightness using hotkeys and gnome-power-manager, the brightness changes apparently randomly up or down.

Vladimir 'phcoder' Serbinenko has traced this problem to acpi in the kernel and has posted his desciption and patch on the linux-acpi mailing list. His post from the mailing list [http://marc.info/?l=linux-acpi&m=124456088526538&w=2]:

"Hello. On the following model
	Manufacturer: Packard Bell BV
	Product Name: EasyNote_BG45-U-001CH
BQC returns a value between 0 and 15 but the brightness levels are
between 0 and 8. I could make a table of correspondance however I find
it too inflexible. So I propose the following patch which disables BQC
if it behaves badly

-- 
Regards
Vladimir 'phcoder' Serbinenko

["bad_bqc.patch" (text/x-patch)]

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 1bdfb37..1735c34 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -594,6 +594,7 @@ acpi_video_device_lcd_get_level_current(struct acpi_video_device *device,
 					unsigned long long *level)
 {
 	acpi_status status = AE_OK;
+	int i;
 
 	if (device->cap._BQC || device->cap._BCQ) {
 		char *buf = device->cap._BQC ? "_BQC" : "_BCQ";
@@ -609,8 +610,15 @@ acpi_video_device_lcd_get_level_current(struct acpi_video_device *device,
 
 			}
 			*level += bqc_offset_aml_bug_workaround;
-			device->brightness->curr = *level;
-			return 0;
+ 			for (i = 2; i < device->brightness->count; i++)
+ 			        if (device->brightness->levels[i] == *level) {
+ 				        device->brightness->curr = *level;
+ 			                return 0;
+ 			        }
+ 			/* BQC returned an invalid level. Stop using it.  */
+ 			ACPI_WARNING((AE_INFO, "%s returned an invalid level",
+ 				      buf));
+ 			device->cap._BQC = device->cap._BCQ = 0;
 		} else {
 			/* Fixme:
 			 * should we return an error or ignore this failure?
"


I have attached his patch to this bug report.

Additional reporters have tested the patch and confirmed that it has been working at https://bugs.launchpad.net/ubuntu/+source/gnome-power-manager/+bug/376924
Comment 1 showard314 2009-06-11 19:53:46 UTC 
Created attachment 21859 [details]
Patch as provided by Vladimir 'phcoder' Serbinenko
Comment 2 ykzhao 2009-06-12 07:55:35 UTC 
Will you please resend your patch to linux acpi mailing list again so that this patch can be shipped?
   Thanks.
Comment 3 Vladimir Serbinenko 2009-06-13 10:47:30 UTC 
Created attachment 21889 [details]
My patch as requested
Comment 4 Zhang Rui 2009-06-17 02:43:17 UTC 
Created attachment 21952 [details]
patch: create symbol link from backlight sys device to ACPI video device

you have only one backlight sysfs I/F, i.e. /sys/class/backlight/acpi_video0, right?
please apply this patch, and get the output of "cat /sys/class/backlight/acpi_video0/device/path".
this will help us to know which ACPI video device is used.
Comment 5 Zhang Rui 2009-06-18 05:38:18 UTC 
Created attachment 21978 [details]
patch: show the physical device node of the backlight class device

please try this refreshed patch instead.
Comment 6 Dmitriy Geels 2009-06-20 15:11:42 UTC 
Just disassembled laptop's DSDT, there a 3 device LCDD descriptors. All have method _BCL.
First 2 instances:
Device (LCDD)
{
...
Method (_BCL, 0, NotSerialized)
{
    Return (Package (0x10)
    {
        0x0F,
        0x0E,
        0x0D,
        0x0C,
        0x0B,
        0x0A,
        0x09,
        0x08,
        0x07,
        0x06,
        0x05,
        0x04,
        0x03,
        0x02,
        One,
        Zero
    })
}
...
}
And here is third one:
Device (LCDD)
{
...
Method (_BCL, 0, NotSerialized)
{
    Return (Package (0x09)
    {
        0x08,
        0x07,
        0x06,
        0x05,
        0x04,
        0x03,
        0x02,
        One,
        Zero
    })
}
...
}

As far as I understand, this causes backlight control problems and modifying first 2 instances to be like 3d one will fix problem without kernel modification (by overriding DSDT in initrd)?
Comment 7 Dmitriy Geels 2009-06-20 15:32:45 UTC 
Tested patched DSDT on 2.6.28.13 kernel -- no success. It doesn't fix brightness control.
Comment 8 Dmitriy Geels 2009-06-20 15:36:02 UTC 
dmig@dmig-laptop:~$ cat /sys/class/backlight/acpi_video0/device/path
\_SB_.PCI0.VGA_.LCDD
Comment 9 Zhang Rui 2009-06-25 05:57:03 UTC 
hi, Vladimir Serbinenko,
I have send the patch to ACPI mail list, can you reply to that thread and state that you're the author of the patch?
or you can re-attach the refreshed patch with your signed-off?
Comment 10 Zhang Rui 2009-06-25 05:57:43 UTC 
cc   Vladimir Serbinenko
Comment 11 Dmitriy Geels 2009-06-25 18:54:15 UTC 
I noticed strange behavior, which reproduces frequently, but not every boot: gnome-power-manager shows level 0 in popup for actual brightness levels 0 and 1, and shows 100% for others, also increasing brightness, when level is maximal (8) sets it to 0, decreasing when brightness is 0 doesn't have any effect.

Haven't find out conditions to reproduce, sometimes brightness shown correctly and increasing doesn't reset it, sometimes not. If not, reboot may help.

P.S. do I understand right, that bqc patch is included in .31rc1 kernel?
Comment 12 Dmitriy Geels 2009-06-25 19:12:18 UTC 
Just tested kernel .31rc1
Some part of acpi causes oops: http://paste.org.ru/index.pl?rdmv8b
Comment 13 showard314 2009-06-27 23:09:23 UTC 
Zhang: would comment #3 from above suffice as a "signed-off-by" from Vladimir?
Comment 14 Vladimir Serbinenko 2009-06-28 20:48:00 UTC 
@ Comment #13: normally such a change is small enough to be considered trivial and as such not copyrightable. But if you need anything: 
Hereby I release my changes as represented by the patch from comment #3 to a public domain.
Comment 15 Zhang Rui 2009-06-29 01:07:19 UTC 
Vladimir and showard314,
thanks for finding & fixing this problem.

Dmitriy,
I don't know if you have the same bug as them.
please open a new bug report with a detailed description of your bug and attach your acpidump there.
Comment 16 Len Brown 2009-08-29 21:44:52 UTC 
Created attachment 22903 [details]
patch above, as applied to ACPI tree

patch refreshed with correct author & SOB, as applied to ACPI tree.
Comment 18 Len Brown 2009-09-24 23:12:42 UTC 
patch above shipped in linux-2.6.31-git14 -- so you can get
it from kernel.org now.

closed.

commit 4e231fa4cbd3ff53fcb7d76eccd6fd86a152a95f
Author: Vladimir Serbinenko <phcoder@gmail.com>
Date:   Wed Jun 24 15:17:36 2009 +0800

    ACPI video: ignore buggy _BQC