platform-driver-x86.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] asus-wmi: Add support for TUF laptop keyboard RGB
@ 2022-08-03 23:16 Luke D. Jones
  2022-08-04 13:36 ` Hans de Goede
  2022-08-13 13:37 ` kernel test robot
  0 siblings, 2 replies; 10+ messages in thread
From: Luke D. Jones @ 2022-08-03 23:16 UTC (permalink / raw)
  To: hdegoede; +Cc: markgross, platform-driver-x86, linux-kernel, Luke D. Jones

Adds support for TUF laptop RGB control. This adds a multicolor LED
device, and two sysfs paths for extra feature control.

/sys/devices/platform/asus-nb-wmi/tuf_krgb_mode_index provides
labels for the index fields as "save mode speed"

/sys/devices/platform/asus-nb-wmi/tuf_krgb_mode has the following
as input options via U8 "n n n":
- Save or set, if set, then settings revert on cold boot
- Mode, 0 = Static, 1 = Breathe, 2 = Colour cycle, 3 = Pulse
- Speed, 0 = Slow, 1 = Medium, 2 = Fast

Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
 drivers/platform/x86/asus-wmi.c            | 213 +++++++++++++++++++++
 include/linux/platform_data/x86/asus-wmi.h |   3 +
 2 files changed, 216 insertions(+)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 0e7fbed8a50d..2959f17047a8 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -25,6 +25,7 @@
 #include <linux/input/sparse-keymap.h>
 #include <linux/kernel.h>
 #include <linux/leds.h>
+#include <linux/led-class-multicolor.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/pci_hotplug.h>
@@ -117,6 +118,9 @@ static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL };
 
 static int throttle_thermal_policy_write(struct asus_wmi *);
 
+static int tuf_rgb_brightness_set(struct led_classdev *cdev,
+							enum led_brightness brightness);
+
 static bool ashs_present(void)
 {
 	int i = 0;
@@ -190,6 +194,14 @@ struct fan_curve_data {
 	u8 percents[FAN_CURVE_POINTS];
 };
 
+struct tuf_rgb_led {
+	struct led_classdev_mc dev;
+	struct mc_subled subled_info[3]; /* r g b */
+	u8 save;
+	u8 mode;
+	u8 speed;
+};
+
 struct asus_wmi {
 	int dsts_id;
 	int spec;
@@ -234,6 +246,9 @@ struct asus_wmi {
 	bool dgpu_disable_available;
 	bool dgpu_disable;
 
+	bool tuf_krgb_mode_available;
+	struct tuf_rgb_led tuf_krgb_mode;
+
 	bool throttle_thermal_policy_available;
 	u8 throttle_thermal_policy_mode;
 
@@ -734,6 +749,116 @@ static ssize_t egpu_enable_store(struct device *dev,
 
 static DEVICE_ATTR_RW(egpu_enable);
 
+/* TUF Laptop Keyboard RGB Modes **********************************************/
+static int tuf_krgb_mode_check_present(struct asus_wmi *asus)
+{
+	u32 result;
+	int err;
+
+	asus->tuf_krgb_mode_available = false;
+
+	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_MODE, &result);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		return err;
+	}
+
+	if (result & ASUS_WMI_DSTS_PRESENCE_BIT) {
+		asus->tuf_krgb_mode_available = true;
+		/* set some sane defaults since we can't read this from WMI */
+		asus->tuf_krgb_mode.save = 1;
+		asus->tuf_krgb_mode.mode = 0;
+		asus->tuf_krgb_mode.speed = 1;
+	}
+
+	return 0;
+}
+
+static ssize_t tuf_krgb_mode_store(struct device *device,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	char *data, *part, *end;
+	u8 res, tmp, arg_num;
+	int err;
+
+	struct asus_wmi *asus = dev_get_drvdata(device);
+	struct led_classdev *cdev = &asus->tuf_krgb_mode.dev.led_cdev;
+
+	data = end = kstrdup(buf, GFP_KERNEL);
+	arg_num = 0;
+
+	while ((part = strsep(&end, " ")) != NULL) {
+		if (part == NULL)
+			return -1;
+
+		res = kstrtou8(part, 10, &tmp);
+		if (res)
+			return -1;
+
+		if (arg_num == 0)
+			asus->tuf_krgb_mode.save = tmp;
+		else if (arg_num == 1)
+			/* These are the known usable modes across all TUF/ROG */
+			asus->tuf_krgb_mode.mode = tmp < 12 && tmp != 9 ? tmp : 0x0a;
+		else if (arg_num == 2) {
+			if (tmp == 0)
+				asus->tuf_krgb_mode.speed = 0xe1;
+			else if (tmp == 1)
+				asus->tuf_krgb_mode.speed = 0xeb;
+			else if (tmp == 2)
+				asus->tuf_krgb_mode.speed = 0xf5;
+			else
+				asus->tuf_krgb_mode.speed = 0xeb;
+		}
+
+		arg_num += 1;
+	}
+
+	err = tuf_rgb_brightness_set(cdev, cdev->brightness);
+	if (err)
+		return err;
+	return 0;
+}
+
+static ssize_t tuf_krgb_mode_show(struct device *device,
+						 struct device_attribute *attr,
+						 char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(device);
+	u8 speed = asus->tuf_krgb_mode.speed;
+	int len;
+
+	if (speed == 0xe1)
+		speed = 0;
+	else if (speed == 0xeb)
+		speed = 1;
+	else if (speed == 0xf5)
+		speed = 2;
+	else
+		speed = 1;
+
+	len = sprintf(buf, "%d %d %d",
+						asus->tuf_krgb_mode.save,
+						asus->tuf_krgb_mode.mode,
+						speed);
+
+	return len;
+}
+
+static DEVICE_ATTR_RW(tuf_krgb_mode);
+
+static ssize_t tuf_krgb_mode_index_show(struct device *device,
+						 struct device_attribute *attr,
+						 char *buf)
+{
+	int len = sprintf(buf, "%s", "save mode speed\n");
+	return len;
+}
+
+static DEVICE_ATTR_RO(tuf_krgb_mode_index);
+
 /* Battery ********************************************************************/
 
 /* The battery maximum charging percentage */
@@ -1028,6 +1153,38 @@ static enum led_brightness lightbar_led_get(struct led_classdev *led_cdev)
 	return result & ASUS_WMI_DSTS_LIGHTBAR_MASK;
 }
 
+static int tuf_rgb_brightness_set(struct led_classdev *cdev,
+	enum led_brightness brightness)
+{
+	u8 r, g, b, mode, speed, save;
+	int err;
+	u32 ret;
+	struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev);
+	struct asus_wmi *asus = container_of(mc_cdev, struct asus_wmi, tuf_krgb_mode.dev);
+
+	led_mc_calc_color_components(mc_cdev, brightness);
+	r = mc_cdev->subled_info[0].brightness;
+	g = mc_cdev->subled_info[1].brightness;
+	b = mc_cdev->subled_info[2].brightness;
+	/* 0 still sets the mode/rgb, but does not stick on reboot */
+	save = asus->tuf_krgb_mode.save == 1 ? 0xb5 : 0xb4;
+	mode = asus->tuf_krgb_mode.mode;
+	speed = asus->tuf_krgb_mode.speed;
+
+	err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS, ASUS_WMI_DEVID_TUF_RGB_MODE,
+			save | (mode << 8) | (r << 16) | (g << 24), (b) | (speed << 8), &ret);
+	if (err) {
+		pr_err("Unable to set TUF RGB data?\n");
+		return err;
+	}
+	return 0;
+}
+
+static enum led_brightness tuf_rgb_brightness_get(struct led_classdev *cdev)
+{
+	return cdev->brightness;
+}
+
 static void asus_wmi_led_exit(struct asus_wmi *asus)
 {
 	led_classdev_unregister(&asus->kbd_led);
@@ -1105,6 +1262,51 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
 					   &asus->lightbar_led);
 	}
 
+	if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_MODE)) {
+		struct led_classdev_mc *mc_cdev;
+		struct mc_subled *mc_led_info;
+		u8 brightness = 127;
+
+		mc_cdev = &asus->tuf_krgb_mode.dev;
+
+		mc_cdev->led_cdev.name = "asus::multicolour";
+		mc_cdev->led_cdev.flags = LED_CORE_SUSPENDRESUME | LED_RETAIN_AT_SHUTDOWN;
+		mc_cdev->led_cdev.brightness_set_blocking = tuf_rgb_brightness_set;
+		mc_cdev->led_cdev.brightness_get = tuf_rgb_brightness_get;
+
+		/* Let the multicolour LED own the info */
+		mc_led_info = devm_kmalloc_array(
+			&asus->platform_device->dev,
+			3,
+			sizeof(*mc_led_info),
+			GFP_KERNEL | __GFP_ZERO);
+
+		if (!mc_led_info)
+			return -ENOMEM;
+
+		mc_led_info[0].color_index = LED_COLOR_ID_RED;
+		mc_led_info[1].color_index = LED_COLOR_ID_GREEN;
+		mc_led_info[2].color_index = LED_COLOR_ID_BLUE;
+
+		/* It's not possible to get last set data from device so set defaults */
+		asus->tuf_krgb_mode.save = 1;
+		asus->tuf_krgb_mode.mode = 0;
+		asus->tuf_krgb_mode.speed = 1;
+		mc_cdev->led_cdev.brightness = brightness;
+		mc_cdev->led_cdev.max_brightness = brightness;
+		mc_led_info[0].intensity = brightness;
+		mc_led_info[0].brightness = mc_cdev->led_cdev.brightness;
+		mc_led_info[1].brightness = mc_cdev->led_cdev.brightness;
+		mc_led_info[2].brightness = mc_cdev->led_cdev.brightness;
+		led_mc_calc_color_components(mc_cdev, brightness);
+
+		mc_cdev->subled_info = mc_led_info;
+		mc_cdev->num_colors = 3;
+
+		tuf_rgb_brightness_set(&mc_cdev->led_cdev, brightness);
+		rv = led_classdev_multicolor_register(&asus->platform_device->dev, mc_cdev);
+	}
+
 error:
 	if (rv)
 		asus_wmi_led_exit(asus);
@@ -3258,6 +3460,8 @@ static struct attribute *platform_attributes[] = {
 	&dev_attr_touchpad.attr,
 	&dev_attr_egpu_enable.attr,
 	&dev_attr_dgpu_disable.attr,
+	&dev_attr_tuf_krgb_mode.attr,
+	&dev_attr_tuf_krgb_mode_index.attr,
 	&dev_attr_lid_resume.attr,
 	&dev_attr_als_enable.attr,
 	&dev_attr_fan_boost_mode.attr,
@@ -3288,6 +3492,10 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
 		ok = asus->egpu_enable_available;
 	else if (attr == &dev_attr_dgpu_disable.attr)
 		ok = asus->dgpu_disable_available;
+	else if (attr == &dev_attr_tuf_krgb_mode.attr)
+		ok = asus->tuf_krgb_mode_available;
+	else if (attr == &dev_attr_tuf_krgb_mode_index.attr)
+		ok = asus->tuf_krgb_mode_available;
 	else if (attr == &dev_attr_fan_boost_mode.attr)
 		ok = asus->fan_boost_mode_available;
 	else if (attr == &dev_attr_throttle_thermal_policy.attr)
@@ -3557,6 +3765,10 @@ static int asus_wmi_add(struct platform_device *pdev)
 	if (err)
 		goto fail_dgpu_disable;
 
+	err = tuf_krgb_mode_check_present(asus);
+	if (err)
+		goto fail_tuf_krgb_mode;
+
 	err = fan_boost_mode_check_present(asus);
 	if (err)
 		goto fail_fan_boost_mode;
@@ -3671,6 +3883,7 @@ static int asus_wmi_add(struct platform_device *pdev)
 fail_fan_boost_mode:
 fail_egpu_enable:
 fail_dgpu_disable:
+fail_tuf_krgb_mode:
 fail_platform:
 fail_panel_od:
 	kfree(asus);
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index a571b47ff362..5049c153a3fe 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -98,6 +98,9 @@
 /* dgpu on/off */
 #define ASUS_WMI_DEVID_DGPU		0x00090020
 
+/* TUF laptop RGB modes */
+#define ASUS_WMI_DEVID_TUF_RGB_MODE	0x00100056
+
 /* DSTS masks */
 #define ASUS_WMI_DSTS_STATUS_BIT	0x00000001
 #define ASUS_WMI_DSTS_UNKNOWN_BIT	0x00000002
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] asus-wmi: Add support for TUF laptop keyboard RGB
  2022-08-03 23:16 [PATCH] asus-wmi: Add support for TUF laptop keyboard RGB Luke D. Jones
@ 2022-08-04 13:36 ` Hans de Goede
  2022-08-13 13:37 ` kernel test robot
  1 sibling, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2022-08-04 13:36 UTC (permalink / raw)
  To: Luke D. Jones; +Cc: markgross, platform-driver-x86, linux-kernel

Hi,

On 8/4/22 01:16, Luke D. Jones wrote:
> Adds support for TUF laptop RGB control. This adds a multicolor LED
> device, and two sysfs paths for extra feature control.
> 
> /sys/devices/platform/asus-nb-wmi/tuf_krgb_mode_index provides
> labels for the index fields as "save mode speed"

As mentioned in my review of "[PATCH v2 1/1] asus-wmi: Add support
for TUF laptop keyboard states" the new tuf_krgb_mode attribute
should be an extra attribute under the led_class_device, you can do
this by adding this attribute to a separate attribute_group,
lets say e.g. "tuf_rgb_attributes" and then in the code of this
patch add:

	mc_cdev->led_cdev.groups = tuf_rgb_attributes;

and then the "tuf_krgb_mode" file should show up as:
/sys/class/leds/asus::multicolour/tuf_krgb_mode

Also again please drop the tuf_krgb_mode_index file and document
things in Documentation/ABI/testing/sysfs-platform-asus-wmi.

I've not done a detailed review of this yet, but overall this looks
good, definitely moving in the right direction.

My only other remark is that the led_class_device name should be
something like: "asus_wmi::kbd_backlight".

For easier reviewing of the next version, please split this
into 3 patches:

1. Add just the multi color led_class_dev
2. Add tuf_krgb_state attribute under the led_class_dev
3. Add tuf_krgb_mode attribute under the led_class_dev

Also see some further comments inline / below.


> /sys/devices/platform/asus-nb-wmi/tuf_krgb_mode has the following
> as input options via U8 "n n n":
> - Save or set, if set, then settings revert on cold boot
> - Mode, 0 = Static, 1 = Breathe, 2 = Colour cycle, 3 = Pulse
> - Speed, 0 = Slow, 1 = Medium, 2 = Fast
> 
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> ---
>  drivers/platform/x86/asus-wmi.c            | 213 +++++++++++++++++++++
>  include/linux/platform_data/x86/asus-wmi.h |   3 +
>  2 files changed, 216 insertions(+)
> 
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 0e7fbed8a50d..2959f17047a8 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -25,6 +25,7 @@
>  #include <linux/input/sparse-keymap.h>
>  #include <linux/kernel.h>
>  #include <linux/leds.h>
> +#include <linux/led-class-multicolor.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
>  #include <linux/pci_hotplug.h>
> @@ -117,6 +118,9 @@ static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL };
>  
>  static int throttle_thermal_policy_write(struct asus_wmi *);
>  
> +static int tuf_rgb_brightness_set(struct led_classdev *cdev,
> +							enum led_brightness brightness);
> +
>  static bool ashs_present(void)
>  {
>  	int i = 0;
> @@ -190,6 +194,14 @@ struct fan_curve_data {
>  	u8 percents[FAN_CURVE_POINTS];
>  };
>  
> +struct tuf_rgb_led {
> +	struct led_classdev_mc dev;
> +	struct mc_subled subled_info[3]; /* r g b */
> +	u8 save;
> +	u8 mode;
> +	u8 speed;
> +};
> +
>  struct asus_wmi {
>  	int dsts_id;
>  	int spec;
> @@ -234,6 +246,9 @@ struct asus_wmi {
>  	bool dgpu_disable_available;
>  	bool dgpu_disable;
>  
> +	bool tuf_krgb_mode_available;
> +	struct tuf_rgb_led tuf_krgb_mode;
> +
>  	bool throttle_thermal_policy_available;
>  	u8 throttle_thermal_policy_mode;
>  
> @@ -734,6 +749,116 @@ static ssize_t egpu_enable_store(struct device *dev,
>  
>  static DEVICE_ATTR_RW(egpu_enable);
>  
> +/* TUF Laptop Keyboard RGB Modes **********************************************/
> +static int tuf_krgb_mode_check_present(struct asus_wmi *asus)
> +{
> +	u32 result;
> +	int err;
> +
> +	asus->tuf_krgb_mode_available = false;
> +
> +	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_MODE, &result);
> +	if (err) {
> +		if (err == -ENODEV)
> +			return 0;
> +		return err;
> +	}
> +
> +	if (result & ASUS_WMI_DSTS_PRESENCE_BIT) {
> +		asus->tuf_krgb_mode_available = true;
> +		/* set some sane defaults since we can't read this from WMI */
> +		asus->tuf_krgb_mode.save = 1;
> +		asus->tuf_krgb_mode.mode = 0;
> +		asus->tuf_krgb_mode.speed = 1;

Why not just make tuf_krgb_mode write-only like you have done for tuf_krgb_state ?

> +	}
> +
> +	return 0;
> +}
> +
> +static ssize_t tuf_krgb_mode_store(struct device *device,
> +				 struct device_attribute *attr,
> +				 const char *buf, size_t count)
> +{
> +	char *data, *part, *end;
> +	u8 res, tmp, arg_num;
> +	int err;
> +
> +	struct asus_wmi *asus = dev_get_drvdata(device);
> +	struct led_classdev *cdev = &asus->tuf_krgb_mode.dev.led_cdev;
> +
> +	data = end = kstrdup(buf, GFP_KERNEL);
> +	arg_num = 0;
> +
> +	while ((part = strsep(&end, " ")) != NULL) {
> +		if (part == NULL)
> +			return -1;

return -EINVAL please.

> +
> +		res = kstrtou8(part, 10, &tmp);
> +		if (res)
> +			return -1;

return -EINVAL please.

> +
> +		if (arg_num == 0)
> +			asus->tuf_krgb_mode.save = tmp;
> +		else if (arg_num == 1)
> +			/* These are the known usable modes across all TUF/ROG */
> +			asus->tuf_krgb_mode.mode = tmp < 12 && tmp != 9 ? tmp : 0x0a;
> +		else if (arg_num == 2) {
> +			if (tmp == 0)
> +				asus->tuf_krgb_mode.speed = 0xe1;
> +			else if (tmp == 1)
> +				asus->tuf_krgb_mode.speed = 0xeb;
> +			else if (tmp == 2)
> +				asus->tuf_krgb_mode.speed = 0xf5;
> +			else
> +				asus->tuf_krgb_mode.speed = 0xeb;
> +		}
> +
> +		arg_num += 1;
> +	}

Maybe just replace the kstrdup + the entire while loop with:

	int a, b, c;

	if (sscanf(buf, "%d %d %d", &a, &b, &c) != 3)
		return -EINVAL;

	asus->tuf_krgb_mode.save = a;
	asus->tuf_krgb_mode.mode = b < 12 && b != 9 ? b : 0x0a;

	if (c == 0)
		asus->tuf_krgb_mode.speed = 0xe1;
	else if (c == 1)
		asus->tuf_krgb_mode.speed = 0xeb;
	else if (c == 2)
		asus->tuf_krgb_mode.speed = 0xf5;
	else
		asus->tuf_krgb_mode.speed = 0xeb;
	
That certainly seems a lot cleaner to me ?

And perhaps you can do something similar for
tuf_krgb_state_store  ?



> +
> +	err = tuf_rgb_brightness_set(cdev, cdev->brightness);
> +	if (err)
> +		return err;
> +	return 0;
> +}
> +
> +static ssize_t tuf_krgb_mode_show(struct device *device,
> +						 struct device_attribute *attr,
> +						 char *buf)
> +{
> +	struct asus_wmi *asus = dev_get_drvdata(device);
> +	u8 speed = asus->tuf_krgb_mode.speed;
> +	int len;
> +
> +	if (speed == 0xe1)
> +		speed = 0;
> +	else if (speed == 0xeb)
> +		speed = 1;
> +	else if (speed == 0xf5)
> +		speed = 2;
> +	else
> +		speed = 1;
> +
> +	len = sprintf(buf, "%d %d %d",
> +						asus->tuf_krgb_mode.save,
> +						asus->tuf_krgb_mode.mode,
> +						speed);
> +
> +	return len;
> +}
> +
> +static DEVICE_ATTR_RW(tuf_krgb_mode);

As mentioned above why not just make this write-only
like you have done for tuf_krgb_state ?

> +
> +static ssize_t tuf_krgb_mode_index_show(struct device *device,
> +						 struct device_attribute *attr,
> +						 char *buf)
> +{
> +	int len = sprintf(buf, "%s", "save mode speed\n");
> +	return len;
> +}
> +
> +static DEVICE_ATTR_RO(tuf_krgb_mode_index);
> +
>  /* Battery ********************************************************************/
>  
>  /* The battery maximum charging percentage */
> @@ -1028,6 +1153,38 @@ static enum led_brightness lightbar_led_get(struct led_classdev *led_cdev)
>  	return result & ASUS_WMI_DSTS_LIGHTBAR_MASK;
>  }
>  
> +static int tuf_rgb_brightness_set(struct led_classdev *cdev,
> +	enum led_brightness brightness)
> +{
> +	u8 r, g, b, mode, speed, save;
> +	int err;
> +	u32 ret;
> +	struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev);
> +	struct asus_wmi *asus = container_of(mc_cdev, struct asus_wmi, tuf_krgb_mode.dev);
> +
> +	led_mc_calc_color_components(mc_cdev, brightness);
> +	r = mc_cdev->subled_info[0].brightness;
> +	g = mc_cdev->subled_info[1].brightness;
> +	b = mc_cdev->subled_info[2].brightness;
> +	/* 0 still sets the mode/rgb, but does not stick on reboot */
> +	save = asus->tuf_krgb_mode.save == 1 ? 0xb5 : 0xb4;
> +	mode = asus->tuf_krgb_mode.mode;
> +	speed = asus->tuf_krgb_mode.speed;
> +
> +	err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS, ASUS_WMI_DEVID_TUF_RGB_MODE,
> +			save | (mode << 8) | (r << 16) | (g << 24), (b) | (speed << 8), &ret);
> +	if (err) {
> +		pr_err("Unable to set TUF RGB data?\n");
> +		return err;
> +	}
> +	return 0;
> +}
> +
> +static enum led_brightness tuf_rgb_brightness_get(struct led_classdev *cdev)
> +{
> +	return cdev->brightness;
> +}
> +
>  static void asus_wmi_led_exit(struct asus_wmi *asus)
>  {
>  	led_classdev_unregister(&asus->kbd_led);
> @@ -1105,6 +1262,51 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
>  					   &asus->lightbar_led);
>  	}
>  
> +	if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_MODE)) {
> +		struct led_classdev_mc *mc_cdev;
> +		struct mc_subled *mc_led_info;
> +		u8 brightness = 127;
> +
> +		mc_cdev = &asus->tuf_krgb_mode.dev;
> +
> +		mc_cdev->led_cdev.name = "asus::multicolour";
> +		mc_cdev->led_cdev.flags = LED_CORE_SUSPENDRESUME | LED_RETAIN_AT_SHUTDOWN;
> +		mc_cdev->led_cdev.brightness_set_blocking = tuf_rgb_brightness_set;
> +		mc_cdev->led_cdev.brightness_get = tuf_rgb_brightness_get;
> +
> +		/* Let the multicolour LED own the info */
> +		mc_led_info = devm_kmalloc_array(
> +			&asus->platform_device->dev,
> +			3,
> +			sizeof(*mc_led_info),
> +			GFP_KERNEL | __GFP_ZERO);
> +
> +		if (!mc_led_info)
> +			return -ENOMEM;
> +
> +		mc_led_info[0].color_index = LED_COLOR_ID_RED;
> +		mc_led_info[1].color_index = LED_COLOR_ID_GREEN;
> +		mc_led_info[2].color_index = LED_COLOR_ID_BLUE;
> +
> +		/* It's not possible to get last set data from device so set defaults */
> +		asus->tuf_krgb_mode.save = 1;
> +		asus->tuf_krgb_mode.mode = 0;
> +		asus->tuf_krgb_mode.speed = 1;
> +		mc_cdev->led_cdev.brightness = brightness;
> +		mc_cdev->led_cdev.max_brightness = brightness;
> +		mc_led_info[0].intensity = brightness;
> +		mc_led_info[0].brightness = mc_cdev->led_cdev.brightness;
> +		mc_led_info[1].brightness = mc_cdev->led_cdev.brightness;
> +		mc_led_info[2].brightness = mc_cdev->led_cdev.brightness;
> +		led_mc_calc_color_components(mc_cdev, brightness);
> +
> +		mc_cdev->subled_info = mc_led_info;
> +		mc_cdev->num_colors = 3;
> +
> +		tuf_rgb_brightness_set(&mc_cdev->led_cdev, brightness);
> +		rv = led_classdev_multicolor_register(&asus->platform_device->dev, mc_cdev);
> +	}
> +
>  error:
>  	if (rv)
>  		asus_wmi_led_exit(asus);
> @@ -3258,6 +3460,8 @@ static struct attribute *platform_attributes[] = {
>  	&dev_attr_touchpad.attr,
>  	&dev_attr_egpu_enable.attr,
>  	&dev_attr_dgpu_disable.attr,
> +	&dev_attr_tuf_krgb_mode.attr,
> +	&dev_attr_tuf_krgb_mode_index.attr,
>  	&dev_attr_lid_resume.attr,
>  	&dev_attr_als_enable.attr,
>  	&dev_attr_fan_boost_mode.attr,
> @@ -3288,6 +3492,10 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
>  		ok = asus->egpu_enable_available;
>  	else if (attr == &dev_attr_dgpu_disable.attr)
>  		ok = asus->dgpu_disable_available;
> +	else if (attr == &dev_attr_tuf_krgb_mode.attr)
> +		ok = asus->tuf_krgb_mode_available;
> +	else if (attr == &dev_attr_tuf_krgb_mode_index.attr)
> +		ok = asus->tuf_krgb_mode_available;
>  	else if (attr == &dev_attr_fan_boost_mode.attr)
>  		ok = asus->fan_boost_mode_available;
>  	else if (attr == &dev_attr_throttle_thermal_policy.attr)
> @@ -3557,6 +3765,10 @@ static int asus_wmi_add(struct platform_device *pdev)
>  	if (err)
>  		goto fail_dgpu_disable;
>  
> +	err = tuf_krgb_mode_check_present(asus);
> +	if (err)
> +		goto fail_tuf_krgb_mode;
> +
>  	err = fan_boost_mode_check_present(asus);
>  	if (err)
>  		goto fail_fan_boost_mode;
> @@ -3671,6 +3883,7 @@ static int asus_wmi_add(struct platform_device *pdev)
>  fail_fan_boost_mode:
>  fail_egpu_enable:
>  fail_dgpu_disable:
> +fail_tuf_krgb_mode:
>  fail_platform:
>  fail_panel_od:
>  	kfree(asus);
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index a571b47ff362..5049c153a3fe 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -98,6 +98,9 @@
>  /* dgpu on/off */
>  #define ASUS_WMI_DEVID_DGPU		0x00090020
>  
> +/* TUF laptop RGB modes */
> +#define ASUS_WMI_DEVID_TUF_RGB_MODE	0x00100056
> +
>  /* DSTS masks */
>  #define ASUS_WMI_DSTS_STATUS_BIT	0x00000001
>  #define ASUS_WMI_DSTS_UNKNOWN_BIT	0x00000002



Regards,

Hans


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] asus-wmi: Add support for TUF laptop keyboard RGB
  2022-08-03 23:16 [PATCH] asus-wmi: Add support for TUF laptop keyboard RGB Luke D. Jones
  2022-08-04 13:36 ` Hans de Goede
@ 2022-08-13 13:37 ` kernel test robot
  1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2022-08-13 13:37 UTC (permalink / raw)
  To: Luke D. Jones, hdegoede
  Cc: kbuild-all, markgross, platform-driver-x86, linux-kernel, Luke D. Jones

Hi "Luke,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.19]
[cannot apply to next-20220812]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Luke-D-Jones/asus-wmi-Add-support-for-TUF-laptop-keyboard-RGB/20220804-071716
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 526942b8134cc34d25d27f95dfff98b8ce2f6fcd
config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20220813/202208132140.7Y3f2Xhn-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/e25083de01260bb6c1b3070f7f3e6915de07c44c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Luke-D-Jones/asus-wmi-Add-support-for-TUF-laptop-keyboard-RGB/20220804-071716
        git checkout e25083de01260bb6c1b3070f7f3e6915de07c44c
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/platform/x86/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/platform/x86/asus-wmi.c: In function 'tuf_krgb_mode_store':
>> drivers/platform/x86/asus-wmi.c:782:15: warning: variable 'data' set but not used [-Wunused-but-set-variable]
     782 |         char *data, *part, *end;
         |               ^~~~


vim +/data +782 drivers/platform/x86/asus-wmi.c

   777	
   778	static ssize_t tuf_krgb_mode_store(struct device *device,
   779					 struct device_attribute *attr,
   780					 const char *buf, size_t count)
   781	{
 > 782		char *data, *part, *end;
   783		u8 res, tmp, arg_num;
   784		int err;
   785	
   786		struct asus_wmi *asus = dev_get_drvdata(device);
   787		struct led_classdev *cdev = &asus->tuf_krgb_mode.dev.led_cdev;
   788	
   789		data = end = kstrdup(buf, GFP_KERNEL);
   790		arg_num = 0;
   791	
   792		while ((part = strsep(&end, " ")) != NULL) {
   793			if (part == NULL)
   794				return -1;
   795	
   796			res = kstrtou8(part, 10, &tmp);
   797			if (res)
   798				return -1;
   799	
   800			if (arg_num == 0)
   801				asus->tuf_krgb_mode.save = tmp;
   802			else if (arg_num == 1)
   803				/* These are the known usable modes across all TUF/ROG */
   804				asus->tuf_krgb_mode.mode = tmp < 12 && tmp != 9 ? tmp : 0x0a;
   805			else if (arg_num == 2) {
   806				if (tmp == 0)
   807					asus->tuf_krgb_mode.speed = 0xe1;
   808				else if (tmp == 1)
   809					asus->tuf_krgb_mode.speed = 0xeb;
   810				else if (tmp == 2)
   811					asus->tuf_krgb_mode.speed = 0xf5;
   812				else
   813					asus->tuf_krgb_mode.speed = 0xeb;
   814			}
   815	
   816			arg_num += 1;
   817		}
   818	
   819		err = tuf_rgb_brightness_set(cdev, cdev->brightness);
   820		if (err)
   821			return err;
   822		return 0;
   823	}
   824	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] asus-wmi: Add support for TUF laptop keyboard RGB
  2022-08-02 22:26     ` Luke Jones
@ 2022-08-04 13:03       ` Hans de Goede
  0 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2022-08-04 13:03 UTC (permalink / raw)
  To: Luke Jones; +Cc: corentin.chary, markgross, linux-kernel, platform-driver-x86

Hi Luke,

On 8/3/22 00:26, Luke Jones wrote:
> Hi Hans,
> 
> Not a problem at all mate (it didn't take long to write), I was expecting this and intended the patch more as a "let's see where we go".
> 
> Thank you for the information on how to handle proper bring-up, it seems I should be able to use this for both the TUF ACPI controlled keyboard along with the ROG USB connected keyboards - which would be excellent for everyone using these laptops. The TUF series of laptops are relatively nice and simple to work with due to the limited feature-set, so this is a great starting point.

Cool, thank you.

> To ensure I am on the correct footing, using the information provided, the goal would be to provide the following?
> 
> Base RGB setting under:
> /sys/class/leds/multicolor:status/multi_index
> /sys/class/leds/multicolor:status/multi_intensity
> /sys/class/leds/multicolor:status/max_brightness
> 
> Extra features under:
> /sys/class/leds/asus::kbd_backlight/mode
> /sys/class/leds/asus::kbd_backlight/speed
> /sys/class/leds/asus::kbd_backlight/direction

Erm, no the multicolor:status thing is just an example from the docs. You can
pick your own name for the LED class device in the driver and it should end
with ::kbd_backlight to make clear that it is a keyboard backlight (existing
userspace code checks for this postfix).

So you would just have everything under /sys/class/leds/asus::kbd_backlight/;
or maybe even better since you mentioned you also want to support USB keyboards
(presumably through drivers/hid/hid-asus.c ?) avoid a possible conflict there
by using: /sys/class/leds/asus_wmi::kbd_backlight/ so you would get:

Everything in one place:
/sys/class/leds/asus_wmi::kbd_backlight/multi_index
/sys/class/leds/asus_wmi::kbd_backlight/multi_intensity
/sys/class/leds/asus_wmi::kbd_backlight/max_brightness
/sys/class/leds/asus_wmi::kbd_backlight/mode
/sys/class/leds/asus_wmi::kbd_backlight/speed
/sys/class/leds/asus_wmi::kbd_backlight/direction

Note I must admit I don't have any experience with the multi-color
LED API. But for a non RGB (so single color) example see the
dell-laptop code:

/* whole bunch of show/store functions */

static struct attribute *kbd_led_attrs[] = {
        &dev_attr_stop_timeout.attr,
        &dev_attr_start_triggers.attr,
        NULL,
};

static const struct attribute_group kbd_led_group = {
        .attrs = kbd_led_attrs,
};

static struct attribute *kbd_led_als_attrs[] = {
        &dev_attr_als_enabled.attr,
        &dev_attr_als_setting.attr,
        NULL,
};

static const struct attribute_group kbd_led_als_group = {
        .attrs = kbd_led_als_attrs,
};

static const struct attribute_group *kbd_led_groups[] = {
        &kbd_led_group,
        &kbd_led_als_group,
        NULL,
};

static enum led_brightness kbd_led_level_get(struct led_classdev *led_cdev)
{
	return <current-brightness>
}

static int kbd_led_level_set(struct led_classdev *led_cdev,
                             enum led_brightness value)
{
	<write value to hw>;
}

static struct led_classdev kbd_led = {
        .name           = "dell::kbd_backlight",
        .brightness_set_blocking = kbd_led_level_set,
        .brightness_get = kbd_led_level_get,
        .groups         = kbd_led_groups,
};

static int __init kbd_led_init(struct device *dev)
{
       return led_classdev_register(dev, &kbd_led);
}

Notice how the name is chosen to be "dell::kbd_backlight" and
additional attributes for handling special features are
passed in the led_classdev.groups attribute.

> Some questions I have:
> 1. If there is more than one RGB capable keyboard, what happens to /sys/class/leds/multicolor:status ?

See above, you should pick a unique name for the asus_wmi supported
RGB keyboard, of which there will presumably be only one.

> 2. There is no way to fetch the current values from the keyboard and multicolor seems to expect that the current settings can be read - the issue here I think is that if the keyboard is changed perhaps by Windows dual-boot then these settings won't match. How would this be handled?

Yes I've seen this problem before with some USB keyboards with
backlight support.

Can you read back the "saved" profile at least ? Then I would just
default to that. Otherwise maybe default to the factory default settings?

And then I would expect any userspace daemon like code using this
interface to save/restore settings over reboot; and after the first
write the driver can just cache the last written values.

> 3. I'm still relatively new to sysfs. Is it possible for sysfs to save these values (e.g, multicolour) and re-apply on boot?

This would require userspace support. systemd already does something
like this for the display panel backlight. I can imagine a similar
small helper + udev rules to trigger it being part of asusctl
(since you also want to save/restore some Asus special settings) ?

> 4. In the case of the ROG USB connected laptop keyboards, some models have per-key control. How would I enable this using the multicolour API? Something like:
> 
> /sys/class/leds/multicolor:status/multi_a_index
> /sys/class/leds/multicolor:status/multi_a_intensity
> /sys/class/leds/multicolor:status/multi_left_ctrl_index
> /sys/class/leds/multicolor:status/multi_left_ctrl_intensity
> /sys/class/leds/multicolor:status/multi_spacebar_index
> /sys/class/leds/multicolor:status/multi_spacebar_intensity
> /sys/class/leds/multicolor:status/max_brightness (individual or all?)
> 
> For these keyboards the USB takes 9-12 packets of 64 bytes, and I've indexed the locations of colours in each. To be honest I'd very much like to work out how to do this for per-key as it can potentially save a lot of time in writing each packet if the kernel is able to batch writes internally. A single packet for half a row is ~1ms, but can vary up to 5ms..

This really needs to be discussed on the linux-leds@vger.kernel.org
list (please Cc me).

ATM AFAIK most Linux tools to program USB keyboards with per key
settings just use /dev/hidraw to directly talk to the kernel,
so there is no existing kernel API for this at all yet.

> 4. I'm unsure of how to structure the USB-RGB code.. Would I need to write an extra driver perhaps?

Normally the HID subsystem tries to stick to one driver per vendor,
so that would make drivers/hid/hid-asus.c the right place to stick
this. But if it is big and does not share much code with the rest
a new driver under say drivers/hid/hid-asus-rog.c might make sense...

Regards,

Hans



> On Tue, Aug 2 2022 at 14:13:06 +0200, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 8/2/22 13:09, Hans de Goede wrote:
>>>  Hi Luke,
>>>
>>>  On 8/2/22 06:59, Luke D. Jones wrote:
>>>>  Adds support for TUF laptop RGB control. This creates two sysfs
>>>>  paths to add control of basic keyboard LEDs, and power states.
>>>>
>>>>  /sys/devices/platform/asus-nb-wmi/tuf_krgb_mode has the following
>>>>  as input options via U8 "n n n n n n":
>>>>  - Save or set, if set, then settings revert on cold boot
>>>>  - Mode, 0-8 for regular modes (if supported), 10-12 for "warning" styles
>>>>  - Red, 0-255
>>>>  - Green, 0-255
>>>>  - Blue, 0-255
>>>>  - Speed, 0 = Slow, 1 = Medium, 2 = Fast
>>>>
>>>>  /sys/devices/platform/asus-nb-wmi/tuf_krgb_state has the following
>>>>  as input options via boolean "b b b b b":
>>>>  - Save or set, if set, then settings revert on cold boot
>>>>  - Boot, if true, the keyboard displays animation on boot
>>>>  - Awake, if true, the keyboard LED's are on while device is awake
>>>>  - Sleep, if true, the keyboard shows animation while device is suspended
>>>>  - Keybaord, appears to have no effect
>>>
>>>  Typo in Keybaord here.
>>>
>>>  Thank you for your patch. I really appreciate your continued
>>>  efforts to make Asus laptops work well with Linux.
>>>
>>>  For keyboard backlight support Linux has standardized on
>>>  using the /sys/class/leds API. So I'm afraid that this patch
>>>  will need to be rewritten to use the standard LED API
>>>  and then specifically the somewhat new multicolor LED API
>>>  at least for setting the RGB values (within the current mode)
>>>
>>>  Any extra functionality can then be added as extra sysfs
>>>  attributes under the /sys/class/leds/asus::kbd_backlight
>>>  device, see e.g. the use of kbd_led_groups in:
>>>  drivers/platform/x86/dell/dell-laptop.c
>>>
>>>  Note the kbd_backlight part of the name is important this
>>>  will allow upower to recognize this as a keyboard backlight
>>>  and will then enable desktop-environments which use
>>>  upower for kbd backlight control to at least control
>>>  the overall brightness of the kbd-backlight.
>>>
>>>  I realize that this means that you need to redo a whole
>>>  bunch of work here; and I presume also in your
>>>  asusctl userspace utility, sorry about that. But it
>>>  really is important that standard userspace APIs are
>>>  used for things like this where ever possible.
>>>
>>>  Regards,
>>>
>>>  Hans
>>
>> p.s.
>>
>> For more info on the multi-color LED API see:
>>
>> https://www.kernel.org/doc/html/latest/leds/leds-class-multicolor.html
>> https://www.phoronix.com/scan.php?page=news_item&px=Linux-5.9-Multi-Color-LEDs
>>
>>
>>
>>>
>>>
>>>
>>>
>>>>
>>>>  Signed-off-by: Luke D. Jones <luke@ljones.dev>
>>>>  ---
>>>>   drivers/platform/x86/asus-wmi.c            | 168 +++++++++++++++++++++
>>>>   include/linux/platform_data/x86/asus-wmi.h |   6 +
>>>>   2 files changed, 174 insertions(+)
>>>>
>>>>  diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
>>>>  index 62ce198a3463..09277bd98249 100644
>>>>  --- a/drivers/platform/x86/asus-wmi.c
>>>>  +++ b/drivers/platform/x86/asus-wmi.c
>>>>  @@ -234,6 +234,9 @@ struct asus_wmi {
>>>>       bool dgpu_disable_available;
>>>>       bool dgpu_disable;
>>>>
>>>>  +    bool tuf_kb_rgb_mode_available;
>>>>  +    bool tuf_kb_rgb_state_available;
>>>>  +
>>>>       bool throttle_thermal_policy_available;
>>>>       u8 throttle_thermal_policy_mode;
>>>>
>>>>  @@ -734,6 +737,153 @@ static ssize_t egpu_enable_store(struct device *dev,
>>>>
>>>>   static DEVICE_ATTR_RW(egpu_enable);
>>>>
>>>>  +/* TUF Laptop Keyboard RGB Modes **********************************************/
>>>>  +static int tuf_kb_rgb_mode_check_present(struct asus_wmi *asus)
>>>>  +{
>>>>  +    u32 result;
>>>>  +    int err;
>>>>  +
>>>>  +    asus->tuf_kb_rgb_mode_available = false;
>>>>  +
>>>>  +    err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_MODE, &result);
>>>>  +    if (err) {
>>>>  +        if (err == -ENODEV)
>>>>  +            return 0;
>>>>  +        return err;
>>>>  +    }
>>>>  +
>>>>  +    if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
>>>>  +        asus->tuf_kb_rgb_mode_available = true;
>>>>  +
>>>>  +    return 0;
>>>>  +}
>>>>  +
>>>>  +static ssize_t tuf_kb_rgb_mode_store(struct device *dev,
>>>>  +                 struct device_attribute *attr,
>>>>  +                 const char *buf, size_t count)
>>>>  +{
>>>>  +    int err;
>>>>  +    u32 ret;
>>>>  +    u8 res, tmp, arg_num;
>>>>  +    char *data, *part, *end;
>>>>  +    u8 cmd, mode, r, g,  b,  speed;
>>>>  +
>>>>  +    data = end = kstrdup(buf, GFP_KERNEL);
>>>>  +    cmd = mode = r = g = b = speed = arg_num = 0;
>>>>  +
>>>>  +    while ((part = strsep(&end, " ")) != NULL) {
>>>>  +        if (part == NULL)
>>>>  +            return -1;
>>>>  +
>>>>  +        res = kstrtou8(part, 10, &tmp);
>>>>  +        if (res)
>>>>  +            return -1;
>>>>  +
>>>>  +        if (arg_num == 0)
>>>>  +            // apply : set
>>>>  +            cmd = tmp == 1 ? 0xb5 : 0xb4;
>>>>  +        else if (arg_num == 1)
>>>>  +            // From 0-8 are valid modes with 10-12 being "warning"
>>>>  +            // style modes. All models have "pulse" mode 10.
>>>>  +            mode = (tmp <= 12 && tmp != 9) ? tmp : 10;
>>>>  +        else if (arg_num == 2)
>>>>  +            r = tmp;
>>>>  +        else if (arg_num == 3)
>>>>  +            g = tmp;
>>>>  +        else if (arg_num == 4)
>>>>  +            b = tmp;
>>>>  +        else if (arg_num == 5) {
>>>>  +            if (tmp == 0)
>>>>  +                speed = 0xe1;
>>>>  +            else if (tmp == 1)
>>>>  +                speed = 0xeb;
>>>>  +            else if (tmp == 2)
>>>>  +                speed = 0xf5;
>>>>  +        }
>>>>  +
>>>>  +        arg_num += 1;
>>>>  +    }
>>>>  +
>>>>  +    err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS, ASUS_WMI_DEVID_TUF_RGB_MODE,
>>>>  +            cmd | (mode << 8) | (r << 16) | (g << 24), (b) | (speed << 8), &ret);
>>>>  +    if (err)
>>>>  +        return err;
>>>>  +
>>>>  +    kfree(data);
>>>>  +    return count;
>>>>  +}
>>>>  +
>>>>  +static DEVICE_ATTR_WO(tuf_kb_rgb_mode);
>>>>  +
>>>>  +/* TUF Laptop Keyboard RGB States *********************************************/
>>>>  +static int tuf_kb_rgb_state_check_present(struct asus_wmi *asus)
>>>>  +{
>>>>  +    u32 result;
>>>>  +    int err;
>>>>  +
>>>>  +    asus->tuf_kb_rgb_state_available = false;
>>>>  +
>>>>  +    err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_STATE, &result);
>>>>  +    if (err) {
>>>>  +        if (err == -ENODEV)
>>>>  +            return 0;
>>>>  +        return err;
>>>>  +    }
>>>>  +
>>>>  +    if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
>>>>  +        asus->tuf_kb_rgb_state_available = true;
>>>>  +
>>>>  +    return 0;
>>>>  +}
>>>>  +
>>>>  +static ssize_t tuf_kb_rgb_state_store(struct device *dev,
>>>>  +                   struct device_attribute *attr,
>>>>  +                   const char *buf, size_t count)
>>>>  +{
>>>>  +    int err;
>>>>  +    u32 ret;
>>>>  +    bool tmp;
>>>>  +    char *data, *part, *end;
>>>>  +    u8 save, flags, res, arg_num;
>>>>  +
>>>>  +    save = flags = arg_num = 0;
>>>>  +    data = end = kstrdup(buf, GFP_KERNEL);
>>>>  +
>>>>  +    while ((part = strsep(&end, " ")) != NULL) {
>>>>  +        if (part == NULL)
>>>>  +            return -1;
>>>>  +
>>>>  +        res = kstrtobool(part, &tmp);
>>>>  +        if (res)
>>>>  +            return -1;
>>>>  +
>>>>  +        if (tmp) {
>>>>  +            if (arg_num == 0) // save  :  set
>>>>  +                save = tmp == 0 ? 0x0100 : 0x0000;
>>>>  +            else if (arg_num == 1)
>>>>  +                flags |= 0x02; // boot
>>>>  +            else if (arg_num == 2)
>>>>  +                flags |= 0x08; // awake
>>>>  +            else if (arg_num == 3)
>>>>  +                flags |= 0x20; // sleep
>>>>  +            else if (arg_num == 4)
>>>>  +                flags |= 0x80; // keyboard
>>>>  +        }
>>>>  +
>>>>  +        arg_num += 1;
>>>>  +    }
>>>>  +
>>>>  +    err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS,
>>>>  +            ASUS_WMI_DEVID_TUF_RGB_STATE, 0xBD | save | (flags << 16), 0, &ret);
>>>>  +    if (err)
>>>>  +        return err;
>>>>  +
>>>>  +    kfree(data);
>>>>  +    return count;
>>>>  +}
>>>>  +
>>>>  +static DEVICE_ATTR_WO(tuf_kb_rgb_state);
>>>>  +
>>>>   /* Battery ********************************************************************/
>>>>
>>>>   /* The battery maximum charging percentage */
>>>>  @@ -3258,6 +3408,8 @@ static struct attribute *platform_attributes[] = {
>>>>       &dev_attr_touchpad.attr,
>>>>       &dev_attr_egpu_enable.attr,
>>>>       &dev_attr_dgpu_disable.attr,
>>>>  +    &dev_attr_tuf_kb_rgb_mode.attr,
>>>>  +    &dev_attr_tuf_kb_rgb_state.attr,
>>>>       &dev_attr_lid_resume.attr,
>>>>       &dev_attr_als_enable.attr,
>>>>       &dev_attr_fan_boost_mode.attr,
>>>>  @@ -3286,6 +3438,12 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
>>>>           devid = ASUS_WMI_DEVID_ALS_ENABLE;
>>>>       else if (attr == &dev_attr_egpu_enable.attr)
>>>>           ok = asus->egpu_enable_available;
>>>>  +    else if (attr == &dev_attr_tuf_kb_rgb_mode.attr)
>>>>  +        ok = asus->tuf_kb_rgb_mode_available;
>>>>  +    else if (attr == &dev_attr_tuf_kb_rgb_state.attr)
>>>>  +        ok = asus->tuf_kb_rgb_state_available;
>>>>  +    else if (attr == &dev_attr_dgpu_disable.attr)
>>>>  +        ok = asus->dgpu_disable_available;
>>>>       else if (attr == &dev_attr_dgpu_disable.attr)
>>>>           ok = asus->dgpu_disable_available;
>>>>       else if (attr == &dev_attr_fan_boost_mode.attr)
>>>>  @@ -3557,6 +3715,14 @@ static int asus_wmi_add(struct platform_device *pdev)
>>>>       if (err)
>>>>           goto fail_dgpu_disable;
>>>>
>>>>  +    err = tuf_kb_rgb_mode_check_present(asus);
>>>>  +    if (err)
>>>>  +        goto fail_tuf_kb_rgb_mode;
>>>>  +
>>>>  +    err = tuf_kb_rgb_state_check_present(asus);
>>>>  +    if (err)
>>>>  +        goto fail_tuf_kb_rgb_state;
>>>>  +
>>>>       err = fan_boost_mode_check_present(asus);
>>>>       if (err)
>>>>           goto fail_fan_boost_mode;
>>>>  @@ -3671,6 +3837,8 @@ static int asus_wmi_add(struct platform_device *pdev)
>>>>   fail_fan_boost_mode:
>>>>   fail_egpu_enable:
>>>>   fail_dgpu_disable:
>>>>  +fail_tuf_kb_rgb_mode:
>>>>  +fail_tuf_kb_rgb_state:
>>>>   fail_platform:
>>>>   fail_panel_od:
>>>>       kfree(asus);
>>>>  diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
>>>>  index a571b47ff362..af4191fb0508 100644
>>>>  --- a/include/linux/platform_data/x86/asus-wmi.h
>>>>  +++ b/include/linux/platform_data/x86/asus-wmi.h
>>>>  @@ -98,6 +98,12 @@
>>>>   /* dgpu on/off */
>>>>   #define ASUS_WMI_DEVID_DGPU        0x00090020
>>>>
>>>>  +/* TUF laptop RGB modes/colours */
>>>>  +#define ASUS_WMI_DEVID_TUF_RGB_MODE    0x00100056
>>>>  +
>>>>  +/* TUF laptop RGB power/state */
>>>>  +#define ASUS_WMI_DEVID_TUF_RGB_STATE    0x00100057
>>>>  +
>>>>   /* DSTS masks */
>>>>   #define ASUS_WMI_DSTS_STATUS_BIT    0x00000001
>>>>   #define ASUS_WMI_DSTS_UNKNOWN_BIT    0x00000002
>>
> 
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] asus-wmi: Add support for TUF laptop keyboard RGB
  2022-08-02 12:13   ` Hans de Goede
@ 2022-08-02 22:26     ` Luke Jones
  2022-08-04 13:03       ` Hans de Goede
  0 siblings, 1 reply; 10+ messages in thread
From: Luke Jones @ 2022-08-02 22:26 UTC (permalink / raw)
  To: Hans de Goede
  Cc: corentin.chary, markgross, linux-kernel, platform-driver-x86

Hi Hans,

Not a problem at all mate (it didn't take long to write), I was 
expecting this and intended the patch more as a "let's see where we go".

Thank you for the information on how to handle proper bring-up, it 
seems I should be able to use this for both the TUF ACPI controlled 
keyboard along with the ROG USB connected keyboards - which would be 
excellent for everyone using these laptops. The TUF series of laptops 
are relatively nice and simple to work with due to the limited 
feature-set, so this is a great starting point.

To ensure I am on the correct footing, using the information provided, 
the goal would be to provide the following?

Base RGB setting under:
/sys/class/leds/multicolor:status/multi_index
/sys/class/leds/multicolor:status/multi_intensity
/sys/class/leds/multicolor:status/max_brightness

Extra features under:
/sys/class/leds/asus::kbd_backlight/mode
/sys/class/leds/asus::kbd_backlight/speed
/sys/class/leds/asus::kbd_backlight/direction

Some questions I have:
1. If there is more than one RGB capable keyboard, what happens to 
/sys/class/leds/multicolor:status ?
2. There is no way to fetch the current values from the keyboard and 
multicolor seems to expect that the current settings can be read - the 
issue here I think is that if the keyboard is changed perhaps by 
Windows dual-boot then these settings won't match. How would this be 
handled?
3. I'm still relatively new to sysfs. Is it possible for sysfs to save 
these values (e.g, multicolour) and re-apply on boot?
4. In the case of the ROG USB connected laptop keyboards, some models 
have per-key control. How would I enable this using the multicolour 
API? Something like:

/sys/class/leds/multicolor:status/multi_a_index
/sys/class/leds/multicolor:status/multi_a_intensity
/sys/class/leds/multicolor:status/multi_left_ctrl_index
/sys/class/leds/multicolor:status/multi_left_ctrl_intensity
/sys/class/leds/multicolor:status/multi_spacebar_index
/sys/class/leds/multicolor:status/multi_spacebar_intensity
/sys/class/leds/multicolor:status/max_brightness (individual or all?)

For these keyboards the USB takes 9-12 packets of 64 bytes, and I've 
indexed the locations of colours in each. To be honest I'd very much 
like to work out how to do this for per-key as it can potentially save 
a lot of time in writing each packet if the kernel is able to batch 
writes internally. A single packet for half a row is ~1ms, but can vary 
up to 5ms..

4. I'm unsure of how to structure the USB-RGB code.. Would I need to 
write an extra driver perhaps?

Kind regards,
Luke.

On Tue, Aug 2 2022 at 14:13:06 +0200, Hans de Goede 
<hdegoede@redhat.com> wrote:
> Hi,
> 
> On 8/2/22 13:09, Hans de Goede wrote:
>>  Hi Luke,
>> 
>>  On 8/2/22 06:59, Luke D. Jones wrote:
>>>  Adds support for TUF laptop RGB control. This creates two sysfs
>>>  paths to add control of basic keyboard LEDs, and power states.
>>> 
>>>  /sys/devices/platform/asus-nb-wmi/tuf_krgb_mode has the following
>>>  as input options via U8 "n n n n n n":
>>>  - Save or set, if set, then settings revert on cold boot
>>>  - Mode, 0-8 for regular modes (if supported), 10-12 for "warning" 
>>> styles
>>>  - Red, 0-255
>>>  - Green, 0-255
>>>  - Blue, 0-255
>>>  - Speed, 0 = Slow, 1 = Medium, 2 = Fast
>>> 
>>>  /sys/devices/platform/asus-nb-wmi/tuf_krgb_state has the following
>>>  as input options via boolean "b b b b b":
>>>  - Save or set, if set, then settings revert on cold boot
>>>  - Boot, if true, the keyboard displays animation on boot
>>>  - Awake, if true, the keyboard LED's are on while device is awake
>>>  - Sleep, if true, the keyboard shows animation while device is 
>>> suspended
>>>  - Keybaord, appears to have no effect
>> 
>>  Typo in Keybaord here.
>> 
>>  Thank you for your patch. I really appreciate your continued
>>  efforts to make Asus laptops work well with Linux.
>> 
>>  For keyboard backlight support Linux has standardized on
>>  using the /sys/class/leds API. So I'm afraid that this patch
>>  will need to be rewritten to use the standard LED API
>>  and then specifically the somewhat new multicolor LED API
>>  at least for setting the RGB values (within the current mode)
>> 
>>  Any extra functionality can then be added as extra sysfs
>>  attributes under the /sys/class/leds/asus::kbd_backlight
>>  device, see e.g. the use of kbd_led_groups in:
>>  drivers/platform/x86/dell/dell-laptop.c
>> 
>>  Note the kbd_backlight part of the name is important this
>>  will allow upower to recognize this as a keyboard backlight
>>  and will then enable desktop-environments which use
>>  upower for kbd backlight control to at least control
>>  the overall brightness of the kbd-backlight.
>> 
>>  I realize that this means that you need to redo a whole
>>  bunch of work here; and I presume also in your
>>  asusctl userspace utility, sorry about that. But it
>>  really is important that standard userspace APIs are
>>  used for things like this where ever possible.
>> 
>>  Regards,
>> 
>>  Hans
> 
> p.s.
> 
> For more info on the multi-color LED API see:
> 
> https://www.kernel.org/doc/html/latest/leds/leds-class-multicolor.html
> https://www.phoronix.com/scan.php?page=news_item&px=Linux-5.9-Multi-Color-LEDs
> 
> 
> 
>> 
>> 
>> 
>> 
>>> 
>>>  Signed-off-by: Luke D. Jones <luke@ljones.dev>
>>>  ---
>>>   drivers/platform/x86/asus-wmi.c            | 168 
>>> +++++++++++++++++++++
>>>   include/linux/platform_data/x86/asus-wmi.h |   6 +
>>>   2 files changed, 174 insertions(+)
>>> 
>>>  diff --git a/drivers/platform/x86/asus-wmi.c 
>>> b/drivers/platform/x86/asus-wmi.c
>>>  index 62ce198a3463..09277bd98249 100644
>>>  --- a/drivers/platform/x86/asus-wmi.c
>>>  +++ b/drivers/platform/x86/asus-wmi.c
>>>  @@ -234,6 +234,9 @@ struct asus_wmi {
>>>   	bool dgpu_disable_available;
>>>   	bool dgpu_disable;
>>> 
>>>  +	bool tuf_kb_rgb_mode_available;
>>>  +	bool tuf_kb_rgb_state_available;
>>>  +
>>>   	bool throttle_thermal_policy_available;
>>>   	u8 throttle_thermal_policy_mode;
>>> 
>>>  @@ -734,6 +737,153 @@ static ssize_t egpu_enable_store(struct 
>>> device *dev,
>>> 
>>>   static DEVICE_ATTR_RW(egpu_enable);
>>> 
>>>  +/* TUF Laptop Keyboard RGB Modes 
>>> **********************************************/
>>>  +static int tuf_kb_rgb_mode_check_present(struct asus_wmi *asus)
>>>  +{
>>>  +	u32 result;
>>>  +	int err;
>>>  +
>>>  +	asus->tuf_kb_rgb_mode_available = false;
>>>  +
>>>  +	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_MODE, 
>>> &result);
>>>  +	if (err) {
>>>  +		if (err == -ENODEV)
>>>  +			return 0;
>>>  +		return err;
>>>  +	}
>>>  +
>>>  +	if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
>>>  +		asus->tuf_kb_rgb_mode_available = true;
>>>  +
>>>  +	return 0;
>>>  +}
>>>  +
>>>  +static ssize_t tuf_kb_rgb_mode_store(struct device *dev,
>>>  +				 struct device_attribute *attr,
>>>  +				 const char *buf, size_t count)
>>>  +{
>>>  +	int err;
>>>  +	u32 ret;
>>>  +	u8 res, tmp, arg_num;
>>>  +	char *data, *part, *end;
>>>  +	u8 cmd, mode, r, g,  b,  speed;
>>>  +
>>>  +	data = end = kstrdup(buf, GFP_KERNEL);
>>>  +	cmd = mode = r = g = b = speed = arg_num = 0;
>>>  +
>>>  +	while ((part = strsep(&end, " ")) != NULL) {
>>>  +		if (part == NULL)
>>>  +			return -1;
>>>  +
>>>  +		res = kstrtou8(part, 10, &tmp);
>>>  +		if (res)
>>>  +			return -1;
>>>  +
>>>  +		if (arg_num == 0)
>>>  +			// apply : set
>>>  +			cmd = tmp == 1 ? 0xb5 : 0xb4;
>>>  +		else if (arg_num == 1)
>>>  +			// From 0-8 are valid modes with 10-12 being "warning"
>>>  +			// style modes. All models have "pulse" mode 10.
>>>  +			mode = (tmp <= 12 && tmp != 9) ? tmp : 10;
>>>  +		else if (arg_num == 2)
>>>  +			r = tmp;
>>>  +		else if (arg_num == 3)
>>>  +			g = tmp;
>>>  +		else if (arg_num == 4)
>>>  +			b = tmp;
>>>  +		else if (arg_num == 5) {
>>>  +			if (tmp == 0)
>>>  +				speed = 0xe1;
>>>  +			else if (tmp == 1)
>>>  +				speed = 0xeb;
>>>  +			else if (tmp == 2)
>>>  +				speed = 0xf5;
>>>  +		}
>>>  +
>>>  +		arg_num += 1;
>>>  +	}
>>>  +
>>>  +	err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS, 
>>> ASUS_WMI_DEVID_TUF_RGB_MODE,
>>>  +			cmd | (mode << 8) | (r << 16) | (g << 24), (b) | (speed << 8), 
>>> &ret);
>>>  +	if (err)
>>>  +		return err;
>>>  +
>>>  +	kfree(data);
>>>  +	return count;
>>>  +}
>>>  +
>>>  +static DEVICE_ATTR_WO(tuf_kb_rgb_mode);
>>>  +
>>>  +/* TUF Laptop Keyboard RGB States 
>>> *********************************************/
>>>  +static int tuf_kb_rgb_state_check_present(struct asus_wmi *asus)
>>>  +{
>>>  +	u32 result;
>>>  +	int err;
>>>  +
>>>  +	asus->tuf_kb_rgb_state_available = false;
>>>  +
>>>  +	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_STATE, 
>>> &result);
>>>  +	if (err) {
>>>  +		if (err == -ENODEV)
>>>  +			return 0;
>>>  +		return err;
>>>  +	}
>>>  +
>>>  +	if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
>>>  +		asus->tuf_kb_rgb_state_available = true;
>>>  +
>>>  +	return 0;
>>>  +}
>>>  +
>>>  +static ssize_t tuf_kb_rgb_state_store(struct device *dev,
>>>  +				   struct device_attribute *attr,
>>>  +				   const char *buf, size_t count)
>>>  +{
>>>  +	int err;
>>>  +	u32 ret;
>>>  +	bool tmp;
>>>  +	char *data, *part, *end;
>>>  +	u8 save, flags, res, arg_num;
>>>  +
>>>  +	save = flags = arg_num = 0;
>>>  +	data = end = kstrdup(buf, GFP_KERNEL);
>>>  +
>>>  +	while ((part = strsep(&end, " ")) != NULL) {
>>>  +		if (part == NULL)
>>>  +			return -1;
>>>  +
>>>  +		res = kstrtobool(part, &tmp);
>>>  +		if (res)
>>>  +			return -1;
>>>  +
>>>  +		if (tmp) {
>>>  +			if (arg_num == 0) // save  :  set
>>>  +				save = tmp == 0 ? 0x0100 : 0x0000;
>>>  +			else if (arg_num == 1)
>>>  +				flags |= 0x02; // boot
>>>  +			else if (arg_num == 2)
>>>  +				flags |= 0x08; // awake
>>>  +			else if (arg_num == 3)
>>>  +				flags |= 0x20; // sleep
>>>  +			else if (arg_num == 4)
>>>  +				flags |= 0x80; // keyboard
>>>  +		}
>>>  +
>>>  +		arg_num += 1;
>>>  +	}
>>>  +
>>>  +	err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS,
>>>  +			ASUS_WMI_DEVID_TUF_RGB_STATE, 0xBD | save | (flags << 16), 0, 
>>> &ret);
>>>  +	if (err)
>>>  +		return err;
>>>  +
>>>  +	kfree(data);
>>>  +	return count;
>>>  +}
>>>  +
>>>  +static DEVICE_ATTR_WO(tuf_kb_rgb_state);
>>>  +
>>>   /* Battery 
>>> ********************************************************************/
>>> 
>>>   /* The battery maximum charging percentage */
>>>  @@ -3258,6 +3408,8 @@ static struct attribute 
>>> *platform_attributes[] = {
>>>   	&dev_attr_touchpad.attr,
>>>   	&dev_attr_egpu_enable.attr,
>>>   	&dev_attr_dgpu_disable.attr,
>>>  +	&dev_attr_tuf_kb_rgb_mode.attr,
>>>  +	&dev_attr_tuf_kb_rgb_state.attr,
>>>   	&dev_attr_lid_resume.attr,
>>>   	&dev_attr_als_enable.attr,
>>>   	&dev_attr_fan_boost_mode.attr,
>>>  @@ -3286,6 +3438,12 @@ static umode_t asus_sysfs_is_visible(struct 
>>> kobject *kobj,
>>>   		devid = ASUS_WMI_DEVID_ALS_ENABLE;
>>>   	else if (attr == &dev_attr_egpu_enable.attr)
>>>   		ok = asus->egpu_enable_available;
>>>  +	else if (attr == &dev_attr_tuf_kb_rgb_mode.attr)
>>>  +		ok = asus->tuf_kb_rgb_mode_available;
>>>  +	else if (attr == &dev_attr_tuf_kb_rgb_state.attr)
>>>  +		ok = asus->tuf_kb_rgb_state_available;
>>>  +	else if (attr == &dev_attr_dgpu_disable.attr)
>>>  +		ok = asus->dgpu_disable_available;
>>>   	else if (attr == &dev_attr_dgpu_disable.attr)
>>>   		ok = asus->dgpu_disable_available;
>>>   	else if (attr == &dev_attr_fan_boost_mode.attr)
>>>  @@ -3557,6 +3715,14 @@ static int asus_wmi_add(struct 
>>> platform_device *pdev)
>>>   	if (err)
>>>   		goto fail_dgpu_disable;
>>> 
>>>  +	err = tuf_kb_rgb_mode_check_present(asus);
>>>  +	if (err)
>>>  +		goto fail_tuf_kb_rgb_mode;
>>>  +
>>>  +	err = tuf_kb_rgb_state_check_present(asus);
>>>  +	if (err)
>>>  +		goto fail_tuf_kb_rgb_state;
>>>  +
>>>   	err = fan_boost_mode_check_present(asus);
>>>   	if (err)
>>>   		goto fail_fan_boost_mode;
>>>  @@ -3671,6 +3837,8 @@ static int asus_wmi_add(struct 
>>> platform_device *pdev)
>>>   fail_fan_boost_mode:
>>>   fail_egpu_enable:
>>>   fail_dgpu_disable:
>>>  +fail_tuf_kb_rgb_mode:
>>>  +fail_tuf_kb_rgb_state:
>>>   fail_platform:
>>>   fail_panel_od:
>>>   	kfree(asus);
>>>  diff --git a/include/linux/platform_data/x86/asus-wmi.h 
>>> b/include/linux/platform_data/x86/asus-wmi.h
>>>  index a571b47ff362..af4191fb0508 100644
>>>  --- a/include/linux/platform_data/x86/asus-wmi.h
>>>  +++ b/include/linux/platform_data/x86/asus-wmi.h
>>>  @@ -98,6 +98,12 @@
>>>   /* dgpu on/off */
>>>   #define ASUS_WMI_DEVID_DGPU		0x00090020
>>> 
>>>  +/* TUF laptop RGB modes/colours */
>>>  +#define ASUS_WMI_DEVID_TUF_RGB_MODE	0x00100056
>>>  +
>>>  +/* TUF laptop RGB power/state */
>>>  +#define ASUS_WMI_DEVID_TUF_RGB_STATE	0x00100057
>>>  +
>>>   /* DSTS masks */
>>>   #define ASUS_WMI_DSTS_STATUS_BIT	0x00000001
>>>   #define ASUS_WMI_DSTS_UNKNOWN_BIT	0x00000002
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] asus-wmi: Add support for TUF laptop keyboard RGB
  2022-08-02 11:09 ` Hans de Goede
@ 2022-08-02 12:13   ` Hans de Goede
  2022-08-02 22:26     ` Luke Jones
  0 siblings, 1 reply; 10+ messages in thread
From: Hans de Goede @ 2022-08-02 12:13 UTC (permalink / raw)
  To: Luke D. Jones
  Cc: corentin.chary, markgross, linux-kernel, platform-driver-x86

Hi,

On 8/2/22 13:09, Hans de Goede wrote:
> Hi Luke,
> 
> On 8/2/22 06:59, Luke D. Jones wrote:
>> Adds support for TUF laptop RGB control. This creates two sysfs
>> paths to add control of basic keyboard LEDs, and power states.
>>
>> /sys/devices/platform/asus-nb-wmi/tuf_krgb_mode has the following
>> as input options via U8 "n n n n n n":
>> - Save or set, if set, then settings revert on cold boot
>> - Mode, 0-8 for regular modes (if supported), 10-12 for "warning" styles
>> - Red, 0-255
>> - Green, 0-255
>> - Blue, 0-255
>> - Speed, 0 = Slow, 1 = Medium, 2 = Fast
>>
>> /sys/devices/platform/asus-nb-wmi/tuf_krgb_state has the following
>> as input options via boolean "b b b b b":
>> - Save or set, if set, then settings revert on cold boot
>> - Boot, if true, the keyboard displays animation on boot
>> - Awake, if true, the keyboard LED's are on while device is awake
>> - Sleep, if true, the keyboard shows animation while device is suspended
>> - Keybaord, appears to have no effect
> 
> Typo in Keybaord here.
> 
> Thank you for your patch. I really appreciate your continued
> efforts to make Asus laptops work well with Linux.
> 
> For keyboard backlight support Linux has standardized on
> using the /sys/class/leds API. So I'm afraid that this patch
> will need to be rewritten to use the standard LED API
> and then specifically the somewhat new multicolor LED API
> at least for setting the RGB values (within the current mode)
> 
> Any extra functionality can then be added as extra sysfs
> attributes under the /sys/class/leds/asus::kbd_backlight
> device, see e.g. the use of kbd_led_groups in:
> drivers/platform/x86/dell/dell-laptop.c
> 
> Note the kbd_backlight part of the name is important this
> will allow upower to recognize this as a keyboard backlight
> and will then enable desktop-environments which use
> upower for kbd backlight control to at least control
> the overall brightness of the kbd-backlight.
> 
> I realize that this means that you need to redo a whole
> bunch of work here; and I presume also in your
> asusctl userspace utility, sorry about that. But it
> really is important that standard userspace APIs are
> used for things like this where ever possible.
> 
> Regards,
> 
> Hans

p.s.

For more info on the multi-color LED API see:

https://www.kernel.org/doc/html/latest/leds/leds-class-multicolor.html
https://www.phoronix.com/scan.php?page=news_item&px=Linux-5.9-Multi-Color-LEDs



> 
> 
> 
> 
>>
>> Signed-off-by: Luke D. Jones <luke@ljones.dev>
>> ---
>>  drivers/platform/x86/asus-wmi.c            | 168 +++++++++++++++++++++
>>  include/linux/platform_data/x86/asus-wmi.h |   6 +
>>  2 files changed, 174 insertions(+)
>>
>> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
>> index 62ce198a3463..09277bd98249 100644
>> --- a/drivers/platform/x86/asus-wmi.c
>> +++ b/drivers/platform/x86/asus-wmi.c
>> @@ -234,6 +234,9 @@ struct asus_wmi {
>>  	bool dgpu_disable_available;
>>  	bool dgpu_disable;
>>  
>> +	bool tuf_kb_rgb_mode_available;
>> +	bool tuf_kb_rgb_state_available;
>> +
>>  	bool throttle_thermal_policy_available;
>>  	u8 throttle_thermal_policy_mode;
>>  
>> @@ -734,6 +737,153 @@ static ssize_t egpu_enable_store(struct device *dev,
>>  
>>  static DEVICE_ATTR_RW(egpu_enable);
>>  
>> +/* TUF Laptop Keyboard RGB Modes **********************************************/
>> +static int tuf_kb_rgb_mode_check_present(struct asus_wmi *asus)
>> +{
>> +	u32 result;
>> +	int err;
>> +
>> +	asus->tuf_kb_rgb_mode_available = false;
>> +
>> +	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_MODE, &result);
>> +	if (err) {
>> +		if (err == -ENODEV)
>> +			return 0;
>> +		return err;
>> +	}
>> +
>> +	if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
>> +		asus->tuf_kb_rgb_mode_available = true;
>> +
>> +	return 0;
>> +}
>> +
>> +static ssize_t tuf_kb_rgb_mode_store(struct device *dev,
>> +				 struct device_attribute *attr,
>> +				 const char *buf, size_t count)
>> +{
>> +	int err;
>> +	u32 ret;
>> +	u8 res, tmp, arg_num;
>> +	char *data, *part, *end;
>> +	u8 cmd, mode, r, g,  b,  speed;
>> +
>> +	data = end = kstrdup(buf, GFP_KERNEL);
>> +	cmd = mode = r = g = b = speed = arg_num = 0;
>> +
>> +	while ((part = strsep(&end, " ")) != NULL) {
>> +		if (part == NULL)
>> +			return -1;
>> +
>> +		res = kstrtou8(part, 10, &tmp);
>> +		if (res)
>> +			return -1;
>> +
>> +		if (arg_num == 0)
>> +			// apply : set
>> +			cmd = tmp == 1 ? 0xb5 : 0xb4;
>> +		else if (arg_num == 1)
>> +			// From 0-8 are valid modes with 10-12 being "warning"
>> +			// style modes. All models have "pulse" mode 10.
>> +			mode = (tmp <= 12 && tmp != 9) ? tmp : 10;
>> +		else if (arg_num == 2)
>> +			r = tmp;
>> +		else if (arg_num == 3)
>> +			g = tmp;
>> +		else if (arg_num == 4)
>> +			b = tmp;
>> +		else if (arg_num == 5) {
>> +			if (tmp == 0)
>> +				speed = 0xe1;
>> +			else if (tmp == 1)
>> +				speed = 0xeb;
>> +			else if (tmp == 2)
>> +				speed = 0xf5;
>> +		}
>> +
>> +		arg_num += 1;
>> +	}
>> +
>> +	err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS, ASUS_WMI_DEVID_TUF_RGB_MODE,
>> +			cmd | (mode << 8) | (r << 16) | (g << 24), (b) | (speed << 8), &ret);
>> +	if (err)
>> +		return err;
>> +
>> +	kfree(data);
>> +	return count;
>> +}
>> +
>> +static DEVICE_ATTR_WO(tuf_kb_rgb_mode);
>> +
>> +/* TUF Laptop Keyboard RGB States *********************************************/
>> +static int tuf_kb_rgb_state_check_present(struct asus_wmi *asus)
>> +{
>> +	u32 result;
>> +	int err;
>> +
>> +	asus->tuf_kb_rgb_state_available = false;
>> +
>> +	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_STATE, &result);
>> +	if (err) {
>> +		if (err == -ENODEV)
>> +			return 0;
>> +		return err;
>> +	}
>> +
>> +	if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
>> +		asus->tuf_kb_rgb_state_available = true;
>> +
>> +	return 0;
>> +}
>> +
>> +static ssize_t tuf_kb_rgb_state_store(struct device *dev,
>> +				   struct device_attribute *attr,
>> +				   const char *buf, size_t count)
>> +{
>> +	int err;
>> +	u32 ret;
>> +	bool tmp;
>> +	char *data, *part, *end;
>> +	u8 save, flags, res, arg_num;
>> +
>> +	save = flags = arg_num = 0;
>> +	data = end = kstrdup(buf, GFP_KERNEL);
>> +
>> +	while ((part = strsep(&end, " ")) != NULL) {
>> +		if (part == NULL)
>> +			return -1;
>> +
>> +		res = kstrtobool(part, &tmp);
>> +		if (res)
>> +			return -1;
>> +
>> +		if (tmp) {
>> +			if (arg_num == 0) // save  :  set
>> +				save = tmp == 0 ? 0x0100 : 0x0000;
>> +			else if (arg_num == 1)
>> +				flags |= 0x02; // boot
>> +			else if (arg_num == 2)
>> +				flags |= 0x08; // awake
>> +			else if (arg_num == 3)
>> +				flags |= 0x20; // sleep
>> +			else if (arg_num == 4)
>> +				flags |= 0x80; // keyboard
>> +		}
>> +
>> +		arg_num += 1;
>> +	}
>> +
>> +	err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS,
>> +			ASUS_WMI_DEVID_TUF_RGB_STATE, 0xBD | save | (flags << 16), 0, &ret);
>> +	if (err)
>> +		return err;
>> +
>> +	kfree(data);
>> +	return count;
>> +}
>> +
>> +static DEVICE_ATTR_WO(tuf_kb_rgb_state);
>> +
>>  /* Battery ********************************************************************/
>>  
>>  /* The battery maximum charging percentage */
>> @@ -3258,6 +3408,8 @@ static struct attribute *platform_attributes[] = {
>>  	&dev_attr_touchpad.attr,
>>  	&dev_attr_egpu_enable.attr,
>>  	&dev_attr_dgpu_disable.attr,
>> +	&dev_attr_tuf_kb_rgb_mode.attr,
>> +	&dev_attr_tuf_kb_rgb_state.attr,
>>  	&dev_attr_lid_resume.attr,
>>  	&dev_attr_als_enable.attr,
>>  	&dev_attr_fan_boost_mode.attr,
>> @@ -3286,6 +3438,12 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
>>  		devid = ASUS_WMI_DEVID_ALS_ENABLE;
>>  	else if (attr == &dev_attr_egpu_enable.attr)
>>  		ok = asus->egpu_enable_available;
>> +	else if (attr == &dev_attr_tuf_kb_rgb_mode.attr)
>> +		ok = asus->tuf_kb_rgb_mode_available;
>> +	else if (attr == &dev_attr_tuf_kb_rgb_state.attr)
>> +		ok = asus->tuf_kb_rgb_state_available;
>> +	else if (attr == &dev_attr_dgpu_disable.attr)
>> +		ok = asus->dgpu_disable_available;
>>  	else if (attr == &dev_attr_dgpu_disable.attr)
>>  		ok = asus->dgpu_disable_available;
>>  	else if (attr == &dev_attr_fan_boost_mode.attr)
>> @@ -3557,6 +3715,14 @@ static int asus_wmi_add(struct platform_device *pdev)
>>  	if (err)
>>  		goto fail_dgpu_disable;
>>  
>> +	err = tuf_kb_rgb_mode_check_present(asus);
>> +	if (err)
>> +		goto fail_tuf_kb_rgb_mode;
>> +
>> +	err = tuf_kb_rgb_state_check_present(asus);
>> +	if (err)
>> +		goto fail_tuf_kb_rgb_state;
>> +
>>  	err = fan_boost_mode_check_present(asus);
>>  	if (err)
>>  		goto fail_fan_boost_mode;
>> @@ -3671,6 +3837,8 @@ static int asus_wmi_add(struct platform_device *pdev)
>>  fail_fan_boost_mode:
>>  fail_egpu_enable:
>>  fail_dgpu_disable:
>> +fail_tuf_kb_rgb_mode:
>> +fail_tuf_kb_rgb_state:
>>  fail_platform:
>>  fail_panel_od:
>>  	kfree(asus);
>> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
>> index a571b47ff362..af4191fb0508 100644
>> --- a/include/linux/platform_data/x86/asus-wmi.h
>> +++ b/include/linux/platform_data/x86/asus-wmi.h
>> @@ -98,6 +98,12 @@
>>  /* dgpu on/off */
>>  #define ASUS_WMI_DEVID_DGPU		0x00090020
>>  
>> +/* TUF laptop RGB modes/colours */
>> +#define ASUS_WMI_DEVID_TUF_RGB_MODE	0x00100056
>> +
>> +/* TUF laptop RGB power/state */
>> +#define ASUS_WMI_DEVID_TUF_RGB_STATE	0x00100057
>> +
>>  /* DSTS masks */
>>  #define ASUS_WMI_DSTS_STATUS_BIT	0x00000001
>>  #define ASUS_WMI_DSTS_UNKNOWN_BIT	0x00000002


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] asus-wmi: Add support for TUF laptop keyboard RGB
  2022-08-02  4:59 Luke D. Jones
  2022-08-02  5:43 ` Luke Jones
@ 2022-08-02 11:09 ` Hans de Goede
  2022-08-02 12:13   ` Hans de Goede
  1 sibling, 1 reply; 10+ messages in thread
From: Hans de Goede @ 2022-08-02 11:09 UTC (permalink / raw)
  To: Luke D. Jones
  Cc: corentin.chary, markgross, linux-kernel, platform-driver-x86

Hi Luke,

On 8/2/22 06:59, Luke D. Jones wrote:
> Adds support for TUF laptop RGB control. This creates two sysfs
> paths to add control of basic keyboard LEDs, and power states.
> 
> /sys/devices/platform/asus-nb-wmi/tuf_krgb_mode has the following
> as input options via U8 "n n n n n n":
> - Save or set, if set, then settings revert on cold boot
> - Mode, 0-8 for regular modes (if supported), 10-12 for "warning" styles
> - Red, 0-255
> - Green, 0-255
> - Blue, 0-255
> - Speed, 0 = Slow, 1 = Medium, 2 = Fast
> 
> /sys/devices/platform/asus-nb-wmi/tuf_krgb_state has the following
> as input options via boolean "b b b b b":
> - Save or set, if set, then settings revert on cold boot
> - Boot, if true, the keyboard displays animation on boot
> - Awake, if true, the keyboard LED's are on while device is awake
> - Sleep, if true, the keyboard shows animation while device is suspended
> - Keybaord, appears to have no effect

Typo in Keybaord here.

Thank you for your patch. I really appreciate your continued
efforts to make Asus laptops work well with Linux.

For keyboard backlight support Linux has standardized on
using the /sys/class/leds API. So I'm afraid that this patch
will need to be rewritten to use the standard LED API
and then specifically the somewhat new multicolor LED API
at least for setting the RGB values (within the current mode)

Any extra functionality can then be added as extra sysfs
attributes under the /sys/class/leds/asus::kbd_backlight
device, see e.g. the use of kbd_led_groups in:
drivers/platform/x86/dell/dell-laptop.c

Note the kbd_backlight part of the name is important this
will allow upower to recognize this as a keyboard backlight
and will then enable desktop-environments which use
upower for kbd backlight control to at least control
the overall brightness of the kbd-backlight.

I realize that this means that you need to redo a whole
bunch of work here; and I presume also in your
asusctl userspace utility, sorry about that. But it
really is important that standard userspace APIs are
used for things like this where ever possible.

Regards,

Hans




> 
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> ---
>  drivers/platform/x86/asus-wmi.c            | 168 +++++++++++++++++++++
>  include/linux/platform_data/x86/asus-wmi.h |   6 +
>  2 files changed, 174 insertions(+)
> 
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 62ce198a3463..09277bd98249 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -234,6 +234,9 @@ struct asus_wmi {
>  	bool dgpu_disable_available;
>  	bool dgpu_disable;
>  
> +	bool tuf_kb_rgb_mode_available;
> +	bool tuf_kb_rgb_state_available;
> +
>  	bool throttle_thermal_policy_available;
>  	u8 throttle_thermal_policy_mode;
>  
> @@ -734,6 +737,153 @@ static ssize_t egpu_enable_store(struct device *dev,
>  
>  static DEVICE_ATTR_RW(egpu_enable);
>  
> +/* TUF Laptop Keyboard RGB Modes **********************************************/
> +static int tuf_kb_rgb_mode_check_present(struct asus_wmi *asus)
> +{
> +	u32 result;
> +	int err;
> +
> +	asus->tuf_kb_rgb_mode_available = false;
> +
> +	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_MODE, &result);
> +	if (err) {
> +		if (err == -ENODEV)
> +			return 0;
> +		return err;
> +	}
> +
> +	if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
> +		asus->tuf_kb_rgb_mode_available = true;
> +
> +	return 0;
> +}
> +
> +static ssize_t tuf_kb_rgb_mode_store(struct device *dev,
> +				 struct device_attribute *attr,
> +				 const char *buf, size_t count)
> +{
> +	int err;
> +	u32 ret;
> +	u8 res, tmp, arg_num;
> +	char *data, *part, *end;
> +	u8 cmd, mode, r, g,  b,  speed;
> +
> +	data = end = kstrdup(buf, GFP_KERNEL);
> +	cmd = mode = r = g = b = speed = arg_num = 0;
> +
> +	while ((part = strsep(&end, " ")) != NULL) {
> +		if (part == NULL)
> +			return -1;
> +
> +		res = kstrtou8(part, 10, &tmp);
> +		if (res)
> +			return -1;
> +
> +		if (arg_num == 0)
> +			// apply : set
> +			cmd = tmp == 1 ? 0xb5 : 0xb4;
> +		else if (arg_num == 1)
> +			// From 0-8 are valid modes with 10-12 being "warning"
> +			// style modes. All models have "pulse" mode 10.
> +			mode = (tmp <= 12 && tmp != 9) ? tmp : 10;
> +		else if (arg_num == 2)
> +			r = tmp;
> +		else if (arg_num == 3)
> +			g = tmp;
> +		else if (arg_num == 4)
> +			b = tmp;
> +		else if (arg_num == 5) {
> +			if (tmp == 0)
> +				speed = 0xe1;
> +			else if (tmp == 1)
> +				speed = 0xeb;
> +			else if (tmp == 2)
> +				speed = 0xf5;
> +		}
> +
> +		arg_num += 1;
> +	}
> +
> +	err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS, ASUS_WMI_DEVID_TUF_RGB_MODE,
> +			cmd | (mode << 8) | (r << 16) | (g << 24), (b) | (speed << 8), &ret);
> +	if (err)
> +		return err;
> +
> +	kfree(data);
> +	return count;
> +}
> +
> +static DEVICE_ATTR_WO(tuf_kb_rgb_mode);
> +
> +/* TUF Laptop Keyboard RGB States *********************************************/
> +static int tuf_kb_rgb_state_check_present(struct asus_wmi *asus)
> +{
> +	u32 result;
> +	int err;
> +
> +	asus->tuf_kb_rgb_state_available = false;
> +
> +	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_STATE, &result);
> +	if (err) {
> +		if (err == -ENODEV)
> +			return 0;
> +		return err;
> +	}
> +
> +	if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
> +		asus->tuf_kb_rgb_state_available = true;
> +
> +	return 0;
> +}
> +
> +static ssize_t tuf_kb_rgb_state_store(struct device *dev,
> +				   struct device_attribute *attr,
> +				   const char *buf, size_t count)
> +{
> +	int err;
> +	u32 ret;
> +	bool tmp;
> +	char *data, *part, *end;
> +	u8 save, flags, res, arg_num;
> +
> +	save = flags = arg_num = 0;
> +	data = end = kstrdup(buf, GFP_KERNEL);
> +
> +	while ((part = strsep(&end, " ")) != NULL) {
> +		if (part == NULL)
> +			return -1;
> +
> +		res = kstrtobool(part, &tmp);
> +		if (res)
> +			return -1;
> +
> +		if (tmp) {
> +			if (arg_num == 0) // save  :  set
> +				save = tmp == 0 ? 0x0100 : 0x0000;
> +			else if (arg_num == 1)
> +				flags |= 0x02; // boot
> +			else if (arg_num == 2)
> +				flags |= 0x08; // awake
> +			else if (arg_num == 3)
> +				flags |= 0x20; // sleep
> +			else if (arg_num == 4)
> +				flags |= 0x80; // keyboard
> +		}
> +
> +		arg_num += 1;
> +	}
> +
> +	err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS,
> +			ASUS_WMI_DEVID_TUF_RGB_STATE, 0xBD | save | (flags << 16), 0, &ret);
> +	if (err)
> +		return err;
> +
> +	kfree(data);
> +	return count;
> +}
> +
> +static DEVICE_ATTR_WO(tuf_kb_rgb_state);
> +
>  /* Battery ********************************************************************/
>  
>  /* The battery maximum charging percentage */
> @@ -3258,6 +3408,8 @@ static struct attribute *platform_attributes[] = {
>  	&dev_attr_touchpad.attr,
>  	&dev_attr_egpu_enable.attr,
>  	&dev_attr_dgpu_disable.attr,
> +	&dev_attr_tuf_kb_rgb_mode.attr,
> +	&dev_attr_tuf_kb_rgb_state.attr,
>  	&dev_attr_lid_resume.attr,
>  	&dev_attr_als_enable.attr,
>  	&dev_attr_fan_boost_mode.attr,
> @@ -3286,6 +3438,12 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
>  		devid = ASUS_WMI_DEVID_ALS_ENABLE;
>  	else if (attr == &dev_attr_egpu_enable.attr)
>  		ok = asus->egpu_enable_available;
> +	else if (attr == &dev_attr_tuf_kb_rgb_mode.attr)
> +		ok = asus->tuf_kb_rgb_mode_available;
> +	else if (attr == &dev_attr_tuf_kb_rgb_state.attr)
> +		ok = asus->tuf_kb_rgb_state_available;
> +	else if (attr == &dev_attr_dgpu_disable.attr)
> +		ok = asus->dgpu_disable_available;
>  	else if (attr == &dev_attr_dgpu_disable.attr)
>  		ok = asus->dgpu_disable_available;
>  	else if (attr == &dev_attr_fan_boost_mode.attr)
> @@ -3557,6 +3715,14 @@ static int asus_wmi_add(struct platform_device *pdev)
>  	if (err)
>  		goto fail_dgpu_disable;
>  
> +	err = tuf_kb_rgb_mode_check_present(asus);
> +	if (err)
> +		goto fail_tuf_kb_rgb_mode;
> +
> +	err = tuf_kb_rgb_state_check_present(asus);
> +	if (err)
> +		goto fail_tuf_kb_rgb_state;
> +
>  	err = fan_boost_mode_check_present(asus);
>  	if (err)
>  		goto fail_fan_boost_mode;
> @@ -3671,6 +3837,8 @@ static int asus_wmi_add(struct platform_device *pdev)
>  fail_fan_boost_mode:
>  fail_egpu_enable:
>  fail_dgpu_disable:
> +fail_tuf_kb_rgb_mode:
> +fail_tuf_kb_rgb_state:
>  fail_platform:
>  fail_panel_od:
>  	kfree(asus);
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index a571b47ff362..af4191fb0508 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -98,6 +98,12 @@
>  /* dgpu on/off */
>  #define ASUS_WMI_DEVID_DGPU		0x00090020
>  
> +/* TUF laptop RGB modes/colours */
> +#define ASUS_WMI_DEVID_TUF_RGB_MODE	0x00100056
> +
> +/* TUF laptop RGB power/state */
> +#define ASUS_WMI_DEVID_TUF_RGB_STATE	0x00100057
> +
>  /* DSTS masks */
>  #define ASUS_WMI_DSTS_STATUS_BIT	0x00000001
>  #define ASUS_WMI_DSTS_UNKNOWN_BIT	0x00000002


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] asus-wmi: Add support for TUF laptop keyboard RGB
  2022-08-02  4:59 Luke D. Jones
@ 2022-08-02  5:43 ` Luke Jones
  2022-08-02 11:09 ` Hans de Goede
  1 sibling, 0 replies; 10+ messages in thread
From: Luke Jones @ 2022-08-02  5:43 UTC (permalink / raw)
  To: hdegoede; +Cc: corentin.chary, markgross, linux-kernel, platform-driver-x86

My apologies, I didn't mean to send two emails, I thought the first 
send was cancelled.

This submission is the canonical one.

Regards, Luke.

On Tue, Aug 2 2022 at 16:59:42 +1200, Luke D. Jones <luke@ljones.dev> 
wrote:
> Adds support for TUF laptop RGB control. This creates two sysfs
> paths to add control of basic keyboard LEDs, and power states.
> 
> /sys/devices/platform/asus-nb-wmi/tuf_krgb_mode has the following
> as input options via U8 "n n n n n n":
> - Save or set, if set, then settings revert on cold boot
> - Mode, 0-8 for regular modes (if supported), 10-12 for "warning" 
> styles
> - Red, 0-255
> - Green, 0-255
> - Blue, 0-255
> - Speed, 0 = Slow, 1 = Medium, 2 = Fast
> 
> /sys/devices/platform/asus-nb-wmi/tuf_krgb_state has the following
> as input options via boolean "b b b b b":
> - Save or set, if set, then settings revert on cold boot
> - Boot, if true, the keyboard displays animation on boot
> - Awake, if true, the keyboard LED's are on while device is awake
> - Sleep, if true, the keyboard shows animation while device is 
> suspended
> - Keybaord, appears to have no effect
> 
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> ---
>  drivers/platform/x86/asus-wmi.c            | 168 
> +++++++++++++++++++++
>  include/linux/platform_data/x86/asus-wmi.h |   6 +
>  2 files changed, 174 insertions(+)
> 
> diff --git a/drivers/platform/x86/asus-wmi.c 
> b/drivers/platform/x86/asus-wmi.c
> index 62ce198a3463..09277bd98249 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -234,6 +234,9 @@ struct asus_wmi {
>  	bool dgpu_disable_available;
>  	bool dgpu_disable;
> 
> +	bool tuf_kb_rgb_mode_available;
> +	bool tuf_kb_rgb_state_available;
> +
>  	bool throttle_thermal_policy_available;
>  	u8 throttle_thermal_policy_mode;
> 
> @@ -734,6 +737,153 @@ static ssize_t egpu_enable_store(struct device 
> *dev,
> 
>  static DEVICE_ATTR_RW(egpu_enable);
> 
> +/* TUF Laptop Keyboard RGB Modes 
> **********************************************/
> +static int tuf_kb_rgb_mode_check_present(struct asus_wmi *asus)
> +{
> +	u32 result;
> +	int err;
> +
> +	asus->tuf_kb_rgb_mode_available = false;
> +
> +	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_MODE, 
> &result);
> +	if (err) {
> +		if (err == -ENODEV)
> +			return 0;
> +		return err;
> +	}
> +
> +	if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
> +		asus->tuf_kb_rgb_mode_available = true;
> +
> +	return 0;
> +}
> +
> +static ssize_t tuf_kb_rgb_mode_store(struct device *dev,
> +				 struct device_attribute *attr,
> +				 const char *buf, size_t count)
> +{
> +	int err;
> +	u32 ret;
> +	u8 res, tmp, arg_num;
> +	char *data, *part, *end;
> +	u8 cmd, mode, r, g,  b,  speed;
> +
> +	data = end = kstrdup(buf, GFP_KERNEL);
> +	cmd = mode = r = g = b = speed = arg_num = 0;
> +
> +	while ((part = strsep(&end, " ")) != NULL) {
> +		if (part == NULL)
> +			return -1;
> +
> +		res = kstrtou8(part, 10, &tmp);
> +		if (res)
> +			return -1;
> +
> +		if (arg_num == 0)
> +			// apply : set
> +			cmd = tmp == 1 ? 0xb5 : 0xb4;
> +		else if (arg_num == 1)
> +			// From 0-8 are valid modes with 10-12 being "warning"
> +			// style modes. All models have "pulse" mode 10.
> +			mode = (tmp <= 12 && tmp != 9) ? tmp : 10;
> +		else if (arg_num == 2)
> +			r = tmp;
> +		else if (arg_num == 3)
> +			g = tmp;
> +		else if (arg_num == 4)
> +			b = tmp;
> +		else if (arg_num == 5) {
> +			if (tmp == 0)
> +				speed = 0xe1;
> +			else if (tmp == 1)
> +				speed = 0xeb;
> +			else if (tmp == 2)
> +				speed = 0xf5;
> +		}
> +
> +		arg_num += 1;
> +	}
> +
> +	err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS, 
> ASUS_WMI_DEVID_TUF_RGB_MODE,
> +			cmd | (mode << 8) | (r << 16) | (g << 24), (b) | (speed << 8), 
> &ret);
> +	if (err)
> +		return err;
> +
> +	kfree(data);
> +	return count;
> +}
> +
> +static DEVICE_ATTR_WO(tuf_kb_rgb_mode);
> +
> +/* TUF Laptop Keyboard RGB States 
> *********************************************/
> +static int tuf_kb_rgb_state_check_present(struct asus_wmi *asus)
> +{
> +	u32 result;
> +	int err;
> +
> +	asus->tuf_kb_rgb_state_available = false;
> +
> +	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_STATE, 
> &result);
> +	if (err) {
> +		if (err == -ENODEV)
> +			return 0;
> +		return err;
> +	}
> +
> +	if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
> +		asus->tuf_kb_rgb_state_available = true;
> +
> +	return 0;
> +}
> +
> +static ssize_t tuf_kb_rgb_state_store(struct device *dev,
> +				   struct device_attribute *attr,
> +				   const char *buf, size_t count)
> +{
> +	int err;
> +	u32 ret;
> +	bool tmp;
> +	char *data, *part, *end;
> +	u8 save, flags, res, arg_num;
> +
> +	save = flags = arg_num = 0;
> +	data = end = kstrdup(buf, GFP_KERNEL);
> +
> +	while ((part = strsep(&end, " ")) != NULL) {
> +		if (part == NULL)
> +			return -1;
> +
> +		res = kstrtobool(part, &tmp);
> +		if (res)
> +			return -1;
> +
> +		if (tmp) {
> +			if (arg_num == 0) // save  :  set
> +				save = tmp == 0 ? 0x0100 : 0x0000;
> +			else if (arg_num == 1)
> +				flags |= 0x02; // boot
> +			else if (arg_num == 2)
> +				flags |= 0x08; // awake
> +			else if (arg_num == 3)
> +				flags |= 0x20; // sleep
> +			else if (arg_num == 4)
> +				flags |= 0x80; // keyboard
> +		}
> +
> +		arg_num += 1;
> +	}
> +
> +	err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS,
> +			ASUS_WMI_DEVID_TUF_RGB_STATE, 0xBD | save | (flags << 16), 0, 
> &ret);
> +	if (err)
> +		return err;
> +
> +	kfree(data);
> +	return count;
> +}
> +
> +static DEVICE_ATTR_WO(tuf_kb_rgb_state);
> +
>  /* Battery 
> ********************************************************************/
> 
>  /* The battery maximum charging percentage */
> @@ -3258,6 +3408,8 @@ static struct attribute *platform_attributes[] 
> = {
>  	&dev_attr_touchpad.attr,
>  	&dev_attr_egpu_enable.attr,
>  	&dev_attr_dgpu_disable.attr,
> +	&dev_attr_tuf_kb_rgb_mode.attr,
> +	&dev_attr_tuf_kb_rgb_state.attr,
>  	&dev_attr_lid_resume.attr,
>  	&dev_attr_als_enable.attr,
>  	&dev_attr_fan_boost_mode.attr,
> @@ -3286,6 +3438,12 @@ static umode_t asus_sysfs_is_visible(struct 
> kobject *kobj,
>  		devid = ASUS_WMI_DEVID_ALS_ENABLE;
>  	else if (attr == &dev_attr_egpu_enable.attr)
>  		ok = asus->egpu_enable_available;
> +	else if (attr == &dev_attr_tuf_kb_rgb_mode.attr)
> +		ok = asus->tuf_kb_rgb_mode_available;
> +	else if (attr == &dev_attr_tuf_kb_rgb_state.attr)
> +		ok = asus->tuf_kb_rgb_state_available;
> +	else if (attr == &dev_attr_dgpu_disable.attr)
> +		ok = asus->dgpu_disable_available;
>  	else if (attr == &dev_attr_dgpu_disable.attr)
>  		ok = asus->dgpu_disable_available;
>  	else if (attr == &dev_attr_fan_boost_mode.attr)
> @@ -3557,6 +3715,14 @@ static int asus_wmi_add(struct platform_device 
> *pdev)
>  	if (err)
>  		goto fail_dgpu_disable;
> 
> +	err = tuf_kb_rgb_mode_check_present(asus);
> +	if (err)
> +		goto fail_tuf_kb_rgb_mode;
> +
> +	err = tuf_kb_rgb_state_check_present(asus);
> +	if (err)
> +		goto fail_tuf_kb_rgb_state;
> +
>  	err = fan_boost_mode_check_present(asus);
>  	if (err)
>  		goto fail_fan_boost_mode;
> @@ -3671,6 +3837,8 @@ static int asus_wmi_add(struct platform_device 
> *pdev)
>  fail_fan_boost_mode:
>  fail_egpu_enable:
>  fail_dgpu_disable:
> +fail_tuf_kb_rgb_mode:
> +fail_tuf_kb_rgb_state:
>  fail_platform:
>  fail_panel_od:
>  	kfree(asus);
> diff --git a/include/linux/platform_data/x86/asus-wmi.h 
> b/include/linux/platform_data/x86/asus-wmi.h
> index a571b47ff362..af4191fb0508 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -98,6 +98,12 @@
>  /* dgpu on/off */
>  #define ASUS_WMI_DEVID_DGPU		0x00090020
> 
> +/* TUF laptop RGB modes/colours */
> +#define ASUS_WMI_DEVID_TUF_RGB_MODE	0x00100056
> +
> +/* TUF laptop RGB power/state */
> +#define ASUS_WMI_DEVID_TUF_RGB_STATE	0x00100057
> +
>  /* DSTS masks */
>  #define ASUS_WMI_DSTS_STATUS_BIT	0x00000001
>  #define ASUS_WMI_DSTS_UNKNOWN_BIT	0x00000002
> --
> 2.37.1
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] asus-wmi: Add support for TUF laptop keyboard RGB
@ 2022-08-02  4:59 Luke D. Jones
  2022-08-02  5:43 ` Luke Jones
  2022-08-02 11:09 ` Hans de Goede
  0 siblings, 2 replies; 10+ messages in thread
From: Luke D. Jones @ 2022-08-02  4:59 UTC (permalink / raw)
  To: hdegoede
  Cc: corentin.chary, markgross, linux-kernel, platform-driver-x86,
	Luke D. Jones

Adds support for TUF laptop RGB control. This creates two sysfs
paths to add control of basic keyboard LEDs, and power states.

/sys/devices/platform/asus-nb-wmi/tuf_krgb_mode has the following
as input options via U8 "n n n n n n":
- Save or set, if set, then settings revert on cold boot
- Mode, 0-8 for regular modes (if supported), 10-12 for "warning" styles
- Red, 0-255
- Green, 0-255
- Blue, 0-255
- Speed, 0 = Slow, 1 = Medium, 2 = Fast

/sys/devices/platform/asus-nb-wmi/tuf_krgb_state has the following
as input options via boolean "b b b b b":
- Save or set, if set, then settings revert on cold boot
- Boot, if true, the keyboard displays animation on boot
- Awake, if true, the keyboard LED's are on while device is awake
- Sleep, if true, the keyboard shows animation while device is suspended
- Keybaord, appears to have no effect

Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
 drivers/platform/x86/asus-wmi.c            | 168 +++++++++++++++++++++
 include/linux/platform_data/x86/asus-wmi.h |   6 +
 2 files changed, 174 insertions(+)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 62ce198a3463..09277bd98249 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -234,6 +234,9 @@ struct asus_wmi {
 	bool dgpu_disable_available;
 	bool dgpu_disable;
 
+	bool tuf_kb_rgb_mode_available;
+	bool tuf_kb_rgb_state_available;
+
 	bool throttle_thermal_policy_available;
 	u8 throttle_thermal_policy_mode;
 
@@ -734,6 +737,153 @@ static ssize_t egpu_enable_store(struct device *dev,
 
 static DEVICE_ATTR_RW(egpu_enable);
 
+/* TUF Laptop Keyboard RGB Modes **********************************************/
+static int tuf_kb_rgb_mode_check_present(struct asus_wmi *asus)
+{
+	u32 result;
+	int err;
+
+	asus->tuf_kb_rgb_mode_available = false;
+
+	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_MODE, &result);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		return err;
+	}
+
+	if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
+		asus->tuf_kb_rgb_mode_available = true;
+
+	return 0;
+}
+
+static ssize_t tuf_kb_rgb_mode_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	int err;
+	u32 ret;
+	u8 res, tmp, arg_num;
+	char *data, *part, *end;
+	u8 cmd, mode, r, g,  b,  speed;
+
+	data = end = kstrdup(buf, GFP_KERNEL);
+	cmd = mode = r = g = b = speed = arg_num = 0;
+
+	while ((part = strsep(&end, " ")) != NULL) {
+		if (part == NULL)
+			return -1;
+
+		res = kstrtou8(part, 10, &tmp);
+		if (res)
+			return -1;
+
+		if (arg_num == 0)
+			// apply : set
+			cmd = tmp == 1 ? 0xb5 : 0xb4;
+		else if (arg_num == 1)
+			// From 0-8 are valid modes with 10-12 being "warning"
+			// style modes. All models have "pulse" mode 10.
+			mode = (tmp <= 12 && tmp != 9) ? tmp : 10;
+		else if (arg_num == 2)
+			r = tmp;
+		else if (arg_num == 3)
+			g = tmp;
+		else if (arg_num == 4)
+			b = tmp;
+		else if (arg_num == 5) {
+			if (tmp == 0)
+				speed = 0xe1;
+			else if (tmp == 1)
+				speed = 0xeb;
+			else if (tmp == 2)
+				speed = 0xf5;
+		}
+
+		arg_num += 1;
+	}
+
+	err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS, ASUS_WMI_DEVID_TUF_RGB_MODE,
+			cmd | (mode << 8) | (r << 16) | (g << 24), (b) | (speed << 8), &ret);
+	if (err)
+		return err;
+
+	kfree(data);
+	return count;
+}
+
+static DEVICE_ATTR_WO(tuf_kb_rgb_mode);
+
+/* TUF Laptop Keyboard RGB States *********************************************/
+static int tuf_kb_rgb_state_check_present(struct asus_wmi *asus)
+{
+	u32 result;
+	int err;
+
+	asus->tuf_kb_rgb_state_available = false;
+
+	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_STATE, &result);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		return err;
+	}
+
+	if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
+		asus->tuf_kb_rgb_state_available = true;
+
+	return 0;
+}
+
+static ssize_t tuf_kb_rgb_state_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	int err;
+	u32 ret;
+	bool tmp;
+	char *data, *part, *end;
+	u8 save, flags, res, arg_num;
+
+	save = flags = arg_num = 0;
+	data = end = kstrdup(buf, GFP_KERNEL);
+
+	while ((part = strsep(&end, " ")) != NULL) {
+		if (part == NULL)
+			return -1;
+
+		res = kstrtobool(part, &tmp);
+		if (res)
+			return -1;
+
+		if (tmp) {
+			if (arg_num == 0) // save  :  set
+				save = tmp == 0 ? 0x0100 : 0x0000;
+			else if (arg_num == 1)
+				flags |= 0x02; // boot
+			else if (arg_num == 2)
+				flags |= 0x08; // awake
+			else if (arg_num == 3)
+				flags |= 0x20; // sleep
+			else if (arg_num == 4)
+				flags |= 0x80; // keyboard
+		}
+
+		arg_num += 1;
+	}
+
+	err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS,
+			ASUS_WMI_DEVID_TUF_RGB_STATE, 0xBD | save | (flags << 16), 0, &ret);
+	if (err)
+		return err;
+
+	kfree(data);
+	return count;
+}
+
+static DEVICE_ATTR_WO(tuf_kb_rgb_state);
+
 /* Battery ********************************************************************/
 
 /* The battery maximum charging percentage */
@@ -3258,6 +3408,8 @@ static struct attribute *platform_attributes[] = {
 	&dev_attr_touchpad.attr,
 	&dev_attr_egpu_enable.attr,
 	&dev_attr_dgpu_disable.attr,
+	&dev_attr_tuf_kb_rgb_mode.attr,
+	&dev_attr_tuf_kb_rgb_state.attr,
 	&dev_attr_lid_resume.attr,
 	&dev_attr_als_enable.attr,
 	&dev_attr_fan_boost_mode.attr,
@@ -3286,6 +3438,12 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
 		devid = ASUS_WMI_DEVID_ALS_ENABLE;
 	else if (attr == &dev_attr_egpu_enable.attr)
 		ok = asus->egpu_enable_available;
+	else if (attr == &dev_attr_tuf_kb_rgb_mode.attr)
+		ok = asus->tuf_kb_rgb_mode_available;
+	else if (attr == &dev_attr_tuf_kb_rgb_state.attr)
+		ok = asus->tuf_kb_rgb_state_available;
+	else if (attr == &dev_attr_dgpu_disable.attr)
+		ok = asus->dgpu_disable_available;
 	else if (attr == &dev_attr_dgpu_disable.attr)
 		ok = asus->dgpu_disable_available;
 	else if (attr == &dev_attr_fan_boost_mode.attr)
@@ -3557,6 +3715,14 @@ static int asus_wmi_add(struct platform_device *pdev)
 	if (err)
 		goto fail_dgpu_disable;
 
+	err = tuf_kb_rgb_mode_check_present(asus);
+	if (err)
+		goto fail_tuf_kb_rgb_mode;
+
+	err = tuf_kb_rgb_state_check_present(asus);
+	if (err)
+		goto fail_tuf_kb_rgb_state;
+
 	err = fan_boost_mode_check_present(asus);
 	if (err)
 		goto fail_fan_boost_mode;
@@ -3671,6 +3837,8 @@ static int asus_wmi_add(struct platform_device *pdev)
 fail_fan_boost_mode:
 fail_egpu_enable:
 fail_dgpu_disable:
+fail_tuf_kb_rgb_mode:
+fail_tuf_kb_rgb_state:
 fail_platform:
 fail_panel_od:
 	kfree(asus);
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index a571b47ff362..af4191fb0508 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -98,6 +98,12 @@
 /* dgpu on/off */
 #define ASUS_WMI_DEVID_DGPU		0x00090020
 
+/* TUF laptop RGB modes/colours */
+#define ASUS_WMI_DEVID_TUF_RGB_MODE	0x00100056
+
+/* TUF laptop RGB power/state */
+#define ASUS_WMI_DEVID_TUF_RGB_STATE	0x00100057
+
 /* DSTS masks */
 #define ASUS_WMI_DSTS_STATUS_BIT	0x00000001
 #define ASUS_WMI_DSTS_UNKNOWN_BIT	0x00000002
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH] asus-wmi: Add support for TUF laptop keyboard RGB
@ 2022-08-02  4:57 Luke D. Jones
  0 siblings, 0 replies; 10+ messages in thread
From: Luke D. Jones @ 2022-08-02  4:57 UTC (permalink / raw)
  To: hdegoede
  Cc: corentin.chary, markgross, linux-kernel, platform-driver-x86,
	Luke D. Jones

Adds support for TUF laptop RGB control. This creates two sysfs
paths to add control of basic keyboard LEDs, and power states.

/sys/devices/platform/asus-nb-wmi/tuf_krgb_mode has the following
as input options via U8 "n, n, n, n, n":
- Save or set, if set, then settings revert on cold boot
- Mode, 0-8 for regular modes (if supported), 10-12 for "warning" styles
- Red, 0-255
- Green, 0-255
- Blue, 0-255
- Speed, 0 = Slow, 1 = Medium, 2 = Fast

/sys/devices/platform/asus-nb-wmi/tuf_krgb_state has the following
as input options via boolean "b b b b b":
- Save or set, if set, then settings revert on cold boot
- Boot, if true, the keyboard displays animation on boot
- Awake, if true, the keyboard LED's are on while device is awake
- Sleep, if true, the keyboard shows animation while device is suspended
- Keybaord, appears to have no effect

Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
 drivers/platform/x86/asus-wmi.c            | 168 +++++++++++++++++++++
 include/linux/platform_data/x86/asus-wmi.h |   6 +
 2 files changed, 174 insertions(+)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 62ce198a3463..09277bd98249 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -234,6 +234,9 @@ struct asus_wmi {
 	bool dgpu_disable_available;
 	bool dgpu_disable;
 
+	bool tuf_kb_rgb_mode_available;
+	bool tuf_kb_rgb_state_available;
+
 	bool throttle_thermal_policy_available;
 	u8 throttle_thermal_policy_mode;
 
@@ -734,6 +737,153 @@ static ssize_t egpu_enable_store(struct device *dev,
 
 static DEVICE_ATTR_RW(egpu_enable);
 
+/* TUF Laptop Keyboard RGB Modes **********************************************/
+static int tuf_kb_rgb_mode_check_present(struct asus_wmi *asus)
+{
+	u32 result;
+	int err;
+
+	asus->tuf_kb_rgb_mode_available = false;
+
+	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_MODE, &result);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		return err;
+	}
+
+	if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
+		asus->tuf_kb_rgb_mode_available = true;
+
+	return 0;
+}
+
+static ssize_t tuf_kb_rgb_mode_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	int err;
+	u32 ret;
+	u8 res, tmp, arg_num;
+	char *data, *part, *end;
+	u8 cmd, mode, r, g,  b,  speed;
+
+	data = end = kstrdup(buf, GFP_KERNEL);
+	cmd = mode = r = g = b = speed = arg_num = 0;
+
+	while ((part = strsep(&end, " ")) != NULL) {
+		if (part == NULL)
+			return -1;
+
+		res = kstrtou8(part, 10, &tmp);
+		if (res)
+			return -1;
+
+		if (arg_num == 0)
+			// apply : set
+			cmd = tmp == 1 ? 0xb5 : 0xb4;
+		else if (arg_num == 1)
+			// From 0-8 are valid modes with 10-12 being "warning"
+			// style modes. All models have "pulse" mode 10.
+			mode = (tmp <= 12 && tmp != 9) ? tmp : 10;
+		else if (arg_num == 2)
+			r = tmp;
+		else if (arg_num == 3)
+			g = tmp;
+		else if (arg_num == 4)
+			b = tmp;
+		else if (arg_num == 5) {
+			if (tmp == 0)
+				speed = 0xe1;
+			else if (tmp == 1)
+				speed = 0xeb;
+			else if (tmp == 2)
+				speed = 0xf5;
+		}
+
+		arg_num += 1;
+	}
+
+	err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS, ASUS_WMI_DEVID_TUF_RGB_MODE,
+			cmd | (mode << 8) | (r << 16) | (g << 24), (b) | (speed << 8), &ret);
+	if (err)
+		return err;
+
+	kfree(data);
+	return count;
+}
+
+static DEVICE_ATTR_WO(tuf_kb_rgb_mode);
+
+/* TUF Laptop Keyboard RGB States *********************************************/
+static int tuf_kb_rgb_state_check_present(struct asus_wmi *asus)
+{
+	u32 result;
+	int err;
+
+	asus->tuf_kb_rgb_state_available = false;
+
+	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_STATE, &result);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		return err;
+	}
+
+	if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
+		asus->tuf_kb_rgb_state_available = true;
+
+	return 0;
+}
+
+static ssize_t tuf_kb_rgb_state_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	int err;
+	u32 ret;
+	bool tmp;
+	char *data, *part, *end;
+	u8 save, flags, res, arg_num;
+
+	save = flags = arg_num = 0;
+	data = end = kstrdup(buf, GFP_KERNEL);
+
+	while ((part = strsep(&end, " ")) != NULL) {
+		if (part == NULL)
+			return -1;
+
+		res = kstrtobool(part, &tmp);
+		if (res)
+			return -1;
+
+		if (tmp) {
+			if (arg_num == 0) // save  :  set
+				save = tmp == 0 ? 0x0100 : 0x0000;
+			else if (arg_num == 1)
+				flags |= 0x02; // boot
+			else if (arg_num == 2)
+				flags |= 0x08; // awake
+			else if (arg_num == 3)
+				flags |= 0x20; // sleep
+			else if (arg_num == 4)
+				flags |= 0x80; // keyboard
+		}
+
+		arg_num += 1;
+	}
+
+	err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS,
+			ASUS_WMI_DEVID_TUF_RGB_STATE, 0xBD | save | (flags << 16), 0, &ret);
+	if (err)
+		return err;
+
+	kfree(data);
+	return count;
+}
+
+static DEVICE_ATTR_WO(tuf_kb_rgb_state);
+
 /* Battery ********************************************************************/
 
 /* The battery maximum charging percentage */
@@ -3258,6 +3408,8 @@ static struct attribute *platform_attributes[] = {
 	&dev_attr_touchpad.attr,
 	&dev_attr_egpu_enable.attr,
 	&dev_attr_dgpu_disable.attr,
+	&dev_attr_tuf_kb_rgb_mode.attr,
+	&dev_attr_tuf_kb_rgb_state.attr,
 	&dev_attr_lid_resume.attr,
 	&dev_attr_als_enable.attr,
 	&dev_attr_fan_boost_mode.attr,
@@ -3286,6 +3438,12 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
 		devid = ASUS_WMI_DEVID_ALS_ENABLE;
 	else if (attr == &dev_attr_egpu_enable.attr)
 		ok = asus->egpu_enable_available;
+	else if (attr == &dev_attr_tuf_kb_rgb_mode.attr)
+		ok = asus->tuf_kb_rgb_mode_available;
+	else if (attr == &dev_attr_tuf_kb_rgb_state.attr)
+		ok = asus->tuf_kb_rgb_state_available;
+	else if (attr == &dev_attr_dgpu_disable.attr)
+		ok = asus->dgpu_disable_available;
 	else if (attr == &dev_attr_dgpu_disable.attr)
 		ok = asus->dgpu_disable_available;
 	else if (attr == &dev_attr_fan_boost_mode.attr)
@@ -3557,6 +3715,14 @@ static int asus_wmi_add(struct platform_device *pdev)
 	if (err)
 		goto fail_dgpu_disable;
 
+	err = tuf_kb_rgb_mode_check_present(asus);
+	if (err)
+		goto fail_tuf_kb_rgb_mode;
+
+	err = tuf_kb_rgb_state_check_present(asus);
+	if (err)
+		goto fail_tuf_kb_rgb_state;
+
 	err = fan_boost_mode_check_present(asus);
 	if (err)
 		goto fail_fan_boost_mode;
@@ -3671,6 +3837,8 @@ static int asus_wmi_add(struct platform_device *pdev)
 fail_fan_boost_mode:
 fail_egpu_enable:
 fail_dgpu_disable:
+fail_tuf_kb_rgb_mode:
+fail_tuf_kb_rgb_state:
 fail_platform:
 fail_panel_od:
 	kfree(asus);
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index a571b47ff362..af4191fb0508 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -98,6 +98,12 @@
 /* dgpu on/off */
 #define ASUS_WMI_DEVID_DGPU		0x00090020
 
+/* TUF laptop RGB modes/colours */
+#define ASUS_WMI_DEVID_TUF_RGB_MODE	0x00100056
+
+/* TUF laptop RGB power/state */
+#define ASUS_WMI_DEVID_TUF_RGB_STATE	0x00100057
+
 /* DSTS masks */
 #define ASUS_WMI_DSTS_STATUS_BIT	0x00000001
 #define ASUS_WMI_DSTS_UNKNOWN_BIT	0x00000002
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-08-13 13:38 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-03 23:16 [PATCH] asus-wmi: Add support for TUF laptop keyboard RGB Luke D. Jones
2022-08-04 13:36 ` Hans de Goede
2022-08-13 13:37 ` kernel test robot
  -- strict thread matches above, loose matches on Subject: below --
2022-08-02  4:59 Luke D. Jones
2022-08-02  5:43 ` Luke Jones
2022-08-02 11:09 ` Hans de Goede
2022-08-02 12:13   ` Hans de Goede
2022-08-02 22:26     ` Luke Jones
2022-08-04 13:03       ` Hans de Goede
2022-08-02  4:57 Luke D. Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).