Bug 10387 - rc6+ regression - backlight reset to 0 on boot after 7c0ea45be4f114d85ee35caeead8e1660699c46f
Summary: rc6+ regression - backlight reset to 0 on boot after 7c0ea45be4f114d85ee35cae...
Status: CLOSED CODE_FIX
Alias: None
Product: ACPI
Classification: Unclassified
Component: Power-Video (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: ykzhao
URL:
Keywords:
Depends on:
Blocks: 9832
  Show dependency tree
 
Reported: 2008-04-02 15:41 UTC by Rafael J. Wysocki
Modified: 2008-04-07 03:48 UTC (History)
5 users (show)

See Also:
Kernel Version: 2.6.25-rc6-7c0ea45be4f114d85ee35caeead8e1660699c46f
Tree: Mainline
Regression: Yes


Attachments
try the debug patch (1.28 KB, patch)
2008-04-02 18:48 UTC, ykzhao
Details | Diff
acpidump from Toshiba Portege 4000 (122.62 KB, text/plain)
2008-04-02 19:52 UTC, Andrey Borzenkov
Details

Description Rafael J. Wysocki 2008-04-02 15:41:25 UTC
Subject    : rc6+ regression - backlight reset to 0 on boot after 7c0ea45be4f114d85ee35caeead8e1660699c46f
Submitter  : Andrey Borzenkov <arvidjaar@mail.ru>
Date       : 2008-04-02 22:53
References : http://lkml.org/lkml/2008/4/2/366

This entry is being used for tracking a regression from 2.6.24.  Please don't
close it until the problem is fixed in the mainline.
Comment 1 Rafael J. Wysocki 2008-04-02 15:42:32 UTC
Caused by:

commit 7c0ea45be4f114d85ee35caeead8e1660699c46f
Author: Zhao Yakui <yakui.zhao@intel.com>
Date:   Tue Mar 11 16:56:47 2008 +0800

    ACPI: Ignore _BQC object when registering backlight device

    Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
    Signed-off-by: Zhang Rui  <rui.zhang@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>
Comment 2 ykzhao 2008-04-02 18:48:02 UTC
Created attachment 15586 [details]
try the debug patch

Will you please try the debug patch and see whether the problem still exists?

It is unnecessary to update the status of backligth in boot phase if the _BQC object is missing.

Thanks.
Comment 3 Andrey Borzenkov 2008-04-02 19:52:54 UTC
Created attachment 15588 [details]
acpidump from Toshiba Portege 4000
Comment 4 ykzhao 2008-04-02 23:41:53 UTC
Hi, Andrey
    Will you please try the debug patch in comment #2 and see whether the problem still exists?
    Thanks.
Comment 5 Andrey Borzenkov 2008-04-03 11:35:27 UTC
well, it removes this immediate problem but I still think this commit has to be removed and discussed more; see detailed explanations in LKML thread.
Comment 6 Thomas Renninger 2008-04-06 14:32:43 UTC
Shouldn't the brightness set at boot/init time once if there is no _BQC, or better set/assume brightness->curr to max_brightness in this case?
Everything is better than zero if there is no BQC?
Something like:
device->backlight->props.max_brightness = device->brightness->count-3;
+               if (!device->cap._BQC)
+                       device->brightness->curr = device->backlight->props.max_brightness;
device->backlight->props.brightness =
           acpi_video_get_brightness(device>backlight);
Comment 7 ykzhao 2008-04-06 17:50:11 UTC
It is OK to set the max brightness when there is no _BQC object.
In this case brightness->curr can be initialized correctly. 
Comment 8 Andrey Borzenkov 2008-04-06 19:29:24 UTC
It is *not* OK to set brightness to some arbitrary value. There is no reason to believe it is initialized to max either (what if user boots on battery?).

The only sane thing we can do - do not touch it at all and clearly indicate it to user. Then it is up to user to (try to) change it.

After discussion with Richard I will try to provide patch later this week that

a) initialize brightness to -1 if no _BQC
b) never tries to change brightness in HW if it is -1
c) emits warning on init if no _BQC so user at least knows what's going on
d) always returns -1 if _BQC is not available indicating to user that this value is not known
 
Comment 9 Zhang Rui 2008-04-06 19:50:26 UTC
(In reply to comment #8)
> After discussion with Richard I will try to provide patch later this week
> that
> d) always returns -1 if _BQC is not available indicating to user that this
> value is not known
this doesn't make sense to me.
If it always returns -1, how can user get to know the current brightness levels? how can user set the current brightness level?
IMO, if a value gotten from _BCL can be set to _BCM correctly, we can say that it is the current brightness level, even without _BQC method. :)

Note You need to log in before you can comment on or make changes to this bug.