On Tue, Aug 09, 2016 at 08:21:34PM +0300, Imre Deak wrote:
> Atm, we apply this workaround somewhat inconsistently at the following
> points: driver loading, LVDS init, eDP PPS init, system resume. As this
> workaround also affects registers other than PPS (timing, PLL) a more
> consistent way is to apply it early after the PPS HW context is known to
> be lost: driver loading, system resume and on VLV/CHV/BXT when turning
> on power domains.
>
> This is needed by the next patch that removes saving/restoring of the
> PP_CONTROL register.
>
> This also removes the incorrect programming of the workaround on HSW+
> PCH platforms which don't have the register locking mechanism.
>
> v2: (Ville)
> - Don't apply the workaround on BXT.
> - Simplify platform checks using HAS_DDI().
>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 1 +
> drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_dp.c | 3 ++-
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_lvds.c | 8 --------
> drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++++
> 6 files changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 8cfc264..0fcd1c0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1560,6 +1560,7 @@ static int i915_drm_resume(struct drm_device *dev)
> i915_gem_resume(dev);
>
> i915_restore_state(dev);
> + intel_pps_unlock_regs_wa(dev_priv);
> intel_opregion_setup(dev_priv);
>
> intel_init_pch_refclk(dev);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3d5fd06..bcbf277 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14635,6 +14635,30 @@ static bool intel_crt_present(struct drm_device *dev)
> return true;
> }
>
> +void intel_pps_unlock_regs_wa(struct drm_i915_private *dev_priv)
> +{
> + int pps_num;
> + int pps_idx;
> +
> + if (HAS_DDI(dev_priv))
> + return;
> + /*
> + * This w/a is needed at least on CPT/PPT, but to be sure apply it
> + * everywhere where registers can be write protected.
> + */
> + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> + pps_num = 2;
> + else
> + pps_num = 1;
> +
> + for (pps_idx = 0; pps_idx < pps_num; pps_idx++) {
> + u32 val = I915_READ(PP_CONTROL(pps_idx));
> +
> + val = (val & ~PANEL_UNLOCK_MASK) | PANEL_UNLOCK_REGS;
> + I915_WRITE(PP_CONTROL(pps_idx), val);
> + }
> +}
> +
> static void intel_pps_init(struct drm_i915_private *dev_priv)
> {
> if (HAS_PCH_SPLIT(dev_priv) || IS_BROXTON(dev_priv))
> @@ -14643,6 +14667,8 @@ static void intel_pps_init(struct drm_i915_private *dev_priv)
> dev_priv->pps_mmio_base = VLV_PPS_BASE;
> else
> dev_priv->pps_mmio_base = PPS_BASE;
> +
> + intel_pps_unlock_regs_wa(dev_priv);
> }
>
> static void intel_setup_outputs(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 2ef7b14..3d3d3fb 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1829,7 +1829,8 @@ static u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
> lockdep_assert_held(&dev_priv->pps_mutex);
>
> control = I915_READ(_pp_ctrl_reg(intel_dp));
> - if (!IS_BROXTON(dev)) {
> + if (WARN_ON(!HAS_DDI(dev_priv) &&
> + (control & PANEL_UNLOCK_MASK) != PANEL_UNLOCK_REGS)) {
> control &= ~PANEL_UNLOCK_MASK;
> control |= PANEL_UNLOCK_REGS;
> }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c29a429..cbce786 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1159,6 +1159,7 @@ void intel_mark_busy(struct drm_i915_private *dev_priv);
> void intel_mark_idle(struct drm_i915_private *dev_priv);
> void intel_crtc_restore_mode(struct drm_crtc *crtc);
> int intel_display_suspend(struct drm_device *dev);
> +void intel_pps_unlock_regs_wa(struct drm_i915_private *dev_priv);
> void intel_encoder_destroy(struct drm_encoder *encoder);
> int intel_connector_init(struct intel_connector *);
> struct intel_connector *intel_connector_alloc(void);
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 939f51f..35d55f1 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -977,14 +977,6 @@ void intel_lvds_init(struct drm_device *dev)
> int pipe;
> u8 pin;
>
> - /*
> - * Unlock registers and just leave them unlocked. Do this before
> - * checking quirk lists to avoid bogus WARNINGs.
> - */
> - if (HAS_PCH_SPLIT(dev_priv) || INTEL_GEN(dev_priv) <= 4)
> - I915_WRITE(PP_CONTROL(0),
> - I915_READ(PP_CONTROL(0)) | PANEL_UNLOCK_REGS);
> -
> if (!intel_lvds_supported(dev))
> return;
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 1c603bb..8f51ae3 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -592,6 +592,8 @@ void bxt_disable_dc9(struct drm_i915_private *dev_priv)
> DRM_DEBUG_KMS("Disabling DC9\n");
>
> gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> +
> + intel_pps_unlock_regs_wa(dev_priv);
> }
>
> static void assert_csr_loaded(struct drm_i915_private *dev_priv)
> @@ -1145,6 +1147,8 @@ static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
> vlv_set_power_well(dev_priv, power_well, true);
>
> vlv_display_power_well_init(dev_priv);
> +
> + intel_pps_unlock_regs_wa(dev_priv);
I'd put the call inside vlv_display_power_well_init(). That's where we
initialize all the other display registers.
Either way
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> }
>
> static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
> --
> 2.5.0
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
