[Intel-gfx] [PATCH 02/16] drm/i915: Make {vlv, chv}_{disable, update}_pll() more similar

[Ville Syrjälä]

On Wed, Mar 30, 2016 at 04:31:04PM +0300, Jani Nikula wrote:
> On Tue, 15 Mar 2016, ville.syrjala at linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >
> > The VLV and CHV DPLL disable and update are almost identical in
> > how the DPLL/DPLL_MD registers need to be set up. But the code
> > looks more different than it really is. Try to bring them into
> > line.
> >
> > v2: s/chv_update_pll/chv_compute_dpll/
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 63 ++++++++++++++----------------------
> >  1 file changed, 25 insertions(+), 38 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 22930f05457c..414ed5007e60 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -1759,16 +1759,13 @@ static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
> >  	/* Make sure the pipe isn't still relying on us */
> >  	assert_pipe_disabled(dev_priv, pipe);
> >  
> > -	/*
> > -	 * Leave integrated clock source and reference clock enabled for pipe B.
> > -	 * The latter is needed for VGA hotplug / manual detection.
> > -	 */
> 
> So, you change this to keep the reference clock enabled for both
> pipes. Deserves a mention in the commit message. AFAICT it's the only
> functional change in the patch.

Added the following note while pushing:

"Note that we now leave the refclock always enabled for both
 DPLLs in the dual channel PHY. But that's perfectly fine since
 it's the same clock, and we anyway already do that when turning
 the disp2d power well on."

> 
> Other than that,
> 
> Reviewed-by: Jani Nikula <jani.nikula at intel.com>
> 
> 
> > -	val = DPLL_VGA_MODE_DIS;
> > -	if (pipe == PIPE_B)
> > -		val = DPLL_INTEGRATED_CRI_CLK_VLV | DPLL_REF_CLK_ENABLE_VLV;
> > +	val = DPLL_INTEGRATED_REF_CLK_VLV |
> > +		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> > +	if (pipe != PIPE_A)
> > +		val |= DPLL_INTEGRATED_CRI_CLK_VLV;
> > +
> >  	I915_WRITE(DPLL(pipe), val);
> >  	POSTING_READ(DPLL(pipe));
> > -
> >  }
> >  
> >  static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
> > @@ -1779,11 +1776,11 @@ static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
> >  	/* Make sure the pipe isn't still relying on us */
> >  	assert_pipe_disabled(dev_priv, pipe);
> >  
> > -	/* Set PLL en = 0 */
> >  	val = DPLL_SSC_REF_CLK_CHV |
> >  		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> >  	if (pipe != PIPE_A)
> >  		val |= DPLL_INTEGRATED_CRI_CLK_VLV;
> > +
> >  	I915_WRITE(DPLL(pipe), val);
> >  	POSTING_READ(DPLL(pipe));
> >  
> > @@ -7240,24 +7237,27 @@ void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n)
> >  static void vlv_compute_dpll(struct intel_crtc *crtc,
> >  			     struct intel_crtc_state *pipe_config)
> >  {
> > -	u32 dpll, dpll_md;
> > +	pipe_config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV |
> > +		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS |
> > +		DPLL_VCO_ENABLE | DPLL_EXT_BUFFER_ENABLE_VLV;
> > +	if (crtc->pipe != PIPE_A)
> > +		pipe_config->dpll_hw_state.dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
> >  
> > -	/*
> > -	 * Enable DPIO clock input. We should never disable the reference
> > -	 * clock for pipe B, since VGA hotplug / manual detection depends
> > -	 * on it.
> > -	 */
> > -	dpll = DPLL_EXT_BUFFER_ENABLE_VLV | DPLL_REF_CLK_ENABLE_VLV |
> > -		DPLL_VGA_MODE_DIS | DPLL_INTEGRATED_REF_CLK_VLV;
> > -	/* We should never disable this, set it here for state tracking */
> > -	if (crtc->pipe == PIPE_B)
> > -		dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
> > -	dpll |= DPLL_VCO_ENABLE;
> > -	pipe_config->dpll_hw_state.dpll = dpll;
> > +	pipe_config->dpll_hw_state.dpll_md =
> > +		(pipe_config->pixel_multiplier - 1) << DPLL_MD_UDI_MULTIPLIER_SHIFT;
> > +}
> > +
> > +static void chv_compute_dpll(struct intel_crtc *crtc,
> > +			     struct intel_crtc_state *pipe_config)
> > +{
> > +	pipe_config->dpll_hw_state.dpll = DPLL_SSC_REF_CLK_CHV |
> > +		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS |
> > +		DPLL_VCO_ENABLE;
> > +	if (crtc->pipe != PIPE_A)
> > +		pipe_config->dpll_hw_state.dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
> >  
> > -	dpll_md = (pipe_config->pixel_multiplier - 1)
> > -		<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
> > -	pipe_config->dpll_hw_state.dpll_md = dpll_md;
> > +	pipe_config->dpll_hw_state.dpll_md =
> > +		(pipe_config->pixel_multiplier - 1) << DPLL_MD_UDI_MULTIPLIER_SHIFT;
> >  }
> >  
> >  static void vlv_prepare_pll(struct intel_crtc *crtc,
> > @@ -7351,19 +7351,6 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
> >  	mutex_unlock(&dev_priv->sb_lock);
> >  }
> >  
> > -static void chv_compute_dpll(struct intel_crtc *crtc,
> > -			     struct intel_crtc_state *pipe_config)
> > -{
> > -	pipe_config->dpll_hw_state.dpll = DPLL_SSC_REF_CLK_CHV |
> > -		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS |
> > -		DPLL_VCO_ENABLE;
> > -	if (crtc->pipe != PIPE_A)
> > -		pipe_config->dpll_hw_state.dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
> > -
> > -	pipe_config->dpll_hw_state.dpll_md =
> > -		(pipe_config->pixel_multiplier - 1) << DPLL_MD_UDI_MULTIPLIER_SHIFT;
> > -}
> > -
> >  static void chv_prepare_pll(struct intel_crtc *crtc,
> >  			    const struct intel_crtc_state *pipe_config)
> >  {
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC