All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Power well support for SKL
@ 2015-01-16 15:57 Damien Lespiau
  2015-01-16 15:57 ` [PATCH 1/2] drm/i915/skl: Adding power domains for AUX controllers Damien Lespiau
  2015-01-16 15:57 ` [PATCH 2/2] drm/i915/skl: Implementation of SKL display power well support Damien Lespiau
  0 siblings, 2 replies; 13+ messages in thread
From: Damien Lespiau @ 2015-01-16 15:57 UTC (permalink / raw)
  To: intel-gfx

Those two patches implement power support for SKL. Patch 1 is already reviewed.

For the moment we don't do anything fancy with the AUX power domain, so the
series has no real impact on current hardware to ease its inclusion in 3.20.

-- 
Damien

Satheeshakrishna M (2):
  drm/i915/skl: Adding power domains for AUX controllers
  drm/i915/skl: Implementation of SKL display power well support

 drivers/gpu/drm/i915/i915_debugfs.c     |   8 ++
 drivers/gpu/drm/i915/i915_drv.h         |   4 +
 drivers/gpu/drm/i915/i915_reg.h         |  20 +++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 245 ++++++++++++++++++++++++++++++++
 4 files changed, 277 insertions(+)

-- 
1.8.3.1

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

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

* [PATCH 1/2] drm/i915/skl: Adding power domains for AUX controllers
  2015-01-16 15:57 [PATCH 0/2] Power well support for SKL Damien Lespiau
@ 2015-01-16 15:57 ` Damien Lespiau
  2015-01-20 10:11   ` Daniel Vetter
  2015-01-16 15:57 ` [PATCH 2/2] drm/i915/skl: Implementation of SKL display power well support Damien Lespiau
  1 sibling, 1 reply; 13+ messages in thread
From: Damien Lespiau @ 2015-01-16 15:57 UTC (permalink / raw)
  To: intel-gfx

From: Satheeshakrishna M <satheeshakrishna.m@intel.com>

Adding new power doamins for AUX controllers

v2: Added new power domains in power_domain_str per Imre's comment

v3: Added AUX power domains to older platforms

v4: Rebase on top of POWER_DOMAIN_PLLS.

v5: Modified to address review comments from Imre

Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Satheeshakrishna M <satheeshakrishna.m@intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> (v3)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c     |  8 ++++++++
 drivers/gpu/drm/i915/i915_drv.h         |  4 ++++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++
 3 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e515aad..aac6126 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2397,6 +2397,14 @@ static const char *power_domain_str(enum intel_display_power_domain domain)
 		return "AUDIO";
 	case POWER_DOMAIN_PLLS:
 		return "PLLS";
+	case POWER_DOMAIN_AUX_A:
+		return "AUX_A";
+	case POWER_DOMAIN_AUX_B:
+		return "AUX_B";
+	case POWER_DOMAIN_AUX_C:
+		return "AUX_C";
+	case POWER_DOMAIN_AUX_D:
+		return "AUX_D";
 	case POWER_DOMAIN_INIT:
 		return "INIT";
 	default:
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 66f0c60..a4d026d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -184,6 +184,10 @@ enum intel_display_power_domain {
 	POWER_DOMAIN_VGA,
 	POWER_DOMAIN_AUDIO,
 	POWER_DOMAIN_PLLS,
+	POWER_DOMAIN_AUX_A,
+	POWER_DOMAIN_AUX_B,
+	POWER_DOMAIN_AUX_C,
+	POWER_DOMAIN_AUX_D,
 	POWER_DOMAIN_INIT,
 
 	POWER_DOMAIN_NUM,
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 8bf7bb4..49695d7 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -703,6 +703,10 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
 	BIT(POWER_DOMAIN_PORT_DDI_D_4_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_INIT))
 #define HSW_DISPLAY_POWER_DOMAINS (				\
 	(POWER_DOMAIN_MASK & ~HSW_ALWAYS_ON_POWER_DOMAINS) |	\
@@ -724,24 +728,30 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
 	BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) |	\
 	BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |	\
 	BIT(POWER_DOMAIN_PORT_CRT) |		\
+	BIT(POWER_DOMAIN_AUX_B) |		\
+	BIT(POWER_DOMAIN_AUX_C) |		\
 	BIT(POWER_DOMAIN_INIT))
 
 #define VLV_DPIO_TX_B_LANES_01_POWER_DOMAINS (	\
 	BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) |	\
 	BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |	\
+	BIT(POWER_DOMAIN_AUX_B) |		\
 	BIT(POWER_DOMAIN_INIT))
 
 #define VLV_DPIO_TX_B_LANES_23_POWER_DOMAINS (	\
 	BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |	\
+	BIT(POWER_DOMAIN_AUX_B) |		\
 	BIT(POWER_DOMAIN_INIT))
 
 #define VLV_DPIO_TX_C_LANES_01_POWER_DOMAINS (	\
 	BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) |	\
 	BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |	\
+	BIT(POWER_DOMAIN_AUX_C) |		\
 	BIT(POWER_DOMAIN_INIT))
 
 #define VLV_DPIO_TX_C_LANES_23_POWER_DOMAINS (	\
 	BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |	\
+	BIT(POWER_DOMAIN_AUX_C) |		\
 	BIT(POWER_DOMAIN_INIT))
 
 #define CHV_PIPE_A_POWER_DOMAINS (	\
@@ -761,20 +771,25 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
 	BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |	\
 	BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) |	\
 	BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |	\
+	BIT(POWER_DOMAIN_AUX_B) |		\
+	BIT(POWER_DOMAIN_AUX_C) |		\
 	BIT(POWER_DOMAIN_INIT))
 
 #define CHV_DPIO_CMN_D_POWER_DOMAINS (		\
 	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |	\
 	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |	\
+	BIT(POWER_DOMAIN_AUX_D) |		\
 	BIT(POWER_DOMAIN_INIT))
 
 #define CHV_DPIO_TX_D_LANES_01_POWER_DOMAINS (	\
 	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |	\
 	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |	\
+	BIT(POWER_DOMAIN_AUX_D) |		\
 	BIT(POWER_DOMAIN_INIT))
 
 #define CHV_DPIO_TX_D_LANES_23_POWER_DOMAINS (	\
 	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |	\
+	BIT(POWER_DOMAIN_AUX_D) |		\
 	BIT(POWER_DOMAIN_INIT))
 
 static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
-- 
1.8.3.1

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

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

* [PATCH 2/2] drm/i915/skl: Implementation of SKL display power well support
  2015-01-16 15:57 [PATCH 0/2] Power well support for SKL Damien Lespiau
  2015-01-16 15:57 ` [PATCH 1/2] drm/i915/skl: Adding power domains for AUX controllers Damien Lespiau
@ 2015-01-16 15:57 ` Damien Lespiau
  2015-01-17 10:08   ` shuang.he
  2015-02-02 23:06   ` Imre Deak
  1 sibling, 2 replies; 13+ messages in thread
From: Damien Lespiau @ 2015-01-16 15:57 UTC (permalink / raw)
  To: intel-gfx

From: Satheeshakrishna M <satheeshakrishna.m@intel.com>

This patch implements core logic of SKL display power well.

v2: Addressed Imre's comments
	- Added respective DDIs under power well #1 and #2
	- Simplified repetitive code in power well programming

v3: Implemented Imre's comments
	- Further simplified power well programming
	- Made sure that PW 1 is enabled prior to PW 2

v4: Fix minor conflict with the the cherryview support (Damien)

v5: Add the PLL power domain to the always on power well (Damien)

v6: Disable BIOS power well (Imre)
    Use power well data for comparison (Imre)
    Put the PLL power domain into PW1 as its needed for CDCLK (Satheesh,
    Damien)

v7: Addressed Imre's comments
  - Lowered the time out to 1ms
  - Added parantheses in macro
  - Moved debug message and fixed wait_for interval

v8:
  - Add a WARN() when swiching on an unknown power well (Imre, done by Damien)
  - Whitespace fixes (spaces instead of tabs) (Damien)

v9: (Imre, done by Damien)
  - Merge the register definitions with this patch
  - Merge the MISC IO power well in this patch

Signed-off-by: Satheeshakrishna M <satheeshakrishna.m@intel.com> (v3,v6,v7)
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h         |  20 +++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 230 ++++++++++++++++++++++++++++++++
 2 files changed, 250 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a39bb03..cb96041 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -586,6 +586,19 @@ enum punit_power_well {
 	PUNIT_POWER_WELL_NUM,
 };
 
+enum skl_disp_power_wells {
+	SKL_DISP_PW_MISC_IO,
+	SKL_DISP_PW_DDI_A_E,
+	SKL_DISP_PW_DDI_B,
+	SKL_DISP_PW_DDI_C,
+	SKL_DISP_PW_DDI_D,
+	SKL_DISP_PW_1 = 14,
+	SKL_DISP_PW_2,
+};
+
+#define SKL_POWER_WELL_STATE(pw) (1 << ((pw) * 2))
+#define SKL_POWER_WELL_REQ(pw) (1 << (((pw) * 2) + 1))
+
 #define PUNIT_REG_PWRGT_CTRL			0x60
 #define PUNIT_REG_PWRGT_STATUS			0x61
 #define   PUNIT_PWRGT_MASK(power_well)		(3 << ((power_well) * 2))
@@ -6323,6 +6336,13 @@ enum punit_power_well {
 #define   HSW_PWR_WELL_FORCE_ON			(1<<19)
 #define HSW_PWR_WELL_CTL6			0x45414
 
+/* SKL Fuse Status */
+#define SKL_FUSE_STATUS				0x42000
+#define  SKL_FUSE_DOWNLOAD_STATUS              (1<<31)
+#define  SKL_FUSE_PG0_DIST_STATUS              (1<<27)
+#define  SKL_FUSE_PG1_DIST_STATUS              (1<<26)
+#define  SKL_FUSE_PG2_DIST_STATUS              (1<<25)
+
 /* Per-pipe DDI Function Control */
 #define TRANS_DDI_FUNC_CTL_A		0x60400
 #define TRANS_DDI_FUNC_CTL_B		0x61400
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 49695d7..d72ec13 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -230,6 +230,146 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 	}
 }
 
+#define SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS (		\
+	BIT(POWER_DOMAIN_PIPE_B) |			\
+	BIT(POWER_DOMAIN_TRANSCODER_B) |		\
+	BIT(POWER_DOMAIN_PIPE_C) |			\
+	BIT(POWER_DOMAIN_TRANSCODER_C) |		\
+	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |		\
+	BIT(POWER_DOMAIN_PIPE_C_PANEL_FITTER) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |		\
+	BIT(POWER_DOMAIN_AUX_B) |                       \
+	BIT(POWER_DOMAIN_AUX_C) |			\
+	BIT(POWER_DOMAIN_AUX_D) |			\
+	BIT(POWER_DOMAIN_AUDIO) |			\
+	BIT(POWER_DOMAIN_INIT))
+#define SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS (		\
+	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
+	BIT(POWER_DOMAIN_PLLS) |			\
+	BIT(POWER_DOMAIN_PIPE_A) |			\
+	BIT(POWER_DOMAIN_TRANSCODER_EDP) |		\
+	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) |		\
+	BIT(POWER_DOMAIN_AUX_A) |			\
+	BIT(POWER_DOMAIN_INIT))
+#define SKL_DISPLAY_DDI_A_E_POWER_DOMAINS (		\
+	BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) |		\
+	BIT(POWER_DOMAIN_INIT))
+#define SKL_DISPLAY_DDI_B_POWER_DOMAINS (		\
+	BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |		\
+	BIT(POWER_DOMAIN_INIT))
+#define SKL_DISPLAY_DDI_C_POWER_DOMAINS (		\
+	BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |		\
+	BIT(POWER_DOMAIN_INIT))
+#define SKL_DISPLAY_DDI_D_POWER_DOMAINS (		\
+	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |		\
+	BIT(POWER_DOMAIN_INIT))
+#define SKL_DISPLAY_MISC_IO_POWER_DOMAINS (		\
+	BIT(POWER_DOMAIN_AUX_A) |			\
+	BIT(POWER_DOMAIN_AUX_B) |			\
+	BIT(POWER_DOMAIN_AUX_C) |			\
+	BIT(POWER_DOMAIN_AUX_D) |			\
+	BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |		\
+	BIT(POWER_DOMAIN_AUDIO) |			\
+	BIT(POWER_DOMAIN_INIT))
+#define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS (		\
+	(POWER_DOMAIN_MASK & ~(SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |	\
+	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
+	SKL_DISPLAY_DDI_A_E_POWER_DOMAINS |		\
+	SKL_DISPLAY_DDI_B_POWER_DOMAINS |		\
+	SKL_DISPLAY_DDI_C_POWER_DOMAINS |		\
+	SKL_DISPLAY_DDI_D_POWER_DOMAINS)) |		\
+	BIT(POWER_DOMAIN_INIT))
+
+static void skl_set_power_well(struct drm_i915_private *dev_priv,
+			struct i915_power_well *power_well, bool enable)
+{
+	uint32_t tmp, fuse_status;
+	uint32_t req_mask, state_mask;
+	bool check_fuse_status = false;
+
+	tmp = I915_READ(HSW_PWR_WELL_DRIVER);
+	fuse_status = I915_READ(SKL_FUSE_STATUS);
+
+	switch (power_well->data) {
+	case SKL_DISP_PW_1:
+		if (wait_for((I915_READ(SKL_FUSE_STATUS) &
+			SKL_FUSE_PG0_DIST_STATUS), 1)) {
+			DRM_ERROR("PG0 not enabled\n");
+			return;
+		}
+		break;
+	case SKL_DISP_PW_2:
+		if (!(fuse_status & SKL_FUSE_PG1_DIST_STATUS)) {
+			DRM_ERROR("PG1 in disabled state\n");
+			return;
+		}
+		break;
+	case SKL_DISP_PW_DDI_A_E:
+	case SKL_DISP_PW_DDI_B:
+	case SKL_DISP_PW_DDI_C:
+	case SKL_DISP_PW_DDI_D:
+	case SKL_DISP_PW_MISC_IO:
+		break;
+	default:
+		WARN(1, "Unknown power well %lu\n", power_well->data);
+		return;
+	}
+
+	req_mask = SKL_POWER_WELL_REQ(power_well->data);
+	state_mask = SKL_POWER_WELL_STATE(power_well->data);
+
+	if (enable) {
+		if (!(tmp & req_mask)) {
+			I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | req_mask);
+			DRM_DEBUG_KMS("Enabling %s\n", power_well->name);
+		}
+
+		if (!(tmp & state_mask)) {
+			if (wait_for((I915_READ(HSW_PWR_WELL_DRIVER) &
+				state_mask), 1))
+				DRM_ERROR("%s enable timeout\n",
+					power_well->name);
+			check_fuse_status = true;
+		}
+	} else {
+		if (tmp & req_mask) {
+			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 (check_fuse_status) {
+		if (power_well->data == SKL_DISP_PW_1) {
+			if (wait_for((I915_READ(SKL_FUSE_STATUS) &
+				SKL_FUSE_PG1_DIST_STATUS), 1))
+				DRM_ERROR("PG1 distributing status timeout\n");
+		} else if (power_well->data == SKL_DISP_PW_2) {
+			if (wait_for((I915_READ(SKL_FUSE_STATUS) &
+				SKL_FUSE_PG2_DIST_STATUS), 1))
+				DRM_ERROR("PG2 distributing status timeout\n");
+		}
+	}
+}
+
 static void hsw_power_well_sync_hw(struct drm_i915_private *dev_priv,
 				   struct i915_power_well *power_well)
 {
@@ -255,6 +395,36 @@ static void hsw_power_well_disable(struct drm_i915_private *dev_priv,
 	hsw_set_power_well(dev_priv, power_well, false);
 }
 
+static bool skl_power_well_enabled(struct drm_i915_private *dev_priv,
+					struct i915_power_well *power_well)
+{
+	uint32_t mask = SKL_POWER_WELL_REQ(power_well->data) |
+		SKL_POWER_WELL_STATE(power_well->data);
+
+	return (I915_READ(HSW_PWR_WELL_DRIVER) & mask) == mask;
+}
+
+static void skl_power_well_sync_hw(struct drm_i915_private *dev_priv,
+				struct i915_power_well *power_well)
+{
+	skl_set_power_well(dev_priv, power_well, power_well->count > 0);
+
+	/* Clear any request made by BIOS as driver is taking over */
+	I915_WRITE(HSW_PWR_WELL_BIOS, 0);
+}
+
+static void skl_power_well_enable(struct drm_i915_private *dev_priv,
+				struct i915_power_well *power_well)
+{
+	skl_set_power_well(dev_priv, power_well, true);
+}
+
+static void skl_power_well_disable(struct drm_i915_private *dev_priv,
+				struct i915_power_well *power_well)
+{
+	skl_set_power_well(dev_priv, power_well, false);
+}
+
 static void i9xx_always_on_power_well_noop(struct drm_i915_private *dev_priv,
 					   struct i915_power_well *power_well)
 {
@@ -829,6 +999,13 @@ static const struct i915_power_well_ops hsw_power_well_ops = {
 	.is_enabled = hsw_power_well_enabled,
 };
 
+static const struct i915_power_well_ops skl_power_well_ops = {
+	.sync_hw = skl_power_well_sync_hw,
+	.enable = skl_power_well_enable,
+	.disable = skl_power_well_disable,
+	.is_enabled = skl_power_well_enabled,
+};
+
 static struct i915_power_well hsw_power_wells[] = {
 	{
 		.name = "always-on",
@@ -1059,6 +1236,57 @@ static struct i915_power_well *lookup_power_well(struct drm_i915_private *dev_pr
 	return NULL;
 }
 
+static struct i915_power_well skl_power_wells[] = {
+	{
+		.name = "always-on",
+		.always_on = 1,
+		.domains = SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
+		.ops = &i9xx_always_on_power_well_ops,
+	},
+	{
+		.name = "power well 1",
+		.domains = SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS,
+		.ops = &skl_power_well_ops,
+		.data = SKL_DISP_PW_1,
+	},
+	{
+		.name = "power well 2",
+		.domains = SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS,
+		.ops = &skl_power_well_ops,
+		.data = SKL_DISP_PW_2,
+	},
+	{
+		.name = "DDI A/E power well",
+		.domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS,
+		.ops = &skl_power_well_ops,
+		.data = SKL_DISP_PW_DDI_A_E,
+	},
+	{
+		.name = "DDI B power well",
+		.domains = SKL_DISPLAY_DDI_B_POWER_DOMAINS,
+		.ops = &skl_power_well_ops,
+		.data = SKL_DISP_PW_DDI_B,
+	},
+	{
+		.name = "DDI C power well",
+		.domains = SKL_DISPLAY_DDI_C_POWER_DOMAINS,
+		.ops = &skl_power_well_ops,
+		.data = SKL_DISP_PW_DDI_C,
+	},
+	{
+		.name = "DDI D power well",
+		.domains = SKL_DISPLAY_DDI_D_POWER_DOMAINS,
+		.ops = &skl_power_well_ops,
+		.data = SKL_DISP_PW_DDI_D,
+	},
+	{
+		.name = "MISC IO power well",
+		.domains = SKL_DISPLAY_MISC_IO_POWER_DOMAINS,
+		.ops = &skl_power_well_ops,
+		.data = SKL_DISP_PW_MISC_IO,
+	}
+};
+
 #define set_power_wells(power_domains, __power_wells) ({		\
 	(power_domains)->power_wells = (__power_wells);			\
 	(power_domains)->power_well_count = ARRAY_SIZE(__power_wells);	\
@@ -1085,6 +1313,8 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
 		set_power_wells(power_domains, hsw_power_wells);
 	} else if (IS_BROADWELL(dev_priv->dev)) {
 		set_power_wells(power_domains, bdw_power_wells);
+	} else if (IS_SKYLAKE(dev_priv->dev)) {
+		set_power_wells(power_domains, skl_power_wells);
 	} else if (IS_CHERRYVIEW(dev_priv->dev)) {
 		set_power_wells(power_domains, chv_power_wells);
 	} else if (IS_VALLEYVIEW(dev_priv->dev)) {
-- 
1.8.3.1

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

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

* Re: [PATCH 2/2] drm/i915/skl: Implementation of SKL display power well support
  2015-01-16 15:57 ` [PATCH 2/2] drm/i915/skl: Implementation of SKL display power well support Damien Lespiau
@ 2015-01-17 10:08   ` shuang.he
  2015-02-02 23:06   ` Imre Deak
  1 sibling, 0 replies; 13+ messages in thread
From: shuang.he @ 2015-01-17 10:08 UTC (permalink / raw)
  To: shuang.he, ethan.gao, intel-gfx, damien.lespiau

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5595
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -1              353/353              352/353
ILK                                  353/353              353/353
SNB              +1                 400/422              401/422
IVB                                  487/487              487/487
BYT                                  296/296              296/296
HSW              +21-1              487/508              507/508
BDW                                  401/402              401/402
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt_gen3_render_linear_blits      PASS(4, M25M23)      CRASH(1, M23)
*SNB  igt_kms_flip_event_leak      NSPT(2, M35)      PASS(1, M35)
 HSW  igt_kms_cursor_crc_cursor-size-change      NSPT(1, M19)TIMEOUT(1, M40)PASS(2, M20)      PASS(1, M20)
 HSW  igt_kms_fence_pin_leak      NSPT(1, M19)DMESG_WARN(1, M40)PASS(2, M20)      PASS(1, M20)
 HSW  igt_kms_flip_dpms-vs-vblank-race      DMESG_WARN(3, M20M40)PASS(1, M19)      DMESG_WARN(1, M20)
 HSW  igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip      NSPT(1, M19)TIMEOUT(1, M40)PASS(2, M20)      PASS(1, M20)
 HSW  igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip      NSPT(1, M19)TIMEOUT(1, M40)PASS(2, M20)      PASS(1, M20)
 HSW  igt_pm_lpsp_non-edp      NSPT(1, M19)PASS(2, M20)      PASS(1, M20)
 HSW  igt_pm_rpm_cursor      NSPT(1, M19)PASS(2, M20)      PASS(1, M20)
 HSW  igt_pm_rpm_cursor-dpms      NSPT(1, M19)PASS(2, M20)      PASS(1, M20)
 HSW  igt_pm_rpm_dpms-mode-unset-non-lpsp      NSPT(1, M19)PASS(2, M20)      PASS(1, M20)
 HSW  igt_pm_rpm_dpms-non-lpsp      NSPT(1, M19)PASS(2, M20)      PASS(1, M20)
 HSW  igt_pm_rpm_drm-resources-equal      NSPT(1, M19)PASS(2, M20)      PASS(1, M20)
 HSW  igt_pm_rpm_fences      NSPT(1, M19)PASS(2, M20)      PASS(1, M20)
 HSW  igt_pm_rpm_fences-dpms      NSPT(1, M19)PASS(2, M20)      PASS(1, M20)
 HSW  igt_pm_rpm_gem-execbuf      NSPT(1, M19)PASS(2, M20)      PASS(1, M20)
 HSW  igt_pm_rpm_gem-mmap-cpu      NSPT(1, M19)PASS(2, M20)      PASS(1, M20)
 HSW  igt_pm_rpm_gem-mmap-gtt      NSPT(1, M19)PASS(2, M20)      PASS(1, M20)
 HSW  igt_pm_rpm_gem-pread      NSPT(1, M19)PASS(2, M20)      PASS(1, M20)
 HSW  igt_pm_rpm_i2c      NSPT(1, M19)PASS(2, M20)      PASS(1, M20)
 HSW  igt_pm_rpm_modeset-non-lpsp      NSPT(1, M19)PASS(2, M20)      PASS(1, M20)
 HSW  igt_pm_rpm_modeset-non-lpsp-stress-no-wait      NSPT(1, M19)PASS(2, M20)      PASS(1, M20)
 HSW  igt_pm_rpm_pci-d3-state      NSPT(1, M19)PASS(2, M20)      PASS(1, M20)
 HSW  igt_pm_rpm_rte      NSPT(1, M19)PASS(2, M20)      PASS(1, M20)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/2] drm/i915/skl: Adding power domains for AUX controllers
  2015-01-16 15:57 ` [PATCH 1/2] drm/i915/skl: Adding power domains for AUX controllers Damien Lespiau
@ 2015-01-20 10:11   ` Daniel Vetter
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2015-01-20 10:11 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

On Fri, Jan 16, 2015 at 03:57:51PM +0000, Damien Lespiau wrote:
> From: Satheeshakrishna M <satheeshakrishna.m@intel.com>
> 
> Adding new power doamins for AUX controllers
> 
> v2: Added new power domains in power_domain_str per Imre's comment
> 
> v3: Added AUX power domains to older platforms
> 
> v4: Rebase on top of POWER_DOMAIN_PLLS.
> 
> v5: Modified to address review comments from Imre
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Satheeshakrishna M <satheeshakrishna.m@intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> (v3)
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/2] drm/i915/skl: Implementation of SKL display power well support
  2015-01-16 15:57 ` [PATCH 2/2] drm/i915/skl: Implementation of SKL display power well support Damien Lespiau
  2015-01-17 10:08   ` shuang.he
@ 2015-02-02 23:06   ` Imre Deak
  2015-02-04 13:53     ` Damien Lespiau
  2015-02-04 13:57     ` [PATCH 2/2 v10] " Damien Lespiau
  1 sibling, 2 replies; 13+ messages in thread
From: Imre Deak @ 2015-02-02 23:06 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

On Fri, 2015-01-16 at 15:57 +0000, Damien Lespiau wrote:
> From: Satheeshakrishna M <satheeshakrishna.m@intel.com>
> 
> This patch implements core logic of SKL display power well.
> 
> v2: Addressed Imre's comments
> 	- Added respective DDIs under power well #1 and #2
> 	- Simplified repetitive code in power well programming
> 
> v3: Implemented Imre's comments
> 	- Further simplified power well programming
> 	- Made sure that PW 1 is enabled prior to PW 2
> 
> v4: Fix minor conflict with the the cherryview support (Damien)
> 
> v5: Add the PLL power domain to the always on power well (Damien)
> 
> v6: Disable BIOS power well (Imre)
>     Use power well data for comparison (Imre)
>     Put the PLL power domain into PW1 as its needed for CDCLK (Satheesh,
>     Damien)
> 
> v7: Addressed Imre's comments
>   - Lowered the time out to 1ms
>   - Added parantheses in macro
>   - Moved debug message and fixed wait_for interval
> 
> v8:
>   - Add a WARN() when swiching on an unknown power well (Imre, done by Damien)
>   - Whitespace fixes (spaces instead of tabs) (Damien)
> 
> v9: (Imre, done by Damien)
>   - Merge the register definitions with this patch
>   - Merge the MISC IO power well in this patch
> 
> Signed-off-by: Satheeshakrishna M <satheeshakrishna.m@intel.com> (v3,v6,v7)
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h         |  20 +++
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 230 ++++++++++++++++++++++++++++++++
>  2 files changed, 250 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a39bb03..cb96041 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -586,6 +586,19 @@ enum punit_power_well {
>  	PUNIT_POWER_WELL_NUM,
>  };
>  
> +enum skl_disp_power_wells {
> +	SKL_DISP_PW_MISC_IO,
> +	SKL_DISP_PW_DDI_A_E,
> +	SKL_DISP_PW_DDI_B,
> +	SKL_DISP_PW_DDI_C,
> +	SKL_DISP_PW_DDI_D,
> +	SKL_DISP_PW_1 = 14,
> +	SKL_DISP_PW_2,
> +};
> +
> +#define SKL_POWER_WELL_STATE(pw) (1 << ((pw) * 2))
> +#define SKL_POWER_WELL_REQ(pw) (1 << (((pw) * 2) + 1))
> +
>  #define PUNIT_REG_PWRGT_CTRL			0x60
>  #define PUNIT_REG_PWRGT_STATUS			0x61
>  #define   PUNIT_PWRGT_MASK(power_well)		(3 << ((power_well) * 2))
> @@ -6323,6 +6336,13 @@ enum punit_power_well {
>  #define   HSW_PWR_WELL_FORCE_ON			(1<<19)
>  #define HSW_PWR_WELL_CTL6			0x45414
>  
> +/* SKL Fuse Status */
> +#define SKL_FUSE_STATUS				0x42000
> +#define  SKL_FUSE_DOWNLOAD_STATUS              (1<<31)
> +#define  SKL_FUSE_PG0_DIST_STATUS              (1<<27)
> +#define  SKL_FUSE_PG1_DIST_STATUS              (1<<26)
> +#define  SKL_FUSE_PG2_DIST_STATUS              (1<<25)
> +
>  /* Per-pipe DDI Function Control */
>  #define TRANS_DDI_FUNC_CTL_A		0x60400
>  #define TRANS_DDI_FUNC_CTL_B		0x61400
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 49695d7..d72ec13 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -230,6 +230,146 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>  	}
>  }
>  
> +#define SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_PIPE_B) |			\
> +	BIT(POWER_DOMAIN_TRANSCODER_B) |		\
> +	BIT(POWER_DOMAIN_PIPE_C) |			\
> +	BIT(POWER_DOMAIN_TRANSCODER_C) |		\
> +	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |		\
> +	BIT(POWER_DOMAIN_PIPE_C_PANEL_FITTER) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |		\
> +	BIT(POWER_DOMAIN_AUX_B) |                       \
> +	BIT(POWER_DOMAIN_AUX_C) |			\
> +	BIT(POWER_DOMAIN_AUX_D) |			\
> +	BIT(POWER_DOMAIN_AUDIO) |			\
> +	BIT(POWER_DOMAIN_INIT))

What about transcoder A? It's also on power well 2, and I can't see
anything preventing us to use DP or HDMI with it, so I think it needs to
be added here.

POWER_DOMAIN_VGA needs to be added here too.

> +#define SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS (		\
> +	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
> +	BIT(POWER_DOMAIN_PLLS) |			\
> +	BIT(POWER_DOMAIN_PIPE_A) |			\
> +	BIT(POWER_DOMAIN_TRANSCODER_EDP) |		\
> +	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) |		\
> +	BIT(POWER_DOMAIN_AUX_A) |			\
> +	BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_DDI_A_E_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) |		\
> +	BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_DDI_B_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |		\
> +	BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_DDI_C_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |		\
> +	BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_DDI_D_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |		\
> +	BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_MISC_IO_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_AUX_A) |			\
> +	BIT(POWER_DOMAIN_AUX_B) |			\
> +	BIT(POWER_DOMAIN_AUX_C) |			\
> +	BIT(POWER_DOMAIN_AUX_D) |			\
> +	BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |		\
> +	BIT(POWER_DOMAIN_AUDIO) |			\
> +	BIT(POWER_DOMAIN_INIT))

This changed recently in bspec, so that we need the MISC IO power well
for any display functionality. This means we have to enable it whenever
power well 1 is enabled, so the above should be defined simply as
SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS.

> +#define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS (		\
> +	(POWER_DOMAIN_MASK & ~(SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |	\
> +	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
> +	SKL_DISPLAY_DDI_A_E_POWER_DOMAINS |		\
> +	SKL_DISPLAY_DDI_B_POWER_DOMAINS |		\
> +	SKL_DISPLAY_DDI_C_POWER_DOMAINS |		\
> +	SKL_DISPLAY_DDI_D_POWER_DOMAINS)) |		\

For consistency I would also add SKL_DISPLAY_MISC_IO_POWER_DOMAINS to
the above.

> +	BIT(POWER_DOMAIN_INIT))
> +
> +static void skl_set_power_well(struct drm_i915_private *dev_priv,
> +			struct i915_power_well *power_well, bool enable)
> +{
> +	uint32_t tmp, fuse_status;
> +	uint32_t req_mask, state_mask;
> +	bool check_fuse_status = false;
> +
> +	tmp = I915_READ(HSW_PWR_WELL_DRIVER);
> +	fuse_status = I915_READ(SKL_FUSE_STATUS);
> +
> +	switch (power_well->data) {
> +	case SKL_DISP_PW_1:
> +		if (wait_for((I915_READ(SKL_FUSE_STATUS) &
> +			SKL_FUSE_PG0_DIST_STATUS), 1)) {
> +			DRM_ERROR("PG0 not enabled\n");
> +			return;
> +		}
> +		break;
> +	case SKL_DISP_PW_2:
> +		if (!(fuse_status & SKL_FUSE_PG1_DIST_STATUS)) {
> +			DRM_ERROR("PG1 in disabled state\n");
> +			return;
> +		}
> +		break;
> +	case SKL_DISP_PW_DDI_A_E:
> +	case SKL_DISP_PW_DDI_B:
> +	case SKL_DISP_PW_DDI_C:
> +	case SKL_DISP_PW_DDI_D:
> +	case SKL_DISP_PW_MISC_IO:
> +		break;
> +	default:
> +		WARN(1, "Unknown power well %lu\n", power_well->data);
> +		return;
> +	}
> +
> +	req_mask = SKL_POWER_WELL_REQ(power_well->data);
> +	state_mask = SKL_POWER_WELL_STATE(power_well->data);
> +
> +	if (enable) {
> +		if (!(tmp & req_mask)) {
> +			I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | req_mask);
> +			DRM_DEBUG_KMS("Enabling %s\n", power_well->name);
> +		}
> +
> +		if (!(tmp & state_mask)) {
> +			if (wait_for((I915_READ(HSW_PWR_WELL_DRIVER) &
> +				state_mask), 1))
> +				DRM_ERROR("%s enable timeout\n",
> +					power_well->name);
> +			check_fuse_status = true;
> +		}
> +	} else {
> +		if (tmp & req_mask) {
> +			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 (check_fuse_status) {
> +		if (power_well->data == SKL_DISP_PW_1) {
> +			if (wait_for((I915_READ(SKL_FUSE_STATUS) &
> +				SKL_FUSE_PG1_DIST_STATUS), 1))
> +				DRM_ERROR("PG1 distributing status timeout\n");
> +		} else if (power_well->data == SKL_DISP_PW_2) {
> +			if (wait_for((I915_READ(SKL_FUSE_STATUS) &
> +				SKL_FUSE_PG2_DIST_STATUS), 1))
> +				DRM_ERROR("PG2 distributing status timeout\n");
> +		}
> +	}
> +}
> +
>  static void hsw_power_well_sync_hw(struct drm_i915_private *dev_priv,
>  				   struct i915_power_well *power_well)
>  {
> @@ -255,6 +395,36 @@ static void hsw_power_well_disable(struct drm_i915_private *dev_priv,
>  	hsw_set_power_well(dev_priv, power_well, false);
>  }
>  
> +static bool skl_power_well_enabled(struct drm_i915_private *dev_priv,
> +					struct i915_power_well *power_well)
> +{
> +	uint32_t mask = SKL_POWER_WELL_REQ(power_well->data) |
> +		SKL_POWER_WELL_STATE(power_well->data);
> +
> +	return (I915_READ(HSW_PWR_WELL_DRIVER) & mask) == mask;
> +}
> +
> +static void skl_power_well_sync_hw(struct drm_i915_private *dev_priv,
> +				struct i915_power_well *power_well)
> +{
> +	skl_set_power_well(dev_priv, power_well, power_well->count > 0);
> +
> +	/* Clear any request made by BIOS as driver is taking over */
> +	I915_WRITE(HSW_PWR_WELL_BIOS, 0);
> +}
> +
> +static void skl_power_well_enable(struct drm_i915_private *dev_priv,
> +				struct i915_power_well *power_well)
> +{
> +	skl_set_power_well(dev_priv, power_well, true);
> +}
> +
> +static void skl_power_well_disable(struct drm_i915_private *dev_priv,
> +				struct i915_power_well *power_well)
> +{
> +	skl_set_power_well(dev_priv, power_well, false);
> +}
> +
>  static void i9xx_always_on_power_well_noop(struct drm_i915_private *dev_priv,
>  					   struct i915_power_well *power_well)
>  {
> @@ -829,6 +999,13 @@ static const struct i915_power_well_ops hsw_power_well_ops = {
>  	.is_enabled = hsw_power_well_enabled,
>  };
>  
> +static const struct i915_power_well_ops skl_power_well_ops = {
> +	.sync_hw = skl_power_well_sync_hw,
> +	.enable = skl_power_well_enable,
> +	.disable = skl_power_well_disable,
> +	.is_enabled = skl_power_well_enabled,
> +};
> +
>  static struct i915_power_well hsw_power_wells[] = {
>  	{
>  		.name = "always-on",
> @@ -1059,6 +1236,57 @@ static struct i915_power_well *lookup_power_well(struct drm_i915_private *dev_pr
>  	return NULL;
>  }
>  
> +static struct i915_power_well skl_power_wells[] = {
> +	{
> +		.name = "always-on",
> +		.always_on = 1,
> +		.domains = SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
> +		.ops = &i9xx_always_on_power_well_ops,
> +	},
> +	{
> +		.name = "power well 1",
> +		.domains = SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS,
> +		.ops = &skl_power_well_ops,
> +		.data = SKL_DISP_PW_1,
> +	},
> +	{
> +		.name = "power well 2",
> +		.domains = SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS,
> +		.ops = &skl_power_well_ops,
> +		.data = SKL_DISP_PW_2,
> +	},
> +	{
> +		.name = "DDI A/E power well",
> +		.domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS,
> +		.ops = &skl_power_well_ops,
> +		.data = SKL_DISP_PW_DDI_A_E,
> +	},
> +	{
> +		.name = "DDI B power well",
> +		.domains = SKL_DISPLAY_DDI_B_POWER_DOMAINS,
> +		.ops = &skl_power_well_ops,
> +		.data = SKL_DISP_PW_DDI_B,
> +	},
> +	{
> +		.name = "DDI C power well",
> +		.domains = SKL_DISPLAY_DDI_C_POWER_DOMAINS,
> +		.ops = &skl_power_well_ops,
> +		.data = SKL_DISP_PW_DDI_C,
> +	},
> +	{
> +		.name = "DDI D power well",
> +		.domains = SKL_DISPLAY_DDI_D_POWER_DOMAINS,
> +		.ops = &skl_power_well_ops,
> +		.data = SKL_DISP_PW_DDI_D,
> +	},
> +	{
> +		.name = "MISC IO power well",
> +		.domains = SKL_DISPLAY_MISC_IO_POWER_DOMAINS,
> +		.ops = &skl_power_well_ops,
> +		.data = SKL_DISP_PW_MISC_IO,
> +	}

Again, since the recent bspec change the misc IO power well should be
enabled before anything else, so it needs to be listed before "power
well 1" on the list.

With the above fixed, this looks ok:
Reviewed-by: Imre Deak <imre.deak@intel.com>


> +};
> +
>  #define set_power_wells(power_domains, __power_wells) ({		\
>  	(power_domains)->power_wells = (__power_wells);			\
>  	(power_domains)->power_well_count = ARRAY_SIZE(__power_wells);	\
> @@ -1085,6 +1313,8 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
>  		set_power_wells(power_domains, hsw_power_wells);
>  	} else if (IS_BROADWELL(dev_priv->dev)) {
>  		set_power_wells(power_domains, bdw_power_wells);
> +	} else if (IS_SKYLAKE(dev_priv->dev)) {
> +		set_power_wells(power_domains, skl_power_wells);
>  	} else if (IS_CHERRYVIEW(dev_priv->dev)) {
>  		set_power_wells(power_domains, chv_power_wells);
>  	} else if (IS_VALLEYVIEW(dev_priv->dev)) {


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

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

* Re: [PATCH 2/2] drm/i915/skl: Implementation of SKL display power well support
  2015-02-02 23:06   ` Imre Deak
@ 2015-02-04 13:53     ` Damien Lespiau
  2015-02-04 14:20       ` Imre Deak
  2015-02-04 13:57     ` [PATCH 2/2 v10] " Damien Lespiau
  1 sibling, 1 reply; 13+ messages in thread
From: Damien Lespiau @ 2015-02-04 13:53 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Tue, Feb 03, 2015 at 01:06:31AM +0200, Imre Deak wrote:
> > +static struct i915_power_well skl_power_wells[] = {
> > +	{
> > +		.name = "always-on",
> > +		.always_on = 1,
> > +		.domains = SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
> > +		.ops = &i9xx_always_on_power_well_ops,
> > +	},
> > +	{
> > +		.name = "power well 1",
> > +		.domains = SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS,
> > +		.ops = &skl_power_well_ops,
> > +		.data = SKL_DISP_PW_1,
> > +	},

snip

> > +	{
> > +		.name = "MISC IO power well",
> > +		.domains = SKL_DISPLAY_MISC_IO_POWER_DOMAINS,
> > +		.ops = &skl_power_well_ops,
> > +		.data = SKL_DISP_PW_MISC_IO,
> > +	}
> 
> Again, since the recent bspec change the misc IO power well should be
> enabled before anything else, so it needs to be listed before "power
> well 1" on the list.

So this one was causing problems. When I try to enabled MISC IO before
PW1, the request times out. Enabling MISC IO just right after PW1 seems
to work fine though.

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

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

* [PATCH 2/2 v10] drm/i915/skl: Implementation of SKL display power well support
  2015-02-02 23:06   ` Imre Deak
  2015-02-04 13:53     ` Damien Lespiau
@ 2015-02-04 13:57     ` Damien Lespiau
  2015-02-04 14:19       ` Daniel Vetter
  1 sibling, 1 reply; 13+ messages in thread
From: Damien Lespiau @ 2015-02-04 13:57 UTC (permalink / raw)
  To: intel-gfx

From: Satheeshakrishna M <satheeshakrishna.m@intel.com>

This patch implements core logic of SKL display power well.

v2: Addressed Imre's comments
	- Added respective DDIs under power well #1 and #2
	- Simplified repetitive code in power well programming

v3: Implemented Imre's comments
	- Further simplified power well programming
	- Made sure that PW 1 is enabled prior to PW 2

v4: Fix minor conflict with the the cherryview support (Damien)

v5: Add the PLL power domain to the always on power well (Damien)

v6: Disable BIOS power well (Imre)
    Use power well data for comparison (Imre)
    Put the PLL power domain into PW1 as its needed for CDCLK (Satheesh,
    Damien)

v7: Addressed Imre's comments
  - Lowered the time out to 1ms
  - Added parantheses in macro
  - Moved debug message and fixed wait_for interval

v8:
  - Add a WARN() when swiching on an unknown power well (Imre, done by Damien)
  - Whitespace fixes (spaces instead of tabs) (Damien)

v9: (Imre, done by Damien)
  - Merge the register definitions with this patch
  - Merge the MISC IO power well in this patch

v10: (Imre, done by Damien)

  - Define the Misc I/O power domains to be the power well 1 ones as Misc I/O
    needs to be enabled with PW1
  - Added Transcoder A and VGA domains to PW 2
  - Remove the MISC_IO power domains as well in the the always on
    domains definition
  - Move Misc I/O power well at the top of the power well list so it's turned
    on right after PW1.

Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Satheeshakrishna M <satheeshakrishna.m@intel.com> (v3,v6,v7)
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h         |  20 +++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 220 ++++++++++++++++++++++++++++++++
 2 files changed, 240 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 33b3d0a2..cd3430f9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -586,6 +586,19 @@ enum punit_power_well {
 	PUNIT_POWER_WELL_NUM,
 };
 
+enum skl_disp_power_wells {
+	SKL_DISP_PW_MISC_IO,
+	SKL_DISP_PW_DDI_A_E,
+	SKL_DISP_PW_DDI_B,
+	SKL_DISP_PW_DDI_C,
+	SKL_DISP_PW_DDI_D,
+	SKL_DISP_PW_1 = 14,
+	SKL_DISP_PW_2,
+};
+
+#define SKL_POWER_WELL_STATE(pw) (1 << ((pw) * 2))
+#define SKL_POWER_WELL_REQ(pw) (1 << (((pw) * 2) + 1))
+
 #define PUNIT_REG_PWRGT_CTRL			0x60
 #define PUNIT_REG_PWRGT_STATUS			0x61
 #define   PUNIT_PWRGT_MASK(power_well)		(3 << ((power_well) * 2))
@@ -6351,6 +6364,13 @@ enum punit_power_well {
 #define   HSW_PWR_WELL_FORCE_ON			(1<<19)
 #define HSW_PWR_WELL_CTL6			0x45414
 
+/* SKL Fuse Status */
+#define SKL_FUSE_STATUS				0x42000
+#define  SKL_FUSE_DOWNLOAD_STATUS              (1<<31)
+#define  SKL_FUSE_PG0_DIST_STATUS              (1<<27)
+#define  SKL_FUSE_PG1_DIST_STATUS              (1<<26)
+#define  SKL_FUSE_PG2_DIST_STATUS              (1<<25)
+
 /* Per-pipe DDI Function Control */
 #define TRANS_DDI_FUNC_CTL_A		0x60400
 #define TRANS_DDI_FUNC_CTL_B		0x61400
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 49695d7..6d8e29a 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -230,6 +230,136 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 	}
 }
 
+#define SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS (		\
+	BIT(POWER_DOMAIN_TRANSCODER_A) |		\
+	BIT(POWER_DOMAIN_PIPE_B) |			\
+	BIT(POWER_DOMAIN_TRANSCODER_B) |		\
+	BIT(POWER_DOMAIN_PIPE_C) |			\
+	BIT(POWER_DOMAIN_TRANSCODER_C) |		\
+	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |		\
+	BIT(POWER_DOMAIN_PIPE_C_PANEL_FITTER) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |		\
+	BIT(POWER_DOMAIN_AUX_B) |                       \
+	BIT(POWER_DOMAIN_AUX_C) |			\
+	BIT(POWER_DOMAIN_AUX_D) |			\
+	BIT(POWER_DOMAIN_AUDIO) |			\
+	BIT(POWER_DOMAIN_VGA) |				\
+	BIT(POWER_DOMAIN_INIT))
+#define SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS (		\
+	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
+	BIT(POWER_DOMAIN_PLLS) |			\
+	BIT(POWER_DOMAIN_PIPE_A) |			\
+	BIT(POWER_DOMAIN_TRANSCODER_EDP) |		\
+	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) |		\
+	BIT(POWER_DOMAIN_AUX_A) |			\
+	BIT(POWER_DOMAIN_INIT))
+#define SKL_DISPLAY_DDI_A_E_POWER_DOMAINS (		\
+	BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) |		\
+	BIT(POWER_DOMAIN_INIT))
+#define SKL_DISPLAY_DDI_B_POWER_DOMAINS (		\
+	BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |		\
+	BIT(POWER_DOMAIN_INIT))
+#define SKL_DISPLAY_DDI_C_POWER_DOMAINS (		\
+	BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |		\
+	BIT(POWER_DOMAIN_INIT))
+#define SKL_DISPLAY_DDI_D_POWER_DOMAINS (		\
+	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |		\
+	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |		\
+	BIT(POWER_DOMAIN_INIT))
+#define SKL_DISPLAY_MISC_IO_POWER_DOMAINS (		\
+	SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS)
+#define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS (		\
+	(POWER_DOMAIN_MASK & ~(SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |	\
+	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
+	SKL_DISPLAY_DDI_A_E_POWER_DOMAINS |		\
+	SKL_DISPLAY_DDI_B_POWER_DOMAINS |		\
+	SKL_DISPLAY_DDI_C_POWER_DOMAINS |		\
+	SKL_DISPLAY_DDI_D_POWER_DOMAINS |		\
+	SKL_DISPLAY_MISC_IO_POWER_DOMAINS)) |		\
+	BIT(POWER_DOMAIN_INIT))
+
+static void skl_set_power_well(struct drm_i915_private *dev_priv,
+			struct i915_power_well *power_well, bool enable)
+{
+	uint32_t tmp, fuse_status;
+	uint32_t req_mask, state_mask;
+	bool check_fuse_status = false;
+
+	tmp = I915_READ(HSW_PWR_WELL_DRIVER);
+	fuse_status = I915_READ(SKL_FUSE_STATUS);
+
+	switch (power_well->data) {
+	case SKL_DISP_PW_1:
+		if (wait_for((I915_READ(SKL_FUSE_STATUS) &
+			SKL_FUSE_PG0_DIST_STATUS), 1)) {
+			DRM_ERROR("PG0 not enabled\n");
+			return;
+		}
+		break;
+	case SKL_DISP_PW_2:
+		if (!(fuse_status & SKL_FUSE_PG1_DIST_STATUS)) {
+			DRM_ERROR("PG1 in disabled state\n");
+			return;
+		}
+		break;
+	case SKL_DISP_PW_DDI_A_E:
+	case SKL_DISP_PW_DDI_B:
+	case SKL_DISP_PW_DDI_C:
+	case SKL_DISP_PW_DDI_D:
+	case SKL_DISP_PW_MISC_IO:
+		break;
+	default:
+		WARN(1, "Unknown power well %lu\n", power_well->data);
+		return;
+	}
+
+	req_mask = SKL_POWER_WELL_REQ(power_well->data);
+	state_mask = SKL_POWER_WELL_STATE(power_well->data);
+
+	if (enable) {
+		if (!(tmp & req_mask)) {
+			I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | req_mask);
+			DRM_DEBUG_KMS("Enabling %s\n", power_well->name);
+		}
+
+		if (!(tmp & state_mask)) {
+			if (wait_for((I915_READ(HSW_PWR_WELL_DRIVER) &
+				state_mask), 1))
+				DRM_ERROR("%s enable timeout\n",
+					power_well->name);
+			check_fuse_status = true;
+		}
+	} else {
+		if (tmp & req_mask) {
+			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 (check_fuse_status) {
+		if (power_well->data == SKL_DISP_PW_1) {
+			if (wait_for((I915_READ(SKL_FUSE_STATUS) &
+				SKL_FUSE_PG1_DIST_STATUS), 1))
+				DRM_ERROR("PG1 distributing status timeout\n");
+		} else if (power_well->data == SKL_DISP_PW_2) {
+			if (wait_for((I915_READ(SKL_FUSE_STATUS) &
+				SKL_FUSE_PG2_DIST_STATUS), 1))
+				DRM_ERROR("PG2 distributing status timeout\n");
+		}
+	}
+}
+
 static void hsw_power_well_sync_hw(struct drm_i915_private *dev_priv,
 				   struct i915_power_well *power_well)
 {
@@ -255,6 +385,36 @@ static void hsw_power_well_disable(struct drm_i915_private *dev_priv,
 	hsw_set_power_well(dev_priv, power_well, false);
 }
 
+static bool skl_power_well_enabled(struct drm_i915_private *dev_priv,
+					struct i915_power_well *power_well)
+{
+	uint32_t mask = SKL_POWER_WELL_REQ(power_well->data) |
+		SKL_POWER_WELL_STATE(power_well->data);
+
+	return (I915_READ(HSW_PWR_WELL_DRIVER) & mask) == mask;
+}
+
+static void skl_power_well_sync_hw(struct drm_i915_private *dev_priv,
+				struct i915_power_well *power_well)
+{
+	skl_set_power_well(dev_priv, power_well, power_well->count > 0);
+
+	/* Clear any request made by BIOS as driver is taking over */
+	I915_WRITE(HSW_PWR_WELL_BIOS, 0);
+}
+
+static void skl_power_well_enable(struct drm_i915_private *dev_priv,
+				struct i915_power_well *power_well)
+{
+	skl_set_power_well(dev_priv, power_well, true);
+}
+
+static void skl_power_well_disable(struct drm_i915_private *dev_priv,
+				struct i915_power_well *power_well)
+{
+	skl_set_power_well(dev_priv, power_well, false);
+}
+
 static void i9xx_always_on_power_well_noop(struct drm_i915_private *dev_priv,
 					   struct i915_power_well *power_well)
 {
@@ -829,6 +989,13 @@ static const struct i915_power_well_ops hsw_power_well_ops = {
 	.is_enabled = hsw_power_well_enabled,
 };
 
+static const struct i915_power_well_ops skl_power_well_ops = {
+	.sync_hw = skl_power_well_sync_hw,
+	.enable = skl_power_well_enable,
+	.disable = skl_power_well_disable,
+	.is_enabled = skl_power_well_enabled,
+};
+
 static struct i915_power_well hsw_power_wells[] = {
 	{
 		.name = "always-on",
@@ -1059,6 +1226,57 @@ static struct i915_power_well *lookup_power_well(struct drm_i915_private *dev_pr
 	return NULL;
 }
 
+static struct i915_power_well skl_power_wells[] = {
+	{
+		.name = "always-on",
+		.always_on = 1,
+		.domains = SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
+		.ops = &i9xx_always_on_power_well_ops,
+	},
+	{
+		.name = "power well 1",
+		.domains = SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS,
+		.ops = &skl_power_well_ops,
+		.data = SKL_DISP_PW_1,
+	},
+	{
+		.name = "MISC IO power well",
+		.domains = SKL_DISPLAY_MISC_IO_POWER_DOMAINS,
+		.ops = &skl_power_well_ops,
+		.data = SKL_DISP_PW_MISC_IO,
+	},
+	{
+		.name = "power well 2",
+		.domains = SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS,
+		.ops = &skl_power_well_ops,
+		.data = SKL_DISP_PW_2,
+	},
+	{
+		.name = "DDI A/E power well",
+		.domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS,
+		.ops = &skl_power_well_ops,
+		.data = SKL_DISP_PW_DDI_A_E,
+	},
+	{
+		.name = "DDI B power well",
+		.domains = SKL_DISPLAY_DDI_B_POWER_DOMAINS,
+		.ops = &skl_power_well_ops,
+		.data = SKL_DISP_PW_DDI_B,
+	},
+	{
+		.name = "DDI C power well",
+		.domains = SKL_DISPLAY_DDI_C_POWER_DOMAINS,
+		.ops = &skl_power_well_ops,
+		.data = SKL_DISP_PW_DDI_C,
+	},
+	{
+		.name = "DDI D power well",
+		.domains = SKL_DISPLAY_DDI_D_POWER_DOMAINS,
+		.ops = &skl_power_well_ops,
+		.data = SKL_DISP_PW_DDI_D,
+	},
+};
+
 #define set_power_wells(power_domains, __power_wells) ({		\
 	(power_domains)->power_wells = (__power_wells);			\
 	(power_domains)->power_well_count = ARRAY_SIZE(__power_wells);	\
@@ -1085,6 +1303,8 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
 		set_power_wells(power_domains, hsw_power_wells);
 	} else if (IS_BROADWELL(dev_priv->dev)) {
 		set_power_wells(power_domains, bdw_power_wells);
+	} else if (IS_SKYLAKE(dev_priv->dev)) {
+		set_power_wells(power_domains, skl_power_wells);
 	} else if (IS_CHERRYVIEW(dev_priv->dev)) {
 		set_power_wells(power_domains, chv_power_wells);
 	} else if (IS_VALLEYVIEW(dev_priv->dev)) {
-- 
1.8.3.1

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

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

* Re: [PATCH 2/2 v10] drm/i915/skl: Implementation of SKL display power well support
  2015-02-04 13:57     ` [PATCH 2/2 v10] " Damien Lespiau
@ 2015-02-04 14:19       ` Daniel Vetter
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2015-02-04 14:19 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

On Wed, Feb 04, 2015 at 01:57:44PM +0000, Damien Lespiau wrote:
> From: Satheeshakrishna M <satheeshakrishna.m@intel.com>
> 
> This patch implements core logic of SKL display power well.
> 
> v2: Addressed Imre's comments
> 	- Added respective DDIs under power well #1 and #2
> 	- Simplified repetitive code in power well programming
> 
> v3: Implemented Imre's comments
> 	- Further simplified power well programming
> 	- Made sure that PW 1 is enabled prior to PW 2
> 
> v4: Fix minor conflict with the the cherryview support (Damien)
> 
> v5: Add the PLL power domain to the always on power well (Damien)
> 
> v6: Disable BIOS power well (Imre)
>     Use power well data for comparison (Imre)
>     Put the PLL power domain into PW1 as its needed for CDCLK (Satheesh,
>     Damien)
> 
> v7: Addressed Imre's comments
>   - Lowered the time out to 1ms
>   - Added parantheses in macro
>   - Moved debug message and fixed wait_for interval
> 
> v8:
>   - Add a WARN() when swiching on an unknown power well (Imre, done by Damien)
>   - Whitespace fixes (spaces instead of tabs) (Damien)
> 
> v9: (Imre, done by Damien)
>   - Merge the register definitions with this patch
>   - Merge the MISC IO power well in this patch
> 
> v10: (Imre, done by Damien)
> 
>   - Define the Misc I/O power domains to be the power well 1 ones as Misc I/O
>     needs to be enabled with PW1
>   - Added Transcoder A and VGA domains to PW 2
>   - Remove the MISC_IO power domains as well in the the always on
>     domains definition
>   - Move Misc I/O power well at the top of the power well list so it's turned
>     on right after PW1.
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Satheeshakrishna M <satheeshakrishna.m@intel.com> (v3,v6,v7)
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

Queued for -next, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_reg.h         |  20 +++
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 220 ++++++++++++++++++++++++++++++++
>  2 files changed, 240 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 33b3d0a2..cd3430f9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -586,6 +586,19 @@ enum punit_power_well {
>  	PUNIT_POWER_WELL_NUM,
>  };
>  
> +enum skl_disp_power_wells {
> +	SKL_DISP_PW_MISC_IO,
> +	SKL_DISP_PW_DDI_A_E,
> +	SKL_DISP_PW_DDI_B,
> +	SKL_DISP_PW_DDI_C,
> +	SKL_DISP_PW_DDI_D,
> +	SKL_DISP_PW_1 = 14,
> +	SKL_DISP_PW_2,
> +};
> +
> +#define SKL_POWER_WELL_STATE(pw) (1 << ((pw) * 2))
> +#define SKL_POWER_WELL_REQ(pw) (1 << (((pw) * 2) + 1))
> +
>  #define PUNIT_REG_PWRGT_CTRL			0x60
>  #define PUNIT_REG_PWRGT_STATUS			0x61
>  #define   PUNIT_PWRGT_MASK(power_well)		(3 << ((power_well) * 2))
> @@ -6351,6 +6364,13 @@ enum punit_power_well {
>  #define   HSW_PWR_WELL_FORCE_ON			(1<<19)
>  #define HSW_PWR_WELL_CTL6			0x45414
>  
> +/* SKL Fuse Status */
> +#define SKL_FUSE_STATUS				0x42000
> +#define  SKL_FUSE_DOWNLOAD_STATUS              (1<<31)
> +#define  SKL_FUSE_PG0_DIST_STATUS              (1<<27)
> +#define  SKL_FUSE_PG1_DIST_STATUS              (1<<26)
> +#define  SKL_FUSE_PG2_DIST_STATUS              (1<<25)
> +
>  /* Per-pipe DDI Function Control */
>  #define TRANS_DDI_FUNC_CTL_A		0x60400
>  #define TRANS_DDI_FUNC_CTL_B		0x61400
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 49695d7..6d8e29a 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -230,6 +230,136 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>  	}
>  }
>  
> +#define SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_TRANSCODER_A) |		\
> +	BIT(POWER_DOMAIN_PIPE_B) |			\
> +	BIT(POWER_DOMAIN_TRANSCODER_B) |		\
> +	BIT(POWER_DOMAIN_PIPE_C) |			\
> +	BIT(POWER_DOMAIN_TRANSCODER_C) |		\
> +	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |		\
> +	BIT(POWER_DOMAIN_PIPE_C_PANEL_FITTER) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |		\
> +	BIT(POWER_DOMAIN_AUX_B) |                       \
> +	BIT(POWER_DOMAIN_AUX_C) |			\
> +	BIT(POWER_DOMAIN_AUX_D) |			\
> +	BIT(POWER_DOMAIN_AUDIO) |			\
> +	BIT(POWER_DOMAIN_VGA) |				\
> +	BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS (		\
> +	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
> +	BIT(POWER_DOMAIN_PLLS) |			\
> +	BIT(POWER_DOMAIN_PIPE_A) |			\
> +	BIT(POWER_DOMAIN_TRANSCODER_EDP) |		\
> +	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) |		\
> +	BIT(POWER_DOMAIN_AUX_A) |			\
> +	BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_DDI_A_E_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) |		\
> +	BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_DDI_B_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |		\
> +	BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_DDI_C_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |		\
> +	BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_DDI_D_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |		\
> +	BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_MISC_IO_POWER_DOMAINS (		\
> +	SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS)
> +#define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS (		\
> +	(POWER_DOMAIN_MASK & ~(SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |	\
> +	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
> +	SKL_DISPLAY_DDI_A_E_POWER_DOMAINS |		\
> +	SKL_DISPLAY_DDI_B_POWER_DOMAINS |		\
> +	SKL_DISPLAY_DDI_C_POWER_DOMAINS |		\
> +	SKL_DISPLAY_DDI_D_POWER_DOMAINS |		\
> +	SKL_DISPLAY_MISC_IO_POWER_DOMAINS)) |		\
> +	BIT(POWER_DOMAIN_INIT))
> +
> +static void skl_set_power_well(struct drm_i915_private *dev_priv,
> +			struct i915_power_well *power_well, bool enable)
> +{
> +	uint32_t tmp, fuse_status;
> +	uint32_t req_mask, state_mask;
> +	bool check_fuse_status = false;
> +
> +	tmp = I915_READ(HSW_PWR_WELL_DRIVER);
> +	fuse_status = I915_READ(SKL_FUSE_STATUS);
> +
> +	switch (power_well->data) {
> +	case SKL_DISP_PW_1:
> +		if (wait_for((I915_READ(SKL_FUSE_STATUS) &
> +			SKL_FUSE_PG0_DIST_STATUS), 1)) {
> +			DRM_ERROR("PG0 not enabled\n");
> +			return;
> +		}
> +		break;
> +	case SKL_DISP_PW_2:
> +		if (!(fuse_status & SKL_FUSE_PG1_DIST_STATUS)) {
> +			DRM_ERROR("PG1 in disabled state\n");
> +			return;
> +		}
> +		break;
> +	case SKL_DISP_PW_DDI_A_E:
> +	case SKL_DISP_PW_DDI_B:
> +	case SKL_DISP_PW_DDI_C:
> +	case SKL_DISP_PW_DDI_D:
> +	case SKL_DISP_PW_MISC_IO:
> +		break;
> +	default:
> +		WARN(1, "Unknown power well %lu\n", power_well->data);
> +		return;
> +	}
> +
> +	req_mask = SKL_POWER_WELL_REQ(power_well->data);
> +	state_mask = SKL_POWER_WELL_STATE(power_well->data);
> +
> +	if (enable) {
> +		if (!(tmp & req_mask)) {
> +			I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | req_mask);
> +			DRM_DEBUG_KMS("Enabling %s\n", power_well->name);
> +		}
> +
> +		if (!(tmp & state_mask)) {
> +			if (wait_for((I915_READ(HSW_PWR_WELL_DRIVER) &
> +				state_mask), 1))
> +				DRM_ERROR("%s enable timeout\n",
> +					power_well->name);
> +			check_fuse_status = true;
> +		}
> +	} else {
> +		if (tmp & req_mask) {
> +			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 (check_fuse_status) {
> +		if (power_well->data == SKL_DISP_PW_1) {
> +			if (wait_for((I915_READ(SKL_FUSE_STATUS) &
> +				SKL_FUSE_PG1_DIST_STATUS), 1))
> +				DRM_ERROR("PG1 distributing status timeout\n");
> +		} else if (power_well->data == SKL_DISP_PW_2) {
> +			if (wait_for((I915_READ(SKL_FUSE_STATUS) &
> +				SKL_FUSE_PG2_DIST_STATUS), 1))
> +				DRM_ERROR("PG2 distributing status timeout\n");
> +		}
> +	}
> +}
> +
>  static void hsw_power_well_sync_hw(struct drm_i915_private *dev_priv,
>  				   struct i915_power_well *power_well)
>  {
> @@ -255,6 +385,36 @@ static void hsw_power_well_disable(struct drm_i915_private *dev_priv,
>  	hsw_set_power_well(dev_priv, power_well, false);
>  }
>  
> +static bool skl_power_well_enabled(struct drm_i915_private *dev_priv,
> +					struct i915_power_well *power_well)
> +{
> +	uint32_t mask = SKL_POWER_WELL_REQ(power_well->data) |
> +		SKL_POWER_WELL_STATE(power_well->data);
> +
> +	return (I915_READ(HSW_PWR_WELL_DRIVER) & mask) == mask;
> +}
> +
> +static void skl_power_well_sync_hw(struct drm_i915_private *dev_priv,
> +				struct i915_power_well *power_well)
> +{
> +	skl_set_power_well(dev_priv, power_well, power_well->count > 0);
> +
> +	/* Clear any request made by BIOS as driver is taking over */
> +	I915_WRITE(HSW_PWR_WELL_BIOS, 0);
> +}
> +
> +static void skl_power_well_enable(struct drm_i915_private *dev_priv,
> +				struct i915_power_well *power_well)
> +{
> +	skl_set_power_well(dev_priv, power_well, true);
> +}
> +
> +static void skl_power_well_disable(struct drm_i915_private *dev_priv,
> +				struct i915_power_well *power_well)
> +{
> +	skl_set_power_well(dev_priv, power_well, false);
> +}
> +
>  static void i9xx_always_on_power_well_noop(struct drm_i915_private *dev_priv,
>  					   struct i915_power_well *power_well)
>  {
> @@ -829,6 +989,13 @@ static const struct i915_power_well_ops hsw_power_well_ops = {
>  	.is_enabled = hsw_power_well_enabled,
>  };
>  
> +static const struct i915_power_well_ops skl_power_well_ops = {
> +	.sync_hw = skl_power_well_sync_hw,
> +	.enable = skl_power_well_enable,
> +	.disable = skl_power_well_disable,
> +	.is_enabled = skl_power_well_enabled,
> +};
> +
>  static struct i915_power_well hsw_power_wells[] = {
>  	{
>  		.name = "always-on",
> @@ -1059,6 +1226,57 @@ static struct i915_power_well *lookup_power_well(struct drm_i915_private *dev_pr
>  	return NULL;
>  }
>  
> +static struct i915_power_well skl_power_wells[] = {
> +	{
> +		.name = "always-on",
> +		.always_on = 1,
> +		.domains = SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
> +		.ops = &i9xx_always_on_power_well_ops,
> +	},
> +	{
> +		.name = "power well 1",
> +		.domains = SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS,
> +		.ops = &skl_power_well_ops,
> +		.data = SKL_DISP_PW_1,
> +	},
> +	{
> +		.name = "MISC IO power well",
> +		.domains = SKL_DISPLAY_MISC_IO_POWER_DOMAINS,
> +		.ops = &skl_power_well_ops,
> +		.data = SKL_DISP_PW_MISC_IO,
> +	},
> +	{
> +		.name = "power well 2",
> +		.domains = SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS,
> +		.ops = &skl_power_well_ops,
> +		.data = SKL_DISP_PW_2,
> +	},
> +	{
> +		.name = "DDI A/E power well",
> +		.domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS,
> +		.ops = &skl_power_well_ops,
> +		.data = SKL_DISP_PW_DDI_A_E,
> +	},
> +	{
> +		.name = "DDI B power well",
> +		.domains = SKL_DISPLAY_DDI_B_POWER_DOMAINS,
> +		.ops = &skl_power_well_ops,
> +		.data = SKL_DISP_PW_DDI_B,
> +	},
> +	{
> +		.name = "DDI C power well",
> +		.domains = SKL_DISPLAY_DDI_C_POWER_DOMAINS,
> +		.ops = &skl_power_well_ops,
> +		.data = SKL_DISP_PW_DDI_C,
> +	},
> +	{
> +		.name = "DDI D power well",
> +		.domains = SKL_DISPLAY_DDI_D_POWER_DOMAINS,
> +		.ops = &skl_power_well_ops,
> +		.data = SKL_DISP_PW_DDI_D,
> +	},
> +};
> +
>  #define set_power_wells(power_domains, __power_wells) ({		\
>  	(power_domains)->power_wells = (__power_wells);			\
>  	(power_domains)->power_well_count = ARRAY_SIZE(__power_wells);	\
> @@ -1085,6 +1303,8 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
>  		set_power_wells(power_domains, hsw_power_wells);
>  	} else if (IS_BROADWELL(dev_priv->dev)) {
>  		set_power_wells(power_domains, bdw_power_wells);
> +	} else if (IS_SKYLAKE(dev_priv->dev)) {
> +		set_power_wells(power_domains, skl_power_wells);
>  	} else if (IS_CHERRYVIEW(dev_priv->dev)) {
>  		set_power_wells(power_domains, chv_power_wells);
>  	} else if (IS_VALLEYVIEW(dev_priv->dev)) {
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/2] drm/i915/skl: Implementation of SKL display power well support
  2015-02-04 13:53     ` Damien Lespiau
@ 2015-02-04 14:20       ` Imre Deak
  2015-02-04 14:23         ` Imre Deak
  2015-02-04 14:24         ` Damien Lespiau
  0 siblings, 2 replies; 13+ messages in thread
From: Imre Deak @ 2015-02-04 14:20 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

On ke, 2015-02-04 at 13:53 +0000, Damien Lespiau wrote:
> On Tue, Feb 03, 2015 at 01:06:31AM +0200, Imre Deak wrote:
> > > +static struct i915_power_well skl_power_wells[] = {
> > > +	{
> > > +		.name = "always-on",
> > > +		.always_on = 1,
> > > +		.domains = SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
> > > +		.ops = &i9xx_always_on_power_well_ops,
> > > +	},
> > > +	{
> > > +		.name = "power well 1",
> > > +		.domains = SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS,
> > > +		.ops = &skl_power_well_ops,
> > > +		.data = SKL_DISP_PW_1,
> > > +	},
> 
> snip
> 
> > > +	{
> > > +		.name = "MISC IO power well",
> > > +		.domains = SKL_DISPLAY_MISC_IO_POWER_DOMAINS,
> > > +		.ops = &skl_power_well_ops,
> > > +		.data = SKL_DISP_PW_MISC_IO,
> > > +	}
> > 
> > Again, since the recent bspec change the misc IO power well should be
> > enabled before anything else, so it needs to be listed before "power
> > well 1" on the list.
> 
> So this one was causing problems. When I try to enabled MISC IO before
> PW1, the request times out. Enabling MISC IO just right after PW1 seems
> to work fine though.

Ok. Bspec doesn't say anything about the ordering between PW1 and MISC
IO, just that you have to enable them together and wait for PG1 fuse
afterwards. How about then moving the MISC IO power well right after PW1
in the list and wait for the PG1 fuse after enabling MISC IO?

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

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

* Re: [PATCH 2/2] drm/i915/skl: Implementation of SKL display power well support
  2015-02-04 14:20       ` Imre Deak
@ 2015-02-04 14:23         ` Imre Deak
  2015-02-04 14:24         ` Damien Lespiau
  1 sibling, 0 replies; 13+ messages in thread
From: Imre Deak @ 2015-02-04 14:23 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

On ke, 2015-02-04 at 16:20 +0200, Imre Deak wrote:
> On ke, 2015-02-04 at 13:53 +0000, Damien Lespiau wrote:
> > On Tue, Feb 03, 2015 at 01:06:31AM +0200, Imre Deak wrote:
> > > > +static struct i915_power_well skl_power_wells[] = {
> > > > +	{
> > > > +		.name = "always-on",
> > > > +		.always_on = 1,
> > > > +		.domains = SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
> > > > +		.ops = &i9xx_always_on_power_well_ops,
> > > > +	},
> > > > +	{
> > > > +		.name = "power well 1",
> > > > +		.domains = SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS,
> > > > +		.ops = &skl_power_well_ops,
> > > > +		.data = SKL_DISP_PW_1,
> > > > +	},
> > 
> > snip
> > 
> > > > +	{
> > > > +		.name = "MISC IO power well",
> > > > +		.domains = SKL_DISPLAY_MISC_IO_POWER_DOMAINS,
> > > > +		.ops = &skl_power_well_ops,
> > > > +		.data = SKL_DISP_PW_MISC_IO,
> > > > +	}
> > > 
> > > Again, since the recent bspec change the misc IO power well should be
> > > enabled before anything else, so it needs to be listed before "power
> > > well 1" on the list.
> > 
> > So this one was causing problems. When I try to enabled MISC IO before
> > PW1, the request times out. Enabling MISC IO just right after PW1 seems
> > to work fine though.
> 
> Ok. Bspec doesn't say anything about the ordering between PW1 and MISC
> IO, just that you have to enable them together and wait for PG1 fuse
> afterwards. How about then moving the MISC IO power well right after PW1
> in the list and wait for the PG1 fuse after enabling MISC IO?

Ah, haven't noticed v10, it looks ok to me.

--Imre

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

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

* Re: [PATCH 2/2] drm/i915/skl: Implementation of SKL display power well support
  2015-02-04 14:20       ` Imre Deak
  2015-02-04 14:23         ` Imre Deak
@ 2015-02-04 14:24         ` Damien Lespiau
  2015-02-04 14:29           ` Imre Deak
  1 sibling, 1 reply; 13+ messages in thread
From: Damien Lespiau @ 2015-02-04 14:24 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Wed, Feb 04, 2015 at 04:20:28PM +0200, Imre Deak wrote:
> On ke, 2015-02-04 at 13:53 +0000, Damien Lespiau wrote:
> > On Tue, Feb 03, 2015 at 01:06:31AM +0200, Imre Deak wrote:
> > > > +static struct i915_power_well skl_power_wells[] = {
> > > > +	{
> > > > +		.name = "always-on",
> > > > +		.always_on = 1,
> > > > +		.domains = SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
> > > > +		.ops = &i9xx_always_on_power_well_ops,
> > > > +	},
> > > > +	{
> > > > +		.name = "power well 1",
> > > > +		.domains = SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS,
> > > > +		.ops = &skl_power_well_ops,
> > > > +		.data = SKL_DISP_PW_1,
> > > > +	},
> > 
> > snip
> > 
> > > > +	{
> > > > +		.name = "MISC IO power well",
> > > > +		.domains = SKL_DISPLAY_MISC_IO_POWER_DOMAINS,
> > > > +		.ops = &skl_power_well_ops,
> > > > +		.data = SKL_DISP_PW_MISC_IO,
> > > > +	}
> > > 
> > > Again, since the recent bspec change the misc IO power well should be
> > > enabled before anything else, so it needs to be listed before "power
> > > well 1" on the list.
> > 
> > So this one was causing problems. When I try to enabled MISC IO before
> > PW1, the request times out. Enabling MISC IO just right after PW1 seems
> > to work fine though.
> 
> Ok. Bspec doesn't say anything about the ordering between PW1 and MISC
> IO, just that you have to enable them together and wait for PG1 fuse
> afterwards. How about then moving the MISC IO power well right after PW1
> in the list and wait for the PG1 fuse after enabling MISC IO?
 
I think we can even set the 2 requests in the same write and it should
do the right thing (and so merge the two power wells). That's really a
detail though and as the current code it seems to work, I'll leave such
refinements for later/if needed.

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

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

* Re: [PATCH 2/2] drm/i915/skl: Implementation of SKL display power well support
  2015-02-04 14:24         ` Damien Lespiau
@ 2015-02-04 14:29           ` Imre Deak
  0 siblings, 0 replies; 13+ messages in thread
From: Imre Deak @ 2015-02-04 14:29 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

On ke, 2015-02-04 at 14:24 +0000, Damien Lespiau wrote:
> On Wed, Feb 04, 2015 at 04:20:28PM +0200, Imre Deak wrote:
> > On ke, 2015-02-04 at 13:53 +0000, Damien Lespiau wrote:
> > > On Tue, Feb 03, 2015 at 01:06:31AM +0200, Imre Deak wrote:
> > > > > +static struct i915_power_well skl_power_wells[] = {
> > > > > +	{
> > > > > +		.name = "always-on",
> > > > > +		.always_on = 1,
> > > > > +		.domains = SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS,
> > > > > +		.ops = &i9xx_always_on_power_well_ops,
> > > > > +	},
> > > > > +	{
> > > > > +		.name = "power well 1",
> > > > > +		.domains = SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS,
> > > > > +		.ops = &skl_power_well_ops,
> > > > > +		.data = SKL_DISP_PW_1,
> > > > > +	},
> > > 
> > > snip
> > > 
> > > > > +	{
> > > > > +		.name = "MISC IO power well",
> > > > > +		.domains = SKL_DISPLAY_MISC_IO_POWER_DOMAINS,
> > > > > +		.ops = &skl_power_well_ops,
> > > > > +		.data = SKL_DISP_PW_MISC_IO,
> > > > > +	}
> > > > 
> > > > Again, since the recent bspec change the misc IO power well should be
> > > > enabled before anything else, so it needs to be listed before "power
> > > > well 1" on the list.
> > > 
> > > So this one was causing problems. When I try to enabled MISC IO before
> > > PW1, the request times out. Enabling MISC IO just right after PW1 seems
> > > to work fine though.
> > 
> > Ok. Bspec doesn't say anything about the ordering between PW1 and MISC
> > IO, just that you have to enable them together and wait for PG1 fuse
> > afterwards. How about then moving the MISC IO power well right after PW1
> > in the list and wait for the PG1 fuse after enabling MISC IO?
>  
> I think we can even set the 2 requests in the same write and it should
> do the right thing (and so merge the two power wells). That's really a
> detail though and as the current code it seems to work, I'll leave such
> refinements for later/if needed.

Yes, I had the same thought, agreed that it could be done as a
follow-up.

--Imre

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

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

end of thread, other threads:[~2015-02-04 14:29 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-16 15:57 [PATCH 0/2] Power well support for SKL Damien Lespiau
2015-01-16 15:57 ` [PATCH 1/2] drm/i915/skl: Adding power domains for AUX controllers Damien Lespiau
2015-01-20 10:11   ` Daniel Vetter
2015-01-16 15:57 ` [PATCH 2/2] drm/i915/skl: Implementation of SKL display power well support Damien Lespiau
2015-01-17 10:08   ` shuang.he
2015-02-02 23:06   ` Imre Deak
2015-02-04 13:53     ` Damien Lespiau
2015-02-04 14:20       ` Imre Deak
2015-02-04 14:23         ` Imre Deak
2015-02-04 14:24         ` Damien Lespiau
2015-02-04 14:29           ` Imre Deak
2015-02-04 13:57     ` [PATCH 2/2 v10] " Damien Lespiau
2015-02-04 14:19       ` Daniel Vetter

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.