Adds support for the screenpad(-plus) found on a few ASUS laptops that have a main 16:9 or 16:10 screen and a shorter screen below the main but above the keyboard.
The support consists of:
- On off control
- Setting brightness from 0-255
There are some small quirks with this device when considering only the raw WMI methods:
1. The Off method can only switch the device off
2. Changing the brightness turns the device back on
3. To turn the device back on the brightness must be > 1
4. When the device is off the brightness can't be changed (so it is stored by the driver if device is off).
5. Booting with a value of 0 brightness (retained by bios) means the bios will set a value of > 0, < 15 which is far too dim and was unexpected by testers. The compromise was to set the brightness to 60 which is a usable brightness if the module init brightness was under 15.
6. When the device is off it is "unplugged"
All of the above points are addressed within the patch to create a good user experience and keep within user expectations.
Changelog:
- V3
- Refactor error handling in all functions
- V2
- Complete refactor to use as a backlight device
Luke D. Jones (1):
platform/x86: asus-wmi: add support for ASUS screenpad
drivers/platform/x86/asus-wmi.c | 129 +++++++++++++++++++++
drivers/platform/x86/asus-wmi.h | 1 +
include/linux/platform_data/x86/asus-wmi.h | 4 +
3 files changed, 134 insertions(+)
--
2.40.1
Add support for the WMI methods used to turn off and adjust the
brightness of the secondary "screenpad" device found on some high-end
ASUS laptops like the GX650P series and others.
These methods are utilised in a new backlight device named asus_screenpad.
Signed-off-by: Luke D. Jones <[email protected]>
---
drivers/platform/x86/asus-wmi.c | 129 +++++++++++++++++++++
drivers/platform/x86/asus-wmi.h | 1 +
include/linux/platform_data/x86/asus-wmi.h | 4 +
3 files changed, 134 insertions(+)
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 62cee13f5576..c87fbd81d658 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/minmax.h>
#include <linux/module.h>
#include <linux/pci.h>
#include <linux/pci_hotplug.h>
@@ -212,6 +213,7 @@ struct asus_wmi {
struct input_dev *inputdev;
struct backlight_device *backlight_device;
+ struct backlight_device *screenpad_backlight_device;
struct platform_device *platform_device;
struct led_classdev wlan_led;
@@ -3839,6 +3841,124 @@ static int is_display_toggle(int code)
return 0;
}
+/* Screenpad backlight *******************************************************/
+
+static int read_screenpad_backlight_power(struct asus_wmi *asus)
+{
+ int ret;
+
+ ret = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_SCREENPAD_POWER);
+ if (ret < 0)
+ return ret;
+ /* 1 == powered */
+ return ret ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
+}
+
+static int read_screenpad_brightness(struct backlight_device *bd)
+{
+ struct asus_wmi *asus = bl_get_data(bd);
+ u32 retval;
+ int err;
+
+ err = read_screenpad_backlight_power(asus);
+ if (err < 0)
+ return err;
+ /* The device brightness can only be read if powered, so return stored */
+ if (err == FB_BLANK_POWERDOWN)
+ return asus->driver->screenpad_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;
+}
+
+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;
+
+ power = read_screenpad_backlight_power(asus);
+ if (power < 0)
+ return power;
+
+ if (bd->props.power != power) {
+ if (power != FB_BLANK_UNBLANK) {
+ /* Only brightness > 0 can power it back on */
+ ctrl_param = max(1, asus->driver->screenpad_brightness);
+ 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 == FB_BLANK_UNBLANK) {
+ /* Only set brightness if powered on or we get invalid/unsync state */
+ ctrl_param = bd->props.brightness;
+ err = asus_wmi_set_devstate(ASUS_WMI_DEVID_SCREENPAD_LIGHT, ctrl_param, NULL);
+ }
+
+ /* Ensure brightness is stored to turn back on with */
+ asus->driver->screenpad_brightness = bd->props.brightness;
+
+ return err;
+}
+
+static const struct backlight_ops asus_screenpad_bl_ops = {
+ .get_brightness = read_screenpad_brightness,
+ .update_status = update_screenpad_bl_status,
+ .options = BL_CORE_SUSPENDRESUME,
+};
+
+static int asus_screenpad_init(struct asus_wmi *asus)
+{
+ struct backlight_device *bd;
+ struct backlight_properties props;
+ int power, brightness;
+
+ power = read_screenpad_backlight_power(asus);
+ if (power < 0)
+ return power;
+
+ brightness = read_screenpad_brightness(bd);
+ if (brightness < 0)
+ return brightness;
+
+ memset(&props, 0, sizeof(struct backlight_properties));
+ props.type = BACKLIGHT_RAW; /* ensure this bd is last to be picked */
+ props.max_brightness = 255;
+ bd = backlight_device_register("asus_screenpad",
+ &asus->platform_device->dev, asus,
+ &asus_screenpad_bl_ops, &props);
+ if (IS_ERR(bd)) {
+ pr_err("Could not register backlight device\n");
+ return PTR_ERR(bd);
+ }
+
+ asus->screenpad_backlight_device = bd;
+ /*
+ * Counter an odd behaviour where default is set to < 13 if it was 0 on boot.
+ * 60 is subjective, but accepted as a good compromise to retain visibility.
+ */
+ if (brightness < 60)
+ brightness = 60;
+
+ asus->driver->screenpad_brightness = brightness;
+ bd->props.brightness = brightness;
+ bd->props.power = power;
+ backlight_update_status(bd);
+
+ return 0;
+}
+
+static void asus_screenpad_exit(struct asus_wmi *asus)
+{
+ backlight_device_unregister(asus->screenpad_backlight_device);
+
+ asus->screenpad_backlight_device = NULL;
+}
+
/* Fn-lock ********************************************************************/
static bool asus_wmi_has_fnlock_key(struct asus_wmi *asus)
@@ -4504,6 +4624,12 @@ static int asus_wmi_add(struct platform_device *pdev)
} else if (asus->driver->quirks->wmi_backlight_set_devstate)
err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL);
+ if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_SCREENPAD_LIGHT)) {
+ err = asus_screenpad_init(asus);
+ if (err && err != -ENODEV)
+ goto fail_screenpad;
+ }
+
if (asus_wmi_has_fnlock_key(asus)) {
asus->fnlock_locked = fnlock_default;
asus_wmi_fnlock_update(asus);
@@ -4527,6 +4653,8 @@ static int asus_wmi_add(struct platform_device *pdev)
asus_wmi_backlight_exit(asus);
fail_backlight:
asus_wmi_rfkill_exit(asus);
+fail_screenpad:
+ asus_screenpad_exit(asus);
fail_rfkill:
asus_wmi_led_exit(asus);
fail_leds:
@@ -4553,6 +4681,7 @@ static int asus_wmi_remove(struct platform_device *device)
asus = platform_get_drvdata(device);
wmi_remove_notify_handler(asus->driver->event_guid);
asus_wmi_backlight_exit(asus);
+ asus_screenpad_exit(asus);
asus_wmi_input_exit(asus);
asus_wmi_led_exit(asus);
asus_wmi_rfkill_exit(asus);
diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
index a478ebfd34df..5fbdd0eafa02 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -57,6 +57,7 @@ struct quirk_entry {
struct asus_wmi_driver {
int brightness;
int panel_power;
+ int screenpad_brightness;
int wlan_ctrl_by_user;
const char *name;
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index d17ae2eb0f8d..61ba70b32846 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -58,6 +58,10 @@
#define ASUS_WMI_DEVID_KBD_BACKLIGHT 0x00050021
#define ASUS_WMI_DEVID_LIGHT_SENSOR 0x00050022 /* ?? */
#define ASUS_WMI_DEVID_LIGHTBAR 0x00050025
+/* This can only be used to disable the screen, not re-enable */
+#define ASUS_WMI_DEVID_SCREENPAD_POWER 0x00050031
+/* Writing a brightness re-enables the screen if disabled */
+#define ASUS_WMI_DEVID_SCREENPAD_LIGHT 0x00050032
#define ASUS_WMI_DEVID_FAN_BOOST_MODE 0x00110018
#define ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY 0x00120075
--
2.40.1
Hi Luke,
kernel test robot noticed the following build warnings:
[auto build test WARNING on linus/master]
[also build test WARNING on v6.4-rc7 next-20230620]
[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/platform-x86-asus-wmi-add-support-for-ASUS-screenpad/20230620-110305
base: linus/master
patch link: https://lore.kernel.org/r/20230620030033.55033-2-luke%40ljones.dev
patch subject: [PATCH v3 1/1] platform/x86: asus-wmi: add support for ASUS screenpad
config: x86_64-randconfig-a001-20230620 (https://download.01.org/0day-ci/archive/20230621/[email protected]/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce: (https://download.01.org/0day-ci/archive/20230621/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> drivers/platform/x86/asus-wmi.c:3293:41: warning: variable 'bd' is uninitialized when used here [-Wuninitialized]
brightness = read_screenpad_brightness(bd);
^~
drivers/platform/x86/asus-wmi.c:3285:29: note: initialize the variable 'bd' to silence this warning
struct backlight_device *bd;
^
= NULL
1 warning generated.
vim +/bd +3293 drivers/platform/x86/asus-wmi.c
3282
3283 static int asus_screenpad_init(struct asus_wmi *asus)
3284 {
3285 struct backlight_device *bd;
3286 struct backlight_properties props;
3287 int power, brightness;
3288
3289 power = read_screenpad_backlight_power(asus);
3290 if (power < 0)
3291 return power;
3292
> 3293 brightness = read_screenpad_brightness(bd);
3294 if (brightness < 0)
3295 return brightness;
3296
3297 memset(&props, 0, sizeof(struct backlight_properties));
3298 props.type = BACKLIGHT_RAW; /* ensure this bd is last to be picked */
3299 props.max_brightness = 255;
3300 bd = backlight_device_register("asus_screenpad",
3301 &asus->platform_device->dev, asus,
3302 &asus_screenpad_bl_ops, &props);
3303 if (IS_ERR(bd)) {
3304 pr_err("Could not register backlight device\n");
3305 return PTR_ERR(bd);
3306 }
3307
3308 asus->screenpad_backlight_device = bd;
3309 /*
3310 * Counter an odd behaviour where default is set to < 13 if it was 0 on boot.
3311 * 60 is subjective, but accepted as a good compromise to retain visibility.
3312 */
3313 if (brightness < 60)
3314 brightness = 60;
3315
3316 asus->driver->screenpad_brightness = brightness;
3317 bd->props.brightness = brightness;
3318 bd->props.power = power;
3319 backlight_update_status(bd);
3320
3321 return 0;
3322 }
3323
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Tue, 2023-06-20 at 15:00 +1200, Luke D. Jones wrote:
> Add support for the WMI methods used to turn off and adjust the
> brightness of the secondary "screenpad" device found on some high-end
> ASUS laptops like the GX650P series and others.
>
> These methods are utilised in a new backlight device named
> asus_screenpad.
>
> Signed-off-by: Luke D. Jones <[email protected]>
> ---
> drivers/platform/x86/asus-wmi.c | 129
> +++++++++++++++++++++
> drivers/platform/x86/asus-wmi.h | 1 +
> include/linux/platform_data/x86/asus-wmi.h | 4 +
> 3 files changed, 134 insertions(+)
>
> diff --git a/drivers/platform/x86/asus-wmi.c
> b/drivers/platform/x86/asus-wmi.c
> index 62cee13f5576..c87fbd81d658 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/minmax.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> #include <linux/pci_hotplug.h>
> @@ -212,6 +213,7 @@ struct asus_wmi {
>
> struct input_dev *inputdev;
> struct backlight_device *backlight_device;
> + struct backlight_device *screenpad_backlight_device;
> struct platform_device *platform_device;
>
> struct led_classdev wlan_led;
> @@ -3839,6 +3841,124 @@ static int is_display_toggle(int code)
> return 0;
> }
>
> +/* Screenpad backlight
> *******************************************************/
> +
> +static int read_screenpad_backlight_power(struct asus_wmi *asus)
> +{
> + int ret;
> +
> + ret = asus_wmi_get_devstate_simple(asus,
> ASUS_WMI_DEVID_SCREENPAD_POWER);
> + if (ret < 0)
> + return ret;
> + /* 1 == powered */
> + return ret ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
> +}
> +
> +static int read_screenpad_brightness(struct backlight_device *bd)
> +{
> + struct asus_wmi *asus = bl_get_data(bd);
> + u32 retval;
> + int err;
> +
> + err = read_screenpad_backlight_power(asus);
> + if (err < 0)
> + return err;
> + /* The device brightness can only be read if powered, so
> return stored */
> + if (err == FB_BLANK_POWERDOWN)
> + return asus->driver->screenpad_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;
> +}
> +
> +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;
> +
> + power = read_screenpad_backlight_power(asus);
> + if (power < 0)
> + return power;
> +
> + if (bd->props.power != power) {
> + if (power != FB_BLANK_UNBLANK) {
> + /* Only brightness > 0 can power it back on
> */
> + ctrl_param = max(1, asus->driver-
> >screenpad_brightness);
> + 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 == FB_BLANK_UNBLANK) {
> + /* Only set brightness if powered on or we get
> invalid/unsync state */
> + ctrl_param = bd->props.brightness;
> + err =
> asus_wmi_set_devstate(ASUS_WMI_DEVID_SCREENPAD_LIGHT, ctrl_param,
> NULL);
> + }
> +
> + /* Ensure brightness is stored to turn back on with */
> + asus->driver->screenpad_brightness = bd->props.brightness;
> +
> + return err;
> +}
> +
> +static const struct backlight_ops asus_screenpad_bl_ops = {
> + .get_brightness = read_screenpad_brightness,
> + .update_status = update_screenpad_bl_status,
> + .options = BL_CORE_SUSPENDRESUME,
> +};
> +
> +static int asus_screenpad_init(struct asus_wmi *asus)
> +{
> + struct backlight_device *bd;
> + struct backlight_properties props;
> + int power, brightness;
> +
> + power = read_screenpad_backlight_power(asus);
> + if (power < 0)
> + return power;
> +
> + brightness = read_screenpad_brightness(bd);
> + if (brightness < 0)
> + return brightness;
I have made a mistake here. If power is off then
read_screenpad_backlight_power() tries to return a stored brightness
from the dev struct but this is uninitialised at this point.
I will submit v4.
> +
> + memset(&props, 0, sizeof(struct backlight_properties));
> + props.type = BACKLIGHT_RAW; /* ensure this bd is last to be
> picked */
> + props.max_brightness = 255;
> + bd = backlight_device_register("asus_screenpad",
> + &asus->platform_device->dev,
> asus,
> + &asus_screenpad_bl_ops,
> &props);
> + if (IS_ERR(bd)) {
> + pr_err("Could not register backlight device\n");
> + return PTR_ERR(bd);
> + }
> +
> + asus->screenpad_backlight_device = bd;
> + /*
> + * Counter an odd behaviour where default is set to < 13 if
> it was 0 on boot.
> + * 60 is subjective, but accepted as a good compromise to
> retain visibility.
> + */
> + if (brightness < 60)
> + brightness = 60;
> +
> + asus->driver->screenpad_brightness = brightness;
> + bd->props.brightness = brightness;
> + bd->props.power = power;
> + backlight_update_status(bd);
> +
> + return 0;
> +}
> +
> +static void asus_screenpad_exit(struct asus_wmi *asus)
> +{
> + backlight_device_unregister(asus-
> >screenpad_backlight_device);
> +
> + asus->screenpad_backlight_device = NULL;
> +}
> +
> /* Fn-lock
> ********************************************************************/
>
> static bool asus_wmi_has_fnlock_key(struct asus_wmi *asus)
> @@ -4504,6 +4624,12 @@ static int asus_wmi_add(struct platform_device
> *pdev)
> } else if (asus->driver->quirks->wmi_backlight_set_devstate)
> err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT,
> 2, NULL);
>
> + if (asus_wmi_dev_is_present(asus,
> ASUS_WMI_DEVID_SCREENPAD_LIGHT)) {
> + err = asus_screenpad_init(asus);
> + if (err && err != -ENODEV)
> + goto fail_screenpad;
> + }
> +
> if (asus_wmi_has_fnlock_key(asus)) {
> asus->fnlock_locked = fnlock_default;
> asus_wmi_fnlock_update(asus);
> @@ -4527,6 +4653,8 @@ static int asus_wmi_add(struct platform_device
> *pdev)
> asus_wmi_backlight_exit(asus);
> fail_backlight:
> asus_wmi_rfkill_exit(asus);
> +fail_screenpad:
> + asus_screenpad_exit(asus);
> fail_rfkill:
> asus_wmi_led_exit(asus);
> fail_leds:
> @@ -4553,6 +4681,7 @@ static int asus_wmi_remove(struct
> platform_device *device)
> asus = platform_get_drvdata(device);
> wmi_remove_notify_handler(asus->driver->event_guid);
> asus_wmi_backlight_exit(asus);
> + asus_screenpad_exit(asus);
> asus_wmi_input_exit(asus);
> asus_wmi_led_exit(asus);
> asus_wmi_rfkill_exit(asus);
> diff --git a/drivers/platform/x86/asus-wmi.h
> b/drivers/platform/x86/asus-wmi.h
> index a478ebfd34df..5fbdd0eafa02 100644
> --- a/drivers/platform/x86/asus-wmi.h
> +++ b/drivers/platform/x86/asus-wmi.h
> @@ -57,6 +57,7 @@ struct quirk_entry {
> struct asus_wmi_driver {
> int brightness;
> int panel_power;
> + int screenpad_brightness;
> int wlan_ctrl_by_user;
>
> const char *name;
> diff --git a/include/linux/platform_data/x86/asus-wmi.h
> b/include/linux/platform_data/x86/asus-wmi.h
> index d17ae2eb0f8d..61ba70b32846 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -58,6 +58,10 @@
> #define ASUS_WMI_DEVID_KBD_BACKLIGHT 0x00050021
> #define ASUS_WMI_DEVID_LIGHT_SENSOR 0x00050022 /* ?? */
> #define ASUS_WMI_DEVID_LIGHTBAR 0x00050025
> +/* This can only be used to disable the screen, not re-enable */
> +#define ASUS_WMI_DEVID_SCREENPAD_POWER 0x00050031
> +/* Writing a brightness re-enables the screen if disabled */
> +#define ASUS_WMI_DEVID_SCREENPAD_LIGHT 0x00050032
> #define ASUS_WMI_DEVID_FAN_BOOST_MODE 0x00110018
> #define ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY 0x00120075
>