[PATCH 1/3] drm/i915: Macros to convert PM time interval values to microseconds

[PATCH 1/3] drm/i915: Macros to convert PM time interval values to microseconds

[akash.goel]

From: Akash Goel <akash.goel@intel.com>

Added a new GT_PM_INTERVAL_TO_US macro to perform the platform
specific conversion of PM time interval values to microseconds unit.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9464ba3..eacd9ae 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2939,6 +2939,15 @@ enum skl_disp_power_wells {
 				INTERVAL_1_33_US(us)) : \
 				INTERVAL_1_28_US(us))
 
+#define INTERVAL_1_28_TO_US(interval)  (((interval) << 7) / 100)
+#define INTERVAL_1_33_TO_US(interval)  (((interval) << 2) / 3)
+#define INTERVAL_0_833_TO_US(interval) (((interval) * 5)  / 6)
+#define GT_PM_INTERVAL_TO_US(dev_priv, interval) (IS_GEN9(dev_priv) ? \
+                           (IS_BROXTON(dev_priv) ? \
+                           INTERVAL_0_833_TO_US(interval) : \
+                           INTERVAL_1_33_TO_US(interval)) : \
+                           INTERVAL_1_28_TO_US(interval))
+
 /*
  * Logical Context regs
  */
-- 
1.9.2

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

[PATCH v2 3/3] drm/i915/bxt: Explicitly clear the Turbo control register

[akash.goel]

From: Akash Goel <akash.goel@intel.com>

As a part of WaGsvDisableTurbo, Driver makes an early exit from the
Gen9 Turbo enabling function, so doesn't program the Turbo Control register.
But BIOS could leave the Hw Turbo as enabled, so need to explicitly clear
out the Control register just to avoid inconsitency with debugfs
interface, which will show Turbo as enabled only and that is not expected
after adding the WaGsvDisableTurbo. Apart from this there is no problem
even if the Turbo is left enabled in the Control register, as the Up/Down
interrupts would remain masked.

v2: Add explicit clearing of Turbo Control register to *_disable_rps()
    also for the similar consistency (Chris)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b7c2186..695a464 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4587,7 +4587,7 @@ void intel_set_rps(struct drm_device *dev, u8 val)
 		gen6_set_rps(dev, val);
 }
 
-static void gen9_disable_rps(struct drm_device *dev)
+static void gen9_disable_rc6(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
@@ -4595,12 +4595,20 @@ static void gen9_disable_rps(struct drm_device *dev)
 	I915_WRITE(GEN9_PG_ENABLE, 0);
 }
 
+static void gen9_disable_rps(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	I915_WRITE(GEN6_RP_CONTROL, 0);
+}
+
 static void gen6_disable_rps(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	I915_WRITE(GEN6_RC_CONTROL, 0);
 	I915_WRITE(GEN6_RPNSWREQ, 1 << 31);
+	I915_WRITE(GEN6_RP_CONTROL, 0);
 }
 
 static void cherryview_disable_rps(struct drm_device *dev)
@@ -4804,6 +4812,16 @@ static void gen9_enable_rps(struct drm_device *dev)
 
 	/* WaGsvDisableTurbo: Workaround to disable turbo on BXT A* */
 	if (IS_BXT_REVID(dev, 0, BXT_REVID_A1)) {
+		/*
+		 * BIOS could leave the Hw Turbo enabled, so need to explicitly
+		 * clear out the Control register just to avoid inconsitency
+		 * with debugfs interface, which will show  Turbo as enabled
+		 * only and that is not expected by the User after adding the
+		 * WaGsvDisableTurbo. Apart from this there is no problem even
+		 * if the Turbo is left enabled in the Control register, as the
+		 * Up/Down interrupts would remain masked.
+		 */
+		gen9_disable_rps(dev);
 		intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 		return;
 	}
@@ -6268,9 +6286,10 @@ void intel_disable_gt_powersave(struct drm_device *dev)
 		intel_suspend_gt_powersave(dev);
 
 		mutex_lock(&dev_priv->rps.hw_lock);
-		if (INTEL_INFO(dev)->gen >= 9)
+		if (INTEL_INFO(dev)->gen >= 9) {
+			gen9_disable_rc6(dev);
 			gen9_disable_rps(dev);
-		else if (IS_CHERRYVIEW(dev))
+		} else if (IS_CHERRYVIEW(dev))
 			cherryview_disable_rps(dev);
 		else if (IS_VALLEYVIEW(dev))
 			valleyview_disable_rps(dev);
-- 
1.9.2

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

[PATCH v3 2/3] drm/i915: Correct the i915_frequency_info debugfs output

[akash.goel]

From: Akash Goel <akash.goel@intel.com>

There are certain registers, which captures the time elapsed in the
in current Up/Down EI, for how long GT has been Idle/Busy/Avg in the
current Up/Down EI and also in the previous Up/Down EI.
These register values are reported by the i915_frequency_info debugfs
interface. The Driver prints the 'us' suffix after the values, albeit
they are actually in raw form & not in microsecond units.
This patch removes the 'us' suffix so that its clear to User that values
are indeed in raw form.

v2: Present the values in microseconds unit also, after platform
    specific conversion (Chris)
v3: Add a space between raw & microsecond value (Chris)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 931dc60..5d8e3d4 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1246,12 +1246,12 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
 		rpdeclimit = I915_READ(GEN6_RP_DOWN_THRESHOLD);
 
 		rpstat = I915_READ(GEN6_RPSTAT1);
-		rpupei = I915_READ(GEN6_RP_CUR_UP_EI);
-		rpcurup = I915_READ(GEN6_RP_CUR_UP);
-		rpprevup = I915_READ(GEN6_RP_PREV_UP);
-		rpdownei = I915_READ(GEN6_RP_CUR_DOWN_EI);
-		rpcurdown = I915_READ(GEN6_RP_CUR_DOWN);
-		rpprevdown = I915_READ(GEN6_RP_PREV_DOWN);
+		rpupei = I915_READ(GEN6_RP_CUR_UP_EI) & GEN6_CURICONT_MASK;
+		rpcurup = I915_READ(GEN6_RP_CUR_UP) & GEN6_CURBSYTAVG_MASK;
+		rpprevup = I915_READ(GEN6_RP_PREV_UP) & GEN6_CURBSYTAVG_MASK;
+		rpdownei = I915_READ(GEN6_RP_CUR_DOWN_EI) & GEN6_CURIAVG_MASK;
+		rpcurdown = I915_READ(GEN6_RP_CUR_DOWN) & GEN6_CURBSYTAVG_MASK;
+		rpprevdown = I915_READ(GEN6_RP_PREV_DOWN) & GEN6_CURBSYTAVG_MASK;
 		if (IS_GEN9(dev))
 			cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
 		else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
@@ -1291,21 +1291,21 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
 		seq_printf(m, "RPDECLIMIT: 0x%08x\n", rpdeclimit);
 		seq_printf(m, "RPNSWREQ: %dMHz\n", reqf);
 		seq_printf(m, "CAGF: %dMHz\n", cagf);
-		seq_printf(m, "RP CUR UP EI: %dus\n", rpupei &
-			   GEN6_CURICONT_MASK);
-		seq_printf(m, "RP CUR UP: %dus\n", rpcurup &
-			   GEN6_CURBSYTAVG_MASK);
-		seq_printf(m, "RP PREV UP: %dus\n", rpprevup &
-			   GEN6_CURBSYTAVG_MASK);
+		seq_printf(m, "RP CUR UP EI: %d (%dus)\n",
+			   rpupei, GT_PM_INTERVAL_TO_US(dev_priv, rpupei));
+		seq_printf(m, "RP CUR UP: %d (%dus)\n",
+			   rpcurup, GT_PM_INTERVAL_TO_US(dev_priv, rpcurup));
+		seq_printf(m, "RP PREV UP: %d (%dus)\n",
+			   rpprevup, GT_PM_INTERVAL_TO_US(dev_priv, rpprevup));
 		seq_printf(m, "Up threshold: %d%%\n",
 			   dev_priv->rps.up_threshold);
 
-		seq_printf(m, "RP CUR DOWN EI: %dus\n", rpdownei &
-			   GEN6_CURIAVG_MASK);
-		seq_printf(m, "RP CUR DOWN: %dus\n", rpcurdown &
-			   GEN6_CURBSYTAVG_MASK);
-		seq_printf(m, "RP PREV DOWN: %dus\n", rpprevdown &
-			   GEN6_CURBSYTAVG_MASK);
+		seq_printf(m, "RP CUR DOWN EI: %d (%dus)\n",
+			   rpdownei, GT_PM_INTERVAL_TO_US(dev_priv, rpdownei));
+		seq_printf(m, "RP CUR DOWN: %d (%dus)\n",
+			   rpcurdown, GT_PM_INTERVAL_TO_US(dev_priv, rpcurdown));
+		seq_printf(m, "RP PREV DOWN: %d (%dus)\n",
+			   rpprevdown, GT_PM_INTERVAL_TO_US(dev_priv, rpprevdown));
 		seq_printf(m, "Down threshold: %d%%\n",
 			   dev_priv->rps.down_threshold);
 
-- 
1.9.2

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

✗ Fi.CI.BAT: warning for series starting with [1/3] drm/i915: Macros to convert PM time interval values to microseconds

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Macros to convert PM time interval values to microseconds
URL   : https://patchwork.freedesktop.org/series/6184/
State : warning

== Summary ==

Series 6184v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/6184/revisions/1/mbox/

Test kms_force_connector_basic:
        Subgroup force-edid:
                pass       -> SKIP       (ivb-t430s)

bdw-ultra        total:193  pass:170  dwarn:0   dfail:0   fail:0   skip:23 
bsw-nuc-2        total:192  pass:153  dwarn:0   dfail:0   fail:0   skip:39 
byt-nuc          total:192  pass:154  dwarn:0   dfail:0   fail:0   skip:38 
ilk-hp8440p      total:193  pass:136  dwarn:0   dfail:0   fail:0   skip:57 
ivb-t430s        total:193  pass:164  dwarn:0   dfail:0   fail:0   skip:29 
skl-i7k-2        total:193  pass:168  dwarn:0   dfail:0   fail:0   skip:25 
skl-nuci5        total:193  pass:182  dwarn:0   dfail:0   fail:0   skip:11 

Results at /archive/results/CI_IGT_test/Patchwork_2035/

340c485ad98d0ec0369a3b18d4a09938f3f5537d drm-intel-nightly: 2016y-04m-22d-17h-32m-25s UTC integration manifest
10af37a drm/i915/bxt: Explicitly clear the Turbo control register
220c56f drm/i915: Correct the i915_frequency_info debugfs output
7768dd0 drm/i915: Macros to convert PM time interval values to microseconds

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

Re: ✗ Fi.CI.BAT: warning for series starting with [1/3] drm/i915: Macros to convert PM time interval values to microseconds

[Chris Wilson]

On Sun, Apr 24, 2016 at 09:53:27AM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/3] drm/i915: Macros to convert PM time interval values to microseconds
> URL   : https://patchwork.freedesktop.org/series/6184/
> State : warning
> 
> == Summary ==
> 
> Series 6184v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/6184/revisions/1/mbox/
> 
> Test kms_force_connector_basic:
>         Subgroup force-edid:
>                 pass       -> SKIP       (ivb-t430s)

Unrelated, and pushed.
Thanks,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Macros to convert PM time interval values to microseconds

[Daniel Vetter]

On Sat, Apr 23, 2016 at 12:05:44AM +0530, akash.goel@intel.com wrote:
> From: Akash Goel <akash.goel@intel.com>
> 
> Added a new GT_PM_INTERVAL_TO_US macro to perform the platform
> specific conversion of PM time interval values to microseconds unit.
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Akash Goel <akash.goel@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9464ba3..eacd9ae 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2939,6 +2939,15 @@ enum skl_disp_power_wells {
>  				INTERVAL_1_33_US(us)) : \
>  				INTERVAL_1_28_US(us))
>  
> +#define INTERVAL_1_28_TO_US(interval)  (((interval) << 7) / 100)
> +#define INTERVAL_1_33_TO_US(interval)  (((interval) << 2) / 3)
> +#define INTERVAL_0_833_TO_US(interval) (((interval) * 5)  / 6)
> +#define GT_PM_INTERVAL_TO_US(dev_priv, interval) (IS_GEN9(dev_priv) ? \
> +                           (IS_BROXTON(dev_priv) ? \
> +                           INTERVAL_0_833_TO_US(interval) : \
> +                           INTERVAL_1_33_TO_US(interval)) : \
> +                           INTERVAL_1_28_TO_US(interval))

Bikeshed, but I think once we have platform logic in there a real inline
functino starts to look better.

Feel free to ignore since already pushed.
-Daniel

> +
>  /*
>   * Logical Context regs
>   */
> -- 
> 1.9.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Macros to convert PM time interval values to microseconds

[Chris Wilson]

On Tue, Apr 26, 2016 at 04:06:05PM +0200, Daniel Vetter wrote:
> On Sat, Apr 23, 2016 at 12:05:44AM +0530, akash.goel@intel.com wrote:
> > From: Akash Goel <akash.goel@intel.com>
> > 
> > Added a new GT_PM_INTERVAL_TO_US macro to perform the platform
> > specific conversion of PM time interval values to microseconds unit.
> > 
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Akash Goel <akash.goel@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 9464ba3..eacd9ae 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2939,6 +2939,15 @@ enum skl_disp_power_wells {
> >  				INTERVAL_1_33_US(us)) : \
> >  				INTERVAL_1_28_US(us))
> >  
> > +#define INTERVAL_1_28_TO_US(interval)  (((interval) << 7) / 100)
> > +#define INTERVAL_1_33_TO_US(interval)  (((interval) << 2) / 3)
> > +#define INTERVAL_0_833_TO_US(interval) (((interval) * 5)  / 6)
> > +#define GT_PM_INTERVAL_TO_US(dev_priv, interval) (IS_GEN9(dev_priv) ? \
> > +                           (IS_BROXTON(dev_priv) ? \
> > +                           INTERVAL_0_833_TO_US(interval) : \
> > +                           INTERVAL_1_33_TO_US(interval)) : \
> > +                           INTERVAL_1_28_TO_US(interval))
> 
> Bikeshed, but I think once we have platform logic in there a real inline
> functino starts to look better.

Or when we have many, a feature flag. Could be worse, each could be
expanding to devid.
> 
> Feel free to ignore since already pushed.

Yeah, since this was cut'n'paste for debugfs, I wasn't that concerned
about elegance just that I could compare the macros and check that it
was the inverse.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx