Re: [PATCH 3/3] drm/i915/bxt: Force reprogramming a PHY with invalid HW state

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Apr 20, 2016 at 08:46:06PM +0300, Imre Deak wrote:
> It's possible that BIOS enables PHY0, but it programmes only the first
> channel on it. Since we program the PHYs only during driver loading this
> is an incorrect configuration from the driver's point of view, since we
> may use both channels eventually. Detect this scenario and force
> reprogramming the PHY in this case.
> 
> The actual scenario for me was that the lane optimization for the second
> channel in PHY0 was not setup by BIOS and so a state verification
> warning was triggered. Everything else was setup properly.

One thing that bothers me with this reprogramming scheme is that we
don't fully power down the PHY before reprogramming. If CHV is any
indication that could leave the PHY in some wonky intermediate state
where it doesn't fully power down when unused. That problem was caught
on CHV by assert_chv_phy_status() and/or assert_chv_phy_powergate().

> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e68914a..32ac456 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1767,6 +1767,9 @@ static void broxton_phy_wait_grc_done(struct drm_i915_private *dev_priv,
>  		DRM_ERROR("timeout waiting for PHY1 GRC\n");
>  }
>  
> +static bool broxton_phy_verify_state(struct drm_i915_private *dev_priv,
> +				     enum dpio_phy phy);
> +
>  static void broxton_phy_init(struct drm_i915_private *dev_priv,
>  			     enum dpio_phy phy)
>  {
> @@ -1774,17 +1777,24 @@ static void broxton_phy_init(struct drm_i915_private *dev_priv,
>  	u32 ports, val;
>  
>  	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_PHY0)
>  			dev_priv->bxt_phy_grc = broxton_get_grc(dev_priv, phy);
>  
> -		return;
> +
> +		if (broxton_phy_verify_state(dev_priv, phy)) {
> +			DRM_DEBUG_DRIVER("DDI PHY %d already enabled, "
> +					 "won't reprogram it\n", phy);
> +
> +			return;
> +		}
> +
> +		DRM_DEBUG_DRIVER("DDI PHY %d enabled with invalid state, "
> +				 "force reprogramming it\n", phy);
> +	} else {
> +		DRM_DEBUG_DRIVER("DDI PHY %d not enabled, enabling it\n", phy);
>  	}
>  
> -	DRM_DEBUG_DRIVER("DDI PHY %d not enabled, enabling it\n", phy);
> -
>  	val = I915_READ(BXT_P_CR_GT_DISP_PWRON);
>  	val |= GT_DISPLAY_POWER_ON(phy);
>  	I915_WRITE(BXT_P_CR_GT_DISP_PWRON, val);
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx