Brightness control is slow and not costant with 90-Add-guarded-brightness-stepping-functions.patch 91-Using-guarded-and-scaled-stepping-when-dimming.patch

Bug #345318 reported by Andrea Cimitan
14
Affects Status Importance Assigned to Milestone
gnome-power-manager (Ubuntu)
Fix Released
Low
Unassigned

Bug Description

Binary package hint: gnome-power-manager

90-Add-guarded-brightness-stepping-functions.patch and 91-Using-guarded-and-scaled-stepping-when-dimming.patch are introducing this bug.
When changing the brightness, the control is not costant and it is really slow.
I explain better what I mean.
When I am at 30% and I want to reduce till 20% I have to press 4 times, when I am at 20% and I want to reduce till 10% i have to press 5 times.
That means ~15 key press to reduce the backlight from 50 till 10! Totally unusable. Something like:

60->50: 1 time
50->40: 2 times
40->30: 3 times
30->20: 4 times
20->10: 5 times
10->00: 6 times

...just to give you an idea. This is not the correct behaviour, with default gnome-power-manager the control is ~ 10% step for each key event, in ubuntu the control is choppy for this reason.

Removing
"90-Add-guarded-brightness-stepping-functions.patch"
"91-Using-guarded-and-scaled-stepping-when-dimming.patch"
FIXES the bug.
I am wondering, why we have those two patches? :)

Revision history for this message
Martin Pitt (pitti) wrote :

Ted, do you still remember why those were applied?

gnome-power-manager (2.24.0-1mactel3) unstable; urgency=low

  * Nonlinear stepping for smooth dimming
  * Patch trimming for upstream

 -- Henrik Rydberg <email address hidden> Tue, 28 Oct 2008 02:55:08 +0100

There are no bug references, no tags in the patch, apparently not forwarded upstream, and they are heavily underdocumented. I already threw them out of the 2.25 packages, since they don't apply any more. I contacted Henrik Rydberg via mail back then, but didn't get any answer yet. Do you think it's safe to drop them?

Revision history for this message
Ted Gould (ted) wrote : Re: [Bug 345318] Re: Brightness control is slow and not costant with 90-Add-guarded-brightness-stepping-functions.patch 91-Using-guarded-and-scaled-stepping-when-dimming.patch

On Thu, 2009-03-19 at 16:23 +0000, Martin Pitt wrote:
> Ted, do you still remember why those were applied?

Yes, they were to deal with laptops that have a ton of levels, so you
want to smoothly go through the levels. So if you have a 1000 levels
switching from 10% to 20% shouldn't be a large jump, it should be a
gradient.

I believe that the patches were submitted upstream, though not applied
(we have a few of those). They were rewritten a couple of times to make
them more palatable upstream.

FWIW, the patch works well here for me on my Macbook Pro.

Revision history for this message
Andrea Cimitan (cimi) wrote : Re: [Bug 345318] Re: Brightness control is slow and not costant with 90-Add-guarded-brightness-stepping-functions.patch 91-Using-guarded-and-scaled-stepping-when-dimming.patch

it could work well on your macbook but it makes the brightness control
not usable here :)

What will happen in yout macbook without the patch?

Revision history for this message
Martin Pitt (pitti) wrote :

Hm, I tried dropping the patch on a Samsung NC10 and a Dell Latitude D430. On both, brightness changes have far too many steps which renders the brightness keys hard to use.

However, I do not see a difference with or without the patch.

Revision history for this message
Bart Rose (jbrose3) wrote :

Also an issue on my AAO. I haven't tried removing above mentioned patches. If someone posts them or a modified deb, I could have a go at it. Thanks.

Revision history for this message
Bart Rose (jbrose3) wrote :

Well I tried installing some older versions (gnome-power-manager_2.24.2-2ubuntu4 and gnome-power-manager_2.24.2-2ubuntu5) that should have been released prior to the patches in question. This didn't seem to change the brightness indicator's step number. I'm not sure if this was a valid test. If not, let me know.

Revision history for this message
Andrea Cimitan (cimi) wrote :

Did you restarted gnome-power-manager?

Revision history for this message
Bart Rose (jbrose3) wrote :

I think so. I ran "killall gnome-power-manager && gnome-power-manager"...
That should have done it right? With v4 it switched back to old style meter
and v5 had new style.

On Fri, Apr 3, 2009 at 10:43 PM, Andrea Cimitan <email address hidden>wrote:

> Did you restarted gnome-power-manager?
>
> --
> Brightness control is slow and not costant with
> 90-Add-guarded-brightness-stepping-functions.patch
> 91-Using-guarded-and-scaled-stepping-when-dimming.patch
> https://bugs.launchpad.net/bugs/345318
> You received this bug notification because you are a direct subscriber
> of the bug.
>
> Status in “gnome-power-manager” source package in Ubuntu: New
>
> Bug description:
> Binary package hint: gnome-power-manager
>
> 90-Add-guarded-brightness-stepping-functions.patch and
> 91-Using-guarded-and-scaled-stepping-when-dimming.patch are introducing this
> bug.
> When changing the brightness, the control is not costant and it is really
> slow.
> I explain better what I mean.
> When I am at 30% and I want to reduce till 20% I have to press 4 times,
> when I am at 20% and I want to reduce till 10% i have to press 5 times.
> That means ~15 key press to reduce the backlight from 50 till 10! Totally
> unusable. Something like:
>
> 60->50: 1 time
> 50->40: 2 times
> 40->30: 3 times
> 30->20: 4 times
> 20->10: 5 times
> 10->00: 6 times
>
> ...just to give you an idea. This is not the correct behaviour, with
> default gnome-power-manager the control is ~ 10% step for each key event, in
> ubuntu the control is choppy for this reason.
>
> Removing
> "90-Add-guarded-brightness-stepping-functions.patch"
> "91-Using-guarded-and-scaled-stepping-when-dimming.patch"
> FIXES the bug.
> I am wondering, why we have those two patches? :)
>

Revision history for this message
Bart Rose (jbrose3) wrote :

Here is the output from lshal for my backlight:

udi = '/org/freedesktop/Hal/devices/computer_backlight'
  info.addons = {'hald-addon-generic-backlight'} (string list)
  info.capabilities = {'laptop_panel'} (string list)
  info.category = 'laptop_panel' (string)
  info.interfaces = {'org.freedesktop.Hal.Device.LaptopPanel'} (string list)
  info.parent = '/org/freedesktop/Hal/devices/computer' (string)
  info.product = 'Generic Backlight Device' (string)
  info.subsystem = 'backlight' (string)
  info.udi = '/org/freedesktop/Hal/devices/computer_backlight' (string)
  laptop_panel.access_method = 'general' (string)
  laptop_panel.num_levels = 10 (0xa) (int)
  linux.hotplug_type = 2 (0x2) (int)
  linux.subsystem = 'backlight' (string)
  linux.sysfs_path = '/sys/devices/platform/acer-wmi/backlight/acer-wmi' (string)

The num_level is correctly detected as 10 steps. I'm assuming this is how gpm gets its number of steps, before adjusting for macbooks via the new patch that is.

Revision history for this message
Bart Rose (jbrose3) wrote :

As far as I can tell, this is the last Jaunty bug listed on the AspireOne Community page that doesn't have a workaround. Even though this bug doesn't limit the ability to change brightness, it is highly visible. As Ubuntu is trying to make headway into the netbook market, and the AAO is a very popular model, I think it would be nice to fix this last little nuisance before the 23rd. I am happy to test any code, but unfortunately my coding skills are too rusty to tackle this on my own. Thank you to everyone who has helped so far.

Revision history for this message
Fortunato Ventre (voria) wrote :

I have modified the '90-Add-guarded-brightness-stepping-functions' patch in order to get 12 levels of brightness adjustment, and changed the formulas used to calculate the next step for decreasing/increasing brightness in order to get constant steps.
It seems to work good on Samsung NC10.
Please try it.
Or else you can get a precompiled package (i386, lpia) from my PPA:
https://launchpad.net/~voria/+archive/ppa

Revision history for this message
Bart Rose (jbrose3) wrote :

It definitely decreases the number of steps, but still has 2 too many for my AAO. I think setting GPM_BRIGHTNESS_DIM_LEVELS to 12 by default is the problem. Is it possible to set it to the value of "laptop_panel.num_levels" as reported by hal? I attached my lshal output above and it seems to report the correct number of brightness levels. Is this true for everyone else? Thanks for tackling this!

Revision history for this message
Fortunato Ventre (voria) wrote :

Looking at the code in 'gpm-brightness.c', it seems that xrandr is preferred over hal when both are available. I don't know why but there is certainly a good reason if it's so.

Revision history for this message
Bart Rose (jbrose3) wrote :

Is GPM_BRIGHTNESS_DIM_LEVELS set using xrandr? The patch seems to set it to 12 by default.

+#define GPM_BRIGHTNESS_DIM_LEVELS 12

Revision history for this message
Fortunato Ventre (voria) wrote :

No, it's manually set. It was set to 24 in the original patch, I changed it to 12 to get bigger steps.
IMHO this is not a good solution, because a fixed value is not good for everyone, as you stated.
However, I just modified the original patch, I don't know if there is a more generic way to set a value.

Revision history for this message
Bart Rose (jbrose3) wrote :

What about setting it to the value reported by laptop_panel.num_levels from hal. I borrowed the code below from https://fedorahosted.org/hal-cups-utils/browser/systemv/hal_lpadmin?rev=63db99983c74ba8c7efaee60e9b509e986a9204f for a possible way to incorporate the hal-get-property command. It should be possible to then set the output to GPM_BRIGHTNESS_DIM_LEVELS. I hope this makes sense, like I said before my coding skills are about 15 years rusty.

def get_hal_property(udi, key):
34 # Read out the value of a given key for a given HAL UDI
35 devnull = file ("/dev/null", "r+")
36 try:
37 p = subprocess.Popen (["hal-get-property", "--udi", udi,
38 "--key", key],
39 stdin=devnull,
40 stdout=subprocess.PIPE,
41 stderr=devnull)
42 (stdout, stderr) = p.communicate ()
43 return stdout.strip ()
44 except:
45 return ""

Revision history for this message
Fortunato Ventre (voria) wrote :

It seems that hal support is not always available. For example, 'lshal' does not report any UDI for backlight on Samsung NC10. Apart from that, it would be a bad idea to use hal in gpm xrandr functions.
Looking at the code, there are two distinct way to set brightness, xrandr and hal.

Revision history for this message
Fortunato Ventre (voria) wrote :

It seems that hal support is not always available. For example 'lshal' does not report any UDI for backlight on Samsung NC10. Apart from that, it's a bad idea to use hal in xrandr functions.
As I said in a previous comment xrandr is preferred over hal when both are available, and hal support gets dropped.

Revision history for this message
Bart Rose (jbrose3) wrote :

I guess that won't change until hardware developers start building with linux in mind... A less eloquent solution in the mean time might be to create a gpmbightness.conf file that users can manually set GPM_BRIGHTNESS_DIM_LEVELS to suit their needs. This could be parsed first and if null a default value of 12 could be set.

Changed in gnome-power-manager (Ubuntu):
importance: Undecided → Low
Revision history for this message
Scott Howard (showard314) wrote :

Both of these patches have been dropped as of 2.26.1, so I'm marking as closed. Thanks for the work!

Changed in gnome-power-manager (Ubuntu):
status: New → Fix Released
Revision history for this message
Andrea Cimitan (cimi) wrote : Dear friend!

Hi,
I bought a TV last week from a website:www.hkeles.com. I have
received the product. The quality is very good. They also sell
phones,motor,psp and so on. Because of the financial crisis, their
products are very cheap. by the way, they only sell new and original
products .If you need these products, you can have a look . I think
you will get many benefits.
Greetings!

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.