Op 26-02-16 om 14:54 schreef Ander Conselvan de Oliveira: > Use a table to store the per-platform shared dpll information in one > place. This way, there is no need for platform specific init funtions. > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_display.c | 16 +-- > drivers/gpu/drm/i915/intel_dpll_mgr.c | 189 ++++++++++++++++------------------ > drivers/gpu/drm/i915/intel_dpll_mgr.h | 22 ++-- > 3 files changed, 108 insertions(+), 119 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index e723323..133b6b7 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9148,8 +9148,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc, > intel_get_shared_dpll_by_id(dev_priv, pll_id); > pll = pipe_config->shared_dpll; > > - WARN_ON(!pll->get_hw_state(dev_priv, pll, > - &pipe_config->dpll_hw_state)); > + WARN_ON(!pll->funcs.get_hw_state(dev_priv, pll, > + &pipe_config->dpll_hw_state)); > > tmp = pipe_config->dpll_hw_state.dpll; > pipe_config->pixel_multiplier = > @@ -9695,8 +9695,8 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc, > > pll = pipe_config->shared_dpll; > if (pll) { > - WARN_ON(!pll->get_hw_state(dev_priv, pll, > - &pipe_config->dpll_hw_state)); > + WARN_ON(!pll->funcs.get_hw_state(dev_priv, pll, > + &pipe_config->dpll_hw_state)); > } > > /* > @@ -12728,7 +12728,7 @@ check_shared_dpll_state(struct drm_device *dev) > > DRM_DEBUG_KMS("%s\n", pll->name); > > - active = pll->get_hw_state(dev_priv, pll, &dpll_hw_state); > + active = pll->funcs.get_hw_state(dev_priv, pll, &dpll_hw_state); > > I915_STATE_WARN(pll->active > hweight32(pll->config.crtc_mask), > "more active pll users than references: %i vs %i\n", > @@ -15466,8 +15466,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > for (i = 0; i < dev_priv->num_shared_dpll; i++) { > struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i]; > > - pll->on = pll->get_hw_state(dev_priv, pll, > - &pll->config.hw_state); > + pll->on = pll->funcs.get_hw_state(dev_priv, pll, > + &pll->config.hw_state); > pll->active = 0; > pll->config.crtc_mask = 0; > for_each_intel_crtc(dev, crtc) { > @@ -15602,7 +15602,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev) > > DRM_DEBUG_KMS("%s enabled but not in use, disabling\n", pll->name); > > - pll->disable(dev_priv, pll); > + pll->funcs.disable(dev_priv, pll); > pll->on = false; > } > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c > index 889ceed..e88dc46 100644 > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > @@ -74,7 +74,7 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv, > if (WARN(!pll, "asserting DPLL %s with no DPLL\n", onoff(state))) > return; > > - cur_state = pll->get_hw_state(dev_priv, pll, &hw_state); > + cur_state = pll->funcs.get_hw_state(dev_priv, pll, &hw_state); > I915_STATE_WARN(cur_state != state, > "%s assertion failure (expected %s, current %s)\n", > pll->name, onoff(state), onoff(cur_state)); > @@ -95,7 +95,7 @@ void intel_prepare_shared_dpll(struct intel_crtc *crtc) > WARN_ON(pll->on); > assert_shared_dpll_disabled(dev_priv, pll); > > - pll->mode_set(dev_priv, pll); > + pll->funcs.mode_set(dev_priv, pll); > } > } > > @@ -133,7 +133,7 @@ void intel_enable_shared_dpll(struct intel_crtc *crtc) > intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS); > > DRM_DEBUG_KMS("enabling %s\n", pll->name); > - pll->enable(dev_priv, pll); > + pll->funcs.enable(dev_priv, pll); > pll->on = true; > } > > @@ -168,7 +168,7 @@ void intel_disable_shared_dpll(struct intel_crtc *crtc) > return; > > DRM_DEBUG_KMS("disabling %s\n", pll->name); > - pll->disable(dev_priv, pll); > + pll->funcs.disable(dev_priv, pll); > pll->on = false; > > intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS); > @@ -398,29 +398,13 @@ static void ibx_pch_dpll_disable(struct drm_i915_private *dev_priv, > udelay(200); > } > > -static char *ibx_pch_dpll_names[] = { > - "PCH DPLL A", > - "PCH DPLL B", > +static const struct intel_shared_dpll_funcs ibx_pch_dpll_funcs = { > + .mode_set = ibx_pch_dpll_mode_set, > + .enable = ibx_pch_dpll_enable, > + .disable = ibx_pch_dpll_disable, > + .get_hw_state = ibx_pch_dpll_get_hw_state, > }; > > -static void ibx_pch_dpll_init(struct drm_device *dev) > -{ > - struct drm_i915_private *dev_priv = dev->dev_private; > - int i; > - > - dev_priv->num_shared_dpll = 2; > - > - for (i = 0; i < dev_priv->num_shared_dpll; i++) { > - dev_priv->shared_dplls[i].id = i; > - dev_priv->shared_dplls[i].name = ibx_pch_dpll_names[i]; > - dev_priv->shared_dplls[i].mode_set = ibx_pch_dpll_mode_set; > - dev_priv->shared_dplls[i].enable = ibx_pch_dpll_enable; > - dev_priv->shared_dplls[i].disable = ibx_pch_dpll_disable; > - dev_priv->shared_dplls[i].get_hw_state = > - ibx_pch_dpll_get_hw_state; > - } > -} > - > static void hsw_ddi_wrpll_enable(struct drm_i915_private *dev_priv, > struct intel_shared_dpll *pll) > { > @@ -492,40 +476,16 @@ static bool hsw_ddi_spll_get_hw_state(struct drm_i915_private *dev_priv, > } > > > -static const char * const hsw_ddi_pll_names[] = { > - "WRPLL 1", > - "WRPLL 2", > - "SPLL" > +static const struct intel_shared_dpll_funcs hsw_ddi_wrpll_funcs = { > + .enable = hsw_ddi_wrpll_enable, > + .disable = hsw_ddi_wrpll_disable, > + .get_hw_state = hsw_ddi_wrpll_get_hw_state, > }; > > -static void hsw_shared_dplls_init(struct drm_i915_private *dev_priv) > -{ > - int i; > - > - dev_priv->num_shared_dpll = 3; > - > - for (i = 0; i < 2; i++) { > - dev_priv->shared_dplls[i].id = i; > - dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i]; > - dev_priv->shared_dplls[i].disable = hsw_ddi_wrpll_disable; > - dev_priv->shared_dplls[i].enable = hsw_ddi_wrpll_enable; > - dev_priv->shared_dplls[i].get_hw_state = > - hsw_ddi_wrpll_get_hw_state; > - } > - > - /* SPLL is special, but needs to be initialized anyway.. */ > - dev_priv->shared_dplls[i].id = i; > - dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i]; > - dev_priv->shared_dplls[i].disable = hsw_ddi_spll_disable; > - dev_priv->shared_dplls[i].enable = hsw_ddi_spll_enable; > - dev_priv->shared_dplls[i].get_hw_state = hsw_ddi_spll_get_hw_state; > - > -} > - > -static const char * const skl_ddi_pll_names[] = { > - "DPLL 1", > - "DPLL 2", > - "DPLL 3", > +static const struct intel_shared_dpll_funcs hsw_ddi_spll_funcs = { > + .enable = hsw_ddi_spll_enable, > + .disable = hsw_ddi_spll_disable, > + .get_hw_state = hsw_ddi_spll_get_hw_state, > }; > > struct skl_dpll_regs { > @@ -634,26 +594,10 @@ out: > return ret; > } > > -static void skl_shared_dplls_init(struct drm_i915_private *dev_priv) > -{ > - int i; > - > - dev_priv->num_shared_dpll = 3; > - > - for (i = 0; i < dev_priv->num_shared_dpll; i++) { > - dev_priv->shared_dplls[i].id = i; > - dev_priv->shared_dplls[i].name = skl_ddi_pll_names[i]; > - dev_priv->shared_dplls[i].disable = skl_ddi_pll_disable; > - dev_priv->shared_dplls[i].enable = skl_ddi_pll_enable; > - dev_priv->shared_dplls[i].get_hw_state = > - skl_ddi_pll_get_hw_state; > - } > -} > - > -static const char * const bxt_ddi_pll_names[] = { > - "PORT PLL A", > - "PORT PLL B", > - "PORT PLL C", > +static const struct intel_shared_dpll_funcs skl_ddi_pll_funcs = { > + .enable = skl_ddi_pll_enable, > + .disable = skl_ddi_pll_disable, > + .get_hw_state = skl_ddi_pll_get_hw_state, > }; > > static void bxt_ddi_pll_enable(struct drm_i915_private *dev_priv, > @@ -838,34 +782,17 @@ out: > return ret; > } > > -static void bxt_shared_dplls_init(struct drm_i915_private *dev_priv) > -{ > - int i; > - > - dev_priv->num_shared_dpll = 3; > - > - for (i = 0; i < dev_priv->num_shared_dpll; i++) { > - dev_priv->shared_dplls[i].id = i; > - dev_priv->shared_dplls[i].name = bxt_ddi_pll_names[i]; > - dev_priv->shared_dplls[i].disable = bxt_ddi_pll_disable; > - dev_priv->shared_dplls[i].enable = bxt_ddi_pll_enable; > - dev_priv->shared_dplls[i].get_hw_state = > - bxt_ddi_pll_get_hw_state; > - } > -} > +static const struct intel_shared_dpll_funcs bxt_ddi_pll_funcs = { > + .enable = bxt_ddi_pll_enable, > + .disable = bxt_ddi_pll_disable, > + .get_hw_state = bxt_ddi_pll_get_hw_state, > +}; > > static void intel_ddi_pll_init(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > uint32_t val = I915_READ(LCPLL_CTL); > > - if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) > - skl_shared_dplls_init(dev_priv); > - else if (IS_BROXTON(dev)) > - bxt_shared_dplls_init(dev_priv); > - else > - hsw_shared_dplls_init(dev_priv); > - > if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) { > int cdclk_freq; > > @@ -893,16 +820,72 @@ static void intel_ddi_pll_init(struct drm_device *dev) > } > } > > +struct dpll_info { > + const char *name; > + const int id; > + const struct intel_shared_dpll_funcs *funcs; > +}; > + Seems shared_dplls[i].id == i, so that could be removed. If you also move name to funcs you could kill this struct. ~Maarten _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx