From patchwork Wed Oct 31 22:03:38 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Imre Deak X-Patchwork-Id: 1681901 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork1.kernel.org (Postfix) with ESMTP id DE7B13FDDA for ; Wed, 31 Oct 2012 22:05:14 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A31E7A0C0E for ; Wed, 31 Oct 2012 15:05:14 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id BF7869EEDE; Wed, 31 Oct 2012 15:03:41 -0700 (PDT) Received: from azsmga001.ch.intel.com ([10.2.17.19]) by azsmga101.ch.intel.com with ESMTP; 31 Oct 2012 15:03:41 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.80,688,1344236400"; d="scan'208";a="211950424" Received: from ideak-desk.fi.intel.com (HELO localhost) ([10.237.72.98]) by azsmga001.ch.intel.com with ESMTP; 31 Oct 2012 15:03:40 -0700 From: Imre Deak To: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Subject: [PATCH 1/2] drm: fix order of event_lock wrt. vblank_time_clock Date: Thu, 1 Nov 2012 00:03:38 +0200 Message-Id: <1351721019-8040-1-git-send-email-imre.deak@intel.com> X-Mailer: git-send-email 1.7.9.5 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org drm_vblank_off() requires callers to hold the event_lock, while itself locking vbl_time and vblank_time_lock. This defines a dependency chain that conflicts with the one in drm_handle_vblank() where we first lock vblank_time_lock and then the event_lock. Fix this by reversing the locking order in drm_handle_vblank(). This should've triggered a lockdep warning in the exynos driver, the rest of the drivers were ok, since they are not using drm_vblank_off(), or as in the case of the intel driver were not holding the event_lock when calling drm_vblank_off(). This latter issue is addressed in the next patch. Signed-off-by: Imre Deak --- drivers/gpu/drm/drm_irq.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) Tested with i915, the rest of the drivers should be checked with lockdep enabled. diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 3a3d0ce..2193d4a 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1236,17 +1236,21 @@ done: return ret; } +/** + * drm_handle_vblank_events - send pending vblank events + * @dev: DRM device + * @crtc: crtc where the vblank(s) happened + * + * Must be called with @dev->event_lock held. + */ static void drm_handle_vblank_events(struct drm_device *dev, int crtc) { struct drm_pending_vblank_event *e, *t; struct timeval now; - unsigned long flags; unsigned int seq; seq = drm_vblank_count_and_time(dev, crtc, &now); - spin_lock_irqsave(&dev->event_lock, flags); - list_for_each_entry_safe(e, t, &dev->vblank_event_list, base.link) { if (e->pipe != crtc) continue; @@ -1266,8 +1270,6 @@ static void drm_handle_vblank_events(struct drm_device *dev, int crtc) e->event.sequence); } - spin_unlock_irqrestore(&dev->event_lock, flags); - trace_drm_vblank_event(crtc, seq); } @@ -1285,21 +1287,22 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc) s64 diff_ns; struct timeval tvblank; unsigned long irqflags; + bool ret = false; if (!dev->num_crtcs) return false; + spin_lock_irqsave(&dev->event_lock, irqflags); + /* Need timestamp lock to prevent concurrent execution with * vblank enable/disable, as this would cause inconsistent * or corrupted timestamps and vblank counts. */ - spin_lock_irqsave(&dev->vblank_time_lock, irqflags); + spin_lock(&dev->vblank_time_lock); /* Vblank irq handling disabled. Nothing to do. */ - if (!dev->vblank_enabled[crtc]) { - spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); - return false; - } + if (!dev->vblank_enabled[crtc]) + goto unlock_ret; /* Fetch corresponding timestamp for this vblank interval from * driver and store it in proper slot of timestamp ringbuffer. @@ -1340,7 +1343,12 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc) DRM_WAKEUP(&dev->vbl_queue[crtc]); drm_handle_vblank_events(dev, crtc); - spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); - return true; + ret = true; + +unlock_ret: + spin_unlock(&dev->vblank_time_lock); + spin_unlock_irqrestore(&dev->event_lock, irqflags); + + return ret; } EXPORT_SYMBOL(drm_handle_vblank);