| Message ID | 1447687201-24759-1-git-send-email-patrik.jakobsson@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On ma, 2015-11-16 at 16:20 +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) > v3: > - Remove unneeded DC_OFF case in skl_set_power_well() (Imre) > - Add PW2 dependency to DC_OFF (Imre) > v4: Put DC_OFF before PW2 in BXT power well array > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > Reviewed-by: Imre Deak <imre.deak@intel.com> This has some checkpatch errors, I fixed them up while applying. > --- > 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 0f3849f..b6a3525 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -638,6 +638,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 978b1b9..21385a0 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13323,6 +13323,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); > @@ -13346,6 +13349,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..0ff1646 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,15 @@ 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 ( \ > + SKL_DISPLAY_POWERWELL_2_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 +342,11 @@ 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 ( \ > + BXT_DISPLAY_POWERWELL_2_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 +494,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 +521,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); > @@ -589,17 +596,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 +620,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 +694,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 +1550,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", > @@ -1691,6 +1731,12 @@ static struct i915_power_well > skl_power_wells[] = { > .data = SKL_DISP_PW_MISC_IO, > }, > { > + .name = "DC off", > + .domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS, > + .ops = &gen9_dc_off_power_well_ops, > + .data = SKL_DISP_PW_DC_OFF, > + }, > + { > .name = "power well 2", > .domains = SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS, > .ops = &skl_power_well_ops, > @@ -1765,11 +1811,17 @@ static struct i915_power_well > bxt_power_wells[] = { > .data = SKL_DISP_PW_1, > }, > { > + .name = "DC off", > + .domains = BXT_DISPLAY_DC_OFF_POWER_DOMAINS, > + .ops = &gen9_dc_off_power_well_ops, > + .data = SKL_DISP_PW_DC_OFF, > + }, > + { > .name = "power well 2", > .domains = BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS, > .ops = &skl_power_well_ops, > .data = SKL_DISP_PW_2, > - } > + }, > }; > > #define set_power_wells(power_domains, __power_wells) ({ > \
On Mon, Nov 16, 2015 at 04:20:01PM +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) > v3: > - Remove unneeded DC_OFF case in skl_set_power_well() (Imre) > - Add PW2 dependency to DC_OFF (Imre) > v4: Put DC_OFF before PW2 in BXT power well array > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > Reviewed-by: Imre Deak <imre.deak@intel.com> I've been seeing a BXT regression on recent di-nightly where DPMS off causes the entire platform to power down[1] instead of just the display; my bisect lands on this commit as the culprit. Any idea what the cause could be? I can reproduce by either letting the system sit idle long enough at an fb console, or by doing an "xset dpms force off" in X. Unfortunately I don't have a functioning serial console on this platform, so I can't get any messages that may show up around the DPMS operation. I've attached my boot-time dmesg output in case that helps. Subsequent commits seem to depend on the changes here, so I haven't reverted this commit directly on di-nightly, but I confirmed that if I checkout this commit directly I see DPMS problems, whereas its HEAD~1 works as expected. Matt [1] My BIOS and/or hardware is a bit flaky so even when operating "normally" most attempts to reboot/poweroff/S3 suspend all result in the platform going into some "mostly off" state with an error LED lit and require a full power cycle to resurrect. That's the same state I wind up in after DPMS off when this patch is present. > --- > 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 0f3849f..b6a3525 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -638,6 +638,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 978b1b9..21385a0 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13323,6 +13323,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); > @@ -13346,6 +13349,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..0ff1646 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,15 @@ 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 ( \ > + SKL_DISPLAY_POWERWELL_2_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 +342,11 @@ 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 ( \ > + BXT_DISPLAY_POWERWELL_2_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 +494,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 +521,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); > @@ -589,17 +596,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 +620,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 +694,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 +1550,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", > @@ -1691,6 +1731,12 @@ static struct i915_power_well skl_power_wells[] = { > .data = SKL_DISP_PW_MISC_IO, > }, > { > + .name = "DC off", > + .domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS, > + .ops = &gen9_dc_off_power_well_ops, > + .data = SKL_DISP_PW_DC_OFF, > + }, > + { > .name = "power well 2", > .domains = SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS, > .ops = &skl_power_well_ops, > @@ -1765,11 +1811,17 @@ static struct i915_power_well bxt_power_wells[] = { > .data = SKL_DISP_PW_1, > }, > { > + .name = "DC off", > + .domains = BXT_DISPLAY_DC_OFF_POWER_DOMAINS, > + .ops = &gen9_dc_off_power_well_ops, > + .data = SKL_DISP_PW_DC_OFF, > + }, > + { > .name = "power well 2", > .domains = BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS, > .ops = &skl_power_well_ops, > .data = SKL_DISP_PW_2, > - } > + }, > }; > > #define set_power_wells(power_domains, __power_wells) ({ \ > -- > 2.5.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, 2015-11-23 at 14:58 -0800, Matt Roper wrote: > On Mon, Nov 16, 2015 at 04:20:01PM +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) > > v3: > > - Remove unneeded DC_OFF case in skl_set_power_well() (Imre) > > - Add PW2 dependency to DC_OFF (Imre) > > v4: Put DC_OFF before PW2 in BXT power well array > > > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > > Reviewed-by: Imre Deak <imre.deak@intel.com> > > I've been seeing a BXT regression on recent di-nightly where DPMS off > causes the entire platform to power down[1] instead of just the > display; > my bisect lands on this commit as the culprit. Any idea what the > cause > could be? I can reproduce by either letting the system sit idle long > enough at an fb console, or by doing an "xset dpms force off" in X. > Unfortunately I don't have a functioning serial console on this > platform, so I can't get any messages that may show up around the > DPMS > operation. I've attached my boot-time dmesg output in case that > helps. > > Subsequent commits seem to depend on the changes here, so I haven't > reverted this commit directly on di-nightly, but I confirmed that if > I > checkout this commit directly I see DPMS problems, whereas its HEAD~1 > works as expected. The power well support on BXT is not stable atm, we need to apply at least a similar set of fixes as we did for SKL. So for now I would suggest disabling it, by booting with i915.disable_power_well=0 until things are fixed. This should've been made the default option earlier, I forgot about this. I will follow up with the patch to that extent. Thanks for the report, Imre > Matt > > > [1] My BIOS and/or hardware is a bit flaky so even when operating > "normally" most attempts to reboot/poweroff/S3 suspend all result in > the > platform going into some "mostly off" state with an error LED lit and > require a full power cycle to resurrect. That's the same state I > wind > up in after DPMS off when this patch is present. > > > --- > > 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 0f3849f..b6a3525 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -638,6 +638,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 978b1b9..21385a0 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -13323,6 +13323,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); > > @@ -13346,6 +13349,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..0ff1646 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,15 @@ 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 ( \ > > + SKL_DISPLAY_POWERWELL_2_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 +342,11 @@ 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 ( \ > > + BXT_DISPLAY_POWERWELL_2_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 +494,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 +521,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); > > @@ -589,17 +596,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 +620,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 +694,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 +1550,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", > > @@ -1691,6 +1731,12 @@ static struct i915_power_well > > skl_power_wells[] = { > > .data = SKL_DISP_PW_MISC_IO, > > }, > > { > > + .name = "DC off", > > + .domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS, > > + .ops = &gen9_dc_off_power_well_ops, > > + .data = SKL_DISP_PW_DC_OFF, > > + }, > > + { > > .name = "power well 2", > > .domains = SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS, > > .ops = &skl_power_well_ops, > > @@ -1765,11 +1811,17 @@ static struct i915_power_well > > bxt_power_wells[] = { > > .data = SKL_DISP_PW_1, > > }, > > { > > + .name = "DC off", > > + .domains = BXT_DISPLAY_DC_OFF_POWER_DOMAINS, > > + .ops = &gen9_dc_off_power_well_ops, > > + .data = SKL_DISP_PW_DC_OFF, > > + }, > > + { > > .name = "power well 2", > > .domains = BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS, > > .ops = &skl_power_well_ops, > > .data = SKL_DISP_PW_2, > > - } > > + }, > > }; > > > > #define set_power_wells(power_domains, __power_wells) ({ > > \ > > -- > > 2.5.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Tue, Nov 24, 2015 at 01:09:03AM +0200, Imre Deak wrote: > On Mon, 2015-11-23 at 14:58 -0800, Matt Roper wrote: > > On Mon, Nov 16, 2015 at 04:20:01PM +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) > > > v3: > > > - Remove unneeded DC_OFF case in skl_set_power_well() (Imre) > > > - Add PW2 dependency to DC_OFF (Imre) > > > v4: Put DC_OFF before PW2 in BXT power well array > > > > > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > > > Reviewed-by: Imre Deak <imre.deak@intel.com> > > > > I've been seeing a BXT regression on recent di-nightly where DPMS off > > causes the entire platform to power down[1] instead of just the > > display; > > my bisect lands on this commit as the culprit. Any idea what the > > cause > > could be? I can reproduce by either letting the system sit idle long > > enough at an fb console, or by doing an "xset dpms force off" in X. > > Unfortunately I don't have a functioning serial console on this > > platform, so I can't get any messages that may show up around the > > DPMS > > operation. I've attached my boot-time dmesg output in case that > > helps. > > > > Subsequent commits seem to depend on the changes here, so I haven't > > reverted this commit directly on di-nightly, but I confirmed that if > > I > > checkout this commit directly I see DPMS problems, whereas its HEAD~1 > > works as expected. > > The power well support on BXT is not stable atm, we need to apply at > least a similar set of fixes as we did for SKL. So for now I would > suggest disabling it, by booting with i915.disable_power_well=0 until > things are fixed. This should've been made the default option earlier, > I forgot about this. I will follow up with the patch to that extent. I guess we should pull in that patch asap. Other problem is that current igt tests aren't too good at obeying the dsi encoder/pipe restrictions, so atm pm_rpm just skips. That needs to be fixed too. -Daniel
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 0f3849f..b6a3525 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -638,6 +638,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 978b1b9..21385a0 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13323,6 +13323,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); @@ -13346,6 +13349,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..0ff1646 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,15 @@ 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 ( \ + SKL_DISPLAY_POWERWELL_2_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 +342,11 @@ 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 ( \ + BXT_DISPLAY_POWERWELL_2_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 +494,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 +521,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); @@ -589,17 +596,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 +620,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 +694,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 +1550,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", @@ -1691,6 +1731,12 @@ static struct i915_power_well skl_power_wells[] = { .data = SKL_DISP_PW_MISC_IO, }, { + .name = "DC off", + .domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS, + .ops = &gen9_dc_off_power_well_ops, + .data = SKL_DISP_PW_DC_OFF, + }, + { .name = "power well 2", .domains = SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS, .ops = &skl_power_well_ops, @@ -1765,11 +1811,17 @@ static struct i915_power_well bxt_power_wells[] = { .data = SKL_DISP_PW_1, }, { + .name = "DC off", + .domains = BXT_DISPLAY_DC_OFF_POWER_DOMAINS, + .ops = &gen9_dc_off_power_well_ops, + .data = SKL_DISP_PW_DC_OFF, + }, + { .name = "power well 2", .domains = BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS, .ops = &skl_power_well_ops, .data = SKL_DISP_PW_2, - } + }, }; #define set_power_wells(power_domains, __power_wells) ({ \