On Mon, Aug 31, 2020 at 07:03:47PM +0000, Srivatsa, Anusha wrote:
>
>
> > -----Original Message-----
> > From: Ville Syrjälä <[email protected]>
> > Sent: Monday, August 31, 2020 6:42 AM
> > To: Srivatsa, Anusha <[email protected]>
> > Cc: [email protected]
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/pll: Centralize PLL_ENABLE
> > register
> > lookup
> >
> > On Fri, Aug 28, 2020 at 02:58:32PM -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.
> > >
> > > Suggested-by: Matt Roper <[email protected]>
> > > Cc: Matt Roper <[email protected]>
> > > Signed-off-by: Anusha Srivatsa <[email protected]>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 27
> > > ++++++++++++-------
> > > 1 file changed, 17 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 81ab975fe4f0..388136618bb7 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > @@ -147,6 +147,20 @@ void assert_shared_dpll(struct drm_i915_private
> > *dev_priv,
> > > pll->info->name, onoff(state), onoff(cur_state)); }
> > >
> > > +static
> > > +i915_reg_t intel_get_pll_enable_reg(struct drm_i915_private *dev_priv,
> > > + struct intel_shared_dpll *pll)
> >
> > combo_pll_enable_reg() ?
> Actually want to avoid mentioning combo in the name. We might have platforms
> that do not have combo phys. We still want this function to be one place
> where platforms gets the PLL_ENABLE register.
There's no point in mixing up different PHY types in a single function.
>
> >
> > > +{
> > > +
> > > + if (IS_ELKHARTLAKE(dev_priv)) {
> > > + if (pll->info->id == DPLL_ID_EHL_DPLL4)
> > > + return MG_PLL_ENABLE(0);
> > > + }
> >
> > Ugly nested if.
> Will change it.
>
> Anusha
> > > +
> > > + 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 +3856,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_get_pll_enable_reg(dev_priv, pll);
> > >
> > > return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg); }
> > > @@ -4045,11 +4054,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_get_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,11 +4165,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_get_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);
> > >
> > > intel_display_power_put(dev_priv,
> > POWER_DOMAIN_DPLL_DC_OFF,
> > > --
> > > 2.25.0
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > [email protected]
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Ville Syrjälä
> > Intel
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
