[Intel-gfx] [PATCH] drm/i915/icl: pass cfgcr* register around instead of pll_id
Lucas De Marchi
lucas.demarchi at intel.com
Mon Mar 18 18:40:34 UTC 2019
On Mon, Mar 18, 2019 at 03:31:52PM +0200, Ville Syrjälä wrote:
>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?
nops.
>I thought I changed at least some of these to not read it out
>directly.
What we have is that for ICL we only read it on the
cnl_calc_wrpll_link(). For CNL we read both on cnl_calc_wrpll_link() and
on the ddi_clock_get() function - at least cfgcr0, because we need it
to decide which function to call.
If you prefer the read to be on the ddi_clock_get() what I could do is
to pass the register values.
Lucas De Marchi
>
>>
>> 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