On Tue, 08 Sep 2020, Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx> wrote: > We currenty check for platform at multiple parts in the driver > to grab the correct PLL. Let us begin to centralize it through a > helper function. > > v2: s/intel_get_pll_enable_reg()/intel_combo_pll_enable_reg() (Ville) > > v3: Clean up combo_pll_disable() (Rodrigo) > > Suggested-by: Matt Roper <matthew.d.roper@xxxxxxxxx> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Cc: Matt Roper <matthew.d.roper@xxxxxxxxx> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > --- > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 29 +++++++++++-------- > 1 file changed, 17 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > index c9013f8f766f..441b6f52e808 100644 > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > @@ -147,6 +147,18 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv, > pll->info->name, onoff(state), onoff(cur_state)); > } > > +static > +i915_reg_t intel_combo_pll_enable_reg(struct drm_i915_private *dev_priv, Please keep the static keyword and the return type on the same line with each other. And since you're touching this, please rename dev_priv to i915 in all new code adding it. BR, Jani. > + struct intel_shared_dpll *pll) > +{ > + > + if (IS_ELKHARTLAKE(dev_priv) && (pll->info->id == DPLL_ID_EHL_DPLL4)) > + return MG_PLL_ENABLE(0); > + > + return CNL_DPLL_ENABLE(pll->info->id); > + > + > +} > /** > * intel_prepare_shared_dpll - call a dpll's prepare hook > * @crtc_state: CRTC, and its state, which has a shared dpll > @@ -3842,12 +3854,7 @@ 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) > { > - 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); > - } > + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll); > > return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg); > } > @@ -4045,11 +4052,10 @@ static void icl_pll_enable(struct drm_i915_private *dev_priv, > static void combo_pll_enable(struct drm_i915_private *dev_priv, > struct intel_shared_dpll *pll) > { > - i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); > + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll); > > if (IS_ELKHARTLAKE(dev_priv) && > pll->info->id == DPLL_ID_EHL_DPLL4) { > - enable_reg = MG_PLL_ENABLE(0); > > /* > * We need to disable DC states when this DPLL is enabled. > @@ -4157,19 +4163,18 @@ 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) > { > - i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id); > + i915_reg_t enable_reg = intel_combo_pll_enable_reg(dev_priv, pll); > + > + icl_pll_disable(dev_priv, pll, enable_reg); > > if (IS_ELKHARTLAKE(dev_priv) && > pll->info->id == DPLL_ID_EHL_DPLL4) { > - enable_reg = MG_PLL_ENABLE(0); > - icl_pll_disable(dev_priv, pll, enable_reg); > > intel_display_power_put(dev_priv, POWER_DOMAIN_DPLL_DC_OFF, > pll->wakeref); > return; > } > > - icl_pll_disable(dev_priv, pll, enable_reg); > } > > static void tbt_pll_disable(struct drm_i915_private *dev_priv, -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx