Bug 9277

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

Hi, Danny,
Any comment on this patch? :)

Hi.

Some time ago, when Fn keys on my Satellite A100 used to work, I did monitor their behaviour with omnibook module [1]. 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.

[1] http://omnibook.sf.net

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.

closed.

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.