* [PATCH] acpi: fix brightness level is initialized to zero when BIOS does not restore the brightness value to _BQC.
@ 2012-10-01 5:39 Alex Hung
2012-10-01 6:47 ` joeyli
0 siblings, 1 reply; 11+ messages in thread
From: Alex Hung @ 2012-10-01 5:39 UTC (permalink / raw)
To: alex.hung, rui.zhang, linux-acpi
Signed-off-by: Alex Hung <alex.hung@canonical.com>
---
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];
+
goto set_level;
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] acpi: fix brightness level is initialized to zero when BIOS does not restore the brightness value to _BQC.
2012-10-01 5:39 [PATCH] acpi: fix brightness level is initialized to zero when BIOS does not restore the brightness value to _BQC Alex Hung
@ 2012-10-01 6:47 ` joeyli
2012-10-01 7:03 ` Alex Hung
0 siblings, 1 reply; 11+ messages in thread
From: joeyli @ 2012-10-01 6:47 UTC (permalink / raw)
To: Alex Hung; +Cc: rui.zhang, linux-acpi
Hi Alex,
於 一，2012-10-01 於 13:39 +0800，Alex Hung 提到：
> Signed-off-by: Alex Hung <alex.hung@canonical.com>
> ---
> 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?
> +
> goto set_level;
> }
>
Thanks a lot!
Joey Lee
--
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] acpi: fix brightness level is initialized to zero when BIOS does not restore the brightness value to _BQC.
2012-10-01 6:47 ` joeyli
@ 2012-10-01 7:03 ` Alex Hung
2012-10-01 7:17 ` joeyli
0 siblings, 1 reply; 11+ messages in thread
From: Alex Hung @ 2012-10-01 7:03 UTC (permalink / raw)
To: joeyli; +Cc: rui.zhang, linux-acpi
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@canonical.com>
>> ---
>> 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
>
>> +
>> goto set_level;
>> }
>>
>
> Thanks a lot!
> Joey Lee
>
>
>
>
--
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] acpi: fix brightness level is initialized to zero when BIOS does not restore the brightness value to _BQC.
2012-10-01 7:03 ` Alex Hung
@ 2012-10-01 7:17 ` joeyli
2012-10-01 8:34 ` joeyli
0 siblings, 1 reply; 11+ messages in thread
From: joeyli @ 2012-10-01 7:17 UTC (permalink / raw)
To: Alex Hung; +Cc: rui.zhang, linux-acpi
於 一，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@canonical.com>
> >> ---
> >> 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.
Thanks a lot!
Joey Lee
--
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] acpi: fix brightness level is initialized to zero when BIOS does not restore the brightness value to _BQC.
2012-10-01 7:17 ` joeyli
@ 2012-10-01 8:34 ` joeyli
2012-10-01 9:11 ` Alex Hung
0 siblings, 1 reply; 11+ messages in thread
From: joeyli @ 2012-10-01 8:34 UTC (permalink / raw)
To: Alex Hung; +Cc: rui.zhang, linux-acpi
於 一，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@canonical.com>
> > >> ---
> > >> 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.
Could you please share the _BCL in DSDT from issue machine? Does there
have 0 value in _BCL?
Thanks a lot!
Joey Lee
--
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] acpi: fix brightness level is initialized to zero when BIOS does not restore the brightness value to _BQC.
2012-10-01 8:34 ` joeyli
@ 2012-10-01 9:11 ` Alex Hung
2012-10-01 9:19 ` joeyli
0 siblings, 1 reply; 11+ messages in thread
From: Alex Hung @ 2012-10-01 9:11 UTC (permalink / raw)
To: joeyli; +Cc: rui.zhang, linux-acpi
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@canonical.com>
>>>>> ---
>>>>> 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,
0x0A,
0x14,
0x1E,
0x28,
0x32,
0x3C,
0x46,
0x50,
0x5A,
0x64
})
}
}
>
>
> Thanks a lot!
> Joey Lee
>
>
--
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] acpi: fix brightness level is initialized to zero when BIOS does not restore the brightness value to _BQC.
2012-10-01 9:11 ` Alex Hung
@ 2012-10-01 9:19 ` joeyli
2012-10-01 13:36 ` Zhang, Rui
0 siblings, 1 reply; 11+ messages in thread
From: joeyli @ 2012-10-01 9:19 UTC (permalink / raw)
To: Alex Hung; +Cc: rui.zhang, linux-acpi
於 一，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@canonical.com>
> >>>>> ---
> >>>>> 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@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] acpi: fix brightness level is initialized to zero when BIOS does not restore the brightness value to _BQC.
2012-10-01 9:19 ` joeyli
@ 2012-10-01 13:36 ` Zhang, Rui
2012-10-01 15:26 ` Alex Hung
0 siblings, 1 reply; 11+ messages in thread
From: Zhang, Rui @ 2012-10-01 13:36 UTC (permalink / raw)
To: joeyli, Alex Hung; +Cc: linux-acpi@vger.kernel.org
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@suse.com]
> Sent: Monday, October 01, 2012 5:19 PM
> To: Alex Hung
> Cc: Zhang, Rui; linux-acpi@vger.kernel.org
> 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@canonical.com>
> > >>>>> ---
> > >>>>> 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] acpi: fix brightness level is initialized to zero when BIOS does not restore the brightness value to _BQC.
2012-10-01 13:36 ` Zhang, Rui
@ 2012-10-01 15:26 ` Alex Hung
2012-10-08 4:34 ` Zhang, Rui
0 siblings, 1 reply; 11+ messages in thread
From: Alex Hung @ 2012-10-01 15:26 UTC (permalink / raw)
To: Zhang, Rui; +Cc: joeyli, linux-acpi@vger.kernel.org
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, 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@suse.com]
>> Sent: Monday, October 01, 2012 5:19 PM
>> To: Alex Hung
>> Cc: Zhang, Rui; linux-acpi@vger.kernel.org
>> 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@canonical.com>
>>>>>>>> ---
>>>>>>>> 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@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] acpi: fix brightness level is initialized to zero when BIOS does not restore the brightness value to _BQC.
2012-10-01 15:26 ` Alex Hung
@ 2012-10-08 4:34 ` Zhang, Rui
2012-10-08 4:39 ` Alex Hung
0 siblings, 1 reply; 11+ messages in thread
From: Zhang, Rui @ 2012-10-08 4:34 UTC (permalink / raw)
To: Alex Hung; +Cc: joeyli, linux-acpi@vger.kernel.org
> -----Original Message-----
> From: Alex Hung [mailto:alex.hung@canonical.com]
> Sent: Monday, October 01, 2012 11:27 PM
> To: Zhang, Rui
> Cc: joeyli; linux-acpi@vger.kernel.org
> 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?
> 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@suse.com]
> >> Sent: Monday, October 01, 2012 5:19 PM
> >> To: Alex Hung
> >> Cc: Zhang, Rui; linux-acpi@vger.kernel.org
> >> 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@canonical.com>
> >>>>>>>> ---
> >>>>>>>> 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
> >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] acpi: fix brightness level is initialized to zero when BIOS does not restore the brightness value to _BQC.
2012-10-08 4:34 ` Zhang, Rui
@ 2012-10-08 4:39 ` Alex Hung
0 siblings, 0 replies; 11+ messages in thread
From: Alex Hung @ 2012-10-08 4:39 UTC (permalink / raw)
To: Zhang, Rui; +Cc: joeyli, linux-acpi@vger.kernel.org
On 10/08/2012 12:34 PM, Zhang, Rui wrote:
>> -----Original Message-----
>> From: Alex Hung [mailto:alex.hung@canonical.com]
>> Sent: Monday, October 01, 2012 11:27 PM
>> To: Zhang, Rui
>> Cc: joeyli; linux-acpi@vger.kernel.org
>> 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@suse.com]
>>>> Sent: Monday, October 01, 2012 5:19 PM
>>>> To: Alex Hung
>>>> Cc: Zhang, Rui; linux-acpi@vger.kernel.org
>>>> 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@canonical.com>
>>>>>>>>>> ---
>>>>>>>>>> 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@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-10-08 4:39 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-01 5:39 [PATCH] acpi: fix brightness level is initialized to zero when BIOS does not restore the brightness value to _BQC Alex Hung
2012-10-01 6:47 ` joeyli
2012-10-01 7:03 ` Alex Hung
2012-10-01 7:17 ` joeyli
2012-10-01 8:34 ` joeyli
2012-10-01 9:11 ` Alex Hung
2012-10-01 9:19 ` joeyli
2012-10-01 13:36 ` Zhang, Rui
2012-10-01 15:26 ` Alex Hung
2012-10-08 4:34 ` Zhang, Rui
2012-10-08 4:39 ` Alex Hung
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).