| Message ID | 20190704190519.29525-4-sunpeng.li@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable MST Aux devices (v2) | expand |
On Thu, Jul 04, 2019 at 03:05:12PM -0400, sunpeng.li@amd.com wrote: > From: Leo Li <sunpeng.li@amd.com> > > This can be used to create more descriptive symlinks for MST aux > devices. Consider the following udev rule: > > SUBSYSTEM=="drm_dp_aux_dev", SUBSYSTEMS=="drm", ATTRS{mstpath}=="?*", > SYMLINK+="drm_dp_aux/by-path/$attr{mstpath}" > > The following symlinks will be created (depending on your MST topology): > > $ ls /dev/drm_dp_aux/by-path/ > card0-mst:0-1 card0-mst:0-1-1 card0-mst:0-1-8 card0-mst:0-8 > > Signed-off-by: Leo Li <sunpeng.li@amd.com> > --- > drivers/gpu/drm/drm_sysfs.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c > index ad10810bc972..53fd1f71e900 100644 > --- a/drivers/gpu/drm/drm_sysfs.c > +++ b/drivers/gpu/drm/drm_sysfs.c > @@ -236,16 +236,39 @@ static ssize_t modes_show(struct device *device, > return written; > } > > +static ssize_t mstpath_show(struct device *device, > + struct device_attribute *attr, > + char *buf) > +{ > + struct drm_connector *connector = to_drm_connector(device); > + ssize_t ret = 0; > + char *path; > + > + mutex_lock(&connector->dev->mode_config.mutex); The blob is populated when the connector is created so I don't think this lock is actually doing anything for us. One would also hope that device_unregister() protects us from racing with the removal of the attribute. Eg. if you hold a file descriptor open to the sysfs file does device_unregister() block until the fd is closed? > + if (!connector->path_blob_ptr) > + goto unlock; > + > + path = connector->path_blob_ptr->data; > + ret = snprintf(buf, PAGE_SIZE, "card%d-%s\n", > + connector->dev->primary->index, path); > + > +unlock: > + mutex_unlock(&connector->dev->mode_config.mutex); > + return ret; > +} > + > static DEVICE_ATTR_RW(status); > static DEVICE_ATTR_RO(enabled); > static DEVICE_ATTR_RO(dpms); > static DEVICE_ATTR_RO(modes); > +static DEVICE_ATTR_RO(mstpath); > > static struct attribute *connector_dev_attrs[] = { > &dev_attr_status.attr, > &dev_attr_enabled.attr, > &dev_attr_dpms.attr, > &dev_attr_modes.attr, > + &dev_attr_mstpath.attr, > NULL > }; > > -- > 2.22.0
On 2019-07-04 3:33 p.m., Ville Syrjälä wrote: > On Thu, Jul 04, 2019 at 03:05:12PM -0400, sunpeng.li@amd.com wrote: >> From: Leo Li <sunpeng.li@amd.com> >> >> This can be used to create more descriptive symlinks for MST aux >> devices. Consider the following udev rule: >> >> SUBSYSTEM=="drm_dp_aux_dev", SUBSYSTEMS=="drm", ATTRS{mstpath}=="?*", >> SYMLINK+="drm_dp_aux/by-path/$attr{mstpath}" >> >> The following symlinks will be created (depending on your MST topology): >> >> $ ls /dev/drm_dp_aux/by-path/ >> card0-mst:0-1 card0-mst:0-1-1 card0-mst:0-1-8 card0-mst:0-8 >> >> Signed-off-by: Leo Li <sunpeng.li@amd.com> >> --- >> drivers/gpu/drm/drm_sysfs.c | 23 +++++++++++++++++++++++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c >> index ad10810bc972..53fd1f71e900 100644 >> --- a/drivers/gpu/drm/drm_sysfs.c >> +++ b/drivers/gpu/drm/drm_sysfs.c >> @@ -236,16 +236,39 @@ static ssize_t modes_show(struct device *device, >> return written; >> } >> >> +static ssize_t mstpath_show(struct device *device, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct drm_connector *connector = to_drm_connector(device); >> + ssize_t ret = 0; >> + char *path; >> + >> + mutex_lock(&connector->dev->mode_config.mutex); > > The blob is populated when the connector is created so I don't think > this lock is actually doing anything for us. Right, will drop this. > > One would also hope that device_unregister() protects us from racing > with the removal of the attribute. Eg. if you hold a file descriptor > open to the sysfs file does device_unregister() block until the fd is > closed? For dpcd transactions, as long as the aux device is unregistered through drm_dp_aux_unregister_devnode(), we should be good. There's an atomic_t use_count that syncs against reads and writes. Although I'm not sure what would happen if the device was ripped out from underneath us, say, if the parent connector device is removed before calling aux_unregister_devnode(). If this does happen, I think it's more of an order-of-operations issue from the driver. V2 of patch 2 (and 7-10) actually addresses a specific occurence of this during driver unload. Regarding sysfs attrs, the kernfs underneath does seem to guarantee that any r/w is completed before removal. See kernfs_drain(), called as part of kernfs_remove(). Leo > >> + if (!connector->path_blob_ptr) >> + goto unlock; >> + >> + path = connector->path_blob_ptr->data; >> + ret = snprintf(buf, PAGE_SIZE, "card%d-%s\n", >> + connector->dev->primary->index, path); >> + >> +unlock: >> + mutex_unlock(&connector->dev->mode_config.mutex); >> + return ret; >> +} >> + >> static DEVICE_ATTR_RW(status); >> static DEVICE_ATTR_RO(enabled); >> static DEVICE_ATTR_RO(dpms); >> static DEVICE_ATTR_RO(modes); >> +static DEVICE_ATTR_RO(mstpath); >> >> static struct attribute *connector_dev_attrs[] = { >> &dev_attr_status.attr, >> &dev_attr_enabled.attr, >> &dev_attr_dpms.attr, >> &dev_attr_modes.attr, >> + &dev_attr_mstpath.attr, >> NULL >> }; >> >> -- >> 2.22.0 >
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c index ad10810bc972..53fd1f71e900 100644 --- a/drivers/gpu/drm/drm_sysfs.c +++ b/drivers/gpu/drm/drm_sysfs.c @@ -236,16 +236,39 @@ static ssize_t modes_show(struct device *device, return written; } +static ssize_t mstpath_show(struct device *device, + struct device_attribute *attr, + char *buf) +{ + struct drm_connector *connector = to_drm_connector(device); + ssize_t ret = 0; + char *path; + + mutex_lock(&connector->dev->mode_config.mutex); + if (!connector->path_blob_ptr) + goto unlock; + + path = connector->path_blob_ptr->data; + ret = snprintf(buf, PAGE_SIZE, "card%d-%s\n", + connector->dev->primary->index, path); + +unlock: + mutex_unlock(&connector->dev->mode_config.mutex); + return ret; +} + static DEVICE_ATTR_RW(status); static DEVICE_ATTR_RO(enabled); static DEVICE_ATTR_RO(dpms); static DEVICE_ATTR_RO(modes); +static DEVICE_ATTR_RO(mstpath); static struct attribute *connector_dev_attrs[] = { &dev_attr_status.attr, &dev_attr_enabled.attr, &dev_attr_dpms.attr, &dev_attr_modes.attr, + &dev_attr_mstpath.attr, NULL };