On Fri, Jul 19, 2019 at 08:33:49PM +0200, Daniel Vetter wrote:
On Fri, Jul 19, 2019 at 7:06 PM Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Fri, Jul 19, 2019 at 05:23:12PM +0200, Daniel Vetter wrote:
Noticed while reviewing code. I'm not sure whether this might or might not explain some of the missed vblank hilarity we've been seeing. I think those all go through the vblank completion event, which has unconditional barriers - it always takes the spinlock. Therefore no cc stable.
v2:
- Barrriers are hard, put them in in the right order (Chris).
- Improve the comments a bit.
Cc: Rodrigo Siqueira rodrigosiqueiramelo@gmail.com Cc: Chris Wilson chris@chris-wilson.co.uk Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/drm_vblank.c | 38 +++++++++++++++++++++++++++++++++++- include/drm/drm_vblank.h | 13 +++++++++++- 2 files changed, 49 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 603ab105125d..eb2a8304536c 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -295,11 +295,23 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, static u64 drm_vblank_count(struct drm_device *dev, unsigned int pipe) { struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
u64 count; if (WARN_ON(pipe >= dev->num_crtcs)) return 0;
return vblank->count;
count = vblank->count;Hmm. This is now a 64bit quantity, which means on 32bit the load/store won't be atomic. That doesn't seem particularly great.
Hm ... so read-side seqno here? At least for 32bit, but not sure that's worth it, probably simpler to just do it unconditionally.
Or make it atomic64_t perhaps?
Otoh ... do we care? This matters like once every every year at 120Hz ...
Dunno. Might avoid a few odd bug reports maybe.