From patchwork Mon Sep 15 12:35:01 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 4905601 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 228ABBEEA6 for ; Mon, 15 Sep 2014 14:02:47 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C1F7B202B8 for ; Mon, 15 Sep 2014 14:02:45 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id EE6342028D for ; Mon, 15 Sep 2014 14:02:39 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0959E6E21F; Mon, 15 Sep 2014 07:02:39 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-we0-f171.google.com (mail-we0-f171.google.com [74.125.82.171]) by gabe.freedesktop.org (Postfix) with ESMTP id 794096E199 for ; Mon, 15 Sep 2014 07:02:36 -0700 (PDT) Received: by mail-we0-f171.google.com with SMTP id p10so4036366wes.30 for ; Mon, 15 Sep 2014 07:02:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=ylTwTDrfdi1CYbPvJYQGcx8C+RvU3d3rGBObIOwkDhs=; b=bGJT+Fv+AXBnNr/AW7T5P8QO4y5wV3fTJM4BLJ2ztK4aU7UovsW5Z/00rbRjjXaMnB f4t2E3uGhcIeorWg/2ZiwQH/cotNnIiwomMlckHdH82z2ONSV0ZiV3ow0c6erQz4J7ha ySTpIn2Dv7aezMsojLkk9wgBku9tqgJUTpwOM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=ylTwTDrfdi1CYbPvJYQGcx8C+RvU3d3rGBObIOwkDhs=; b=dY9F3MZVadYayrVG0DE0JNv7osMNUhyUmPr8CSbzPZ9hS0ZV0XiddXnD1awmMs/u2j r21uxw/ila/1U2QspotDA2LEcbUp2ZHlcz4o1NOa5q3NHdaEKmWT2AfKdCZzC3Ae0bcK 8rZ38/MTIL/kbf7ziVj8gA3NKIF3q9VKhA58nuyju/E3JzB0HkV0Xrf/UZiFefYiCV2t eN41Z5rQ0j7VVuWsqgCGjP49xHWZx1PiXvIbr3PZVQ1WOC2CORvrrS++m7iOp/lq05M+ n2W1Jn1/z1F6R2halX9SGp5Nucy72CMRgp4YCobtao4IxXlgG+ncBlhL6lCEMZWvmktd 1Zhw== X-Gm-Message-State: ALoCoQkoL+zm2/mdoeGq6n01qKxvNfpwtTXhS8ckxWwX2nnQceCtFCRDWV93yUuIJftrAN8dPiyy X-Received: by 10.194.78.136 with SMTP id b8mr3956290wjx.106.1410789755426; Mon, 15 Sep 2014 07:02:35 -0700 (PDT) Received: from wespe.ffwll.local (84-73-67-144.dclient.hispeed.ch. [84.73.67.144]) by mx.google.com with ESMTPSA id v9sm13754504wjy.14.2014.09.15.07.02.34 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 15 Sep 2014 07:02:34 -0700 (PDT) From: Daniel Vetter To: DRI Development Subject: [PATCH 01/11] drm/i915: Clarify event_lock locking, process context Date: Mon, 15 Sep 2014 14:35:01 +0200 Message-Id: <1410784511-18460-2-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.9.3 In-Reply-To: <1410784511-18460-1-git-send-email-daniel.vetter@ffwll.ch> References: <1410784511-18460-1-git-send-email-daniel.vetter@ffwll.ch> Cc: Daniel Vetter X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,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 It's good practice to use the more specific versions for irq save spinlocks both as executable documentation and to enforce saner design. The _irqsave version really should only be used if the calling context is unknown and there's a good reason to call a function from all kinds of places. This is the first step whice replaces all occurances of _irqsave in process context with the simpler irq disable/enable variants. We don't have any funky spinlock nesting going on, especially since the event_lock is the outermost of the irq/vblank related spinlocks. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_debugfs.c | 5 ++--- drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++-------------------- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 063b44817e08..0ba5c7145240 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -516,7 +516,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) struct drm_info_node *node = m->private; struct drm_device *dev = node->minor->dev; struct drm_i915_private *dev_priv = dev->dev_private; - unsigned long flags; struct intel_crtc *crtc; int ret; @@ -529,7 +528,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) const char plane = plane_name(crtc->plane); struct intel_unpin_work *work; - spin_lock_irqsave(&dev->event_lock, flags); + spin_lock_irq(&dev->event_lock); work = crtc->unpin_work; if (work == NULL) { seq_printf(m, "No flip due on pipe %c (plane %c)\n", @@ -575,7 +574,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) seq_printf(m, "MMIO update completed? %d\n", addr == work->gtt_offset); } } - spin_unlock_irqrestore(&dev->event_lock, flags); + spin_unlock_irq(&dev->event_lock); } mutex_unlock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 82c0ad1f6a2e..dcbefe510a2a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2765,16 +2765,15 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc) struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - unsigned long flags; bool pending; if (i915_reset_in_progress(&dev_priv->gpu_error) || intel_crtc->reset_counter != atomic_read(&dev_priv->gpu_error.reset_counter)) return false; - spin_lock_irqsave(&dev->event_lock, flags); + spin_lock_irq(&dev->event_lock); pending = to_intel_crtc(crtc)->unpin_work != NULL; - spin_unlock_irqrestore(&dev->event_lock, flags); + spin_unlock_irq(&dev->event_lock); return pending; } @@ -3485,14 +3484,13 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc) !intel_crtc_has_pending_flip(crtc), 60*HZ) == 0)) { struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - unsigned long flags; - spin_lock_irqsave(&dev->event_lock, flags); + spin_lock_irq(&dev->event_lock); if (intel_crtc->unpin_work) { WARN_ONCE(1, "Removing stuck page flip\n"); page_flip_completed(intel_crtc); } - spin_unlock_irqrestore(&dev->event_lock, flags); + spin_unlock_irq(&dev->event_lock); } if (crtc->primary->fb) { @@ -9337,12 +9335,11 @@ static void intel_crtc_destroy(struct drm_crtc *crtc) struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct drm_device *dev = crtc->dev; struct intel_unpin_work *work; - unsigned long flags; - spin_lock_irqsave(&dev->event_lock, flags); + spin_lock_irq(&dev->event_lock); work = intel_crtc->unpin_work; intel_crtc->unpin_work = NULL; - spin_unlock_irqrestore(&dev->event_lock, flags); + spin_unlock_irq(&dev->event_lock); if (work) { cancel_work_sync(&work->work); @@ -9953,7 +9950,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, enum pipe pipe = intel_crtc->pipe; struct intel_unpin_work *work; struct intel_engine_cs *ring; - unsigned long flags; int ret; //trigger software GT busyness calculation @@ -9997,7 +9993,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, goto free_work; /* We borrow the event spin lock for protecting unpin_work */ - spin_lock_irqsave(&dev->event_lock, flags); + spin_lock_irq(&dev->event_lock); if (intel_crtc->unpin_work) { /* Before declaring the flip queue wedged, check if * the hardware completed the operation behind our backs. @@ -10007,7 +10003,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, page_flip_completed(intel_crtc); } else { DRM_DEBUG_DRIVER("flip queue: crtc already busy\n"); - spin_unlock_irqrestore(&dev->event_lock, flags); + spin_unlock_irq(&dev->event_lock); drm_crtc_vblank_put(crtc); kfree(work); @@ -10015,7 +10011,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, } } intel_crtc->unpin_work = work; - spin_unlock_irqrestore(&dev->event_lock, flags); + spin_unlock_irq(&dev->event_lock); if (atomic_read(&intel_crtc->unpin_work_count) >= 2) flush_workqueue(dev_priv->wq); @@ -10102,9 +10098,9 @@ cleanup_pending: mutex_unlock(&dev->struct_mutex); cleanup: - spin_lock_irqsave(&dev->event_lock, flags); + spin_lock_irq(&dev->event_lock); intel_crtc->unpin_work = NULL; - spin_unlock_irqrestore(&dev->event_lock, flags); + spin_unlock_irq(&dev->event_lock); drm_crtc_vblank_put(crtc); free_work: @@ -10115,9 +10111,9 @@ out_hang: intel_crtc_wait_for_pending_flips(crtc); ret = intel_pipe_set_base(crtc, crtc->x, crtc->y, fb); if (ret == 0 && event) { - spin_lock_irqsave(&dev->event_lock, flags); + spin_lock_irq(&dev->event_lock); drm_send_vblank_event(dev, pipe, event); - spin_unlock_irqrestore(&dev->event_lock, flags); + spin_unlock_irq(&dev->event_lock); } } return ret; @@ -13826,9 +13822,8 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file) for_each_intel_crtc(dev, crtc) { struct intel_unpin_work *work; - unsigned long irqflags; - spin_lock_irqsave(&dev->event_lock, irqflags); + spin_lock_irq(&dev->event_lock); work = crtc->unpin_work; @@ -13838,6 +13833,6 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file) work->event = NULL; } - spin_unlock_irqrestore(&dev->event_lock, irqflags); + spin_unlock_irq(&dev->event_lock); } }