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

[Ville Syrjälä]

On Fri, Apr 05, 2019 at 09:33:56PM +0300, Ville Syrjälä wrote:
> On Fri, Apr 05, 2019 at 10:59:53AM -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
> > 
> > Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy at intel.com>
> > Reviewed-by: Bob Paauwe <bob.j.paauwe at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 60 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 59 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > index e01c057ce50b..c3f0b9720c54 100644
> > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > @@ -2870,6 +2870,56 @@ icl_get_dpll(struct intel_crtc_state *crtc_state,
> >  	return pll;
> >  }
> >  
> > +static struct intel_shared_dpll *
> > +ehl_get_dpll(struct intel_crtc_state *crtc_state,
> > +	     struct intel_encoder *encoder)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> > +	struct intel_shared_dpll *pll;
> > +	enum port port = encoder->port;
> > +	enum intel_dpll_id min, max;
> > +	bool ret;
> > +
> > +	if (!intel_port_is_combophy(dev_priv, port)) {
> > +		MISSING_CASE(port);
> > +		return NULL;
> > +	}
> > +
> > +	min = DPLL_ID_ICL_DPLL0;
> > +	max = DPLL_ID_ICL_DPLL1;
> > +	ret = icl_calc_dpll_state(crtc_state, encoder);
> > +	if (ret) {
> > +		pll = intel_find_shared_dpll(crtc_state, min, max);
> > +		if (pll) {
> > +			intel_reference_shared_dpll(pll, crtc_state);
> > +			return pll;
> > +		}
> > +	} else {
> > +		DRM_DEBUG_KMS("Could not calculate PLL state.\n");
> > +	}
> > +
> > +	if (encoder->type == INTEL_OUTPUT_EDP) {
> > +		DRM_DEBUG_KMS("Cannot use DPLL4 with EDP.\n");
> > +		return NULL;
> > +	}
> > +
> > +	min = max = DPLL_ID_ICL_MGPLL1;
> > +	ret = icl_calc_mg_pll_state(crtc_state);
> > +	if (!ret) {
> > +		DRM_DEBUG_KMS("Could not calculate PLL state.\n");
> > +		return NULL;
> > +	}
> > +
> > +	pll = intel_find_shared_dpll(crtc_state, min, max);
> > +	if (!pll) {
> > +		DRM_DEBUG_KMS("No PLL selected\n");
> > +		return NULL;
> > +	}
> > +
> > +	intel_reference_shared_dpll(pll, crtc_state);
> > +	return pll;
> > +}
> > +
> >  static bool mg_pll_get_hw_state(struct drm_i915_private *dev_priv,
> >  				struct intel_shared_dpll *pll,
> >  				struct intel_dpll_hw_state *hw_state)
> > @@ -3115,6 +3165,13 @@ static void mg_pll_enable(struct drm_i915_private *dev_priv,
> >  	i915_reg_t enable_reg =
> >  		MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id));
> >  
> > +	if (IS_ELKHARTLAKE(dev_priv) &&
> > +	   (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5 ||
> > +	    I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6)) {
> > +		DRM_ERROR("Cant enable DPLL4 when DC5 or DC6 are enabled\n");
> > +		return;
> > +	}
> 
> This looks like dead code, or we messed up earlier already (in which
> case it should just be some kind of assert IMO).

Does DMC handle other MG PLLs? I would have thought not, so if we keep
the assert then it should perhaps be unconditional. Hmm. But aren't we
still hodling the modeset power domain when this gets called? If so this
is definitely dead code.

> 
> Also what is this EHL thing anyway? I can't even find it in the spec.
> 
> > +
> >  	icl_pll_power_enable(dev_priv, pll, enable_reg);
> >  
> >  	icl_mg_pll_write(dev_priv, pll);
> > @@ -3249,12 +3306,13 @@ 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", &mg_pll_funcs, DPLL_ID_ICL_MGPLL1,0 },
> >  	{ },
> >  };
> >  
> >  static const struct intel_dpll_mgr ehl_pll_mgr = {
> >  	.dpll_info = ehl_plls,
> > -	.get_dpll = icl_get_dpll,
> > +	.get_dpll = ehl_get_dpll,
> >  	.dump_hw_state = icl_dump_hw_state,
> >  };
> >  
> > -- 
> > 2.14.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel