ACPI / video: Fix initial level validity test
PATCH 1/2 ACPI / video: Fix initial level validity test
When testing if the firmware's initial value is valid, we should use the corrected level value instead of the raw value returned from firmware.
--- drivers/acpi/video.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/acpi/video.c +1 -1
@@ -856,7 +856,7 @@ acpi_video_init_brightness(struct acpi_video_device *device)* or an index). Set the backlight to max_level in this case. */ for (i =2 ; i < br->count; i++) -if (level_old == br->levels[i]) +if (level == br->levels[i])break ;if (i == br->count) level = max_level;
Signature · 1 line
1.8.4.39.ga0d3f10
PATCH 2/2 ACPI / video: Quirk initial backlight level 0
Some firmware doesn't initialize initial backlight level to a proper value and _BQC will return 0 on first time evaluation. We used to be able to detect such incorrect value with our code logic, as value 0 normally isn't a valid value in _BCL. But with the introduction of Win8, firmware begins to fill _BCL with values from 0 to 100, now 0 becomes a valid value but that value will make user's screen black. This patch test initial _BQC for value 0, if such a value is returned, do not use it.
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=64031
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=61231
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=63111
--- drivers/acpi/video.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/acpi/video.c +1 -1
@@ -858,7 +858,7 @@ acpi_video_init_brightness(struct acpi_video_device *device)for (i =2 ; i < br->count; i++)if (level == br->levels[i])break ; -if (i == br->count) +if (i == br->count || !level) level = max_level; }
Signature · 1 line
1.8.4.39.ga0d3f10
Re: [PATCH 2/2] ACPI / video: Quirk initial backlight level 0
Quoted 9 lines · Aaron Lu
Some firmware doesn't initialize initial backlight level to a proper value and
_BQCwill return 0 on first time evaluation. We used to be able to detect such incorrect value with our code logic, as value 0 normally isn't a valid value in_BCL. But with the introduction of Win8, firmware begins to fill_BCLwith values from 0 to 100, now 0 becomes a valid value but that value will make user's screen black. This patch test initial_BQCfor value 0, if such a value is returned, do not use it.
Quoted 7 lines
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=64031
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=61231
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=63111Reported-by Qingshuai Tian <[email protected]>Reported-and-tested-by <[email protected]> on "Acer Aspire V5-573G"Reported-and-tested-by Kirill Tkhai <[email protected]> on "HP 250 G1"
With this patch applied, we do not need commit e37f14a5fb85 "ACPI / video: Ignore BIOS initial backlight value for HP 250 G1" any more.
So I made a revert patch on top of the two.
From: Aaron Lu <[email protected]>
Subject: [PATCH] Revert "ACPI / video: Ignore BIOS initial backlight value for
HP 250 G1"
This reverts commit e37f14a5fb85522f3bbf88ece6134c4e610ed598.
It turned out other systems also share the same problem with bug 63111 so I made a patch to catch initial brightness level 0 problem. With that patch applied, we do not need to place HP 250 G1 in DMI table.
--- drivers/acpi/video.c | 8 -------- 1 file changed, 8 deletions(-)
drivers/acpi/video.c +0 -8
@@ -504,14 +504,6 @@ static struct dmi_system_id video_dmi_table[] __initdata = { DMI_MATCH(DMI_PRODUCT_NAME, "HP Pavilion m4 Notebook PC"), }, }, - { - .callback = video_ignore_initial_backlight, - .ident = "HP 250 G1", - .matches = { - DMI_MATCH(DMI_BOARD_VENDOR, "Hewlett-Packard"), - DMI_MATCH(DMI_PRODUCT_NAME, "HP 250 G1 Notebook PC"), - }, - }, {} };
Signature · 1 line
1.8.4.39.ga0d3f10
Quoted 20 lines
Signed-off-by Aaron Lu <[email protected]>Cc stable <[email protected]>--- drivers/acpi/video.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/acpi/video.c +1
-1@@ -858,7 +858,7 @@ acpi_video_init_brightness(struct acpi_video_device *device)for (i =2 ; i < br->count; i++)if (level == br->levels[i])break ; -if (i == br->count) +if (i == br->count || !level) level = max_level; }
Re: [PATCH 2/2] ACPI / video: Quirk initial backlight level 0
In reply to Aaron LuQuoted 12 lines · Aaron Lu
Some firmware doesn't initialize initial backlight level to a proper value and
_BQCwill return 0 on first time evaluation. We used to be able to detect such incorrect value with our code logic, as value 0 normally isn't a valid value in_BCL. But with the introduction of Win8, firmware begins to fill_BCLwith values from 0 to 100, now 0 becomes a valid value but that value will make user's screen black. This patch test initial_BQCfor value 0, if such a value is returned, do not use it.Reference: https://bugzilla.kernel.org/show_bug.cgi?id=64031
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=61231
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=63111
Quoted 6 lines
Reported-by Qingshuai Tian <[email protected]>Reported-and-tested-by <[email protected]> on "Acer Aspire V5-573G"Reported-and-tested-by Kirill Tkhai <[email protected]> on "HP 250 G1"Signed-off-by Aaron Lu <[email protected]>Cc stable <[email protected]>
All stable, or any particular series?
Quoted 18 lines
--- drivers/acpi/video.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/acpi/video.c +1
-1@@ -858,7 +858,7 @@ acpi_video_init_brightness(struct acpi_video_device *device)for (i =2 ; i < br->count; i++)if (level == br->levels[i])break ; -if (i == br->count) +if (i == br->count || !level) level = max_level; }
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
Re: [PATCH 2/2] ACPI / video: Quirk initial backlight level 0
Quoted 20 lines · Rafael J. Wysocki
Quoted 12 lines · Aaron Lu
Some firmware doesn't initialize initial backlight level to a proper value and
_BQCwill return 0 on first time evaluation. We used to be able to detect such incorrect value with our code logic, as value 0 normally isn't a valid value in_BCL. But with the introduction of Win8, firmware begins to fill_BCLwith values from 0 to 100, now 0 becomes a valid value but that value will make user's screen black. This patch test initial_BQCfor value 0, if such a value is returned, do not use it.Reference: https://bugzilla.kernel.org/show_bug.cgi?id=64031
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=61231
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=63111Quoted 6 lines
Reported-by Qingshuai Tian <[email protected]>Reported-and-tested-by <[email protected]> on "Acer Aspire V5-573G"Reported-and-tested-by Kirill Tkhai <[email protected]> on "HP 250 G1"Signed-off-by Aaron Lu <[email protected]>Cc stable <[email protected]>
Quoted 1 line
All stable, or any particular series?
All stable.
Thanks,
Aaron
Quoted 19 lines
Quoted 19 lines
--- drivers/acpi/video.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/acpi/video.c +1
-1@@ -858,7 +858,7 @@ acpi_video_init_brightness(struct acpi_video_device *device)for (i =2 ; i < br->count; i++)if (level == br->levels[i])break ; -if (i == br->count) +if (i == br->count || !level) level = max_level; }
Quoted 7 lines · Aaron Lu
When testing if the firmware's initial value is valid, we should use the corrected level value instead of the raw value returned from firmware.
Signed-off-by Aaron Lu <[email protected]>Cc Danny Baumann <[email protected]>Cc stable <[email protected]>
Any pointers to bug reports, BZ entries, etc?
Quoted 18 lines
--- drivers/acpi/video.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/acpi/video.c +1
-1@@ -856,7 +856,7 @@ acpi_video_init_brightness(struct acpi_video_device *device)* or an index). Set the backlight to max_level in this case. */ for (i =2 ; i < br->count; i++) -if (level_old == br->levels[i]) +if (level == br->levels[i])break ;if (i == br->count) level = max_level;
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
Quoted 10 lines · Rafael J. Wysocki
Quoted 7 lines · Aaron Lu
When testing if the firmware's initial value is valid, we should use the corrected level value instead of the raw value returned from firmware.
Signed-off-by Aaron Lu <[email protected]>Cc Danny Baumann <[email protected]>Cc stable <[email protected]>Any pointers to bug reports, BZ entries, etc?
This patch doesn't solve any bugs, I found the problem while preparing patch 2/2. Now I remembered some stable rule that patch that solves a potential bug isn't acceptable, so the stable tag should be dropped.
Thanks,
Aaron
Quoted 19 lines
Quoted 19 lines
--- drivers/acpi/video.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/acpi/video.c +1
-1@@ -856,7 +856,7 @@ acpi_video_init_brightness(struct acpi_video_device *device)* or an index). Set the backlight to max_level in this case. */ for (i =2 ; i < br->count; i++) -if (level_old == br->levels[i]) +if (level == br->levels[i])break ;if (i == br->count) level = max_level;