| Message ID | 1470763294-7183-4-git-send-email-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@intel.com> > --- > 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@linux.intel.com> > } > > static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv, > -- > 2.5.0
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); } static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
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@intel.com> --- 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(-)