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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 15 Mar 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@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@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.

Other than that,

Reviewed-by: Jani Nikula <jani.nikula@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx