[Intel-gfx] [PATCH] drm/i915/ehl: Add support for DPLL4 (v4)

[Vivek Kasireddy]

On Wed, 17 Apr 2019 16:06:11 +0300
Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
Hi Ville,

> On Thu, Apr 11, 2019 at 04:36:00PM -0700, Vivek Kasireddy wrote:
> > This patch adds support for DPLL4 on EHL that include the
> > following restrictions:
> > 
> > - DPLL4 cannot be used with DDIA (combo port A internal eDP usage).
> >   DPLL4 can be used with other DDIs, including DDID
> >   (combo port A external usage).
> > 
> > - DPLL4 cannot be enabled when DC5 or DC6 are enabled.
> > 
> > - The DPLL4 enable, lock, power enabled, and power state are
> > connected to the MGPLL1_ENABLE register.
> > 
> > v2: (suggestions from Bob Paauwe)
> > - Rework ehl_get_dpll() function to call intel_find_shared_dpll()
> > and iterate twice: once for Combo plls and once for MG plls.
> > 
> > - Use MG pll funcs for DPLL4 instead of creating new ones and modify
> >   mg_pll_enable to include the restrictions for EHL.
> > 
> > v3: Fix compilation error
> > 
> > v4: (suggestions from Lucas and Ville)
> > - Treat DPLL4 as a combo phy PLL and not as MG PLL
> > - Disable DC states when this DPLL is being enabled
> > - Reuse icl_get_dpll instead of creating a separate one for EHL
> > 
> > Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> > Cc: José Roberto de Souza <jose.souza at intel.com>
> > Cc: Bob Paauwe <bob.j.paauwe at intel.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 35
> > ++++++++++++++++++++++++++++++++---
> > drivers/gpu/drm/i915/intel_dpll_mgr.h |  4 ++++ 2 files changed, 36
> > insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/intel_dpll_mgr.c index
> > e01c057ce50b..207af4af4978 100644 ---
> > a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++
> > b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -2825,6 +2825,12 @@
> > icl_get_dpll(struct intel_crtc_state *crtc_state, if
> > (intel_port_is_combophy(dev_priv, port)) { min = DPLL_ID_ICL_DPLL0;
> >  		max = DPLL_ID_ICL_DPLL1;
> > +
> > +		if (IS_ELKHARTLAKE(dev_priv)) {
> > +			if (encoder->type != INTEL_OUTPUT_EDP)
> > +				max = DPLL_ID_EHL_DPLL4;
> > +		}
> > +
> >  		ret = icl_calc_dpll_state(crtc_state, encoder);
> >  	} else if (intel_port_is_tc(dev_priv, port)) {
> >  		if (encoder->type == INTEL_OUTPUT_DP_MST) {
> > @@ -2964,8 +2970,14 @@ static bool combo_pll_get_hw_state(struct
> > drm_i915_private *dev_priv, struct intel_shared_dpll *pll,
> >  				   struct intel_dpll_hw_state
> > *hw_state) {
> > -	return icl_pll_get_hw_state(dev_priv, pll, hw_state,
> > -
> > CNL_DPLL_ENABLE(pll->info->id));
> > +	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > +
> > +	if (IS_ELKHARTLAKE(dev_priv) &&
> > +	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> > +		enable_reg = MG_PLL_ENABLE(0);
> > +	}
> > +
> > +	return icl_pll_get_hw_state(dev_priv, pll, hw_state,
> > enable_reg); }
> >  
> >  static bool tbt_pll_get_hw_state(struct drm_i915_private *dev_priv,
> > @@ -3076,6 +3088,14 @@ static void combo_pll_enable(struct
> > drm_i915_private *dev_priv, {
> >  	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> >  
> > +	if (IS_ELKHARTLAKE(dev_priv) &&
> > +	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> > +		enable_reg = MG_PLL_ENABLE(0);
> > +
> > +		/* Need to disable DC states when this DPLL is
> > enabled. */
> > +		bxt_disable_dc9(dev_priv);  
> 
> You can't simply call that from random places. It needs to be handled
> by the power domain stuff.
The only other places in the driver, the functions bxt_disable/enable_dc9
are called are intel_runtime_suspend/resume and
i915_drm_suspend_late/resume_early. Are you suggesting that I call one
of these functions instead? Or, do you simply want me to pair
bxt_*able_dc9 with intel_power_domains_suspend/resume and/or other
functions similar to what the above mentioned functions do?

Thanks,
Vivek

> 
> > +	}
> > +
> >  	icl_pll_power_enable(dev_priv, pll, enable_reg);
> >  
> >  	icl_dpll_write(dev_priv, pll);
> > @@ -3171,7 +3191,15 @@ static void icl_pll_disable(struct
> > drm_i915_private *dev_priv, static void combo_pll_disable(struct
> > drm_i915_private *dev_priv, struct intel_shared_dpll *pll)
> >  {
> > -	icl_pll_disable(dev_priv, pll,
> > CNL_DPLL_ENABLE(pll->info->id));
> > +	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > +
> > +	if (IS_ELKHARTLAKE(dev_priv) &&
> > +	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> > +		enable_reg = MG_PLL_ENABLE(0);
> > +		bxt_enable_dc9(dev_priv);
> > +	}
> > +
> > +	icl_pll_disable(dev_priv, pll, enable_reg);
> >  }
> >  
> >  static void tbt_pll_disable(struct drm_i915_private *dev_priv,
> > @@ -3249,6 +3277,7 @@ static const struct intel_dpll_mgr
> > icl_pll_mgr = { static const struct dpll_info ehl_plls[] = {
> >  	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
> >  	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
> > +	{ "DPLL 4", &combo_pll_funcs, DPLL_ID_EHL_DPLL4, 0 },
> >  	{ },
> >  };
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h
> > b/drivers/gpu/drm/i915/intel_dpll_mgr.h index
> > bd8124cc81ed..f3f99929cee8 100644 ---
> > a/drivers/gpu/drm/i915/intel_dpll_mgr.h +++
> > b/drivers/gpu/drm/i915/intel_dpll_mgr.h @@ -113,6 +113,10 @@ enum
> > intel_dpll_id {
> >  	 * @DPLL_ID_ICL_DPLL1: ICL combo PHY DPLL1
> >  	 */
> >  	DPLL_ID_ICL_DPLL1 = 1,
> > +	/**
> > +	 * @DPLL_ID_EHL_DPLL4: EHL combo PHY DPLL4
> > +	 */
> > +	DPLL_ID_EHL_DPLL4 = 2,
> >  	/**
> >  	 * @DPLL_ID_ICL_TBTPLL: ICL TBT PLL
> >  	 */
> > -- 
> > 2.14.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx  
>