On 08/27/2015 04:32 PM, Ville Syrjälä wrote:
On Thu, Aug 27, 2015 at 10:06:09AM +0530, Deepak wrote:
On 07/09/2015 02:16 AM, ville.syrjala@xxxxxxxxxxxxxxx wrote:
From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Add some checks that the state of the DPIO lanes is more or less what we
expect based on the overrides.
The hardware only provides two bits per channel indicating whether all
or some of the lanes are powered down, so we can't do an exact check.
Additionally, CL2 powering down before we can check it adds another
twist. To work around this we simply check for the 0 value of the
CL2 register (which is what we get when it's powered down) and
adjust our expectations.
Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_reg.h | 8 +++++
drivers/gpu/drm/i915/intel_runtime_pm.c | 54 +++++++++++++++++++++++++++++++++
2 files changed, 62 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 395f556..586a0f7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1089,6 +1089,12 @@ enum skl_disp_power_wells {
#define DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE 1 /* 1: coarse & 0 : fine */
#define CHV_PLL_DW9(ch) _PIPE(ch, _CHV_PLL_DW9_CH0, _CHV_PLL_DW9_CH1)
+#define _CHV_CMN_DW0_CH0 0x8100
+#define DPIO_ALLDL_POWERDOWN_SHIFT_CH0 19
+#define DPIO_ANYDL_POWERDOWN_SHIFT_CH0 18
+#define DPIO_ALLDL_POWERDOWN (1 << 1)
+#define DPIO_ANYDL_POWERDOWN (1 << 0)
+
#define _CHV_CMN_DW5_CH0 0x8114
#define CHV_BUFRIGHTENA1_DISABLE (0 << 20)
#define CHV_BUFRIGHTENA1_NORMAL (1 << 20)
@@ -1125,6 +1131,8 @@ enum skl_disp_power_wells {
#define _CHV_CMN_DW19_CH0 0x814c
#define _CHV_CMN_DW6_CH1 0x8098
+#define DPIO_ALLDL_POWERDOWN_SHIFT_CH1 30 /* CL2 DW6 only */
+#define DPIO_ANYDL_POWERDOWN_SHIFT_CH1 29 /* CL2 DW6 only */
#define DPIO_DYNPWRDOWNEN_CH1 (1 << 28) /* CL2 DW6 only */
#define CHV_CMN_USEDCLKCHANNEL (1 << 13)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 002b78f..a1d9676 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1026,6 +1026,58 @@ static void chv_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv,
phy, dev_priv->chv_phy_control);
}
+static void assert_chv_phy_powergate(struct drm_i915_private *dev_priv, enum dpio_phy phy,
+ enum dpio_channel ch, bool override, unsigned int mask)
+{
+ enum pipe pipe = phy == DPIO_PHY0 ? PIPE_A : PIPE_C;
Why not PIPE B? I think PIPE B can also be used to drive using PHY0?
Here we only pass 'pipe' to vlv_dpio_read() and it will just use
'DPIO_PHY(pipe)' to turn it back into an enum dpio_phy :) Since pipe A
and B are hooked up the same phy we don't need pipe B here.
I've been meaning to change vlv_dpio_read/write() to take enum dpio_phy
instead of enum pipe to get rid of this kind of sillyness, but haven't
gotten around to it yet. There may also be a few other functions that
could use the same treatment.
Thanks for the clarification
Reviewed-by: Deepak S <deepak.s@xxxxxxxxxxxxxxx>
+ u32 reg, val, expected, actual;
+
+ if (ch == DPIO_CH0)
+ reg = _CHV_CMN_DW0_CH0;
+ else
+ reg = _CHV_CMN_DW6_CH1;
+
+ mutex_lock(&dev_priv->sb_lock);
+ val = vlv_dpio_read(dev_priv, pipe, reg);
+ mutex_unlock(&dev_priv->sb_lock);
+
+ /*
+ * This assumes !override is only used when the port is disabled.
+ * All lanes should power down even without the override when
+ * the port is disabled.
+ */
+ if (!override || mask == 0xf) {
+ expected = DPIO_ALLDL_POWERDOWN | DPIO_ANYDL_POWERDOWN;
+ /*
+ * If CH1 common lane is not active anymore
+ * (eg. for pipe B DPLL) the entire channel will
+ * shut down, which causes the common lane registers
+ * to read as 0. That means we can't actually check
+ * the lane power down status bits, but as the entire
+ * register reads as 0 it's a good indication that the
+ * channel is indeed entirely powered down.
+ */
+ if (ch == DPIO_CH1 && val == 0)
+ expected = 0;
+ } else if (mask != 0x0) {
+ expected = DPIO_ANYDL_POWERDOWN;
+ } else {
+ expected = 0;
+ }
+
+ if (ch == DPIO_CH0)
+ actual = val >> DPIO_ANYDL_POWERDOWN_SHIFT_CH0;
+ else
+ actual = val >> DPIO_ANYDL_POWERDOWN_SHIFT_CH1;
+ actual &= DPIO_ALLDL_POWERDOWN | DPIO_ANYDL_POWERDOWN;
+
+ WARN(actual != expected,
+ "Unexpected DPIO lane power down: all %d, any %d. Expected: all %d, any %d. (0x%x = 0x%08x)\n",
+ !!(actual & DPIO_ALLDL_POWERDOWN), !!(actual & DPIO_ANYDL_POWERDOWN),
+ !!(expected & DPIO_ALLDL_POWERDOWN), !!(expected & DPIO_ANYDL_POWERDOWN),
+ reg, val);
+}
+
bool chv_phy_powergate_ch(struct drm_i915_private *dev_priv, enum dpio_phy phy,
enum dpio_channel ch, bool override)
{
@@ -1078,6 +1130,8 @@ void chv_phy_powergate_lanes(struct intel_encoder *encoder,
DRM_DEBUG_KMS("Power gating DPIO PHY%d CH%d lanes 0x%x (PHY_CONTROL=0x%08x)\n",
phy, ch, mask, dev_priv->chv_phy_control);
+ assert_chv_phy_powergate(dev_priv, phy, ch, override, mask);
+
mutex_unlock(&power_domains->lock);
}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
