[PATCH 0/8] SKL suspend/resume

[PATCH 0/8] SKL suspend/resume

[Damien Lespiau]

I had this series written on top of the DMC patches and waiting behind their
upstreaming. However Ubuntu wanted S3 support as they have some freeze dealine
coming soon. I rebase S3 support on top of -nightly then and smoke tested it,
seems to still work!

Things are not perfect, especially PC10 on display off is not happening, but
this could be for later.

-- 
Damien

Damien Lespiau (8):
  drm/i915/skl: Add the INIT power domain to the MISC I/O power well
  drm/i915/skl: Fix the CTRL typo in the DPLL_CRTL1 defines
  drm/i915: Re-order the PCU opcodes
  drm/i915: Merge the GEN9 memory latency PCU opcode with its friends
  drm/i915/skl: Make the Misc I/O power well part of the PLLS domain
  drm/i915/skl: Deinit/init the display at suspend/resume
  drm/i915/skl: Change CDCLK behind PCU's back
  drm/i915/skl: gen6+ platforms support runtime PM

 drivers/gpu/drm/i915/i915_drv.c         |  26 +++-
 drivers/gpu/drm/i915/i915_drv.h         |   6 +-
 drivers/gpu/drm/i915/i915_reg.h         |  42 +++---
 drivers/gpu/drm/i915/intel_ddi.c        |  28 ++--
 drivers/gpu/drm/i915/intel_display.c    | 218 +++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_dp.c         |  12 +-
 drivers/gpu/drm/i915/intel_drv.h        |   2 +
 drivers/gpu/drm/i915/intel_runtime_pm.c |   4 +-
 8 files changed, 290 insertions(+), 48 deletions(-)

-- 
2.1.0

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

[PATCH 1/8] drm/i915/skl: Add the INIT power domain to the MISC I/O power well

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 8fe2fde..6188e94 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -308,7 +308,8 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 	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)
+	SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |		\
+	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 |		\
-- 
2.1.0

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

Re: [PATCH 1/8] drm/i915/skl: Add the INIT power domain to the MISC I/O power well

[Ville Syrjälä]

On Thu, Apr 30, 2015 at 04:39:16PM +0100, Damien Lespiau wrote:
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 8fe2fde..6188e94 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -308,7 +308,8 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>  	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)
> +	SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |		\
> +	BIT(POWER_DOMAIN_INIT))

Yep, with the current power domain design INIT domain should be
everywhere.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>


>  #define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS (		\
>  	(POWER_DOMAIN_MASK & ~(SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |	\
>  	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 2/8] drm/i915/skl: Fix the CTRL typo in the DPLL_CRTL1 defines

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  2 +-
 drivers/gpu/drm/i915/i915_reg.h      | 18 +++++++++---------
 drivers/gpu/drm/i915/intel_ddi.c     | 26 +++++++++++++-------------
 drivers/gpu/drm/i915/intel_display.c |  6 +++---
 drivers/gpu/drm/i915/intel_dp.c      | 12 ++++++------
 5 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e8e8145..908c124 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -295,7 +295,7 @@ struct intel_dpll_hw_state {
 	/* skl */
 	/*
 	 * DPLL_CTRL1 has 6 bits for each each this DPLL. We store those in
-	 * lower part of crtl1 and they get shifted into position when writing
+	 * lower part of ctrl1 and they get shifted into position when writing
 	 * the register.  This allows us to easily compare the state to share
 	 * the DPLL.
 	 */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e35d7f2..cebb614 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7135,16 +7135,16 @@ enum skl_disp_power_wells {
 #define DPLL_CTRL1		0x6C058
 #define  DPLL_CTRL1_HDMI_MODE(id)		(1<<((id)*6+5))
 #define  DPLL_CTRL1_SSC(id)			(1<<((id)*6+4))
-#define  DPLL_CRTL1_LINK_RATE_MASK(id)		(7<<((id)*6+1))
-#define  DPLL_CRTL1_LINK_RATE_SHIFT(id)		((id)*6+1)
-#define  DPLL_CRTL1_LINK_RATE(linkrate, id)	((linkrate)<<((id)*6+1))
+#define  DPLL_CTRL1_LINK_RATE_MASK(id)		(7<<((id)*6+1))
+#define  DPLL_CTRL1_LINK_RATE_SHIFT(id)		((id)*6+1)
+#define  DPLL_CTRL1_LINK_RATE(linkrate, id)	((linkrate)<<((id)*6+1))
 #define  DPLL_CTRL1_OVERRIDE(id)		(1<<((id)*6))
-#define  DPLL_CRTL1_LINK_RATE_2700		0
-#define  DPLL_CRTL1_LINK_RATE_1350		1
-#define  DPLL_CRTL1_LINK_RATE_810		2
-#define  DPLL_CRTL1_LINK_RATE_1620		3
-#define  DPLL_CRTL1_LINK_RATE_1080		4
-#define  DPLL_CRTL1_LINK_RATE_2160		5
+#define  DPLL_CTRL1_LINK_RATE_2700		0
+#define  DPLL_CTRL1_LINK_RATE_1350		1
+#define  DPLL_CTRL1_LINK_RATE_810		2
+#define  DPLL_CTRL1_LINK_RATE_1620		3
+#define  DPLL_CTRL1_LINK_RATE_1080		4
+#define  DPLL_CTRL1_LINK_RATE_2160		5
 
 /* DPLL control2 */
 #define DPLL_CTRL2				0x6C05C
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 9c1e74a..d5bee8b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -870,26 +870,26 @@ static void skl_ddi_clock_get(struct intel_encoder *encoder,
 	if (dpll_ctl1 & DPLL_CTRL1_HDMI_MODE(dpll)) {
 		link_clock = skl_calc_wrpll_link(dev_priv, dpll);
 	} else {
-		link_clock = dpll_ctl1 & DPLL_CRTL1_LINK_RATE_MASK(dpll);
-		link_clock >>= DPLL_CRTL1_LINK_RATE_SHIFT(dpll);
+		link_clock = dpll_ctl1 & DPLL_CTRL1_LINK_RATE_MASK(dpll);
+		link_clock >>= DPLL_CTRL1_LINK_RATE_SHIFT(dpll);
 
 		switch (link_clock) {
-		case DPLL_CRTL1_LINK_RATE_810:
+		case DPLL_CTRL1_LINK_RATE_810:
 			link_clock = 81000;
 			break;
-		case DPLL_CRTL1_LINK_RATE_1080:
+		case DPLL_CTRL1_LINK_RATE_1080:
 			link_clock = 108000;
 			break;
-		case DPLL_CRTL1_LINK_RATE_1350:
+		case DPLL_CTRL1_LINK_RATE_1350:
 			link_clock = 135000;
 			break;
-		case DPLL_CRTL1_LINK_RATE_1620:
+		case DPLL_CTRL1_LINK_RATE_1620:
 			link_clock = 162000;
 			break;
-		case DPLL_CRTL1_LINK_RATE_2160:
+		case DPLL_CTRL1_LINK_RATE_2160:
 			link_clock = 216000;
 			break;
-		case DPLL_CRTL1_LINK_RATE_2700:
+		case DPLL_CTRL1_LINK_RATE_2700:
 			link_clock = 270000;
 			break;
 		default:
@@ -1294,13 +1294,13 @@ skl_ddi_pll_select(struct intel_crtc *intel_crtc,
 
 		switch (intel_dp->link_bw) {
 		case DP_LINK_BW_1_62:
-			ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_810, 0);
+			ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, 0);
 			break;
 		case DP_LINK_BW_2_7:
-			ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_1350, 0);
+			ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1350, 0);
 			break;
 		case DP_LINK_BW_5_4:
-			ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_2700, 0);
+			ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2700, 0);
 			break;
 		}
 
@@ -1854,7 +1854,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 
 			val &= ~(DPLL_CTRL1_HDMI_MODE(dpll) |
 				 DPLL_CTRL1_SSC(dpll) |
-				 DPLL_CRTL1_LINK_RATE_MASK(dpll));
+				 DPLL_CTRL1_LINK_RATE_MASK(dpll));
 			val |= crtc->config->dpll_hw_state.ctrl1 << (dpll * 6);
 
 			I915_WRITE(DPLL_CTRL1, val);
@@ -2100,7 +2100,7 @@ static void skl_ddi_pll_enable(struct drm_i915_private *dev_priv,
 	val = I915_READ(DPLL_CTRL1);
 
 	val &= ~(DPLL_CTRL1_HDMI_MODE(dpll) | DPLL_CTRL1_SSC(dpll) |
-		 DPLL_CRTL1_LINK_RATE_MASK(dpll));
+		 DPLL_CTRL1_LINK_RATE_MASK(dpll));
 	val |= pll->config.hw_state.ctrl1 << (dpll * 6);
 
 	I915_WRITE(DPLL_CTRL1, val);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3094b08..f2f4ad5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6383,10 +6383,10 @@ static int skylake_get_display_clock_speed(struct drm_device *dev)
 		return 540000;
 
 	linkrate = (I915_READ(DPLL_CTRL1) &
-		    DPLL_CRTL1_LINK_RATE_MASK(SKL_DPLL0)) >> 1;
+		    DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0)) >> 1;
 
-	if (linkrate == DPLL_CRTL1_LINK_RATE_2160 ||
-	    linkrate == DPLL_CRTL1_LINK_RATE_1080) {
+	if (linkrate == DPLL_CTRL1_LINK_RATE_2160 ||
+	    linkrate == DPLL_CTRL1_LINK_RATE_1080) {
 		/* vco 8640 */
 		switch (cdctl & CDCLK_FREQ_SEL_MASK) {
 		case CDCLK_FREQ_450_432:
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 937ba31..a2b8f02 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1098,30 +1098,30 @@ skl_edp_set_pll_config(struct intel_crtc_state *pipe_config, int link_clock)
 	ctrl1 = DPLL_CTRL1_OVERRIDE(SKL_DPLL0);
 	switch (link_clock / 2) {
 	case 81000:
-		ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_810,
+		ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810,
 					      SKL_DPLL0);
 		break;
 	case 135000:
-		ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_1350,
+		ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1350,
 					      SKL_DPLL0);
 		break;
 	case 270000:
-		ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_2700,
+		ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2700,
 					      SKL_DPLL0);
 		break;
 	case 162000:
-		ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_1620,
+		ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1620,
 					      SKL_DPLL0);
 		break;
 	/* TBD: For DP link rates 2.16 GHz and 4.32 GHz, VCO is 8640 which
 	results in CDCLK change. Need to handle the change of CDCLK by
 	disabling pipes and re-enabling them */
 	case 108000:
-		ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_1080,
+		ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080,
 					      SKL_DPLL0);
 		break;
 	case 216000:
-		ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_2160,
+		ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2160,
 					      SKL_DPLL0);
 		break;
 
-- 
2.1.0

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

[PATCH 3/8] drm/i915: Re-order the PCU opcodes

[Damien Lespiau]

Let's keep that list sorted!

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cebb614..880290d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6638,15 +6638,15 @@ enum skl_disp_power_wells {
 
 #define GEN6_PCODE_MAILBOX			0x138124
 #define   GEN6_PCODE_READY			(1<<31)
-#define   GEN6_READ_OC_PARAMS			0xc
-#define   GEN6_PCODE_WRITE_MIN_FREQ_TABLE	0x8
-#define   GEN6_PCODE_READ_MIN_FREQ_TABLE	0x9
 #define	  GEN6_PCODE_WRITE_RC6VIDS		0x4
 #define	  GEN6_PCODE_READ_RC6VIDS		0x5
+#define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
+#define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
+#define   GEN6_PCODE_WRITE_MIN_FREQ_TABLE	0x8
+#define   GEN6_PCODE_READ_MIN_FREQ_TABLE	0x9
+#define   GEN6_READ_OC_PARAMS			0xc
 #define   GEN6_PCODE_READ_D_COMP		0x10
 #define   GEN6_PCODE_WRITE_D_COMP		0x11
-#define   GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
-#define   GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
 #define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
 #define   DISPLAY_IPS_CONTROL			0x19
 #define	  HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL	0x1A
-- 
2.1.0

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

[PATCH 4/8] drm/i915: Merge the GEN9 memory latency PCU opcode with its friends

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 880290d..6d428a5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6642,6 +6642,11 @@ enum skl_disp_power_wells {
 #define	  GEN6_PCODE_READ_RC6VIDS		0x5
 #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
 #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
+#define   GEN9_PCODE_READ_MEM_LATENCY		0x6
+#define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
+#define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
+#define     GEN9_MEM_LATENCY_LEVEL_2_6_SHIFT	16
+#define     GEN9_MEM_LATENCY_LEVEL_3_7_SHIFT	24
 #define   GEN6_PCODE_WRITE_MIN_FREQ_TABLE	0x8
 #define   GEN6_PCODE_READ_MIN_FREQ_TABLE	0x9
 #define   GEN6_READ_OC_PARAMS			0xc
@@ -6655,12 +6660,6 @@ enum skl_disp_power_wells {
 #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT	16
 #define GEN6_PCODE_DATA1			0x13812C
 
-#define   GEN9_PCODE_READ_MEM_LATENCY		0x6
-#define   GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
-#define   GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
-#define   GEN9_MEM_LATENCY_LEVEL_2_6_SHIFT	16
-#define   GEN9_MEM_LATENCY_LEVEL_3_7_SHIFT	24
-
 #define GEN6_GT_CORE_STATUS		0x138060
 #define   GEN6_CORE_CPD_STATE_MASK	(7<<4)
 #define   GEN6_RCn_MASK			7
-- 
2.1.0

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

Re: [PATCH 4/8] drm/i915: Merge the GEN9 memory latency PCU opcode with its friends

[Ville Syrjälä]

On Thu, Apr 30, 2015 at 04:39:19PM +0100, Damien Lespiau wrote:
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

Patches 2,3,4 satisfy my brain's visual pattern matcher.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 880290d..6d428a5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6642,6 +6642,11 @@ enum skl_disp_power_wells {
>  #define	  GEN6_PCODE_READ_RC6VIDS		0x5
>  #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
>  #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
> +#define   GEN9_PCODE_READ_MEM_LATENCY		0x6
> +#define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
> +#define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
> +#define     GEN9_MEM_LATENCY_LEVEL_2_6_SHIFT	16
> +#define     GEN9_MEM_LATENCY_LEVEL_3_7_SHIFT	24
>  #define   GEN6_PCODE_WRITE_MIN_FREQ_TABLE	0x8
>  #define   GEN6_PCODE_READ_MIN_FREQ_TABLE	0x9
>  #define   GEN6_READ_OC_PARAMS			0xc
> @@ -6655,12 +6660,6 @@ enum skl_disp_power_wells {
>  #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT	16
>  #define GEN6_PCODE_DATA1			0x13812C
>  
> -#define   GEN9_PCODE_READ_MEM_LATENCY		0x6
> -#define   GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
> -#define   GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
> -#define   GEN9_MEM_LATENCY_LEVEL_2_6_SHIFT	16
> -#define   GEN9_MEM_LATENCY_LEVEL_3_7_SHIFT	24
> -
>  #define GEN6_GT_CORE_STATUS		0x138060
>  #define   GEN6_CORE_CPD_STATE_MASK	(7<<4)
>  #define   GEN6_RCn_MASK			7
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 5/8] drm/i915/skl: Make the Misc I/O power well part of the PLLS domain

[Damien Lespiau]

The specs tell us to ungate PG1 and Misc I/O at display init. We'll use
the PLLS power domain to ensure those two power wells are up.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 6188e94..5b0bce5 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -309,6 +309,7 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 	BIT(POWER_DOMAIN_INIT))
 #define SKL_DISPLAY_MISC_IO_POWER_DOMAINS (		\
 	SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |		\
+	BIT(POWER_DOMAIN_PLLS) |			\
 	BIT(POWER_DOMAIN_INIT))
 #define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS (		\
 	(POWER_DOMAIN_MASK & ~(SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |	\
-- 
2.1.0

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

Re: [PATCH 5/8] drm/i915/skl: Make the Misc I/O power well part of the PLLS domain

[Ville Syrjälä]

On Thu, Apr 30, 2015 at 04:39:20PM +0100, Damien Lespiau wrote:
> The specs tell us to ungate PG1 and Misc I/O at display init. We'll use
> the PLLS power domain to ensure those two power wells are up.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

Yep, spec does say that.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 6188e94..5b0bce5 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -309,6 +309,7 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>  	BIT(POWER_DOMAIN_INIT))
>  #define SKL_DISPLAY_MISC_IO_POWER_DOMAINS (		\
>  	SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |		\
> +	BIT(POWER_DOMAIN_PLLS) |			\
>  	BIT(POWER_DOMAIN_INIT))
>  #define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS (		\
>  	(POWER_DOMAIN_MASK & ~(SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |	\
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 6/8] drm/i915/skl: Deinit/init the display at suspend/resume

[Damien Lespiau]

We need to re-init the display hardware when going out of suspend. This
includes:

  - Hooking the PCH to the reset logic
  - Restoring CDCDLK
  - Enabling the DDB power

Among those, only the CDCDLK one is a bit tricky. There's some
complexity in that:

  - DPLL0 (which is the source for CDCLK) has two VCOs, each with a set
    of supported frequencies. As eDP also uses DPLL0 for its link rate,
    once DPLL0 is on, we restrict the possible eDP link rates the chosen
    VCO.
  - CDCLK also limits the bandwidth available to push pixels.
  - My current PCU firmware never ack the frequency change request

So, as a first step, this commit restore what the BIOS set, until I can
do more testing.

----
In case that's of interest for the reviewer, I've unit tested the
function that derives the decimal frequency field:

  #include <stdio.h>
  #include <stdint.h>
  #include <assert.h>

  #define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))

  static const struct dpll_freq {
          unsigned int freq;
          unsigned int decimal;
  } freqs[] = {
          { .freq = 308570, .decimal = 0b01001100111},
          { .freq = 337500, .decimal = 0b01010100001},
          { .freq = 432000, .decimal = 0b01101011110},
          { .freq = 450000, .decimal = 0b01110000010},
          { .freq = 540000, .decimal = 0b10000110110},
          { .freq = 617140, .decimal = 0b10011010000},
          { .freq = 675000, .decimal = 0b10101000100},
  };

  static void intbits(unsigned int v)
  {
          int i;

          for(i = 10; i >= 0; i--)
                  putchar('0' + ((v >> i) & 1));
  }

  static unsigned int freq_decimal(unsigned int freq /* in kHz */)
  {
          unsigned int mhz, dot5;

          mhz = freq / 1000;
          dot5 = (freq - mhz * 1000) >= 500;

          return (mhz - 1) << 1 | dot5;
  }

  static void test_freq(const struct dpll_freq *entry)
  {
          unsigned int decimal = freq_decimal(entry->freq);

          printf("freq: %d, expected: ", entry->freq);
          intbits(entry->decimal);
          printf(", got: ");
          intbits(decimal);
          putchar('\n');

          assert(decimal == entry->decimal);
  }

  int main(int argc, char **argv)
  {
          int i;

          for (i = 0; i < ARRAY_SIZE(freqs); i++)
                  test_freq(&freqs[i]);

          return 0;
  }

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      |  26 ++++-
 drivers/gpu/drm/i915/i915_drv.h      |   1 +
 drivers/gpu/drm/i915/i915_reg.h      |   3 +
 drivers/gpu/drm/i915/intel_ddi.c     |   2 +
 drivers/gpu/drm/i915/intel_display.c | 208 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |   2 +
 6 files changed, 240 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e70adfd..58db0c6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -574,6 +574,7 @@ static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
 static int intel_suspend_complete(struct drm_i915_private *dev_priv);
 static int vlv_resume_prepare(struct drm_i915_private *dev_priv,
 			      bool rpm_resume);
+static int skl_resume_prepare(struct drm_i915_private *dev_priv);
 
 static int i915_drm_suspend(struct drm_device *dev)
 {
@@ -781,6 +782,9 @@ static int i915_drm_resume_early(struct drm_device *dev)
 
 	if (IS_VALLEYVIEW(dev_priv))
 		ret = vlv_resume_prepare(dev_priv, false);
+	else if (IS_SKYLAKE(dev_priv))
+		ret = skl_resume_prepare(dev_priv);
+
 	if (ret)
 		DRM_ERROR("Resume prepare failed: %d,Continuing resume\n", ret);
 
@@ -1009,6 +1013,20 @@ static int hsw_suspend_complete(struct drm_i915_private *dev_priv)
 	return 0;
 }
 
+static int skl_suspend_complete(struct drm_i915_private *dev_priv)
+{
+	skl_display_suspend(dev_priv);
+
+	return 0;
+}
+
+static int skl_resume_prepare(struct drm_i915_private *dev_priv)
+{
+	skl_display_resume(dev_priv);
+
+	return 0;
+}
+
 static int bxt_suspend_complete(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
@@ -1500,7 +1518,9 @@ static int intel_runtime_resume(struct device *device)
 	if (IS_GEN6(dev_priv))
 		intel_init_pch_refclk(dev);
 
-	if (IS_BROXTON(dev))
+	if (IS_SKYLAKE(dev))
+		ret = skl_resume_prepare(dev_priv);
+	else if (IS_BROXTON(dev))
 		ret = bxt_resume_prepare(dev_priv);
 	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
 		hsw_disable_pc8(dev_priv);
@@ -1534,7 +1554,9 @@ static int intel_suspend_complete(struct drm_i915_private *dev_priv)
 	struct drm_device *dev = dev_priv->dev;
 	int ret;
 
-	if (IS_BROXTON(dev))
+	if (IS_SKYLAKE(dev))
+		ret = skl_suspend_complete(dev_priv);
+	else if (IS_BROXTON(dev))
 		ret = bxt_suspend_complete(dev_priv);
 	else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
 		ret = hsw_suspend_complete(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 908c124..f637667 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1659,6 +1659,7 @@ struct drm_i915_private {
 	int num_fence_regs; /* 8 on pre-965, 16 otherwise */
 
 	unsigned int fsb_freq, mem_freq, is_ddr3;
+	unsigned int skl_boot_cdclk;
 	unsigned int cdclk_freq;
 	unsigned int hpll_freq;
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6d428a5..4b063a8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6647,6 +6647,9 @@ enum skl_disp_power_wells {
 #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
 #define     GEN9_MEM_LATENCY_LEVEL_2_6_SHIFT	16
 #define     GEN9_MEM_LATENCY_LEVEL_3_7_SHIFT	24
+#define   SKL_PCODE_CDCLK_CONTROL		0x7
+#define     SKL_CDCLK_PREPARE_FOR_CHANGE	0x3
+#define     SKL_CDCLK_READY_FOR_CHANGE		0x1
 #define   GEN6_PCODE_WRITE_MIN_FREQ_TABLE	0x8
 #define   GEN6_PCODE_READ_MIN_FREQ_TABLE	0x9
 #define   GEN6_READ_OC_PARAMS			0xc
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index d5bee8b..f76bcd3 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2481,6 +2481,8 @@ void intel_ddi_pll_init(struct drm_device *dev)
 	if (IS_SKYLAKE(dev)) {
 		if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
 			DRM_ERROR("LCPLL1 is disabled\n");
+		else
+			intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
 	} else if (IS_BROXTON(dev)) {
 		broxton_init_cdclk(dev);
 		broxton_ddi_phy_init(dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f2f4ad5..9c8338f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5420,6 +5420,213 @@ void broxton_uninit_cdclk(struct drm_device *dev)
 	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
 }
 
+/*
+ * For now, we store the CDCLK frequency set by the BIOS and restore it at
+ * resume.
+ */
+static void skl_init_cdclk(struct drm_i915_private *dev_priv)
+{
+	if (!IS_SKYLAKE(dev_priv))
+		return;
+
+	dev_priv->skl_boot_cdclk =
+		dev_priv->display.get_display_clock_speed(dev_priv->dev);
+
+	DRM_DEBUG_DRIVER("CDCLK: %d kHz\n", dev_priv->skl_boot_cdclk);
+}
+
+static const struct skl_cdclk_entry {
+	unsigned int freq;
+	unsigned int vco;
+} skl_cdclk_frequencies[] = {
+	{ .freq = 308570, .vco = 8640 },
+	{ .freq = 337500, .vco = 8100 },
+	{ .freq = 432000, .vco = 8640 },
+	{ .freq = 450000, .vco = 8100 },
+	{ .freq = 540000, .vco = 8100 },
+	{ .freq = 617140, .vco = 8640 },
+	{ .freq = 675000, .vco = 8100 },
+};
+
+static unsigned int skl_cdclk_get_vco(unsigned int freq)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(skl_cdclk_frequencies); i++) {
+		const struct skl_cdclk_entry *e = &skl_cdclk_frequencies[i];
+
+		if (e->freq == freq)
+			return e->vco;
+	}
+
+	return 8100;
+}
+
+static unsigned int skl_cdlck_decimal(unsigned int freq)
+{
+        unsigned int mhz, dot5;
+
+        mhz = freq / 1000;
+        dot5 = (freq - mhz * 1000) >= 500;
+
+        return (mhz - 1) << 1 | dot5;
+}
+
+static void
+skl_dpll0_enable(struct drm_i915_private *dev_priv, unsigned int required_vco)
+{
+	unsigned int min_freq;
+	u32 val;
+
+	/* select the minimum CDCLK before enabling DPLL 0 */
+	val = I915_READ(CDCLK_CTL);
+	val &= ~CDCLK_FREQ_SEL_MASK | ~CDCLK_FREQ_DECIMAL_MASK;
+	val |= CDCLK_FREQ_337_308;
+
+	if (required_vco == 8640)
+		min_freq = 308570;
+	else
+		min_freq = 337500;
+
+	val = CDCLK_FREQ_337_308 | skl_cdlck_decimal(min_freq);
+
+	I915_WRITE(CDCLK_CTL, val);
+	POSTING_READ(CDCLK_CTL);
+
+	/*
+	 * We always enable DPLL0 with the lowest link rate possible, but still
+	 * taking into account the VCO required to operate the eDP panel at the
+	 * desired frequency. The usual DP link rates operate with a VCO of
+	 * 8100 while the eDP 1.4 alternate link rates need a VCO of 8640.
+	 * The modeset code is responsible for the selection of the exact link
+	 * rate later on, with the constraint of choosing a frequency that
+	 * works with required_vco.
+	 */
+	val = I915_READ(DPLL_CTRL1);
+
+	val &= ~(DPLL_CTRL1_HDMI_MODE(0) | DPLL_CTRL1_SSC(0) |
+		 DPLL_CTRL1_LINK_RATE_MASK(0));
+	val |= DPLL_CTRL1_OVERRIDE(0);
+	if (required_vco == 8640)
+		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080, 0);
+	else
+		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, 0);
+
+	I915_WRITE(DPLL_CTRL1, val);
+	POSTING_READ(DPLL_CTRL1);
+
+	I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) | LCPLL_PLL_ENABLE);
+
+	if (wait_for(I915_READ(LCPLL1_CTL) & (1 << 30), 5))
+		DRM_ERROR("DPLL0 not locked\n");
+}
+
+static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
+{
+	int ret;
+	u32 val, freq_select, freq_decimal, pcu_ack;
+
+	DRM_DEBUG_DRIVER("Changing CDCLK to %dKHz\n", freq);
+
+	/* inform PCU we want to change CDCLK */
+	val = SKL_CDCLK_PREPARE_FOR_CHANGE;
+	mutex_lock(&dev_priv->rps.hw_lock);
+	ret = sandybridge_pcode_read(dev_priv, SKL_PCODE_CDCLK_CONTROL, &val);
+	mutex_unlock(&dev_priv->rps.hw_lock);
+	if (ret || !(val & SKL_CDCLK_READY_FOR_CHANGE)) {
+		DRM_ERROR("failed to inform PCU about cdclk change\n");
+		return;
+	}
+
+	/* set CDCLK_CTL */
+	switch(freq) {
+	case 450000:
+	case 432000:
+		freq_select = CDCLK_FREQ_450_432;
+		pcu_ack = 1;
+		break;
+	case 540000:
+		freq_select = CDCLK_FREQ_540;
+		pcu_ack = 2;
+		break;
+	case 308570:
+	case 337500:
+	default:
+		freq_select = CDCLK_FREQ_337_308;
+		pcu_ack = 0;
+		break;
+	case 617140:
+	case 675000:
+		freq_select = CDCLK_FREQ_675_617;
+		pcu_ack = 3;
+		break;
+	}
+
+	freq_decimal = skl_cdlck_decimal(freq);
+
+	I915_WRITE(CDCLK_CTL, freq_select | freq_decimal);
+	POSTING_READ(CDCLK_CTL);
+
+	/* inform PCU of the change */
+	mutex_lock(&dev_priv->rps.hw_lock);
+	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, pcu_ack);
+	mutex_unlock(&dev_priv->rps.hw_lock);
+}
+
+void skl_display_suspend(struct drm_i915_private *dev_priv)
+{
+	/* disable DBUF power */
+	I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) & ~DBUF_POWER_REQUEST);
+	POSTING_READ(DBUF_CTL);
+
+	udelay(10);
+
+	if (I915_READ(DBUF_CTL) & DBUF_POWER_STATE)
+		DRM_ERROR("DBuf power disable timeout\n");
+
+	/* disable DPLL0 */
+	I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) & ~LCPLL_PLL_ENABLE);
+	if (wait_for(!(I915_READ(LCPLL1_CTL) & (1 << 30)), 1))
+		DRM_ERROR("Couldn't disable DPLL0\n");
+
+	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
+}
+
+void skl_display_resume(struct drm_i915_private *dev_priv)
+{
+	u32 val;
+	unsigned int required_vco;
+
+	/* enable PCH reset handshake */
+	val = I915_READ(HSW_NDE_RSTWRN_OPT);
+	I915_WRITE(HSW_NDE_RSTWRN_OPT, val & RESET_PCH_HANDSHAKE_ENABLE);
+
+	/* enable PG1 and Misc I/O */
+	intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
+
+	/* DPLL0 already enabed !? */
+	if (I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE) {
+		DRM_DEBUG_DRIVER("DPLL0 already running\n");
+		return;
+	}
+
+	/* enable DPLL0 */
+	required_vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk);
+	skl_dpll0_enable(dev_priv, required_vco);
+
+	/* set CDCLK to the frequency the BIOS chose */
+	skl_set_cdclk(dev_priv, dev_priv->skl_boot_cdclk);
+
+	/* enable DBUF power */
+	I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) | DBUF_POWER_REQUEST);
+	POSTING_READ(DBUF_CTL);
+
+	udelay(10);
+
+	if (!(I915_READ(DBUF_CTL) & DBUF_POWER_STATE))
+		DRM_ERROR("DBuf power enable timeout\n");
+}
+
 /* returns HPLL frequency in kHz */
 static int valleyview_get_vco(struct drm_i915_private *dev_priv)
 {
@@ -14573,6 +14780,7 @@ void intel_modeset_init(struct drm_device *dev)
 
 	intel_init_dpio(dev);
 
+	skl_init_cdclk(dev_priv);
 	intel_shared_dpll_init(dev);
 
 	/* Just disable it once at startup */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 43fe003..67275a1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1123,6 +1123,8 @@ void broxton_ddi_phy_init(struct drm_device *dev);
 void broxton_ddi_phy_uninit(struct drm_device *dev);
 void bxt_enable_dc9(struct drm_i915_private *dev_priv);
 void bxt_disable_dc9(struct drm_i915_private *dev_priv);
+void skl_display_suspend(struct drm_i915_private *dev_priv);
+void skl_display_resume(struct drm_i915_private *dev_priv);
 void intel_dp_get_m_n(struct intel_crtc *crtc,
 		      struct intel_crtc_state *pipe_config);
 void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
-- 
2.1.0

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

Re: [PATCH 6/8] drm/i915/skl: Deinit/init the display at suspend/resume

[Ville Syrjälä]

On Thu, Apr 30, 2015 at 04:39:21PM +0100, Damien Lespiau wrote:
> We need to re-init the display hardware when going out of suspend. This
> includes:
> 
>   - Hooking the PCH to the reset logic
>   - Restoring CDCDLK
>   - Enabling the DDB power
> 
> Among those, only the CDCDLK one is a bit tricky. There's some
> complexity in that:
> 
>   - DPLL0 (which is the source for CDCLK) has two VCOs, each with a set
>     of supported frequencies. As eDP also uses DPLL0 for its link rate,
>     once DPLL0 is on, we restrict the possible eDP link rates the chosen
>     VCO.
>   - CDCLK also limits the bandwidth available to push pixels.
>   - My current PCU firmware never ack the frequency change request
> 
> So, as a first step, this commit restore what the BIOS set, until I can
> do more testing.
> 
> ----
> In case that's of interest for the reviewer, I've unit tested the
> function that derives the decimal frequency field:
> 
>   #include <stdio.h>
>   #include <stdint.h>
>   #include <assert.h>
> 
>   #define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
> 
>   static const struct dpll_freq {
>           unsigned int freq;
>           unsigned int decimal;
>   } freqs[] = {
>           { .freq = 308570, .decimal = 0b01001100111},
>           { .freq = 337500, .decimal = 0b01010100001},
>           { .freq = 432000, .decimal = 0b01101011110},
>           { .freq = 450000, .decimal = 0b01110000010},
>           { .freq = 540000, .decimal = 0b10000110110},
>           { .freq = 617140, .decimal = 0b10011010000},
>           { .freq = 675000, .decimal = 0b10101000100},
>   };
> 
>   static void intbits(unsigned int v)
>   {
>           int i;
> 
>           for(i = 10; i >= 0; i--)
>                   putchar('0' + ((v >> i) & 1));
>   }
> 
>   static unsigned int freq_decimal(unsigned int freq /* in kHz */)
>   {
>           unsigned int mhz, dot5;
> 
>           mhz = freq / 1000;
>           dot5 = (freq - mhz * 1000) >= 500;
> 
>           return (mhz - 1) << 1 | dot5;
>   }
> 
>   static void test_freq(const struct dpll_freq *entry)
>   {
>           unsigned int decimal = freq_decimal(entry->freq);
> 
>           printf("freq: %d, expected: ", entry->freq);
>           intbits(entry->decimal);
>           printf(", got: ");
>           intbits(decimal);
>           putchar('\n');
> 
>           assert(decimal == entry->decimal);
>   }
> 
>   int main(int argc, char **argv)
>   {
>           int i;
> 
>           for (i = 0; i < ARRAY_SIZE(freqs); i++)
>                   test_freq(&freqs[i]);
> 
>           return 0;
>   }
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c      |  26 ++++-
>  drivers/gpu/drm/i915/i915_drv.h      |   1 +
>  drivers/gpu/drm/i915/i915_reg.h      |   3 +
>  drivers/gpu/drm/i915/intel_ddi.c     |   2 +
>  drivers/gpu/drm/i915/intel_display.c | 208 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |   2 +
>  6 files changed, 240 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e70adfd..58db0c6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -574,6 +574,7 @@ static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
>  static int intel_suspend_complete(struct drm_i915_private *dev_priv);
>  static int vlv_resume_prepare(struct drm_i915_private *dev_priv,
>  			      bool rpm_resume);
> +static int skl_resume_prepare(struct drm_i915_private *dev_priv);
>  
>  static int i915_drm_suspend(struct drm_device *dev)
>  {
> @@ -781,6 +782,9 @@ static int i915_drm_resume_early(struct drm_device *dev)
>  
>  	if (IS_VALLEYVIEW(dev_priv))
>  		ret = vlv_resume_prepare(dev_priv, false);
> +	else if (IS_SKYLAKE(dev_priv))
> +		ret = skl_resume_prepare(dev_priv);
> +
>  	if (ret)
>  		DRM_ERROR("Resume prepare failed: %d,Continuing resume\n", ret);
>  
> @@ -1009,6 +1013,20 @@ static int hsw_suspend_complete(struct drm_i915_private *dev_priv)
>  	return 0;
>  }
>  
> +static int skl_suspend_complete(struct drm_i915_private *dev_priv)
> +{
> +	skl_display_suspend(dev_priv);
> +
> +	return 0;
> +}
> +
> +static int skl_resume_prepare(struct drm_i915_private *dev_priv)
> +{
> +	skl_display_resume(dev_priv);
> +
> +	return 0;
> +}
> +
>  static int bxt_suspend_complete(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
> @@ -1500,7 +1518,9 @@ static int intel_runtime_resume(struct device *device)
>  	if (IS_GEN6(dev_priv))
>  		intel_init_pch_refclk(dev);
>  
> -	if (IS_BROXTON(dev))
> +	if (IS_SKYLAKE(dev))
> +		ret = skl_resume_prepare(dev_priv);
> +	else if (IS_BROXTON(dev))
>  		ret = bxt_resume_prepare(dev_priv);
>  	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
>  		hsw_disable_pc8(dev_priv);
> @@ -1534,7 +1554,9 @@ static int intel_suspend_complete(struct drm_i915_private *dev_priv)
>  	struct drm_device *dev = dev_priv->dev;
>  	int ret;
>  
> -	if (IS_BROXTON(dev))
> +	if (IS_SKYLAKE(dev))
> +		ret = skl_suspend_complete(dev_priv);
> +	else if (IS_BROXTON(dev))
>  		ret = bxt_suspend_complete(dev_priv);
>  	else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
>  		ret = hsw_suspend_complete(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 908c124..f637667 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1659,6 +1659,7 @@ struct drm_i915_private {
>  	int num_fence_regs; /* 8 on pre-965, 16 otherwise */
>  
>  	unsigned int fsb_freq, mem_freq, is_ddr3;
> +	unsigned int skl_boot_cdclk;
>  	unsigned int cdclk_freq;
>  	unsigned int hpll_freq;
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 6d428a5..4b063a8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6647,6 +6647,9 @@ enum skl_disp_power_wells {
>  #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
>  #define     GEN9_MEM_LATENCY_LEVEL_2_6_SHIFT	16
>  #define     GEN9_MEM_LATENCY_LEVEL_3_7_SHIFT	24
> +#define   SKL_PCODE_CDCLK_CONTROL		0x7
> +#define     SKL_CDCLK_PREPARE_FOR_CHANGE	0x3
> +#define     SKL_CDCLK_READY_FOR_CHANGE		0x1
>  #define   GEN6_PCODE_WRITE_MIN_FREQ_TABLE	0x8
>  #define   GEN6_PCODE_READ_MIN_FREQ_TABLE	0x9
>  #define   GEN6_READ_OC_PARAMS			0xc
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index d5bee8b..f76bcd3 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2481,6 +2481,8 @@ void intel_ddi_pll_init(struct drm_device *dev)
>  	if (IS_SKYLAKE(dev)) {
>  		if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
>  			DRM_ERROR("LCPLL1 is disabled\n");
> +		else
> +			intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
>  	} else if (IS_BROXTON(dev)) {
>  		broxton_init_cdclk(dev);
>  		broxton_ddi_phy_init(dev);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f2f4ad5..9c8338f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5420,6 +5420,213 @@ void broxton_uninit_cdclk(struct drm_device *dev)
>  	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
>  }
>  
> +/*
> + * For now, we store the CDCLK frequency set by the BIOS and restore it at
> + * resume.
> + */
> +static void skl_init_cdclk(struct drm_i915_private *dev_priv)
> +{
> +	if (!IS_SKYLAKE(dev_priv))
> +		return;
> +
> +	dev_priv->skl_boot_cdclk =
> +		dev_priv->display.get_display_clock_speed(dev_priv->dev);
> +
> +	DRM_DEBUG_DRIVER("CDCLK: %d kHz\n", dev_priv->skl_boot_cdclk);
> +}

The naming is a bit confusing wrt. bxt now.

So it looks like broxton_init_cdclk() == skl_display_resume(),
and skl_init_cdclk() just reads out the current setting and stashes it
somewhere, and there's no counterpart for that on bxt. Would be nice to
unify a bit to avoid loads of confusion.

> +
> +static const struct skl_cdclk_entry {
> +	unsigned int freq;
> +	unsigned int vco;
> +} skl_cdclk_frequencies[] = {
> +	{ .freq = 308570, .vco = 8640 },
> +	{ .freq = 337500, .vco = 8100 },
> +	{ .freq = 432000, .vco = 8640 },
> +	{ .freq = 450000, .vco = 8100 },
> +	{ .freq = 540000, .vco = 8100 },
> +	{ .freq = 617140, .vco = 8640 },
> +	{ .freq = 675000, .vco = 8100 },
> +};
> +
> +static unsigned int skl_cdclk_get_vco(unsigned int freq)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(skl_cdclk_frequencies); i++) {
> +		const struct skl_cdclk_entry *e = &skl_cdclk_frequencies[i];
> +
> +		if (e->freq == freq)
> +			return e->vco;
> +	}
> +
> +	return 8100;
> +}
> +
> +static unsigned int skl_cdlck_decimal(unsigned int freq)
> +{
> +        unsigned int mhz, dot5;
> +
> +        mhz = freq / 1000;
> +        dot5 = (freq - mhz * 1000) >= 500;
> +
> +        return (mhz - 1) << 1 | dot5;
> +}

Just '(freq - 1000) / 500' like we do for bxt?

> +
> +static void
> +skl_dpll0_enable(struct drm_i915_private *dev_priv, unsigned int required_vco)
> +{
> +	unsigned int min_freq;
> +	u32 val;
> +
> +	/* select the minimum CDCLK before enabling DPLL 0 */
> +	val = I915_READ(CDCLK_CTL);
> +	val &= ~CDCLK_FREQ_SEL_MASK | ~CDCLK_FREQ_DECIMAL_MASK;
> +	val |= CDCLK_FREQ_337_308;
> +
> +	if (required_vco == 8640)
> +		min_freq = 308570;
> +	else
> +		min_freq = 337500;
> +
> +	val = CDCLK_FREQ_337_308 | skl_cdlck_decimal(min_freq);
> +
> +	I915_WRITE(CDCLK_CTL, val);
> +	POSTING_READ(CDCLK_CTL);
> +
> +	/*
> +	 * We always enable DPLL0 with the lowest link rate possible, but still
> +	 * taking into account the VCO required to operate the eDP panel at the
> +	 * desired frequency. The usual DP link rates operate with a VCO of
> +	 * 8100 while the eDP 1.4 alternate link rates need a VCO of 8640.
> +	 * The modeset code is responsible for the selection of the exact link
> +	 * rate later on, with the constraint of choosing a frequency that
> +	 * works with required_vco.
> +	 */
> +	val = I915_READ(DPLL_CTRL1);
> +
> +	val &= ~(DPLL_CTRL1_HDMI_MODE(0) | DPLL_CTRL1_SSC(0) |
> +		 DPLL_CTRL1_LINK_RATE_MASK(0));
> +	val |= DPLL_CTRL1_OVERRIDE(0);
> +	if (required_vco == 8640)
> +		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080, 0);
> +	else
> +		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, 0);

Hmm. These new pll registers are very confusing. But looks correct
based on my understanding.

> +
> +	I915_WRITE(DPLL_CTRL1, val);
> +	POSTING_READ(DPLL_CTRL1);
> +
> +	I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) | LCPLL_PLL_ENABLE);
> +
> +	if (wait_for(I915_READ(LCPLL1_CTL) & (1 << 30), 5))

LCPLL_PLL_LOCK

> +		DRM_ERROR("DPLL0 not locked\n");
> +}
> +
> +static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
> +{
> +	int ret;
> +	u32 val, freq_select, freq_decimal, pcu_ack;
> +
> +	DRM_DEBUG_DRIVER("Changing CDCLK to %dKHz\n", freq);
> +
> +	/* inform PCU we want to change CDCLK */
> +	val = SKL_CDCLK_PREPARE_FOR_CHANGE;
> +	mutex_lock(&dev_priv->rps.hw_lock);
> +	ret = sandybridge_pcode_read(dev_priv, SKL_PCODE_CDCLK_CONTROL, &val);
> +	mutex_unlock(&dev_priv->rps.hw_lock);
> +	if (ret || !(val & SKL_CDCLK_READY_FOR_CHANGE)) {
> +		DRM_ERROR("failed to inform PCU about cdclk change\n");
> +		return;
> +	}

Spec says we should keep repeating this until we get the
SKL_CDCLK_READY_FOR_CHANGE bit or 150us timeout has passed. The code
however will only do it once.

> +
> +	/* set CDCLK_CTL */
> +	switch(freq) {
> +	case 450000:
> +	case 432000:
> +		freq_select = CDCLK_FREQ_450_432;
> +		pcu_ack = 1;
> +		break;
> +	case 540000:
> +		freq_select = CDCLK_FREQ_540;
> +		pcu_ack = 2;
> +		break;
> +	case 308570:
> +	case 337500:
> +	default:
> +		freq_select = CDCLK_FREQ_337_308;
> +		pcu_ack = 0;
> +		break;
> +	case 617140:
> +	case 675000:
> +		freq_select = CDCLK_FREQ_675_617;
> +		pcu_ack = 3;
> +		break;
> +	}
> +
> +	freq_decimal = skl_cdlck_decimal(freq);
> +
> +	I915_WRITE(CDCLK_CTL, freq_select | freq_decimal);
> +	POSTING_READ(CDCLK_CTL);
> +
> +	/* inform PCU of the change */
> +	mutex_lock(&dev_priv->rps.hw_lock);
> +	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, pcu_ack);
> +	mutex_unlock(&dev_priv->rps.hw_lock);
> +}
> +
> +void skl_display_suspend(struct drm_i915_private *dev_priv)
> +{
> +	/* disable DBUF power */
> +	I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) & ~DBUF_POWER_REQUEST);
> +	POSTING_READ(DBUF_CTL);
> +
> +	udelay(10);
> +
> +	if (I915_READ(DBUF_CTL) & DBUF_POWER_STATE)
> +		DRM_ERROR("DBuf power disable timeout\n");
> +
> +	/* disable DPLL0 */
> +	I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) & ~LCPLL_PLL_ENABLE);
> +	if (wait_for(!(I915_READ(LCPLL1_CTL) & (1 << 30)), 1))

LCPLL_PLL_LOCK

> +		DRM_ERROR("Couldn't disable DPLL0\n");
> +
> +	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
> +}
> +
> +void skl_display_resume(struct drm_i915_private *dev_priv)
> +{
> +	u32 val;
> +	unsigned int required_vco;
> +
> +	/* enable PCH reset handshake */
> +	val = I915_READ(HSW_NDE_RSTWRN_OPT);
> +	I915_WRITE(HSW_NDE_RSTWRN_OPT, val & RESET_PCH_HANDSHAKE_ENABLE);

s/&/|/

> +
> +	/* enable PG1 and Misc I/O */
> +	intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
> +
> +	/* DPLL0 already enabed !? */
> +	if (I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE) {
> +		DRM_DEBUG_DRIVER("DPLL0 already running\n");
> +		return;
> +	}
> +
> +	/* enable DPLL0 */
> +	required_vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk);
> +	skl_dpll0_enable(dev_priv, required_vco);
> +
> +	/* set CDCLK to the frequency the BIOS chose */
> +	skl_set_cdclk(dev_priv, dev_priv->skl_boot_cdclk);
> +
> +	/* enable DBUF power */
> +	I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) | DBUF_POWER_REQUEST);
> +	POSTING_READ(DBUF_CTL);
> +
> +	udelay(10);
> +
> +	if (!(I915_READ(DBUF_CTL) & DBUF_POWER_STATE))
> +		DRM_ERROR("DBuf power enable timeout\n");
> +}
> +
>  /* returns HPLL frequency in kHz */
>  static int valleyview_get_vco(struct drm_i915_private *dev_priv)
>  {
> @@ -14573,6 +14780,7 @@ void intel_modeset_init(struct drm_device *dev)
>  
>  	intel_init_dpio(dev);
>  
> +	skl_init_cdclk(dev_priv);

Maybe just stick that into the ddi pll init code?

>  	intel_shared_dpll_init(dev);
>  
>  	/* Just disable it once at startup */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 43fe003..67275a1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1123,6 +1123,8 @@ void broxton_ddi_phy_init(struct drm_device *dev);
>  void broxton_ddi_phy_uninit(struct drm_device *dev);
>  void bxt_enable_dc9(struct drm_i915_private *dev_priv);
>  void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> +void skl_display_suspend(struct drm_i915_private *dev_priv);
> +void skl_display_resume(struct drm_i915_private *dev_priv);
>  void intel_dp_get_m_n(struct intel_crtc *crtc,
>  		      struct intel_crtc_state *pipe_config);
>  void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 6/8] drm/i915/skl: Deinit/init the display at suspend/resume

[Ville Syrjälä]

On Tue, May 05, 2015 at 09:56:02PM +0300, Ville Syrjälä wrote:
> On Thu, Apr 30, 2015 at 04:39:21PM +0100, Damien Lespiau wrote:
> > +static void
> > +skl_dpll0_enable(struct drm_i915_private *dev_priv, unsigned int required_vco)
> > +{
> > +	unsigned int min_freq;
> > +	u32 val;
> > +
> > +	/* select the minimum CDCLK before enabling DPLL 0 */
> > +	val = I915_READ(CDCLK_CTL);
> > +	val &= ~CDCLK_FREQ_SEL_MASK | ~CDCLK_FREQ_DECIMAL_MASK;
> > +	val |= CDCLK_FREQ_337_308;
> > +
> > +	if (required_vco == 8640)
> > +		min_freq = 308570;
> > +	else
> > +		min_freq = 337500;
> > +
> > +	val = CDCLK_FREQ_337_308 | skl_cdlck_decimal(min_freq);
> > +
> > +	I915_WRITE(CDCLK_CTL, val);
> > +	POSTING_READ(CDCLK_CTL);
> > +
> > +	/*
> > +	 * We always enable DPLL0 with the lowest link rate possible, but still
> > +	 * taking into account the VCO required to operate the eDP panel at the
> > +	 * desired frequency. The usual DP link rates operate with a VCO of
> > +	 * 8100 while the eDP 1.4 alternate link rates need a VCO of 8640.
> > +	 * The modeset code is responsible for the selection of the exact link
> > +	 * rate later on, with the constraint of choosing a frequency that
> > +	 * works with required_vco.
> > +	 */
> > +	val = I915_READ(DPLL_CTRL1);
> > +
> > +	val &= ~(DPLL_CTRL1_HDMI_MODE(0) | DPLL_CTRL1_SSC(0) |
> > +		 DPLL_CTRL1_LINK_RATE_MASK(0));
> > +	val |= DPLL_CTRL1_OVERRIDE(0);
> > +	if (required_vco == 8640)
> > +		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080, 0);
> > +	else
> > +		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, 0);
> 
> Hmm. These new pll registers are very confusing. But looks correct
> based on my understanding.

BTW replacing the magic numbers with some kind of enum for the DPLLs
might make this stuff less confusing.

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

Re: [PATCH 6/8] drm/i915/skl: Deinit/init the display at suspend/resume

[Daniel Vetter]

I merged all the preceding patches to dinq. This one starts to conflict,
since dmc code landed meanwhile.

On Thu, Apr 30, 2015 at 04:39:21PM +0100, Damien Lespiau wrote:
> We need to re-init the display hardware when going out of suspend. This
> includes:
> 
>   - Hooking the PCH to the reset logic
>   - Restoring CDCDLK
>   - Enabling the DDB power
> 
> Among those, only the CDCDLK one is a bit tricky. There's some
> complexity in that:
> 
>   - DPLL0 (which is the source for CDCLK) has two VCOs, each with a set
>     of supported frequencies. As eDP also uses DPLL0 for its link rate,
>     once DPLL0 is on, we restrict the possible eDP link rates the chosen
>     VCO.
>   - CDCLK also limits the bandwidth available to push pixels.
>   - My current PCU firmware never ack the frequency change request
> 
> So, as a first step, this commit restore what the BIOS set, until I can
> do more testing.
> 
> ----

Hm, I think this will cause git am to drop everything below for the commit
message, which doesn't look like the intention. At least I'd like to keep
it ;-)
-Daniel

> In case that's of interest for the reviewer, I've unit tested the
> function that derives the decimal frequency field:
> 
>   #include <stdio.h>
>   #include <stdint.h>
>   #include <assert.h>
> 
>   #define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
> 
>   static const struct dpll_freq {
>           unsigned int freq;
>           unsigned int decimal;
>   } freqs[] = {
>           { .freq = 308570, .decimal = 0b01001100111},
>           { .freq = 337500, .decimal = 0b01010100001},
>           { .freq = 432000, .decimal = 0b01101011110},
>           { .freq = 450000, .decimal = 0b01110000010},
>           { .freq = 540000, .decimal = 0b10000110110},
>           { .freq = 617140, .decimal = 0b10011010000},
>           { .freq = 675000, .decimal = 0b10101000100},
>   };
> 
>   static void intbits(unsigned int v)
>   {
>           int i;
> 
>           for(i = 10; i >= 0; i--)
>                   putchar('0' + ((v >> i) & 1));
>   }
> 
>   static unsigned int freq_decimal(unsigned int freq /* in kHz */)
>   {
>           unsigned int mhz, dot5;
> 
>           mhz = freq / 1000;
>           dot5 = (freq - mhz * 1000) >= 500;
> 
>           return (mhz - 1) << 1 | dot5;
>   }
> 
>   static void test_freq(const struct dpll_freq *entry)
>   {
>           unsigned int decimal = freq_decimal(entry->freq);
> 
>           printf("freq: %d, expected: ", entry->freq);
>           intbits(entry->decimal);
>           printf(", got: ");
>           intbits(decimal);
>           putchar('\n');
> 
>           assert(decimal == entry->decimal);
>   }
> 
>   int main(int argc, char **argv)
>   {
>           int i;
> 
>           for (i = 0; i < ARRAY_SIZE(freqs); i++)
>                   test_freq(&freqs[i]);
> 
>           return 0;
>   }
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c      |  26 ++++-
>  drivers/gpu/drm/i915/i915_drv.h      |   1 +
>  drivers/gpu/drm/i915/i915_reg.h      |   3 +
>  drivers/gpu/drm/i915/intel_ddi.c     |   2 +
>  drivers/gpu/drm/i915/intel_display.c | 208 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |   2 +
>  6 files changed, 240 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e70adfd..58db0c6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -574,6 +574,7 @@ static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
>  static int intel_suspend_complete(struct drm_i915_private *dev_priv);
>  static int vlv_resume_prepare(struct drm_i915_private *dev_priv,
>  			      bool rpm_resume);
> +static int skl_resume_prepare(struct drm_i915_private *dev_priv);
>  
>  static int i915_drm_suspend(struct drm_device *dev)
>  {
> @@ -781,6 +782,9 @@ static int i915_drm_resume_early(struct drm_device *dev)
>  
>  	if (IS_VALLEYVIEW(dev_priv))
>  		ret = vlv_resume_prepare(dev_priv, false);
> +	else if (IS_SKYLAKE(dev_priv))
> +		ret = skl_resume_prepare(dev_priv);
> +
>  	if (ret)
>  		DRM_ERROR("Resume prepare failed: %d,Continuing resume\n", ret);
>  
> @@ -1009,6 +1013,20 @@ static int hsw_suspend_complete(struct drm_i915_private *dev_priv)
>  	return 0;
>  }
>  
> +static int skl_suspend_complete(struct drm_i915_private *dev_priv)
> +{
> +	skl_display_suspend(dev_priv);
> +
> +	return 0;
> +}
> +
> +static int skl_resume_prepare(struct drm_i915_private *dev_priv)
> +{
> +	skl_display_resume(dev_priv);
> +
> +	return 0;
> +}
> +
>  static int bxt_suspend_complete(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
> @@ -1500,7 +1518,9 @@ static int intel_runtime_resume(struct device *device)
>  	if (IS_GEN6(dev_priv))
>  		intel_init_pch_refclk(dev);
>  
> -	if (IS_BROXTON(dev))
> +	if (IS_SKYLAKE(dev))
> +		ret = skl_resume_prepare(dev_priv);
> +	else if (IS_BROXTON(dev))
>  		ret = bxt_resume_prepare(dev_priv);
>  	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
>  		hsw_disable_pc8(dev_priv);
> @@ -1534,7 +1554,9 @@ static int intel_suspend_complete(struct drm_i915_private *dev_priv)
>  	struct drm_device *dev = dev_priv->dev;
>  	int ret;
>  
> -	if (IS_BROXTON(dev))
> +	if (IS_SKYLAKE(dev))
> +		ret = skl_suspend_complete(dev_priv);
> +	else if (IS_BROXTON(dev))
>  		ret = bxt_suspend_complete(dev_priv);
>  	else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
>  		ret = hsw_suspend_complete(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 908c124..f637667 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1659,6 +1659,7 @@ struct drm_i915_private {
>  	int num_fence_regs; /* 8 on pre-965, 16 otherwise */
>  
>  	unsigned int fsb_freq, mem_freq, is_ddr3;
> +	unsigned int skl_boot_cdclk;
>  	unsigned int cdclk_freq;
>  	unsigned int hpll_freq;
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 6d428a5..4b063a8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6647,6 +6647,9 @@ enum skl_disp_power_wells {
>  #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
>  #define     GEN9_MEM_LATENCY_LEVEL_2_6_SHIFT	16
>  #define     GEN9_MEM_LATENCY_LEVEL_3_7_SHIFT	24
> +#define   SKL_PCODE_CDCLK_CONTROL		0x7
> +#define     SKL_CDCLK_PREPARE_FOR_CHANGE	0x3
> +#define     SKL_CDCLK_READY_FOR_CHANGE		0x1
>  #define   GEN6_PCODE_WRITE_MIN_FREQ_TABLE	0x8
>  #define   GEN6_PCODE_READ_MIN_FREQ_TABLE	0x9
>  #define   GEN6_READ_OC_PARAMS			0xc
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index d5bee8b..f76bcd3 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2481,6 +2481,8 @@ void intel_ddi_pll_init(struct drm_device *dev)
>  	if (IS_SKYLAKE(dev)) {
>  		if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
>  			DRM_ERROR("LCPLL1 is disabled\n");
> +		else
> +			intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
>  	} else if (IS_BROXTON(dev)) {
>  		broxton_init_cdclk(dev);
>  		broxton_ddi_phy_init(dev);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f2f4ad5..9c8338f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5420,6 +5420,213 @@ void broxton_uninit_cdclk(struct drm_device *dev)
>  	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
>  }
>  
> +/*
> + * For now, we store the CDCLK frequency set by the BIOS and restore it at
> + * resume.
> + */
> +static void skl_init_cdclk(struct drm_i915_private *dev_priv)
> +{
> +	if (!IS_SKYLAKE(dev_priv))
> +		return;
> +
> +	dev_priv->skl_boot_cdclk =
> +		dev_priv->display.get_display_clock_speed(dev_priv->dev);
> +
> +	DRM_DEBUG_DRIVER("CDCLK: %d kHz\n", dev_priv->skl_boot_cdclk);
> +}
> +
> +static const struct skl_cdclk_entry {
> +	unsigned int freq;
> +	unsigned int vco;
> +} skl_cdclk_frequencies[] = {
> +	{ .freq = 308570, .vco = 8640 },
> +	{ .freq = 337500, .vco = 8100 },
> +	{ .freq = 432000, .vco = 8640 },
> +	{ .freq = 450000, .vco = 8100 },
> +	{ .freq = 540000, .vco = 8100 },
> +	{ .freq = 617140, .vco = 8640 },
> +	{ .freq = 675000, .vco = 8100 },
> +};
> +
> +static unsigned int skl_cdclk_get_vco(unsigned int freq)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(skl_cdclk_frequencies); i++) {
> +		const struct skl_cdclk_entry *e = &skl_cdclk_frequencies[i];
> +
> +		if (e->freq == freq)
> +			return e->vco;
> +	}
> +
> +	return 8100;
> +}
> +
> +static unsigned int skl_cdlck_decimal(unsigned int freq)
> +{
> +        unsigned int mhz, dot5;
> +
> +        mhz = freq / 1000;
> +        dot5 = (freq - mhz * 1000) >= 500;
> +
> +        return (mhz - 1) << 1 | dot5;
> +}
> +
> +static void
> +skl_dpll0_enable(struct drm_i915_private *dev_priv, unsigned int required_vco)
> +{
> +	unsigned int min_freq;
> +	u32 val;
> +
> +	/* select the minimum CDCLK before enabling DPLL 0 */
> +	val = I915_READ(CDCLK_CTL);
> +	val &= ~CDCLK_FREQ_SEL_MASK | ~CDCLK_FREQ_DECIMAL_MASK;
> +	val |= CDCLK_FREQ_337_308;
> +
> +	if (required_vco == 8640)
> +		min_freq = 308570;
> +	else
> +		min_freq = 337500;
> +
> +	val = CDCLK_FREQ_337_308 | skl_cdlck_decimal(min_freq);
> +
> +	I915_WRITE(CDCLK_CTL, val);
> +	POSTING_READ(CDCLK_CTL);
> +
> +	/*
> +	 * We always enable DPLL0 with the lowest link rate possible, but still
> +	 * taking into account the VCO required to operate the eDP panel at the
> +	 * desired frequency. The usual DP link rates operate with a VCO of
> +	 * 8100 while the eDP 1.4 alternate link rates need a VCO of 8640.
> +	 * The modeset code is responsible for the selection of the exact link
> +	 * rate later on, with the constraint of choosing a frequency that
> +	 * works with required_vco.
> +	 */
> +	val = I915_READ(DPLL_CTRL1);
> +
> +	val &= ~(DPLL_CTRL1_HDMI_MODE(0) | DPLL_CTRL1_SSC(0) |
> +		 DPLL_CTRL1_LINK_RATE_MASK(0));
> +	val |= DPLL_CTRL1_OVERRIDE(0);
> +	if (required_vco == 8640)
> +		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080, 0);
> +	else
> +		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, 0);
> +
> +	I915_WRITE(DPLL_CTRL1, val);
> +	POSTING_READ(DPLL_CTRL1);
> +
> +	I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) | LCPLL_PLL_ENABLE);
> +
> +	if (wait_for(I915_READ(LCPLL1_CTL) & (1 << 30), 5))
> +		DRM_ERROR("DPLL0 not locked\n");
> +}
> +
> +static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
> +{
> +	int ret;
> +	u32 val, freq_select, freq_decimal, pcu_ack;
> +
> +	DRM_DEBUG_DRIVER("Changing CDCLK to %dKHz\n", freq);
> +
> +	/* inform PCU we want to change CDCLK */
> +	val = SKL_CDCLK_PREPARE_FOR_CHANGE;
> +	mutex_lock(&dev_priv->rps.hw_lock);
> +	ret = sandybridge_pcode_read(dev_priv, SKL_PCODE_CDCLK_CONTROL, &val);
> +	mutex_unlock(&dev_priv->rps.hw_lock);
> +	if (ret || !(val & SKL_CDCLK_READY_FOR_CHANGE)) {
> +		DRM_ERROR("failed to inform PCU about cdclk change\n");
> +		return;
> +	}
> +
> +	/* set CDCLK_CTL */
> +	switch(freq) {
> +	case 450000:
> +	case 432000:
> +		freq_select = CDCLK_FREQ_450_432;
> +		pcu_ack = 1;
> +		break;
> +	case 540000:
> +		freq_select = CDCLK_FREQ_540;
> +		pcu_ack = 2;
> +		break;
> +	case 308570:
> +	case 337500:
> +	default:
> +		freq_select = CDCLK_FREQ_337_308;
> +		pcu_ack = 0;
> +		break;
> +	case 617140:
> +	case 675000:
> +		freq_select = CDCLK_FREQ_675_617;
> +		pcu_ack = 3;
> +		break;
> +	}
> +
> +	freq_decimal = skl_cdlck_decimal(freq);
> +
> +	I915_WRITE(CDCLK_CTL, freq_select | freq_decimal);
> +	POSTING_READ(CDCLK_CTL);
> +
> +	/* inform PCU of the change */
> +	mutex_lock(&dev_priv->rps.hw_lock);
> +	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, pcu_ack);
> +	mutex_unlock(&dev_priv->rps.hw_lock);
> +}
> +
> +void skl_display_suspend(struct drm_i915_private *dev_priv)
> +{
> +	/* disable DBUF power */
> +	I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) & ~DBUF_POWER_REQUEST);
> +	POSTING_READ(DBUF_CTL);
> +
> +	udelay(10);
> +
> +	if (I915_READ(DBUF_CTL) & DBUF_POWER_STATE)
> +		DRM_ERROR("DBuf power disable timeout\n");
> +
> +	/* disable DPLL0 */
> +	I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) & ~LCPLL_PLL_ENABLE);
> +	if (wait_for(!(I915_READ(LCPLL1_CTL) & (1 << 30)), 1))
> +		DRM_ERROR("Couldn't disable DPLL0\n");
> +
> +	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
> +}
> +
> +void skl_display_resume(struct drm_i915_private *dev_priv)
> +{
> +	u32 val;
> +	unsigned int required_vco;
> +
> +	/* enable PCH reset handshake */
> +	val = I915_READ(HSW_NDE_RSTWRN_OPT);
> +	I915_WRITE(HSW_NDE_RSTWRN_OPT, val & RESET_PCH_HANDSHAKE_ENABLE);
> +
> +	/* enable PG1 and Misc I/O */
> +	intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
> +
> +	/* DPLL0 already enabed !? */
> +	if (I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE) {
> +		DRM_DEBUG_DRIVER("DPLL0 already running\n");
> +		return;
> +	}
> +
> +	/* enable DPLL0 */
> +	required_vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk);
> +	skl_dpll0_enable(dev_priv, required_vco);
> +
> +	/* set CDCLK to the frequency the BIOS chose */
> +	skl_set_cdclk(dev_priv, dev_priv->skl_boot_cdclk);
> +
> +	/* enable DBUF power */
> +	I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) | DBUF_POWER_REQUEST);
> +	POSTING_READ(DBUF_CTL);
> +
> +	udelay(10);
> +
> +	if (!(I915_READ(DBUF_CTL) & DBUF_POWER_STATE))
> +		DRM_ERROR("DBuf power enable timeout\n");
> +}
> +
>  /* returns HPLL frequency in kHz */
>  static int valleyview_get_vco(struct drm_i915_private *dev_priv)
>  {
> @@ -14573,6 +14780,7 @@ void intel_modeset_init(struct drm_device *dev)
>  
>  	intel_init_dpio(dev);
>  
> +	skl_init_cdclk(dev_priv);
>  	intel_shared_dpll_init(dev);
>  
>  	/* Just disable it once at startup */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 43fe003..67275a1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1123,6 +1123,8 @@ void broxton_ddi_phy_init(struct drm_device *dev);
>  void broxton_ddi_phy_uninit(struct drm_device *dev);
>  void bxt_enable_dc9(struct drm_i915_private *dev_priv);
>  void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> +void skl_display_suspend(struct drm_i915_private *dev_priv);
> +void skl_display_resume(struct drm_i915_private *dev_priv);
>  void intel_dp_get_m_n(struct intel_crtc *crtc,
>  		      struct intel_crtc_state *pipe_config);
>  void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 7/8] drm/i915/skl: Change CDCLK behind PCU's back

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9c8338f..2cbae9a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5525,6 +5525,7 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
 {
 	int ret;
 	u32 val, freq_select, freq_decimal, pcu_ack;
+	bool do_pcu_ack = true;
 
 	DRM_DEBUG_DRIVER("Changing CDCLK to %dKHz\n", freq);
 
@@ -5534,8 +5535,8 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
 	ret = sandybridge_pcode_read(dev_priv, SKL_PCODE_CDCLK_CONTROL, &val);
 	mutex_unlock(&dev_priv->rps.hw_lock);
 	if (ret || !(val & SKL_CDCLK_READY_FOR_CHANGE)) {
-		DRM_ERROR("failed to inform PCU about cdclk change\n");
-		return;
+		DRM_DEBUG_KMS("failed to inform PCU about cdclk change\n");
+		do_pcu_ack = false;
 	}
 
 	/* set CDCLK_CTL */
@@ -5567,6 +5568,9 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
 	I915_WRITE(CDCLK_CTL, freq_select | freq_decimal);
 	POSTING_READ(CDCLK_CTL);
 
+	if (!do_pcu_ack)
+		return;
+
 	/* inform PCU of the change */
 	mutex_lock(&dev_priv->rps.hw_lock);
 	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, pcu_ack);
-- 
2.1.0

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

Re: [PATCH 7/8] drm/i915/skl: Change CDCLK behind PCU's back

[Daniel Vetter]

On Thu, Apr 30, 2015 at 04:39:22PM +0100, Damien Lespiau wrote:
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

Why? I.e. way too terse commit message.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9c8338f..2cbae9a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5525,6 +5525,7 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
>  {
>  	int ret;
>  	u32 val, freq_select, freq_decimal, pcu_ack;
> +	bool do_pcu_ack = true;
>  
>  	DRM_DEBUG_DRIVER("Changing CDCLK to %dKHz\n", freq);
>  
> @@ -5534,8 +5535,8 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
>  	ret = sandybridge_pcode_read(dev_priv, SKL_PCODE_CDCLK_CONTROL, &val);
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  	if (ret || !(val & SKL_CDCLK_READY_FOR_CHANGE)) {
> -		DRM_ERROR("failed to inform PCU about cdclk change\n");
> -		return;
> +		DRM_DEBUG_KMS("failed to inform PCU about cdclk change\n");
> +		do_pcu_ack = false;
>  	}
>  
>  	/* set CDCLK_CTL */
> @@ -5567,6 +5568,9 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
>  	I915_WRITE(CDCLK_CTL, freq_select | freq_decimal);
>  	POSTING_READ(CDCLK_CTL);
>  
> +	if (!do_pcu_ack)
> +		return;
> +
>  	/* inform PCU of the change */
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, pcu_ack);
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/8] drm/i915/skl: Change CDCLK behind PCU's back

[Damien Lespiau]

On Wed, May 06, 2015 at 12:53:14PM +0200, Daniel Vetter wrote:
> On Thu, Apr 30, 2015 at 04:39:22PM +0100, Damien Lespiau wrote:
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> 
> Why? I.e. way too terse commit message.

Because I couldn't get the PCU to answer to the handcheck we should be
doing. Ville noticed (and, to be fair, I knew this as well but didn't
have time to investigate before an early week-end) that I'm not exactly
doing what's documented and we need to wait in a busy loop for the PCU
ack. Will try that when coming back at it.

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

[PATCH 8/8] drm/i915/skl: gen6+ platforms support runtime PM

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f637667..25618fd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2422,8 +2422,7 @@ struct drm_i915_cmd_table {
 #define HAS_PSR(dev)		(IS_HASWELL(dev) || IS_BROADWELL(dev) || \
 				 IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
 				 IS_SKYLAKE(dev))
-#define HAS_RUNTIME_PM(dev)	(IS_GEN6(dev) || IS_HASWELL(dev) || \
-				 IS_BROADWELL(dev) || IS_VALLEYVIEW(dev))
+#define HAS_RUNTIME_PM(dev)	(INTEL_INFO(dev)->gen >= 6)
 #define HAS_RC6(dev)		(INTEL_INFO(dev)->gen >= 6)
 #define HAS_RC6p(dev)		(INTEL_INFO(dev)->gen == 6 || IS_IVYBRIDGE(dev))
 
-- 
2.1.0

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

Re: [PATCH 8/8] drm/i915/skl: gen6+ platforms support runtime PM

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6301
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  302/302              302/302
SNB                                  316/316              316/316
IVB                                  264/264              264/264
BYT                 -3              227/227              224/227
BDW                                  318/318              318/318
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*BYT  igt@gem_dummy_reloc_loop@render      FAIL(1)PASS(18)      TIMEOUT(1)PASS(1)
 BYT  igt@gem_pipe_control_store_loop@fresh-buffer      FAIL(1)TIMEOUT(10)PASS(9)      TIMEOUT(1)PASS(1)
*BYT  igt@gem_pwrite_pread@uncached-copy-performance      FAIL(1)PASS(2)      DMESG_FAIL(1)PASS(1)
(dmesg patch applied)drm:i915_context_is_banned[i915]]*ERROR*gpu_hanging_too_fast,banning@gpu hanging too
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

Re: [PATCH 8/8] drm/i915/skl: gen6+ platforms support runtime PM

[Ville Syrjälä]

On Thu, Apr 30, 2015 at 04:39:23PM +0100, Damien Lespiau wrote:
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f637667..25618fd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2422,8 +2422,7 @@ struct drm_i915_cmd_table {
>  #define HAS_PSR(dev)		(IS_HASWELL(dev) || IS_BROADWELL(dev) || \
>  				 IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
>  				 IS_SKYLAKE(dev))
> -#define HAS_RUNTIME_PM(dev)	(IS_GEN6(dev) || IS_HASWELL(dev) || \
> -				 IS_BROADWELL(dev) || IS_VALLEYVIEW(dev))
> +#define HAS_RUNTIME_PM(dev)	(INTEL_INFO(dev)->gen >= 6)

I really wanted to cook up some kind of tool to actually verify we don't
lose some important registers due to D3/S0ix on each platform, but doesn't
look like I'll find enough time for that anytime soon, so might as well
flip the switch and hope for the best. IVB,SKL and BXT are the only ones
missing in any case.

I won't pretend that I've reviewed this as that would require me to cook
up that tool, but I can toss in an ack.

Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  #define HAS_RC6(dev)		(INTEL_INFO(dev)->gen >= 6)
>  #define HAS_RC6p(dev)		(INTEL_INFO(dev)->gen == 6 || IS_IVYBRIDGE(dev))
>  
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 8/8] drm/i915/skl: gen6+ platforms support runtime PM

[Daniel Vetter]

On Thu, Apr 30, 2015 at 04:39:23PM +0100, Damien Lespiau wrote:
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f637667..25618fd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2422,8 +2422,7 @@ struct drm_i915_cmd_table {
>  #define HAS_PSR(dev)		(IS_HASWELL(dev) || IS_BROADWELL(dev) || \
>  				 IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
>  				 IS_SKYLAKE(dev))
> -#define HAS_RUNTIME_PM(dev)	(IS_GEN6(dev) || IS_HASWELL(dev) || \
> -				 IS_BROADWELL(dev) || IS_VALLEYVIEW(dev))
> +#define HAS_RUNTIME_PM(dev)	(INTEL_INFO(dev)->gen >= 6)
>  #define HAS_RC6(dev)		(INTEL_INFO(dev)->gen >= 6)
>  #define HAS_RC6p(dev)		(INTEL_INFO(dev)->gen == 6 || IS_IVYBRIDGE(dev))

iirc ilk fdi lane sharing won't get restored correctly, at least that was
the case way back. Have you double-checked that that still works on an
ivb? Ander has a testcase and setup for it ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/skl: gen6+ platforms support runtime PM

[Damien Lespiau]

On Wed, May 06, 2015 at 12:54:23PM +0200, Daniel Vetter wrote:
> On Thu, Apr 30, 2015 at 04:39:23PM +0100, Damien Lespiau wrote:
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index f637667..25618fd 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2422,8 +2422,7 @@ struct drm_i915_cmd_table {
> >  #define HAS_PSR(dev)		(IS_HASWELL(dev) || IS_BROADWELL(dev) || \
> >  				 IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
> >  				 IS_SKYLAKE(dev))
> > -#define HAS_RUNTIME_PM(dev)	(IS_GEN6(dev) || IS_HASWELL(dev) || \
> > -				 IS_BROADWELL(dev) || IS_VALLEYVIEW(dev))
> > +#define HAS_RUNTIME_PM(dev)	(INTEL_INFO(dev)->gen >= 6)
> >  #define HAS_RC6(dev)		(INTEL_INFO(dev)->gen >= 6)
> >  #define HAS_RC6p(dev)		(INTEL_INFO(dev)->gen == 6 || IS_IVYBRIDGE(dev))
> 
> iirc ilk fdi lane sharing won't get restored correctly, at least that was
> the case way back. Have you double-checked that that still works on an
> ivb? Ander has a testcase and setup for it ...

It wasn't my intention at all to change the behaviour here, I just
forgot about IVB...

As it was just to reduce the number of conditions, might as well make it
a device info flag.

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