[PATCH v2] drm/sysfs: Add mstpath attribute to connector devices

[Li, Sun peng (Leo)]

On 2019-07-10 6:50 p.m., Lyude Paul wrote:
> gah. So, I was originally going to ask "why didn't we add the connector name
> into this?", but then I realized we're doing the right thing already and just
> doing card%d-%s % (card_number, path_prop). Which means we probably really don't
> want to add a property called mstpath, since it's hardly different from path
> (whoops!).
> 
> Additionally, after some thinking I realized I may have made a mistake as I'm
> not entirely sure if we would need to specify the DRM card in the path prop for
> udev, considering that's specified in the sysfs path all ready. Even if I'm
> wrong on that though, I think it might be better not to add an mstpath property
> and just go the route of just adding a new path_v2 property that we could use
> for both MST and non-MST connector paths. (I cc'd you on the email thread about
> this, so you can read more about this there.

Funny enough, I was originally trying to make this work for SST devices.
It didn't make sense to have by-name and by-path, but only have SST
exist in the by-name symlinks. The question there was "what to use for
sst paths?" Eventually I settled with keeping this purely for user
friendliness. But since discussion is already underway for a better
'path', it makes sense to delay this.

> 
> So, I would actually suggest we just drop this patch entirely for now. We should
> be fine without it, even though the dp_aux_dev paths will be kind of ugly for a
> little while. I'd rather the rest of this series get upstream first, and try to
> do the path prop stuff separately.>

Sounds fair, going to spin up v3.

Thanks!
Leo

> 
> On Fri, 2019-07-05 at 10:32 -0400, sunpeng.li at amd.com wrote:
>> From: Leo Li <sunpeng.li at 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
>>
>> v2: remove unnecessary locking of mode_config.mutex
>>
>> Signed-off-by: Leo Li <sunpeng.li at amd.com>
>> ---
>>  drivers/gpu/drm/drm_sysfs.c | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
>> index ad10810bc972..7d483ab684a0 100644
>> --- a/drivers/gpu/drm/drm_sysfs.c
>> +++ b/drivers/gpu/drm/drm_sysfs.c
>> @@ -236,16 +236,36 @@ 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;
>> +
>> +	if (!connector->path_blob_ptr)
>> +		return ret;
>> +
>> +	path = connector->path_blob_ptr->data;
>> +	ret = snprintf(buf, PAGE_SIZE, "card%d-%s\n",
>> +		       connector->dev->primary->index, path);
>> +
>> +	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
>>  };
>>  
>