| Message ID | 1349069987-23992-1-git-send-email-alex.hung@canonical.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
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
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
? ??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
? ??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
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
? ??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
SSB0aGluayB0aGlzIGlzIHByb2JhYmx5IHdoYXQgeW91J3JlIGxvb2tpbmcgZm9yLA0KDQovKg0K ICogU29tZSBCSU9TZXMgY2xhaW0gdGhleSB1c2UgbWluaW11bSBiYWNrbGlnaHQgYXQgYm9vdCwN CiAqIGFuZCB0aGlzIG1heSBicmluZyBkaW1taW5nIHNjcmVlbiBhZnRlciBib290DQogKi8NCnN0 YXRpYyBib29sIHVzZV9iaW9zX2luaXRpYWxfYmFja2xpZ2h0ID0gMTsNCm1vZHVsZV9wYXJhbSh1 c2VfYmlvc19pbml0aWFsX2JhY2tsaWdodCwgYm9vbCwgMDY0NCk7DQoNCg0KTG93ZXN0IGJyaWdo dG5lc3MgbGV2ZWwgZG9lcyBub3QgZXF1YWwgYmxhY2sgc2NyZWVuLg0KSXQgaXMgbm90IGEgYnVn IGlmIGEgcGxhdGZvcm0gd2FudHMgdG8gc2V0IGl0cyBicmlnaHRuZXNzIHRvIGxvd2VzdCBicmln aHRuZXNzIGxldmVsIGR1cmluZyBib290Lg0KSWYgaXQgSVMgaW4geW91ciBjYXNlLCB5b3UgY2Fu IHVzZSB0aGlzIG1vZHVsZSBwYXJhbWV0ZXIgdG8gaWdub3JlIHRoZSBpbml0aWFsIF9CUUMgdmFs dWUgYW5kIHVzZSBtYXhfbGV2ZWwgaW5zdGVhZC4NCg0KVGhhbmtzLA0KcnVpDQoNCj4gLS0tLS1P cmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogam9leWxpIFttYWlsdG86amxlZUBzdXNlLmNv bV0NCj4gU2VudDogTW9uZGF5LCBPY3RvYmVyIDAxLCAyMDEyIDU6MTkgUE0NCj4gVG86IEFsZXgg SHVuZw0KPiBDYzogWmhhbmcsIFJ1aTsgbGludXgtYWNwaUB2Z2VyLmtlcm5lbC5vcmcNCj4gU3Vi amVjdDogUmU6IFtQQVRDSF0gYWNwaTogZml4IGJyaWdodG5lc3MgbGV2ZWwgaXMgaW5pdGlhbGl6 ZWQgdG8gemVybw0KPiB3aGVuIEJJT1MgZG9lcyBub3QgcmVzdG9yZSB0aGUgYnJpZ2h0bmVzcyB2 YWx1ZSB0byBfQlFDLg0KPiBJbXBvcnRhbmNlOiBIaWdoDQo+IA0KPiDmlrwg5LiA77yMMjAxMi0x MC0wMSDmlrwgMTc6MTEgKzA4MDDvvIxBbGV4IEh1bmcg5o+Q5Yiw77yaDQo+ID4gT24gMTAvMDEv MjAxMiAwNDozNCBQTSwgam9leWxpIHdyb3RlOg0KPiA+ID4g5pa8IOS4gO+8jDIwMTItMTAtMDEg 5pa8IDE1OjE3ICswODAw77yMam9leWxpIOaPkOWIsO+8mg0KPiA+ID4+IOaWvCDkuIDvvIwyMDEy LTEwLTAxIOaWvCAxNTowMyArMDgwMO+8jEFsZXggSHVuZyDmj5DliLDvvJoNCj4gPiA+Pj4gT24g MTAvMDEvMjAxMiAwMjo0NyBQTSwgam9leWxpIHdyb3RlOg0KPiA+ID4+Pj4gSGkgQWxleCwNCj4g PiA+Pj4+DQo+ID4gPj4+PiDmlrwg5LiA77yMMjAxMi0xMC0wMSDmlrwgMTM6MzkgKzA4MDDvvIxB bGV4IEh1bmcg5o+Q5Yiw77yaDQo+ID4gPj4+Pj4gU2lnbmVkLW9mZi1ieTogQWxleCBIdW5nIDxh bGV4Lmh1bmdAY2Fub25pY2FsLmNvbT4NCj4gPiA+Pj4+PiAtLS0NCj4gPiA+Pj4+PiAgICBkcml2 ZXJzL2FjcGkvdmlkZW8uYyB8ICAgIDQgKysrKw0KPiA+ID4+Pj4+ICAgIDEgZmlsZXMgY2hhbmdl ZCwgNCBpbnNlcnRpb25zKCspLCAwIGRlbGV0aW9ucygtKQ0KPiA+ID4+Pj4+DQo+ID4gPj4+Pj4g ZGlmZiAtLWdpdCBhL2RyaXZlcnMvYWNwaS92aWRlby5jIGIvZHJpdmVycy9hY3BpL3ZpZGVvLmMg aW5kZXgNCj4gPiA+Pj4+PiA0MmIyMjZlLi5lYWE5NTczIDEwMDY0NA0KPiA+ID4+Pj4+IC0tLSBh L2RyaXZlcnMvYWNwaS92aWRlby5jDQo+ID4gPj4+Pj4gKysrIGIvZHJpdmVycy9hY3BpL3ZpZGVv LmMNCj4gPiA+Pj4+PiBAQCAtNzI0LDYgKzcyNCwxMCBAQCBhY3BpX3ZpZGVvX2luaXRfYnJpZ2h0 bmVzcyhzdHJ1Y3QNCj4gYWNwaV92aWRlb19kZXZpY2UgKmRldmljZSkNCj4gPiA+Pj4+PiAgICAJ CQkJaWYgKGxldmVsX29sZCA9PSBici0+bGV2ZWxzW2ldKQ0KPiA+ID4+Pj4+ICAgIAkJCQkJbGV2 ZWwgPSBsZXZlbF9vbGQ7DQo+ID4gPj4+Pj4gICAgCQl9DQo+ID4gPj4+Pj4gKw0KPiA+ID4+Pj4+ ICsJCWlmIChsZXZlbCA9PSAwKQ0KPiA+ID4+Pj4+ICsJCQlsZXZlbCA9IGJyLT5sZXZlbHNbKGJy LT5jb3VudCkgLyAyICsgMV07DQo+ID4gPj4+Pg0KPiA+ID4+Pj4gTG9va3MgaGVyZSB1c2VkIHRo ZSA1MCUgYnJpZ2h0bmVzcyBsZXZlbC4NCj4gPiA+Pj4+DQo+ID4gPj4+PiBQZXIgY29tbWVudCBp biB2aWRlby5jLCB3ZSB3YW50IHNldCB0aGUgYmFja2xpZ2h0IHRvIG1heF9sZXZlbA0KPiA+ID4+ Pj4gd2hlbiBsZXZlbF9vbGQgaXMgaW52YWxpZDoNCj4gPiA+Pj4+DQo+ID4gPj4+PiAgICAgICAg ICAgaWYgKCFici0+ZmxhZ3MuX0JRQ191c2VfaW5kZXgpIHsNCj4gPiA+Pj4+ICAgICAgICAgICAg ICAgICAgIC8qDQo+ID4gPj4+PiAgICAgICAgICAgICAgICAgICAgKiBTZXQgdGhlIGJhY2tsaWdo dCB0byB0aGUgaW5pdGlhbCBzdGF0ZS4NCj4gPiA+Pj4+ICAgICAgICAgICAgICAgICAgICAqIE9u IHNvbWUgYnVnZ3kgbGFwdG9wcywgX0JRQyByZXR1cm5zIGFuDQo+IHVuaW5pdGlhbGl6ZWQgdmFs dWUNCj4gPiA+Pj4+ICAgICAgICAgICAgICAgICAgICAqIHdoZW4gaW52b2tlZCBmb3IgdGhlIGZp cnN0IHRpbWUsIGkuZS4NCj4gbGV2ZWxfb2xkIGlzIGludmFsaWQuDQo+ID4gPj4+PiAgICAgICAg ICAgICAgICAgICAgKiBzZXQgdGhlIGJhY2tsaWdodCB0byBtYXhfbGV2ZWwgaW4gdGhpcyBjYXNl DQo+ID4gPj4+PiAgICAgICAgICAgICAgICAgICAgKi8NCj4gPiA+Pj4+DQo+ID4gPj4+PiBJIHRo aW5rIGhlcmUgdXNlZCBtYXhfbGV2ZWwgdG8gZnVsZmlsbCBpdCwgZS5nLg0KPiA+ID4+Pj4NCj4g PiA+Pj4+ICsJCWlmIChsZXZlbCA9PSAwKQ0KPiA+ID4+Pj4gKwkJCWxldmVsID0gbWF4X2xldmVs Ow0KPiA+ID4+Pj4NCj4gPiA+Pj4+IEhvdyBkbyB5b3UgdGhpbms/DQo+ID4gPj4+IEhpIEpvZXks DQo+ID4gPj4+DQo+ID4gPj4+IEkgd2FzIGRlYmF0aW5nIHdpdGggbXlzZWxmIHdoaWNoIGxldmVs IHRvIGJlIHNldCwgZXguIDUwJSwgfjc1JQ0KPiBvcg0KPiA+ID4+PiAxMDAlLCBhbmQgSSB0aGlu ayA1MCUgKm1pZ2h0KiBiZSBjbG9zZXIgdG8gbm9ybWFsIHVzZS1jYXNlIChqdXN0DQo+IGENCj4g PiA+Pj4gcGVyc29uYWwgZ3Vlc3MpLg0KPiA+ID4+Pg0KPiA+ID4+PiBIb3dldmVyLCAibWF4X2xl dmVsIiBzZWVtcyB0byBmaXQgYmVzdCBpZiB3ZSB0cmVhdCB0aGUgaW5pdGlhbA0KPiA+ID4+PiB6 ZXJvIGJyaWdodG5lc3MgaW4gaW52YWxpZC4gSSBjYW4gbW9kaWZ5IGl0IGFjY29yZGluZyBpdCB0 aGF0J3MNCj4gcHJlZmVycmVkLg0KPiA+ID4+Pg0KPiA+ID4+PiBUaGFua3MgZm9yIHRoZSBmZWVk YmFjay4NCj4gPiA+Pj4NCj4gPiA+Pj4gQ2hlZXJzLA0KPiA+ID4+PiBBbGV4IEh1bmcNCj4gPiA+ Pj4NCj4gPiA+Pg0KPiA+ID4+IGhtLi4uLiBJIGhhdmUgYSBxdWVzdGlvbiBmb3Igd2hhdCdzIHRo ZSBCSU9TJ3MgcHJvYmxlbSB0aGF0IGNhdXNlcw0KPiA+ID4+ICdsZXZlbCA9PSAwJz8NCj4gPiA+ PiBUaGF0IGltcGxpZWQgdGhlIGlzc3VlIG1hY2hpbmUncyBtYXhfbGV2ZWwgaXMgMD8NCj4gPiA+ Pg0KPiA+ID4+IAkJLyoNCj4gPiA+PiAJCSAqIFNldCB0aGUgbGV2ZWwgdG8gbWF4aW11bSBhbmQg Y2hlY2sgaWYgX0JRQyB1c2VzIGluZGV4ZWQNCj4gdmFsdWUNCj4gPiA+PiAJCSAqLw0KPiA+ID4+ IAkJcmVzdWx0ID0gYWNwaV92aWRlb19kZXZpY2VfbGNkX3NldF9sZXZlbChkZXZpY2UsIG1heF9s ZXZlbCk7DQo+IAkJLyogd3JpdGUgbWF4X2xldmVsIHB1cnBvc2VseSwgdGhlbiByZWFkIGxldmVs IGJhY2ssIGNvbXBhcmUNCj4gdGhlbSAqLw0KPiA+ID4+IAkJLi4uDQo+ID4gPj4gCQlyZXN1bHQg PSBhY3BpX3ZpZGVvX2RldmljZV9sY2RfZ2V0X2xldmVsX2N1cnJlbnQoZGV2aWNlLA0KPiAmbGV2 ZWwsIDApOw0KPiA+ID4+IAkJLi4uDQo+ID4gPj4gCQlici0+ZmxhZ3MuX0JRQ191c2VfaW5kZXgg PSAobGV2ZWwgPT0gbWF4X2xldmVsID8gMCA6IDEpOw0KPiA+ID4+IAkJaWYgKCFici0+ZmxhZ3Mu X0JRQ191c2VfaW5kZXgpIHsJCQkJLyoNCj4gX0JRQ191c2VfaW5kZXggaXMgMCB3aWxsIHJ1biBp bnRvIGlmLCBtZWFucyBsZXZlbCA9PSBtYXhfbGV2ZWwgKi8NCj4gPiA+Pg0KPiA+ID4+IFNvLCBs b29rcyB0aGUgJ2xldmVsID09IG1heF9sZXZlbCA9PSAwJyB3aGVuIGxldmVsX29sZCBpcyBpbnZh bGlkLg0KPiA+ID4+DQo+ID4gPj4gSnVzdCB3b25kZXIgd2hhdCdzIGRlZmVjdCBvZiBCSU9TIChp biBfQkNMPykgY2F1c2VzIHByb2JsZW0uDQo+ID4gPj4NCj4gPiA+Pg0KPiA+ID4NCj4gPiA+IFNv cnJ5IGZvciBteSBtaXN1bmRlcnN0b29kIQ0KPiA+ID4NCj4gPiA+IEkgdGhpbmsgdGhhdCdzIHBv c3NpYmxlIHRoZSBsZXZlbF9vbGQgaXMgMCBhbmQgdGhlcmUgaGF2ZSBhIDAgdmFsdWUNCj4gPiA+ IGluIHRoZSByZXR1cm4gcGFja2FnZSBmcm9tIF9CQ0wuDQo+ID4gPg0KPiA+DQo+ID4gWWVzLCB0 aGVyZSBpcyBub3RoaW5nIHdyb25nIHdpdGggX0JDTCBhbmQgX0JRQyBleGNlcHQgdGhhdCBfQlFD DQo+ID4gcmV0dXJucyBhIHplcm8gaW5pdGlhbGx5Lg0KPiA+DQo+ID4gPiBDb3VsZCB5b3UgcGxl YXNlIHNoYXJlIHRoZSBfQkNMIGluIERTRFQgZnJvbSBpc3N1ZSBtYWNoaW5lPyBEb2VzDQo+ID4g PiB0aGVyZSBoYXZlIDAgdmFsdWUgaW4gX0JDTD8NCj4gPg0KPiA+IF9CQ0wgcmV0dXJucyBiZWxv dyBkYXRhIGFuZCB0aGVyZSBpcyBhIHplcm8gaW4gdGhlIGxpc3QuDQo+ID4NCj4gPiBbICA3NDQu NTcyMjg5XSBCcmlnaHRuZXNzWzBdID0gMTAwDQo+ID4gWyAgNzQ0LjU3MjI5M10gQnJpZ2h0bmVz c1sxXSA9IDUwDQo+ID4gWyAgNzQ0LjU3MjI5NV0gQnJpZ2h0bmVzc1syXSA9IDANCj4gPiBbICA3 NDQuNTcyMjk3XSBCcmlnaHRuZXNzWzNdID0gMTANCj4gPiBbICA3NDQuNTcyMjk5XSBCcmlnaHRu ZXNzWzRdID0gMjANCj4gPiBbICA3NDQuNTcyMzAxXSBCcmlnaHRuZXNzWzVdID0gMzANCj4gPiBb ICA3NDQuNTcyMzAzXSBCcmlnaHRuZXNzWzZdID0gNDANCj4gPiBbICA3NDQuNTcyMzA1XSBCcmln aHRuZXNzWzddID0gNTANCj4gPiBbICA3NDQuNTcyMzA2XSBCcmlnaHRuZXNzWzhdID0gNjANCj4g PiBbICA3NDQuNTcyMzA4XSBCcmlnaHRuZXNzWzldID0gNzANCj4gPiBbICA3NDQuNTcyMzEwXSBC cmlnaHRuZXNzWzEwXSA9IDgwDQo+ID4gWyAgNzQ0LjU3MjMxMl0gQnJpZ2h0bmVzc1sxMV0gPSA5 MA0KPiA+IFsgIDc0NC41NzIzMTRdIEJyaWdodG5lc3NbMTJdID0gMTAwDQo+ID4NCj4gPiBUaGUg YmVsb3cgaXMgdGhlIGNvbXBsZXRlIF9CQ0wgZm9yIHJlZmVyZW5jZXMNCj4gPg0KPiA+ICAgICAg ICAgICAgICAgICAgTWV0aG9kIChfQkNMLCAwLCBTZXJpYWxpemVkKQ0KPiA+ICAgICAgICAgICAg ICAgICAgew0KPiA+ICAgICAgICAgICAgICAgICAgICAgIE5hbWUgKF9UXzAsIFplcm8pDQo+ID4g ICAgICAgICAgICAgICAgICAgICAgSWYgKF9PU0kgKCJOT1RfV0lOUF9LRVkiKSkNCj4gPiAgICAg ICAgICAgICAgICAgICAgICB7DQo+ID4gICAgICAgICAgICAgICAgICAgICAgICAgIFdoaWxlIChP bmUpDQo+ID4gICAgICAgICAgICAgICAgICAgICAgICAgIHsNCj4gPiAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgIFN0b3JlIChMQ0RELCBfVF8wKQ0KPiA+ICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgSWYgKExFcXVhbCAoX1RfMCwgMHgzMDNDQUYwNikpDQo+ID4gICAgICAgICAgICAg ICAgICAgICAgICAgICAgICB7DQo+ID4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg UmV0dXJuIChBVU9MKQ0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgfQ0KPiA+ICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgRWxzZQ0KPiA+ICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgew0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIElmIChMRXF1 YWwgKF9UXzAsIDB4MTQ3NUFFMEQpKQ0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgIHsNCj4gPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgUmV0dXJuIChD TUlMKQ0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIH0NCj4gPiAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICBFbHNlDQo+ID4gICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgew0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBJ ZiAoTEVxdWFsIChfVF8wLCAweDAzM0ZFNDMwKSkNCj4gPiAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgew0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgUmV0dXJuIChMR0RMKQ0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICB9DQo+ID4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIEVsc2UNCj4g PiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgew0KPiA+ICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgSWYgKExFcXVhbCAoX1RfMCwNCj4gMHgzOTQy QTM0QykpDQo+ID4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICB7DQo+ ID4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgUmV0dXJuIChT QU1MKQ0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgfQ0KPiA+ ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgRWxzZQ0KPiA+ICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgew0KPiA+ICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIFJldHVybiAoREVGTCkNCj4gPiAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIH0NCj4gPiAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgfQ0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgIH0NCj4gPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIH0NCj4gPg0KPiA+ICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgQnJlYWsNCj4gPiAgICAgICAgICAgICAgICAgICAgICAg ICAgfQ0KPiA+ICAgICAgICAgICAgICAgICAgICAgIH0NCj4gPiAgICAgICAgICAgICAgICAgICAg ICBFbHNlDQo+ID4gICAgICAgICAgICAgICAgICAgICAgew0KPiA+ICAgICAgICAgICAgICAgICAg ICAgICAgICBSZXR1cm4gKFBhY2thZ2UgKDB4MEQpDQo+ID4gICAgICAgICAgICAgICAgICAgICAg ICAgIHsNCj4gPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIDB4NjQsDQo+ID4gICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAweDMyLA0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgWmVybywNCj4gDQo+IFllcywgaGF2ZSBaZXJvIHZhbHVlIGluIF9CQ0wgcmV0dXJuIHBh Y2thZ2UuDQo+IA0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgMHgwQSwNCj4gPiAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgIDB4MTQsDQo+ID4gICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAweDFFLA0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgMHgyOCwN Cj4gPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIDB4MzIsDQo+ID4gICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAweDNDLA0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg MHg0NiwNCj4gPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIDB4NTAsDQo+ID4gICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAweDVBLA0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgMHg2NA0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICB9KQ0KPiA+ICAgICAgICAg ICAgICAgICAgICAgIH0NCj4gPiAgICAgICAgICAgICAgICAgIH0NCj4gPg0KPiA+DQo+IA0KPiBB Y2NvcmRpbmcgdG8gdGhlIGFib3ZlIGluZm9ybWF0aW9uLCBpdCBtYWtlIHNlbnNlIG5vdyENCj4g DQo+IA0KPiBUaGFuayBhIGxvdCENCj4gSm9leSBMZWUNCg0K -- 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
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
> -----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 > >
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
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; }
Signed-off-by: Alex Hung <alex.hung@canonical.com> --- drivers/acpi/video.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)