On Sun, May 25, 2014 at 07:16:43PM -0400, Rob Clark wrote:
On Sun, May 25, 2014 at 6:10 PM, Daniel Vetter daniel@ffwll.ch wrote:
On Sat, May 24, 2014 at 8:30 PM, Rob Clark robdclark@gmail.com wrote:
@@ -8002,7 +8002,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector, if (encoder->crtc) { crtc = encoder->crtc;
mutex_lock(&crtc->mutex);
drm_modeset_lock(&crtc->mutex, NULL);This is pretty much the reason why I think switching the mode_config.mutex to a ww_mutex is a bad idea: This call here nests within the mode_config.mutex and so must be acquired. Wiring the acquire context through everything is going to be fairly horrible, especially since you must be able to bail out when trying to lock with an axquire context.
which is the call-path to here from mode_config.mutex? Is it possible to just move the locking to a higher level for a drm_modeset_lock_all()?
Connector probing. And the entire point of crtc locks was to _not_ block all screen updates while we poke for a new edid or do load balancing. If you want to test this you need a gen3/4 with tv-out (native, not through sdvo) or a gen2 or i915g/gm with vga.
My original design behind the crtc->mutex and mode_config.mutex split was that as long as the connector->crtc links didn't change you can get away with the crtc lock. setplane made a bit a mess out of this, but strictly speaking as long as you acquire all crtc locks involved in a potential plane switch (which ww_mtuxes can do) it'll be fine. Since noticing whether any connector properties change should be doable upfront I think we should try _really_ hard to keep the mode_config.mutex a plain mutex which wraps all the more fine-grained locks and is a catch-all for everything else but crtcs/planes.
That is basically how it was in one of the earlier iterations of atomic.. but didn't hold mode_config.mutex in a lot of places where it was previously held.. and while I do want to make locking more fine grained I didn't want to try and do it at the same time as landing all these other changes.
Hm, maybe we should land the locking stuff first? I.e. just convert crtc->mutex into a ww_mutex, and then add more fine-grained locking to e.g. setplane by only grabbing the locks of the involved crtcs with the w/w logic. We might need an additional plane mutex to make it work. Iirc Ville had some patches for just this.
I'm arguing this since locking at the current interface I have a really hard time seeing how we're going to implement this in i915. Still reading around though. -Daniel