GNOME Bugzilla – Bug 436717
gnome-power-manager should properly show the brightness OSD on laptops which do brightness adjustments in hardware but report keypresses via the keyboard
Last modified: 2007-08-18 08:57:27 UTC
I am not quite sure which apps shows the brightness adjustment OSD, but I assume it is gpm. If not, feel free to reassign this bug. Currently the HAL fdi file "10-laptop-panel-hardware.fdi" contains this comment: <!-- On some broken laptops, the brightness control is all done in hardware but the hardware also synthesizes keypresses when the brightness is changed. This gives power manager software problems as the brightness can get into a feedback state so the panel flashes uncontrollably. This is a hardware "feature" seen on some IBM and Lenovo laptops. --> Actually, those laptops are not "broken", but they do it the "right way": they handle brightness in hardware but report that to the software. That way brightness control works everywhere - and software can show a nice OSD if it wants to. gpm should be able to deal with this properly: If one of these laptops are detected, just read back the panel brightness after one of these keypresses is received. And if it changed show the OSD -- but don't change the brightness from software anymore. Actually you can even extend this behaviour in a way that makes it work on both laptops which do brightness control in hardware and those who do not while doing away with this FDI based matching stuff: Every time one of the keypress events is received, read out the brightness. Compare it with what you read the last time. If it changed, the hardware probably changed the brightness internally, and we just show the OSD. If it did not change, then increment or decrement it -- and also show the OSD. That's a simple, robust scheme that will gpm allow to show the OSD for both types of laptops. Of course, if some other app changes the brightness without going thorugh gpm, this algorithm might get confused, but the effect is limited. And AFAIK gpm should be the only one controlling the brightness, right?
>Actually, those laptops are not "broken" They are according to ACPI specifications, and the buttons sent are ChangeBrightness (active) rather than BrightnessChanged (passive). >That way brightness control works everywhere Bzzt. If it's done in hardware you can't do soft-dimming or set policy when on battery and AC - trust me I have one of these machines. There are actually four classes of machines, those that provide input and change in hardware, those that provide input and don't change, those that change in hardware and don't notify and those that provide input and can't be changed. >If it changed, the hardware probably changed the brightness internally, and we just show the OSD. Yes, there is code in g-pm to do exactly that: /* We only want to change the brightness if the machine does not do it on it's own updates, as this can make the panel flash in a feedback loop. */ res = gpm_hal_device_get_bool (brightness->priv->hal, brightness->priv->udi, "laptop_panel.brightness_in_hardware", &brightness->priv->does_own_updates, NULL); What laptop do you have? What is the value of laptop_panel.brightness_in_hardware? What versions hal and hal-info do you have installed? Thanks.
Mhmm. I am not sure where you found anything about handling keypresses in the ACPI spec. Also the handling of the brightness on my laptop is unrelated to ACPI. My laptop doesn't have any ACPI methods for controlling the brightness. My laptop supports changing the brightness from software. Doing "soft-dimming" and setting policy when on battery and AC works perfectly. I don't really understand what you are trying to say? Have you thought about my suggestion to remove "laptop_panel.brightness_in_hardware" by just reading back the brightness everytime a keypress event is received and iff it didn't change from the last time doing the increase/decrease in software?
Oh, and the algorithm I suggested works with three of the four types you mentioned. It works with all types which provide input. It doesn't work -- of course -- with the type that doesn't notify about input.
(In reply to comment #3) > Oh, and the algorithm I suggested works with three of the four types you > mentioned. It works with all types which provide input. It doesn't work -- of > course -- with the type that doesn't notify about input. You're assuming the event comes after the action. Richard.
Yes, I do. But I have a good reason to: the keypress is being forwarded to software only for the purpose of showing the OSD. Thus reading out the brightness after getting the key is definitely clean. On my MSI S270 laptop the windows driver works that way. It just hooks into the Brightness-up/brightness-down keys and everytime an event is received it reads out the brightness and shows an OSD. And if you really feel that this is too unsafe, you might wait 10ms or so after receiving the event, before reading out the brightness. Even the slowest hardware should be able to do brightness changes in that time. Also remember that usually the ACPI EC handles those brightness changes -- and is integrated with the kbd controller. The EC is a microcontroller with connections to the SMBUS. It doesn't do much besides being the kbd controller and managing brightness. Thus, it should be really fast when handling those jobs. Certainly much faster than Linux is with passing the kbd event through the kernel and several software layers to HAL and gpm. But honestly I believe that these 10ms are not necessary, in no case.
I don't appreciate being bullied, I'm quite aware how the EC is wired into the hardware. I'm not prepared to do a wait, read, and then write on each keypress, it's slow enough already, sorry.
Hmm, sorry you feel bullied, that was not my intention. Sorry for that. Could you please elaborate why exactly you have set this bug to WONTFIX? As I mentioned I am quite sure that the 10ms are not necessary in any case. Also, please note that even if they event comes before the EC data is updated, this is not a problem in any way -- because those cases you can still catch by using the .fdi data you already ship. So, the logic I propose will make the brightness OSD work perfectly on most laptops. And if there really are laptops where the data is updated after the keypress you get them to work by .fdi files. Looks like an easy and comprehensive solution to me. And no 10ms necessary anywhere. Also, EC reads themselves are very fast. This won't slow down brightness control in any measurable way.
>So, the logic I propose will make the brightness OSD work perfectly on most laptops. If you work on a patch, I'll test it on my laptops here. >Also, EC reads themselves are very fast. I know, it's the userspace HAL/script bits I'm worried about.
Okay, lets give this a try; 2007-07-26 Richard Hughes <richard@hughsie.com> * src/gpm-brightness-lcd.c: (gpm_brightness_lcd_set_hw), (gpm_brightness_lcd_dim_hw_step), (gpm_brightness_lcd_dim_hw), (gpm_brightness_lcd_get), (gpm_brightness_lcd_up), (gpm_brightness_lcd_down), (gpm_brightness_lcd_init): Try to discover the laptop backlight type by checking the backlight state before we try and set it. Fixes #436717 If this breaks for people with 2.19.6, I'll revert it for 2.20.
Seems to work fine for me! Thank you very much! Lennart
seems it breaks for some people on ubuntu now, see bug https://bugs.launchpad.net/gnome-power/+bug/122666