| Message ID | 20200324201429.29153-5-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/6] drm/i915/tc/tgl: Implement TCCOLD sequences | expand |
On Tue, Mar 24, 2020 at 01:14:28PM -0700, José Roberto de Souza wrote: > This is required for legacy/static TC ports as IOM is not aware of > the connection and will not trigger the TC cold exit. > > Just request PCODE to exit TCCOLD is not enough as it could enter > again be driver makes use of the port, to prevent it BSpec states that > aux powerwell should be held. > > So before detecting the mode, aux power is requested without wait for > hardware ack, PCODE is requested to exit TCCOLD and the TC detection > sequences follows as normal. > After detection if mode is not static aux can be powered off otherwise > we need to wait for HW ack as future calls to intel_display_power_get() > over aux will not check for HW ack. Based on the earlier comments tc_cold_off would be requested any time a legacy AUX power ref is acquired, so we wouldn't need a new no_ack power well interface. > > v2: > - fixed typo tc_lock_wakeref to tc_cold_wakeref in icl_tc_cold_request() > > v3: > - fixed non initialized ret in icl_tc_cold_request() > - added missing sleep step, initially it was not added because I > thought that the aux enable and then HW ack wait would take care of > that but that is not the case > > BSpec: 21750 > Fixes: https://gitlab.freedesktop.org/drm/intel/issues/1296 > Cc: Imre Deak <imre.deak@intel.com> > Cc: Cooper Chiou <cooper.chiou@intel.com> > Cc: Kai-Heng Feng <kai.heng.feng@canonical.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > .../drm/i915/display/intel_display_power.c | 30 +++++++++- > .../drm/i915/display/intel_display_types.h | 1 + > drivers/gpu/drm/i915/display/intel_tc.c | 60 +++++++++++++++++-- > drivers/gpu/drm/i915/i915_reg.h | 1 + > 4 files changed, 84 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > index a7e531b64e16..71a4c5d790ea 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > @@ -573,8 +573,9 @@ static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv, > #define TGL_AUX_PW_TO_TC_PORT(pw_idx) ((pw_idx) - TGL_PW_CTL_IDX_AUX_TC1) > > static void > -icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, > - struct i915_power_well *power_well) > +_icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, > + struct i915_power_well *power_well, > + bool wait_ack) > { > enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well); > u32 val; > @@ -587,7 +588,7 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, > val |= DP_AUX_CH_CTL_TBT_IO; > intel_de_write(dev_priv, DP_AUX_CH_CTL(aux_ch), val); > > - hsw_power_well_enable(dev_priv, power_well); > + _hsw_power_well_enable(dev_priv, power_well, wait_ack); > > if (INTEL_GEN(dev_priv) >= 12 && !power_well->desc->hsw.is_tc_tbt) { > enum tc_port tc_port; > @@ -603,6 +604,20 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, > } > } > > +static void > +icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, > + struct i915_power_well *power_well) > +{ > + _icl_tc_phy_aux_power_well_enable(dev_priv, power_well, true); > +} > + > +static void > +icl_tc_phy_aux_power_well_enable_without_ack(struct drm_i915_private *dev_priv, > + struct i915_power_well *power_well) > +{ > + _icl_tc_phy_aux_power_well_enable(dev_priv, power_well, false); > +} > + > static void > icl_tc_phy_aux_power_well_disable(struct drm_i915_private *dev_priv, > struct i915_power_well *power_well) > @@ -642,6 +657,13 @@ static bool hsw_power_well_enabled(struct drm_i915_private *dev_priv, > return (val & mask) == mask; > } > > +static void > +hsw_power_well_wait_ack(struct drm_i915_private *dev_priv, > + struct i915_power_well *power_well) > +{ > + hsw_wait_for_power_well_enable(dev_priv, power_well); > +} > + > static void assert_can_enable_dc9(struct drm_i915_private *dev_priv) > { > drm_WARN_ONCE(&dev_priv->drm, > @@ -3582,8 +3604,10 @@ static const struct i915_power_well_ops icl_combo_phy_aux_power_well_ops = { > static const struct i915_power_well_ops icl_tc_phy_aux_power_well_ops = { > .sync_hw = hsw_power_well_sync_hw, > .enable = icl_tc_phy_aux_power_well_enable, > + .enable_without_ack = icl_tc_phy_aux_power_well_enable_without_ack, > .disable = icl_tc_phy_aux_power_well_disable, > .is_enabled = hsw_power_well_enabled, > + .wait_enable_ack = hsw_power_well_wait_ack, > }; > > static const struct i915_power_well_regs icl_aux_power_well_regs = { > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > index 176ab5f1e867..42954be80435 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1397,6 +1397,7 @@ struct intel_digital_port { > enum tc_port_mode tc_mode; > enum phy_fia tc_phy_fia; > u8 tc_phy_fia_idx; > + intel_wakeref_t tc_cold_wakeref; > > void (*write_infoframe)(struct intel_encoder *encoder, > const struct intel_crtc_state *crtc_state, > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c > index e4c5de5ce874..588fca873b55 100644 > --- a/drivers/gpu/drm/i915/display/intel_tc.c > +++ b/drivers/gpu/drm/i915/display/intel_tc.c > @@ -416,9 +416,6 @@ static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port, > enum tc_port_mode old_tc_mode = dig_port->tc_mode; > > intel_display_power_flush_work(i915); > - drm_WARN_ON(&i915->drm, > - intel_display_power_is_enabled(i915, > - intel_aux_power_domain(dig_port))); The flush_work/warn is still needed, but we should do that before getting an AUX power ref around FIA accesses. > > icl_tc_phy_disconnect(dig_port); > icl_tc_phy_connect(dig_port, required_lanes); > @@ -528,6 +525,39 @@ static inline int tgl_tc_cold_request(struct intel_digital_port *dig_port, > return ret; > } > > +static inline int icl_tc_cold_request(struct intel_digital_port *dig_port, > + bool block) > +{ > + struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); > + enum intel_display_power_domain aux_domain; > + int ret; > + > + aux_domain = intel_aux_ch_to_power_domain(dig_port->aux_ch); > + > + if (block) { > + dig_port->tc_cold_wakeref = > + intel_display_power_get_without_ack(i915, aux_domain); > + > + do { > + ret = sandybridge_pcode_write_timeout(i915, > + ICL_PCODE_EXIT_TCCOLD, > + 0, 250, 1); > + > + } while (ret == -EAGAIN); > + > + if (!ret) > + msleep(1); > + } else if (dig_port->tc_mode == TC_PORT_LEGACY) { > + drm_WARN_ON(&i915->drm, !dig_port->tc_cold_wakeref); > + intel_display_power_put(i915, aux_domain, > + dig_port->tc_cold_wakeref); > + dig_port->tc_cold_wakeref = 0; > + ret = 0; > + } > + > + return ret; > +} > + > static int tc_cold_request(struct intel_digital_port *dig_port, bool block) > { > struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); > @@ -536,8 +566,7 @@ static int tc_cold_request(struct intel_digital_port *dig_port, bool block) > if (INTEL_GEN(i915) >= 12) > ret = tgl_tc_cold_request(dig_port, block); > else > - /* TODO: implement GEN11 TCCOLD sequences */ > - ret = 0; > + ret = icl_tc_cold_request(dig_port, block); > > drm_dbg_kms(&i915->drm, "Port %s: TCCOLD %sblock %s\n", > dig_port->tc_port_name, (block ? "" : "un"), > @@ -546,6 +575,26 @@ static int tc_cold_request(struct intel_digital_port *dig_port, bool block) > return ret; > } > > +static void tc_cold_after_reset_mode(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; > + > + if (INTEL_GEN(i915) >= 12) > + return; > + > + aux_domain = intel_aux_ch_to_power_domain(dig_port->aux_ch); > + > + if (dig_port->tc_mode == TC_PORT_LEGACY) { > + intel_display_power_wait_enable_ack(i915, aux_domain); > + } else { > + drm_WARN_ON(&i915->drm, !dig_port->tc_cold_wakeref); > + intel_display_power_put(i915, aux_domain, > + dig_port->tc_cold_wakeref); > + dig_port->tc_cold_wakeref = 0; > + } > +} > + > static void __intel_tc_port_lock(struct intel_digital_port *dig_port, > int required_lanes) > { > @@ -560,6 +609,7 @@ static void __intel_tc_port_lock(struct intel_digital_port *dig_port, > tc_cold_request(dig_port, true); > intel_tc_port_needs_reset(dig_port); > intel_tc_port_reset_mode(dig_port, required_lanes); > + tc_cold_after_reset_mode(dig_port); > } > > drm_WARN_ON(&i915->drm, dig_port->tc_lock_wakeref); > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 7e341d9945b3..8d4f40a70a4d 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -9017,6 +9017,7 @@ enum { > #define ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point) (((point) << 16) | (0x1 << 8)) > #define GEN6_PCODE_READ_D_COMP 0x10 > #define GEN6_PCODE_WRITE_D_COMP 0x11 > +#define ICL_PCODE_EXIT_TCCOLD 0x12 > #define HSW_PCODE_DE_WRITE_FREQ_REQ 0x17 > #define DISPLAY_IPS_CONTROL 0x19 > #define TGL_PCODE_TCCOLD 0x26 > -- > 2.26.0 >
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c index a7e531b64e16..71a4c5d790ea 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.c +++ b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -573,8 +573,9 @@ static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv, #define TGL_AUX_PW_TO_TC_PORT(pw_idx) ((pw_idx) - TGL_PW_CTL_IDX_AUX_TC1) static void -icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, - struct i915_power_well *power_well) +_icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, + struct i915_power_well *power_well, + bool wait_ack) { enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well); u32 val; @@ -587,7 +588,7 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, val |= DP_AUX_CH_CTL_TBT_IO; intel_de_write(dev_priv, DP_AUX_CH_CTL(aux_ch), val); - hsw_power_well_enable(dev_priv, power_well); + _hsw_power_well_enable(dev_priv, power_well, wait_ack); if (INTEL_GEN(dev_priv) >= 12 && !power_well->desc->hsw.is_tc_tbt) { enum tc_port tc_port; @@ -603,6 +604,20 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, } } +static void +icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, + struct i915_power_well *power_well) +{ + _icl_tc_phy_aux_power_well_enable(dev_priv, power_well, true); +} + +static void +icl_tc_phy_aux_power_well_enable_without_ack(struct drm_i915_private *dev_priv, + struct i915_power_well *power_well) +{ + _icl_tc_phy_aux_power_well_enable(dev_priv, power_well, false); +} + static void icl_tc_phy_aux_power_well_disable(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) @@ -642,6 +657,13 @@ static bool hsw_power_well_enabled(struct drm_i915_private *dev_priv, return (val & mask) == mask; } +static void +hsw_power_well_wait_ack(struct drm_i915_private *dev_priv, + struct i915_power_well *power_well) +{ + hsw_wait_for_power_well_enable(dev_priv, power_well); +} + static void assert_can_enable_dc9(struct drm_i915_private *dev_priv) { drm_WARN_ONCE(&dev_priv->drm, @@ -3582,8 +3604,10 @@ static const struct i915_power_well_ops icl_combo_phy_aux_power_well_ops = { static const struct i915_power_well_ops icl_tc_phy_aux_power_well_ops = { .sync_hw = hsw_power_well_sync_hw, .enable = icl_tc_phy_aux_power_well_enable, + .enable_without_ack = icl_tc_phy_aux_power_well_enable_without_ack, .disable = icl_tc_phy_aux_power_well_disable, .is_enabled = hsw_power_well_enabled, + .wait_enable_ack = hsw_power_well_wait_ack, }; static const struct i915_power_well_regs icl_aux_power_well_regs = { diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 176ab5f1e867..42954be80435 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1397,6 +1397,7 @@ struct intel_digital_port { enum tc_port_mode tc_mode; enum phy_fia tc_phy_fia; u8 tc_phy_fia_idx; + intel_wakeref_t tc_cold_wakeref; void (*write_infoframe)(struct intel_encoder *encoder, const struct intel_crtc_state *crtc_state, diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c index e4c5de5ce874..588fca873b55 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.c +++ b/drivers/gpu/drm/i915/display/intel_tc.c @@ -416,9 +416,6 @@ static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port, enum tc_port_mode old_tc_mode = dig_port->tc_mode; intel_display_power_flush_work(i915); - drm_WARN_ON(&i915->drm, - intel_display_power_is_enabled(i915, - intel_aux_power_domain(dig_port))); icl_tc_phy_disconnect(dig_port); icl_tc_phy_connect(dig_port, required_lanes); @@ -528,6 +525,39 @@ static inline int tgl_tc_cold_request(struct intel_digital_port *dig_port, return ret; } +static inline int icl_tc_cold_request(struct intel_digital_port *dig_port, + bool block) +{ + struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); + enum intel_display_power_domain aux_domain; + int ret; + + aux_domain = intel_aux_ch_to_power_domain(dig_port->aux_ch); + + if (block) { + dig_port->tc_cold_wakeref = + intel_display_power_get_without_ack(i915, aux_domain); + + do { + ret = sandybridge_pcode_write_timeout(i915, + ICL_PCODE_EXIT_TCCOLD, + 0, 250, 1); + + } while (ret == -EAGAIN); + + if (!ret) + msleep(1); + } else if (dig_port->tc_mode == TC_PORT_LEGACY) { + drm_WARN_ON(&i915->drm, !dig_port->tc_cold_wakeref); + intel_display_power_put(i915, aux_domain, + dig_port->tc_cold_wakeref); + dig_port->tc_cold_wakeref = 0; + ret = 0; + } + + return ret; +} + static int tc_cold_request(struct intel_digital_port *dig_port, bool block) { struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); @@ -536,8 +566,7 @@ static int tc_cold_request(struct intel_digital_port *dig_port, bool block) if (INTEL_GEN(i915) >= 12) ret = tgl_tc_cold_request(dig_port, block); else - /* TODO: implement GEN11 TCCOLD sequences */ - ret = 0; + ret = icl_tc_cold_request(dig_port, block); drm_dbg_kms(&i915->drm, "Port %s: TCCOLD %sblock %s\n", dig_port->tc_port_name, (block ? "" : "un"), @@ -546,6 +575,26 @@ static int tc_cold_request(struct intel_digital_port *dig_port, bool block) return ret; } +static void tc_cold_after_reset_mode(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; + + if (INTEL_GEN(i915) >= 12) + return; + + aux_domain = intel_aux_ch_to_power_domain(dig_port->aux_ch); + + if (dig_port->tc_mode == TC_PORT_LEGACY) { + intel_display_power_wait_enable_ack(i915, aux_domain); + } else { + drm_WARN_ON(&i915->drm, !dig_port->tc_cold_wakeref); + intel_display_power_put(i915, aux_domain, + dig_port->tc_cold_wakeref); + dig_port->tc_cold_wakeref = 0; + } +} + static void __intel_tc_port_lock(struct intel_digital_port *dig_port, int required_lanes) { @@ -560,6 +609,7 @@ static void __intel_tc_port_lock(struct intel_digital_port *dig_port, tc_cold_request(dig_port, true); intel_tc_port_needs_reset(dig_port); intel_tc_port_reset_mode(dig_port, required_lanes); + tc_cold_after_reset_mode(dig_port); } drm_WARN_ON(&i915->drm, dig_port->tc_lock_wakeref); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 7e341d9945b3..8d4f40a70a4d 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -9017,6 +9017,7 @@ enum { #define ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point) (((point) << 16) | (0x1 << 8)) #define GEN6_PCODE_READ_D_COMP 0x10 #define GEN6_PCODE_WRITE_D_COMP 0x11 +#define ICL_PCODE_EXIT_TCCOLD 0x12 #define HSW_PCODE_DE_WRITE_FREQ_REQ 0x17 #define DISPLAY_IPS_CONTROL 0x19 #define TGL_PCODE_TCCOLD 0x26
This is required for legacy/static TC ports as IOM is not aware of the connection and will not trigger the TC cold exit. Just request PCODE to exit TCCOLD is not enough as it could enter again be driver makes use of the port, to prevent it BSpec states that aux powerwell should be held. So before detecting the mode, aux power is requested without wait for hardware ack, PCODE is requested to exit TCCOLD and the TC detection sequences follows as normal. After detection if mode is not static aux can be powered off otherwise we need to wait for HW ack as future calls to intel_display_power_get() over aux will not check for HW ack. v2: - fixed typo tc_lock_wakeref to tc_cold_wakeref in icl_tc_cold_request() v3: - fixed non initialized ret in icl_tc_cold_request() - added missing sleep step, initially it was not added because I thought that the aux enable and then HW ack wait would take care of that but that is not the case BSpec: 21750 Fixes: https://gitlab.freedesktop.org/drm/intel/issues/1296 Cc: Imre Deak <imre.deak@intel.com> Cc: Cooper Chiou <cooper.chiou@intel.com> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- .../drm/i915/display/intel_display_power.c | 30 +++++++++- .../drm/i915/display/intel_display_types.h | 1 + drivers/gpu/drm/i915/display/intel_tc.c | 60 +++++++++++++++++-- drivers/gpu/drm/i915/i915_reg.h | 1 + 4 files changed, 84 insertions(+), 8 deletions(-)