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

[Lyude Paul]

On Tue, 2019-07-16 at 18:28 +0000, Li, Sun peng (Leo) wrote:
> 
> 
> 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.

Yeah-I think Ville's suggestion of making object IDs global instead of per-
device might be what we want to  go with, but getting the aux dev stuff in the
kernel first is priority imo

> 
> > 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
> > >  };
> > >  
-- 
Cheers,
	Lyude Paul