Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Ville Syrjala <ville.syrjala@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: [PATCH] drm/i915: Clear bogus DMC BIOS/debug power well requests
Date: Wed,  5 Dec 2018 22:20:23 +0200	[thread overview]
Message-ID: <20181205202023.25398-1-ville.syrjala@linux.intel.com> (raw)

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The DMC firmware is confused and forces on the BIOS and debug
power well requests for PW1 and MISC IO on some platforms. On
BXT I measured this to waste about 10mW in the freeze system
suspend state with the SoC in s0. I didn't get conclusive
numbers for s0ix on account of the power consumption being
much more noisy than in s0.

This is pretty much undoing part of commit 42d9366d41a9
("drm/i915/gen9+: Don't remove secondary power well requests")
where we stopped sanitizing the DMCs bogus request bits.

Cc: Imre Deak <imre.deak@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 35 +++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 4350a5270423..6e349181dd1c 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -336,10 +336,17 @@ static void hsw_wait_for_power_well_disable(struct drm_i915_private *dev_priv,
 	 * Bspec doesn't require waiting for PWs to get disabled, but still do
 	 * this for paranoia. The known cases where a PW will be forced on:
 	 * - a KVMR request on any power well via the KVMR request register
-	 * - a DMC request on PW1 and MISC_IO power wells via the BIOS and
-	 *   DEBUG request registers
+	 * - a debug request on any power well via the DEBUG request register
 	 * Skip the wait in case any of the request bits are set and print a
 	 * diagnostic message.
+	 *
+	 * Note that DMC firmware will also force on the PW1 BIOS request
+	 * on SKL-CNL, MISC_IO BIOS request on SKL-GLK (although MISC_IO
+	 * does not even exits on BXT/GLK so the bit doesn't stick),
+	 * and the PW1/MISC_IO debug request on BXT. We simply clear
+	 * those spurious requests in hsw_power_well_disable() to make
+	 * sure they don't waste power. Starting from ICL the DMC firmware
+	 * has been fixed to only force on the PW1 driver request bit.
 	 */
 	wait_for((disabled = !(I915_READ(regs->driver) &
 			       HSW_PWR_WELL_CTL_STATE(pw_idx))) ||
@@ -347,6 +354,11 @@ static void hsw_wait_for_power_well_disable(struct drm_i915_private *dev_priv,
 	if (disabled)
 		return;
 
+	WARN(reqs & 3,
+	     "%s left on (bios:%d driver:%d kvmr:%d debug:%d)\n",
+	     power_well->desc->name,
+	     !!(reqs & 1), !!(reqs & 2), !!(reqs & 4), !!(reqs & 8));
+
 	DRM_DEBUG_KMS("%s forced on (bios:%d driver:%d kvmr:%d debug:%d)\n",
 		      power_well->desc->name,
 		      !!(reqs & 1), !!(reqs & 2), !!(reqs & 4), !!(reqs & 8));
@@ -409,6 +421,7 @@ static void hsw_power_well_disable(struct drm_i915_private *dev_priv,
 				   struct i915_power_well *power_well)
 {
 	const struct i915_power_well_regs *regs = power_well->desc->hsw.regs;
+	enum i915_power_well_id id = power_well->desc->id;
 	int pw_idx = power_well->desc->hsw.idx;
 	u32 val;
 
@@ -417,6 +430,24 @@ static void hsw_power_well_disable(struct drm_i915_private *dev_priv,
 
 	val = I915_READ(regs->driver);
 	I915_WRITE(regs->driver, val & ~HSW_PWR_WELL_CTL_REQ(pw_idx));
+	/*
+	 * On SKL-CNL DMC firmware forces on the BIOS request.
+	 * This wastes a bit of power so clear it.
+	 */
+	if (INTEL_GEN(dev_priv) < 11 &&
+	    (id == SKL_DISP_PW_1 || id == SKL_DISP_PW_MISC_IO)) {
+		val = I915_READ(regs->bios);
+		I915_WRITE(regs->bios, val & ~HSW_PWR_WELL_CTL_REQ(pw_idx));
+	}
+	/*
+	 * On BXT DMC firmware forces on the debug request.
+	 * This wastes a bit of power so clear it.
+	 */
+	if (IS_BROXTON(dev_priv) &&
+	    (id == SKL_DISP_PW_1 || id == SKL_DISP_PW_MISC_IO)) {
+		val = I915_READ(regs->debug);
+		I915_WRITE(regs->debug, val & ~HSW_PWR_WELL_CTL_REQ(pw_idx));
+	}
 	hsw_wait_for_power_well_disable(dev_priv, power_well);
 }
 
-- 
2.18.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

             reply	other threads:[~2018-12-05 20:20 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-05 20:20 Ville Syrjala [this message]
2018-12-05 20:54 ` ✓ Fi.CI.BAT: success for drm/i915: Clear bogus DMC BIOS/debug power well requests Patchwork
2018-12-06  2:55 ` ✓ Fi.CI.IGT: " Patchwork
2018-12-06 12:23 ` [PATCH] " Imre Deak
2018-12-06 12:47   ` Imre Deak
2018-12-07 17:13   ` Ville Syrjälä
2018-12-07 18:20     ` Imre Deak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181205202023.25398-1-ville.syrjala@linux.intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=rodrigo.vivi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox