Ubuntu
hal package

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

Bug #372874 reported by Johannes Hessellund on 2009-05-06

This bug affects 2 people

	Status	Importance	Assigned to
HAL	Fix Released	Medium	freedesktop-bugs #21612
hal (Ubuntu)	Fix Released	Low	Steve Langasek
Karmic	Fix Released	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!

See original description

Related branches

lp:~ubuntu-core-dev/hal/ubuntu

lp:ubuntu/lucid/hal

Revision history for this message

Johannes Hessellund (osos) wrote on 2009-05-06:

Fixing this would partly fix bug #357673.

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

Revision history for this message

In freedesktop.org Bugzilla #21612, Johannes Hessellund (osos) wrote on 2009-05-06:

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

Revision history for this message

In freedesktop.org Bugzilla #21612, Johannes Hessellund (osos) wrote on 2009-05-07:

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

This patch fixes this issue. Please test and commit!

Revision history for this message

Johannes Hessellund (osos) wrote on 2009-05-07: Re: Generic get_brightness function should read 'actual_brightness' not 'brightness'

Filed bug upstream:
http://bugs.freedesktop.org/show_bug.cgi?id=21612

Revision history for this message

Johannes Hessellund (osos) wrote on 2009-05-07:

addon-generic-backlight.c.diff Edit (410 bytes, text/plain)

Tested attached patch, which fixes this issue.

Brightness notifications works again.

Please include patch in jaunty hal!

Bug Watch Updater (bug-watch-updater) on 2009-05-09

Changed in hal:
status:	Unknown → Confirmed

Revision history for this message

In freedesktop.org Bugzilla #21612, Johannes Hessellund (osos) wrote on 2009-05-25:

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.

Revision history for this message

Michael Rooney (mrooney) wrote on 2009-05-31: Re: Generic get_brightness function should read 'actual_brightness' not 'brightness'

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

Changed in hal (Ubuntu):
importance:	Undecided → Low
status:	New → Triaged

Revision history for this message

Johannes Hessellund (osos) wrote on 2009-06-09:

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)

Revision history for this message

Johannes Hessellund (osos) wrote on 2009-07-07:

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?

Revision history for this message

In freedesktop.org Bugzilla #21612, Danny Kukawka (danny-kukawka) wrote on 2009-07-24:

#10

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

get merged before the next release

Bug Watch Updater (bug-watch-updater) on 2009-07-25

Changed in hal:
status:	Confirmed → Fix Released

Revision history for this message

Johannes Hessellund (osos) wrote on 2009-07-26:

#11

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

Revision history for this message

Robbie Williamson (robbiew) wrote on 2009-08-25:

#12

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).

Revision history for this message

Kai Jauch (kaijauch) wrote on 2009-09-23:

#13

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

Revision history for this message

Steve Langasek (vorlon) wrote on 2009-09-24:

#14

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.