From patchwork Fri Jul 20 11:10:16 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 10536489 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 1E3A2602CA for ; Fri, 20 Jul 2018 11:10:38 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 028A22968B for ; Fri, 20 Jul 2018 11:10:38 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id EAB32296B1; Fri, 20 Jul 2018 11:10:37 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id E06872968B for ; Fri, 20 Jul 2018 11:10:36 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1001B6EF91; Fri, 20 Jul 2018 11:10:35 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id B37E86EF91 for ; Fri, 20 Jul 2018 11:10:33 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from haswell.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 12411218-1500050 for multiple; Fri, 20 Jul 2018 12:10:14 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Fri, 20 Jul 2018 12:10:16 +0100 Message-Id: <20180720111016.11497-1-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.18.0 Subject: [Intel-gfx] [PATCH] drm/i915: Mark runtime_pm as a special class of lock X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Daniel Vetter MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP Runtime power management acts as a type of "wakelock" that code must hold in order to access the device. Such a lock has all the ordering issues of a regular lock, and so it would be convenient to use lockdep to catch violations and cyclic deadlocks. In the long run, it will be interesting to use cross-release tracking so that we could mark the runtime wakelock as held for as long as the device was suspended, so that we catch whenever we might be trying to access the device having forgotten about acquiring the wakelock. Signed-off-by: Chris Wilson Cc: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.c | 16 ++++++++++++++++ drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_runtime_pm.c | 20 ++++++++++++++++++++ 4 files changed, 38 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index ef482f817ad6..c70f1f950c9f 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -921,6 +921,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv, /* This must be called before any calls to HAS_PCH_* */ intel_detect_pch(dev_priv); + intel_runtime_pm_init_early(dev_priv); intel_wopcm_init_early(&dev_priv->wopcm); intel_uc_init_early(dev_priv); intel_pm_setup(dev_priv); @@ -2403,6 +2404,7 @@ static int intel_runtime_suspend(struct device *kdev) DRM_DEBUG_KMS("Suspending device\n"); disable_rpm_wakeref_asserts(dev_priv); + lock_map_acquire(&dev_priv->runtime_pm.lock); /* * We are safe here against re-faults, since the fault handler takes @@ -2437,11 +2439,23 @@ static int intel_runtime_suspend(struct device *kdev) i915_gem_init_swizzling(dev_priv); i915_gem_restore_fences(dev_priv); + lock_map_release(&dev_priv->runtime_pm.lock); enable_rpm_wakeref_asserts(dev_priv); return ret; } + /* + * XXX We want to keep the runtime_pm.lock held across suspend. + * + * We want to treat the whole runtime sleep period as a locked state + * and catch any lock ordering problems that may arise from trying + * to access the device without first acquiring the runtime reference. + * However, this will need lockdep cross-release support as the lock + * will no longer be tied to a process. Once we have that working, we + * should endeavour on moving this lockdep_map to the PM core. + */ + lock_map_release(&dev_priv->runtime_pm.lock); enable_rpm_wakeref_asserts(dev_priv); WARN_ON_ONCE(atomic_read(&dev_priv->runtime_pm.wakeref_count)); @@ -2496,6 +2510,7 @@ static int intel_runtime_resume(struct device *kdev) WARN_ON_ONCE(atomic_read(&dev_priv->runtime_pm.wakeref_count)); disable_rpm_wakeref_asserts(dev_priv); + lock_map_acquire(&dev_priv->runtime_pm.lock); intel_opregion_notify_adapter(dev_priv, PCI_D0); dev_priv->runtime_pm.suspended = false; @@ -2537,6 +2552,7 @@ static int intel_runtime_resume(struct device *kdev) intel_enable_ipc(dev_priv); + lock_map_release(&dev_priv->runtime_pm.lock); enable_rpm_wakeref_asserts(dev_priv); if (ret) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 493e31fba3f5..dda4078b981a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1168,6 +1168,7 @@ struct skl_wm_params { * For more, read the Documentation/power/runtime_pm.txt. */ struct i915_runtime_pm { + struct lockdep_map lock; atomic_t wakeref_count; bool suspended; bool irqs_enabled; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index c6d90058d0a9..42b8bdb81d80 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1944,6 +1944,7 @@ void intel_power_domains_suspend(struct drm_i915_private *dev_priv); void intel_power_domains_verify_state(struct drm_i915_private *dev_priv); void bxt_display_core_init(struct drm_i915_private *dev_priv, bool resume); void bxt_display_core_uninit(struct drm_i915_private *dev_priv); +void intel_runtime_pm_init_early(struct drm_i915_private *dev_priv); void intel_runtime_pm_enable(struct drm_i915_private *dev_priv); const char * intel_display_power_domain_str(enum intel_display_power_domain domain); diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 30f6c9d23a9a..e4220a147cde 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -3698,6 +3698,8 @@ void intel_runtime_pm_get(struct drm_i915_private *dev_priv) ret = pm_runtime_get_sync(kdev); WARN_ONCE(ret < 0, "pm_runtime_get_sync() failed: %d\n", ret); + lock_map_acquire_read(&dev_priv->runtime_pm.lock); + atomic_inc(&dev_priv->runtime_pm.wakeref_count); assert_rpm_wakelock_held(dev_priv); } @@ -3731,6 +3733,8 @@ bool intel_runtime_pm_get_if_in_use(struct drm_i915_private *dev_priv) return false; } + lock_map_acquire_read(&dev_priv->runtime_pm.lock); + atomic_inc(&dev_priv->runtime_pm.wakeref_count); assert_rpm_wakelock_held(dev_priv); @@ -3759,9 +3763,15 @@ void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv) struct pci_dev *pdev = dev_priv->drm.pdev; struct device *kdev = &pdev->dev; +#if IS_ENABLED(CONFIG_LOCKDEP) + WARN_ON_ONCE(!lock_is_held_type(&dev_priv->runtime_pm.lock, 1)); +#endif + assert_rpm_wakelock_held(dev_priv); pm_runtime_get_noresume(kdev); + lock_map_acquire_read(&dev_priv->runtime_pm.lock); + atomic_inc(&dev_priv->runtime_pm.wakeref_count); } @@ -3781,6 +3791,8 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv) assert_rpm_wakelock_held(dev_priv); atomic_dec(&dev_priv->runtime_pm.wakeref_count); + lock_map_release(&dev_priv->runtime_pm.lock); + pm_runtime_mark_last_busy(kdev); pm_runtime_put_autosuspend(kdev); } @@ -3826,3 +3838,11 @@ void intel_runtime_pm_enable(struct drm_i915_private *dev_priv) */ pm_runtime_put_autosuspend(kdev); } + +void intel_runtime_pm_init_early(struct drm_i915_private *dev_priv) +{ + static struct lock_class_key lock_key; + + lockdep_init_map(&dev_priv->runtime_pm.lock, + "i915->runtime_pm", &lock_key, 0); +}