Skip to contentSkip to conversation

platform/x86: asus-wmi: fix screenpad brightness/power management

PATCH In review Denis Benato 2 participants 4 messages Mar 2 – Mar 24

Subsystems (2)
x86 platform drivers ASUS notebooks and eeepc ACPI/WMI extras drivers
lore
Share & tools
lore

Denis Benato

author maintainer
#0 lore raw

v3 cover platform/x86: asus-wmi: fix screenpad brightness/power management

Fix up some inconsistent behaviour involving the screenpad on some ASUS laptops. This fixes:
- illogical screen off control (0/1 flipped depending on WMI state)
- bad brightness depending on the last screenpad power state
- incorrect brightness scaling

Changelog:
- v1
- Initial submission
- v2
- Split patch in two
- Remove a redundant variable copy
- v3
- Fix variable declaration (reverse x-mas tree)
- Change commit message of 2/2 to reflect the actual change
- Add "Fixes" tag to both patches

Denis Benato (2):
platform/x86: asus-wmi: adjust screenpad power/brightness handling platform/x86: asus-wmi: fix screenpad brightness range

 drivers/platform/x86/asus-wmi.c | 50 +++++++++++++--------------------
 1 file changed, 19 insertions(+), 31 deletions(-)
Signature · 1 line
2.53.0
author maintainer
#1 lore raw

PATCH v3 1/2 platform/x86: asus-wmi: adjust screenpad power/brightness handling

Fix illogical screen off control by hardcoding 0 and 1 depending on the requested brightness and also do not rely on the last screenpad power state to issue screen brightness commands.

Fixes 2c97d3e55b70 ("platform/x86: asus-wmi: add support for ASUS screenpad")
Signed-off-by Denis Benato <[email protected]>
Signed-off-by Luke Jones <[email protected]>
---
 drivers/platform/x86/asus-wmi.c | 34 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 21 deletions(-)
drivers/platform/x86/asus-wmi.c +13 -21
index 7c0915e097ba..8925a0ee26af 100644
@@ -4419,32 +4419,24 @@ static int read_screenpad_brightness(struct backlight_device *bd)
 
 static int update_screenpad_bl_status(struct backlight_device *bd)
 {
-	struct asus_wmi *asus = bl_get_data(bd);
-	int power, err = 0;
-	u32 ctrl_param;
+	u32 ctrl_param = bd->props.brightness;
+	int err = 0;
 
-	power = read_screenpad_backlight_power(asus);
-	if (power < 0)
-		return power;
+	if (ctrl_param >= 0 && bd->props.power) {
+		err = asus_wmi_set_devstate(ASUS_WMI_DEVID_SCREENPAD_POWER, 1, NULL);
+		if (err < 0)
+			return err;
 
-	if (bd->props.power != power) {
-		if (power != BACKLIGHT_POWER_ON) {
-			/* Only brightness > 0 can power it back on */
-			ctrl_param = asus->driver->screenpad_brightness - ASUS_SCREENPAD_BRIGHT_MIN;
-			err = asus_wmi_set_devstate(ASUS_WMI_DEVID_SCREENPAD_LIGHT,
-						    ctrl_param, NULL);
-		} else {
-			err = asus_wmi_set_devstate(ASUS_WMI_DEVID_SCREENPAD_POWER, 0, NULL);
-		}
-	} else if (power == BACKLIGHT_POWER_ON) {
-		/* Only set brightness if powered on or we get invalid/unsync state */
-		ctrl_param = bd->props.brightness + ASUS_SCREENPAD_BRIGHT_MIN;
 		err = asus_wmi_set_devstate(ASUS_WMI_DEVID_SCREENPAD_LIGHT, ctrl_param, NULL);
+		if (err < 0)
+			return err;
 	}
 
-	/* Ensure brightness is stored to turn back on with */
-	if (err == 0)
-		asus->driver->screenpad_brightness = bd->props.brightness + ASUS_SCREENPAD_BRIGHT_MIN;
+	if (!bd->props.power) {
+		err = asus_wmi_set_devstate(ASUS_WMI_DEVID_SCREENPAD_POWER, 0, NULL);
+		if (err < 0)
+			return err;
+	}
 
 	return err;
 }
Signature · 1 line
2.53.0
author maintainer
#2 lore raw

PATCH v3 2/2 platform/x86: asus-wmi: fix screenpad brightness range

In reply to Denis Benato

Fix screenpad brightness range being too limited without reason:
testing this patch on a Zenbook Duo showed the hardware minimum not being too low, therefore allow the user to configure the entire range, and expose to userspace the hardware brightness range and value.

Fixes 2c97d3e55b70 ("platform/x86: asus-wmi: add support for ASUS screenpad")
Signed-off-by Denis Benato <[email protected]>
Signed-off-by Luke Jones <[email protected]>
---
 drivers/platform/x86/asus-wmi.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
drivers/platform/x86/asus-wmi.c +6 -10
index 8925a0ee26af..17e2dcaf2b4c 100644
@@ -125,7 +125,6 @@ module_param(fnlock_default, bool, 0444);
 #define NVIDIA_TEMP_MIN		75
 #define NVIDIA_TEMP_MAX		87
 
-#define ASUS_SCREENPAD_BRIGHT_MIN 20
 #define ASUS_SCREENPAD_BRIGHT_MAX 255
 #define ASUS_SCREENPAD_BRIGHT_DEFAULT 60
 

@@ -4408,13 +4407,13 @@ static int read_screenpad_brightness(struct backlight_device *bd)
 		return err;
 	/* The device brightness can only be read if powered, so return stored */
 	if (err == BACKLIGHT_POWER_OFF)
-		return asus->driver->screenpad_brightness - ASUS_SCREENPAD_BRIGHT_MIN;
+		return bd->props.brightness;
 
 	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_SCREENPAD_LIGHT, &retval);
 	if (err < 0)
 		return err;
 
-	return (retval & ASUS_WMI_DSTS_BRIGHTNESS_MASK) - ASUS_SCREENPAD_BRIGHT_MIN;
+	return (retval & ASUS_WMI_DSTS_BRIGHTNESS_MASK);
 }
 
 static int update_screenpad_bl_status(struct backlight_device *bd)

@@ -4454,22 +4453,19 @@ static int asus_screenpad_init(struct asus_wmi *asus)
 	int err, power;
 	int brightness = 0;
 
-	power = read_screenpad_backlight_power(asus);
+	power = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_SCREENPAD_POWER);
 	if (power < 0)
 		return power;
 
-	if (power != BACKLIGHT_POWER_OFF) {
+	if (power) {
 		err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_SCREENPAD_LIGHT, &brightness);
 		if (err < 0)
 			return err;
 	}
-	/* default to an acceptable min brightness on boot if too low */
-	if (brightness < ASUS_SCREENPAD_BRIGHT_MIN)
-		brightness = ASUS_SCREENPAD_BRIGHT_DEFAULT;
 
 	memset(&props, 0, sizeof(struct backlight_properties));
 	props.type = BACKLIGHT_RAW; /* ensure this bd is last to be picked */
-	props.max_brightness = ASUS_SCREENPAD_BRIGHT_MAX - ASUS_SCREENPAD_BRIGHT_MIN;
+	props.max_brightness = ASUS_SCREENPAD_BRIGHT_MAX;
 	bd = backlight_device_register("asus_screenpad",
 				       &asus->platform_device->dev, asus,
 				       &asus_screenpad_bl_ops, &props);

@@ -4480,7 +4476,7 @@ static int asus_screenpad_init(struct asus_wmi *asus)
 
 	asus->screenpad_backlight_device = bd;
 	asus->driver->screenpad_brightness = brightness;
-	bd->props.brightness = brightness - ASUS_SCREENPAD_BRIGHT_MIN;
+	bd->props.brightness = brightness;
 	bd->props.power = power;
 	backlight_update_status(bd);
Signature · 1 line
2.53.0
maintainer
#3 lore raw
In reply to Denis Benato
Quoted 17 lines · Denis Benato

Fix up some inconsistent behaviour involving the screenpad on some ASUS laptops. This fixes:
- illogical screen off control (0/1 flipped depending on WMI state)
- bad brightness depending on the last screenpad power state
- incorrect brightness scaling

Changelog:
- v1
- Initial submission
- v2
- Split patch in two
- Remove a redundant variable copy
- v3
- Fix variable declaration (reverse x-mas tree)
- Change commit message of 2/2 to reflect the actual change
- Add "Fixes" tag to both patches

Quoted 1 line

[...]

Thank you for your contribution, it has been applied to my local review-ilpo-next branch. Note it will show up in the public platform-drivers-x86/review-ilpo-next branch only once I've pushed my local branch there, which might take a while.

The list of commits applied:
[1/2] platform/x86: asus-wmi: adjust screenpad power/brightness handling

      commit: 034f5efd362fb87a3970d61eaf982664f84e6c5a
[2/2] platform/x86: asus-wmi: fix screenpad brightness range
      commit: fab7c51693e95aa874d6d9db22bfae7bd0b23c66

--
i.