Re: [PATCH 0/1] Backlight driver for the Apple Studio Display

[Julius Zint]

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/20230806091403.10002-1-julius@xxxxxxx/
[2] https://lore.kernel.org/dri-devel/20230820094118.20521-1-julius@xxxxxxx/

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);