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@xxxxxxxxx>
> ---
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
