[Intel-gfx] [PATCH v2] drm: Pass along the hotplug connector to the uevent

[Chris Wilson]

On Fri, Oct 21, 2016 at 02:45:41PM +0200, Daniel Vetter wrote:
> On Fri, Oct 21, 2016 at 10:14:21AM +0100, Chris Wilson wrote:
> > If we know which connector was plugged/unplugged or
> > connected/disconnected, we can pass that information along to userspace
> > inside the uevent to reduce the amount of work userspace has to perform
> > after the event (i.e. instead of looking over all connectors, it can
> > just reprobe the affected one).
> > 
> > v2: Don't convert intel_hotplug.c, it does a light probe and doesn't
> > need the force.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Villle Syrjälä <ville.syrjala at linux.intel.com>
> > Cc: Manasi Navare <manasi.d.navare at intel.com>
> 
> Nothing is preventing multiple connectors to change state at the same
> time. Or at least almost the same time, e.g. yank an entire dp mst chain.

Or from sending multiple uevents in the time it takes userspace to respond.
 
> I think we need something that works per-connector, so either send out
> uevents for one that changed individually. Or go with the epoche counter
> idea. The later has the upside that it disconnects probing from reporting:
> Sometimes only deep down in the driver's probe function do we realize that
> something changed (e.g. different edid, or different dpcd). With a helper
> to increment the epoche there would be no need to wire the hotplug status
> through the entire callchain.
> 
> To give us the same speed-up benefits like this here the only thing we'd
> need to do is make sure reading a read-only (i.e. not userspace setable
> prop) doesn't take any heavyweight locks. That should be easy to achieve.
> Of course that leaves us with num_connector ioctl calls, but that should
> be acceptable.
> 
> And it's an uabi change either way.

This is a very simple extension to the current abi that provides a small
additional hint. (And it is purely an optional hint.) I agree it is not
as impactful as being able to categorically detect whether or not connector
state has changed using an epoch counter, but it can provide a meaningful
improvement right now. :)

The simplest way to add the epoch would either be an extension to
GETCONNECTOR or GETPROPBLOB, as there is no way to query a connector
property otherwise (at least not after a cursory double check).
getconnector->pad has never been set, so it would require
GETCONNECTORv2 (just to add a new 64bit field to the struct, the
simplest being a u64 nsec timestamp of last change). A simple
extension either way.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre