[PATCH 0/5] SKL S3, take 2.5

[PATCH 0/5] SKL S3, take 2.5

[Damien Lespiau]

I fell into a well crafted trap in the previous series, breaking a resume
ordering constraint for VLV/CHV. Might as well split the series in two then as
we really want the SKL S3 stuff in sooner rather than later.

-- 
Damien

Damien Lespiau (5):
  drm/i915/bxt: Also add bxt_resume_prepare() to the S3/S4 path
  drm/i915: Add a space after ',' and don't capitalize mid-sentence
  drm/i915: Kill the dev variable in intel_suspend_complete()
  drm/i915: Store boot CDCLK in dev_priv on DDI platforms
  drm/i915/skl: Deinit/init the display at suspend/resume

 drivers/gpu/drm/i915/i915_drv.c      |  22 ++--
 drivers/gpu/drm/i915/i915_reg.h      |   3 +
 drivers/gpu/drm/i915/intel_ddi.c     |   6 +-
 drivers/gpu/drm/i915/intel_display.c | 210 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |   2 +
 5 files changed, 233 insertions(+), 10 deletions(-)

-- 
2.1.0

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

[PATCH 1/5] drm/i915/bxt: Also add bxt_resume_prepare() to the S3/S4 path

[Damien Lespiau]

Currently bxt_resume_prepare() is only used in the runtime-resume path.
Add it to the full S3/S4 path as well.

v2: Rebase on top of the vlv_resume_prepare() shuffling around

Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com> (v1)
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5cc57f2..77f9818 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -595,6 +595,7 @@ 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 bxt_resume_prepare(struct drm_i915_private *dev_priv);
 
 
 static int i915_drm_suspend(struct drm_device *dev)
@@ -815,10 +816,12 @@ static int i915_drm_resume_early(struct drm_device *dev)
 
 	intel_uncore_early_sanitize(dev, true);
 
-	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-		hsw_disable_pc8(dev_priv);
+	if (IS_BROXTON(dev))
+		ret = bxt_resume_prepare(dev_priv);
 	else if (IS_SKYLAKE(dev_priv))
 		ret = skl_resume_prepare(dev_priv);
+	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+		hsw_disable_pc8(dev_priv);
 
 	intel_uncore_sanitize(dev);
 	intel_power_domains_init_hw(dev_priv);
-- 
2.1.0

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

[PATCH 2/5] drm/i915: Add a space after ', ' and don't capitalize mid-sentence

[Damien Lespiau]

Couldn't let it go!

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

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 77f9818..442dd6c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -812,7 +812,8 @@ static int i915_drm_resume_early(struct drm_device *dev)
 	if (IS_VALLEYVIEW(dev_priv))
 		ret = vlv_resume_prepare(dev_priv, false);
 	if (ret)
-		DRM_ERROR("Resume prepare failed: %d,Continuing resume\n", ret);
+		DRM_ERROR("Resume prepare failed: %d, continuing anyway\n",
+			  ret);
 
 	intel_uncore_early_sanitize(dev, true);
 
-- 
2.1.0

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

[PATCH 3/5] drm/i915: Kill the dev variable in intel_suspend_complete()

[Damien Lespiau]

The macros we use there are the magic ones that can take either dev or
dev_priv. We'd like to move as much as possible towards dev_priv though.

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

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 442dd6c..93191c1 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1590,16 +1590,15 @@ static int intel_runtime_resume(struct device *device)
  */
 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_BROXTON(dev_priv))
 		ret = bxt_suspend_complete(dev_priv);
-	else if (IS_SKYLAKE(dev))
+	else if (IS_SKYLAKE(dev_priv))
 		ret = skl_suspend_complete(dev_priv);
-	else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
+	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
 		ret = hsw_suspend_complete(dev_priv);
-	else if (IS_VALLEYVIEW(dev))
+	else if (IS_VALLEYVIEW(dev_priv))
 		ret = vlv_suspend_complete(dev_priv);
 	else
 		ret = 0;
-- 
2.1.0

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

[PATCH 4/5] drm/i915: Store boot CDCLK in dev_priv on DDI platforms

[Damien Lespiau]

Right not we don't initialize the stored CDCLK on DDI platforms. Fix
that.

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

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index d602db2..b57f156 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2518,8 +2518,8 @@ void intel_ddi_pll_init(struct drm_device *dev)
 	else
 		hsw_shared_dplls_init(dev_priv);
 
-	DRM_DEBUG_KMS("CDCLK running at %dKHz\n",
-		      dev_priv->display.get_display_clock_speed(dev));
+	dev_priv->cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
+	DRM_DEBUG_KMS("CDCLK running at %dKHz\n", dev_priv->cdclk_freq);
 
 	if (IS_SKYLAKE(dev)) {
 		if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
-- 
2.1.0

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

[PATCH 5/5] 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.

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 */)
  {
          return (freq - 1000) / 500;
  }

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

v2:
  - Rebase on top of -nightly
  - Use (freq - 1000) / 500 for the decimal frequency (Ville)
  - Fix setting the enable bit of HSW_NDE_RSTWRN_OPT (Ville)
  - Rename skl_display_{resume,suspend} to skl_{init,uninit}_cdclk to
    be consistent with the BXT code (Ville)
  - Store boot CDCLK in ddi_pll_init (Ville)
  - Merge dev_priv's skl_boot_cdclk into cdclk_freq
  - Use LCPLL_PLL_LOCK instead of (1 << 30) (Ville)
  - Replace various '0' by SKL_DPLL0 to be a bit more explicit that
    we're programming DPLL0
  - Busy poll the PCU before doing the frequency change. It takes about
    3/4 cycles, each separated by 10us, to get the ACK from the CPU
    (Ville)

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

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 93191c1..884b4f9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1047,6 +1047,8 @@ static int skl_suspend_complete(struct drm_i915_private *dev_priv)
 	 */
 	intel_csr_load_status_set(dev_priv, FW_UNINITIALIZED);
 
+	skl_uninit_cdclk(dev_priv);
+
 	return 0;
 }
 
@@ -1093,6 +1095,7 @@ static int skl_resume_prepare(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 
+	skl_init_cdclk(dev_priv);
 	intel_csr_load_program(dev);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 84af255..4df5cdd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6698,6 +6698,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 b57f156..bc6b899 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2524,6 +2524,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 c97b496..1f2f4f9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5527,6 +5527,216 @@ void broxton_uninit_cdclk(struct drm_device *dev)
 	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
 }
 
+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_decimal(unsigned int freq)
+{
+	return (freq - 1000) / 500;
+}
+
+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 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_cdclk_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(SKL_DPLL0) | DPLL_CTRL1_SSC(SKL_DPLL0) |
+		 DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0));
+	val |= DPLL_CTRL1_OVERRIDE(SKL_DPLL0);
+	if (required_vco == 8640)
+		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080,
+					    SKL_DPLL0);
+	else
+		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810,
+					    SKL_DPLL0);
+
+	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) & LCPLL_PLL_LOCK, 5))
+		DRM_ERROR("DPLL0 not locked\n");
+}
+
+static bool skl_cdclk_pcu_ready(struct drm_i915_private *dev_priv)
+{
+	int ret;
+	u32 val;
+
+	/* 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);
+
+	return ret == 0 && (val & SKL_CDCLK_READY_FOR_CHANGE);
+}
+
+static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv)
+{
+	unsigned int i;
+
+	for (i = 0; i < 15; i++) {
+		if (skl_cdclk_pcu_ready(dev_priv))
+			return true;
+		udelay(10);
+	}
+
+	return false;
+}
+
+static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
+{
+	u32 freq_select, pcu_ack;
+
+	DRM_DEBUG_DRIVER("Changing CDCLK to %dKHz\n", freq);
+
+	if (!skl_cdclk_wait_for_pcu_ready(dev_priv)) {
+		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;
+	}
+
+	I915_WRITE(CDCLK_CTL, freq_select | skl_cdclk_decimal(freq));
+	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);
+
+	dev_priv->cdclk_freq = freq;
+}
+
+void skl_uninit_cdclk(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) & LCPLL_PLL_LOCK), 1))
+		DRM_ERROR("Couldn't disable DPLL0\n");
+
+	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
+}
+
+void skl_init_cdclk(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->cdclk_freq);
+	skl_dpll0_enable(dev_priv, required_vco);
+
+	/* set CDCLK to the frequency the BIOS chose */
+	skl_set_cdclk(dev_priv, dev_priv->cdclk_freq);
+
+	/* 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)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 47bc729..4640fb6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1117,6 +1117,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_init_cdclk(struct drm_i915_private *dev_priv);
+void skl_uninit_cdclk(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 4/5] drm/i915: Store boot CDCLK in dev_priv on DDI platforms

[Daniel Vetter]

On Wed, May 20, 2015 at 02:45:17PM +0100, Damien Lespiau wrote:
> Right not we don't initialize the stored CDCLK on DDI platforms. Fix
> that.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

Merged up to this patch, thanks, Daniel

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index d602db2..b57f156 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2518,8 +2518,8 @@ void intel_ddi_pll_init(struct drm_device *dev)
>  	else
>  		hsw_shared_dplls_init(dev_priv);
>  
> -	DRM_DEBUG_KMS("CDCLK running at %dKHz\n",
> -		      dev_priv->display.get_display_clock_speed(dev));
> +	dev_priv->cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
> +	DRM_DEBUG_KMS("CDCLK running at %dKHz\n", dev_priv->cdclk_freq);
>  
>  	if (IS_SKYLAKE(dev)) {
>  		if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
> -- 
> 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 4/5] drm/i915: Store boot CDCLK in dev_priv on DDI platforms

[Daniel Vetter]

On Wed, May 20, 2015 at 05:53:42PM +0200, Daniel Vetter wrote:
> On Wed, May 20, 2015 at 02:45:17PM +0100, Damien Lespiau wrote:
> > Right not we don't initialize the stored CDCLK on DDI platforms. Fix
> > that.
> > 
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> 
> Merged up to this patch, thanks, Daniel

I'll take that back, our usage of dev_priv->cdclk_freq seems rather
inconsistent and there's patches in-flight to completely rework all this.
Hence I dropped this patch here again.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index d602db2..b57f156 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2518,8 +2518,8 @@ void intel_ddi_pll_init(struct drm_device *dev)
> >  	else
> >  		hsw_shared_dplls_init(dev_priv);
> >  
> > -	DRM_DEBUG_KMS("CDCLK running at %dKHz\n",
> > -		      dev_priv->display.get_display_clock_speed(dev));
> > +	dev_priv->cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
> > +	DRM_DEBUG_KMS("CDCLK running at %dKHz\n", dev_priv->cdclk_freq);
> >  
> >  	if (IS_SKYLAKE(dev)) {
> >  		if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
> > -- 
> > 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

-- 
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 4/5] drm/i915: Store boot CDCLK in dev_priv on DDI platforms

[Ville Syrjälä]

On Wed, May 20, 2015 at 05:56:30PM +0200, Daniel Vetter wrote:
> On Wed, May 20, 2015 at 05:53:42PM +0200, Daniel Vetter wrote:
> > On Wed, May 20, 2015 at 02:45:17PM +0100, Damien Lespiau wrote:
> > > Right not we don't initialize the stored CDCLK on DDI platforms. Fix
> > > that.
> > > 
> > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > 
> > Merged up to this patch, thanks, Daniel
> 
> I'll take that back, our usage of dev_priv->cdclk_freq seems rather
> inconsistent and there's patches in-flight to completely rework all this.
> Hence I dropped this patch here again.

dev_priv->cdclk_freq is going to be the current cdclk frequency. So if
we actually want to hang on to the boot cdclk it needs to be stored
separately.

> -Daniel
> 
> > 
> > > ---
> > >  drivers/gpu/drm/i915/intel_ddi.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > > index d602db2..b57f156 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -2518,8 +2518,8 @@ void intel_ddi_pll_init(struct drm_device *dev)
> > >  	else
> > >  		hsw_shared_dplls_init(dev_priv);
> > >  
> > > -	DRM_DEBUG_KMS("CDCLK running at %dKHz\n",
> > > -		      dev_priv->display.get_display_clock_speed(dev));
> > > +	dev_priv->cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
> > > +	DRM_DEBUG_KMS("CDCLK running at %dKHz\n", dev_priv->cdclk_freq);
> > >  
> > >  	if (IS_SKYLAKE(dev)) {
> > >  		if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
> > > -- 
> > > 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
> 
> -- 
> 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

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

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

[Ville Syrjälä]

On Wed, May 20, 2015 at 02:45:18PM +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.
> 
> 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 */)
>   {
>           return (freq - 1000) / 500;
>   }
> 
>   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;
>   }
> 
> v2:
>   - Rebase on top of -nightly
>   - Use (freq - 1000) / 500 for the decimal frequency (Ville)
>   - Fix setting the enable bit of HSW_NDE_RSTWRN_OPT (Ville)
>   - Rename skl_display_{resume,suspend} to skl_{init,uninit}_cdclk to
>     be consistent with the BXT code (Ville)
>   - Store boot CDCLK in ddi_pll_init (Ville)
>   - Merge dev_priv's skl_boot_cdclk into cdclk_freq
>   - Use LCPLL_PLL_LOCK instead of (1 << 30) (Ville)
>   - Replace various '0' by SKL_DPLL0 to be a bit more explicit that
>     we're programming DPLL0
>   - Busy poll the PCU before doing the frequency change. It takes about
>     3/4 cycles, each separated by 10us, to get the ACK from the CPU
>     (Ville)
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

Looks like all of my complaints got addressed. So the boot cdclk thing
is still a bit open, but apart from that this gets:

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

> ---
>  drivers/gpu/drm/i915/i915_drv.c      |   3 +
>  drivers/gpu/drm/i915/i915_reg.h      |   3 +
>  drivers/gpu/drm/i915/intel_ddi.c     |   2 +
>  drivers/gpu/drm/i915/intel_display.c | 210 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |   2 +
>  5 files changed, 220 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 93191c1..884b4f9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1047,6 +1047,8 @@ static int skl_suspend_complete(struct drm_i915_private *dev_priv)
>  	 */
>  	intel_csr_load_status_set(dev_priv, FW_UNINITIALIZED);
>  
> +	skl_uninit_cdclk(dev_priv);
> +
>  	return 0;
>  }
>  
> @@ -1093,6 +1095,7 @@ static int skl_resume_prepare(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
>  
> +	skl_init_cdclk(dev_priv);
>  	intel_csr_load_program(dev);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 84af255..4df5cdd 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6698,6 +6698,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 b57f156..bc6b899 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2524,6 +2524,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 c97b496..1f2f4f9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5527,6 +5527,216 @@ void broxton_uninit_cdclk(struct drm_device *dev)
>  	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
>  }
>  
> +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_decimal(unsigned int freq)
> +{
> +	return (freq - 1000) / 500;
> +}
> +
> +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 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_cdclk_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(SKL_DPLL0) | DPLL_CTRL1_SSC(SKL_DPLL0) |
> +		 DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0));
> +	val |= DPLL_CTRL1_OVERRIDE(SKL_DPLL0);
> +	if (required_vco == 8640)
> +		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080,
> +					    SKL_DPLL0);
> +	else
> +		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810,
> +					    SKL_DPLL0);
> +
> +	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) & LCPLL_PLL_LOCK, 5))
> +		DRM_ERROR("DPLL0 not locked\n");
> +}
> +
> +static bool skl_cdclk_pcu_ready(struct drm_i915_private *dev_priv)
> +{
> +	int ret;
> +	u32 val;
> +
> +	/* 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);
> +
> +	return ret == 0 && (val & SKL_CDCLK_READY_FOR_CHANGE);
> +}
> +
> +static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < 15; i++) {
> +		if (skl_cdclk_pcu_ready(dev_priv))
> +			return true;
> +		udelay(10);
> +	}
> +
> +	return false;
> +}
> +
> +static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
> +{
> +	u32 freq_select, pcu_ack;
> +
> +	DRM_DEBUG_DRIVER("Changing CDCLK to %dKHz\n", freq);
> +
> +	if (!skl_cdclk_wait_for_pcu_ready(dev_priv)) {
> +		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;
> +	}
> +
> +	I915_WRITE(CDCLK_CTL, freq_select | skl_cdclk_decimal(freq));
> +	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);
> +
> +	dev_priv->cdclk_freq = freq;
> +}
> +
> +void skl_uninit_cdclk(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) & LCPLL_PLL_LOCK), 1))
> +		DRM_ERROR("Couldn't disable DPLL0\n");
> +
> +	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
> +}
> +
> +void skl_init_cdclk(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->cdclk_freq);
> +	skl_dpll0_enable(dev_priv, required_vco);
> +
> +	/* set CDCLK to the frequency the BIOS chose */
> +	skl_set_cdclk(dev_priv, dev_priv->cdclk_freq);
> +
> +	/* 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)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 47bc729..4640fb6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1117,6 +1117,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_init_cdclk(struct drm_i915_private *dev_priv);
> +void skl_uninit_cdclk(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 3/5] drm/i915: Kill the dev variable in intel_suspend_complete()

[Ville Syrjälä]

On Wed, May 20, 2015 at 02:45:16PM +0100, Damien Lespiau wrote:
> The macros we use there are the magic ones that can take either dev or
> dev_priv. We'd like to move as much as possible towards dev_priv though.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

Patches 2 and 3 look fine to me.

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

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 442dd6c..93191c1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1590,16 +1590,15 @@ static int intel_runtime_resume(struct device *device)
>   */
>  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_BROXTON(dev_priv))
>  		ret = bxt_suspend_complete(dev_priv);
> -	else if (IS_SKYLAKE(dev))
> +	else if (IS_SKYLAKE(dev_priv))
>  		ret = skl_suspend_complete(dev_priv);
> -	else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> +	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
>  		ret = hsw_suspend_complete(dev_priv);
> -	else if (IS_VALLEYVIEW(dev))
> +	else if (IS_VALLEYVIEW(dev_priv))
>  		ret = vlv_suspend_complete(dev_priv);
>  	else
>  		ret = 0;
> -- 
> 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 v3] 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.

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 */)
  {
          return (freq - 1000) / 500;
  }

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

v2:
  - Rebase on top of -nightly
  - Use (freq - 1000) / 500 for the decimal frequency (Ville)
  - Fix setting the enable bit of HSW_NDE_RSTWRN_OPT (Ville)
  - Rename skl_display_{resume,suspend} to skl_{init,uninit}_cdclk to
    be consistent with the BXT code (Ville)
  - Store boot CDCLK in ddi_pll_init (Ville)
  - Merge dev_priv's skl_boot_cdclk into cdclk_freq
  - Use LCPLL_PLL_LOCK instead of (1 << 30) (Ville)
  - Replace various '0' by SKL_DPLL0 to be a bit more explicit that
    we're programming DPLL0
  - Busy poll the PCU before doing the frequency change. It takes about
    3/4 cycles, each separated by 10us, to get the ACK from the CPU
    (Ville)

v3:
  - Restore dev_priv->skl_boot_cdclk, leaving unification with
    dev_priv->cdclk_freq for a later patch (Daniel, Ville)

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      |   3 +
 drivers/gpu/drm/i915/i915_drv.h      |   1 +
 drivers/gpu/drm/i915/i915_reg.h      |   3 +
 drivers/gpu/drm/i915/intel_ddi.c     |   8 +-
 drivers/gpu/drm/i915/intel_display.c | 208 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |   2 +
 6 files changed, 223 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 93191c1..884b4f9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1047,6 +1047,8 @@ static int skl_suspend_complete(struct drm_i915_private *dev_priv)
 	 */
 	intel_csr_load_status_set(dev_priv, FW_UNINITIALIZED);
 
+	skl_uninit_cdclk(dev_priv);
+
 	return 0;
 }
 
@@ -1093,6 +1095,7 @@ static int skl_resume_prepare(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 
+	skl_init_cdclk(dev_priv);
 	intel_csr_load_program(dev);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b4eebd0..44d592f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1705,6 +1705,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 5afff3a..4d7282e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6699,6 +6699,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 d602db2..cacb07b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2510,6 +2510,7 @@ void intel_ddi_pll_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t val = I915_READ(LCPLL_CTL);
+	int cdclk_freq;
 
 	if (IS_SKYLAKE(dev))
 		skl_shared_dplls_init(dev_priv);
@@ -2518,12 +2519,15 @@ void intel_ddi_pll_init(struct drm_device *dev)
 	else
 		hsw_shared_dplls_init(dev_priv);
 
-	DRM_DEBUG_KMS("CDCLK running at %dKHz\n",
-		      dev_priv->display.get_display_clock_speed(dev));
+	cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
+	DRM_DEBUG_KMS("CDCLK running at %dKHz\n", cdclk_freq);
 
 	if (IS_SKYLAKE(dev)) {
+		dev_priv->skl_boot_cdclk = cdclk_freq;
 		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 0485655..268043f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5527,6 +5527,214 @@ void broxton_uninit_cdclk(struct drm_device *dev)
 	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
 }
 
+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_decimal(unsigned int freq)
+{
+	return (freq - 1000) / 500;
+}
+
+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 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_cdclk_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(SKL_DPLL0) | DPLL_CTRL1_SSC(SKL_DPLL0) |
+		 DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0));
+	val |= DPLL_CTRL1_OVERRIDE(SKL_DPLL0);
+	if (required_vco == 8640)
+		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080,
+					    SKL_DPLL0);
+	else
+		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810,
+					    SKL_DPLL0);
+
+	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) & LCPLL_PLL_LOCK, 5))
+		DRM_ERROR("DPLL0 not locked\n");
+}
+
+static bool skl_cdclk_pcu_ready(struct drm_i915_private *dev_priv)
+{
+	int ret;
+	u32 val;
+
+	/* 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);
+
+	return ret == 0 && (val & SKL_CDCLK_READY_FOR_CHANGE);
+}
+
+static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv)
+{
+	unsigned int i;
+
+	for (i = 0; i < 15; i++) {
+		if (skl_cdclk_pcu_ready(dev_priv))
+			return true;
+		udelay(10);
+	}
+
+	return false;
+}
+
+static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
+{
+	u32 freq_select, pcu_ack;
+
+	DRM_DEBUG_DRIVER("Changing CDCLK to %dKHz\n", freq);
+
+	if (!skl_cdclk_wait_for_pcu_ready(dev_priv)) {
+		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;
+	}
+
+	I915_WRITE(CDCLK_CTL, freq_select | skl_cdclk_decimal(freq));
+	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_uninit_cdclk(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) & LCPLL_PLL_LOCK), 1))
+		DRM_ERROR("Couldn't disable DPLL0\n");
+
+	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
+}
+
+void skl_init_cdclk(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)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e94cb6e..02d8317 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1118,6 +1118,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_init_cdclk(struct drm_i915_private *dev_priv);
+void skl_uninit_cdclk(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 v3] drm/i915/skl: Deinit/init the display at suspend/resume

[Ville Syrjälä]

On Thu, May 21, 2015 at 04:37:48PM +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.
> 
> 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 */)
>   {
>           return (freq - 1000) / 500;
>   }
> 
>   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;
>   }
> 
> v2:
>   - Rebase on top of -nightly
>   - Use (freq - 1000) / 500 for the decimal frequency (Ville)
>   - Fix setting the enable bit of HSW_NDE_RSTWRN_OPT (Ville)
>   - Rename skl_display_{resume,suspend} to skl_{init,uninit}_cdclk to
>     be consistent with the BXT code (Ville)
>   - Store boot CDCLK in ddi_pll_init (Ville)
>   - Merge dev_priv's skl_boot_cdclk into cdclk_freq
>   - Use LCPLL_PLL_LOCK instead of (1 << 30) (Ville)
>   - Replace various '0' by SKL_DPLL0 to be a bit more explicit that
>     we're programming DPLL0
>   - Busy poll the PCU before doing the frequency change. It takes about
>     3/4 cycles, each separated by 10us, to get the ACK from the CPU
>     (Ville)
> 
> v3:
>   - Restore dev_priv->skl_boot_cdclk, leaving unification with
>     dev_priv->cdclk_freq for a later patch (Daniel, Ville)
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c      |   3 +
>  drivers/gpu/drm/i915/i915_drv.h      |   1 +
>  drivers/gpu/drm/i915/i915_reg.h      |   3 +
>  drivers/gpu/drm/i915/intel_ddi.c     |   8 +-
>  drivers/gpu/drm/i915/intel_display.c | 208 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |   2 +
>  6 files changed, 223 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 93191c1..884b4f9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1047,6 +1047,8 @@ static int skl_suspend_complete(struct drm_i915_private *dev_priv)
>  	 */
>  	intel_csr_load_status_set(dev_priv, FW_UNINITIALIZED);
>  
> +	skl_uninit_cdclk(dev_priv);
> +
>  	return 0;
>  }
>  
> @@ -1093,6 +1095,7 @@ static int skl_resume_prepare(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
>  
> +	skl_init_cdclk(dev_priv);
>  	intel_csr_load_program(dev);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b4eebd0..44d592f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1705,6 +1705,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 5afff3a..4d7282e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6699,6 +6699,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 d602db2..cacb07b 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2510,6 +2510,7 @@ void intel_ddi_pll_init(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	uint32_t val = I915_READ(LCPLL_CTL);
> +	int cdclk_freq;
>  
>  	if (IS_SKYLAKE(dev))
>  		skl_shared_dplls_init(dev_priv);
> @@ -2518,12 +2519,15 @@ void intel_ddi_pll_init(struct drm_device *dev)
>  	else
>  		hsw_shared_dplls_init(dev_priv);
>  
> -	DRM_DEBUG_KMS("CDCLK running at %dKHz\n",
> -		      dev_priv->display.get_display_clock_speed(dev));
> +	cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
> +	DRM_DEBUG_KMS("CDCLK running at %dKHz\n", cdclk_freq);
>  
>  	if (IS_SKYLAKE(dev)) {
> +		dev_priv->skl_boot_cdclk = cdclk_freq;
>  		if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
>  			DRM_ERROR("LCPLL1 is disabled\n");
> +		else
> +			intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);

Hmm, actually are we not calling the skl_init_cdclk() on boot at all? I see
it only in the resume path.

>  	} 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 0485655..268043f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5527,6 +5527,214 @@ void broxton_uninit_cdclk(struct drm_device *dev)
>  	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
>  }
>  
> +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_decimal(unsigned int freq)
> +{
> +	return (freq - 1000) / 500;
> +}
> +
> +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 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_cdclk_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(SKL_DPLL0) | DPLL_CTRL1_SSC(SKL_DPLL0) |
> +		 DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0));
> +	val |= DPLL_CTRL1_OVERRIDE(SKL_DPLL0);
> +	if (required_vco == 8640)
> +		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080,
> +					    SKL_DPLL0);
> +	else
> +		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810,
> +					    SKL_DPLL0);
> +
> +	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) & LCPLL_PLL_LOCK, 5))
> +		DRM_ERROR("DPLL0 not locked\n");
> +}
> +
> +static bool skl_cdclk_pcu_ready(struct drm_i915_private *dev_priv)
> +{
> +	int ret;
> +	u32 val;
> +
> +	/* 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);
> +
> +	return ret == 0 && (val & SKL_CDCLK_READY_FOR_CHANGE);
> +}
> +
> +static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < 15; i++) {
> +		if (skl_cdclk_pcu_ready(dev_priv))
> +			return true;
> +		udelay(10);
> +	}
> +
> +	return false;
> +}
> +
> +static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
> +{
> +	u32 freq_select, pcu_ack;
> +
> +	DRM_DEBUG_DRIVER("Changing CDCLK to %dKHz\n", freq);
> +
> +	if (!skl_cdclk_wait_for_pcu_ready(dev_priv)) {
> +		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;
> +	}
> +
> +	I915_WRITE(CDCLK_CTL, freq_select | skl_cdclk_decimal(freq));
> +	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_uninit_cdclk(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) & LCPLL_PLL_LOCK), 1))
> +		DRM_ERROR("Couldn't disable DPLL0\n");
> +
> +	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
> +}
> +
> +void skl_init_cdclk(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)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index e94cb6e..02d8317 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1118,6 +1118,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_init_cdclk(struct drm_i915_private *dev_priv);
> +void skl_uninit_cdclk(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

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

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

[Damien Lespiau]

On Thu, May 21, 2015 at 06:44:53PM +0300, Ville Syrjälä wrote:
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index d602db2..cacb07b 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2510,6 +2510,7 @@ void intel_ddi_pll_init(struct drm_device *dev)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	uint32_t val = I915_READ(LCPLL_CTL);
> > +	int cdclk_freq;
> >  
> >  	if (IS_SKYLAKE(dev))
> >  		skl_shared_dplls_init(dev_priv);
> > @@ -2518,12 +2519,15 @@ void intel_ddi_pll_init(struct drm_device *dev)
> >  	else
> >  		hsw_shared_dplls_init(dev_priv);
> >  
> > -	DRM_DEBUG_KMS("CDCLK running at %dKHz\n",
> > -		      dev_priv->display.get_display_clock_speed(dev));
> > +	cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
> > +	DRM_DEBUG_KMS("CDCLK running at %dKHz\n", cdclk_freq);
> >  
> >  	if (IS_SKYLAKE(dev)) {
> > +		dev_priv->skl_boot_cdclk = cdclk_freq;
> >  		if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
> >  			DRM_ERROR("LCPLL1 is disabled\n");
> > +		else
> > +			intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
> 
> Hmm, actually are we not calling the skl_init_cdclk() on boot at all? I see
> it only in the resume path.

Yes that's correct, we always have display intialized by the firmware on
big core as far as I know (which may not be true for BXT).

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

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

[Daniel Vetter]

On Thu, May 21, 2015 at 04:37:48PM +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.
> 
> 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 */)
>   {
>           return (freq - 1000) / 500;
>   }
> 
>   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;
>   }
> 
> v2:
>   - Rebase on top of -nightly
>   - Use (freq - 1000) / 500 for the decimal frequency (Ville)
>   - Fix setting the enable bit of HSW_NDE_RSTWRN_OPT (Ville)
>   - Rename skl_display_{resume,suspend} to skl_{init,uninit}_cdclk to
>     be consistent with the BXT code (Ville)
>   - Store boot CDCLK in ddi_pll_init (Ville)
>   - Merge dev_priv's skl_boot_cdclk into cdclk_freq
>   - Use LCPLL_PLL_LOCK instead of (1 << 30) (Ville)
>   - Replace various '0' by SKL_DPLL0 to be a bit more explicit that
>     we're programming DPLL0
>   - Busy poll the PCU before doing the frequency change. It takes about
>     3/4 cycles, each separated by 10us, to get the ACK from the CPU
>     (Ville)
> 
> v3:
>   - Restore dev_priv->skl_boot_cdclk, leaving unification with
>     dev_priv->cdclk_freq for a later patch (Daniel, Ville)
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

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

> ---
>  drivers/gpu/drm/i915/i915_drv.c      |   3 +
>  drivers/gpu/drm/i915/i915_drv.h      |   1 +
>  drivers/gpu/drm/i915/i915_reg.h      |   3 +
>  drivers/gpu/drm/i915/intel_ddi.c     |   8 +-
>  drivers/gpu/drm/i915/intel_display.c | 208 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |   2 +
>  6 files changed, 223 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 93191c1..884b4f9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1047,6 +1047,8 @@ static int skl_suspend_complete(struct drm_i915_private *dev_priv)
>  	 */
>  	intel_csr_load_status_set(dev_priv, FW_UNINITIALIZED);
>  
> +	skl_uninit_cdclk(dev_priv);
> +
>  	return 0;
>  }
>  
> @@ -1093,6 +1095,7 @@ static int skl_resume_prepare(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
>  
> +	skl_init_cdclk(dev_priv);
>  	intel_csr_load_program(dev);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b4eebd0..44d592f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1705,6 +1705,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 5afff3a..4d7282e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6699,6 +6699,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 d602db2..cacb07b 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2510,6 +2510,7 @@ void intel_ddi_pll_init(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	uint32_t val = I915_READ(LCPLL_CTL);
> +	int cdclk_freq;
>  
>  	if (IS_SKYLAKE(dev))
>  		skl_shared_dplls_init(dev_priv);
> @@ -2518,12 +2519,15 @@ void intel_ddi_pll_init(struct drm_device *dev)
>  	else
>  		hsw_shared_dplls_init(dev_priv);
>  
> -	DRM_DEBUG_KMS("CDCLK running at %dKHz\n",
> -		      dev_priv->display.get_display_clock_speed(dev));
> +	cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
> +	DRM_DEBUG_KMS("CDCLK running at %dKHz\n", cdclk_freq);
>  
>  	if (IS_SKYLAKE(dev)) {
> +		dev_priv->skl_boot_cdclk = cdclk_freq;
>  		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 0485655..268043f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5527,6 +5527,214 @@ void broxton_uninit_cdclk(struct drm_device *dev)
>  	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
>  }
>  
> +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_decimal(unsigned int freq)
> +{
> +	return (freq - 1000) / 500;
> +}
> +
> +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 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_cdclk_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(SKL_DPLL0) | DPLL_CTRL1_SSC(SKL_DPLL0) |
> +		 DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0));
> +	val |= DPLL_CTRL1_OVERRIDE(SKL_DPLL0);
> +	if (required_vco == 8640)
> +		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080,
> +					    SKL_DPLL0);
> +	else
> +		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810,
> +					    SKL_DPLL0);
> +
> +	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) & LCPLL_PLL_LOCK, 5))
> +		DRM_ERROR("DPLL0 not locked\n");
> +}
> +
> +static bool skl_cdclk_pcu_ready(struct drm_i915_private *dev_priv)
> +{
> +	int ret;
> +	u32 val;
> +
> +	/* 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);
> +
> +	return ret == 0 && (val & SKL_CDCLK_READY_FOR_CHANGE);
> +}
> +
> +static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < 15; i++) {
> +		if (skl_cdclk_pcu_ready(dev_priv))
> +			return true;
> +		udelay(10);
> +	}
> +
> +	return false;
> +}
> +
> +static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
> +{
> +	u32 freq_select, pcu_ack;
> +
> +	DRM_DEBUG_DRIVER("Changing CDCLK to %dKHz\n", freq);
> +
> +	if (!skl_cdclk_wait_for_pcu_ready(dev_priv)) {
> +		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;
> +	}
> +
> +	I915_WRITE(CDCLK_CTL, freq_select | skl_cdclk_decimal(freq));
> +	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_uninit_cdclk(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) & LCPLL_PLL_LOCK), 1))
> +		DRM_ERROR("Couldn't disable DPLL0\n");
> +
> +	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
> +}
> +
> +void skl_init_cdclk(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)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index e94cb6e..02d8317 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1118,6 +1118,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_init_cdclk(struct drm_i915_private *dev_priv);
> +void skl_uninit_cdclk(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
> 

-- 
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 v3] drm/i915/skl: Deinit/init the display at suspend/resume

[Ville Syrjälä]

On Thu, May 21, 2015 at 04:49:40PM +0100, Damien Lespiau wrote:
> On Thu, May 21, 2015 at 06:44:53PM +0300, Ville Syrjälä wrote:
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > > index d602db2..cacb07b 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -2510,6 +2510,7 @@ void intel_ddi_pll_init(struct drm_device *dev)
> > >  {
> > >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > >  	uint32_t val = I915_READ(LCPLL_CTL);
> > > +	int cdclk_freq;
> > >  
> > >  	if (IS_SKYLAKE(dev))
> > >  		skl_shared_dplls_init(dev_priv);
> > > @@ -2518,12 +2519,15 @@ void intel_ddi_pll_init(struct drm_device *dev)
> > >  	else
> > >  		hsw_shared_dplls_init(dev_priv);
> > >  
> > > -	DRM_DEBUG_KMS("CDCLK running at %dKHz\n",
> > > -		      dev_priv->display.get_display_clock_speed(dev));
> > > +	cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
> > > +	DRM_DEBUG_KMS("CDCLK running at %dKHz\n", cdclk_freq);
> > >  
> > >  	if (IS_SKYLAKE(dev)) {
> > > +		dev_priv->skl_boot_cdclk = cdclk_freq;
> > >  		if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
> > >  			DRM_ERROR("LCPLL1 is disabled\n");
> > > +		else
> > > +			intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
> > 
> > Hmm, actually are we not calling the skl_init_cdclk() on boot at all? I see
> > it only in the resume path.
> 
> Yes that's correct, we always have display intialized by the firmware on
> big core as far as I know (which may not be true for BXT).

What about module reload?

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

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

[Damien Lespiau]

On Thu, May 21, 2015 at 06:58:48PM +0300, Ville Syrjälä wrote:
> > > Hmm, actually are we not calling the skl_init_cdclk() on boot at all? I see
> > > it only in the resume path.
> > 
> > Yes that's correct, we always have display intialized by the firmware on
> > big core as far as I know (which may not be true for BXT).
> 
> What about module reload?

I can see your point, but we're not totally de-initializing display on
unload today, so we keep the current CDCLK (among other things) across
module reload.

Just tried tests/drm_module_reload and it works as expected, display
comes back up.

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

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

[Ville Syrjälä]

On Thu, May 21, 2015 at 05:11:57PM +0100, Damien Lespiau wrote:
> On Thu, May 21, 2015 at 06:58:48PM +0300, Ville Syrjälä wrote:
> > > > Hmm, actually are we not calling the skl_init_cdclk() on boot at all? I see
> > > > it only in the resume path.
> > > 
> > > Yes that's correct, we always have display intialized by the firmware on
> > > big core as far as I know (which may not be true for BXT).
> > 
> > What about module reload?
> 
> I can see your point, but we're not totally de-initializing display on
> unload today, so we keep the current CDCLK (among other things) across
> module reload.
> 
> Just tried tests/drm_module_reload and it works as expected, display
> comes back up.

Fair enough. Good enough for now at least then.

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

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

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6443
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  234/234              234/234
ILK                                  262/262              262/262
SNB                 -1              282/282              281/282
IVB                                  300/300              300/300
BYT                                  254/254              254/254
BDW                 -1              275/275              274/275
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 SNB  igt@pm_rpm@dpms-mode-unset-non-lpsp      DMESG_WARN(16)PASS(1)      DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x
*BDW  igt@gem_flink_race@flink_name      PASS(2)      DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_display.c:#assert_plane[i915]()@WARNING:.* at .* assert_plane
assertion_failure@assertion failure
WARNING:at_drivers/gpu/drm/drm_irq.c:#drm_wait_one_vblank[drm]()@WARNING:.* at .* drm_wait_one_vblank+0x
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 v3] drm/i915/skl: Deinit/init the display at suspend/resume

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6452
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  234/234              234/234
ILK                                  262/262              262/262
SNB                 -1              282/282              281/282
IVB                                  300/300              300/300
BYT                                  254/254              254/254
BDW                                  275/275              275/275
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 SNB  igt@pm_rpm@dpms-mode-unset-non-lpsp      DMESG_WARN(18)PASS(1)      DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x
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