[PATCH v1 1/4] platform/x86: asus-wmi: fix unclear usage of bd->props.power

[PATCH v1 1/4] platform/x86: asus-wmi: fix unclear usage of bd->props.power

[Denis Benato]

The bd->props.power is checked in parts of the driver correctly
comparing with BACKLIGHT_POWER_ON, while in others with a raw usage
of bd->props.power and !bd->props.power, moreover in certain checks
the logic has been inverted due to BACKLIGHT_POWER_ON being defined
as 0: fix both the wrong usage and the inconsistencies by using
proper comparisons.

Fixes: 130d29c5627c ("platform/x86: asus-wmi: adjust screenpad power/brightness handling")
Closes: https://lore.kernel.org/all/178362762638.911488.8564892548331679884@eldamar.lan/
Closes: https://lore.kernel.org/all/ea9c63d1-4776-49d5-9dc4-6c09498f99c9@linux.dev/
Tested-by: Hugo Baigue <hugobaigue2004@gmail.com>
Tested-by: Ponali <ponali2k@gmail.com>
Cc: stable@vger.kernel.org
Assisted-by: zcode:glm-5.3-flash
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
 drivers/platform/x86/asus-wmi.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index b4fd2257bc8e..7382f2b38678 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -4532,7 +4532,8 @@ static int update_screenpad_bl_status(struct backlight_device *bd)
 	u32 ctrl_param = bd->props.brightness;
 	int err = 0;
 
-	if (bd->props.power) {
+	switch (bd->props.power) {
+	case BACKLIGHT_POWER_ON:
 		err = asus_wmi_set_devstate(ASUS_WMI_DEVID_SCREENPAD_POWER, 1, NULL);
 		if (err < 0)
 			return err;
@@ -4540,12 +4541,17 @@ static int update_screenpad_bl_status(struct backlight_device *bd)
 		err = asus_wmi_set_devstate(ASUS_WMI_DEVID_SCREENPAD_LIGHT, ctrl_param, NULL);
 		if (err < 0)
 			return err;
-	}
+		break;
 
-	if (!bd->props.power) {
+	case BACKLIGHT_POWER_OFF:
 		err = asus_wmi_set_devstate(ASUS_WMI_DEVID_SCREENPAD_POWER, 0, NULL);
 		if (err < 0)
 			return err;
+		break;
+
+	default:
+		pr_warn("Invalid screenpad backlight power state: %d\n", bd->props.power);
+		return -EINVAL;
 	}
 
 	return err;
-- 
2.47.3

[PATCH v1 2/4] platform/x86: asus-wmi: fix screenpad power state detection

[Denis Benato]

On some models, e.g. the UX5400EA, the screenpad power state cannot be
read back: DSTS(ASUS_WMI_DEVID_SCREENPAD_POWER) never sets
ASUS_WMI_DSTS_STATUS_BIT, so asus_wmi_get_devstate_simple() reports the
panel as powered off regardless of its real state.

The DSDT shows DSTS 0x00050031 (POWER) and DSTS 0x00050032 (LIGHT)
both read the same two-byte EC register, each returning a different
byte: byte 0 is a raw EC status byte, which is 0xa0 when the panel is
powered and 0x00 when it is off, and byte 1 is the brightness. Read
the power state from the low byte instead of
ASUS_WMI_DSTS_STATUS_BIT: firmware reporting the power through the
status bit is covered too, since ASUS_WMI_DSTS_STATUS_BIT lies inside
ASUS_WMI_DSTS_BRIGHTNESS_MASK.

Reuse read_screenpad_backlight_power() in asus_screenpad_init() in
place of the raw devstate read. This makes the brightness read
reachable on models like the UX5400EA, so mask the SCREENPAD_LIGHT
value with ASUS_WMI_DSTS_BRIGHTNESS_MASK before storing it: the
unmasked devstate (0x0001ffa0 when the panel is on) would otherwise be
exposed to userspace as an out-of-range brightness (max_brightness is
255) which systemd-backlight then persists.

While at it, pass asus_wmi_get_devstate() the u32 it expects.

Fixes: 130d29c5627c ("platform/x86: asus-wmi: adjust screenpad power/brightness handling")
Reported-by: Hugo Baigue <hugobaigue2004@gmail.com>
Closes: https://lore.kernel.org/all/CAO84+x+P2_xyHP89+nGVSMV6bL+dy0P9=vyEEfZGnFhv0hBNWw@mail.gmail.com/
Suggested-by: Hugo Baigue <hugobaigue2004@gmail.com>
Tested-by: Hugo Baigue <hugobaigue2004@gmail.com>
Cc: stable@vger.kernel.org
Assisted-by: zcode:glm-5.3-flash
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
 drivers/platform/x86/asus-wmi.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 7382f2b38678..ded1aa356cf3 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -4498,13 +4498,20 @@ static int is_display_toggle(int code)
 
 static int read_screenpad_backlight_power(struct asus_wmi *asus)
 {
-	int ret;
+	int ret, retval;
 
-	ret = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_SCREENPAD_POWER);
+	ret = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_SCREENPAD_POWER, &retval);
 	if (ret < 0)
 		return ret;
-	/* 1 == powered */
-	return ret ? BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF;
+
+	/*
+	 * The firmware reports the panel power in the low byte of the
+	 * devstate as a raw EC status byte that is non-zero when the
+	 * panel is powered; other models report it through
+	 * ASUS_WMI_DSTS_STATUS_BIT, which lies inside the same mask.
+	 */
+	return (retval & ASUS_WMI_DSTS_BRIGHTNESS_MASK) ?
+		BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF;
 }
 
 static int read_screenpad_brightness(struct backlight_device *bd)
@@ -4568,16 +4575,17 @@ static int asus_screenpad_init(struct asus_wmi *asus)
 	struct backlight_device *bd;
 	struct backlight_properties props;
 	int err, power;
-	int brightness = 0;
+	u32 brightness = 0;
 
-	power = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_SCREENPAD_POWER);
+	power = read_screenpad_backlight_power(asus);
 	if (power < 0)
 		return power;
 
-	if (power) {
+	if (power == BACKLIGHT_POWER_ON) {
 		err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_SCREENPAD_LIGHT, &brightness);
 		if (err < 0)
 			return err;
+		brightness &= ASUS_WMI_DSTS_BRIGHTNESS_MASK;
 	}
 
 	memset(&props, 0, sizeof(struct backlight_properties));
-- 
2.47.3