| Message ID | 2B5BC9B0-F882-481C-9B09-1FF3978B655D@live.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | Touch Bar driver for Apple Macs with T2 Security Chip | expand |
On Mon, 3 Feb 2025, Aditya Garg wrote: > From: Kerem Karabay <kekrby@gmail.com> > > This commit adds a driver for the backlight of Apple Touch Bars on x86 > Macs. Note that currently only T2 Macs are supported. > > This driver is based on previous work done by Ronald Tschalär > <ronald@innovation.ch>. > > Signed-off-by: Kerem Karabay <kekrby@gmail.com> > Co-developed-by: Aditya Garg <gargaditya08@live.com> > Signed-off-by: Aditya Garg <gargaditya08@live.com> > --- > drivers/hid/Kconfig | 10 ++ > drivers/hid/Makefile | 1 + > drivers/hid/hid-appletb-bl.c | 207 +++++++++++++++++++++++++++++++++++ > drivers/hid/hid-quirks.c | 4 +- > 4 files changed, 221 insertions(+), 1 deletion(-) > create mode 100644 drivers/hid/hid-appletb-bl.c > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > index 4d2a89d65..f6678db27 100644 > --- a/drivers/hid/Kconfig > +++ b/drivers/hid/Kconfig > @@ -148,6 +148,16 @@ config HID_APPLEIR > > Say Y here if you want support for Apple infrared remote control. > > +config HID_APPLETB_BL > + tristate "Apple Touch Bar Backlight" > + depends on BACKLIGHT_CLASS_DEVICE > + help > + Say Y here if you want support for the backlight of Touch Bars on x86 > + MacBook Pros. > + > + To compile this driver as a module, choose M here: the > + module will be called hid-appletb-bl. > + > config HID_ASUS > tristate "Asus" > depends on USB_HID > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > index 24de45f36..444d24cec 100644 > --- a/drivers/hid/Makefile > +++ b/drivers/hid/Makefile > @@ -29,6 +29,7 @@ obj-$(CONFIG_HID_ALPS) += hid-alps.o > obj-$(CONFIG_HID_ACRUX) += hid-axff.o > obj-$(CONFIG_HID_APPLE) += hid-apple.o > obj-$(CONFIG_HID_APPLEIR) += hid-appleir.o > +obj-$(CONFIG_HID_APPLETB_BL) += hid-appletb-bl.o Is there a reason not to build hid-appletb-bl into hid-apple(.ko) driver proper? Otherwise the code looks good to me, but I'd prefer to have this built-in as we generally try to keep things contained in one single per-vendor HID driver. Thanks,
Hi Jiri > On 4 Feb 2025, at 3:25 AM, Jiri Kosina <jikos@kernel.org> wrote: > > On Mon, 3 Feb 2025, Aditya Garg wrote: > >> From: Kerem Karabay <kekrby@gmail.com> >> >> This commit adds a driver for the backlight of Apple Touch Bars on x86 >> Macs. Note that currently only T2 Macs are supported. >> >> This driver is based on previous work done by Ronald Tschalär >> <ronald@innovation.ch>. >> >> Signed-off-by: Kerem Karabay <kekrby@gmail.com> >> Co-developed-by: Aditya Garg <gargaditya08@live.com> >> Signed-off-by: Aditya Garg <gargaditya08@live.com> >> --- >> drivers/hid/Kconfig | 10 ++ >> drivers/hid/Makefile | 1 + >> drivers/hid/hid-appletb-bl.c | 207 +++++++++++++++++++++++++++++++++++ >> drivers/hid/hid-quirks.c | 4 +- >> 4 files changed, 221 insertions(+), 1 deletion(-) >> create mode 100644 drivers/hid/hid-appletb-bl.c >> >> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig >> index 4d2a89d65..f6678db27 100644 >> --- a/drivers/hid/Kconfig >> +++ b/drivers/hid/Kconfig >> @@ -148,6 +148,16 @@ config HID_APPLEIR >> >> Say Y here if you want support for Apple infrared remote control. >> >> +config HID_APPLETB_BL >> + tristate "Apple Touch Bar Backlight" >> + depends on BACKLIGHT_CLASS_DEVICE >> + help >> + Say Y here if you want support for the backlight of Touch Bars on x86 >> + MacBook Pros. >> + >> + To compile this driver as a module, choose M here: the >> + module will be called hid-appletb-bl. >> + >> config HID_ASUS >> tristate "Asus" >> depends on USB_HID >> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile >> index 24de45f36..444d24cec 100644 >> --- a/drivers/hid/Makefile >> +++ b/drivers/hid/Makefile >> @@ -29,6 +29,7 @@ obj-$(CONFIG_HID_ALPS) += hid-alps.o >> obj-$(CONFIG_HID_ACRUX) += hid-axff.o >> obj-$(CONFIG_HID_APPLE) += hid-apple.o >> obj-$(CONFIG_HID_APPLEIR) += hid-appleir.o >> +obj-$(CONFIG_HID_APPLETB_BL) += hid-appletb-bl.o > > Is there a reason not to build hid-appletb-bl into hid-apple(.ko) driver > proper? > You might have noticed that the hid-apple driver has code for the magic backlight. Now we have a case of MacBook Air 2020, which does not have a touchbar but still shows presence of the touchbar backlight device, just because it needs it for the keyboard backlight. In case we merge both the backlight and touchbar code in a driver, on that model, the backlight breaks. If kept separate, the touchbar driver simply doesn't load and backlight works. In the future, we also shall send another driver, which shall support the macOS mode of the touchbar, that is we can use the touchbar as a second display. It shall be a drm driver and both current and the drm driver need the touchbar backlight driver in common. > Otherwise the code looks good to me, but I'd prefer to have this built-in > as we generally try to keep things contained in one single per-vendor HID > driver. > > Thanks, > > -- > Jiri Kosina > SUSE Labs >
On Tue, 4 Feb 2025, Aditya Garg wrote: > You might have noticed that the hid-apple driver has code for the magic > backlight. Now we have a case of MacBook Air 2020, which does not have a > touchbar but still shows presence of the touchbar backlight device, just > because it needs it for the keyboard backlight. In case we merge both > the backlight and touchbar code in a driver, on that model, the > backlight breaks. If kept separate, the touchbar driver simply doesn't > load and backlight works. Sorry for being dense, but does that mean that it's either hid-appletb-bl or hid-apple, but never both to make a good user experience on those devices? If so, can you please point out what exactly is the reason? Either those have different VID/PID combination, and then it can be easily made conditional both in code and in runtime. Are we talking about conflicting VID/PID combinations, some of them needing current hid-apple, and some of them needing (in a mutually exclusive way) hid-appletb-bl? Thanks,
> On 4 Feb 2025, at 7:47 AM, Jiri Kosina <jikos@kernel.org> wrote: > > On Tue, 4 Feb 2025, Aditya Garg wrote: > >> You might have noticed that the hid-apple driver has code for the magic >> backlight. Now we have a case of MacBook Air 2020, which does not have a >> touchbar but still shows presence of the touchbar backlight device, just >> because it needs it for the keyboard backlight. In case we merge both >> the backlight and touchbar code in a driver, on that model, the >> backlight breaks. If kept separate, the touchbar driver simply doesn't >> load and backlight works. > > Sorry for being dense, but does that mean that it's either hid-appletb-bl > or hid-apple, but never both to make a good user experience on those > devices? > > If so, can you please point out what exactly is the reason? > > Either those have different VID/PID combination, and then it can be easily > made conditional both in code and in runtime. > Are we talking about conflicting VID/PID combinations, some of them > needing current hid-apple, and some of them needing (in a mutually > exclusive way) hid-appletb-bl? > The driver uses the 0th interface for managing the touchbar and the 1st interface for backlight, but there is no touchbar on MacBook Air 2020, so the 0th interface is used for backlight on that. I remember the author trying various combinations, but having a separate driver was the most feasible option, both for backlight, and the drivers for windows and macOS mode. > Thanks, > > -- > Jiri Kosina > SUSE Labs >
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index 4d2a89d65..f6678db27 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -148,6 +148,16 @@ config HID_APPLEIR Say Y here if you want support for Apple infrared remote control. +config HID_APPLETB_BL + tristate "Apple Touch Bar Backlight" + depends on BACKLIGHT_CLASS_DEVICE + help + Say Y here if you want support for the backlight of Touch Bars on x86 + MacBook Pros. + + To compile this driver as a module, choose M here: the + module will be called hid-appletb-bl. + config HID_ASUS tristate "Asus" depends on USB_HID diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile index 24de45f36..444d24cec 100644 --- a/drivers/hid/Makefile +++ b/drivers/hid/Makefile @@ -29,6 +29,7 @@ obj-$(CONFIG_HID_ALPS) += hid-alps.o obj-$(CONFIG_HID_ACRUX) += hid-axff.o obj-$(CONFIG_HID_APPLE) += hid-apple.o obj-$(CONFIG_HID_APPLEIR) += hid-appleir.o +obj-$(CONFIG_HID_APPLETB_BL) += hid-appletb-bl.o obj-$(CONFIG_HID_CREATIVE_SB0540) += hid-creative-sb0540.o obj-$(CONFIG_HID_ASUS) += hid-asus.o obj-$(CONFIG_HID_AUREAL) += hid-aureal.o diff --git a/drivers/hid/hid-appletb-bl.c b/drivers/hid/hid-appletb-bl.c new file mode 100644 index 000000000..819157686 --- /dev/null +++ b/drivers/hid/hid-appletb-bl.c @@ -0,0 +1,207 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Apple Touch Bar Backlight Driver + * + * Copyright (c) 2017-2018 Ronald Tschalär + * Copyright (c) 2022-2023 Kerem Karabay <kekrby@gmail.com> + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/hid.h> +#include <linux/backlight.h> +#include <linux/device.h> + +#include "hid-ids.h" + +#define APPLETB_BL_ON 1 +#define APPLETB_BL_DIM 3 +#define APPLETB_BL_OFF 4 + +#define HID_UP_APPLEVENDOR_TB_BL 0xff120000 + +#define HID_VD_APPLE_TB_BRIGHTNESS 0xff120001 +#define HID_USAGE_AUX1 0xff120020 +#define HID_USAGE_BRIGHTNESS 0xff120021 + +static int appletb_bl_def_brightness = 2; +module_param_named(brightness, appletb_bl_def_brightness, int, 0444); +MODULE_PARM_DESC(brightness, "Default brightness:\n" + " 0 - Touchbar is off\n" + " 1 - Dim brightness\n" + " [2] - Full brightness"); + +struct appletb_bl { + struct hid_field *aux1_field, *brightness_field; + struct backlight_device *bdev; + + bool full_on; +}; + +static const u8 appletb_bl_brightness_map[] = { + APPLETB_BL_OFF, + APPLETB_BL_DIM, + APPLETB_BL_ON, +}; + +static int appletb_bl_set_brightness(struct appletb_bl *bl, u8 brightness) +{ + struct hid_report *report = bl->brightness_field->report; + struct hid_device *hdev = report->device; + int ret; + + ret = hid_set_field(bl->aux1_field, 0, 1); + if (ret) { + hid_err(hdev, "Failed to set auxiliary field (%pe)\n", ERR_PTR(ret)); + return ret; + } + + ret = hid_set_field(bl->brightness_field, 0, brightness); + if (ret) { + hid_err(hdev, "Failed to set brightness field (%pe)\n", ERR_PTR(ret)); + return ret; + } + + if (!bl->full_on) { + ret = hid_hw_power(hdev, PM_HINT_FULLON); + if (ret < 0) { + hid_err(hdev, "Device didn't power on (%pe)\n", ERR_PTR(ret)); + return ret; + } + + bl->full_on = true; + } + + hid_hw_request(hdev, report, HID_REQ_SET_REPORT); + + if (brightness == APPLETB_BL_OFF) { + hid_hw_power(hdev, PM_HINT_NORMAL); + bl->full_on = false; + } + + return 0; +} + +static int appletb_bl_update_status(struct backlight_device *bdev) +{ + struct appletb_bl *bl = bl_get_data(bdev); + u8 brightness; + + if (backlight_is_blank(bdev)) + brightness = APPLETB_BL_OFF; + else + brightness = appletb_bl_brightness_map[backlight_get_brightness(bdev)]; + + return appletb_bl_set_brightness(bl, brightness); +} + +static const struct backlight_ops appletb_bl_backlight_ops = { + .options = BL_CORE_SUSPENDRESUME, + .update_status = appletb_bl_update_status, +}; + +static int appletb_bl_probe(struct hid_device *hdev, const struct hid_device_id *id) +{ + struct hid_field *aux1_field, *brightness_field; + struct backlight_properties bl_props = { 0 }; + struct device *dev = &hdev->dev; + struct appletb_bl *bl; + int ret; + + ret = hid_parse(hdev); + if (ret) + return dev_err_probe(dev, ret, "HID parse failed\n"); + + aux1_field = hid_find_field(hdev, HID_FEATURE_REPORT, + HID_VD_APPLE_TB_BRIGHTNESS, HID_USAGE_AUX1); + + brightness_field = hid_find_field(hdev, HID_FEATURE_REPORT, + HID_VD_APPLE_TB_BRIGHTNESS, HID_USAGE_BRIGHTNESS); + + if (!aux1_field || !brightness_field) + return -ENODEV; + + if (aux1_field->report != brightness_field->report) + return dev_err_probe(dev, -ENODEV, "Encountered unexpected report structure\n"); + + bl = devm_kzalloc(dev, sizeof(*bl), GFP_KERNEL); + if (!bl) + return -ENOMEM; + + ret = hid_hw_start(hdev, HID_CONNECT_DRIVER); + if (ret) + return dev_err_probe(dev, ret, "HID hardware start failed\n"); + + ret = hid_hw_open(hdev); + if (ret) { + dev_err_probe(dev, ret, "HID hardware open failed\n"); + goto stop_hw; + } + + bl->aux1_field = aux1_field; + bl->brightness_field = brightness_field; + + if (appletb_bl_def_brightness == 0) + ret = appletb_bl_set_brightness(bl, APPLETB_BL_OFF); + else if (appletb_bl_def_brightness == 1) + ret = appletb_bl_set_brightness(bl, APPLETB_BL_DIM); + else + ret = appletb_bl_set_brightness(bl, APPLETB_BL_ON); + + if (ret) { + dev_err_probe(dev, ret, "Failed to set touch bar brightness to off\n"); + goto close_hw; + } + + bl_props.type = BACKLIGHT_RAW; + bl_props.max_brightness = ARRAY_SIZE(appletb_bl_brightness_map) - 1; + + bl->bdev = devm_backlight_device_register(dev, "appletb_backlight", dev, bl, + &appletb_bl_backlight_ops, &bl_props); + if (IS_ERR(bl->bdev)) { + ret = PTR_ERR(bl->bdev); + dev_err_probe(dev, ret, "Failed to register backlight device\n"); + goto close_hw; + } + + hid_set_drvdata(hdev, bl); + + return 0; + +close_hw: + hid_hw_close(hdev); +stop_hw: + hid_hw_stop(hdev); + + return ret; +} + +static void appletb_bl_remove(struct hid_device *hdev) +{ + struct appletb_bl *bl = hid_get_drvdata(hdev); + + appletb_bl_set_brightness(bl, APPLETB_BL_OFF); + + hid_hw_close(hdev); + hid_hw_stop(hdev); +} + +static const struct hid_device_id appletb_bl_hid_ids[] = { + /* MacBook Pro's 2018, 2019, with T2 chip: iBridge DFR Brightness */ + { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_TOUCHBAR_BACKLIGHT) }, + { } +}; +MODULE_DEVICE_TABLE(hid, appletb_bl_hid_ids); + +static struct hid_driver appletb_bl_hid_driver = { + .name = "hid-appletb-bl", + .id_table = appletb_bl_hid_ids, + .probe = appletb_bl_probe, + .remove = appletb_bl_remove, +}; +module_hid_driver(appletb_bl_hid_driver); + +MODULE_AUTHOR("Ronald Tschalär"); +MODULE_AUTHOR("Kerem Karabay <kekrby@gmail.com>"); +MODULE_DESCRIPTION("MacBookPro Touch Bar Backlight Driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c index e0bbf0c63..818d41a35 100644 --- a/drivers/hid/hid-quirks.c +++ b/drivers/hid/hid-quirks.c @@ -328,7 +328,6 @@ static const struct hid_device_id hid_have_special_driver[] = { { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY) }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_2021) }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_2021) }, - { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_TOUCHBAR_BACKLIGHT) }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_TOUCHBAR_DISPLAY) }, #endif #if IS_ENABLED(CONFIG_HID_APPLEIR) @@ -338,6 +337,9 @@ static const struct hid_device_id hid_have_special_driver[] = { { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL4) }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL5) }, #endif +#if IS_ENABLED(CONFIG_HID_APPLETB_BL) + { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_TOUCHBAR_BACKLIGHT) }, +#endif #if IS_ENABLED(CONFIG_HID_ASUS) { HID_I2C_DEVICE(USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_I2C_KEYBOARD) }, { HID_I2C_DEVICE(USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_I2C_TOUCHPAD) },