| Message ID | 20230701120806.11812-2-julius@zint.sh (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Backlight driver for the Apple Studio Display | expand |
On 1.07.2023 14:08, Julius Zint wrote: > The Apple Studio Display does not have any physical buttons and the only > way to get or set the brightness is by sending USB control transfers to a > HID device exposed by the display. > > These control transfers take the form of a HID_(GET|SET)_REPORT request > and the payload looks like this: > > struct brightness_ctrl_message_data { > u8 unknown_1; > __le16 brightness; > u8 unkown_2[4]; > } __packed; > > When compiled as a module this driver needs to be part of the early boot > environment, otherwise the generic USB HID driver will claim the device. > > Signed-off-by: Julius Zint <julius@zint.sh> Few comments below. > --- > drivers/video/backlight/Kconfig | 8 + > drivers/video/backlight/Makefile | 1 + > drivers/video/backlight/apple_bl_usb.c | 264 +++++++++++++++++++++++++ > 3 files changed, 273 insertions(+) > create mode 100644 drivers/video/backlight/apple_bl_usb.c > > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig > index 51387b1ef012..9383d402ebed 100644 > --- a/drivers/video/backlight/Kconfig > +++ b/drivers/video/backlight/Kconfig > @@ -290,6 +290,14 @@ config BACKLIGHT_APPLE > If you have an Intel-based Apple say Y to enable a driver for its > backlight. > > +config BACKLIGHT_APPLE_USB > + tristate "Apple USB Backlight Driver" > + depends on USB > + help > + If you have an external display from Apple that is attached via USB > + say Y to enable a driver for its backlight. Currently it supports the > + Apple Studio Display. > + > config BACKLIGHT_QCOM_WLED > tristate "Qualcomm PMIC WLED Driver" > select REGMAP > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile > index f72e1c3c59e9..c42880655113 100644 > --- a/drivers/video/backlight/Makefile > +++ b/drivers/video/backlight/Makefile > @@ -23,6 +23,7 @@ obj-$(CONFIG_BACKLIGHT_ADP5520) += adp5520_bl.o > obj-$(CONFIG_BACKLIGHT_ADP8860) += adp8860_bl.o > obj-$(CONFIG_BACKLIGHT_ADP8870) += adp8870_bl.o > obj-$(CONFIG_BACKLIGHT_APPLE) += apple_bl.o > +obj-$(CONFIG_BACKLIGHT_APPLE_USB) += apple_bl_usb.o > obj-$(CONFIG_BACKLIGHT_AS3711) += as3711_bl.o > obj-$(CONFIG_BACKLIGHT_BD6107) += bd6107.o > obj-$(CONFIG_BACKLIGHT_CARILLO_RANCH) += cr_bllcd.o > diff --git a/drivers/video/backlight/apple_bl_usb.c b/drivers/video/backlight/apple_bl_usb.c > new file mode 100644 > index 000000000000..b746b7822974 > --- /dev/null > +++ b/drivers/video/backlight/apple_bl_usb.c > @@ -0,0 +1,264 @@ > +// SPDX-License-Identifier: GPL-2.0 OR MIT > +#include <linux/init.h> > +#include <linux/module.h> > +#include <linux/usb.h> > +#include <linux/backlight.h> > +#include <asm/byteorder.h> > + > +#define APPLE_STUDIO_DISPLAY_VENDOR_ID 0x05ac > +#define APPLE_STUDIO_DISPLAY_PRODUCT_ID 0x1114 > + > +#define HID_GET_REPORT 0x01 > +#define HID_SET_REPORT 0x09 > + > +#define HID_REPORT_TYPE_FEATURE 0x0300 > + > +struct apple_bl_usb_data { > + struct usb_interface *usb_interface; > + struct usb_device *usb_dev; > +}; > + > +struct brightness_ctrl_message_data { > + u8 unknown_1; > + __le16 brightness; > + u8 unkown_2[4]; Typo > +} __packed; > + > +void init_ctrl_msg_data(struct brightness_ctrl_message_data *msg) > +{ > + memset(msg, 0, sizeof(struct brightness_ctrl_message_data)); > + msg->unknown_1 = 0x01; > +} > + > +void set_ctrl_message_brightness(struct brightness_ctrl_message_data *msg, > + u16 brightness_value) > +{ > + msg->brightness = cpu_to_le16(brightness_value + 400); > +} > + > +u16 get_ctrl_message_brightness(struct brightness_ctrl_message_data *msg) > +{ > + return le16_to_cpu(msg->brightness) - 400; > +} > + > +int apple_bl_usb_usb_get_brightness(struct usb_interface *interface, > + struct usb_device *usb_dev, > + int *brightness) > +{ > + int err; > + u16 interface_nr; > + int msg_data_size; > + struct brightness_ctrl_message_data *msg_data; > + > + msg_data_size = sizeof(struct brightness_ctrl_message_data); > + msg_data = kzalloc(msg_data_size, GFP_KERNEL); Check for NULL as kzalloc() may fail > + memset(msg_data, 0x00, msg_data_size); > + interface_nr = interface->cur_altsetting->desc.bInterfaceNumber; > + > + err = usb_control_msg(usb_dev, > + usb_rcvctrlpipe(usb_dev, 0), > + HID_GET_REPORT, > + USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE, > + /* wValue: HID-Report Type and Report ID */ > + HID_REPORT_TYPE_FEATURE | 0x01, > + interface_nr /* wIndex */, > + msg_data, > + msg_data_size, > + HZ); > + if (err < 0) { > + dev_err(&interface->dev, > + "get: usb control message err: %d\n", > + err); Shouldn't you kfree() and return err here? > + } > + *brightness = get_ctrl_message_brightness(msg_data); > + kfree(msg_data); > + dev_dbg(&interface->dev, "get brightness: %d\n", *brightness); > + return 0; > +} > + > +int apple_bl_usb_usb_set_brightness(struct usb_interface *interface, > + struct usb_device *usb_dev, > + int brightness) > +{ > + int err; > + u16 interface_nr; > + struct brightness_ctrl_message_data *msg_data; > + > + msg_data = kzalloc(sizeof(struct brightness_ctrl_message_data), GFP_KERNEL); Same here > + interface_nr = interface->cur_altsetting->desc.bInterfaceNumber; > + init_ctrl_msg_data(msg_data); > + set_ctrl_message_brightness(msg_data, brightness); > + > + err = usb_control_msg(usb_dev, > + usb_sndctrlpipe(usb_dev, 0), > + HID_SET_REPORT, > + USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE, > + /* wValue: HID-Report Type and Report ID */ > + HID_REPORT_TYPE_FEATURE | 0x01, > + interface_nr /* wIndex */, > + msg_data, > + sizeof(struct brightness_ctrl_message_data), > + HZ); > + kfree(msg_data); > + if (err < 0) { > + dev_err(&interface->dev, > + "set: usb control message err: %d\n", > + err); > + return err; > + } > + dev_dbg(&interface->dev, "set brightness: %d\n", brightness); > + return 0; > +} > + > +int apple_bl_usb_check_fb(struct backlight_device *bd, struct fb_info *info) > +{ > + dev_info(&bd->dev, "check fb\n"); > + return 0; > +} Do we need that function / print? > + > +int apple_bl_usb_get_brightness(struct backlight_device *bl) > +{ > + int ret; > + struct apple_bl_usb_data *data; > + int hw_brightness; > + > + data = bl_get_data(bl); > + ret = apple_bl_usb_usb_get_brightness(data->usb_interface, > + data->usb_dev, > + &hw_brightness); > + if (!ret) > + ret = hw_brightness; > + > + return ret; > +} > + > +int apple_bl_usb_update_status(struct backlight_device *bl) > +{ > + int err; > + struct apple_bl_usb_data *data; > + > + data = bl_get_data(bl); > + err = apple_bl_usb_usb_set_brightness(data->usb_interface, > + data->usb_dev, > + bl->props.brightness); > + return err; > +} > + > +static const struct backlight_ops apple_bl_usb_ops = { > + .update_status = apple_bl_usb_update_status, > + .get_brightness = apple_bl_usb_get_brightness, > + .check_fb = apple_bl_usb_check_fb, > +}; > + > +static void apple_bl_usb_disconnect(struct usb_interface *interface) > +{ > + struct backlight_device *bl; > + > + dev_dbg(&interface->dev, "disconnect\n"); > + > + bl = usb_get_intfdata(interface); > + usb_set_intfdata(interface, NULL); > + backlight_device_unregister(bl); > +} > + > +static int apple_bl_usb_probe(struct usb_interface *interface, > + const struct usb_device_id *id) > +{ > + struct backlight_properties props; > + struct backlight_device *bl; > + struct usb_device *usb_dev; > + struct device *dev; > + struct apple_bl_usb_data *data; > + int brightness_interface_nr; > + > + dev_dbg(&interface->dev, "probe\n"); > + > + dev = &interface->dev; > + usb_dev = interface_to_usbdev(interface); > + > + switch (usb_dev->config->desc.bConfigurationValue) { > + case 1: > + brightness_interface_nr = 0x7; > + break; > + case 2: > + brightness_interface_nr = 0x9; > + break; > + case 3: > + brightness_interface_nr = 0xc; > + break; > + default: > + dev_err(dev, > + "unexpected configuration value: %d\n", > + usb_dev->config->desc.bConfigurationValue); > + return -EINVAL; > + } > + > + if (interface->cur_altsetting->desc.bInterfaceNumber != brightness_interface_nr) > + return -ENODEV; > + > + data = devm_kzalloc(dev, > + sizeof(struct apple_bl_usb_data), > + GFP_KERNEL); I don't think devm_kzalloc() can return error pointer. Just check for NULL below. > + if (IS_ERR(data)) { > + dev_err(dev, "failed to allocate memory\n"); > + return PTR_ERR(bl); > + } > + data->usb_interface = interface; > + data->usb_dev = usb_dev; > + > + // Valid brightness values for the apple studio display range from 400 > + // to 60000. Since the backlight subsystem´s brightness value starts > + // from 0, we use 0 to 59600 and offset it by the minimum value. > + memset(&props, 0, sizeof(props)); > + props.type = BACKLIGHT_RAW; > + props.max_brightness = 59600; > + > + bl = backlight_device_register("apple_studio_display", > + dev, > + data, > + &apple_bl_usb_ops, > + &props); > + if (IS_ERR(bl)) { > + dev_err(dev, "failed to register backlight\n"); > + return PTR_ERR(bl); > + } > + usb_set_intfdata(interface, bl); > + return 0; > +} > + > +static int apple_bl_usb_suspend(struct usb_interface *interface, > + pm_message_t message) > +{ > + dev_dbg(&interface->dev, "suspend\n"); > + return 0; > +} > + > +static int apple_bl_usb_resume(struct usb_interface *interface) > +{ > + dev_dbg(&interface->dev, "resume\n"); > + return 0; > +} > + > +static const struct usb_device_id id_table[] = { > + { > + .idVendor = APPLE_STUDIO_DISPLAY_VENDOR_ID, > + .idProduct = APPLE_STUDIO_DISPLAY_PRODUCT_ID, > + }, > + {}, > +}; > +MODULE_DEVICE_TABLE(usb, id_table); > + > +static struct usb_driver usb_asdbl_driver = { > + .name = "apple_bl_usb", > + .probe = apple_bl_usb_probe, > + .disconnect = apple_bl_usb_disconnect, > + .id_table = id_table, > + .suspend = apple_bl_usb_suspend, > + .resume = apple_bl_usb_resume, > + .reset_resume = apple_bl_usb_resume > +}; > +module_usb_driver(usb_asdbl_driver); > + > +MODULE_AUTHOR("Julius Zint <julius@zint.sh>"); > +MODULE_LICENSE("Dual MIT/GPL"); > +MODULE_DESCRIPTION("Backlight control for USB-C Thunderbolt Apple displays");
Hi Julius, kernel test robot noticed the following build warnings: [auto build test WARNING on lee-backlight/for-backlight-next] [also build test WARNING on lee-leds/for-leds-next drm-misc/drm-misc-next drm-tip/drm-tip linus/master v6.4 next-20230630] [cannot apply to lee-backlight/for-backlight-fixes] [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/Julius-Zint/backlight-apple_bl_usb-Add-Apple-Studio-Display-support/20230701-202142 base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git for-backlight-next patch link: https://lore.kernel.org/r/20230701120806.11812-2-julius%40zint.sh patch subject: [PATCH 1/1] backlight: apple_bl_usb: Add Apple Studio Display support config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20230701/202307012107.OW4d1gBR-lkp@intel.com/config) compiler: m68k-linux-gcc (GCC) 12.3.0 reproduce: (https://download.01.org/0day-ci/archive/20230701/202307012107.OW4d1gBR-lkp@intel.com/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 <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202307012107.OW4d1gBR-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/video/backlight/apple_bl_usb.c:27:6: warning: no previous prototype for 'init_ctrl_msg_data' [-Wmissing-prototypes] 27 | void init_ctrl_msg_data(struct brightness_ctrl_message_data *msg) | ^~~~~~~~~~~~~~~~~~ >> drivers/video/backlight/apple_bl_usb.c:33:6: warning: no previous prototype for 'set_ctrl_message_brightness' [-Wmissing-prototypes] 33 | void set_ctrl_message_brightness(struct brightness_ctrl_message_data *msg, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/video/backlight/apple_bl_usb.c:39:5: warning: no previous prototype for 'get_ctrl_message_brightness' [-Wmissing-prototypes] 39 | u16 get_ctrl_message_brightness(struct brightness_ctrl_message_data *msg) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/video/backlight/apple_bl_usb.c:44:5: warning: no previous prototype for 'apple_bl_usb_usb_get_brightness' [-Wmissing-prototypes] 44 | int apple_bl_usb_usb_get_brightness(struct usb_interface *interface, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/video/backlight/apple_bl_usb.c:79:5: warning: no previous prototype for 'apple_bl_usb_usb_set_brightness' [-Wmissing-prototypes] 79 | int apple_bl_usb_usb_set_brightness(struct usb_interface *interface, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/video/backlight/apple_bl_usb.c:113:5: warning: no previous prototype for 'apple_bl_usb_check_fb' [-Wmissing-prototypes] 113 | int apple_bl_usb_check_fb(struct backlight_device *bd, struct fb_info *info) | ^~~~~~~~~~~~~~~~~~~~~ >> drivers/video/backlight/apple_bl_usb.c:119:5: warning: no previous prototype for 'apple_bl_usb_get_brightness' [-Wmissing-prototypes] 119 | int apple_bl_usb_get_brightness(struct backlight_device *bl) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/video/backlight/apple_bl_usb.c:135:5: warning: no previous prototype for 'apple_bl_usb_update_status' [-Wmissing-prototypes] 135 | int apple_bl_usb_update_status(struct backlight_device *bl) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ vim +/init_ctrl_msg_data +27 drivers/video/backlight/apple_bl_usb.c 26 > 27 void init_ctrl_msg_data(struct brightness_ctrl_message_data *msg) 28 { 29 memset(msg, 0, sizeof(struct brightness_ctrl_message_data)); 30 msg->unknown_1 = 0x01; 31 } 32 > 33 void set_ctrl_message_brightness(struct brightness_ctrl_message_data *msg, 34 u16 brightness_value) 35 { 36 msg->brightness = cpu_to_le16(brightness_value + 400); 37 } 38 > 39 u16 get_ctrl_message_brightness(struct brightness_ctrl_message_data *msg) 40 { 41 return le16_to_cpu(msg->brightness) - 400; 42 } 43 > 44 int apple_bl_usb_usb_get_brightness(struct usb_interface *interface, 45 struct usb_device *usb_dev, 46 int *brightness) 47 { 48 int err; 49 u16 interface_nr; 50 int msg_data_size; 51 struct brightness_ctrl_message_data *msg_data; 52 53 msg_data_size = sizeof(struct brightness_ctrl_message_data); 54 msg_data = kzalloc(msg_data_size, GFP_KERNEL); 55 memset(msg_data, 0x00, msg_data_size); 56 interface_nr = interface->cur_altsetting->desc.bInterfaceNumber; 57 58 err = usb_control_msg(usb_dev, 59 usb_rcvctrlpipe(usb_dev, 0), 60 HID_GET_REPORT, 61 USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE, 62 /* wValue: HID-Report Type and Report ID */ 63 HID_REPORT_TYPE_FEATURE | 0x01, 64 interface_nr /* wIndex */, 65 msg_data, 66 msg_data_size, 67 HZ); 68 if (err < 0) { 69 dev_err(&interface->dev, 70 "get: usb control message err: %d\n", 71 err); 72 } 73 *brightness = get_ctrl_message_brightness(msg_data); 74 kfree(msg_data); 75 dev_dbg(&interface->dev, "get brightness: %d\n", *brightness); 76 return 0; 77 } 78 > 79 int apple_bl_usb_usb_set_brightness(struct usb_interface *interface, 80 struct usb_device *usb_dev, 81 int brightness) 82 { 83 int err; 84 u16 interface_nr; 85 struct brightness_ctrl_message_data *msg_data; 86 87 msg_data = kzalloc(sizeof(struct brightness_ctrl_message_data), GFP_KERNEL); 88 interface_nr = interface->cur_altsetting->desc.bInterfaceNumber; 89 init_ctrl_msg_data(msg_data); 90 set_ctrl_message_brightness(msg_data, brightness); 91 92 err = usb_control_msg(usb_dev, 93 usb_sndctrlpipe(usb_dev, 0), 94 HID_SET_REPORT, 95 USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE, 96 /* wValue: HID-Report Type and Report ID */ 97 HID_REPORT_TYPE_FEATURE | 0x01, 98 interface_nr /* wIndex */, 99 msg_data, 100 sizeof(struct brightness_ctrl_message_data), 101 HZ); 102 kfree(msg_data); 103 if (err < 0) { 104 dev_err(&interface->dev, 105 "set: usb control message err: %d\n", 106 err); 107 return err; 108 } 109 dev_dbg(&interface->dev, "set brightness: %d\n", brightness); 110 return 0; 111 } 112 > 113 int apple_bl_usb_check_fb(struct backlight_device *bd, struct fb_info *info) 114 { 115 dev_info(&bd->dev, "check fb\n"); 116 return 0; 117 } 118 > 119 int apple_bl_usb_get_brightness(struct backlight_device *bl) 120 { 121 int ret; 122 struct apple_bl_usb_data *data; 123 int hw_brightness; 124 125 data = bl_get_data(bl); 126 ret = apple_bl_usb_usb_get_brightness(data->usb_interface, 127 data->usb_dev, 128 &hw_brightness); 129 if (!ret) 130 ret = hw_brightness; 131 132 return ret; 133 } 134 > 135 int apple_bl_usb_update_status(struct backlight_device *bl) 136 { 137 int err; 138 struct apple_bl_usb_data *data; 139 140 data = bl_get_data(bl); 141 err = apple_bl_usb_usb_set_brightness(data->usb_interface, 142 data->usb_dev, 143 bl->props.brightness); 144 return err; 145 } 146
Hi Julius. Thanks for posting this. I few nits in the following where you as the author decide what to ignore and what to update. Sam On Sat, Jul 01, 2023 at 02:08:03PM +0200, Julius Zint wrote: > The Apple Studio Display does not have any physical buttons and the only > way to get or set the brightness is by sending USB control transfers to a > HID device exposed by the display. > > These control transfers take the form of a HID_(GET|SET)_REPORT request > and the payload looks like this: > > struct brightness_ctrl_message_data { > u8 unknown_1; > __le16 brightness; > u8 unkown_2[4]; > } __packed; > > When compiled as a module this driver needs to be part of the early boot > environment, otherwise the generic USB HID driver will claim the device. I hope someone else can help here, as I have no clue. > > Signed-off-by: Julius Zint <julius@zint.sh> > --- > drivers/video/backlight/Kconfig | 8 + > drivers/video/backlight/Makefile | 1 + > drivers/video/backlight/apple_bl_usb.c | 264 +++++++++++++++++++++++++ > 3 files changed, 273 insertions(+) > create mode 100644 drivers/video/backlight/apple_bl_usb.c > > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig > index 51387b1ef012..9383d402ebed 100644 > --- a/drivers/video/backlight/Kconfig > +++ b/drivers/video/backlight/Kconfig > @@ -290,6 +290,14 @@ config BACKLIGHT_APPLE > If you have an Intel-based Apple say Y to enable a driver for its > backlight. > > +config BACKLIGHT_APPLE_USB > + tristate "Apple USB Backlight Driver" > + depends on USB > + help > + If you have an external display from Apple that is attached via USB > + say Y to enable a driver for its backlight. Currently it supports the > + Apple Studio Display. > + > config BACKLIGHT_QCOM_WLED > tristate "Qualcomm PMIC WLED Driver" > select REGMAP > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile > index f72e1c3c59e9..c42880655113 100644 > --- a/drivers/video/backlight/Makefile > +++ b/drivers/video/backlight/Makefile > @@ -23,6 +23,7 @@ obj-$(CONFIG_BACKLIGHT_ADP5520) += adp5520_bl.o > obj-$(CONFIG_BACKLIGHT_ADP8860) += adp8860_bl.o > obj-$(CONFIG_BACKLIGHT_ADP8870) += adp8870_bl.o > obj-$(CONFIG_BACKLIGHT_APPLE) += apple_bl.o > +obj-$(CONFIG_BACKLIGHT_APPLE_USB) += apple_bl_usb.o > obj-$(CONFIG_BACKLIGHT_AS3711) += as3711_bl.o > obj-$(CONFIG_BACKLIGHT_BD6107) += bd6107.o > obj-$(CONFIG_BACKLIGHT_CARILLO_RANCH) += cr_bllcd.o > diff --git a/drivers/video/backlight/apple_bl_usb.c b/drivers/video/backlight/apple_bl_usb.c > new file mode 100644 > index 000000000000..b746b7822974 > --- /dev/null > +++ b/drivers/video/backlight/apple_bl_usb.c > @@ -0,0 +1,264 @@ > +// SPDX-License-Identifier: GPL-2.0 OR MIT > +#include <linux/init.h> > +#include <linux/module.h> > +#include <linux/usb.h> > +#include <linux/backlight.h> > +#include <asm/byteorder.h> > + > +#define APPLE_STUDIO_DISPLAY_VENDOR_ID 0x05ac > +#define APPLE_STUDIO_DISPLAY_PRODUCT_ID 0x1114 > + > +#define HID_GET_REPORT 0x01 > +#define HID_SET_REPORT 0x09 > + > +#define HID_REPORT_TYPE_FEATURE 0x0300 > + > +struct apple_bl_usb_data { > + struct usb_interface *usb_interface; > + struct usb_device *usb_dev; > +}; > + > +struct brightness_ctrl_message_data { > + u8 unknown_1; > + __le16 brightness; > + u8 unkown_2[4]; > +} __packed; A different way to set the brightness value could be: struct brightness_ctrl_message_data { u8 cmd; u8[2] brightness; u8 unknown[4]; } __packed; static void set_ctrl_message_brightness(struct brightness_ctrl_message_data *msg, u16 brightness_value) { u16 brightness = brightness_value + 400; msg->brightness[0] = brightness & 0xff; msg->brightness[2] = brightness >> 8; } This is similar to what is done in drm_mipi_dsi. Other backlight drivers (except one) uses similar tricks to handle when the brightness is more than one byte. The magic number 400 would be better represented by a constant like: #define BACKLIGHT_INTENSITY_OFFSET 400 Or something like this. It also from the code looks like unknown_1 is a command byte. #define GET_BACKLIGHT_INTENSITY 0x0 #define SET_BACKLIGHT_INTENSITY 0x1 Looks more descriptive than the current hard coding, implicit via memset or explicit. > +void init_ctrl_msg_data(struct brightness_ctrl_message_data *msg) > +{ > + memset(msg, 0, sizeof(struct brightness_ctrl_message_data)); > + msg->unknown_1 = 0x01; > +} As the build bot already told you, please use static everywhere possible. In this case just drop the helper as it has only one user. > + > +void set_ctrl_message_brightness(struct brightness_ctrl_message_data *msg, > + u16 brightness_value) > +{ > + msg->brightness = cpu_to_le16(brightness_value + 400); > +} > + > +u16 get_ctrl_message_brightness(struct brightness_ctrl_message_data *msg) > +{ > + return le16_to_cpu(msg->brightness) - 400; > +} > + > +int apple_bl_usb_usb_get_brightness(struct usb_interface *interface, > + struct usb_device *usb_dev, > + int *brightness) > +{ > + int err; > + u16 interface_nr; > + int msg_data_size; > + struct brightness_ctrl_message_data *msg_data; > + > + msg_data_size = sizeof(struct brightness_ctrl_message_data); > + msg_data = kzalloc(msg_data_size, GFP_KERNEL); The struct is so small that you can safely have it as a local variable. The pointer is almost the same size. And then there is no need to check for failing allocations. > + memset(msg_data, 0x00, msg_data_size); > + interface_nr = interface->cur_altsetting->desc.bInterfaceNumber; > + > + err = usb_control_msg(usb_dev, > + usb_rcvctrlpipe(usb_dev, 0), > + HID_GET_REPORT, > + USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE, > + /* wValue: HID-Report Type and Report ID */ > + HID_REPORT_TYPE_FEATURE | 0x01, > + interface_nr /* wIndex */, > + msg_data, > + msg_data_size, > + HZ); Consider specifying the number of msec to wait, rather than the platform dependent HZ value that may or may not work. I found: #define USB_CTRL_GET_TIMEOUT 5000 #define USB_CTRL_SET_TIMEOUT 5000 They look like the right choices here. > + if (err < 0) { > + dev_err(&interface->dev, > + "get: usb control message err: %d\n", > + err); > + } > + *brightness = get_ctrl_message_brightness(msg_data); Should this check the first byte that I assume is a command byte? > + kfree(msg_data); > + dev_dbg(&interface->dev, "get brightness: %d\n", *brightness); > + return 0; > +} > + > +int apple_bl_usb_usb_set_brightness(struct usb_interface *interface, > + struct usb_device *usb_dev, > + int brightness) > +{ > + int err; > + u16 interface_nr; > + struct brightness_ctrl_message_data *msg_data; > + > + msg_data = kzalloc(sizeof(struct brightness_ctrl_message_data), GFP_KERNEL); As above, declare it on the stack. > + interface_nr = interface->cur_altsetting->desc.bInterfaceNumber; > + init_ctrl_msg_data(msg_data); > + set_ctrl_message_brightness(msg_data, brightness); > + > + err = usb_control_msg(usb_dev, > + usb_sndctrlpipe(usb_dev, 0), > + HID_SET_REPORT, > + USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE, > + /* wValue: HID-Report Type and Report ID */ > + HID_REPORT_TYPE_FEATURE | 0x01, > + interface_nr /* wIndex */, > + msg_data, > + sizeof(struct brightness_ctrl_message_data), > + HZ); > + kfree(msg_data); > + if (err < 0) { > + dev_err(&interface->dev, > + "set: usb control message err: %d\n", > + err); > + return err; > + } > + dev_dbg(&interface->dev, "set brightness: %d\n", brightness); > + return 0; > +} > + > +int apple_bl_usb_check_fb(struct backlight_device *bd, struct fb_info *info) > +{ > + dev_info(&bd->dev, "check fb\n"); > + return 0; > +} > + > +int apple_bl_usb_get_brightness(struct backlight_device *bl) > +{ > + int ret; > + struct apple_bl_usb_data *data; > + int hw_brightness; > + > + data = bl_get_data(bl); > + ret = apple_bl_usb_usb_get_brightness(data->usb_interface, > + data->usb_dev, > + &hw_brightness); > + if (!ret) > + ret = hw_brightness; > + > + return ret; > +} > + > +int apple_bl_usb_update_status(struct backlight_device *bl) > +{ > + int err; > + struct apple_bl_usb_data *data; > + > + data = bl_get_data(bl); > + err = apple_bl_usb_usb_set_brightness(data->usb_interface, > + data->usb_dev, > + bl->props.brightness); Here you should replace bl->props.brightness with backlight_get_brightness(bl). This will give you the value 0 when the backlight device is power down or blank which is often the right thing. > + return err; > +} > + > +static const struct backlight_ops apple_bl_usb_ops = { > + .update_status = apple_bl_usb_update_status, > + .get_brightness = apple_bl_usb_get_brightness, > + .check_fb = apple_bl_usb_check_fb, > +}; > + > +static void apple_bl_usb_disconnect(struct usb_interface *interface) > +{ > + struct backlight_device *bl; > + > + dev_dbg(&interface->dev, "disconnect\n"); > + > + bl = usb_get_intfdata(interface); > + usb_set_intfdata(interface, NULL); > + backlight_device_unregister(bl); > +} > + > +static int apple_bl_usb_probe(struct usb_interface *interface, > + const struct usb_device_id *id) > +{ > + struct backlight_properties props; > + struct backlight_device *bl; > + struct usb_device *usb_dev; > + struct device *dev; > + struct apple_bl_usb_data *data; > + int brightness_interface_nr; > + > + dev_dbg(&interface->dev, "probe\n"); > + > + dev = &interface->dev; > + usb_dev = interface_to_usbdev(interface); > + > + switch (usb_dev->config->desc.bConfigurationValue) { > + case 1: > + brightness_interface_nr = 0x7; > + break; > + case 2: > + brightness_interface_nr = 0x9; > + break; > + case 3: > + brightness_interface_nr = 0xc; > + break; > + default: > + dev_err(dev, > + "unexpected configuration value: %d\n", > + usb_dev->config->desc.bConfigurationValue); > + return -EINVAL; Use dev_err_probe() here. > + } > + > + if (interface->cur_altsetting->desc.bInterfaceNumber != brightness_interface_nr) > + return -ENODEV; Same here, so you also log something. > + > + data = devm_kzalloc(dev, > + sizeof(struct apple_bl_usb_data), > + GFP_KERNEL); > + if (IS_ERR(data)) { > + dev_err(dev, "failed to allocate memory\n"); > + return PTR_ERR(bl); Same here. > + } > + data->usb_interface = interface; > + data->usb_dev = usb_dev; > + > + // Valid brightness values for the apple studio display range from 400 > + // to 60000. Since the backlight subsystem´s brightness value starts > + // from 0, we use 0 to 59600 and offset it by the minimum value. > + memset(&props, 0, sizeof(props)); > + props.type = BACKLIGHT_RAW; > + props.max_brightness = 59600; > + > + bl = backlight_device_register("apple_studio_display", > + dev, > + data, > + &apple_bl_usb_ops, > + &props); Any particular reason NOT to use devm_backlight_device_register()? > + if (IS_ERR(bl)) { > + dev_err(dev, "failed to register backlight\n"); > + return PTR_ERR(bl); > + } > + usb_set_intfdata(interface, bl); > + return 0; > +} > + > +static int apple_bl_usb_suspend(struct usb_interface *interface, > + pm_message_t message) > +{ > + dev_dbg(&interface->dev, "suspend\n"); > + return 0; > +} > + > +static int apple_bl_usb_resume(struct usb_interface *interface) > +{ > + dev_dbg(&interface->dev, "resume\n"); > + return 0; > +} > + > +static const struct usb_device_id id_table[] = { > + { > + .idVendor = APPLE_STUDIO_DISPLAY_VENDOR_ID, > + .idProduct = APPLE_STUDIO_DISPLAY_PRODUCT_ID, > + }, > + {}, > +}; > +MODULE_DEVICE_TABLE(usb, id_table); > + > +static struct usb_driver usb_asdbl_driver = { > + .name = "apple_bl_usb", > + .probe = apple_bl_usb_probe, > + .disconnect = apple_bl_usb_disconnect, > + .id_table = id_table, > + .suspend = apple_bl_usb_suspend, > + .resume = apple_bl_usb_resume, > + .reset_resume = apple_bl_usb_resume > +}; > +module_usb_driver(usb_asdbl_driver); > + > +MODULE_AUTHOR("Julius Zint <julius@zint.sh>"); > +MODULE_LICENSE("Dual MIT/GPL"); > +MODULE_DESCRIPTION("Backlight control for USB-C Thunderbolt Apple displays"); > -- > 2.41.0
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig index 51387b1ef012..9383d402ebed 100644 --- a/drivers/video/backlight/Kconfig +++ b/drivers/video/backlight/Kconfig @@ -290,6 +290,14 @@ config BACKLIGHT_APPLE If you have an Intel-based Apple say Y to enable a driver for its backlight. +config BACKLIGHT_APPLE_USB + tristate "Apple USB Backlight Driver" + depends on USB + help + If you have an external display from Apple that is attached via USB + say Y to enable a driver for its backlight. Currently it supports the + Apple Studio Display. + config BACKLIGHT_QCOM_WLED tristate "Qualcomm PMIC WLED Driver" select REGMAP diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile index f72e1c3c59e9..c42880655113 100644 --- a/drivers/video/backlight/Makefile +++ b/drivers/video/backlight/Makefile @@ -23,6 +23,7 @@ obj-$(CONFIG_BACKLIGHT_ADP5520) += adp5520_bl.o obj-$(CONFIG_BACKLIGHT_ADP8860) += adp8860_bl.o obj-$(CONFIG_BACKLIGHT_ADP8870) += adp8870_bl.o obj-$(CONFIG_BACKLIGHT_APPLE) += apple_bl.o +obj-$(CONFIG_BACKLIGHT_APPLE_USB) += apple_bl_usb.o obj-$(CONFIG_BACKLIGHT_AS3711) += as3711_bl.o obj-$(CONFIG_BACKLIGHT_BD6107) += bd6107.o obj-$(CONFIG_BACKLIGHT_CARILLO_RANCH) += cr_bllcd.o diff --git a/drivers/video/backlight/apple_bl_usb.c b/drivers/video/backlight/apple_bl_usb.c new file mode 100644 index 000000000000..b746b7822974 --- /dev/null +++ b/drivers/video/backlight/apple_bl_usb.c @@ -0,0 +1,264 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT +#include <linux/init.h> +#include <linux/module.h> +#include <linux/usb.h> +#include <linux/backlight.h> +#include <asm/byteorder.h> + +#define APPLE_STUDIO_DISPLAY_VENDOR_ID 0x05ac +#define APPLE_STUDIO_DISPLAY_PRODUCT_ID 0x1114 + +#define HID_GET_REPORT 0x01 +#define HID_SET_REPORT 0x09 + +#define HID_REPORT_TYPE_FEATURE 0x0300 + +struct apple_bl_usb_data { + struct usb_interface *usb_interface; + struct usb_device *usb_dev; +}; + +struct brightness_ctrl_message_data { + u8 unknown_1; + __le16 brightness; + u8 unkown_2[4]; +} __packed; + +void init_ctrl_msg_data(struct brightness_ctrl_message_data *msg) +{ + memset(msg, 0, sizeof(struct brightness_ctrl_message_data)); + msg->unknown_1 = 0x01; +} + +void set_ctrl_message_brightness(struct brightness_ctrl_message_data *msg, + u16 brightness_value) +{ + msg->brightness = cpu_to_le16(brightness_value + 400); +} + +u16 get_ctrl_message_brightness(struct brightness_ctrl_message_data *msg) +{ + return le16_to_cpu(msg->brightness) - 400; +} + +int apple_bl_usb_usb_get_brightness(struct usb_interface *interface, + struct usb_device *usb_dev, + int *brightness) +{ + int err; + u16 interface_nr; + int msg_data_size; + struct brightness_ctrl_message_data *msg_data; + + msg_data_size = sizeof(struct brightness_ctrl_message_data); + msg_data = kzalloc(msg_data_size, GFP_KERNEL); + memset(msg_data, 0x00, msg_data_size); + interface_nr = interface->cur_altsetting->desc.bInterfaceNumber; + + err = usb_control_msg(usb_dev, + usb_rcvctrlpipe(usb_dev, 0), + HID_GET_REPORT, + USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE, + /* wValue: HID-Report Type and Report ID */ + HID_REPORT_TYPE_FEATURE | 0x01, + interface_nr /* wIndex */, + msg_data, + msg_data_size, + HZ); + if (err < 0) { + dev_err(&interface->dev, + "get: usb control message err: %d\n", + err); + } + *brightness = get_ctrl_message_brightness(msg_data); + kfree(msg_data); + dev_dbg(&interface->dev, "get brightness: %d\n", *brightness); + return 0; +} + +int apple_bl_usb_usb_set_brightness(struct usb_interface *interface, + struct usb_device *usb_dev, + int brightness) +{ + int err; + u16 interface_nr; + struct brightness_ctrl_message_data *msg_data; + + msg_data = kzalloc(sizeof(struct brightness_ctrl_message_data), GFP_KERNEL); + interface_nr = interface->cur_altsetting->desc.bInterfaceNumber; + init_ctrl_msg_data(msg_data); + set_ctrl_message_brightness(msg_data, brightness); + + err = usb_control_msg(usb_dev, + usb_sndctrlpipe(usb_dev, 0), + HID_SET_REPORT, + USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE, + /* wValue: HID-Report Type and Report ID */ + HID_REPORT_TYPE_FEATURE | 0x01, + interface_nr /* wIndex */, + msg_data, + sizeof(struct brightness_ctrl_message_data), + HZ); + kfree(msg_data); + if (err < 0) { + dev_err(&interface->dev, + "set: usb control message err: %d\n", + err); + return err; + } + dev_dbg(&interface->dev, "set brightness: %d\n", brightness); + return 0; +} + +int apple_bl_usb_check_fb(struct backlight_device *bd, struct fb_info *info) +{ + dev_info(&bd->dev, "check fb\n"); + return 0; +} + +int apple_bl_usb_get_brightness(struct backlight_device *bl) +{ + int ret; + struct apple_bl_usb_data *data; + int hw_brightness; + + data = bl_get_data(bl); + ret = apple_bl_usb_usb_get_brightness(data->usb_interface, + data->usb_dev, + &hw_brightness); + if (!ret) + ret = hw_brightness; + + return ret; +} + +int apple_bl_usb_update_status(struct backlight_device *bl) +{ + int err; + struct apple_bl_usb_data *data; + + data = bl_get_data(bl); + err = apple_bl_usb_usb_set_brightness(data->usb_interface, + data->usb_dev, + bl->props.brightness); + return err; +} + +static const struct backlight_ops apple_bl_usb_ops = { + .update_status = apple_bl_usb_update_status, + .get_brightness = apple_bl_usb_get_brightness, + .check_fb = apple_bl_usb_check_fb, +}; + +static void apple_bl_usb_disconnect(struct usb_interface *interface) +{ + struct backlight_device *bl; + + dev_dbg(&interface->dev, "disconnect\n"); + + bl = usb_get_intfdata(interface); + usb_set_intfdata(interface, NULL); + backlight_device_unregister(bl); +} + +static int apple_bl_usb_probe(struct usb_interface *interface, + const struct usb_device_id *id) +{ + struct backlight_properties props; + struct backlight_device *bl; + struct usb_device *usb_dev; + struct device *dev; + struct apple_bl_usb_data *data; + int brightness_interface_nr; + + dev_dbg(&interface->dev, "probe\n"); + + dev = &interface->dev; + usb_dev = interface_to_usbdev(interface); + + switch (usb_dev->config->desc.bConfigurationValue) { + case 1: + brightness_interface_nr = 0x7; + break; + case 2: + brightness_interface_nr = 0x9; + break; + case 3: + brightness_interface_nr = 0xc; + break; + default: + dev_err(dev, + "unexpected configuration value: %d\n", + usb_dev->config->desc.bConfigurationValue); + return -EINVAL; + } + + if (interface->cur_altsetting->desc.bInterfaceNumber != brightness_interface_nr) + return -ENODEV; + + data = devm_kzalloc(dev, + sizeof(struct apple_bl_usb_data), + GFP_KERNEL); + if (IS_ERR(data)) { + dev_err(dev, "failed to allocate memory\n"); + return PTR_ERR(bl); + } + data->usb_interface = interface; + data->usb_dev = usb_dev; + + // Valid brightness values for the apple studio display range from 400 + // to 60000. Since the backlight subsystem´s brightness value starts + // from 0, we use 0 to 59600 and offset it by the minimum value. + memset(&props, 0, sizeof(props)); + props.type = BACKLIGHT_RAW; + props.max_brightness = 59600; + + bl = backlight_device_register("apple_studio_display", + dev, + data, + &apple_bl_usb_ops, + &props); + if (IS_ERR(bl)) { + dev_err(dev, "failed to register backlight\n"); + return PTR_ERR(bl); + } + usb_set_intfdata(interface, bl); + return 0; +} + +static int apple_bl_usb_suspend(struct usb_interface *interface, + pm_message_t message) +{ + dev_dbg(&interface->dev, "suspend\n"); + return 0; +} + +static int apple_bl_usb_resume(struct usb_interface *interface) +{ + dev_dbg(&interface->dev, "resume\n"); + return 0; +} + +static const struct usb_device_id id_table[] = { + { + .idVendor = APPLE_STUDIO_DISPLAY_VENDOR_ID, + .idProduct = APPLE_STUDIO_DISPLAY_PRODUCT_ID, + }, + {}, +}; +MODULE_DEVICE_TABLE(usb, id_table); + +static struct usb_driver usb_asdbl_driver = { + .name = "apple_bl_usb", + .probe = apple_bl_usb_probe, + .disconnect = apple_bl_usb_disconnect, + .id_table = id_table, + .suspend = apple_bl_usb_suspend, + .resume = apple_bl_usb_resume, + .reset_resume = apple_bl_usb_resume +}; +module_usb_driver(usb_asdbl_driver); + +MODULE_AUTHOR("Julius Zint <julius@zint.sh>"); +MODULE_LICENSE("Dual MIT/GPL"); +MODULE_DESCRIPTION("Backlight control for USB-C Thunderbolt Apple displays");
The Apple Studio Display does not have any physical buttons and the only way to get or set the brightness is by sending USB control transfers to a HID device exposed by the display. These control transfers take the form of a HID_(GET|SET)_REPORT request and the payload looks like this: struct brightness_ctrl_message_data { u8 unknown_1; __le16 brightness; u8 unkown_2[4]; } __packed; When compiled as a module this driver needs to be part of the early boot environment, otherwise the generic USB HID driver will claim the device. Signed-off-by: Julius Zint <julius@zint.sh> --- drivers/video/backlight/Kconfig | 8 + drivers/video/backlight/Makefile | 1 + drivers/video/backlight/apple_bl_usb.c | 264 +++++++++++++++++++++++++ 3 files changed, 273 insertions(+) create mode 100644 drivers/video/backlight/apple_bl_usb.c