Implementing a platform driver w/ LEDs

PATCH Mario Limonciello 2 participants 4 messages Feb 5, 2014 – Feb 7, 2014

Subsystems (2)

x86 platform drivers LED

lore

Share & tools

lore

Mario Limonciello

author

2014-02-05 18:52 UTC #0 lore raw

Hi,

I recently submitted a patch to add a platform driver for Alienware that one of the functions will be to control some lighting zones on the machine. I originally started implementing it using the LED class but started to find it didn't really fit the LED class well so decided to do a custom sysfs interface.

After submitting it, the platform driver maintainer (mjg59) recommended that I instead reach out to you to extend the LED class to better fit how the system is architected.

The system has 3 lighting zones (HEAD, Left, Right). Each of those lighting zones can independently customize the shades of red, green, blue, as well as the brightness intensity for the zone.

So each zone has:
0x00 - 0xFF red shade
0x00 - 0xFF blue shade
0x00 - 0xFF green shade
0x00 - 0xFF brightness intensity

Additionally the zone can be configured for different lighting combinations for three different states: Running, Booting, Suspended. Running immediately modifies the colors in the zone. Booting modifies the colors when turned on but before the next modification of "running". Suspended modifies the colors while the system is in S3.

When I first started to implement this using the LED class I create it with 36 different LED nodes:

alienware-wmi::running::head_red
alienware-wmi::running::head_blue
alienware-wmi::running::head_green
alienware-wmi::running::head_brightness
alienware-wmi::running::left_red
alienware-wmi::running::left_blue
alienware-wmi::running::left_green
alienware-wmi::running::left_brightness
alienware-wmi::running::right_red
alienware-wmi::running::right_blue
alienware-wmi::running::right_green
alienware-wmi::running::right_brightness

alienware-wmi::booting::head_red
alienware-wmi::booting::head_blue
alienware-wmi::booting::head_green
alienware-wmi::booting::head_brightness
alienware-wmi::booting::left_red
alienware-wmi::booting::left_blue
alienware-wmi::booting::left_green
alienware-wmi::booting::left_brightness
alienware-wmi::booting::right_red
alienware-wmi::booting::right_blue
alienware-wmi::booting::right_green
alienware-wmi::booting::right_brightness

alienware-wmi::suspend::head_red
alienware-wmi::suspend::head_blue
alienware-wmi::suspend::head_green
alienware-wmi::suspend::head_brightness
alienware-wmi::suspend::left_red
alienware-wmi::suspend::left_blue
alienware-wmi::suspend::left_green
alienware-wmi::suspend::left_brightness
alienware-wmi::suspend::right_red
alienware-wmi::suspend::right_blue
alienware-wmi::suspend::right_green
alienware-wmi::suspend::right_brightness

I thought this was rather confusing though because each individual node only has a single "brightness" member which doesn't really identify what is being changed. The brightness node changes the overall brightness of the whole zone. The color shades selected for each node are mixed to come up with the overall color for the zone.

The three different modes (running/booting/suspend) all modify the same LEDs too, so it didn't seem to make sense to me that they had their own nodes.

So given all of that, can you advise how you think this should be implemented? Would it make sense to extend the LED class to better adapt to this? Or do you think this is better suited for a custom sysfs interface as I've already done? I'll attach the patch for the driver so you can see how I put it together with the custom sysfs interface.

Thanks,

Quoted 1 line

From e8e4cadc5e467835f3aabfa603eb44e757bdf9a8 Mon Sep 17 00:00:00 2001

From: Mario Limonciello <[email protected]>
Date: Wed, 5 Feb 2014 10:08:13 -0600
Subject: [PATCH 1/1] Add WMI driver for controlling AlienFX on Alienware

Specifically for platforms that don't contain an AlienFX USB based MCU such as the Alienware X51-R2.

Signed-off-by Mario Limonciello <[email protected]>

---
 drivers/platform/x86/Kconfig         |   13 ++
 drivers/platform/x86/Makefile        |    1 +
 drivers/platform/x86/alienware-wmi.c |  345 ++++++++++++++++++++++++++++++++++
 3 files changed, 359 insertions(+)
 create mode 100644 drivers/platform/x86/alienware-wmi.c

drivers/platform/x86/Kconfig +13 -0

index 5ae65c1..d030525 100644
@@ -55,6 +55,19 @@ config ACERHDF
 	  If you have an Acer Aspire One netbook, say Y or M
 	  here.
 
+config ALIENWARE_WMI
+	tristate "Alienware Special feature control"
+	depends on ACPI
+	depends on ACPI_WMI
+	---help---
+	  This is a driver for controlling ALienware BIOS driven
+	  features.  It exposes an interface for controlling the AlienFX
+	  zones on Alienware machines that don't contain a dedicated AlienFX
+	  USB MCU such as the X51-R2.
+
+	  If you have an ACPI-WMI compatible Alienware desktop, say Y or M
+	  here.
+
 config ASUS_LAPTOP
 	tristate "Asus Laptop Extras"
 	depends on ACPI

drivers/platform/x86/Makefile +1 -0

index 9b87cfc..0e9b157 100644
@@ -56,3 +56,4 @@ obj-$(CONFIG_INTEL_SMARTCONNECT)	+= intel-smartconnect.o
 
 obj-$(CONFIG_PVPANIC)           += pvpanic.o
 obj-$(CONFIG_INTEL_BAYTRAIL_MBI)	+= intel_baytrail.o
+obj-$(CONFIG_ALIENWARE_WMI) 	+= alienware-wmi.o

drivers/platform/x86/alienware-wmi.c new file +345 -0

345 lines not on this page · raw message

Signature · 1 line

1.7.9.5

Bryan Wu

2014-02-05 19:25 UTC #1 lore raw

Quoted 22 lines · Mario Limonciello

Hi,
I recently submitted a patch to add a platform driver for Alienware that one of the functions will be to control some lighting zones on the machine. I originally started implementing it using the LED class but started to find it didn't really fit the LED class well so decided to do a custom sysfs interface.
After submitting it, the platform driver maintainer (mjg59) recommended that I instead reach out to you to extend the LED class to better fit how the system is architected.
The system has 3 lighting zones (HEAD, Left, Right). Each of those lighting zones can independently customize the shades of red, green, blue, as well as the brightness intensity for the zone.
So each zone has:
0x00 - 0xFF red shade
0x00 - 0xFF blue shade
0x00 - 0xFF green shade
0x00 - 0xFF brightness intensity

Quoted 6 lines

Additionally the zone can be configured for different lighting combinations for three different states: Running, Booting, Suspended. Running immediately modifies the colors in the zone. Booting modifies the colors when turned on but before the next modification of "running". Suspended modifies the colors while the system is in S3.

I think for this use case, we need 2 drivers:

1. LED device driver for each lighting zone, it will supports red, blue, green and brightness operation.

2. LED trigger driver provides events trigger when system running, booting and suspended. It can send out event to your Alienware LED device driver to do proper LED operations.

So the basic concept is splitting out real LED operations and event triggers.

Quoted 48 lines

When I first started to implement this using the LED class I create it with 36 different LED nodes:
alienware-wmi::running::head_red
alienware-wmi::running::head_blue
alienware-wmi::running::head_green
alienware-wmi::running::head_brightness
alienware-wmi::running::left_red
alienware-wmi::running::left_blue
alienware-wmi::running::left_green
alienware-wmi::running::left_brightness
alienware-wmi::running::right_red
alienware-wmi::running::right_blue
alienware-wmi::running::right_green
alienware-wmi::running::right_brightness
alienware-wmi::booting::head_red
alienware-wmi::booting::head_blue
alienware-wmi::booting::head_green
alienware-wmi::booting::head_brightness
alienware-wmi::booting::left_red
alienware-wmi::booting::left_blue
alienware-wmi::booting::left_green
alienware-wmi::booting::left_brightness
alienware-wmi::booting::right_red
alienware-wmi::booting::right_blue
alienware-wmi::booting::right_green
alienware-wmi::booting::right_brightness
alienware-wmi::suspend::head_red
alienware-wmi::suspend::head_blue
alienware-wmi::suspend::head_green
alienware-wmi::suspend::head_brightness
alienware-wmi::suspend::left_red
alienware-wmi::suspend::left_blue
alienware-wmi::suspend::left_green
alienware-wmi::suspend::left_brightness
alienware-wmi::suspend::right_red
alienware-wmi::suspend::right_blue
alienware-wmi::suspend::right_green
alienware-wmi::suspend::right_brightness
I thought this was rather confusing though because each individual node only has a single "brightness" member which doesn't really identify what is being changed. The brightness node changes the overall brightness of the whole zone. The color shades selected for each node are mixed to come up with the overall color for the zone.

Quoted 9 lines

The three different modes (running/booting/suspend) all modify the same LEDs too, so it didn't seem to make sense to me that they had their own nodes.
So given all of that, can you advise how you think this should be implemented? Would it make sense to extend the LED class to better adapt to this? Or do you think this is better suited for a custom sysfs interface as I've already done? I'll attach the patch for the driver so you can see how I put it together with the custom sysfs interface.

I just went through the patch you attached quickly, but failed to find anything related to system booting/running/suspended. So you plan to do that by using user space script to talk with those sysfs interface?

Thanks,
-Bryan

Mario Limonciello

author

2014-02-05 20:10 UTC #2 lore raw

Quoted 11 lines · Bryan Wu

I think for this use case, we need 2 drivers:
1. LED device driver for each lighting zone, it will supports red, blue, green and brightness operation.
2. LED trigger driver provides events trigger when system running, booting and suspended. It can send out event to your Alienware LED device driver to do proper LED operations.
So the basic concept is splitting out real LED operations and event triggers.

Sorry, I should have clarified - the LEDs operations themselves are BIOS controlled. I don't think there should be any trigger operation necessary. It's a different WMI BIOS call to configure the LEDs in each different state.

Quoted 57 lines

Quoted 48 lines
When I first started to implement this using the LED class I create it with 36 different LED nodes:
alienware-wmi::running::head_red
alienware-wmi::running::head_blue
alienware-wmi::running::head_green
alienware-wmi::running::head_brightness
alienware-wmi::running::left_red
alienware-wmi::running::left_blue
alienware-wmi::running::left_green
alienware-wmi::running::left_brightness
alienware-wmi::running::right_red
alienware-wmi::running::right_blue
alienware-wmi::running::right_green
alienware-wmi::running::right_brightness
alienware-wmi::booting::head_red
alienware-wmi::booting::head_blue
alienware-wmi::booting::head_green
alienware-wmi::booting::head_brightness
alienware-wmi::booting::left_red
alienware-wmi::booting::left_blue
alienware-wmi::booting::left_green
alienware-wmi::booting::left_brightness
alienware-wmi::booting::right_red
alienware-wmi::booting::right_blue
alienware-wmi::booting::right_green
alienware-wmi::booting::right_brightness
alienware-wmi::suspend::head_red
alienware-wmi::suspend::head_blue
alienware-wmi::suspend::head_green
alienware-wmi::suspend::head_brightness
alienware-wmi::suspend::left_red
alienware-wmi::suspend::left_blue
alienware-wmi::suspend::left_green
alienware-wmi::suspend::left_brightness
alienware-wmi::suspend::right_red
alienware-wmi::suspend::right_blue
alienware-wmi::suspend::right_green
alienware-wmi::suspend::right_brightness
I thought this was rather confusing though because each individual node only has a single "brightness" member which doesn't really identify what is being changed. The brightness node changes the overall brightness of the whole zone. The color shades selected for each node are mixed to come up with the overall color for the zone.
Quoted 9 lines
The three different modes (running/booting/suspend) all modify the same LEDs too, so it didn't seem to make sense to me that they had their own nodes.
So given all of that, can you advise how you think this should be implemented? Would it make sense to extend the LED class to better adapt to this? Or do you think this is better suited for a custom sysfs interface as I've already done? I'll attach the patch for the driver so you can see how I put it together with the custom sysfs interface.

Quoted 3 lines

I just went through the patch you attached quickly, but failed to find anything related to system booting/running/suspended. So you plan to do that by using user space script to talk with those sysfs interface?

It's already in there enum LIGHTING_CONTROL_STATE defines the three different states. There's a sysfs node called lighting_control_state that was made to control it.

static DEVICE_ATTR(lighting_control_state, S_IRUGO | S_IWUSR,

                   show_control_state, set_control_state);

The idea here should be that later a userspace GUI will be able to toggle lighting_control_state to the right enum and then make any lighting color change requests. The BIOS would handle the actual implementation of the right time it would be effective for.

Bryan Wu

2014-02-07 18:32 UTC #3 lore raw

Quoted 12 lines · Mario Limonciello

Quoted 11 lines · Bryan Wu
I think for this use case, we need 2 drivers:
1. LED device driver for each lighting zone, it will supports red, blue, green and brightness operation.
2. LED trigger driver provides events trigger when system running, booting and suspended. It can send out event to your Alienware LED device driver to do proper LED operations.

Quoted 8 lines

Quoted 2 lines
So the basic concept is splitting out real LED operations and event triggers.
Sorry, I should have clarified - the LEDs operations themselves are BIOS controlled. I don't think there should be any trigger operation necessary. It's a different WMI BIOS call to configure the LEDs in each different state.

OK, I got it. So you might just need to write a LED device driver which implements some interface like .brightness_set() and also you can provide your own sysfs interface in the same driver.

Take a look at drivers/leds/dell-led.c, I think the idea is similar and you just need to think about support your multiple zones and sysfs interface.

Actually I think your fully custom driver here should also work since BIOS controls when to blink and how to blink. Normally, the LED device driver deals with how to blink no matter it's software control or hardware control and LED trigger driver deals with when to blink and what pattern to blink. But looks like your driver just simple setting and let BIOS to handle everything.

Thanks,
-Bryan

Quoted 80 lines

Quoted 65 lines
Quoted 56 lines
When I first started to implement this using the LED class I create it with
36 different LED nodes:
alienware-wmi::running::head_red
alienware-wmi::running::head_blue
alienware-wmi::running::head_green
alienware-wmi::running::head_brightness
alienware-wmi::running::left_red
alienware-wmi::running::left_blue
alienware-wmi::running::left_green
alienware-wmi::running::left_brightness
alienware-wmi::running::right_red
alienware-wmi::running::right_blue
alienware-wmi::running::right_green
alienware-wmi::running::right_brightness
alienware-wmi::booting::head_red
alienware-wmi::booting::head_blue
alienware-wmi::booting::head_green
alienware-wmi::booting::head_brightness
alienware-wmi::booting::left_red
alienware-wmi::booting::left_blue
alienware-wmi::booting::left_green
alienware-wmi::booting::left_brightness
alienware-wmi::booting::right_red
alienware-wmi::booting::right_blue
alienware-wmi::booting::right_green
alienware-wmi::booting::right_brightness
alienware-wmi::suspend::head_red
alienware-wmi::suspend::head_blue
alienware-wmi::suspend::head_green
alienware-wmi::suspend::head_brightness
alienware-wmi::suspend::left_red
alienware-wmi::suspend::left_blue
alienware-wmi::suspend::left_green
alienware-wmi::suspend::left_brightness
alienware-wmi::suspend::right_red
alienware-wmi::suspend::right_blue
alienware-wmi::suspend::right_green
alienware-wmi::suspend::right_brightness
I thought this was rather confusing though because each individual node only
has a single "brightness" member which doesn't really identify what is being
changed. The brightness node changes the overall brightness of the whole zone. The color shades selected for each node are mixed to come up with the
overall color for the zone.
The three different modes (running/booting/suspend) all modify the same LEDs
too, so it didn't seem to make sense to me that they had their own nodes.
Quoted 9 lines
So given all of that, can you advise how you think this should be implemented? Would it make sense to extend the LED class to better adapt to
this? Or do you think this is better suited for a custom sysfs interface as
I've already done? I'll attach the patch for the driver so you can see how
I put it together with the custom sysfs interface.
Quoted 3 lines
I just went through the patch you attached quickly, but failed to find anything related to system booting/running/suspended. So you plan to do that by using user space script to talk with those sysfs interface?
It's already in there enum LIGHTING_CONTROL_STATE defines the three different states. There's a sysfs node called lighting_control_state that was made to control it.
static DEVICE_ATTR(lighting_control_state, S_IRUGO | S_IWUSR,
                   show_control_state, set_control_state);
The idea here should be that later a userspace GUI will be able to toggle lighting_control_state to the right enum and then make any lighting color change requests. The BIOS would handle the actual implementation of the right time it would be effective for.