drm/vblank: Fixup and document timestamp update/read barriers
authorDaniel Vetter <daniel.vetter@ffwll.ch>
Wed, 15 Apr 2015 17:34:43 +0000 (19:34 +0200)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Mon, 4 May 2015 08:57:00 +0000 (10:57 +0200)
commit99264a61dfcda41d86d0960cf2d4c0fc2758a773
tree8af532ebfccbddc982fed5429e132ed0f4e7de64
parenta3c6d686443e912e33cebdf9cd80f94df3ded7b0
drm/vblank: Fixup and document timestamp update/read barriers

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: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-and-tested-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
drivers/gpu/drm/drm_irq.c
include/drm/drmP.h