On Thu, Dec 13, 2012 at 12:54:44PM +0100, Daniel Vetter wrote:
On Thu, Dec 13, 2012 at 12:38 PM, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
And if we _really_ want such semantics, we can always get them by introducing another pageflip mutex between the mode_config.mutex and the individual crtc locks. Pageflips crossing more than one crtc would then need to take that lock first, to lock out concurrent multi-crtc pageflips.
I'm actually concerned with multi CRTC page flips, not for moving planes between CRTCs, but mainly for updating content on genlocked displays atomically. We need to avoid deadlocks between multiple CRTC locks. Trying to take the CRTC locks in the same order would be a solution, but since user space can send the props in any order, it's going to require extra of work.
Ordering CRTC locks should also work - modeset_lock_all takes them always in the same order, and as long as you only take a single crtc nested within the modeset lock that's still ok (e.g. the load-detect code). We don't have many CRTCs, so even the dumbest sort will be fast enough. A bit of work will be required to make lockdep happy. But we can achieve this by nesting CRTCs within a fake lock encapsulated by the lock/unlock helper functions.
Yeah it would mean pre-processing the object ID list in the atomic ioctl. Currently there is at most num_crtc+num_plane object IDs in the list, assuming userspace didn't send any duplicates. For each of those we'd need to take the CRTC lock. So a bit of a change to my code, but not too bad I suppose.
But this also has to handle planes that can move between CRTCs, so it's not quite as simple as that. Maybe grab the lock for plane->crtc, and once you have the lock re-check that plane->crtc didn't change before we got the lock.
We also need to change things so that plane->crtc can never be NULL. Currently when a plane is disabled, we set plane->crtc to NULL, but since we need that information for taking the lock, and to prevent two guys from accessing the same disabled plane, we can no longer allow that.