Re: [RFC][PATCH 0/2] drm: PATH prop for all connectors?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 10 Jul 2019 18:43:53 -0400
Lyude Paul <lyude@xxxxxxxxxx> wrote:

> (adding sunpeng.li@xxxxxxx to the thread here, since this is relevant to the DP
> aux device work)
> 
> I mentioned this in IRC, but figured I should mention it on the ML as well so it
> can be discussed further. Honestly: I don't like the way we implement the path
> prop for MST. Mainly because
> 
>  * It looks ugly: mst:<obj-id>-<port#1>-<port#2> is ambiguous looking. I didn't
>    even realize the first number was actually supposed to be the object ID until
>    I looked at the code
>  * I strongly doubt object IDs are consistent enough for the path prop to
>    actually be as meaningful as it looks
>  * 
> Obviously we can't just remove the path property, since it's being used in
> userspace. This has me somewhat convinced that I think it might be a better idea
> to just make a whole new path_v2 prop, and document that the path prop was a bad
> idea and is now deprecated (but still functional). If we did this, we could come
> up with a much nicer MST naming scheme as well! Consider:
> 
> For a connector with the RAD 0.1 living on the topology on DP-1 on card0:
> 
> mst:DP-1:0.1
> 
> I see multiple benefits to this:
>  * Look how easy it is to read!
>  * DP-1 isn't guaranteed to be consistent, but it is certainly far more likely
>    to be consistent than an object ID.

Hi,

please, do not go with "more consistent but not fully". If the name is
not all the way persistent, then it's useless because it will break
things in rare cases. We already have a 80% or a 95% solutions, adding
one more <100% solution does not help.

>  * This seems a lot easier to write udev rules for, imho

Wait, one can write udev rules for connectors and stuff?
How? What can they do?

> The only thing I'm not sure about whether or not we should also prepend the
> connector name with the device (e.g. card0, card1, etc.). I thought this might
> be necessary at first, but thinking about it - it shouldn't be hard to figure
> out the device in question from looking at sysfs since any userspace application
> will know which DRM fd the connector comes from.

From a display server perspective using the libdrm KMS API, yes, we
always know from which device a connector comes from, and can make up a
persistent name for the device ("card0" is not a persistent name).


Thanks,
pq

> 
> Does this sound like a good idea? If so, I'd be happy to write up some patches
> for this
> 
> On Thu, 2019-06-13 at 21:43 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > 
> > Here's a possible apporoach for providing userspace with
> > some stable connector identifiers. Combine with the bus
> > name of the GPU and you should have some kind of real
> > physical path description. Unfortunately the ship has
> > sailed for MST connectors because userspace is already
> > parsing the property and expects to find certain things
> > there. So if we want stable names for those we'd probably
> > have introduce another PATH prop (PHYS_PATH?).
> > 
> > I suppose one alternative would to make the connector 
> > type_id stable. Currently that is being populated by drm 
> > core and it's just a global counter. Not sure how badly
> > things would turn out if we'd allow each driver to set
> > that. It could result in conflicting xrandr connector
> > names between different GPUs which I suppose would
> > confuse existing userspace?
> > 
> > Cc: Daniel Vetter <daniel@xxxxxxxx>
> > Cc: Pekka Paalanen <ppaalanen@xxxxxxxxx>
> > Cc: Ilia Mirkin <imirkin@xxxxxxxxxxxx>
> > 
> > Ville Syrjälä (2):
> >   drm: Improve PATH prop docs
> >   drm/i915: Populate PATH prop for every connector
> > 
> >  drivers/gpu/drm/drm_connector.c        | 13 ++++++++--
> >  drivers/gpu/drm/i915/icl_dsi.c         |  3 +++
> >  drivers/gpu/drm/i915/intel_connector.c | 20 +++++++++++++++
> >  drivers/gpu/drm/i915/intel_connector.h |  3 +++
> >  drivers/gpu/drm/i915/intel_crt.c       |  2 ++
> >  drivers/gpu/drm/i915/intel_dp.c        |  6 ++++-
> >  drivers/gpu/drm/i915/intel_dp_mst.c    |  3 +--
> >  drivers/gpu/drm/i915/intel_dvo.c       |  3 +++
> >  drivers/gpu/drm/i915/intel_hdmi.c      |  4 +++
> >  drivers/gpu/drm/i915/intel_lvds.c      |  2 ++
> >  drivers/gpu/drm/i915/intel_sdvo.c      | 35 ++++++++++++++++++--------
> >  drivers/gpu/drm/i915/intel_tv.c        |  2 ++
> >  drivers/gpu/drm/i915/vlv_dsi.c         |  3 +++
> >  13 files changed, 83 insertions(+), 16 deletions(-)
> >   
> 

Attachment: pgpVYRKLSvHAW.pgp
Description: OpenPGP digital signature

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux