* [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).