Bug 6729

Summary: Sony brightness setting not working
Product: hal Reporter: Bastien Nocera <bugzilla>
Component: miscAssignee: David Zeuthen (not reading bugmail) <zeuthen>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: high CC: pborelli
Version: unspecified   
Hardware: x86 (IA32)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: hal-spicctrl-support.patch
hal-spicctrl-support-2.patch
hal-spicctrl-support-3.patch
hal-spicctrl-support-dont-handle-all-vaios.patch
hal-spicctrl-support-dont-handle-all-vaios-2.patch

Description Bastien Nocera 2006-04-25 02:15:59 UTC
The code to set the brightness on Sony laptops uses the Sony ACPI module, which:
- isn't upstream yet (so not in Fedora)
- won't work on older Sony laptops

---8<--
elif [ "$HAL_PROP_LAPTOP_PANEL_ACCESS_METHOD" == "sony" ]; then
        # cat /proc/acpi/sony/brightness
        #  5
        value="`cat $HAL_PROP_LINUX_ACPI_PATH`"
        let "value = ${value} - 1"
---8<--

It should be possible to use sonypi instead, but this would require either:
- spicctl to be present
- the hal-system-lcd-get-brightness to be rewritten in C

I'll try with spicctl for now
Comment 1 Bastien Nocera 2006-04-25 06:05:42 UTC
Created attachment 5462 [details] [review]
hal-spicctrl-support.patch

2 changes:
- if there's no ACPI path, and the LCD type is Sony, and spicctrl is available,
use spicctrl to set/get the brightness
The range is 0..255, not 0..7, so we need to convert the values

- in the set brightness, bail out when the value is *equal* to
HAL_PROP_LAPTOP_PANEL_NUM_LEVELS as well. For example, 8 values means 0 to 7,
not 0 to 8 (which would be 9 levels)

I tested this by hand, on a current FC5, and the sonypi driver is automatically
loaded.
Comment 2 Bastien Nocera 2006-04-25 08:37:53 UTC
I wrongly assumed that the sonypi module would get loaded automatically, it
doesn't. Should I go one step further and make it load the sonypi driver (urgh)?
Comment 3 Richard Hughes 2006-04-25 08:56:11 UTC
No, this should not be modprobed by hal IMO. Does the sonipi diver work if
modprobed manually?
Comment 4 Bastien Nocera 2006-04-25 17:09:12 UTC
Yeah, works fine once modprobed manually. Are the acpi modules auto-loaded on
startup?
Comment 5 Paolo Borelli 2006-04-25 17:42:26 UTC
But this would break laptops which use the sony-acpi driver, like mine.
Comment 6 Bastien Nocera 2006-04-25 18:05:36 UTC
Why would break exactly? We would only load it when $HAL_PROP_LINUX_ACPI_PATH
isn't available.
Comment 7 Paolo Borelli 2006-04-25 18:17:40 UTC
Because I went to sleep 5 hours ago and woke up 5 minutes ago and didn't get
coffee yet.
Sorry, "It should be possible to use sonypi instead" and misreading your patch
made me think you were replacing the previous script with spicctrl.
Comment 8 Bastien Nocera 2006-04-27 02:07:31 UTC
For the module loading, this is the RH bug so that udev gets to load it:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190004
Comment 9 Bastien Nocera 2006-04-27 04:11:10 UTC
The changes I posted aren't enough. The panel isn't available as a an ACPI
object, so it's not getting added to the list, and we don't even try to get/set
the brightness.
Comment 10 Bastien Nocera 2006-04-27 06:27:21 UTC
Created attachment 5487 [details] [review]
hal-spicctrl-support-2.patch

Updated (and working) patch.
This one also creates an LCD Panel object, necessary for getting the methods
supported, also corrects the bug mentioned above in the set script.
Comment 11 Bastien Nocera 2006-04-27 07:53:23 UTC
Created attachment 5490 [details] [review]
hal-spicctrl-support-3.patch

Updated patch against CVS HEAD.
Comment 12 Richard Hughes 2006-04-27 08:27:44 UTC
2006-04-26  Richard Hughes  <richard@hughsie.com>

	Patch from Bastien Nocera <hadess@hadess.net>:

	* hald/linux2/acpi.c: (laptop_panel_refresh),
	(acpi_synthesize_sonypi_display), (acpi_synthesize_hotplug_events):
	Create an LCD Panel object of type sonypi, necessary for getting the
	methods supported for panasonic notebooks.
	Fixes https://bugs.freedesktop.org/show_bug.cgi?id=6729

	* tools/hal-system-lcd-get-brightness:
	* tools/hal-system-lcd-set-brightness:
	Support the sonypi brightness type.
Comment 13 Bastien Nocera 2006-04-28 06:48:03 UTC
The detection code is broken for systems where sonypi loads, but offers no
brightness support (ie. Type 3 VAIOs).
Comment 14 Bastien Nocera 2006-04-28 07:20:11 UTC
Created attachment 5507 [details] [review]
hal-spicctrl-support-dont-handle-all-vaios.patch

Don't handle through sonypi if we have acpi brightness support
Comment 15 Bastien Nocera 2006-04-28 07:27:11 UTC
Created attachment 5508 [details] [review]
hal-spicctrl-support-dont-handle-all-vaios-2.patch

And against CVS...
Comment 16 Bastien Nocera 2006-04-28 17:08:15 UTC
2006-04-27  Richard Hughes  <richard@hughsie.com>

        Patch from Bastien Nocera <hadess@hadess.net>:

        * hald/linux2/acpi.c: (acpi_synthesize_sonypi_display):
        Check that we don't support brightness change through ACPI
        with the presence of /proc/acpi/sony/brightness for type3 VAIOs.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.