On Tue, Jun 25, 2019 at 12:24:40PM +0200, Andrzej Pietrasiewicz wrote: > Add generic code which creates symbolic links in sysfs, pointing to ddc > interface used by a particular video output. For example: > > ls -l /sys/class/drm/card0-HDMI-A-1/ddc > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \ > -> ../../../../soc/13880000.i2c/i2c-2 > > This makes it easy for user to associate a display with its ddc adapter > and use e.g. ddcutil to control the chosen monitor. > > This patch adds an i2c_adapter pointer to struct drm_connector. Particular > drivers can then use it instead of using their own private instance. If a > connector contains a ddc, then create a symbolic link in sysfs. > > Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxxxx> Acked-by: Daniel Vetter <daniel.vetter@xxxxxxxx> As mentioned in the other subthread, I think the important bit to sell this is rolling it out to as many drivers as feasible and collecting all the acks from driver maintainers. -Daniel > --- > drivers/gpu/drm/drm_sysfs.c | 7 +++++++ > include/drm/drm_connector.h | 11 +++++++++++ > 2 files changed, 18 insertions(+) > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c > index ad10810bc972..26d359b39785 100644 > --- a/drivers/gpu/drm/drm_sysfs.c > +++ b/drivers/gpu/drm/drm_sysfs.c > @@ -294,6 +294,9 @@ int drm_sysfs_connector_add(struct drm_connector *connector) > /* Let userspace know we have a new connector */ > drm_sysfs_hotplug_event(dev); > > + if (connector->ddc) > + return sysfs_create_link(&connector->kdev->kobj, > + &connector->ddc->dev.kobj, "ddc"); > return 0; > } > > @@ -301,6 +304,10 @@ void drm_sysfs_connector_remove(struct drm_connector *connector) > { > if (!connector->kdev) > return; > + > + if (connector->ddc) > + sysfs_remove_link(&connector->kdev->kobj, "ddc"); > + > DRM_DEBUG("removing \"%s\" from sysfs\n", > connector->name); > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index ca745d9feaf5..1ad3d1d54ba7 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -23,6 +23,7 @@ > #ifndef __DRM_CONNECTOR_H__ > #define __DRM_CONNECTOR_H__ > > +#include <linux/i2c.h> > #include <linux/list.h> > #include <linux/llist.h> > #include <linux/ctype.h> > @@ -1308,6 +1309,16 @@ struct drm_connector { > * [0]: progressive, [1]: interlaced > */ > int audio_latency[2]; > + > + /** > + * @ddc: associated ddc adapter. > + * A connector usually has its associated ddc adapter. If a driver uses > + * this field, then an appropriate symbolic link is created in connector > + * sysfs directory to make it easy for the user to tell which i2c > + * adapter is for a particular display. > + */ > + struct i2c_adapter *ddc; > + > /** > * @null_edid_counter: track sinks that give us all zeros for the EDID. > * Needed to workaround some HW bugs where we get all 0s > -- > 2.17.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch