diff mbox

[02/10] drm/i915: Separate RPS and RC6 handling for BDW

Message ID 1507126045-24526-3-git-send-email-sagar.a.kamble@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

sagar.a.kamble@intel.com Oct. 4, 2017, 2:07 p.m. UTC 
This patch separates RC6 and RPS enabling for BDW.
RC6/RPS Disabling are handled through gen6 functions.
PM Programming guide recommends a sequence within forcewakes to
configure RC6, RPS and ring frequencies in sequence. With this
patch the order is still maintained.

v2: Update sequence numbers in RC6 programming and comment about
intent of reset_rps during gen8_enable_rps. (Radoslaw)

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

Comments

Chris Wilson Oct. 5, 2017, 5:39 p.m. UTC | #1 
Quoting Sagar Arun Kamble (2017-10-04 15:07:17)
> This patch separates RC6 and RPS enabling for BDW.
> RC6/RPS Disabling are handled through gen6 functions.
> PM Programming guide recommends a sequence within forcewakes to
> configure RC6, RPS and ring frequencies in sequence. With this
> patch the order is still maintained.
> 
> v2: Update sequence numbers in RC6 programming and comment about
> intent of reset_rps during gen8_enable_rps. (Radoslaw)
> 
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Reviewed-by: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
> ---
> @@ -6645,16 +6645,18 @@ static void gen8_enable_rps(struct drm_i915_private *dev_priv)
>         if (intel_enable_rc6() & INTEL_RC6_ENABLE)
>                 rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
>         intel_print_rc6_info(dev_priv, rc6_mask);
> -       if (IS_BROADWELL(dev_priv))
> -               I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
> -                               GEN7_RC_CTL_TO_MODE |
> -                               rc6_mask);
> -       else
> -               I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
> -                               GEN6_RC_CTL_EI_MODE(1) |
> -                               rc6_mask);
> +       I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
> +                       GEN7_RC_CTL_TO_MODE |
> +                       rc6_mask);

Ah, presumably you noted that this function is only called for bdw.
Please make this a separate patch, and fixup the earlier IS_BROADWELL as
well.

The rest of the patch makes sense,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fa916e0..908d384 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6613,7 +6613,7 @@  static void gen9_enable_rc6(struct drm_i915_private *dev_priv)
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 }
 
-static void gen8_enable_rps(struct drm_i915_private *dev_priv)
+static void gen8_enable_rc6(struct drm_i915_private *dev_priv)
 {
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
@@ -6622,7 +6622,7 @@  static void gen8_enable_rps(struct drm_i915_private *dev_priv)
 	/* 1a: Software RC state - RC0 */
 	I915_WRITE(GEN6_RC_STATE, 0);
 
-	/* 1c & 1d: Get forcewake during program sequence. Although the driver
+	/* 1b: Get forcewake during program sequence. Although the driver
 	 * hasn't enabled a state yet where we need forcewake, BIOS may have.*/
 	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
 
@@ -6645,16 +6645,18 @@  static void gen8_enable_rps(struct drm_i915_private *dev_priv)
 	if (intel_enable_rc6() & INTEL_RC6_ENABLE)
 		rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
 	intel_print_rc6_info(dev_priv, rc6_mask);
-	if (IS_BROADWELL(dev_priv))
-		I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
-				GEN7_RC_CTL_TO_MODE |
-				rc6_mask);
-	else
-		I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
-				GEN6_RC_CTL_EI_MODE(1) |
-				rc6_mask);
+	I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
+			GEN7_RC_CTL_TO_MODE |
+			rc6_mask);
 
-	/* 4 Program defaults and thresholds for RPS*/
+	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
+}
+
+static void gen8_enable_rps(struct drm_i915_private *dev_priv)
+{
+	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+
+	/* 1 Program defaults and thresholds for RPS*/
 	I915_WRITE(GEN6_RPNSWREQ,
 		   HSW_FREQUENCY(dev_priv->rps.rp1_freq));
 	I915_WRITE(GEN6_RC_VIDEO_FREQ,
@@ -6674,7 +6676,7 @@  static void gen8_enable_rps(struct drm_i915_private *dev_priv)
 
 	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
 
-	/* 5: Enable RPS */
+	/* 2: Enable RPS */
 	I915_WRITE(GEN6_RP_CONTROL,
 		   GEN6_RP_MEDIA_TURBO |
 		   GEN6_RP_MEDIA_HW_NORMAL_MODE |
@@ -6683,8 +6685,6 @@  static void gen8_enable_rps(struct drm_i915_private *dev_priv)
 		   GEN6_RP_UP_BUSY_AVG |
 		   GEN6_RP_DOWN_IDLE_AVG);
 
-	/* 6: Ring frequency + overclocking (our driver does this later */
-
 	reset_rps(dev_priv, gen6_set_rps);
 
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
@@ -7976,6 +7976,7 @@  void intel_enable_gt_powersave(struct drm_i915_private *dev_priv)
 		if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv))
 			gen6_update_ring_freq(dev_priv);
 	} else if (IS_BROADWELL(dev_priv)) {
+		gen8_enable_rc6(dev_priv);
 		gen8_enable_rps(dev_priv);
 		gen6_update_ring_freq(dev_priv);
 	} else if (INTEL_GEN(dev_priv) >= 6) {