From patchwork Fri Feb 12 16:55:10 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Imre Deak X-Patchwork-Id: 8293541 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 32904C02B9 for ; Fri, 12 Feb 2016 16:55:41 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 29BD120351 for ; Fri, 12 Feb 2016 16:55:40 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 32B6120380 for ; Fri, 12 Feb 2016 16:55:36 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 647426EB1F; Fri, 12 Feb 2016 08:55:35 -0800 (PST) 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 A5D1A6EB18 for ; Fri, 12 Feb 2016 08:55:29 -0800 (PST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga103.jf.intel.com with ESMTP; 12 Feb 2016 08:55:24 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,436,1449561600"; d="scan'208";a="882980401" Received: from ideak-desk.fi.intel.com ([10.237.72.61]) by orsmga001.jf.intel.com with ESMTP; 12 Feb 2016 08:55:23 -0800 From: Imre Deak To: intel-gfx@lists.freedesktop.org Date: Fri, 12 Feb 2016 18:55:10 +0200 Message-Id: <1455296121-4742-2-git-send-email-imre.deak@intel.com> X-Mailer: git-send-email 2.5.0 In-Reply-To: <1455296121-4742-1-git-send-email-imre.deak@intel.com> References: <1455296121-4742-1-git-send-email-imre.deak@intel.com> MIME-Version: 1.0 Cc: Mika Kuoppala Subject: [Intel-gfx] [PATCH 01/12] drm/i915: add helper to get a display power ref if it was already enabled 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.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, 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 We have many places in the code where we check if a given display power domain is enabled and if so access registers backed by this power domain. We assumed that some modeset lock will prevent the power reference from vanishing in the middle of the HW access, but this assumption doesn't always hold. In such cases we get either the wakeref not held, or an unclaimed register access error message. To fix this in a future-proof way that's independent of other locks wrap any such access with a get_ref_if_enabled()/put_ref() pair. Kudos to Ville and Joonas for the ideas of this new interface. CC: Mika Kuoppala CC: Chris Wilson CC: Joonas Lahtinen CC: Ville Syrjälä Signed-off-by: Imre Deak Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_drv.h | 3 + drivers/gpu/drm/i915/intel_runtime_pm.c | 111 ++++++++++++++++++++++++++------ 2 files changed, 96 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 878172a..9380ffe 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1436,6 +1436,8 @@ bool __intel_display_power_is_enabled(struct drm_i915_private *dev_priv, enum intel_display_power_domain domain); void intel_display_power_get(struct drm_i915_private *dev_priv, enum intel_display_power_domain domain); +bool intel_display_power_get_if_enabled(struct drm_i915_private *dev_priv, + enum intel_display_power_domain domain); void intel_display_power_put(struct drm_i915_private *dev_priv, enum intel_display_power_domain domain); @@ -1522,6 +1524,7 @@ enable_rpm_wakeref_asserts(struct drm_i915_private *dev_priv) enable_rpm_wakeref_asserts(dev_priv) void intel_runtime_pm_get(struct drm_i915_private *dev_priv); +bool intel_runtime_pm_get_if_in_use(struct drm_i915_private *dev_priv); void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv); void intel_runtime_pm_put(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index bbca527..a81f965 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -1435,39 +1435,90 @@ static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv, chv_set_pipe_power_well(dev_priv, power_well, false); } -/** - * intel_display_power_get - grab a power domain reference - * @dev_priv: i915 device instance - * @domain: power domain to reference - * - * This function grabs a power domain reference for @domain and ensures that the - * power domain and all its parents are powered up. Therefore users should only - * grab a reference to the innermost power domain they need. - * - * Any power domain reference obtained by this function must have a symmetric - * call to intel_display_power_put() to release the reference again. - */ -void intel_display_power_get(struct drm_i915_private *dev_priv, - enum intel_display_power_domain domain) +static void +__intel_display_power_get_domain(struct drm_i915_private *dev_priv, + enum intel_display_power_domain domain) { struct i915_power_domains *power_domains; struct i915_power_well *power_well; int i; - intel_runtime_pm_get(dev_priv); - power_domains = &dev_priv->power_domains; - mutex_lock(&power_domains->lock); - for_each_power_well(i, power_well, BIT(domain), power_domains) { if (!power_well->count++) intel_power_well_enable(dev_priv, power_well); } power_domains->domain_use_count[domain]++; +} + +/** + * intel_display_power_get - grab a power domain reference + * @dev_priv: i915 device instance + * @domain: power domain to reference + * + * This function grabs a power domain reference for @domain and ensures that the + * power domain and all its parents are powered up. Therefore users should only + * grab a reference to the innermost power domain they need. + * + * Any power domain reference obtained by this function must have a symmetric + * call to intel_display_power_put() to release the reference again. + */ +void intel_display_power_get(struct drm_i915_private *dev_priv, + enum intel_display_power_domain domain) +{ + struct i915_power_domains *power_domains; + + intel_runtime_pm_get(dev_priv); + + power_domains = &dev_priv->power_domains; + + mutex_lock(&power_domains->lock); + + __intel_display_power_get_domain(dev_priv, domain); + + mutex_unlock(&power_domains->lock); +} + +/** + * intel_display_power_get_if_enabled - grab a reference for an enabled display power domain + * @dev_priv: i915 device instance + * @domain: power domain to reference + * + * This function grabs a power domain reference for @domain and ensures that the + * power domain and all its parents are powered up. Therefore users should only + * grab a reference to the innermost power domain they need. + * + * Any power domain reference obtained by this function must have a symmetric + * call to intel_display_power_put() to release the reference again. + */ +bool intel_display_power_get_if_enabled(struct drm_i915_private *dev_priv, + enum intel_display_power_domain domain) +{ + struct i915_power_domains *power_domains; + bool is_enabled; + + if (!intel_runtime_pm_get_if_in_use(dev_priv)) + return false; + + power_domains = &dev_priv->power_domains; + + mutex_lock(&power_domains->lock); + + if (__intel_display_power_is_enabled(dev_priv, domain)) { + __intel_display_power_get_domain(dev_priv, domain); + is_enabled = true; + } else { + is_enabled = false; + } mutex_unlock(&power_domains->lock); + + if (!is_enabled) + intel_runtime_pm_put(dev_priv); + + return is_enabled; } /** @@ -2239,6 +2290,30 @@ void intel_runtime_pm_get(struct drm_i915_private *dev_priv) } /** + * intel_runtime_pm_get_if_in_use - grab a runtime pm reference if device in use + * @dev_priv: i915 device instance + * + * This function grabs a device-level runtime pm reference if the device is + * already in use and ensures that it is powered up. + * + * Any runtime pm reference obtained by this function must have a symmetric + * call to intel_runtime_pm_put() to release the reference again. + */ +bool intel_runtime_pm_get_if_in_use(struct drm_i915_private *dev_priv) +{ + struct drm_device *dev = dev_priv->dev; + struct device *device = &dev->pdev->dev; + + if (IS_ENABLED(CONFIG_PM) && !pm_runtime_get_if_in_use(device)) + return false; + + atomic_inc(&dev_priv->pm.wakeref_count); + assert_rpm_wakelock_held(dev_priv); + + return true; +} + +/** * intel_runtime_pm_get_noresume - grab a runtime pm reference * @dev_priv: i915 device instance *