On Thu, Apr 02, 2020 at 02:36:53AM +0300, Souza, Jose wrote: > On Wed, 2020-04-01 at 15:55 +0300, Imre Deak wrote: > > On Tue, Mar 31, 2020 at 05:41:20PM -0700, José Roberto de Souza > > wrote: > > > TC ports can enter in TCCOLD to save power and is required to > > > request > > > to PCODE to exit this state before use or read to TC registers. > > > > > > For TGL there is a new MBOX command to do that with a parameter to > > > ask > > > PCODE to exit and block TCCOLD entry or unblock TCCOLD entry. > > > > > > So adding a new power domain to reuse the refcount and only allow > > > TC cold when all TC ports are not in use. > > > > > > BSpec: 49294 > > > Cc: Imre Deak <imre.deak@xxxxxxxxx> > > > Cc: Cooper Chiou <cooper.chiou@xxxxxxxxx> > > > Cc: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> > > > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx> > > > --- > > > .../drm/i915/display/intel_display_power.c | 46 ++++++++++++++ > > > .../drm/i915/display/intel_display_power.h | 1 + > > > drivers/gpu/drm/i915/display/intel_tc.c | 63 > > > +++++++++++++++---- > > > drivers/gpu/drm/i915/display/intel_tc.h | 1 + > > > drivers/gpu/drm/i915/i915_reg.h | 3 + > > > 5 files changed, 103 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c > > > b/drivers/gpu/drm/i915/display/intel_display_power.c > > > index 1ccd57d645c7..5de115583146 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > > > @@ -2842,6 +2842,8 @@ void intel_display_power_put(struct > > > drm_i915_private *dev_priv, > > > #define TGL_AUX_I_TBT6_IO_POWER_DOMAINS ( \ > > > BIT_ULL(POWER_DOMAIN_AUX_I_TBT)) > > > > > > +#define TGL_TC_COLD_OFF (BIT_ULL(POWER_DOMAIN_TC_COLD_OFF)) > > > > TGL_TC_COLD_OFF_POWER_DOMAINS > > Okay > > > and should also include all the AUX power domains. > > So we would call intel_display_power_get() in intel_tc with aux domain > instead of POWER_DOMAIN_TC_COLD_OFF? No, but we also need to block tc-cold whenever getting an AUX power reference. > > > + > > > static const struct i915_power_well_ops > > > i9xx_always_on_power_well_ops = { > > > .sync_hw = i9xx_power_well_sync_hw_noop, > > > .enable = i9xx_always_on_power_well_noop, > > > @@ -3944,6 +3946,44 @@ static const struct i915_power_well_desc > > > ehl_power_wells[] = { > > > }, > > > }; > > > > > > +static void > > > +tgl_tc_cold_off_power_well_enable(struct drm_i915_private *i915, > > > + struct i915_power_well *power_well) > > > +{ > > > + intel_tc_tgl_tc_cold_request(i915, true); > > > +} > > > + > > > +static void > > > +tgl_tc_cold_off_power_well_disable(struct drm_i915_private *i915, > > > + struct i915_power_well *power_well) > > > +{ > > > + intel_tc_tgl_tc_cold_request(i915, false); > > > +} > > > + > > > +static void > > > +tgl_tc_cold_off_power_well_sync_hw(struct drm_i915_private *i915, > > > + struct i915_power_well *power_well) > > > +{ > > > + if (power_well->count > 0) > > > + tgl_tc_cold_off_power_well_enable(i915, power_well); > > > + else > > > + tgl_tc_cold_off_power_well_disable(i915, power_well); > > > +} > > > + > > > +static bool tgl_tc_cold_off_power_well_is_enabled(struct > > > drm_i915_private *dev_priv, > > > + struct > > > i915_power_well *power_well) > > > +{ > > > + /* There is no way to just read it from PCODE */ > > > + return false; > > > +} > > > + > > > +static const struct i915_power_well_ops tgl_tc_cold_off_ops = { > > > + .sync_hw = tgl_tc_cold_off_power_well_sync_hw, > > > + .enable = tgl_tc_cold_off_power_well_enable, > > > + .disable = tgl_tc_cold_off_power_well_disable, > > > + .is_enabled = tgl_tc_cold_off_power_well_is_enabled, > > > +}; > > > + > > > static const struct i915_power_well_desc tgl_power_wells[] = { > > > { > > > .name = "always-on", > > > @@ -4271,6 +4311,12 @@ static const struct i915_power_well_desc > > > tgl_power_wells[] = { > > > .hsw.irq_pipe_mask = BIT(PIPE_D), > > > }, > > > }, > > > + { > > > + .name = "TC cold off", > > > + .domains = POWER_DOMAIN_TC_COLD_OFF, > > > > TGL_TC_COLD_OFF_POWER_DOMAINS > > > > > + .ops = &tgl_tc_cold_off_ops, > > > + .id = DISP_PW_ID_NONE, > > > + }, > > > }; > > > > > > static int > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h > > > b/drivers/gpu/drm/i915/display/intel_display_power.h > > > index da64a5edae7a..070457e7b948 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > > > @@ -76,6 +76,7 @@ enum intel_display_power_domain { > > > POWER_DOMAIN_MODESET, > > > POWER_DOMAIN_GT_IRQ, > > > POWER_DOMAIN_DPLL_DC_OFF, > > > + POWER_DOMAIN_TC_COLD_OFF, > > > POWER_DOMAIN_INIT, > > > > > > POWER_DOMAIN_NUM, > > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c > > > b/drivers/gpu/drm/i915/display/intel_tc.c > > > index b6d67f069ef7..58f19037411a 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_tc.c > > > +++ b/drivers/gpu/drm/i915/display/intel_tc.c > > > @@ -507,11 +507,16 @@ static void __intel_tc_port_lock(struct > > > intel_digital_port *dig_port, > > > > > > mutex_lock(&dig_port->tc_lock); > > > > > > - if (INTEL_GEN(i915) == 11 && dig_port->tc_link_refcount == 0) { > > > - enum intel_display_power_domain aux_domain; > > > + if (dig_port->tc_link_refcount == 0) { > > > + enum intel_display_power_domain domain; > > > > > > - aux_domain = intel_aux_ch_to_power_domain(dig_port- > > > >aux_ch); > > > - dig_port->tc_cold_wakeref = > > > intel_display_power_get(i915, aux_domain); > > > + if (INTEL_GEN(i915) == 11) > > > + domain = intel_aux_ch_to_power_domain(dig_port- > > > >aux_ch); > > > + else > > > + domain = POWER_DOMAIN_TC_COLD_OFF; > > > + > > > + dig_port->tc_cold_wakeref = > > > intel_display_power_get(i915, > > > + dom > > > ain); > > > } > > > > > > if (!dig_port->tc_link_refcount && > > > @@ -527,18 +532,23 @@ void intel_tc_port_lock(struct > > > intel_digital_port *dig_port) > > > __intel_tc_port_lock(dig_port, 1); > > > } > > > > > > -static void icl_tc_cold_unblock(struct intel_digital_port > > > *dig_port) > > > +static void tc_cold_unblock(struct intel_digital_port *dig_port) > > > { > > > struct drm_i915_private *i915 = to_i915(dig_port- > > > >base.base.dev); > > > - enum intel_display_power_domain aux_domain; > > > + enum intel_display_power_domain domain; > > > intel_wakeref_t tc_cold_wakeref; > > > > > > - if (INTEL_GEN(i915) != 11 || dig_port->tc_link_refcount > 0) > > > + if (dig_port->tc_link_refcount > 0) > > > > You could drop the ref whenever wakeref passed to this function is > > not NULL. > > > > > return; > > > > > > tc_cold_wakeref = fetch_and_zero(&dig_port->tc_cold_wakeref); > > > - aux_domain = intel_aux_ch_to_power_domain(dig_port->aux_ch); > > > - intel_display_power_put_async(i915, aux_domain, > > > tc_cold_wakeref); > > > + > > > + if (INTEL_GEN(i915) == 11) > > > + domain = intel_aux_ch_to_power_domain(dig_port- > > > >aux_ch); > > > + else > > > + domain = POWER_DOMAIN_TC_COLD_OFF; > > > + > > > + intel_display_power_put_async(i915, domain, tc_cold_wakeref); > > > } > > > > > > void intel_tc_port_unlock(struct intel_digital_port *dig_port) > > > @@ -546,7 +556,7 @@ void intel_tc_port_unlock(struct > > > intel_digital_port *dig_port) > > > struct drm_i915_private *i915 = to_i915(dig_port- > > > >base.base.dev); > > > intel_wakeref_t wakeref = fetch_and_zero(&dig_port- > > > >tc_lock_wakeref); > > > > > > - icl_tc_cold_unblock(dig_port); > > > + tc_cold_unblock(dig_port); > > > > > > mutex_unlock(&dig_port->tc_lock); > > > > > > @@ -571,7 +581,7 @@ void intel_tc_port_put_link(struct > > > intel_digital_port *dig_port) > > > { > > > mutex_lock(&dig_port->tc_lock); > > > dig_port->tc_link_refcount--; > > > - icl_tc_cold_unblock(dig_port); > > > + tc_cold_unblock(dig_port); > > > mutex_unlock(&dig_port->tc_lock); > > > } > > > > > > @@ -611,3 +621,34 @@ void intel_tc_icl_tc_cold_exit(struct > > > drm_i915_private *i915) > > > drm_dbg_kms(&i915->drm, "TC cold block %s\n", > > > (ret == 0 ? "succeeded" : "failed")); > > > } > > > + > > > +void > > > +intel_tc_tgl_tc_cold_request(struct drm_i915_private *i915, bool > > > block) > > > +{ > > > + u32 low_val, high_val; > > > + u8 tries = 0; > > > + int ret; > > > + > > > + do { > > > + low_val = 0; > > > + high_val = block ? 0 : > > > TGL_PCODE_EXIT_TCCOLD_DATA_H_UNBLOCK_REQ; > > > + > > > + ret = sandybridge_pcode_read(i915, TGL_PCODE_TCCOLD, > > > &low_val, > > > + &high_val); > > > + if (ret == 0) { > > > + if (block && > > > + (low_val & > > > TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED)) > > > + ret = -EIO; > > > + else > > > + break; > > > + } > > > + > > > + if (ret != -EAGAIN) > > > + tries++; > > > + } while (tries < 3); > > > + > > > + if (ret) > > > + drm_dbg_kms(&i915->drm, "TC cold %sblock %s\n", > > > + (block ? "" : "un"), > > > + (ret == 0 ? "succeeded" : "failed")); > > > +} > > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.h > > > b/drivers/gpu/drm/i915/display/intel_tc.h > > > index 168d8896fcfd..8bb358cc8f15 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_tc.h > > > +++ b/drivers/gpu/drm/i915/display/intel_tc.h > > > @@ -31,5 +31,6 @@ void intel_tc_port_init(struct intel_digital_port > > > *dig_port, bool is_legacy); > > > > > > u32 intel_tc_port_live_status_mask(struct intel_digital_port > > > *dig_port); > > > void intel_tc_icl_tc_cold_exit(struct drm_i915_private *i915); > > > +void intel_tc_tgl_tc_cold_request(struct drm_i915_private *i915, > > > bool block); > > > > > > #endif /* __INTEL_TC_H__ */ > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > b/drivers/gpu/drm/i915/i915_reg.h > > > index b111815d6596..5548f3b56c0b 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -9110,6 +9110,9 @@ enum { > > > #define ICL_PCODE_EXIT_TCCOLD 0x12 > > > #define HSW_PCODE_DE_WRITE_FREQ_REQ 0x17 > > > #define DISPLAY_IPS_CONTROL 0x19 > > > +#define TGL_PCODE_TCCOLD 0x26 > > > +#define TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED REG_BIT(0) > > > +#define TGL_PCODE_EXIT_TCCOLD_DATA_H_UNBLOCK_REQ REG_BIT(0) > > > /* See also IPS_CTL */ > > > #define IPS_PCODE_CONTROL (1 << 30) > > > #define HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL 0x1A > > > -- > > > 2.26.0 > > > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx