[PATCH 00/11] drm/i915: rawclk/cdclk stuff

[PATCH 00/11] drm/i915: rawclk/cdclk stuff

[ville.syrjala]

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

Imre and Jani were discussing issues related to the rawclk/cdclk->pwm
frequency conversions, so I figured it's a good time to post whatever
cleanups and fixes I have lying around for that code. So here it is

Entire series avaible here:
git://github.com/vsyrjala/linux.git rawclk_freq_8

Ville Syrjälä (11):
  drm/i915: Fix VBT backlight Hz to PWM conversion for PNV
  drm/i915: Fix vbt PWM max setup for CTG
  drm/i915: Add HAS_PCH_LPT_H()
  drm/i915: Kill duplicated PNV .get_display_clock_speed() assignment
  drm/i915: Round the AUX clock divider to closest on all platforms
  drm/i915: Use cached cdclk_freq for PWM calculations
  drm/i915: Store rawclk_freq in dev_priv
  drm/i915: Rename s/i9xx/g4x/ in DP code
  drm/i915: Use g4x_get_aux_clock_divider() for VLV/CHV
  drm/i915: Read out hrawclk from CCK on vlv/chv
  drm/i915: Clean up .get_aux_clock_divider() functions

 drivers/gpu/drm/i915/i915_drv.h      |  2 ++
 drivers/gpu/drm/i915/i915_reg.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c | 59 ++++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_dp.c      | 61 ++++++++++++++++--------------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 --
 drivers/gpu/drm/i915/intel_panel.c   | 36 +++++++++++----------
 6 files changed, 85 insertions(+), 76 deletions(-)

-- 
2.4.10

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

[PATCH 01/11] drm/i915: Fix VBT backlight Hz to PWM conversion for PNV

[ville.syrjala]

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

Convert the MHz number coming from intel_rawclk() into Hz in
i9xx_hz_to_pwm() on PNV. Otherwise we'll get something totally
bogus as a result.

Fixes: aa17cdb4f836 ("drm/i915: initialize backlight max from VBT")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_panel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index a24df35e11e7..ea528ca854e8 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1335,7 +1335,7 @@ static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 	int clock;
 
 	if (IS_PINEVIEW(dev))
-		clock = intel_hrawclk(dev);
+		clock = MHz(intel_hrawclk(dev));
 	else
 		clock = 1000 * dev_priv->display.get_display_clock_speed(dev);
 
-- 
2.4.10

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

[PATCH 02/11] drm/i915: Fix vbt PWM max setup for CTG

[ville.syrjala]

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

CTG uses hrawclk for backlight, so calculate the max based on that
instead of cdclk.

Fixes: aa17cdb4f836 ("drm/i915: initialize backlight max from VBT")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_panel.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index ea528ca854e8..737e12349e81 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1344,13 +1344,19 @@ static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 
 /*
  * Gen4: This value represents the period of the PWM stream in display core
- * clocks multiplied by 128.
+ * clocks ([DevCTG] HRAW clocks) multiplied by 128.
+ *
  */
 static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 {
 	struct drm_device *dev = connector->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int clock = 1000 * dev_priv->display.get_display_clock_speed(dev);
+	int clock;
+
+	if (IS_G4X(dev_priv))
+		clock = MHz(intel_hrawclk(dev));
+	else
+		clock = 1000 * dev_priv->display.get_display_clock_speed(dev);
 
 	return clock / (pwm_freq_hz * 128);
 }
-- 
2.4.10

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

[PATCH 03/11] drm/i915: Add HAS_PCH_LPT_H()

[ville.syrjala]

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

We have HAS_PCH_LPT_LP() already, so add HAS_PCH_LPT_H() and use it
where appropriate.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h    | 1 +
 drivers/gpu/drm/i915/intel_dp.c    | 2 +-
 drivers/gpu/drm/i915/intel_panel.c | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bc865e234df2..9ab3e25ddf38 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2615,6 +2615,7 @@ struct drm_i915_cmd_table {
 #define HAS_PCH_SPT(dev) (INTEL_PCH_TYPE(dev) == PCH_SPT)
 #define HAS_PCH_LPT(dev) (INTEL_PCH_TYPE(dev) == PCH_LPT)
 #define HAS_PCH_LPT_LP(dev) (__I915__(dev)->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE)
+#define HAS_PCH_LPT_H(dev) (__I915__(dev)->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE)
 #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT)
 #define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX)
 #define HAS_PCH_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f2bfca0ae6b6..77ae5821e9c0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -711,7 +711,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 		if (index)
 			return 0;
 		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
-	} else if (dev_priv->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) {
+	} else if (HAS_PCH_LPT_H(dev_priv)) {
 		/* Workaround for non-ULT HSW */
 		switch (index) {
 		case 0: return 63;
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 737e12349e81..d4b18b56f3ca 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1300,7 +1300,7 @@ static u32 lpt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 	else
 		mul = 128;
 
-	if (dev_priv->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE)
+	if (HAS_PCH_LPT_H(dev_priv))
 		clock = MHz(135); /* LPT:H */
 	else
 		clock = MHz(24); /* LPT:LP */
-- 
2.4.10

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

[PATCH 04/11] drm/i915: Kill duplicated PNV .get_display_clock_speed() assignment

[ville.syrjala]

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

Somehow we accumulated a duplicated .get_display_clock_speed()
assignment for PNV in
commit 34edce2fea69 ("drm/i915: Add cdclk extraction for g33, g965gm and g4x")

No real harm on having two, we just never reach the second one, so
simply kill it.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8dbe5b398af4..84395c8c9dce 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14791,9 +14791,6 @@ static void intel_init_display(struct drm_device *dev)
 	else if (IS_I945GM(dev) || IS_845G(dev))
 		dev_priv->display.get_display_clock_speed =
 			i9xx_misc_get_display_clock_speed;
-	else if (IS_PINEVIEW(dev))
-		dev_priv->display.get_display_clock_speed =
-			pnv_get_display_clock_speed;
 	else if (IS_I915GM(dev))
 		dev_priv->display.get_display_clock_speed =
 			i915gm_get_display_clock_speed;
-- 
2.4.10

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

[PATCH 05/11] drm/i915: Round the AUX clock divider to closest on all platforms

[ville.syrjala]

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

Currently we round the AUX clock divider down on g4x, to closest
on HSW/BDW port A, and up everywhere else. We are supposed to get
as close to 2MHz as we can, so round to closest seems like the
best option.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 77ae5821e9c0..f335c92b4fa7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -681,7 +681,7 @@ static uint32_t i9xx_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 	 * The clock divider is based off the hrawclk, and would like to run at
 	 * 2MHz.  So, take the hrawclk value and divide by 2 and use that
 	 */
-	return index ? 0 : intel_hrawclk(dev) / 2;
+	return index ? 0 : DIV_ROUND_CLOSEST(intel_hrawclk(dev), 2);
 }
 
 static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
@@ -694,10 +694,10 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 		return 0;
 
 	if (intel_dig_port->port == PORT_A) {
-		return DIV_ROUND_UP(dev_priv->cdclk_freq, 2000);
+		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
 
 	} else {
-		return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
+		return DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
 	}
 }
 
@@ -719,7 +719,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 		default: return 0;
 		}
 	} else  {
-		return index ? 0 : DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
+		return index ? 0 : DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
 	}
 }
 
-- 
2.4.10

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

[PATCH 06/11] drm/i915: Use cached cdclk_freq for PWM calculations

[ville.syrjala]

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

No need to read out cdclk from the hardware, we have it already
cached in dev_priv.

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

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index d4b18b56f3ca..891a587225e2 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1337,7 +1337,7 @@ static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 	if (IS_PINEVIEW(dev))
 		clock = MHz(intel_hrawclk(dev));
 	else
-		clock = 1000 * dev_priv->display.get_display_clock_speed(dev);
+		clock = 1000 * dev_priv->cdclk_freq;
 
 	return clock / (pwm_freq_hz * 32);
 }
@@ -1356,7 +1356,7 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 	if (IS_G4X(dev_priv))
 		clock = MHz(intel_hrawclk(dev));
 	else
-		clock = 1000 * dev_priv->display.get_display_clock_speed(dev);
+		clock = 1000 * dev_priv->cdclk_freq;
 
 	return clock / (pwm_freq_hz * 128);
 }
-- 
2.4.10

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

[PATCH 07/11] drm/i915: Store rawclk_freq in dev_priv

[ville.syrjala]

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

Generalize rawclk handling by storing it in dev_priv.

Presumably our hrawclk readout works at least for CTG and ELK
since we've been using it for DP AUX on those platforms. There
are no real docs anymore after configdb vanished, so the only
reference is the public CTG GMCH spec. What bits are listed in
that doc match our code. The ELK GMCH spec have no relevant
details unfortunately.

The PNV situation is less clear. Starting from
commit aa17cdb4f836 ("drm/i915: initialize backlight max from VBT")
we assume that the CTG/ELK hrawclk readout works for PNV as well.
At least the results *seem* reasonable for one PNV machine (Lenovo
Ideapad S10-3t). Sadly the PNV GMCH spec doesn't have the goods on
the relevant register either.

So let's keep assuming it works for PNV,ELK,CTG and read it out on
those platforms. G33 also has hrawclk according to some notes
in BSpec, but we don't actually need it for anything, so let's not
even try to read it out there.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_dp.c      | 16 +++++------
 drivers/gpu/drm/i915/intel_drv.h     |  2 --
 drivers/gpu/drm/i915/intel_panel.c   | 24 ++++++++--------
 5 files changed, 53 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9ab3e25ddf38..64facd410037 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1781,6 +1781,7 @@ struct drm_i915_private {
 	unsigned int skl_boot_cdclk;
 	unsigned int cdclk_freq, max_cdclk_freq;
 	unsigned int max_dotclk_freq;
+	unsigned int rawclk_freq;
 	unsigned int hpll_freq;
 	unsigned int czclk_freq;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 84395c8c9dce..84a1359ecba5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -168,49 +168,61 @@ static int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
 	return DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, divider + 1);
 }
 
-int
-intel_pch_rawclk(struct drm_device *dev)
+static int
+intel_pch_rawclk(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	WARN_ON(!HAS_PCH_SPLIT(dev));
+	return (I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK) * 1000;
+}
 
-	return I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK;
+static int
+intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
+{
+	return 200000;
 }
 
-/* hrawclock is 1/4 the FSB frequency */
-int intel_hrawclk(struct drm_device *dev)
+static int
+intel_g4x_hrawclk(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t clkcfg;
 
-	/* There is no CLKCFG reg in Valleyview. VLV hrawclk is 200 MHz */
-	if (IS_VALLEYVIEW(dev))
-		return 200;
-
+	/* hrawclock is 1/4 the FSB frequency */
 	clkcfg = I915_READ(CLKCFG);
 	switch (clkcfg & CLKCFG_FSB_MASK) {
 	case CLKCFG_FSB_400:
-		return 100;
+		return 100000;
 	case CLKCFG_FSB_533:
-		return 133;
+		return 133333;
 	case CLKCFG_FSB_667:
-		return 166;
+		return 166667;
 	case CLKCFG_FSB_800:
-		return 200;
+		return 200000;
 	case CLKCFG_FSB_1067:
-		return 266;
+		return 266667;
 	case CLKCFG_FSB_1333:
-		return 333;
+		return 333333;
 	/* these two are just a guess; one of them might be right */
 	case CLKCFG_FSB_1600:
 	case CLKCFG_FSB_1600_ALT:
-		return 400;
+		return 400000;
 	default:
-		return 133;
+		return 133333;
 	}
 }
 
+static void intel_update_rawclk(struct drm_i915_private *dev_priv)
+{
+	if (HAS_PCH_SPLIT(dev_priv))
+		dev_priv->rawclk_freq = intel_pch_rawclk(dev_priv);
+	else if (IS_VALLEYVIEW(dev_priv))
+		dev_priv->rawclk_freq = intel_vlv_hrawclk(dev_priv);
+	else if (IS_G4X(dev_priv) || IS_PINEVIEW(dev_priv))
+		dev_priv->rawclk_freq = intel_g4x_hrawclk(dev_priv);
+	else
+		return; /* no rawclk on other platforms, or no need to know it */
+
+	DRM_DEBUG_DRIVER("rawclk rate: %d kHz\n", dev_priv->rawclk_freq);
+}
+
 static void intel_update_czclk(struct drm_i915_private *dev_priv)
 {
 	if (!IS_VALLEYVIEW(dev_priv))
@@ -15145,6 +15157,7 @@ void intel_modeset_init(struct drm_device *dev)
 	}
 
 	intel_update_czclk(dev_priv);
+	intel_update_rawclk(dev_priv);
 	intel_update_cdclk(dev);
 
 	intel_shared_dpll_init(dev);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f335c92b4fa7..2a9b5710ee83 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -675,13 +675,13 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
 static uint32_t i9xx_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
 
 	/*
 	 * The clock divider is based off the hrawclk, and would like to run at
 	 * 2MHz.  So, take the hrawclk value and divide by 2 and use that
 	 */
-	return index ? 0 : DIV_ROUND_CLOSEST(intel_hrawclk(dev), 2);
+	return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
 }
 
 static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
@@ -693,12 +693,10 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 	if (index)
 		return 0;
 
-	if (intel_dig_port->port == PORT_A) {
+	if (intel_dig_port->port == PORT_A)
 		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
-
-	} else {
-		return DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
-	}
+	else
+		return DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
 }
 
 static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
@@ -719,7 +717,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 		default: return 0;
 		}
 	} else  {
-		return index ? 0 : DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
+		return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
 	}
 }
 
@@ -5237,7 +5235,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 pp_on, pp_off, pp_div, port_sel = 0;
-	int div = HAS_PCH_SPLIT(dev) ? intel_pch_rawclk(dev) : intel_hrawclk(dev);
+	int div = dev_priv->rawclk_freq / 1000;
 	i915_reg_t pp_on_reg, pp_off_reg, pp_div_reg, pp_ctrl_reg;
 	enum port port = dp_to_dig_port(intel_dp)->port;
 	const struct edp_power_seq *seq = &intel_dp->pps_delays;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1ffd8d5c3235..54b65951622d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1050,8 +1050,6 @@ void i915_audio_component_cleanup(struct drm_i915_private *dev_priv);
 /* intel_display.c */
 extern const struct drm_plane_funcs intel_plane_funcs;
 bool intel_has_pending_fb_unpin(struct drm_device *dev);
-int intel_pch_rawclk(struct drm_device *dev);
-int intel_hrawclk(struct drm_device *dev);
 void intel_mark_busy(struct drm_device *dev);
 void intel_mark_idle(struct drm_device *dev);
 void intel_crtc_restore_mode(struct drm_crtc *crtc);
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 891a587225e2..b8c78d885311 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1314,8 +1314,8 @@ static u32 lpt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
  */
 static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 {
-	struct drm_device *dev = connector->base.dev;
-	int clock = MHz(intel_pch_rawclk(dev));
+	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+	int clock = 1000 * dev_priv->rawclk_freq;
 
 	return clock / (pwm_freq_hz * 128);
 }
@@ -1330,12 +1330,11 @@ static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
  */
 static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 {
-	struct drm_device *dev = connector->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	int clock;
 
-	if (IS_PINEVIEW(dev))
-		clock = MHz(intel_hrawclk(dev));
+	if (IS_PINEVIEW(dev_priv))
+		clock = 1000 * dev_priv->rawclk_freq;
 	else
 		clock = 1000 * dev_priv->cdclk_freq;
 
@@ -1354,7 +1353,7 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 	int clock;
 
 	if (IS_G4X(dev_priv))
-		clock = MHz(intel_hrawclk(dev));
+		clock = 1000 * dev_priv->rawclk_freq;
 	else
 		clock = 1000 * dev_priv->cdclk_freq;
 
@@ -1368,18 +1367,17 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
  */
 static u32 vlv_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
 {
-	struct drm_device *dev = connector->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	int clock;
+	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 
 	if ((I915_READ(CBR1_VLV) & CBR_PWM_CLOCK_MUX_SELECT) == 0) {
-		if (IS_CHERRYVIEW(dev))
+		if (IS_CHERRYVIEW(dev_priv))
 			return KHz(19200) / (pwm_freq_hz * 16);
 		else
 			return MHz(25) / (pwm_freq_hz * 16);
 	} else {
-		clock = intel_hrawclk(dev);
-		return MHz(clock) / (pwm_freq_hz * 128);
+		int clock = 1000 * dev_priv->rawclk_freq;
+
+		return clock / (pwm_freq_hz * 128);
 	}
 }
 
-- 
2.4.10

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

[PATCH 08/11] drm/i915: Rename s/i9xx/g4x/ in DP code

[ville.syrjala]

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

g4x is the first platform with DP support, so let's name the relevant
functions as g4x_ instead i9xx_ to avoid confusion.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2a9b5710ee83..06afcc101a2c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -672,7 +672,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
 	return status;
 }
 
-static uint32_t i9xx_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
+static uint32_t g4x_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
@@ -736,10 +736,10 @@ static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 	return index ? 0 : 1;
 }
 
-static uint32_t i9xx_get_aux_send_ctl(struct intel_dp *intel_dp,
-				      bool has_aux_irq,
-				      int send_bytes,
-				      uint32_t aux_clock_divider)
+static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
+				     bool has_aux_irq,
+				     int send_bytes,
+				     uint32_t aux_clock_divider)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
@@ -5830,12 +5830,12 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	else if (HAS_PCH_SPLIT(dev))
 		intel_dp->get_aux_clock_divider = ilk_get_aux_clock_divider;
 	else
-		intel_dp->get_aux_clock_divider = i9xx_get_aux_clock_divider;
+		intel_dp->get_aux_clock_divider = g4x_get_aux_clock_divider;
 
 	if (INTEL_INFO(dev)->gen >= 9)
 		intel_dp->get_aux_send_ctl = skl_get_aux_send_ctl;
 	else
-		intel_dp->get_aux_send_ctl = i9xx_get_aux_send_ctl;
+		intel_dp->get_aux_send_ctl = g4x_get_aux_send_ctl;
 
 	if (HAS_DDI(dev))
 		intel_dp->prepare_link_retrain = intel_ddi_prepare_link_retrain;
-- 
2.4.10

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

[PATCH 09/11] drm/i915: Use g4x_get_aux_clock_divider() for VLV/CHV

[ville.syrjala]

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

With the hrawclk frequency cached in dev_priv, we can simply use
g4x_get_aux_clock_divider() for VLV/CHV.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 06afcc101a2c..39a1689bac7d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -721,11 +721,6 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 	}
 }
 
-static uint32_t vlv_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
-{
-	return index ? 0 : 100;
-}
-
 static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 {
 	/*
@@ -5823,8 +5818,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	/* intel_dp vfuncs */
 	if (INTEL_INFO(dev)->gen >= 9)
 		intel_dp->get_aux_clock_divider = skl_get_aux_clock_divider;
-	else if (IS_VALLEYVIEW(dev))
-		intel_dp->get_aux_clock_divider = vlv_get_aux_clock_divider;
 	else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
 		intel_dp->get_aux_clock_divider = hsw_get_aux_clock_divider;
 	else if (HAS_PCH_SPLIT(dev))
-- 
2.4.10

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

[PATCH 10/11] drm/i915: Read out hrawclk from CCK on vlv/chv

[ville.syrjala]

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

Currently we assume that hrawclk is 200MHz on VLV/CHV. That should
be true always, but just to avoid such asumptions we can read out the
actual frequency from CCK.

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 487224572022..5806df01997f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -785,6 +785,7 @@ enum skl_disp_power_wells {
 #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
 #define CCK_CZ_CLOCK_CONTROL			0x62
 #define CCK_DISPLAY_CLOCK_CONTROL		0x6b
+#define CCK_DISPLAY_REF_CLOCK_CONTROL		0x6c
 #define  CCK_TRUNK_FORCE_ON			(1 << 17)
 #define  CCK_TRUNK_FORCE_OFF			(1 << 16)
 #define  CCK_FREQUENCY_STATUS			(0x1f << 8)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 84a1359ecba5..09aa5361d994 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -177,7 +177,8 @@ intel_pch_rawclk(struct drm_i915_private *dev_priv)
 static int
 intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
 {
-	return 200000;
+	return vlv_get_cck_clock_hpll(dev_priv, "hrawclk",
+				      CCK_DISPLAY_REF_CLOCK_CONTROL);
 }
 
 static int
-- 
2.4.10

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

[PATCH 11/11] drm/i915: Clean up .get_aux_clock_divider() functions

[ville.syrjala]

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

Now that the mess with AUX clock divder rounding is sorted out and
we have both cdclk and rawclk cached in dev_priv, we can clean up
the .get_aux_clock_divider() functions a bit.

The main thing here is just calling ilk_get_aux_clock_divider()
from hsw_get_aux_clock_divider() except for the LPT:H special
case.

We could got further and call g4x_get_aux_clock_divider() from
ilk_get_aux_clock_divider() for the PCH ports, but I'm sure Jani
would object, so leave that be.

While at it repeat the comment where the AUX clock comes from
in ilk_get_aux_clock_divider().

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 39a1689bac7d..df9fc396d18d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -677,22 +677,29 @@ static uint32_t g4x_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
 
+	if (index)
+		return 0;
+
 	/*
 	 * The clock divider is based off the hrawclk, and would like to run at
-	 * 2MHz.  So, take the hrawclk value and divide by 2 and use that
+	 * 2MHz.  So, take the hrawclk value and divide by 2000 and use that
 	 */
-	return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
+	return DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
 }
 
 static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-	struct drm_device *dev = intel_dig_port->base.base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
 
 	if (index)
 		return 0;
 
+	/*
+	 * The clock divider is based off the cdclk or PCH rawclk, and would
+	 * like to run at 2MHz.  So, take the cdclk or PCH rawclk value and
+	 * divide by 2000 and use that
+	 */
 	if (intel_dig_port->port == PORT_A)
 		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
 	else
@@ -702,23 +709,18 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-	struct drm_device *dev = intel_dig_port->base.base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
 
-	if (intel_dig_port->port == PORT_A) {
-		if (index)
-			return 0;
-		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
-	} else if (HAS_PCH_LPT_H(dev_priv)) {
+	if (intel_dig_port->port != PORT_A && HAS_PCH_LPT_H(dev_priv)) {
 		/* Workaround for non-ULT HSW */
 		switch (index) {
 		case 0: return 63;
 		case 1: return 72;
 		default: return 0;
 		}
-	} else  {
-		return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
 	}
+
+	return ilk_get_aux_clock_divider(intel_dp, index);
 }
 
 static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
-- 
2.4.10

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

Re: [PATCH 04/11] drm/i915: Kill duplicated PNV .get_display_clock_speed() assignment

[Daniel Vetter]

On Mon, Nov 30, 2015 at 04:23:45PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Somehow we accumulated a duplicated .get_display_clock_speed()
> assignment for PNV in
> commit 34edce2fea69 ("drm/i915: Add cdclk extraction for g33, g965gm and g4x")
> 
> No real harm on having two, we just never reach the second one, so
> simply kill it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8dbe5b398af4..84395c8c9dce 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14791,9 +14791,6 @@ static void intel_init_display(struct drm_device *dev)
>  	else if (IS_I945GM(dev) || IS_845G(dev))
>  		dev_priv->display.get_display_clock_speed =
>  			i9xx_misc_get_display_clock_speed;
> -	else if (IS_PINEVIEW(dev))
> -		dev_priv->display.get_display_clock_speed =
> -			pnv_get_display_clock_speed;
>  	else if (IS_I915GM(dev))
>  		dev_priv->display.get_display_clock_speed =
>  			i915gm_get_display_clock_speed;
> -- 
> 2.4.10
> 
> _______________________________________________
> 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 01/11] drm/i915: Fix VBT backlight Hz to PWM conversion for PNV

[Jani Nikula]

On Mon, 30 Nov 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Convert the MHz number coming from intel_rawclk() into Hz in
> i9xx_hz_to_pwm() on PNV. Otherwise we'll get something totally
> bogus as a result.
>
> Fixes: aa17cdb4f836 ("drm/i915: initialize backlight max from VBT")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_panel.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index a24df35e11e7..ea528ca854e8 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1335,7 +1335,7 @@ static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  	int clock;
>  
>  	if (IS_PINEVIEW(dev))
> -		clock = intel_hrawclk(dev);
> +		clock = MHz(intel_hrawclk(dev));
>  	else
>  		clock = 1000 * dev_priv->display.get_display_clock_speed(dev);

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

Re: [PATCH 02/11] drm/i915: Fix vbt PWM max setup for CTG

[Jani Nikula]

On Mon, 30 Nov 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> CTG uses hrawclk for backlight, so calculate the max based on that
> instead of cdclk.

If you say so.

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

> Fixes: aa17cdb4f836 ("drm/i915: initialize backlight max from VBT")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_panel.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index ea528ca854e8..737e12349e81 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1344,13 +1344,19 @@ static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  
>  /*
>   * Gen4: This value represents the period of the PWM stream in display core
> - * clocks multiplied by 128.
> + * clocks ([DevCTG] HRAW clocks) multiplied by 128.
> + *
>   */
>  static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  {
>  	struct drm_device *dev = connector->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int clock = 1000 * dev_priv->display.get_display_clock_speed(dev);
> +	int clock;
> +
> +	if (IS_G4X(dev_priv))
> +		clock = MHz(intel_hrawclk(dev));
> +	else
> +		clock = 1000 * dev_priv->display.get_display_clock_speed(dev);
>  
>  	return clock / (pwm_freq_hz * 128);
>  }

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

Re: [PATCH 03/11] drm/i915: Add HAS_PCH_LPT_H()

[Jani Nikula]

On Mon, 30 Nov 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We have HAS_PCH_LPT_LP() already, so add HAS_PCH_LPT_H() and use it
> where appropriate.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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


> ---
>  drivers/gpu/drm/i915/i915_drv.h    | 1 +
>  drivers/gpu/drm/i915/intel_dp.c    | 2 +-
>  drivers/gpu/drm/i915/intel_panel.c | 2 +-
>  3 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bc865e234df2..9ab3e25ddf38 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2615,6 +2615,7 @@ struct drm_i915_cmd_table {
>  #define HAS_PCH_SPT(dev) (INTEL_PCH_TYPE(dev) == PCH_SPT)
>  #define HAS_PCH_LPT(dev) (INTEL_PCH_TYPE(dev) == PCH_LPT)
>  #define HAS_PCH_LPT_LP(dev) (__I915__(dev)->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE)
> +#define HAS_PCH_LPT_H(dev) (__I915__(dev)->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE)
>  #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT)
>  #define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX)
>  #define HAS_PCH_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f2bfca0ae6b6..77ae5821e9c0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -711,7 +711,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  		if (index)
>  			return 0;
>  		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
> -	} else if (dev_priv->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) {
> +	} else if (HAS_PCH_LPT_H(dev_priv)) {
>  		/* Workaround for non-ULT HSW */
>  		switch (index) {
>  		case 0: return 63;
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 737e12349e81..d4b18b56f3ca 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1300,7 +1300,7 @@ static u32 lpt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  	else
>  		mul = 128;
>  
> -	if (dev_priv->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE)
> +	if (HAS_PCH_LPT_H(dev_priv))
>  		clock = MHz(135); /* LPT:H */
>  	else
>  		clock = MHz(24); /* LPT:LP */

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

Re: [PATCH 04/11] drm/i915: Kill duplicated PNV .get_display_clock_speed() assignment

[Jani Nikula]

On Mon, 30 Nov 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Somehow we accumulated a duplicated .get_display_clock_speed()
> assignment for PNV in
> commit 34edce2fea69 ("drm/i915: Add cdclk extraction for g33, g965gm and g4x")
>
> No real harm on having two, we just never reach the second one, so
> simply kill it.
>
> 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 | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8dbe5b398af4..84395c8c9dce 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14791,9 +14791,6 @@ static void intel_init_display(struct drm_device *dev)
>  	else if (IS_I945GM(dev) || IS_845G(dev))
>  		dev_priv->display.get_display_clock_speed =
>  			i9xx_misc_get_display_clock_speed;
> -	else if (IS_PINEVIEW(dev))
> -		dev_priv->display.get_display_clock_speed =
> -			pnv_get_display_clock_speed;
>  	else if (IS_I915GM(dev))
>  		dev_priv->display.get_display_clock_speed =
>  			i915gm_get_display_clock_speed;

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

Re: [PATCH 02/11] drm/i915: Fix vbt PWM max setup for CTG

[Ville Syrjälä]

On Tue, Dec 01, 2015 at 02:21:37PM +0200, Jani Nikula wrote:
> On Mon, 30 Nov 2015, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > CTG uses hrawclk for backlight, so calculate the max based on that
> > instead of cdclk.
> 
> If you say so.

Not me. Bspec says so.

> 
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> 
> > Fixes: aa17cdb4f836 ("drm/i915: initialize backlight max from VBT")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_panel.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > index ea528ca854e8..737e12349e81 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -1344,13 +1344,19 @@ static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  
> >  /*
> >   * Gen4: This value represents the period of the PWM stream in display core
> > - * clocks multiplied by 128.
> > + * clocks ([DevCTG] HRAW clocks) multiplied by 128.
> > + *
> >   */
> >  static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  {
> >  	struct drm_device *dev = connector->base.dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > -	int clock = 1000 * dev_priv->display.get_display_clock_speed(dev);
> > +	int clock;
> > +
> > +	if (IS_G4X(dev_priv))
> > +		clock = MHz(intel_hrawclk(dev));
> > +	else
> > +		clock = 1000 * dev_priv->display.get_display_clock_speed(dev);
> >  
> >  	return clock / (pwm_freq_hz * 128);
> >  }
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

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

Re: [PATCH 02/11] drm/i915: Fix vbt PWM max setup for CTG

[Jani Nikula]

On Tue, 01 Dec 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Mon, 30 Nov 2015, ville.syrjala@linux.intel.com wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> CTG uses hrawclk for backlight, so calculate the max based on that
>> instead of cdclk.
>
> If you say so.
>
> Acked-by: Jani Nikula <jani.nikula@intel.com>

Okay, found it, promote that to

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



>
>> Fixes: aa17cdb4f836 ("drm/i915: initialize backlight max from VBT")
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_panel.c | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
>> index ea528ca854e8..737e12349e81 100644
>> --- a/drivers/gpu/drm/i915/intel_panel.c
>> +++ b/drivers/gpu/drm/i915/intel_panel.c
>> @@ -1344,13 +1344,19 @@ static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>>  
>>  /*
>>   * Gen4: This value represents the period of the PWM stream in display core
>> - * clocks multiplied by 128.
>> + * clocks ([DevCTG] HRAW clocks) multiplied by 128.
>> + *
>>   */
>>  static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>>  {
>>  	struct drm_device *dev = connector->base.dev;
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>> -	int clock = 1000 * dev_priv->display.get_display_clock_speed(dev);
>> +	int clock;
>> +
>> +	if (IS_G4X(dev_priv))
>> +		clock = MHz(intel_hrawclk(dev));
>> +	else
>> +		clock = 1000 * dev_priv->display.get_display_clock_speed(dev);
>>  
>>  	return clock / (pwm_freq_hz * 128);
>>  }

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

Re: [PATCH 05/11] drm/i915: Round the AUX clock divider to closest on all platforms

[Jani Nikula]

On Mon, 30 Nov 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently we round the AUX clock divider down on g4x, to closest
> on HSW/BDW port A, and up everywhere else. We are supposed to get
> as close to 2MHz as we can, so round to closest seems like the
> best option.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Makes sense.

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


> ---
>  drivers/gpu/drm/i915/intel_dp.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 77ae5821e9c0..f335c92b4fa7 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -681,7 +681,7 @@ static uint32_t i9xx_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  	 * The clock divider is based off the hrawclk, and would like to run at
>  	 * 2MHz.  So, take the hrawclk value and divide by 2 and use that
>  	 */
> -	return index ? 0 : intel_hrawclk(dev) / 2;
> +	return index ? 0 : DIV_ROUND_CLOSEST(intel_hrawclk(dev), 2);
>  }
>  
>  static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> @@ -694,10 +694,10 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  		return 0;
>  
>  	if (intel_dig_port->port == PORT_A) {
> -		return DIV_ROUND_UP(dev_priv->cdclk_freq, 2000);
> +		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
>  
>  	} else {
> -		return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
> +		return DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
>  	}
>  }
>  
> @@ -719,7 +719,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  		default: return 0;
>  		}
>  	} else  {
> -		return index ? 0 : DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
> +		return index ? 0 : DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
>  	}
>  }

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

Re: [PATCH 06/11] drm/i915: Use cached cdclk_freq for PWM calculations

[Jani Nikula]

On Mon, 30 Nov 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> No need to read out cdclk from the hardware, we have it already
> cached in dev_priv.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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


> ---
>  drivers/gpu/drm/i915/intel_panel.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index d4b18b56f3ca..891a587225e2 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1337,7 +1337,7 @@ static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  	if (IS_PINEVIEW(dev))
>  		clock = MHz(intel_hrawclk(dev));
>  	else
> -		clock = 1000 * dev_priv->display.get_display_clock_speed(dev);
> +		clock = 1000 * dev_priv->cdclk_freq;
>  
>  	return clock / (pwm_freq_hz * 32);
>  }
> @@ -1356,7 +1356,7 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  	if (IS_G4X(dev_priv))
>  		clock = MHz(intel_hrawclk(dev));
>  	else
> -		clock = 1000 * dev_priv->display.get_display_clock_speed(dev);
> +		clock = 1000 * dev_priv->cdclk_freq;
>  
>  	return clock / (pwm_freq_hz * 128);
>  }

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

Re: [PATCH 08/11] drm/i915: Rename s/i9xx/g4x/ in DP code

[Jani Nikula]

On Mon, 30 Nov 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> g4x is the first platform with DP support, so let's name the relevant
> functions as g4x_ instead i9xx_ to avoid confusion.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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


> ---
>  drivers/gpu/drm/i915/intel_dp.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 2a9b5710ee83..06afcc101a2c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -672,7 +672,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
>  	return status;
>  }
>  
> -static uint32_t i9xx_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> +static uint32_t g4x_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> @@ -736,10 +736,10 @@ static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  	return index ? 0 : 1;
>  }
>  
> -static uint32_t i9xx_get_aux_send_ctl(struct intel_dp *intel_dp,
> -				      bool has_aux_irq,
> -				      int send_bytes,
> -				      uint32_t aux_clock_divider)
> +static uint32_t g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> +				     bool has_aux_irq,
> +				     int send_bytes,
> +				     uint32_t aux_clock_divider)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_device *dev = intel_dig_port->base.base.dev;
> @@ -5830,12 +5830,12 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  	else if (HAS_PCH_SPLIT(dev))
>  		intel_dp->get_aux_clock_divider = ilk_get_aux_clock_divider;
>  	else
> -		intel_dp->get_aux_clock_divider = i9xx_get_aux_clock_divider;
> +		intel_dp->get_aux_clock_divider = g4x_get_aux_clock_divider;
>  
>  	if (INTEL_INFO(dev)->gen >= 9)
>  		intel_dp->get_aux_send_ctl = skl_get_aux_send_ctl;
>  	else
> -		intel_dp->get_aux_send_ctl = i9xx_get_aux_send_ctl;
> +		intel_dp->get_aux_send_ctl = g4x_get_aux_send_ctl;
>  
>  	if (HAS_DDI(dev))
>  		intel_dp->prepare_link_retrain = intel_ddi_prepare_link_retrain;

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

Re: [PATCH 07/11] drm/i915: Store rawclk_freq in dev_priv

[Jani Nikula]

On Mon, 30 Nov 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Generalize rawclk handling by storing it in dev_priv.
>
> Presumably our hrawclk readout works at least for CTG and ELK
> since we've been using it for DP AUX on those platforms. There
> are no real docs anymore after configdb vanished, so the only
> reference is the public CTG GMCH spec. What bits are listed in
> that doc match our code. The ELK GMCH spec have no relevant
> details unfortunately.
>
> The PNV situation is less clear. Starting from
> commit aa17cdb4f836 ("drm/i915: initialize backlight max from VBT")
> we assume that the CTG/ELK hrawclk readout works for PNV as well.
> At least the results *seem* reasonable for one PNV machine (Lenovo
> Ideapad S10-3t). Sadly the PNV GMCH spec doesn't have the goods on
> the relevant register either.
>
> So let's keep assuming it works for PNV,ELK,CTG and read it out on
> those platforms. G33 also has hrawclk according to some notes
> in BSpec, but we don't actually need it for anything, so let's not
> even try to read it out there.

Looks sensible but moderately scary, did not review properly yet,
however a few nitpicks below.

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_dp.c      | 16 +++++------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 --
>  drivers/gpu/drm/i915/intel_panel.c   | 24 ++++++++--------
>  5 files changed, 53 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9ab3e25ddf38..64facd410037 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1781,6 +1781,7 @@ struct drm_i915_private {
>  	unsigned int skl_boot_cdclk;
>  	unsigned int cdclk_freq, max_cdclk_freq;
>  	unsigned int max_dotclk_freq;
> +	unsigned int rawclk_freq;
>  	unsigned int hpll_freq;
>  	unsigned int czclk_freq;
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 84395c8c9dce..84a1359ecba5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -168,49 +168,61 @@ static int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
>  	return DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, divider + 1);
>  }
>  
> -int
> -intel_pch_rawclk(struct drm_device *dev)
> +static int
> +intel_pch_rawclk(struct drm_i915_private *dev_priv)

I guess intel_ prefix would not be needed anymore. *shrug*.

>  {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	WARN_ON(!HAS_PCH_SPLIT(dev));
> +	return (I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK) * 1000;
> +}
>  
> -	return I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK;
> +static int
> +intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
> +{
> +	return 200000;
>  }
>  
> -/* hrawclock is 1/4 the FSB frequency */
> -int intel_hrawclk(struct drm_device *dev)
> +static int
> +intel_g4x_hrawclk(struct drm_i915_private *dev_priv)
>  {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	uint32_t clkcfg;
>  
> -	/* There is no CLKCFG reg in Valleyview. VLV hrawclk is 200 MHz */
> -	if (IS_VALLEYVIEW(dev))
> -		return 200;
> -
> +	/* hrawclock is 1/4 the FSB frequency */
>  	clkcfg = I915_READ(CLKCFG);
>  	switch (clkcfg & CLKCFG_FSB_MASK) {
>  	case CLKCFG_FSB_400:
> -		return 100;
> +		return 100000;
>  	case CLKCFG_FSB_533:
> -		return 133;
> +		return 133333;
>  	case CLKCFG_FSB_667:
> -		return 166;
> +		return 166667;
>  	case CLKCFG_FSB_800:
> -		return 200;
> +		return 200000;
>  	case CLKCFG_FSB_1067:
> -		return 266;
> +		return 266667;
>  	case CLKCFG_FSB_1333:
> -		return 333;
> +		return 333333;
>  	/* these two are just a guess; one of them might be right */
>  	case CLKCFG_FSB_1600:
>  	case CLKCFG_FSB_1600_ALT:
> -		return 400;
> +		return 400000;
>  	default:
> -		return 133;
> +		return 133333;
>  	}
>  }
>  
> +static void intel_update_rawclk(struct drm_i915_private *dev_priv)
> +{
> +	if (HAS_PCH_SPLIT(dev_priv))
> +		dev_priv->rawclk_freq = intel_pch_rawclk(dev_priv);
> +	else if (IS_VALLEYVIEW(dev_priv))
> +		dev_priv->rawclk_freq = intel_vlv_hrawclk(dev_priv);
> +	else if (IS_G4X(dev_priv) || IS_PINEVIEW(dev_priv))
> +		dev_priv->rawclk_freq = intel_g4x_hrawclk(dev_priv);
> +	else
> +		return; /* no rawclk on other platforms, or no need to know it */
> +
> +	DRM_DEBUG_DRIVER("rawclk rate: %d kHz\n", dev_priv->rawclk_freq);
> +}
> +
>  static void intel_update_czclk(struct drm_i915_private *dev_priv)
>  {
>  	if (!IS_VALLEYVIEW(dev_priv))
> @@ -15145,6 +15157,7 @@ void intel_modeset_init(struct drm_device *dev)
>  	}
>  
>  	intel_update_czclk(dev_priv);
> +	intel_update_rawclk(dev_priv);
>  	intel_update_cdclk(dev);
>  
>  	intel_shared_dpll_init(dev);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f335c92b4fa7..2a9b5710ee83 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -675,13 +675,13 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
>  static uint32_t i9xx_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> -	struct drm_device *dev = intel_dig_port->base.base.dev;
> +	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
>  
>  	/*
>  	 * The clock divider is based off the hrawclk, and would like to run at
>  	 * 2MHz.  So, take the hrawclk value and divide by 2 and use that
>  	 */

The comment needs updating. Possibly elsewhere too outside of patch
context.

> -	return index ? 0 : DIV_ROUND_CLOSEST(intel_hrawclk(dev), 2);
> +	return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
>  }
>  
>  static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> @@ -693,12 +693,10 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  	if (index)
>  		return 0;
>  
> -	if (intel_dig_port->port == PORT_A) {
> +	if (intel_dig_port->port == PORT_A)
>  		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
> -
> -	} else {
> -		return DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
> -	}
> +	else
> +		return DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
>  }
>  
>  static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> @@ -719,7 +717,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  		default: return 0;
>  		}
>  	} else  {
> -		return index ? 0 : DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
> +		return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
>  	}
>  }
>  
> @@ -5237,7 +5235,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 pp_on, pp_off, pp_div, port_sel = 0;
> -	int div = HAS_PCH_SPLIT(dev) ? intel_pch_rawclk(dev) : intel_hrawclk(dev);
> +	int div = dev_priv->rawclk_freq / 1000;
>  	i915_reg_t pp_on_reg, pp_off_reg, pp_div_reg, pp_ctrl_reg;
>  	enum port port = dp_to_dig_port(intel_dp)->port;
>  	const struct edp_power_seq *seq = &intel_dp->pps_delays;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 1ffd8d5c3235..54b65951622d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1050,8 +1050,6 @@ void i915_audio_component_cleanup(struct drm_i915_private *dev_priv);
>  /* intel_display.c */
>  extern const struct drm_plane_funcs intel_plane_funcs;
>  bool intel_has_pending_fb_unpin(struct drm_device *dev);
> -int intel_pch_rawclk(struct drm_device *dev);
> -int intel_hrawclk(struct drm_device *dev);
>  void intel_mark_busy(struct drm_device *dev);
>  void intel_mark_idle(struct drm_device *dev);
>  void intel_crtc_restore_mode(struct drm_crtc *crtc);
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 891a587225e2..b8c78d885311 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1314,8 +1314,8 @@ static u32 lpt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>   */
>  static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  {
> -	struct drm_device *dev = connector->base.dev;
> -	int clock = MHz(intel_pch_rawclk(dev));
> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> +	int clock = 1000 * dev_priv->rawclk_freq;
>  
>  	return clock / (pwm_freq_hz * 128);

Could use:

	return KHz(dev_priv->rawclk_freq) / (pwm_freq_hz * 128);

Same thing all around.

Side note, maybe these should also round closest (follow-up).


>  }
> @@ -1330,12 +1330,11 @@ static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>   */
>  static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  {
> -	struct drm_device *dev = connector->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	int clock;
>  
> -	if (IS_PINEVIEW(dev))
> -		clock = MHz(intel_hrawclk(dev));
> +	if (IS_PINEVIEW(dev_priv))
> +		clock = 1000 * dev_priv->rawclk_freq;
>  	else
>  		clock = 1000 * dev_priv->cdclk_freq;
>  
> @@ -1354,7 +1353,7 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  	int clock;
>  
>  	if (IS_G4X(dev_priv))
> -		clock = MHz(intel_hrawclk(dev));
> +		clock = 1000 * dev_priv->rawclk_freq;
>  	else
>  		clock = 1000 * dev_priv->cdclk_freq;
>  
> @@ -1368,18 +1367,17 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>   */
>  static u32 vlv_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  {
> -	struct drm_device *dev = connector->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int clock;
> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  
>  	if ((I915_READ(CBR1_VLV) & CBR_PWM_CLOCK_MUX_SELECT) == 0) {
> -		if (IS_CHERRYVIEW(dev))
> +		if (IS_CHERRYVIEW(dev_priv))
>  			return KHz(19200) / (pwm_freq_hz * 16);
>  		else
>  			return MHz(25) / (pwm_freq_hz * 16);
>  	} else {
> -		clock = intel_hrawclk(dev);
> -		return MHz(clock) / (pwm_freq_hz * 128);
> +		int clock = 1000 * dev_priv->rawclk_freq;
> +
> +		return clock / (pwm_freq_hz * 128);
>  	}
>  }

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

Re: [PATCH 09/11] drm/i915: Use g4x_get_aux_clock_divider() for VLV/CHV

[Jani Nikula]

On Mon, 30 Nov 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> With the hrawclk frequency cached in dev_priv, we can simply use
> g4x_get_aux_clock_divider() for VLV/CHV.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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


> ---
>  drivers/gpu/drm/i915/intel_dp.c | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 06afcc101a2c..39a1689bac7d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -721,11 +721,6 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  	}
>  }
>  
> -static uint32_t vlv_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> -{
> -	return index ? 0 : 100;
> -}
> -
>  static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  {
>  	/*
> @@ -5823,8 +5818,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  	/* intel_dp vfuncs */
>  	if (INTEL_INFO(dev)->gen >= 9)
>  		intel_dp->get_aux_clock_divider = skl_get_aux_clock_divider;
> -	else if (IS_VALLEYVIEW(dev))
> -		intel_dp->get_aux_clock_divider = vlv_get_aux_clock_divider;
>  	else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
>  		intel_dp->get_aux_clock_divider = hsw_get_aux_clock_divider;
>  	else if (HAS_PCH_SPLIT(dev))

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

Re: [PATCH 11/11] drm/i915: Clean up .get_aux_clock_divider() functions

[Jani Nikula]

On Mon, 30 Nov 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Now that the mess with AUX clock divder rounding is sorted out and
> we have both cdclk and rawclk cached in dev_priv, we can clean up
> the .get_aux_clock_divider() functions a bit.
>
> The main thing here is just calling ilk_get_aux_clock_divider()
> from hsw_get_aux_clock_divider() except for the LPT:H special
> case.
>
> We could got further and call g4x_get_aux_clock_divider() from
> ilk_get_aux_clock_divider() for the PCH ports, but I'm sure Jani
> would object, so leave that be.

Thanks. :)

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


> While at it repeat the comment where the AUX clock comes from
> in ilk_get_aux_clock_divider().
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 39a1689bac7d..df9fc396d18d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -677,22 +677,29 @@ static uint32_t g4x_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
>  
> +	if (index)
> +		return 0;
> +
>  	/*
>  	 * The clock divider is based off the hrawclk, and would like to run at
> -	 * 2MHz.  So, take the hrawclk value and divide by 2 and use that
> +	 * 2MHz.  So, take the hrawclk value and divide by 2000 and use that
>  	 */
> -	return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
> +	return DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
>  }
>  
>  static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> -	struct drm_device *dev = intel_dig_port->base.base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
>  
>  	if (index)
>  		return 0;
>  
> +	/*
> +	 * The clock divider is based off the cdclk or PCH rawclk, and would
> +	 * like to run at 2MHz.  So, take the cdclk or PCH rawclk value and
> +	 * divide by 2000 and use that
> +	 */
>  	if (intel_dig_port->port == PORT_A)
>  		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
>  	else
> @@ -702,23 +709,18 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> -	struct drm_device *dev = intel_dig_port->base.base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
>  
> -	if (intel_dig_port->port == PORT_A) {
> -		if (index)
> -			return 0;
> -		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
> -	} else if (HAS_PCH_LPT_H(dev_priv)) {
> +	if (intel_dig_port->port != PORT_A && HAS_PCH_LPT_H(dev_priv)) {
>  		/* Workaround for non-ULT HSW */
>  		switch (index) {
>  		case 0: return 63;
>  		case 1: return 72;
>  		default: return 0;
>  		}
> -	} else  {
> -		return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
>  	}
> +
> +	return ilk_get_aux_clock_divider(intel_dp, index);
>  }
>  
>  static uint32_t skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)

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

Re: [PATCH 07/11] drm/i915: Store rawclk_freq in dev_priv

[Ville Syrjälä]

On Tue, Dec 01, 2015 at 02:47:41PM +0200, Jani Nikula wrote:
> On Mon, 30 Nov 2015, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Generalize rawclk handling by storing it in dev_priv.
> >
> > Presumably our hrawclk readout works at least for CTG and ELK
> > since we've been using it for DP AUX on those platforms. There
> > are no real docs anymore after configdb vanished, so the only
> > reference is the public CTG GMCH spec. What bits are listed in
> > that doc match our code. The ELK GMCH spec have no relevant
> > details unfortunately.
> >
> > The PNV situation is less clear. Starting from
> > commit aa17cdb4f836 ("drm/i915: initialize backlight max from VBT")
> > we assume that the CTG/ELK hrawclk readout works for PNV as well.
> > At least the results *seem* reasonable for one PNV machine (Lenovo
> > Ideapad S10-3t). Sadly the PNV GMCH spec doesn't have the goods on
> > the relevant register either.
> >
> > So let's keep assuming it works for PNV,ELK,CTG and read it out on
> > those platforms. G33 also has hrawclk according to some notes
> > in BSpec, but we don't actually need it for anything, so let's not
> > even try to read it out there.
> 
> Looks sensible but moderately scary, did not review properly yet,
> however a few nitpicks below.
> 
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      |  1 +
> >  drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++--------------
> >  drivers/gpu/drm/i915/intel_dp.c      | 16 +++++------
> >  drivers/gpu/drm/i915/intel_drv.h     |  2 --
> >  drivers/gpu/drm/i915/intel_panel.c   | 24 ++++++++--------
> >  5 files changed, 53 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 9ab3e25ddf38..64facd410037 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1781,6 +1781,7 @@ struct drm_i915_private {
> >  	unsigned int skl_boot_cdclk;
> >  	unsigned int cdclk_freq, max_cdclk_freq;
> >  	unsigned int max_dotclk_freq;
> > +	unsigned int rawclk_freq;
> >  	unsigned int hpll_freq;
> >  	unsigned int czclk_freq;
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 84395c8c9dce..84a1359ecba5 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -168,49 +168,61 @@ static int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
> >  	return DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, divider + 1);
> >  }
> >  
> > -int
> > -intel_pch_rawclk(struct drm_device *dev)
> > +static int
> > +intel_pch_rawclk(struct drm_i915_private *dev_priv)
> 
> I guess intel_ prefix would not be needed anymore. *shrug*.

Yeah, lazyness won and I didn't start renaming stuff too much.

> 
> >  {
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > -
> > -	WARN_ON(!HAS_PCH_SPLIT(dev));
> > +	return (I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK) * 1000;
> > +}
> >  
> > -	return I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK;
> > +static int
> > +intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
> > +{
> > +	return 200000;
> >  }
> >  
> > -/* hrawclock is 1/4 the FSB frequency */
> > -int intel_hrawclk(struct drm_device *dev)
> > +static int
> > +intel_g4x_hrawclk(struct drm_i915_private *dev_priv)
> >  {
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	uint32_t clkcfg;
> >  
> > -	/* There is no CLKCFG reg in Valleyview. VLV hrawclk is 200 MHz */
> > -	if (IS_VALLEYVIEW(dev))
> > -		return 200;
> > -
> > +	/* hrawclock is 1/4 the FSB frequency */
> >  	clkcfg = I915_READ(CLKCFG);
> >  	switch (clkcfg & CLKCFG_FSB_MASK) {
> >  	case CLKCFG_FSB_400:
> > -		return 100;
> > +		return 100000;
> >  	case CLKCFG_FSB_533:
> > -		return 133;
> > +		return 133333;
> >  	case CLKCFG_FSB_667:
> > -		return 166;
> > +		return 166667;
> >  	case CLKCFG_FSB_800:
> > -		return 200;
> > +		return 200000;
> >  	case CLKCFG_FSB_1067:
> > -		return 266;
> > +		return 266667;
> >  	case CLKCFG_FSB_1333:
> > -		return 333;
> > +		return 333333;
> >  	/* these two are just a guess; one of them might be right */
> >  	case CLKCFG_FSB_1600:
> >  	case CLKCFG_FSB_1600_ALT:
> > -		return 400;
> > +		return 400000;
> >  	default:
> > -		return 133;
> > +		return 133333;
> >  	}
> >  }
> >  
> > +static void intel_update_rawclk(struct drm_i915_private *dev_priv)
> > +{
> > +	if (HAS_PCH_SPLIT(dev_priv))
> > +		dev_priv->rawclk_freq = intel_pch_rawclk(dev_priv);
> > +	else if (IS_VALLEYVIEW(dev_priv))
> > +		dev_priv->rawclk_freq = intel_vlv_hrawclk(dev_priv);
> > +	else if (IS_G4X(dev_priv) || IS_PINEVIEW(dev_priv))
> > +		dev_priv->rawclk_freq = intel_g4x_hrawclk(dev_priv);
> > +	else
> > +		return; /* no rawclk on other platforms, or no need to know it */
> > +
> > +	DRM_DEBUG_DRIVER("rawclk rate: %d kHz\n", dev_priv->rawclk_freq);
> > +}
> > +
> >  static void intel_update_czclk(struct drm_i915_private *dev_priv)
> >  {
> >  	if (!IS_VALLEYVIEW(dev_priv))
> > @@ -15145,6 +15157,7 @@ void intel_modeset_init(struct drm_device *dev)
> >  	}
> >  
> >  	intel_update_czclk(dev_priv);
> > +	intel_update_rawclk(dev_priv);
> >  	intel_update_cdclk(dev);
> >  
> >  	intel_shared_dpll_init(dev);
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index f335c92b4fa7..2a9b5710ee83 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -675,13 +675,13 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> >  static uint32_t i9xx_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> >  {
> >  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > -	struct drm_device *dev = intel_dig_port->base.base.dev;
> > +	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> >  
> >  	/*
> >  	 * The clock divider is based off the hrawclk, and would like to run at
> >  	 * 2MHz.  So, take the hrawclk value and divide by 2 and use that
> >  	 */
> 
> The comment needs updating. Possibly elsewhere too outside of patch
> context.

That's being done in patch 11/11. I assume that's good enough place for
you?

> 
> > -	return index ? 0 : DIV_ROUND_CLOSEST(intel_hrawclk(dev), 2);
> > +	return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
> >  }
> >  
> >  static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> > @@ -693,12 +693,10 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> >  	if (index)
> >  		return 0;
> >  
> > -	if (intel_dig_port->port == PORT_A) {
> > +	if (intel_dig_port->port == PORT_A)
> >  		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
> > -
> > -	} else {
> > -		return DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
> > -	}
> > +	else
> > +		return DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
> >  }
> >  
> >  static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> > @@ -719,7 +717,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> >  		default: return 0;
> >  		}
> >  	} else  {
> > -		return index ? 0 : DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
> > +		return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
> >  	}
> >  }
> >  
> > @@ -5237,7 +5235,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	u32 pp_on, pp_off, pp_div, port_sel = 0;
> > -	int div = HAS_PCH_SPLIT(dev) ? intel_pch_rawclk(dev) : intel_hrawclk(dev);
> > +	int div = dev_priv->rawclk_freq / 1000;

BTW here still round down when calculating the power sequencer divisor.
Considering that one is used to enforce the panel power delays, I'm
thinking we should really round up to make sure we never end up with a
shorter delay than specified. But that disagrees with the examples in
the spec, so not really sure what's best.

> >  	i915_reg_t pp_on_reg, pp_off_reg, pp_div_reg, pp_ctrl_reg;
> >  	enum port port = dp_to_dig_port(intel_dp)->port;
> >  	const struct edp_power_seq *seq = &intel_dp->pps_delays;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 1ffd8d5c3235..54b65951622d 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1050,8 +1050,6 @@ void i915_audio_component_cleanup(struct drm_i915_private *dev_priv);
> >  /* intel_display.c */
> >  extern const struct drm_plane_funcs intel_plane_funcs;
> >  bool intel_has_pending_fb_unpin(struct drm_device *dev);
> > -int intel_pch_rawclk(struct drm_device *dev);
> > -int intel_hrawclk(struct drm_device *dev);
> >  void intel_mark_busy(struct drm_device *dev);
> >  void intel_mark_idle(struct drm_device *dev);
> >  void intel_crtc_restore_mode(struct drm_crtc *crtc);
> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > index 891a587225e2..b8c78d885311 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -1314,8 +1314,8 @@ static u32 lpt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >   */
> >  static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  {
> > -	struct drm_device *dev = connector->base.dev;
> > -	int clock = MHz(intel_pch_rawclk(dev));
> > +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > +	int clock = 1000 * dev_priv->rawclk_freq;
> >  
> >  	return clock / (pwm_freq_hz * 128);
> 
> Could use:
> 
> 	return KHz(dev_priv->rawclk_freq) / (pwm_freq_hz * 128);
> 
> Same thing all around.

I can respin with that.

> 
> Side note, maybe these should also round closest (follow-up).

Hmm. Yeah, I guess closest would be warranted for PWM.

> 
> 
> >  }
> > @@ -1330,12 +1330,11 @@ static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >   */
> >  static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  {
> > -	struct drm_device *dev = connector->base.dev;
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >  	int clock;
> >  
> > -	if (IS_PINEVIEW(dev))
> > -		clock = MHz(intel_hrawclk(dev));
> > +	if (IS_PINEVIEW(dev_priv))
> > +		clock = 1000 * dev_priv->rawclk_freq;
> >  	else
> >  		clock = 1000 * dev_priv->cdclk_freq;
> >  
> > @@ -1354,7 +1353,7 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  	int clock;
> >  
> >  	if (IS_G4X(dev_priv))
> > -		clock = MHz(intel_hrawclk(dev));
> > +		clock = 1000 * dev_priv->rawclk_freq;
> >  	else
> >  		clock = 1000 * dev_priv->cdclk_freq;
> >  
> > @@ -1368,18 +1367,17 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >   */
> >  static u32 vlv_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >  {
> > -	struct drm_device *dev = connector->base.dev;
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > -	int clock;
> > +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >  
> >  	if ((I915_READ(CBR1_VLV) & CBR_PWM_CLOCK_MUX_SELECT) == 0) {
> > -		if (IS_CHERRYVIEW(dev))
> > +		if (IS_CHERRYVIEW(dev_priv))
> >  			return KHz(19200) / (pwm_freq_hz * 16);
> >  		else
> >  			return MHz(25) / (pwm_freq_hz * 16);
> >  	} else {
> > -		clock = intel_hrawclk(dev);
> > -		return MHz(clock) / (pwm_freq_hz * 128);
> > +		int clock = 1000 * dev_priv->rawclk_freq;
> > +
> > +		return clock / (pwm_freq_hz * 128);
> >  	}
> >  }
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

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

Re: [PATCH 07/11] drm/i915: Store rawclk_freq in dev_priv

[Jani Nikula]

On Tue, 01 Dec 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, Dec 01, 2015 at 02:47:41PM +0200, Jani Nikula wrote:
>> On Mon, 30 Nov 2015, ville.syrjala@linux.intel.com wrote:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > Generalize rawclk handling by storing it in dev_priv.
>> >
>> > Presumably our hrawclk readout works at least for CTG and ELK
>> > since we've been using it for DP AUX on those platforms. There
>> > are no real docs anymore after configdb vanished, so the only
>> > reference is the public CTG GMCH spec. What bits are listed in
>> > that doc match our code. The ELK GMCH spec have no relevant
>> > details unfortunately.
>> >
>> > The PNV situation is less clear. Starting from
>> > commit aa17cdb4f836 ("drm/i915: initialize backlight max from VBT")
>> > we assume that the CTG/ELK hrawclk readout works for PNV as well.
>> > At least the results *seem* reasonable for one PNV machine (Lenovo
>> > Ideapad S10-3t). Sadly the PNV GMCH spec doesn't have the goods on
>> > the relevant register either.
>> >
>> > So let's keep assuming it works for PNV,ELK,CTG and read it out on
>> > those platforms. G33 also has hrawclk according to some notes
>> > in BSpec, but we don't actually need it for anything, so let's not
>> > even try to read it out there.
>> 
>> Looks sensible but moderately scary, did not review properly yet,
>> however a few nitpicks below.
>> 
>> >
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_drv.h      |  1 +
>> >  drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++--------------
>> >  drivers/gpu/drm/i915/intel_dp.c      | 16 +++++------
>> >  drivers/gpu/drm/i915/intel_drv.h     |  2 --
>> >  drivers/gpu/drm/i915/intel_panel.c   | 24 ++++++++--------
>> >  5 files changed, 53 insertions(+), 45 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> > index 9ab3e25ddf38..64facd410037 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -1781,6 +1781,7 @@ struct drm_i915_private {
>> >  	unsigned int skl_boot_cdclk;
>> >  	unsigned int cdclk_freq, max_cdclk_freq;
>> >  	unsigned int max_dotclk_freq;
>> > +	unsigned int rawclk_freq;
>> >  	unsigned int hpll_freq;
>> >  	unsigned int czclk_freq;
>> >  
>> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> > index 84395c8c9dce..84a1359ecba5 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -168,49 +168,61 @@ static int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
>> >  	return DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, divider + 1);
>> >  }
>> >  
>> > -int
>> > -intel_pch_rawclk(struct drm_device *dev)
>> > +static int
>> > +intel_pch_rawclk(struct drm_i915_private *dev_priv)
>> 
>> I guess intel_ prefix would not be needed anymore. *shrug*.
>
> Yeah, lazyness won and I didn't start renaming stuff too much.

No worries.

>
>> 
>> >  {
>> > -	struct drm_i915_private *dev_priv = dev->dev_private;
>> > -
>> > -	WARN_ON(!HAS_PCH_SPLIT(dev));
>> > +	return (I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK) * 1000;
>> > +}
>> >  
>> > -	return I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK;
>> > +static int
>> > +intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
>> > +{
>> > +	return 200000;
>> >  }
>> >  
>> > -/* hrawclock is 1/4 the FSB frequency */
>> > -int intel_hrawclk(struct drm_device *dev)
>> > +static int
>> > +intel_g4x_hrawclk(struct drm_i915_private *dev_priv)
>> >  {
>> > -	struct drm_i915_private *dev_priv = dev->dev_private;
>> >  	uint32_t clkcfg;
>> >  
>> > -	/* There is no CLKCFG reg in Valleyview. VLV hrawclk is 200 MHz */
>> > -	if (IS_VALLEYVIEW(dev))
>> > -		return 200;
>> > -
>> > +	/* hrawclock is 1/4 the FSB frequency */
>> >  	clkcfg = I915_READ(CLKCFG);
>> >  	switch (clkcfg & CLKCFG_FSB_MASK) {
>> >  	case CLKCFG_FSB_400:
>> > -		return 100;
>> > +		return 100000;
>> >  	case CLKCFG_FSB_533:
>> > -		return 133;
>> > +		return 133333;
>> >  	case CLKCFG_FSB_667:
>> > -		return 166;
>> > +		return 166667;
>> >  	case CLKCFG_FSB_800:
>> > -		return 200;
>> > +		return 200000;
>> >  	case CLKCFG_FSB_1067:
>> > -		return 266;
>> > +		return 266667;
>> >  	case CLKCFG_FSB_1333:
>> > -		return 333;
>> > +		return 333333;
>> >  	/* these two are just a guess; one of them might be right */
>> >  	case CLKCFG_FSB_1600:
>> >  	case CLKCFG_FSB_1600_ALT:
>> > -		return 400;
>> > +		return 400000;
>> >  	default:
>> > -		return 133;
>> > +		return 133333;
>> >  	}
>> >  }
>> >  
>> > +static void intel_update_rawclk(struct drm_i915_private *dev_priv)
>> > +{
>> > +	if (HAS_PCH_SPLIT(dev_priv))
>> > +		dev_priv->rawclk_freq = intel_pch_rawclk(dev_priv);
>> > +	else if (IS_VALLEYVIEW(dev_priv))
>> > +		dev_priv->rawclk_freq = intel_vlv_hrawclk(dev_priv);
>> > +	else if (IS_G4X(dev_priv) || IS_PINEVIEW(dev_priv))
>> > +		dev_priv->rawclk_freq = intel_g4x_hrawclk(dev_priv);
>> > +	else
>> > +		return; /* no rawclk on other platforms, or no need to know it */
>> > +
>> > +	DRM_DEBUG_DRIVER("rawclk rate: %d kHz\n", dev_priv->rawclk_freq);
>> > +}
>> > +
>> >  static void intel_update_czclk(struct drm_i915_private *dev_priv)
>> >  {
>> >  	if (!IS_VALLEYVIEW(dev_priv))
>> > @@ -15145,6 +15157,7 @@ void intel_modeset_init(struct drm_device *dev)
>> >  	}
>> >  
>> >  	intel_update_czclk(dev_priv);
>> > +	intel_update_rawclk(dev_priv);
>> >  	intel_update_cdclk(dev);
>> >  
>> >  	intel_shared_dpll_init(dev);
>> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> > index f335c92b4fa7..2a9b5710ee83 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > @@ -675,13 +675,13 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
>> >  static uint32_t i9xx_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>> >  {
>> >  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> > -	struct drm_device *dev = intel_dig_port->base.base.dev;
>> > +	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
>> >  
>> >  	/*
>> >  	 * The clock divider is based off the hrawclk, and would like to run at
>> >  	 * 2MHz.  So, take the hrawclk value and divide by 2 and use that
>> >  	 */
>> 
>> The comment needs updating. Possibly elsewhere too outside of patch
>> context.
>
> That's being done in patch 11/11. I assume that's good enough place for
> you?

Yes.

>
>> 
>> > -	return index ? 0 : DIV_ROUND_CLOSEST(intel_hrawclk(dev), 2);
>> > +	return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
>> >  }
>> >  
>> >  static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>> > @@ -693,12 +693,10 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>> >  	if (index)
>> >  		return 0;
>> >  
>> > -	if (intel_dig_port->port == PORT_A) {
>> > +	if (intel_dig_port->port == PORT_A)
>> >  		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
>> > -
>> > -	} else {
>> > -		return DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
>> > -	}
>> > +	else
>> > +		return DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
>> >  }
>> >  
>> >  static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>> > @@ -719,7 +717,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>> >  		default: return 0;
>> >  		}
>> >  	} else  {
>> > -		return index ? 0 : DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
>> > +		return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
>> >  	}
>> >  }
>> >  
>> > @@ -5237,7 +5235,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>> >  {
>> >  	struct drm_i915_private *dev_priv = dev->dev_private;
>> >  	u32 pp_on, pp_off, pp_div, port_sel = 0;
>> > -	int div = HAS_PCH_SPLIT(dev) ? intel_pch_rawclk(dev) : intel_hrawclk(dev);
>> > +	int div = dev_priv->rawclk_freq / 1000;
>
> BTW here still round down when calculating the power sequencer divisor.
> Considering that one is used to enforce the panel power delays, I'm
> thinking we should really round up to make sure we never end up with a
> shorter delay than specified. But that disagrees with the examples in
> the spec, so not really sure what's best.
>
>> >  	i915_reg_t pp_on_reg, pp_off_reg, pp_div_reg, pp_ctrl_reg;
>> >  	enum port port = dp_to_dig_port(intel_dp)->port;
>> >  	const struct edp_power_seq *seq = &intel_dp->pps_delays;
>> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> > index 1ffd8d5c3235..54b65951622d 100644
>> > --- a/drivers/gpu/drm/i915/intel_drv.h
>> > +++ b/drivers/gpu/drm/i915/intel_drv.h
>> > @@ -1050,8 +1050,6 @@ void i915_audio_component_cleanup(struct drm_i915_private *dev_priv);
>> >  /* intel_display.c */
>> >  extern const struct drm_plane_funcs intel_plane_funcs;
>> >  bool intel_has_pending_fb_unpin(struct drm_device *dev);
>> > -int intel_pch_rawclk(struct drm_device *dev);
>> > -int intel_hrawclk(struct drm_device *dev);
>> >  void intel_mark_busy(struct drm_device *dev);
>> >  void intel_mark_idle(struct drm_device *dev);
>> >  void intel_crtc_restore_mode(struct drm_crtc *crtc);
>> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
>> > index 891a587225e2..b8c78d885311 100644
>> > --- a/drivers/gpu/drm/i915/intel_panel.c
>> > +++ b/drivers/gpu/drm/i915/intel_panel.c
>> > @@ -1314,8 +1314,8 @@ static u32 lpt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>> >   */
>> >  static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>> >  {
>> > -	struct drm_device *dev = connector->base.dev;
>> > -	int clock = MHz(intel_pch_rawclk(dev));
>> > +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>> > +	int clock = 1000 * dev_priv->rawclk_freq;
>> >  
>> >  	return clock / (pwm_freq_hz * 128);
>> 
>> Could use:
>> 
>> 	return KHz(dev_priv->rawclk_freq) / (pwm_freq_hz * 128);
>> 
>> Same thing all around.
>
> I can respin with that.

You can choose to respin, or to wait until I get around to actually
reviewing the patch instead of just bikeshedding. ;)

BR,
Jani.

>
>> 
>> Side note, maybe these should also round closest (follow-up).
>
> Hmm. Yeah, I guess closest would be warranted for PWM.
>
>> 
>> 
>> >  }
>> > @@ -1330,12 +1330,11 @@ static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>> >   */
>> >  static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>> >  {
>> > -	struct drm_device *dev = connector->base.dev;
>> > -	struct drm_i915_private *dev_priv = dev->dev_private;
>> > +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>> >  	int clock;
>> >  
>> > -	if (IS_PINEVIEW(dev))
>> > -		clock = MHz(intel_hrawclk(dev));
>> > +	if (IS_PINEVIEW(dev_priv))
>> > +		clock = 1000 * dev_priv->rawclk_freq;
>> >  	else
>> >  		clock = 1000 * dev_priv->cdclk_freq;
>> >  
>> > @@ -1354,7 +1353,7 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>> >  	int clock;
>> >  
>> >  	if (IS_G4X(dev_priv))
>> > -		clock = MHz(intel_hrawclk(dev));
>> > +		clock = 1000 * dev_priv->rawclk_freq;
>> >  	else
>> >  		clock = 1000 * dev_priv->cdclk_freq;
>> >  
>> > @@ -1368,18 +1367,17 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>> >   */
>> >  static u32 vlv_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>> >  {
>> > -	struct drm_device *dev = connector->base.dev;
>> > -	struct drm_i915_private *dev_priv = dev->dev_private;
>> > -	int clock;
>> > +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>> >  
>> >  	if ((I915_READ(CBR1_VLV) & CBR_PWM_CLOCK_MUX_SELECT) == 0) {
>> > -		if (IS_CHERRYVIEW(dev))
>> > +		if (IS_CHERRYVIEW(dev_priv))
>> >  			return KHz(19200) / (pwm_freq_hz * 16);
>> >  		else
>> >  			return MHz(25) / (pwm_freq_hz * 16);
>> >  	} else {
>> > -		clock = intel_hrawclk(dev);
>> > -		return MHz(clock) / (pwm_freq_hz * 128);
>> > +		int clock = 1000 * dev_priv->rawclk_freq;
>> > +
>> > +		return clock / (pwm_freq_hz * 128);
>> >  	}
>> >  }
>> 
>> -- 
>> Jani Nikula, Intel Open Source Technology Center

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

Re: [PATCH 06/11] drm/i915: Use cached cdclk_freq for PWM calculations

[Ville Syrjälä]

On Tue, Dec 01, 2015 at 02:37:04PM +0200, Jani Nikula wrote:
> On Mon, 30 Nov 2015, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > No need to read out cdclk from the hardware, we have it already
> > cached in dev_priv.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Merged up to this patch. Thanks for the reviews.

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

Re: [PATCH 02/11] drm/i915: Fix vbt PWM max setup for CTG

[Daniel Vetter]

On Tue, Dec 01, 2015 at 02:30:46PM +0200, Jani Nikula wrote:
> On Tue, 01 Dec 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > On Mon, 30 Nov 2015, ville.syrjala@linux.intel.com wrote:
> >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> CTG uses hrawclk for backlight, so calculate the max based on that
> >> instead of cdclk.
> >
> > If you say so.
> >
> > Acked-by: Jani Nikula <jani.nikula@intel.com>
> 
> Okay, found it, promote that to

Should probably contain a Bspec reference then, hunting in that beast
isn't much fun.
-Daniel

> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
> 
> >
> >> Fixes: aa17cdb4f836 ("drm/i915: initialize backlight max from VBT")
> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_panel.c | 10 ++++++++--
> >>  1 file changed, 8 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> >> index ea528ca854e8..737e12349e81 100644
> >> --- a/drivers/gpu/drm/i915/intel_panel.c
> >> +++ b/drivers/gpu/drm/i915/intel_panel.c
> >> @@ -1344,13 +1344,19 @@ static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >>  
> >>  /*
> >>   * Gen4: This value represents the period of the PWM stream in display core
> >> - * clocks multiplied by 128.
> >> + * clocks ([DevCTG] HRAW clocks) multiplied by 128.
> >> + *
> >>   */
> >>  static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >>  {
> >>  	struct drm_device *dev = connector->base.dev;
> >>  	struct drm_i915_private *dev_priv = dev->dev_private;
> >> -	int clock = 1000 * dev_priv->display.get_display_clock_speed(dev);
> >> +	int clock;
> >> +
> >> +	if (IS_G4X(dev_priv))
> >> +		clock = MHz(intel_hrawclk(dev));
> >> +	else
> >> +		clock = 1000 * dev_priv->display.get_display_clock_speed(dev);
> >>  
> >>  	return clock / (pwm_freq_hz * 128);
> >>  }
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> 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 07/11] drm/i915: Store rawclk_freq in dev_priv

[Ville Syrjälä]

On Tue, Dec 01, 2015 at 05:43:33PM +0200, Jani Nikula wrote:
> On Tue, 01 Dec 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Tue, Dec 01, 2015 at 02:47:41PM +0200, Jani Nikula wrote:
> >> On Mon, 30 Nov 2015, ville.syrjala@linux.intel.com wrote:
> >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> >
> >> > Generalize rawclk handling by storing it in dev_priv.
> >> >
> >> > Presumably our hrawclk readout works at least for CTG and ELK
> >> > since we've been using it for DP AUX on those platforms. There
> >> > are no real docs anymore after configdb vanished, so the only
> >> > reference is the public CTG GMCH spec. What bits are listed in
> >> > that doc match our code. The ELK GMCH spec have no relevant
> >> > details unfortunately.
> >> >
> >> > The PNV situation is less clear. Starting from
> >> > commit aa17cdb4f836 ("drm/i915: initialize backlight max from VBT")
> >> > we assume that the CTG/ELK hrawclk readout works for PNV as well.
> >> > At least the results *seem* reasonable for one PNV machine (Lenovo
> >> > Ideapad S10-3t). Sadly the PNV GMCH spec doesn't have the goods on
> >> > the relevant register either.
> >> >
> >> > So let's keep assuming it works for PNV,ELK,CTG and read it out on
> >> > those platforms. G33 also has hrawclk according to some notes
> >> > in BSpec, but we don't actually need it for anything, so let's not
> >> > even try to read it out there.
> >> 
> >> Looks sensible but moderately scary, did not review properly yet,
> >> however a few nitpicks below.
> >> 
> >> >
> >> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/i915_drv.h      |  1 +
> >> >  drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++--------------
> >> >  drivers/gpu/drm/i915/intel_dp.c      | 16 +++++------
> >> >  drivers/gpu/drm/i915/intel_drv.h     |  2 --
> >> >  drivers/gpu/drm/i915/intel_panel.c   | 24 ++++++++--------
> >> >  5 files changed, 53 insertions(+), 45 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> > index 9ab3e25ddf38..64facd410037 100644
> >> > --- a/drivers/gpu/drm/i915/i915_drv.h
> >> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> > @@ -1781,6 +1781,7 @@ struct drm_i915_private {
> >> >  	unsigned int skl_boot_cdclk;
> >> >  	unsigned int cdclk_freq, max_cdclk_freq;
> >> >  	unsigned int max_dotclk_freq;
> >> > +	unsigned int rawclk_freq;
> >> >  	unsigned int hpll_freq;
> >> >  	unsigned int czclk_freq;
> >> >  
> >> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> > index 84395c8c9dce..84a1359ecba5 100644
> >> > --- a/drivers/gpu/drm/i915/intel_display.c
> >> > +++ b/drivers/gpu/drm/i915/intel_display.c
> >> > @@ -168,49 +168,61 @@ static int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
> >> >  	return DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, divider + 1);
> >> >  }
> >> >  
> >> > -int
> >> > -intel_pch_rawclk(struct drm_device *dev)
> >> > +static int
> >> > +intel_pch_rawclk(struct drm_i915_private *dev_priv)
> >> 
> >> I guess intel_ prefix would not be needed anymore. *shrug*.
> >
> > Yeah, lazyness won and I didn't start renaming stuff too much.
> 
> No worries.
> 
> >
> >> 
> >> >  {
> >> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> >> > -
> >> > -	WARN_ON(!HAS_PCH_SPLIT(dev));
> >> > +	return (I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK) * 1000;
> >> > +}
> >> >  
> >> > -	return I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK;
> >> > +static int
> >> > +intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
> >> > +{
> >> > +	return 200000;
> >> >  }
> >> >  
> >> > -/* hrawclock is 1/4 the FSB frequency */
> >> > -int intel_hrawclk(struct drm_device *dev)
> >> > +static int
> >> > +intel_g4x_hrawclk(struct drm_i915_private *dev_priv)
> >> >  {
> >> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> >> >  	uint32_t clkcfg;
> >> >  
> >> > -	/* There is no CLKCFG reg in Valleyview. VLV hrawclk is 200 MHz */
> >> > -	if (IS_VALLEYVIEW(dev))
> >> > -		return 200;
> >> > -
> >> > +	/* hrawclock is 1/4 the FSB frequency */
> >> >  	clkcfg = I915_READ(CLKCFG);
> >> >  	switch (clkcfg & CLKCFG_FSB_MASK) {
> >> >  	case CLKCFG_FSB_400:
> >> > -		return 100;
> >> > +		return 100000;
> >> >  	case CLKCFG_FSB_533:
> >> > -		return 133;
> >> > +		return 133333;
> >> >  	case CLKCFG_FSB_667:
> >> > -		return 166;
> >> > +		return 166667;
> >> >  	case CLKCFG_FSB_800:
> >> > -		return 200;
> >> > +		return 200000;
> >> >  	case CLKCFG_FSB_1067:
> >> > -		return 266;
> >> > +		return 266667;
> >> >  	case CLKCFG_FSB_1333:
> >> > -		return 333;
> >> > +		return 333333;
> >> >  	/* these two are just a guess; one of them might be right */
> >> >  	case CLKCFG_FSB_1600:
> >> >  	case CLKCFG_FSB_1600_ALT:
> >> > -		return 400;
> >> > +		return 400000;
> >> >  	default:
> >> > -		return 133;
> >> > +		return 133333;
> >> >  	}
> >> >  }
> >> >  
> >> > +static void intel_update_rawclk(struct drm_i915_private *dev_priv)
> >> > +{
> >> > +	if (HAS_PCH_SPLIT(dev_priv))
> >> > +		dev_priv->rawclk_freq = intel_pch_rawclk(dev_priv);
> >> > +	else if (IS_VALLEYVIEW(dev_priv))
> >> > +		dev_priv->rawclk_freq = intel_vlv_hrawclk(dev_priv);
> >> > +	else if (IS_G4X(dev_priv) || IS_PINEVIEW(dev_priv))
> >> > +		dev_priv->rawclk_freq = intel_g4x_hrawclk(dev_priv);
> >> > +	else
> >> > +		return; /* no rawclk on other platforms, or no need to know it */
> >> > +
> >> > +	DRM_DEBUG_DRIVER("rawclk rate: %d kHz\n", dev_priv->rawclk_freq);
> >> > +}
> >> > +
> >> >  static void intel_update_czclk(struct drm_i915_private *dev_priv)
> >> >  {
> >> >  	if (!IS_VALLEYVIEW(dev_priv))
> >> > @@ -15145,6 +15157,7 @@ void intel_modeset_init(struct drm_device *dev)
> >> >  	}
> >> >  
> >> >  	intel_update_czclk(dev_priv);
> >> > +	intel_update_rawclk(dev_priv);
> >> >  	intel_update_cdclk(dev);
> >> >  
> >> >  	intel_shared_dpll_init(dev);
> >> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> > index f335c92b4fa7..2a9b5710ee83 100644
> >> > --- a/drivers/gpu/drm/i915/intel_dp.c
> >> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> > @@ -675,13 +675,13 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> >> >  static uint32_t i9xx_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> >> >  {
> >> >  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >> > -	struct drm_device *dev = intel_dig_port->base.base.dev;
> >> > +	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> >> >  
> >> >  	/*
> >> >  	 * The clock divider is based off the hrawclk, and would like to run at
> >> >  	 * 2MHz.  So, take the hrawclk value and divide by 2 and use that
> >> >  	 */
> >> 
> >> The comment needs updating. Possibly elsewhere too outside of patch
> >> context.
> >
> > That's being done in patch 11/11. I assume that's good enough place for
> > you?
> 
> Yes.
> 
> >
> >> 
> >> > -	return index ? 0 : DIV_ROUND_CLOSEST(intel_hrawclk(dev), 2);
> >> > +	return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
> >> >  }
> >> >  
> >> >  static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> >> > @@ -693,12 +693,10 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> >> >  	if (index)
> >> >  		return 0;
> >> >  
> >> > -	if (intel_dig_port->port == PORT_A) {
> >> > +	if (intel_dig_port->port == PORT_A)
> >> >  		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
> >> > -
> >> > -	} else {
> >> > -		return DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
> >> > -	}
> >> > +	else
> >> > +		return DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
> >> >  }
> >> >  
> >> >  static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> >> > @@ -719,7 +717,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> >> >  		default: return 0;
> >> >  		}
> >> >  	} else  {
> >> > -		return index ? 0 : DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2);
> >> > +		return index ? 0 : DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
> >> >  	}
> >> >  }
> >> >  
> >> > @@ -5237,7 +5235,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
> >> >  {
> >> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >> >  	u32 pp_on, pp_off, pp_div, port_sel = 0;
> >> > -	int div = HAS_PCH_SPLIT(dev) ? intel_pch_rawclk(dev) : intel_hrawclk(dev);
> >> > +	int div = dev_priv->rawclk_freq / 1000;
> >
> > BTW here still round down when calculating the power sequencer divisor.
> > Considering that one is used to enforce the panel power delays, I'm
> > thinking we should really round up to make sure we never end up with a
> > shorter delay than specified. But that disagrees with the examples in
> > the spec, so not really sure what's best.
> >
> >> >  	i915_reg_t pp_on_reg, pp_off_reg, pp_div_reg, pp_ctrl_reg;
> >> >  	enum port port = dp_to_dig_port(intel_dp)->port;
> >> >  	const struct edp_power_seq *seq = &intel_dp->pps_delays;
> >> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >> > index 1ffd8d5c3235..54b65951622d 100644
> >> > --- a/drivers/gpu/drm/i915/intel_drv.h
> >> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> > @@ -1050,8 +1050,6 @@ void i915_audio_component_cleanup(struct drm_i915_private *dev_priv);
> >> >  /* intel_display.c */
> >> >  extern const struct drm_plane_funcs intel_plane_funcs;
> >> >  bool intel_has_pending_fb_unpin(struct drm_device *dev);
> >> > -int intel_pch_rawclk(struct drm_device *dev);
> >> > -int intel_hrawclk(struct drm_device *dev);
> >> >  void intel_mark_busy(struct drm_device *dev);
> >> >  void intel_mark_idle(struct drm_device *dev);
> >> >  void intel_crtc_restore_mode(struct drm_crtc *crtc);
> >> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> >> > index 891a587225e2..b8c78d885311 100644
> >> > --- a/drivers/gpu/drm/i915/intel_panel.c
> >> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> >> > @@ -1314,8 +1314,8 @@ static u32 lpt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >> >   */
> >> >  static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >> >  {
> >> > -	struct drm_device *dev = connector->base.dev;
> >> > -	int clock = MHz(intel_pch_rawclk(dev));
> >> > +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >> > +	int clock = 1000 * dev_priv->rawclk_freq;
> >> >  
> >> >  	return clock / (pwm_freq_hz * 128);
> >> 
> >> Could use:
> >> 
> >> 	return KHz(dev_priv->rawclk_freq) / (pwm_freq_hz * 128);
> >> 
> >> Same thing all around.
> >
> > I can respin with that.
> 
> You can choose to respin, or to wait until I get around to actually
> reviewing the patch instead of just bikeshedding. ;)

Did I wait long enough? ;)

> 
> BR,
> Jani.
> 
> >
> >> 
> >> Side note, maybe these should also round closest (follow-up).
> >
> > Hmm. Yeah, I guess closest would be warranted for PWM.
> >
> >> 
> >> 
> >> >  }
> >> > @@ -1330,12 +1330,11 @@ static u32 pch_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >> >   */
> >> >  static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >> >  {
> >> > -	struct drm_device *dev = connector->base.dev;
> >> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> >> > +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >> >  	int clock;
> >> >  
> >> > -	if (IS_PINEVIEW(dev))
> >> > -		clock = MHz(intel_hrawclk(dev));
> >> > +	if (IS_PINEVIEW(dev_priv))
> >> > +		clock = 1000 * dev_priv->rawclk_freq;
> >> >  	else
> >> >  		clock = 1000 * dev_priv->cdclk_freq;
> >> >  
> >> > @@ -1354,7 +1353,7 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >> >  	int clock;
> >> >  
> >> >  	if (IS_G4X(dev_priv))
> >> > -		clock = MHz(intel_hrawclk(dev));
> >> > +		clock = 1000 * dev_priv->rawclk_freq;
> >> >  	else
> >> >  		clock = 1000 * dev_priv->cdclk_freq;
> >> >  
> >> > @@ -1368,18 +1367,17 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >> >   */
> >> >  static u32 vlv_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> >> >  {
> >> > -	struct drm_device *dev = connector->base.dev;
> >> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> >> > -	int clock;
> >> > +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >> >  
> >> >  	if ((I915_READ(CBR1_VLV) & CBR_PWM_CLOCK_MUX_SELECT) == 0) {
> >> > -		if (IS_CHERRYVIEW(dev))
> >> > +		if (IS_CHERRYVIEW(dev_priv))
> >> >  			return KHz(19200) / (pwm_freq_hz * 16);
> >> >  		else
> >> >  			return MHz(25) / (pwm_freq_hz * 16);
> >> >  	} else {
> >> > -		clock = intel_hrawclk(dev);
> >> > -		return MHz(clock) / (pwm_freq_hz * 128);
> >> > +		int clock = 1000 * dev_priv->rawclk_freq;
> >> > +
> >> > +		return clock / (pwm_freq_hz * 128);
> >> >  	}
> >> >  }
> >> 
> >> -- 
> >> Jani Nikula, Intel Open Source Technology Center
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

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