On Wed, 2014-04-09 at 13:28 +0300, ville.syrjala@linux.intel.com wrote:
> From: Chon Ming Lee <chon.ming.lee@intel.com>
> 
> During cold boot, the display controller needs to deassert the common
> lane reset.  Only do it once during intel_init_dpio for both PHYx2 and
> PHYx1.
> 
> Besides, assert the common lane reset when disable pll.  This still
> to be determined whether need to do it by driver.
> 
> Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com>
> [vsyrjala: Don't disable DPIO PLL when using DSI]
> [vsyrjala: Don't call vlv_disable_pll() by accident on CHV]
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  8 +++++
>  drivers/gpu/drm/i915/intel_display.c | 66 ++++++++++++++++++++++++++++--------
>  2 files changed, 59 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8aea092..8fcf4ea 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1391,6 +1391,14 @@ enum punit_power_well {
>  /* Additional CHV pll/phy registers */
>  #define DPIO_PHY_STATUS			(VLV_DISPLAY_BASE + 0x6240)
>  #define   DPLL_PORTD_READY_MASK		(0xf)
> +#define DISPLAY_PHY_CONTROL (VLV_DISPLAY_BASE + 0x60100)
> +#define   PHY_COM_LANE_RESET_DEASSERT(phy, val) \
> +				((phy == DPIO_PHY0) ? (val | 1) : (val | 2))
> +#define   PHY_COM_LANE_RESET_ASSERT(phy, val) \
> +				((phy == DPIO_PHY0) ? (val & ~1) : (val & ~2))
> +#define DISPLAY_PHY_STATUS (VLV_DISPLAY_BASE + 0x60104)
> +#define   PHY_POWERGOOD(phy)	((phy == DPIO_PHY0) ? (1<<31) : (1<<30))
> +
>  /*
>   * The i830 generation, in LVDS mode, defines P1 as the bit number set within
>   * this field (only one bit may be set).
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 153f244..e33667d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1395,17 +1395,36 @@ static void intel_reset_dpio(struct drm_device *dev)
>  		   DPLL_REFA_CLK_ENABLE_VLV |
>  		   DPLL_INTEGRATED_CRI_CLK_VLV);
>  
> -	/*
> -	 * From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx -
> -	 *  6.	De-assert cmn_reset/side_reset. Same as VLV X0.
> -	 *   a.	GUnit 0x2110 bit[0] set to 1 (def 0)
> -	 *   b.	The other bits such as sfr settings / modesel may all be set
> -	 *      to 0.

This is VLV specific, so ok to be moved,

> -	 *
> -	 * This should only be done on init and resume from S3 with both
> -	 * PLLs disabled, or we risk losing DPIO and PLL synchronization.
> -	 */

but this is also true for CHV, so should stay.

> -	I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_CMNRST);
> +	if (IS_CHERRYVIEW(dev)) {
> +		enum dpio_phy phy;
> +		u32 val;
> +
> +		for (phy = DPIO_PHY0; phy < I915_NUM_PHYS_VLV; phy++) {
> +			/* Poll for phypwrgood signal */
> +			if (wait_for(I915_READ(DISPLAY_PHY_STATUS) &
> +						PHY_POWERGOOD(phy), 1))
> +				DRM_ERROR("Display PHY %d is not power up\n", phy);
> +
> +			/* Deassert common lane reset for PHY*/
> +			val = I915_READ(DISPLAY_PHY_CONTROL);
> +			I915_WRITE(DISPLAY_PHY_CONTROL,
> +				PHY_COM_LANE_RESET_DEASSERT(phy, val));

Would be clearer not to hide the 'or' in the macro and let
PHY_COM_LANE_RESET_DEASSERT be just the flag itself and do here
I915_WRITE(DISPLAY_PHY_CONTROL, val | PHY_COM_LANE_RESET_DEASSERT(phy));

The above issues are minor, so even without fixing them this patch is
Reviewed-by: Imre Deak <imre.deak@intel.com>

> +		}
> +
> +	} else {
> +		/*
> +		 * From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx -
> +		 *  6.	De-assert cmn_reset/side_reset. Same as VLV X0.
> +		 *   a.	GUnit 0x2110 bit[0] set to 1 (def 0)
> +		 *   b.	The other bits such as sfr settings / modesel may all
> +		 *	be set to 0.
> +		 *
> +		 * This should only be done on init and resume from S3 with
> +		 * both PLLs disabled, or we risk losing DPIO and PLL
> +		 * synchronization.
> +		 */
> +		I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_CMNRST);
> +	}
>  }
>  
>  static void vlv_enable_pll(struct intel_crtc *crtc)
> @@ -1529,6 +1548,19 @@ static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
>  		val = DPLL_INTEGRATED_CRI_CLK_VLV | DPLL_REFA_CLK_ENABLE_VLV;
>  	I915_WRITE(DPLL(pipe), val);
>  	POSTING_READ(DPLL(pipe));
> +
> +}
> +
> +static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
> +{
> +	int dpll = DPLL(pipe);
> +	u32 val;
> +
> +	/* Set PLL en = 0 */
> +	val = I915_READ(dpll);
> +	val &= ~DPLL_VCO_ENABLE;
> +	I915_WRITE(dpll, val);
> +
>  }
>  
>  void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
> @@ -4511,10 +4543,14 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>  		if (encoder->post_disable)
>  			encoder->post_disable(encoder);
>  
> -	if (IS_VALLEYVIEW(dev) && !intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI))
> -		vlv_disable_pll(dev_priv, pipe);
> -	else if (!IS_VALLEYVIEW(dev))
> -		i9xx_disable_pll(dev_priv, pipe);
> +	if (!intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI)) {
> +		if (IS_CHERRYVIEW(dev))
> +			chv_disable_pll(dev_priv, pipe);
> +		else if (IS_VALLEYVIEW(dev))
> +			vlv_disable_pll(dev_priv, pipe);
> +		else
> +			i9xx_disable_pll(dev_priv, pipe);
> +	}
>  
>  	intel_crtc->active = false;
>  	intel_update_watermarks(crtc);