• Daniel Vetter's avatar
    drm/vblank: Fixup and document timestamp update/read barriers · 99264a61
    Daniel Vetter authored
    This was a bit too much cargo-culted, so lets make it solid:
    - vblank->count doesn't need to be an atomic, writes are always done
      under the protection of dev->vblank_time_lock. Switch to an unsigned
      long instead and update comments. Note that atomic_read is just a
      normal read of a volatile variable, so no need to audit all the
      read-side access specifically.
    
    - The barriers for the vblank counter seqlock weren't complete: The
      read-side was missing the first barrier between the counter read and
      the timestamp read, it only had a barrier between the ts and the
      counter read. We need both.
    
    - Barriers weren't properly documented. Since barriers only work if
      you have them on boths sides of the transaction it's prudent to
      reference where the other side is. To avoid duplicating the
      write-side comment 3 times extract a little store_vblank() helper.
      In that helper also assert that we do indeed hold
      dev->vblank_time_lock, since in some cases the lock is acquired a
      few functions up in the callchain.
    
    Spotted while reviewing a patch from Chris Wilson to add a fastpath to
    the vblank_wait ioctl.
    
    v2: Add comment to better explain how store_vblank works, suggested by
    Chris.
    
    v3: Peter noticed that as-is the 2nd smp_wmb is redundant with the
    implicit barrier in the spin_unlock. But that can only be proven by
    auditing all callers and my point in extracting this little helper was
    to localize all the locking into just one place. Hence I think that
    additional optimization is too risky.
    
    Cc: Chris Wilson <chris@chris-wilson.co.uk>
    Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
    Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
    Cc: Michel Dänzer <michel@daenzer.net>
    Cc: Peter Hurley <peter@hurleysoftware.com>
    Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
    Reviewed-and-tested-by: default avatarMario Kleiner <mario.kleiner.de@gmail.com>
    Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
    99264a61
drm_irq.c 52.2 KB