After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 392022 - brightness slider (on battery only) doesn't work properly
brightness slider (on battery only) doesn't work properly
Status: RESOLVED FIXED
Product: gnome-power-manager
Classification: Deprecated
Component: gnome-power-manager
SVN TRUNK
Other Linux
: Normal normal
: ---
Assigned To: GNOME Power Manager Maintainer(s)
GNOME Power Manager Maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2007-01-02 19:51 UTC by Matteo Zandi
Modified: 2007-02-09 23:28 UTC
See Also:
GNOME target: ---
GNOME version: 2.15/2.16


Attachments
fglrx script to change powerstates at ac/battery event (1.90 KB, application/x-shellscript)
2007-01-02 20:44 UTC, Matteo Zandi
Details

Description Matteo Zandi 2007-01-02 19:51:59 UTC
This is just to report that, when my toshiba satellite M40-281 (omnibook module) is running on battery, the brightness slider (in either g-p-m or the brightness applet) doesn't work properly. If you entered the battery 'state' with the slider set to 0, the brightness actually gets set to zero, if you slide just one step up then the screen gets completely bright (as if it was a the end of the slider). I'm not able  to get brightness back to zero, unless I plug and unplug AC.

Sorry if I was not able to get a look at the code, I compiled CVS (with great efforts) in order the file the bug wich I also found on 2.17.4 and in order to have the very latest version of this wonderful application!

Would also be possible to add the fglrx-powersave setting? IE

matteo@burnt:~/testroot2/src/pics$ aticonfig --lsp
    core/mem      [flags]
-----------------
* 1: 105/149 MHz  [low voltage]
  2: 209/182 MHz  [low voltage]
  3: 351/284 MHz  [default state]
matteo@burnt:~/testroot2/src/pics$ aticonfig --set-powerstate 3
Warning : Option 'PowerState' is exclusive, other options will be ignored.
matteo@burnt:~/testroot2/src/pics$ aticonfig --lsp
    core/mem      [flags]
-----------------
  1: 105/149 MHz  [low voltage]
  2: 209/182 MHz  [low voltage]
* 3: 351/284 MHz  [default state]

Thanks,
Matteo
Comment 1 Richard Hughes 2007-01-02 20:19:02 UTC
What's the output of "lshal | grep laptop_panel"?
What does (using SVN/CVS head) do?:

dbus-send --session --dest=org.gnome.PowerManager --type=method_call --print-reply /org/gnome/PowerManager/BrightnessLcd org.gnome.PowerManager.SetPolicy int32:0
dbus-send --session --dest=org.gnome.PowerManager --type=method_call --print-reply /org/gnome/PowerManager/BrightnessLcd org.gnome.PowerManager.SetPolicy int32:50
dbus-send --session --dest=org.gnome.PowerManager --type=method_call --print-reply /org/gnome/PowerManager/BrightnessLcd org.gnome.PowerManager.SetPolicy int32:100
dbus-send --session --dest=org.gnome.PowerManager --type=method_call --print-reply /org/gnome/PowerManager/BrightnessLcd org.gnome.PowerManager.SetPolicy int32:75

dbus-send --session --dest=org.gnome.PowerManager --type=method_call --print-reply /org/gnome/PowerManager/BrightnessLcd org.gnome.PowerManager.GetPolicy

Regarding the ati config tool, I think this either belongs in HAL or pm-utils (the latter being my best guess). As I don't have an ATI card, I've never used the application before, but I assume it's like the nvidia card utility that uses coolbits. This has been discussed on the HAL lists before, with no big argument either way. Is there a 100% fail-safe command that can be issued trivially to set the card into a low power state, and vice-versa to maximum or do we have to process the output of "aticonfig --lsp"?

Thanks, Richard.
Comment 2 Matteo Zandi 2007-01-02 20:44:48 UTC
Created attachment 79215 [details]
fglrx script to change powerstates at ac/battery event
Comment 3 Matteo Zandi 2007-01-02 20:45:30 UTC
> What's the output of "lshal | grep laptop_panel"?

$ lshal | grep laptop_panel
  info.capabilities = {'laptop_panel'} (string list)
  laptop_panel.num_levels = 11  (0xb)  (int)
  laptop_panel.access_method = 'omnibook'  (string)
  info.category = 'laptop_panel'  (string)

If 11 is the number of brigthness levels, then it is wrong, since they are 8 [0-7].

> What does (using SVN/CVS head) do?:

Using CVS head, actually I noticed that when the slider is 73%, then

$ cat /proc/omnibook/lcd 
LCD brightness:  7 (max value: 7)

and screen brightness is at max level

Still the brightness slider in the 'on battery power' tab doesn't reduce brightness but the one in the brightness applet does.

matteo@burnt:~/testroot2/src$ dbus-send --session --dest=org.gnome.PowerManager --type=method_call --print-reply /org/gnome/PowerManager/BrightnessLcd org.gnome.PowerManager.SetPolicy int32:0
method return sender=:1.6 -> dest=:1.30
process 32359: Applications must not close shared connections - see dbus_connection_close() docs. This is a bug in the application.
matteo@burnt:~/testroot2/src$ dbus-send --session --dest=org.gnome.PowerManager --type=method_call --print-reply /org/gnome/PowerManager/BrightnessLcd org.gnome.PowerManager.SetPolicy int32:50
method return sender=:1.6 -> dest=:1.31
process 32371: Applications must not close shared connections - see dbus_connection_close() docs. This is a bug in the application.
matteo@burnt:~/testroot2/src$ dbus-send --session --dest=org.gnome.PowerManager --type=method_call --print-reply /org/gnome/PowerManager/BrightnessLcd org.gnome.PowerManager.SetPolicy int32:100
method return sender=:1.6 -> dest=:1.32
process 32383: Applications must not close shared connections - see dbus_connection_close() docs. This is a bug in the application.
matteo@burnt:~/testroot2/src$ dbus-send --session --dest=org.gnome.PowerManager --type=method_call --print-reply /org/gnome/PowerManager/BrightnessLcd org.gnome.PowerManager.SetPolicy int32:75
method return sender=:1.6 -> dest=:1.33
process 32393: Applications must not close shared connections - see dbus_connection_close() docs. This is a bug in the application.

they work, but as noticed before, 75% is already 100% brightness
 
$ dbus-send --session --dest=org.gnome.PowerManager --type=method_call --print-reply /org/gnome/PowerManager/BrightnessLcd org.gnome.PowerManager.GetPolicy
method return sender=:1.6 -> dest=:1.36
   int32 70
process 32459: Applications must not close shared connections - see dbus_connection_close() docs. This is a bug in the application.

> Regarding the ati config tool, I think this either belongs in HAL or pm-utils
> (the latter being my best guess). As I don't have an ATI card, I've never used
> the application before, but I assume it's like the nvidia card utility that
> uses coolbits. This has been discussed on the HAL lists before, with no big
> argument either way. Is there a 100% fail-safe command that can be issued
> trivially to set the card into a low power state, and vice-versa to maximum or
> do we have to process the output of "aticonfig --lsp"?

when you install the fglrx proprietary driver, a script is placed in /etc/acpi which you can find attached, if enabled the powerstate is automatically changed (battery->1 while ac->3)

$ cat /etc/default/fglrx

# Uncomment the next line to enable powerstate switching on ACPI
# events for lid open/close and AC adapter on/off
FGLRX_ACPI_SWITCH_POWERSTATES=true

This behaviour is probably what 99% of the users would want, but still you have to know about the flag to enable it, not so user friendly.

Thanks,
Matteo
Comment 4 Richard Hughes 2007-01-02 21:35:07 UTC
I've send a mail to the relevant mailing lists, I've cc'd you. Regarding the omnibook problem, what distro are you running, and what hal versions have you tried. Thanks.
Comment 5 Matteo Zandi 2007-01-02 21:51:27 UTC
> I've send a mail to the relevant mailing lists, I've cc'd you. Regarding the
> omnibook problem, what distro are you running, and what hal versions have you
> tried. Thanks.

I run ubuntu/edgy, but I upgraded gnome-power-manager using feisty repos (before compiling cvs), thus apt-get upgraded also hal as a dependecy. Omnibook afaik is not in ubuntu's repos, I compiled cvs source.

$ dpkg -l | grep hal
ii  hal                              0.5.8.1-4ubuntu1                        Hardware Abstraction Layer
ii  hal-device-manager               0.5.7.1-0ubuntu17                       Hardware Abstraction Layer user interface
ii  libhal-dev                       0.5.8.1-4ubuntu1                        Hardware Abstraction Layer - development fil
ii  libhal-storage1                  0.5.7.1-0ubuntu17                       Hardware Abstraction Layer - shared library 
ii  libhal1                          0.5.8.1-4ubuntu1                        Hardware Abstraction Layer - shared library

Thank you very much,
Matteo
Comment 6 Richard Hughes 2007-01-07 22:41:23 UTC
I'm a bit confused how max_levels is 11.

I wrote the original code in HAL for omnibook support, and I have:

	} else if (acpi_type == ACPI_TYPE_OMNIBOOK_DISPLAY) {
		type = "omnibook";
		desc = "Omnibook LCD Panel";
		br_levels = 8;

So 8 levels should be the correctly set value (0-7). Maybe it's a distributor patch? What distro do you use?

Richard.
Comment 7 Matteo Zandi 2007-01-08 08:19:06 UTC
It is a distributor patch, see http://archive.ubuntu.com/ubuntu/pool/main/h/hal/hal_0.5.8.1-4ubuntu1.diff.gz, it has been patched due to bug #44962 reported in launchpad https://launchpad.net/ubuntu/+source/hal/+bug/44962

The user, wich reported the bug in launchpad, has different hardware than mine (same omnibook module) and different number of levels (10 instead of 8).

I think that hal should not have a fixed number of levels, but retrieve it from '$ cat /proc/omnibook/lcd' wich returns

LCD brightness:  7 (max value: 7)

I don't know if this is the right 'hook' or if it should be requested a better one to the omnibook module developer.

I hope this helped,
Matteo
Comment 8 Richard Hughes 2007-01-08 10:08:13 UTC
>distributor patch

Uhh, I hate it when ubuntu do this just to fix a bugzilla. It wasn't even mentioned on the mailing list.

You are correct, the best fix is to read it from the lcd file in acpi.c - but a better fix might be to convert the omnibook module to the new backlight class. Can you cc the maintainter please? Thanks.
Comment 9 Matteo Zandi 2007-01-08 17:46:44 UTC
> Uhh, I hate it when ubuntu do this just to fix a bugzilla. It wasn't even
> mentioned on the mailing list.

I recompiled hal from source without the patch 

debian/patches/28-fix-omnibook-brightness-levels.patch: Omnibooks have brightness levels, not 8.

then recompiled g-p-m, now the applet slider works perfectly, if I cat /proc/omnibook/lcd when the slider is at max level, I get 7 as lcd level.

matteo@burnt:~$ lshal | grep laptop_panel
  info.capabilities = {'laptop_panel'} (string list)
  laptop_panel.num_levels = 8  (0x8)  (int)
  laptop_panel.access_method = 'omnibook'  (string)
  info.category = 'laptop_panel'  (string)

> You are correct, the best fix is to read it from the lcd file in acpi.c - but a
> better fix might be to convert the omnibook module to the new backlight class.
> Can you cc the maintainter please? Thanks.

hum, I'll comment launchpad bug #44962 so that at least Ubuntu is aware of the problem the patch caused. I'll inform the omnibook module mantainer as well, but I guess he's rather busy, if you can guide me, I might try to do the backlight class thing.

Still, the 'preferences' brighter slider (not the applet one), doesn't reduce brightness when on battery. It only increases it (works one way only).

dbus-send --session --dest=org.gnome.PowerManager --type=method_call --print-reply /org/gnome/PowerManager/BrightnessLcd org.gnome.PowerManager.SetPolicy int32:0
method return sender=:1.6 -> dest=:1.14

works, but the slider gets confused somehow, it doesn't work properly when on battery.
Comment 10 Matteo Zandi 2007-01-08 17:55:30 UTC
steps to reproduce the bug:

the comp is running on ac, brightness is 100%, both brightness sliders (applet and preferences->ac are set at 100%). Brightness slider on battery is set at 0%.

Disconnect the power, comp runs on battery, lcd brightness is gently reduced to 0% by g-p-m. Now, brightness applet correctly reports 0%.

If you slide up the brightness with the applet, everything is fine, you can go up and down. Now the brightness is set again to 0% with the applet.

If I use the slider in preferences (of course, battery section), I can slide brightness up, but NOT down!!! A few ticks, set it to the maximum and then the slider is not able to put it down again.. the applet slider works tough.

I hope this claryfies the bug.
Comment 11 Matteo Zandi 2007-01-09 19:23:01 UTC
ok, if you enable it, omnibook supports the backlight class

matteo@burnt:~/download/omnibook/trunk2$ ls /sys/class/backlight/omnibook/ actual_brightness  brightness  max_brightness  power  subsystem  uevent

compiling hal 0.5.9 from source was too difficult, so I choosed to copy only the relevant scripts hal-system-lcd-set-brightness-linux and hal-system-lcd-get-brightness-linux

with a

HAL_PROP_LINUX_SYSFS_PATH="/sys/class/backlight/omnibook"

at the beginning and a

	echo $(date) ${value} >> /home/matteo/test

inside the first if clause, commenting

#       echo "$value" > $HAL_PROP_LINUX_ACPI_PATH

the result is:

matteo@burnt:~$ cat test
Tue Jan 9 19:59:55 CET 2007
Tue Jan 9 20:02:26 CET 2007 0
Tue Jan 9 20:02:26 CET 2007 1
Tue Jan 9 20:02:26 CET 2007 1
Tue Jan 9 20:02:26 CET 2007 2
Tue Jan 9 20:02:26 CET 2007 2
Tue Jan 9 20:02:26 CET 2007 3

IT WORKS!!!

so just to recap, the omnibook issue should be resolved, right? I won't mess with hal until release 0.5.9. If you agree I can ask Ubuntu to remove the patch we mentioned before (the one wich substitued br_levels from 8 with 10 in acpi.c) since everything will be fine if omnibook is compiled with backlight class enabled.

The bug described in Comment #10 still remains, let me know if you need further info. Thanks, Matteo
Comment 12 Richard Hughes 2007-01-21 20:36:05 UTC
(In reply to comment #10)
> If I use the slider in preferences (of course, battery section), I can slide
> brightness up, but NOT down!!! A few ticks, set it to the maximum and then the
> slider is not able to put it down again.. the applet slider works tough.

Can you grab the output of:

killall gnome-power-manager
gnome-power-preferences --verbose

and try to get the error when you set the brightness lower? Thanks.
Comment 13 Matteo Zandi 2007-01-29 16:33:33 UTC
When I set the brightness higher, the screen turns completely bright with a very little move of the slider. I had a look at the output of 

gnome-power-manager --no-daemon --verbose

but there is nothing interesting, just messages of brightness being changed. There is nothing wrong with the brightness being changed, the problem is how fast it gets changed in respect to the little move of the slider.

I tried to record the desktop so that you get the impression of it, had no luck yet, do you know a good desktop recorder? I'd like to upload a .avi somewhere
Comment 14 Matteo Zandi 2007-02-04 11:05:34 UTC
I recored it, you can see it here http://kate.homeunix.net/~matteo/gpm-0000.mpeg, I hope this makes clarifies what I mean. I also recompiled current cvs head (2.17.9)  but the problem persists.

Thanks,
Matteo
Comment 15 Richard Hughes 2007-02-04 12:36:27 UTC
What does "lshal | grep laptop_panel" print? Thanks.
Comment 16 Matteo Zandi 2007-02-04 14:53:52 UTC
here it is

$ lshal | grep laptop_panel
  info.capabilities = {'laptop_panel'} (string list)
  laptop_panel.num_levels = 8  (0x8)  (int)
  laptop_panel.access_method = 'omnibook'  (string)
  info.category = 'laptop_panel'  (string)

did you check the video?
Matteo
Comment 17 Richard Hughes 2007-02-04 15:04:24 UTC
Yes, video was helpfull, thanks. I still can't work out why g-p-m is getting the divisor wrong. Let me play with my system to see if I can reproduce.
Comment 18 Richard Hughes 2007-02-05 00:36:20 UTC
Can you try with svn pls:

2007-02-05  Richard Hughes  <richard@hughsie.com>

	* src/gpm-brightness-lcd.c:
	Use the old (2.16) GetBrightness and SetBrightness methods because
	these work. I think the DELL smbios addon is returning an integer rather
	than an unsigned integer and this is the source of confusion.

	* src/gpm-common.c:
	Check that the levels are not zero, else we get non-sensical results.

	* src/gpm-srv-backlight.c:
	Set the dim value after the first GetBrightness to avoid setting the
	dim value as the current brightness.
Comment 19 Matteo Zandi 2007-02-09 21:15:44 UTC
yes yes yes!! bug solved!!
Comment 20 Richard Hughes 2007-02-09 23:28:50 UTC
Okay, many thanks for your testing.