On Tue, Mar 28, 2023 at 6:23 PM Won Chung <wonchung@xxxxxxxxxx> wrote: > > Create a symlink pointing to USB Type-C connector for DRM connectors > when they are created. The link will be created only if the firmware is > able to describe the connection beween the two connectors. > > Currently, even if a display uses a USB Type-C port, there is no way for > the userspace to find which port is used for which display. With the > symlink, display information would be accessible from Type-C connectors > and port information would be accessible from DRM connectors. > > Associating the two subsystems, userspace would have potential to expose > and utilize more complex information. ChromeOS intend to use this > information for metrics collection. For example, we want to tell which > port is deriving which displays. Also, combined with USB PD information, > we can tell whether user is charging their device through display. > Chromium patch for parsing the symlink from the kernel is at > http://crrev.com/c/4317207. > > We already have a framework in typec port-mapper.c where it goes through > component devices and runs the bind functions for those with matching > _PLD (physical location of device). > https://elixir.bootlin.com/linux/v5.18.1/source/drivers/usb/typec/port-mapper.c > Since _PLD is ACPI specific field, this linking would only work on ACPI > x86 as long as _PLD field for Type-C connectors and DRM connectors are > correctly added to the firmware. > > Currently, USB ports and USB4 ports are added as components to create a > symlink with Type C connector. > USB: > https://lore.kernel.org/all/20211223082349.45616-1-heikki.krogerus@xxxxxxxxxxxxxxx/ > USB4: > https://lore.kernel.org/all/20220418175932.1809770-3-wonchung@xxxxxxxxxx/ > So, we follow the same pattern in this patch. > > Signed-off-by: Won Chung <wonchung@xxxxxxxxxx> > Acked-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> > --- > Changes from v4: (4 months ago) > - Update commit message with an actual use case from cros userspace > - Move component_add to before ddc check which possibly returns > - Rebased > > Changes from v3: > - Append to the commit message on why this patch is needed > > Changes from v2: > - Resend the patch to dri-devel list > > Changes from v1: > - Fix multiple lines to single line > > > drivers/gpu/drm/drm_sysfs.c | 40 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c > index 183130355997..c21312e87aa2 100644 > --- a/drivers/gpu/drm/drm_sysfs.c > +++ b/drivers/gpu/drm/drm_sysfs.c > @@ -11,12 +11,14 @@ > */ > > #include <linux/acpi.h> > +#include <linux/component.h> > #include <linux/device.h> > #include <linux/err.h> > #include <linux/export.h> > #include <linux/gfp.h> > #include <linux/i2c.h> > #include <linux/kdev_t.h> > +#include <linux/property.h> > #include <linux/slab.h> > > #include <drm/drm_accel.h> > @@ -96,6 +98,34 @@ static char *drm_devnode(const struct device *dev, umode_t *mode) > return kasprintf(GFP_KERNEL, "dri/%s", dev_name(dev)); > } > > +static int typec_connector_bind(struct device *dev, > + struct device *typec_connector, void *data) > +{ > + int ret; > + > + ret = sysfs_create_link(&dev->kobj, &typec_connector->kobj, "typec_connector"); > + if (ret) > + return ret; > + > + ret = sysfs_create_link(&typec_connector->kobj, &dev->kobj, "drm_connector"); > + if (ret) > + sysfs_remove_link(&dev->kobj, "typec_connector"); > + > + return ret; > +} > + > +static void typec_connector_unbind(struct device *dev, > + struct device *typec_connector, void *data) > +{ > + sysfs_remove_link(&typec_connector->kobj, "drm_connector"); > + sysfs_remove_link(&dev->kobj, "typec_connector"); > +} > + > +static const struct component_ops typec_connector_ops = { > + .bind = typec_connector_bind, > + .unbind = typec_connector_unbind, > +}; > + > static CLASS_ATTR_STRING(version, S_IRUGO, "drm 1.1.0 20060810"); > > /** > @@ -353,9 +383,16 @@ int drm_sysfs_connector_add(struct drm_connector *connector) > > connector->kdev = kdev; > > + if (dev_fwnode(kdev)) { > + r = component_add(kdev, &typec_connector_ops); > + if (r) > + drm_err(dev, "failed to add component\n"); Can you add more info in this drm_err message about the component that failed to make it more intuitive After this fix, Reviewed-by: Manasi Navare <navaremanasi@xxxxxxxxxxxx> Manasi > + } > + > if (connector->ddc) > return sysfs_create_link(&connector->kdev->kobj, > &connector->ddc->dev.kobj, "ddc"); > + > return 0; > > err_free: > @@ -371,6 +408,9 @@ void drm_sysfs_connector_remove(struct drm_connector *connector) > if (connector->ddc) > sysfs_remove_link(&connector->kdev->kobj, "ddc"); > > + if (dev_fwnode(connector->kdev)) > + component_del(connector->kdev, &typec_connector_ops); > + > DRM_DEBUG("removing \"%s\" from sysfs\n", > connector->name); > > -- > 2.40.0.348.gf938b09366-goog >
