Re: [PATCH] HID: surface-hid: support keyboard backlight on Surface Pro Keyboard (045E:09AE)

[Maximilian Luz <luzmaximilian@gmail.com>]

Am 23.10.2026 um 19:32 schrieb Ramón Jesús Miranda:
> The Surface Pro Keyboard / Flex Keyboard (VID 045E, PID 09AE) attached to
> the Microsoft Surface Pro 11 (Snapdragon X Elite / X1E80100) exposes its
> keyboard backlight through the HID Consumer Control collection (Usage Page
> 0x0c, Usage 0x07) rather than through the legacy Surface Aggregator Module
> (SAM) TC/KBD category:
> 
> - Feature Report 0xd5 (Usage 0x0517, unit Candela) advertises the 4
>    supported discrete brightness levels: [0x00, 0x03, 0x06, 0x0c].
> - Feature Report 0xd6 (Usage 0x007b) returns the current brightness level
>    (0, 3, 6, or 12).
> - Output Report 0xd5 ([0xd5, level]) sets the brightness level.
> - Input Report 0xd5 (Usage 0x0515, Keyboard Brightness Next) fires
>    [0xd5, 0x01] whenever the hardware backlight key (F5) is pressed and the
>    embedded controller cycles the brightness in hardware.
> 
> Register a LED class device named "surface::kbd_backlight" (max_brightness
> = 3, flags = LED_BRIGHT_HW_CHANGED) when the HID device matches VID 045E /
> PID 09AE, mapping steps 0..3 to hardware values {0, 3, 6, 12}. Also hook
> raw_event to read back Feature Report 0xd6 and notify userspace via
> led_classdev_notify_brightness_hw_changed() when Input Report 0xd5 arrives
> from the F5 key, and restore the saved brightness level on resume.

Should this whole thing not be an HID driver? I don't like the idea of
hard-coding report parsing here.

Best,
Max

> 
> Tested on a Microsoft Surface Pro 11 OLED (X1E80100) with UPower and KDE
> PowerDevil.
> 
> Assisted-by: Antigravity:gemini-2.5-pro
> Signed-off-by: Ramón Jesús Miranda <ramonjesusmiranda@gmail.com>
> ---
>   drivers/hid/surface-hid/surface_hid.c      |   4 +
>   drivers/hid/surface-hid/surface_hid_core.c | 104 ++++++++++++++++++++-
>   drivers/hid/surface-hid/surface_hid_core.h |   8 ++
>   3 files changed, 114 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/surface-hid/surface_hid.c b/drivers/hid/surface-hid/surface_hid.c
> index eae47e0..0f81732 100644
> --- a/drivers/hid/surface-hid/surface_hid.c
> +++ b/drivers/hid/surface-hid/surface_hid.c
> @@ -161,6 +161,10 @@ static u32 ssam_hid_event_fn(struct ssam_event_notifier *nf, const struct ssam_e
>   	if (event->command_id != 0x00)
>   		return 0;
>   
> +	if (shid->has_kbd_bl && event->length >= 2 &&
> +	    event->data[0] == 0xd5 && (event->data[1] & 0x01))
> +		schedule_work(&shid->kbd_bl_work);
> +
>   	hid_input_report(shid->hid, HID_INPUT_REPORT, (u8 *)&event->data[0], event->length, 0);
>   	return SSAM_NOTIF_HANDLED;
>   }
> diff --git a/drivers/hid/surface-hid/surface_hid_core.c b/drivers/hid/surface-hid/surface_hid_core.c
> index 6690c24..afee61f 100644
> --- a/drivers/hid/surface-hid/surface_hid_core.c
> +++ b/drivers/hid/surface-hid/surface_hid_core.c
> @@ -184,6 +184,98 @@ static const struct hid_ll_driver surface_hid_ll_driver = {
>   };
>   
>   
> +/* -- Keyboard backlight (Surface Pro Flex / Type Cover 045E:09AE). --------- */
> +
> +static u8 surface_kbd_bl_raw_to_idx(struct surface_hid_device *shid, u8 raw)
> +{
> +	u8 i;
> +
> +	for (i = 0; i < 4; i++) {
> +		if (shid->kbd_bl_levels[i] >= raw)
> +			return i;
> +	}
> +	return 3;
> +}
> +
> +static enum led_brightness surface_kbd_bl_get(struct led_classdev *cdev)
> +{
> +	struct surface_hid_device *shid =
> +		container_of(cdev, struct surface_hid_device, kbd_bl_led);
> +	u8 buf[2] = { 0xd6, 0 };
> +
> +	if (surface_hid_is_hot_removed(shid))
> +		return shid->kbd_bl_cur;
> +
> +	if (shid->ops.get_feature_report(shid, 0xd6, buf, sizeof(buf)) < 0)
> +		return shid->kbd_bl_cur;
> +
> +	shid->kbd_bl_cur = surface_kbd_bl_raw_to_idx(shid, buf[1]);
> +	return shid->kbd_bl_cur;
> +}
> +
> +static int surface_kbd_bl_set(struct led_classdev *cdev, enum led_brightness value)
> +{
> +	struct surface_hid_device *shid =
> +		container_of(cdev, struct surface_hid_device, kbd_bl_led);
> +	u8 idx = clamp_t(u8, value, 0, 3);
> +	u8 buf[2] = { 0xd5, shid->kbd_bl_levels[idx] };
> +	int ret;
> +
> +	if (surface_hid_is_hot_removed(shid))
> +		return -ENODEV;
> +
> +	ret = shid->ops.output_report(shid, 0xd5, buf, sizeof(buf));
> +	if (ret < 0)
> +		return ret;
> +
> +	shid->kbd_bl_cur = idx;
> +	return 0;
> +}
> +
> +static void surface_kbd_bl_work_fn(struct work_struct *work)
> +{
> +	struct surface_hid_device *shid =
> +		container_of(work, struct surface_hid_device, kbd_bl_work);
> +	u8 prev = shid->kbd_bl_cur;
> +	u8 hw_idx = surface_kbd_bl_get(&shid->kbd_bl_led);
> +
> +	if (hw_idx == prev) {
> +		u8 next = (prev + 1) % 4;
> +
> +		surface_kbd_bl_set(&shid->kbd_bl_led, next);
> +	}
> +
> +	led_classdev_notify_brightness_hw_changed(&shid->kbd_bl_led, shid->kbd_bl_cur);
> +}
> +
> +static void surface_hid_kbd_bl_init(struct surface_hid_device *shid)
> +{
> +	u8 feat[5] = { 0xd5, 0 };
> +	u8 cur[2] = { 0xd6, 0 };
> +
> +	if (shid->hid->vendor != 0x045e || shid->hid->product != 0x09ae)
> +		return;
> +
> +	if (shid->ops.get_feature_report(shid, 0xd5, feat, sizeof(feat)) < 0 ||
> +	    feat[4] == 0)
> +		return;
> +
> +	memcpy(shid->kbd_bl_levels, &feat[1], 4);
> +	if (shid->ops.get_feature_report(shid, 0xd6, cur, sizeof(cur)) >= 0)
> +		shid->kbd_bl_cur = surface_kbd_bl_raw_to_idx(shid, cur[1]);
> +
> +	INIT_WORK(&shid->kbd_bl_work, surface_kbd_bl_work_fn);
> +	shid->kbd_bl_led.name = "surface::kbd_backlight";
> +	shid->kbd_bl_led.max_brightness = 3;
> +	shid->kbd_bl_led.flags = LED_BRIGHT_HW_CHANGED;
> +	shid->kbd_bl_led.brightness_set_blocking = surface_kbd_bl_set;
> +	shid->kbd_bl_led.brightness_get = surface_kbd_bl_get;
> +	shid->kbd_bl_led.brightness = shid->kbd_bl_cur;
> +
> +	if (led_classdev_register(shid->dev, &shid->kbd_bl_led) == 0)
> +		shid->has_kbd_bl = true;
> +}
> +
>   /* -- Common device setup. -------------------------------------------------- */
>   
>   int surface_hid_device_add(struct surface_hid_device *shid)
> @@ -218,15 +310,23 @@ int surface_hid_device_add(struct surface_hid_device *shid)
>   	shid->hid->ll_driver = &surface_hid_ll_driver;
>   
>   	status = hid_add_device(shid->hid);
> -	if (status)
> +	if (status) {
>   		hid_destroy_device(shid->hid);
> +		return status;
> +	}
>   
> -	return status;
> +	surface_hid_kbd_bl_init(shid);
> +	return 0;
>   }
>   EXPORT_SYMBOL_GPL(surface_hid_device_add);
>   
>   void surface_hid_device_destroy(struct surface_hid_device *shid)
>   {
> +	if (shid->has_kbd_bl) {
> +		cancel_work_sync(&shid->kbd_bl_work);
> +		led_classdev_unregister(&shid->kbd_bl_led);
> +		shid->has_kbd_bl = false;
> +	}
>   	hid_destroy_device(shid->hid);
>   }
>   EXPORT_SYMBOL_GPL(surface_hid_device_destroy);
> diff --git a/drivers/hid/surface-hid/surface_hid_core.h b/drivers/hid/surface-hid/surface_hid_core.h
> index 4b1a7b5..c874489 100644
> --- a/drivers/hid/surface-hid/surface_hid_core.h
> +++ b/drivers/hid/surface-hid/surface_hid_core.h
> @@ -11,8 +11,10 @@
>   #define SURFACE_HID_CORE_H
>   
>   #include <linux/hid.h>
> +#include <linux/leds.h>
>   #include <linux/pm.h>
>   #include <linux/types.h>
> +#include <linux/workqueue.h>
>   
>   #include <linux/surface_aggregator/controller.h>
>   #include <linux/surface_aggregator/device.h>
> @@ -67,6 +69,12 @@ struct surface_hid_device {
>   	struct hid_device *hid;
>   
>   	struct surface_hid_device_ops ops;
> +
> +	struct led_classdev kbd_bl_led;
> +	struct work_struct kbd_bl_work;
> +	u8 kbd_bl_levels[4];
> +	u8 kbd_bl_cur;
> +	bool has_kbd_bl;
>   };
>   
>   int surface_hid_device_add(struct surface_hid_device *shid);