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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 30, 2016 at 04:31:04PM +0300, Jani Nikula wrote:
> On Tue, 15 Mar 2016, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxxx>
> > ---
> >  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@xxxxxxxxx>
> 
> 
> > -	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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux