I have been using and testing this as a DKMS for 6 months now without
any known issues. It bothers me, that it needs to be part of the
initramfs instead of just working out of the box. Maybe someone else
here knows, how to tell the USB HID driver, that this is not a HID device
and it should keep its fingers from it.
Julius Zint (1):
backlight: apple_bl_usb: Add Apple Studio Display support
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
--
2.41.0
On Sat, Jul 01, 2023 at 02:08:02PM +0200, Julius Zint wrote:
> I have been using and testing this as a DKMS for 6 months now without
> any known issues. It bothers me, that it needs to be part of the
> initramfs instead of just working out of the box. Maybe someone else
> here knows, how to tell the USB HID driver, that this is not a HID device
> and it should keep its fingers from it.
If is says it is a HID device and is uses HID reports for control then
it *is* a HID device!
In other words you need your driver to register as a HID driver instead
of sending raw HID frames using the USB stack. If you do that then the
HID core infrastructure will ensure the right driver gets loaded (it
has special logic to automatically unregister hid-generic and load the
better driver as soon as one becomes available).
Daniel.
I appreciate all of the feedback, this should be plenty for a v2.
Thanks,
Julius
On Fri, 17 Nov 2023, Sean Aguinaga wrote:
Hi,
> Did you get a chance to implement V2?
Yes, v2 is here [1] and v3 is here [2]. Currently I do have the
a few changes on top of v3 that are appended here as a patch. I use it
with DMKS and it works (mostly). I do see the userspace confusion when
the monitor is plugged in after boot.
Its still possible to set it by writing to the brightness file.
Julius
[1] https://lore.kernel.org/dri-devel/[email protected]/
[2] https://lore.kernel.org/dri-devel/[email protected]/
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index b964a820956d..35864cc1afee 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -477,8 +477,7 @@ config BACKLIGHT_HID
depends on HID
help
If you have an external display with VESA compliant HID brightness
- controls then say Y to enable this backlight driver. Currently the
- only supported device is the Apple Studio Display.
+ controls then say Y to enable this backlight driver.
endif # BACKLIGHT_CLASS_DEVICE
diff --git a/drivers/video/backlight/hid_bl.c b/drivers/video/backlight/hid_bl.c
index b40f8f412ee2..38b82de8224f 100644
--- a/drivers/video/backlight/hid_bl.c
+++ b/drivers/video/backlight/hid_bl.c
@@ -4,8 +4,8 @@
#include <linux/module.h>
#include <linux/backlight.h>
-#define APPLE_STUDIO_DISPLAY_VENDOR_ID 0x05ac
-#define APPLE_STUDIO_DISPLAY_PRODUCT_ID 0x1114
+#define APPLE_STUDIO_DISPLAY_VENDOR_ID 0x05ac
+#define APPLE_STUDIO_DISPLAY_PRODUCT_ID 0x1114
#define HID_USAGE_MONITOR_CTRL 0x800001
#define HID_USAGE_VESA_VCP_BRIGHTNESS 0x820010
@@ -59,7 +59,8 @@ struct hid_bl_data {
static struct hid_field *hid_get_usage_field(struct hid_device *hdev,
unsigned int report_type,
- unsigned int application, unsigned int usage)
+ unsigned int application,
+ unsigned int usage)
{
struct hid_report_enum *re = &hdev->report_enum[report_type];
struct hid_report *report;
@@ -82,18 +83,8 @@ static struct hid_field *hid_get_usage_field(struct hid_device *hdev,
static void hid_bl_remove(struct hid_device *hdev)
{
- struct backlight_device *bl;
- struct hid_bl_data *data;
-
- hid_dbg(hdev, "remove\n");
- bl = hid_get_drvdata(hdev);
- data = bl_get_data(bl);
-
- devm_backlight_device_unregister(&hdev->dev, bl);
hid_hw_close(hdev);
hid_hw_stop(hdev);
- hid_set_drvdata(hdev, NULL);
- devm_kfree(&hdev->dev, data);
}
static int hid_bl_get_brightness_raw(struct hid_bl_data *data)
@@ -105,7 +96,6 @@ static int hid_bl_get_brightness_raw(struct hid_bl_data *data)
hid_hw_request(data->hdev, field->report, HID_REQ_GET_REPORT);
hid_hw_wait(data->hdev);
result = *field->new_value;
- hid_dbg(data->hdev, "get brightness: %d\n", result);
return result;
}
@@ -128,7 +118,6 @@ static void hid_bl_set_brightness_raw(struct hid_bl_data *data, int brightness)
*field->value = brightness;
hid_hw_request(data->hdev, field->report, HID_REQ_SET_REPORT);
hid_hw_wait(data->hdev);
- hid_dbg(data->hdev, "set brightness: %d\n", brightness);
}
static int hid_bl_update_status(struct backlight_device *bl)
@@ -157,8 +146,6 @@ static int hid_bl_probe(struct hid_device *hdev, const struct hid_device_id *id)
struct backlight_properties props;
struct backlight_device *bl;
- hid_dbg(hdev, "probe\n");
-
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "parse failed: %d\n", ret);
@@ -203,8 +190,6 @@ static int hid_bl_probe(struct hid_device *hdev, const struct hid_device_id *id)
goto exit_stop;
}
- hid_dbg(hdev, "Monitor VESA VCP with brightness control\n");
-
ret = hid_hw_open(hdev);
if (ret) {
hid_err(hdev, "hw open failed: %d\n", ret);
@@ -214,7 +199,7 @@ static int hid_bl_probe(struct hid_device *hdev, const struct hid_device_id *id)
data = devm_kzalloc(&hdev->dev, sizeof(*data), GFP_KERNEL);
if (data == NULL) {
ret = -ENOMEM;
- goto exit_stop;
+ goto exit_close;
}
data->hdev = hdev;
data->min_brightness = input_field->logical_minimum;
@@ -233,16 +218,15 @@ static int hid_bl_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (IS_ERR(bl)) {
ret = PTR_ERR(bl);
hid_err(hdev, "failed to register backlight: %d\n", ret);
- goto exit_free;
+ goto exit_close;
}
hid_set_drvdata(hdev, bl);
return 0;
-exit_free:
+exit_close:
hid_hw_close(hdev);
- devm_kfree(&hdev->dev, data);
exit_stop:
hid_hw_stop(hdev);