[PATCH 21/24] platform/x86: ideapad-laptop: add keyboard backlight control support

[PATCH 21/24] platform/x86: ideapad-laptop: add keyboard backlight control support

[Barnabás Pőcze]

On certain models it is possible to control/query the keyboard backlight
via the SALS/HALS ACPI methods. Add support for that, and register an LED
class device to expose this functionality.
Tested on: Lenovo YOGA 520-14IKB 80X8

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 075056075b2d..a43574e65f61 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -9,6 +9,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <acpi/video.h>
+#include <dt-bindings/leds/common.h>
 #include <linux/acpi.h>
 #include <linux/backlight.h>
 #include <linux/bitops.h>
@@ -22,6 +23,7 @@
 #include <linux/input/sparse-keymap.h>
 #include <linux/jiffies.h>
 #include <linux/kernel.h>
+#include <linux/leds.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/rfkill.h>
@@ -56,12 +58,16 @@ enum {
 };
 
 enum {
+	HALS_KBD_BL_SUPPORT_BIT  = 4,
+	HALS_KBD_BL_STATE_BIT    = 5,
 	HALS_FNLOCK_SUPPORT_BIT  = 9,
 	HALS_FNLOCK_STATE_BIT    = 10,
 	HALS_HOTKEYS_PRIMARY_BIT = 11,
 };
 
 enum {
+	SALS_KBD_BL_ON  = 0x8,
+	SALS_KBD_BL_OFF = 0x9,
 	SALS_FNLOCK_ON  = 0xe,
 	SALS_FNLOCK_OFF = 0xf,
 };
@@ -114,6 +120,11 @@ struct ideapad_private {
 		     fn_lock           : 1,
 		     hw_rfkill_switch  : 1;
 	} features;
+	struct {
+		bool initialized;
+		struct led_classdev led;
+		unsigned int last_brightness;
+	} kbd_bl;
 };
 
 static bool no_bt_rfkill;
@@ -934,6 +945,110 @@ static void ideapad_backlight_notify_brightness(struct ideapad_private *priv)
 		backlight_force_update(priv->blightdev, BACKLIGHT_UPDATE_HOTKEY);
 }
 
+/*
+ * keyboard backlight
+ */
+static int ideapad_kbd_bl_brightness_get(struct ideapad_private *priv)
+{
+	unsigned long hals;
+	int err;
+
+	err = eval_hals(priv->adev->handle, &hals);
+	if (err)
+		return err;
+
+	return test_bit(HALS_KBD_BL_STATE_BIT, &hals);
+}
+
+static enum led_brightness ideapad_kbd_bl_led_cdev_brightness_get(struct led_classdev *led_cdev)
+{
+	struct ideapad_private *priv = container_of(led_cdev, struct ideapad_private, kbd_bl.led);
+
+	return ideapad_kbd_bl_brightness_get(priv);
+}
+
+static int ideapad_kbd_bl_brightness_set(struct ideapad_private *priv, unsigned int brightness)
+{
+	int err;
+
+	err = eval_sals(priv->adev->handle,
+			brightness ? SALS_KBD_BL_ON : SALS_KBD_BL_OFF);
+	if (err)
+		return err;
+
+	priv->kbd_bl.last_brightness = brightness;
+
+	return 0;
+}
+
+static int ideapad_kbd_bl_led_cdev_brightness_set(struct led_classdev *led_cdev,
+						  enum led_brightness brightness)
+{
+	struct ideapad_private *priv = container_of(led_cdev, struct ideapad_private, kbd_bl.led);
+
+	return ideapad_kbd_bl_brightness_set(priv, brightness);
+}
+
+static void ideapad_kbd_bl_notify(struct ideapad_private *priv)
+{
+	int brightness;
+
+	if (!priv->kbd_bl.initialized)
+		return;
+
+	brightness = ideapad_kbd_bl_brightness_get(priv);
+	if (brightness < 0)
+		return;
+
+	if (brightness == priv->kbd_bl.last_brightness)
+		return;
+
+	priv->kbd_bl.last_brightness = brightness;
+
+	led_classdev_notify_brightness_hw_changed(&priv->kbd_bl.led, brightness);
+}
+
+static int ideapad_kbd_bl_init(struct ideapad_private *priv)
+{
+	unsigned long hals;
+	int err;
+
+	err = eval_hals(priv->adev->handle, &hals);
+	if (err)
+		return err;
+
+	if (!test_bit(HALS_KBD_BL_SUPPORT_BIT, &hals))
+		return -ENODEV;
+
+	err = ideapad_kbd_bl_brightness_get(priv);
+	if (err < 0)
+		return err;
+
+	priv->kbd_bl.last_brightness = err;
+
+	priv->kbd_bl.led.name                    = "platform::" LED_FUNCTION_KBD_BACKLIGHT;
+	priv->kbd_bl.led.max_brightness          = 1;
+	priv->kbd_bl.led.brightness_get          = ideapad_kbd_bl_led_cdev_brightness_get;
+	priv->kbd_bl.led.brightness_set_blocking = ideapad_kbd_bl_led_cdev_brightness_set;
+	priv->kbd_bl.led.flags                   = LED_BRIGHT_HW_CHANGED;
+
+	err = led_classdev_register(&priv->platform_device->dev, &priv->kbd_bl.led);
+	if (err)
+		return err;
+
+	priv->kbd_bl.initialized = true;
+
+	return 0;
+}
+
+static void ideapad_kbd_bl_exit(struct ideapad_private *priv)
+{
+	if (!priv->kbd_bl.initialized)
+		return;
+
+	led_classdev_unregister(&priv->kbd_bl.led);
+}
+
 /*
  * module init/exit
  */
@@ -1000,8 +1115,9 @@ static void ideapad_acpi_notify(acpi_handle handle, u32 event, void *data)
 			 * Some IdeaPads report event 1 every ~20
 			 * seconds while on battery power; some
 			 * report this when changing to/from tablet
-			 * mode. Squelch this event.
+			 * mode.
 			 */
+			ideapad_kbd_bl_notify(priv);
 			break;
 		default:
 			dev_warn(&priv->platform_device->dev,
@@ -1101,6 +1217,11 @@ static int ideapad_acpi_add(struct platform_device *pdev)
 	if (err)
 		goto input_failed;
 
+	err = ideapad_kbd_bl_init(priv);
+	if (err && err != -ENODEV)
+		dev_warn(&pdev->dev,
+			 "Could not register keyboard backlight led: %d\n", err);
+
 	/*
 	 * On some models without a hw-switch (the yoga 2 13 at least)
 	 * VPCCMD_W_RF must be explicitly set to 1 for the wifi to work.
@@ -1161,6 +1282,7 @@ static int ideapad_acpi_add(struct platform_device *pdev)
 	for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
 		ideapad_unregister_rfkill(priv, i);
 
+	ideapad_kbd_bl_exit(priv);
 	ideapad_input_exit(priv);
 
 input_failed:
@@ -1188,6 +1310,7 @@ static int ideapad_acpi_remove(struct platform_device *pdev)
 	for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
 		ideapad_unregister_rfkill(priv, i);
 
+	ideapad_kbd_bl_exit(priv);
 	ideapad_input_exit(priv);
 	ideapad_debugfs_exit(priv);
 	ideapad_sysfs_exit(priv);
-- 
2.29.2

Re: [PATCH 21/24] platform/x86: ideapad-laptop: add keyboard backlight control support

[Hans de Goede]

Hi,

On 12/16/20 2:40 AM, Barnabás Pőcze wrote:
> On certain models it is possible to control/query the keyboard backlight
> via the SALS/HALS ACPI methods. Add support for that, and register an LED
> class device to expose this functionality.
> Tested on: Lenovo YOGA 520-14IKB 80X8
> 
> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
> 
> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> index 075056075b2d..a43574e65f61 100644
> --- a/drivers/platform/x86/ideapad-laptop.c
> +++ b/drivers/platform/x86/ideapad-laptop.c
> @@ -9,6 +9,7 @@
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
>  #include <acpi/video.h>
> +#include <dt-bindings/leds/common.h>
>  #include <linux/acpi.h>
>  #include <linux/backlight.h>
>  #include <linux/bitops.h>
> @@ -22,6 +23,7 @@
>  #include <linux/input/sparse-keymap.h>
>  #include <linux/jiffies.h>
>  #include <linux/kernel.h>
> +#include <linux/leds.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/rfkill.h>

I guess you need a "depends on LEDS_CLASS" in the Kconfig for this?

> @@ -56,12 +58,16 @@ enum {
>  };
>  
>  enum {
> +	HALS_KBD_BL_SUPPORT_BIT  = 4,
> +	HALS_KBD_BL_STATE_BIT    = 5,
>  	HALS_FNLOCK_SUPPORT_BIT  = 9,
>  	HALS_FNLOCK_STATE_BIT    = 10,
>  	HALS_HOTKEYS_PRIMARY_BIT = 11,
>  };
>  
>  enum {
> +	SALS_KBD_BL_ON  = 0x8,
> +	SALS_KBD_BL_OFF = 0x9,
>  	SALS_FNLOCK_ON  = 0xe,
>  	SALS_FNLOCK_OFF = 0xf,
>  };
> @@ -114,6 +120,11 @@ struct ideapad_private {
>  		     fn_lock           : 1,
>  		     hw_rfkill_switch  : 1;
>  	} features;
> +	struct {
> +		bool initialized;
> +		struct led_classdev led;
> +		unsigned int last_brightness;
> +	} kbd_bl;
>  };
>  
>  static bool no_bt_rfkill;
> @@ -934,6 +945,110 @@ static void ideapad_backlight_notify_brightness(struct ideapad_private *priv)
>  		backlight_force_update(priv->blightdev, BACKLIGHT_UPDATE_HOTKEY);
>  }
>  
> +/*
> + * keyboard backlight
> + */
> +static int ideapad_kbd_bl_brightness_get(struct ideapad_private *priv)
> +{
> +	unsigned long hals;
> +	int err;
> +
> +	err = eval_hals(priv->adev->handle, &hals);
> +	if (err)
> +		return err;
> +
> +	return test_bit(HALS_KBD_BL_STATE_BIT, &hals);
> +}
> +
> +static enum led_brightness ideapad_kbd_bl_led_cdev_brightness_get(struct led_classdev *led_cdev)
> +{
> +	struct ideapad_private *priv = container_of(led_cdev, struct ideapad_private, kbd_bl.led);
> +
> +	return ideapad_kbd_bl_brightness_get(priv);
> +}
> +
> +static int ideapad_kbd_bl_brightness_set(struct ideapad_private *priv, unsigned int brightness)
> +{
> +	int err;
> +
> +	err = eval_sals(priv->adev->handle,
> +			brightness ? SALS_KBD_BL_ON : SALS_KBD_BL_OFF);
> +	if (err)
> +		return err;
> +
> +	priv->kbd_bl.last_brightness = brightness;
> +
> +	return 0;
> +}
> +
> +static int ideapad_kbd_bl_led_cdev_brightness_set(struct led_classdev *led_cdev,
> +						  enum led_brightness brightness)
> +{
> +	struct ideapad_private *priv = container_of(led_cdev, struct ideapad_private, kbd_bl.led);
> +
> +	return ideapad_kbd_bl_brightness_set(priv, brightness);
> +}
> +
> +static void ideapad_kbd_bl_notify(struct ideapad_private *priv)
> +{
> +	int brightness;
> +
> +	if (!priv->kbd_bl.initialized)
> +		return;
> +
> +	brightness = ideapad_kbd_bl_brightness_get(priv);
> +	if (brightness < 0)
> +		return;
> +
> +	if (brightness == priv->kbd_bl.last_brightness)
> +		return;
> +
> +	priv->kbd_bl.last_brightness = brightness;
> +
> +	led_classdev_notify_brightness_hw_changed(&priv->kbd_bl.led, brightness);
> +}

So I guess that there is some hotkey combo on the laptops keyboards which
directly changes the state of the kbd backlight "underneath" us and that
is why this is necessary ?

> +
> +static int ideapad_kbd_bl_init(struct ideapad_private *priv)
> +{
> +	unsigned long hals;
> +	int err;
> +
> +	err = eval_hals(priv->adev->handle, &hals);
> +	if (err)
> +		return err;

So you are checking for presence of the HALS method here, but not
for SALS which is being used in ideapad_kbd_bl_led_cdev_brightness_set()
and you are needlessly re-evaluating HALS here. Would it not be better
to add a features.kbd_bl flag and set that from ideapad_check_features()
as done for other features ? That would avoid an extra evaluation of
the HALS method and also check for SALS.

You can then still unconditionally call this function but just bail
with -ENODEV if the feature flag is not set.

Otherwise this looks good to me.

Regards,

Hans



> +
> +	if (!test_bit(HALS_KBD_BL_SUPPORT_BIT, &hals))
> +		return -ENODEV;
> +
> +	err = ideapad_kbd_bl_brightness_get(priv);
> +	if (err < 0)
> +		return err;
> +
> +	priv->kbd_bl.last_brightness = err;
> +
> +	priv->kbd_bl.led.name                    = "platform::" LED_FUNCTION_KBD_BACKLIGHT;
> +	priv->kbd_bl.led.max_brightness          = 1;
> +	priv->kbd_bl.led.brightness_get          = ideapad_kbd_bl_led_cdev_brightness_get;
> +	priv->kbd_bl.led.brightness_set_blocking = ideapad_kbd_bl_led_cdev_brightness_set;
> +	priv->kbd_bl.led.flags                   = LED_BRIGHT_HW_CHANGED;
> +
> +	err = led_classdev_register(&priv->platform_device->dev, &priv->kbd_bl.led);
> +	if (err)
> +		return err;
> +
> +	priv->kbd_bl.initialized = true;
> +
> +	return 0;
> +}
> +
> +static void ideapad_kbd_bl_exit(struct ideapad_private *priv)
> +{
> +	if (!priv->kbd_bl.initialized)
> +		return;
> +
> +	led_classdev_unregister(&priv->kbd_bl.led);
> +}
> +
>  /*
>   * module init/exit
>   */
> @@ -1000,8 +1115,9 @@ static void ideapad_acpi_notify(acpi_handle handle, u32 event, void *data)
>  			 * Some IdeaPads report event 1 every ~20
>  			 * seconds while on battery power; some
>  			 * report this when changing to/from tablet
> -			 * mode. Squelch this event.
> +			 * mode.
>  			 */
> +			ideapad_kbd_bl_notify(priv);
>  			break;
>  		default:
>  			dev_warn(&priv->platform_device->dev,
> @@ -1101,6 +1217,11 @@ static int ideapad_acpi_add(struct platform_device *pdev)
>  	if (err)
>  		goto input_failed;
>  
> +	err = ideapad_kbd_bl_init(priv);
> +	if (err && err != -ENODEV)
> +		dev_warn(&pdev->dev,
> +			 "Could not register keyboard backlight led: %d\n", err);
> +
>  	/*
>  	 * On some models without a hw-switch (the yoga 2 13 at least)
>  	 * VPCCMD_W_RF must be explicitly set to 1 for the wifi to work.
> @@ -1161,6 +1282,7 @@ static int ideapad_acpi_add(struct platform_device *pdev)
>  	for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
>  		ideapad_unregister_rfkill(priv, i);
>  
> +	ideapad_kbd_bl_exit(priv);
>  	ideapad_input_exit(priv);
>  
>  input_failed:
> @@ -1188,6 +1310,7 @@ static int ideapad_acpi_remove(struct platform_device *pdev)
>  	for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
>  		ideapad_unregister_rfkill(priv, i);
>  
> +	ideapad_kbd_bl_exit(priv);
>  	ideapad_input_exit(priv);
>  	ideapad_debugfs_exit(priv);
>  	ideapad_sysfs_exit(priv);
>