On Tue, Mar 05, 2019 at 05:26:33PM -0800, Lucas De Marchi wrote:
> This allows us to share the icl_pll_enable() between the different types
> of PLL while allowing the caller to differentiate how to write the
> registers.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_dpll_mgr.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 3b3de99756d6..5511bc23ea3d 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -3118,9 +3118,10 @@ static void icl_mg_pll_write(struct drm_i915_private *dev_priv,
>
> static void icl_pll_enable(struct drm_i915_private *dev_priv,
> struct intel_shared_dpll *pll,
> - i915_reg_t enable_reg)
> + i915_reg_t enable_reg,
> + void (*pll_write)(struct drm_i915_private *dev_priv,
> + struct intel_shared_dpll *pll))
> {
> - const enum intel_dpll_id id = pll->info->id;
> u32 val;
>
> val = I915_READ(enable_reg);
> @@ -3133,12 +3134,9 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
> */
> if (intel_wait_for_register(dev_priv, enable_reg, PLL_POWER_STATE,
> PLL_POWER_STATE, 1))
> - DRM_ERROR("PLL %d Power not enabled\n", id);
> + DRM_ERROR("PLL %d Power not enabled\n", pll->info->id);
>
> - if (intel_dpll_is_combophy(id) || id == DPLL_ID_ICL_TBTPLL)
> - icl_dpll_write(dev_priv, pll);
> - else
> - icl_mg_pll_write(dev_priv, pll);
> + pll_write(dev_priv, pll);
Hmm. Would it be cleaner to just exract the pll power
enable/disable and pll enable/disable parts into small helpers?
It looks like like glk/cnl also follow this same pattern, so
there may be a chance to reuse the code on those platforms
as well.
>
> /*
> * DVFS pre sequence would be here, but in our driver the cdclk code
> @@ -3152,7 +3150,7 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv,
>
> if (intel_wait_for_register(dev_priv, enable_reg, PLL_LOCK, PLL_LOCK,
> 1)) /* 600us actually. */
> - DRM_ERROR("PLL %d not locked\n", id);
> + DRM_ERROR("PLL %d not locked\n", pll->info->id);
>
> /* DVFS post sequence would be here. See the comment above. */
> }
> @@ -3162,7 +3160,7 @@ static void combo_pll_enable(struct drm_i915_private *dev_priv,
> {
> i915_reg_t enable_reg = icl_pll_id_to_enable_reg(pll->info->id);
>
> - icl_pll_enable(dev_priv, pll, enable_reg);
> + icl_pll_enable(dev_priv, pll, enable_reg, icl_dpll_write);
> }
>
> static void mg_pll_enable(struct drm_i915_private *dev_priv,
> @@ -3171,7 +3169,7 @@ 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));
>
> - icl_pll_enable(dev_priv, pll, enable_reg);
> + icl_pll_enable(dev_priv, pll, enable_reg, icl_mg_pll_write);
> }
>
> static void icl_pll_disable(struct drm_i915_private *dev_priv,
> --
> 2.20.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
