| Message ID | 20180720141504.22832-5-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Em Sex, 2018-07-20 às 17:14 +0300, Imre Deak escreveu: > It makes sense to keep unchanging data const. Extract such fields > from > the i915_power_well struct into a new i915_power_well_desc struct > that > we initialize during compile time. For the rest of the dynamic > fields allocate an array of i915_power_well objects in i915 dev_priv, > and link to each of these objects their corresponding > i915_power_well_desc object. > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Jani Nikula <jani.nikula@intel.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> Quite a few issues pointed by checkpatch for this patch, please take a look at them. More below: > --- > drivers/gpu/drm/i915/i915_debugfs.c | 4 +- > drivers/gpu/drm/i915/i915_drv.c | 8 +- > drivers/gpu/drm/i915/i915_drv.h | 14 ++- > drivers/gpu/drm/i915/intel_display.h | 4 +- > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_hdcp.c | 6 +- > drivers/gpu/drm/i915/intel_runtime_pm.c | 204 +++++++++++++++++++--- > ---------- > 7 files changed, 144 insertions(+), 97 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index b3aefd623557..eb284cac8fda 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -2833,10 +2833,10 @@ static int i915_power_domain_info(struct > seq_file *m, void *unused) > enum intel_display_power_domain power_domain; > > power_well = &power_domains->power_wells[i]; > - seq_printf(m, "%-25s %d\n", power_well->name, > + seq_printf(m, "%-25s %d\n", power_well->desc->name, > power_well->count); > > - for_each_power_domain(power_domain, power_well- > >domains) > + for_each_power_domain(power_domain, power_well- > >desc->domains) > seq_printf(m, " %-23s %d\n", > intel_display_power_domain_str(powe > r_domain), > power_domains- > >domain_use_count[power_domain]); > diff --git a/drivers/gpu/drm/i915/i915_drv.c > b/drivers/gpu/drm/i915/i915_drv.c > index 3c984530fef9..5743db4500fb 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -922,7 +922,9 @@ static int i915_driver_init_early(struct > drm_i915_private *dev_priv, > intel_uc_init_early(dev_priv); > intel_pm_setup(dev_priv); > intel_init_dpio(dev_priv); > - intel_power_domains_init(dev_priv); > + ret = intel_power_domains_init(dev_priv); > + if (ret < 0) > + goto err_uc; > intel_irq_init(dev_priv); > intel_hangcheck_init(dev_priv); > intel_init_display_hooks(dev_priv); > @@ -934,6 +936,9 @@ static int i915_driver_init_early(struct > drm_i915_private *dev_priv, > > return 0; > > +err_uc: > + intel_uc_cleanup_early(dev_priv); Please leave the guc fixes for a different patch, regardless of how innocent they look. Everything else looks good! Thanks, Paulo > + i915_gem_cleanup_early(dev_priv); > err_workqueues: > i915_workqueues_cleanup(dev_priv); > err_engines: > @@ -948,6 +953,7 @@ static int i915_driver_init_early(struct > drm_i915_private *dev_priv, > static void i915_driver_cleanup_early(struct drm_i915_private > *dev_priv) > { > intel_irq_fini(dev_priv); > + intel_power_domains_cleanup(dev_priv); > intel_uc_cleanup_early(dev_priv); > i915_gem_cleanup_early(dev_priv); > i915_workqueues_cleanup(dev_priv); > diff --git a/drivers/gpu/drm/i915/i915_drv.h > b/drivers/gpu/drm/i915/i915_drv.h > index 4fb937399440..3ae200a9e8f1 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -862,13 +862,9 @@ struct i915_power_well_ops { > }; > > /* Power well structure for haswell */ > -struct i915_power_well { > +struct i915_power_well_desc { > const char *name; > bool always_on; > - /* power well enable/disable usage count */ > - int count; > - /* cached hw enabled state */ > - bool hw_enabled; > u64 domains; > /* unique identifier for this power well */ > enum i915_power_well_id id; > @@ -891,6 +887,14 @@ struct i915_power_well { > const struct i915_power_well_ops *ops; > }; > > +struct i915_power_well { > + const struct i915_power_well_desc *desc; > + /* power well enable/disable usage count */ > + int count; > + /* cached hw enabled state */ > + bool hw_enabled; > +}; > + > struct i915_power_domains { > /* > * Power wells needed for initialization at driver init and > suspend > diff --git a/drivers/gpu/drm/i915/intel_display.h > b/drivers/gpu/drm/i915/intel_display.h > index 9292001cdd14..a626282d590b 100644 > --- a/drivers/gpu/drm/i915/intel_display.h > +++ b/drivers/gpu/drm/i915/intel_display.h > @@ -322,11 +322,11 @@ struct intel_link_m_n { > > #define for_each_power_domain_well(__dev_priv, __power_well, > __domain_mask) \ > for_each_power_well(__dev_priv, __power_well) > \ > - for_each_if((__power_well)->domains & > (__domain_mask)) > + for_each_if((__power_well)->desc->domains & > (__domain_mask)) > > #define for_each_power_domain_well_rev(__dev_priv, __power_well, > __domain_mask) \ > for_each_power_well_rev(__dev_priv, __power_well) > \ > - for_each_if((__power_well)->domains & > (__domain_mask)) > + for_each_if((__power_well)->desc->domains & > (__domain_mask)) > > #define for_each_new_intel_plane_in_state(__state, plane, > new_plane_state, __i) \ > for ((__i) = 0; \ > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 32be305c0e89..25f9b035cfe8 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1943,6 +1943,7 @@ int intel_psr_wait_for_idle(const struct > intel_crtc_state *new_crtc_state); > > /* intel_runtime_pm.c */ > int intel_power_domains_init(struct drm_i915_private *); > +void intel_power_domains_cleanup(struct drm_i915_private *); > void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, > bool resume); > void intel_power_domains_fini_hw(struct drm_i915_private *); > void intel_power_domains_suspend(struct drm_i915_private *dev_priv); > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c > b/drivers/gpu/drm/i915/intel_hdcp.c > index 0cc6a861bcf8..26e48fc95543 100644 > --- a/drivers/gpu/drm/i915/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/intel_hdcp.c > @@ -57,9 +57,9 @@ static bool hdcp_key_loadable(struct > drm_i915_private *dev_priv) > > /* PG1 (power well #1) needs to be enabled */ > for_each_power_well(dev_priv, power_well) { > - if (power_well->id == id) { > - enabled = power_well->ops- > >is_enabled(dev_priv, > - power_ > well); > + if (power_well->desc->id == id) { > + enabled = power_well->desc->ops- > >is_enabled(dev_priv, > + > power_well); > break; > } > } > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index f119cbe4f61d..8b3c241bee55 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -159,17 +159,17 @@ intel_display_power_domain_str(enum > intel_display_power_domain domain) > static void intel_power_well_enable(struct drm_i915_private > *dev_priv, > struct i915_power_well > *power_well) > { > - DRM_DEBUG_KMS("enabling %s\n", power_well->name); > - power_well->ops->enable(dev_priv, power_well); > + DRM_DEBUG_KMS("enabling %s\n", power_well->desc->name); > + power_well->desc->ops->enable(dev_priv, power_well); > power_well->hw_enabled = true; > } > > static void intel_power_well_disable(struct drm_i915_private > *dev_priv, > struct i915_power_well > *power_well) > { > - DRM_DEBUG_KMS("disabling %s\n", power_well->name); > + DRM_DEBUG_KMS("disabling %s\n", power_well->desc->name); > power_well->hw_enabled = false; > - power_well->ops->disable(dev_priv, power_well); > + power_well->desc->ops->disable(dev_priv, power_well); > } > > static void intel_power_well_get(struct drm_i915_private *dev_priv, > @@ -183,7 +183,7 @@ static void intel_power_well_put(struct > drm_i915_private *dev_priv, > struct i915_power_well *power_well) > { > WARN(!power_well->count, "Use count on power well %s is > already zero", > - power_well->name); > + power_well->desc->name); > > if (!--power_well->count) > intel_power_well_disable(dev_priv, power_well); > @@ -213,7 +213,7 @@ bool __intel_display_power_is_enabled(struct > drm_i915_private *dev_priv, > is_enabled = true; > > for_each_power_domain_well_rev(dev_priv, power_well, > BIT_ULL(domain)) { > - if (power_well->always_on) > + if (power_well->desc->always_on) > continue; > > if (!power_well->hw_enabled) { > @@ -323,7 +323,7 @@ static void hsw_power_well_pre_disable(struct > drm_i915_private *dev_priv, > static void hsw_wait_for_power_well_enable(struct drm_i915_private > *dev_priv, > struct i915_power_well > *power_well) > { > - enum i915_power_well_id id = power_well->id; > + enum i915_power_well_id id = power_well->desc->id; > > /* Timeout for PW1:10 us, AUX:not specified, other PWs:20 > us. */ > WARN_ON(intel_wait_for_register(dev_priv, > @@ -350,7 +350,7 @@ static u32 hsw_power_well_requesters(struct > drm_i915_private *dev_priv, > static void hsw_wait_for_power_well_disable(struct drm_i915_private > *dev_priv, > struct i915_power_well > *power_well) > { > - enum i915_power_well_id id = power_well->id; > + enum i915_power_well_id id = power_well->desc->id; > bool disabled; > u32 reqs; > > @@ -370,7 +370,7 @@ static void > hsw_wait_for_power_well_disable(struct drm_i915_private *dev_priv, > return; > > DRM_DEBUG_KMS("%s forced on (bios:%d driver:%d kvmr:%d > debug:%d)\n", > - power_well->name, > + power_well->desc->name, > !!(reqs & 1), !!(reqs & 2), !!(reqs & 4), > !!(reqs & 8)); > } > > @@ -386,8 +386,8 @@ static void gen9_wait_for_power_well_fuses(struct > drm_i915_private *dev_priv, > static void hsw_power_well_enable(struct drm_i915_private *dev_priv, > struct i915_power_well > *power_well) > { > - enum i915_power_well_id id = power_well->id; > - bool wait_fuses = power_well->hsw.has_fuses; > + enum i915_power_well_id id = power_well->desc->id; > + bool wait_fuses = power_well->desc->hsw.has_fuses; > enum skl_power_gate uninitialized_var(pg); > u32 val; > > @@ -421,17 +421,19 @@ static void hsw_power_well_enable(struct > drm_i915_private *dev_priv, > if (wait_fuses) > gen9_wait_for_power_well_fuses(dev_priv, pg); > > - hsw_power_well_post_enable(dev_priv, power_well- > >hsw.irq_pipe_mask, > - power_well->hsw.has_vga); > + hsw_power_well_post_enable(dev_priv, > + power_well->desc- > >hsw.irq_pipe_mask, > + power_well->desc->hsw.has_vga); > } > > static void hsw_power_well_disable(struct drm_i915_private > *dev_priv, > struct i915_power_well > *power_well) > { > - enum i915_power_well_id id = power_well->id; > + enum i915_power_well_id id = power_well->desc->id; > u32 val; > > - hsw_power_well_pre_disable(dev_priv, power_well- > >hsw.irq_pipe_mask); > + hsw_power_well_pre_disable(dev_priv, > + power_well->desc- > >hsw.irq_pipe_mask); > > val = I915_READ(HSW_PWR_WELL_CTL_DRIVER(id)); > I915_WRITE(HSW_PWR_WELL_CTL_DRIVER(id), > @@ -445,7 +447,7 @@ static void > icl_combo_phy_aux_power_well_enable(struct drm_i915_private > *dev_priv, > struct i915_power_well > *power_well) > { > - enum i915_power_well_id id = power_well->id; > + enum i915_power_well_id id = power_well->desc->id; > enum port port = ICL_AUX_PW_TO_PORT(id); > u32 val; > > @@ -462,7 +464,7 @@ static void > icl_combo_phy_aux_power_well_disable(struct drm_i915_private > *dev_priv, > struct i915_power_well > *power_well) > { > - enum i915_power_well_id id = power_well->id; > + enum i915_power_well_id id = power_well->desc->id; > enum port port = ICL_AUX_PW_TO_PORT(id); > u32 val; > > @@ -484,7 +486,7 @@ icl_combo_phy_aux_power_well_disable(struct > drm_i915_private *dev_priv, > static bool hsw_power_well_enabled(struct drm_i915_private > *dev_priv, > struct i915_power_well > *power_well) > { > - enum i915_power_well_id id = power_well->id; > + enum i915_power_well_id id = power_well->desc->id; > u32 mask = HSW_PWR_WELL_CTL_REQ(id) | > HSW_PWR_WELL_CTL_STATE(id); > > return (I915_READ(HSW_PWR_WELL_CTL_DRIVER(id)) & mask) == > mask; > @@ -723,7 +725,7 @@ static void skl_enable_dc6(struct > drm_i915_private *dev_priv) > static void hsw_power_well_sync_hw(struct drm_i915_private > *dev_priv, > struct i915_power_well > *power_well) > { > - enum i915_power_well_id id = power_well->id; > + enum i915_power_well_id id = power_well->desc->id; > u32 mask = HSW_PWR_WELL_CTL_REQ(id); > u32 bios_req = I915_READ(HSW_PWR_WELL_CTL_BIOS(id)); > > @@ -740,19 +742,19 @@ static void hsw_power_well_sync_hw(struct > drm_i915_private *dev_priv, > static void bxt_dpio_cmn_power_well_enable(struct drm_i915_private > *dev_priv, > struct i915_power_well > *power_well) > { > - bxt_ddi_phy_init(dev_priv, power_well->bxt.phy); > + bxt_ddi_phy_init(dev_priv, power_well->desc->bxt.phy); > } > > static void bxt_dpio_cmn_power_well_disable(struct drm_i915_private > *dev_priv, > struct i915_power_well > *power_well) > { > - bxt_ddi_phy_uninit(dev_priv, power_well->bxt.phy); > + bxt_ddi_phy_uninit(dev_priv, power_well->desc->bxt.phy); > } > > static bool bxt_dpio_cmn_power_well_enabled(struct drm_i915_private > *dev_priv, > struct i915_power_well > *power_well) > { > - return bxt_ddi_phy_is_enabled(dev_priv, power_well- > >bxt.phy); > + return bxt_ddi_phy_is_enabled(dev_priv, power_well->desc- > >bxt.phy); > } > > static void bxt_verify_ddi_phy_power_wells(struct drm_i915_private > *dev_priv) > @@ -761,16 +763,17 @@ static void > bxt_verify_ddi_phy_power_wells(struct drm_i915_private *dev_priv) > > power_well = lookup_power_well(dev_priv, BXT_DPIO_CMN_A); > if (power_well->count > 0) > - bxt_ddi_phy_verify_state(dev_priv, power_well- > >bxt.phy); > + bxt_ddi_phy_verify_state(dev_priv, power_well->desc- > >bxt.phy); > > power_well = lookup_power_well(dev_priv, BXT_DPIO_CMN_BC); > if (power_well->count > 0) > - bxt_ddi_phy_verify_state(dev_priv, power_well- > >bxt.phy); > + bxt_ddi_phy_verify_state(dev_priv, power_well->desc- > >bxt.phy); > > if (IS_GEMINILAKE(dev_priv)) { > power_well = lookup_power_well(dev_priv, > GLK_DPIO_CMN_C); > if (power_well->count > 0) > - bxt_ddi_phy_verify_state(dev_priv, > power_well->bxt.phy); > + bxt_ddi_phy_verify_state(dev_priv, > + power_well->desc- > >bxt.phy); > } > } > > @@ -869,7 +872,7 @@ static void i830_pipes_power_well_sync_hw(struct > drm_i915_private *dev_priv, > static void vlv_set_power_well(struct drm_i915_private *dev_priv, > struct i915_power_well *power_well, > bool enable) > { > - enum i915_power_well_id power_well_id = power_well->id; > + enum i915_power_well_id power_well_id = power_well->desc- > >id; > u32 mask; > u32 state; > u32 ctrl; > @@ -917,7 +920,7 @@ static void vlv_power_well_disable(struct > drm_i915_private *dev_priv, > static bool vlv_power_well_enabled(struct drm_i915_private > *dev_priv, > struct i915_power_well > *power_well) > { > - enum i915_power_well_id power_well_id = power_well->id; > + enum i915_power_well_id power_well_id = power_well->desc- > >id; > bool enabled = false; > u32 mask; > u32 state; > @@ -1107,7 +1110,7 @@ lookup_power_well(struct drm_i915_private > *dev_priv, > struct i915_power_well *power_well; > > power_well = &power_domains->power_wells[i]; > - if (power_well->id == power_well_id) > + if (power_well->desc->id == power_well_id) > return power_well; > } > > @@ -1146,7 +1149,7 @@ static void assert_chv_phy_status(struct > drm_i915_private *dev_priv) > PHY_STATUS_SPLINE_LDO(DPIO_PHY1 > , DPIO_CH0, 0) | > PHY_STATUS_SPLINE_LDO(DPIO_PHY1 > , DPIO_CH0, 1)); > > - if (cmn_bc->ops->is_enabled(dev_priv, cmn_bc)) { > + if (cmn_bc->desc->ops->is_enabled(dev_priv, cmn_bc)) { > phy_status |= PHY_POWERGOOD(DPIO_PHY0); > > /* this assumes override is only used to enable > lanes */ > @@ -1187,7 +1190,7 @@ static void assert_chv_phy_status(struct > drm_i915_private *dev_priv) > phy_status |= > PHY_STATUS_SPLINE_LDO(DPIO_PHY0, DPIO_CH1, 1); > } > > - if (cmn_d->ops->is_enabled(dev_priv, cmn_d)) { > + if (cmn_d->desc->ops->is_enabled(dev_priv, cmn_d)) { > phy_status |= PHY_POWERGOOD(DPIO_PHY1); > > /* this assumes override is only used to enable > lanes */ > @@ -1231,10 +1234,10 @@ static void > chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv, > enum pipe pipe; > uint32_t tmp; > > - WARN_ON_ONCE(power_well->id != PUNIT_POWER_WELL_DPIO_CMN_BC > && > - power_well->id != PUNIT_POWER_WELL_DPIO_CMN_D); > + WARN_ON_ONCE(power_well->desc->id != > PUNIT_POWER_WELL_DPIO_CMN_BC && > + power_well->desc->id != > PUNIT_POWER_WELL_DPIO_CMN_D); > > - if (power_well->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { > + if (power_well->desc->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { > pipe = PIPE_A; > phy = DPIO_PHY0; > } else { > @@ -1262,7 +1265,7 @@ static void > chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv, > DPIO_SUS_CLK_CONFIG_GATE_CLKREQ; > vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW28, tmp); > > - if (power_well->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { > + if (power_well->desc->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { > tmp = vlv_dpio_read(dev_priv, pipe, > _CHV_CMN_DW6_CH1); > tmp |= DPIO_DYNPWRDOWNEN_CH1; > vlv_dpio_write(dev_priv, pipe, _CHV_CMN_DW6_CH1, > tmp); > @@ -1293,10 +1296,10 @@ static void > chv_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv, > { > enum dpio_phy phy; > > - WARN_ON_ONCE(power_well->id != PUNIT_POWER_WELL_DPIO_CMN_BC > && > - power_well->id != PUNIT_POWER_WELL_DPIO_CMN_D); > + WARN_ON_ONCE(power_well->desc->id != > PUNIT_POWER_WELL_DPIO_CMN_BC && > + power_well->desc->id != > PUNIT_POWER_WELL_DPIO_CMN_D); > > - if (power_well->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { > + if (power_well->desc->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { > phy = DPIO_PHY0; > assert_pll_disabled(dev_priv, PIPE_A); > assert_pll_disabled(dev_priv, PIPE_B); > @@ -2051,7 +2054,7 @@ static const struct i915_power_well_ops > chv_dpio_cmn_power_well_ops = { > .is_enabled = vlv_power_well_enabled, > }; > > -static struct i915_power_well i9xx_always_on_power_well[] = { > +static const struct i915_power_well_desc i9xx_always_on_power_well[] > = { > { > .name = "always-on", > .always_on = 1, > @@ -2068,7 +2071,7 @@ static const struct i915_power_well_ops > i830_pipes_power_well_ops = { > .is_enabled = i830_pipes_power_well_enabled, > }; > > -static struct i915_power_well i830_power_wells[] = { > +static const struct i915_power_well_desc i830_power_wells[] = { > { > .name = "always-on", > .always_on = 1, > @@ -2105,7 +2108,7 @@ static const struct i915_power_well_ops > bxt_dpio_cmn_power_well_ops = { > .is_enabled = bxt_dpio_cmn_power_well_enabled, > }; > > -static struct i915_power_well hsw_power_wells[] = { > +static const struct i915_power_well_desc hsw_power_wells[] = { > { > .name = "always-on", > .always_on = 1, > @@ -2124,7 +2127,7 @@ static struct i915_power_well hsw_power_wells[] > = { > }, > }; > > -static struct i915_power_well bdw_power_wells[] = { > +static const struct i915_power_well_desc bdw_power_wells[] = { > { > .name = "always-on", > .always_on = 1, > @@ -2165,7 +2168,7 @@ static const struct i915_power_well_ops > vlv_dpio_power_well_ops = { > .is_enabled = vlv_power_well_enabled, > }; > > -static struct i915_power_well vlv_power_wells[] = { > +static const struct i915_power_well_desc vlv_power_wells[] = { > { > .name = "always-on", > .always_on = 1, > @@ -2223,7 +2226,7 @@ static struct i915_power_well vlv_power_wells[] > = { > }, > }; > > -static struct i915_power_well chv_power_wells[] = { > +static const struct i915_power_well_desc chv_power_wells[] = { > { > .name = "always-on", > .always_on = 1, > @@ -2263,12 +2266,12 @@ bool > intel_display_power_well_is_enabled(struct drm_i915_private > *dev_priv, > bool ret; > > power_well = lookup_power_well(dev_priv, power_well_id); > - ret = power_well->ops->is_enabled(dev_priv, power_well); > + ret = power_well->desc->ops->is_enabled(dev_priv, > power_well); > > return ret; > } > > -static struct i915_power_well skl_power_wells[] = { > +static const struct i915_power_well_desc skl_power_wells[] = { > { > .name = "always-on", > .always_on = 1, > @@ -2336,7 +2339,7 @@ static struct i915_power_well skl_power_wells[] > = { > }, > }; > > -static struct i915_power_well bxt_power_wells[] = { > +static const struct i915_power_well_desc bxt_power_wells[] = { > { > .name = "always-on", > .always_on = 1, > @@ -2390,7 +2393,7 @@ static struct i915_power_well bxt_power_wells[] > = { > }, > }; > > -static struct i915_power_well glk_power_wells[] = { > +static const struct i915_power_well_desc glk_power_wells[] = { > { > .name = "always-on", > .always_on = 1, > @@ -2490,7 +2493,7 @@ static struct i915_power_well glk_power_wells[] > = { > }, > }; > > -static struct i915_power_well cnl_power_wells[] = { > +static const struct i915_power_well_desc cnl_power_wells[] = { > { > .name = "always-on", > .always_on = 1, > @@ -2594,7 +2597,7 @@ static const struct i915_power_well_ops > icl_combo_phy_aux_power_well_ops = { > .is_enabled = hsw_power_well_enabled, > }; > > -static struct i915_power_well icl_power_wells[] = { > +static const struct i915_power_well_desc icl_power_wells[] = { > { > .name = "always-on", > .always_on = 1, > @@ -2813,7 +2816,7 @@ static void assert_power_well_ids_unique(struct > drm_i915_private *dev_priv) > > power_well_ids = 0; > for (i = 0; i < power_domains->power_well_count; i++) { > - enum i915_power_well_id id = power_domains- > >power_wells[i].id; > + enum i915_power_well_id id = power_domains- > >power_wells[i].desc->id; > > WARN_ON(id >= sizeof(power_well_ids) * 8); > WARN_ON(power_well_ids & BIT_ULL(id)); > @@ -2821,10 +2824,28 @@ static void > assert_power_well_ids_unique(struct drm_i915_private *dev_priv) > } > } > > -#define set_power_wells(power_domains, __power_wells) ({ > \ > - (power_domains)->power_wells = (__power_wells); > \ > - (power_domains)->power_well_count = > ARRAY_SIZE(__power_wells); \ > -}) > +static int __set_power_wells(struct i915_power_domains > *power_domains, > + const struct i915_power_well_desc > *power_well_descs, > + int power_well_count) > +{ > + int i; > + > + power_domains->power_well_count = power_well_count; > + power_domains->power_wells = kcalloc(power_well_count, > + sizeof(*power_domains- > >power_wells), > + GFP_KERNEL); > + if (!power_domains->power_wells) > + return -ENOMEM; > + > + for (i = 0; i < power_well_count; i++) > + power_domains->power_wells[i].desc = > &power_well_descs[i]; > + > + return 0; > +} > + > +#define set_power_wells(power_domains, __power_well_descs) \ > + __set_power_wells(power_domains, __power_well_descs, \ > + ARRAY_SIZE(__power_well_descs)) > > /** > * intel_power_domains_init - initializes the power domain > structures > @@ -2836,6 +2857,7 @@ static void assert_power_well_ids_unique(struct > drm_i915_private *dev_priv) > int intel_power_domains_init(struct drm_i915_private *dev_priv) > { > struct i915_power_domains *power_domains = &dev_priv- > >power_domains; > + int err; > > i915_modparams.disable_power_well = > sanitize_disable_power_well_option(dev_priv, > @@ -2852,15 +2874,15 @@ int intel_power_domains_init(struct > drm_i915_private *dev_priv) > * the disabling order is reversed. > */ > if (IS_ICELAKE(dev_priv)) { > - set_power_wells(power_domains, icl_power_wells); > + err = set_power_wells(power_domains, > icl_power_wells); > } else if (IS_HASWELL(dev_priv)) { > - set_power_wells(power_domains, hsw_power_wells); > + err = set_power_wells(power_domains, > hsw_power_wells); > } else if (IS_BROADWELL(dev_priv)) { > - set_power_wells(power_domains, bdw_power_wells); > + err = set_power_wells(power_domains, > bdw_power_wells); > } else if (IS_GEN9_BC(dev_priv)) { > - set_power_wells(power_domains, skl_power_wells); > + err = set_power_wells(power_domains, > skl_power_wells); > } else if (IS_CANNONLAKE(dev_priv)) { > - set_power_wells(power_domains, cnl_power_wells); > + err = set_power_wells(power_domains, > cnl_power_wells); > > /* > * DDI and Aux IO are getting enabled for all ports > @@ -2872,22 +2894,34 @@ int intel_power_domains_init(struct > drm_i915_private *dev_priv) > power_domains->power_well_count -= 2; > > } else if (IS_BROXTON(dev_priv)) { > - set_power_wells(power_domains, bxt_power_wells); > + err = set_power_wells(power_domains, > bxt_power_wells); > } else if (IS_GEMINILAKE(dev_priv)) { > - set_power_wells(power_domains, glk_power_wells); > + err = set_power_wells(power_domains, > glk_power_wells); > } else if (IS_CHERRYVIEW(dev_priv)) { > - set_power_wells(power_domains, chv_power_wells); > + err = set_power_wells(power_domains, > chv_power_wells); > } else if (IS_VALLEYVIEW(dev_priv)) { > - set_power_wells(power_domains, vlv_power_wells); > + err = set_power_wells(power_domains, > vlv_power_wells); > } else if (IS_I830(dev_priv)) { > - set_power_wells(power_domains, i830_power_wells); > + err = set_power_wells(power_domains, > i830_power_wells); > } else { > - set_power_wells(power_domains, > i9xx_always_on_power_well); > + err = set_power_wells(power_domains, > i9xx_always_on_power_well); > } > > - assert_power_well_ids_unique(dev_priv); > + if (!err) > + assert_power_well_ids_unique(dev_priv); > > - return 0; > + return err; > +} > + > +/** > + * intel_power_domains_cleanup - clean up power domains resources > + * @dev_priv: i915 device instance > + * > + * Release any resources acquired by intel_power_domains_init() > + */ > +void intel_power_domains_cleanup(struct drm_i915_private *dev_priv) > +{ > + kfree(dev_priv->power_domains.power_wells); > } > > static void intel_power_domains_sync_hw(struct drm_i915_private > *dev_priv) > @@ -2897,9 +2931,9 @@ static void intel_power_domains_sync_hw(struct > drm_i915_private *dev_priv) > > mutex_lock(&power_domains->lock); > for_each_power_well(dev_priv, power_well) { > - power_well->ops->sync_hw(dev_priv, power_well); > - power_well->hw_enabled = power_well->ops- > >is_enabled(dev_priv, > - > power_well); > + power_well->desc->ops->sync_hw(dev_priv, > power_well); > + power_well->hw_enabled = > + power_well->desc->ops->is_enabled(dev_priv, > power_well); > } > mutex_unlock(&power_domains->lock); > } > @@ -3402,7 +3436,7 @@ static void chv_phy_control_init(struct > drm_i915_private *dev_priv) > * override and set the lane powerdown bits accding to the > * current lane status. > */ > - if (cmn_bc->ops->is_enabled(dev_priv, cmn_bc)) { > + if (cmn_bc->desc->ops->is_enabled(dev_priv, cmn_bc)) { > uint32_t status = I915_READ(DPLL(PIPE_A)); > unsigned int mask; > > @@ -3433,7 +3467,7 @@ static void chv_phy_control_init(struct > drm_i915_private *dev_priv) > dev_priv->chv_phy_assert[DPIO_PHY0] = true; > } > > - if (cmn_d->ops->is_enabled(dev_priv, cmn_d)) { > + if (cmn_d->desc->ops->is_enabled(dev_priv, cmn_d)) { > uint32_t status = I915_READ(DPIO_PHY_STATUS); > unsigned int mask; > > @@ -3469,15 +3503,15 @@ static void vlv_cmnlane_wa(struct > drm_i915_private *dev_priv) > lookup_power_well(dev_priv, > PUNIT_POWER_WELL_DISP2D); > > /* If the display might be already active skip this */ > - if (cmn->ops->is_enabled(dev_priv, cmn) && > - disp2d->ops->is_enabled(dev_priv, disp2d) && > + if (cmn->desc->ops->is_enabled(dev_priv, cmn) && > + disp2d->desc->ops->is_enabled(dev_priv, disp2d) && > I915_READ(DPIO_CTL) & DPIO_CMNRST) > return; > > DRM_DEBUG_KMS("toggling display PHY side reset\n"); > > /* cmnlane needs DPLL registers */ > - disp2d->ops->enable(dev_priv, disp2d); > + disp2d->desc->ops->enable(dev_priv, disp2d); > > /* > * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx: > @@ -3486,7 +3520,7 @@ static void vlv_cmnlane_wa(struct > drm_i915_private *dev_priv) > * Simply ungating isn't enough to reset the PHY enough to > get > * ports and lanes running. > */ > - cmn->ops->disable(dev_priv, cmn); > + cmn->desc->ops->disable(dev_priv, cmn); > } > > /** > @@ -3602,9 +3636,9 @@ static void > intel_power_domains_dump_info(struct drm_i915_private *dev_priv) > enum intel_display_power_domain domain; > > DRM_DEBUG_DRIVER("%-25s %d\n", > - power_well->name, power_well- > >count); > + power_well->desc->name, power_well- > >count); > > - for_each_power_domain(domain, power_well->domains) > + for_each_power_domain(domain, power_well->desc- > >domains) > DRM_DEBUG_DRIVER(" %-23s %d\n", > intel_display_power_domain_ > str(domain), > power_domains- > >domain_use_count[domain]); > @@ -3640,22 +3674,24 @@ void intel_power_domains_verify_state(struct > drm_i915_private *dev_priv) > * and PW1 power wells) are under FW control, so > ignore them, > * since their state can change asynchronously. > */ > - if (!power_well->domains) > + if (!power_well->desc->domains) > continue; > > - enabled = power_well->ops->is_enabled(dev_priv, > power_well); > - if ((power_well->count || power_well->always_on) != > enabled) > + enabled = power_well->desc->ops- > >is_enabled(dev_priv, > + power_we > ll); > + if ((power_well->count || power_well->desc- > >always_on) != enabled) > DRM_ERROR("power well %s state mismatch > (refcount %d/enabled %d)", > - power_well->name, power_well- > >count, enabled); > + power_well->desc->name, > + power_well->count, enabled); > > domains_count = 0; > - for_each_power_domain(domain, power_well->domains) > + for_each_power_domain(domain, power_well->desc- > >domains) > domains_count += power_domains- > >domain_use_count[domain]; > > if (power_well->count != domains_count) { > DRM_ERROR("power well %s refcount/domain > refcount mismatch " > "(refcount %d/domains refcount > %d)\n", > - power_well->name, power_well- > >count, > + power_well->desc->name, > power_well->count, > domains_count); > dump_domain_info = true; > }
On Wed, Aug 01, 2018 at 02:39:31PM -0700, Paulo Zanoni wrote: > Em Sex, 2018-07-20 às 17:14 +0300, Imre Deak escreveu: > > It makes sense to keep unchanging data const. Extract such fields > > from > > the i915_power_well struct into a new i915_power_well_desc struct > > that > > we initialize during compile time. For the rest of the dynamic > > fields allocate an array of i915_power_well objects in i915 dev_priv, > > and link to each of these objects their corresponding > > i915_power_well_desc object. > > > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Cc: Jani Nikula <jani.nikula@intel.com> > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > Quite a few issues pointed by checkpatch for this patch, please take a > look at them. > > More below: > > > --- > > drivers/gpu/drm/i915/i915_debugfs.c | 4 +- > > drivers/gpu/drm/i915/i915_drv.c | 8 +- > > drivers/gpu/drm/i915/i915_drv.h | 14 ++- > > drivers/gpu/drm/i915/intel_display.h | 4 +- > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > drivers/gpu/drm/i915/intel_hdcp.c | 6 +- > > drivers/gpu/drm/i915/intel_runtime_pm.c | 204 +++++++++++++++++++--- > > ---------- > > 7 files changed, 144 insertions(+), 97 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > b/drivers/gpu/drm/i915/i915_debugfs.c > > index b3aefd623557..eb284cac8fda 100644 > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > @@ -2833,10 +2833,10 @@ static int i915_power_domain_info(struct > > seq_file *m, void *unused) > > enum intel_display_power_domain power_domain; > > > > power_well = &power_domains->power_wells[i]; > > - seq_printf(m, "%-25s %d\n", power_well->name, > > + seq_printf(m, "%-25s %d\n", power_well->desc->name, > > power_well->count); > > > > - for_each_power_domain(power_domain, power_well- > > >domains) > > + for_each_power_domain(power_domain, power_well- > > >desc->domains) > > seq_printf(m, " %-23s %d\n", > > intel_display_power_domain_str(powe > > r_domain), > > power_domains- > > >domain_use_count[power_domain]); > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c > > index 3c984530fef9..5743db4500fb 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -922,7 +922,9 @@ static int i915_driver_init_early(struct > > drm_i915_private *dev_priv, > > intel_uc_init_early(dev_priv); > > intel_pm_setup(dev_priv); > > intel_init_dpio(dev_priv); > > - intel_power_domains_init(dev_priv); > > + ret = intel_power_domains_init(dev_priv); > > + if (ret < 0) > > + goto err_uc; > > intel_irq_init(dev_priv); > > intel_hangcheck_init(dev_priv); > > intel_init_display_hooks(dev_priv); > > @@ -934,6 +936,9 @@ static int i915_driver_init_early(struct > > drm_i915_private *dev_priv, > > > > return 0; > > > > +err_uc: > > + intel_uc_cleanup_early(dev_priv); > > Please leave the guc fixes for a different patch, regardless of how > innocent they look. Well, at least I didn't intend to fix guc. intel_uc_cleanup_early() is already called properly from i915_driver_cleanup_early(), not adding the call here would introduce a new problem if intel_power_domains_init() failed. > > Everything else looks good! > > Thanks, > Paulo > > > + i915_gem_cleanup_early(dev_priv); > > err_workqueues: > > i915_workqueues_cleanup(dev_priv); > > err_engines: > > @@ -948,6 +953,7 @@ static int i915_driver_init_early(struct > > drm_i915_private *dev_priv, > > static void i915_driver_cleanup_early(struct drm_i915_private > > *dev_priv) > > { > > intel_irq_fini(dev_priv); > > + intel_power_domains_cleanup(dev_priv); > > intel_uc_cleanup_early(dev_priv); > > i915_gem_cleanup_early(dev_priv); > > i915_workqueues_cleanup(dev_priv); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 4fb937399440..3ae200a9e8f1 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -862,13 +862,9 @@ struct i915_power_well_ops { > > }; > > > > /* Power well structure for haswell */ > > -struct i915_power_well { > > +struct i915_power_well_desc { > > const char *name; > > bool always_on; > > - /* power well enable/disable usage count */ > > - int count; > > - /* cached hw enabled state */ > > - bool hw_enabled; > > u64 domains; > > /* unique identifier for this power well */ > > enum i915_power_well_id id; > > @@ -891,6 +887,14 @@ struct i915_power_well { > > const struct i915_power_well_ops *ops; > > }; > > > > +struct i915_power_well { > > + const struct i915_power_well_desc *desc; > > + /* power well enable/disable usage count */ > > + int count; > > + /* cached hw enabled state */ > > + bool hw_enabled; > > +}; > > + > > struct i915_power_domains { > > /* > > * Power wells needed for initialization at driver init and > > suspend > > diff --git a/drivers/gpu/drm/i915/intel_display.h > > b/drivers/gpu/drm/i915/intel_display.h > > index 9292001cdd14..a626282d590b 100644 > > --- a/drivers/gpu/drm/i915/intel_display.h > > +++ b/drivers/gpu/drm/i915/intel_display.h > > @@ -322,11 +322,11 @@ struct intel_link_m_n { > > > > #define for_each_power_domain_well(__dev_priv, __power_well, > > __domain_mask) \ > > for_each_power_well(__dev_priv, __power_well) > > \ > > - for_each_if((__power_well)->domains & > > (__domain_mask)) > > + for_each_if((__power_well)->desc->domains & > > (__domain_mask)) > > > > #define for_each_power_domain_well_rev(__dev_priv, __power_well, > > __domain_mask) \ > > for_each_power_well_rev(__dev_priv, __power_well) > > \ > > - for_each_if((__power_well)->domains & > > (__domain_mask)) > > + for_each_if((__power_well)->desc->domains & > > (__domain_mask)) > > > > #define for_each_new_intel_plane_in_state(__state, plane, > > new_plane_state, __i) \ > > for ((__i) = 0; \ > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > b/drivers/gpu/drm/i915/intel_drv.h > > index 32be305c0e89..25f9b035cfe8 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1943,6 +1943,7 @@ int intel_psr_wait_for_idle(const struct > > intel_crtc_state *new_crtc_state); > > > > /* intel_runtime_pm.c */ > > int intel_power_domains_init(struct drm_i915_private *); > > +void intel_power_domains_cleanup(struct drm_i915_private *); > > void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, > > bool resume); > > void intel_power_domains_fini_hw(struct drm_i915_private *); > > void intel_power_domains_suspend(struct drm_i915_private *dev_priv); > > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c > > b/drivers/gpu/drm/i915/intel_hdcp.c > > index 0cc6a861bcf8..26e48fc95543 100644 > > --- a/drivers/gpu/drm/i915/intel_hdcp.c > > +++ b/drivers/gpu/drm/i915/intel_hdcp.c > > @@ -57,9 +57,9 @@ static bool hdcp_key_loadable(struct > > drm_i915_private *dev_priv) > > > > /* PG1 (power well #1) needs to be enabled */ > > for_each_power_well(dev_priv, power_well) { > > - if (power_well->id == id) { > > - enabled = power_well->ops- > > >is_enabled(dev_priv, > > - power_ > > well); > > + if (power_well->desc->id == id) { > > + enabled = power_well->desc->ops- > > >is_enabled(dev_priv, > > + > > power_well); > > break; > > } > > } > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index f119cbe4f61d..8b3c241bee55 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -159,17 +159,17 @@ intel_display_power_domain_str(enum > > intel_display_power_domain domain) > > static void intel_power_well_enable(struct drm_i915_private > > *dev_priv, > > struct i915_power_well > > *power_well) > > { > > - DRM_DEBUG_KMS("enabling %s\n", power_well->name); > > - power_well->ops->enable(dev_priv, power_well); > > + DRM_DEBUG_KMS("enabling %s\n", power_well->desc->name); > > + power_well->desc->ops->enable(dev_priv, power_well); > > power_well->hw_enabled = true; > > } > > > > static void intel_power_well_disable(struct drm_i915_private > > *dev_priv, > > struct i915_power_well > > *power_well) > > { > > - DRM_DEBUG_KMS("disabling %s\n", power_well->name); > > + DRM_DEBUG_KMS("disabling %s\n", power_well->desc->name); > > power_well->hw_enabled = false; > > - power_well->ops->disable(dev_priv, power_well); > > + power_well->desc->ops->disable(dev_priv, power_well); > > } > > > > static void intel_power_well_get(struct drm_i915_private *dev_priv, > > @@ -183,7 +183,7 @@ static void intel_power_well_put(struct > > drm_i915_private *dev_priv, > > struct i915_power_well *power_well) > > { > > WARN(!power_well->count, "Use count on power well %s is > > already zero", > > - power_well->name); > > + power_well->desc->name); > > > > if (!--power_well->count) > > intel_power_well_disable(dev_priv, power_well); > > @@ -213,7 +213,7 @@ bool __intel_display_power_is_enabled(struct > > drm_i915_private *dev_priv, > > is_enabled = true; > > > > for_each_power_domain_well_rev(dev_priv, power_well, > > BIT_ULL(domain)) { > > - if (power_well->always_on) > > + if (power_well->desc->always_on) > > continue; > > > > if (!power_well->hw_enabled) { > > @@ -323,7 +323,7 @@ static void hsw_power_well_pre_disable(struct > > drm_i915_private *dev_priv, > > static void hsw_wait_for_power_well_enable(struct drm_i915_private > > *dev_priv, > > struct i915_power_well > > *power_well) > > { > > - enum i915_power_well_id id = power_well->id; > > + enum i915_power_well_id id = power_well->desc->id; > > > > /* Timeout for PW1:10 us, AUX:not specified, other PWs:20 > > us. */ > > WARN_ON(intel_wait_for_register(dev_priv, > > @@ -350,7 +350,7 @@ static u32 hsw_power_well_requesters(struct > > drm_i915_private *dev_priv, > > static void hsw_wait_for_power_well_disable(struct drm_i915_private > > *dev_priv, > > struct i915_power_well > > *power_well) > > { > > - enum i915_power_well_id id = power_well->id; > > + enum i915_power_well_id id = power_well->desc->id; > > bool disabled; > > u32 reqs; > > > > @@ -370,7 +370,7 @@ static void > > hsw_wait_for_power_well_disable(struct drm_i915_private *dev_priv, > > return; > > > > DRM_DEBUG_KMS("%s forced on (bios:%d driver:%d kvmr:%d > > debug:%d)\n", > > - power_well->name, > > + power_well->desc->name, > > !!(reqs & 1), !!(reqs & 2), !!(reqs & 4), > > !!(reqs & 8)); > > } > > > > @@ -386,8 +386,8 @@ static void gen9_wait_for_power_well_fuses(struct > > drm_i915_private *dev_priv, > > static void hsw_power_well_enable(struct drm_i915_private *dev_priv, > > struct i915_power_well > > *power_well) > > { > > - enum i915_power_well_id id = power_well->id; > > - bool wait_fuses = power_well->hsw.has_fuses; > > + enum i915_power_well_id id = power_well->desc->id; > > + bool wait_fuses = power_well->desc->hsw.has_fuses; > > enum skl_power_gate uninitialized_var(pg); > > u32 val; > > > > @@ -421,17 +421,19 @@ static void hsw_power_well_enable(struct > > drm_i915_private *dev_priv, > > if (wait_fuses) > > gen9_wait_for_power_well_fuses(dev_priv, pg); > > > > - hsw_power_well_post_enable(dev_priv, power_well- > > >hsw.irq_pipe_mask, > > - power_well->hsw.has_vga); > > + hsw_power_well_post_enable(dev_priv, > > + power_well->desc- > > >hsw.irq_pipe_mask, > > + power_well->desc->hsw.has_vga); > > } > > > > static void hsw_power_well_disable(struct drm_i915_private > > *dev_priv, > > struct i915_power_well > > *power_well) > > { > > - enum i915_power_well_id id = power_well->id; > > + enum i915_power_well_id id = power_well->desc->id; > > u32 val; > > > > - hsw_power_well_pre_disable(dev_priv, power_well- > > >hsw.irq_pipe_mask); > > + hsw_power_well_pre_disable(dev_priv, > > + power_well->desc- > > >hsw.irq_pipe_mask); > > > > val = I915_READ(HSW_PWR_WELL_CTL_DRIVER(id)); > > I915_WRITE(HSW_PWR_WELL_CTL_DRIVER(id), > > @@ -445,7 +447,7 @@ static void > > icl_combo_phy_aux_power_well_enable(struct drm_i915_private > > *dev_priv, > > struct i915_power_well > > *power_well) > > { > > - enum i915_power_well_id id = power_well->id; > > + enum i915_power_well_id id = power_well->desc->id; > > enum port port = ICL_AUX_PW_TO_PORT(id); > > u32 val; > > > > @@ -462,7 +464,7 @@ static void > > icl_combo_phy_aux_power_well_disable(struct drm_i915_private > > *dev_priv, > > struct i915_power_well > > *power_well) > > { > > - enum i915_power_well_id id = power_well->id; > > + enum i915_power_well_id id = power_well->desc->id; > > enum port port = ICL_AUX_PW_TO_PORT(id); > > u32 val; > > > > @@ -484,7 +486,7 @@ icl_combo_phy_aux_power_well_disable(struct > > drm_i915_private *dev_priv, > > static bool hsw_power_well_enabled(struct drm_i915_private > > *dev_priv, > > struct i915_power_well > > *power_well) > > { > > - enum i915_power_well_id id = power_well->id; > > + enum i915_power_well_id id = power_well->desc->id; > > u32 mask = HSW_PWR_WELL_CTL_REQ(id) | > > HSW_PWR_WELL_CTL_STATE(id); > > > > return (I915_READ(HSW_PWR_WELL_CTL_DRIVER(id)) & mask) == > > mask; > > @@ -723,7 +725,7 @@ static void skl_enable_dc6(struct > > drm_i915_private *dev_priv) > > static void hsw_power_well_sync_hw(struct drm_i915_private > > *dev_priv, > > struct i915_power_well > > *power_well) > > { > > - enum i915_power_well_id id = power_well->id; > > + enum i915_power_well_id id = power_well->desc->id; > > u32 mask = HSW_PWR_WELL_CTL_REQ(id); > > u32 bios_req = I915_READ(HSW_PWR_WELL_CTL_BIOS(id)); > > > > @@ -740,19 +742,19 @@ static void hsw_power_well_sync_hw(struct > > drm_i915_private *dev_priv, > > static void bxt_dpio_cmn_power_well_enable(struct drm_i915_private > > *dev_priv, > > struct i915_power_well > > *power_well) > > { > > - bxt_ddi_phy_init(dev_priv, power_well->bxt.phy); > > + bxt_ddi_phy_init(dev_priv, power_well->desc->bxt.phy); > > } > > > > static void bxt_dpio_cmn_power_well_disable(struct drm_i915_private > > *dev_priv, > > struct i915_power_well > > *power_well) > > { > > - bxt_ddi_phy_uninit(dev_priv, power_well->bxt.phy); > > + bxt_ddi_phy_uninit(dev_priv, power_well->desc->bxt.phy); > > } > > > > static bool bxt_dpio_cmn_power_well_enabled(struct drm_i915_private > > *dev_priv, > > struct i915_power_well > > *power_well) > > { > > - return bxt_ddi_phy_is_enabled(dev_priv, power_well- > > >bxt.phy); > > + return bxt_ddi_phy_is_enabled(dev_priv, power_well->desc- > > >bxt.phy); > > } > > > > static void bxt_verify_ddi_phy_power_wells(struct drm_i915_private > > *dev_priv) > > @@ -761,16 +763,17 @@ static void > > bxt_verify_ddi_phy_power_wells(struct drm_i915_private *dev_priv) > > > > power_well = lookup_power_well(dev_priv, BXT_DPIO_CMN_A); > > if (power_well->count > 0) > > - bxt_ddi_phy_verify_state(dev_priv, power_well- > > >bxt.phy); > > + bxt_ddi_phy_verify_state(dev_priv, power_well->desc- > > >bxt.phy); > > > > power_well = lookup_power_well(dev_priv, BXT_DPIO_CMN_BC); > > if (power_well->count > 0) > > - bxt_ddi_phy_verify_state(dev_priv, power_well- > > >bxt.phy); > > + bxt_ddi_phy_verify_state(dev_priv, power_well->desc- > > >bxt.phy); > > > > if (IS_GEMINILAKE(dev_priv)) { > > power_well = lookup_power_well(dev_priv, > > GLK_DPIO_CMN_C); > > if (power_well->count > 0) > > - bxt_ddi_phy_verify_state(dev_priv, > > power_well->bxt.phy); > > + bxt_ddi_phy_verify_state(dev_priv, > > + power_well->desc- > > >bxt.phy); > > } > > } > > > > @@ -869,7 +872,7 @@ static void i830_pipes_power_well_sync_hw(struct > > drm_i915_private *dev_priv, > > static void vlv_set_power_well(struct drm_i915_private *dev_priv, > > struct i915_power_well *power_well, > > bool enable) > > { > > - enum i915_power_well_id power_well_id = power_well->id; > > + enum i915_power_well_id power_well_id = power_well->desc- > > >id; > > u32 mask; > > u32 state; > > u32 ctrl; > > @@ -917,7 +920,7 @@ static void vlv_power_well_disable(struct > > drm_i915_private *dev_priv, > > static bool vlv_power_well_enabled(struct drm_i915_private > > *dev_priv, > > struct i915_power_well > > *power_well) > > { > > - enum i915_power_well_id power_well_id = power_well->id; > > + enum i915_power_well_id power_well_id = power_well->desc- > > >id; > > bool enabled = false; > > u32 mask; > > u32 state; > > @@ -1107,7 +1110,7 @@ lookup_power_well(struct drm_i915_private > > *dev_priv, > > struct i915_power_well *power_well; > > > > power_well = &power_domains->power_wells[i]; > > - if (power_well->id == power_well_id) > > + if (power_well->desc->id == power_well_id) > > return power_well; > > } > > > > @@ -1146,7 +1149,7 @@ static void assert_chv_phy_status(struct > > drm_i915_private *dev_priv) > > PHY_STATUS_SPLINE_LDO(DPIO_PHY1 > > , DPIO_CH0, 0) | > > PHY_STATUS_SPLINE_LDO(DPIO_PHY1 > > , DPIO_CH0, 1)); > > > > - if (cmn_bc->ops->is_enabled(dev_priv, cmn_bc)) { > > + if (cmn_bc->desc->ops->is_enabled(dev_priv, cmn_bc)) { > > phy_status |= PHY_POWERGOOD(DPIO_PHY0); > > > > /* this assumes override is only used to enable > > lanes */ > > @@ -1187,7 +1190,7 @@ static void assert_chv_phy_status(struct > > drm_i915_private *dev_priv) > > phy_status |= > > PHY_STATUS_SPLINE_LDO(DPIO_PHY0, DPIO_CH1, 1); > > } > > > > - if (cmn_d->ops->is_enabled(dev_priv, cmn_d)) { > > + if (cmn_d->desc->ops->is_enabled(dev_priv, cmn_d)) { > > phy_status |= PHY_POWERGOOD(DPIO_PHY1); > > > > /* this assumes override is only used to enable > > lanes */ > > @@ -1231,10 +1234,10 @@ static void > > chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv, > > enum pipe pipe; > > uint32_t tmp; > > > > - WARN_ON_ONCE(power_well->id != PUNIT_POWER_WELL_DPIO_CMN_BC > > && > > - power_well->id != PUNIT_POWER_WELL_DPIO_CMN_D); > > + WARN_ON_ONCE(power_well->desc->id != > > PUNIT_POWER_WELL_DPIO_CMN_BC && > > + power_well->desc->id != > > PUNIT_POWER_WELL_DPIO_CMN_D); > > > > - if (power_well->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { > > + if (power_well->desc->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { > > pipe = PIPE_A; > > phy = DPIO_PHY0; > > } else { > > @@ -1262,7 +1265,7 @@ static void > > chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv, > > DPIO_SUS_CLK_CONFIG_GATE_CLKREQ; > > vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW28, tmp); > > > > - if (power_well->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { > > + if (power_well->desc->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { > > tmp = vlv_dpio_read(dev_priv, pipe, > > _CHV_CMN_DW6_CH1); > > tmp |= DPIO_DYNPWRDOWNEN_CH1; > > vlv_dpio_write(dev_priv, pipe, _CHV_CMN_DW6_CH1, > > tmp); > > @@ -1293,10 +1296,10 @@ static void > > chv_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv, > > { > > enum dpio_phy phy; > > > > - WARN_ON_ONCE(power_well->id != PUNIT_POWER_WELL_DPIO_CMN_BC > > && > > - power_well->id != PUNIT_POWER_WELL_DPIO_CMN_D); > > + WARN_ON_ONCE(power_well->desc->id != > > PUNIT_POWER_WELL_DPIO_CMN_BC && > > + power_well->desc->id != > > PUNIT_POWER_WELL_DPIO_CMN_D); > > > > - if (power_well->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { > > + if (power_well->desc->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { > > phy = DPIO_PHY0; > > assert_pll_disabled(dev_priv, PIPE_A); > > assert_pll_disabled(dev_priv, PIPE_B); > > @@ -2051,7 +2054,7 @@ static const struct i915_power_well_ops > > chv_dpio_cmn_power_well_ops = { > > .is_enabled = vlv_power_well_enabled, > > }; > > > > -static struct i915_power_well i9xx_always_on_power_well[] = { > > +static const struct i915_power_well_desc i9xx_always_on_power_well[] > > = { > > { > > .name = "always-on", > > .always_on = 1, > > @@ -2068,7 +2071,7 @@ static const struct i915_power_well_ops > > i830_pipes_power_well_ops = { > > .is_enabled = i830_pipes_power_well_enabled, > > }; > > > > -static struct i915_power_well i830_power_wells[] = { > > +static const struct i915_power_well_desc i830_power_wells[] = { > > { > > .name = "always-on", > > .always_on = 1, > > @@ -2105,7 +2108,7 @@ static const struct i915_power_well_ops > > bxt_dpio_cmn_power_well_ops = { > > .is_enabled = bxt_dpio_cmn_power_well_enabled, > > }; > > > > -static struct i915_power_well hsw_power_wells[] = { > > +static const struct i915_power_well_desc hsw_power_wells[] = { > > { > > .name = "always-on", > > .always_on = 1, > > @@ -2124,7 +2127,7 @@ static struct i915_power_well hsw_power_wells[] > > = { > > }, > > }; > > > > -static struct i915_power_well bdw_power_wells[] = { > > +static const struct i915_power_well_desc bdw_power_wells[] = { > > { > > .name = "always-on", > > .always_on = 1, > > @@ -2165,7 +2168,7 @@ static const struct i915_power_well_ops > > vlv_dpio_power_well_ops = { > > .is_enabled = vlv_power_well_enabled, > > }; > > > > -static struct i915_power_well vlv_power_wells[] = { > > +static const struct i915_power_well_desc vlv_power_wells[] = { > > { > > .name = "always-on", > > .always_on = 1, > > @@ -2223,7 +2226,7 @@ static struct i915_power_well vlv_power_wells[] > > = { > > }, > > }; > > > > -static struct i915_power_well chv_power_wells[] = { > > +static const struct i915_power_well_desc chv_power_wells[] = { > > { > > .name = "always-on", > > .always_on = 1, > > @@ -2263,12 +2266,12 @@ bool > > intel_display_power_well_is_enabled(struct drm_i915_private > > *dev_priv, > > bool ret; > > > > power_well = lookup_power_well(dev_priv, power_well_id); > > - ret = power_well->ops->is_enabled(dev_priv, power_well); > > + ret = power_well->desc->ops->is_enabled(dev_priv, > > power_well); > > > > return ret; > > } > > > > -static struct i915_power_well skl_power_wells[] = { > > +static const struct i915_power_well_desc skl_power_wells[] = { > > { > > .name = "always-on", > > .always_on = 1, > > @@ -2336,7 +2339,7 @@ static struct i915_power_well skl_power_wells[] > > = { > > }, > > }; > > > > -static struct i915_power_well bxt_power_wells[] = { > > +static const struct i915_power_well_desc bxt_power_wells[] = { > > { > > .name = "always-on", > > .always_on = 1, > > @@ -2390,7 +2393,7 @@ static struct i915_power_well bxt_power_wells[] > > = { > > }, > > }; > > > > -static struct i915_power_well glk_power_wells[] = { > > +static const struct i915_power_well_desc glk_power_wells[] = { > > { > > .name = "always-on", > > .always_on = 1, > > @@ -2490,7 +2493,7 @@ static struct i915_power_well glk_power_wells[] > > = { > > }, > > }; > > > > -static struct i915_power_well cnl_power_wells[] = { > > +static const struct i915_power_well_desc cnl_power_wells[] = { > > { > > .name = "always-on", > > .always_on = 1, > > @@ -2594,7 +2597,7 @@ static const struct i915_power_well_ops > > icl_combo_phy_aux_power_well_ops = { > > .is_enabled = hsw_power_well_enabled, > > }; > > > > -static struct i915_power_well icl_power_wells[] = { > > +static const struct i915_power_well_desc icl_power_wells[] = { > > { > > .name = "always-on", > > .always_on = 1, > > @@ -2813,7 +2816,7 @@ static void assert_power_well_ids_unique(struct > > drm_i915_private *dev_priv) > > > > power_well_ids = 0; > > for (i = 0; i < power_domains->power_well_count; i++) { > > - enum i915_power_well_id id = power_domains- > > >power_wells[i].id; > > + enum i915_power_well_id id = power_domains- > > >power_wells[i].desc->id; > > > > WARN_ON(id >= sizeof(power_well_ids) * 8); > > WARN_ON(power_well_ids & BIT_ULL(id)); > > @@ -2821,10 +2824,28 @@ static void > > assert_power_well_ids_unique(struct drm_i915_private *dev_priv) > > } > > } > > > > -#define set_power_wells(power_domains, __power_wells) ({ > > \ > > - (power_domains)->power_wells = (__power_wells); > > \ > > - (power_domains)->power_well_count = > > ARRAY_SIZE(__power_wells); \ > > -}) > > +static int __set_power_wells(struct i915_power_domains > > *power_domains, > > + const struct i915_power_well_desc > > *power_well_descs, > > + int power_well_count) > > +{ > > + int i; > > + > > + power_domains->power_well_count = power_well_count; > > + power_domains->power_wells = kcalloc(power_well_count, > > + sizeof(*power_domains- > > >power_wells), > > + GFP_KERNEL); > > + if (!power_domains->power_wells) > > + return -ENOMEM; > > + > > + for (i = 0; i < power_well_count; i++) > > + power_domains->power_wells[i].desc = > > &power_well_descs[i]; > > + > > + return 0; > > +} > > + > > +#define set_power_wells(power_domains, __power_well_descs) \ > > + __set_power_wells(power_domains, __power_well_descs, \ > > + ARRAY_SIZE(__power_well_descs)) > > > > /** > > * intel_power_domains_init - initializes the power domain > > structures > > @@ -2836,6 +2857,7 @@ static void assert_power_well_ids_unique(struct > > drm_i915_private *dev_priv) > > int intel_power_domains_init(struct drm_i915_private *dev_priv) > > { > > struct i915_power_domains *power_domains = &dev_priv- > > >power_domains; > > + int err; > > > > i915_modparams.disable_power_well = > > sanitize_disable_power_well_option(dev_priv, > > @@ -2852,15 +2874,15 @@ int intel_power_domains_init(struct > > drm_i915_private *dev_priv) > > * the disabling order is reversed. > > */ > > if (IS_ICELAKE(dev_priv)) { > > - set_power_wells(power_domains, icl_power_wells); > > + err = set_power_wells(power_domains, > > icl_power_wells); > > } else if (IS_HASWELL(dev_priv)) { > > - set_power_wells(power_domains, hsw_power_wells); > > + err = set_power_wells(power_domains, > > hsw_power_wells); > > } else if (IS_BROADWELL(dev_priv)) { > > - set_power_wells(power_domains, bdw_power_wells); > > + err = set_power_wells(power_domains, > > bdw_power_wells); > > } else if (IS_GEN9_BC(dev_priv)) { > > - set_power_wells(power_domains, skl_power_wells); > > + err = set_power_wells(power_domains, > > skl_power_wells); > > } else if (IS_CANNONLAKE(dev_priv)) { > > - set_power_wells(power_domains, cnl_power_wells); > > + err = set_power_wells(power_domains, > > cnl_power_wells); > > > > /* > > * DDI and Aux IO are getting enabled for all ports > > @@ -2872,22 +2894,34 @@ int intel_power_domains_init(struct > > drm_i915_private *dev_priv) > > power_domains->power_well_count -= 2; > > > > } else if (IS_BROXTON(dev_priv)) { > > - set_power_wells(power_domains, bxt_power_wells); > > + err = set_power_wells(power_domains, > > bxt_power_wells); > > } else if (IS_GEMINILAKE(dev_priv)) { > > - set_power_wells(power_domains, glk_power_wells); > > + err = set_power_wells(power_domains, > > glk_power_wells); > > } else if (IS_CHERRYVIEW(dev_priv)) { > > - set_power_wells(power_domains, chv_power_wells); > > + err = set_power_wells(power_domains, > > chv_power_wells); > > } else if (IS_VALLEYVIEW(dev_priv)) { > > - set_power_wells(power_domains, vlv_power_wells); > > + err = set_power_wells(power_domains, > > vlv_power_wells); > > } else if (IS_I830(dev_priv)) { > > - set_power_wells(power_domains, i830_power_wells); > > + err = set_power_wells(power_domains, > > i830_power_wells); > > } else { > > - set_power_wells(power_domains, > > i9xx_always_on_power_well); > > + err = set_power_wells(power_domains, > > i9xx_always_on_power_well); > > } > > > > - assert_power_well_ids_unique(dev_priv); > > + if (!err) > > + assert_power_well_ids_unique(dev_priv); > > > > - return 0; > > + return err; > > +} > > + > > +/** > > + * intel_power_domains_cleanup - clean up power domains resources > > + * @dev_priv: i915 device instance > > + * > > + * Release any resources acquired by intel_power_domains_init() > > + */ > > +void intel_power_domains_cleanup(struct drm_i915_private *dev_priv) > > +{ > > + kfree(dev_priv->power_domains.power_wells); > > } > > > > static void intel_power_domains_sync_hw(struct drm_i915_private > > *dev_priv) > > @@ -2897,9 +2931,9 @@ static void intel_power_domains_sync_hw(struct > > drm_i915_private *dev_priv) > > > > mutex_lock(&power_domains->lock); > > for_each_power_well(dev_priv, power_well) { > > - power_well->ops->sync_hw(dev_priv, power_well); > > - power_well->hw_enabled = power_well->ops- > > >is_enabled(dev_priv, > > - > > power_well); > > + power_well->desc->ops->sync_hw(dev_priv, > > power_well); > > + power_well->hw_enabled = > > + power_well->desc->ops->is_enabled(dev_priv, > > power_well); > > } > > mutex_unlock(&power_domains->lock); > > } > > @@ -3402,7 +3436,7 @@ static void chv_phy_control_init(struct > > drm_i915_private *dev_priv) > > * override and set the lane powerdown bits accding to the > > * current lane status. > > */ > > - if (cmn_bc->ops->is_enabled(dev_priv, cmn_bc)) { > > + if (cmn_bc->desc->ops->is_enabled(dev_priv, cmn_bc)) { > > uint32_t status = I915_READ(DPLL(PIPE_A)); > > unsigned int mask; > > > > @@ -3433,7 +3467,7 @@ static void chv_phy_control_init(struct > > drm_i915_private *dev_priv) > > dev_priv->chv_phy_assert[DPIO_PHY0] = true; > > } > > > > - if (cmn_d->ops->is_enabled(dev_priv, cmn_d)) { > > + if (cmn_d->desc->ops->is_enabled(dev_priv, cmn_d)) { > > uint32_t status = I915_READ(DPIO_PHY_STATUS); > > unsigned int mask; > > > > @@ -3469,15 +3503,15 @@ static void vlv_cmnlane_wa(struct > > drm_i915_private *dev_priv) > > lookup_power_well(dev_priv, > > PUNIT_POWER_WELL_DISP2D); > > > > /* If the display might be already active skip this */ > > - if (cmn->ops->is_enabled(dev_priv, cmn) && > > - disp2d->ops->is_enabled(dev_priv, disp2d) && > > + if (cmn->desc->ops->is_enabled(dev_priv, cmn) && > > + disp2d->desc->ops->is_enabled(dev_priv, disp2d) && > > I915_READ(DPIO_CTL) & DPIO_CMNRST) > > return; > > > > DRM_DEBUG_KMS("toggling display PHY side reset\n"); > > > > /* cmnlane needs DPLL registers */ > > - disp2d->ops->enable(dev_priv, disp2d); > > + disp2d->desc->ops->enable(dev_priv, disp2d); > > > > /* > > * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx: > > @@ -3486,7 +3520,7 @@ static void vlv_cmnlane_wa(struct > > drm_i915_private *dev_priv) > > * Simply ungating isn't enough to reset the PHY enough to > > get > > * ports and lanes running. > > */ > > - cmn->ops->disable(dev_priv, cmn); > > + cmn->desc->ops->disable(dev_priv, cmn); > > } > > > > /** > > @@ -3602,9 +3636,9 @@ static void > > intel_power_domains_dump_info(struct drm_i915_private *dev_priv) > > enum intel_display_power_domain domain; > > > > DRM_DEBUG_DRIVER("%-25s %d\n", > > - power_well->name, power_well- > > >count); > > + power_well->desc->name, power_well- > > >count); > > > > - for_each_power_domain(domain, power_well->domains) > > + for_each_power_domain(domain, power_well->desc- > > >domains) > > DRM_DEBUG_DRIVER(" %-23s %d\n", > > intel_display_power_domain_ > > str(domain), > > power_domains- > > >domain_use_count[domain]); > > @@ -3640,22 +3674,24 @@ void intel_power_domains_verify_state(struct > > drm_i915_private *dev_priv) > > * and PW1 power wells) are under FW control, so > > ignore them, > > * since their state can change asynchronously. > > */ > > - if (!power_well->domains) > > + if (!power_well->desc->domains) > > continue; > > > > - enabled = power_well->ops->is_enabled(dev_priv, > > power_well); > > - if ((power_well->count || power_well->always_on) != > > enabled) > > + enabled = power_well->desc->ops- > > >is_enabled(dev_priv, > > + power_we > > ll); > > + if ((power_well->count || power_well->desc- > > >always_on) != enabled) > > DRM_ERROR("power well %s state mismatch > > (refcount %d/enabled %d)", > > - power_well->name, power_well- > > >count, enabled); > > + power_well->desc->name, > > + power_well->count, enabled); > > > > domains_count = 0; > > - for_each_power_domain(domain, power_well->domains) > > + for_each_power_domain(domain, power_well->desc- > > >domains) > > domains_count += power_domains- > > >domain_use_count[domain]; > > > > if (power_well->count != domains_count) { > > DRM_ERROR("power well %s refcount/domain > > refcount mismatch " > > "(refcount %d/domains refcount > > %d)\n", > > - power_well->name, power_well- > > >count, > > + power_well->desc->name, > > power_well->count, > > domains_count); > > dump_domain_info = true; > > }
Em Qui, 2018-08-02 às 15:03 +0300, Imre Deak escreveu: > On Wed, Aug 01, 2018 at 02:39:31PM -0700, Paulo Zanoni wrote: > > Em Sex, 2018-07-20 às 17:14 +0300, Imre Deak escreveu: > > > It makes sense to keep unchanging data const. Extract such fields > > > from > > > the i915_power_well struct into a new i915_power_well_desc struct > > > that > > > we initialize during compile time. For the rest of the dynamic > > > fields allocate an array of i915_power_well objects in i915 > > > dev_priv, > > > and link to each of these objects their corresponding > > > i915_power_well_desc object. > > > > > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > Cc: Jani Nikula <jani.nikula@intel.com> > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > > > Quite a few issues pointed by checkpatch for this patch, please > > take a > > look at them. > > > > More below: > > > > > --- > > > drivers/gpu/drm/i915/i915_debugfs.c | 4 +- > > > drivers/gpu/drm/i915/i915_drv.c | 8 +- > > > drivers/gpu/drm/i915/i915_drv.h | 14 ++- > > > drivers/gpu/drm/i915/intel_display.h | 4 +- > > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > > drivers/gpu/drm/i915/intel_hdcp.c | 6 +- > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 204 > > > +++++++++++++++++++--- > > > ---------- > > > 7 files changed, 144 insertions(+), 97 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > > b/drivers/gpu/drm/i915/i915_debugfs.c > > > index b3aefd623557..eb284cac8fda 100644 > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > > @@ -2833,10 +2833,10 @@ static int i915_power_domain_info(struct > > > seq_file *m, void *unused) > > > enum intel_display_power_domain power_domain; > > > > > > power_well = &power_domains->power_wells[i]; > > > - seq_printf(m, "%-25s %d\n", power_well->name, > > > + seq_printf(m, "%-25s %d\n", power_well->desc- > > > >name, > > > power_well->count); > > > > > > - for_each_power_domain(power_domain, power_well- > > > > domains) > > > > > > + for_each_power_domain(power_domain, power_well- > > > > desc->domains) > > > > > > seq_printf(m, " %-23s %d\n", > > > intel_display_power_domain_str( > > > powe > > > r_domain), > > > power_domains- > > > > domain_use_count[power_domain]); > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > b/drivers/gpu/drm/i915/i915_drv.c > > > index 3c984530fef9..5743db4500fb 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -922,7 +922,9 @@ static int i915_driver_init_early(struct > > > drm_i915_private *dev_priv, > > > intel_uc_init_early(dev_priv); > > > intel_pm_setup(dev_priv); > > > intel_init_dpio(dev_priv); > > > - intel_power_domains_init(dev_priv); > > > + ret = intel_power_domains_init(dev_priv); > > > + if (ret < 0) > > > + goto err_uc; > > > intel_irq_init(dev_priv); > > > intel_hangcheck_init(dev_priv); > > > intel_init_display_hooks(dev_priv); > > > @@ -934,6 +936,9 @@ static int i915_driver_init_early(struct > > > drm_i915_private *dev_priv, > > > > > > return 0; > > > > > > +err_uc: > > > + intel_uc_cleanup_early(dev_priv); > > > > Please leave the guc fixes for a different patch, regardless of how > > innocent they look. > > Well, at least I didn't intend to fix guc. intel_uc_cleanup_early() > is > already called properly from i915_driver_cleanup_early(), not adding > the > call here would introduce a new problem if intel_power_domains_init() > failed. Ooops, I failed to realize we didn't have the guc cleanup call originally since there was no way to return non-zero after it. You're right. So with the checkpatch issues fixed: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > > Everything else looks good! > > > > Thanks, > > Paulo > > > > > + i915_gem_cleanup_early(dev_priv); > > > err_workqueues: > > > i915_workqueues_cleanup(dev_priv); > > > err_engines: > > > @@ -948,6 +953,7 @@ static int i915_driver_init_early(struct > > > drm_i915_private *dev_priv, > > > static void i915_driver_cleanup_early(struct drm_i915_private > > > *dev_priv) > > > { > > > intel_irq_fini(dev_priv); > > > + intel_power_domains_cleanup(dev_priv); > > > intel_uc_cleanup_early(dev_priv); > > > i915_gem_cleanup_early(dev_priv); > > > i915_workqueues_cleanup(dev_priv); > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > b/drivers/gpu/drm/i915/i915_drv.h > > > index 4fb937399440..3ae200a9e8f1 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -862,13 +862,9 @@ struct i915_power_well_ops { > > > }; > > > > > > /* Power well structure for haswell */ > > > -struct i915_power_well { > > > +struct i915_power_well_desc { > > > const char *name; > > > bool always_on; > > > - /* power well enable/disable usage count */ > > > - int count; > > > - /* cached hw enabled state */ > > > - bool hw_enabled; > > > u64 domains; > > > /* unique identifier for this power well */ > > > enum i915_power_well_id id; > > > @@ -891,6 +887,14 @@ struct i915_power_well { > > > const struct i915_power_well_ops *ops; > > > }; > > > > > > +struct i915_power_well { > > > + const struct i915_power_well_desc *desc; > > > + /* power well enable/disable usage count */ > > > + int count; > > > + /* cached hw enabled state */ > > > + bool hw_enabled; > > > +}; > > > + > > > struct i915_power_domains { > > > /* > > > * Power wells needed for initialization at driver init > > > and > > > suspend > > > diff --git a/drivers/gpu/drm/i915/intel_display.h > > > b/drivers/gpu/drm/i915/intel_display.h > > > index 9292001cdd14..a626282d590b 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.h > > > +++ b/drivers/gpu/drm/i915/intel_display.h > > > @@ -322,11 +322,11 @@ struct intel_link_m_n { > > > > > > #define for_each_power_domain_well(__dev_priv, __power_well, > > > __domain_mask) \ > > > for_each_power_well(__dev_priv, __power_well) > > > \ > > > - for_each_if((__power_well)->domains & > > > (__domain_mask)) > > > + for_each_if((__power_well)->desc->domains & > > > (__domain_mask)) > > > > > > #define for_each_power_domain_well_rev(__dev_priv, __power_well, > > > __domain_mask) \ > > > for_each_power_well_rev(__dev_priv, __power_well) > > > > > > \ > > > - for_each_if((__power_well)->domains & > > > (__domain_mask)) > > > + for_each_if((__power_well)->desc->domains & > > > (__domain_mask)) > > > > > > #define for_each_new_intel_plane_in_state(__state, plane, > > > new_plane_state, __i) \ > > > for ((__i) = 0; \ > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > > b/drivers/gpu/drm/i915/intel_drv.h > > > index 32be305c0e89..25f9b035cfe8 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -1943,6 +1943,7 @@ int intel_psr_wait_for_idle(const struct > > > intel_crtc_state *new_crtc_state); > > > > > > /* intel_runtime_pm.c */ > > > int intel_power_domains_init(struct drm_i915_private *); > > > +void intel_power_domains_cleanup(struct drm_i915_private *); > > > void intel_power_domains_init_hw(struct drm_i915_private > > > *dev_priv, > > > bool resume); > > > void intel_power_domains_fini_hw(struct drm_i915_private *); > > > void intel_power_domains_suspend(struct drm_i915_private > > > *dev_priv); > > > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c > > > b/drivers/gpu/drm/i915/intel_hdcp.c > > > index 0cc6a861bcf8..26e48fc95543 100644 > > > --- a/drivers/gpu/drm/i915/intel_hdcp.c > > > +++ b/drivers/gpu/drm/i915/intel_hdcp.c > > > @@ -57,9 +57,9 @@ static bool hdcp_key_loadable(struct > > > drm_i915_private *dev_priv) > > > > > > /* PG1 (power well #1) needs to be enabled */ > > > for_each_power_well(dev_priv, power_well) { > > > - if (power_well->id == id) { > > > - enabled = power_well->ops- > > > > is_enabled(dev_priv, > > > > > > - po > > > wer_ > > > well); > > > + if (power_well->desc->id == id) { > > > + enabled = power_well->desc->ops- > > > > is_enabled(dev_priv, > > > > > > + > > > > > > power_well); > > > break; > > > } > > > } > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > index f119cbe4f61d..8b3c241bee55 100644 > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > @@ -159,17 +159,17 @@ intel_display_power_domain_str(enum > > > intel_display_power_domain domain) > > > static void intel_power_well_enable(struct drm_i915_private > > > *dev_priv, > > > struct i915_power_well > > > *power_well) > > > { > > > - DRM_DEBUG_KMS("enabling %s\n", power_well->name); > > > - power_well->ops->enable(dev_priv, power_well); > > > + DRM_DEBUG_KMS("enabling %s\n", power_well->desc->name); > > > + power_well->desc->ops->enable(dev_priv, power_well); > > > power_well->hw_enabled = true; > > > } > > > > > > static void intel_power_well_disable(struct drm_i915_private > > > *dev_priv, > > > struct i915_power_well > > > *power_well) > > > { > > > - DRM_DEBUG_KMS("disabling %s\n", power_well->name); > > > + DRM_DEBUG_KMS("disabling %s\n", power_well->desc->name); > > > power_well->hw_enabled = false; > > > - power_well->ops->disable(dev_priv, power_well); > > > + power_well->desc->ops->disable(dev_priv, power_well); > > > } > > > > > > static void intel_power_well_get(struct drm_i915_private > > > *dev_priv, > > > @@ -183,7 +183,7 @@ static void intel_power_well_put(struct > > > drm_i915_private *dev_priv, > > > struct i915_power_well > > > *power_well) > > > { > > > WARN(!power_well->count, "Use count on power well %s is > > > already zero", > > > - power_well->name); > > > + power_well->desc->name); > > > > > > if (!--power_well->count) > > > intel_power_well_disable(dev_priv, power_well); > > > @@ -213,7 +213,7 @@ bool __intel_display_power_is_enabled(struct > > > drm_i915_private *dev_priv, > > > is_enabled = true; > > > > > > for_each_power_domain_well_rev(dev_priv, power_well, > > > BIT_ULL(domain)) { > > > - if (power_well->always_on) > > > + if (power_well->desc->always_on) > > > continue; > > > > > > if (!power_well->hw_enabled) { > > > @@ -323,7 +323,7 @@ static void hsw_power_well_pre_disable(struct > > > drm_i915_private *dev_priv, > > > static void hsw_wait_for_power_well_enable(struct > > > drm_i915_private > > > *dev_priv, > > > struct > > > i915_power_well > > > *power_well) > > > { > > > - enum i915_power_well_id id = power_well->id; > > > + enum i915_power_well_id id = power_well->desc->id; > > > > > > /* Timeout for PW1:10 us, AUX:not specified, other > > > PWs:20 > > > us. */ > > > WARN_ON(intel_wait_for_register(dev_priv, > > > @@ -350,7 +350,7 @@ static u32 hsw_power_well_requesters(struct > > > drm_i915_private *dev_priv, > > > static void hsw_wait_for_power_well_disable(struct > > > drm_i915_private > > > *dev_priv, > > > struct > > > i915_power_well > > > *power_well) > > > { > > > - enum i915_power_well_id id = power_well->id; > > > + enum i915_power_well_id id = power_well->desc->id; > > > bool disabled; > > > u32 reqs; > > > > > > @@ -370,7 +370,7 @@ static void > > > hsw_wait_for_power_well_disable(struct drm_i915_private > > > *dev_priv, > > > return; > > > > > > DRM_DEBUG_KMS("%s forced on (bios:%d driver:%d kvmr:%d > > > debug:%d)\n", > > > - power_well->name, > > > + power_well->desc->name, > > > !!(reqs & 1), !!(reqs & 2), !!(reqs & 4), > > > !!(reqs & 8)); > > > } > > > > > > @@ -386,8 +386,8 @@ static void > > > gen9_wait_for_power_well_fuses(struct > > > drm_i915_private *dev_priv, > > > static void hsw_power_well_enable(struct drm_i915_private > > > *dev_priv, > > > struct i915_power_well > > > *power_well) > > > { > > > - enum i915_power_well_id id = power_well->id; > > > - bool wait_fuses = power_well->hsw.has_fuses; > > > + enum i915_power_well_id id = power_well->desc->id; > > > + bool wait_fuses = power_well->desc->hsw.has_fuses; > > > enum skl_power_gate uninitialized_var(pg); > > > u32 val; > > > > > > @@ -421,17 +421,19 @@ static void hsw_power_well_enable(struct > > > drm_i915_private *dev_priv, > > > if (wait_fuses) > > > gen9_wait_for_power_well_fuses(dev_priv, pg); > > > > > > - hsw_power_well_post_enable(dev_priv, power_well- > > > > hsw.irq_pipe_mask, > > > > > > - power_well->hsw.has_vga); > > > + hsw_power_well_post_enable(dev_priv, > > > + power_well->desc- > > > > hsw.irq_pipe_mask, > > > > > > + power_well->desc- > > > >hsw.has_vga); > > > } > > > > > > static void hsw_power_well_disable(struct drm_i915_private > > > *dev_priv, > > > struct i915_power_well > > > *power_well) > > > { > > > - enum i915_power_well_id id = power_well->id; > > > + enum i915_power_well_id id = power_well->desc->id; > > > u32 val; > > > > > > - hsw_power_well_pre_disable(dev_priv, power_well- > > > > hsw.irq_pipe_mask); > > > > > > + hsw_power_well_pre_disable(dev_priv, > > > + power_well->desc- > > > > hsw.irq_pipe_mask); > > > > > > > > > val = I915_READ(HSW_PWR_WELL_CTL_DRIVER(id)); > > > I915_WRITE(HSW_PWR_WELL_CTL_DRIVER(id), > > > @@ -445,7 +447,7 @@ static void > > > icl_combo_phy_aux_power_well_enable(struct drm_i915_private > > > *dev_priv, > > > struct i915_power_well > > > *power_well) > > > { > > > - enum i915_power_well_id id = power_well->id; > > > + enum i915_power_well_id id = power_well->desc->id; > > > enum port port = ICL_AUX_PW_TO_PORT(id); > > > u32 val; > > > > > > @@ -462,7 +464,7 @@ static void > > > icl_combo_phy_aux_power_well_disable(struct drm_i915_private > > > *dev_priv, > > > struct i915_power_well > > > *power_well) > > > { > > > - enum i915_power_well_id id = power_well->id; > > > + enum i915_power_well_id id = power_well->desc->id; > > > enum port port = ICL_AUX_PW_TO_PORT(id); > > > u32 val; > > > > > > @@ -484,7 +486,7 @@ icl_combo_phy_aux_power_well_disable(struct > > > drm_i915_private *dev_priv, > > > static bool hsw_power_well_enabled(struct drm_i915_private > > > *dev_priv, > > > struct i915_power_well > > > *power_well) > > > { > > > - enum i915_power_well_id id = power_well->id; > > > + enum i915_power_well_id id = power_well->desc->id; > > > u32 mask = HSW_PWR_WELL_CTL_REQ(id) | > > > HSW_PWR_WELL_CTL_STATE(id); > > > > > > return (I915_READ(HSW_PWR_WELL_CTL_DRIVER(id)) & mask) > > > == > > > mask; > > > @@ -723,7 +725,7 @@ static void skl_enable_dc6(struct > > > drm_i915_private *dev_priv) > > > static void hsw_power_well_sync_hw(struct drm_i915_private > > > *dev_priv, > > > struct i915_power_well > > > *power_well) > > > { > > > - enum i915_power_well_id id = power_well->id; > > > + enum i915_power_well_id id = power_well->desc->id; > > > u32 mask = HSW_PWR_WELL_CTL_REQ(id); > > > u32 bios_req = I915_READ(HSW_PWR_WELL_CTL_BIOS(id)); > > > > > > @@ -740,19 +742,19 @@ static void hsw_power_well_sync_hw(struct > > > drm_i915_private *dev_priv, > > > static void bxt_dpio_cmn_power_well_enable(struct > > > drm_i915_private > > > *dev_priv, > > > struct > > > i915_power_well > > > *power_well) > > > { > > > - bxt_ddi_phy_init(dev_priv, power_well->bxt.phy); > > > + bxt_ddi_phy_init(dev_priv, power_well->desc->bxt.phy); > > > } > > > > > > static void bxt_dpio_cmn_power_well_disable(struct > > > drm_i915_private > > > *dev_priv, > > > struct > > > i915_power_well > > > *power_well) > > > { > > > - bxt_ddi_phy_uninit(dev_priv, power_well->bxt.phy); > > > + bxt_ddi_phy_uninit(dev_priv, power_well->desc->bxt.phy); > > > } > > > > > > static bool bxt_dpio_cmn_power_well_enabled(struct > > > drm_i915_private > > > *dev_priv, > > > struct > > > i915_power_well > > > *power_well) > > > { > > > - return bxt_ddi_phy_is_enabled(dev_priv, power_well- > > > > bxt.phy); > > > > > > + return bxt_ddi_phy_is_enabled(dev_priv, power_well- > > > >desc- > > > > bxt.phy); > > > > > > } > > > > > > static void bxt_verify_ddi_phy_power_wells(struct > > > drm_i915_private > > > *dev_priv) > > > @@ -761,16 +763,17 @@ static void > > > bxt_verify_ddi_phy_power_wells(struct drm_i915_private *dev_priv) > > > > > > power_well = lookup_power_well(dev_priv, > > > BXT_DPIO_CMN_A); > > > if (power_well->count > 0) > > > - bxt_ddi_phy_verify_state(dev_priv, power_well- > > > > bxt.phy); > > > > > > + bxt_ddi_phy_verify_state(dev_priv, power_well- > > > >desc- > > > > bxt.phy); > > > > > > > > > power_well = lookup_power_well(dev_priv, > > > BXT_DPIO_CMN_BC); > > > if (power_well->count > 0) > > > - bxt_ddi_phy_verify_state(dev_priv, power_well- > > > > bxt.phy); > > > > > > + bxt_ddi_phy_verify_state(dev_priv, power_well- > > > >desc- > > > > bxt.phy); > > > > > > > > > if (IS_GEMINILAKE(dev_priv)) { > > > power_well = lookup_power_well(dev_priv, > > > GLK_DPIO_CMN_C); > > > if (power_well->count > 0) > > > - bxt_ddi_phy_verify_state(dev_priv, > > > power_well->bxt.phy); > > > + bxt_ddi_phy_verify_state(dev_priv, > > > + power_well- > > > >desc- > > > > bxt.phy); > > > > > > } > > > } > > > > > > @@ -869,7 +872,7 @@ static void > > > i830_pipes_power_well_sync_hw(struct > > > drm_i915_private *dev_priv, > > > static void vlv_set_power_well(struct drm_i915_private > > > *dev_priv, > > > struct i915_power_well > > > *power_well, > > > bool enable) > > > { > > > - enum i915_power_well_id power_well_id = power_well->id; > > > + enum i915_power_well_id power_well_id = power_well- > > > >desc- > > > > id; > > > > > > u32 mask; > > > u32 state; > > > u32 ctrl; > > > @@ -917,7 +920,7 @@ static void vlv_power_well_disable(struct > > > drm_i915_private *dev_priv, > > > static bool vlv_power_well_enabled(struct drm_i915_private > > > *dev_priv, > > > struct i915_power_well > > > *power_well) > > > { > > > - enum i915_power_well_id power_well_id = power_well->id; > > > + enum i915_power_well_id power_well_id = power_well- > > > >desc- > > > > id; > > > > > > bool enabled = false; > > > u32 mask; > > > u32 state; > > > @@ -1107,7 +1110,7 @@ lookup_power_well(struct drm_i915_private > > > *dev_priv, > > > struct i915_power_well *power_well; > > > > > > power_well = &power_domains->power_wells[i]; > > > - if (power_well->id == power_well_id) > > > + if (power_well->desc->id == power_well_id) > > > return power_well; > > > } > > > > > > @@ -1146,7 +1149,7 @@ static void assert_chv_phy_status(struct > > > drm_i915_private *dev_priv) > > > PHY_STATUS_SPLINE_LDO(DPIO_ > > > PHY1 > > > , DPIO_CH0, 0) | > > > PHY_STATUS_SPLINE_LDO(DPIO_ > > > PHY1 > > > , DPIO_CH0, 1)); > > > > > > - if (cmn_bc->ops->is_enabled(dev_priv, cmn_bc)) { > > > + if (cmn_bc->desc->ops->is_enabled(dev_priv, cmn_bc)) { > > > phy_status |= PHY_POWERGOOD(DPIO_PHY0); > > > > > > /* this assumes override is only used to enable > > > lanes */ > > > @@ -1187,7 +1190,7 @@ static void assert_chv_phy_status(struct > > > drm_i915_private *dev_priv) > > > phy_status |= > > > PHY_STATUS_SPLINE_LDO(DPIO_PHY0, DPIO_CH1, 1); > > > } > > > > > > - if (cmn_d->ops->is_enabled(dev_priv, cmn_d)) { > > > + if (cmn_d->desc->ops->is_enabled(dev_priv, cmn_d)) { > > > phy_status |= PHY_POWERGOOD(DPIO_PHY1); > > > > > > /* this assumes override is only used to enable > > > lanes */ > > > @@ -1231,10 +1234,10 @@ static void > > > chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv, > > > enum pipe pipe; > > > uint32_t tmp; > > > > > > - WARN_ON_ONCE(power_well->id != > > > PUNIT_POWER_WELL_DPIO_CMN_BC > > > && > > > - power_well->id != > > > PUNIT_POWER_WELL_DPIO_CMN_D); > > > + WARN_ON_ONCE(power_well->desc->id != > > > PUNIT_POWER_WELL_DPIO_CMN_BC && > > > + power_well->desc->id != > > > PUNIT_POWER_WELL_DPIO_CMN_D); > > > > > > - if (power_well->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { > > > + if (power_well->desc->id == > > > PUNIT_POWER_WELL_DPIO_CMN_BC) { > > > pipe = PIPE_A; > > > phy = DPIO_PHY0; > > > } else { > > > @@ -1262,7 +1265,7 @@ static void > > > chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv, > > > DPIO_SUS_CLK_CONFIG_GATE_CLKREQ; > > > vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW28, tmp); > > > > > > - if (power_well->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { > > > + if (power_well->desc->id == > > > PUNIT_POWER_WELL_DPIO_CMN_BC) { > > > tmp = vlv_dpio_read(dev_priv, pipe, > > > _CHV_CMN_DW6_CH1); > > > tmp |= DPIO_DYNPWRDOWNEN_CH1; > > > vlv_dpio_write(dev_priv, pipe, _CHV_CMN_DW6_CH1, > > > tmp); > > > @@ -1293,10 +1296,10 @@ static void > > > chv_dpio_cmn_power_well_disable(struct drm_i915_private > > > *dev_priv, > > > { > > > enum dpio_phy phy; > > > > > > - WARN_ON_ONCE(power_well->id != > > > PUNIT_POWER_WELL_DPIO_CMN_BC > > > && > > > - power_well->id != > > > PUNIT_POWER_WELL_DPIO_CMN_D); > > > + WARN_ON_ONCE(power_well->desc->id != > > > PUNIT_POWER_WELL_DPIO_CMN_BC && > > > + power_well->desc->id != > > > PUNIT_POWER_WELL_DPIO_CMN_D); > > > > > > - if (power_well->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { > > > + if (power_well->desc->id == > > > PUNIT_POWER_WELL_DPIO_CMN_BC) { > > > phy = DPIO_PHY0; > > > assert_pll_disabled(dev_priv, PIPE_A); > > > assert_pll_disabled(dev_priv, PIPE_B); > > > @@ -2051,7 +2054,7 @@ static const struct i915_power_well_ops > > > chv_dpio_cmn_power_well_ops = { > > > .is_enabled = vlv_power_well_enabled, > > > }; > > > > > > -static struct i915_power_well i9xx_always_on_power_well[] = { > > > +static const struct i915_power_well_desc > > > i9xx_always_on_power_well[] > > > = { > > > { > > > .name = "always-on", > > > .always_on = 1, > > > @@ -2068,7 +2071,7 @@ static const struct i915_power_well_ops > > > i830_pipes_power_well_ops = { > > > .is_enabled = i830_pipes_power_well_enabled, > > > }; > > > > > > -static struct i915_power_well i830_power_wells[] = { > > > +static const struct i915_power_well_desc i830_power_wells[] = { > > > { > > > .name = "always-on", > > > .always_on = 1, > > > @@ -2105,7 +2108,7 @@ static const struct i915_power_well_ops > > > bxt_dpio_cmn_power_well_ops = { > > > .is_enabled = bxt_dpio_cmn_power_well_enabled, > > > }; > > > > > > -static struct i915_power_well hsw_power_wells[] = { > > > +static const struct i915_power_well_desc hsw_power_wells[] = { > > > { > > > .name = "always-on", > > > .always_on = 1, > > > @@ -2124,7 +2127,7 @@ static struct i915_power_well > > > hsw_power_wells[] > > > = { > > > }, > > > }; > > > > > > -static struct i915_power_well bdw_power_wells[] = { > > > +static const struct i915_power_well_desc bdw_power_wells[] = { > > > { > > > .name = "always-on", > > > .always_on = 1, > > > @@ -2165,7 +2168,7 @@ static const struct i915_power_well_ops > > > vlv_dpio_power_well_ops = { > > > .is_enabled = vlv_power_well_enabled, > > > }; > > > > > > -static struct i915_power_well vlv_power_wells[] = { > > > +static const struct i915_power_well_desc vlv_power_wells[] = { > > > { > > > .name = "always-on", > > > .always_on = 1, > > > @@ -2223,7 +2226,7 @@ static struct i915_power_well > > > vlv_power_wells[] > > > = { > > > }, > > > }; > > > > > > -static struct i915_power_well chv_power_wells[] = { > > > +static const struct i915_power_well_desc chv_power_wells[] = { > > > { > > > .name = "always-on", > > > .always_on = 1, > > > @@ -2263,12 +2266,12 @@ bool > > > intel_display_power_well_is_enabled(struct drm_i915_private > > > *dev_priv, > > > bool ret; > > > > > > power_well = lookup_power_well(dev_priv, power_well_id); > > > - ret = power_well->ops->is_enabled(dev_priv, power_well); > > > + ret = power_well->desc->ops->is_enabled(dev_priv, > > > power_well); > > > > > > return ret; > > > } > > > > > > -static struct i915_power_well skl_power_wells[] = { > > > +static const struct i915_power_well_desc skl_power_wells[] = { > > > { > > > .name = "always-on", > > > .always_on = 1, > > > @@ -2336,7 +2339,7 @@ static struct i915_power_well > > > skl_power_wells[] > > > = { > > > }, > > > }; > > > > > > -static struct i915_power_well bxt_power_wells[] = { > > > +static const struct i915_power_well_desc bxt_power_wells[] = { > > > { > > > .name = "always-on", > > > .always_on = 1, > > > @@ -2390,7 +2393,7 @@ static struct i915_power_well > > > bxt_power_wells[] > > > = { > > > }, > > > }; > > > > > > -static struct i915_power_well glk_power_wells[] = { > > > +static const struct i915_power_well_desc glk_power_wells[] = { > > > { > > > .name = "always-on", > > > .always_on = 1, > > > @@ -2490,7 +2493,7 @@ static struct i915_power_well > > > glk_power_wells[] > > > = { > > > }, > > > }; > > > > > > -static struct i915_power_well cnl_power_wells[] = { > > > +static const struct i915_power_well_desc cnl_power_wells[] = { > > > { > > > .name = "always-on", > > > .always_on = 1, > > > @@ -2594,7 +2597,7 @@ static const struct i915_power_well_ops > > > icl_combo_phy_aux_power_well_ops = { > > > .is_enabled = hsw_power_well_enabled, > > > }; > > > > > > -static struct i915_power_well icl_power_wells[] = { > > > +static const struct i915_power_well_desc icl_power_wells[] = { > > > { > > > .name = "always-on", > > > .always_on = 1, > > > @@ -2813,7 +2816,7 @@ static void > > > assert_power_well_ids_unique(struct > > > drm_i915_private *dev_priv) > > > > > > power_well_ids = 0; > > > for (i = 0; i < power_domains->power_well_count; i++) { > > > - enum i915_power_well_id id = power_domains- > > > > power_wells[i].id; > > > > > > + enum i915_power_well_id id = power_domains- > > > > power_wells[i].desc->id; > > > > > > > > > WARN_ON(id >= sizeof(power_well_ids) * 8); > > > WARN_ON(power_well_ids & BIT_ULL(id)); > > > @@ -2821,10 +2824,28 @@ static void > > > assert_power_well_ids_unique(struct drm_i915_private *dev_priv) > > > } > > > } > > > > > > -#define set_power_wells(power_domains, __power_wells) ({ > > > \ > > > - (power_domains)->power_wells = (__power_wells); > > > \ > > > - (power_domains)->power_well_count = > > > ARRAY_SIZE(__power_wells); \ > > > -}) > > > +static int __set_power_wells(struct i915_power_domains > > > *power_domains, > > > + const struct i915_power_well_desc > > > *power_well_descs, > > > + int power_well_count) > > > +{ > > > + int i; > > > + > > > + power_domains->power_well_count = power_well_count; > > > + power_domains->power_wells = kcalloc(power_well_count, > > > + sizeof(*power_domai > > > ns- > > > > power_wells), > > > > > > + GFP_KERNEL); > > > + if (!power_domains->power_wells) > > > + return -ENOMEM; > > > + > > > + for (i = 0; i < power_well_count; i++) > > > + power_domains->power_wells[i].desc = > > > &power_well_descs[i]; > > > + > > > + return 0; > > > +} > > > + > > > +#define set_power_wells(power_domains, __power_well_descs) \ > > > + __set_power_wells(power_domains, __power_well_descs, \ > > > + ARRAY_SIZE(__power_well_descs)) > > > > > > /** > > > * intel_power_domains_init - initializes the power domain > > > structures > > > @@ -2836,6 +2857,7 @@ static void > > > assert_power_well_ids_unique(struct > > > drm_i915_private *dev_priv) > > > int intel_power_domains_init(struct drm_i915_private *dev_priv) > > > { > > > struct i915_power_domains *power_domains = &dev_priv- > > > > power_domains; > > > > > > + int err; > > > > > > i915_modparams.disable_power_well = > > > sanitize_disable_power_well_option(dev_priv, > > > @@ -2852,15 +2874,15 @@ int intel_power_domains_init(struct > > > drm_i915_private *dev_priv) > > > * the disabling order is reversed. > > > */ > > > if (IS_ICELAKE(dev_priv)) { > > > - set_power_wells(power_domains, icl_power_wells); > > > + err = set_power_wells(power_domains, > > > icl_power_wells); > > > } else if (IS_HASWELL(dev_priv)) { > > > - set_power_wells(power_domains, hsw_power_wells); > > > + err = set_power_wells(power_domains, > > > hsw_power_wells); > > > } else if (IS_BROADWELL(dev_priv)) { > > > - set_power_wells(power_domains, bdw_power_wells); > > > + err = set_power_wells(power_domains, > > > bdw_power_wells); > > > } else if (IS_GEN9_BC(dev_priv)) { > > > - set_power_wells(power_domains, skl_power_wells); > > > + err = set_power_wells(power_domains, > > > skl_power_wells); > > > } else if (IS_CANNONLAKE(dev_priv)) { > > > - set_power_wells(power_domains, cnl_power_wells); > > > + err = set_power_wells(power_domains, > > > cnl_power_wells); > > > > > > /* > > > * DDI and Aux IO are getting enabled for all > > > ports > > > @@ -2872,22 +2894,34 @@ int intel_power_domains_init(struct > > > drm_i915_private *dev_priv) > > > power_domains->power_well_count -= 2; > > > > > > } else if (IS_BROXTON(dev_priv)) { > > > - set_power_wells(power_domains, bxt_power_wells); > > > + err = set_power_wells(power_domains, > > > bxt_power_wells); > > > } else if (IS_GEMINILAKE(dev_priv)) { > > > - set_power_wells(power_domains, glk_power_wells); > > > + err = set_power_wells(power_domains, > > > glk_power_wells); > > > } else if (IS_CHERRYVIEW(dev_priv)) { > > > - set_power_wells(power_domains, chv_power_wells); > > > + err = set_power_wells(power_domains, > > > chv_power_wells); > > > } else if (IS_VALLEYVIEW(dev_priv)) { > > > - set_power_wells(power_domains, vlv_power_wells); > > > + err = set_power_wells(power_domains, > > > vlv_power_wells); > > > } else if (IS_I830(dev_priv)) { > > > - set_power_wells(power_domains, > > > i830_power_wells); > > > + err = set_power_wells(power_domains, > > > i830_power_wells); > > > } else { > > > - set_power_wells(power_domains, > > > i9xx_always_on_power_well); > > > + err = set_power_wells(power_domains, > > > i9xx_always_on_power_well); > > > } > > > > > > - assert_power_well_ids_unique(dev_priv); > > > + if (!err) > > > + assert_power_well_ids_unique(dev_priv); > > > > > > - return 0; > > > + return err; > > > +} > > > + > > > +/** > > > + * intel_power_domains_cleanup - clean up power domains > > > resources > > > + * @dev_priv: i915 device instance > > > + * > > > + * Release any resources acquired by intel_power_domains_init() > > > + */ > > > +void intel_power_domains_cleanup(struct drm_i915_private > > > *dev_priv) > > > +{ > > > + kfree(dev_priv->power_domains.power_wells); > > > } > > > > > > static void intel_power_domains_sync_hw(struct drm_i915_private > > > *dev_priv) > > > @@ -2897,9 +2931,9 @@ static void > > > intel_power_domains_sync_hw(struct > > > drm_i915_private *dev_priv) > > > > > > mutex_lock(&power_domains->lock); > > > for_each_power_well(dev_priv, power_well) { > > > - power_well->ops->sync_hw(dev_priv, power_well); > > > - power_well->hw_enabled = power_well->ops- > > > > is_enabled(dev_priv, > > > > > > - > > > > > > power_well); > > > + power_well->desc->ops->sync_hw(dev_priv, > > > power_well); > > > + power_well->hw_enabled = > > > + power_well->desc->ops- > > > >is_enabled(dev_priv, > > > power_well); > > > } > > > mutex_unlock(&power_domains->lock); > > > } > > > @@ -3402,7 +3436,7 @@ static void chv_phy_control_init(struct > > > drm_i915_private *dev_priv) > > > * override and set the lane powerdown bits accding to > > > the > > > * current lane status. > > > */ > > > - if (cmn_bc->ops->is_enabled(dev_priv, cmn_bc)) { > > > + if (cmn_bc->desc->ops->is_enabled(dev_priv, cmn_bc)) { > > > uint32_t status = I915_READ(DPLL(PIPE_A)); > > > unsigned int mask; > > > > > > @@ -3433,7 +3467,7 @@ static void chv_phy_control_init(struct > > > drm_i915_private *dev_priv) > > > dev_priv->chv_phy_assert[DPIO_PHY0] = true; > > > } > > > > > > - if (cmn_d->ops->is_enabled(dev_priv, cmn_d)) { > > > + if (cmn_d->desc->ops->is_enabled(dev_priv, cmn_d)) { > > > uint32_t status = I915_READ(DPIO_PHY_STATUS); > > > unsigned int mask; > > > > > > @@ -3469,15 +3503,15 @@ static void vlv_cmnlane_wa(struct > > > drm_i915_private *dev_priv) > > > lookup_power_well(dev_priv, > > > PUNIT_POWER_WELL_DISP2D); > > > > > > /* If the display might be already active skip this */ > > > - if (cmn->ops->is_enabled(dev_priv, cmn) && > > > - disp2d->ops->is_enabled(dev_priv, disp2d) && > > > + if (cmn->desc->ops->is_enabled(dev_priv, cmn) && > > > + disp2d->desc->ops->is_enabled(dev_priv, disp2d) && > > > I915_READ(DPIO_CTL) & DPIO_CMNRST) > > > return; > > > > > > DRM_DEBUG_KMS("toggling display PHY side reset\n"); > > > > > > /* cmnlane needs DPLL registers */ > > > - disp2d->ops->enable(dev_priv, disp2d); > > > + disp2d->desc->ops->enable(dev_priv, disp2d); > > > > > > /* > > > * From > > > VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx: > > > @@ -3486,7 +3520,7 @@ static void vlv_cmnlane_wa(struct > > > drm_i915_private *dev_priv) > > > * Simply ungating isn't enough to reset the PHY enough > > > to > > > get > > > * ports and lanes running. > > > */ > > > - cmn->ops->disable(dev_priv, cmn); > > > + cmn->desc->ops->disable(dev_priv, cmn); > > > } > > > > > > /** > > > @@ -3602,9 +3636,9 @@ static void > > > intel_power_domains_dump_info(struct drm_i915_private *dev_priv) > > > enum intel_display_power_domain domain; > > > > > > DRM_DEBUG_DRIVER("%-25s %d\n", > > > - power_well->name, power_well- > > > > count); > > > > > > + power_well->desc->name, > > > power_well- > > > > count); > > > > > > > > > - for_each_power_domain(domain, power_well- > > > >domains) > > > + for_each_power_domain(domain, power_well->desc- > > > > domains) > > > > > > DRM_DEBUG_DRIVER(" %-23s %d\n", > > > intel_display_power_dom > > > ain_ > > > str(domain), > > > power_domains- > > > > domain_use_count[domain]); > > > > > > @@ -3640,22 +3674,24 @@ void > > > intel_power_domains_verify_state(struct > > > drm_i915_private *dev_priv) > > > * and PW1 power wells) are under FW control, so > > > ignore them, > > > * since their state can change asynchronously. > > > */ > > > - if (!power_well->domains) > > > + if (!power_well->desc->domains) > > > continue; > > > > > > - enabled = power_well->ops->is_enabled(dev_priv, > > > power_well); > > > - if ((power_well->count || power_well->always_on) > > > != > > > enabled) > > > + enabled = power_well->desc->ops- > > > > is_enabled(dev_priv, > > > > > > + powe > > > r_we > > > ll); > > > + if ((power_well->count || power_well->desc- > > > > always_on) != enabled) > > > > > > DRM_ERROR("power well %s state mismatch > > > (refcount %d/enabled %d)", > > > - power_well->name, power_well- > > > > count, enabled); > > > > > > + power_well->desc->name, > > > + power_well->count, enabled); > > > > > > domains_count = 0; > > > - for_each_power_domain(domain, power_well- > > > >domains) > > > + for_each_power_domain(domain, power_well->desc- > > > > domains) > > > > > > domains_count += power_domains- > > > > domain_use_count[domain]; > > > > > > > > > if (power_well->count != domains_count) { > > > DRM_ERROR("power well %s refcount/domain > > > refcount mismatch " > > > "(refcount %d/domains refcount > > > %d)\n", > > > - power_well->name, power_well- > > > > count, > > > > > > + power_well->desc->name, > > > power_well->count, > > > domains_count); > > > dump_domain_info = true; > > > }
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index b3aefd623557..eb284cac8fda 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2833,10 +2833,10 @@ static int i915_power_domain_info(struct seq_file *m, void *unused) enum intel_display_power_domain power_domain; power_well = &power_domains->power_wells[i]; - seq_printf(m, "%-25s %d\n", power_well->name, + seq_printf(m, "%-25s %d\n", power_well->desc->name, power_well->count); - for_each_power_domain(power_domain, power_well->domains) + for_each_power_domain(power_domain, power_well->desc->domains) seq_printf(m, " %-23s %d\n", intel_display_power_domain_str(power_domain), power_domains->domain_use_count[power_domain]); diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 3c984530fef9..5743db4500fb 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -922,7 +922,9 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv, intel_uc_init_early(dev_priv); intel_pm_setup(dev_priv); intel_init_dpio(dev_priv); - intel_power_domains_init(dev_priv); + ret = intel_power_domains_init(dev_priv); + if (ret < 0) + goto err_uc; intel_irq_init(dev_priv); intel_hangcheck_init(dev_priv); intel_init_display_hooks(dev_priv); @@ -934,6 +936,9 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv, return 0; +err_uc: + intel_uc_cleanup_early(dev_priv); + i915_gem_cleanup_early(dev_priv); err_workqueues: i915_workqueues_cleanup(dev_priv); err_engines: @@ -948,6 +953,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv, static void i915_driver_cleanup_early(struct drm_i915_private *dev_priv) { intel_irq_fini(dev_priv); + intel_power_domains_cleanup(dev_priv); intel_uc_cleanup_early(dev_priv); i915_gem_cleanup_early(dev_priv); i915_workqueues_cleanup(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4fb937399440..3ae200a9e8f1 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -862,13 +862,9 @@ struct i915_power_well_ops { }; /* Power well structure for haswell */ -struct i915_power_well { +struct i915_power_well_desc { const char *name; bool always_on; - /* power well enable/disable usage count */ - int count; - /* cached hw enabled state */ - bool hw_enabled; u64 domains; /* unique identifier for this power well */ enum i915_power_well_id id; @@ -891,6 +887,14 @@ struct i915_power_well { const struct i915_power_well_ops *ops; }; +struct i915_power_well { + const struct i915_power_well_desc *desc; + /* power well enable/disable usage count */ + int count; + /* cached hw enabled state */ + bool hw_enabled; +}; + struct i915_power_domains { /* * Power wells needed for initialization at driver init and suspend diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h index 9292001cdd14..a626282d590b 100644 --- a/drivers/gpu/drm/i915/intel_display.h +++ b/drivers/gpu/drm/i915/intel_display.h @@ -322,11 +322,11 @@ struct intel_link_m_n { #define for_each_power_domain_well(__dev_priv, __power_well, __domain_mask) \ for_each_power_well(__dev_priv, __power_well) \ - for_each_if((__power_well)->domains & (__domain_mask)) + for_each_if((__power_well)->desc->domains & (__domain_mask)) #define for_each_power_domain_well_rev(__dev_priv, __power_well, __domain_mask) \ for_each_power_well_rev(__dev_priv, __power_well) \ - for_each_if((__power_well)->domains & (__domain_mask)) + for_each_if((__power_well)->desc->domains & (__domain_mask)) #define for_each_new_intel_plane_in_state(__state, plane, new_plane_state, __i) \ for ((__i) = 0; \ diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 32be305c0e89..25f9b035cfe8 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1943,6 +1943,7 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state); /* intel_runtime_pm.c */ int intel_power_domains_init(struct drm_i915_private *); +void intel_power_domains_cleanup(struct drm_i915_private *); void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume); void intel_power_domains_fini_hw(struct drm_i915_private *); void intel_power_domains_suspend(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c index 0cc6a861bcf8..26e48fc95543 100644 --- a/drivers/gpu/drm/i915/intel_hdcp.c +++ b/drivers/gpu/drm/i915/intel_hdcp.c @@ -57,9 +57,9 @@ static bool hdcp_key_loadable(struct drm_i915_private *dev_priv) /* PG1 (power well #1) needs to be enabled */ for_each_power_well(dev_priv, power_well) { - if (power_well->id == id) { - enabled = power_well->ops->is_enabled(dev_priv, - power_well); + if (power_well->desc->id == id) { + enabled = power_well->desc->ops->is_enabled(dev_priv, + power_well); break; } } diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index f119cbe4f61d..8b3c241bee55 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -159,17 +159,17 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) static void intel_power_well_enable(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { - DRM_DEBUG_KMS("enabling %s\n", power_well->name); - power_well->ops->enable(dev_priv, power_well); + DRM_DEBUG_KMS("enabling %s\n", power_well->desc->name); + power_well->desc->ops->enable(dev_priv, power_well); power_well->hw_enabled = true; } static void intel_power_well_disable(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { - DRM_DEBUG_KMS("disabling %s\n", power_well->name); + DRM_DEBUG_KMS("disabling %s\n", power_well->desc->name); power_well->hw_enabled = false; - power_well->ops->disable(dev_priv, power_well); + power_well->desc->ops->disable(dev_priv, power_well); } static void intel_power_well_get(struct drm_i915_private *dev_priv, @@ -183,7 +183,7 @@ static void intel_power_well_put(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { WARN(!power_well->count, "Use count on power well %s is already zero", - power_well->name); + power_well->desc->name); if (!--power_well->count) intel_power_well_disable(dev_priv, power_well); @@ -213,7 +213,7 @@ bool __intel_display_power_is_enabled(struct drm_i915_private *dev_priv, is_enabled = true; for_each_power_domain_well_rev(dev_priv, power_well, BIT_ULL(domain)) { - if (power_well->always_on) + if (power_well->desc->always_on) continue; if (!power_well->hw_enabled) { @@ -323,7 +323,7 @@ static void hsw_power_well_pre_disable(struct drm_i915_private *dev_priv, static void hsw_wait_for_power_well_enable(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { - enum i915_power_well_id id = power_well->id; + enum i915_power_well_id id = power_well->desc->id; /* Timeout for PW1:10 us, AUX:not specified, other PWs:20 us. */ WARN_ON(intel_wait_for_register(dev_priv, @@ -350,7 +350,7 @@ static u32 hsw_power_well_requesters(struct drm_i915_private *dev_priv, static void hsw_wait_for_power_well_disable(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { - enum i915_power_well_id id = power_well->id; + enum i915_power_well_id id = power_well->desc->id; bool disabled; u32 reqs; @@ -370,7 +370,7 @@ static void hsw_wait_for_power_well_disable(struct drm_i915_private *dev_priv, return; DRM_DEBUG_KMS("%s forced on (bios:%d driver:%d kvmr:%d debug:%d)\n", - power_well->name, + power_well->desc->name, !!(reqs & 1), !!(reqs & 2), !!(reqs & 4), !!(reqs & 8)); } @@ -386,8 +386,8 @@ static void gen9_wait_for_power_well_fuses(struct drm_i915_private *dev_priv, static void hsw_power_well_enable(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { - enum i915_power_well_id id = power_well->id; - bool wait_fuses = power_well->hsw.has_fuses; + enum i915_power_well_id id = power_well->desc->id; + bool wait_fuses = power_well->desc->hsw.has_fuses; enum skl_power_gate uninitialized_var(pg); u32 val; @@ -421,17 +421,19 @@ static void hsw_power_well_enable(struct drm_i915_private *dev_priv, if (wait_fuses) gen9_wait_for_power_well_fuses(dev_priv, pg); - hsw_power_well_post_enable(dev_priv, power_well->hsw.irq_pipe_mask, - power_well->hsw.has_vga); + hsw_power_well_post_enable(dev_priv, + power_well->desc->hsw.irq_pipe_mask, + power_well->desc->hsw.has_vga); } static void hsw_power_well_disable(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { - enum i915_power_well_id id = power_well->id; + enum i915_power_well_id id = power_well->desc->id; u32 val; - hsw_power_well_pre_disable(dev_priv, power_well->hsw.irq_pipe_mask); + hsw_power_well_pre_disable(dev_priv, + power_well->desc->hsw.irq_pipe_mask); val = I915_READ(HSW_PWR_WELL_CTL_DRIVER(id)); I915_WRITE(HSW_PWR_WELL_CTL_DRIVER(id), @@ -445,7 +447,7 @@ static void icl_combo_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { - enum i915_power_well_id id = power_well->id; + enum i915_power_well_id id = power_well->desc->id; enum port port = ICL_AUX_PW_TO_PORT(id); u32 val; @@ -462,7 +464,7 @@ static void icl_combo_phy_aux_power_well_disable(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { - enum i915_power_well_id id = power_well->id; + enum i915_power_well_id id = power_well->desc->id; enum port port = ICL_AUX_PW_TO_PORT(id); u32 val; @@ -484,7 +486,7 @@ icl_combo_phy_aux_power_well_disable(struct drm_i915_private *dev_priv, static bool hsw_power_well_enabled(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { - enum i915_power_well_id id = power_well->id; + enum i915_power_well_id id = power_well->desc->id; u32 mask = HSW_PWR_WELL_CTL_REQ(id) | HSW_PWR_WELL_CTL_STATE(id); return (I915_READ(HSW_PWR_WELL_CTL_DRIVER(id)) & mask) == mask; @@ -723,7 +725,7 @@ static void skl_enable_dc6(struct drm_i915_private *dev_priv) static void hsw_power_well_sync_hw(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { - enum i915_power_well_id id = power_well->id; + enum i915_power_well_id id = power_well->desc->id; u32 mask = HSW_PWR_WELL_CTL_REQ(id); u32 bios_req = I915_READ(HSW_PWR_WELL_CTL_BIOS(id)); @@ -740,19 +742,19 @@ static void hsw_power_well_sync_hw(struct drm_i915_private *dev_priv, static void bxt_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { - bxt_ddi_phy_init(dev_priv, power_well->bxt.phy); + bxt_ddi_phy_init(dev_priv, power_well->desc->bxt.phy); } static void bxt_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { - bxt_ddi_phy_uninit(dev_priv, power_well->bxt.phy); + bxt_ddi_phy_uninit(dev_priv, power_well->desc->bxt.phy); } static bool bxt_dpio_cmn_power_well_enabled(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { - return bxt_ddi_phy_is_enabled(dev_priv, power_well->bxt.phy); + return bxt_ddi_phy_is_enabled(dev_priv, power_well->desc->bxt.phy); } static void bxt_verify_ddi_phy_power_wells(struct drm_i915_private *dev_priv) @@ -761,16 +763,17 @@ static void bxt_verify_ddi_phy_power_wells(struct drm_i915_private *dev_priv) power_well = lookup_power_well(dev_priv, BXT_DPIO_CMN_A); if (power_well->count > 0) - bxt_ddi_phy_verify_state(dev_priv, power_well->bxt.phy); + bxt_ddi_phy_verify_state(dev_priv, power_well->desc->bxt.phy); power_well = lookup_power_well(dev_priv, BXT_DPIO_CMN_BC); if (power_well->count > 0) - bxt_ddi_phy_verify_state(dev_priv, power_well->bxt.phy); + bxt_ddi_phy_verify_state(dev_priv, power_well->desc->bxt.phy); if (IS_GEMINILAKE(dev_priv)) { power_well = lookup_power_well(dev_priv, GLK_DPIO_CMN_C); if (power_well->count > 0) - bxt_ddi_phy_verify_state(dev_priv, power_well->bxt.phy); + bxt_ddi_phy_verify_state(dev_priv, + power_well->desc->bxt.phy); } } @@ -869,7 +872,7 @@ static void i830_pipes_power_well_sync_hw(struct drm_i915_private *dev_priv, static void vlv_set_power_well(struct drm_i915_private *dev_priv, struct i915_power_well *power_well, bool enable) { - enum i915_power_well_id power_well_id = power_well->id; + enum i915_power_well_id power_well_id = power_well->desc->id; u32 mask; u32 state; u32 ctrl; @@ -917,7 +920,7 @@ static void vlv_power_well_disable(struct drm_i915_private *dev_priv, static bool vlv_power_well_enabled(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { - enum i915_power_well_id power_well_id = power_well->id; + enum i915_power_well_id power_well_id = power_well->desc->id; bool enabled = false; u32 mask; u32 state; @@ -1107,7 +1110,7 @@ lookup_power_well(struct drm_i915_private *dev_priv, struct i915_power_well *power_well; power_well = &power_domains->power_wells[i]; - if (power_well->id == power_well_id) + if (power_well->desc->id == power_well_id) return power_well; } @@ -1146,7 +1149,7 @@ static void assert_chv_phy_status(struct drm_i915_private *dev_priv) PHY_STATUS_SPLINE_LDO(DPIO_PHY1, DPIO_CH0, 0) | PHY_STATUS_SPLINE_LDO(DPIO_PHY1, DPIO_CH0, 1)); - if (cmn_bc->ops->is_enabled(dev_priv, cmn_bc)) { + if (cmn_bc->desc->ops->is_enabled(dev_priv, cmn_bc)) { phy_status |= PHY_POWERGOOD(DPIO_PHY0); /* this assumes override is only used to enable lanes */ @@ -1187,7 +1190,7 @@ static void assert_chv_phy_status(struct drm_i915_private *dev_priv) phy_status |= PHY_STATUS_SPLINE_LDO(DPIO_PHY0, DPIO_CH1, 1); } - if (cmn_d->ops->is_enabled(dev_priv, cmn_d)) { + if (cmn_d->desc->ops->is_enabled(dev_priv, cmn_d)) { phy_status |= PHY_POWERGOOD(DPIO_PHY1); /* this assumes override is only used to enable lanes */ @@ -1231,10 +1234,10 @@ static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv, enum pipe pipe; uint32_t tmp; - WARN_ON_ONCE(power_well->id != PUNIT_POWER_WELL_DPIO_CMN_BC && - power_well->id != PUNIT_POWER_WELL_DPIO_CMN_D); + WARN_ON_ONCE(power_well->desc->id != PUNIT_POWER_WELL_DPIO_CMN_BC && + power_well->desc->id != PUNIT_POWER_WELL_DPIO_CMN_D); - if (power_well->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { + if (power_well->desc->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { pipe = PIPE_A; phy = DPIO_PHY0; } else { @@ -1262,7 +1265,7 @@ static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv, DPIO_SUS_CLK_CONFIG_GATE_CLKREQ; vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW28, tmp); - if (power_well->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { + if (power_well->desc->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { tmp = vlv_dpio_read(dev_priv, pipe, _CHV_CMN_DW6_CH1); tmp |= DPIO_DYNPWRDOWNEN_CH1; vlv_dpio_write(dev_priv, pipe, _CHV_CMN_DW6_CH1, tmp); @@ -1293,10 +1296,10 @@ static void chv_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv, { enum dpio_phy phy; - WARN_ON_ONCE(power_well->id != PUNIT_POWER_WELL_DPIO_CMN_BC && - power_well->id != PUNIT_POWER_WELL_DPIO_CMN_D); + WARN_ON_ONCE(power_well->desc->id != PUNIT_POWER_WELL_DPIO_CMN_BC && + power_well->desc->id != PUNIT_POWER_WELL_DPIO_CMN_D); - if (power_well->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { + if (power_well->desc->id == PUNIT_POWER_WELL_DPIO_CMN_BC) { phy = DPIO_PHY0; assert_pll_disabled(dev_priv, PIPE_A); assert_pll_disabled(dev_priv, PIPE_B); @@ -2051,7 +2054,7 @@ static const struct i915_power_well_ops chv_dpio_cmn_power_well_ops = { .is_enabled = vlv_power_well_enabled, }; -static struct i915_power_well i9xx_always_on_power_well[] = { +static const struct i915_power_well_desc i9xx_always_on_power_well[] = { { .name = "always-on", .always_on = 1, @@ -2068,7 +2071,7 @@ static const struct i915_power_well_ops i830_pipes_power_well_ops = { .is_enabled = i830_pipes_power_well_enabled, }; -static struct i915_power_well i830_power_wells[] = { +static const struct i915_power_well_desc i830_power_wells[] = { { .name = "always-on", .always_on = 1, @@ -2105,7 +2108,7 @@ static const struct i915_power_well_ops bxt_dpio_cmn_power_well_ops = { .is_enabled = bxt_dpio_cmn_power_well_enabled, }; -static struct i915_power_well hsw_power_wells[] = { +static const struct i915_power_well_desc hsw_power_wells[] = { { .name = "always-on", .always_on = 1, @@ -2124,7 +2127,7 @@ static struct i915_power_well hsw_power_wells[] = { }, }; -static struct i915_power_well bdw_power_wells[] = { +static const struct i915_power_well_desc bdw_power_wells[] = { { .name = "always-on", .always_on = 1, @@ -2165,7 +2168,7 @@ static const struct i915_power_well_ops vlv_dpio_power_well_ops = { .is_enabled = vlv_power_well_enabled, }; -static struct i915_power_well vlv_power_wells[] = { +static const struct i915_power_well_desc vlv_power_wells[] = { { .name = "always-on", .always_on = 1, @@ -2223,7 +2226,7 @@ static struct i915_power_well vlv_power_wells[] = { }, }; -static struct i915_power_well chv_power_wells[] = { +static const struct i915_power_well_desc chv_power_wells[] = { { .name = "always-on", .always_on = 1, @@ -2263,12 +2266,12 @@ bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv, bool ret; power_well = lookup_power_well(dev_priv, power_well_id); - ret = power_well->ops->is_enabled(dev_priv, power_well); + ret = power_well->desc->ops->is_enabled(dev_priv, power_well); return ret; } -static struct i915_power_well skl_power_wells[] = { +static const struct i915_power_well_desc skl_power_wells[] = { { .name = "always-on", .always_on = 1, @@ -2336,7 +2339,7 @@ static struct i915_power_well skl_power_wells[] = { }, }; -static struct i915_power_well bxt_power_wells[] = { +static const struct i915_power_well_desc bxt_power_wells[] = { { .name = "always-on", .always_on = 1, @@ -2390,7 +2393,7 @@ static struct i915_power_well bxt_power_wells[] = { }, }; -static struct i915_power_well glk_power_wells[] = { +static const struct i915_power_well_desc glk_power_wells[] = { { .name = "always-on", .always_on = 1, @@ -2490,7 +2493,7 @@ static struct i915_power_well glk_power_wells[] = { }, }; -static struct i915_power_well cnl_power_wells[] = { +static const struct i915_power_well_desc cnl_power_wells[] = { { .name = "always-on", .always_on = 1, @@ -2594,7 +2597,7 @@ static const struct i915_power_well_ops icl_combo_phy_aux_power_well_ops = { .is_enabled = hsw_power_well_enabled, }; -static struct i915_power_well icl_power_wells[] = { +static const struct i915_power_well_desc icl_power_wells[] = { { .name = "always-on", .always_on = 1, @@ -2813,7 +2816,7 @@ static void assert_power_well_ids_unique(struct drm_i915_private *dev_priv) power_well_ids = 0; for (i = 0; i < power_domains->power_well_count; i++) { - enum i915_power_well_id id = power_domains->power_wells[i].id; + enum i915_power_well_id id = power_domains->power_wells[i].desc->id; WARN_ON(id >= sizeof(power_well_ids) * 8); WARN_ON(power_well_ids & BIT_ULL(id)); @@ -2821,10 +2824,28 @@ static void assert_power_well_ids_unique(struct drm_i915_private *dev_priv) } } -#define set_power_wells(power_domains, __power_wells) ({ \ - (power_domains)->power_wells = (__power_wells); \ - (power_domains)->power_well_count = ARRAY_SIZE(__power_wells); \ -}) +static int __set_power_wells(struct i915_power_domains *power_domains, + const struct i915_power_well_desc *power_well_descs, + int power_well_count) +{ + int i; + + power_domains->power_well_count = power_well_count; + power_domains->power_wells = kcalloc(power_well_count, + sizeof(*power_domains->power_wells), + GFP_KERNEL); + if (!power_domains->power_wells) + return -ENOMEM; + + for (i = 0; i < power_well_count; i++) + power_domains->power_wells[i].desc = &power_well_descs[i]; + + return 0; +} + +#define set_power_wells(power_domains, __power_well_descs) \ + __set_power_wells(power_domains, __power_well_descs, \ + ARRAY_SIZE(__power_well_descs)) /** * intel_power_domains_init - initializes the power domain structures @@ -2836,6 +2857,7 @@ static void assert_power_well_ids_unique(struct drm_i915_private *dev_priv) int intel_power_domains_init(struct drm_i915_private *dev_priv) { struct i915_power_domains *power_domains = &dev_priv->power_domains; + int err; i915_modparams.disable_power_well = sanitize_disable_power_well_option(dev_priv, @@ -2852,15 +2874,15 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv) * the disabling order is reversed. */ if (IS_ICELAKE(dev_priv)) { - set_power_wells(power_domains, icl_power_wells); + err = set_power_wells(power_domains, icl_power_wells); } else if (IS_HASWELL(dev_priv)) { - set_power_wells(power_domains, hsw_power_wells); + err = set_power_wells(power_domains, hsw_power_wells); } else if (IS_BROADWELL(dev_priv)) { - set_power_wells(power_domains, bdw_power_wells); + err = set_power_wells(power_domains, bdw_power_wells); } else if (IS_GEN9_BC(dev_priv)) { - set_power_wells(power_domains, skl_power_wells); + err = set_power_wells(power_domains, skl_power_wells); } else if (IS_CANNONLAKE(dev_priv)) { - set_power_wells(power_domains, cnl_power_wells); + err = set_power_wells(power_domains, cnl_power_wells); /* * DDI and Aux IO are getting enabled for all ports @@ -2872,22 +2894,34 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv) power_domains->power_well_count -= 2; } else if (IS_BROXTON(dev_priv)) { - set_power_wells(power_domains, bxt_power_wells); + err = set_power_wells(power_domains, bxt_power_wells); } else if (IS_GEMINILAKE(dev_priv)) { - set_power_wells(power_domains, glk_power_wells); + err = set_power_wells(power_domains, glk_power_wells); } else if (IS_CHERRYVIEW(dev_priv)) { - set_power_wells(power_domains, chv_power_wells); + err = set_power_wells(power_domains, chv_power_wells); } else if (IS_VALLEYVIEW(dev_priv)) { - set_power_wells(power_domains, vlv_power_wells); + err = set_power_wells(power_domains, vlv_power_wells); } else if (IS_I830(dev_priv)) { - set_power_wells(power_domains, i830_power_wells); + err = set_power_wells(power_domains, i830_power_wells); } else { - set_power_wells(power_domains, i9xx_always_on_power_well); + err = set_power_wells(power_domains, i9xx_always_on_power_well); } - assert_power_well_ids_unique(dev_priv); + if (!err) + assert_power_well_ids_unique(dev_priv); - return 0; + return err; +} + +/** + * intel_power_domains_cleanup - clean up power domains resources + * @dev_priv: i915 device instance + * + * Release any resources acquired by intel_power_domains_init() + */ +void intel_power_domains_cleanup(struct drm_i915_private *dev_priv) +{ + kfree(dev_priv->power_domains.power_wells); } static void intel_power_domains_sync_hw(struct drm_i915_private *dev_priv) @@ -2897,9 +2931,9 @@ static void intel_power_domains_sync_hw(struct drm_i915_private *dev_priv) mutex_lock(&power_domains->lock); for_each_power_well(dev_priv, power_well) { - power_well->ops->sync_hw(dev_priv, power_well); - power_well->hw_enabled = power_well->ops->is_enabled(dev_priv, - power_well); + power_well->desc->ops->sync_hw(dev_priv, power_well); + power_well->hw_enabled = + power_well->desc->ops->is_enabled(dev_priv, power_well); } mutex_unlock(&power_domains->lock); } @@ -3402,7 +3436,7 @@ static void chv_phy_control_init(struct drm_i915_private *dev_priv) * override and set the lane powerdown bits accding to the * current lane status. */ - if (cmn_bc->ops->is_enabled(dev_priv, cmn_bc)) { + if (cmn_bc->desc->ops->is_enabled(dev_priv, cmn_bc)) { uint32_t status = I915_READ(DPLL(PIPE_A)); unsigned int mask; @@ -3433,7 +3467,7 @@ static void chv_phy_control_init(struct drm_i915_private *dev_priv) dev_priv->chv_phy_assert[DPIO_PHY0] = true; } - if (cmn_d->ops->is_enabled(dev_priv, cmn_d)) { + if (cmn_d->desc->ops->is_enabled(dev_priv, cmn_d)) { uint32_t status = I915_READ(DPIO_PHY_STATUS); unsigned int mask; @@ -3469,15 +3503,15 @@ static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv) lookup_power_well(dev_priv, PUNIT_POWER_WELL_DISP2D); /* If the display might be already active skip this */ - if (cmn->ops->is_enabled(dev_priv, cmn) && - disp2d->ops->is_enabled(dev_priv, disp2d) && + if (cmn->desc->ops->is_enabled(dev_priv, cmn) && + disp2d->desc->ops->is_enabled(dev_priv, disp2d) && I915_READ(DPIO_CTL) & DPIO_CMNRST) return; DRM_DEBUG_KMS("toggling display PHY side reset\n"); /* cmnlane needs DPLL registers */ - disp2d->ops->enable(dev_priv, disp2d); + disp2d->desc->ops->enable(dev_priv, disp2d); /* * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx: @@ -3486,7 +3520,7 @@ static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv) * Simply ungating isn't enough to reset the PHY enough to get * ports and lanes running. */ - cmn->ops->disable(dev_priv, cmn); + cmn->desc->ops->disable(dev_priv, cmn); } /** @@ -3602,9 +3636,9 @@ static void intel_power_domains_dump_info(struct drm_i915_private *dev_priv) enum intel_display_power_domain domain; DRM_DEBUG_DRIVER("%-25s %d\n", - power_well->name, power_well->count); + power_well->desc->name, power_well->count); - for_each_power_domain(domain, power_well->domains) + for_each_power_domain(domain, power_well->desc->domains) DRM_DEBUG_DRIVER(" %-23s %d\n", intel_display_power_domain_str(domain), power_domains->domain_use_count[domain]); @@ -3640,22 +3674,24 @@ void intel_power_domains_verify_state(struct drm_i915_private *dev_priv) * and PW1 power wells) are under FW control, so ignore them, * since their state can change asynchronously. */ - if (!power_well->domains) + if (!power_well->desc->domains) continue; - enabled = power_well->ops->is_enabled(dev_priv, power_well); - if ((power_well->count || power_well->always_on) != enabled) + enabled = power_well->desc->ops->is_enabled(dev_priv, + power_well); + if ((power_well->count || power_well->desc->always_on) != enabled) DRM_ERROR("power well %s state mismatch (refcount %d/enabled %d)", - power_well->name, power_well->count, enabled); + power_well->desc->name, + power_well->count, enabled); domains_count = 0; - for_each_power_domain(domain, power_well->domains) + for_each_power_domain(domain, power_well->desc->domains) domains_count += power_domains->domain_use_count[domain]; if (power_well->count != domains_count) { DRM_ERROR("power well %s refcount/domain refcount mismatch " "(refcount %d/domains refcount %d)\n", - power_well->name, power_well->count, + power_well->desc->name, power_well->count, domains_count); dump_domain_info = true; }
It makes sense to keep unchanging data const. Extract such fields from the i915_power_well struct into a new i915_power_well_desc struct that we initialize during compile time. For the rest of the dynamic fields allocate an array of i915_power_well objects in i915 dev_priv, and link to each of these objects their corresponding i915_power_well_desc object. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 4 +- drivers/gpu/drm/i915/i915_drv.c | 8 +- drivers/gpu/drm/i915/i915_drv.h | 14 ++- drivers/gpu/drm/i915/intel_display.h | 4 +- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_hdcp.c | 6 +- drivers/gpu/drm/i915/intel_runtime_pm.c | 204 +++++++++++++++++++------------- 7 files changed, 144 insertions(+), 97 deletions(-)