2022-04-05 00:33:41

by Yusuf Khan

[permalink] [raw]
Subject: [PATCH v10 0/3] drivers: ddcci: add drivers for DDCCI

This patch adds the DDCCI driver by Christoph Grenz into the kernel.
The original gitlab page is loacted at https://gitlab.com/ddcci-driv
er-linux/ddcci-driver-linux/-/tree/master.

DDC/CI is a control protocol for monitor settings supported by most
monitors since about 2005. A chardev and sysfs interface is provided.
A backlight driver using DDCCI is also provided in the seccond patch.

Signed-off-by: Yusuf Khan <[email protected]>
Signed-off-by: Christoph Grenz <[email protected]>
---
v2: Fix typos.

v3: Add documentation, move files around, replace semaphores with
mutexes, and replaced <asm-generic/fcntl.h> with <linux/fcntl.h>.
"imirkin"(which due to his involvement in the dri-devel irc channel
I cant help but assume to be a DRM developer) said that the DDC/CI
bus does not intefere with the buses that DRM is involved with.

v4: Move some documentation, fix grammer mistakes, remove usages of
likely(), and clarify some documentation.

v5: Fix grammer mistakes, remove usages of likely(), and clarify
some documentation.

v6: Change contact information to reference Christoph Grenz.

v7: Remove all instances of the unlikely() macro.

v8: Modify documentation to provide updated date and kernel
documentation, fix SPDX lines, use isalpha instead of redefining
logic, change maximum amount of bytes that can be written to be
conformant with DDC/CI specification, prevent userspace from holding
locks with the open file descriptor, remove ddcci_cdev_seek, dont
refine sysfs_emit() logic, use EXPORT_SYMBOL_GPL instead of
EXPORT_SYMBOL, remove ddcci_device_remove_void, remove module
paramaters and version, and split into 2 patches.

v9: Fix IS_ANY_ID matching for compilers and archs where char is
unsigned char and use the cannonical patch format.
Reported-by: kernel test robot <[email protected]>

v10: Change patch title to "drivers: ddcci: add drivers for DDCCI
and change" and change patch descriptions to add more detailed
explanations of function.

Patch 1: Add the main DDCCI component.

Patch 2: Add the backlight driver that utilizes the DDCCI driver.

Patch 3: Add documentation for the DDCCI drivers.

Yusuf Khan (3):
drivers: ddcci: add drivers for DDCCI
drivers: ddcci: add drivers for DDCCI
drivers: ddcci: add drivers for DDCCI

Documentation/ABI/testing/sysfs-driver-ddcci | 57 +
Documentation/driver-api/ddcci.rst | 122 ++
drivers/char/Kconfig | 11 +
drivers/char/Makefile | 1 +
drivers/char/ddcci.c | 1805 ++++++++++++++++++
drivers/video/backlight/Kconfig | 11 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/ddcci-backlight.c | 411 ++++
include/linux/ddcci.h | 163 ++
9 files changed, 2582 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-ddcci
create mode 100644 Documentation/driver-api/ddcci.rst
create mode 100644 drivers/char/ddcci.c
create mode 100644 drivers/video/backlight/ddcci-backlight.c
create mode 100644 include/linux/ddcci.h

--
2.25.1


2022-04-05 00:52:12

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v10 0/3] drivers: ddcci: add drivers for DDCCI

Hi Yusuf,

On 4/4/22 01:08, Yusuf Khan wrote:
> This patch adds the DDCCI driver by Christoph Grenz into the kernel.
> The original gitlab page is loacted at https://gitlab.com/ddcci-driv
> er-linux/ddcci-driver-linux/-/tree/master.
>
> DDC/CI is a control protocol for monitor settings supported by most
> monitors since about 2005. A chardev and sysfs interface is provided.
> A backlight driver using DDCCI is also provided in the seccond patch.
>
> Signed-off-by: Yusuf Khan <[email protected]>
> Signed-off-by: Christoph Grenz <[email protected]>

Thank you for your submission of this patch series.

I must say that I'm a bit surprised that this series was NOT
also send to the drm/kms subsystem maintainers and mailinglists
since this deals with monitors and thus is highly relevant for
those folks. Luckily I saw an article about this at Phoronix
(you write in the changelog that you believe that there is
no interaction with the DRM/KMS subsystems but that is wrong).

One very important thing which I'm missing in this cover-letter
is why you want to have this in the kernel at all? There are
already various tools which do DDCCI from userspace just fine.

I guess you may be interested in offering /sys/class/backlight
functionality for external monitors. That is actually something
which I'm interested in too, but it is not that simple.

The current /sys/class/backlight interface does not offer a
way for userspace to map a /sys/class/backlight device to
a specific display-output / monitor. So userspace currently
assumes that there is only 1 (1 valid) /sys/class/backlight
device and that that *always* belongs to the internal LCD
panel of a laptop.

So just adding /sys/class/backlight device(s) for internal
monitors without addressing the short-comings of the existing
userspace interface is simply not possible because it would
break existing userspace which is something which is not
allowed.

So NACK from me for the backlight part at least and without
that, I really see no reason to do this in the kernel rather
then userspace.

I've recently been discussing this with a colleague of mine,
Sebastian Wick and as a result of that I'm giving a talk
with a proposal for a better userspace API for this at
kernel-recipes:
https://kernel-recipes.org/en/2022/talks/new-userspace-api-for-display-panel-brightness-control/

I hope to start working on a RFC patch series for this soon.

IMHO merging this series should be put on hold until we
have a better idea of how we want to solve the userspace
API challenges, esp. the monitor <-> /sys/class/backlight
mapping problem.

Regards,

Hans



p.s.

This is not the first time this has come up:

https://lore.kernel.org/all/[email protected]/
https://www.x.org/wiki/Events/XDC2014/XDC2014GoedeBacklight/



> ---
> v2: Fix typos.
>
> v3: Add documentation, move files around, replace semaphores with
> mutexes, and replaced <asm-generic/fcntl.h> with <linux/fcntl.h>.
> "imirkin"(which due to his involvement in the dri-devel irc channel
> I cant help but assume to be a DRM developer) said that the DDC/CI
> bus does not intefere with the buses that DRM is involved with.
>
> v4: Move some documentation, fix grammer mistakes, remove usages of
> likely(), and clarify some documentation.
>
> v5: Fix grammer mistakes, remove usages of likely(), and clarify
> some documentation.
>
> v6: Change contact information to reference Christoph Grenz.
>
> v7: Remove all instances of the unlikely() macro.
>
> v8: Modify documentation to provide updated date and kernel
> documentation, fix SPDX lines, use isalpha instead of redefining
> logic, change maximum amount of bytes that can be written to be
> conformant with DDC/CI specification, prevent userspace from holding
> locks with the open file descriptor, remove ddcci_cdev_seek, dont
> refine sysfs_emit() logic, use EXPORT_SYMBOL_GPL instead of
> EXPORT_SYMBOL, remove ddcci_device_remove_void, remove module
> paramaters and version, and split into 2 patches.
>
> v9: Fix IS_ANY_ID matching for compilers and archs where char is
> unsigned char and use the cannonical patch format.
> Reported-by: kernel test robot <[email protected]>
>
> v10: Change patch title to "drivers: ddcci: add drivers for DDCCI
> and change" and change patch descriptions to add more detailed
> explanations of function.
>
> Patch 1: Add the main DDCCI component.
>
> Patch 2: Add the backlight driver that utilizes the DDCCI driver.
>
> Patch 3: Add documentation for the DDCCI drivers.
>
> Yusuf Khan (3):
> drivers: ddcci: add drivers for DDCCI
> drivers: ddcci: add drivers for DDCCI
> drivers: ddcci: add drivers for DDCCI
>
> Documentation/ABI/testing/sysfs-driver-ddcci | 57 +
> Documentation/driver-api/ddcci.rst | 122 ++
> drivers/char/Kconfig | 11 +
> drivers/char/Makefile | 1 +
> drivers/char/ddcci.c | 1805 ++++++++++++++++++
> drivers/video/backlight/Kconfig | 11 +
> drivers/video/backlight/Makefile | 1 +
> drivers/video/backlight/ddcci-backlight.c | 411 ++++
> include/linux/ddcci.h | 163 ++
> 9 files changed, 2582 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-driver-ddcci
> create mode 100644 Documentation/driver-api/ddcci.rst
> create mode 100644 drivers/char/ddcci.c
> create mode 100644 drivers/video/backlight/ddcci-backlight.c
> create mode 100644 include/linux/ddcci.h
>

2022-04-05 01:10:36

by Yusuf Khan

[permalink] [raw]
Subject: [PATCH v10 2/3] drivers: ddcci: add drivers for DDCCI

This patch adds the backlight driver that utilizes the DDCCI
driver from patch one to add a backlight driver.

Signed-off-by: Yusuf Khan <[email protected]>
Signed-off-by: Christoph Grenz <[email protected]>
---
drivers/video/backlight/Kconfig | 11 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/ddcci-backlight.c | 411 ++++++++++++++++++++++
3 files changed, 423 insertions(+)
create mode 100644 drivers/video/backlight/ddcci-backlight.c

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index e32694c13da5..7a26088c3c3f 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -289,6 +289,17 @@ config BACKLIGHT_QCOM_WLED
If you have the Qualcomm PMIC, say Y to enable a driver for the
WLED block. Currently it supports PM8941 and PMI8998.

+config BACKLIGHT_DDCCI
+ tristate "DDCCI Backlight Driver"
+ depends on DDCCI
+ help
+ If you have a DDC/CI supporing monitor, say Y to enable a driver
+ to control its backlight using DDC/CI. This could be useful if
+ your monitor does not include a backlight driver. For this to be
+ useful you need to enable DDCCI support which can be found in
+ Device Drivers -> Character devices and that further depends on
+ I2C.
+
config BACKLIGHT_RT4831
tristate "Richtek RT4831 Backlight Driver"
depends on MFD_RT4831
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index cae2c83422ae..7bfb6e506b35 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -58,3 +58,4 @@ obj-$(CONFIG_BACKLIGHT_WM831X) += wm831x_bl.o
obj-$(CONFIG_BACKLIGHT_ARCXCNN) += arcxcnn_bl.o
obj-$(CONFIG_BACKLIGHT_RAVE_SP) += rave-sp-backlight.o
obj-$(CONFIG_BACKLIGHT_LED) += led_bl.o
+obj-$(CONFIG_BACKLIGHT_DDCCI) += ddcci-backlight.o
diff --git a/drivers/video/backlight/ddcci-backlight.c b/drivers/video/backlight/ddcci-backlight.c
new file mode 100644
index 000000000000..d37eb142311d
--- /dev/null
+++ b/drivers/video/backlight/ddcci-backlight.c
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * DDC/CI monitor backlight driver
+ *
+ * Copyright (c) 2015 Christoph Grenz
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/backlight.h>
+#include <linux/module.h>
+#include <linux/fb.h>
+#include <linux/sysfs.h>
+
+#include <linux/ddcci.h>
+
+
+#define DDCCI_COMMAND_READ 0x01 /* read ctrl value */
+#define DDCCI_REPLY_READ 0x02 /* read ctrl value reply */
+#define DDCCI_COMMAND_WRITE 0x03 /* write ctrl value */
+#define DDCCI_COMMAND_SAVE 0x0c /* save current settings */
+
+#define DDCCI_MONITOR_LUMINANCE 0x10
+#define DDCCI_MONITOR_BACKLIGHT 0x13
+#define DDCCI_MONITOR_BL_WHITE 0x6B
+
+static bool convenience_symlink = true;
+
+struct ddcci_monitor_drv_data {
+ struct ddcci_device *device;
+ struct backlight_device *bl_dev;
+ struct device *fb_dev;
+ unsigned char used_vcp;
+};
+
+static int ddcci_monitor_writectrl(struct ddcci_device *device,
+ unsigned char ctrl, unsigned short value)
+{
+ unsigned char buf[4];
+ int ret;
+
+ buf[0] = DDCCI_COMMAND_WRITE;
+ buf[1] = ctrl;
+ buf[2] = (value >> 8);
+ buf[3] = (value & 255);
+
+ ret = ddcci_device_write(device, true, buf, sizeof(buf));
+
+ return ret;
+}
+
+static int ddcci_monitor_readctrl(struct ddcci_device *device,
+ unsigned char ctrl, unsigned short *value,
+ unsigned short *maximum)
+{
+ int ret;
+ unsigned char buf[10];
+
+ buf[0] = DDCCI_COMMAND_READ;
+ buf[1] = ctrl;
+
+ ret = ddcci_device_writeread(device, true, buf, 2, sizeof(buf));
+ if (ret < 0)
+ return ret;
+
+ if (ret == 0)
+ return -ENOTSUPP;
+
+ if (ret == 8 && buf[0] == DDCCI_REPLY_READ && buf[2] == ctrl) {
+ if (value)
+ *value = buf[6] * 256 + buf[7];
+
+ if (maximum)
+ *maximum = buf[4] * 256 + buf[5];
+
+ if (buf[1] == 1)
+ return -ENOTSUPP;
+ if (buf[1] != 0)
+ return -EIO;
+ return 0;
+ }
+
+ return -EIO;
+}
+
+static int ddcci_backlight_check_fb(struct backlight_device *bl,
+ struct fb_info *info)
+{
+ struct ddcci_monitor_drv_data *drv_data = bl_get_data(bl);
+
+ return drv_data->fb_dev == NULL || drv_data->fb_dev == info->dev;
+}
+
+static int ddcci_backlight_update_status(struct backlight_device *bl)
+{
+ struct ddcci_monitor_drv_data *drv_data = bl_get_data(bl);
+ int brightness = bl->props.brightness;
+ int ret;
+
+ if (bl->props.power != FB_BLANK_UNBLANK ||
+ bl->props.state & BL_CORE_FBBLANK)
+ brightness = 0;
+
+ ret = ddcci_monitor_writectrl(drv_data->device, drv_data->used_vcp,
+ brightness);
+ if (ret > 0)
+ ret = 0;
+ return ret;
+}
+
+static int ddcci_backlight_get_brightness(struct backlight_device *bl)
+{
+ unsigned short value = 0, maxval = 0;
+ int ret;
+ struct ddcci_monitor_drv_data *drv_data = bl_get_data(bl);
+
+ ret = ddcci_monitor_readctrl(drv_data->device, drv_data->used_vcp,
+ &value, &maxval);
+ if (ret < 0)
+ return ret;
+
+ bl->props.brightness = value;
+ bl->props.max_brightness = maxval;
+ ret = value;
+
+ return ret;
+}
+
+static const struct backlight_ops ddcci_backlight_ops = {
+ .options = 0,
+ .update_status = ddcci_backlight_update_status,
+ .get_brightness = ddcci_backlight_get_brightness,
+ .check_fb = ddcci_backlight_check_fb,
+};
+
+static const char *ddcci_monitor_vcp_name(unsigned char vcp)
+{
+ switch (vcp) {
+ case DDCCI_MONITOR_BL_WHITE:
+ return "backlight";
+ case DDCCI_MONITOR_LUMINANCE:
+ return "luminance";
+ default:
+ return "???";
+ }
+}
+
+static const char *ddcci_monitor_next_vcp_item(const char *ptr)
+{
+ int depth = 0;
+
+ /* Sanity check */
+ if (ptr == NULL || ptr[0] == '\0')
+ return NULL;
+
+ /* Find next white space outside of parentheses */
+ while ((ptr = strpbrk(ptr, " ()"))) {
+ if (!ptr || depth == INT_MAX)
+ return NULL;
+ else if (*ptr == '(')
+ depth++;
+ else if (depth > 0) {
+ if (*ptr == ')')
+ depth--;
+ } else
+ break;
+ ++ptr;
+ }
+
+ /* Skip over whitespace */
+ ptr = skip_spaces(ptr);
+
+ /* Check if we're now at the end of the list */
+ if (*ptr == '\0' || *ptr == ')')
+ return NULL;
+
+ return ptr;
+}
+
+static bool ddcci_monitor_find_vcp(unsigned char vcp, const char *s)
+{
+ const char *ptr = s;
+ char vcp_hex[3];
+
+ /* Sanity check */
+ if (s == NULL || s[0] == '\0')
+ return false;
+
+ /* Create hex representation of VCP */
+ if (snprintf(vcp_hex, 3, "%02hhX", vcp) != 2) {
+ pr_err("snprintf failed to convert to hex. This should not happen.\n");
+ return false;
+ }
+
+ /* Search for it */
+ do {
+ if (strncasecmp(vcp_hex, ptr, 2) == 0) {
+ if (ptr[2] == ' ' || ptr[2] == '(' || ptr[2] == ')')
+ return true;
+ }
+ } while ((ptr = ddcci_monitor_next_vcp_item(ptr)));
+
+ return false;
+}
+
+static int ddcci_backlight_create_symlink(struct ddcci_device *ddcci_dev)
+{
+ int i, result;
+ struct device *dev = &ddcci_dev->dev;
+ struct kernfs_node *dirent;
+
+ for (i = 0; i < 3; ++i) {
+ dev = dev->parent;
+ if (!dev) {
+ dev_dbg(&ddcci_dev->dev, "failed to create convenience symlink: ancestor device not found\n");
+ return -ENOENT;
+ }
+ }
+ dirent = sysfs_get_dirent(dev->kobj.sd, "ddcci_backlight");
+ if (dirent) {
+ sysfs_put(dirent);
+ dev_dbg(&ddcci_dev->dev, "failed to create convenience symlink: %s/ddcci_backlight already exists\n", dev_name(dev));
+ return -EEXIST;
+ }
+
+ result = sysfs_create_link(&dev->kobj, &ddcci_dev->dev.kobj, "ddcci_backlight");
+ if (result == 0)
+ dev_dbg(&ddcci_dev->dev, "created symlink %s/ddcci_backlight\n", dev_name(dev));
+ else
+ dev_info(&ddcci_dev->dev, "failed to create convenience symlink: %d\n", result);
+ return result;
+}
+
+static int ddcci_backlight_remove_symlink(struct ddcci_device *ddcci_dev)
+{
+ int i;
+ struct device *dev = &ddcci_dev->dev;
+ struct kernfs_node *dirent;
+
+ for (i = 0; i < 3; ++i) {
+ dev = dev->parent;
+ if (!dev)
+ return -ENOENT;
+ }
+ dirent = sysfs_get_dirent(dev->kobj.sd, "ddcci_backlight");
+ if (!dirent)
+ return -ENOENT;
+
+ if ((dirent->flags & KERNFS_LINK) == 0) {
+ sysfs_put(dirent);
+ dev_dbg(&ddcci_dev->dev, "won't remove %s/ddcci_backlight: not a symlink\n", dev_name(dev));
+ return -EINVAL;
+ }
+
+ if (dirent->symlink.target_kn != ddcci_dev->dev.kobj.sd) {
+ sysfs_put(dirent);
+ dev_dbg(&ddcci_dev->dev, "won't remove %s/ddcci_backlight: we are not the link target\n", dev_name(dev));
+ return -EINVAL;
+ }
+
+ sysfs_put(dirent);
+
+ sysfs_remove_link(&dev->kobj, "ddcci_backlight");
+ dev_dbg(&ddcci_dev->dev, "removed symlink %s/ddcci_backlight\n", dev_name(dev));
+ return 0;
+}
+
+static int ddcci_monitor_probe(struct ddcci_device *dev,
+ const struct ddcci_device_id *id)
+{
+ struct ddcci_monitor_drv_data *drv_data;
+ struct backlight_properties props;
+ struct backlight_device *bl = NULL;
+ int ret = 0;
+ bool support_luminance, support_bl_white;
+ unsigned short brightness = 0, max_brightness = 0;
+ const char *vcps;
+
+ dev_dbg(&dev->dev, "probing monitor backlight device\n");
+
+ /* Get VCP list */
+ vcps = ddcci_find_capstr_item(dev->capabilities, "vcp", NULL);
+ if (IS_ERR(vcps)) {
+ dev_info(&dev->dev,
+ "monitor doesn't provide a list of supported controls.\n");
+ support_bl_white = support_luminance = true;
+ } else {
+ /* Check VCP list for supported VCPs */
+ support_bl_white = ddcci_monitor_find_vcp(DDCCI_MONITOR_BL_WHITE, vcps);
+ support_luminance = ddcci_monitor_find_vcp(DDCCI_MONITOR_LUMINANCE, vcps);
+ /* Fallback to trying if no support is found */
+ if (!support_bl_white && !support_luminance) {
+ dev_info(&dev->dev,
+ "monitor doesn't announce support for backlight or luminance controls.\n");
+ support_bl_white = support_luminance = true;
+ }
+ }
+
+ /* Initialize driver data structure */
+ drv_data = devm_kzalloc(&dev->dev, sizeof(struct ddcci_monitor_drv_data),
+ GFP_KERNEL);
+ if (!drv_data)
+ return -ENOMEM;
+ drv_data->device = dev;
+
+ if (support_bl_white) {
+ /* Try getting backlight level */
+ dev_dbg(&dev->dev,
+ "trying to access \"backlight level white\" control\n");
+ ret = ddcci_monitor_readctrl(drv_data->device, DDCCI_MONITOR_BL_WHITE,
+ &brightness, &max_brightness);
+ if (ret < 0) {
+ if (ret == -ENOTSUPP)
+ dev_info(&dev->dev,
+ "monitor does not support reading backlight level\n");
+ else
+ goto err_free;
+ } else {
+ drv_data->used_vcp = DDCCI_MONITOR_BL_WHITE;
+ }
+ }
+
+ if (support_luminance && !drv_data->used_vcp) {
+ /* Try getting luminance */
+ dev_dbg(&dev->dev,
+ "trying to access \"luminance\" control\n");
+ ret = ddcci_monitor_readctrl(drv_data->device, DDCCI_MONITOR_LUMINANCE,
+ &brightness, &max_brightness);
+ if (ret < 0) {
+ if (ret == -ENOTSUPP)
+ dev_info(&dev->dev,
+ "monitor does not support reading luminance\n");
+ else
+ goto err_free;
+ } else {
+ drv_data->used_vcp = DDCCI_MONITOR_LUMINANCE;
+ }
+ drv_data->used_vcp = DDCCI_MONITOR_LUMINANCE;
+ }
+
+ if (!drv_data->used_vcp)
+ goto err_free;
+
+ /* Create brightness device */
+ memset(&props, 0, sizeof(props));
+ props.type = BACKLIGHT_RAW;
+ props.max_brightness = max_brightness;
+ props.brightness = brightness;
+ bl = devm_backlight_device_register(&dev->dev, dev_name(&dev->dev),
+ &dev->dev, drv_data,
+ &ddcci_backlight_ops, &props);
+ drv_data->bl_dev = bl;
+ if (IS_ERR(bl)) {
+ dev_err(&dev->dev, "failed to register backlight\n");
+ return PTR_ERR(bl);
+ }
+ dev_info(&dev->dev, "registered %s as backlight device %s\n",
+ ddcci_monitor_vcp_name(drv_data->used_vcp),
+ dev_name(&dev->dev));
+
+ if (convenience_symlink)
+ ddcci_backlight_create_symlink(dev);
+
+ goto end;
+err_free:
+ devm_kfree(&dev->dev, drv_data);
+end:
+ return ret;
+}
+
+static int ddcci_monitor_remove(struct ddcci_device *dev)
+{
+ dev_dbg(&dev->dev, "removing device\n");
+ ddcci_backlight_remove_symlink(dev);
+ return 0;
+}
+
+static struct ddcci_device_id ddcci_monitor_idtable[] = {
+ { "monitor", DDCCI_ANY_ID, DDCCI_ANY_ID, DDCCI_ANY_ID, DDCCI_ANY_ID, 0 },
+ {}
+};
+
+static struct ddcci_driver ddcci_backlight_driver = {
+ .driver = {
+ .name = "ddcci-backlight",
+ .owner = THIS_MODULE,
+ },
+
+ .id_table = ddcci_monitor_idtable,
+ .probe = ddcci_monitor_probe,
+ .remove = ddcci_monitor_remove,
+};
+
+module_ddcci_driver(ddcci_backlight_driver);
+
+/* Module parameter description */
+module_param(convenience_symlink, bool, 0644);
+MODULE_PARM_DESC(convenience_symlink, "add convenience symlink \"ddcci_backlight\" to ancestor device in sysfs (default true)");
+
+MODULE_AUTHOR("Christoph Grenz <[email protected]>");
+MODULE_DESCRIPTION("DDC/CI generic monitor backlight driver");
+MODULE_VERSION("0.4.2");
+MODULE_LICENSE("GPL");
+
+MODULE_ALIAS("ddcci:monitor-*-*-*-*");
--
2.25.1

2022-04-05 01:19:13

by Yusuf Khan

[permalink] [raw]
Subject: [PATCH v10 3/3] drivers: ddcci: add drivers for DDCCI

This patch adds Documentation for the DDCCI drivers.

Signed-off-by: Yusuf Khan <[email protected]>
Signed-off-by: Christoph Grenz <[email protected]>
---
Documentation/ABI/testing/sysfs-driver-ddcci | 57 +++++++++
Documentation/driver-api/ddcci.rst | 122 +++++++++++++++++++
2 files changed, 179 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-ddcci
create mode 100644 Documentation/driver-api/ddcci.rst

diff --git a/Documentation/ABI/testing/sysfs-driver-ddcci b/Documentation/ABI/testing/sysfs-driver-ddcci
new file mode 100644
index 000000000000..19f77ccf3ed0
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-ddcci
@@ -0,0 +1,57 @@
+What: /sys/bus/ddcci/ddcci<I²C bus number>i<hex address>
+Date: March 2022
+KernelVersion: 5.18
+Contact: Christoph Grenz <[email protected]>
+Description: This file is a user interface for an internal
+ dependent device on the I2C bus, it exports the same
+ information as the master device(/sys/bus/ddcci/
+ ddcci<I²C bus number>) that is referenced in this
+ document.
+
+What: /sys/bus/ddcci/ddcci<I²C bus number>e<hex address>
+Date: March 2022
+KernelVersion: 5.18
+Contact: Christoph Grenz <[email protected]>
+Description: This file is a user interface for an external
+ dependent device on the I2C bus, it exports the same
+ information as the master device(/sys/bus/ddcci/
+ ddcci<I²C bus number>) that is referenced in this
+ document.
+
+What: /sys/bus/ddcci/ddcci<I²C bus number>
+Date: March 2022
+KernelVersion: 5.18
+Contact: Christoph Grenz <[email protected]>
+Description: This file provides the user interface for the
+ master device on the I2C bus. It exports the following
+ peices of information:
+ - idProt
+ ACCESS.bus protocol supported by the device. Usually
+ "monitor".
+
+ - idType
+ ACCESS.bus device subtype. Usually "LCD" or "CRT".
+
+ - idModel
+ ACCESS.bus device model identifier. Usually a
+ shortened form of the device model name.
+
+ - idVendor
+ ACCESS.bus device vendor identifier. Empty if the
+ Identification command is not supported.
+
+ - idModule
+ ACCESS.bus device module identifier. Empty if the
+ Identification command is not supported.
+
+ - idSerial
+ 32 bit device number. A fixed serial number if it's
+ positive, a temporary serial number if negative and zero
+ if the Identification command is not supported.
+
+ - modalias
+ A combined identifier for driver selection. It has the form:
+ ddcci:<idProt>-<idType>-<idModel>-<idVendor>-<idModule>.
+ All non-alphanumeric characters (including whitespace)
+ in the model, vendor or module parts are replaced by
+ underscores to prevent issues with software like systemd-udevd.
diff --git a/Documentation/driver-api/ddcci.rst b/Documentation/driver-api/ddcci.rst
new file mode 100644
index 000000000000..2b7de1ac2656
--- /dev/null
+++ b/Documentation/driver-api/ddcci.rst
@@ -0,0 +1,122 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+==============
+DDC/CI
+==============
+
+1. Introduction
+===============
+DDC/CI is a control protocol for monitor settings supported by most
+monitors since about 2005. It is based on ACCESS.bus (an early USB predecessor).
+This could be used to create drivers that communicate with the DDCCI component,
+see ddcci-backlight for an example.
+
+2. sysfs interface
+==================
+Each detected DDC/CI device gets a directory in /sys/bus/ddcci/devices.
+The main device on a bus is named ddcci[I²C bus number].
+Internal dependent devices are named ddcci[I²C bus number]i[hex address]
+External dependent devices are named ddcci[I²C bus number]e[hex address]
+There the following files export information about the device:
+
+capabilities
+The full ACCESS.bus capabilities string. It contains the protocol,
+type and model of the device, a list of all supported command
+codes, etc. See the ACCESS.bus spec for more information.
+
+- idProt
+ACCESS.bus protocol supported by the device. Usually "monitor".
+
+- idType
+ACCESS.bus device subtype. Usually "LCD" or "CRT".
+
+- idModel
+ACCESS.bus device model identifier. Usually a shortened form of the
+device model name.
+
+- idVendor
+ACCESS.bus device vendor identifier. Empty if the Identification command
+is not supported.
+
+- idModule
+ACCESS.bus device module identifier. Empty if the Identification command
+is not supported.
+
+- idSerial
+32 bit device number. A fixed serial number if it's positive, a temporary
+serial number if negative and zero if the
+Identification command is not supported.
+
+- modalias
+A combined identifier for driver selection. It has the form:
+ddcci:<idProt>-<idType>-<idModel>-<idVendor>-<idModule>.
+All non-alphanumeric characters (including whitespace) in the model,
+vendor or module parts are replaced by underscores to prevent issues
+with software like systemd-udevd.
+
+3. Character device interface
+=============================
+For each DDC/CI device a character device in
+/dev/bus/ddcci/[I²C bus number]/ is created,
+127 devices are assigned in total.
+
+The main device on the bus is named display.
+
+Internal dependent devices are named i[hex address]
+
+External dependent devices are named e[hex address]
+
+These character devices can be used to issue commands to a DDC/CI device
+more easily than over i2c-dev devices. They should be opened unbuffered.
+To send a command just write the command byte and the arguments with a
+single write() operation. The length byte and checksum are automatically
+calculated.
+
+To read a response use read() with a buffer big enough for the expected answer.
+
+NOTE: The maximum length of a DDC/CI message is 32 bytes.
+
+4. ddcci-backlight (monitor backlight driver)
+=============================================
+[This is not specific to the DDC/CI backlight driver, if you already dealt with
+backlight drivers, skip over this.]
+
+For each monitor that supports accessing the Backlight Level White
+or the Luminance property, a backlight device of type "raw" named like the
+corresponding ddcci device is created. You can find them in /sys/class/backlight/.
+For convenience a symlink "ddcci_backlight" on the device associated with the
+display connector in /sys/class/drm/ to the backlight device is created, as
+long as the graphics driver allows to make this association.
+
+5. Limitations
+==============
+
+-Dependent devices (sub devices using DDC/CI directly wired to the monitor,
+like Calibration devices, IR remotes, etc.) aren't automatically detected.
+You can force detection of external dependent devices by writing
+"ddcci-dependent [address]" into /sys/bus/i2c/i2c-?/new_device.
+
+There is no direct synchronization if you manually change the luminance
+with the buttons on your monitor, as this can only be realized through polling
+and some monitors close their OSD every time a DDC/CI command is received.
+
+Monitor hotplugging is not detected. You need to detach/reattach the I²C driver
+or reload the module.
+
+6. Debugging
+============
+Both drivers use the dynamic debugging feature of the Linux kernel.
+To get detailed debugging messages, set the dyndbg module parameter.
+If you want to enable debugging permanently across reboots, create a file
+/etc/modprobe.d/ddcci.conf containing lines like the following before loading the modules:
+
+options ddcci dyndbg
+options ddcci-backlight dyndbg
+
+7. Origin
+============
+This driver originally came from Christoph Grenz in DKMS form here:
+https://gitlab.com/ddcci-driver-linux/ddcci-driver-linux
+with multiple backups available on the wayback machine. It also
+inlcudes a example program for the usage of this driver in
+userland.
--
2.25.1

2022-04-05 03:27:38

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v10 0/3] drivers: ddcci: add drivers for DDCCI

Hi--

On 4/3/22 16:08, Yusuf Khan wrote:
> This patch adds the DDCCI driver by Christoph Grenz into the kernel.
> The original gitlab page is loacted at https://gitlab.com/ddcci-driv
> er-linux/ddcci-driver-linux/-/tree/master.
>
...

>
> v10: Change patch title to "drivers: ddcci: add drivers for DDCCI
> and change" and change patch descriptions to add more detailed
> explanations of function.

Greg KH recently said [1] that the Subject: for each patch should not
be the same, yet they still are the same. This is not good.


[1] https://lore.kernel.org/lkml/[email protected]/

--
~Randy