diff mbox series

[v2,4/8] drm/i915/tc/icl: Implement TC cold sequences

Message ID 20200407011157.362092-4-jose.souza@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/8] drm/i915/display: Move out code to return the digital_port of the aux ch | expand

Commit Message

Souza, Jose April 7, 2020, 1:11 a.m. UTC 
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 before driver makes use of the port, to prevent it BSpec states
that aux powerwell should be held.

So here embedding the TC cold exit sequence into ICL aux enable,
it will enable aux and then request TC cold to exit.

The TC cold block(exit and aux hold) and unblock was added to some
exported TC functions for the others and to access PHY registers,
callers should enable and keep aux powerwell enabled during access.

Also adding TC cold check and warnig in tc_port_load_fia_params() as
at this point of the driver initialization we can't request power
wells, if we get this warning we will need to figure out how to handle
it.

v2:
- moved ICL TC cold exit function to intel_display_power
- using dig_port->tc_legacy_port to only execute sequences for legacy
ports, hopefully VBTs will have this right
- fixed check to call _hsw_power_well_continue_enable()
- calling _hsw_power_well_continue_enable() unconditionally in
icl_tc_phy_aux_power_well_enable(), if needed we will surpress timeout
warnings of TC legacy ports
- only blocking TC cold around fia access

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>

squash icl
---
 .../drm/i915/display/intel_display_power.c    | 22 ++++++-
 .../drm/i915/display/intel_display_types.h    |  1 +
 drivers/gpu/drm/i915/display/intel_tc.c       | 64 +++++++++++++++++--
 drivers/gpu/drm/i915/i915_reg.h               |  1 +
 4 files changed, 80 insertions(+), 8 deletions(-)

Comments

Imre Deak April 7, 2020, 3:42 p.m. UTC | #1 
On Mon, Apr 06, 2020 at 06:11:53PM -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 before driver makes use of the port, to prevent it BSpec states
> that aux powerwell should be held.
> 
> So here embedding the TC cold exit sequence into ICL aux enable,
> it will enable aux and then request TC cold to exit.
> 
> The TC cold block(exit and aux hold) and unblock was added to some
> exported TC functions for the others and to access PHY registers,
> callers should enable and keep aux powerwell enabled during access.
> 
> Also adding TC cold check and warnig in tc_port_load_fia_params() as
> at this point of the driver initialization we can't request power
> wells, if we get this warning we will need to figure out how to handle
> it.
> 
> v2:
> - moved ICL TC cold exit function to intel_display_power
> - using dig_port->tc_legacy_port to only execute sequences for legacy
> ports, hopefully VBTs will have this right
> - fixed check to call _hsw_power_well_continue_enable()
> - calling _hsw_power_well_continue_enable() unconditionally in
> icl_tc_phy_aux_power_well_enable(), if needed we will surpress timeout
> warnings of TC legacy ports
> - only blocking TC cold around fia access
> 
> 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>
> 
> squash icl

Leftover.

> ---
>  .../drm/i915/display/intel_display_power.c    | 22 ++++++-
>  .../drm/i915/display/intel_display_types.h    |  1 +
>  drivers/gpu/drm/i915/display/intel_tc.c       | 64 +++++++++++++++++--
>  drivers/gpu/drm/i915/i915_reg.h               |  1 +
>  4 files changed, 80 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 62e49f06d467..1336247743c4 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -575,6 +575,25 @@ 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_cold_exit(struct drm_i915_private *i915)
> +{
> +	int ret;
> +
> +	do {
> +		ret = sandybridge_pcode_write_timeout(i915,
> +						      ICL_PCODE_EXIT_TCCOLD,
> +						      0, 250, 1);
> +

Extra w/s.

> +	} while (ret == -EAGAIN);

Let's protect against an endless loop.

> +
> +	/* Spec states that TC cold exit can take up to 1ms to complete */
> +	if (!ret)
> +		msleep(1);
> +
> +	drm_dbg_kms(&i915->drm, "TC cold block %s\n", ret == 0 ? "succeeded" :
> +		    "failed");
> +}
> +
>  static void
>  icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
>  				 struct i915_power_well *power_well)
> @@ -593,7 +612,8 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
>  
>  	hsw_power_well_enable_prepare(dev_priv, power_well);
>  
> -	/* TODO ICL TC cold handling */
> +	if (INTEL_GEN(dev_priv) == 11 && dig_port->tc_legacy_port)
> +		icl_tc_cold_exit(dev_priv);
>  
>  	hsw_power_well_enable_complete(dev_priv, power_well);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 5a0adf14ebef..f7506ac40eb4 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1400,6 +1400,7 @@ struct intel_digital_port {
>  	enum intel_display_power_domain ddi_io_power_domain;
>  	struct mutex tc_lock;	/* protects the TypeC port mode */
>  	intel_wakeref_t tc_lock_wakeref;
> +	intel_wakeref_t tc_cold_wakeref;

Not needed any more.

>  	int tc_link_refcount;
>  	bool tc_legacy_port:1;
>  	char tc_port_name[8];
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> index 9b850c11aa78..7564259d677e 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.c
> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> @@ -34,6 +34,7 @@ tc_port_load_fia_params(struct drm_i915_private *i915,
>  	if (INTEL_INFO(i915)->display.has_modular_fia) {
>  		modular_fia = intel_uncore_read(&i915->uncore,
>  						PORT_TX_DFLEXDPSP(FIA1));
> +		drm_WARN_ON(&i915->drm, modular_fia == 0xffffffff);
>  		modular_fia &= MODULAR_FIA_MASK;
>  	} else {
>  		modular_fia = 0;
> @@ -52,6 +53,37 @@ tc_port_load_fia_params(struct drm_i915_private *i915,
>  	}
>  }
>  
> +static intel_wakeref_t
> +tc_cold_block(struct intel_digital_port *dig_port)
> +{
> +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> +	enum intel_display_power_domain domain;
> +
> +	if (INTEL_GEN(i915) != 11 || !dig_port->tc_legacy_port)
> +		return 0;
> +
> +	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
> +	return intel_display_power_get(i915, domain);
> +}
> +
> +static void
> +tc_cold_unblock(struct intel_digital_port *dig_port, intel_wakeref_t wakeref)
> +{
> +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> +	enum intel_display_power_domain domain;
> +
> +	/*
> +	 * wakeref == -1, means some error happened saving save_depot_stack but
> +	 * power should still be put down and 0 is a invalid save_depot_stack
> +	 * id so can be used to skip it for non TC legacy ports.
> +	 */
> +	if (wakeref == 0)
> +		return;
> +
> +	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
> +	intel_display_power_put_async(i915, domain, wakeref);
> +}
> +
>  u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
>  {
>  	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> @@ -415,9 +447,14 @@ 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)));
> +	if (INTEL_GEN(i915) != 11 || !dig_port->tc_legacy_port) {
> +		enum intel_display_power_domain aux_domain;
> +		bool aux_powered;
> +
> +		aux_domain = intel_aux_power_domain(dig_port);
> +		aux_powered = intel_display_power_is_enabled(i915, aux_domain);
> +		drm_WARN_ON(&i915->drm, aux_powered);
> +	}
>  
>  	icl_tc_phy_disconnect(dig_port);
>  	icl_tc_phy_connect(dig_port, required_lanes);
> @@ -439,9 +476,11 @@ intel_tc_port_link_init_refcount(struct intel_digital_port *dig_port,
>  void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
>  {
>  	struct intel_encoder *encoder = &dig_port->base;
> +	intel_wakeref_t tc_cold_wref;
>  	int active_links = 0;
>  
>  	mutex_lock(&dig_port->tc_lock);
> +	tc_cold_wref = tc_cold_block(dig_port);
>  
>  	dig_port->tc_mode = intel_tc_port_get_current_mode(dig_port);
>  	if (dig_port->dp.is_mst)
> @@ -466,6 +505,7 @@ void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
>  		      dig_port->tc_port_name,
>  		      tc_port_mode_name(dig_port->tc_mode));
>  
> +	tc_cold_unblock(dig_port, tc_cold_wref);
>  	mutex_unlock(&dig_port->tc_lock);
>  }
>  
> @@ -487,10 +527,15 @@ static bool intel_tc_port_needs_reset(struct intel_digital_port *dig_port)
>  bool intel_tc_port_connected(struct intel_digital_port *dig_port)
>  {
>  	bool is_connected;
> +	intel_wakeref_t tc_cold_wref;
>  
>  	intel_tc_port_lock(dig_port);
> +	tc_cold_wref = tc_cold_block(dig_port);
> +
>  	is_connected = tc_port_live_status_mask(dig_port) &
>  		       BIT(dig_port->tc_mode);
> +
> +	tc_cold_unblock(dig_port, tc_cold_wref);
>  	intel_tc_port_unlock(dig_port);
>  
>  	return is_connected;
> @@ -500,15 +545,20 @@ static void __intel_tc_port_lock(struct intel_digital_port *dig_port,
>  				 int required_lanes)
>  {
>  	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> -	intel_wakeref_t wakeref;
> +	intel_wakeref_t wakeref, tc_cold_wref;

Could be moved into its scope.

>  
>  	wakeref = intel_display_power_get(i915, POWER_DOMAIN_DISPLAY_CORE);
>  
>  	mutex_lock(&dig_port->tc_lock);
>  
> -	if (!dig_port->tc_link_refcount &&
> -	    intel_tc_port_needs_reset(dig_port))
> -		intel_tc_port_reset_mode(dig_port, required_lanes);
> +	if (!dig_port->tc_link_refcount) {
> +		tc_cold_wref = tc_cold_block(dig_port);
> +
> +		if (intel_tc_port_needs_reset(dig_port))
> +			intel_tc_port_reset_mode(dig_port, required_lanes);
> +
> +		tc_cold_unblock(dig_port, tc_cold_wref);
> +	}
>  
>  	drm_WARN_ON(&i915->drm, dig_port->tc_lock_wakeref);
>  	dig_port->tc_lock_wakeref = wakeref;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8cebb7a86b8c..5cbcd01ac3d5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9107,6 +9107,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
>              /* See also IPS_CTL */
> -- 
> 2.26.0
>
Souza, Jose April 7, 2020, 8:01 p.m. UTC | #2 
On Tue, 2020-04-07 at 18:42 +0300, Imre Deak wrote:
> On Mon, Apr 06, 2020 at 06:11:53PM -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 before driver makes use of the port, to prevent it BSpec
> > states
> > that aux powerwell should be held.
> > 
> > So here embedding the TC cold exit sequence into ICL aux enable,
> > it will enable aux and then request TC cold to exit.
> > 
> > The TC cold block(exit and aux hold) and unblock was added to some
> > exported TC functions for the others and to access PHY registers,
> > callers should enable and keep aux powerwell enabled during access.
> > 
> > Also adding TC cold check and warnig in tc_port_load_fia_params()
> > as
> > at this point of the driver initialization we can't request power
> > wells, if we get this warning we will need to figure out how to
> > handle
> > it.
> > 
> > v2:
> > - moved ICL TC cold exit function to intel_display_power
> > - using dig_port->tc_legacy_port to only execute sequences for
> > legacy
> > ports, hopefully VBTs will have this right
> > - fixed check to call _hsw_power_well_continue_enable()
> > - calling _hsw_power_well_continue_enable() unconditionally in
> > icl_tc_phy_aux_power_well_enable(), if needed we will surpress
> > timeout
> > warnings of TC legacy ports
> > - only blocking TC cold around fia access
> > 
> > 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>
> > 
> > squash icl
> 
> Leftover.

Ops, thanks

> 
> > ---
> >  .../drm/i915/display/intel_display_power.c    | 22 ++++++-
> >  .../drm/i915/display/intel_display_types.h    |  1 +
> >  drivers/gpu/drm/i915/display/intel_tc.c       | 64
> > +++++++++++++++++--
> >  drivers/gpu/drm/i915/i915_reg.h               |  1 +
> >  4 files changed, 80 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 62e49f06d467..1336247743c4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -575,6 +575,25 @@ 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_cold_exit(struct drm_i915_private *i915)
> > +{
> > +	int ret;
> > +
> > +	do {
> > +		ret = sandybridge_pcode_write_timeout(i915,
> > +						      ICL_PCODE_EXIT_TC
> > COLD,
> > +						      0, 250, 1);
> > +
> 
> Extra w/s.

Removed

> 
> > +	} while (ret == -EAGAIN);
> 
> Let's protect against an endless loop.

const ktime_t timeout = ktime_add_ms(ktime_get_raw(), 3);
int ret;

do {
	ret = sandybridge_pcode_write_timeout(i915,
					      ICL_PCODE_EXIT_TCCOLD,
					      0, 250, 1);
} while (ret == -EAGAIN && ktime_compare(timeout, ktime_get_raw()) >
0);
Changing to:



> 
> > +
> > +	/* Spec states that TC cold exit can take up to 1ms to complete
> > */
> > +	if (!ret)
> > +		msleep(1);
> > +
> > +	drm_dbg_kms(&i915->drm, "TC cold block %s\n", ret == 0 ?
> > "succeeded" :
> > +		    "failed");
> > +}
> > +
> >  static void
> >  icl_tc_phy_aux_power_well_enable(struct drm_i915_private
> > *dev_priv,
> >  				 struct i915_power_well *power_well)
> > @@ -593,7 +612,8 @@ icl_tc_phy_aux_power_well_enable(struct
> > drm_i915_private *dev_priv,
> >  
> >  	hsw_power_well_enable_prepare(dev_priv, power_well);
> >  
> > -	/* TODO ICL TC cold handling */
> > +	if (INTEL_GEN(dev_priv) == 11 && dig_port->tc_legacy_port)
> > +		icl_tc_cold_exit(dev_priv);
> >  
> >  	hsw_power_well_enable_complete(dev_priv, power_well);
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 5a0adf14ebef..f7506ac40eb4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1400,6 +1400,7 @@ struct intel_digital_port {
> >  	enum intel_display_power_domain ddi_io_power_domain;
> >  	struct mutex tc_lock;	/* protects the TypeC port mode */
> >  	intel_wakeref_t tc_lock_wakeref;
> > +	intel_wakeref_t tc_cold_wakeref;
> 
> Not needed any more.

Ops, thanks

> 
> >  	int tc_link_refcount;
> >  	bool tc_legacy_port:1;
> >  	char tc_port_name[8];
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c
> > b/drivers/gpu/drm/i915/display/intel_tc.c
> > index 9b850c11aa78..7564259d677e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> > @@ -34,6 +34,7 @@ tc_port_load_fia_params(struct drm_i915_private
> > *i915,
> >  	if (INTEL_INFO(i915)->display.has_modular_fia) {
> >  		modular_fia = intel_uncore_read(&i915->uncore,
> >  						PORT_TX_DFLEXDPSP(FIA1)
> > );
> > +		drm_WARN_ON(&i915->drm, modular_fia == 0xffffffff);
> >  		modular_fia &= MODULAR_FIA_MASK;
> >  	} else {
> >  		modular_fia = 0;
> > @@ -52,6 +53,37 @@ tc_port_load_fia_params(struct drm_i915_private
> > *i915,
> >  	}
> >  }
> >  
> > +static intel_wakeref_t
> > +tc_cold_block(struct intel_digital_port *dig_port)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> > +	enum intel_display_power_domain domain;
> > +
> > +	if (INTEL_GEN(i915) != 11 || !dig_port->tc_legacy_port)
> > +		return 0;
> > +
> > +	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
> > +	return intel_display_power_get(i915, domain);
> > +}
> > +
> > +static void
> > +tc_cold_unblock(struct intel_digital_port *dig_port,
> > intel_wakeref_t wakeref)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> > +	enum intel_display_power_domain domain;
> > +
> > +	/*
> > +	 * wakeref == -1, means some error happened saving
> > save_depot_stack but
> > +	 * power should still be put down and 0 is a invalid
> > save_depot_stack
> > +	 * id so can be used to skip it for non TC legacy ports.
> > +	 */
> > +	if (wakeref == 0)
> > +		return;
> > +
> > +	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
> > +	intel_display_power_put_async(i915, domain, wakeref);
> > +}
> > +
> >  u32 intel_tc_port_get_lane_mask(struct intel_digital_port
> > *dig_port)
> >  {
> >  	struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> > @@ -415,9 +447,14 @@ 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
> > )));
> > +	if (INTEL_GEN(i915) != 11 || !dig_port->tc_legacy_port) {
> > +		enum intel_display_power_domain aux_domain;
> > +		bool aux_powered;
> > +
> > +		aux_domain = intel_aux_power_domain(dig_port);
> > +		aux_powered = intel_display_power_is_enabled(i915,
> > aux_domain);
> > +		drm_WARN_ON(&i915->drm, aux_powered);
> > +	}
> >  
> >  	icl_tc_phy_disconnect(dig_port);
> >  	icl_tc_phy_connect(dig_port, required_lanes);
> > @@ -439,9 +476,11 @@ intel_tc_port_link_init_refcount(struct
> > intel_digital_port *dig_port,
> >  void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
> >  {
> >  	struct intel_encoder *encoder = &dig_port->base;
> > +	intel_wakeref_t tc_cold_wref;
> >  	int active_links = 0;
> >  
> >  	mutex_lock(&dig_port->tc_lock);
> > +	tc_cold_wref = tc_cold_block(dig_port);
> >  
> >  	dig_port->tc_mode = intel_tc_port_get_current_mode(dig_port);
> >  	if (dig_port->dp.is_mst)
> > @@ -466,6 +505,7 @@ void intel_tc_port_sanitize(struct
> > intel_digital_port *dig_port)
> >  		      dig_port->tc_port_name,
> >  		      tc_port_mode_name(dig_port->tc_mode));
> >  
> > +	tc_cold_unblock(dig_port, tc_cold_wref);
> >  	mutex_unlock(&dig_port->tc_lock);
> >  }
> >  
> > @@ -487,10 +527,15 @@ static bool intel_tc_port_needs_reset(struct
> > intel_digital_port *dig_port)
> >  bool intel_tc_port_connected(struct intel_digital_port *dig_port)
> >  {
> >  	bool is_connected;
> > +	intel_wakeref_t tc_cold_wref;
> >  
> >  	intel_tc_port_lock(dig_port);
> > +	tc_cold_wref = tc_cold_block(dig_port);
> > +
> >  	is_connected = tc_port_live_status_mask(dig_port) &
> >  		       BIT(dig_port->tc_mode);
> > +
> > +	tc_cold_unblock(dig_port, tc_cold_wref);
> >  	intel_tc_port_unlock(dig_port);
> >  
> >  	return is_connected;
> > @@ -500,15 +545,20 @@ static void __intel_tc_port_lock(struct
> > intel_digital_port *dig_port,
> >  				 int required_lanes)
> >  {
> >  	struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> > -	intel_wakeref_t wakeref;
> > +	intel_wakeref_t wakeref, tc_cold_wref;
> 
> Could be moved into its scope.

Done

> 
> >  
> >  	wakeref = intel_display_power_get(i915,
> > POWER_DOMAIN_DISPLAY_CORE);
> >  
> >  	mutex_lock(&dig_port->tc_lock);
> >  
> > -	if (!dig_port->tc_link_refcount &&
> > -	    intel_tc_port_needs_reset(dig_port))
> > -		intel_tc_port_reset_mode(dig_port, required_lanes);
> > +	if (!dig_port->tc_link_refcount) {
> > +		tc_cold_wref = tc_cold_block(dig_port);
> > +
> > +		if (intel_tc_port_needs_reset(dig_port))
> > +			intel_tc_port_reset_mode(dig_port,
> > required_lanes);
> > +
> > +		tc_cold_unblock(dig_port, tc_cold_wref);
> > +	}
> >  
> >  	drm_WARN_ON(&i915->drm, dig_port->tc_lock_wakeref);
> >  	dig_port->tc_lock_wakeref = wakeref;
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 8cebb7a86b8c..5cbcd01ac3d5 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -9107,6 +9107,7 @@ enum {
> >  #define     ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point)	(((poin
> > t) << 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
> >              /* See also IPS_CTL */
> > -- 
> > 2.26.0
> >
Imre Deak April 7, 2020, 9:52 p.m. UTC | #3 
On Tue, Apr 07, 2020 at 11:01:14PM +0300, Souza, Jose wrote:
> [...]
> > > +	} while (ret == -EAGAIN);
> > 
> > Let's protect against an endless loop.
> 
> const ktime_t timeout = ktime_add_ms(ktime_get_raw(), 3);
> int ret;
> 
> do {
> 	ret = sandybridge_pcode_write_timeout(i915,
> 					      ICL_PCODE_EXIT_TCCOLD,
> 					      0, 250, 1);
> } while (ret == -EAGAIN && ktime_compare(timeout, ktime_get_raw()) > 0);

Why not just a simple

	trial = 0;
	while (1) {
		ret = pcode_write();
		if (ret != -EAGAIN || ++trial == 3)
			break;
		msleep(1);
	}

with the msleep(1), as if the PCODE run/busy flag didn't get cleared after
the 1ms polling, it probably doesn't make sense to retry in a tight loop.

--Imre
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 62e49f06d467..1336247743c4 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -575,6 +575,25 @@  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_cold_exit(struct drm_i915_private *i915)
+{
+	int ret;
+
+	do {
+		ret = sandybridge_pcode_write_timeout(i915,
+						      ICL_PCODE_EXIT_TCCOLD,
+						      0, 250, 1);
+
+	} while (ret == -EAGAIN);
+
+	/* Spec states that TC cold exit can take up to 1ms to complete */
+	if (!ret)
+		msleep(1);
+
+	drm_dbg_kms(&i915->drm, "TC cold block %s\n", ret == 0 ? "succeeded" :
+		    "failed");
+}
+
 static void
 icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
 				 struct i915_power_well *power_well)
@@ -593,7 +612,8 @@  icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
 
 	hsw_power_well_enable_prepare(dev_priv, power_well);
 
-	/* TODO ICL TC cold handling */
+	if (INTEL_GEN(dev_priv) == 11 && dig_port->tc_legacy_port)
+		icl_tc_cold_exit(dev_priv);
 
 	hsw_power_well_enable_complete(dev_priv, power_well);
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 5a0adf14ebef..f7506ac40eb4 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1400,6 +1400,7 @@  struct intel_digital_port {
 	enum intel_display_power_domain ddi_io_power_domain;
 	struct mutex tc_lock;	/* protects the TypeC port mode */
 	intel_wakeref_t tc_lock_wakeref;
+	intel_wakeref_t tc_cold_wakeref;
 	int tc_link_refcount;
 	bool tc_legacy_port:1;
 	char tc_port_name[8];
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index 9b850c11aa78..7564259d677e 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -34,6 +34,7 @@  tc_port_load_fia_params(struct drm_i915_private *i915,
 	if (INTEL_INFO(i915)->display.has_modular_fia) {
 		modular_fia = intel_uncore_read(&i915->uncore,
 						PORT_TX_DFLEXDPSP(FIA1));
+		drm_WARN_ON(&i915->drm, modular_fia == 0xffffffff);
 		modular_fia &= MODULAR_FIA_MASK;
 	} else {
 		modular_fia = 0;
@@ -52,6 +53,37 @@  tc_port_load_fia_params(struct drm_i915_private *i915,
 	}
 }
 
+static intel_wakeref_t
+tc_cold_block(struct intel_digital_port *dig_port)
+{
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+	enum intel_display_power_domain domain;
+
+	if (INTEL_GEN(i915) != 11 || !dig_port->tc_legacy_port)
+		return 0;
+
+	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
+	return intel_display_power_get(i915, domain);
+}
+
+static void
+tc_cold_unblock(struct intel_digital_port *dig_port, intel_wakeref_t wakeref)
+{
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+	enum intel_display_power_domain domain;
+
+	/*
+	 * wakeref == -1, means some error happened saving save_depot_stack but
+	 * power should still be put down and 0 is a invalid save_depot_stack
+	 * id so can be used to skip it for non TC legacy ports.
+	 */
+	if (wakeref == 0)
+		return;
+
+	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
+	intel_display_power_put_async(i915, domain, wakeref);
+}
+
 u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
 {
 	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
@@ -415,9 +447,14 @@  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)));
+	if (INTEL_GEN(i915) != 11 || !dig_port->tc_legacy_port) {
+		enum intel_display_power_domain aux_domain;
+		bool aux_powered;
+
+		aux_domain = intel_aux_power_domain(dig_port);
+		aux_powered = intel_display_power_is_enabled(i915, aux_domain);
+		drm_WARN_ON(&i915->drm, aux_powered);
+	}
 
 	icl_tc_phy_disconnect(dig_port);
 	icl_tc_phy_connect(dig_port, required_lanes);
@@ -439,9 +476,11 @@  intel_tc_port_link_init_refcount(struct intel_digital_port *dig_port,
 void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
 {
 	struct intel_encoder *encoder = &dig_port->base;
+	intel_wakeref_t tc_cold_wref;
 	int active_links = 0;
 
 	mutex_lock(&dig_port->tc_lock);
+	tc_cold_wref = tc_cold_block(dig_port);
 
 	dig_port->tc_mode = intel_tc_port_get_current_mode(dig_port);
 	if (dig_port->dp.is_mst)
@@ -466,6 +505,7 @@  void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
 		      dig_port->tc_port_name,
 		      tc_port_mode_name(dig_port->tc_mode));
 
+	tc_cold_unblock(dig_port, tc_cold_wref);
 	mutex_unlock(&dig_port->tc_lock);
 }
 
@@ -487,10 +527,15 @@  static bool intel_tc_port_needs_reset(struct intel_digital_port *dig_port)
 bool intel_tc_port_connected(struct intel_digital_port *dig_port)
 {
 	bool is_connected;
+	intel_wakeref_t tc_cold_wref;
 
 	intel_tc_port_lock(dig_port);
+	tc_cold_wref = tc_cold_block(dig_port);
+
 	is_connected = tc_port_live_status_mask(dig_port) &
 		       BIT(dig_port->tc_mode);
+
+	tc_cold_unblock(dig_port, tc_cold_wref);
 	intel_tc_port_unlock(dig_port);
 
 	return is_connected;
@@ -500,15 +545,20 @@  static void __intel_tc_port_lock(struct intel_digital_port *dig_port,
 				 int required_lanes)
 {
 	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
-	intel_wakeref_t wakeref;
+	intel_wakeref_t wakeref, tc_cold_wref;
 
 	wakeref = intel_display_power_get(i915, POWER_DOMAIN_DISPLAY_CORE);
 
 	mutex_lock(&dig_port->tc_lock);
 
-	if (!dig_port->tc_link_refcount &&
-	    intel_tc_port_needs_reset(dig_port))
-		intel_tc_port_reset_mode(dig_port, required_lanes);
+	if (!dig_port->tc_link_refcount) {
+		tc_cold_wref = tc_cold_block(dig_port);
+
+		if (intel_tc_port_needs_reset(dig_port))
+			intel_tc_port_reset_mode(dig_port, required_lanes);
+
+		tc_cold_unblock(dig_port, tc_cold_wref);
+	}
 
 	drm_WARN_ON(&i915->drm, dig_port->tc_lock_wakeref);
 	dig_port->tc_lock_wakeref = wakeref;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8cebb7a86b8c..5cbcd01ac3d5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9107,6 +9107,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
             /* See also IPS_CTL */