Re: rc6+ regression - backlight reset to 0 on boot after 7c0ea45be4f114d85ee35caeead8e1660699c46f

[Andrey Borzenkov <arvidjaar@mail.ru>]

[-- Attachment #1: Type: text/plain, Size: 2683 bytes --]

On Thursday 03 April 2008, Zhao Yakui wrote:
> On Wed, 2008-04-02 at 22:53 +0400, Andrey Borzenkov wrote:
> > Commit 7c0ea45be4f114d85ee35caeead8e1660699c46f registers ACPI backlight
> > even if _BQC method (query current backlight level) is missing. The effect is:
> > 
> > during initialization video.c:acpi_video_device_find_cap() calls backlight_update_status(). It tries to fetch actual level via acpi_video_get_brightness() - but as _BQC is missing it just returns current value
> > as stored in memory - i.e. zero, because it was never set. So effectively
> > backlight_update_status() reset brightness to minimal value == 0.
> > 
> > This happens on Toshiba Portege 4000. Verified by reverting commit on top of
> > rc8.
> It seems that _BQC object is missing on the Toshiba Portege 4000. And
> acpi video driver will continue to update the status of backlight even
> when _BQC object is missing, which is inappropriate. 
> Maybe it is more appropriate that OS doesn't update the status of
> backlight in boot phase when _BQC object is missing.
> 
> Of course please attach the output of acpidump in kernel bugzilla.
> http://bugzilla.kernel.org/show_bug.cgi?id=10387

While patch in this bugzilla fixes this immediate regression, I still think
this commit should be reverted for 2.6.25 to discuss design a bit more.
Rationale:

- on systems without _BQC it makes sysfs attribute actual_brightness useless.
Semantic of this is to return *real* brightness as reported by hardware; but
this is noop without _BQC. So currently ACPI simply lies about its value.
If we are going support hardware without _BQC we probably should not define
->get_brightness at all allowing read of actual_brightness to fail.

- on at least some Toshibas we already have brightness control via HCI
(toshiba_acpi):

{pts/0}% ll /sys/class/backlight
итого 0
lrwxrwxrwx 1 root root 0 2008-04-03 22:20 acpi_video0 -> ../../devices/virtual/backlight/acpi_video0/
lrwxrwxrwx 1 root root 0 2008-04-03 22:19 toshiba -> ../../devices/virtual/backlight/toshiba/

both of them refer to exactly the same hardware which is rather confusing.
Something has to be done about it. This is even more confusing because ...

- ... on those old Toshibas ACPI brightness control is far inferior. It
effectively supports only three level of brightness while tochiba_acpi
supports seven of them. So there is no need to keep inferior implementation
if more advanced already exists and works just fine.

This has strong chances of confusing user space about which control is real.
So I think we really have to refrain from pushing this unless the issues above
are settled.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]