All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] drm/i915: Power domain fixes
@ 2016-04-18 11:02 ville.syrjala
  2016-04-18 11:02 ` [PATCH 1/3] drm/i915: Set .domains=POWER_DOMAIN_MASK for the always-on well ville.syrjala
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: ville.syrjala @ 2016-04-18 11:02 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Try to define the display power well power domains properly for
HSW/BDW/VLV/CHV, and make the always-on well part of every domain
on every platform.

Ville Syrjälä (3):
  drm/i915: Set .domains=POWER_DOMAIN_MASK for the always-on well
  drm/i915: Define VLV/CHV display power well domains properly
  drm/i915: Define HSW/BDW display power domains the right way up

 drivers/gpu/drm/i915/intel_runtime_pm.c | 109 +++++++++++++++++++++-----------
 1 file changed, 72 insertions(+), 37 deletions(-)

-- 
2.7.4

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

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

* [PATCH 1/3] drm/i915: Set .domains=POWER_DOMAIN_MASK for the always-on well
  2016-04-18 11:02 [PATCH 0/3] drm/i915: Power domain fixes ville.syrjala
@ 2016-04-18 11:02 ` ville.syrjala
  2016-04-19  5:23   ` Imre Deak
  2016-04-18 11:02 ` [PATCH 2/3] drm/i915: Define VLV/CHV display power well domains properly ville.syrjala
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: ville.syrjala @ 2016-04-18 11:02 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The always-on well is the same as runtime PM, so we should just
"enable" it for any power domain. Throw out the usless
FOO_ALWAYS_ON_DOMAINS defines and just use POWER_DOMAIN_MASK.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 259f66f94854..fe63d77b7dfb 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -397,11 +397,6 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 	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_DC_OFF_POWER_DOMAINS)) |		\
-	BIT(POWER_DOMAIN_INIT))
 
 #define BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS (		\
 	BIT(POWER_DOMAIN_TRANSCODER_A) |		\
@@ -424,10 +419,6 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 	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_2_POWER_DOMAINS)) |	\
-	BIT(POWER_DOMAIN_INIT))
 
 static void assert_can_enable_dc9(struct drm_i915_private *dev_priv)
 {
@@ -1624,7 +1615,6 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
 	(POWER_DOMAIN_MASK & ~BDW_ALWAYS_ON_POWER_DOMAINS) |	\
 	BIT(POWER_DOMAIN_INIT))
 
-#define VLV_ALWAYS_ON_POWER_DOMAINS	BIT(POWER_DOMAIN_INIT)
 #define VLV_DISPLAY_POWER_DOMAINS	POWER_DOMAIN_MASK
 
 #define VLV_DPIO_CMN_BC_POWER_DOMAINS (		\
@@ -1722,7 +1712,7 @@ static struct i915_power_well hsw_power_wells[] = {
 	{
 		.name = "always-on",
 		.always_on = 1,
-		.domains = HSW_ALWAYS_ON_POWER_DOMAINS,
+		.domains = POWER_DOMAIN_MASK,
 		.ops = &i9xx_always_on_power_well_ops,
 	},
 	{
@@ -1736,7 +1726,7 @@ static struct i915_power_well bdw_power_wells[] = {
 	{
 		.name = "always-on",
 		.always_on = 1,
-		.domains = BDW_ALWAYS_ON_POWER_DOMAINS,
+		.domains = POWER_DOMAIN_MASK,
 		.ops = &i9xx_always_on_power_well_ops,
 	},
 	{
@@ -1771,7 +1761,7 @@ static struct i915_power_well vlv_power_wells[] = {
 	{
 		.name = "always-on",
 		.always_on = 1,
-		.domains = VLV_ALWAYS_ON_POWER_DOMAINS,
+		.domains = POWER_DOMAIN_MASK,
 		.ops = &i9xx_always_on_power_well_ops,
 		.data = PUNIT_POWER_WELL_ALWAYS_ON,
 	},
@@ -1829,7 +1819,7 @@ static struct i915_power_well chv_power_wells[] = {
 	{
 		.name = "always-on",
 		.always_on = 1,
-		.domains = VLV_ALWAYS_ON_POWER_DOMAINS,
+		.domains = POWER_DOMAIN_MASK,
 		.ops = &i9xx_always_on_power_well_ops,
 	},
 	{
@@ -1873,7 +1863,7 @@ static struct i915_power_well skl_power_wells[] = {
 	{
 		.name = "always-on",
 		.always_on = 1,
-		.domains = SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
+		.domains = POWER_DOMAIN_MASK,
 		.ops = &i9xx_always_on_power_well_ops,
 		.data = SKL_DISP_PW_ALWAYS_ON,
 	},
@@ -1933,7 +1923,7 @@ static struct i915_power_well bxt_power_wells[] = {
 	{
 		.name = "always-on",
 		.always_on = 1,
-		.domains = BXT_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
+		.domains = POWER_DOMAIN_MASK,
 		.ops = &i9xx_always_on_power_well_ops,
 	},
 	{
-- 
2.7.4

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

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

* [PATCH 2/3] drm/i915: Define VLV/CHV display power well domains properly
  2016-04-18 11:02 [PATCH 0/3] drm/i915: Power domain fixes ville.syrjala
  2016-04-18 11:02 ` [PATCH 1/3] drm/i915: Set .domains=POWER_DOMAIN_MASK for the always-on well ville.syrjala
@ 2016-04-18 11:02 ` ville.syrjala
  2016-04-19  5:54   ` Imre Deak
  2016-04-18 11:02 ` [PATCH 3/3] drm/i915: Define HSW/BDW display power domains the right way up ville.syrjala
  2016-04-18 12:03 ` ✗ Fi.CI.BAT: failure for drm/i915: Power domain fixes Patchwork
  3 siblings, 1 reply; 11+ messages in thread
From: ville.syrjala @ 2016-04-18 11:02 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently we're using POWER_DOMAIN_MASK as the power domains for the
display power well on VLV/CHV. That includes all power domains even
though the disp2d/pipe-a power well is not needed for a lot of things.
Let's reduce these to what we actually need.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 42 +++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index fe63d77b7dfb..585bbe7cefa6 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1615,7 +1615,23 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
 	(POWER_DOMAIN_MASK & ~BDW_ALWAYS_ON_POWER_DOMAINS) |	\
 	BIT(POWER_DOMAIN_INIT))
 
-#define VLV_DISPLAY_POWER_DOMAINS	POWER_DOMAIN_MASK
+#define VLV_DISPLAY_POWER_DOMAINS (		\
+	BIT(POWER_DOMAIN_PIPE_A) |		\
+	BIT(POWER_DOMAIN_PIPE_B) |		\
+	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |	\
+	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |	\
+	BIT(POWER_DOMAIN_TRANSCODER_A) |	\
+	BIT(POWER_DOMAIN_TRANSCODER_B) |	\
+	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
+	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |	\
+	BIT(POWER_DOMAIN_PORT_DSI) |		\
+	BIT(POWER_DOMAIN_PORT_CRT) |		\
+	BIT(POWER_DOMAIN_VGA) |			\
+	BIT(POWER_DOMAIN_AUDIO) |		\
+	BIT(POWER_DOMAIN_AUX_B) |		\
+	BIT(POWER_DOMAIN_AUX_C) |		\
+	BIT(POWER_DOMAIN_GMBUS) |		\
+	BIT(POWER_DOMAIN_INIT))
 
 #define VLV_DPIO_CMN_BC_POWER_DOMAINS (		\
 	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
@@ -1645,6 +1661,28 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
 	BIT(POWER_DOMAIN_AUX_C) |		\
 	BIT(POWER_DOMAIN_INIT))
 
+#define CHV_DISPLAY_POWER_DOMAINS (		\
+	BIT(POWER_DOMAIN_PIPE_A) |		\
+	BIT(POWER_DOMAIN_PIPE_B) |		\
+	BIT(POWER_DOMAIN_PIPE_C) |		\
+	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |	\
+	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |	\
+	BIT(POWER_DOMAIN_PIPE_C_PANEL_FITTER) |	\
+	BIT(POWER_DOMAIN_TRANSCODER_A) |	\
+	BIT(POWER_DOMAIN_TRANSCODER_B) |	\
+	BIT(POWER_DOMAIN_TRANSCODER_C) |	\
+	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
+	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |	\
+	BIT(POWER_DOMAIN_PORT_DDI_D_LANES) |	\
+	BIT(POWER_DOMAIN_PORT_DSI) |		\
+	BIT(POWER_DOMAIN_VGA) |			\
+	BIT(POWER_DOMAIN_AUDIO) |		\
+	BIT(POWER_DOMAIN_AUX_B) |		\
+	BIT(POWER_DOMAIN_AUX_C) |		\
+	BIT(POWER_DOMAIN_AUX_D) |		\
+	BIT(POWER_DOMAIN_GMBUS) |		\
+	BIT(POWER_DOMAIN_INIT))
+
 #define CHV_DPIO_CMN_BC_POWER_DOMAINS (		\
 	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
 	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |	\
@@ -1829,7 +1867,7 @@ static struct i915_power_well chv_power_wells[] = {
 		 * power wells don't actually exist. Pipe A power well is
 		 * required for any pipe to work.
 		 */
-		.domains = VLV_DISPLAY_POWER_DOMAINS,
+		.domains = CHV_DISPLAY_POWER_DOMAINS,
 		.data = PIPE_A,
 		.ops = &chv_pipe_power_well_ops,
 	},
-- 
2.7.4

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

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

* [PATCH 3/3] drm/i915: Define HSW/BDW display power domains the right way up
  2016-04-18 11:02 [PATCH 0/3] drm/i915: Power domain fixes ville.syrjala
  2016-04-18 11:02 ` [PATCH 1/3] drm/i915: Set .domains=POWER_DOMAIN_MASK for the always-on well ville.syrjala
  2016-04-18 11:02 ` [PATCH 2/3] drm/i915: Define VLV/CHV display power well domains properly ville.syrjala
@ 2016-04-18 11:02 ` ville.syrjala
  2016-04-19  6:25   ` Imre Deak
  2016-04-18 12:03 ` ✗ Fi.CI.BAT: failure for drm/i915: Power domain fixes Patchwork
  3 siblings, 1 reply; 11+ messages in thread
From: ville.syrjala @ 2016-04-18 11:02 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently we're trying to define HSW/BDW power wells by what's not
included. Let's do it the other way around, so that you can actually
tell when the power well would get enabled. This will also allow us to
add new power domains without accidentally adding it to the HSW/BDW
display power domains.

The current set of domains looks rather buggy even:
- POWER_DOMAIN_MODESET is included in the display power well needlessly
- DDI-B to DDI-E were not part of the display power well when they
  should be

So let's fix that up while at it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 45 +++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 585bbe7cefa6..2cead6cb95a1 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1589,30 +1589,37 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
 	intel_runtime_pm_put(dev_priv);
 }
 
-#define HSW_ALWAYS_ON_POWER_DOMAINS (			\
-	BIT(POWER_DOMAIN_PIPE_A) |			\
-	BIT(POWER_DOMAIN_TRANSCODER_EDP) |		\
-	BIT(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
+#define HSW_DISPLAY_POWER_DOMAINS (			\
+	BIT(POWER_DOMAIN_PIPE_B) |			\
+	BIT(POWER_DOMAIN_PIPE_C) |			\
+	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |		\
+	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |		\
+	BIT(POWER_DOMAIN_PIPE_C_PANEL_FITTER) |		\
+	BIT(POWER_DOMAIN_TRANSCODER_A) |		\
+	BIT(POWER_DOMAIN_TRANSCODER_B) |		\
+	BIT(POWER_DOMAIN_TRANSCODER_C) |		\
 	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |		\
 	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |		\
 	BIT(POWER_DOMAIN_PORT_DDI_D_LANES) |		\
-	BIT(POWER_DOMAIN_PORT_CRT) |			\
-	BIT(POWER_DOMAIN_PLLS) |			\
-	BIT(POWER_DOMAIN_AUX_A) |			\
-	BIT(POWER_DOMAIN_AUX_B) |			\
-	BIT(POWER_DOMAIN_AUX_C) |			\
-	BIT(POWER_DOMAIN_AUX_D) |			\
-	BIT(POWER_DOMAIN_GMBUS) |			\
-	BIT(POWER_DOMAIN_INIT))
-#define HSW_DISPLAY_POWER_DOMAINS (				\
-	(POWER_DOMAIN_MASK & ~HSW_ALWAYS_ON_POWER_DOMAINS) |	\
+	BIT(POWER_DOMAIN_PORT_CRT) | /* DDI E */	\
+	BIT(POWER_DOMAIN_VGA) |				\
+	BIT(POWER_DOMAIN_AUDIO) |			\
 	BIT(POWER_DOMAIN_INIT))
 
-#define BDW_ALWAYS_ON_POWER_DOMAINS (			\
-	HSW_ALWAYS_ON_POWER_DOMAINS |			\
-	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER))
-#define BDW_DISPLAY_POWER_DOMAINS (				\
-	(POWER_DOMAIN_MASK & ~BDW_ALWAYS_ON_POWER_DOMAINS) |	\
+#define BDW_DISPLAY_POWER_DOMAINS (			\
+	BIT(POWER_DOMAIN_PIPE_B) |			\
+	BIT(POWER_DOMAIN_PIPE_C) |			\
+	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |		\
+	BIT(POWER_DOMAIN_PIPE_C_PANEL_FITTER) |		\
+	BIT(POWER_DOMAIN_TRANSCODER_A) |		\
+	BIT(POWER_DOMAIN_TRANSCODER_B) |		\
+	BIT(POWER_DOMAIN_TRANSCODER_C) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_D_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_CRT) | /* DDI E */	\
+	BIT(POWER_DOMAIN_VGA) |				\
+	BIT(POWER_DOMAIN_AUDIO) |			\
 	BIT(POWER_DOMAIN_INIT))
 
 #define VLV_DISPLAY_POWER_DOMAINS (		\
-- 
2.7.4

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

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

* ✗ Fi.CI.BAT: failure for drm/i915: Power domain fixes
  2016-04-18 11:02 [PATCH 0/3] drm/i915: Power domain fixes ville.syrjala
                   ` (2 preceding siblings ...)
  2016-04-18 11:02 ` [PATCH 3/3] drm/i915: Define HSW/BDW display power domains the right way up ville.syrjala
@ 2016-04-18 12:03 ` Patchwork
  2016-04-18 16:42   ` Ville Syrjälä
  3 siblings, 1 reply; 11+ messages in thread
From: Patchwork @ 2016-04-18 12:03 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Power domain fixes
URL   : https://patchwork.freedesktop.org/series/5863/
State : failure

== Summary ==

Series 5863v1 drm/i915: Power domain fixes
http://patchwork.freedesktop.org/api/1.0/series/5863/revisions/1/mbox/

Test gem_ringfill:
        Subgroup basic-default-hang:
                pass       -> INCOMPLETE (snb-dellxps)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-c:
                pass       -> FAIL       (ivb-t430s)

bdw-nuci7        total:203  pass:191  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:203  pass:180  dwarn:0   dfail:0   fail:0   skip:23 
bsw-nuc-2        total:202  pass:162  dwarn:0   dfail:0   fail:0   skip:40 
byt-nuc          total:202  pass:164  dwarn:0   dfail:0   fail:0   skip:38 
hsw-brixbox      total:203  pass:179  dwarn:0   dfail:0   fail:0   skip:24 
hsw-gt2          total:203  pass:184  dwarn:0   dfail:0   fail:0   skip:19 
ilk-hp8440p      total:203  pass:135  dwarn:0   dfail:0   fail:0   skip:68 
ivb-t430s        total:203  pass:174  dwarn:0   dfail:0   fail:1   skip:28 
skl-i7k-2        total:203  pass:178  dwarn:0   dfail:0   fail:0   skip:25 
skl-nuci5        total:203  pass:192  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:27   pass:21   dwarn:0   dfail:0   fail:0   skip:5  
snb-x220t        total:203  pass:165  dwarn:0   dfail:0   fail:1   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_1924/

bcda59813a0c8cea72200c94bfd23f99342476cb drm-intel-nightly: 2016y-04m-18d-10h-25m-54s UTC integration manifest
c2f16de drm/i915: Define HSW/BDW display power domains the right way up
36fc47b drm/i915: Define VLV/CHV display power well domains properly
56f8a66 drm/i915: Set .domains=POWER_DOMAIN_MASK for the always-on well

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

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

* Re: ✗ Fi.CI.BAT: failure for drm/i915: Power domain fixes
  2016-04-18 12:03 ` ✗ Fi.CI.BAT: failure for drm/i915: Power domain fixes Patchwork
@ 2016-04-18 16:42   ` Ville Syrjälä
  2016-04-18 16:52     ` Ville Syrjälä
  0 siblings, 1 reply; 11+ messages in thread
From: Ville Syrjälä @ 2016-04-18 16:42 UTC (permalink / raw)
  To: intel-gfx

On Mon, Apr 18, 2016 at 12:03:01PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Power domain fixes
> URL   : https://patchwork.freedesktop.org/series/5863/
> State : failure
> 
> == Summary ==
> 
> Series 5863v1 drm/i915: Power domain fixes
> http://patchwork.freedesktop.org/api/1.0/series/5863/revisions/1/mbox/
> 
> Test gem_ringfill:
>         Subgroup basic-default-hang:
>                 pass       -> INCOMPLETE (snb-dellxps)

That machine is failing all the time these days.

> Test kms_pipe_crc_basic:
>         Subgroup hang-read-crc-pipe-c:
>                 pass       -> FAIL       (ivb-t430s)

hang-read-crc-pipe-C: Testing connector LVDS-1 using pipe C
hang-read-crc-pipe-C: Testing connector VGA-1 using pipe C
Timed out: CRC reading
Subtest hang-read-crc-pipe-C: FAIL (7.944s)

[  251.726624] [drm:drm_mode_setcrtc] [CRTC:34:crtc-2]
[  251.726647] [drm:drm_mode_setcrtc] [CONNECTOR:36:LVDS-1]
[  251.726743] [drm:connected_sink_compute_bpp] [CONNECTOR:36:LVDS-1] checking for sink bpp constrains
[  251.726746] [drm:connected_sink_compute_bpp] clamping display bpp (was 36) to default limit of 24
[  251.726748] [drm:intel_lvds_compute_config] forcing display bpp (was 24) to LVDS (18)
[  251.726752] [drm:ironlake_check_fdi_lanes] checking fdi config on pipe C, lanes 1
[  251.726763] [drm:intel_modeset_pipe_config] hw max bpp: 36, pipe bpp: 18, dithering: 1
[  251.726766] [drm:intel_dump_pipe_config] [CRTC:34][modeset] config ffff8800b6880008 for pipe C
...
[  251.728878] [drm:intel_enable_pipe] enabling pipe C
[  251.729054] [drm:ivb_manual_fdi_link_train] FDI_RX_IIR before link train 0x0
[  251.729073] [drm:ivb_manual_fdi_link_train] FDI_RX_IIR 0x100
[  251.729077] [drm:ivb_manual_fdi_link_train] FDI train 1 done, level 0.
[  251.729088] [drm:ivb_manual_fdi_link_train] FDI_RX_IIR 0x200
[  251.729092] [drm:ivb_manual_fdi_link_train] FDI train 2 done, level 0.
[  251.729094] [drm:ivb_manual_fdi_link_train] FDI train done.
[  251.729100] [drm:intel_enable_shared_dpll] enable PCH DPLL A (active 4, on? 0) for crtc 34
[  251.729102] [drm:intel_enable_shared_dpll] enabling PCH DPLL A
[  252.229929] [drm:intel_panel_enable_backlight] pipe C
[  252.229939] [drm:intel_panel_actually_set_backlight] set backlight PWM = 261
[  252.360235] [drm:intel_connector_verify_state] [CONNECTOR:36:LVDS-1]
[  252.360243] [drm:verify_crtc_state] [CRTC:34]
[  252.360266] [drm:verify_single_dpll_state] PCH DPLL A
[  252.400463] [drm:pipe_crc_set_source] collecting CRCs for pipe C, pf
[  252.560686] [drm:pipe_crc_set_source] stopping CRCs for pipe C
...
[  252.593092] [drm:drm_mode_setcrtc] [CRTC:34:crtc-2]
[  252.680937] [drm:intel_panel_actually_set_backlight] set backlight PWM = 0
[  252.680951] [drm:intel_disable_pipe] disabling pipe C
[  252.751085] [drm:intel_get_hpd_pins] hotplug event received, stat 0x00080000, dig 0x00101010, pins 0x00000002
[  252.751088] [drm:intel_hpd_irq_storm_detect] Received HPD interrupt on PIN 1 - cnt: 0
[  252.991855] [drm:intel_disable_shared_dpll] disable PCH DPLL A (active 4, on? 1) for crtc 34
[  252.991865] [drm:intel_disable_shared_dpll] disabling PCH DPLL A
...
[  253.000105] [drm:pipe_crc_set_source] collecting CRCs for pipe C, pf
[  258.003168] kms_pipe_crc_basic: exiting, ret=99
[  258.003308] [drm:pipe_crc_set_source] stopping CRCs for pipe C

So it looks like it was expecting to get CRCs from a disabled pipe.
No clue what made it do that.

Anyways, IVB doesn't have power wells so unlikely this is related.

> 
> bdw-nuci7        total:203  pass:191  dwarn:0   dfail:0   fail:0   skip:12 
> bdw-ultra        total:203  pass:180  dwarn:0   dfail:0   fail:0   skip:23 
> bsw-nuc-2        total:202  pass:162  dwarn:0   dfail:0   fail:0   skip:40 
> byt-nuc          total:202  pass:164  dwarn:0   dfail:0   fail:0   skip:38 
> hsw-brixbox      total:203  pass:179  dwarn:0   dfail:0   fail:0   skip:24 
> hsw-gt2          total:203  pass:184  dwarn:0   dfail:0   fail:0   skip:19 
> ilk-hp8440p      total:203  pass:135  dwarn:0   dfail:0   fail:0   skip:68 
> ivb-t430s        total:203  pass:174  dwarn:0   dfail:0   fail:1   skip:28 
> skl-i7k-2        total:203  pass:178  dwarn:0   dfail:0   fail:0   skip:25 
> skl-nuci5        total:203  pass:192  dwarn:0   dfail:0   fail:0   skip:11 
> snb-dellxps      total:27   pass:21   dwarn:0   dfail:0   fail:0   skip:5  
> snb-x220t        total:203  pass:165  dwarn:0   dfail:0   fail:1   skip:37 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1924/
> 
> bcda59813a0c8cea72200c94bfd23f99342476cb drm-intel-nightly: 2016y-04m-18d-10h-25m-54s UTC integration manifest
> c2f16de drm/i915: Define HSW/BDW display power domains the right way up
> 36fc47b drm/i915: Define VLV/CHV display power well domains properly
> 56f8a66 drm/i915: Set .domains=POWER_DOMAIN_MASK for the always-on well

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: ✗ Fi.CI.BAT:  failure for drm/i915: Power domain fixes
  2016-04-18 16:42   ` Ville Syrjälä
@ 2016-04-18 16:52     ` Ville Syrjälä
  0 siblings, 0 replies; 11+ messages in thread
From: Ville Syrjälä @ 2016-04-18 16:52 UTC (permalink / raw)
  To: intel-gfx

On Mon, Apr 18, 2016 at 07:42:00PM +0300, Ville Syrjälä wrote:
> On Mon, Apr 18, 2016 at 12:03:01PM -0000, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: drm/i915: Power domain fixes
> > URL   : https://patchwork.freedesktop.org/series/5863/
> > State : failure
> > 
> > == Summary ==
> > 
> > Series 5863v1 drm/i915: Power domain fixes
> > http://patchwork.freedesktop.org/api/1.0/series/5863/revisions/1/mbox/
> > 
> > Test gem_ringfill:
> >         Subgroup basic-default-hang:
> >                 pass       -> INCOMPLETE (snb-dellxps)
> 
> That machine is failing all the time these days.
> 
> > Test kms_pipe_crc_basic:
> >         Subgroup hang-read-crc-pipe-c:
> >                 pass       -> FAIL       (ivb-t430s)
> 
> hang-read-crc-pipe-C: Testing connector LVDS-1 using pipe C
> hang-read-crc-pipe-C: Testing connector VGA-1 using pipe C
> Timed out: CRC reading
> Subtest hang-read-crc-pipe-C: FAIL (7.944s)
> 
> [  251.726624] [drm:drm_mode_setcrtc] [CRTC:34:crtc-2]
> [  251.726647] [drm:drm_mode_setcrtc] [CONNECTOR:36:LVDS-1]
> [  251.726743] [drm:connected_sink_compute_bpp] [CONNECTOR:36:LVDS-1] checking for sink bpp constrains
> [  251.726746] [drm:connected_sink_compute_bpp] clamping display bpp (was 36) to default limit of 24
> [  251.726748] [drm:intel_lvds_compute_config] forcing display bpp (was 24) to LVDS (18)
> [  251.726752] [drm:ironlake_check_fdi_lanes] checking fdi config on pipe C, lanes 1
> [  251.726763] [drm:intel_modeset_pipe_config] hw max bpp: 36, pipe bpp: 18, dithering: 1
> [  251.726766] [drm:intel_dump_pipe_config] [CRTC:34][modeset] config ffff8800b6880008 for pipe C
> ...
> [  251.728878] [drm:intel_enable_pipe] enabling pipe C
> [  251.729054] [drm:ivb_manual_fdi_link_train] FDI_RX_IIR before link train 0x0
> [  251.729073] [drm:ivb_manual_fdi_link_train] FDI_RX_IIR 0x100
> [  251.729077] [drm:ivb_manual_fdi_link_train] FDI train 1 done, level 0.
> [  251.729088] [drm:ivb_manual_fdi_link_train] FDI_RX_IIR 0x200
> [  251.729092] [drm:ivb_manual_fdi_link_train] FDI train 2 done, level 0.
> [  251.729094] [drm:ivb_manual_fdi_link_train] FDI train done.
> [  251.729100] [drm:intel_enable_shared_dpll] enable PCH DPLL A (active 4, on? 0) for crtc 34
> [  251.729102] [drm:intel_enable_shared_dpll] enabling PCH DPLL A
> [  252.229929] [drm:intel_panel_enable_backlight] pipe C
> [  252.229939] [drm:intel_panel_actually_set_backlight] set backlight PWM = 261
> [  252.360235] [drm:intel_connector_verify_state] [CONNECTOR:36:LVDS-1]
> [  252.360243] [drm:verify_crtc_state] [CRTC:34]
> [  252.360266] [drm:verify_single_dpll_state] PCH DPLL A
> [  252.400463] [drm:pipe_crc_set_source] collecting CRCs for pipe C, pf
> [  252.560686] [drm:pipe_crc_set_source] stopping CRCs for pipe C
> ...
> [  252.593092] [drm:drm_mode_setcrtc] [CRTC:34:crtc-2]
> [  252.680937] [drm:intel_panel_actually_set_backlight] set backlight PWM = 0
> [  252.680951] [drm:intel_disable_pipe] disabling pipe C
> [  252.751085] [drm:intel_get_hpd_pins] hotplug event received, stat 0x00080000, dig 0x00101010, pins 0x00000002
> [  252.751088] [drm:intel_hpd_irq_storm_detect] Received HPD interrupt on PIN 1 - cnt: 0
> [  252.991855] [drm:intel_disable_shared_dpll] disable PCH DPLL A (active 4, on? 1) for crtc 34
> [  252.991865] [drm:intel_disable_shared_dpll] disabling PCH DPLL A
> ...
> [  253.000105] [drm:pipe_crc_set_source] collecting CRCs for pipe C, pf
> [  258.003168] kms_pipe_crc_basic: exiting, ret=99
> [  258.003308] [drm:pipe_crc_set_source] stopping CRCs for pipe C
> 
> So it looks like it was expecting to get CRCs from a disabled pipe.
> No clue what made it do that.

Actually there was a CRT got disconnect in the log shortly before it
failed. Might be the test is just racy w.r.t. disconnects.

> 
> Anyways, IVB doesn't have power wells so unlikely this is related.
> 
> > 
> > bdw-nuci7        total:203  pass:191  dwarn:0   dfail:0   fail:0   skip:12 
> > bdw-ultra        total:203  pass:180  dwarn:0   dfail:0   fail:0   skip:23 
> > bsw-nuc-2        total:202  pass:162  dwarn:0   dfail:0   fail:0   skip:40 
> > byt-nuc          total:202  pass:164  dwarn:0   dfail:0   fail:0   skip:38 
> > hsw-brixbox      total:203  pass:179  dwarn:0   dfail:0   fail:0   skip:24 
> > hsw-gt2          total:203  pass:184  dwarn:0   dfail:0   fail:0   skip:19 
> > ilk-hp8440p      total:203  pass:135  dwarn:0   dfail:0   fail:0   skip:68 
> > ivb-t430s        total:203  pass:174  dwarn:0   dfail:0   fail:1   skip:28 
> > skl-i7k-2        total:203  pass:178  dwarn:0   dfail:0   fail:0   skip:25 
> > skl-nuci5        total:203  pass:192  dwarn:0   dfail:0   fail:0   skip:11 
> > snb-dellxps      total:27   pass:21   dwarn:0   dfail:0   fail:0   skip:5  
> > snb-x220t        total:203  pass:165  dwarn:0   dfail:0   fail:1   skip:37 
> > 
> > Results at /archive/results/CI_IGT_test/Patchwork_1924/
> > 
> > bcda59813a0c8cea72200c94bfd23f99342476cb drm-intel-nightly: 2016y-04m-18d-10h-25m-54s UTC integration manifest
> > c2f16de drm/i915: Define HSW/BDW display power domains the right way up
> > 36fc47b drm/i915: Define VLV/CHV display power well domains properly
> > 56f8a66 drm/i915: Set .domains=POWER_DOMAIN_MASK for the always-on well
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/3] drm/i915: Set .domains=POWER_DOMAIN_MASK for the always-on well
  2016-04-18 11:02 ` [PATCH 1/3] drm/i915: Set .domains=POWER_DOMAIN_MASK for the always-on well ville.syrjala
@ 2016-04-19  5:23   ` Imre Deak
  0 siblings, 0 replies; 11+ messages in thread
From: Imre Deak @ 2016-04-19  5:23 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Mon, 2016-04-18 at 14:02 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The always-on well is the same as runtime PM, so we should just
> "enable" it for any power domain. Throw out the usless
> FOO_ALWAYS_ON_DOMAINS defines and just use POWER_DOMAIN_MASK.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Yep, nice clarification of the concept and code. It gets rid of the
on/off ping pong of the always on well too. Btw, this would make it
possible to get an RPM reference only from the always-on well, though
that would only simplify things somewhat on the concept level. Looks
ok:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 22 ++++++----------------
>  1 file changed, 6 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 259f66f94854..fe63d77b7dfb 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -397,11 +397,6 @@ static void hsw_set_power_well(struct
> drm_i915_private *dev_priv,
>  	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_DC_OFF_POWER_DOMAINS)) |		\
> -	BIT(POWER_DOMAIN_INIT))
>  
>  #define BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS (		\
>  	BIT(POWER_DOMAIN_TRANSCODER_A) |		\
> @@ -424,10 +419,6 @@ static void hsw_set_power_well(struct
> drm_i915_private *dev_priv,
>  	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_2_POWER_DOMAINS)) |	\
> -	BIT(POWER_DOMAIN_INIT))
>  
>  static void assert_can_enable_dc9(struct drm_i915_private *dev_priv)
>  {
> @@ -1624,7 +1615,6 @@ void intel_display_power_put(struct
> drm_i915_private *dev_priv,
>  	(POWER_DOMAIN_MASK & ~BDW_ALWAYS_ON_POWER_DOMAINS) |	
> \
>  	BIT(POWER_DOMAIN_INIT))
>  
> -#define VLV_ALWAYS_ON_POWER_DOMAINS	BIT(POWER_DOMAIN_INIT)
>  #define VLV_DISPLAY_POWER_DOMAINS	POWER_DOMAIN_MASK
>  
>  #define VLV_DPIO_CMN_BC_POWER_DOMAINS (		\
> @@ -1722,7 +1712,7 @@ static struct i915_power_well hsw_power_wells[]
> = {
>  	{
>  		.name = "always-on",
>  		.always_on = 1,
> -		.domains = HSW_ALWAYS_ON_POWER_DOMAINS,
> +		.domains = POWER_DOMAIN_MASK,
>  		.ops = &i9xx_always_on_power_well_ops,
>  	},
>  	{
> @@ -1736,7 +1726,7 @@ static struct i915_power_well bdw_power_wells[]
> = {
>  	{
>  		.name = "always-on",
>  		.always_on = 1,
> -		.domains = BDW_ALWAYS_ON_POWER_DOMAINS,
> +		.domains = POWER_DOMAIN_MASK,
>  		.ops = &i9xx_always_on_power_well_ops,
>  	},
>  	{
> @@ -1771,7 +1761,7 @@ static struct i915_power_well vlv_power_wells[]
> = {
>  	{
>  		.name = "always-on",
>  		.always_on = 1,
> -		.domains = VLV_ALWAYS_ON_POWER_DOMAINS,
> +		.domains = POWER_DOMAIN_MASK,
>  		.ops = &i9xx_always_on_power_well_ops,
>  		.data = PUNIT_POWER_WELL_ALWAYS_ON,
>  	},
> @@ -1829,7 +1819,7 @@ static struct i915_power_well chv_power_wells[]
> = {
>  	{
>  		.name = "always-on",
>  		.always_on = 1,
> -		.domains = VLV_ALWAYS_ON_POWER_DOMAINS,
> +		.domains = POWER_DOMAIN_MASK,
>  		.ops = &i9xx_always_on_power_well_ops,
>  	},
>  	{
> @@ -1873,7 +1863,7 @@ static struct i915_power_well skl_power_wells[]
> = {
>  	{
>  		.name = "always-on",
>  		.always_on = 1,
> -		.domains = SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
> +		.domains = POWER_DOMAIN_MASK,
>  		.ops = &i9xx_always_on_power_well_ops,
>  		.data = SKL_DISP_PW_ALWAYS_ON,
>  	},
> @@ -1933,7 +1923,7 @@ static struct i915_power_well bxt_power_wells[]
> = {
>  	{
>  		.name = "always-on",
>  		.always_on = 1,
> -		.domains = BXT_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
> +		.domains = POWER_DOMAIN_MASK,
>  		.ops = &i9xx_always_on_power_well_ops,
>  	},
>  	{
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/3] drm/i915: Define VLV/CHV display power well domains properly
  2016-04-18 11:02 ` [PATCH 2/3] drm/i915: Define VLV/CHV display power well domains properly ville.syrjala
@ 2016-04-19  5:54   ` Imre Deak
  2016-04-19 12:04     ` Ville Syrjälä
  0 siblings, 1 reply; 11+ messages in thread
From: Imre Deak @ 2016-04-19  5:54 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Mon, 2016-04-18 at 14:02 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we're using POWER_DOMAIN_MASK as the power domains for the
> display power well on VLV/CHV. That includes all power domains even
> though the disp2d/pipe-a power well is not needed for a lot of
> things.
> Let's reduce these to what we actually need.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Ok, so this mostly removes domains that are not defined on the given
platform and so shouldn't be requested. (Btw, the always-on well will
be different now in this regard after patch 1, since for that we also
include the not-defined domains, but I think that's ok.) The modeset
domain is the exception which may power things atm unnecessarily. Looks
ok:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 42
> +++++++++++++++++++++++++++++++--
>  1 file changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index fe63d77b7dfb..585bbe7cefa6 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1615,7 +1615,23 @@ void intel_display_power_put(struct
> drm_i915_private *dev_priv,
>  	(POWER_DOMAIN_MASK & ~BDW_ALWAYS_ON_POWER_DOMAINS) |	
> \
>  	BIT(POWER_DOMAIN_INIT))
>  
> -#define VLV_DISPLAY_POWER_DOMAINS	POWER_DOMAIN_MASK
> +#define VLV_DISPLAY_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_PIPE_A) |		\
> +	BIT(POWER_DOMAIN_PIPE_B) |		\
> +	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |	\
> +	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |	\
> +	BIT(POWER_DOMAIN_TRANSCODER_A) |	\
> +	BIT(POWER_DOMAIN_TRANSCODER_B) |	\
> +	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
> +	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |	\
> +	BIT(POWER_DOMAIN_PORT_DSI) |		\
> +	BIT(POWER_DOMAIN_PORT_CRT) |		\
> +	BIT(POWER_DOMAIN_VGA) |			\
> +	BIT(POWER_DOMAIN_AUDIO) |		\
> +	BIT(POWER_DOMAIN_AUX_B) |		\
> +	BIT(POWER_DOMAIN_AUX_C) |		\
> +	BIT(POWER_DOMAIN_GMBUS) |		\
> +	BIT(POWER_DOMAIN_INIT))
>  
>  #define VLV_DPIO_CMN_BC_POWER_DOMAINS (		\
>  	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
> @@ -1645,6 +1661,28 @@ void intel_display_power_put(struct
> drm_i915_private *dev_priv,
>  	BIT(POWER_DOMAIN_AUX_C) |		\
>  	BIT(POWER_DOMAIN_INIT))
>  
> +#define CHV_DISPLAY_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_PIPE_A) |		\
> +	BIT(POWER_DOMAIN_PIPE_B) |		\
> +	BIT(POWER_DOMAIN_PIPE_C) |		\
> +	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |	\
> +	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |	\
> +	BIT(POWER_DOMAIN_PIPE_C_PANEL_FITTER) |	\
> +	BIT(POWER_DOMAIN_TRANSCODER_A) |	\
> +	BIT(POWER_DOMAIN_TRANSCODER_B) |	\
> +	BIT(POWER_DOMAIN_TRANSCODER_C) |	\
> +	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
> +	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |	\
> +	BIT(POWER_DOMAIN_PORT_DDI_D_LANES) |	\
> +	BIT(POWER_DOMAIN_PORT_DSI) |		\
> +	BIT(POWER_DOMAIN_VGA) |			\
> +	BIT(POWER_DOMAIN_AUDIO) |		\
> +	BIT(POWER_DOMAIN_AUX_B) |		\
> +	BIT(POWER_DOMAIN_AUX_C) |		\
> +	BIT(POWER_DOMAIN_AUX_D) |		\
> +	BIT(POWER_DOMAIN_GMBUS) |		\
> +	BIT(POWER_DOMAIN_INIT))
> +
>  #define CHV_DPIO_CMN_BC_POWER_DOMAINS (		\
>  	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
>  	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |	\
> @@ -1829,7 +1867,7 @@ static struct i915_power_well chv_power_wells[]
> = {
>  		 * power wells don't actually exist. Pipe A power
> well is
>  		 * required for any pipe to work.
>  		 */
> -		.domains = VLV_DISPLAY_POWER_DOMAINS,
> +		.domains = CHV_DISPLAY_POWER_DOMAINS,
>  		.data = PIPE_A,
>  		.ops = &chv_pipe_power_well_ops,
>  	},
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/3] drm/i915: Define HSW/BDW display power domains the right way up
  2016-04-18 11:02 ` [PATCH 3/3] drm/i915: Define HSW/BDW display power domains the right way up ville.syrjala
@ 2016-04-19  6:25   ` Imre Deak
  0 siblings, 0 replies; 11+ messages in thread
From: Imre Deak @ 2016-04-19  6:25 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Mon, 2016-04-18 at 14:02 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we're trying to define HSW/BDW power wells by what's not
> included. Let's do it the other way around, so that you can actually
> tell when the power well would get enabled. This will also allow us
> to
> add new power domains without accidentally adding it to the HSW/BDW
> display power domains.
> 
> The current set of domains looks rather buggy even:
> - POWER_DOMAIN_MODESET is included in the display power well
> needlessly
> - DDI-B to DDI-E were not part of the display power well when they
>   should be
> 
> So let's fix that up while at it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Yes, much clearer without negations and indirections. We discussed this
already in person, but for reference: not including the DDI domains in
the display power well happened not to cause a problem, since they are
always requested along with their respective pipe and transcoder
domains. Nice work,
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 45 +++++++++++++++++++--
> ------------
>  1 file changed, 26 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 585bbe7cefa6..2cead6cb95a1 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1589,30 +1589,37 @@ void intel_display_power_put(struct
> drm_i915_private *dev_priv,
>  	intel_runtime_pm_put(dev_priv);
>  }
>  
> -#define HSW_ALWAYS_ON_POWER_DOMAINS (			\
> -	BIT(POWER_DOMAIN_PIPE_A) |			\
> -	BIT(POWER_DOMAIN_TRANSCODER_EDP) |		\
> -	BIT(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
> +#define HSW_DISPLAY_POWER_DOMAINS (			\
> +	BIT(POWER_DOMAIN_PIPE_B) |			\
> +	BIT(POWER_DOMAIN_PIPE_C) |			\
> +	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |		\
> +	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |		\
> +	BIT(POWER_DOMAIN_PIPE_C_PANEL_FITTER) |		\
> +	BIT(POWER_DOMAIN_TRANSCODER_A) |		\
> +	BIT(POWER_DOMAIN_TRANSCODER_B) |		\
> +	BIT(POWER_DOMAIN_TRANSCODER_C) |		\
>  	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |		\
>  	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |		\
>  	BIT(POWER_DOMAIN_PORT_DDI_D_LANES) |		\
> -	BIT(POWER_DOMAIN_PORT_CRT) |			\
> -	BIT(POWER_DOMAIN_PLLS) |			\
> -	BIT(POWER_DOMAIN_AUX_A) |			\
> -	BIT(POWER_DOMAIN_AUX_B) |			\
> -	BIT(POWER_DOMAIN_AUX_C) |			\
> -	BIT(POWER_DOMAIN_AUX_D) |			\
> -	BIT(POWER_DOMAIN_GMBUS) |			\
> -	BIT(POWER_DOMAIN_INIT))
> -#define HSW_DISPLAY_POWER_DOMAINS (				\
> -	(POWER_DOMAIN_MASK & ~HSW_ALWAYS_ON_POWER_DOMAINS) |	
> \
> +	BIT(POWER_DOMAIN_PORT_CRT) | /* DDI E */	\
> +	BIT(POWER_DOMAIN_VGA) |				\
> +	BIT(POWER_DOMAIN_AUDIO) |			\
>  	BIT(POWER_DOMAIN_INIT))
>  
> -#define BDW_ALWAYS_ON_POWER_DOMAINS (			\
> -	HSW_ALWAYS_ON_POWER_DOMAINS |			\
> -	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER))
> -#define BDW_DISPLAY_POWER_DOMAINS (				\
> -	(POWER_DOMAIN_MASK & ~BDW_ALWAYS_ON_POWER_DOMAINS) |	
> \
> +#define BDW_DISPLAY_POWER_DOMAINS (			\
> +	BIT(POWER_DOMAIN_PIPE_B) |			\
> +	BIT(POWER_DOMAIN_PIPE_C) |			\
> +	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |		\
> +	BIT(POWER_DOMAIN_PIPE_C_PANEL_FITTER) |		\
> +	BIT(POWER_DOMAIN_TRANSCODER_A) |		\
> +	BIT(POWER_DOMAIN_TRANSCODER_B) |		\
> +	BIT(POWER_DOMAIN_TRANSCODER_C) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_D_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_CRT) | /* DDI E */	\
> +	BIT(POWER_DOMAIN_VGA) |				\
> +	BIT(POWER_DOMAIN_AUDIO) |			\
>  	BIT(POWER_DOMAIN_INIT))
>  
>  #define VLV_DISPLAY_POWER_DOMAINS (		\
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/3] drm/i915: Define VLV/CHV display power well domains properly
  2016-04-19  5:54   ` Imre Deak
@ 2016-04-19 12:04     ` Ville Syrjälä
  0 siblings, 0 replies; 11+ messages in thread
From: Ville Syrjälä @ 2016-04-19 12:04 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Tue, Apr 19, 2016 at 08:54:05AM +0300, Imre Deak wrote:
> On Mon, 2016-04-18 at 14:02 +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Currently we're using POWER_DOMAIN_MASK as the power domains for the
> > display power well on VLV/CHV. That includes all power domains even
> > though the disp2d/pipe-a power well is not needed for a lot of
> > things.
> > Let's reduce these to what we actually need.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Ok, so this mostly removes domains that are not defined on the given
> platform and so shouldn't be requested. (Btw, the always-on well will
> be different now in this regard after patch 1, since for that we also
> include the not-defined domains, but I think that's ok.) The modeset
> domain is the exception which may power things atm unnecessarily.

True. But probably not worth making an exception there.

> Looks
> ok:
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Thanks. Entire series pushed to dinq.

> 
> > ---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 42
> > +++++++++++++++++++++++++++++++--
> >  1 file changed, 40 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index fe63d77b7dfb..585bbe7cefa6 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -1615,7 +1615,23 @@ void intel_display_power_put(struct
> > drm_i915_private *dev_priv,
> >  	(POWER_DOMAIN_MASK & ~BDW_ALWAYS_ON_POWER_DOMAINS) |	
> > \
> >  	BIT(POWER_DOMAIN_INIT))
> >  
> > -#define VLV_DISPLAY_POWER_DOMAINS	POWER_DOMAIN_MASK
> > +#define VLV_DISPLAY_POWER_DOMAINS (		\
> > +	BIT(POWER_DOMAIN_PIPE_A) |		\
> > +	BIT(POWER_DOMAIN_PIPE_B) |		\
> > +	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |	\
> > +	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |	\
> > +	BIT(POWER_DOMAIN_TRANSCODER_A) |	\
> > +	BIT(POWER_DOMAIN_TRANSCODER_B) |	\
> > +	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
> > +	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |	\
> > +	BIT(POWER_DOMAIN_PORT_DSI) |		\
> > +	BIT(POWER_DOMAIN_PORT_CRT) |		\
> > +	BIT(POWER_DOMAIN_VGA) |			\
> > +	BIT(POWER_DOMAIN_AUDIO) |		\
> > +	BIT(POWER_DOMAIN_AUX_B) |		\
> > +	BIT(POWER_DOMAIN_AUX_C) |		\
> > +	BIT(POWER_DOMAIN_GMBUS) |		\
> > +	BIT(POWER_DOMAIN_INIT))
> >  
> >  #define VLV_DPIO_CMN_BC_POWER_DOMAINS (		\
> >  	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
> > @@ -1645,6 +1661,28 @@ void intel_display_power_put(struct
> > drm_i915_private *dev_priv,
> >  	BIT(POWER_DOMAIN_AUX_C) |		\
> >  	BIT(POWER_DOMAIN_INIT))
> >  
> > +#define CHV_DISPLAY_POWER_DOMAINS (		\
> > +	BIT(POWER_DOMAIN_PIPE_A) |		\
> > +	BIT(POWER_DOMAIN_PIPE_B) |		\
> > +	BIT(POWER_DOMAIN_PIPE_C) |		\
> > +	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |	\
> > +	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |	\
> > +	BIT(POWER_DOMAIN_PIPE_C_PANEL_FITTER) |	\
> > +	BIT(POWER_DOMAIN_TRANSCODER_A) |	\
> > +	BIT(POWER_DOMAIN_TRANSCODER_B) |	\
> > +	BIT(POWER_DOMAIN_TRANSCODER_C) |	\
> > +	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
> > +	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |	\
> > +	BIT(POWER_DOMAIN_PORT_DDI_D_LANES) |	\
> > +	BIT(POWER_DOMAIN_PORT_DSI) |		\
> > +	BIT(POWER_DOMAIN_VGA) |			\
> > +	BIT(POWER_DOMAIN_AUDIO) |		\
> > +	BIT(POWER_DOMAIN_AUX_B) |		\
> > +	BIT(POWER_DOMAIN_AUX_C) |		\
> > +	BIT(POWER_DOMAIN_AUX_D) |		\
> > +	BIT(POWER_DOMAIN_GMBUS) |		\
> > +	BIT(POWER_DOMAIN_INIT))
> > +
> >  #define CHV_DPIO_CMN_BC_POWER_DOMAINS (		\
> >  	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
> >  	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |	\
> > @@ -1829,7 +1867,7 @@ static struct i915_power_well chv_power_wells[]
> > = {
> >  		 * power wells don't actually exist. Pipe A power
> > well is
> >  		 * required for any pipe to work.
> >  		 */
> > -		.domains = VLV_DISPLAY_POWER_DOMAINS,
> > +		.domains = CHV_DISPLAY_POWER_DOMAINS,
> >  		.data = PIPE_A,
> >  		.ops = &chv_pipe_power_well_ops,
> >  	},

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2016-04-19 12:04 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-18 11:02 [PATCH 0/3] drm/i915: Power domain fixes ville.syrjala
2016-04-18 11:02 ` [PATCH 1/3] drm/i915: Set .domains=POWER_DOMAIN_MASK for the always-on well ville.syrjala
2016-04-19  5:23   ` Imre Deak
2016-04-18 11:02 ` [PATCH 2/3] drm/i915: Define VLV/CHV display power well domains properly ville.syrjala
2016-04-19  5:54   ` Imre Deak
2016-04-19 12:04     ` Ville Syrjälä
2016-04-18 11:02 ` [PATCH 3/3] drm/i915: Define HSW/BDW display power domains the right way up ville.syrjala
2016-04-19  6:25   ` Imre Deak
2016-04-18 12:03 ` ✗ Fi.CI.BAT: failure for drm/i915: Power domain fixes Patchwork
2016-04-18 16:42   ` Ville Syrjälä
2016-04-18 16:52     ` Ville Syrjälä

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.