On Mon, Apr 04, 2016 at 05:27:10PM +0300, Imre Deak wrote:
> I caught a few errors in our current PHY/CDCLK programming by sanity
> checking the actual programmed state, so I thought it would be also
> useful for the future. In addition to verifying the state after
> programming it also verify it after exiting DC5, to make sure DMC
> restored/kept intact everything related.
>
> v2:
> - Inlining __phy_reg_verify_state() doesn't make sense and also
> incorrect, so don't do it (PW/CI gcc)
> v3:
> - Rebase on latest -nightly
>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
Reviewed-by: David Weinehall <david.weinehall@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_ddi.c | 124 +++++++++++++++++++++++++++++++-
> drivers/gpu/drm/i915/intel_display.c | 5 ++
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> drivers/gpu/drm/i915/intel_runtime_pm.c | 8 +++
> 5 files changed, 138 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index dd18772..6f4a721 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1908,6 +1908,7 @@ struct drm_i915_private {
> * crappiness (can't read out DPLL_MD for pipes B & C).
> */
> u32 chv_dpll_md[I915_MAX_PIPES];
> + u32 bxt_phy_grc;
>
> u32 suspend_count;
> bool suspended_to_idle;
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index d944bff..fd20119 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1753,6 +1753,13 @@ static bool broxton_phy_is_enabled(struct drm_i915_private *dev_priv,
> return true;
> }
>
> +static u32 broxton_get_grc(struct drm_i915_private *dev_priv, enum dpio_phy phy)
> +{
> + u32 val = I915_READ(BXT_PORT_REF_DW6(phy));
> +
> + return (val & GRC_CODE_MASK) >> GRC_CODE_SHIFT;
> +}
> +
> static void broxton_phy_init(struct drm_i915_private *dev_priv,
> enum dpio_phy phy)
> {
> @@ -1762,6 +1769,9 @@ static void broxton_phy_init(struct drm_i915_private *dev_priv,
> if (broxton_phy_is_enabled(dev_priv, phy)) {
> DRM_DEBUG_DRIVER("DDI PHY %d already enabled, "
> "won't reprogram it\n", phy);
> + /* Still read out the GRC value for state verification */
> + if (phy == DPIO_PHY1)
> + dev_priv->bxt_phy_grc = broxton_get_grc(dev_priv, phy);
>
> return;
> }
> @@ -1857,8 +1867,8 @@ static void broxton_phy_init(struct drm_i915_private *dev_priv,
> 10))
> DRM_ERROR("timeout waiting for PHY1 GRC\n");
>
> - val = I915_READ(BXT_PORT_REF_DW6(DPIO_PHY1));
> - val = (val & GRC_CODE_MASK) >> GRC_CODE_SHIFT;
> + val = dev_priv->bxt_phy_grc = broxton_get_grc(dev_priv,
> + DPIO_PHY1);
> grc_code = val << GRC_CODE_FAST_SHIFT |
> val << GRC_CODE_SLOW_SHIFT |
> val;
> @@ -1901,6 +1911,116 @@ void broxton_ddi_phy_uninit(struct drm_i915_private *dev_priv)
> broxton_phy_uninit(dev_priv, DPIO_PHY0);
> }
>
> +static bool __printf(6, 7)
> +__phy_reg_verify_state(struct drm_i915_private *dev_priv, enum dpio_phy phy,
> + i915_reg_t reg, u32 mask, u32 expected,
> + const char *reg_fmt, ...)
> +{
> + struct va_format vaf;
> + va_list args;
> + u32 val;
> +
> + val = I915_READ(reg);
> + if ((val & mask) == expected)
> + return true;
> +
> + va_start(args, reg_fmt);
> + vaf.fmt = reg_fmt;
> + vaf.va = &args;
> +
> + DRM_DEBUG_DRIVER("DDI PHY %d reg %pV [%08x] state mismatch: "
> + "current %08x, expected %08x (mask %08x)\n",
> + phy, &vaf, reg.reg, val, (val & ~mask) | expected,
> + mask);
> +
> + va_end(args);
> +
> + return false;
> +}
> +
> +static bool broxton_phy_verify_state(struct drm_i915_private *dev_priv,
> + enum dpio_phy phy)
> +{
> + enum port port;
> + u32 ports;
> + uint32_t mask;
> + bool ok;
> +
> +#define _CHK(reg, mask, exp, fmt, ...) \
> + __phy_reg_verify_state(dev_priv, phy, reg, mask, exp, fmt, \
> + ## __VA_ARGS__)
> +
> + /* We expect the PHY to be always enabled */
> + if (!broxton_phy_is_enabled(dev_priv, phy))
> + return false;
> +
> + ok = true;
> +
> + if (phy == DPIO_PHY0)
> + ports = BIT(PORT_B) | BIT(PORT_C);
> + else
> + ports = BIT(PORT_A);
> +
> + for_each_port_masked(port, ports) {
> + int lane;
> +
> + for (lane = 0; lane < 4; lane++)
> + ok &= _CHK(BXT_PORT_TX_DW14_LN(port, lane),
> + LATENCY_OPTIM,
> + lane != 1 ? LATENCY_OPTIM : 0,
> + "BXT_PORT_TX_DW14_LN(%d, %d)", port, lane);
> + }
> +
> + /* PLL Rcomp code offset */
> + ok &= _CHK(BXT_PORT_CL1CM_DW9(phy),
> + IREF0RC_OFFSET_MASK, 0xe4 << IREF0RC_OFFSET_SHIFT,
> + "BXT_PORT_CL1CM_DW9(%d)", phy);
> + ok &= _CHK(BXT_PORT_CL1CM_DW10(phy),
> + IREF1RC_OFFSET_MASK, 0xe4 << IREF1RC_OFFSET_SHIFT,
> + "BXT_PORT_CL1CM_DW10(%d)", phy);
> +
> + /* Power gating */
> + mask = OCL1_POWER_DOWN_EN | DW28_OLDO_DYN_PWR_DOWN_EN | SUS_CLK_CONFIG;
> + ok &= _CHK(BXT_PORT_CL1CM_DW28(phy), mask, mask,
> + "BXT_PORT_CL1CM_DW28(%d)", phy);
> +
> + if (phy == DPIO_PHY0)
> + ok &= _CHK(BXT_PORT_CL2CM_DW6_BC,
> + DW6_OLDO_DYN_PWR_DOWN_EN, DW6_OLDO_DYN_PWR_DOWN_EN,
> + "BXT_PORT_CL2CM_DW6_BC");
> +
> + /*
> + * TODO: Verify BXT_PORT_CL1CM_DW30 bit OCL2_LDOFUSE_PWR_DIS,
> + * at least on stepping A this bit is read-only and fixed at 0.
> + */
> +
> + if (phy == DPIO_PHY0) {
> + u32 grc_code = dev_priv->bxt_phy_grc;
> +
> + grc_code = grc_code << GRC_CODE_FAST_SHIFT |
> + grc_code << GRC_CODE_SLOW_SHIFT |
> + grc_code;
> + mask = GRC_CODE_FAST_MASK | GRC_CODE_SLOW_MASK |
> + GRC_CODE_NOM_MASK;
> + ok &= _CHK(BXT_PORT_REF_DW6(DPIO_PHY0), mask, grc_code,
> + "BXT_PORT_REF_DW6(%d)", DPIO_PHY0);
> +
> + mask = GRC_DIS | GRC_RDY_OVRD;
> + ok &= _CHK(BXT_PORT_REF_DW8(DPIO_PHY0), mask, mask,
> + "BXT_PORT_REF_DW8(%d)", DPIO_PHY0);
> + }
> +
> + return ok;
> +#undef _CHK
> +}
> +
> +void broxton_ddi_phy_verify_state(struct drm_i915_private *dev_priv)
> +{
> + if (!broxton_phy_verify_state(dev_priv, DPIO_PHY0) ||
> + !broxton_phy_verify_state(dev_priv, DPIO_PHY1))
> + i915_report_error(dev_priv, "DDI PHY state mismatch\n");
> +}
> +
> void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
> {
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8999fe8..842ca5d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5460,6 +5460,11 @@ static bool broxton_cdclk_is_enabled(struct drm_i915_private *dev_priv)
> return true;
> }
>
> +bool broxton_cdclk_verify_state(struct drm_i915_private *dev_priv)
> +{
> + return broxton_cdclk_is_enabled(dev_priv);
> +}
> +
> void broxton_init_cdclk(struct drm_i915_private *dev_priv)
> {
> /* check if cd clock is enabled */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4c2083d..4708c49 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1224,8 +1224,10 @@ void hsw_enable_pc8(struct drm_i915_private *dev_priv);
> void hsw_disable_pc8(struct drm_i915_private *dev_priv);
> void broxton_init_cdclk(struct drm_i915_private *dev_priv);
> void broxton_uninit_cdclk(struct drm_i915_private *dev_priv);
> +bool broxton_cdclk_verify_state(struct drm_i915_private *dev_priv);
> void broxton_ddi_phy_init(struct drm_i915_private *dev_priv);
> void broxton_ddi_phy_uninit(struct drm_i915_private *dev_priv);
> +void broxton_ddi_phy_verify_state(struct drm_i915_private *dev_priv);
> void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> void skl_init_cdclk(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 e60b02e..59b30c9 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -797,6 +797,11 @@ static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
> struct i915_power_well *power_well)
> {
> gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> +
> + if (IS_BROXTON(dev_priv)) {
> + broxton_cdclk_verify_state(dev_priv);
> + broxton_ddi_phy_verify_state(dev_priv);
> + }
> }
>
> static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
> @@ -2184,6 +2189,9 @@ void bxt_display_core_init(struct drm_i915_private *dev_priv,
> broxton_init_cdclk(dev_priv);
> broxton_ddi_phy_init(dev_priv);
>
> + broxton_cdclk_verify_state(dev_priv);
> + broxton_ddi_phy_verify_state(dev_priv);
> +
> if (resume && dev_priv->csr.dmc_payload)
> intel_csr_load_program(dev_priv);
> }
> --
> 2.5.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
