On Fri, Mar 22, 2019 at 12:50:07PM -0700, Lucas De Marchi wrote:
> On Fri, Mar 22, 2019 at 03:09:59PM +0200, Ville Syrjälä wrote:
> >On Thu, Mar 21, 2019 at 03:02:57PM -0700, Lucas De Marchi wrote:
> >> By the time icl_ddi_clock_get() is called we've just got the hw state
> >> from the pll registers. We don't need to read them again: we can rather
> >> reuse what was cached in the dpll_hw_state.
> >>
> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> >> ---
> >> drivers/gpu/drm/i915/intel_ddi.c | 39 +++++++++++++++-----------------
> >> 1 file changed, 18 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >> index fe52af9fa4aa..0ea5a97dfe9d 100644
> >> --- a/drivers/gpu/drm/i915/intel_ddi.c
> >> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >> @@ -1372,26 +1372,20 @@ static int icl_calc_tbt_pll_link(struct drm_i915_private *dev_priv,
> >> }
> >> }
> >>
> >> -static int icl_calc_mg_pll_link(struct drm_i915_private *dev_priv,
> >> - enum port port)
> >> +
> >> +static int icl_calc_mg_pll_link(struct intel_dpll_hw_state *state, u32 refclk)
> >
> >Inconsistent with the cnl variant.
>
>
> see below
>
> >
> >> {
> >> - enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> >> - u32 mg_pll_div0, mg_clktop_hsclkctl;
> >> - u32 m1, m2_int, m2_frac, div1, div2, refclk;
> >> + u32 m1, m2_int, m2_frac, div1, div2;
> >> u64 tmp;
> >>
> >> - refclk = dev_priv->cdclk.hw.ref;
>
> because this one needs the refclk. If i don't add refclk to the
> arguments I need to leave dev_priv there and it will still not be
> consistent. What do you suggest?
Didn't cnl pass in the dev_priv also for this exact same reason?
>
> Lucas De Marchi
>
> >> -
> >> - mg_pll_div0 = I915_READ(MG_PLL_DIV0(tc_port));
> >> - mg_clktop_hsclkctl = I915_READ(MG_CLKTOP2_HSCLKCTL(tc_port));
> >> -
> >> - m1 = I915_READ(MG_PLL_DIV1(tc_port)) & MG_PLL_DIV1_FBPREDIV_MASK;
> >> - m2_int = mg_pll_div0 & MG_PLL_DIV0_FBDIV_INT_MASK;
> >> - m2_frac = (mg_pll_div0 & MG_PLL_DIV0_FRACNEN_H) ?
> >> - (mg_pll_div0 & MG_PLL_DIV0_FBDIV_FRAC_MASK) >>
> >> + m1 = state->mg_pll_div1 & MG_PLL_DIV1_FBPREDIV_MASK;
> >> + m2_int = state->mg_pll_div0 & MG_PLL_DIV0_FBDIV_INT_MASK;
> >> + m2_frac = (state->mg_pll_div0 & MG_PLL_DIV0_FRACNEN_H) ?
> >> + (state->mg_pll_div0 & MG_PLL_DIV0_FBDIV_FRAC_MASK) >>
> >> MG_PLL_DIV0_FBDIV_FRAC_SHIFT : 0;
> >>
> >> - switch (mg_clktop_hsclkctl & MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK) {
> >> + switch (state->mg_clktop2_hsclkctl &
> >> + MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK) {
> >> case MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_2:
> >> div1 = 2;
> >> break;
> >> @@ -1405,12 +1399,14 @@ static int icl_calc_mg_pll_link(struct drm_i915_private *dev_priv,
> >> div1 = 7;
> >> break;
> >> default:
> >> - MISSING_CASE(mg_clktop_hsclkctl);
> >> + MISSING_CASE(state->mg_clktop2_hsclkctl);
> >> return 0;
> >> }
> >>
> >> - div2 = (mg_clktop_hsclkctl & MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK) >>
> >> + div2 = (state->mg_clktop2_hsclkctl &
> >> + MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK) >>
> >> MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_SHIFT;
> >> +
> >> /* div2 value of 0 is same as 1 means no div */
> >> if (div2 == 0)
> >> div2 = 1;
> >> @@ -1457,9 +1453,6 @@ static void icl_ddi_clock_get(struct intel_encoder *encoder,
> >> struct intel_dpll_hw_state *state;
> >> 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);
> >>
> >> /* For DDI ports we always use a shared PLL. */
> >> if (WARN_ON(!pipe_config->shared_dpll))
> >> @@ -1470,10 +1463,14 @@ static void icl_ddi_clock_get(struct intel_encoder *encoder,
> >> if (intel_port_is_combophy(dev_priv, port)) {
> >> link_clock = cnl_calc_wrpll_link(dev_priv, state);
> >> } else {
> >> + enum intel_dpll_id pll_id = intel_get_shared_dpll_id(dev_priv,
> >> + pipe_config->shared_dpll);
> >> +
> >> if (pll_id == DPLL_ID_ICL_TBTPLL)
> >> link_clock = icl_calc_tbt_pll_link(dev_priv, port);
> >> else
> >> - link_clock = icl_calc_mg_pll_link(dev_priv, port);
> >> + link_clock = icl_calc_mg_pll_link(
> >> + state, dev_priv->cdclk.hw.ref);
> >> }
> >>
> >> end:
> >> --
> >> 2.20.1
> >
> >--
> >Ville Syrjälä
> >Intel
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
