[v3,10/12] drm/i915/gen9: Turn DC handling into a power well

Submitted by Patrik Jakobsson on Nov. 16, 2015, 2:01 p.m.

Details

Message ID 1447682467-6237-4-git-send-email-patrik.jakobsson@linux.intel.com
State New
Headers show
Series "Skylake DMC/DC-state fixes and redesign" ( rev: 7 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Patrik Jakobsson Nov. 16, 2015, 2:01 p.m.
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)

Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Reviewed-by: Imre Deak <imre.deak@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 | 111 +++++++++++++++++++++++---------
 4 files changed, 89 insertions(+), 35 deletions(-)

Patch hide | download patch | download mbox

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..b6ee48e 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,
@@ -1769,7 +1815,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) ({		\

Comments

On Mon, Nov 16, 2015 at 03:01:07PM +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)
> 
> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> Reviewed-by: Imre Deak <imre.deak@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 | 111 +++++++++++++++++++++++---------
>  4 files changed, 89 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..b6ee48e 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,
> @@ -1769,7 +1815,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,
> +	},
> +

Just realized that I forgot to move DC off for BXT above PW2. Will resend, sorry
for the noise.

-Patrik

>  };
>  
>  #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 ma, 2015-11-16 at 15:01 +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)
> 
> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> Reviewed-by: Imre Deak <imre.deak@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 | 111 +++++++++++++++++++++++---------
>  4 files changed, 89 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..b6ee48e 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,
> @@ -1769,7 +1815,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,
> +	},
> +

You forgot to move this one. With that fixed the patch looks ok and my
R-b holds.

>  };
>  
>  #define set_power_wells(power_domains, __power_wells) ({		\
On Mon, Nov 16, 2015 at 8:28 PM, Imre Deak <imre.deak@intel.com> wrote:
> On ma, 2015-11-16 at 15:01 +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)
>>
>> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
>> Reviewed-by: Imre Deak <imre.deak@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 | 111 +++++++++++++++++++++++---------
>>  4 files changed, 89 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..b6ee48e 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,
>> @@ -1769,7 +1815,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,
>> +     },
>> +
>
> You forgot to move this one. With that fixed the patch looks ok and my
> R-b holds.

Yea sorry about that. There's already a v4 available.

>
>>  };
>>
>>  #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