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@xxxxxxxxxxxxxxx>
> > ---
> > 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) ({
> > \
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
