> -----Original Message-----
> From: Jani Nikula <[email protected]>
> Sent: Thursday, September 10, 2020 6:31 AM
> To: Srivatsa, Anusha <[email protected]>; intel-
> [email protected]
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE register
> lookup
> 
> On Tue, 08 Sep 2020, Anusha Srivatsa <[email protected]> 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 <[email protected]>
> > Cc: Ville Syrjälä <[email protected]>
> > Cc: Matt Roper <[email protected]>
> > Signed-off-by: Anusha Srivatsa <[email protected]>
> > Reviewed-by: Rodrigo Vivi <[email protected]>
> > ---
> >  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.

Sure. Thanks for the feedback Jani.

Anusha
> 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to