[Intel-gfx] [PATCH 2/2] drm/i915: Use single set of AUX powerwell ops for gen11+

Souza, Jose jose.souza at intel.com
Mon Apr 20 20:09:05 UTC 2020


On Wed, 2020-04-15 at 16:34 -0700, Matt Roper wrote:
> AUX power wells sometimes need additional handling besides just
> programming the specific power well registers:
>  * Type-C PHY's also require additional Type-C register programming
>  * ICL combo PHY's require additional workarounds
>  * TGL & EHL combo PHY's can be treated like any other power well
> 
> Today we have dedicated aux ops for the ICL combo PHY and Type-C
> cases.
> This works fine, but means that when a new platform shows up with
> identical general power well handling, but different types of PHYs on
> its outputs, we have to define an entire new power well table for
> that
> platform and can't just re-use the table from the earlier platform --
> as
> an example, see ehl_power_wells[], which is a subset of
> icl_power_wells[], *except* that we need to specify different AUX ops
> for the third display.
> 
> If we instead create a single set of top-level aux ops that will
> check
> the PHY type and then dispatch to the appropriate handlers, we can
> get
> more reuse out of our power well definitions.  This allows us to
> immediately eliminate ehl_power_wells[] and simply reuse the ICL
> table;
> if future platforms follow the same general power well assignments as
> either ICL or TGL, we'll be able to re-use those tables in the same
> way.
> 
> Note that I've only changed ICL+ platforms over to using the new
> icl_aux
> ops; at this point it's unlikely that we'll have any new platforms
> that
> re-use gen9 or earlier power well configurations.
> 
> v2:
>  - ICL_AUX_PW_TO_PHY() won't return the proper PHY for TBT AUX power
>    wells.  But we know those wells will only used on Type-C outputs
>    anyway, so we can just check is is_tc_tbt flag in the condition.
>    (Jose).
> 

Reviewed-by: José Roberto de Souza <jose.souza at intel.com>

> Cc: José Roberto de Souza <jose.souza at intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> ---
>  .../drm/i915/display/intel_display_power.c    | 244 +++++-----------
> --
>  1 file changed, 62 insertions(+), 182 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 03bdde19c8c9..f72935146778 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -593,6 +593,40 @@ icl_tc_phy_aux_power_well_disable(struct
> drm_i915_private *dev_priv,
>  	hsw_power_well_disable(dev_priv, power_well);
>  }
>  
> +static void
> +icl_aux_power_well_enable(struct drm_i915_private *dev_priv,
> +			  struct i915_power_well *power_well)
> +{
> +	int pw_idx = power_well->desc->hsw.idx;
> +	enum phy phy = ICL_AUX_PW_TO_PHY(pw_idx);  /* non-TBT only */
> +	bool is_tbt = power_well->desc->hsw.is_tc_tbt;
> +
> +	if (is_tbt || intel_phy_is_tc(dev_priv, phy))
> +		return icl_tc_phy_aux_power_well_enable(dev_priv,
> power_well);
> +	else if (IS_ICELAKE(dev_priv))
> +		return icl_combo_phy_aux_power_well_enable(dev_priv,
> +							   power_well);
> +	else
> +		return hsw_power_well_enable(dev_priv, power_well);
> +}
> +
> +static void
> +icl_aux_power_well_disable(struct drm_i915_private *dev_priv,
> +			   struct i915_power_well *power_well)
> +{
> +	int pw_idx = power_well->desc->hsw.idx;
> +	enum phy phy = ICL_AUX_PW_TO_PHY(pw_idx);  /* non-TBT only */
> +	bool is_tbt = power_well->desc->hsw.is_tc_tbt;
> +
> +	if (is_tbt || intel_phy_is_tc(dev_priv, phy))
> +		return icl_tc_phy_aux_power_well_disable(dev_priv,
> power_well);
> +	else if (IS_ICELAKE(dev_priv))
> +		return icl_combo_phy_aux_power_well_disable(dev_priv,
> +							    power_well)
> ;
> +	else
> +		return hsw_power_well_disable(dev_priv, power_well);
> +}
> +
>  /*
>   * We should only use the power well if we explicitly asked the
> hardware to
>   * enable it, so check if it's enabled and also check if we've
> requested it to
> @@ -3503,17 +3537,10 @@ static const struct i915_power_well_desc
> cnl_power_wells[] = {
>  	},
>  };
>  
> -static const struct i915_power_well_ops
> icl_combo_phy_aux_power_well_ops = {
> -	.sync_hw = hsw_power_well_sync_hw,
> -	.enable = icl_combo_phy_aux_power_well_enable,
> -	.disable = icl_combo_phy_aux_power_well_disable,
> -	.is_enabled = hsw_power_well_enabled,
> -};
> -
> -static const struct i915_power_well_ops
> icl_tc_phy_aux_power_well_ops = {
> +static const struct i915_power_well_ops icl_aux_power_well_ops = {
>  	.sync_hw = hsw_power_well_sync_hw,
> -	.enable = icl_tc_phy_aux_power_well_enable,
> -	.disable = icl_tc_phy_aux_power_well_disable,
> +	.enable = icl_aux_power_well_enable,
> +	.disable = icl_aux_power_well_disable,
>  	.is_enabled = hsw_power_well_enabled,
>  };
>  
> @@ -3643,7 +3670,7 @@ static const struct i915_power_well_desc
> icl_power_wells[] = {
>  	{
>  		.name = "AUX A",
>  		.domains = ICL_AUX_A_IO_POWER_DOMAINS,
> -		.ops = &icl_combo_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -3653,7 +3680,7 @@ static const struct i915_power_well_desc
> icl_power_wells[] = {
>  	{
>  		.name = "AUX B",
>  		.domains = ICL_AUX_B_IO_POWER_DOMAINS,
> -		.ops = &icl_combo_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -3663,7 +3690,7 @@ static const struct i915_power_well_desc
> icl_power_wells[] = {
>  	{
>  		.name = "AUX C TC1",
>  		.domains = ICL_AUX_C_TC1_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -3674,7 +3701,7 @@ static const struct i915_power_well_desc
> icl_power_wells[] = {
>  	{
>  		.name = "AUX D TC2",
>  		.domains = ICL_AUX_D_TC2_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -3685,7 +3712,7 @@ static const struct i915_power_well_desc
> icl_power_wells[] = {
>  	{
>  		.name = "AUX E TC3",
>  		.domains = ICL_AUX_E_TC3_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -3696,7 +3723,7 @@ static const struct i915_power_well_desc
> icl_power_wells[] = {
>  	{
>  		.name = "AUX F TC4",
>  		.domains = ICL_AUX_F_TC4_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -3707,7 +3734,7 @@ static const struct i915_power_well_desc
> icl_power_wells[] = {
>  	{
>  		.name = "AUX C TBT1",
>  		.domains = ICL_AUX_C_TBT1_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -3718,7 +3745,7 @@ static const struct i915_power_well_desc
> icl_power_wells[] = {
>  	{
>  		.name = "AUX D TBT2",
>  		.domains = ICL_AUX_D_TBT2_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -3729,7 +3756,7 @@ static const struct i915_power_well_desc
> icl_power_wells[] = {
>  	{
>  		.name = "AUX E TBT3",
>  		.domains = ICL_AUX_E_TBT3_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -3740,7 +3767,7 @@ static const struct i915_power_well_desc
> icl_power_wells[] = {
>  	{
>  		.name = "AUX F TBT4",
>  		.domains = ICL_AUX_F_TBT4_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -3762,151 +3789,6 @@ static const struct i915_power_well_desc
> icl_power_wells[] = {
>  	},
>  };
>  
> -static const struct i915_power_well_desc ehl_power_wells[] = {
> -	{
> -		.name = "always-on",
> -		.always_on = true,
> -		.domains = POWER_DOMAIN_MASK,
> -		.ops = &i9xx_always_on_power_well_ops,
> -		.id = DISP_PW_ID_NONE,
> -	},
> -	{
> -		.name = "power well 1",
> -		/* Handled by the DMC firmware */
> -		.always_on = true,
> -		.domains = 0,
> -		.ops = &hsw_power_well_ops,
> -		.id = SKL_DISP_PW_1,
> -		{
> -			.hsw.regs = &hsw_power_well_regs,
> -			.hsw.idx = ICL_PW_CTL_IDX_PW_1,
> -			.hsw.has_fuses = true,
> -		},
> -	},
> -	{
> -		.name = "DC off",
> -		.domains = ICL_DISPLAY_DC_OFF_POWER_DOMAINS,
> -		.ops = &gen9_dc_off_power_well_ops,
> -		.id = SKL_DISP_DC_OFF,
> -	},
> -	{
> -		.name = "power well 2",
> -		.domains = ICL_PW_2_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = SKL_DISP_PW_2,
> -		{
> -			.hsw.regs = &hsw_power_well_regs,
> -			.hsw.idx = ICL_PW_CTL_IDX_PW_2,
> -			.hsw.has_fuses = true,
> -		},
> -	},
> -	{
> -		.name = "power well 3",
> -		.domains = ICL_PW_3_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = DISP_PW_ID_NONE,
> -		{
> -			.hsw.regs = &hsw_power_well_regs,
> -			.hsw.idx = ICL_PW_CTL_IDX_PW_3,
> -			.hsw.irq_pipe_mask = BIT(PIPE_B),
> -			.hsw.has_vga = true,
> -			.hsw.has_fuses = true,
> -		},
> -	},
> -	{
> -		.name = "DDI A IO",
> -		.domains = ICL_DDI_IO_A_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = DISP_PW_ID_NONE,
> -		{
> -			.hsw.regs = &icl_ddi_power_well_regs,
> -			.hsw.idx = ICL_PW_CTL_IDX_DDI_A,
> -		},
> -	},
> -	{
> -		.name = "DDI B IO",
> -		.domains = ICL_DDI_IO_B_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = DISP_PW_ID_NONE,
> -		{
> -			.hsw.regs = &icl_ddi_power_well_regs,
> -			.hsw.idx = ICL_PW_CTL_IDX_DDI_B,
> -		},
> -	},
> -	{
> -		.name = "DDI C IO",
> -		.domains = ICL_DDI_IO_C_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = DISP_PW_ID_NONE,
> -		{
> -			.hsw.regs = &icl_ddi_power_well_regs,
> -			.hsw.idx = ICL_PW_CTL_IDX_DDI_C,
> -		},
> -	},
> -	{
> -		.name = "DDI D IO",
> -		.domains = ICL_DDI_IO_D_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = DISP_PW_ID_NONE,
> -		{
> -			.hsw.regs = &icl_ddi_power_well_regs,
> -			.hsw.idx = ICL_PW_CTL_IDX_DDI_D,
> -		},
> -	},
> -	{
> -		.name = "AUX A",
> -		.domains = ICL_AUX_A_IO_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = DISP_PW_ID_NONE,
> -		{
> -			.hsw.regs = &icl_aux_power_well_regs,
> -			.hsw.idx = ICL_PW_CTL_IDX_AUX_A,
> -		},
> -	},
> -	{
> -		.name = "AUX B",
> -		.domains = ICL_AUX_B_IO_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = DISP_PW_ID_NONE,
> -		{
> -			.hsw.regs = &icl_aux_power_well_regs,
> -			.hsw.idx = ICL_PW_CTL_IDX_AUX_B,
> -		},
> -	},
> -	{
> -		.name = "AUX C",
> -		.domains = ICL_AUX_C_TC1_IO_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = DISP_PW_ID_NONE,
> -		{
> -			.hsw.regs = &icl_aux_power_well_regs,
> -			.hsw.idx = ICL_PW_CTL_IDX_AUX_C,
> -		},
> -	},
> -	{
> -		.name = "AUX D",
> -		.domains = ICL_AUX_D_TC2_IO_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = DISP_PW_ID_NONE,
> -		{
> -			.hsw.regs = &icl_aux_power_well_regs,
> -			.hsw.idx = ICL_PW_CTL_IDX_AUX_D,
> -		},
> -	},
> -	{
> -		.name = "power well 4",
> -		.domains = ICL_PW_4_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = DISP_PW_ID_NONE,
> -		{
> -			.hsw.regs = &hsw_power_well_regs,
> -			.hsw.idx = ICL_PW_CTL_IDX_PW_4,
> -			.hsw.has_fuses = true,
> -			.hsw.irq_pipe_mask = BIT(PIPE_C),
> -		},
> -	},
> -};
> -
>  static const struct i915_power_well_desc tgl_power_wells[] = {
>  	{
>  		.name = "always-on",
> @@ -4051,7 +3933,7 @@ static const struct i915_power_well_desc
> tgl_power_wells[] = {
>  	{
>  		.name = "AUX A",
>  		.domains = TGL_AUX_A_IO_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -4061,7 +3943,7 @@ static const struct i915_power_well_desc
> tgl_power_wells[] = {
>  	{
>  		.name = "AUX B",
>  		.domains = TGL_AUX_B_IO_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -4071,7 +3953,7 @@ static const struct i915_power_well_desc
> tgl_power_wells[] = {
>  	{
>  		.name = "AUX C",
>  		.domains = TGL_AUX_C_IO_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -4081,7 +3963,7 @@ static const struct i915_power_well_desc
> tgl_power_wells[] = {
>  	{
>  		.name = "AUX D TC1",
>  		.domains = TGL_AUX_D_TC1_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -4092,7 +3974,7 @@ static const struct i915_power_well_desc
> tgl_power_wells[] = {
>  	{
>  		.name = "AUX E TC2",
>  		.domains = TGL_AUX_E_TC2_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -4103,7 +3985,7 @@ static const struct i915_power_well_desc
> tgl_power_wells[] = {
>  	{
>  		.name = "AUX F TC3",
>  		.domains = TGL_AUX_F_TC3_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -4114,7 +3996,7 @@ static const struct i915_power_well_desc
> tgl_power_wells[] = {
>  	{
>  		.name = "AUX G TC4",
>  		.domains = TGL_AUX_G_TC4_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -4125,7 +4007,7 @@ static const struct i915_power_well_desc
> tgl_power_wells[] = {
>  	{
>  		.name = "AUX H TC5",
>  		.domains = TGL_AUX_H_TC5_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -4136,7 +4018,7 @@ static const struct i915_power_well_desc
> tgl_power_wells[] = {
>  	{
>  		.name = "AUX I TC6",
>  		.domains = TGL_AUX_I_TC6_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -4147,7 +4029,7 @@ static const struct i915_power_well_desc
> tgl_power_wells[] = {
>  	{
>  		.name = "AUX D TBT1",
>  		.domains = TGL_AUX_D_TBT1_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -4158,7 +4040,7 @@ static const struct i915_power_well_desc
> tgl_power_wells[] = {
>  	{
>  		.name = "AUX E TBT2",
>  		.domains = TGL_AUX_E_TBT2_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -4169,7 +4051,7 @@ static const struct i915_power_well_desc
> tgl_power_wells[] = {
>  	{
>  		.name = "AUX F TBT3",
>  		.domains = TGL_AUX_F_TBT3_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -4180,7 +4062,7 @@ static const struct i915_power_well_desc
> tgl_power_wells[] = {
>  	{
>  		.name = "AUX G TBT4",
>  		.domains = TGL_AUX_G_TBT4_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -4191,7 +4073,7 @@ static const struct i915_power_well_desc
> tgl_power_wells[] = {
>  	{
>  		.name = "AUX H TBT5",
>  		.domains = TGL_AUX_H_TBT5_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -4202,7 +4084,7 @@ static const struct i915_power_well_desc
> tgl_power_wells[] = {
>  	{
>  		.name = "AUX I TBT6",
>  		.domains = TGL_AUX_I_TBT6_IO_POWER_DOMAINS,
> -		.ops = &icl_tc_phy_aux_power_well_ops,
> +		.ops = &icl_aux_power_well_ops,
>  		.id = DISP_PW_ID_NONE,
>  		{
>  			.hsw.regs = &icl_aux_power_well_regs,
> @@ -4383,8 +4265,6 @@ int intel_power_domains_init(struct
> drm_i915_private *dev_priv)
>  	 */
>  	if (IS_GEN(dev_priv, 12)) {
>  		err = set_power_wells(power_domains, tgl_power_wells);
> -	} else if (IS_ELKHARTLAKE(dev_priv)) {
> -		err = set_power_wells(power_domains, ehl_power_wells);
>  	} else if (IS_GEN(dev_priv, 11)) {
>  		err = set_power_wells(power_domains, icl_power_wells);
>  	} else if (IS_CANNONLAKE(dev_priv)) {


More information about the Intel-gfx mailing list