On Fri, 13 Jun 2014 13:37:56 +0300
ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Now that the CMNRESET deassert is part of the cmnlane power well,
> intel_reset_dpio() is called too late to make any difference. We've
> deasserted CMNRESET by that time, and so the off+on toggle w/a will
> never kick in.
>
> Move the workaround to intel_power_domains_init_hw() where it gets
> called before we enable the init power domain.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_display.c | 23 -------------
> drivers/gpu/drm/i915/intel_drv.h | 3 +-
> drivers/gpu/drm/i915/intel_pm.c | 67 ++++++++++++++++++++++++++++++------
> 3 files changed, 58 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 33cc213..bcd32322 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1508,9 +1508,6 @@ static void intel_reset_dpio(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> - if (!IS_VALLEYVIEW(dev))
> - return;
> -
> if (IS_CHERRYVIEW(dev)) {
> enum dpio_phy phy;
> u32 val;
> @@ -1532,26 +1529,6 @@ static void intel_reset_dpio(struct drm_device *dev)
> I915_WRITE(DISPLAY_PHY_CONTROL,
> PHY_COM_LANE_RESET_DEASSERT(phy, val));
> }
> -
> - } else {
> - /*
> - * If DPIO has already been reset, e.g. by BIOS, just skip all
> - * this.
> - */
> - if (I915_READ(DPIO_CTL) & DPIO_CMNRST)
> - return;
> -
> - /*
> - * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
> - * Need to assert and de-assert PHY SB reset by gating the
> - * common lane power, then un-gating it.
> - * Simply ungating isn't enough to reset the PHY enough to get
> - * ports and lanes running.
> - */
> - __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
> - false);
> - __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
> - true);
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 5740be0..e565906 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -970,8 +970,7 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
> void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
> void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
> void ilk_wm_get_hw_state(struct drm_device *dev);
> -void __vlv_set_power_well(struct drm_i915_private *dev_priv,
> - enum punit_power_well power_well_id, bool enable);
> +
>
> /* intel_sdvo.c */
> bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e9a8565..d8e20d3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5962,9 +5962,10 @@ static bool i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv,
> return true;
> }
>
> -void __vlv_set_power_well(struct drm_i915_private *dev_priv,
> - enum punit_power_well power_well_id, bool enable)
> +static void vlv_set_power_well(struct drm_i915_private *dev_priv,
> + struct i915_power_well *power_well, bool enable)
> {
> + enum punit_power_well power_well_id = power_well->data;
> u32 mask;
> u32 state;
> u32 ctrl;
> @@ -5997,14 +5998,6 @@ out:
> mutex_unlock(&dev_priv->rps.hw_lock);
> }
>
> -static void vlv_set_power_well(struct drm_i915_private *dev_priv,
> - struct i915_power_well *power_well, bool enable)
> -{
> - enum punit_power_well power_well_id = power_well->data;
> -
> - __vlv_set_power_well(dev_priv, power_well_id, enable);
> -}
> -
> static void vlv_power_well_sync_hw(struct drm_i915_private *dev_priv,
> struct i915_power_well *power_well)
> {
> @@ -6435,6 +6428,21 @@ static struct i915_power_well vlv_power_wells[] = {
> },
> };
>
> +static struct i915_power_well *lookup_power_well(struct drm_i915_private *dev_priv,
> + enum punit_power_well power_well_id)
> +{
> + struct i915_power_domains *power_domains = &dev_priv->power_domains;
> + struct i915_power_well *power_well;
> + int i;
> +
> + for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) {
> + if (power_well->data == power_well_id)
> + return power_well;
> + }
> +
> + return NULL;
> +}
> +
> #define set_power_wells(power_domains, __power_wells) ({ \
> (power_domains)->power_wells = (__power_wells); \
> (power_domains)->power_well_count = ARRAY_SIZE(__power_wells); \
> @@ -6482,11 +6490,50 @@ static void intel_power_domains_resume(struct drm_i915_private *dev_priv)
> mutex_unlock(&power_domains->lock);
> }
>
> +static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv)
> +{
> + struct i915_power_well *cmn =
> + lookup_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC);
> + struct i915_power_well *disp2d =
> + lookup_power_well(dev_priv, PUNIT_POWER_WELL_DISP2D);
> +
> + /* nothing to do if common lane is already off */
> + if (!cmn->ops->is_enabled(dev_priv, cmn))
> + return;
> +
> + /* If the display might be already active skip this */
> + if (disp2d->ops->is_enabled(dev_priv, disp2d) &&
> + I915_READ(DPIO_CTL) & DPIO_CMNRST)
> + return;
> +
> + DRM_DEBUG_KMS("toggling display PHY side reset\n");
> +
> + /* cmnlane needs DPLL registers */
> + disp2d->ops->enable(dev_priv, disp2d);
> +
> + /*
> + * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
> + * Need to assert and de-assert PHY SB reset by gating the
> + * common lane power, then un-gating it.
> + * Simply ungating isn't enough to reset the PHY enough to get
> + * ports and lanes running.
> + */
> + cmn->ops->disable(dev_priv, cmn);
> +}
> +
> void intel_power_domains_init_hw(struct drm_i915_private *dev_priv)
> {
> + struct drm_device *dev = dev_priv->dev;
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
>
> power_domains->initializing = true;
> +
> + if (IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)) {
> + mutex_lock(&power_domains->lock);
> + vlv_cmnlane_wa(dev_priv);
> + mutex_unlock(&power_domains->lock);
> + }
> +
> /* For now, we need the power well to be always enabled. */
> intel_display_set_init_power(dev_priv, true);
> intel_power_domains_resume(dev_priv);
I kind of preferred the low level function that just took a well ID
directly since the idea of looking something we already know seems
silly (and it should probably be a separate patch anyway). Also I'm not
sure I would describe this as a W/A; the phy needs to get reset
somehow...
But with or without those changes, we need this.
Reviewed-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
--
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
