[Intel-gfx] [RFC 1/3] drm/i915/icl: Restructure ICL DPLL enable functionality

[Ville Syrjälä]

On Fri, Sep 14, 2018 at 12:24:12PM +0530, Vandita Kulkarni wrote:
> From: Madhav Chauhan <madhav.chauhan at intel.com>
> 
> In Gen11, DPLL 0 and 1 are shared between DDI and DSI.
> Most of the steps for enabling DPLL are common across DDI
> and DSI. This patch makes icl_dpll_enable() generic which
> will be used by all the encoders.
> 
> Signed-off-by: Madhav Chauhan <madhav.chauhan at intel.com>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c      | 12 ++++++++++++
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 19 ++-----------------
>  drivers/gpu/drm/i915/intel_dpll_mgr.h |  2 +-
>  3 files changed, 15 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index cd01a09..2942a24 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2810,6 +2810,18 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder,
>  	mutex_lock(&dev_priv->dpll_lock);
>  
>  	if (IS_ICELAKE(dev_priv)) {
> +		enum intel_dpll_id id = pll->info->id;
> +		i915_reg_t enable_reg = icl_pll_id_to_enable_reg(id);
> +
> +		val = I915_READ(enable_reg);
> +		val |= PLL_ENABLE;
> +		I915_WRITE(enable_reg, val);
> +
> +		/* TODO: wait times missing from the spec. */
> +		if (intel_wait_for_register(dev_priv, enable_reg, PLL_LOCK,
> +					    PLL_LOCK, 5))
> +			DRM_ERROR("PLL %d not locked\n", id);
> +

I don't really see why this can't stay in the dpll mgr.

>  		if (port >= PORT_C)
>  			I915_WRITE(DDI_CLK_SEL(port),
>  				   icl_pll_to_ddi_pll_sel(encoder, pll));
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index e6cac92..36ed155 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -2930,7 +2930,7 @@ static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
>  	return pll;
>  }
>  
> -static i915_reg_t icl_pll_id_to_enable_reg(enum intel_dpll_id id)
> +i915_reg_t icl_pll_id_to_enable_reg(enum intel_dpll_id id)
>  {
>  	switch (id) {
>  	default:
> @@ -3119,22 +3119,7 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
>  	default:
>  		MISSING_CASE(id);
>  	}
> -
> -	/*
> -	 * DVFS pre sequence would be here, but in our driver the cdclk code
> -	 * paths should already be setting the appropriate voltage, hence we do
> -	 * nothign here.
> -	 */
> -
> -	val = I915_READ(enable_reg);
> -	val |= PLL_ENABLE;
> -	I915_WRITE(enable_reg, val);
> -
> -	if (intel_wait_for_register(dev_priv, enable_reg, PLL_LOCK, PLL_LOCK,
> -				    1)) /* 600us actually. */
> -		DRM_ERROR("PLL %d not locked\n", id);
> -
> -	/* DVFS post sequence would be here. See the comment above. */
> +	/* Encoder specific PLL enable steps are added in encoder file */
>  }
>  
>  static void icl_pll_disable(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> index bf0de8a..9e89265 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> @@ -345,5 +345,5 @@ void intel_dpll_dump_hw_state(struct drm_i915_private *dev_priv,
>  int icl_calc_dp_combo_pll_link(struct drm_i915_private *dev_priv,
>  			       uint32_t pll_id);
>  int cnl_hdmi_pll_ref_clock(struct drm_i915_private *dev_priv);
> -
> +i915_reg_t icl_pll_id_to_enable_reg(enum intel_dpll_id id);
>  #endif /* _INTEL_DPLL_MGR_H_ */
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel