[Intel-gfx] [PATCH 6/6] drm/i915/tc/tgl: Implement TC cold sequences

Imre Deak imre.deak at intel.com
Thu Apr 2 01:08:21 UTC 2020


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 at intel.com>
> > > Cc: Cooper Chiou <cooper.chiou at intel.com>
> > > Cc: Kai-Heng Feng <kai.heng.feng at canonical.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > > ---
> > >  .../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
> > > 


More information about the Intel-gfx mailing list