[Intel-gfx] [PATCH 3/3] drm/i915/icl: use previous pll hw readout

[Lucas De Marchi]

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 at intel.com>
>>>---
>>> 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?

humn.. I should have checked the other patch again. I thought about
always passing the refclk via argument, but that will make it ugly. I
will add dev_priv back.

Lucas De Marchi

>
>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