Skip to content
  • Daniel Vetter's avatar
    drm: add per-crtc locks · 29494c17
    Daniel Vetter authored
    
    
    *drumroll*
    
    The basic idea is to protect per-crtc state which can change without
    touching the output configuration with separate mutexes, i.e.  all the
    input side state to a crtc like framebuffers, cursor settings or plane
    configuration. Holding such a crtc lock gives a read-lock on all the
    other crtc state which can be changed by e.g. a modeset.
    
    All non-crtc state is still protected by the mode_config mutex.
    Callers that need to change modeset state of a crtc (e.g. dpms or
    set_mode) need to grab both the mode_config lock and nested within any
    crtc locks.
    
    Note that since there can only ever be one holder of the mode_config
    lock we can grab the subordinate crtc locks in any order (if we need
    to grab more than one of them). Lockdep can handle such nesting with
    the mutex_lock_nest_lock call correctly.
    
    With this functions that only touch connectors/encoders but not crtcs
    only need to take the mode_config lock. The biggest such case is the
    output probing, which means that we can now pageflip and move cursors
    while the output probe code is reading an edid.
    
    Most cases neatly fall into the three buckets:
    - Only touches connectors and similar output state and so only needs
      the mode_config lock.
    - Touches the global configuration and so needs all locks.
    - Only touches the crtc input side and so only needs the crtc lock.
    
    But a few cases that need special consideration:
    
    - Load detection which requires a crtc. The mode_config lock already
      prevents a modeset change, so we can use any unused crtc as we like
      to do load detection. The only thing to consider is that such
      temporary state changes don't leak out to userspace through ioctls
      that only take the crtc look (like a pageflip). Hence the load
      detect code needs to grab the crtc of any output pipes it touches
      (but only if it touches state used by the pageflip or cursor
      ioctls).
    
    - Atomic pageflip when moving planes. The first case is sane hw, where
      planes have a fixed association with crtcs - nothing needs to be
      done there. More insane^Wflexible hw needs to have plane->crtc
      mapping which is separately protect with a lock that nests within
      the crtc lock. If the plane is unused we can just assign it to the
      current crtc and continue. But if a plane is already in use by
      another crtc we can't just reassign it.
    
      Two solution present themselves: Either go back to a slow-path which
      takes all modeset locks, potentially incure quite a hefty delay. Or
      simply disallowing such changes in one atomic pageflip - in general
      the vblanks of two crtcs are not synced, so there's no sane way to
      atomically flip such plane changes accross more than one crtc. I'd
      heavily favour the later approach, going as far as mandating it as
      part of the ABI of such a new a nuclear pageflip.
    
      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.
    
    - Optimized global modeset operations: We could just take the
      mode_config lock and then lazily lock all crtc which are affected by
      a modeset operation. This has the advantage that pageflip could
      continue unhampered on unaffected crtc. But if e.g. global resources
      like plls need to be reassigned and so affect unrelated crtcs we can
      still do that - nested locking works in any order.
    
    This patch just adds the locks and takes them in drm_modeset_lock_all,
    no real locking changes yet.
    
    v2: Need to initialize the new lock in crtc_init and lock it righ
    away, for otherwise the modeset_unlock_all below will try to unlock a
    not-locked mutex.
    
    Reviewed-by: default avatarRob Clark <rob@ti.com>
    Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
    29494c17