On Mon, Mar 18, 2019 at 08:53:23PM +0200, Ville Syrjälä wrote:
On Mon, Mar 18, 2019 at 11:40:34AM -0700, Lucas De Marchi wrote: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@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? nops.We must have it since bxt is already using it. Either that or bxt is broken.
humn... I need to double check it then. bxt is the only one not reading the registers and instead using what's saved on dpll_hw_state - in fact hsw, skl, cnl, icl - all read wrpll/spll/cfgcr* registers that they need :-o Lucas De Marchi
-- Ville Syrjälä Intel
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
