On 10/08/2012 12:34 PM, Zhang, Rui wrote:
-----Original Message-----
From: Alex Hung [mailto:alex.hung@xxxxxxxxxxxxx]
Sent: Monday, October 01, 2012 11:27 PM
To: Zhang, Rui
Cc: joeyli; linux-acpi@xxxxxxxxxxxxxxx
Subject: Re: [PATCH] acpi: fix brightness level is initialized to zero
when BIOS does not restore the brightness value to _BQC.
Importance: High
Hi Rui,
Yes the module parameter is used temporarily. However, we want to
eliminate the need of using any parameter since non-technical users
won't know them without any help.
The question of whether lowest lowest brightness equals black screen is
debatable and I heard good / bad stories from both sides and they all
makes sense. This, however, does not necessarily play a role here. The
patch is to fix a problem - brightness is set to lowest value (not
user-friendly) when it was not intended.
I also agree that there is nothing wrong with platform setting the
brightness to lowest level during boot and that's a side-effect of this
patch indeed.. In fact, it took me a few weeks to decide it may be an
improvement for the below reasons.
1. In most of cases, lowest level is less desirable or less commonly
used. If a BIOS returns uninitialized valube such as a zero in this
case, Linux causes confusion and problems to users with such platforms.
Comparing to the side-effect, this (may) benefits more users.
2. I also checked ACPI sepc and it says:
B.5.4 _BQC (Brightness Query Current level) This method returns the
current brightness level of a built-in display output device.
The definition makes it hard to argue that it is a BIOS bug not to
restore previous brightness to _BQC. I think it is ambiguous and this
small fix eliminates very dim screen (whether complete black or not) at
boot time.
PS. This may not be relevant, but Windows restores the previous
brightness without BIOS's _BQC,
Where does windows get the previous brightness?
It seems Windows remember the brightness before shutting down and uses
that value when it boots up. It was observed in Windows 7.
and therefore it does not suffer from
very dim / black screen.
Cheers,
Alex Hung
On 10/01/2012 09:36 PM, Zhang, Rui wrote:
I think this is probably what you're looking for,
/*
* Some BIOSes claim they use minimum backlight at boot,
* and this may bring dimming screen after boot
*/
static bool use_bios_initial_backlight = 1;
module_param(use_bios_initial_backlight, bool, 0644);
Lowest brightness level does not equal black screen.
It is not a bug if a platform wants to set its brightness to lowest
brightness level during boot.
If it IS in your case, you can use this module parameter to ignore
the initial _BQC value and use max_level instead.
Thanks,
rui
-----Original Message-----
From: joeyli [mailto:jlee@xxxxxxxx]
Sent: Monday, October 01, 2012 5:19 PM
To: Alex Hung
Cc: Zhang, Rui; linux-acpi@xxxxxxxxxxxxxxx
Subject: Re: [PATCH] acpi: fix brightness level is initialized to
zero when BIOS does not restore the brightness value to _BQC.
Importance: High
於 一，2012-10-01 於 17:11 +0800，Alex Hung 提到：
On 10/01/2012 04:34 PM, joeyli wrote:
於 一，2012-10-01 於 15:17 +0800，joeyli 提到：
於 一，2012-10-01 於 15:03 +0800，Alex Hung 提到：
On 10/01/2012 02:47 PM, joeyli wrote:
Hi Alex,
於 一，2012-10-01 於 13:39 +0800，Alex Hung 提到：
Signed-off-by: Alex Hung <alex.hung@xxxxxxxxxxxxx>
---
drivers/acpi/video.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index
42b226e..eaa9573 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -724,6 +724,10 @@ acpi_video_init_brightness(struct
acpi_video_device *device)
if (level_old == br->levels[i])
level = level_old;
}
+
+ if (level == 0)
+ level = br->levels[(br->count) / 2 + 1];
Looks here used the 50% brightness level.
Per comment in video.c, we want set the backlight to max_level
when level_old is invalid:
if (!br->flags._BQC_use_index) {
/*
* 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
*/
I think here used max_level to fulfill it, e.g.
+ if (level == 0)
+ level = max_level;
How do you think?
Hi Joey,
I was debating with myself which level to be set, ex. 50%, ~75%
or
100%, and I think 50% *might* be closer to normal use-case (just
a
personal guess).
However, "max_level" seems to fit best if we treat the initial
zero brightness in invalid. I can modify it according it that's
preferred.
Thanks for the feedback.
Cheers,
Alex Hung
hm.... I have a question for what's the BIOS's problem that
causes
'level == 0'?
That implied the issue machine's max_level is 0?
/*
* Set the level to maximum and check if _BQC uses
indexed
value
*/
result = acpi_video_device_lcd_set_level(device,
max_level);
/* write max_level purposely, then read level back, compare
them */
...
result =
acpi_video_device_lcd_get_level_current(device,
&level, 0);
...
br->flags._BQC_use_index = (level == max_level ? 0 :
1);
if (!br->flags._BQC_use_index) {
/*
_BQC_use_index is 0 will run into if, means level == max_level */
So, looks the 'level == max_level == 0' when level_old is invalid.
Just wonder what's defect of BIOS (in _BCL?) causes problem.
Sorry for my misunderstood!
I think that's possible the level_old is 0 and there have a 0
value
in the return package from _BCL.
Yes, there is nothing wrong with _BCL and _BQC except that _BQC
returns a zero initially.
Could you please share the _BCL in DSDT from issue machine? Does
there have 0 value in _BCL?
_BCL returns below data and there is a zero in the list.
[ 744.572289] Brightness[0] = 100
[ 744.572293] Brightness[1] = 50
[ 744.572295] Brightness[2] = 0
[ 744.572297] Brightness[3] = 10
[ 744.572299] Brightness[4] = 20
[ 744.572301] Brightness[5] = 30
[ 744.572303] Brightness[6] = 40
[ 744.572305] Brightness[7] = 50
[ 744.572306] Brightness[8] = 60
[ 744.572308] Brightness[9] = 70
[ 744.572310] Brightness[10] = 80
[ 744.572312] Brightness[11] = 90
[ 744.572314] Brightness[12] = 100
The below is the complete _BCL for references
Method (_BCL, 0, Serialized)
{
Name (_T_0, Zero)
If (_OSI ("NOT_WINP_KEY"))
{
While (One)
{
Store (LCDD, _T_0)
If (LEqual (_T_0, 0x303CAF06))
{
Return (AUOL)
}
Else
{
If (LEqual (_T_0, 0x1475AE0D))
{
Return (CMIL)
}
Else
{
If (LEqual (_T_0, 0x033FE430))
{
Return (LGDL)
}
Else
{
If (LEqual (_T_0,
0x3942A34C))
{
Return (SAML)
}
Else
{
Return (DEFL)
}
}
}
}
Break
}
}
Else
{
Return (Package (0x0D)
{
0x64,
0x32,
Zero,
Yes, have Zero value in _BCL return package.
0x0A,
0x14,
0x1E,
0x28,
0x32,
0x3C,
0x46,
0x50,
0x5A,
0x64
})
}
}
According to the above information, it make sense now!
Thank a lot!
Joey Lee
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
