Re: [PATCH v3 1/9] drm/i915/tgl: Add initial dkl pll support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2019-09-24 at 17:20 +0300, Imre Deak wrote:
> On Mon, Sep 23, 2019 at 12:55:05PM -0700, José Roberto de Souza
> wrote:
> > From: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> > 
> > The disable function can be the same as for MG phy since the same
> > registers are used. The others are different as registers changed,
> > also adding a empty dkl_pll_write() to be implemented later.
> > 
> > v2:
> > Setting the right HIP_INDEX_REG bits (José)
> > 
> > Acked-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 115
> > +++++++++++++++++-
> >  1 file changed, 114 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index 84e734d44828..46dde614bfb5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -3086,6 +3086,76 @@ static bool mg_pll_get_hw_state(struct
> > drm_i915_private *dev_priv,
> >  	return ret;
> >  }
> >  
> > +static bool dkl_pll_get_hw_state(struct drm_i915_private
> > *dev_priv,
> > +				 struct intel_shared_dpll *pll,
> > +				 struct intel_dpll_hw_state *hw_state)
> > +{
> > +	const enum intel_dpll_id id = pll->info->id;
> > +	enum tc_port tc_port = icl_pll_id_to_tc_port(id);
> > +	intel_wakeref_t wakeref;
> > +	bool ret = false;
> > +	u32 val;
> > +
> > +	wakeref = intel_display_power_get_if_enabled(dev_priv,
> > +						     POWER_DOMAIN_DISPL
> > AY_CORE);
> > +	if (!wakeref)
> > +		return false;
> > +
> > +	val = I915_READ(MG_PLL_ENABLE(tc_port));
> > +	if (!(val & PLL_ENABLE))
> > +		goto out;
> > +
> > +	/*
> > +	 * All registers read here have the same HIP_INDEX_REG even
> > though
> > +	 * they are on different building blocks
> > +	 */
> > +	I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port,
> > 0x2));
> > +
> > +	hw_state->mg_refclkin_ctl =
> > I915_READ(DKL_REFCLKIN_CTL(tc_port));
> > +	hw_state->mg_refclkin_ctl &= MG_REFCLKIN_CTL_OD_2_MUX_MASK;
> > +
> > +	hw_state->mg_clktop2_hsclkctl =
> > +		I915_READ(DKL_CLKTOP2_HSCLKCTL(tc_port));
> > +	hw_state->mg_clktop2_hsclkctl &=
> > +		MG_CLKTOP2_HSCLKCTL_TLINEDRV_CLKSEL_MASK |
> > +		MG_CLKTOP2_HSCLKCTL_CORE_INPUTSEL_MASK |
> > +		MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK |
> > +		MG_CLKTOP2_HSCLKCTL_DSDIV_RATIO_MASK;
> > +
> > +	hw_state->mg_clktop2_coreclkctl1 =
> > +		I915_READ(DKL_CLKTOP2_CORECLKCTL1(tc_port));
> > +	hw_state->mg_clktop2_coreclkctl1 &=
> > +		MG_CLKTOP2_CORECLKCTL1_A_DIVRATIO_MASK;
> > +
> > +	hw_state->mg_pll_div0 = I915_READ(DKL_PLL_DIV0(tc_port));
> > +	hw_state->mg_pll_div0 &= (DKL_PLL_DIV0_INTEG_COEFF_MASK |
> > +				  DKL_PLL_DIV0_PROP_COEFF_MASK |
> > +				  DKL_PLL_DIV0_FBPREDIV_MASK |
> > +				  DKL_PLL_DIV0_FBDIV_INT_MASK);
> > +
> > +	hw_state->mg_pll_div1 = I915_READ(DKL_PLL_DIV1(tc_port));
> > +	hw_state->mg_pll_div1 &= (DKL_PLL_DIV1_IREF_TRIM_MASK |
> > +				  DKL_PLL_DIV1_TDC_TARGET_CNT_MASK);
> > +
> > +	hw_state->mg_pll_ssc = I915_READ(DKL_PLL_SSC(tc_port));
> > +	hw_state->mg_pll_ssc &= (DKL_PLL_SSC_IREF_NDIV_RATIO_MASK |
> > +				 DKL_PLL_SSC_STEP_LEN_MASK |
> > +				 DKL_PLL_SSC_STEP_NUM_MASK |
> > +				 DKL_PLL_SSC_EN);
> > +
> > +	hw_state->mg_pll_bias = I915_READ(DKL_PLL_BIAS(tc_port));
> > +	hw_state->mg_pll_bias &= (DKL_PLL_BIAS_FRAC_EN_H |
> > +				  DKL_PLL_BIAS_FBDIV_FRAC_MASK);
> > +
> > +	hw_state->mg_pll_tdc_coldst_bias =
> > +		I915_READ(DKL_PLL_TDC_COLDST_BIAS(tc_port));
> 
> For the above masking the computed fields is missing.

Done, thanks for catching this

> 
> > +
> > +	ret = true;
> > +out:
> > +	intel_display_power_put(dev_priv, POWER_DOMAIN_DISPLAY_CORE,
> > wakeref);
> > +	return ret;
> > +}
> > +
> >  static bool icl_pll_get_hw_state(struct drm_i915_private
> > *dev_priv,
> >  				 struct intel_shared_dpll *pll,
> >  				 struct intel_dpll_hw_state *hw_state,
> > @@ -3220,6 +3290,12 @@ static void icl_mg_pll_write(struct
> > drm_i915_private *dev_priv,
> >  	POSTING_READ(MG_PLL_TDC_COLDST_BIAS(tc_port));
> >  }
> >  
> > +static void dkl_pll_write(struct drm_i915_private *dev_priv,
> > +			  struct intel_shared_dpll *pll)
> > +{
> > +	/* TODO */
> > +}
> > +
> >  static void icl_pll_power_enable(struct drm_i915_private
> > *dev_priv,
> >  				 struct intel_shared_dpll *pll,
> >  				 i915_reg_t enable_reg)
> > @@ -3325,6 +3401,32 @@ static void mg_pll_enable(struct
> > drm_i915_private *dev_priv,
> >  	/* DVFS post sequence would be here. See the comment above. */
> >  }
> >  
> > +static void dkl_pll_enable(struct drm_i915_private *dev_priv,
> > +			   struct intel_shared_dpll *pll)
> > +{
> > +	/*
> > +	 * From spec: MG register instances are being used for TypeC in
> > general.
> > +	 * The same MG register instances should be programmed for
> > Dekel PLLs
> > +	 * as well.
> 
> Imo BSpec Index/49204 should state it explicitly that the MG and DKL
> enable/disable sequence are the same, but let's assume that.
> 
> > +	 */
> > +	i915_reg_t enable_reg =
> > +		MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id));
> > +
> > +	icl_pll_power_enable(dev_priv, pll, enable_reg);
> > +
> > +	dkl_pll_write(dev_priv, pll);
> 
> Nit: reusing mg_pll_enable() instead with a platform check here would
> make things more symmetric.

Done

> 
> With the above fix:
> Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>

Thanks

> 
> > +
> > +	/*
> > +	 * DVFS pre sequence would be here, but in our driver the cdclk
> > code
> > +	 * paths should already be setting the appropriate voltage,
> > hence we do
> > +	 * nothing here.
> > +	 */
> > +
> > +	icl_pll_enable(dev_priv, pll, enable_reg);
> > +
> > +	/* DVFS post sequence would be here. See the comment above. */
> > +}
> > +
> >  static void icl_pll_disable(struct drm_i915_private *dev_priv,
> >  			    struct intel_shared_dpll *pll,
> >  			    i915_reg_t enable_reg)
> > @@ -3467,11 +3569,22 @@ static const struct intel_dpll_mgr
> > ehl_pll_mgr = {
> >  	.dump_hw_state = icl_dump_hw_state,
> >  };
> >  
> > +static const struct intel_shared_dpll_funcs dkl_pll_funcs = {
> > +	.enable = dkl_pll_enable,
> > +	.disable = mg_pll_disable,
> > +	.get_hw_state = dkl_pll_get_hw_state,
> > +};
> > +
> >  static const struct dpll_info tgl_plls[] = {
> >  	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0,  0 },
> >  	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1,  0 },
> >  	{ "TBT PLL",  &tbt_pll_funcs, DPLL_ID_ICL_TBTPLL, 0 },
> > -	/* TODO: Add typeC plls */
> > +	{ "TC PLL 1", &dkl_pll_funcs, DPLL_ID_ICL_MGPLL1, 0 },
> > +	{ "TC PLL 2", &dkl_pll_funcs, DPLL_ID_ICL_MGPLL2, 0 },
> > +	{ "TC PLL 3", &dkl_pll_funcs, DPLL_ID_ICL_MGPLL3, 0 },
> > +	{ "TC PLL 4", &dkl_pll_funcs, DPLL_ID_ICL_MGPLL4, 0 },
> > +	{ "TC PLL 5", &dkl_pll_funcs, DPLL_ID_TGL_MGPLL5, 0 },
> > +	{ "TC PLL 6", &dkl_pll_funcs, DPLL_ID_TGL_MGPLL6, 0 },
> >  	{ },
> >  };
> >  
> > -- 
> > 2.23.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux