Kernel Bug Tracker – Bug 9277
ACPI video driver should validate brightness level before setting it via _BCM
Last modified: 2008-02-10 14:58:55 UTC
Most recent kernel where this bug did not occur: none
Distribution: Fedora 8
The function acpi_video_device_set_level should validate the brightness level passed to it using the brightness levels obtained from _BCL. This is required in the ACPI spec, section B.6.3.
Created attachment 13372 [details]
Implement brightness validation before calling _BCM
This patch implements the required validation. It also removes this validation from acpi_video_device_write_brightness as the validation would be duplicated otherwise.
Created attachment 13373 [details]
Don't set current brightness variable from acpi_video_device_write_brightness
We don't need to set brightness->curr from acpi_video_device_write_brightness. acpi_video_device_lcd_set_level already does that.
Any updates on the status of this problem and patch? Anything I can do/change to get this bit included?
It seems that Len didn't merge this patch.
Created attachment 14114 [details]
video brightness control fixup
Any comment on this patch? :)
Some time ago, when Fn keys on my Satellite A100 used to work, I did monitor their behaviour with omnibook module . It can only read brightness value for this model, not set it. It turned out then, that every keypress changed the brightness by two values (7 -> 5 -> 3 -> 1 -> 0). There are eight brightness levels supported by this laptop. Now, it is possible to set the desired brightness level by setting a value in /proc/omnibook/lcd, and then pressing fn-f6 or fn-f7 once. Reading the value always returns 0 or 7.
Now the behaviour is screwed up, but at least the buttons work.
I'm not clear about what the omnibook is, but it's apparently not an acpi issue, right?
Sorry that I can't help you on this problem. Please open another bug and assign it to the proper category. :)
Danny, will you please review the patch in comment #5?
(In reply to comment #7)
> I'm not clear about what the omnibook is, but it's apparently not an acpi
> issue, right?
Omnibook is a custom ACPI driver for Toshiba and HP notebooks made by Compal.
> Sorry that I can't help you on this problem. Please open another bug and assign
> it to the proper category. :)
Agreed, the issue in comment #6 looks like the "duplicate video device" issue, so I would also propose opening a new bug with an acpidump attached.
> Danny, will you please review the patch in comment #5?
Will do ASAP - the patch itself looks fine to me, I just need to find some time to build a kernel and test.
Sorry for causing confusion, I'll open a new bug for duplicate video device. I just thought that these issues are related.
(In reply to comment #5)
> Created an attachment (id=14114) [details]
> video brightness control fixup
> Hi, Danny,
> Any comment on this patch? :)
It seems to work fine. Just two comments:
- The value "2" in the validation loop deserves a small comment IMO - it's not intuitive that the first two values in the list are default values ;)
- The brightness list returned by _BCL is not guaranteed to be sorted by values (at least I couldn't find an order requirement in the spec), which your code assumes. I don't know how relevant this is, but we should keep this in mind.
Created attachment 14196 [details]
set the proper brightness levels
Len, is this patch in acpi test tree now? :)
I can confirm that this patch fixes the flicker problem with gnome-power-manager. Remaining issues are most likely caused by duplicate video device issue, described in bug #9614.
patch in comment #11 applied to acpi test
Created attachment 14680 [details]
patch vs 2.6.24
patch in comment #11 dropped from acpi-test.
This patch from Matthew Garret applied instead.
Rather than (fake) percentages, the generic Linux API
wants a series of settings from 0 to max_brightness
all of which do something useful.
This one works even better. Thanks.
thanks for testing.
the patch in comment #15 shipped in Linux-2.6.24-rc22.
ups, i meant 2.6.24-git22
that is the release 22 days after 2.6.24, and presumably
just before 2.6.25-rc1.
IC, thanks for clarification.