On Wed, May 07, 2014 at 09:20:41AM +1000, Dave Airlie wrote: > So now I've been playing with MST I think get the feeling I might need > some explicit locking on the AUX channel, I think at the moment the > mode_config mutex implicitly defends the aux channel as the only real > paths into it are > > a) from userspace connector probing, > b) from HPD irqs, c) Directly through the i2c-over-dp_aux /dev nodes with e.g. ddc tools. > currently both of these on i915 at least take mode config, > > however with MST I can't use mode_config for this, so I'm wondering if > I should be adding some explicit locking in the helpers or make it the > drivers problem to lock around helper access? > > Any ideas, it would most likely have to be a mutex since DPCD can in > theory sleep. Definitely needs to be a mutex since at leas i915 does sleep doing dp_aux transactions. With gmbus we've had a similar issue since that one's shared and there's the direct i2c access path from userspace (or other kernel drivers, e.g. google's pixie has a touchpad connected to the i915 gmbus controller iirc). Like Ben says that kind of locking must be done in the driver since. But on hw which doesn't have shared dp aux pads I think the locking for concurrent access from a)-c) should be done in the helper. I think we actually need 2 locks: 1. Low-level hardware lock which is just held around calls to the drivers aux.transfer callback. This avoids races between a) and b). I guess we could debate whether this should be in drivers or not, but doing it in the helper should restrict us and prevents stupid bugs. Drivers should have a locking assert in their low-level dp aux functions to make sure other (driver-internal callers) also obey this rule. 2. High-level dp aux interface lock. We need this to prevent races between a) and c), which is currently completely broken. And maybe we'll add a raw dp aux interface eventually too. All dp aux helpers which are exposed to drivers and the i2c transfer functions should grab this except the single entry point used to handle dp hpd events. Otherwise b) will deadlock against a)&c). Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
