public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v2 1/4] drm/i915/skl: Fix power domain suspend sequence
@ 2016-02-29 20:49 Imre Deak
  2016-02-29 20:49 ` [PATCH v2 2/4] drm/i915/gen9: Sanitize handling of allowed DC states Imre Deak
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Imre Deak @ 2016-02-29 20:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable

During system suspend we need to first disable power wells then
unitialize the display core. In case power well support is disabled we
did this in the wrong order, so fix this up.

Fixes: d314cd43 ("drm/i915: fix handling of the disable_power_well module option")
CC: stable@vger.kernel.org
CC: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 4172e73..6e54d97 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -2319,15 +2319,15 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
  */
 void intel_power_domains_suspend(struct drm_i915_private *dev_priv)
 {
-	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
-		skl_display_core_uninit(dev_priv);
-
 	/*
 	 * Even if power well support was disabled we still want to disable
 	 * power wells while we are system suspended.
 	 */
 	if (!i915.disable_power_well)
 		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
+
+	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
+		skl_display_core_uninit(dev_priv);
 }
 
 /**
-- 
2.5.0

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/4] drm/i915/gen9: Sanitize handling of allowed DC states
  2016-02-29 20:49 [PATCH v2 1/4] drm/i915/skl: Fix power domain suspend sequence Imre Deak
@ 2016-02-29 20:49 ` Imre Deak
  2016-03-01 12:58   ` Patrik Jakobsson
  2016-02-29 20:49 ` [PATCH v2 3/4] drm/i915/gen9: Disable DC states if power well support is disabled Imre Deak
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Imre Deak @ 2016-02-29 20:49 UTC (permalink / raw)
  To: intel-gfx

We can simplify the conditions selecting the target DC state during
runtime by calculating the allowed DC states in advance during driver
loading. This also makes it easier to disable DC states depending on the
i915.disable_power_well module option, added in the next patch.

v2:
- Print a debug message if the requested max DC value was adjusted due
  to a platform limit. Also debug print the calculated mask value. (Patrik)

CC: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |  1 +
 drivers/gpu/drm/i915/intel_runtime_pm.c | 78 ++++++++++++++++++++++++---------
 2 files changed, 58 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6712955..dc554dd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -754,6 +754,7 @@ struct intel_csr {
 	i915_reg_t mmioaddr[8];
 	uint32_t mmiodata[8];
 	uint32_t dc_state;
+	uint32_t allowed_dc_mask;
 };
 
 #define DEV_INFO_FOR_EACH_FLAG(func, sep) \
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 6e54d97..30df9de 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -538,12 +538,8 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, uint32_t state)
 	else
 		mask |= DC_STATE_EN_UPTO_DC6;
 
-	WARN_ON_ONCE(state & ~mask);
-
-	if (i915.enable_dc == 0)
-		state = DC_STATE_DISABLE;
-	else if (i915.enable_dc == 1 && state > DC_STATE_EN_UPTO_DC5)
-		state = DC_STATE_EN_UPTO_DC5;
+	if (WARN_ON_ONCE(state & ~dev_priv->csr.allowed_dc_mask))
+		state &= dev_priv->csr.allowed_dc_mask;
 
 	val = I915_READ(DC_STATE_EN);
 	DRM_DEBUG_KMS("Setting DC state from %02x to %02x\n",
@@ -659,8 +655,7 @@ static void gen9_disable_dc5_dc6(struct drm_i915_private *dev_priv)
 {
 	assert_can_disable_dc5(dev_priv);
 
-	if ((IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) &&
-	    i915.enable_dc != 0 && i915.enable_dc != 1)
+	if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
 		assert_can_disable_dc6(dev_priv);
 
 	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
@@ -839,26 +834,19 @@ static void gen9_dc_off_power_well_enable(struct drm_i915_private *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) || IS_KABYLAKE(dev_priv)) &&
-	    i915.enable_dc != 0 && i915.enable_dc != 1)
+	if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
 		skl_enable_dc6(dev_priv);
-	else
+	else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)
 		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) || IS_KABYLAKE(dev_priv)) &&
-		    i915.enable_dc != 0 &&
-		    i915.enable_dc != 1)
-			gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6);
-		else
-			gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC5);
-	}
+	if (power_well->count > 0)
+		gen9_dc_off_power_well_enable(dev_priv, power_well);
+	else
+		gen9_dc_off_power_well_disable(dev_priv, power_well);
 }
 
 static void i9xx_always_on_power_well_noop(struct drm_i915_private *dev_priv,
@@ -2023,6 +2011,52 @@ sanitize_disable_power_well_option(const struct drm_i915_private *dev_priv,
 	return 1;
 }
 
+static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
+				    int enable_dc)
+{
+	uint32_t mask;
+	int requested_dc;
+	int max_dc;
+
+	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
+		max_dc = 2;
+		mask = 0;
+	} else if (IS_BROXTON(dev_priv)) {
+		max_dc = 1;
+		/*
+		 * DC9 has a separate HW flow from the rest of the DC states,
+		 * not depending on the DMC firmware. It's needed by system
+		 * suspend/resume, so allow it unconditionally.
+		 */
+		mask = DC_STATE_EN_DC9;
+	} else {
+		max_dc = 0;
+		mask = 0;
+	}
+
+	if (enable_dc >= 0 && enable_dc <= max_dc) {
+		requested_dc = enable_dc;
+	} else if (enable_dc == -1) {
+		requested_dc = max_dc;
+	} else if (enable_dc > max_dc && enable_dc <= 2) {
+		DRM_DEBUG_KMS("Adjusting requested max DC state (%d->%d)\n",
+			      enable_dc, max_dc);
+		requested_dc = max_dc;
+	} else {
+		DRM_ERROR("Unexpected value for enable_dc (%d)\n", enable_dc);
+		requested_dc = max_dc;
+	}
+
+	if (requested_dc > 1)
+		mask |= DC_STATE_EN_UPTO_DC6;
+	if (requested_dc > 0)
+		mask |= DC_STATE_EN_UPTO_DC5;
+
+	DRM_DEBUG_KMS("Allowed DC state mask %02x\n", mask);
+
+	return mask;
+}
+
 #define set_power_wells(power_domains, __power_wells) ({		\
 	(power_domains)->power_wells = (__power_wells);			\
 	(power_domains)->power_well_count = ARRAY_SIZE(__power_wells);	\
@@ -2041,6 +2075,8 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
 
 	i915.disable_power_well = sanitize_disable_power_well_option(dev_priv,
 						     i915.disable_power_well);
+	dev_priv->csr.allowed_dc_mask = get_allowed_dc_mask(dev_priv,
+							    i915.enable_dc);
 
 	BUILD_BUG_ON(POWER_DOMAIN_NUM > 31);
 
-- 
2.5.0

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/4] drm/i915/gen9: Disable DC states if power well support is disabled
  2016-02-29 20:49 [PATCH v2 1/4] drm/i915/skl: Fix power domain suspend sequence Imre Deak
  2016-02-29 20:49 ` [PATCH v2 2/4] drm/i915/gen9: Sanitize handling of allowed DC states Imre Deak
@ 2016-02-29 20:49 ` Imre Deak
  2016-03-01 13:01   ` Patrik Jakobsson
  2016-02-29 20:49 ` [PATCH v2 4/4] drm/i915/gen9: Remove state asserts when disabling DC states Imre Deak
  2016-03-01 12:14 ` ✗ Fi.CI.BAT: warning for series starting with [v2,1/4] drm/i915/skl: Fix power domain suspend sequence Patchwork
  3 siblings, 1 reply; 8+ messages in thread
From: Imre Deak @ 2016-02-29 20:49 UTC (permalink / raw)
  To: intel-gfx

If power well support is disabled via the i915.disable_power_well module
option we should never enable DC states. Currently we would enable DC
states even in this case during system suspend, where we need to disable
all power wells regardless of the disable_power_well option.

CC: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 30df9de..f0ca5134 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -2034,6 +2034,9 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
 		mask = 0;
 	}
 
+	if (!i915.disable_power_well)
+		max_dc = 0;
+
 	if (enable_dc >= 0 && enable_dc <= max_dc) {
 		requested_dc = enable_dc;
 	} else if (enable_dc == -1) {
-- 
2.5.0

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 4/4] drm/i915/gen9: Remove state asserts when disabling DC states
  2016-02-29 20:49 [PATCH v2 1/4] drm/i915/skl: Fix power domain suspend sequence Imre Deak
  2016-02-29 20:49 ` [PATCH v2 2/4] drm/i915/gen9: Sanitize handling of allowed DC states Imre Deak
  2016-02-29 20:49 ` [PATCH v2 3/4] drm/i915/gen9: Disable DC states if power well support is disabled Imre Deak
@ 2016-02-29 20:49 ` Imre Deak
  2016-03-01 12:14 ` ✗ Fi.CI.BAT: warning for series starting with [v2,1/4] drm/i915/skl: Fix power domain suspend sequence Patchwork
  3 siblings, 0 replies; 8+ messages in thread
From: Imre Deak @ 2016-02-29 20:49 UTC (permalink / raw)
  To: intel-gfx

Disabling the DC states when it's already disabled is a valid scenario,
for example during HW state sanitization during driver loading and
resuming or when DC states are disabled via the i915.enable_dc or
disable_power_well option.

CC: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 41 +--------------------------------
 1 file changed, 1 insertion(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index f0ca5134..09c52b1 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -458,8 +458,6 @@ static void assert_can_enable_dc9(struct drm_i915_private *dev_priv)
 static void assert_can_disable_dc9(struct drm_i915_private *dev_priv)
 {
 	WARN(intel_irqs_enabled(dev_priv), "Interrupts not disabled yet.\n");
-	WARN(!(I915_READ(DC_STATE_EN) & DC_STATE_EN_DC9),
-		"DC9 already programmed to be disabled.\n");
 	WARN(I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5,
 		"DC5 still not disabled.\n");
 
@@ -602,18 +600,6 @@ static void assert_can_enable_dc5(struct drm_i915_private *dev_priv)
 	assert_csr_loaded(dev_priv);
 }
 
-static void assert_can_disable_dc5(struct drm_i915_private *dev_priv)
-{
-	/*
-	 * During initialization, the firmware may not be loaded yet.
-	 * We still want to make sure that the DC enabling flag is cleared.
-	 */
-	if (dev_priv->power_domains.initializing)
-		return;
-
-	assert_rpm_wakelock_held(dev_priv);
-}
-
 static void gen9_enable_dc5(struct drm_i915_private *dev_priv)
 {
 	assert_can_enable_dc5(dev_priv);
@@ -638,29 +624,6 @@ static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
 	assert_csr_loaded(dev_priv);
 }
 
-static void assert_can_disable_dc6(struct drm_i915_private *dev_priv)
-{
-	/*
-	 * During initialization, the firmware may not be loaded yet.
-	 * We still want to make sure that the DC enabling flag is cleared.
-	 */
-	if (dev_priv->power_domains.initializing)
-		return;
-
-	WARN_ONCE(!(I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6),
-		  "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);
-
-	if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
-		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);
@@ -673,8 +636,6 @@ void skl_enable_dc6(struct drm_i915_private *dev_priv)
 
 void skl_disable_dc6(struct drm_i915_private *dev_priv)
 {
-	assert_can_disable_dc6(dev_priv);
-
 	DRM_DEBUG_KMS("Disabling DC6\n");
 
 	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
@@ -828,7 +789,7 @@ static bool gen9_dc_off_power_well_enabled(struct drm_i915_private *dev_priv,
 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);
+	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
 }
 
 static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
-- 
2.5.0

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* ✗ Fi.CI.BAT: warning for series starting with [v2,1/4] drm/i915/skl: Fix power domain suspend sequence
  2016-02-29 20:49 [PATCH v2 1/4] drm/i915/skl: Fix power domain suspend sequence Imre Deak
                   ` (2 preceding siblings ...)
  2016-02-29 20:49 ` [PATCH v2 4/4] drm/i915/gen9: Remove state asserts when disabling DC states Imre Deak
@ 2016-03-01 12:14 ` Patchwork
  2016-03-01 17:18   ` Imre Deak
  3 siblings, 1 reply; 8+ messages in thread
From: Patchwork @ 2016-03-01 12:14 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/4] drm/i915/skl: Fix power domain suspend sequence
URL   : https://patchwork.freedesktop.org/series/3931/
State : warning

== Summary ==

Series 3931v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/3931/revisions/1/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                dmesg-warn -> PASS       (hsw-brixbox)
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (snb-x220t)
                pass       -> DMESG-WARN (hsw-brixbox)
        Subgroup basic-plain-flip:
                pass       -> DMESG-WARN (hsw-brixbox)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (snb-x220t)
        Subgroup suspend-read-crc-pipe-a:
                incomplete -> PASS       (hsw-gt2)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> DMESG-WARN (snb-dellxps)
        Subgroup basic-rte:
                dmesg-warn -> PASS       (snb-x220t)
                dmesg-warn -> PASS       (snb-dellxps)
                pass       -> DMESG-WARN (bsw-nuc-2)

bdw-nuci7        total:169  pass:158  dwarn:0   dfail:0   fail:0   skip:11 
bdw-ultra        total:169  pass:155  dwarn:0   dfail:0   fail:0   skip:14 
bsw-nuc-2        total:169  pass:137  dwarn:1   dfail:0   fail:1   skip:30 
byt-nuc          total:169  pass:144  dwarn:0   dfail:0   fail:0   skip:25 
hsw-brixbox      total:169  pass:152  dwarn:2   dfail:0   fail:0   skip:15 
hsw-gt2          total:169  pass:157  dwarn:1   dfail:0   fail:1   skip:10 
ilk-hp8440p      total:169  pass:118  dwarn:0   dfail:0   fail:1   skip:50 
ivb-t430s        total:169  pass:153  dwarn:0   dfail:0   fail:1   skip:15 
skl-i5k-2        total:169  pass:153  dwarn:0   dfail:0   fail:0   skip:16 
skl-i7k-2        total:169  pass:153  dwarn:0   dfail:0   fail:0   skip:16 
snb-dellxps      total:169  pass:144  dwarn:1   dfail:0   fail:1   skip:23 
snb-x220t        total:169  pass:144  dwarn:1   dfail:0   fail:2   skip:22 

Results at /archive/results/CI_IGT_test/Patchwork_1498/

deb861ea08c6d5bbf682fb40ea1f0471a448aafd drm-intel-nightly: 2016y-03m-01d-11h-12m-16s UTC integration manifest
d5cab5c3d22e85d2a27a36ac0e8797101da08fbf drm/i915/gen9: Remove state asserts when disabling DC states
fd6d00afc02e9a7ebae582854e3514ae48f0a016 drm/i915/gen9: Disable DC states if power well support is disabled
c0a4746a06e556f60f40296fd7eff223b9d70f7a drm/i915/gen9: Sanitize handling of allowed DC states
bd509a542321c86b5ebbd984740e7f010c196faa drm/i915/skl: Fix power domain suspend sequence

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/4] drm/i915/gen9: Sanitize handling of allowed DC states
  2016-02-29 20:49 ` [PATCH v2 2/4] drm/i915/gen9: Sanitize handling of allowed DC states Imre Deak
@ 2016-03-01 12:58   ` Patrik Jakobsson
  0 siblings, 0 replies; 8+ messages in thread
From: Patrik Jakobsson @ 2016-03-01 12:58 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Mon, Feb 29, 2016 at 10:49:03PM +0200, Imre Deak wrote:
> We can simplify the conditions selecting the target DC state during
> runtime by calculating the allowed DC states in advance during driver
> loading. This also makes it easier to disable DC states depending on the
> i915.disable_power_well module option, added in the next patch.
> 
> v2:
> - Print a debug message if the requested max DC value was adjusted due
>   to a platform limit. Also debug print the calculated mask value. (Patrik)
> 
> CC: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  1 +
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 78 ++++++++++++++++++++++++---------
>  2 files changed, 58 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6712955..dc554dd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -754,6 +754,7 @@ struct intel_csr {
>  	i915_reg_t mmioaddr[8];
>  	uint32_t mmiodata[8];
>  	uint32_t dc_state;
> +	uint32_t allowed_dc_mask;
>  };
>  
>  #define DEV_INFO_FOR_EACH_FLAG(func, sep) \
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 6e54d97..30df9de 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -538,12 +538,8 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, uint32_t state)
>  	else
>  		mask |= DC_STATE_EN_UPTO_DC6;
>  
> -	WARN_ON_ONCE(state & ~mask);
> -
> -	if (i915.enable_dc == 0)
> -		state = DC_STATE_DISABLE;
> -	else if (i915.enable_dc == 1 && state > DC_STATE_EN_UPTO_DC5)
> -		state = DC_STATE_EN_UPTO_DC5;
> +	if (WARN_ON_ONCE(state & ~dev_priv->csr.allowed_dc_mask))
> +		state &= dev_priv->csr.allowed_dc_mask;
>  
>  	val = I915_READ(DC_STATE_EN);
>  	DRM_DEBUG_KMS("Setting DC state from %02x to %02x\n",
> @@ -659,8 +655,7 @@ static void gen9_disable_dc5_dc6(struct drm_i915_private *dev_priv)
>  {
>  	assert_can_disable_dc5(dev_priv);
>  
> -	if ((IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) &&
> -	    i915.enable_dc != 0 && i915.enable_dc != 1)
> +	if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
>  		assert_can_disable_dc6(dev_priv);
>  
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> @@ -839,26 +834,19 @@ static void gen9_dc_off_power_well_enable(struct drm_i915_private *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) || IS_KABYLAKE(dev_priv)) &&
> -	    i915.enable_dc != 0 && i915.enable_dc != 1)
> +	if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC6)
>  		skl_enable_dc6(dev_priv);
> -	else
> +	else if (dev_priv->csr.allowed_dc_mask & DC_STATE_EN_UPTO_DC5)
>  		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) || IS_KABYLAKE(dev_priv)) &&
> -		    i915.enable_dc != 0 &&
> -		    i915.enable_dc != 1)
> -			gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6);
> -		else
> -			gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC5);
> -	}
> +	if (power_well->count > 0)
> +		gen9_dc_off_power_well_enable(dev_priv, power_well);
> +	else
> +		gen9_dc_off_power_well_disable(dev_priv, power_well);
>  }
>  
>  static void i9xx_always_on_power_well_noop(struct drm_i915_private *dev_priv,
> @@ -2023,6 +2011,52 @@ sanitize_disable_power_well_option(const struct drm_i915_private *dev_priv,
>  	return 1;
>  }
>  
> +static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
> +				    int enable_dc)
> +{
> +	uint32_t mask;
> +	int requested_dc;
> +	int max_dc;
> +
> +	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
> +		max_dc = 2;
> +		mask = 0;
> +	} else if (IS_BROXTON(dev_priv)) {
> +		max_dc = 1;
> +		/*
> +		 * DC9 has a separate HW flow from the rest of the DC states,
> +		 * not depending on the DMC firmware. It's needed by system
> +		 * suspend/resume, so allow it unconditionally.
> +		 */
> +		mask = DC_STATE_EN_DC9;
> +	} else {
> +		max_dc = 0;
> +		mask = 0;
> +	}
> +
> +	if (enable_dc >= 0 && enable_dc <= max_dc) {
> +		requested_dc = enable_dc;
> +	} else if (enable_dc == -1) {
> +		requested_dc = max_dc;
> +	} else if (enable_dc > max_dc && enable_dc <= 2) {
> +		DRM_DEBUG_KMS("Adjusting requested max DC state (%d->%d)\n",
> +			      enable_dc, max_dc);
> +		requested_dc = max_dc;
> +	} else {
> +		DRM_ERROR("Unexpected value for enable_dc (%d)\n", enable_dc);
> +		requested_dc = max_dc;
> +	}
> +
> +	if (requested_dc > 1)
> +		mask |= DC_STATE_EN_UPTO_DC6;
> +	if (requested_dc > 0)
> +		mask |= DC_STATE_EN_UPTO_DC5;
> +
> +	DRM_DEBUG_KMS("Allowed DC state mask %02x\n", mask);
> +
> +	return mask;
> +}
> +
>  #define set_power_wells(power_domains, __power_wells) ({		\
>  	(power_domains)->power_wells = (__power_wells);			\
>  	(power_domains)->power_well_count = ARRAY_SIZE(__power_wells);	\
> @@ -2041,6 +2075,8 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
>  
>  	i915.disable_power_well = sanitize_disable_power_well_option(dev_priv,
>  						     i915.disable_power_well);
> +	dev_priv->csr.allowed_dc_mask = get_allowed_dc_mask(dev_priv,
> +							    i915.enable_dc);
>  
>  	BUILD_BUG_ON(POWER_DOMAIN_NUM > 31);
>  
> -- 
> 2.5.0
> 

-- 
---
Intel Sweden AB Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden Registration Number: 556189-6027 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 3/4] drm/i915/gen9: Disable DC states if power well support is disabled
  2016-02-29 20:49 ` [PATCH v2 3/4] drm/i915/gen9: Disable DC states if power well support is disabled Imre Deak
@ 2016-03-01 13:01   ` Patrik Jakobsson
  0 siblings, 0 replies; 8+ messages in thread
From: Patrik Jakobsson @ 2016-03-01 13:01 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Mon, Feb 29, 2016 at 10:49:04PM +0200, Imre Deak wrote:
> If power well support is disabled via the i915.disable_power_well module
> option we should never enable DC states. Currently we would enable DC
> states even in this case during system suspend, where we need to disable
> all power wells regardless of the disable_power_well option.
> 
> CC: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 30df9de..f0ca5134 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -2034,6 +2034,9 @@ static uint32_t get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
>  		mask = 0;
>  	}
>  
> +	if (!i915.disable_power_well)
> +		max_dc = 0;
> +
>  	if (enable_dc >= 0 && enable_dc <= max_dc) {
>  		requested_dc = enable_dc;
>  	} else if (enable_dc == -1) {
> -- 
> 2.5.0
> 

-- 
---
Intel Sweden AB Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden Registration Number: 556189-6027 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ✗ Fi.CI.BAT: warning for series starting with [v2,1/4] drm/i915/skl: Fix power domain suspend sequence
  2016-03-01 12:14 ` ✗ Fi.CI.BAT: warning for series starting with [v2,1/4] drm/i915/skl: Fix power domain suspend sequence Patchwork
@ 2016-03-01 17:18   ` Imre Deak
  0 siblings, 0 replies; 8+ messages in thread
From: Imre Deak @ 2016-03-01 17:18 UTC (permalink / raw)
  To: intel-gfx, Patrik Jakobsson

On ti, 2016-03-01 at 12:14 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [v2,1/4] drm/i915/skl: Fix power domain
> suspend sequence
> URL   : https://patchwork.freedesktop.org/series/3931/
> State : warning
> 
> == Summary ==
> 
> Series 3931v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/3931/revisions/1/mbox
> /
> 
> Test kms_flip:
>         Subgroup basic-flip-vs-dpms:
>                 dmesg-warn -> PASS       (hsw-brixbox)
>         Subgroup basic-flip-vs-wf_vblank:
>                 fail       -> PASS       (snb-x220t)
>                 pass       -> DMESG-WARN (hsw-brixbox)

Unclaimed-reg-access/access-while-suspended during atomic
commit/watermark optimization mentioned already in earlier CI BAT
results.

>         Subgroup basic-plain-flip:
>                 pass       -> DMESG-WARN (hsw-brixbox)

Same as above.

> Test kms_pipe_crc_basic:
>         Subgroup read-crc-pipe-a-frame-sequence:
>                 pass       -> DMESG-WARN (snb-x220t)

Same as above.

>         Subgroup suspend-read-crc-pipe-a:
>                 incomplete -> PASS       (hsw-gt2)
> Test pm_rpm:
>         Subgroup basic-pci-d3-state:
>                 pass       -> DMESG-WARN (snb-dellxps)

Same as above.

>         Subgroup basic-rte:
>                 dmesg-warn -> PASS       (snb-x220t)
>                 dmesg-warn -> PASS       (snb-dellxps)
>                 pass       -> DMESG-WARN (bsw-nuc-2)

https://bugs.freedesktop.org/show_bug.cgi?id=94350

I pushed the patches to -dinq, thanks for the review.

--Imre

> 
> bdw-
> nuci7        total:169  pass:158  dwarn:0   dfail:0   fail:0   skip:1
> 1 
> bdw-
> ultra        total:169  pass:155  dwarn:0   dfail:0   fail:0   skip:1
> 4 
> bsw-nuc-
> 2        total:169  pass:137  dwarn:1   dfail:0   fail:1   skip:30 
> byt-
> nuc          total:169  pass:144  dwarn:0   dfail:0   fail:0   skip:2
> 5 
> hsw-
> brixbox      total:169  pass:152  dwarn:2   dfail:0   fail:0   skip:1
> 5 
> hsw-
> gt2          total:169  pass:157  dwarn:1   dfail:0   fail:1   skip:1
> 0 
> ilk-
> hp8440p      total:169  pass:118  dwarn:0   dfail:0   fail:1   skip:5
> 0 
> ivb-
> t430s        total:169  pass:153  dwarn:0   dfail:0   fail:1   skip:1
> 5 
> skl-i5k-
> 2        total:169  pass:153  dwarn:0   dfail:0   fail:0   skip:16 
> skl-i7k-
> 2        total:169  pass:153  dwarn:0   dfail:0   fail:0   skip:16 
> snb-
> dellxps      total:169  pass:144  dwarn:1   dfail:0   fail:1   skip:2
> 3 
> snb-
> x220t        total:169  pass:144  dwarn:1   dfail:0   fail:2   skip:2
> 2 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1498/
> 
> deb861ea08c6d5bbf682fb40ea1f0471a448aafd drm-intel-nightly: 2016y-
> 03m-01d-11h-12m-16s UTC integration manifest
> d5cab5c3d22e85d2a27a36ac0e8797101da08fbf drm/i915/gen9: Remove state
> asserts when disabling DC states
> fd6d00afc02e9a7ebae582854e3514ae48f0a016 drm/i915/gen9: Disable DC
> states if power well support is disabled
> c0a4746a06e556f60f40296fd7eff223b9d70f7a drm/i915/gen9: Sanitize
> handling of allowed DC states
> bd509a542321c86b5ebbd984740e7f010c196faa drm/i915/skl: Fix power
> domain suspend sequence
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-03-01 17:18 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-29 20:49 [PATCH v2 1/4] drm/i915/skl: Fix power domain suspend sequence Imre Deak
2016-02-29 20:49 ` [PATCH v2 2/4] drm/i915/gen9: Sanitize handling of allowed DC states Imre Deak
2016-03-01 12:58   ` Patrik Jakobsson
2016-02-29 20:49 ` [PATCH v2 3/4] drm/i915/gen9: Disable DC states if power well support is disabled Imre Deak
2016-03-01 13:01   ` Patrik Jakobsson
2016-02-29 20:49 ` [PATCH v2 4/4] drm/i915/gen9: Remove state asserts when disabling DC states Imre Deak
2016-03-01 12:14 ` ✗ Fi.CI.BAT: warning for series starting with [v2,1/4] drm/i915/skl: Fix power domain suspend sequence Patchwork
2016-03-01 17:18   ` Imre Deak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox