From patchwork Mon Sep 14 19:43:52 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?VmlsbGUgU3lyasOkbMOk?= X-Patchwork-Id: 7178291 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 8BFFDBEEC1 for ; Mon, 14 Sep 2015 19:44:34 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8D88620630 for ; Mon, 14 Sep 2015 19:44:33 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 8953D20621 for ; Mon, 14 Sep 2015 19:44:32 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B1B466E609; Mon, 14 Sep 2015 12:44:31 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTP id 443EF6E8DD; Mon, 14 Sep 2015 12:44:30 -0700 (PDT) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga103.jf.intel.com with ESMTP; 14 Sep 2015 12:44:30 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,530,1437462000"; d="scan'208";a="805045617" Received: from stinkbox.fi.intel.com (HELO stinkbox) ([10.237.72.174]) by fmsmga002.fm.intel.com with SMTP; 14 Sep 2015 12:44:27 -0700 Received: by stinkbox (sSMTP sendmail emulation); Mon, 14 Sep 2015 22:44:26 +0300 From: ville.syrjala@linux.intel.com To: dri-devel@lists.freedesktop.org Date: Mon, 14 Sep 2015 22:43:52 +0300 Message-Id: <1442259832-23424-12-git-send-email-ville.syrjala@linux.intel.com> X-Mailer: git-send-email 2.4.6 In-Reply-To: <1442259832-23424-1-git-send-email-ville.syrjala@linux.intel.com> References: <1442259832-23424-1-git-send-email-ville.syrjala@linux.intel.com> MIME-Version: 1.0 Cc: intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] [PATCH 11/11] drm: Fix vblank timestamp races X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Ville Syrjälä The vblank timestamp ringbuffer only has two entries, so if the vblank->count is incremented by an even number readers may end up seeing the new vblank timestamp alongside the old vblank counter value. Fix the problem by storing the vblank counter in a ringbuffer as well, and always increment the ringbuffer "slot" by one when storing a new timestamp+counter pair. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_irq.c | 40 ++++++++++++++++++++++++---------------- include/drm/drmP.h | 12 ++++++------ 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 88fbee4..8de236a 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -43,8 +43,10 @@ #include /* Access macro for slots in vblank timestamp ringbuffer. */ -#define vblanktimestamp(dev, pipe, count) \ - ((dev)->vblank[pipe].time[(count) % DRM_VBLANKTIME_RBSIZE]) +#define vblanktimestamp(dev, pipe, slot) \ + ((dev)->vblank[pipe].time[(slot) % DRM_VBLANK_RBSIZE]) +#define vblankcount(dev, pipe, slot) \ + ((dev)->vblank[pipe].count[(slot) % DRM_VBLANK_RBSIZE]) /* Retry timestamp calculation up to 3 times to satisfy * drm_timestamp_precision before giving up. @@ -76,20 +78,23 @@ module_param_named(timestamp_monotonic, drm_timestamp_monotonic, int, 0600); static void store_vblank(struct drm_device *dev, unsigned int pipe, u32 vblank_count_inc, - struct timeval *t_vblank, u32 last) + const struct timeval *t_vblank, u32 last) { struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; - u32 tslot; + u32 slot; assert_spin_locked(&dev->vblank_time_lock); + slot = vblank->slot + 1; + vblank->last = last; /* All writers hold the spinlock, but readers are serialized by - * the latching of vblank->count below. + * the latching of vblank->slot below. */ - tslot = vblank->count + vblank_count_inc; - vblanktimestamp(dev, pipe, tslot) = *t_vblank; + vblankcount(dev, pipe, slot) = + vblankcount(dev, pipe, vblank->slot) + vblank_count_inc; + vblanktimestamp(dev, pipe, slot) = *t_vblank; /* * vblank timestamp updates are protected on the write side with @@ -100,7 +105,7 @@ static void store_vblank(struct drm_device *dev, unsigned int pipe, * The read-side barriers for this are in drm_vblank_count_and_time. */ smp_wmb(); - vblank->count += vblank_count_inc; + vblank->slot = slot; smp_wmb(); } @@ -202,7 +207,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, const struct timeval *t_old; u64 diff_ns; - t_old = &vblanktimestamp(dev, pipe, vblank->count); + t_old = &vblanktimestamp(dev, pipe, vblank->slot); diff_ns = timeval_to_ns(&t_vblank) - timeval_to_ns(t_old); /* @@ -223,7 +228,8 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, DRM_DEBUG("updating vblank count on crtc %u:" " current=%u, diff=%u, hw=%u hw_last=%u\n", - pipe, vblank->count, diff, cur_vblank, vblank->last); + pipe, vblankcount(dev, pipe, vblank->slot), + diff, cur_vblank, vblank->last); if (diff == 0) { WARN_ON_ONCE(cur_vblank != vblank->last); @@ -883,7 +889,7 @@ u32 drm_vblank_count(struct drm_device *dev, int pipe) if (WARN_ON(pipe >= dev->num_crtcs)) return 0; - return vblank->count; + return vblankcount(dev, pipe, vblank->slot); } EXPORT_SYMBOL(drm_vblank_count); @@ -923,7 +929,8 @@ u32 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe, { struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; int count = DRM_TIMESTAMP_MAXRETRIES; - u32 cur_vblank; + u32 vblankcount; + u32 slot; if (WARN_ON(pipe >= dev->num_crtcs)) return 0; @@ -934,13 +941,14 @@ u32 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe, * This works like a seqlock. The write-side barriers are in store_vblank. */ do { - cur_vblank = vblank->count; + slot = vblank->slot; smp_rmb(); - *vblanktime = vblanktimestamp(dev, pipe, cur_vblank); + *vblanktime = vblanktimestamp(dev, pipe, slot); + vblankcount = vblankcount(dev, pipe, slot); smp_rmb(); - } while (cur_vblank != vblank->count && --count > 0); + } while (slot != vblank->slot && --count > 0); - return cur_vblank; + return vblankcount; } EXPORT_SYMBOL(drm_vblank_count_and_time); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 6717a7d..9de9fde 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -374,10 +374,10 @@ struct drm_master { void *driver_priv; }; -/* Size of ringbuffer for vblank timestamps. Just double-buffer +/* Size of ringbuffer for vblank counts/timestamps. Just double-buffer * in initial implementation. */ -#define DRM_VBLANKTIME_RBSIZE 2 +#define DRM_VBLANK_RBSIZE 2 /* Flags and return codes for get_vblank_timestamp() driver function. */ #define DRM_CALLED_FROM_VBLIRQ 1 @@ -692,10 +692,10 @@ struct drm_vblank_crtc { wait_queue_head_t queue; /**< VBLANK wait queue */ struct timer_list disable_timer; /* delayed disable timer */ - /* vblank counter, protected by dev->vblank_time_lock for writes */ - u32 count; - /* vblank timestamps, protected by dev->vblank_time_lock for writes */ - struct timeval time[DRM_VBLANKTIME_RBSIZE]; + u32 slot; + /* vblank timestamps and counter, protected by dev->vblank_time_lock for writes */ + struct timeval time[DRM_VBLANK_RBSIZE]; + u32 count[DRM_VBLANK_RBSIZE]; atomic_t refcount; /* number of users of vblank interruptsper crtc */ u32 last; /* protected by dev->vbl_lock, used */