From patchwork Wed Dec 6 22:47:40 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dhinakaran Pandiyan X-Patchwork-Id: 10097325 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 298FD60327 for ; Wed, 6 Dec 2017 22:48:38 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1B2BA29B49 for ; Wed, 6 Dec 2017 22:48:38 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0D55929B30; Wed, 6 Dec 2017 22:48:38 +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=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=unavailable 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 86EBC29B30 for ; Wed, 6 Dec 2017 22:48:37 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 04EEF6E741; Wed, 6 Dec 2017 22:48:17 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0E42F89F75; Wed, 6 Dec 2017 22:48:14 +0000 (UTC) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Dec 2017 14:48:13 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,369,1508828400"; d="scan'208";a="11207246" Received: from dk-thinkpad-x260.jf.intel.com (HELO localhost.localdomain) ([10.54.75.38]) by fmsmga001.fm.intel.com with ESMTP; 06 Dec 2017 14:48:13 -0800 From: Dhinakaran Pandiyan To: intel-gfx@lists.freedesktop.org Date: Wed, 6 Dec 2017 14:47:40 -0800 Message-Id: <20171206224741.8600-4-dhinakaran.pandiyan@intel.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20171206224741.8600-1-dhinakaran.pandiyan@intel.com> References: <20171206224741.8600-1-dhinakaran.pandiyan@intel.com> Cc: Daniel Vetter , Dhinakaran Pandiyan , dri-devel@lists.freedesktop.org, Rodrigo Vivi Subject: [Intel-gfx] [PATCH 4/5] drm/i915: Introduce a non-blocking power domain for vblank interrupts 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: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP When DC states are enabled and PSR is active, the hardware enters DC5/DC6 states resulting in frame counter resets. The frame counter resets mess up the vblank counting logic. So in order to disable DC states when vblank interrupts are required and to disallow DC states when vblanks interrupts are already enabled, introduce a new power domain. Since this power domain reference needs to be acquired and released in atomic context, the corresponding _get() and _put() methods skip the power_domain mutex. Signed-off-by: Dhinakaran Pandiyan --- drivers/gpu/drm/i915/i915_drv.h | 5 ++ drivers/gpu/drm/i915/intel_drv.h | 3 +- drivers/gpu/drm/i915/intel_runtime_pm.c | 125 +++++++++++++++++++++++++++++++- 3 files changed, 128 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 18d42885205b..ba9107ec1ed1 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -397,6 +397,7 @@ enum intel_display_power_domain { POWER_DOMAIN_AUX_C, POWER_DOMAIN_AUX_D, POWER_DOMAIN_GMBUS, + POWER_DOMAIN_VBLANK, POWER_DOMAIN_MODESET, POWER_DOMAIN_INIT, @@ -1475,6 +1476,10 @@ struct i915_power_well { bool has_vga:1; bool has_fuses:1; } hsw; + struct { + spinlock_t lock; + bool was_disabled; + } dc_off; }; const struct i915_power_well_ops *ops; }; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 30f791f89d64..93ca503f18bb 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1865,7 +1865,8 @@ void chv_phy_powergate_lanes(struct intel_encoder *encoder, bool override, unsigned int mask); bool chv_phy_powergate_ch(struct drm_i915_private *dev_priv, enum dpio_phy phy, enum dpio_channel ch, bool override); - +bool intel_display_power_vblank_get(struct drm_i915_private *dev_priv); +void intel_display_power_vblank_put(struct drm_i915_private *dev_priv); /* intel_pm.c */ void intel_init_clock_gating(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 f88f2c070c5f..f1807bd74242 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -126,6 +126,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) return "AUX_D"; case POWER_DOMAIN_GMBUS: return "GMBUS"; + case POWER_DOMAIN_VBLANK: + return "VBLANK"; case POWER_DOMAIN_INIT: return "INIT"; case POWER_DOMAIN_MODESET: @@ -196,10 +198,17 @@ bool __intel_display_power_is_enabled(struct drm_i915_private *dev_priv, if (power_well->always_on) continue; - if (!power_well->hw_enabled) { + if (power_well->id == SKL_DISP_PW_DC_OFF) + spin_lock(&power_well->dc_off.lock); + + if (!power_well->hw_enabled) is_enabled = false; + + if (power_well->id == SKL_DISP_PW_DC_OFF) + spin_unlock(&power_well->dc_off.lock); + + if (!is_enabled) break; - } } return is_enabled; @@ -724,6 +733,7 @@ static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv, skl_enable_dc6(dev_priv); else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5) gen9_enable_dc5(dev_priv); + power_well->dc_off.was_disabled = true; } static void i9xx_power_well_sync_hw_noop(struct drm_i915_private *dev_priv, @@ -1441,6 +1451,77 @@ 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_vblank_get - acquire a VBLANK power domain reference atomically + * @dev_priv: i915 device instance + * + * This function gets a POWER_DOMAIN_VBLANK reference without blocking and + * returns true if the DC_OFF power well was disabled since this function was + * called the last time. + */ +bool intel_display_power_vblank_get(struct drm_i915_private *dev_priv) +{ + struct i915_power_domains *power_domains = &dev_priv->power_domains; + struct i915_power_well *power_well; + bool needs_restore = false; + + if (!HAS_CSR(dev_priv) || !dev_priv->psr.source_ok) + return false; + + /* The corresponding CRTC should be active by the time driver turns on + * vblank interrupts, which in turn means the enabled pipe power domain + * would have acquired the device runtime pm reference. + */ + intel_runtime_pm_get_if_in_use(dev_priv); + + for_each_power_domain_well(dev_priv, power_well, BIT_ULL(POWER_DOMAIN_VBLANK)) { + if (power_well->id == SKL_DISP_PW_DC_OFF) + spin_lock(&power_well->dc_off.lock); + + intel_power_well_get(dev_priv, power_well); + + if (power_well->id == SKL_DISP_PW_DC_OFF) { + needs_restore = power_well->dc_off.was_disabled; + power_well->dc_off.was_disabled = false; + spin_unlock(&power_well->dc_off.lock); + } + } + atomic_inc(&power_domains->domain_use_count[POWER_DOMAIN_VBLANK]); + + return needs_restore; +} + +/** + * intel_display_power_vblank_put - drop a VBLANK power domain reference atomically + * + * A non-blocking version intel_display_power_put() specifically to control the + * DC_OFF power well in atomic contexts. + */ +void intel_display_power_vblank_put(struct drm_i915_private *dev_priv) +{ + struct i915_power_domains *power_domains = &dev_priv->power_domains; + struct i915_power_well *power_well; + + if (!HAS_CSR(dev_priv) || !dev_priv->psr.source_ok) + return; + + WARN(atomic_dec_return(&power_domains->domain_use_count[POWER_DOMAIN_VBLANK]) < 0, + "Use count on domain %s was already zero\n", + intel_display_power_domain_str(POWER_DOMAIN_VBLANK)); + + for_each_power_domain_well_rev(dev_priv, power_well, BIT_ULL(POWER_DOMAIN_VBLANK)) { + if (power_well->id == SKL_DISP_PW_DC_OFF) + spin_lock(&power_well->dc_off.lock); + + intel_power_well_put(dev_priv, power_well); + + if (power_well->id == SKL_DISP_PW_DC_OFF) + spin_unlock(&power_well->dc_off.lock); + } + + intel_runtime_pm_put(dev_priv); +} + static void __intel_display_power_get_domain(struct drm_i915_private *dev_priv, enum intel_display_power_domain domain) @@ -1448,9 +1529,16 @@ __intel_display_power_get_domain(struct drm_i915_private *dev_priv, struct i915_power_domains *power_domains = &dev_priv->power_domains; struct i915_power_well *power_well; - for_each_power_domain_well(dev_priv, power_well, BIT_ULL(domain)) + for_each_power_domain_well(dev_priv, power_well, BIT_ULL(domain)) { + if (power_well->id == SKL_DISP_PW_DC_OFF) + spin_lock(&power_well->dc_off.lock); + intel_power_well_get(dev_priv, power_well); + if (power_well->id == SKL_DISP_PW_DC_OFF) + spin_unlock(&power_well->dc_off.lock); + } + atomic_inc(&power_domains->domain_use_count[domain]); } @@ -1541,9 +1629,16 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, "Use count on domain %s was already zero\n", intel_display_power_domain_str(domain)); - for_each_power_domain_well_rev(dev_priv, power_well, BIT_ULL(domain)) + for_each_power_domain_well_rev(dev_priv, power_well, BIT_ULL(domain)) { + if (power_well->id == SKL_DISP_PW_DC_OFF) + spin_lock(&power_well->dc_off.lock); + intel_power_well_put(dev_priv, power_well); + if (power_well->id == SKL_DISP_PW_DC_OFF) + spin_unlock(&power_well->dc_off.lock); + } + mutex_unlock(&power_domains->lock); intel_runtime_pm_put(dev_priv); @@ -1706,6 +1801,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS | \ BIT_ULL(POWER_DOMAIN_MODESET) | \ BIT_ULL(POWER_DOMAIN_AUX_A) | \ + BIT_ULL(POWER_DOMAIN_VBLANK) | \ BIT_ULL(POWER_DOMAIN_INIT)) #define BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS ( \ @@ -2342,6 +2438,9 @@ static struct i915_power_well cnl_power_wells[] = { .domains = CNL_DISPLAY_DC_OFF_POWER_DOMAINS, .ops = &gen9_dc_off_power_well_ops, .id = SKL_DISP_PW_DC_OFF, + { + .dc_off.was_disabled = false, + }, }, { .name = "power well 2", @@ -2470,6 +2569,7 @@ static void assert_power_well_ids_unique(struct drm_i915_private *dev_priv) int intel_power_domains_init(struct drm_i915_private *dev_priv) { struct i915_power_domains *power_domains = &dev_priv->power_domains; + struct i915_power_well *dc_off_pw; i915_modparams.disable_power_well = sanitize_disable_power_well_option(dev_priv, @@ -2507,6 +2607,10 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv) set_power_wells(power_domains, i9xx_always_on_power_well); } + dc_off_pw = lookup_power_well(dev_priv, SKL_DISP_PW_DC_OFF); + if (dc_off_pw) + spin_lock_init(&dc_off_pw->dc_off.lock); + assert_power_well_ids_unique(dev_priv); return 0; @@ -2555,8 +2659,14 @@ static void intel_power_domains_sync_hw(struct drm_i915_private *dev_priv) mutex_lock(&power_domains->lock); for_each_power_well(dev_priv, power_well) { power_well->ops->sync_hw(dev_priv, power_well); + + if (power_well->id == SKL_DISP_PW_DC_OFF) + spin_lock(&power_well->dc_off.lock); + power_well->hw_enabled = power_well->ops->is_enabled(dev_priv, power_well); + if (power_well->id == SKL_DISP_PW_DC_OFF) + spin_unlock(&power_well->dc_off.lock); } mutex_unlock(&power_domains->lock); } @@ -3079,6 +3189,13 @@ void intel_power_domains_verify_state(struct drm_i915_private *dev_priv) if (!power_well->domains) continue; + /* + * Reading SKL_DISP_PW_DC_OFF power_well->count without its + * private spinlock should be safe here as power_well->count + * gets modified only in power_well_get() and power_well_put() + * and they are not called until drm_crtc_vblank_on(). + */ + enabled = power_well->ops->is_enabled(dev_priv, power_well); if ((power_well->count || power_well->always_on) != enabled) DRM_ERROR("power well %s state mismatch (refcount %d/enabled %d)",