| Message ID | 1447084107-8521-11-git-send-email-patrik.jakobsson@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On ma, 2015-11-09 at 16:48 +0100, Patrik Jakobsson wrote: > Handle DC off as a power well where enabling the power well will > prevent > the DMC to enter selected DC states (required around modesets and Aux > A). Disabling the power well will allow DC states again. For now the > highest DC state is DC6 for Skylake and DC5 for Broxton but will be > configurable for Skylake in a later patch. > > v2: Check both DC5 and DC6 bits in power well enabled function > (Ville) > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 6 -- > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/intel_display.c | 6 ++ > drivers/gpu/drm/i915/intel_runtime_pm.c | 110 > +++++++++++++++++++++++--------- > 4 files changed, 88 insertions(+), 35 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > b/drivers/gpu/drm/i915/i915_drv.c > index 5a63f9a..0c7f435 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1072,9 +1072,6 @@ static int i915_pm_resume(struct device *dev) > > static int skl_suspend_complete(struct drm_i915_private *dev_priv) > { > - if (dev_priv->csr.dmc_payload) > - skl_enable_dc6(dev_priv); > - > return 0; > } > > @@ -1119,9 +1116,6 @@ static int bxt_resume_prepare(struct > drm_i915_private *dev_priv) > > static int skl_resume_prepare(struct drm_i915_private *dev_priv) > { > - if (dev_priv->csr.dmc_payload) > - skl_disable_dc6(dev_priv); > - > return 0; > } > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > index 31b3a84..df445ba 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -636,6 +636,7 @@ enum skl_disp_power_wells { > > /* Not actual bit groups. Used as IDs for > lookup_power_well() */ > SKL_DISP_PW_ALWAYS_ON, > + SKL_DISP_PW_DC_OFF, Imo it would be less confusing to call it DC3 power well. Looking at th e DC spec, DC4 is only a transitory state to DC5/6, so what we expect when we disable DC6/5 is DC3 or shallower power states (DC2/1/0). > }; > > #define SKL_POWER_WELL_STATE(pw) (1 << ((pw) * 2)) > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 649ac34..856d801 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13320,6 +13320,9 @@ static int intel_atomic_commit(struct > drm_device *dev, > to_intel_crtc_state(crtc->state) > ->update_pipe; > unsigned long put_domains = 0; > > + if (modeset) > + intel_display_power_get(dev_priv, > POWER_DOMAIN_MODESET); > + > if (modeset && crtc->state->active) { > update_scanline_offset(to_intel_crtc(crtc)); > dev_priv->display.crtc_enable(crtc); > @@ -13343,6 +13346,9 @@ static int intel_atomic_commit(struct > drm_device *dev, > modeset_put_power_domains(dev_priv, > put_domains); > > intel_post_plane_update(intel_crtc); > + > + if (modeset) > + intel_display_power_put(dev_priv, > POWER_DOMAIN_MODESET); > } > > /* FIXME: add subpixel order */ > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index edf753e..95c3fcc 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -49,9 +49,6 @@ > * present for a given platform. > */ > > -#define GEN9_ENABLE_DC5(dev) 0 > -#define SKL_ENABLE_DC6(dev) IS_SKYLAKE(dev) > - > #define for_each_power_well(i, power_well, domain_mask, > power_domains) \ > for (i = 0; > \ > i < (power_domains)->power_well_count && > \ > @@ -309,9 +306,14 @@ static void hsw_set_power_well(struct > drm_i915_private *dev_priv, > #define SKL_DISPLAY_DDI_D_POWER_DOMAINS ( \ > BIT(POWER_DOMAIN_PORT_DDI_D_LANES) | \ > BIT(POWER_DOMAIN_INIT)) > +#define SKL_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > + BIT(POWER_DOMAIN_MODESET) | \ > + BIT(POWER_DOMAIN_AUX_A) | \ > + BIT(POWER_DOMAIN_INIT)) > #define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS ( \ > (POWER_DOMAIN_MASK & ~( \ > - SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS)) | \ > + SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS | \ > + SKL_DISPLAY_DC_OFF_POWER_DOMAINS)) | \ > BIT(POWER_DOMAIN_INIT)) > > #define BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS ( \ > @@ -339,6 +341,10 @@ static void hsw_set_power_well(struct > drm_i915_private *dev_priv, > BIT(POWER_DOMAIN_AUX_A) | \ > BIT(POWER_DOMAIN_PLLS) | \ > BIT(POWER_DOMAIN_INIT)) > +#define BXT_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > + BIT(POWER_DOMAIN_MODESET) | \ > + BIT(POWER_DOMAIN_AUX_A) | \ > + BIT(POWER_DOMAIN_INIT)) > #define BXT_DISPLAY_ALWAYS_ON_POWER_DOMAINS ( \ > (POWER_DOMAIN_MASK & ~(BXT_DISPLAY_POWERWELL_1_POWER_DOMAINS > | \ > BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS)) | \ > @@ -486,15 +492,6 @@ static void gen9_enable_dc5(struct > drm_i915_private *dev_priv) > gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC5); > } > > -static void gen9_disable_dc5(struct drm_i915_private *dev_priv) > -{ > - assert_can_disable_dc5(dev_priv); > - > - DRM_DEBUG_KMS("Disabling DC5\n"); > - > - gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > -} > - > static void assert_can_enable_dc6(struct drm_i915_private *dev_priv) > { > struct drm_device *dev = dev_priv->dev; > @@ -522,6 +519,14 @@ static void assert_can_disable_dc6(struct > drm_i915_private *dev_priv) > "DC6 already programmed to be disabled.\n"); > } > > +static void gen9_disable_dc5_dc6(struct drm_i915_private *dev_priv) > +{ > + assert_can_disable_dc5(dev_priv); > + assert_can_disable_dc6(dev_priv); > + > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > +} > + > void skl_enable_dc6(struct drm_i915_private *dev_priv) > { > assert_can_enable_dc6(dev_priv); > @@ -571,6 +576,7 @@ static void skl_set_power_well(struct > drm_i915_private *dev_priv, > case SKL_DISP_PW_DDI_C: > case SKL_DISP_PW_DDI_D: > case SKL_DISP_PW_MISC_IO: > + case SKL_DISP_PW_DC_OFF: > break; > default: > WARN(1, "Unknown power well %lu\n", power_well > ->data); > @@ -589,17 +595,13 @@ static void skl_set_power_well(struct > drm_i915_private *dev_priv, > "Invalid for power well status to be > enabled, unless done by the BIOS, \ > when request is to disable!\n"); > if (power_well->data == SKL_DISP_PW_2) { > - if (GEN9_ENABLE_DC5(dev)) > - gen9_disable_dc5(dev_priv); > - if (SKL_ENABLE_DC6(dev)) { > - /* > - * DDI buffer programming > unnecessary during driver-load/resume > - * as it's already done > during modeset initialization then. > - * It's also invalid here as > encoder list is still uninitialized. > - */ > - if (!dev_priv > ->power_domains.initializing) > - intel_prepare_ddi(de > v); > - } > + /* > + * DDI buffer programming > unnecessary during driver-load/resume > + * as it's already done during > modeset initialization then. > + * It's also invalid here as encoder > list is still uninitialized. > + */ > + if (!dev_priv > ->power_domains.initializing) > + intel_prepare_ddi(dev); > } > I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | > req_mask); > } > @@ -617,10 +619,6 @@ static void skl_set_power_well(struct > drm_i915_private *dev_priv, > I915_WRITE(HSW_PWR_WELL_DRIVER, tmp & > ~req_mask); > POSTING_READ(HSW_PWR_WELL_DRIVER); > DRM_DEBUG_KMS("Disabling %s\n", power_well > ->name); > - > - if (GEN9_ENABLE_DC5(dev) && > - power_well->data == SKL_DISP_PW_2) > - gen9_enable_dc5(dev_priv); > } > } > > @@ -695,6 +693,40 @@ static void skl_power_well_disable(struct > drm_i915_private *dev_priv, > skl_set_power_well(dev_priv, power_well, false); > } > > +static bool gen9_dc_off_power_well_enabled(struct drm_i915_private > *dev_priv, > + struct i915_power_well > *power_well) > +{ > + return (I915_READ(DC_STATE_EN) & > DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0; > +} > + > +static void gen9_dc_off_power_well_enable(struct drm_i915_private > *dev_priv, > + struct i915_power_well > *power_well) > +{ > + gen9_disable_dc5_dc6(dev_priv); > +} > + > +static void gen9_dc_off_power_well_disable(struct drm_i915_private > *dev_priv, > + struct i915_power_well > *power_well) > +{ > + if (IS_SKYLAKE(dev_priv)) > + skl_enable_dc6(dev_priv); > + else > + gen9_enable_dc5(dev_priv); > +} > + > +static void gen9_dc_off_power_well_sync_hw(struct drm_i915_private > *dev_priv, > + struct i915_power_well > *power_well) > +{ > + if (power_well->count > 0) { > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > + } else { > + if (IS_SKYLAKE(dev_priv)) > + gen9_set_dc_state(dev_priv, > DC_STATE_EN_UPTO_DC6); > + else > + gen9_set_dc_state(dev_priv, > DC_STATE_EN_UPTO_DC5); > + } > +} > + > static void i9xx_always_on_power_well_noop(struct drm_i915_private > *dev_priv, > struct i915_power_well > *power_well) > { > @@ -1517,6 +1549,13 @@ static const struct i915_power_well_ops > skl_power_well_ops = { > .is_enabled = skl_power_well_enabled, > }; > > +static const struct i915_power_well_ops gen9_dc_off_power_well_ops = > { > + .sync_hw = gen9_dc_off_power_well_sync_hw, > + .enable = gen9_dc_off_power_well_enable, > + .disable = gen9_dc_off_power_well_disable, > + .is_enabled = gen9_dc_off_power_well_enabled, > +}; > + > static struct i915_power_well hsw_power_wells[] = { > { > .name = "always-on", > @@ -1697,6 +1736,12 @@ static struct i915_power_well > skl_power_wells[] = { > .data = SKL_DISP_PW_2, > }, > { > + .name = "DC off", > + .domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS, > + .ops = &gen9_dc_off_power_well_ops, > + .data = SKL_DISP_PW_DC_OFF, > + }, > + { > .name = "DDI A/E power well", > .domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS, > .ops = &skl_power_well_ops, > @@ -1769,7 +1814,14 @@ static struct i915_power_well > bxt_power_wells[] = { > .domains = BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS, > .ops = &skl_power_well_ops, > .data = SKL_DISP_PW_2, > - } > + }, > + { > + .name = "DC off", > + .domains = BXT_DISPLAY_DC_OFF_POWER_DOMAINS, > + .ops = &gen9_dc_off_power_well_ops, > + .data = SKL_DISP_PW_DC_OFF, > + }, > + > }; > > #define set_power_wells(power_domains, __power_wells) ({ > \
On ma, 2015-11-09 at 16:48 +0100, Patrik Jakobsson wrote: > Handle DC off as a power well where enabling the power well will > prevent > the DMC to enter selected DC states (required around modesets and Aux > A). Disabling the power well will allow DC states again. For now the > highest DC state is DC6 for Skylake and DC5 for Broxton but will be > configurable for Skylake in a later patch. > > v2: Check both DC5 and DC6 bits in power well enabled function > (Ville) > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 6 -- > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/intel_display.c | 6 ++ > drivers/gpu/drm/i915/intel_runtime_pm.c | 110 > +++++++++++++++++++++++--------- > 4 files changed, 88 insertions(+), 35 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > b/drivers/gpu/drm/i915/i915_drv.c > index 5a63f9a..0c7f435 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1072,9 +1072,6 @@ static int i915_pm_resume(struct device *dev) > > static int skl_suspend_complete(struct drm_i915_private *dev_priv) > { > - if (dev_priv->csr.dmc_payload) > - skl_enable_dc6(dev_priv); > - > return 0; > } > > @@ -1119,9 +1116,6 @@ static int bxt_resume_prepare(struct > drm_i915_private *dev_priv) > > static int skl_resume_prepare(struct drm_i915_private *dev_priv) > { > - if (dev_priv->csr.dmc_payload) > - skl_disable_dc6(dev_priv); > - > return 0; > } > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > index 31b3a84..df445ba 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -636,6 +636,7 @@ enum skl_disp_power_wells { > > /* Not actual bit groups. Used as IDs for > lookup_power_well() */ > SKL_DISP_PW_ALWAYS_ON, > + SKL_DISP_PW_DC_OFF, > }; > > #define SKL_POWER_WELL_STATE(pw) (1 << ((pw) * 2)) > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 649ac34..856d801 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13320,6 +13320,9 @@ static int intel_atomic_commit(struct > drm_device *dev, > to_intel_crtc_state(crtc->state) > ->update_pipe; > unsigned long put_domains = 0; > > + if (modeset) > + intel_display_power_get(dev_priv, > POWER_DOMAIN_MODESET); > + > if (modeset && crtc->state->active) { > update_scanline_offset(to_intel_crtc(crtc)); > dev_priv->display.crtc_enable(crtc); > @@ -13343,6 +13346,9 @@ static int intel_atomic_commit(struct > drm_device *dev, > modeset_put_power_domains(dev_priv, > put_domains); > > intel_post_plane_update(intel_crtc); > + > + if (modeset) > + intel_display_power_put(dev_priv, > POWER_DOMAIN_MODESET); > } > > /* FIXME: add subpixel order */ > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index edf753e..95c3fcc 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -49,9 +49,6 @@ > * present for a given platform. > */ > > -#define GEN9_ENABLE_DC5(dev) 0 > -#define SKL_ENABLE_DC6(dev) IS_SKYLAKE(dev) > - > #define for_each_power_well(i, power_well, domain_mask, > power_domains) \ > for (i = 0; > \ > i < (power_domains)->power_well_count && > \ > @@ -309,9 +306,14 @@ static void hsw_set_power_well(struct > drm_i915_private *dev_priv, > #define SKL_DISPLAY_DDI_D_POWER_DOMAINS ( \ > BIT(POWER_DOMAIN_PORT_DDI_D_LANES) | \ > BIT(POWER_DOMAIN_INIT)) > +#define SKL_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > + BIT(POWER_DOMAIN_MODESET) | \ > + BIT(POWER_DOMAIN_AUX_A) | \ > + BIT(POWER_DOMAIN_INIT)) > #define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS ( \ > (POWER_DOMAIN_MASK & ~( \ > - SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS)) | \ > + SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS | \ > + SKL_DISPLAY_DC_OFF_POWER_DOMAINS)) | \ > BIT(POWER_DOMAIN_INIT)) > > #define BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS ( \ > @@ -339,6 +341,10 @@ static void hsw_set_power_well(struct > drm_i915_private *dev_priv, > BIT(POWER_DOMAIN_AUX_A) | \ > BIT(POWER_DOMAIN_PLLS) | \ > BIT(POWER_DOMAIN_INIT)) > +#define BXT_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > + BIT(POWER_DOMAIN_MODESET) | \ > + BIT(POWER_DOMAIN_AUX_A) | \ > + BIT(POWER_DOMAIN_INIT)) > #define BXT_DISPLAY_ALWAYS_ON_POWER_DOMAINS ( \ > (POWER_DOMAIN_MASK & ~(BXT_DISPLAY_POWERWELL_1_POWER_DOMAINS > | \ > BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS)) | \ > @@ -486,15 +492,6 @@ static void gen9_enable_dc5(struct > drm_i915_private *dev_priv) > gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC5); > } > > -static void gen9_disable_dc5(struct drm_i915_private *dev_priv) > -{ > - assert_can_disable_dc5(dev_priv); > - > - DRM_DEBUG_KMS("Disabling DC5\n"); > - > - gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > -} > - > static void assert_can_enable_dc6(struct drm_i915_private *dev_priv) > { > struct drm_device *dev = dev_priv->dev; > @@ -522,6 +519,14 @@ static void assert_can_disable_dc6(struct > drm_i915_private *dev_priv) > "DC6 already programmed to be disabled.\n"); > } > > +static void gen9_disable_dc5_dc6(struct drm_i915_private *dev_priv) > +{ > + assert_can_disable_dc5(dev_priv); > + assert_can_disable_dc6(dev_priv); > + > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > +} > + > void skl_enable_dc6(struct drm_i915_private *dev_priv) > { > assert_can_enable_dc6(dev_priv); > @@ -571,6 +576,7 @@ static void skl_set_power_well(struct > drm_i915_private *dev_priv, > case SKL_DISP_PW_DDI_C: > case SKL_DISP_PW_DDI_D: > case SKL_DISP_PW_MISC_IO: > + case SKL_DISP_PW_DC_OFF: Why is this needed? I think this function won't be called for DC5/6. > break; > default: > WARN(1, "Unknown power well %lu\n", power_well > ->data); > @@ -589,17 +595,13 @@ static void skl_set_power_well(struct > drm_i915_private *dev_priv, > "Invalid for power well status to be > enabled, unless done by the BIOS, \ > when request is to disable!\n"); > if (power_well->data == SKL_DISP_PW_2) { > - if (GEN9_ENABLE_DC5(dev)) > - gen9_disable_dc5(dev_priv); > - if (SKL_ENABLE_DC6(dev)) { > - /* > - * DDI buffer programming > unnecessary during driver-load/resume > - * as it's already done > during modeset initialization then. > - * It's also invalid here as > encoder list is still uninitialized. > - */ > - if (!dev_priv > ->power_domains.initializing) > - intel_prepare_ddi(de > v); > - } > + /* > + * DDI buffer programming > unnecessary during driver-load/resume > + * as it's already done during > modeset initialization then. > + * It's also invalid here as encoder > list is still uninitialized. > + */ > + if (!dev_priv > ->power_domains.initializing) > + intel_prepare_ddi(dev); > } > I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | > req_mask); > } > @@ -617,10 +619,6 @@ static void skl_set_power_well(struct > drm_i915_private *dev_priv, > I915_WRITE(HSW_PWR_WELL_DRIVER, tmp & > ~req_mask); > POSTING_READ(HSW_PWR_WELL_DRIVER); > DRM_DEBUG_KMS("Disabling %s\n", power_well > ->name); > - > - if (GEN9_ENABLE_DC5(dev) && > - power_well->data == SKL_DISP_PW_2) > - gen9_enable_dc5(dev_priv); > } > } > > @@ -695,6 +693,40 @@ static void skl_power_well_disable(struct > drm_i915_private *dev_priv, > skl_set_power_well(dev_priv, power_well, false); > } > > +static bool gen9_dc_off_power_well_enabled(struct drm_i915_private > *dev_priv, > + struct i915_power_well > *power_well) > +{ > + return (I915_READ(DC_STATE_EN) & > DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0; > +} > + > +static void gen9_dc_off_power_well_enable(struct drm_i915_private > *dev_priv, > + struct i915_power_well > *power_well) > +{ > + gen9_disable_dc5_dc6(dev_priv); > +} > + > +static void gen9_dc_off_power_well_disable(struct drm_i915_private > *dev_priv, > + struct i915_power_well > *power_well) > +{ > + if (IS_SKYLAKE(dev_priv)) > + skl_enable_dc6(dev_priv); > + else > + gen9_enable_dc5(dev_priv); > +} > + > +static void gen9_dc_off_power_well_sync_hw(struct drm_i915_private > *dev_priv, > + struct i915_power_well > *power_well) > +{ > + if (power_well->count > 0) { > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > + } else { > + if (IS_SKYLAKE(dev_priv)) > + gen9_set_dc_state(dev_priv, > DC_STATE_EN_UPTO_DC6); > + else > + gen9_set_dc_state(dev_priv, > DC_STATE_EN_UPTO_DC5); > + } > +} > + > static void i9xx_always_on_power_well_noop(struct drm_i915_private > *dev_priv, > struct i915_power_well > *power_well) > { > @@ -1517,6 +1549,13 @@ static const struct i915_power_well_ops > skl_power_well_ops = { > .is_enabled = skl_power_well_enabled, > }; > > +static const struct i915_power_well_ops gen9_dc_off_power_well_ops = > { > + .sync_hw = gen9_dc_off_power_well_sync_hw, > + .enable = gen9_dc_off_power_well_enable, > + .disable = gen9_dc_off_power_well_disable, > + .is_enabled = gen9_dc_off_power_well_enabled, > +}; > + > static struct i915_power_well hsw_power_wells[] = { > { > .name = "always-on", > @@ -1697,6 +1736,12 @@ static struct i915_power_well > skl_power_wells[] = { > .data = SKL_DISP_PW_2, > }, > { > + .name = "DC off", > + .domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS, > + .ops = &gen9_dc_off_power_well_ops, > + .data = SKL_DISP_PW_DC_OFF, > + }, > + { > .name = "DDI A/E power well", > .domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS, > .ops = &skl_power_well_ops, > @@ -1769,7 +1814,14 @@ static struct i915_power_well > bxt_power_wells[] = { > .domains = BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS, > .ops = &skl_power_well_ops, > .data = SKL_DISP_PW_2, > - } > + }, > + { > + .name = "DC off", > + .domains = BXT_DISPLAY_DC_OFF_POWER_DOMAINS, > + .ops = &gen9_dc_off_power_well_ops, > + .data = SKL_DISP_PW_DC_OFF, > + }, > + > }; > > #define set_power_wells(power_domains, __power_wells) ({ > \
On Wed, Nov 11, 2015 at 08:57:19PM +0200, Imre Deak wrote: > On ma, 2015-11-09 at 16:48 +0100, Patrik Jakobsson wrote: > > Handle DC off as a power well where enabling the power well will > > prevent > > the DMC to enter selected DC states (required around modesets and Aux > > A). Disabling the power well will allow DC states again. For now the > > highest DC state is DC6 for Skylake and DC5 for Broxton but will be > > configurable for Skylake in a later patch. > > > > v2: Check both DC5 and DC6 bits in power well enabled function > > (Ville) > > > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.c | 6 -- > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > drivers/gpu/drm/i915/intel_display.c | 6 ++ > > drivers/gpu/drm/i915/intel_runtime_pm.c | 110 > > +++++++++++++++++++++++--------- > > 4 files changed, 88 insertions(+), 35 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c > > index 5a63f9a..0c7f435 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -1072,9 +1072,6 @@ static int i915_pm_resume(struct device *dev) > > > > static int skl_suspend_complete(struct drm_i915_private *dev_priv) > > { > > - if (dev_priv->csr.dmc_payload) > > - skl_enable_dc6(dev_priv); > > - > > return 0; > > } > > > > @@ -1119,9 +1116,6 @@ static int bxt_resume_prepare(struct > > drm_i915_private *dev_priv) > > > > static int skl_resume_prepare(struct drm_i915_private *dev_priv) > > { > > - if (dev_priv->csr.dmc_payload) > > - skl_disable_dc6(dev_priv); > > - > > return 0; > > } > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index 31b3a84..df445ba 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -636,6 +636,7 @@ enum skl_disp_power_wells { > > > > /* Not actual bit groups. Used as IDs for > > lookup_power_well() */ > > SKL_DISP_PW_ALWAYS_ON, > > + SKL_DISP_PW_DC_OFF, > > Imo it would be less confusing to call it DC3 power well. Looking at th > e DC spec, DC4 is only a transitory state to DC5/6, so what we expect > when we disable DC6/5 is DC3 or shallower power states (DC2/1/0). I've been changing the name quite a few times but settled on "DC off" to keep it generic. The main mechanism for the power well is to prevent any DC states that can cause us problems during certain operations (i.e. modeset). The DC states we need to block could change or be different between platforms. For that reason I would prefer not to be as specific with the naming. > > > }; > > > > #define SKL_POWER_WELL_STATE(pw) (1 << ((pw) * 2)) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 649ac34..856d801 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -13320,6 +13320,9 @@ static int intel_atomic_commit(struct > > drm_device *dev, > > to_intel_crtc_state(crtc->state) > > ->update_pipe; > > unsigned long put_domains = 0; > > > > + if (modeset) > > + intel_display_power_get(dev_priv, > > POWER_DOMAIN_MODESET); > > + > > if (modeset && crtc->state->active) { > > update_scanline_offset(to_intel_crtc(crtc)); > > dev_priv->display.crtc_enable(crtc); > > @@ -13343,6 +13346,9 @@ static int intel_atomic_commit(struct > > drm_device *dev, > > modeset_put_power_domains(dev_priv, > > put_domains); > > > > intel_post_plane_update(intel_crtc); > > + > > + if (modeset) > > + intel_display_power_put(dev_priv, > > POWER_DOMAIN_MODESET); > > } > > > > /* FIXME: add subpixel order */ > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index edf753e..95c3fcc 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -49,9 +49,6 @@ > > * present for a given platform. > > */ > > > > -#define GEN9_ENABLE_DC5(dev) 0 > > -#define SKL_ENABLE_DC6(dev) IS_SKYLAKE(dev) > > - > > #define for_each_power_well(i, power_well, domain_mask, > > power_domains) \ > > for (i = 0; > > \ > > i < (power_domains)->power_well_count && > > \ > > @@ -309,9 +306,14 @@ static void hsw_set_power_well(struct > > drm_i915_private *dev_priv, > > #define SKL_DISPLAY_DDI_D_POWER_DOMAINS ( \ > > BIT(POWER_DOMAIN_PORT_DDI_D_LANES) | \ > > BIT(POWER_DOMAIN_INIT)) > > +#define SKL_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > > + BIT(POWER_DOMAIN_MODESET) | \ > > + BIT(POWER_DOMAIN_AUX_A) | \ > > + BIT(POWER_DOMAIN_INIT)) > > #define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS ( \ > > (POWER_DOMAIN_MASK & ~( \ > > - SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS)) | \ > > + SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS | \ > > + SKL_DISPLAY_DC_OFF_POWER_DOMAINS)) | \ > > BIT(POWER_DOMAIN_INIT)) > > > > #define BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS ( \ > > @@ -339,6 +341,10 @@ static void hsw_set_power_well(struct > > drm_i915_private *dev_priv, > > BIT(POWER_DOMAIN_AUX_A) | \ > > BIT(POWER_DOMAIN_PLLS) | \ > > BIT(POWER_DOMAIN_INIT)) > > +#define BXT_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > > + BIT(POWER_DOMAIN_MODESET) | \ > > + BIT(POWER_DOMAIN_AUX_A) | \ > > + BIT(POWER_DOMAIN_INIT)) > > #define BXT_DISPLAY_ALWAYS_ON_POWER_DOMAINS ( \ > > (POWER_DOMAIN_MASK & ~(BXT_DISPLAY_POWERWELL_1_POWER_DOMAINS > > | \ > > BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS)) | \ > > @@ -486,15 +492,6 @@ static void gen9_enable_dc5(struct > > drm_i915_private *dev_priv) > > gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC5); > > } > > > > -static void gen9_disable_dc5(struct drm_i915_private *dev_priv) > > -{ > > - assert_can_disable_dc5(dev_priv); > > - > > - DRM_DEBUG_KMS("Disabling DC5\n"); > > - > > - gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > > -} > > - > > static void assert_can_enable_dc6(struct drm_i915_private *dev_priv) > > { > > struct drm_device *dev = dev_priv->dev; > > @@ -522,6 +519,14 @@ static void assert_can_disable_dc6(struct > > drm_i915_private *dev_priv) > > "DC6 already programmed to be disabled.\n"); > > } > > > > +static void gen9_disable_dc5_dc6(struct drm_i915_private *dev_priv) > > +{ > > + assert_can_disable_dc5(dev_priv); > > + assert_can_disable_dc6(dev_priv); > > + > > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > > +} > > + > > void skl_enable_dc6(struct drm_i915_private *dev_priv) > > { > > assert_can_enable_dc6(dev_priv); > > @@ -571,6 +576,7 @@ static void skl_set_power_well(struct > > drm_i915_private *dev_priv, > > case SKL_DISP_PW_DDI_C: > > case SKL_DISP_PW_DDI_D: > > case SKL_DISP_PW_MISC_IO: > > + case SKL_DISP_PW_DC_OFF: > > break; > > default: > > WARN(1, "Unknown power well %lu\n", power_well > > ->data); > > @@ -589,17 +595,13 @@ static void skl_set_power_well(struct > > drm_i915_private *dev_priv, > > "Invalid for power well status to be > > enabled, unless done by the BIOS, \ > > when request is to disable!\n"); > > if (power_well->data == SKL_DISP_PW_2) { > > - if (GEN9_ENABLE_DC5(dev)) > > - gen9_disable_dc5(dev_priv); > > - if (SKL_ENABLE_DC6(dev)) { > > - /* > > - * DDI buffer programming > > unnecessary during driver-load/resume > > - * as it's already done > > during modeset initialization then. > > - * It's also invalid here as > > encoder list is still uninitialized. > > - */ > > - if (!dev_priv > > ->power_domains.initializing) > > - intel_prepare_ddi(de > > v); > > - } > > + /* > > + * DDI buffer programming > > unnecessary during driver-load/resume > > + * as it's already done during > > modeset initialization then. > > + * It's also invalid here as encoder > > list is still uninitialized. > > + */ > > + if (!dev_priv > > ->power_domains.initializing) > > + intel_prepare_ddi(dev); > > } > > I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | > > req_mask); > > } > > @@ -617,10 +619,6 @@ static void skl_set_power_well(struct > > drm_i915_private *dev_priv, > > I915_WRITE(HSW_PWR_WELL_DRIVER, tmp & > > ~req_mask); > > POSTING_READ(HSW_PWR_WELL_DRIVER); > > DRM_DEBUG_KMS("Disabling %s\n", power_well > > ->name); > > - > > - if (GEN9_ENABLE_DC5(dev) && > > - power_well->data == SKL_DISP_PW_2) > > - gen9_enable_dc5(dev_priv); > > } > > } > > > > @@ -695,6 +693,40 @@ static void skl_power_well_disable(struct > > drm_i915_private *dev_priv, > > skl_set_power_well(dev_priv, power_well, false); > > } > > > > +static bool gen9_dc_off_power_well_enabled(struct drm_i915_private > > *dev_priv, > > + struct i915_power_well > > *power_well) > > +{ > > + return (I915_READ(DC_STATE_EN) & > > DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0; > > +} > > + > > +static void gen9_dc_off_power_well_enable(struct drm_i915_private > > *dev_priv, > > + struct i915_power_well > > *power_well) > > +{ > > + gen9_disable_dc5_dc6(dev_priv); > > +} > > + > > +static void gen9_dc_off_power_well_disable(struct drm_i915_private > > *dev_priv, > > + struct i915_power_well > > *power_well) > > +{ > > + if (IS_SKYLAKE(dev_priv)) > > + skl_enable_dc6(dev_priv); > > + else > > + gen9_enable_dc5(dev_priv); > > +} > > + > > +static void gen9_dc_off_power_well_sync_hw(struct drm_i915_private > > *dev_priv, > > + struct i915_power_well > > *power_well) > > +{ > > + if (power_well->count > 0) { > > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > > + } else { > > + if (IS_SKYLAKE(dev_priv)) > > + gen9_set_dc_state(dev_priv, > > DC_STATE_EN_UPTO_DC6); > > + else > > + gen9_set_dc_state(dev_priv, > > DC_STATE_EN_UPTO_DC5); > > + } > > +} > > + > > static void i9xx_always_on_power_well_noop(struct drm_i915_private > > *dev_priv, > > struct i915_power_well > > *power_well) > > { > > @@ -1517,6 +1549,13 @@ static const struct i915_power_well_ops > > skl_power_well_ops = { > > .is_enabled = skl_power_well_enabled, > > }; > > > > +static const struct i915_power_well_ops gen9_dc_off_power_well_ops = > > { > > + .sync_hw = gen9_dc_off_power_well_sync_hw, > > + .enable = gen9_dc_off_power_well_enable, > > + .disable = gen9_dc_off_power_well_disable, > > + .is_enabled = gen9_dc_off_power_well_enabled, > > +}; > > + > > static struct i915_power_well hsw_power_wells[] = { > > { > > .name = "always-on", > > @@ -1697,6 +1736,12 @@ static struct i915_power_well > > skl_power_wells[] = { > > .data = SKL_DISP_PW_2, > > }, > > { > > + .name = "DC off", > > + .domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS, > > + .ops = &gen9_dc_off_power_well_ops, > > + .data = SKL_DISP_PW_DC_OFF, > > + }, > > + { > > .name = "DDI A/E power well", > > .domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS, > > .ops = &skl_power_well_ops, > > @@ -1769,7 +1814,14 @@ static struct i915_power_well > > bxt_power_wells[] = { > > .domains = BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS, > > .ops = &skl_power_well_ops, > > .data = SKL_DISP_PW_2, > > - } > > + }, > > + { > > + .name = "DC off", > > + .domains = BXT_DISPLAY_DC_OFF_POWER_DOMAINS, > > + .ops = &gen9_dc_off_power_well_ops, > > + .data = SKL_DISP_PW_DC_OFF, > > + }, > > + > > }; > > > > #define set_power_wells(power_domains, __power_wells) ({ > > \
On Wed, Nov 11, 2015 at 09:23:32PM +0200, Imre Deak wrote: > On ma, 2015-11-09 at 16:48 +0100, Patrik Jakobsson wrote: > > Handle DC off as a power well where enabling the power well will > > prevent > > the DMC to enter selected DC states (required around modesets and Aux > > A). Disabling the power well will allow DC states again. For now the > > highest DC state is DC6 for Skylake and DC5 for Broxton but will be > > configurable for Skylake in a later patch. > > > > v2: Check both DC5 and DC6 bits in power well enabled function > > (Ville) > > > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.c | 6 -- > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > drivers/gpu/drm/i915/intel_display.c | 6 ++ > > drivers/gpu/drm/i915/intel_runtime_pm.c | 110 > > +++++++++++++++++++++++--------- > > 4 files changed, 88 insertions(+), 35 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c > > index 5a63f9a..0c7f435 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -1072,9 +1072,6 @@ static int i915_pm_resume(struct device *dev) > > > > static int skl_suspend_complete(struct drm_i915_private *dev_priv) > > { > > - if (dev_priv->csr.dmc_payload) > > - skl_enable_dc6(dev_priv); > > - > > return 0; > > } > > > > @@ -1119,9 +1116,6 @@ static int bxt_resume_prepare(struct > > drm_i915_private *dev_priv) > > > > static int skl_resume_prepare(struct drm_i915_private *dev_priv) > > { > > - if (dev_priv->csr.dmc_payload) > > - skl_disable_dc6(dev_priv); > > - > > return 0; > > } > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index 31b3a84..df445ba 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -636,6 +636,7 @@ enum skl_disp_power_wells { > > > > /* Not actual bit groups. Used as IDs for > > lookup_power_well() */ > > SKL_DISP_PW_ALWAYS_ON, > > + SKL_DISP_PW_DC_OFF, > > }; > > > > #define SKL_POWER_WELL_STATE(pw) (1 << ((pw) * 2)) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 649ac34..856d801 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -13320,6 +13320,9 @@ static int intel_atomic_commit(struct > > drm_device *dev, > > to_intel_crtc_state(crtc->state) > > ->update_pipe; > > unsigned long put_domains = 0; > > > > + if (modeset) > > + intel_display_power_get(dev_priv, > > POWER_DOMAIN_MODESET); > > + > > if (modeset && crtc->state->active) { > > update_scanline_offset(to_intel_crtc(crtc)); > > dev_priv->display.crtc_enable(crtc); > > @@ -13343,6 +13346,9 @@ static int intel_atomic_commit(struct > > drm_device *dev, > > modeset_put_power_domains(dev_priv, > > put_domains); > > > > intel_post_plane_update(intel_crtc); > > + > > + if (modeset) > > + intel_display_power_put(dev_priv, > > POWER_DOMAIN_MODESET); > > } > > > > /* FIXME: add subpixel order */ > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index edf753e..95c3fcc 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -49,9 +49,6 @@ > > * present for a given platform. > > */ > > > > -#define GEN9_ENABLE_DC5(dev) 0 > > -#define SKL_ENABLE_DC6(dev) IS_SKYLAKE(dev) > > - > > #define for_each_power_well(i, power_well, domain_mask, > > power_domains) \ > > for (i = 0; > > \ > > i < (power_domains)->power_well_count && > > \ > > @@ -309,9 +306,14 @@ static void hsw_set_power_well(struct > > drm_i915_private *dev_priv, > > #define SKL_DISPLAY_DDI_D_POWER_DOMAINS ( \ > > BIT(POWER_DOMAIN_PORT_DDI_D_LANES) | \ > > BIT(POWER_DOMAIN_INIT)) > > +#define SKL_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > > + BIT(POWER_DOMAIN_MODESET) | \ > > + BIT(POWER_DOMAIN_AUX_A) | \ > > + BIT(POWER_DOMAIN_INIT)) > > #define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS ( \ > > (POWER_DOMAIN_MASK & ~( \ > > - SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS)) | \ > > + SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS | \ > > + SKL_DISPLAY_DC_OFF_POWER_DOMAINS)) | \ > > BIT(POWER_DOMAIN_INIT)) > > > > #define BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS ( \ > > @@ -339,6 +341,10 @@ static void hsw_set_power_well(struct > > drm_i915_private *dev_priv, > > BIT(POWER_DOMAIN_AUX_A) | \ > > BIT(POWER_DOMAIN_PLLS) | \ > > BIT(POWER_DOMAIN_INIT)) > > +#define BXT_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > > + BIT(POWER_DOMAIN_MODESET) | \ > > + BIT(POWER_DOMAIN_AUX_A) | \ > > + BIT(POWER_DOMAIN_INIT)) > > #define BXT_DISPLAY_ALWAYS_ON_POWER_DOMAINS ( \ > > (POWER_DOMAIN_MASK & ~(BXT_DISPLAY_POWERWELL_1_POWER_DOMAINS > > | \ > > BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS)) | \ > > @@ -486,15 +492,6 @@ static void gen9_enable_dc5(struct > > drm_i915_private *dev_priv) > > gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC5); > > } > > > > -static void gen9_disable_dc5(struct drm_i915_private *dev_priv) > > -{ > > - assert_can_disable_dc5(dev_priv); > > - > > - DRM_DEBUG_KMS("Disabling DC5\n"); > > - > > - gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > > -} > > - > > static void assert_can_enable_dc6(struct drm_i915_private *dev_priv) > > { > > struct drm_device *dev = dev_priv->dev; > > @@ -522,6 +519,14 @@ static void assert_can_disable_dc6(struct > > drm_i915_private *dev_priv) > > "DC6 already programmed to be disabled.\n"); > > } > > > > +static void gen9_disable_dc5_dc6(struct drm_i915_private *dev_priv) > > +{ > > + assert_can_disable_dc5(dev_priv); > > + assert_can_disable_dc6(dev_priv); > > + > > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > > +} > > + > > void skl_enable_dc6(struct drm_i915_private *dev_priv) > > { > > assert_can_enable_dc6(dev_priv); > > @@ -571,6 +576,7 @@ static void skl_set_power_well(struct > > drm_i915_private *dev_priv, > > case SKL_DISP_PW_DDI_C: > > case SKL_DISP_PW_DDI_D: > > case SKL_DISP_PW_MISC_IO: > > + case SKL_DISP_PW_DC_OFF: > > Why is this needed? I think this function won't be called for DC5/6. You're right, it's not needed. I'll remove it. > > > break; > > default: > > WARN(1, "Unknown power well %lu\n", power_well > > ->data); > > @@ -589,17 +595,13 @@ static void skl_set_power_well(struct > > drm_i915_private *dev_priv, > > "Invalid for power well status to be > > enabled, unless done by the BIOS, \ > > when request is to disable!\n"); > > if (power_well->data == SKL_DISP_PW_2) { > > - if (GEN9_ENABLE_DC5(dev)) > > - gen9_disable_dc5(dev_priv); > > - if (SKL_ENABLE_DC6(dev)) { > > - /* > > - * DDI buffer programming > > unnecessary during driver-load/resume > > - * as it's already done > > during modeset initialization then. > > - * It's also invalid here as > > encoder list is still uninitialized. > > - */ > > - if (!dev_priv > > ->power_domains.initializing) > > - intel_prepare_ddi(de > > v); > > - } > > + /* > > + * DDI buffer programming > > unnecessary during driver-load/resume > > + * as it's already done during > > modeset initialization then. > > + * It's also invalid here as encoder > > list is still uninitialized. > > + */ > > + if (!dev_priv > > ->power_domains.initializing) > > + intel_prepare_ddi(dev); > > } > > I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | > > req_mask); > > } > > @@ -617,10 +619,6 @@ static void skl_set_power_well(struct > > drm_i915_private *dev_priv, > > I915_WRITE(HSW_PWR_WELL_DRIVER, tmp & > > ~req_mask); > > POSTING_READ(HSW_PWR_WELL_DRIVER); > > DRM_DEBUG_KMS("Disabling %s\n", power_well > > ->name); > > - > > - if (GEN9_ENABLE_DC5(dev) && > > - power_well->data == SKL_DISP_PW_2) > > - gen9_enable_dc5(dev_priv); > > } > > } > > > > @@ -695,6 +693,40 @@ static void skl_power_well_disable(struct > > drm_i915_private *dev_priv, > > skl_set_power_well(dev_priv, power_well, false); > > } > > > > +static bool gen9_dc_off_power_well_enabled(struct drm_i915_private > > *dev_priv, > > + struct i915_power_well > > *power_well) > > +{ > > + return (I915_READ(DC_STATE_EN) & > > DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0; > > +} > > + > > +static void gen9_dc_off_power_well_enable(struct drm_i915_private > > *dev_priv, > > + struct i915_power_well > > *power_well) > > +{ > > + gen9_disable_dc5_dc6(dev_priv); > > +} > > + > > +static void gen9_dc_off_power_well_disable(struct drm_i915_private > > *dev_priv, > > + struct i915_power_well > > *power_well) > > +{ > > + if (IS_SKYLAKE(dev_priv)) > > + skl_enable_dc6(dev_priv); > > + else > > + gen9_enable_dc5(dev_priv); > > +} > > + > > +static void gen9_dc_off_power_well_sync_hw(struct drm_i915_private > > *dev_priv, > > + struct i915_power_well > > *power_well) > > +{ > > + if (power_well->count > 0) { > > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > > + } else { > > + if (IS_SKYLAKE(dev_priv)) > > + gen9_set_dc_state(dev_priv, > > DC_STATE_EN_UPTO_DC6); > > + else > > + gen9_set_dc_state(dev_priv, > > DC_STATE_EN_UPTO_DC5); > > + } > > +} > > + > > static void i9xx_always_on_power_well_noop(struct drm_i915_private > > *dev_priv, > > struct i915_power_well > > *power_well) > > { > > @@ -1517,6 +1549,13 @@ static const struct i915_power_well_ops > > skl_power_well_ops = { > > .is_enabled = skl_power_well_enabled, > > }; > > > > +static const struct i915_power_well_ops gen9_dc_off_power_well_ops = > > { > > + .sync_hw = gen9_dc_off_power_well_sync_hw, > > + .enable = gen9_dc_off_power_well_enable, > > + .disable = gen9_dc_off_power_well_disable, > > + .is_enabled = gen9_dc_off_power_well_enabled, > > +}; > > + > > static struct i915_power_well hsw_power_wells[] = { > > { > > .name = "always-on", > > @@ -1697,6 +1736,12 @@ static struct i915_power_well > > skl_power_wells[] = { > > .data = SKL_DISP_PW_2, > > }, > > { > > + .name = "DC off", > > + .domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS, > > + .ops = &gen9_dc_off_power_well_ops, > > + .data = SKL_DISP_PW_DC_OFF, > > + }, > > + { > > .name = "DDI A/E power well", > > .domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS, > > .ops = &skl_power_well_ops, > > @@ -1769,7 +1814,14 @@ static struct i915_power_well > > bxt_power_wells[] = { > > .domains = BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS, > > .ops = &skl_power_well_ops, > > .data = SKL_DISP_PW_2, > > - } > > + }, > > + { > > + .name = "DC off", > > + .domains = BXT_DISPLAY_DC_OFF_POWER_DOMAINS, > > + .ops = &gen9_dc_off_power_well_ops, > > + .data = SKL_DISP_PW_DC_OFF, > > + }, > > + > > }; > > > > #define set_power_wells(power_domains, __power_wells) ({ > > \
On to, 2015-11-12 at 13:24 +0100, Patrik Jakobsson wrote: > On Wed, Nov 11, 2015 at 08:57:19PM +0200, Imre Deak wrote: > > On ma, 2015-11-09 at 16:48 +0100, Patrik Jakobsson wrote: > > > Handle DC off as a power well where enabling the power well will > > > prevent > > > the DMC to enter selected DC states (required around modesets and > > > Aux > > > A). Disabling the power well will allow DC states again. For now > > > the > > > highest DC state is DC6 for Skylake and DC5 for Broxton but will > > > be > > > configurable for Skylake in a later patch. > > > > > > v2: Check both DC5 and DC6 bits in power well enabled function > > > (Ville) > > > > > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com > > > > > > > --- > > > drivers/gpu/drm/i915/i915_drv.c | 6 -- > > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > > drivers/gpu/drm/i915/intel_display.c | 6 ++ > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 110 > > > +++++++++++++++++++++++--------- > > > 4 files changed, 88 insertions(+), 35 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > b/drivers/gpu/drm/i915/i915_drv.c > > > index 5a63f9a..0c7f435 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -1072,9 +1072,6 @@ static int i915_pm_resume(struct device > > > *dev) > > > > > > static int skl_suspend_complete(struct drm_i915_private > > > *dev_priv) > > > { > > > - if (dev_priv->csr.dmc_payload) > > > - skl_enable_dc6(dev_priv); > > > - > > > return 0; > > > } > > > > > > @@ -1119,9 +1116,6 @@ static int bxt_resume_prepare(struct > > > drm_i915_private *dev_priv) > > > > > > static int skl_resume_prepare(struct drm_i915_private *dev_priv) > > > { > > > - if (dev_priv->csr.dmc_payload) > > > - skl_disable_dc6(dev_priv); > > > - > > > return 0; > > > } > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > b/drivers/gpu/drm/i915/i915_reg.h > > > index 31b3a84..df445ba 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -636,6 +636,7 @@ enum skl_disp_power_wells { > > > > > > /* Not actual bit groups. Used as IDs for > > > lookup_power_well() */ > > > SKL_DISP_PW_ALWAYS_ON, > > > + SKL_DISP_PW_DC_OFF, > > > > Imo it would be less confusing to call it DC3 power well. Looking > > at th > > e DC spec, DC4 is only a transitory state to DC5/6, so what we > > expect > > when we disable DC6/5 is DC3 or shallower power states (DC2/1/0). > > I've been changing the name quite a few times but settled on "DC off" > to keep it > generic. The main mechanism for the power well is to prevent any DC > states that > can cause us problems during certain operations (i.e. modeset). The > DC states we > need to block could change or be different between platforms. For > that reason I > would prefer not to be as specific with the naming. All the power well names are platform specific so it would be logical and more meaningful to give this one a platform specific name too. Also 'DC off' is kind of a double negation leading to debug messages like 'enable DC off' which isn't that nice. But now that I think of it my idea isn't that great either. 'enable DC3' is confusing too, one could think we enable some power saving state at that point, which is not the case. So let's keep things as-is for now at least: Reviewed-by: Imre Deak <imre.deak@intel.com> > > > > > > }; > > > > > > #define SKL_POWER_WELL_STATE(pw) (1 << ((pw) * 2)) > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > b/drivers/gpu/drm/i915/intel_display.c > > > index 649ac34..856d801 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -13320,6 +13320,9 @@ static int intel_atomic_commit(struct > > > drm_device *dev, > > > to_intel_crtc_state(crtc->state) > > > ->update_pipe; > > > unsigned long put_domains = 0; > > > > > > + if (modeset) > > > + intel_display_power_get(dev_priv, > > > POWER_DOMAIN_MODESET); > > > + > > > if (modeset && crtc->state->active) { > > > update_scanline_offset(to_intel_crtc(crt > > > c)); > > > dev_priv->display.crtc_enable(crtc); > > > @@ -13343,6 +13346,9 @@ static int intel_atomic_commit(struct > > > drm_device *dev, > > > modeset_put_power_domains(dev_priv, > > > put_domains); > > > > > > intel_post_plane_update(intel_crtc); > > > + > > > + if (modeset) > > > + intel_display_power_put(dev_priv, > > > POWER_DOMAIN_MODESET); > > > } > > > > > > /* FIXME: add subpixel order */ > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > index edf753e..95c3fcc 100644 > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > @@ -49,9 +49,6 @@ > > > * present for a given platform. > > > */ > > > > > > -#define GEN9_ENABLE_DC5(dev) 0 > > > -#define SKL_ENABLE_DC6(dev) IS_SKYLAKE(dev) > > > - > > > #define for_each_power_well(i, power_well, domain_mask, > > > power_domains) \ > > > for (i = 0; > > > > > > \ > > > i < (power_domains)->power_well_count && > > > \ > > > @@ -309,9 +306,14 @@ static void hsw_set_power_well(struct > > > drm_i915_private *dev_priv, > > > #define SKL_DISPLAY_DDI_D_POWER_DOMAINS ( \ > > > BIT(POWER_DOMAIN_PORT_DDI_D_LANES) | \ > > > BIT(POWER_DOMAIN_INIT)) > > > +#define SKL_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > > > + BIT(POWER_DOMAIN_MODESET) | \ > > > + BIT(POWER_DOMAIN_AUX_A) | \ > > > + BIT(POWER_DOMAIN_INIT)) > > > #define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS ( \ > > > (POWER_DOMAIN_MASK & ~( \ > > > - SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS)) | \ > > > + SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS | \ > > > + SKL_DISPLAY_DC_OFF_POWER_DOMAINS)) | \ > > > BIT(POWER_DOMAIN_INIT)) > > > > > > #define BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS ( \ > > > @@ -339,6 +341,10 @@ static void hsw_set_power_well(struct > > > drm_i915_private *dev_priv, > > > BIT(POWER_DOMAIN_AUX_A) | \ > > > BIT(POWER_DOMAIN_PLLS) | \ > > > BIT(POWER_DOMAIN_INIT)) > > > +#define BXT_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > > > + BIT(POWER_DOMAIN_MODESET) | \ > > > + BIT(POWER_DOMAIN_AUX_A) | \ > > > + BIT(POWER_DOMAIN_INIT)) > > > #define BXT_DISPLAY_ALWAYS_ON_POWER_DOMAINS ( \ > > > (POWER_DOMAIN_MASK & > > > ~(BXT_DISPLAY_POWERWELL_1_POWER_DOMAINS > > > > \ > > > BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS)) | \ > > > @@ -486,15 +492,6 @@ static void gen9_enable_dc5(struct > > > drm_i915_private *dev_priv) > > > gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC5); > > > } > > > > > > -static void gen9_disable_dc5(struct drm_i915_private *dev_priv) > > > -{ > > > - assert_can_disable_dc5(dev_priv); > > > - > > > - DRM_DEBUG_KMS("Disabling DC5\n"); > > > - > > > - gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > > > -} > > > - > > > static void assert_can_enable_dc6(struct drm_i915_private > > > *dev_priv) > > > { > > > struct drm_device *dev = dev_priv->dev; > > > @@ -522,6 +519,14 @@ static void assert_can_disable_dc6(struct > > > drm_i915_private *dev_priv) > > > "DC6 already programmed to be disabled.\n"); > > > } > > > > > > +static void gen9_disable_dc5_dc6(struct drm_i915_private > > > *dev_priv) > > > +{ > > > + assert_can_disable_dc5(dev_priv); > > > + assert_can_disable_dc6(dev_priv); > > > + > > > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > > > +} > > > + > > > void skl_enable_dc6(struct drm_i915_private *dev_priv) > > > { > > > assert_can_enable_dc6(dev_priv); > > > @@ -571,6 +576,7 @@ static void skl_set_power_well(struct > > > drm_i915_private *dev_priv, > > > case SKL_DISP_PW_DDI_C: > > > case SKL_DISP_PW_DDI_D: > > > case SKL_DISP_PW_MISC_IO: > > > + case SKL_DISP_PW_DC_OFF: > > > break; > > > default: > > > WARN(1, "Unknown power well %lu\n", power_well > > > ->data); > > > @@ -589,17 +595,13 @@ static void skl_set_power_well(struct > > > drm_i915_private *dev_priv, > > > "Invalid for power well status > > > to be > > > enabled, unless done by the BIOS, \ > > > when request is to disable!\n"); > > > if (power_well->data == SKL_DISP_PW_2) { > > > - if (GEN9_ENABLE_DC5(dev)) > > > - gen9_disable_dc5(dev_pri > > > v); > > > - if (SKL_ENABLE_DC6(dev)) { > > > - /* > > > - * DDI buffer > > > programming > > > unnecessary during driver-load/resume > > > - * as it's already done > > > during modeset initialization then. > > > - * It's also invalid > > > here as > > > encoder list is still uninitialized. > > > - */ > > > - if (!dev_priv > > > ->power_domains.initializing) > > > - intel_prepare_dd > > > i(de > > > v); > > > - } > > > + /* > > > + * DDI buffer programming > > > unnecessary during driver-load/resume > > > + * as it's already done during > > > modeset initialization then. > > > + * It's also invalid here as > > > encoder > > > list is still uninitialized. > > > + */ > > > + if (!dev_priv > > > ->power_domains.initializing) > > > + intel_prepare_ddi(dev); > > > } > > > I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | > > > req_mask); > > > } > > > @@ -617,10 +619,6 @@ static void skl_set_power_well(struct > > > drm_i915_private *dev_priv, > > > I915_WRITE(HSW_PWR_WELL_DRIVER, t > > > mp & > > > ~req_mask); > > > POSTING_READ(HSW_PWR_WELL_DRIVER); > > > DRM_DEBUG_KMS("Disabling %s\n", > > > power_well > > > ->name); > > > - > > > - if (GEN9_ENABLE_DC5(dev) && > > > - power_well->data == > > > SKL_DISP_PW_2) > > > - gen9_enable_dc5(dev_priv > > > ); > > > } > > > } > > > > > > @@ -695,6 +693,40 @@ static void skl_power_well_disable(struct > > > drm_i915_private *dev_priv, > > > skl_set_power_well(dev_priv, power_well, false); > > > } > > > > > > +static bool gen9_dc_off_power_well_enabled(struct > > > drm_i915_private > > > *dev_priv, > > > + struct i915_power_well > > > *power_well) > > > +{ > > > + return (I915_READ(DC_STATE_EN) & > > > DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0; > > > +} > > > + > > > +static void gen9_dc_off_power_well_enable(struct > > > drm_i915_private > > > *dev_priv, > > > + struct i915_power_well > > > *power_well) > > > +{ > > > + gen9_disable_dc5_dc6(dev_priv); > > > +} > > > + > > > +static void gen9_dc_off_power_well_disable(struct > > > drm_i915_private > > > *dev_priv, > > > + struct i915_power_well > > > *power_well) > > > +{ > > > + if (IS_SKYLAKE(dev_priv)) > > > + skl_enable_dc6(dev_priv); > > > + else > > > + gen9_enable_dc5(dev_priv); > > > +} > > > + > > > +static void gen9_dc_off_power_well_sync_hw(struct > > > drm_i915_private > > > *dev_priv, > > > + struct i915_power_well > > > *power_well) > > > +{ > > > + if (power_well->count > 0) { > > > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > > > + } else { > > > + if (IS_SKYLAKE(dev_priv)) > > > + gen9_set_dc_state(dev_priv, > > > DC_STATE_EN_UPTO_DC6); > > > + else > > > + gen9_set_dc_state(dev_priv, > > > DC_STATE_EN_UPTO_DC5); > > > + } > > > +} > > > + > > > static void i9xx_always_on_power_well_noop(struct > > > drm_i915_private > > > *dev_priv, > > > struct > > > i915_power_well > > > *power_well) > > > { > > > @@ -1517,6 +1549,13 @@ static const struct i915_power_well_ops > > > skl_power_well_ops = { > > > .is_enabled = skl_power_well_enabled, > > > }; > > > > > > +static const struct i915_power_well_ops > > > gen9_dc_off_power_well_ops = > > > { > > > + .sync_hw = gen9_dc_off_power_well_sync_hw, > > > + .enable = gen9_dc_off_power_well_enable, > > > + .disable = gen9_dc_off_power_well_disable, > > > + .is_enabled = gen9_dc_off_power_well_enabled, > > > +}; > > > + > > > static struct i915_power_well hsw_power_wells[] = { > > > { > > > .name = "always-on", > > > @@ -1697,6 +1736,12 @@ static struct i915_power_well > > > skl_power_wells[] = { > > > .data = SKL_DISP_PW_2, > > > }, > > > { > > > + .name = "DC off", > > > + .domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS, > > > + .ops = &gen9_dc_off_power_well_ops, > > > + .data = SKL_DISP_PW_DC_OFF, > > > + }, > > > + { > > > .name = "DDI A/E power well", > > > .domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS, > > > .ops = &skl_power_well_ops, > > > @@ -1769,7 +1814,14 @@ static struct i915_power_well > > > bxt_power_wells[] = { > > > .domains = > > > BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS, > > > .ops = &skl_power_well_ops, > > > .data = SKL_DISP_PW_2, > > > - } > > > + }, > > > + { > > > + .name = "DC off", > > > + .domains = BXT_DISPLAY_DC_OFF_POWER_DOMAINS, > > > + .ops = &gen9_dc_off_power_well_ops, > > > + .data = SKL_DISP_PW_DC_OFF, > > > + }, > > > + > > > }; > > > > > > #define set_power_wells(power_domains, __power_wells) ({ > > > \
On to, 2015-11-12 at 15:30 +0200, Imre Deak wrote: > On to, 2015-11-12 at 13:24 +0100, Patrik Jakobsson wrote: > > On Wed, Nov 11, 2015 at 08:57:19PM +0200, Imre Deak wrote: > > > On ma, 2015-11-09 at 16:48 +0100, Patrik Jakobsson wrote: > > > > Handle DC off as a power well where enabling the power well > > > > will > > > > prevent > > > > the DMC to enter selected DC states (required around modesets > > > > and > > > > Aux > > > > A). Disabling the power well will allow DC states again. For > > > > now > > > > the > > > > highest DC state is DC6 for Skylake and DC5 for Broxton but > > > > will > > > > be > > > > configurable for Skylake in a later patch. > > > > > > > > v2: Check both DC5 and DC6 bits in power well enabled function > > > > (Ville) > > > > > > > > Signed-off-by: Patrik Jakobsson < > > > > patrik.jakobsson@linux.intel.com > > > > > > > > > --- > > > > drivers/gpu/drm/i915/i915_drv.c | 6 -- > > > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > > > drivers/gpu/drm/i915/intel_display.c | 6 ++ > > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 110 > > > > +++++++++++++++++++++++--------- > > > > 4 files changed, 88 insertions(+), 35 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > > b/drivers/gpu/drm/i915/i915_drv.c > > > > index 5a63f9a..0c7f435 100644 > > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > > @@ -1072,9 +1072,6 @@ static int i915_pm_resume(struct device > > > > *dev) > > > > > > > > static int skl_suspend_complete(struct drm_i915_private > > > > *dev_priv) > > > > { > > > > - if (dev_priv->csr.dmc_payload) > > > > - skl_enable_dc6(dev_priv); > > > > - > > > > return 0; > > > > } > > > > > > > > @@ -1119,9 +1116,6 @@ static int bxt_resume_prepare(struct > > > > drm_i915_private *dev_priv) > > > > > > > > static int skl_resume_prepare(struct drm_i915_private > > > > *dev_priv) > > > > { > > > > - if (dev_priv->csr.dmc_payload) > > > > - skl_disable_dc6(dev_priv); > > > > - > > > > return 0; > > > > } > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > > b/drivers/gpu/drm/i915/i915_reg.h > > > > index 31b3a84..df445ba 100644 > > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > > @@ -636,6 +636,7 @@ enum skl_disp_power_wells { > > > > > > > > /* Not actual bit groups. Used as IDs for > > > > lookup_power_well() */ > > > > SKL_DISP_PW_ALWAYS_ON, > > > > + SKL_DISP_PW_DC_OFF, > > > > > > Imo it would be less confusing to call it DC3 power well. Looking > > > at th > > > e DC spec, DC4 is only a transitory state to DC5/6, so what we > > > expect > > > when we disable DC6/5 is DC3 or shallower power states (DC2/1/0). > > > > I've been changing the name quite a few times but settled on "DC > > off" > > to keep it > > generic. The main mechanism for the power well is to prevent any DC > > states that > > can cause us problems during certain operations (i.e. modeset). The > > DC states we > > need to block could change or be different between platforms. For > > that reason I > > would prefer not to be as specific with the naming. > > All the power well names are platform specific so it would be logical > and more meaningful to give this one a platform specific name too. > Also > 'DC off' is kind of a double negation leading to debug messages like > 'enable DC off' which isn't that nice. But now that I think of it my > idea isn't that great either. 'enable DC3' is confusing too, one > could > think we enable some power saving state at that point, which is not > the > case. So let's keep things as-is for now at least: > > Reviewed-by: Imre Deak <imre.deak@intel.com> I noticed some issue only now, when testing it more: > > > }; > > > > > > > > #define SKL_POWER_WELL_STATE(pw) (1 << ((pw) * 2)) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > > b/drivers/gpu/drm/i915/intel_display.c > > > > index 649ac34..856d801 100644 > > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > > @@ -13320,6 +13320,9 @@ static int intel_atomic_commit(struct > > > > drm_device *dev, > > > > to_intel_crtc_state(crtc->state) > > > > ->update_pipe; > > > > unsigned long put_domains = 0; > > > > > > > > + if (modeset) > > > > + intel_display_power_get(dev_priv, > > > > POWER_DOMAIN_MODESET); > > > > + > > > > if (modeset && crtc->state->active) { > > > > update_scanline_offset(to_intel_crtc(c > > > > rt > > > > c)); > > > > dev_priv->display.crtc_enable(crtc); > > > > @@ -13343,6 +13346,9 @@ static int intel_atomic_commit(struct > > > > drm_device *dev, > > > > modeset_put_power_domains(dev_priv, > > > > put_domains); > > > > > > > > intel_post_plane_update(intel_crtc); > > > > + > > > > + if (modeset) > > > > + intel_display_power_put(dev_priv, > > > > POWER_DOMAIN_MODESET); > > > > } > > > > > > > > /* FIXME: add subpixel order */ > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > index edf753e..95c3fcc 100644 > > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > > @@ -49,9 +49,6 @@ > > > > * present for a given platform. > > > > */ > > > > > > > > -#define GEN9_ENABLE_DC5(dev) 0 > > > > -#define SKL_ENABLE_DC6(dev) IS_SKYLAKE(dev) > > > > - > > > > #define for_each_power_well(i, power_well, domain_mask, > > > > power_domains) \ > > > > for (i = 0; > > > > > > > > \ > > > > i < (power_domains)->power_well_count && > > > > \ > > > > @@ -309,9 +306,14 @@ static void hsw_set_power_well(struct > > > > drm_i915_private *dev_priv, > > > > #define SKL_DISPLAY_DDI_D_POWER_DOMAINS ( \ > > > > BIT(POWER_DOMAIN_PORT_DDI_D_LANES) | \ > > > > BIT(POWER_DOMAIN_INIT)) > > > > +#define SKL_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > > > > + BIT(POWER_DOMAIN_MODESET) | \ > > > > + BIT(POWER_DOMAIN_AUX_A) | \ > > > > + BIT(POWER_DOMAIN_INIT)) This needs SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS as well, so that we enable/disable DC5/6 whenever we toggle PW_2. > > > > #define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS ( \ > > > > (POWER_DOMAIN_MASK & ~( > > > > \ > > > > - SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS)) | \ > > > > + SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS | > > > > \ > > > > + SKL_DISPLAY_DC_OFF_POWER_DOMAINS)) | \ > > > > BIT(POWER_DOMAIN_INIT)) > > > > > > > > #define BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS ( > > > > \ > > > > @@ -339,6 +341,10 @@ static void hsw_set_power_well(struct > > > > drm_i915_private *dev_priv, > > > > BIT(POWER_DOMAIN_AUX_A) | \ > > > > BIT(POWER_DOMAIN_PLLS) | \ > > > > BIT(POWER_DOMAIN_INIT)) > > > > +#define BXT_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > > > > + BIT(POWER_DOMAIN_MODESET) | \ > > > > + BIT(POWER_DOMAIN_AUX_A) | \ > > > > + BIT(POWER_DOMAIN_INIT)) As above, this needs BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS. > > > > #define BXT_DISPLAY_ALWAYS_ON_POWER_DOMAINS ( \ > > > > (POWER_DOMAIN_MASK & > > > > ~(BXT_DISPLAY_POWERWELL_1_POWER_DOMAINS > > > > > \ > > > > BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS)) | \ > > > > @@ -486,15 +492,6 @@ static void gen9_enable_dc5(struct > > > > drm_i915_private *dev_priv) > > > > gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC5); > > > > } > > > > > > > > -static void gen9_disable_dc5(struct drm_i915_private > > > > *dev_priv) > > > > -{ > > > > - assert_can_disable_dc5(dev_priv); > > > > - > > > > - DRM_DEBUG_KMS("Disabling DC5\n"); > > > > - > > > > - gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > > > > -} > > > > - > > > > static void assert_can_enable_dc6(struct drm_i915_private > > > > *dev_priv) > > > > { > > > > struct drm_device *dev = dev_priv->dev; > > > > @@ -522,6 +519,14 @@ static void assert_can_disable_dc6(struct > > > > drm_i915_private *dev_priv) > > > > "DC6 already programmed to be disabled.\n"); > > > > } > > > > > > > > +static void gen9_disable_dc5_dc6(struct drm_i915_private > > > > *dev_priv) > > > > +{ > > > > + assert_can_disable_dc5(dev_priv); > > > > + assert_can_disable_dc6(dev_priv); > > > > + > > > > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > > > > +} > > > > + > > > > void skl_enable_dc6(struct drm_i915_private *dev_priv) > > > > { > > > > assert_can_enable_dc6(dev_priv); > > > > @@ -571,6 +576,7 @@ static void skl_set_power_well(struct > > > > drm_i915_private *dev_priv, > > > > case SKL_DISP_PW_DDI_C: > > > > case SKL_DISP_PW_DDI_D: > > > > case SKL_DISP_PW_MISC_IO: > > > > + case SKL_DISP_PW_DC_OFF: > > > > break; > > > > default: > > > > WARN(1, "Unknown power well %lu\n", power_well > > > > ->data); > > > > @@ -589,17 +595,13 @@ static void skl_set_power_well(struct > > > > drm_i915_private *dev_priv, > > > > "Invalid for power well status > > > > to be > > > > enabled, unless done by the BIOS, \ > > > > when request is to > > > > disable!\n"); > > > > if (power_well->data == SKL_DISP_PW_2) > > > > { > > > > - if (GEN9_ENABLE_DC5(dev)) > > > > - gen9_disable_dc5(dev_p > > > > ri > > > > v); > > > > - if (SKL_ENABLE_DC6(dev)) { > > > > - /* > > > > - * DDI buffer > > > > programming > > > > unnecessary during driver-load/resume > > > > - * as it's already > > > > done > > > > during modeset initialization then. > > > > - * It's also invalid > > > > here as > > > > encoder list is still uninitialized. > > > > - */ > > > > - if (!dev_priv > > > > ->power_domains.initializing) > > > > - intel_prepare_ > > > > dd > > > > i(de > > > > v); > > > > - } > > > > + /* > > > > + * DDI buffer programming > > > > unnecessary during driver-load/resume > > > > + * as it's already done during > > > > modeset initialization then. > > > > + * It's also invalid here as > > > > encoder > > > > list is still uninitialized. > > > > + */ > > > > + if (!dev_priv > > > > ->power_domains.initializing) > > > > + intel_prepare_ddi(dev) > > > > ; > > > > } > > > > I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | > > > > req_mask); > > > > } > > > > @@ -617,10 +619,6 @@ static void skl_set_power_well(struct > > > > drm_i915_private *dev_priv, > > > > I915_WRITE(HSW_PWR_WELL_DRIVER, > > > > t > > > > mp & > > > > ~req_mask); > > > > POSTING_READ(HSW_PWR_WELL_DRIVER); > > > > DRM_DEBUG_KMS("Disabling %s\n", > > > > power_well > > > > ->name); > > > > - > > > > - if (GEN9_ENABLE_DC5(dev) && > > > > - power_well->data == > > > > SKL_DISP_PW_2) > > > > - gen9_enable_dc5(dev_pr > > > > iv > > > > ); > > > > } > > > > } > > > > > > > > @@ -695,6 +693,40 @@ static void skl_power_well_disable(struct > > > > drm_i915_private *dev_priv, > > > > skl_set_power_well(dev_priv, power_well, false); > > > > } > > > > > > > > +static bool gen9_dc_off_power_well_enabled(struct > > > > drm_i915_private > > > > *dev_priv, > > > > + struct > > > > i915_power_well > > > > *power_well) > > > > +{ > > > > + return (I915_READ(DC_STATE_EN) & > > > > DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0; > > > > +} > > > > + > > > > +static void gen9_dc_off_power_well_enable(struct > > > > drm_i915_private > > > > *dev_priv, > > > > + struct > > > > i915_power_well > > > > *power_well) > > > > +{ > > > > + gen9_disable_dc5_dc6(dev_priv); > > > > +} > > > > + > > > > +static void gen9_dc_off_power_well_disable(struct > > > > drm_i915_private > > > > *dev_priv, > > > > + struct > > > > i915_power_well > > > > *power_well) > > > > +{ > > > > + if (IS_SKYLAKE(dev_priv)) > > > > + skl_enable_dc6(dev_priv); > > > > + else > > > > + gen9_enable_dc5(dev_priv); > > > > +} > > > > + > > > > +static void gen9_dc_off_power_well_sync_hw(struct > > > > drm_i915_private > > > > *dev_priv, > > > > + struct > > > > i915_power_well > > > > *power_well) > > > > +{ > > > > + if (power_well->count > 0) { > > > > + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > > > > + } else { > > > > + if (IS_SKYLAKE(dev_priv)) > > > > + gen9_set_dc_state(dev_priv, > > > > DC_STATE_EN_UPTO_DC6); > > > > + else > > > > + gen9_set_dc_state(dev_priv, > > > > DC_STATE_EN_UPTO_DC5); > > > > + } > > > > +} > > > > + > > > > static void i9xx_always_on_power_well_noop(struct > > > > drm_i915_private > > > > *dev_priv, > > > > struct > > > > i915_power_well > > > > *power_well) > > > > { > > > > @@ -1517,6 +1549,13 @@ static const struct i915_power_well_ops > > > > skl_power_well_ops = { > > > > .is_enabled = skl_power_well_enabled, > > > > }; > > > > > > > > +static const struct i915_power_well_ops > > > > gen9_dc_off_power_well_ops = > > > > { > > > > + .sync_hw = gen9_dc_off_power_well_sync_hw, > > > > + .enable = gen9_dc_off_power_well_enable, > > > > + .disable = gen9_dc_off_power_well_disable, > > > > + .is_enabled = gen9_dc_off_power_well_enabled, > > > > +}; > > > > + > > > > static struct i915_power_well hsw_power_wells[] = { > > > > { > > > > .name = "always-on", > > > > @@ -1697,6 +1736,12 @@ static struct i915_power_well > > > > skl_power_wells[] = { > > > > .data = SKL_DISP_PW_2, > > > > }, > > > > { > > > > + .name = "DC off", > > > > + .domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS, > > > > + .ops = &gen9_dc_off_power_well_ops, > > > > + .data = SKL_DISP_PW_DC_OFF, > > > > + }, This should be listed right before the PW_2 well, so that the we have the following sequences: 1. disable DC5/6 2. enable PW_2 1. disable PW_2 2. enable DC5/6 > > > > + { > > > > .name = "DDI A/E power well", > > > > .domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS, > > > > .ops = &skl_power_well_ops, > > > > @@ -1769,7 +1814,14 @@ static struct i915_power_well > > > > bxt_power_wells[] = { > > > > .domains = > > > > BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS, > > > > .ops = &skl_power_well_ops, > > > > .data = SKL_DISP_PW_2, > > > > - } > > > > + }, > > > > + { > > > > + .name = "DC off", > > > > + .domains = BXT_DISPLAY_DC_OFF_POWER_DOMAINS, > > > > + .ops = &gen9_dc_off_power_well_ops, > > > > + .data = SKL_DISP_PW_DC_OFF, > > > > + }, As above, this should be listed right before the PW_2 well. > > > > + > > > > }; > > > > > > > > #define set_power_wells(power_domains, __power_wells) ({ > > > > > > > > \ > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 5a63f9a..0c7f435 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1072,9 +1072,6 @@ static int i915_pm_resume(struct device *dev) static int skl_suspend_complete(struct drm_i915_private *dev_priv) { - if (dev_priv->csr.dmc_payload) - skl_enable_dc6(dev_priv); - return 0; } @@ -1119,9 +1116,6 @@ static int bxt_resume_prepare(struct drm_i915_private *dev_priv) static int skl_resume_prepare(struct drm_i915_private *dev_priv) { - if (dev_priv->csr.dmc_payload) - skl_disable_dc6(dev_priv); - return 0; } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 31b3a84..df445ba 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -636,6 +636,7 @@ enum skl_disp_power_wells { /* Not actual bit groups. Used as IDs for lookup_power_well() */ SKL_DISP_PW_ALWAYS_ON, + SKL_DISP_PW_DC_OFF, }; #define SKL_POWER_WELL_STATE(pw) (1 << ((pw) * 2)) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 649ac34..856d801 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13320,6 +13320,9 @@ static int intel_atomic_commit(struct drm_device *dev, to_intel_crtc_state(crtc->state)->update_pipe; unsigned long put_domains = 0; + if (modeset) + intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET); + if (modeset && crtc->state->active) { update_scanline_offset(to_intel_crtc(crtc)); dev_priv->display.crtc_enable(crtc); @@ -13343,6 +13346,9 @@ static int intel_atomic_commit(struct drm_device *dev, modeset_put_power_domains(dev_priv, put_domains); intel_post_plane_update(intel_crtc); + + if (modeset) + intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET); } /* FIXME: add subpixel order */ diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index edf753e..95c3fcc 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -49,9 +49,6 @@ * present for a given platform. */ -#define GEN9_ENABLE_DC5(dev) 0 -#define SKL_ENABLE_DC6(dev) IS_SKYLAKE(dev) - #define for_each_power_well(i, power_well, domain_mask, power_domains) \ for (i = 0; \ i < (power_domains)->power_well_count && \ @@ -309,9 +306,14 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv, #define SKL_DISPLAY_DDI_D_POWER_DOMAINS ( \ BIT(POWER_DOMAIN_PORT_DDI_D_LANES) | \ BIT(POWER_DOMAIN_INIT)) +#define SKL_DISPLAY_DC_OFF_POWER_DOMAINS ( \ + BIT(POWER_DOMAIN_MODESET) | \ + BIT(POWER_DOMAIN_AUX_A) | \ + BIT(POWER_DOMAIN_INIT)) #define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS ( \ (POWER_DOMAIN_MASK & ~( \ - SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS)) | \ + SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS | \ + SKL_DISPLAY_DC_OFF_POWER_DOMAINS)) | \ BIT(POWER_DOMAIN_INIT)) #define BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS ( \ @@ -339,6 +341,10 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv, BIT(POWER_DOMAIN_AUX_A) | \ BIT(POWER_DOMAIN_PLLS) | \ BIT(POWER_DOMAIN_INIT)) +#define BXT_DISPLAY_DC_OFF_POWER_DOMAINS ( \ + BIT(POWER_DOMAIN_MODESET) | \ + BIT(POWER_DOMAIN_AUX_A) | \ + BIT(POWER_DOMAIN_INIT)) #define BXT_DISPLAY_ALWAYS_ON_POWER_DOMAINS ( \ (POWER_DOMAIN_MASK & ~(BXT_DISPLAY_POWERWELL_1_POWER_DOMAINS | \ BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS)) | \ @@ -486,15 +492,6 @@ static void gen9_enable_dc5(struct drm_i915_private *dev_priv) gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC5); } -static void gen9_disable_dc5(struct drm_i915_private *dev_priv) -{ - assert_can_disable_dc5(dev_priv); - - DRM_DEBUG_KMS("Disabling DC5\n"); - - gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); -} - static void assert_can_enable_dc6(struct drm_i915_private *dev_priv) { struct drm_device *dev = dev_priv->dev; @@ -522,6 +519,14 @@ static void assert_can_disable_dc6(struct drm_i915_private *dev_priv) "DC6 already programmed to be disabled.\n"); } +static void gen9_disable_dc5_dc6(struct drm_i915_private *dev_priv) +{ + assert_can_disable_dc5(dev_priv); + assert_can_disable_dc6(dev_priv); + + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); +} + void skl_enable_dc6(struct drm_i915_private *dev_priv) { assert_can_enable_dc6(dev_priv); @@ -571,6 +576,7 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv, case SKL_DISP_PW_DDI_C: case SKL_DISP_PW_DDI_D: case SKL_DISP_PW_MISC_IO: + case SKL_DISP_PW_DC_OFF: break; default: WARN(1, "Unknown power well %lu\n", power_well->data); @@ -589,17 +595,13 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv, "Invalid for power well status to be enabled, unless done by the BIOS, \ when request is to disable!\n"); if (power_well->data == SKL_DISP_PW_2) { - if (GEN9_ENABLE_DC5(dev)) - gen9_disable_dc5(dev_priv); - if (SKL_ENABLE_DC6(dev)) { - /* - * DDI buffer programming unnecessary during driver-load/resume - * as it's already done during modeset initialization then. - * It's also invalid here as encoder list is still uninitialized. - */ - if (!dev_priv->power_domains.initializing) - intel_prepare_ddi(dev); - } + /* + * DDI buffer programming unnecessary during driver-load/resume + * as it's already done during modeset initialization then. + * It's also invalid here as encoder list is still uninitialized. + */ + if (!dev_priv->power_domains.initializing) + intel_prepare_ddi(dev); } I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | req_mask); } @@ -617,10 +619,6 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv, I915_WRITE(HSW_PWR_WELL_DRIVER, tmp & ~req_mask); POSTING_READ(HSW_PWR_WELL_DRIVER); DRM_DEBUG_KMS("Disabling %s\n", power_well->name); - - if (GEN9_ENABLE_DC5(dev) && - power_well->data == SKL_DISP_PW_2) - gen9_enable_dc5(dev_priv); } } @@ -695,6 +693,40 @@ static void skl_power_well_disable(struct drm_i915_private *dev_priv, skl_set_power_well(dev_priv, power_well, false); } +static bool gen9_dc_off_power_well_enabled(struct drm_i915_private *dev_priv, + struct i915_power_well *power_well) +{ + return (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0; +} + +static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv, + struct i915_power_well *power_well) +{ + gen9_disable_dc5_dc6(dev_priv); +} + +static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv, + struct i915_power_well *power_well) +{ + if (IS_SKYLAKE(dev_priv)) + skl_enable_dc6(dev_priv); + else + gen9_enable_dc5(dev_priv); +} + +static void gen9_dc_off_power_well_sync_hw(struct drm_i915_private *dev_priv, + struct i915_power_well *power_well) +{ + if (power_well->count > 0) { + gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); + } else { + if (IS_SKYLAKE(dev_priv)) + gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6); + else + gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC5); + } +} + static void i9xx_always_on_power_well_noop(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { @@ -1517,6 +1549,13 @@ static const struct i915_power_well_ops skl_power_well_ops = { .is_enabled = skl_power_well_enabled, }; +static const struct i915_power_well_ops gen9_dc_off_power_well_ops = { + .sync_hw = gen9_dc_off_power_well_sync_hw, + .enable = gen9_dc_off_power_well_enable, + .disable = gen9_dc_off_power_well_disable, + .is_enabled = gen9_dc_off_power_well_enabled, +}; + static struct i915_power_well hsw_power_wells[] = { { .name = "always-on", @@ -1697,6 +1736,12 @@ static struct i915_power_well skl_power_wells[] = { .data = SKL_DISP_PW_2, }, { + .name = "DC off", + .domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS, + .ops = &gen9_dc_off_power_well_ops, + .data = SKL_DISP_PW_DC_OFF, + }, + { .name = "DDI A/E power well", .domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS, .ops = &skl_power_well_ops, @@ -1769,7 +1814,14 @@ static struct i915_power_well bxt_power_wells[] = { .domains = BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS, .ops = &skl_power_well_ops, .data = SKL_DISP_PW_2, - } + }, + { + .name = "DC off", + .domains = BXT_DISPLAY_DC_OFF_POWER_DOMAINS, + .ops = &gen9_dc_off_power_well_ops, + .data = SKL_DISP_PW_DC_OFF, + }, + }; #define set_power_wells(power_domains, __power_wells) ({ \
Handle DC off as a power well where enabling the power well will prevent the DMC to enter selected DC states (required around modesets and Aux A). Disabling the power well will allow DC states again. For now the highest DC state is DC6 for Skylake and DC5 for Broxton but will be configurable for Skylake in a later patch. v2: Check both DC5 and DC6 bits in power well enabled function (Ville) Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 6 -- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_display.c | 6 ++ drivers/gpu/drm/i915/intel_runtime_pm.c | 110 +++++++++++++++++++++++--------- 4 files changed, 88 insertions(+), 35 deletions(-)