[PATCH 1/2] ACPI / video: Fix initial level validity test

[PATCH 1/2] ACPI / video: Fix initial level validity test

[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 <aaron.lu@intel.com>
Cc: Danny Baumann <dannybaumann@web.de>
Cc: stable <stable@kernel.org>
---
 drivers/acpi/video.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 38c3a28d6392..bf521b36c2f9 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -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;
-- 
1.8.4.39.ga0d3f10

[PATCH 2/2] ACPI / video: Quirk initial backlight level 0

[Aaron Lu]

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
Reported-by: Qingshuai Tian <qingshuai.tian@intel.com>
Tested-by: Aaron Lu <aaron.lu@intel.com> on "Idealpad u330p"
Reported-and-tested-by: <erno@iki.fi> on "Acer Aspire V5-573G"
Reported-and-tested-by: Kirill Tkhai <tkhai@yandex.ru> on "HP 250 G1"
Signed-off-by: Aaron Lu <aaron.lu@intel.com>
Cc: stable <stable@vger.kernel.org>
---
 drivers/acpi/video.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index bf521b36c2f9..a049fa9360d0 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -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;
 	}
 
-- 
1.8.4.39.ga0d3f10

Re: [PATCH 2/2] ACPI / video: Quirk initial backlight level 0

[Aaron Lu]

On 11/06/2013 09:07 AM, Aaron Lu wrote:
> 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
> Reported-by: Qingshuai Tian <qingshuai.tian@intel.com>
> Tested-by: Aaron Lu <aaron.lu@intel.com> on "Idealpad u330p"
> Reported-and-tested-by: <erno@iki.fi> on "Acer Aspire V5-573G"
> Reported-and-tested-by: Kirill Tkhai <tkhai@yandex.ru> 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 <aaron.lu@intel.com>
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.

Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
 drivers/acpi/video.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index a049fa9360d0..18dbdff4656e 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -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"),
-		},
-	},
 	{}
 };
 
-- 
1.8.4.39.ga0d3f10


> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> Cc: stable <stable@vger.kernel.org>
> ---
>  drivers/acpi/video.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index bf521b36c2f9..a049fa9360d0 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -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 1/2] ACPI / video: Fix initial level validity test

[Rafael J. Wysocki]

On Wednesday, November 06, 2013 09:03:15 AM Aaron Lu wrote:
> 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 <aaron.lu@intel.com>
> Cc: Danny Baumann <dannybaumann@web.de>
> Cc: stable <stable@kernel.org>

Any pointers to bug reports, BZ entries, etc?

> ---
>  drivers/acpi/video.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 38c3a28d6392..bf521b36c2f9 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -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;
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 2/2] ACPI / video: Quirk initial backlight level 0

[Rafael J. Wysocki]

On Wednesday, November 06, 2013 09:07:10 AM Aaron Lu wrote:
> 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
> Reported-by: Qingshuai Tian <qingshuai.tian@intel.com>
> Tested-by: Aaron Lu <aaron.lu@intel.com> on "Idealpad u330p"
> Reported-and-tested-by: <erno@iki.fi> on "Acer Aspire V5-573G"
> Reported-and-tested-by: Kirill Tkhai <tkhai@yandex.ru> on "HP 250 G1"
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> Cc: stable <stable@vger.kernel.org>

All stable, or any particular series?

> ---
>  drivers/acpi/video.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index bf521b36c2f9..a049fa9360d0 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -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;
>  	}
>  
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 1/2] ACPI / video: Fix initial level validity test

[Aaron Lu]

On 11/07/2013 06:03 AM, Rafael J. Wysocki wrote:
> On Wednesday, November 06, 2013 09:03:15 AM Aaron Lu wrote:
>> 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 <aaron.lu@intel.com>
>> Cc: Danny Baumann <dannybaumann@web.de>
>> Cc: stable <stable@kernel.org>
> 
> 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

> 
>> ---
>>  drivers/acpi/video.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
>> index 38c3a28d6392..bf521b36c2f9 100644
>> --- a/drivers/acpi/video.c
>> +++ b/drivers/acpi/video.c
>> @@ -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;
>>

Re: [PATCH 2/2] ACPI / video: Quirk initial backlight level 0

[Aaron Lu]

On 11/07/2013 06:04 AM, Rafael J. Wysocki wrote:
> On Wednesday, November 06, 2013 09:07:10 AM Aaron Lu wrote:
>> 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
>> Reported-by: Qingshuai Tian <qingshuai.tian@intel.com>
>> Tested-by: Aaron Lu <aaron.lu@intel.com> on "Idealpad u330p"
>> Reported-and-tested-by: <erno@iki.fi> on "Acer Aspire V5-573G"
>> Reported-and-tested-by: Kirill Tkhai <tkhai@yandex.ru> on "HP 250 G1"
>> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
>> Cc: stable <stable@vger.kernel.org>
> 
> All stable, or any particular series?

All stable.

Thanks,
Aaron

> 
>> ---
>>  drivers/acpi/video.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
>> index bf521b36c2f9..a049fa9360d0 100644
>> --- a/drivers/acpi/video.c
>> +++ b/drivers/acpi/video.c
>> @@ -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;
>>  	}
>>  
>>