> -----Original Message-----
> From: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Sent: Tuesday, September 1, 2020 12:30 PM
> To: Srivatsa, Anusha <anusha.srivatsa@xxxxxxxxx>
> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH] drm/i915/pll: Centralize PLL_ENABLE register
> lookup
>
> On Tue, Sep 01, 2020 at 11:27:58AM -0700, Anusha Srivatsa 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)
> >
> > 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>
> > ---
> > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 25
> > +++++++++++--------
> > 1 file changed, 15 insertions(+), 10 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..7440836c5e44 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,
> > + 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) {
>
> there's probably something else that we can do now with the
> power_{put,get} to get rid of the, now, doubled if checks...
Don't follow you here Rodrigo. Are you suggesting using power_{put/get} to somehow get rid of doubled if?
> > - enable_reg = MG_PLL_ENABLE(0);
> >
> > /*
> > * We need to disable DC states when this DPLL is enabled.
> > @@ -4157,11 +4163,10 @@ 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);
> >
> > 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);
>
> but here, at least, let's clean this function now...
> move this call above and out of the if and delete the one below and keep
> just the power_put inside the if...
Good change. Thanks!
Will change that.
Anusha
> >
> > intel_display_power_put(dev_priv,
> POWER_DOMAIN_DPLL_DC_OFF,
> > --
> > 2.25.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
