[Intel-gfx] [PATCH] drm/i915/icl: pass cfgcr* register around instead of pll_id

Ville Syrjälä ville.syrjala at linux.intel.com
Mon Mar 18 13:31:52 UTC 2019


On Fri, Mar 15, 2019 at 05:45:26PM -0700, Lucas De Marchi wrote:
> The caller already knows what platform that is and what register should
> be used. Instead of keep adding if/else chains on a leaf functions,
> let the caller pass the register.
> 
> We read cfgcr0 twice for CNL, but we were already doing that anyway.
> 
> icl_calc_dp_combo_pll_link() is only used for ICL, but let's keep
> consistency with cnl_calc_wrpll_link().
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
>  drivers/gpu/drm/i915/icl_dsi.c        |  4 +++-
>  drivers/gpu/drm/i915/intel_ddi.c      | 25 ++++++++++++++-----------
>  drivers/gpu/drm/i915/intel_dpll_mgr.c |  6 +++---
>  drivers/gpu/drm/i915/intel_dpll_mgr.h |  2 +-
>  drivers/gpu/drm/i915/intel_drv.h      |  2 +-
>  5 files changed, 22 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
> index beb30d9a855c..28f5da697693 100644
> --- a/drivers/gpu/drm/i915/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/icl_dsi.c
> @@ -1183,7 +1183,9 @@ static void gen11_dsi_get_config(struct intel_encoder *encoder,
>  
>  	/* FIXME: adapt icl_ddi_clock_get() for DSI and use that? */
>  	pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
> -	pipe_config->port_clock = cnl_calc_wrpll_link(dev_priv, pll_id);
> +	pipe_config->port_clock = cnl_calc_wrpll_link(dev_priv,
> +						      ICL_DPLL_CFGCR0(pll_id),
> +						      ICL_DPLL_CFGCR1(pll_id));
>  	pipe_config->base.adjusted_mode.crtc_clock = intel_dsi->pclk;
>  	pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
>  }
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 69aa0d148795..24675ef8b262 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1304,18 +1304,13 @@ static int skl_calc_wrpll_link(struct drm_i915_private *dev_priv,
>  }
>  
>  int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
> -			enum intel_dpll_id pll_id)
> +			i915_reg_t cfgcr0_reg, i915_reg_t cfgcr1_reg)
>  {
>  	u32 cfgcr0, cfgcr1;
>  	u32 p0, p1, p2, dco_freq, ref_clock;
>  
> -	if (INTEL_GEN(dev_priv) >= 11) {
> -		cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(pll_id));
> -		cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(pll_id));
> -	} else {
> -		cfgcr0 = I915_READ(CNL_DPLL_CFGCR0(pll_id));
> -		cfgcr1 = I915_READ(CNL_DPLL_CFGCR1(pll_id));
> -	}
> +	cfgcr0 = I915_READ(cfgcr0_reg);
> +	cfgcr1 = I915_READ(cfgcr1_reg);

Don't we alredy have the dpll state read out at this point?
I thought I changed at least some of these to not read it out
directly.

>  
>  	p0 = cfgcr1 & DPLL_CFGCR1_PDIV_MASK;
>  	p2 = cfgcr1 & DPLL_CFGCR1_KDIV_MASK;
> @@ -1471,17 +1466,23 @@ static void icl_ddi_clock_get(struct intel_encoder *encoder,
>  			      struct intel_crtc_state *pipe_config)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	i915_reg_t cfgcr0_reg, cfgcr1_reg;
>  	enum port port = encoder->port;
>  	int link_clock = 0;
>  	u32 pll_id;
>  
>  	pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
> +	cfgcr0_reg = ICL_DPLL_CFGCR0(pll_id);
> +	cfgcr1_reg = ICL_DPLL_CFGCR1(pll_id);
> +
>  	if (intel_port_is_combophy(dev_priv, port)) {
>  		if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
> -			link_clock = cnl_calc_wrpll_link(dev_priv, pll_id);
> +			link_clock = cnl_calc_wrpll_link(dev_priv, cfgcr0_reg,
> +							 cfgcr1_reg);
>  		else
>  			link_clock = icl_calc_dp_combo_pll_link(dev_priv,
> -								pll_id);
> +								cfgcr0_reg,
> +								cfgcr1_reg);
>  	} else {
>  		if (pll_id == DPLL_ID_ICL_TBTPLL)
>  			link_clock = icl_calc_tbt_pll_link(dev_priv, port);
> @@ -1506,7 +1507,9 @@ static void cnl_ddi_clock_get(struct intel_encoder *encoder,
>  	cfgcr0 = I915_READ(CNL_DPLL_CFGCR0(pll_id));
>  
>  	if (cfgcr0 & DPLL_CFGCR0_HDMI_MODE) {
> -		link_clock = cnl_calc_wrpll_link(dev_priv, pll_id);
> +		link_clock = cnl_calc_wrpll_link(dev_priv,
> +						 CNL_DPLL_CFGCR0(pll_id),
> +						 CNL_DPLL_CFGCR1(pll_id));
>  	} else {
>  		link_clock = cfgcr0 & DPLL_CFGCR0_LINK_RATE_MASK;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index ca975213da2a..ed4024f4394b 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -2569,7 +2569,7 @@ static bool icl_calc_dpll_state(struct intel_crtc_state *crtc_state,
>  }
>  
>  int icl_calc_dp_combo_pll_link(struct drm_i915_private *dev_priv,
> -			       u32 pll_id)
> +			       i915_reg_t cfgcr0_reg, i915_reg_t cfgcr1_reg)
>  {
>  	u32 cfgcr0, cfgcr1;
>  	u32 pdiv, kdiv, qdiv_mode, qdiv_ratio, dco_integer, dco_fraction;
> @@ -2577,8 +2577,8 @@ int icl_calc_dp_combo_pll_link(struct drm_i915_private *dev_priv,
>  	int index, n_entries, link_clock;
>  
>  	/* Read back values from DPLL CFGCR registers */
> -	cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(pll_id));
> -	cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(pll_id));
> +	cfgcr0 = I915_READ(cfgcr0_reg);
> +	cfgcr1 = I915_READ(cfgcr1_reg);
>  
>  	dco_integer = cfgcr0 & DPLL_CFGCR0_DCO_INTEGER_MASK;
>  	dco_fraction = (cfgcr0 & DPLL_CFGCR0_DCO_FRACTION_MASK) >>
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> index 40e8391a92f2..0aa504e9bfbf 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> @@ -342,7 +342,7 @@ void intel_shared_dpll_init(struct drm_device *dev);
>  void intel_dpll_dump_hw_state(struct drm_i915_private *dev_priv,
>  			      struct intel_dpll_hw_state *hw_state);
>  int icl_calc_dp_combo_pll_link(struct drm_i915_private *dev_priv,
> -			       u32 pll_id);
> +			       i915_reg_t cfgcr0_reg, i915_reg_t cfgcr1_reg);
>  int cnl_hdmi_pll_ref_clock(struct drm_i915_private *dev_priv);
>  enum intel_dpll_id icl_tc_port_to_pll_id(enum tc_port tc_port);
>  bool intel_dpll_is_combophy(enum intel_dpll_id id);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d9f188ef21f4..9d0bf1914773 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1659,7 +1659,7 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
>  				     bool enable);
>  void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder);
>  int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
> -			enum intel_dpll_id pll_id);
> +			i915_reg_t cgfcr0_reg, i915_reg_t cgfcr1_reg);
>  
>  unsigned int intel_fb_align_height(const struct drm_framebuffer *fb,
>  				   int color_plane, unsigned int height);
> -- 
> 2.20.1

-- 
Ville Syrjälä
Intel


More information about the Intel-gfx mailing list