[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