[ThinkPads] No brightness-notification - Bug in generic get_brightness function

Bug #372874 reported by Johannes Hessellund on 2009-05-06
32
This bug affects 2 people
Affects Status Importance Assigned to Milestone
HAL
Fix Released
Medium
hal (Ubuntu)
Low
Steve Langasek
Karmic
Low
Steve Langasek

Bug Description

Binary package hint: hal

Brightness notification always shows the same value for laptops with hardwired brightness control, ex T42. Thus not reflecting the true brightness value.

Problem is get_backlight () function in: hald/linux/addons/addon-generic-backlight.c

Line 67 reads: g_snprintf (sysfs_path, sizeof (sysfs_path), "%s/brightness", path);

should be: g_snprintf (sysfs_path, sizeof (sysfs_path), "%s/actual_brightness", path);

'actual_brightness' holds the current value of brightness.
'brightness' is used to set the brightness, and when read gives the last writen value!

Johannes Hessellund (osos) wrote :

Fixing this would partly fix bug #357673.

thinkpad hotkeys for brightness should be enabled by default after this fix.

In addon-generic-backlight.c line 67
http://cgit.freedesktop.org/hal/tree/hald/linux/addons/addon-generic-backlight.c

get_backlight function read from the wrong sysfs file.
The real backlight value is available from 'actual_brightness', whereas 'brightness' is used for changing/setting the backlight through software.
'brightness' always reads last value writen to it.
Backlight which is hardware controlled, as with many thinkpads (ex T42), does not update 'brightness'. Thus hald is not returning the right value!

line 67 should be:
   g_snprintf (sysfs_path, sizeof (sysfs_path), "%s/actual_brightness", path);

On my T42 the sysfs path is:
   /sys/class/backlight/thinkpad_screen/actual_brightness

Created an attachment (id=25584)
addon-generic-backlight.c.diff

This patch fixes this issue. Please test and commit!

Johannes Hessellund (osos) wrote :

Tested attached patch, which fixes this issue.

Brightness notifications works again.

Please include patch in jaunty hal!

Changed in hal:
status: Unknown → Confirmed

Additional info:

I'm using a Thinkpad T42.

This patch solves the issue for thinkpads using thinkpad_acpi.

On Ubuntu 9.04 this causes an issue with wrong notifications when changing brightness. Gnome-power-manager reads the brightness value through hal, thus always showing the last value writen to 'brightness', even though the brightness lvel actually might be higher or lower, if controlled thru hardwired brightness control.

Please apply this simple patch.

Thanks, since there is a patch (and upstream report) I'll mark as triaged!

Changed in hal (Ubuntu):
importance: Undecided → Low
status: New → Triaged
Johannes Hessellund (osos) wrote :

Ubuntu should not wait for upstream. Try out the patch.

It solves the brightness-notification issue.

1. Apply attached patch to hald
2. echo "options thinkpad_acpi hotkey=enable,0xfdffff" > /etc/modprobe.d/thinkpad_acpi.conf
3. Restart hal, reload thinkpad_acpi module

=> brightness notification works on Thinkpads, thus partly solving bug #357673.

description: updated
summary: - Generic get_brightness function should read 'actual_brightness' not
- 'brightness'
+ [ThinkPads] No brightness-notification - Bug in generic get_brightness
+ function
Steve Langasek (vorlon) on 2009-06-09
Changed in hal (Ubuntu):
assignee: nobody → Canonical Foundations Team (canonical-foundations)
Johannes Hessellund (osos) wrote :

Will this ever get any love?

I have seen hal updated several times. Why not include this patch?

Simply add the patch file to the debian/patches directory inside the hal package!

What is missing?

Changed in hal:
status: Confirmed → Fix Released
Johannes Hessellund (osos) wrote :

This bug is resolved upstream now.

http://cgit.freedesktop.org/~dkukawka/hal/commit/?id=d792a792846f9632edfdea3651a74fcd24b2ead7

Please update Jaunty version of HAL with this patch.
Brightness notifications on Thinkpads will to solved.

To enable brightness buttons they should be enabled in the thinkpad_acpi module:

options thinkpad_acpi hotkey=enable,0xfdffff

Robbie Williamson (robbiew) wrote :

Is this working in Karmic Alpha 4? I'm not sure if this fix would qualify for a Jaunty SRU (https://wiki.ubuntu.com/StableReleaseUpdates).

Kai Jauch (kaijauch) wrote :

hal 0.5.13-1ubuntu4 in karmic still uses backlight instead of actual_backlight. This causes gnome-power-manager to dim the display of my notebook (Dell Latitude E6400) on first startup and then increase it again to maximum because it thinks the brightness is zero (e.g. brightness = 0, actual_brightness = 15).

I've patched this version of hal to use actual_brightness instead of brightness in hald-addon-generic-backlight and can confirm that it fixes this problem.

Steve Langasek (vorlon) on 2009-09-23
Changed in hal (Ubuntu Karmic):
assignee: Canonical Foundations Team (canonical-foundations) → Steve Langasek (vorlon)
Steve Langasek (vorlon) on 2009-09-23
Changed in hal (Ubuntu Karmic):
status: Triaged → Fix Committed
Steve Langasek (vorlon) wrote :

Fixed in the latest upload. Changelog:

hal (0.5.13-1ubuntu6) karmic; urgency=low

  * 60_upstream_backlight_actual_brightness.patch: pull fix from upstream to
    read the current brightness value from the correct file under /sys.
    LP: #372874.

Changed in hal (Ubuntu Karmic):
status: Fix Committed → Fix Released
Changed in hal:
importance: Unknown → Medium
Changed in hal:
importance: Medium → Unknown
Changed in hal:
importance: Unknown → Medium
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

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