Re: [PATCH v3 1/1] backlight: hid_bl: Add VESA VCP HID backlight driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Le 20/08/2023 à 11:41, Julius Zint a écrit :
The HID spec defines the following Usage IDs (p. 345 ff):

- Monitor Page (0x80) -> Monitor Control (0x01)
- VESA Virtual Controls Page (0x82) -> Brightness (0x10)

Apple made use of them in their Apple Studio Display and most likely on
other external displays (LG UltraFine 5k, Pro Display XDR).

The driver will work for any HID device with a report, where the
application matches the Monitor Control Usage ID and:

1. An Input field in this report with the Brightness Usage ID (to get
    the current brightness)
2. A Feature field in this report with the Brightness Usage ID (to
    set the current brightness)

This driver has been developed and tested with the Apple Studio Display.
Here is a small excerpt from the decoded HID descriptor showing the
feature field for setting the brightness:

   Usage Page (Monitor VESA VCP),  ; Monitor VESA VPC (82h, monitor page)
   Usage (10h, Brightness),
   Logical Minimum (400),
   Logical Maximum (60000),
   Unit (Centimeter^-2 * Candela),
   Unit Exponent (14),
   Report Size (32),
   Report Count (1),
   Feature (Variable, Null State),

The full HID descriptor dump is available as a comment in the source
code.

Signed-off-by: Julius Zint <julius@xxxxxxx>
---

[...]

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

Hi,

If this need to be called here, before the hid_() calls below, there seems to be no real point in using devm_backlight_device_register() / devm_backlight_device_unregister().

The non-devm_ version should be enough.

+	hid_hw_close(hdev);
+	hid_hw_stop(hdev);
+	hid_set_drvdata(hdev, NULL);
+	devm_kfree(&hdev->dev, data);

'data' is devm_kzalloc()'ed.
It is likely that this explicit devm_kfree() could be saved. It will be freed by the framework.

+}

[...]

+static int hid_bl_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+	int ret;
+	struct hid_field *input_field;
+	struct hid_field *feature_field;
+	struct hid_bl_data *data;
+	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);
+		return ret;
+	}
+
+	ret = hid_hw_start(hdev, HID_CONNECT_DRIVER);
+	if (ret) {
+		hid_err(hdev, "hw start failed: %d\n", ret);
+		return ret;
+	}
+
+	input_field = hid_get_usage_field(hdev, HID_INPUT_REPORT,
+					  HID_USAGE_MONITOR_CTRL,
+					  HID_USAGE_VESA_VCP_BRIGHTNESS);
+	if (input_field == NULL) {
+		ret = -ENODEV;
+		goto exit_stop;
+	}
+
+	feature_field = hid_get_usage_field(hdev, HID_FEATURE_REPORT,
+					    HID_USAGE_MONITOR_CTRL,
+					    HID_USAGE_VESA_VCP_BRIGHTNESS);
+	if (feature_field == NULL) {
+		ret = -ENODEV;
+		goto exit_stop;
+	}
+
+	if (input_field->logical_minimum != feature_field->logical_minimum) {
+		hid_warn(hdev, "minimums do not match: %d / %d\n",
+			 input_field->logical_minimum,
+			 feature_field->logical_minimum);
+		ret = -ENODEV;
+		goto exit_stop;
+	}
+
+	if (input_field->logical_maximum != feature_field->logical_maximum) {
+		hid_warn(hdev, "maximums do not match: %d / %d\n",
+			 input_field->logical_maximum,
+			 feature_field->logical_maximum);
+		ret = -ENODEV;
+		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);
+		goto exit_stop;
+	}
+
+	data = devm_kzalloc(&hdev->dev, sizeof(*data), GFP_KERNEL);
+	if (data == NULL) {
+		ret = -ENOMEM;
+		goto exit_stop;

exit_free?
in order to undo the hid_hw_open() just above.

+	}
+	data->hdev = hdev;
+	data->min_brightness = input_field->logical_minimum;
+	data->max_brightness = input_field->logical_maximum;
+	data->input_field = input_field;
+	data->feature_field = feature_field;
+
+	memset(&props, 0, sizeof(props));
+	props.type = BACKLIGHT_RAW;
+	props.max_brightness = data->max_brightness - data->min_brightness;
+
+	bl = devm_backlight_device_register(&hdev->dev, "vesa_vcp",
+					    &hdev->dev, data,
+					    &hid_bl_ops,
+					    &props);
+	if (IS_ERR(bl)) {
+		ret = PTR_ERR(bl);
+		hid_err(hdev, "failed to register backlight: %d\n", ret);
+		goto exit_free;
+	}
+
+	hid_set_drvdata(hdev, bl);
+
+	return 0;
+
+exit_free:
+	hid_hw_close(hdev);
+	devm_kfree(&hdev->dev, data);

'data' is devm_kzalloc()'ed.
It is likely that this explicit devm_kfree() could be saved. It will be freed by the framework.

+
+exit_stop:
+	hid_hw_stop(hdev);
+	return ret;
+}

[...]




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux