public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Lyude Paul <lyude@redhat.com>
Cc: "Leo (Sunpeng) Li" <sunpeng.li@amd.com>,
	intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [RFC][PATCH 0/2] drm: PATH prop for all connectors?
Date: Thu, 11 Jul 2019 09:20:26 +0200	[thread overview]
Message-ID: <20190711072026.GC15868@phenom.ffwll.local> (raw)
In-Reply-To: <11bb4ca1d27d3ab2d169deb57951855c4ddcd563.camel@redhat.com>

On Wed, Jul 10, 2019 at 06:43:53PM -0400, Lyude Paul wrote:
> (adding sunpeng.li@amd.com 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.
>  * This seems a lot easier to write udev rules for, imho

We just had an epic discussion on irc about how connector names are _also_
not stable. At least not if you have more than drm_device (they're
allocated from a global idr, because lolz and it's apparently uapi, xrandr
will die if they're not globally unique). So actually worse than the
object id, which at least on a given machine, and for any driver not doing
something real funny with undefined load ordering, should be stable.

So maybe not so stable for soc drivers with lots of EPROBE_DEFER.

> 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.
> 
> Does this sound like a good idea? If so, I'd be happy to write up some patches
> for this

Stable naming is offiically hard. I think the most reasonable approach,
and the one every DE implements, is to look at the serial/vendor in edid
and just assume that cables/connectors/cards are fungible.

Anyway I'm not sure how useful any of this is ... There's also the
hilarity that on older i915 cards, there's both hdmi and dp "connectors"
for DP++ ports.
-Daniel

> 
> On Thu, 2019-06-13 at 21:43 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > 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@ffwll.ch>
> > Cc: Pekka Paalanen <ppaalanen@gmail.com>
> > Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> > 
> > 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(-)
> > 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2019-07-11  7:20 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-13 18:43 [RFC][PATCH 0/2] drm: PATH prop for all connectors? Ville Syrjala
2019-06-13 18:43 ` [RFC][PATCH 1/2] drm: Improve PATH prop docs Ville Syrjala
2019-06-27  7:36   ` Pekka Paalanen
2019-06-13 18:43 ` [RFC][PATCH 2/2] drm/i915: Populate PATH prop for every connector Ville Syrjala
2019-06-27  7:37   ` Pekka Paalanen
2019-06-13 18:50 ` ✗ Fi.CI.CHECKPATCH: warning for drm: PATH prop for all connectors? Patchwork
2019-06-13 20:42 ` [RFC][PATCH 0/2] " Daniel Vetter
2019-06-27  7:35   ` Pekka Paalanen
2019-06-27  7:44     ` Michel Dänzer
2019-06-14 11:38 ` ✓ Fi.CI.BAT: success for " Patchwork
2019-06-15  8:26 ` ✓ Fi.CI.IGT: " Patchwork
2019-07-10 22:43 ` [Intel-gfx] [RFC][PATCH 0/2] " Lyude Paul
2019-07-11  7:20   ` Daniel Vetter [this message]
     [not found]   ` <20190711102923.3d219640@eldfell.localdomain>
2019-07-16 14:59     ` Li, Sun peng (Leo)
2019-08-01  9:51       ` Pekka Paalanen
2019-08-01 18:24         ` Li, Sun peng (Leo)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190711072026.GC15868@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=lyude@redhat.com \
    --cc=sunpeng.li@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox