laptop_panel.brightness_in_hardware: add all Asus laptops

PATCH Alan Jenkins 2 participants 2 messages Jul 16, 2009 5 messages not in the archive ACPI

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Alan Jenkins

author

2009-07-16 22:22 UTC #5 lore raw

Quoted 4 lines · Matthew Garrett

The problem in that case is that the kernel changes the backlight for us. The lack of consistency on this front makes life somewhat harder.
In terms of the "Some hardware sends keyboard events but also changes the brightness", I'm working on a cleaner solution for this. The easiest would seem to be to generate a uevent when the backlight is changed, which would then allow userspace to pop up UI even though the key press events aren't propagated. It would seem to deal with the Eee (and older Thinkpad) cases quite nicely, but does require some more code in userspace.

Ok. Maybe it's trivial or "niche", but I really appreciate this. I think it's the only remaining issue I have with my EeePC :-).

I just saw the patch to generate uevents for the acpi video driver. It looks like it still generates KEY_BRIGHTNESSDOWN. Are you planning a followup patch, to suppress the input events when brightness_switch_enabled == 1?

Equally, w.r.t patch 3, I don't think eeepc-laptop, should generate brightness events on the *input* device anymore.

The rationale is that KEY_BRIGHTNESSDOWN is a user request to increase brightness. In these cases, the firmware/driver has already increased the brightness. We've notified userspace via the backlight device. So we shouldn't pass the request on to userspace; it has already been acted upon. If we still generate KEY_BRIGHTNESS*, userspace has to guess whether or not the change has already been applied. If there is too much latency, it guesses wrong :-(.

Thanks!
Alan

Matthew Garrett

2009-07-16 22:27 UTC #6 lore raw

Quoted 4 lines · Alan Jenkins

I just saw the patch to generate uevents for the acpi video driver. It looks like it still generates KEY_BRIGHTNESSDOWN. Are you planning a followup patch, to suppress the input events when brightness_switch_enabled == 1?

I didn't want to change the behaviour of the driver with existing userspace, but yes, I agree that that's the correct behaviour. To make it consistent with everything else it should probably also default to brightness_switch_enabled = 0 (and we ship it that way in Fedora), but again, compatibility.

Quoted 2 lines

Equally, w.r.t patch 3, I don't think eeepc-laptop, should generate brightness events on the *input* device anymore.

Right.

Quoted 7 lines

The rationale is that KEY_BRIGHTNESSDOWN is a user request to increase brightness. In these cases, the firmware/driver has already increased the brightness. We've notified userspace via the backlight device. So we shouldn't pass the request on to userspace; it has already been acted upon. If we still generate KEY_BRIGHTNESS*, userspace has to guess whether or not the change has already been applied. If there is too much latency, it guesses wrong :-(.

Absolutely. The other thing that needs doing is for dell-laptop to consume the brightness keys, or alternatively for us to remap them on Dells. Right now (in the worst case) we're getting two sets of them.

Signature · 1 line

Matthew Garrett | [email protected]