On ti, 2015-11-03 at 13:31 +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 but will be configurable in a later patch.
> 
> Signed-off-by: Patrik Jakobsson <patrik.jakobs...@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c         |   6 --
>  drivers/gpu/drm/i915/intel_display.c    |   6 ++
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 107 
> +++++++++++++++++++++-----------
>  3 files changed, 78 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 37319b0..e190237 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1069,9 +1069,6 @@ static int skl_suspend_complete(struct drm_i915_private 
> *dev_priv)
>  {
>       skl_uninit_cdclk(dev_priv);
>  
> -     if (dev_priv->csr.dmc_payload)
> -             skl_enable_dc6(dev_priv);
> -
>       return 0;
>  }
>  
> @@ -1116,9 +1113,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);
> -
>       skl_init_cdclk(dev_priv);
>       intel_csr_load_program(dev_priv);
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index c6d60b8..e401871 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13296,6 +13296,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);
> @@ -13319,6 +13322,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 c901b19..042d92f 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 &&                   \
> @@ -336,10 +333,15 @@ static void hsw_set_power_well(struct drm_i915_private 
> *dev_priv,
>       SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |         \
>       BIT(POWER_DOMAIN_PLLS) |                        \
>       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_1_POWER_DOMAINS |  \
>       SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |         \
> -     SKL_DISPLAY_MISC_IO_POWER_DOMAINS)) |           \
> +     SKL_DISPLAY_MISC_IO_POWER_DOMAINS |             \
> +     SKL_DISPLAY_DC_OFF_POWER_DOMAINS)) |            \
>       BIT(POWER_DOMAIN_INIT))
>  
>  #define BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS (              \
> @@ -511,20 +513,6 @@ static void gen9_enable_dc5(struct drm_i915_private 
> *dev_priv)
>       POSTING_READ(DC_STATE_EN);
>  }
>  
> -static void gen9_disable_dc5(struct drm_i915_private *dev_priv)
> -{
> -     uint32_t val;
> -
> -     assert_can_disable_dc5(dev_priv);
> -
> -     DRM_DEBUG_KMS("Disabling DC5\n");
> -
> -     val = I915_READ(DC_STATE_EN);
> -     val &= ~DC_STATE_EN_UPTO_DC5;
> -     I915_WRITE(DC_STATE_EN, val);
> -     POSTING_READ(DC_STATE_EN);
> -}
> -
>  static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
>  {
>       struct drm_device *dev = dev_priv->dev;
> @@ -553,6 +541,21 @@ 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)
> +{
> +     uint32_t val;
> +
> +     assert_can_disable_dc5(dev_priv);
> +     assert_can_disable_dc6(dev_priv);
> +
> +     DRM_DEBUG_KMS("Disabling DC5 and DC6\n");
> +
> +     val = I915_READ(DC_STATE_EN);
> +     val &= ~DC_STATE_EN_UPTO_DC5_DC6_MASK;
> +     I915_WRITE(DC_STATE_EN, val);
> +     POSTING_READ(DC_STATE_EN);
> +}
> +
>  void skl_enable_dc6(struct drm_i915_private *dev_priv)
>  {
>       uint32_t val;
> @@ -632,17 +635,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);
>               }
> @@ -658,17 +657,13 @@ static void skl_set_power_well(struct drm_i915_private 
> *dev_priv,
>       } else {
>               if (enable_requested) {
>                       if (IS_SKYLAKE(dev) &&
> -                             (power_well->data == SKL_DISP_PW_1))
> +                             (power_well->data == SKL_DISP_PW_1)) {
>                               DRM_DEBUG_KMS("Not Disabling PW1, dmc will 
> handle\n");
> -                     else {
> +                     } else {
>                               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);
>               }
>       }
>  
> @@ -743,6 +738,36 @@ static void skl_power_well_disable(struct 
> drm_i915_private *dev_priv,
>       skl_set_power_well(dev_priv, power_well, false);
>  }
>  
> +static bool skl_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_DC6);
> +}
> +
> +static void skl_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 skl_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 skl_dc_off_power_well_sync_hw(struct drm_i915_private *dev_priv,
> +                                       struct i915_power_well *power_well)
> +{
> +     if (power_well->count > 0)
> +             skl_dc_off_power_well_enable(dev_priv, power_well);
> +     else
> +             skl_dc_off_power_well_disable(dev_priv, power_well);
> +}
> +
>  static void i9xx_always_on_power_well_noop(struct drm_i915_private *dev_priv,
>                                          struct i915_power_well *power_well)
>  {
> @@ -1574,6 +1599,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 skl_dc_off_power_well_ops = {
> +     .sync_hw = skl_dc_off_power_well_sync_hw,
> +     .enable = skl_dc_off_power_well_enable,
> +     .disable = skl_dc_off_power_well_disable,
> +     .is_enabled = skl_dc_off_power_well_enabled,
> +};
> +
>  static struct i915_power_well hsw_power_wells[] = {
>       {
>               .name = "always-on",
> @@ -1738,6 +1770,11 @@ static struct i915_power_well skl_power_wells[] = {
>               .data = SKL_DISP_PW_1,
>       },
>       {
> +             .name = "DC off",
> +             .domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS,
> +             .ops = &skl_dc_off_power_well_ops,

We need to set a unique ID in .data, since we use it to look up power
wells. I have a patch to fix up the other places missing an ID, but you
could fix this one up already now.

> +     },
> +     {
>               .name = "MISC IO power well",
>               .domains = SKL_DISPLAY_MISC_IO_POWER_DOMAINS,
>               .ops = &skl_power_well_ops,


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to