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 at 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. -- Ville Syrj?l? Intel OTC