[Intel-gfx] [PATCH 10/10] drm/i915/cnl: Don't try to manage Port F power wells on all CNL.

Imre Deak imre.deak at intel.com
Mon Jan 22 12:12:28 UTC 2018


On Fri, Jan 19, 2018 at 04:05:24PM -0800, Rodrigo Vivi wrote:
> SKUs that lacks on the full port F split will just time out
> when touching this power well bits, causing a noisy warn.
> 
> This macro style is a deviation from the original definition in use
> for other platforms, but it at least avoid code duplication.
> Other smart alternatives like providing a joint list was also considered
> but it would require some extra memory handling that would be
> a deviation from the original simplistic definitions here anyways,
> plus requiring extra tests and possibly creating some corner cases
> for one single platform. So let's move with the simplest and safest
> approach.
> 
> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> Cc: Imre Deak <imre.deak at intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 177 +++++++++++++++++---------------
>  1 file changed, 94 insertions(+), 83 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 433048ffa5c6..8dbc9b138ffd 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -2334,89 +2334,96 @@ static struct i915_power_well glk_power_wells[] = {
>  	},
>  };
>  
> +#define basic_cnl_power_wells						\
> +	{								\
> +		.name = "always-on",					\
> +		.always_on = 1,						\
> +		.domains = POWER_DOMAIN_MASK,				\
> +		.ops = &i9xx_always_on_power_well_ops,			\
> +		.id = I915_DISP_PW_ALWAYS_ON,				\
> +	},								\
> +	{								\
> +		.name = "power well 1",					\
> +		/* Handled by the DMC firmware */			\
> +		.domains = 0,						\
> +		.ops = &hsw_power_well_ops,				\
> +		.id = SKL_DISP_PW_1,					\
> +		{							\
> +			.hsw.has_fuses = true,				\
> +		},							\
> +	},								\
> +	{								\
> +		.name = "AUX A",					\
> +		.domains = CNL_DISPLAY_AUX_A_POWER_DOMAINS,		\
> +		.ops = &hsw_power_well_ops,				\
> +		.id = CNL_DISP_PW_AUX_A,				\
> +	},								\
> +	{								\
> +		.name = "AUX B",					\
> +		.domains = CNL_DISPLAY_AUX_B_POWER_DOMAINS,		\
> +		.ops = &hsw_power_well_ops,				\
> +		.id = CNL_DISP_PW_AUX_B,				\
> +	},								\
> +	{								\
> +		.name = "AUX C",					\
> +		.domains = CNL_DISPLAY_AUX_C_POWER_DOMAINS,		\
> +		.ops = &hsw_power_well_ops,				\
> +		.id = CNL_DISP_PW_AUX_C,				\
> +	},								\
> +	{								\
> +		.name = "AUX D",					\
> +		.domains = CNL_DISPLAY_AUX_D_POWER_DOMAINS,		\
> +		.ops = &hsw_power_well_ops,				\
> +		.id = CNL_DISP_PW_AUX_D,				\
> +		},							\
> +	{								\
> +		.name = "DC off",					\
> +		.domains = CNL_DISPLAY_DC_OFF_POWER_DOMAINS,		\
> +		.ops = &gen9_dc_off_power_well_ops,			\
> +		.id = SKL_DISP_PW_DC_OFF,				\
> +	},								\
> +	{								\
> +		.name = "power well 2",					\
> +		.domains = CNL_DISPLAY_POWERWELL_2_POWER_DOMAINS,	\
> +		.ops = &hsw_power_well_ops,				\
> +		.id = SKL_DISP_PW_2,					\
> +		{							\
> +			.hsw.irq_pipe_mask = BIT(PIPE_B) | BIT(PIPE_C),	\
> +			.hsw.has_vga = true,				\
> +			.hsw.has_fuses = true,				\
> +		},							\
> +	},								\
> +	{								\
> +		.name = "DDI A IO power well",				\
> +		.domains = CNL_DISPLAY_DDI_A_IO_POWER_DOMAINS,		\
> +		.ops = &hsw_power_well_ops,				\
> +		.id = CNL_DISP_PW_DDI_A,				\
> +	},								\
> +	{								\
> +		.name = "DDI B IO power well",				\
> +		.domains = CNL_DISPLAY_DDI_B_IO_POWER_DOMAINS,		\
> +		.ops = &hsw_power_well_ops,				\
> +		.id = SKL_DISP_PW_DDI_B,				\
> +	},								\
> +	{								\
> +		.name = "DDI C IO power well",				\
> +		.domains = CNL_DISPLAY_DDI_C_IO_POWER_DOMAINS,		\
> +		.ops = &hsw_power_well_ops,				\
> +		.id = SKL_DISP_PW_DDI_C,				\
> +	},								\
> +	{								\
> +		.name = "DDI D IO power well",				\
> +		.domains = CNL_DISPLAY_DDI_D_IO_POWER_DOMAINS,		\
> +		.ops = &hsw_power_well_ops,				\
> +		.id = SKL_DISP_PW_DDI_D,				\
> +	}
> +
>  static struct i915_power_well cnl_power_wells[] = {
> -	{
> -		.name = "always-on",
> -		.always_on = 1,
> -		.domains = POWER_DOMAIN_MASK,
> -		.ops = &i9xx_always_on_power_well_ops,
> -		.id = I915_DISP_PW_ALWAYS_ON,
> -	},
> -	{
> -		.name = "power well 1",
> -		/* Handled by the DMC firmware */
> -		.domains = 0,
> -		.ops = &hsw_power_well_ops,
> -		.id = SKL_DISP_PW_1,
> -		{
> -			.hsw.has_fuses = true,
> -		},
> -	},
> -	{
> -		.name = "AUX A",
> -		.domains = CNL_DISPLAY_AUX_A_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = CNL_DISP_PW_AUX_A,
> -	},
> -	{
> -		.name = "AUX B",
> -		.domains = CNL_DISPLAY_AUX_B_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = CNL_DISP_PW_AUX_B,
> -	},
> -	{
> -		.name = "AUX C",
> -		.domains = CNL_DISPLAY_AUX_C_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = CNL_DISP_PW_AUX_C,
> -	},
> -	{
> -		.name = "AUX D",
> -		.domains = CNL_DISPLAY_AUX_D_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = CNL_DISP_PW_AUX_D,
> -	},
> -	{
> -		.name = "DC off",
> -		.domains = CNL_DISPLAY_DC_OFF_POWER_DOMAINS,
> -		.ops = &gen9_dc_off_power_well_ops,
> -		.id = SKL_DISP_PW_DC_OFF,
> -	},
> -	{
> -		.name = "power well 2",
> -		.domains = CNL_DISPLAY_POWERWELL_2_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = SKL_DISP_PW_2,
> -		{
> -			.hsw.irq_pipe_mask = BIT(PIPE_B) | BIT(PIPE_C),
> -			.hsw.has_vga = true,
> -			.hsw.has_fuses = true,
> -		},
> -	},
> -	{
> -		.name = "DDI A IO power well",
> -		.domains = CNL_DISPLAY_DDI_A_IO_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = CNL_DISP_PW_DDI_A,
> -	},
> -	{
> -		.name = "DDI B IO power well",
> -		.domains = CNL_DISPLAY_DDI_B_IO_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = SKL_DISP_PW_DDI_B,
> -	},
> -	{
> -		.name = "DDI C IO power well",
> -		.domains = CNL_DISPLAY_DDI_C_IO_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = SKL_DISP_PW_DDI_C,
> -	},
> -	{
> -		.name = "DDI D IO power well",
> -		.domains = CNL_DISPLAY_DDI_D_IO_POWER_DOMAINS,
> -		.ops = &hsw_power_well_ops,
> -		.id = SKL_DISP_PW_DDI_D,
> -	},
> +	basic_cnl_power_wells,
> +};
> +
> +static struct i915_power_well cnl_power_wells_with_port_f[] = {
> +	basic_cnl_power_wells,
>  	{
>  		.name = "DDI F IO power well",
>  		.domains = CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS,
> @@ -2543,7 +2550,11 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
>  	} else if (IS_GEN9_BC(dev_priv)) {
>  		set_power_wells(power_domains, skl_power_wells);
>  	} else if (IS_CANNONLAKE(dev_priv)) {
> -		set_power_wells(power_domains, cnl_power_wells);
> +		if (IS_CNL_WITH_PORT_F(dev_priv))
> +			set_power_wells(power_domains,
> +					cnl_power_wells_with_port_f);
> +		else
> +			set_power_wells(power_domains, cnl_power_wells);

I guess it's the AUX power well timing out during driver loading where
we enable all the power wells. I plan to change that so we don't enable
those during init similarly to DDI IO power wells. Until that could we
just remove the port F power wells like the following?:

--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -2410,6 +2410,8 @@ static struct i915_power_well cnl_power_wells[] = {
 		.ops = &hsw_power_well_ops,
 		.id = SKL_DISP_PW_DDI_D,
 	},
+/* Power wells for CNL with port F after this */
+#define CNL_FIRST_PORT_F_PW	CNL_DISP_PW_AUX_F
 	{
 		.name = "AUX F",
 		.domains = CNL_DISPLAY_AUX_F_POWER_DOMAINS,
@@ -2531,6 +2533,16 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
 		set_power_wells(power_domains, skl_power_wells);
 	} else if (IS_CANNONLAKE(dev_priv)) {
 		set_power_wells(power_domains, cnl_power_wells);
+		if (IS_CNL_WITH_PORT_F(dev_priv)) {
+			int i;
+
+			for (i = 0; i < power_domains->power_well_count; i++)
+				if (power_domains->power_wells[i].id ==
+				    CNL_FIRST_PORT_F_PW)
+					break;
+			WARN_ON(i == power_domains->power_well_count);
+			power_domains->power_well_count = i;
+		}
 	} else if (IS_BROXTON(dev_priv)) {
 		set_power_wells(power_domains, bxt_power_wells);
 	} else if (IS_GEMINILAKE(dev_priv)) {



>  	} else if (IS_BROXTON(dev_priv)) {
>  		set_power_wells(power_domains, bxt_power_wells);
>  	} else if (IS_GEMINILAKE(dev_priv)) {
> -- 
> 2.13.6
> 


More information about the Intel-gfx mailing list