[PATCH 00/13] drm/i915: cdclk code cleanup

[PATCH 00/13] drm/i915: cdclk code cleanup

[ville.syrjala]

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

Here's a bunch of cleanup, refactoring, etc. of the cdclk code. Much of
this serves as prep work for a larger series I have lined up which
takes Clint's SKL cdclk patch, expands on it, and then tries to unify
a lot of the SKL and BXT cdclk and display core init/uninit code. The
final goal is some future hardware enabling, and I really want that to
follow the same unified style as much as possible.

Anyways, I thought I'd try to get some of the less controversial cleanups
out of the way first, and so I'm sending out this series first.

Series available here:
git://github.com/vsyrjala/linux.git skl_bxt_cdclk_part_1

Ville Syrjälä (13):
  drm/i915: Drop checks for max_pixclk failures in cdclk computation
  drm/i915: Extract broadwell_calc_cdclk()
  drm/i915: Untangle .fdi_link_train and cdclk vfunc setup
  drm/i915: Don't pass dev_priv to broxton_calc_cdclk()
  drm/i915: Use ilk_max_pixel_rate() for BXT cdclk calculation
  drm/i915: Use skl_cdclk_decimal() on bxt
  drm/i915: Remove 10% cdclk guardband on BXT
  drm/i915: Extract skl_dpll0_disable()
  drm/i915: Kill off dead code from skl_dpll0_enable()
  drm/i915: s/freq/cdclk/
  drm/i915: s/required_vco/vco/ in skl cdclk code
  drm/i915: Program BXT_CDCLK_CD2X_PIPE
  drm/i915: Eliminate the CDCLK_CTL RMW on BXT

 drivers/gpu/drm/i915/i915_reg.h      |   5 +-
 drivers/gpu/drm/i915/intel_display.c | 160 +++++++++++++++++------------------
 2 files changed, 81 insertions(+), 84 deletions(-)

-- 
2.7.4

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

[PATCH 01/13] drm/i915: Drop checks for max_pixclk failures in cdclk computation

[ville.syrjala]

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

commit 565602d7501a ("drm/i915: Do not acquire crtc state to check clock during modeset, v4.")
removed the possibility that intel_mode_max_pixclk() or
ilk_max_pixel_rate() might return an error, so let's get rid of the
error checks in the callers as well.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b60d9b67e033..775ab3cf1eee 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5957,9 +5957,6 @@ static int valleyview_modeset_calc_cdclk(struct drm_atomic_state *state)
 	struct intel_atomic_state *intel_state =
 		to_intel_atomic_state(state);
 
-	if (max_pixclk < 0)
-		return max_pixclk;
-
 	intel_state->cdclk = intel_state->dev_cdclk =
 		valleyview_calc_cdclk(dev_priv, max_pixclk);
 
@@ -5977,9 +5974,6 @@ static int broxton_modeset_calc_cdclk(struct drm_atomic_state *state)
 	struct intel_atomic_state *intel_state =
 		to_intel_atomic_state(state);
 
-	if (max_pixclk < 0)
-		return max_pixclk;
-
 	intel_state->cdclk = intel_state->dev_cdclk =
 		broxton_calc_cdclk(dev_priv, max_pixclk);
 
-- 
2.7.4

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

[PATCH 02/13] drm/i915: Extract broadwell_calc_cdclk()

[ville.syrjala]

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

Try to reduce the amount of duplicated cdclk magic numbers by
moving the max_pixclk->cdclk conversion into a helper.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 775ab3cf1eee..b55075e6020c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9675,6 +9675,18 @@ static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
 	     cdclk, dev_priv->cdclk_freq);
 }
 
+static int broadwell_calc_cdclk(int max_pixclk)
+{
+	if (max_pixclk > 540000)
+		return 675000;
+	else if (max_pixclk > 450000)
+		return 540000;
+	else if (max_pixclk > 337500)
+		return 450000;
+	else
+		return 337500;
+}
+
 static int broadwell_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
@@ -9686,14 +9698,7 @@ static int broadwell_modeset_calc_cdclk(struct drm_atomic_state *state)
 	 * FIXME should also account for plane ratio
 	 * once 64bpp pixel formats are supported.
 	 */
-	if (max_pixclk > 540000)
-		cdclk = 675000;
-	else if (max_pixclk > 450000)
-		cdclk = 540000;
-	else if (max_pixclk > 337500)
-		cdclk = 450000;
-	else
-		cdclk = 337500;
+	cdclk = broadwell_calc_cdclk(max_pixclk);
 
 	if (cdclk > dev_priv->max_cdclk_freq) {
 		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
@@ -9703,7 +9708,7 @@ static int broadwell_modeset_calc_cdclk(struct drm_atomic_state *state)
 
 	intel_state->cdclk = intel_state->dev_cdclk = cdclk;
 	if (!intel_state->active_crtcs)
-		intel_state->dev_cdclk = 337500;
+		intel_state->dev_cdclk = broadwell_calc_cdclk(0);
 
 	return 0;
 }
-- 
2.7.4

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

[PATCH 03/13] drm/i915: Untangle .fdi_link_train and cdclk vfunc setup

[ville.syrjala]

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

Split the .fdi_link_train and .modeset_commit_cdclk/.modeset_calc_cdclk
into two separate if ladders. Much easier to read when you're not
confusing two totally separate subjects.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b55075e6020c..a9e5d6b81043 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15044,12 +15044,13 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
 	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
 		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
-		if (IS_BROADWELL(dev_priv)) {
-			dev_priv->display.modeset_commit_cdclk =
-				broadwell_modeset_commit_cdclk;
-			dev_priv->display.modeset_calc_cdclk =
-				broadwell_modeset_calc_cdclk;
-		}
+	}
+
+	if (IS_BROADWELL(dev_priv)) {
+		dev_priv->display.modeset_commit_cdclk =
+			broadwell_modeset_commit_cdclk;
+		dev_priv->display.modeset_calc_cdclk =
+			broadwell_modeset_calc_cdclk;
 	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		dev_priv->display.modeset_commit_cdclk =
 			valleyview_modeset_commit_cdclk;
-- 
2.7.4

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

[PATCH 04/13] drm/i915: Don't pass dev_priv to broxton_calc_cdclk()

[ville.syrjala]

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

broxton_calc_cdclk() doesn't need dev_priv for anything, so let's not
bother passing it around.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a9e5d6b81043..deeaf3ba1dee 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5900,8 +5900,7 @@ static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
 		return 200000;
 }
 
-static int broxton_calc_cdclk(struct drm_i915_private *dev_priv,
-			      int max_pixclk)
+static int broxton_calc_cdclk(int max_pixclk)
 {
 	/*
 	 * FIXME:
@@ -5969,16 +5968,15 @@ static int valleyview_modeset_calc_cdclk(struct drm_atomic_state *state)
 static int broxton_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
 	struct drm_device *dev = state->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	int max_pixclk = intel_mode_max_pixclk(dev, state);
 	struct intel_atomic_state *intel_state =
 		to_intel_atomic_state(state);
 
 	intel_state->cdclk = intel_state->dev_cdclk =
-		broxton_calc_cdclk(dev_priv, max_pixclk);
+		broxton_calc_cdclk(max_pixclk);
 
 	if (!intel_state->active_crtcs)
-		intel_state->dev_cdclk = broxton_calc_cdclk(dev_priv, 0);
+		intel_state->dev_cdclk = broxton_calc_cdclk(0);
 
 	return 0;
 }
-- 
2.7.4

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

[PATCH 05/13] drm/i915: Use ilk_max_pixel_rate() for BXT cdclk calculation

[ville.syrjala]

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

BXT uses the "pch" panel fitter configuration, so we can use
ilk_max_pixel_rate() instead of intel_mode_max_pixclk() to compute the
pipe pixel rate. ilk_max_pixel_rate() will account for the pipe
scaler downscaling factor whereas intel_mode_max_pixclk() will not.

I'm pretty sure the same limitation is there on GMCH platforms, but
no one just bothered to implement the downscaling adjustment for them.
Probably should just unify the panel fitter setup more across the
platforms and use the exact same code on all platforms for this.
But in the meantime, let's at least make BXT a bit more correct.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index deeaf3ba1dee..fd55112f266d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -117,6 +117,7 @@ static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
 static void ironlake_pfit_enable(struct intel_crtc *crtc);
 static void intel_modeset_setup_hw_state(struct drm_device *dev);
 static void intel_pre_disable_primary_noatomic(struct drm_crtc *crtc);
+static int ilk_max_pixel_rate(struct drm_atomic_state *state);
 
 typedef struct {
 	int	min, max;
@@ -5967,8 +5968,7 @@ static int valleyview_modeset_calc_cdclk(struct drm_atomic_state *state)
 
 static int broxton_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
-	struct drm_device *dev = state->dev;
-	int max_pixclk = intel_mode_max_pixclk(dev, state);
+	int max_pixclk = ilk_max_pixel_rate(state);
 	struct intel_atomic_state *intel_state =
 		to_intel_atomic_state(state);
 
-- 
2.7.4

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

[PATCH 06/13] drm/i915: Use skl_cdclk_decimal() on bxt

[ville.syrjala]

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

Both SKL and BXT need to fill in the "decimal" cdclk frequency into
the CDCLK_CTL register. SKL uses a small helper to do the kHz->"decimal"
conversion, whereas BXT has it open-coded. Use the helper on BXT too.

While at it, change it to round to closest rather than down. It doesn't
actually matter with the frequencies we have to deal with, but it seems
like the right thing to do.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fd55112f266d..9564719b53e3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5340,6 +5340,12 @@ static void intel_update_cdclk(struct drm_device *dev)
 		intel_update_max_cdclk(dev);
 }
 
+/* convert from kHz to .1 fixpoint MHz with -1MHz offset */
+static int skl_cdclk_decimal(int cdclk)
+{
+	return DIV_ROUND_CLOSEST(cdclk - 1000, 500);
+}
+
 static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int frequency)
 {
 	uint32_t divider;
@@ -5439,8 +5445,7 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int frequency)
 			val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
 
 		val &= ~CDCLK_FREQ_DECIMAL_MASK;
-		/* convert from kHz to .1 fixpoint MHz with -1MHz offset */
-		val |= (frequency - 1000) / 500;
+		val |= skl_cdclk_decimal(frequency);
 		I915_WRITE(CDCLK_CTL, val);
 	}
 
@@ -5540,11 +5545,6 @@ static const struct skl_cdclk_entry {
 	{ .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;
-- 
2.7.4

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

[PATCH 07/13] drm/i915: Remove 10% cdclk guardband on BXT

[ville.syrjala]

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

We don't need any pixel clock vs. cdclk guardband since HSW. BXT still
tries to add one though. Get rid of it.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9564719b53e3..57ff51172065 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5905,16 +5905,15 @@ static int broxton_calc_cdclk(int max_pixclk)
 {
 	/*
 	 * FIXME:
-	 * - remove the guardband, it's not needed on BXT
 	 * - set 19.2MHz bypass frequency if there are no active pipes
 	 */
-	if (max_pixclk > 576000*9/10)
+	if (max_pixclk > 576000)
 		return 624000;
-	else if (max_pixclk > 384000*9/10)
+	else if (max_pixclk > 384000)
 		return 576000;
-	else if (max_pixclk > 288000*9/10)
+	else if (max_pixclk > 288000)
 		return 384000;
-	else if (max_pixclk > 144000*9/10)
+	else if (max_pixclk > 144000)
 		return 288000;
 	else
 		return 144000;
-- 
2.7.4

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

[PATCH 08/13] drm/i915: Extract skl_dpll0_disable()

[ville.syrjala]

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

Make thins a bit easier to read by extracting the SKL DPLL0
disable into separate functions. We already have the enable
counterpart. Down the line this will also help make the cdclk
programming on SKL, BXT, and following platforms look rather
consistent.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 57ff51172065..0186f775f353 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5610,6 +5610,14 @@ skl_dpll0_enable(struct drm_i915_private *dev_priv, unsigned int required_vco)
 		DRM_ERROR("DPLL0 not locked\n");
 }
 
+static void
+skl_dpll0_disable(struct drm_i915_private *dev_priv)
+{
+	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");
+}
+
 static bool skl_cdclk_pcu_ready(struct drm_i915_private *dev_priv)
 {
 	int ret;
@@ -5695,10 +5703,7 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
 	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");
+	skl_dpll0_disable(dev_priv);
 }
 
 void skl_init_cdclk(struct drm_i915_private *dev_priv)
-- 
2.7.4

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

[PATCH 09/13] drm/i915: Kill off dead code from skl_dpll0_enable()

[ville.syrjala]

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

We calculate the CDCLK_CTL value from scratch so no need to attempt
some form of RMW first.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0186f775f353..a21f9d3fb869 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5566,17 +5566,12 @@ skl_dpll0_enable(struct drm_i915_private *dev_priv, unsigned int required_vco)
 	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);
 
-- 
2.7.4

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

[PATCH 10/13] drm/i915: s/freq/cdclk/

[ville.syrjala]

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

Rename the generic sounding freq/frequency parameters to the cdclk
functions to 'cdclk' so that we'll know which clock we're talking about
once we have to deal with the vco frequencies as well.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a21f9d3fb869..f58bbac6204b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5346,15 +5346,15 @@ static int skl_cdclk_decimal(int cdclk)
 	return DIV_ROUND_CLOSEST(cdclk - 1000, 500);
 }
 
-static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int frequency)
+static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
 {
 	uint32_t divider;
 	uint32_t ratio;
-	uint32_t current_freq;
+	uint32_t current_cdclk;
 	int ret;
 
 	/* frequency = 19.2MHz * ratio / 2 / div{1,1.5,2,4} */
-	switch (frequency) {
+	switch (cdclk) {
 	case 144000:
 		divider = BXT_CDCLK_CD2X_DIV_SEL_4;
 		ratio = BXT_DE_PLL_RATIO(60);
@@ -5384,7 +5384,7 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int frequency)
 		divider = 0;
 		break;
 	default:
-		DRM_ERROR("unsupported CDCLK freq %d", frequency);
+		DRM_ERROR("unsupported CDCLK freq %d", cdclk);
 
 		return;
 	}
@@ -5397,13 +5397,13 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int frequency)
 
 	if (ret) {
 		DRM_ERROR("PCode CDCLK freq change notify failed (err %d, freq %d)\n",
-			  ret, frequency);
+			  ret, cdclk);
 		return;
 	}
 
-	current_freq = I915_READ(CDCLK_CTL) & CDCLK_FREQ_DECIMAL_MASK;
+	current_cdclk = I915_READ(CDCLK_CTL) & CDCLK_FREQ_DECIMAL_MASK;
 	/* convert from .1 fixpoint MHz with -1MHz offset to kHz */
-	current_freq = current_freq * 500 + 1000;
+	current_cdclk = current_cdclk * 500 + 1000;
 
 	/*
 	 * DE PLL has to be disabled when
@@ -5411,8 +5411,8 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int frequency)
 	 * - before setting to 624MHz (PLL needs toggling)
 	 * - before setting to any frequency from 624MHz (PLL needs toggling)
 	 */
-	if (frequency == 19200 || frequency == 624000 ||
-	    current_freq == 624000) {
+	if (cdclk == 19200 || cdclk == 624000 ||
+	    current_cdclk == 624000) {
 		I915_WRITE(BXT_DE_PLL_ENABLE, ~BXT_DE_PLL_PLL_ENABLE);
 		/* Timeout 200us */
 		if (wait_for(!(I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_LOCK),
@@ -5420,7 +5420,7 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int frequency)
 			DRM_ERROR("timout waiting for DE PLL unlock\n");
 	}
 
-	if (frequency != 19200) {
+	if (cdclk != 19200) {
 		uint32_t val;
 
 		val = I915_READ(BXT_DE_PLL_CTL);
@@ -5441,22 +5441,22 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int frequency)
 		 * enable otherwise.
 		 */
 		val &= ~BXT_CDCLK_SSA_PRECHARGE_ENABLE;
-		if (frequency >= 500000)
+		if (cdclk >= 500000)
 			val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
 
 		val &= ~CDCLK_FREQ_DECIMAL_MASK;
-		val |= skl_cdclk_decimal(frequency);
+		val |= skl_cdclk_decimal(cdclk);
 		I915_WRITE(CDCLK_CTL, val);
 	}
 
 	mutex_lock(&dev_priv->rps.hw_lock);
 	ret = sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ,
-				      DIV_ROUND_UP(frequency, 25000));
+				      DIV_ROUND_UP(cdclk, 25000));
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	if (ret) {
 		DRM_ERROR("PCode CDCLK freq set failed, (err %d, freq %d)\n",
-			  ret, frequency);
+			  ret, cdclk);
 		return;
 	}
 
@@ -5562,16 +5562,16 @@ static unsigned int skl_cdclk_get_vco(unsigned int freq)
 static void
 skl_dpll0_enable(struct drm_i915_private *dev_priv, unsigned int required_vco)
 {
-	unsigned int min_freq;
+	int min_cdclk;
 	u32 val;
 
 	/* select the minimum CDCLK before enabling DPLL 0 */
 	if (required_vco == 8640)
-		min_freq = 308570;
+		min_cdclk = 308570;
 	else
-		min_freq = 337500;
+		min_cdclk = 337500;
 
-	val = CDCLK_FREQ_337_308 | skl_cdclk_decimal(min_freq);
+	val = CDCLK_FREQ_337_308 | skl_cdclk_decimal(min_cdclk);
 	I915_WRITE(CDCLK_CTL, val);
 	POSTING_READ(CDCLK_CTL);
 
@@ -5640,12 +5640,12 @@ static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv)
 	return false;
 }
 
-static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
+static void skl_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
 {
 	struct drm_device *dev = dev_priv->dev;
 	u32 freq_select, pcu_ack;
 
-	DRM_DEBUG_DRIVER("Changing CDCLK to %dKHz\n", freq);
+	DRM_DEBUG_DRIVER("Changing CDCLK to %dKHz\n", cdclk);
 
 	if (!skl_cdclk_wait_for_pcu_ready(dev_priv)) {
 		DRM_ERROR("failed to inform PCU about cdclk change\n");
@@ -5653,7 +5653,7 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
 	}
 
 	/* set CDCLK_CTL */
-	switch(freq) {
+	switch (cdclk) {
 	case 450000:
 	case 432000:
 		freq_select = CDCLK_FREQ_450_432;
@@ -5676,7 +5676,7 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
 		break;
 	}
 
-	I915_WRITE(CDCLK_CTL, freq_select | skl_cdclk_decimal(freq));
+	I915_WRITE(CDCLK_CTL, freq_select | skl_cdclk_decimal(cdclk));
 	POSTING_READ(CDCLK_CTL);
 
 	/* inform PCU of the change */
-- 
2.7.4

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

[PATCH 11/13] drm/i915: s/required_vco/vco/ in skl cdclk code

[ville.syrjala]

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

The 'required' part of 'required_vco' should be obvious. Let's just call
it 'vco' for brevity.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f58bbac6204b..61133fdedc8d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5560,13 +5560,13 @@ static unsigned int skl_cdclk_get_vco(unsigned int freq)
 }
 
 static void
-skl_dpll0_enable(struct drm_i915_private *dev_priv, unsigned int required_vco)
+skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco)
 {
 	int min_cdclk;
 	u32 val;
 
 	/* select the minimum CDCLK before enabling DPLL 0 */
-	if (required_vco == 8640)
+	if (vco == 8640)
 		min_cdclk = 308570;
 	else
 		min_cdclk = 337500;
@@ -5589,7 +5589,7 @@ skl_dpll0_enable(struct drm_i915_private *dev_priv, unsigned int required_vco)
 	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)
+	if (vco == 8640)
 		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080,
 					    SKL_DPLL0);
 	else
@@ -5703,13 +5703,13 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
 
 void skl_init_cdclk(struct drm_i915_private *dev_priv)
 {
-	unsigned int required_vco;
+	unsigned int vco;
 
 	/* DPLL0 not enabled (happens on early BIOS versions) */
 	if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) {
 		/* enable DPLL0 */
-		required_vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk);
-		skl_dpll0_enable(dev_priv, required_vco);
+		vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk);
+		skl_dpll0_enable(dev_priv, vco);
 	}
 
 	/* set CDCLK to the frequency the BIOS chose */
-- 
2.7.4

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

[PATCH 12/13] drm/i915: Program BXT_CDCLK_CD2X_PIPE

[ville.syrjala]

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

BXT could change the CD2X divider synchronized with a single pipe.
So assuming the DE PLL frequency doesn't need to be changed, we could
change cdclk without shutting off the pipe (when only a single pipe is
enabled). In the meantime let's configure CDCLK_CTL for non-double
buffered CD2X update, although it shouldn't really matter as long as
the selected pipe is disabled when reprogramming the divider.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 5 +++--
 drivers/gpu/drm/i915/intel_display.c | 5 +++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 54ce0b105956..27a781260d33 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7566,14 +7566,15 @@ enum skl_disp_power_wells {
 #define  CDCLK_FREQ_540			(1<<26)
 #define  CDCLK_FREQ_337_308		(2<<26)
 #define  CDCLK_FREQ_675_617		(3<<26)
-#define  CDCLK_FREQ_DECIMAL_MASK	(0x7ff)
-
+#define  BXT_CDCLK_CD2X_PIPE(pipe)	((pipe)<<20)
+#define  BXT_CDCLK_CD2X_PIPE_NONE	BXT_CDCLK_CD2X_PIPE(3)
 #define  BXT_CDCLK_CD2X_DIV_SEL_MASK	(3<<22)
 #define  BXT_CDCLK_CD2X_DIV_SEL_1	(0<<22)
 #define  BXT_CDCLK_CD2X_DIV_SEL_1_5	(1<<22)
 #define  BXT_CDCLK_CD2X_DIV_SEL_2	(2<<22)
 #define  BXT_CDCLK_CD2X_DIV_SEL_4	(3<<22)
 #define  BXT_CDCLK_SSA_PRECHARGE_ENABLE	(1<<16)
+#define  CDCLK_FREQ_DECIMAL_MASK	(0x7ff)
 
 /* LCPLL_CTL */
 #define LCPLL1_CTL		_MMIO(0x46010)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 61133fdedc8d..39990bfe47f2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5434,6 +5434,11 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
 			DRM_ERROR("timeout waiting for DE PLL lock\n");
 
 		val = I915_READ(CDCLK_CTL);
+		/*
+		 * FIXME if only the cd2x divider needs changing, it could be done
+		 * without shutting off the pipe (if only one pipe is active).
+		 */
+		val |= BXT_CDCLK_CD2X_PIPE_NONE;
 		val &= ~BXT_CDCLK_CD2X_DIV_SEL_MASK;
 		val |= divider;
 		/*
-- 
2.7.4

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

[PATCH 13/13] drm/i915: Eliminate the CDCLK_CTL RMW on BXT

[ville.syrjala]

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

All the fields in CDCLK_CTL we don't program should be left at zero, so
let's just get rid of the RMW.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 39990bfe47f2..3b6b9de968c5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5433,24 +5433,18 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
 		if (wait_for(I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_LOCK, 1))
 			DRM_ERROR("timeout waiting for DE PLL lock\n");
 
-		val = I915_READ(CDCLK_CTL);
+		val = divider | skl_cdclk_decimal(cdclk);
 		/*
 		 * FIXME if only the cd2x divider needs changing, it could be done
 		 * without shutting off the pipe (if only one pipe is active).
 		 */
 		val |= BXT_CDCLK_CD2X_PIPE_NONE;
-		val &= ~BXT_CDCLK_CD2X_DIV_SEL_MASK;
-		val |= divider;
 		/*
 		 * Disable SSA Precharge when CD clock frequency < 500 MHz,
 		 * enable otherwise.
 		 */
-		val &= ~BXT_CDCLK_SSA_PRECHARGE_ENABLE;
 		if (cdclk >= 500000)
 			val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
-
-		val &= ~CDCLK_FREQ_DECIMAL_MASK;
-		val |= skl_cdclk_decimal(cdclk);
 		I915_WRITE(CDCLK_CTL, val);
 	}
 
-- 
2.7.4

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

✗ Ro.CI.BAT: failure for drm/i915: cdclk code cleanup

[Patchwork]

== Series Details ==

Series: drm/i915: cdclk code cleanup
URL   : https://patchwork.freedesktop.org/series/7038/
State : failure

== Summary ==

Series 7038v1 drm/i915: cdclk code cleanup
http://patchwork.freedesktop.org/api/1.0/series/7038/revisions/1/mbox

Test drv_hangman:
        Subgroup error-state-basic:
                pass       -> FAIL       (ro-ilk1-i5-650)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-cmd:
                fail       -> PASS       (ro-byt-n2820)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (ro-ivb2-i7-3770)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (ro-ivb2-i7-3770)
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                fail       -> PASS       (ro-snb-i7-2620M)

ro-bdw-i5-5250u  total:219  pass:181  dwarn:0   dfail:0   fail:0   skip:38 
ro-bdw-i7-5600u  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
ro-bsw-n3050     total:219  pass:175  dwarn:0   dfail:0   fail:2   skip:42 
ro-byt-n2820     total:218  pass:175  dwarn:0   dfail:0   fail:2   skip:41 
ro-hsw-i3-4010u  total:218  pass:193  dwarn:0   dfail:0   fail:0   skip:25 
ro-hsw-i7-4770r  total:219  pass:194  dwarn:0   dfail:0   fail:0   skip:25 
ro-ilk-i7-620lm  total:219  pass:151  dwarn:0   dfail:0   fail:1   skip:67 
ro-ilk1-i5-650   total:214  pass:151  dwarn:0   dfail:0   fail:2   skip:61 
ro-ivb-i7-3770   total:219  pass:183  dwarn:0   dfail:0   fail:0   skip:36 
ro-ivb2-i7-3770  total:219  pass:186  dwarn:1   dfail:0   fail:0   skip:32 
ro-skl-i7-6700hq total:214  pass:190  dwarn:0   dfail:0   fail:0   skip:24 
ro-snb-i7-2620M  total:219  pass:177  dwarn:0   dfail:0   fail:1   skip:41 
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_856/

576b6d9 drm-intel-nightly: 2016y-05m-11d-17h-10m-57s UTC integration manifest
b1238a0 drm/i915: Eliminate the CDCLK_CTL RMW on BXT
b2a049e drm/i915: Program BXT_CDCLK_CD2X_PIPE
6c649e1 drm/i915: s/required_vco/vco/ in skl cdclk code
b953b1f drm/i915: s/freq/cdclk/
339731a drm/i915: Kill off dead code from skl_dpll0_enable()
43ac272 drm/i915: Extract skl_dpll0_disable()
5939fa9 drm/i915: Remove 10% cdclk guardband on BXT
b4269a8 drm/i915: Use skl_cdclk_decimal() on bxt
4ed70f5 drm/i915: Use ilk_max_pixel_rate() for BXT cdclk calculation
e79c6f8 drm/i915: Don't pass dev_priv to broxton_calc_cdclk()
5efaf3f drm/i915: Untangle .fdi_link_train and cdclk vfunc setup
57c6353 drm/i915: Extract broadwell_calc_cdclk()
3be8a28 drm/i915: Drop checks for max_pixclk failures in cdclk computation

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

Re: [PATCH 01/13] drm/i915: Drop checks for max_pixclk failures in cdclk computation

[Jani Nikula]

On Wed, 11 May 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> commit 565602d7501a ("drm/i915: Do not acquire crtc state to check clock during modeset, v4.")
> removed the possibility that intel_mode_max_pixclk() or
> ilk_max_pixel_rate() might return an error, so let's get rid of the
> error checks in the callers as well.
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/intel_display.c | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b60d9b67e033..775ab3cf1eee 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5957,9 +5957,6 @@ static int valleyview_modeset_calc_cdclk(struct drm_atomic_state *state)
>  	struct intel_atomic_state *intel_state =
>  		to_intel_atomic_state(state);
>  
> -	if (max_pixclk < 0)
> -		return max_pixclk;
> -
>  	intel_state->cdclk = intel_state->dev_cdclk =
>  		valleyview_calc_cdclk(dev_priv, max_pixclk);
>  
> @@ -5977,9 +5974,6 @@ static int broxton_modeset_calc_cdclk(struct drm_atomic_state *state)
>  	struct intel_atomic_state *intel_state =
>  		to_intel_atomic_state(state);
>  
> -	if (max_pixclk < 0)
> -		return max_pixclk;
> -
>  	intel_state->cdclk = intel_state->dev_cdclk =
>  		broxton_calc_cdclk(dev_priv, max_pixclk);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/13] drm/i915: Extract broadwell_calc_cdclk()

[Jani Nikula]

On Wed, 11 May 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Try to reduce the amount of duplicated cdclk magic numbers by
> moving the max_pixclk->cdclk conversion into a helper.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/intel_display.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 775ab3cf1eee..b55075e6020c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9675,6 +9675,18 @@ static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
>  	     cdclk, dev_priv->cdclk_freq);
>  }
>  
> +static int broadwell_calc_cdclk(int max_pixclk)
> +{
> +	if (max_pixclk > 540000)
> +		return 675000;
> +	else if (max_pixclk > 450000)
> +		return 540000;
> +	else if (max_pixclk > 337500)
> +		return 450000;
> +	else
> +		return 337500;
> +}
> +
>  static int broadwell_modeset_calc_cdclk(struct drm_atomic_state *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->dev);
> @@ -9686,14 +9698,7 @@ static int broadwell_modeset_calc_cdclk(struct drm_atomic_state *state)
>  	 * FIXME should also account for plane ratio
>  	 * once 64bpp pixel formats are supported.
>  	 */
> -	if (max_pixclk > 540000)
> -		cdclk = 675000;
> -	else if (max_pixclk > 450000)
> -		cdclk = 540000;
> -	else if (max_pixclk > 337500)
> -		cdclk = 450000;
> -	else
> -		cdclk = 337500;
> +	cdclk = broadwell_calc_cdclk(max_pixclk);
>  
>  	if (cdclk > dev_priv->max_cdclk_freq) {
>  		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> @@ -9703,7 +9708,7 @@ static int broadwell_modeset_calc_cdclk(struct drm_atomic_state *state)
>  
>  	intel_state->cdclk = intel_state->dev_cdclk = cdclk;
>  	if (!intel_state->active_crtcs)
> -		intel_state->dev_cdclk = 337500;
> +		intel_state->dev_cdclk = broadwell_calc_cdclk(0);
>  
>  	return 0;
>  }

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/13] drm/i915: Untangle .fdi_link_train and cdclk vfunc setup

[Jani Nikula]

On Wed, 11 May 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Split the .fdi_link_train and .modeset_commit_cdclk/.modeset_calc_cdclk
> into two separate if ladders. Much easier to read when you're not
> confusing two totally separate subjects.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/intel_display.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b55075e6020c..a9e5d6b81043 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15044,12 +15044,13 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
>  		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
>  	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
>  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
> -		if (IS_BROADWELL(dev_priv)) {
> -			dev_priv->display.modeset_commit_cdclk =
> -				broadwell_modeset_commit_cdclk;
> -			dev_priv->display.modeset_calc_cdclk =
> -				broadwell_modeset_calc_cdclk;
> -		}
> +	}
> +
> +	if (IS_BROADWELL(dev_priv)) {
> +		dev_priv->display.modeset_commit_cdclk =
> +			broadwell_modeset_commit_cdclk;
> +		dev_priv->display.modeset_calc_cdclk =
> +			broadwell_modeset_calc_cdclk;
>  	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
>  		dev_priv->display.modeset_commit_cdclk =
>  			valleyview_modeset_commit_cdclk;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 04/13] drm/i915: Don't pass dev_priv to broxton_calc_cdclk()

[Jani Nikula]

On Wed, 11 May 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> broxton_calc_cdclk() doesn't need dev_priv for anything, so let's not
> bother passing it around.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/intel_display.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a9e5d6b81043..deeaf3ba1dee 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5900,8 +5900,7 @@ static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
>  		return 200000;
>  }
>  
> -static int broxton_calc_cdclk(struct drm_i915_private *dev_priv,
> -			      int max_pixclk)
> +static int broxton_calc_cdclk(int max_pixclk)
>  {
>  	/*
>  	 * FIXME:
> @@ -5969,16 +5968,15 @@ static int valleyview_modeset_calc_cdclk(struct drm_atomic_state *state)
>  static int broxton_modeset_calc_cdclk(struct drm_atomic_state *state)
>  {
>  	struct drm_device *dev = state->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int max_pixclk = intel_mode_max_pixclk(dev, state);
>  	struct intel_atomic_state *intel_state =
>  		to_intel_atomic_state(state);
>  
>  	intel_state->cdclk = intel_state->dev_cdclk =
> -		broxton_calc_cdclk(dev_priv, max_pixclk);
> +		broxton_calc_cdclk(max_pixclk);
>  
>  	if (!intel_state->active_crtcs)
> -		intel_state->dev_cdclk = broxton_calc_cdclk(dev_priv, 0);
> +		intel_state->dev_cdclk = broxton_calc_cdclk(0);
>  
>  	return 0;
>  }

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/13] drm/i915: Use ilk_max_pixel_rate() for BXT cdclk calculation

[Jani Nikula]

On Wed, 11 May 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> BXT uses the "pch" panel fitter configuration, so we can use
> ilk_max_pixel_rate() instead of intel_mode_max_pixclk() to compute the
> pipe pixel rate. ilk_max_pixel_rate() will account for the pipe
> scaler downscaling factor whereas intel_mode_max_pixclk() will not.
>
> I'm pretty sure the same limitation is there on GMCH platforms, but
> no one just bothered to implement the downscaling adjustment for them.
> Probably should just unify the panel fitter setup more across the
> platforms and use the exact same code on all platforms for this.
> But in the meantime, let's at least make BXT a bit more correct.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/intel_display.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index deeaf3ba1dee..fd55112f266d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -117,6 +117,7 @@ static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
>  static void ironlake_pfit_enable(struct intel_crtc *crtc);
>  static void intel_modeset_setup_hw_state(struct drm_device *dev);
>  static void intel_pre_disable_primary_noatomic(struct drm_crtc *crtc);
> +static int ilk_max_pixel_rate(struct drm_atomic_state *state);
>  
>  typedef struct {
>  	int	min, max;
> @@ -5967,8 +5968,7 @@ static int valleyview_modeset_calc_cdclk(struct drm_atomic_state *state)
>  
>  static int broxton_modeset_calc_cdclk(struct drm_atomic_state *state)
>  {
> -	struct drm_device *dev = state->dev;
> -	int max_pixclk = intel_mode_max_pixclk(dev, state);
> +	int max_pixclk = ilk_max_pixel_rate(state);
>  	struct intel_atomic_state *intel_state =
>  		to_intel_atomic_state(state);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/13] drm/i915: Use skl_cdclk_decimal() on bxt

[Jani Nikula]

On Wed, 11 May 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Both SKL and BXT need to fill in the "decimal" cdclk frequency into
> the CDCLK_CTL register. SKL uses a small helper to do the kHz->"decimal"
> conversion, whereas BXT has it open-coded. Use the helper on BXT too.
>
> While at it, change it to round to closest rather than down. It doesn't
> actually matter with the frequencies we have to deal with, but it seems
> like the right thing to do.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fd55112f266d..9564719b53e3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5340,6 +5340,12 @@ static void intel_update_cdclk(struct drm_device *dev)
>  		intel_update_max_cdclk(dev);
>  }
>  
> +/* convert from kHz to .1 fixpoint MHz with -1MHz offset */
> +static int skl_cdclk_decimal(int cdclk)
> +{
> +	return DIV_ROUND_CLOSEST(cdclk - 1000, 500);
> +}
> +
>  static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int frequency)
>  {
>  	uint32_t divider;
> @@ -5439,8 +5445,7 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int frequency)
>  			val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
>  
>  		val &= ~CDCLK_FREQ_DECIMAL_MASK;
> -		/* convert from kHz to .1 fixpoint MHz with -1MHz offset */
> -		val |= (frequency - 1000) / 500;
> +		val |= skl_cdclk_decimal(frequency);
>  		I915_WRITE(CDCLK_CTL, val);
>  	}
>  
> @@ -5540,11 +5545,6 @@ static const struct skl_cdclk_entry {
>  	{ .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;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 08/13] drm/i915: Extract skl_dpll0_disable()

[Jani Nikula]

On Wed, 11 May 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Make thins a bit easier to read by extracting the SKL DPLL0
> disable into separate functions. We already have the enable
> counterpart. Down the line this will also help make the cdclk
> programming on SKL, BXT, and following platforms look rather
> consistent.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/intel_display.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 57ff51172065..0186f775f353 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5610,6 +5610,14 @@ skl_dpll0_enable(struct drm_i915_private *dev_priv, unsigned int required_vco)
>  		DRM_ERROR("DPLL0 not locked\n");
>  }
>  
> +static void
> +skl_dpll0_disable(struct drm_i915_private *dev_priv)
> +{
> +	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");
> +}
> +
>  static bool skl_cdclk_pcu_ready(struct drm_i915_private *dev_priv)
>  {
>  	int ret;
> @@ -5695,10 +5703,7 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
>  	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");
> +	skl_dpll0_disable(dev_priv);
>  }
>  
>  void skl_init_cdclk(struct drm_i915_private *dev_priv)

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/13] drm/i915: Kill off dead code from skl_dpll0_enable()

[Jani Nikula]

On Wed, 11 May 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We calculate the CDCLK_CTL value from scratch so no need to attempt
> some form of RMW first.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/intel_display.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0186f775f353..a21f9d3fb869 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5566,17 +5566,12 @@ skl_dpll0_enable(struct drm_i915_private *dev_priv, unsigned int required_vco)
>  	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);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/13] drm/i915: s/freq/cdclk/

[Jani Nikula]

On Wed, 11 May 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Rename the generic sounding freq/frequency parameters to the cdclk
> functions to 'cdclk' so that we'll know which clock we're talking about
> once we have to deal with the vco frequencies as well.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/intel_display.c | 44 ++++++++++++++++++------------------
>  1 file changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a21f9d3fb869..f58bbac6204b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5346,15 +5346,15 @@ static int skl_cdclk_decimal(int cdclk)
>  	return DIV_ROUND_CLOSEST(cdclk - 1000, 500);
>  }
>  
> -static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int frequency)
> +static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
>  {
>  	uint32_t divider;
>  	uint32_t ratio;
> -	uint32_t current_freq;
> +	uint32_t current_cdclk;
>  	int ret;
>  
>  	/* frequency = 19.2MHz * ratio / 2 / div{1,1.5,2,4} */
> -	switch (frequency) {
> +	switch (cdclk) {
>  	case 144000:
>  		divider = BXT_CDCLK_CD2X_DIV_SEL_4;
>  		ratio = BXT_DE_PLL_RATIO(60);
> @@ -5384,7 +5384,7 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int frequency)
>  		divider = 0;
>  		break;
>  	default:
> -		DRM_ERROR("unsupported CDCLK freq %d", frequency);
> +		DRM_ERROR("unsupported CDCLK freq %d", cdclk);
>  
>  		return;
>  	}
> @@ -5397,13 +5397,13 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int frequency)
>  
>  	if (ret) {
>  		DRM_ERROR("PCode CDCLK freq change notify failed (err %d, freq %d)\n",
> -			  ret, frequency);
> +			  ret, cdclk);
>  		return;
>  	}
>  
> -	current_freq = I915_READ(CDCLK_CTL) & CDCLK_FREQ_DECIMAL_MASK;
> +	current_cdclk = I915_READ(CDCLK_CTL) & CDCLK_FREQ_DECIMAL_MASK;
>  	/* convert from .1 fixpoint MHz with -1MHz offset to kHz */
> -	current_freq = current_freq * 500 + 1000;
> +	current_cdclk = current_cdclk * 500 + 1000;
>  
>  	/*
>  	 * DE PLL has to be disabled when
> @@ -5411,8 +5411,8 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int frequency)
>  	 * - before setting to 624MHz (PLL needs toggling)
>  	 * - before setting to any frequency from 624MHz (PLL needs toggling)
>  	 */
> -	if (frequency == 19200 || frequency == 624000 ||
> -	    current_freq == 624000) {
> +	if (cdclk == 19200 || cdclk == 624000 ||
> +	    current_cdclk == 624000) {
>  		I915_WRITE(BXT_DE_PLL_ENABLE, ~BXT_DE_PLL_PLL_ENABLE);
>  		/* Timeout 200us */
>  		if (wait_for(!(I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_LOCK),
> @@ -5420,7 +5420,7 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int frequency)
>  			DRM_ERROR("timout waiting for DE PLL unlock\n");
>  	}
>  
> -	if (frequency != 19200) {
> +	if (cdclk != 19200) {
>  		uint32_t val;
>  
>  		val = I915_READ(BXT_DE_PLL_CTL);
> @@ -5441,22 +5441,22 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int frequency)
>  		 * enable otherwise.
>  		 */
>  		val &= ~BXT_CDCLK_SSA_PRECHARGE_ENABLE;
> -		if (frequency >= 500000)
> +		if (cdclk >= 500000)
>  			val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
>  
>  		val &= ~CDCLK_FREQ_DECIMAL_MASK;
> -		val |= skl_cdclk_decimal(frequency);
> +		val |= skl_cdclk_decimal(cdclk);
>  		I915_WRITE(CDCLK_CTL, val);
>  	}
>  
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  	ret = sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ,
> -				      DIV_ROUND_UP(frequency, 25000));
> +				      DIV_ROUND_UP(cdclk, 25000));
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
>  	if (ret) {
>  		DRM_ERROR("PCode CDCLK freq set failed, (err %d, freq %d)\n",
> -			  ret, frequency);
> +			  ret, cdclk);
>  		return;
>  	}
>  
> @@ -5562,16 +5562,16 @@ static unsigned int skl_cdclk_get_vco(unsigned int freq)
>  static void
>  skl_dpll0_enable(struct drm_i915_private *dev_priv, unsigned int required_vco)
>  {
> -	unsigned int min_freq;
> +	int min_cdclk;
>  	u32 val;
>  
>  	/* select the minimum CDCLK before enabling DPLL 0 */
>  	if (required_vco == 8640)
> -		min_freq = 308570;
> +		min_cdclk = 308570;
>  	else
> -		min_freq = 337500;
> +		min_cdclk = 337500;
>  
> -	val = CDCLK_FREQ_337_308 | skl_cdclk_decimal(min_freq);
> +	val = CDCLK_FREQ_337_308 | skl_cdclk_decimal(min_cdclk);
>  	I915_WRITE(CDCLK_CTL, val);
>  	POSTING_READ(CDCLK_CTL);
>  
> @@ -5640,12 +5640,12 @@ static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv)
>  	return false;
>  }
>  
> -static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
> +static void skl_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
>  {
>  	struct drm_device *dev = dev_priv->dev;
>  	u32 freq_select, pcu_ack;
>  
> -	DRM_DEBUG_DRIVER("Changing CDCLK to %dKHz\n", freq);
> +	DRM_DEBUG_DRIVER("Changing CDCLK to %dKHz\n", cdclk);
>  
>  	if (!skl_cdclk_wait_for_pcu_ready(dev_priv)) {
>  		DRM_ERROR("failed to inform PCU about cdclk change\n");
> @@ -5653,7 +5653,7 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
>  	}
>  
>  	/* set CDCLK_CTL */
> -	switch(freq) {
> +	switch (cdclk) {
>  	case 450000:
>  	case 432000:
>  		freq_select = CDCLK_FREQ_450_432;
> @@ -5676,7 +5676,7 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq)
>  		break;
>  	}
>  
> -	I915_WRITE(CDCLK_CTL, freq_select | skl_cdclk_decimal(freq));
> +	I915_WRITE(CDCLK_CTL, freq_select | skl_cdclk_decimal(cdclk));
>  	POSTING_READ(CDCLK_CTL);
>  
>  	/* inform PCU of the change */

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 11/13] drm/i915: s/required_vco/vco/ in skl cdclk code

[Jani Nikula]

On Wed, 11 May 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The 'required' part of 'required_vco' should be obvious. Let's just call
> it 'vco' for brevity.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/intel_display.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f58bbac6204b..61133fdedc8d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5560,13 +5560,13 @@ static unsigned int skl_cdclk_get_vco(unsigned int freq)
>  }
>  
>  static void
> -skl_dpll0_enable(struct drm_i915_private *dev_priv, unsigned int required_vco)
> +skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco)
>  {
>  	int min_cdclk;
>  	u32 val;
>  
>  	/* select the minimum CDCLK before enabling DPLL 0 */
> -	if (required_vco == 8640)
> +	if (vco == 8640)
>  		min_cdclk = 308570;
>  	else
>  		min_cdclk = 337500;
> @@ -5589,7 +5589,7 @@ skl_dpll0_enable(struct drm_i915_private *dev_priv, unsigned int required_vco)
>  	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)
> +	if (vco == 8640)
>  		val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080,
>  					    SKL_DPLL0);
>  	else
> @@ -5703,13 +5703,13 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
>  
>  void skl_init_cdclk(struct drm_i915_private *dev_priv)
>  {
> -	unsigned int required_vco;
> +	unsigned int vco;
>  
>  	/* DPLL0 not enabled (happens on early BIOS versions) */
>  	if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) {
>  		/* enable DPLL0 */
> -		required_vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk);
> -		skl_dpll0_enable(dev_priv, required_vco);
> +		vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk);
> +		skl_dpll0_enable(dev_priv, vco);
>  	}
>  
>  	/* set CDCLK to the frequency the BIOS chose */

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Ro.CI.BAT: failure for drm/i915: cdclk code cleanup

[Ville Syrjälä]

On Wed, May 11, 2016 at 08:23:00PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: cdclk code cleanup
> URL   : https://patchwork.freedesktop.org/series/7038/
> State : failure
> 
> == Summary ==
> 
> Series 7038v1 drm/i915: cdclk code cleanup
> http://patchwork.freedesktop.org/api/1.0/series/7038/revisions/1/mbox
> 
> Test drv_hangman:
>         Subgroup error-state-basic:
>                 pass       -> FAIL       (ro-ilk1-i5-650)

(drv_hangman:6245) CRITICAL: contents of i915_error_state: 'no error state collected' (expected not 'no error state collected'
https://bugs.freedesktop.org/show_bug.cgi?id=94305

> Test gem_exec_flush:
>         Subgroup basic-batch-kernel-default-cmd:
>                 fail       -> PASS       (ro-byt-n2820)
> Test kms_flip:
>         Subgroup basic-flip-vs-wf_vblank:
>                 fail       -> PASS       (ro-ivb2-i7-3770)
> Test kms_pipe_crc_basic:
>         Subgroup hang-read-crc-pipe-a:
>                 pass       -> DMESG-WARN (ro-ivb2-i7-3770)

[  520.693920] ------------[ cut here ]------------
[  520.693953] WARNING: CPU: 4 PID: 7798 at drivers/gpu/drm/i915/intel_display.c:13525 intel_atomic_commit+0x138d/0x1440 [i915]
[  520.693955] pipe A vblank wait timed out
[  520.693957] Modules linked in: snd_hda_intel i915 drm_kms_helper drm snd_hda_codec_realtek snd_hda_codec_generic i2c_algo_bit syscopyarea sysfillrect sysimgblt fb_sys_fops snd_hda_codec snd_hwdep snd_hda_core x86_pkg_temp_thermal intel_powerclamp lpc_ich snd_pcm mei_me mei coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel r8169 mii [last unloaded: drm]
[  520.693977] CPU: 4 PID: 7798 Comm: kms_pipe_crc_ba Tainted: G     U          4.6.0-rc7-gfxbench-RO_Patchwork_856+ #1
[  520.693978] Hardware name: Hewlett-Packard HP Pro 3500 Series/2ABF, BIOS 8.11 10/24/2012
[  520.693979]  0000000000000000 ffff8800d68c3b28 ffffffff81409235 ffff8800d68c3b78
[  520.693981]  0000000000000000 ffff8800d68c3b68 ffffffff81079d66 000034d500000282
[  520.693983]  0000000000000000 ffff8800d5c5a2a8 0000000000000000 0000000000000000
[  520.693985] Call Trace:
[  520.693988]  [<ffffffff81409235>] dump_stack+0x67/0x92
[  520.693991]  [<ffffffff81079d66>] __warn+0xc6/0xe0
[  520.693992]  [<ffffffff81079dca>] warn_slowpath_fmt+0x4a/0x50
[  520.693994]  [<ffffffff810c3cb9>] ? finish_wait+0x59/0x70
[  520.694009]  [<ffffffffa04ebd2d>] intel_atomic_commit+0x138d/0x1440 [i915]
[  520.694011]  [<ffffffff810c3e80>] ? wake_atomic_t_function+0x60/0x60
[  520.694023]  [<ffffffffa0437022>] drm_atomic_commit+0x32/0x50 [drm]
[  520.694031]  [<ffffffffa01018ac>] drm_atomic_helper_set_config+0x7c/0xb0 [drm_kms_helper]
[  520.694039]  [<ffffffffa0425a00>] drm_mode_set_config_internal+0x60/0x110 [drm]
[  520.694047]  [<ffffffffa042a9d0>] drm_mode_setcrtc+0x410/0x540 [drm]
[  520.694053]  [<ffffffffa041c9ae>] drm_ioctl+0x13e/0x530 [drm]
[  520.694059]  [<ffffffffa042a5c0>] ? drm_mode_setplane+0x1c0/0x1c0 [drm]
[  520.694061]  [<ffffffff810e57bd>] ? debug_lockdep_rcu_enabled+0x1d/0x20
[  520.694064]  [<ffffffff811f065e>] do_vfs_ioctl+0x8e/0x660
[  520.694065]  [<ffffffff810e57bd>] ? debug_lockdep_rcu_enabled+0x1d/0x20
[  520.694067]  [<ffffffff811fc50a>] ? __fget_light+0x6a/0x90
[  520.694069]  [<ffffffff811f0c6c>] SyS_ioctl+0x3c/0x70
[  520.694071]  [<ffffffff817a17a9>] entry_SYSCALL_64_fastpath+0x1c/0xac
[  520.694072] ---[ end trace 5d06b745269e1aff ]---
[  534.612060] drm/i915: Resetting chip after gpu hang

Smells a lot like
https://bugs.freedesktop.org/show_bug.cgi?id=95125
except this is an IVB whereas previously it was only seen on SNB.

>         Subgroup nonblocking-crc-pipe-a-frame-sequence:
>                 fail       -> PASS       (ro-snb-i7-2620M)
> 
> ro-bdw-i5-5250u  total:219  pass:181  dwarn:0   dfail:0   fail:0   skip:38 
> ro-bdw-i7-5600u  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
> ro-bsw-n3050     total:219  pass:175  dwarn:0   dfail:0   fail:2   skip:42 
> ro-byt-n2820     total:218  pass:175  dwarn:0   dfail:0   fail:2   skip:41 
> ro-hsw-i3-4010u  total:218  pass:193  dwarn:0   dfail:0   fail:0   skip:25 
> ro-hsw-i7-4770r  total:219  pass:194  dwarn:0   dfail:0   fail:0   skip:25 
> ro-ilk-i7-620lm  total:219  pass:151  dwarn:0   dfail:0   fail:1   skip:67 
> ro-ilk1-i5-650   total:214  pass:151  dwarn:0   dfail:0   fail:2   skip:61 
> ro-ivb-i7-3770   total:219  pass:183  dwarn:0   dfail:0   fail:0   skip:36 
> ro-ivb2-i7-3770  total:219  pass:186  dwarn:1   dfail:0   fail:0   skip:32 
> ro-skl-i7-6700hq total:214  pass:190  dwarn:0   dfail:0   fail:0   skip:24 
> ro-snb-i7-2620M  total:219  pass:177  dwarn:0   dfail:0   fail:1   skip:41 
> ro-bdw-i7-5557U failed to connect after reboot
> 
> Results at /archive/results/CI_IGT_test/RO_Patchwork_856/
> 
> 576b6d9 drm-intel-nightly: 2016y-05m-11d-17h-10m-57s UTC integration manifest
> b1238a0 drm/i915: Eliminate the CDCLK_CTL RMW on BXT
> b2a049e drm/i915: Program BXT_CDCLK_CD2X_PIPE
> 6c649e1 drm/i915: s/required_vco/vco/ in skl cdclk code
> b953b1f drm/i915: s/freq/cdclk/
> 339731a drm/i915: Kill off dead code from skl_dpll0_enable()
> 43ac272 drm/i915: Extract skl_dpll0_disable()
> 5939fa9 drm/i915: Remove 10% cdclk guardband on BXT
> b4269a8 drm/i915: Use skl_cdclk_decimal() on bxt
> 4ed70f5 drm/i915: Use ilk_max_pixel_rate() for BXT cdclk calculation
> e79c6f8 drm/i915: Don't pass dev_priv to broxton_calc_cdclk()
> 5efaf3f drm/i915: Untangle .fdi_link_train and cdclk vfunc setup
> 57c6353 drm/i915: Extract broadwell_calc_cdclk()
> 3be8a28 drm/i915: Drop checks for max_pixclk failures in cdclk computation

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

Re: [PATCH 07/13] drm/i915: Remove 10% cdclk guardband on BXT

[Imre Deak]

On Wed, 2016-05-11 at 22:44 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We don't need any pixel clock vs. cdclk guardband since HSW. BXT still
> tries to add one though. Get rid of it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Matches "Gen9 Display Resolution Support" which only sets scaling
specific limits and doesn't mention the "90% rule" required by the IVB
"Pixel Rate Limitations":
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9564719b53e3..57ff51172065 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5905,16 +5905,15 @@ static int broxton_calc_cdclk(int max_pixclk)
>  {
>  	/*
>  	 * FIXME:
> -	 * - remove the guardband, it's not needed on BXT
>  	 * - set 19.2MHz bypass frequency if there are no active pipes
>  	 */
> -	if (max_pixclk > 576000*9/10)
> +	if (max_pixclk > 576000)
>  		return 624000;
> -	else if (max_pixclk > 384000*9/10)
> +	else if (max_pixclk > 384000)
>  		return 576000;
> -	else if (max_pixclk > 288000*9/10)
> +	else if (max_pixclk > 288000)
>  		return 384000;
> -	else if (max_pixclk > 144000*9/10)
> +	else if (max_pixclk > 144000)
>  		return 288000;
>  	else
>  		return 144000;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 12/13] drm/i915: Program BXT_CDCLK_CD2X_PIPE

[Imre Deak]

On Wed, 2016-05-11 at 22:44 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> BXT could change the CD2X divider synchronized with a single pipe.
> So assuming the DE PLL frequency doesn't need to be changed, we could
> change cdclk without shutting off the pipe (when only a single pipe is
> enabled). In the meantime let's configure CDCLK_CTL for non-double
> buffered CD2X update, although it shouldn't really matter as long as
> the selected pipe is disabled when reprogramming the divider.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 5 +++--
>  drivers/gpu/drm/i915/intel_display.c | 5 +++++
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 54ce0b105956..27a781260d33 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7566,14 +7566,15 @@ enum skl_disp_power_wells {
>  #define  CDCLK_FREQ_540			(1<<26)
>  #define  CDCLK_FREQ_337_308		(2<<26)
>  #define  CDCLK_FREQ_675_617		(3<<26)
> -#define  CDCLK_FREQ_DECIMAL_MASK	(0x7ff)
> -
> +#define  BXT_CDCLK_CD2X_PIPE(pipe)	((pipe)<<20)
> +#define  BXT_CDCLK_CD2X_PIPE_NONE	BXT_CDCLK_CD2X_PIPE(3)
>  #define  BXT_CDCLK_CD2X_DIV_SEL_MASK	(3<<22)
>  #define  BXT_CDCLK_CD2X_DIV_SEL_1	(0<<22)
>  #define  BXT_CDCLK_CD2X_DIV_SEL_1_5	(1<<22)
>  #define  BXT_CDCLK_CD2X_DIV_SEL_2	(2<<22)
>  #define  BXT_CDCLK_CD2X_DIV_SEL_4	(3<<22)
>  #define  BXT_CDCLK_SSA_PRECHARGE_ENABLE	(1<<16)
> +#define  CDCLK_FREQ_DECIMAL_MASK	(0x7ff)
>  
>  /* LCPLL_CTL */
>  #define LCPLL1_CTL		_MMIO(0x46010)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 61133fdedc8d..39990bfe47f2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5434,6 +5434,11 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
>  			DRM_ERROR("timeout waiting for DE PLL lock\n");
>  
>  		val = I915_READ(CDCLK_CTL);
> +		/*
> +		 * FIXME if only the cd2x divider needs changing, it could be done
> +		 * without shutting off the pipe (if only one pipe is active).
> +		 */
> +		val |= BXT_CDCLK_CD2X_PIPE_NONE;
>  		val &= ~BXT_CDCLK_CD2X_DIV_SEL_MASK;
>  		val |= divider;
>  		/*
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 13/13] drm/i915: Eliminate the CDCLK_CTL RMW on BXT

[Imre Deak]

On Wed, 2016-05-11 at 22:44 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> All the fields in CDCLK_CTL we don't program should be left at zero, so
> let's just get rid of the RMW.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_display.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 39990bfe47f2..3b6b9de968c5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5433,24 +5433,18 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
>  		if (wait_for(I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_LOCK, 1))
>  			DRM_ERROR("timeout waiting for DE PLL lock\n");
>  
> -		val = I915_READ(CDCLK_CTL);
> +		val = divider | skl_cdclk_decimal(cdclk);
>  		/*
>  		 * FIXME if only the cd2x divider needs changing, it could be done
>  		 * without shutting off the pipe (if only one pipe is active).
>  		 */
>  		val |= BXT_CDCLK_CD2X_PIPE_NONE;
> -		val &= ~BXT_CDCLK_CD2X_DIV_SEL_MASK;
> -		val |= divider;
>  		/*
>  		 * Disable SSA Precharge when CD clock frequency < 500 MHz,
>  		 * enable otherwise.
>  		 */
> -		val &= ~BXT_CDCLK_SSA_PRECHARGE_ENABLE;
>  		if (cdclk >= 500000)
>  			val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
> -
> -		val &= ~CDCLK_FREQ_DECIMAL_MASK;
> -		val |= skl_cdclk_decimal(cdclk);
>  		I915_WRITE(CDCLK_CTL, val);
>  	}
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/13] drm/i915: cdclk code cleanup

[Ville Syrjälä]

On Wed, May 11, 2016 at 10:44:39PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Here's a bunch of cleanup, refactoring, etc. of the cdclk code. Much of
> this serves as prep work for a larger series I have lined up which
> takes Clint's SKL cdclk patch, expands on it, and then tries to unify
> a lot of the SKL and BXT cdclk and display core init/uninit code. The
> final goal is some future hardware enabling, and I really want that to
> follow the same unified style as much as possible.
> 
> Anyways, I thought I'd try to get some of the less controversial cleanups
> out of the way first, and so I'm sending out this series first.
> 
> Series available here:
> git://github.com/vsyrjala/linux.git skl_bxt_cdclk_part_1
> 
> Ville Syrjälä (13):
>   drm/i915: Drop checks for max_pixclk failures in cdclk computation
>   drm/i915: Extract broadwell_calc_cdclk()
>   drm/i915: Untangle .fdi_link_train and cdclk vfunc setup
>   drm/i915: Don't pass dev_priv to broxton_calc_cdclk()
>   drm/i915: Use ilk_max_pixel_rate() for BXT cdclk calculation
>   drm/i915: Use skl_cdclk_decimal() on bxt
>   drm/i915: Remove 10% cdclk guardband on BXT
>   drm/i915: Extract skl_dpll0_disable()
>   drm/i915: Kill off dead code from skl_dpll0_enable()
>   drm/i915: s/freq/cdclk/
>   drm/i915: s/required_vco/vco/ in skl cdclk code
>   drm/i915: Program BXT_CDCLK_CD2X_PIPE
>   drm/i915: Eliminate the CDCLK_CTL RMW on BXT

Entire series pushed to dinq. Thanks for the reviews.

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