[PATCH v4 00/12] All sort of cdclk stuff

[PATCH v4 00/12] All sort of cdclk stuff

[Mika Kahola]

This patch series rebases Ville's original cdclk patch series
excluding the ones that	are already reviewed.

http://lists.freedesktop.org/archives/intel-gfx/2014-November/055633.html

The patches are rebased to the latest drm-intel-nightly and while I was
doing it I tagged the reviewed-by. Maybe Daniel/Jani can comment if this
is procedure is ok or not. There is one exception to this and that
is the patch 'HSW cdclk support' which I had to modify to support the
recent atomic changes. This patch requires a review. 

Ville Syrjälä (12):
  drm/i915: Fix i855 get_display_clock_speed
  drm/i915: Fix 852GM/GMV cdclk
  drm/i915: Add cdclk extraction for g33, g965gm and g4x
  drm/i915: Warn when cdclk for the platforms is not known
  drm/i915: Cache current cdclk frequency in dev_priv
  drm/i915: Use cached cdclk value
  drm/i915: Unify ilk and hsw .get_aux_clock_divider
  drm/i915: Store max cdclk value in dev_priv
  drm/i915: Don't enable IPS when pixel rate exceeds 95%
  drm/i915: HSW cdclk support
  drm/i915: Add IS_BDW_ULX
  drm/i915: BDW clock change support

 drivers/gpu/drm/i915/i915_drv.h      |   5 +-
 drivers/gpu/drm/i915/i915_reg.h      |  18 +-
 drivers/gpu/drm/i915/intel_display.c | 546 +++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_dp.c      |  24 +-
 drivers/gpu/drm/i915/intel_drv.h     |   2 +-
 drivers/gpu/drm/i915/intel_pm.c      |  19 +-
 6 files changed, 560 insertions(+), 54 deletions(-)

-- 
1.9.1

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

[PATCH v4 01/12] drm/i915: Fix i855 get_display_clock_speed

[Mika Kahola]

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

Actually read the HPLLCC register insted of assuming it's 0. Fix the
HPLLCC bit definitions and all the missing ones from the 852GME spec.

852GME, 854 and 855 all seem to match the same HPLLC encoding even
though only some of the values are valid is some of the platforms.

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

v2: Rebased to the latest
v3: Rebased to the latest

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 11 ++++++++---
 drivers/gpu/drm/i915/intel_display.c | 15 ++++++++++++---
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 84af255..6625fb3 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -50,12 +50,17 @@
 
 /* PCI config space */
 
-#define HPLLCC	0xc0 /* 855 only */
-#define   GC_CLOCK_CONTROL_MASK		(0xf << 0)
+#define HPLLCC	0xc0 /* 85x only */
+#define   GC_CLOCK_CONTROL_MASK		(0x7 << 0)
 #define   GC_CLOCK_133_200		(0 << 0)
 #define   GC_CLOCK_100_200		(1 << 0)
 #define   GC_CLOCK_100_133		(2 << 0)
-#define   GC_CLOCK_166_250		(3 << 0)
+#define   GC_CLOCK_133_266		(3 << 0)
+#define   GC_CLOCK_133_200_2		(4 << 0)
+#define   GC_CLOCK_133_266_2		(5 << 0)
+#define   GC_CLOCK_166_266		(6 << 0)
+#define   GC_CLOCK_166_250		(7 << 0)
+
 #define GCFGC2	0xda
 #define GCFGC	0xf0 /* 915+ only */
 #define   GC_LOW_FREQUENCY_ENABLE	(1 << 7)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c97b496..64debfb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6627,20 +6627,29 @@ static int i865_get_display_clock_speed(struct drm_device *dev)
 	return 266667;
 }
 
-static int i855_get_display_clock_speed(struct drm_device *dev)
+static int i85x_get_display_clock_speed(struct drm_device *dev)
 {
 	u16 hpllcc = 0;
+
+	pci_bus_read_config_word(dev->pdev->bus,
+				 PCI_DEVFN(0, 3), HPLLCC, &hpllcc);
+
 	/* Assume that the hardware is in the high speed state.  This
 	 * should be the default.
 	 */
 	switch (hpllcc & GC_CLOCK_CONTROL_MASK) {
 	case GC_CLOCK_133_200:
+	case GC_CLOCK_133_200_2:
 	case GC_CLOCK_100_200:
 		return 200000;
 	case GC_CLOCK_166_250:
 		return 250000;
 	case GC_CLOCK_100_133:
 		return 133333;
+	case GC_CLOCK_133_266:
+	case GC_CLOCK_133_266_2:
+	case GC_CLOCK_166_266:
+		return 266667;
 	}
 
 	/* Shouldn't happen */
@@ -14199,8 +14208,8 @@ static void intel_init_display(struct drm_device *dev)
 			i865_get_display_clock_speed;
 	else if (IS_I85X(dev))
 		dev_priv->display.get_display_clock_speed =
-			i855_get_display_clock_speed;
-	else /* 852, 830 */
+			i85x_get_display_clock_speed;
+	else /* 830 */
 		dev_priv->display.get_display_clock_speed =
 			i830_get_display_clock_speed;
 
-- 
1.9.1

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

Re: [PATCH v4 01/12] drm/i915: Fix i855 get_display_clock_speed

[Damien Lespiau]

On Fri, May 22, 2015 at 11:22:31AM +0300, Mika Kahola wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Actually read the HPLLCC register insted of assuming it's 0. Fix the
> HPLLCC bit definitions and all the missing ones from the 852GME spec.
> 
> 852GME, 854 and 855 all seem to match the same HPLLC encoding even
> though only some of the values are valid is some of the platforms.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> v2: Rebased to the latest
> v3: Rebased to the latest
> 
> Reviewed-by: Mika Kahola <mika.kahola@intel.com>
> 
> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>

-ENODOC

Acked -by: Damien Lespiau <damien.lespiau@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 11 ++++++++---
>  drivers/gpu/drm/i915/intel_display.c | 15 ++++++++++++---
>  2 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 84af255..6625fb3 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -50,12 +50,17 @@
>  
>  /* PCI config space */
>  
> -#define HPLLCC	0xc0 /* 855 only */
> -#define   GC_CLOCK_CONTROL_MASK		(0xf << 0)
> +#define HPLLCC	0xc0 /* 85x only */
> +#define   GC_CLOCK_CONTROL_MASK		(0x7 << 0)
>  #define   GC_CLOCK_133_200		(0 << 0)
>  #define   GC_CLOCK_100_200		(1 << 0)
>  #define   GC_CLOCK_100_133		(2 << 0)
> -#define   GC_CLOCK_166_250		(3 << 0)
> +#define   GC_CLOCK_133_266		(3 << 0)
> +#define   GC_CLOCK_133_200_2		(4 << 0)
> +#define   GC_CLOCK_133_266_2		(5 << 0)
> +#define   GC_CLOCK_166_266		(6 << 0)
> +#define   GC_CLOCK_166_250		(7 << 0)
> +
>  #define GCFGC2	0xda
>  #define GCFGC	0xf0 /* 915+ only */
>  #define   GC_LOW_FREQUENCY_ENABLE	(1 << 7)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c97b496..64debfb 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6627,20 +6627,29 @@ static int i865_get_display_clock_speed(struct drm_device *dev)
>  	return 266667;
>  }
>  
> -static int i855_get_display_clock_speed(struct drm_device *dev)
> +static int i85x_get_display_clock_speed(struct drm_device *dev)
>  {
>  	u16 hpllcc = 0;
> +
> +	pci_bus_read_config_word(dev->pdev->bus,
> +				 PCI_DEVFN(0, 3), HPLLCC, &hpllcc);
> +
>  	/* Assume that the hardware is in the high speed state.  This
>  	 * should be the default.
>  	 */
>  	switch (hpllcc & GC_CLOCK_CONTROL_MASK) {
>  	case GC_CLOCK_133_200:
> +	case GC_CLOCK_133_200_2:
>  	case GC_CLOCK_100_200:
>  		return 200000;
>  	case GC_CLOCK_166_250:
>  		return 250000;
>  	case GC_CLOCK_100_133:
>  		return 133333;
> +	case GC_CLOCK_133_266:
> +	case GC_CLOCK_133_266_2:
> +	case GC_CLOCK_166_266:
> +		return 266667;
>  	}
>  
>  	/* Shouldn't happen */
> @@ -14199,8 +14208,8 @@ static void intel_init_display(struct drm_device *dev)
>  			i865_get_display_clock_speed;
>  	else if (IS_I85X(dev))
>  		dev_priv->display.get_display_clock_speed =
> -			i855_get_display_clock_speed;
> -	else /* 852, 830 */
> +			i85x_get_display_clock_speed;
> +	else /* 830 */
>  		dev_priv->display.get_display_clock_speed =
>  			i830_get_display_clock_speed;
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v4 02/12] drm/i915: Fix 852GM/GMV cdclk

[Mika Kahola]

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

It seems 852GM/GMV uses a different HPLLCC encoding than the other
85x platforms. For 852GM/GMV cdclk is always 133MHz. Try to detect that
using the PCI revision (sinc the device ID seems useless for that). I'm
not at all sure this is a good idea, but according to the specs it
should work.

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

v2: Rebased to the latest
v3: Rebased to the latest

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 64debfb..4b17aad 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6631,6 +6631,14 @@ static int i85x_get_display_clock_speed(struct drm_device *dev)
 {
 	u16 hpllcc = 0;
 
+	/*
+	 * 852GM/852GMV only supports 133 MHz and the HPLLCC
+	 * encoding is different :(
+	 * FIXME is this the right way to detect 852GM/852GMV?
+	 */
+	if (dev->pdev->revision == 0x1)
+		return 133333;
+
 	pci_bus_read_config_word(dev->pdev->bus,
 				 PCI_DEVFN(0, 3), HPLLCC, &hpllcc);
 
-- 
1.9.1

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

Re: [PATCH v4 02/12] drm/i915: Fix 852GM/GMV cdclk

[Damien Lespiau]

On Fri, May 22, 2015 at 11:22:32AM +0300, Mika Kahola wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> It seems 852GM/GMV uses a different HPLLCC encoding than the other
> 85x platforms. For 852GM/GMV cdclk is always 133MHz. Try to detect that
> using the PCI revision (sinc the device ID seems useless for that). I'm
> not at all sure this is a good idea, but according to the specs it
> should work.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> v2: Rebased to the latest
> v3: Rebased to the latest
> 
> Reviewed-by: Mika Kahola <mika.kahola@intel.com>

-ENODOC, FIXME and commit message -> not really inspiring.

Still, a probably better try then decoding HPLLCC.

Acked-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 64debfb..4b17aad 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6631,6 +6631,14 @@ static int i85x_get_display_clock_speed(struct drm_device *dev)
>  {
>  	u16 hpllcc = 0;
>  
> +	/*
> +	 * 852GM/852GMV only supports 133 MHz and the HPLLCC
> +	 * encoding is different :(
> +	 * FIXME is this the right way to detect 852GM/852GMV?
> +	 */
> +	if (dev->pdev->revision == 0x1)
> +		return 133333;
> +
>  	pci_bus_read_config_word(dev->pdev->bus,
>  				 PCI_DEVFN(0, 3), HPLLCC, &hpllcc);
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v4 03/12] drm/i915: Add cdclk extraction for g33, g965gm and g4x

[Mika Kahola]

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

Implement cdclk extraction for g33, 965gm and g4x platforms. The details
came from configdb. Sadly there isn't anything there for other gen3/gen4
chipsets.

So far I've tested this on one ELK where it gave me a HPLL VCO of 5333
MHz and cdclk of 444 MHz which seems perfectly sane for this machine.

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

v2: Rebased to the latest
v3: Rebased to the latest

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6625fb3..14366c8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2491,6 +2491,9 @@ enum skl_disp_power_wells {
 #define CLKCFG_MEM_800					(3 << 4)
 #define CLKCFG_MEM_MASK					(7 << 4)
 
+#define HPLLVCO                 (MCHBAR_MIRROR_BASE + 0xc38)
+#define HPLLVCO_MOBILE          (MCHBAR_MIRROR_BASE + 0xc0f)
+
 #define TSC1			0x11001
 #define   TSE			(1<<0)
 #define TR1			0x11006
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4b17aad..86c9140 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6669,6 +6669,175 @@ static int i830_get_display_clock_speed(struct drm_device *dev)
 	return 133333;
 }
 
+static unsigned int intel_hpll_vco(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	static const unsigned int blb_vco[8] = {
+		[0] = 3200000,
+		[1] = 4000000,
+		[2] = 5333333,
+		[3] = 4800000,
+		[4] = 6400000,
+	};
+	static const unsigned int pnv_vco[8] = {
+		[0] = 3200000,
+		[1] = 4000000,
+		[2] = 5333333,
+		[3] = 4800000,
+		[4] = 2666667,
+	};
+	static const unsigned int cl_vco[8] = {
+		[0] = 3200000,
+		[1] = 4000000,
+		[2] = 5333333,
+		[3] = 6400000,
+		[4] = 3333333,
+		[5] = 3566667,
+		[6] = 4266667,
+	};
+	static const unsigned int elk_vco[8] = {
+		[0] = 3200000,
+		[1] = 4000000,
+		[2] = 5333333,
+		[3] = 4800000,
+	};
+	static const unsigned int ctg_vco[8] = {
+		[0] = 3200000,
+		[1] = 4000000,
+		[2] = 5333333,
+		[3] = 6400000,
+		[4] = 2666667,
+		[5] = 4266667,
+	};
+	const unsigned int *vco_table;
+	unsigned int vco;
+	uint8_t tmp = 0;
+
+	/* FIXME other chipsets? */
+	if (IS_GM45(dev))
+		vco_table = ctg_vco;
+	else if (IS_G4X(dev))
+		vco_table = elk_vco;
+	else if (IS_CRESTLINE(dev))
+		vco_table = cl_vco;
+	else if (IS_PINEVIEW(dev))
+		vco_table = pnv_vco;
+	else if (IS_G33(dev))
+		vco_table = blb_vco;
+	else
+		return 0;
+
+	tmp = I915_READ(IS_MOBILE(dev) ? HPLLVCO_MOBILE : HPLLVCO);
+
+	vco = vco_table[tmp & 0x7];
+	if (vco == 0)
+		DRM_ERROR("Bad HPLL VCO (HPLLVCO=0x%02x)\n", tmp);
+	else
+		DRM_DEBUG_KMS("HPLL VCO %u kHz\n", vco);
+
+	return vco;
+}
+
+static int gm45_get_display_clock_speed(struct drm_device *dev)
+{
+	unsigned int cdclk_sel, vco = intel_hpll_vco(dev);
+	uint16_t tmp = 0;
+
+	pci_read_config_word(dev->pdev, GCFGC, &tmp);
+
+	cdclk_sel = (tmp >> 12) & 0x1;
+
+	switch (vco) {
+	case 2666667:
+	case 4000000:
+	case 5333333:
+		return cdclk_sel ? 333333 : 222222;
+	case 3200000:
+		return cdclk_sel ? 320000 : 228571;
+	default:
+		DRM_ERROR("Unable to determine CDCLK. HPLL VCO=%u, CFGC=0x%04x\n", vco, tmp);
+		return 222222;
+	}
+}
+
+static int i965gm_get_display_clock_speed(struct drm_device *dev)
+{
+	static const uint8_t div_3200[] = { 16, 10,  8 };
+	static const uint8_t div_4000[] = { 20, 12, 10 };
+	static const uint8_t div_5333[] = { 24, 16, 14 };
+	const uint8_t *div_table;
+	unsigned int cdclk_sel, vco = intel_hpll_vco(dev);
+	uint16_t tmp = 0;
+
+	pci_read_config_word(dev->pdev, GCFGC, &tmp);
+
+	cdclk_sel = ((tmp >> 8) & 0x1f) - 1;
+
+	if (cdclk_sel >= ARRAY_SIZE(div_3200))
+		goto fail;
+
+	switch (vco) {
+	case 3200000:
+		div_table = div_3200;
+		break;
+	case 4000000:
+		div_table = div_4000;
+		break;
+	case 5333333:
+		div_table = div_5333;
+		break;
+	default:
+		goto fail;
+	}
+
+	return DIV_ROUND_CLOSEST(vco, div_table[cdclk_sel]);
+
+ fail:
+	DRM_ERROR("Unable to determine CDCLK. HPLL VCO=%u kHz, CFGC=0x%04x\n", vco, tmp);
+	return 200000;
+}
+
+static int g33_get_display_clock_speed(struct drm_device *dev)
+{
+	static const uint8_t div_3200[] = { 12, 10,  8,  7, 5, 16 };
+	static const uint8_t div_4000[] = { 14, 12, 10,  8, 6, 20 };
+	static const uint8_t div_4800[] = { 20, 14, 12, 10, 8, 24 };
+	static const uint8_t div_5333[] = { 20, 16, 12, 12, 8, 28 };
+	const uint8_t *div_table;
+	unsigned int cdclk_sel, vco = intel_hpll_vco(dev);
+	uint16_t tmp = 0;
+
+	pci_read_config_word(dev->pdev, GCFGC, &tmp);
+
+	cdclk_sel = (tmp >> 4) & 0x7;
+
+	if (cdclk_sel >= ARRAY_SIZE(div_3200))
+		goto fail;
+
+	switch (vco) {
+	case 3200000:
+		div_table = div_3200;
+		break;
+	case 4000000:
+		div_table = div_4000;
+		break;
+	case 4800000:
+		div_table = div_4800;
+		break;
+	case 5333333:
+		div_table = div_5333;
+		break;
+	default:
+		goto fail;
+	}
+
+	return DIV_ROUND_CLOSEST(vco, div_table[cdclk_sel]);
+
+ fail:
+	DRM_ERROR("Unable to determine CDCLK. HPLL VCO=%u kHz, CFGC=0x%08x\n", vco, tmp);
+	return 190476;
+}
+
 static void
 intel_reduce_m_n_ratio(uint32_t *num, uint32_t *den)
 {
@@ -14196,9 +14365,21 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.get_display_clock_speed =
 			ilk_get_display_clock_speed;
 	else if (IS_I945G(dev) || IS_BROADWATER(dev) ||
-		 IS_GEN6(dev) || IS_IVYBRIDGE(dev) || (IS_G33(dev) && !IS_PINEVIEW_M(dev)))
+		 IS_GEN6(dev) || IS_IVYBRIDGE(dev))
 		dev_priv->display.get_display_clock_speed =
 			i945_get_display_clock_speed;
+	else if (IS_GM45(dev))
+		dev_priv->display.get_display_clock_speed =
+			gm45_get_display_clock_speed;
+	else if (IS_CRESTLINE(dev))
+		dev_priv->display.get_display_clock_speed =
+			i965gm_get_display_clock_speed;
+	else if (IS_PINEVIEW(dev))
+		dev_priv->display.get_display_clock_speed =
+			pnv_get_display_clock_speed;
+	else if (IS_G33(dev) || IS_G4X(dev))
+		dev_priv->display.get_display_clock_speed =
+			g33_get_display_clock_speed;
 	else if (IS_I915G(dev))
 		dev_priv->display.get_display_clock_speed =
 			i915_get_display_clock_speed;
-- 
1.9.1

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

Re: [PATCH v4 03/12] drm/i915: Add cdclk extraction for g33, g965gm and g4x

[Damien Lespiau]

On Fri, May 22, 2015 at 11:22:33AM +0300, Mika Kahola wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Implement cdclk extraction for g33, 965gm and g4x platforms. The details
> came from configdb. Sadly there isn't anything there for other gen3/gen4
> chipsets.
> 
> So far I've tested this on one ELK where it gave me a HPLL VCO of 5333
> MHz and cdclk of 444 MHz which seems perfectly sane for this machine.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> v2: Rebased to the latest
> v3: Rebased to the latest
> 
> Reviewed-by: Mika Kahola <mika.kahola@intel.com>
> 
> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>

Acked-by: Damien Lespiau <damien.lespiau@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h      |   3 +
>  drivers/gpu/drm/i915/intel_display.c | 183 ++++++++++++++++++++++++++++++++++-
>  2 files changed, 185 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 6625fb3..14366c8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2491,6 +2491,9 @@ enum skl_disp_power_wells {
>  #define CLKCFG_MEM_800					(3 << 4)
>  #define CLKCFG_MEM_MASK					(7 << 4)
>  
> +#define HPLLVCO                 (MCHBAR_MIRROR_BASE + 0xc38)
> +#define HPLLVCO_MOBILE          (MCHBAR_MIRROR_BASE + 0xc0f)
> +
>  #define TSC1			0x11001
>  #define   TSE			(1<<0)
>  #define TR1			0x11006
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4b17aad..86c9140 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6669,6 +6669,175 @@ static int i830_get_display_clock_speed(struct drm_device *dev)
>  	return 133333;
>  }
>  
> +static unsigned int intel_hpll_vco(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	static const unsigned int blb_vco[8] = {
> +		[0] = 3200000,
> +		[1] = 4000000,
> +		[2] = 5333333,
> +		[3] = 4800000,
> +		[4] = 6400000,
> +	};
> +	static const unsigned int pnv_vco[8] = {
> +		[0] = 3200000,
> +		[1] = 4000000,
> +		[2] = 5333333,
> +		[3] = 4800000,
> +		[4] = 2666667,
> +	};
> +	static const unsigned int cl_vco[8] = {
> +		[0] = 3200000,
> +		[1] = 4000000,
> +		[2] = 5333333,
> +		[3] = 6400000,
> +		[4] = 3333333,
> +		[5] = 3566667,
> +		[6] = 4266667,
> +	};
> +	static const unsigned int elk_vco[8] = {
> +		[0] = 3200000,
> +		[1] = 4000000,
> +		[2] = 5333333,
> +		[3] = 4800000,
> +	};
> +	static const unsigned int ctg_vco[8] = {
> +		[0] = 3200000,
> +		[1] = 4000000,
> +		[2] = 5333333,
> +		[3] = 6400000,
> +		[4] = 2666667,
> +		[5] = 4266667,
> +	};
> +	const unsigned int *vco_table;
> +	unsigned int vco;
> +	uint8_t tmp = 0;
> +
> +	/* FIXME other chipsets? */
> +	if (IS_GM45(dev))
> +		vco_table = ctg_vco;
> +	else if (IS_G4X(dev))
> +		vco_table = elk_vco;
> +	else if (IS_CRESTLINE(dev))
> +		vco_table = cl_vco;
> +	else if (IS_PINEVIEW(dev))
> +		vco_table = pnv_vco;
> +	else if (IS_G33(dev))
> +		vco_table = blb_vco;
> +	else
> +		return 0;
> +
> +	tmp = I915_READ(IS_MOBILE(dev) ? HPLLVCO_MOBILE : HPLLVCO);
> +
> +	vco = vco_table[tmp & 0x7];
> +	if (vco == 0)
> +		DRM_ERROR("Bad HPLL VCO (HPLLVCO=0x%02x)\n", tmp);
> +	else
> +		DRM_DEBUG_KMS("HPLL VCO %u kHz\n", vco);
> +
> +	return vco;
> +}
> +
> +static int gm45_get_display_clock_speed(struct drm_device *dev)
> +{
> +	unsigned int cdclk_sel, vco = intel_hpll_vco(dev);
> +	uint16_t tmp = 0;
> +
> +	pci_read_config_word(dev->pdev, GCFGC, &tmp);
> +
> +	cdclk_sel = (tmp >> 12) & 0x1;
> +
> +	switch (vco) {
> +	case 2666667:
> +	case 4000000:
> +	case 5333333:
> +		return cdclk_sel ? 333333 : 222222;
> +	case 3200000:
> +		return cdclk_sel ? 320000 : 228571;
> +	default:
> +		DRM_ERROR("Unable to determine CDCLK. HPLL VCO=%u, CFGC=0x%04x\n", vco, tmp);
> +		return 222222;
> +	}
> +}
> +
> +static int i965gm_get_display_clock_speed(struct drm_device *dev)
> +{
> +	static const uint8_t div_3200[] = { 16, 10,  8 };
> +	static const uint8_t div_4000[] = { 20, 12, 10 };
> +	static const uint8_t div_5333[] = { 24, 16, 14 };
> +	const uint8_t *div_table;
> +	unsigned int cdclk_sel, vco = intel_hpll_vco(dev);
> +	uint16_t tmp = 0;
> +
> +	pci_read_config_word(dev->pdev, GCFGC, &tmp);
> +
> +	cdclk_sel = ((tmp >> 8) & 0x1f) - 1;
> +
> +	if (cdclk_sel >= ARRAY_SIZE(div_3200))
> +		goto fail;
> +
> +	switch (vco) {
> +	case 3200000:
> +		div_table = div_3200;
> +		break;
> +	case 4000000:
> +		div_table = div_4000;
> +		break;
> +	case 5333333:
> +		div_table = div_5333;
> +		break;
> +	default:
> +		goto fail;
> +	}
> +
> +	return DIV_ROUND_CLOSEST(vco, div_table[cdclk_sel]);
> +
> + fail:
> +	DRM_ERROR("Unable to determine CDCLK. HPLL VCO=%u kHz, CFGC=0x%04x\n", vco, tmp);
> +	return 200000;
> +}
> +
> +static int g33_get_display_clock_speed(struct drm_device *dev)
> +{
> +	static const uint8_t div_3200[] = { 12, 10,  8,  7, 5, 16 };
> +	static const uint8_t div_4000[] = { 14, 12, 10,  8, 6, 20 };
> +	static const uint8_t div_4800[] = { 20, 14, 12, 10, 8, 24 };
> +	static const uint8_t div_5333[] = { 20, 16, 12, 12, 8, 28 };
> +	const uint8_t *div_table;
> +	unsigned int cdclk_sel, vco = intel_hpll_vco(dev);
> +	uint16_t tmp = 0;
> +
> +	pci_read_config_word(dev->pdev, GCFGC, &tmp);
> +
> +	cdclk_sel = (tmp >> 4) & 0x7;
> +
> +	if (cdclk_sel >= ARRAY_SIZE(div_3200))
> +		goto fail;
> +
> +	switch (vco) {
> +	case 3200000:
> +		div_table = div_3200;
> +		break;
> +	case 4000000:
> +		div_table = div_4000;
> +		break;
> +	case 4800000:
> +		div_table = div_4800;
> +		break;
> +	case 5333333:
> +		div_table = div_5333;
> +		break;
> +	default:
> +		goto fail;
> +	}
> +
> +	return DIV_ROUND_CLOSEST(vco, div_table[cdclk_sel]);
> +
> + fail:
> +	DRM_ERROR("Unable to determine CDCLK. HPLL VCO=%u kHz, CFGC=0x%08x\n", vco, tmp);
> +	return 190476;
> +}
> +
>  static void
>  intel_reduce_m_n_ratio(uint32_t *num, uint32_t *den)
>  {
> @@ -14196,9 +14365,21 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.get_display_clock_speed =
>  			ilk_get_display_clock_speed;
>  	else if (IS_I945G(dev) || IS_BROADWATER(dev) ||
> -		 IS_GEN6(dev) || IS_IVYBRIDGE(dev) || (IS_G33(dev) && !IS_PINEVIEW_M(dev)))
> +		 IS_GEN6(dev) || IS_IVYBRIDGE(dev))
>  		dev_priv->display.get_display_clock_speed =
>  			i945_get_display_clock_speed;
> +	else if (IS_GM45(dev))
> +		dev_priv->display.get_display_clock_speed =
> +			gm45_get_display_clock_speed;
> +	else if (IS_CRESTLINE(dev))
> +		dev_priv->display.get_display_clock_speed =
> +			i965gm_get_display_clock_speed;
> +	else if (IS_PINEVIEW(dev))
> +		dev_priv->display.get_display_clock_speed =
> +			pnv_get_display_clock_speed;
> +	else if (IS_G33(dev) || IS_G4X(dev))
> +		dev_priv->display.get_display_clock_speed =
> +			g33_get_display_clock_speed;
>  	else if (IS_I915G(dev))
>  		dev_priv->display.get_display_clock_speed =
>  			i915_get_display_clock_speed;
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v4 04/12] drm/i915: Warn when cdclk for the platforms is not known

[Mika Kahola]

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

Print a warning if we fall through the .get_display_clock_speed() function
pointer setup. We end up assuming a 133MHz cdclk which should mean that
at least we avoid any 0 deivisions and whatnot. But this could at least
help remind people that they have to provide this function for new platforms.

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

v2: Rebased to the latest
v3: Rebased to the latest

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 86c9140..de65737 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14398,9 +14398,11 @@ static void intel_init_display(struct drm_device *dev)
 	else if (IS_I85X(dev))
 		dev_priv->display.get_display_clock_speed =
 			i85x_get_display_clock_speed;
-	else /* 830 */
+	else { /* 830 */
+		WARN(!IS_I830(dev), "Unknown platform. Assuming 133 MHz CDCLK\n");
 		dev_priv->display.get_display_clock_speed =
 			i830_get_display_clock_speed;
+	}
 
 	if (IS_GEN5(dev)) {
 		dev_priv->display.fdi_link_train = ironlake_fdi_link_train;
-- 
1.9.1

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

Re: [PATCH v4 04/12] drm/i915: Warn when cdclk for the platforms is not known

[Damien Lespiau]

On Fri, May 22, 2015 at 11:22:34AM +0300, Mika Kahola wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Print a warning if we fall through the .get_display_clock_speed() function
> pointer setup. We end up assuming a 133MHz cdclk which should mean that
> at least we avoid any 0 deivisions and whatnot. But this could at least
> help remind people that they have to provide this function for new platforms.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> v2: Rebased to the latest
> v3: Rebased to the latest
> 
> Reviewed-by: Mika Kahola <mika.kahola@intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

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

Re: [PATCH v4 04/12] drm/i915: Warn when cdclk for the platforms is not known

[Daniel Vetter]

On Thu, May 28, 2015 at 07:19:51PM +0100, Damien Lespiau wrote:
> On Fri, May 22, 2015 at 11:22:34AM +0300, Mika Kahola wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Print a warning if we fall through the .get_display_clock_speed() function
> > pointer setup. We end up assuming a 133MHz cdclk which should mean that
> > at least we avoid any 0 deivisions and whatnot. But this could at least
> > help remind people that they have to provide this function for new platforms.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > v2: Rebased to the latest
> > v3: Rebased to the latest
> > 
> > Reviewed-by: Mika Kahola <mika.kahola@intel.com>
> 
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

Merged up to this one here, with the sob section of commit messages
aligned to what we usually do.

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

[PATCH v4 05/12] drm/i915: Cache current cdclk frequency in dev_priv

[Mika Kahola]

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

Rather that extracting the current cdclk freuqncy every time someone
wants to know it, cache the current value and use that. VLV/CHV already
stored a cached value there so just expand that to cover all platforms.

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

v2: Rebased to the latest
v3: Rebased to the latest

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index de65737..75b11d6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5541,7 +5541,7 @@ static int valleyview_get_vco(struct drm_i915_private *dev_priv)
 	return vco_freq[hpll_freq] * 1000;
 }
 
-static void vlv_update_cdclk(struct drm_device *dev)
+static void intel_update_cdclk(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
@@ -5554,7 +5554,14 @@ static void vlv_update_cdclk(struct drm_device *dev)
 	 * BSpec erroneously claims we should aim for 4MHz, but
 	 * in fact 1MHz is the correct frequency.
 	 */
-	I915_WRITE(GMBUSFREQ_VLV, DIV_ROUND_UP(dev_priv->cdclk_freq, 1000));
+	if (IS_VALLEYVIEW(dev)) {
+		/*
+		 * Program the gmbus_freq based on the cdclk frequency.
+		 * BSpec erroneously claims we should aim for 4MHz, but
+		 * in fact 1MHz is the correct frequency.
+		 */
+		I915_WRITE(GMBUSFREQ_VLV, DIV_ROUND_UP(dev_priv->cdclk_freq, 1000));
+	}
 }
 
 /* Adjust CDclk dividers to allow high res or save power if possible */
@@ -5620,7 +5627,7 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
 	vlv_bunit_write(dev_priv, BUNIT_REG_BISOC, val);
 	mutex_unlock(&dev_priv->dpio_lock);
 
-	vlv_update_cdclk(dev);
+	intel_update_cdclk(dev);
 }
 
 static void cherryview_set_cdclk(struct drm_device *dev, int cdclk)
@@ -5661,7 +5668,7 @@ static void cherryview_set_cdclk(struct drm_device *dev, int cdclk)
 	}
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
-	vlv_update_cdclk(dev);
+	intel_update_cdclk(dev);
 }
 
 static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
@@ -5850,6 +5857,8 @@ static void valleyview_modeset_global_resources(struct drm_atomic_state *old_sta
 
 		intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
 	}
+
+	intel_update_cdclk(dev);
 }
 
 static void valleyview_crtc_enable(struct drm_crtc *crtc)
@@ -9273,6 +9282,7 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
 	}
 
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
+	intel_update_cdclk(dev_priv->dev);
 }
 
 /*
@@ -13065,6 +13075,8 @@ static void intel_shared_dpll_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	intel_update_cdclk(dev);
+
 	if (HAS_DDI(dev))
 		intel_ddi_pll_init(dev);
 	else if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
@@ -14637,10 +14649,9 @@ static void i915_disable_vga(struct drm_device *dev)
 
 void intel_modeset_init_hw(struct drm_device *dev)
 {
-	intel_prepare_ddi(dev);
+	intel_update_cdclk(dev);
 
-	if (IS_VALLEYVIEW(dev))
-		vlv_update_cdclk(dev);
+	intel_prepare_ddi(dev);
 
 	intel_init_clock_gating(dev);
 
-- 
1.9.1

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

Re: [PATCH v4 05/12] drm/i915: Cache current cdclk frequency in dev_priv

[Damien Lespiau]

On Fri, May 22, 2015 at 11:22:35AM +0300, Mika Kahola wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Rather that extracting the current cdclk freuqncy every time someone
> wants to know it, cache the current value and use that. VLV/CHV already
> stored a cached value there so just expand that to cover all platforms.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> v2: Rebased to the latest
> v3: Rebased to the latest
> 
> Reviewed-by: Mika Kahola <mika.kahola@intel.com>
> 
> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 25 ++++++++++++++++++-------
>  static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
> @@ -5850,6 +5857,8 @@ static void valleyview_modeset_global_resources(struct drm_atomic_state *old_sta
>  
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
>  	}
> +
> +	intel_update_cdclk(dev);
>  }

This ones looks like a spurious one to me.

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

[PATCH v4 06/12] drm/i915: Use cached cdclk value

[Mika Kahola]

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

Rather than reading out the current cdclk value use the cached value we
have tucked away in dev_priv.

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

v2: Rebased to the latest
v3: Rebased to the latest

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 3 +--
 drivers/gpu/drm/i915/intel_dp.c      | 5 +++--
 drivers/gpu/drm/i915/intel_pm.c      | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 75b11d6..fcd4112 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6406,8 +6406,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 
 	/* FIXME should check pixel clock limits on all platforms */
 	if (INTEL_INFO(dev)->gen < 4) {
-		int clock_limit =
-			dev_priv->display.get_display_clock_speed(dev);
+		int clock_limit = dev_priv->cdclk_freq;
 
 		/*
 		 * Enable pixel doubling when the dot clock
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0edc305..2dd4d28 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -708,7 +708,8 @@ 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->display.get_display_clock_speed(dev), 2000);
+		return DIV_ROUND_UP(dev_priv->cdclk_freq, 2000);
+
 	} else {
 		return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
 	}
@@ -723,7 +724,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 	if (intel_dig_port->port == PORT_A) {
 		if (index)
 			return 0;
-		return DIV_ROUND_CLOSEST(dev_priv->display.get_display_clock_speed(dev), 2000);
+		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
 	} else if (dev_priv->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) {
 		/* Workaround for non-ULT HSW */
 		switch (index) {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ce1d079..0046cb4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1815,7 +1815,7 @@ hsw_compute_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
 	linetime = DIV_ROUND_CLOSEST(mode->crtc_htotal * 1000 * 8,
 				     mode->crtc_clock);
 	ips_linetime = DIV_ROUND_CLOSEST(mode->crtc_htotal * 1000 * 8,
-					 dev_priv->display.get_display_clock_speed(dev_priv->dev));
+					 dev_priv->cdclk_freq);
 
 	return PIPE_WM_LINETIME_IPS_LINETIME(ips_linetime) |
 	       PIPE_WM_LINETIME_TIME(linetime);
-- 
1.9.1

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

Re: [PATCH v4 06/12] drm/i915: Use cached cdclk value

[Damien Lespiau]

On Fri, May 22, 2015 at 11:22:36AM +0300, Mika Kahola wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Rather than reading out the current cdclk value use the cached value we
> have tucked away in dev_priv.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> v2: Rebased to the latest
> v3: Rebased to the latest
> 
> Reviewed-by: Mika Kahola <mika.kahola@intel.com>
> 
> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>


> ---
>  drivers/gpu/drm/i915/intel_display.c | 3 +--
>  drivers/gpu/drm/i915/intel_dp.c      | 5 +++--
>  drivers/gpu/drm/i915/intel_pm.c      | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 75b11d6..fcd4112 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6406,8 +6406,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  
>  	/* FIXME should check pixel clock limits on all platforms */
>  	if (INTEL_INFO(dev)->gen < 4) {
> -		int clock_limit =
> -			dev_priv->display.get_display_clock_speed(dev);
> +		int clock_limit = dev_priv->cdclk_freq;
>  
>  		/*
>  		 * Enable pixel doubling when the dot clock
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0edc305..2dd4d28 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -708,7 +708,8 @@ 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->display.get_display_clock_speed(dev), 2000);
> +		return DIV_ROUND_UP(dev_priv->cdclk_freq, 2000);
> +
>  	} else {
>  		return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
>  	}
> @@ -723,7 +724,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  	if (intel_dig_port->port == PORT_A) {
>  		if (index)
>  			return 0;
> -		return DIV_ROUND_CLOSEST(dev_priv->display.get_display_clock_speed(dev), 2000);
> +		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
>  	} else if (dev_priv->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) {
>  		/* Workaround for non-ULT HSW */
>  		switch (index) {
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ce1d079..0046cb4 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1815,7 +1815,7 @@ hsw_compute_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>  	linetime = DIV_ROUND_CLOSEST(mode->crtc_htotal * 1000 * 8,
>  				     mode->crtc_clock);
>  	ips_linetime = DIV_ROUND_CLOSEST(mode->crtc_htotal * 1000 * 8,
> -					 dev_priv->display.get_display_clock_speed(dev_priv->dev));
> +					 dev_priv->cdclk_freq);
>  
>  	return PIPE_WM_LINETIME_IPS_LINETIME(ips_linetime) |
>  	       PIPE_WM_LINETIME_TIME(linetime);
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v4 07/12] drm/i915: Unify ilk and hsw .get_aux_clock_divider

[Mika Kahola]

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

ilk_get_aux_clock_divider() is now a subset of
hsw_get_aux_clock_divider() so unify them.

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

v2: Rebased to the latest
v3: Rebased to the latest

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2dd4d28..8d9c928 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -704,23 +704,6 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (index)
-		return 0;
-
-	if (intel_dig_port->port == PORT_A) {
-		return DIV_ROUND_UP(dev_priv->cdclk_freq, 2000);
-
-	} else {
-		return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
-	}
-}
-
-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;
-
 	if (intel_dig_port->port == PORT_A) {
 		if (index)
 			return 0;
@@ -733,7 +716,9 @@ 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);
+		if (index)
+			return 0;
+		return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
 	}
 }
 
@@ -5731,8 +5716,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		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))
 		intel_dp->get_aux_clock_divider = ilk_get_aux_clock_divider;
 	else
-- 
1.9.1

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

Re: [PATCH v4 07/12] drm/i915: Unify ilk and hsw .get_aux_clock_divider

[Damien Lespiau]

On Fri, May 22, 2015 at 11:22:37AM +0300, Mika Kahola wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> ilk_get_aux_clock_divider() is now a subset of
> hsw_get_aux_clock_divider() so unify them.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> v2: Rebased to the latest
> v3: Rebased to the latest
> 
> Reviewed-by: Mika Kahola <mika.kahola@intel.com>
> 
> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

>  drivers/gpu/drm/i915/intel_dp.c | 23 +++--------------------
>  1 file changed, 3 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 2dd4d28..8d9c928 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -704,23 +704,6 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>  	struct drm_device *dev = intel_dig_port->base.base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	if (index)
> -		return 0;
> -
> -	if (intel_dig_port->port == PORT_A) {
> -		return DIV_ROUND_UP(dev_priv->cdclk_freq, 2000);
> -
> -	} else {
> -		return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
> -	}
> -}
> -
> -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;
> -
>  	if (intel_dig_port->port == PORT_A) {
>  		if (index)
>  			return 0;
> @@ -733,7 +716,9 @@ 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);
> +		if (index)
> +			return 0;
> +		return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
>  	}
>  }
>  
> @@ -5731,8 +5716,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  		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))
>  		intel_dp->get_aux_clock_divider = ilk_get_aux_clock_divider;
>  	else
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v4 8/8] drm/i915: Store max cdclk value in dev_priv

[Mika Kahola]

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

Keep the cdclk maximum supported frequency around in dev_priv so that we
can verify certain things against it before actually changing the cdclk
frequency.

For now only VLV/CHV have support changing cdclk frequency, so other
plarforms get to assume cdclk is fixed.

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

v2: Rebased to the latest
v3: Rebased to the latest

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 20 +++++++++++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1038f5c..186a9e6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1712,7 +1712,7 @@ struct drm_i915_private {
 
 	unsigned int fsb_freq, mem_freq, is_ddr3;
 	unsigned int skl_boot_cdclk;
-	unsigned int cdclk_freq;
+	unsigned int cdclk_freq, max_cdclk_freq;
 	unsigned int hpll_freq;
 
 	/**
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1bdd2d7..51cbbca 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5750,6 +5750,21 @@ static int valleyview_get_vco(struct drm_i915_private *dev_priv)
 	return vco_freq[hpll_freq] * 1000;
 }
 
+static void intel_update_max_cdclk(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (IS_VALLEYVIEW(dev)) {
+		dev_priv->max_cdclk_freq = 400000;
+	} else {
+		/* otherwise assume cdclk is fixed */
+		dev_priv->max_cdclk_freq = dev_priv->cdclk_freq;
+	}
+
+	DRM_DEBUG_DRIVER("Max CD clock rate: %d kHz\n",
+			 dev_priv->max_cdclk_freq);
+}
+
 static void intel_update_cdclk(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -5771,6 +5786,9 @@ static void intel_update_cdclk(struct drm_device *dev)
 		 */
 		I915_WRITE(GMBUSFREQ_VLV, DIV_ROUND_UP(dev_priv->cdclk_freq, 1000));
 	}
+
+	if (dev_priv->max_cdclk_freq == 0)
+		intel_update_max_cdclk(dev);
 }
 
 /* Adjust CDclk dividers to allow high res or save power if possible */
@@ -6615,7 +6633,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 
 	/* FIXME should check pixel clock limits on all platforms */
 	if (INTEL_INFO(dev)->gen < 4) {
-		int clock_limit = dev_priv->cdclk_freq;
+		int clock_limit = dev_priv->max_cdclk_freq;
 
 		/*
 		 * Enable pixel doubling when the dot clock
-- 
1.9.1

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

Re: [PATCH v4 8/8] drm/i915: Store max cdclk value in dev_priv

[Damien Lespiau]

On Fri, May 22, 2015 at 11:22:38AM +0300, Mika Kahola wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Keep the cdclk maximum supported frequency around in dev_priv so that we
> can verify certain things against it before actually changing the cdclk
> frequency.
> 
> For now only VLV/CHV have support changing cdclk frequency, so other
> plarforms get to assume cdclk is fixed.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> v2: Rebased to the latest
> v3: Rebased to the latest
> 
> Reviewed-by: Mika Kahola <mika.kahola@intel.com>
> 
> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

>  drivers/gpu/drm/i915/i915_drv.h      |  2 +-
>  drivers/gpu/drm/i915/intel_display.c | 20 +++++++++++++++++++-
>  2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1038f5c..186a9e6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1712,7 +1712,7 @@ struct drm_i915_private {
>  
>  	unsigned int fsb_freq, mem_freq, is_ddr3;
>  	unsigned int skl_boot_cdclk;
> -	unsigned int cdclk_freq;
> +	unsigned int cdclk_freq, max_cdclk_freq;
>  	unsigned int hpll_freq;
>  
>  	/**
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1bdd2d7..51cbbca 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5750,6 +5750,21 @@ static int valleyview_get_vco(struct drm_i915_private *dev_priv)
>  	return vco_freq[hpll_freq] * 1000;
>  }
>  
> +static void intel_update_max_cdclk(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	if (IS_VALLEYVIEW(dev)) {
> +		dev_priv->max_cdclk_freq = 400000;
> +	} else {
> +		/* otherwise assume cdclk is fixed */
> +		dev_priv->max_cdclk_freq = dev_priv->cdclk_freq;
> +	}
> +
> +	DRM_DEBUG_DRIVER("Max CD clock rate: %d kHz\n",
> +			 dev_priv->max_cdclk_freq);
> +}
> +
>  static void intel_update_cdclk(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -5771,6 +5786,9 @@ static void intel_update_cdclk(struct drm_device *dev)
>  		 */
>  		I915_WRITE(GMBUSFREQ_VLV, DIV_ROUND_UP(dev_priv->cdclk_freq, 1000));
>  	}
> +
> +	if (dev_priv->max_cdclk_freq == 0)
> +		intel_update_max_cdclk(dev);
>  }
>  
>  /* Adjust CDclk dividers to allow high res or save power if possible */
> @@ -6615,7 +6633,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  
>  	/* FIXME should check pixel clock limits on all platforms */
>  	if (INTEL_INFO(dev)->gen < 4) {
> -		int clock_limit = dev_priv->cdclk_freq;
> +		int clock_limit = dev_priv->max_cdclk_freq;
>  
>  		/*
>  		 * Enable pixel doubling when the dot clock
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v4 09/12] drm/i915: Don't enable IPS when pixel rate exceeds 95%

[Mika Kahola]

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

Bspec says we shouldn't enable IPS on BDW when the pipe pixel rate
exceeds 95% of the core display clock. Apparently this can cause
underruns.

There's no similar restriction listed for HSW, so leave that one alone
for now.

v2: Add pipe_config_supports_ips() (Chris)
v3: Compare against the max cdclk insted of the current cdclk

Tested-by: Timo Aaltonen <tjaalton@ubuntu.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83497
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

v4: Rebased to the latest
v5: Rebased to the latest

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h     |  2 +-
 drivers/gpu/drm/i915/intel_pm.c      | 17 ++++++++---------
 3 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7db1eee..febf993 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6406,12 +6406,38 @@ retry:
 	return ret;
 }
 
+static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv,
+				     struct intel_crtc_state *pipe_config)
+{
+	if (pipe_config->pipe_bpp > 24)
+		return false;
+
+	/* HSW can handle pixel rate up to cdclk? */
+	if (IS_HASWELL(dev_priv->dev))
+		return true;
+
+	/*
+	 * FIXME if we compare against max we should then
+	 * increase the cdclk frequency when the current
+	 * value is too low. The other option is to compare
+	 * against the cdclk frequency we're going have post
+	 * modeset (ie. one we computed using other constraints).
+	 * Need to measure whether using a lower cdclk w/o IPS
+	 * is better or worse than a higher cdclk w/ IPS.
+	 */
+	return ilk_pipe_pixel_rate(pipe_config) <=
+		dev_priv->max_cdclk_freq * 95 / 100;
+}
+
 static void hsw_compute_ips_config(struct intel_crtc *crtc,
 				   struct intel_crtc_state *pipe_config)
 {
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
 	pipe_config->ips_enabled = i915.enable_ips &&
-				   hsw_crtc_supports_ips(crtc) &&
-				   pipe_config->pipe_bpp <= 24;
+		hsw_crtc_supports_ips(crtc) &&
+		pipe_config_supports_ips(dev_priv, pipe_config);
 }
 
 static int intel_crtc_compute_config(struct intel_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 47bc729..09df4b5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1371,7 +1371,7 @@ void ilk_wm_get_hw_state(struct drm_device *dev);
 void skl_wm_get_hw_state(struct drm_device *dev);
 void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
 			  struct skl_ddb_allocation *ddb /* out */);
-
+uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
 
 /* intel_sdvo.c */
 bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0046cb4..282ad59 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1434,23 +1434,22 @@ static void i845_update_wm(struct drm_crtc *unused_crtc)
 	I915_WRITE(FW_BLC, fwater_lo);
 }
 
-static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
-				    struct drm_crtc *crtc)
+uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config)
 {
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	uint32_t pixel_rate;
 
-	pixel_rate = intel_crtc->config->base.adjusted_mode.crtc_clock;
+	pixel_rate = pipe_config->base.adjusted_mode.crtc_clock;
 
 	/* We only use IF-ID interlacing. If we ever use PF-ID we'll need to
 	 * adjust the pixel_rate here. */
 
-	if (intel_crtc->config->pch_pfit.enabled) {
+	if (pipe_config->pch_pfit.enabled) {
 		uint64_t pipe_w, pipe_h, pfit_w, pfit_h;
-		uint32_t pfit_size = intel_crtc->config->pch_pfit.size;
+		uint32_t pfit_size = pipe_config->pch_pfit.size;
+
+		pipe_w = pipe_config->pipe_src_w;
+		pipe_h = pipe_config->pipe_src_h;
 
-		pipe_w = intel_crtc->config->pipe_src_w;
-		pipe_h = intel_crtc->config->pipe_src_h;
 		pfit_w = (pfit_size >> 16) & 0xFFFF;
 		pfit_h = pfit_size & 0xFFFF;
 		if (pipe_w < pfit_w)
@@ -2066,7 +2065,7 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
 
 	p->active = true;
 	p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
-	p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
+	p->pixel_rate = ilk_pipe_pixel_rate(intel_crtc->config);
 
 	if (crtc->primary->state->fb)
 		p->pri.bytes_per_pixel =
-- 
1.9.1

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

Re: [PATCH v4 09/12] drm/i915: Don't enable IPS when pixel rate exceeds 95%

[Damien Lespiau]

On Fri, May 22, 2015 at 11:22:39AM +0300, Mika Kahola wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Bspec says we shouldn't enable IPS on BDW when the pipe pixel rate
> exceeds 95% of the core display clock. Apparently this can cause
> underruns.
> 
> There's no similar restriction listed for HSW, so leave that one alone
> for now.
> 
> v2: Add pipe_config_supports_ips() (Chris)
> v3: Compare against the max cdclk insted of the current cdclk
> 
> Tested-by: Timo Aaltonen <tjaalton@ubuntu.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83497
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> v4: Rebased to the latest
> v5: Rebased to the latest
> 
> Reviewed-by: Mika Kahola <mika.kahola@intel.com>
> 
> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

>  drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_drv.h     |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c      | 17 ++++++++---------
>  3 files changed, 37 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7db1eee..febf993 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6406,12 +6406,38 @@ retry:
>  	return ret;
>  }
>  
> +static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv,
> +				     struct intel_crtc_state *pipe_config)
> +{
> +	if (pipe_config->pipe_bpp > 24)
> +		return false;
> +
> +	/* HSW can handle pixel rate up to cdclk? */
> +	if (IS_HASWELL(dev_priv->dev))
> +		return true;
> +
> +	/*
> +	 * FIXME if we compare against max we should then
> +	 * increase the cdclk frequency when the current
> +	 * value is too low. The other option is to compare
> +	 * against the cdclk frequency we're going have post
> +	 * modeset (ie. one we computed using other constraints).
> +	 * Need to measure whether using a lower cdclk w/o IPS
> +	 * is better or worse than a higher cdclk w/ IPS.
> +	 */
> +	return ilk_pipe_pixel_rate(pipe_config) <=
> +		dev_priv->max_cdclk_freq * 95 / 100;
> +}
> +
>  static void hsw_compute_ips_config(struct intel_crtc *crtc,
>  				   struct intel_crtc_state *pipe_config)
>  {
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
>  	pipe_config->ips_enabled = i915.enable_ips &&
> -				   hsw_crtc_supports_ips(crtc) &&
> -				   pipe_config->pipe_bpp <= 24;
> +		hsw_crtc_supports_ips(crtc) &&
> +		pipe_config_supports_ips(dev_priv, pipe_config);
>  }
>  
>  static int intel_crtc_compute_config(struct intel_crtc *crtc,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 47bc729..09df4b5 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1371,7 +1371,7 @@ void ilk_wm_get_hw_state(struct drm_device *dev);
>  void skl_wm_get_hw_state(struct drm_device *dev);
>  void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
>  			  struct skl_ddb_allocation *ddb /* out */);
> -
> +uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
>  
>  /* intel_sdvo.c */
>  bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0046cb4..282ad59 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1434,23 +1434,22 @@ static void i845_update_wm(struct drm_crtc *unused_crtc)
>  	I915_WRITE(FW_BLC, fwater_lo);
>  }
>  
> -static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
> -				    struct drm_crtc *crtc)
> +uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config)
>  {
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	uint32_t pixel_rate;
>  
> -	pixel_rate = intel_crtc->config->base.adjusted_mode.crtc_clock;
> +	pixel_rate = pipe_config->base.adjusted_mode.crtc_clock;
>  
>  	/* We only use IF-ID interlacing. If we ever use PF-ID we'll need to
>  	 * adjust the pixel_rate here. */
>  
> -	if (intel_crtc->config->pch_pfit.enabled) {
> +	if (pipe_config->pch_pfit.enabled) {
>  		uint64_t pipe_w, pipe_h, pfit_w, pfit_h;
> -		uint32_t pfit_size = intel_crtc->config->pch_pfit.size;
> +		uint32_t pfit_size = pipe_config->pch_pfit.size;
> +
> +		pipe_w = pipe_config->pipe_src_w;
> +		pipe_h = pipe_config->pipe_src_h;
>  
> -		pipe_w = intel_crtc->config->pipe_src_w;
> -		pipe_h = intel_crtc->config->pipe_src_h;
>  		pfit_w = (pfit_size >> 16) & 0xFFFF;
>  		pfit_h = pfit_size & 0xFFFF;
>  		if (pipe_w < pfit_w)
> @@ -2066,7 +2065,7 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
>  
>  	p->active = true;
>  	p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
> -	p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
> +	p->pixel_rate = ilk_pipe_pixel_rate(intel_crtc->config);
>  
>  	if (crtc->primary->state->fb)
>  		p->pri.bytes_per_pixel =
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v4 10/12] drm/i915: HSW cdclk support

[Mika Kahola]

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

Implement support for changing the cdclk frequency during runtime on
HSW. VLV/CHV already have support for this, so we can follow their
example for the most part. Only the actual hardware programming differs,
the rest is pretty much the same.

The pipe pixel rate stuff is handled a bit differently for now due to
the difference in pch vs. gmch pfit handling. Eventually we should unify
that part to eliminate what is essentially duplicated code.

v2: Grab rps.hw_lock around sandybridge_pcode_write()
v3: Rebase due to power well vs. .global_resources() reordering

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

v3: Rebased to the latest
v4: Reformatting 'haswell_modeset_global_pipes' function to
    support atomic state

Signed-off-by: Mika Kahola <mika.kahola@intel.com>

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 14366c8..015fe12 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6701,6 +6701,7 @@ enum skl_disp_power_wells {
 #define	  GEN6_PCODE_READ_RC6VIDS		0x5
 #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
 #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
+#define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
 #define   GEN9_PCODE_READ_MEM_LATENCY		0x6
 #define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
 #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
@@ -7159,10 +7160,12 @@ enum skl_disp_power_wells {
 #define  LCPLL_PLL_LOCK			(1<<30)
 #define  LCPLL_CLK_FREQ_MASK		(3<<26)
 #define  LCPLL_CLK_FREQ_450		(0<<26)
+#define  LCPLL_CLK_FREQ_ALT_HSW		(1<<26) /* 337.5 (ULX) or 540 */
 #define  LCPLL_CLK_FREQ_54O_BDW		(1<<26)
 #define  LCPLL_CLK_FREQ_337_5_BDW	(2<<26)
 #define  LCPLL_CLK_FREQ_675_BDW		(3<<26)
 #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
+#define  LCPLL_ROOT_CD_CLOCK_DISABLE	(1<<24)
 #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
 #define  LCPLL_POWER_DOWN_ALLOW		(1<<22)
 #define  LCPLL_CD_SOURCE_FCLK		(1<<21)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index febf993..556d0ec7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5545,7 +5545,16 @@ static void intel_update_max_cdclk(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (IS_VALLEYVIEW(dev)) {
+	if (IS_HASWELL(dev)) {
+		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
+			dev_priv->max_cdclk_freq = 450000;
+		else if (IS_HSW_ULX(dev))
+			dev_priv->max_cdclk_freq = 337500;
+		else if (IS_HSW_ULT(dev))
+			dev_priv->max_cdclk_freq = 450000;
+		else
+			dev_priv->max_cdclk_freq = 540000;
+	} else if (IS_VALLEYVIEW(dev)) {
 		dev_priv->max_cdclk_freq = 400000;
 	} else {
 		/* otherwise assume cdclk is fixed */
@@ -9404,6 +9413,139 @@ static void broxton_modeset_global_resources(struct drm_atomic_state *old_state)
 		broxton_set_cdclk(dev, req_cdclk);
 }
 
+/* compute the max rate for new configuration */
+static int ilk_max_pixel_rate(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = dev_priv->dev;
+	struct intel_crtc *crtc;
+	int max_pixel_rate = 0;
+
+	for_each_intel_crtc(dev, crtc) {
+		if (crtc->new_enabled)
+			max_pixel_rate = max((int)max_pixel_rate,
+					     (int)ilk_pipe_pixel_rate(crtc->config));
+	}
+
+	return max_pixel_rate;
+}
+
+static int haswell_calc_cdclk(struct drm_i915_private *dev_priv,
+			      int max_pixel_rate)
+{
+	int cdclk;
+
+	/*
+	 * FIXME should also account for plane ratio
+	 * once 64bpp pixel formats are supported.
+	 */
+	if (max_pixel_rate > 450000)
+		cdclk = 540000;
+	else if (max_pixel_rate > 337500 || !IS_HSW_ULX(dev_priv))
+		cdclk = 450000;
+	else
+		cdclk = 337500;
+
+	/*
+	 * FIXME move the cdclk caclulation to
+	 * compute_config() so we can fail gracegully.
+	 */
+	if (cdclk > dev_priv->max_cdclk_freq) {
+		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
+			  cdclk, dev_priv->max_cdclk_freq);
+		cdclk = dev_priv->max_cdclk_freq;
+	}
+
+	return cdclk;
+}
+
+static void haswell_set_cdclk(struct drm_device *dev, int cdclk)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t val;
+
+	if (WARN((I915_READ(LCPLL_CTL) &
+		  (LCPLL_PLL_DISABLE | LCPLL_PLL_LOCK |
+		   LCPLL_CD_CLOCK_DISABLE | LCPLL_ROOT_CD_CLOCK_DISABLE |
+		   LCPLL_CD2X_CLOCK_DISABLE | LCPLL_POWER_DOWN_ALLOW |
+		   LCPLL_CD_SOURCE_FCLK)) != LCPLL_PLL_LOCK,
+		 "trying to change cdclk frequency with cdclk not enabled\n"))
+		return;
+
+	val = I915_READ(LCPLL_CTL);
+	val &= ~LCPLL_CLK_FREQ_MASK;
+
+	switch (cdclk) {
+	case 450000:
+		val |= LCPLL_CLK_FREQ_450;
+		break;
+	case 337500:
+	case 540000:
+		val |= LCPLL_CLK_FREQ_ALT_HSW;
+		break;
+	default:
+		WARN(1, "invalid cdclk frequency\n");
+		return;
+	}
+
+	I915_WRITE(LCPLL_CTL, val);
+
+	if (IS_HSW_ULX(dev)) {
+		mutex_lock(&dev_priv->rps.hw_lock);
+		sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ,
+					cdclk == 337500);
+		mutex_unlock(&dev_priv->rps.hw_lock);
+	}
+
+	intel_update_cdclk(dev);
+
+	WARN(cdclk != dev_priv->cdclk_freq,
+	     "cdclk requested %d kHz but got %d kHz\n",
+	     cdclk, dev_priv->cdclk_freq);
+}
+
+static int haswell_modeset_global_pipes(struct drm_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->dev);
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+	int max_pixclk = ilk_max_pixel_rate(dev_priv);
+	int cdclk, i;
+
+	cdclk = haswell_calc_cdclk(dev_priv, max_pixclk);
+
+	if (cdclk == dev_priv->cdclk_freq)
+		return 0;
+
+	/* add all active pipes to the state */
+	for_each_crtc(state->dev, crtc) {
+		if (!crtc->state->enable)
+			continue;
+
+		crtc_state = drm_atomic_get_crtc_state(state, crtc);
+		if (IS_ERR(crtc_state))
+			return PTR_ERR(crtc_state);
+	}
+
+	/* disable/enable all currently active pipes while we change cdclk */
+	for_each_crtc_in_state(state, crtc, crtc_state, i)
+		if (crtc_state->enable)
+			crtc_state->mode_changed = true;
+
+	return 0;
+}
+
+static void haswell_modeset_global_resources(struct drm_atomic_state *state)
+{
+	struct drm_device *dev = state->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
+	int req_cdclk = haswell_calc_cdclk(dev_priv, max_pixel_rate);
+
+	if (req_cdclk != dev_priv->cdclk_freq) {
+		haswell_set_cdclk(dev, req_cdclk);
+	}
+}
+
 static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
 				      struct intel_crtc_state *crtc_state)
 {
@@ -12582,10 +12724,16 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
 	 * mode set on this crtc.  For other crtcs we need to use the
 	 * adjusted_mode bits in the crtc directly.
 	 */
-	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
-		ret = valleyview_modeset_global_pipes(state);
-		if (ret)
-			return ret;
+	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_HASWELL(dev)) {
+		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
+			ret = valleyview_modeset_global_pipes(state);
+			if (ret)
+				return ret;
+		} else {
+			ret = haswell_modeset_global_pipes(state);
+			if (ret)
+				return ret;
+		}
 	}
 
 	ret = __intel_set_mode_setup_plls(state);
@@ -14468,6 +14616,9 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
 	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
 		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
+		if (IS_HASWELL(dev))
+			dev_priv->display.modeset_global_resources =
+				haswell_modeset_global_resources;
 	} else if (IS_VALLEYVIEW(dev)) {
 		dev_priv->display.modeset_global_resources =
 			valleyview_modeset_global_resources;
-- 
1.9.1

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

Re: [PATCH v4 10/12] drm/i915: HSW cdclk support

[Damien Lespiau]

On Fri, May 22, 2015 at 11:22:40AM +0300, Mika Kahola wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Implement support for changing the cdclk frequency during runtime on
> HSW. VLV/CHV already have support for this, so we can follow their
> example for the most part. Only the actual hardware programming differs,
> the rest is pretty much the same.
> 
> The pipe pixel rate stuff is handled a bit differently for now due to
> the difference in pch vs. gmch pfit handling. Eventually we should unify
> that part to eliminate what is essentially duplicated code.
> 
> v2: Grab rps.hw_lock around sandybridge_pcode_write()
> v3: Rebase due to power well vs. .global_resources() reordering
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> v3: Rebased to the latest
> v4: Reformatting 'haswell_modeset_global_pipes' function to
>     support atomic state
> 
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> 
> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---

Apart from the small remark below, it looks like it does what the spec
tell us to do.

However there are a few extra comments:

  - I remember someone saying changing CDCLK hasn't been validated on
    HSW, that's usually a red flag. Even if it seems like it's working,
    I've no idea what the pcode does with the notification on ULX for
    instance
  - Changing CDCLK means we have to bring whole display down and up
    again, which will make the current display flicker when hotplugging
    a screen that needs a CDCLK bump for instance.
  - it'd be nice to have some idea of the power gain running at a lower
    frequency

-- 
Damien

>  drivers/gpu/drm/i915/i915_reg.h      |   3 +
>  drivers/gpu/drm/i915/intel_display.c | 161 +++++++++++++++++++++++++++++++++--
>  2 files changed, 159 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 14366c8..015fe12 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6701,6 +6701,7 @@ enum skl_disp_power_wells {
>  #define	  GEN6_PCODE_READ_RC6VIDS		0x5
>  #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
>  #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
> +#define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
>  #define   GEN9_PCODE_READ_MEM_LATENCY		0x6
>  #define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
>  #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
> @@ -7159,10 +7160,12 @@ enum skl_disp_power_wells {
>  #define  LCPLL_PLL_LOCK			(1<<30)
>  #define  LCPLL_CLK_FREQ_MASK		(3<<26)
>  #define  LCPLL_CLK_FREQ_450		(0<<26)
> +#define  LCPLL_CLK_FREQ_ALT_HSW		(1<<26) /* 337.5 (ULX) or 540 */
>  #define  LCPLL_CLK_FREQ_54O_BDW		(1<<26)
>  #define  LCPLL_CLK_FREQ_337_5_BDW	(2<<26)
>  #define  LCPLL_CLK_FREQ_675_BDW		(3<<26)
>  #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
> +#define  LCPLL_ROOT_CD_CLOCK_DISABLE	(1<<24)
>  #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
>  #define  LCPLL_POWER_DOWN_ALLOW		(1<<22)
>  #define  LCPLL_CD_SOURCE_FCLK		(1<<21)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index febf993..556d0ec7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5545,7 +5545,16 @@ static void intel_update_max_cdclk(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	if (IS_VALLEYVIEW(dev)) {
> +	if (IS_HASWELL(dev)) {
> +		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
> +			dev_priv->max_cdclk_freq = 450000;
> +		else if (IS_HSW_ULX(dev))
> +			dev_priv->max_cdclk_freq = 337500;
> +		else if (IS_HSW_ULT(dev))
> +			dev_priv->max_cdclk_freq = 450000;
> +		else
> +			dev_priv->max_cdclk_freq = 540000;
> +	} else if (IS_VALLEYVIEW(dev)) {
>  		dev_priv->max_cdclk_freq = 400000;
>  	} else {
>  		/* otherwise assume cdclk is fixed */
> @@ -9404,6 +9413,139 @@ static void broxton_modeset_global_resources(struct drm_atomic_state *old_state)
>  		broxton_set_cdclk(dev, req_cdclk);
>  }
>  
> +/* compute the max rate for new configuration */
> +static int ilk_max_pixel_rate(struct drm_i915_private *dev_priv)
> +{
> +	struct drm_device *dev = dev_priv->dev;
> +	struct intel_crtc *crtc;
> +	int max_pixel_rate = 0;
> +
> +	for_each_intel_crtc(dev, crtc) {
> +		if (crtc->new_enabled)
> +			max_pixel_rate = max((int)max_pixel_rate,
> +					     (int)ilk_pipe_pixel_rate(crtc->config));
> +	}
> +
> +	return max_pixel_rate;
> +}

new_enabled doesn't look like what we want to look at, it looks like a
temporary field we shouldn't be using in new code. Maybe
crtc->state->enable instead?

> +
> +static int haswell_calc_cdclk(struct drm_i915_private *dev_priv,
> +			      int max_pixel_rate)
> +{
> +	int cdclk;
> +
> +	/*
> +	 * FIXME should also account for plane ratio
> +	 * once 64bpp pixel formats are supported.
> +	 */
> +	if (max_pixel_rate > 450000)
> +		cdclk = 540000;
> +	else if (max_pixel_rate > 337500 || !IS_HSW_ULX(dev_priv))
> +		cdclk = 450000;
> +	else
> +		cdclk = 337500;
> +
> +	/*
> +	 * FIXME move the cdclk caclulation to
> +	 * compute_config() so we can fail gracegully.
> +	 */
> +	if (cdclk > dev_priv->max_cdclk_freq) {
> +		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> +			  cdclk, dev_priv->max_cdclk_freq);
> +		cdclk = dev_priv->max_cdclk_freq;
> +	}
> +
> +	return cdclk;
> +}
> +
> +static void haswell_set_cdclk(struct drm_device *dev, int cdclk)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	uint32_t val;
> +
> +	if (WARN((I915_READ(LCPLL_CTL) &
> +		  (LCPLL_PLL_DISABLE | LCPLL_PLL_LOCK |
> +		   LCPLL_CD_CLOCK_DISABLE | LCPLL_ROOT_CD_CLOCK_DISABLE |
> +		   LCPLL_CD2X_CLOCK_DISABLE | LCPLL_POWER_DOWN_ALLOW |
> +		   LCPLL_CD_SOURCE_FCLK)) != LCPLL_PLL_LOCK,
> +		 "trying to change cdclk frequency with cdclk not enabled\n"))
> +		return;
> +
> +	val = I915_READ(LCPLL_CTL);
> +	val &= ~LCPLL_CLK_FREQ_MASK;
> +
> +	switch (cdclk) {
> +	case 450000:
> +		val |= LCPLL_CLK_FREQ_450;
> +		break;
> +	case 337500:
> +	case 540000:
> +		val |= LCPLL_CLK_FREQ_ALT_HSW;
> +		break;
> +	default:
> +		WARN(1, "invalid cdclk frequency\n");
> +		return;
> +	}
> +
> +	I915_WRITE(LCPLL_CTL, val);
> +
> +	if (IS_HSW_ULX(dev)) {
> +		mutex_lock(&dev_priv->rps.hw_lock);
> +		sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ,
> +					cdclk == 337500);
> +		mutex_unlock(&dev_priv->rps.hw_lock);
> +	}
> +
> +	intel_update_cdclk(dev);
> +
> +	WARN(cdclk != dev_priv->cdclk_freq,
> +	     "cdclk requested %d kHz but got %d kHz\n",
> +	     cdclk, dev_priv->cdclk_freq);
> +}
> +
> +static int haswell_modeset_global_pipes(struct drm_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> +	struct drm_crtc *crtc;
> +	struct drm_crtc_state *crtc_state;
> +	int max_pixclk = ilk_max_pixel_rate(dev_priv);
> +	int cdclk, i;
> +
> +	cdclk = haswell_calc_cdclk(dev_priv, max_pixclk);
> +
> +	if (cdclk == dev_priv->cdclk_freq)
> +		return 0;
> +
> +	/* add all active pipes to the state */
> +	for_each_crtc(state->dev, crtc) {
> +		if (!crtc->state->enable)
> +			continue;
> +
> +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> +		if (IS_ERR(crtc_state))
> +			return PTR_ERR(crtc_state);
> +	}
> +
> +	/* disable/enable all currently active pipes while we change cdclk */
> +	for_each_crtc_in_state(state, crtc, crtc_state, i)
> +		if (crtc_state->enable)
> +			crtc_state->mode_changed = true;
> +
> +	return 0;
> +}
> +
> +static void haswell_modeset_global_resources(struct drm_atomic_state *state)
> +{
> +	struct drm_device *dev = state->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
> +	int req_cdclk = haswell_calc_cdclk(dev_priv, max_pixel_rate);
> +
> +	if (req_cdclk != dev_priv->cdclk_freq) {
> +		haswell_set_cdclk(dev, req_cdclk);
> +	}
> +}
> +
>  static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
>  				      struct intel_crtc_state *crtc_state)
>  {
> @@ -12582,10 +12724,16 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
>  	 * mode set on this crtc.  For other crtcs we need to use the
>  	 * adjusted_mode bits in the crtc directly.
>  	 */
> -	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
> -		ret = valleyview_modeset_global_pipes(state);
> -		if (ret)
> -			return ret;
> +	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_HASWELL(dev)) {
> +		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
> +			ret = valleyview_modeset_global_pipes(state);
> +			if (ret)
> +				return ret;
> +		} else {
> +			ret = haswell_modeset_global_pipes(state);
> +			if (ret)
> +				return ret;
> +		}
>  	}
>  
>  	ret = __intel_set_mode_setup_plls(state);
> @@ -14468,6 +14616,9 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
>  	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
> +		if (IS_HASWELL(dev))
> +			dev_priv->display.modeset_global_resources =
> +				haswell_modeset_global_resources;
>  	} else if (IS_VALLEYVIEW(dev)) {
>  		dev_priv->display.modeset_global_resources =
>  			valleyview_modeset_global_resources;
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 10/12] drm/i915: HSW cdclk support

[Kahola, Mika]

> -----Original Message-----
> From: Lespiau, Damien
> Sent: Friday, May 29, 2015 2:31 PM
> To: Kahola, Mika
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v4 10/12] drm/i915: HSW cdclk support
> 
> On Fri, May 22, 2015 at 11:22:40AM +0300, Mika Kahola wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Implement support for changing the cdclk frequency during runtime on
> > HSW. VLV/CHV already have support for this, so we can follow their
> > example for the most part. Only the actual hardware programming
> > differs, the rest is pretty much the same.
> >
> > The pipe pixel rate stuff is handled a bit differently for now due to
> > the difference in pch vs. gmch pfit handling. Eventually we should
> > unify that part to eliminate what is essentially duplicated code.
> >
> > v2: Grab rps.hw_lock around sandybridge_pcode_write()
> > v3: Rebase due to power well vs. .global_resources() reordering
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > v3: Rebased to the latest
> > v4: Reformatting 'haswell_modeset_global_pipes' function to
> >     support atomic state
> >
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> >
> > Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> 
> Apart from the small remark below, it looks like it does what the spec tell us
> to do.
> 
> However there are a few extra comments:
> 
>   - I remember someone saying changing CDCLK hasn't been validated on
>     HSW, that's usually a red flag. Even if it seems like it's working,
>     I've no idea what the pcode does with the notification on ULX for
>     instance
>   - Changing CDCLK means we have to bring whole display down and up
>     again, which will make the current display flicker when hotplugging
>     a screen that needs a CDCLK bump for instance.
>   - it'd be nice to have some idea of the power gain running at a lower
>     frequency
> 
> --
> Damien

Thanks for the comments and review.

> >  drivers/gpu/drm/i915/i915_reg.h      |   3 +
> >  drivers/gpu/drm/i915/intel_display.c | 161
> > +++++++++++++++++++++++++++++++++--
> >  2 files changed, 159 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 14366c8..015fe12 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6701,6 +6701,7 @@ enum skl_disp_power_wells {
> >  #define	  GEN6_PCODE_READ_RC6VIDS		0x5
> >  #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
> >  #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
> > +#define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
> >  #define   GEN9_PCODE_READ_MEM_LATENCY		0x6
> >  #define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
> >  #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
> > @@ -7159,10 +7160,12 @@ enum skl_disp_power_wells {
> >  #define  LCPLL_PLL_LOCK			(1<<30)
> >  #define  LCPLL_CLK_FREQ_MASK		(3<<26)
> >  #define  LCPLL_CLK_FREQ_450		(0<<26)
> > +#define  LCPLL_CLK_FREQ_ALT_HSW		(1<<26) /* 337.5
> (ULX) or 540 */
> >  #define  LCPLL_CLK_FREQ_54O_BDW		(1<<26)
> >  #define  LCPLL_CLK_FREQ_337_5_BDW	(2<<26)
> >  #define  LCPLL_CLK_FREQ_675_BDW		(3<<26)
> >  #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
> > +#define  LCPLL_ROOT_CD_CLOCK_DISABLE	(1<<24)
> >  #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
> >  #define  LCPLL_POWER_DOWN_ALLOW		(1<<22)
> >  #define  LCPLL_CD_SOURCE_FCLK		(1<<21)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index febf993..556d0ec7 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5545,7 +5545,16 @@ static void intel_update_max_cdclk(struct
> > drm_device *dev)  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >
> > -	if (IS_VALLEYVIEW(dev)) {
> > +	if (IS_HASWELL(dev)) {
> > +		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
> > +			dev_priv->max_cdclk_freq = 450000;
> > +		else if (IS_HSW_ULX(dev))
> > +			dev_priv->max_cdclk_freq = 337500;
> > +		else if (IS_HSW_ULT(dev))
> > +			dev_priv->max_cdclk_freq = 450000;
> > +		else
> > +			dev_priv->max_cdclk_freq = 540000;
> > +	} else if (IS_VALLEYVIEW(dev)) {
> >  		dev_priv->max_cdclk_freq = 400000;
> >  	} else {
> >  		/* otherwise assume cdclk is fixed */ @@ -9404,6 +9413,139
> @@
> > static void broxton_modeset_global_resources(struct drm_atomic_state
> *old_state)
> >  		broxton_set_cdclk(dev, req_cdclk);
> >  }
> >
> > +/* compute the max rate for new configuration */ static int
> > +ilk_max_pixel_rate(struct drm_i915_private *dev_priv) {
> > +	struct drm_device *dev = dev_priv->dev;
> > +	struct intel_crtc *crtc;
> > +	int max_pixel_rate = 0;
> > +
> > +	for_each_intel_crtc(dev, crtc) {
> > +		if (crtc->new_enabled)
> > +			max_pixel_rate = max((int)max_pixel_rate,
> > +					     (int)ilk_pipe_pixel_rate(crtc-
> >config));
> > +	}
> > +
> > +	return max_pixel_rate;
> > +}
> 
> new_enabled doesn't look like what we want to look at, it looks like a
> temporary field we shouldn't be using in new code. Maybe
> crtc->state->enable instead?

You're right! This was a good catch. I was a bit confused which one to use here. I'll revise the patch

Cheers,
Mika
> > +
> > +static int haswell_calc_cdclk(struct drm_i915_private *dev_priv,
> > +			      int max_pixel_rate)
> > +{
> > +	int cdclk;
> > +
> > +	/*
> > +	 * FIXME should also account for plane ratio
> > +	 * once 64bpp pixel formats are supported.
> > +	 */
> > +	if (max_pixel_rate > 450000)
> > +		cdclk = 540000;
> > +	else if (max_pixel_rate > 337500 || !IS_HSW_ULX(dev_priv))
> > +		cdclk = 450000;
> > +	else
> > +		cdclk = 337500;
> > +
> > +	/*
> > +	 * FIXME move the cdclk caclulation to
> > +	 * compute_config() so we can fail gracegully.
> > +	 */
> > +	if (cdclk > dev_priv->max_cdclk_freq) {
> > +		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d
> kHz)\n",
> > +			  cdclk, dev_priv->max_cdclk_freq);
> > +		cdclk = dev_priv->max_cdclk_freq;
> > +	}
> > +
> > +	return cdclk;
> > +}
> > +
> > +static void haswell_set_cdclk(struct drm_device *dev, int cdclk) {
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	uint32_t val;
> > +
> > +	if (WARN((I915_READ(LCPLL_CTL) &
> > +		  (LCPLL_PLL_DISABLE | LCPLL_PLL_LOCK |
> > +		   LCPLL_CD_CLOCK_DISABLE |
> LCPLL_ROOT_CD_CLOCK_DISABLE |
> > +		   LCPLL_CD2X_CLOCK_DISABLE |
> LCPLL_POWER_DOWN_ALLOW |
> > +		   LCPLL_CD_SOURCE_FCLK)) != LCPLL_PLL_LOCK,
> > +		 "trying to change cdclk frequency with cdclk not
> enabled\n"))
> > +		return;
> > +
> > +	val = I915_READ(LCPLL_CTL);
> > +	val &= ~LCPLL_CLK_FREQ_MASK;
> > +
> > +	switch (cdclk) {
> > +	case 450000:
> > +		val |= LCPLL_CLK_FREQ_450;
> > +		break;
> > +	case 337500:
> > +	case 540000:
> > +		val |= LCPLL_CLK_FREQ_ALT_HSW;
> > +		break;
> > +	default:
> > +		WARN(1, "invalid cdclk frequency\n");
> > +		return;
> > +	}
> > +
> > +	I915_WRITE(LCPLL_CTL, val);
> > +
> > +	if (IS_HSW_ULX(dev)) {
> > +		mutex_lock(&dev_priv->rps.hw_lock);
> > +		sandybridge_pcode_write(dev_priv,
> HSW_PCODE_DE_WRITE_FREQ_REQ,
> > +					cdclk == 337500);
> > +		mutex_unlock(&dev_priv->rps.hw_lock);
> > +	}
> > +
> > +	intel_update_cdclk(dev);
> > +
> > +	WARN(cdclk != dev_priv->cdclk_freq,
> > +	     "cdclk requested %d kHz but got %d kHz\n",
> > +	     cdclk, dev_priv->cdclk_freq);
> > +}
> > +
> > +static int haswell_modeset_global_pipes(struct drm_atomic_state
> > +*state) {
> > +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> > +	struct drm_crtc *crtc;
> > +	struct drm_crtc_state *crtc_state;
> > +	int max_pixclk = ilk_max_pixel_rate(dev_priv);
> > +	int cdclk, i;
> > +
> > +	cdclk = haswell_calc_cdclk(dev_priv, max_pixclk);
> > +
> > +	if (cdclk == dev_priv->cdclk_freq)
> > +		return 0;
> > +
> > +	/* add all active pipes to the state */
> > +	for_each_crtc(state->dev, crtc) {
> > +		if (!crtc->state->enable)
> > +			continue;
> > +
> > +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> > +		if (IS_ERR(crtc_state))
> > +			return PTR_ERR(crtc_state);
> > +	}
> > +
> > +	/* disable/enable all currently active pipes while we change cdclk */
> > +	for_each_crtc_in_state(state, crtc, crtc_state, i)
> > +		if (crtc_state->enable)
> > +			crtc_state->mode_changed = true;
> > +
> > +	return 0;
> > +}
> > +
> > +static void haswell_modeset_global_resources(struct drm_atomic_state
> > +*state) {
> > +	struct drm_device *dev = state->dev;
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
> > +	int req_cdclk = haswell_calc_cdclk(dev_priv, max_pixel_rate);
> > +
> > +	if (req_cdclk != dev_priv->cdclk_freq) {
> > +		haswell_set_cdclk(dev, req_cdclk);
> > +	}
> > +}
> > +
> >  static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
> >  				      struct intel_crtc_state *crtc_state)  { @@ -
> 12582,10
> > +12724,16 @@ static int __intel_set_mode_checks(struct
> drm_atomic_state *state)
> >  	 * mode set on this crtc.  For other crtcs we need to use the
> >  	 * adjusted_mode bits in the crtc directly.
> >  	 */
> > -	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
> > -		ret = valleyview_modeset_global_pipes(state);
> > -		if (ret)
> > -			return ret;
> > +	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_HASWELL(dev)) {
> > +		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
> > +			ret = valleyview_modeset_global_pipes(state);
> > +			if (ret)
> > +				return ret;
> > +		} else {
> > +			ret = haswell_modeset_global_pipes(state);
> > +			if (ret)
> > +				return ret;
> > +		}
> >  	}
> >
> >  	ret = __intel_set_mode_setup_plls(state);
> > @@ -14468,6 +14616,9 @@ static void intel_init_display(struct drm_device
> *dev)
> >  		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
> >  	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> >  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
> > +		if (IS_HASWELL(dev))
> > +			dev_priv->display.modeset_global_resources =
> > +				haswell_modeset_global_resources;
> >  	} else if (IS_VALLEYVIEW(dev)) {
> >  		dev_priv->display.modeset_global_resources =
> >  			valleyview_modeset_global_resources;
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 10/12] drm/i915: HSW cdclk support

[Damien Lespiau]

On Fri, May 29, 2015 at 01:06:47PM +0100, Kahola, Mika wrote:
> > > static void broxton_modeset_global_resources(struct drm_atomic_state
> > *old_state)
> > >  		broxton_set_cdclk(dev, req_cdclk);
> > >  }
> > >
> > > +/* compute the max rate for new configuration */ static int
> > > +ilk_max_pixel_rate(struct drm_i915_private *dev_priv) {
> > > +	struct drm_device *dev = dev_priv->dev;
> > > +	struct intel_crtc *crtc;
> > > +	int max_pixel_rate = 0;
> > > +
> > > +	for_each_intel_crtc(dev, crtc) {
> > > +		if (crtc->new_enabled)
> > > +			max_pixel_rate = max((int)max_pixel_rate,
> > > +					     (int)ilk_pipe_pixel_rate(crtc-
> > >config));
> > > +	}
> > > +
> > > +	return max_pixel_rate;
> > > +}
> > 
> > new_enabled doesn't look like what we want to look at, it looks like a
> > temporary field we shouldn't be using in new code. Maybe
> > crtc->state->enable instead?
> 
> You're right! This was a good catch. I was a bit confused which one to
> use here. I'll revise the patch

I've noticed that the BDW patch after that has to be rebased on top of
this change as well.

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

Re: [PATCH v4 10/12] drm/i915: HSW cdclk support

[Ville Syrjälä]

On Fri, May 29, 2015 at 12:30:41PM +0100, Damien Lespiau wrote:
> On Fri, May 22, 2015 at 11:22:40AM +0300, Mika Kahola wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Implement support for changing the cdclk frequency during runtime on
> > HSW. VLV/CHV already have support for this, so we can follow their
> > example for the most part. Only the actual hardware programming differs,
> > the rest is pretty much the same.
> > 
> > The pipe pixel rate stuff is handled a bit differently for now due to
> > the difference in pch vs. gmch pfit handling. Eventually we should unify
> > that part to eliminate what is essentially duplicated code.
> > 
> > v2: Grab rps.hw_lock around sandybridge_pcode_write()
> > v3: Rebase due to power well vs. .global_resources() reordering
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > v3: Rebased to the latest
> > v4: Reformatting 'haswell_modeset_global_pipes' function to
> >     support atomic state
> > 
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > 
> > Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> 
> Apart from the small remark below, it looks like it does what the spec
> tell us to do.
> 
> However there are a few extra comments:
> 
>   - I remember someone saying changing CDCLK hasn't been validated on
>     HSW, that's usually a red flag. Even if it seems like it's working,
>     I've no idea what the pcode does with the notification on ULX for
>     instance

Yeah I suppose it's better to reorder things so that the BDW patch comes
first. Then if we decide to drop the HSW one it won't cause problems for
the BDW patch.

>   - Changing CDCLK means we have to bring whole display down and up
>     again, which will make the current display flicker when hotplugging
>     a screen that needs a CDCLK bump for instance.
>   - it'd be nice to have some idea of the power gain running at a lower
>     frequency
> 
> -- 
> Damien
> 
> >  drivers/gpu/drm/i915/i915_reg.h      |   3 +
> >  drivers/gpu/drm/i915/intel_display.c | 161 +++++++++++++++++++++++++++++++++--
> >  2 files changed, 159 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 14366c8..015fe12 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6701,6 +6701,7 @@ enum skl_disp_power_wells {
> >  #define	  GEN6_PCODE_READ_RC6VIDS		0x5
> >  #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
> >  #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
> > +#define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
> >  #define   GEN9_PCODE_READ_MEM_LATENCY		0x6
> >  #define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
> >  #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
> > @@ -7159,10 +7160,12 @@ enum skl_disp_power_wells {
> >  #define  LCPLL_PLL_LOCK			(1<<30)
> >  #define  LCPLL_CLK_FREQ_MASK		(3<<26)
> >  #define  LCPLL_CLK_FREQ_450		(0<<26)
> > +#define  LCPLL_CLK_FREQ_ALT_HSW		(1<<26) /* 337.5 (ULX) or 540 */
> >  #define  LCPLL_CLK_FREQ_54O_BDW		(1<<26)
> >  #define  LCPLL_CLK_FREQ_337_5_BDW	(2<<26)
> >  #define  LCPLL_CLK_FREQ_675_BDW		(3<<26)
> >  #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
> > +#define  LCPLL_ROOT_CD_CLOCK_DISABLE	(1<<24)
> >  #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
> >  #define  LCPLL_POWER_DOWN_ALLOW		(1<<22)
> >  #define  LCPLL_CD_SOURCE_FCLK		(1<<21)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index febf993..556d0ec7 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5545,7 +5545,16 @@ static void intel_update_max_cdclk(struct drm_device *dev)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  
> > -	if (IS_VALLEYVIEW(dev)) {
> > +	if (IS_HASWELL(dev)) {
> > +		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
> > +			dev_priv->max_cdclk_freq = 450000;
> > +		else if (IS_HSW_ULX(dev))
> > +			dev_priv->max_cdclk_freq = 337500;
> > +		else if (IS_HSW_ULT(dev))
> > +			dev_priv->max_cdclk_freq = 450000;
> > +		else
> > +			dev_priv->max_cdclk_freq = 540000;
> > +	} else if (IS_VALLEYVIEW(dev)) {
> >  		dev_priv->max_cdclk_freq = 400000;
> >  	} else {
> >  		/* otherwise assume cdclk is fixed */
> > @@ -9404,6 +9413,139 @@ static void broxton_modeset_global_resources(struct drm_atomic_state *old_state)
> >  		broxton_set_cdclk(dev, req_cdclk);
> >  }
> >  
> > +/* compute the max rate for new configuration */
> > +static int ilk_max_pixel_rate(struct drm_i915_private *dev_priv)
> > +{
> > +	struct drm_device *dev = dev_priv->dev;
> > +	struct intel_crtc *crtc;
> > +	int max_pixel_rate = 0;
> > +
> > +	for_each_intel_crtc(dev, crtc) {
> > +		if (crtc->new_enabled)
> > +			max_pixel_rate = max((int)max_pixel_rate,
> > +					     (int)ilk_pipe_pixel_rate(crtc->config));
> > +	}
> > +
> > +	return max_pixel_rate;
> > +}
> 
> new_enabled doesn't look like what we want to look at, it looks like a
> temporary field we shouldn't be using in new code. Maybe
> crtc->state->enable instead?
> 
> > +
> > +static int haswell_calc_cdclk(struct drm_i915_private *dev_priv,
> > +			      int max_pixel_rate)
> > +{
> > +	int cdclk;
> > +
> > +	/*
> > +	 * FIXME should also account for plane ratio
> > +	 * once 64bpp pixel formats are supported.
> > +	 */
> > +	if (max_pixel_rate > 450000)
> > +		cdclk = 540000;
> > +	else if (max_pixel_rate > 337500 || !IS_HSW_ULX(dev_priv))
> > +		cdclk = 450000;
> > +	else
> > +		cdclk = 337500;
> > +
> > +	/*
> > +	 * FIXME move the cdclk caclulation to
> > +	 * compute_config() so we can fail gracegully.
> > +	 */
> > +	if (cdclk > dev_priv->max_cdclk_freq) {
> > +		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> > +			  cdclk, dev_priv->max_cdclk_freq);
> > +		cdclk = dev_priv->max_cdclk_freq;
> > +	}
> > +
> > +	return cdclk;
> > +}
> > +
> > +static void haswell_set_cdclk(struct drm_device *dev, int cdclk)
> > +{
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	uint32_t val;
> > +
> > +	if (WARN((I915_READ(LCPLL_CTL) &
> > +		  (LCPLL_PLL_DISABLE | LCPLL_PLL_LOCK |
> > +		   LCPLL_CD_CLOCK_DISABLE | LCPLL_ROOT_CD_CLOCK_DISABLE |
> > +		   LCPLL_CD2X_CLOCK_DISABLE | LCPLL_POWER_DOWN_ALLOW |
> > +		   LCPLL_CD_SOURCE_FCLK)) != LCPLL_PLL_LOCK,
> > +		 "trying to change cdclk frequency with cdclk not enabled\n"))
> > +		return;
> > +
> > +	val = I915_READ(LCPLL_CTL);
> > +	val &= ~LCPLL_CLK_FREQ_MASK;
> > +
> > +	switch (cdclk) {
> > +	case 450000:
> > +		val |= LCPLL_CLK_FREQ_450;
> > +		break;
> > +	case 337500:
> > +	case 540000:
> > +		val |= LCPLL_CLK_FREQ_ALT_HSW;
> > +		break;
> > +	default:
> > +		WARN(1, "invalid cdclk frequency\n");
> > +		return;
> > +	}
> > +
> > +	I915_WRITE(LCPLL_CTL, val);
> > +
> > +	if (IS_HSW_ULX(dev)) {
> > +		mutex_lock(&dev_priv->rps.hw_lock);
> > +		sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ,
> > +					cdclk == 337500);
> > +		mutex_unlock(&dev_priv->rps.hw_lock);
> > +	}
> > +
> > +	intel_update_cdclk(dev);
> > +
> > +	WARN(cdclk != dev_priv->cdclk_freq,
> > +	     "cdclk requested %d kHz but got %d kHz\n",
> > +	     cdclk, dev_priv->cdclk_freq);
> > +}
> > +
> > +static int haswell_modeset_global_pipes(struct drm_atomic_state *state)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> > +	struct drm_crtc *crtc;
> > +	struct drm_crtc_state *crtc_state;
> > +	int max_pixclk = ilk_max_pixel_rate(dev_priv);
> > +	int cdclk, i;
> > +
> > +	cdclk = haswell_calc_cdclk(dev_priv, max_pixclk);
> > +
> > +	if (cdclk == dev_priv->cdclk_freq)
> > +		return 0;
> > +
> > +	/* add all active pipes to the state */
> > +	for_each_crtc(state->dev, crtc) {
> > +		if (!crtc->state->enable)
> > +			continue;
> > +
> > +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> > +		if (IS_ERR(crtc_state))
> > +			return PTR_ERR(crtc_state);
> > +	}
> > +
> > +	/* disable/enable all currently active pipes while we change cdclk */
> > +	for_each_crtc_in_state(state, crtc, crtc_state, i)
> > +		if (crtc_state->enable)
> > +			crtc_state->mode_changed = true;
> > +
> > +	return 0;
> > +}
> > +
> > +static void haswell_modeset_global_resources(struct drm_atomic_state *state)
> > +{
> > +	struct drm_device *dev = state->dev;
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
> > +	int req_cdclk = haswell_calc_cdclk(dev_priv, max_pixel_rate);
> > +
> > +	if (req_cdclk != dev_priv->cdclk_freq) {
> > +		haswell_set_cdclk(dev, req_cdclk);
> > +	}
> > +}
> > +
> >  static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
> >  				      struct intel_crtc_state *crtc_state)
> >  {
> > @@ -12582,10 +12724,16 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
> >  	 * mode set on this crtc.  For other crtcs we need to use the
> >  	 * adjusted_mode bits in the crtc directly.
> >  	 */
> > -	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
> > -		ret = valleyview_modeset_global_pipes(state);
> > -		if (ret)
> > -			return ret;
> > +	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_HASWELL(dev)) {
> > +		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
> > +			ret = valleyview_modeset_global_pipes(state);
> > +			if (ret)
> > +				return ret;
> > +		} else {
> > +			ret = haswell_modeset_global_pipes(state);
> > +			if (ret)
> > +				return ret;
> > +		}
> >  	}
> >  
> >  	ret = __intel_set_mode_setup_plls(state);
> > @@ -14468,6 +14616,9 @@ static void intel_init_display(struct drm_device *dev)
> >  		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
> >  	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> >  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
> > +		if (IS_HASWELL(dev))
> > +			dev_priv->display.modeset_global_resources =
> > +				haswell_modeset_global_resources;
> >  	} else if (IS_VALLEYVIEW(dev)) {
> >  		dev_priv->display.modeset_global_resources =
> >  			valleyview_modeset_global_resources;
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH v4 11/12] drm/i915: Add IS_BDW_ULX

[Mika Kahola]

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

We need to tell BDW ULT and ULX apart.

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

v2: Rebased to the latest
v3: Rebased to the latest

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 37e8e9d..c85d802 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2362,6 +2362,9 @@ struct drm_i915_cmd_table {
 				 ((INTEL_DEVID(dev) & 0xf) == 0x6 ||	\
 				 (INTEL_DEVID(dev) & 0xf) == 0xb ||	\
 				 (INTEL_DEVID(dev) & 0xf) == 0xe))
+/* ULX machines are also considered ULT. */
+#define IS_BDW_ULX(dev)		(IS_BROADWELL(dev) && \
+				 (INTEL_DEVID(dev) & 0xf) == 0xe)
 #define IS_BDW_GT3(dev)		(IS_BROADWELL(dev) && \
 				 (INTEL_DEVID(dev) & 0x00F0) == 0x0020)
 #define IS_HSW_ULT(dev)		(IS_HASWELL(dev) && \
-- 
1.9.1

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

Re: [PATCH v4 11/12] drm/i915: Add IS_BDW_ULX

[Damien Lespiau]

On Fri, May 22, 2015 at 11:22:41AM +0300, Mika Kahola wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We need to tell BDW ULT and ULX apart.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> v2: Rebased to the latest
> v3: Rebased to the latest
> 
> Reviewed-by: Mika Kahola <mika.kahola@intel.com>
> 
> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_drv.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 37e8e9d..c85d802 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2362,6 +2362,9 @@ struct drm_i915_cmd_table {
>  				 ((INTEL_DEVID(dev) & 0xf) == 0x6 ||	\
>  				 (INTEL_DEVID(dev) & 0xf) == 0xb ||	\
>  				 (INTEL_DEVID(dev) & 0xf) == 0xe))
> +/* ULX machines are also considered ULT. */
> +#define IS_BDW_ULX(dev)		(IS_BROADWELL(dev) && \
> +				 (INTEL_DEVID(dev) & 0xf) == 0xe)
>  #define IS_BDW_GT3(dev)		(IS_BROADWELL(dev) && \
>  				 (INTEL_DEVID(dev) & 0x00F0) == 0x0020)
>  #define IS_HSW_ULT(dev)		(IS_HASWELL(dev) && \
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v4 12/12] drm/i915: BDW clock change support

[Mika Kahola]

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

Add support for changing cdclk frequency during runtime on BDW. The
procedure is quite a bit different on BDW from the one on HSW, so
add a separate function for it.

Also with IPS enabled the actual pixel rate mustn't exceed 95% of cdclk,
so take that into account when computing the max pixel rate.

v2: Grab rps.hw_lock around sandybridge_pcode_write()
v3: Rebase due to power well vs. .global_resources() reordering

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

v4: Rebased to the latest
v5: Rebased to the latest

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |   1 +
 drivers/gpu/drm/i915/intel_display.c | 139 ++++++++++++++++++++++++++++++-----
 2 files changed, 122 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 015fe12..64ec500 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6702,6 +6702,7 @@ enum skl_disp_power_wells {
 #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
 #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
 #define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
+#define   BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ	0x18
 #define   GEN9_PCODE_READ_MEM_LATENCY		0x6
 #define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
 #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 556d0ec7..71efd2f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5545,7 +5545,22 @@ static void intel_update_max_cdclk(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (IS_HASWELL(dev)) {
+	if (IS_BROADWELL(dev))  {
+		/*
+		 * FIXME with extra cooling we can allow
+		 * 540 MHz for ULX and 675 Mhz for ULT.
+		 * How can we know if extra cooling is
+		 * available? PCI ID, VTB, something else?
+		 */
+		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
+			dev_priv->max_cdclk_freq = 450000;
+		else if (IS_BDW_ULX(dev))
+			dev_priv->max_cdclk_freq = 450000;
+		else if (IS_BDW_ULT(dev))
+			dev_priv->max_cdclk_freq = 540000;
+		else
+			dev_priv->max_cdclk_freq = 675000;
+	} else if (IS_HASWELL(dev)) {
 		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
 			dev_priv->max_cdclk_freq = 450000;
 		else if (IS_HSW_ULX(dev))
@@ -6426,13 +6441,11 @@ static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv,
 		return true;
 
 	/*
-	 * FIXME if we compare against max we should then
-	 * increase the cdclk frequency when the current
-	 * value is too low. The other option is to compare
-	 * against the cdclk frequency we're going have post
-	 * modeset (ie. one we computed using other constraints).
-	 * Need to measure whether using a lower cdclk w/o IPS
-	 * is better or worse than a higher cdclk w/ IPS.
+	 * We compare against max which means we must take
+	 * the increased cdclk requirement into account when
+	 * calculating the new cdclk.
+	 *
+	 * Should measure whether using a lower cdclk w/o IPS
 	 */
 	return ilk_pipe_pixel_rate(pipe_config) <=
 		dev_priv->max_cdclk_freq * 95 / 100;
@@ -9421,9 +9434,18 @@ static int ilk_max_pixel_rate(struct drm_i915_private *dev_priv)
 	int max_pixel_rate = 0;
 
 	for_each_intel_crtc(dev, crtc) {
-		if (crtc->new_enabled)
-			max_pixel_rate = max((int)max_pixel_rate,
-					     (int)ilk_pipe_pixel_rate(crtc->config));
+		int pixel_rate;
+
+		if (!crtc->new_enabled)
+			continue;
+
+		pixel_rate = ilk_pipe_pixel_rate(crtc->config);
+
+		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
+		if (IS_BROADWELL(dev) && crtc->config->ips_enabled)
+			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
+
+		max_pixel_rate = max(max_pixel_rate, pixel_rate);
 	}
 
 	return max_pixel_rate;
@@ -9438,7 +9460,9 @@ static int haswell_calc_cdclk(struct drm_i915_private *dev_priv,
 	 * FIXME should also account for plane ratio
 	 * once 64bpp pixel formats are supported.
 	 */
-	if (max_pixel_rate > 450000)
+	if (max_pixel_rate > 540000)
+		cdclk = 675000;
+	else if (max_pixel_rate > 450000)
 		cdclk = 540000;
 	else if (max_pixel_rate > 337500 || !IS_HSW_ULX(dev_priv))
 		cdclk = 450000;
@@ -9503,6 +9527,83 @@ static void haswell_set_cdclk(struct drm_device *dev, int cdclk)
 	     cdclk, dev_priv->cdclk_freq);
 }
 
+static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t val, data;
+	int ret;
+
+	if (WARN((I915_READ(LCPLL_CTL) &
+		  (LCPLL_PLL_DISABLE | LCPLL_PLL_LOCK |
+		   LCPLL_CD_CLOCK_DISABLE | LCPLL_ROOT_CD_CLOCK_DISABLE |
+		   LCPLL_CD2X_CLOCK_DISABLE | LCPLL_POWER_DOWN_ALLOW |
+		   LCPLL_CD_SOURCE_FCLK)) != LCPLL_PLL_LOCK,
+		 "trying to change cdclk frequency with cdclk not enabled\n"))
+		return;
+
+	mutex_lock(&dev_priv->rps.hw_lock);
+	ret = sandybridge_pcode_write(dev_priv,
+				      BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
+	mutex_unlock(&dev_priv->rps.hw_lock);
+	if (ret) {
+		DRM_ERROR("failed to inform pcode about cdclk change\n");
+		return;
+	}
+
+	val = I915_READ(LCPLL_CTL);
+	val |= LCPLL_CD_SOURCE_FCLK;
+	I915_WRITE(LCPLL_CTL, val);
+
+	if (wait_for_atomic_us(I915_READ(LCPLL_CTL) &
+			       LCPLL_CD_SOURCE_FCLK_DONE, 1))
+		DRM_ERROR("Switching to FCLK failed\n");
+
+	val = I915_READ(LCPLL_CTL);
+	val &= ~LCPLL_CLK_FREQ_MASK;
+
+	switch (cdclk) {
+	case 450000:
+		val |= LCPLL_CLK_FREQ_450;
+		data = 0;
+		break;
+	case 540000:
+		val |= LCPLL_CLK_FREQ_54O_BDW;
+		data = 1;
+		break;
+	case 337500:
+		val |= LCPLL_CLK_FREQ_337_5_BDW;
+		data = 2;
+		break;
+	case 675000:
+		val |= LCPLL_CLK_FREQ_675_BDW;
+		data = 3;
+		break;
+	default:
+		WARN(1, "invalid cdclk frequency\n");
+		return;
+	}
+
+	I915_WRITE(LCPLL_CTL, val);
+
+	val = I915_READ(LCPLL_CTL);
+	val &= ~LCPLL_CD_SOURCE_FCLK;
+	I915_WRITE(LCPLL_CTL, val);
+
+	if (wait_for_atomic_us((I915_READ(LCPLL_CTL) &
+				LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
+		DRM_ERROR("Switching back to LCPLL failed\n");
+
+	mutex_lock(&dev_priv->rps.hw_lock);
+	sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ, data);
+	mutex_unlock(&dev_priv->rps.hw_lock);
+
+	intel_update_cdclk(dev);
+
+	WARN(cdclk != dev_priv->cdclk_freq,
+	     "cdclk requested %d kHz but got %d kHz\n",
+	     cdclk, dev_priv->cdclk_freq);
+}
+
 static int haswell_modeset_global_pipes(struct drm_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
@@ -9542,7 +9643,10 @@ static void haswell_modeset_global_resources(struct drm_atomic_state *state)
 	int req_cdclk = haswell_calc_cdclk(dev_priv, max_pixel_rate);
 
 	if (req_cdclk != dev_priv->cdclk_freq) {
-		haswell_set_cdclk(dev, req_cdclk);
+		if (IS_BROADWELL(dev))
+			broadwell_set_cdclk(dev, req_cdclk);
+		else
+			haswell_set_cdclk(dev, req_cdclk);
 	}
 }
 
@@ -12724,8 +12828,8 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
 	 * mode set on this crtc.  For other crtcs we need to use the
 	 * adjusted_mode bits in the crtc directly.
 	 */
-	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_HASWELL(dev)) {
-		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
+	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_HASWELL(dev) || IS_BROADWELL(dev)) {
+		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_BROADWELL(dev)) {
 			ret = valleyview_modeset_global_pipes(state);
 			if (ret)
 				return ret;
@@ -14616,9 +14720,8 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
 	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
 		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
-		if (IS_HASWELL(dev))
-			dev_priv->display.modeset_global_resources =
-				haswell_modeset_global_resources;
+		dev_priv->display.modeset_global_resources =
+			haswell_modeset_global_resources;
 	} else if (IS_VALLEYVIEW(dev)) {
 		dev_priv->display.modeset_global_resources =
 			valleyview_modeset_global_resources;
-- 
1.9.1

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

Re: [PATCH v4 12/12] drm/i915: BDW clock change support

[Damien Lespiau]

On Fri, May 22, 2015 at 11:22:42AM +0300, Mika Kahola wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add support for changing cdclk frequency during runtime on BDW. The
> procedure is quite a bit different on BDW from the one on HSW, so
> add a separate function for it.
> 
> Also with IPS enabled the actual pixel rate mustn't exceed 95% of cdclk,
> so take that into account when computing the max pixel rate.
> 
> v2: Grab rps.hw_lock around sandybridge_pcode_write()
> v3: Rebase due to power well vs. .global_resources() reordering
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> v4: Rebased to the latest
> v5: Rebased to the latest
> 
> Reviewed-by: Mika Kahola <mika.kahola@intel.com>
> 
> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---

(fwiw, I'm making the SKL cdclk code look like those HSW/BDW patches a
bit more. We may want to extract a few low level vfuncs
(set_core_display_clock(), ...) but that's for another time)

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

>  drivers/gpu/drm/i915/i915_reg.h      |   1 +
>  drivers/gpu/drm/i915/intel_display.c | 139 ++++++++++++++++++++++++++++++-----
>  2 files changed, 122 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 015fe12..64ec500 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6702,6 +6702,7 @@ enum skl_disp_power_wells {
>  #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
>  #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
>  #define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
> +#define   BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ	0x18
>  #define   GEN9_PCODE_READ_MEM_LATENCY		0x6
>  #define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
>  #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 556d0ec7..71efd2f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5545,7 +5545,22 @@ static void intel_update_max_cdclk(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	if (IS_HASWELL(dev)) {
> +	if (IS_BROADWELL(dev))  {
> +		/*
> +		 * FIXME with extra cooling we can allow
> +		 * 540 MHz for ULX and 675 Mhz for ULT.
> +		 * How can we know if extra cooling is
> +		 * available? PCI ID, VTB, something else?
> +		 */
> +		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
> +			dev_priv->max_cdclk_freq = 450000;
> +		else if (IS_BDW_ULX(dev))
> +			dev_priv->max_cdclk_freq = 450000;
> +		else if (IS_BDW_ULT(dev))
> +			dev_priv->max_cdclk_freq = 540000;
> +		else
> +			dev_priv->max_cdclk_freq = 675000;
> +	} else if (IS_HASWELL(dev)) {
>  		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
>  			dev_priv->max_cdclk_freq = 450000;
>  		else if (IS_HSW_ULX(dev))
> @@ -6426,13 +6441,11 @@ static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv,
>  		return true;
>  
>  	/*
> -	 * FIXME if we compare against max we should then
> -	 * increase the cdclk frequency when the current
> -	 * value is too low. The other option is to compare
> -	 * against the cdclk frequency we're going have post
> -	 * modeset (ie. one we computed using other constraints).
> -	 * Need to measure whether using a lower cdclk w/o IPS
> -	 * is better or worse than a higher cdclk w/ IPS.
> +	 * We compare against max which means we must take
> +	 * the increased cdclk requirement into account when
> +	 * calculating the new cdclk.
> +	 *
> +	 * Should measure whether using a lower cdclk w/o IPS
>  	 */
>  	return ilk_pipe_pixel_rate(pipe_config) <=
>  		dev_priv->max_cdclk_freq * 95 / 100;
> @@ -9421,9 +9434,18 @@ static int ilk_max_pixel_rate(struct drm_i915_private *dev_priv)
>  	int max_pixel_rate = 0;
>  
>  	for_each_intel_crtc(dev, crtc) {
> -		if (crtc->new_enabled)
> -			max_pixel_rate = max((int)max_pixel_rate,
> -					     (int)ilk_pipe_pixel_rate(crtc->config));
> +		int pixel_rate;
> +
> +		if (!crtc->new_enabled)
> +			continue;
> +
> +		pixel_rate = ilk_pipe_pixel_rate(crtc->config);
> +
> +		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> +		if (IS_BROADWELL(dev) && crtc->config->ips_enabled)
> +			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> +
> +		max_pixel_rate = max(max_pixel_rate, pixel_rate);
>  	}
>  
>  	return max_pixel_rate;
> @@ -9438,7 +9460,9 @@ static int haswell_calc_cdclk(struct drm_i915_private *dev_priv,
>  	 * FIXME should also account for plane ratio
>  	 * once 64bpp pixel formats are supported.
>  	 */
> -	if (max_pixel_rate > 450000)
> +	if (max_pixel_rate > 540000)
> +		cdclk = 675000;
> +	else if (max_pixel_rate > 450000)
>  		cdclk = 540000;
>  	else if (max_pixel_rate > 337500 || !IS_HSW_ULX(dev_priv))
>  		cdclk = 450000;
> @@ -9503,6 +9527,83 @@ static void haswell_set_cdclk(struct drm_device *dev, int cdclk)
>  	     cdclk, dev_priv->cdclk_freq);
>  }

Quite frankly, I'd split out a broadwell cacl_cdclk out.

>  
> +static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	uint32_t val, data;
> +	int ret;
> +
> +	if (WARN((I915_READ(LCPLL_CTL) &
> +		  (LCPLL_PLL_DISABLE | LCPLL_PLL_LOCK |
> +		   LCPLL_CD_CLOCK_DISABLE | LCPLL_ROOT_CD_CLOCK_DISABLE |
> +		   LCPLL_CD2X_CLOCK_DISABLE | LCPLL_POWER_DOWN_ALLOW |
> +		   LCPLL_CD_SOURCE_FCLK)) != LCPLL_PLL_LOCK,
> +		 "trying to change cdclk frequency with cdclk not enabled\n"))
> +		return;
> +
> +	mutex_lock(&dev_priv->rps.hw_lock);
> +	ret = sandybridge_pcode_write(dev_priv,
> +				      BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
> +	mutex_unlock(&dev_priv->rps.hw_lock);
> +	if (ret) {
> +		DRM_ERROR("failed to inform pcode about cdclk change\n");
> +		return;
> +	}
> +
> +	val = I915_READ(LCPLL_CTL);
> +	val |= LCPLL_CD_SOURCE_FCLK;
> +	I915_WRITE(LCPLL_CTL, val);
> +
> +	if (wait_for_atomic_us(I915_READ(LCPLL_CTL) &
> +			       LCPLL_CD_SOURCE_FCLK_DONE, 1))
> +		DRM_ERROR("Switching to FCLK failed\n");
> +
> +	val = I915_READ(LCPLL_CTL);
> +	val &= ~LCPLL_CLK_FREQ_MASK;
> +
> +	switch (cdclk) {
> +	case 450000:
> +		val |= LCPLL_CLK_FREQ_450;
> +		data = 0;
> +		break;
> +	case 540000:
> +		val |= LCPLL_CLK_FREQ_54O_BDW;
> +		data = 1;
> +		break;
> +	case 337500:
> +		val |= LCPLL_CLK_FREQ_337_5_BDW;
> +		data = 2;
> +		break;
> +	case 675000:
> +		val |= LCPLL_CLK_FREQ_675_BDW;
> +		data = 3;
> +		break;
> +	default:
> +		WARN(1, "invalid cdclk frequency\n");
> +		return;
> +	}
> +
> +	I915_WRITE(LCPLL_CTL, val);
> +
> +	val = I915_READ(LCPLL_CTL);
> +	val &= ~LCPLL_CD_SOURCE_FCLK;
> +	I915_WRITE(LCPLL_CTL, val);
> +
> +	if (wait_for_atomic_us((I915_READ(LCPLL_CTL) &
> +				LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
> +		DRM_ERROR("Switching back to LCPLL failed\n");
> +
> +	mutex_lock(&dev_priv->rps.hw_lock);
> +	sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ, data);
> +	mutex_unlock(&dev_priv->rps.hw_lock);
> +
> +	intel_update_cdclk(dev);
> +
> +	WARN(cdclk != dev_priv->cdclk_freq,
> +	     "cdclk requested %d kHz but got %d kHz\n",
> +	     cdclk, dev_priv->cdclk_freq);
> +}
> +
>  static int haswell_modeset_global_pipes(struct drm_atomic_state *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->dev);
> @@ -9542,7 +9643,10 @@ static void haswell_modeset_global_resources(struct drm_atomic_state *state)
>  	int req_cdclk = haswell_calc_cdclk(dev_priv, max_pixel_rate);
>  
>  	if (req_cdclk != dev_priv->cdclk_freq) {
> -		haswell_set_cdclk(dev, req_cdclk);
> +		if (IS_BROADWELL(dev))
> +			broadwell_set_cdclk(dev, req_cdclk);
> +		else
> +			haswell_set_cdclk(dev, req_cdclk);
>  	}
>  }
>  
> @@ -12724,8 +12828,8 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
>  	 * mode set on this crtc.  For other crtcs we need to use the
>  	 * adjusted_mode bits in the crtc directly.
>  	 */
> -	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_HASWELL(dev)) {
> -		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
> +	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> +		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_BROADWELL(dev)) {
>  			ret = valleyview_modeset_global_pipes(state);
>  			if (ret)
>  				return ret;
> @@ -14616,9 +14720,8 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
>  	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
> -		if (IS_HASWELL(dev))
> -			dev_priv->display.modeset_global_resources =
> -				haswell_modeset_global_resources;
> +		dev_priv->display.modeset_global_resources =
> +			haswell_modeset_global_resources;
>  	} else if (IS_VALLEYVIEW(dev)) {
>  		dev_priv->display.modeset_global_resources =
>  			valleyview_modeset_global_resources;
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 00/12] All sort of cdclk stuff

[Jani Nikula]

On Fri, 22 May 2015, Mika Kahola <mika.kahola@intel.com> wrote:
> This patch series rebases Ville's original cdclk patch series
> excluding the ones that	are already reviewed.
>
> http://lists.freedesktop.org/archives/intel-gfx/2014-November/055633.html
>
> The patches are rebased to the latest drm-intel-nightly and while I was
> doing it I tagged the reviewed-by. Maybe Daniel/Jani can comment if this
> is procedure is ok or not. There is one exception to this and that
> is the patch 'HSW cdclk support' which I had to modify to support the
> recent atomic changes. This patch requires a review.

Since you're sending the patches, you do need to add your Signed-off-by:
line after Ville's, and IMO you don't need to add your Reviewed-by: if
you have your s-o-b there (but if there's review from someone else that
still applies, you should add that).

Also, you don't need to add any Author: tags; if you have git configured
properly, and it seems that you do, git will automatically add a From:
line at the beginning, and when we apply the patches, that gets used for
authorship.

To recap, you'd end up with this, From: line gets added by git in
format-patch/send-email if you're not the author:

---

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

Commit message, bla bla blah.

v2: version history goes here for us.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Mika Kahola <mika.kahola@intel.com>

---

BR,
Jani.

>
> Ville Syrjälä (12):
>   drm/i915: Fix i855 get_display_clock_speed
>   drm/i915: Fix 852GM/GMV cdclk
>   drm/i915: Add cdclk extraction for g33, g965gm and g4x
>   drm/i915: Warn when cdclk for the platforms is not known
>   drm/i915: Cache current cdclk frequency in dev_priv
>   drm/i915: Use cached cdclk value
>   drm/i915: Unify ilk and hsw .get_aux_clock_divider
>   drm/i915: Store max cdclk value in dev_priv
>   drm/i915: Don't enable IPS when pixel rate exceeds 95%
>   drm/i915: HSW cdclk support
>   drm/i915: Add IS_BDW_ULX
>   drm/i915: BDW clock change support
>
>  drivers/gpu/drm/i915/i915_drv.h      |   5 +-
>  drivers/gpu/drm/i915/i915_reg.h      |  18 +-
>  drivers/gpu/drm/i915/intel_display.c | 546 +++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_dp.c      |  24 +-
>  drivers/gpu/drm/i915/intel_drv.h     |   2 +-
>  drivers/gpu/drm/i915/intel_pm.c      |  19 +-
>  6 files changed, 560 insertions(+), 54 deletions(-)
>
> -- 
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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 v4 00/12] All sort of cdclk stuff

[Jani Nikula]

On Fri, 22 May 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Fri, 22 May 2015, Mika Kahola <mika.kahola@intel.com> wrote:
>> This patch series rebases Ville's original cdclk patch series
>> excluding the ones that	are already reviewed.
>>
>> http://lists.freedesktop.org/archives/intel-gfx/2014-November/055633.html
>>
>> The patches are rebased to the latest drm-intel-nightly and while I was
>> doing it I tagged the reviewed-by. Maybe Daniel/Jani can comment if this
>> is procedure is ok or not. There is one exception to this and that
>> is the patch 'HSW cdclk support' which I had to modify to support the
>> recent atomic changes. This patch requires a review.
>
> Since you're sending the patches, you do need to add your Signed-off-by:
> line after Ville's, and IMO you don't need to add your Reviewed-by: if
> you have your s-o-b there (but if there's review from someone else that
> still applies, you should add that).
>
> Also, you don't need to add any Author: tags; if you have git configured
> properly, and it seems that you do, git will automatically add a From:
> line at the beginning, and when we apply the patches, that gets used for
> authorship.
>
> To recap, you'd end up with this, From: line gets added by git in
> format-patch/send-email if you're not the author:
>
> ---
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Commit message, bla bla blah.
>
> v2: version history goes here for us.

Oh, and I tend to add e.g. "v2 by Jani" if I'm picking up someone else's
work. And for patches that I basically rewrite, I'll take the authorship
to myself, and, depending on the case, add e.g. "Based on a patch by
J. Random Hacker <someone@example.com>".

It is a bit of a pity that you can only have one author even when shared
credit would be fair.


J.

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
>
> ---
>
> BR,
> Jani.
>
>>
>> Ville Syrjälä (12):
>>   drm/i915: Fix i855 get_display_clock_speed
>>   drm/i915: Fix 852GM/GMV cdclk
>>   drm/i915: Add cdclk extraction for g33, g965gm and g4x
>>   drm/i915: Warn when cdclk for the platforms is not known
>>   drm/i915: Cache current cdclk frequency in dev_priv
>>   drm/i915: Use cached cdclk value
>>   drm/i915: Unify ilk and hsw .get_aux_clock_divider
>>   drm/i915: Store max cdclk value in dev_priv
>>   drm/i915: Don't enable IPS when pixel rate exceeds 95%
>>   drm/i915: HSW cdclk support
>>   drm/i915: Add IS_BDW_ULX
>>   drm/i915: BDW clock change support
>>
>>  drivers/gpu/drm/i915/i915_drv.h      |   5 +-
>>  drivers/gpu/drm/i915/i915_reg.h      |  18 +-
>>  drivers/gpu/drm/i915/intel_display.c | 546 +++++++++++++++++++++++++++++++++--
>>  drivers/gpu/drm/i915/intel_dp.c      |  24 +-
>>  drivers/gpu/drm/i915/intel_drv.h     |   2 +-
>>  drivers/gpu/drm/i915/intel_pm.c      |  19 +-
>>  6 files changed, 560 insertions(+), 54 deletions(-)
>>
>> -- 
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> 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 v4 00/12] All sort of cdclk stuff

[Joe Konno]

Do we have an idea when this patch series will be reviewed? Customers
are awaiting for this to be merged to the drm-intel fd.o repository.

Cheers

On 05/22/2015 01:22 AM, Mika Kahola wrote:
> This patch series rebases Ville's original cdclk patch series
> excluding the ones that	are already reviewed.
> 
> http://lists.freedesktop.org/archives/intel-gfx/2014-November/055633.html
> 
> The patches are rebased to the latest drm-intel-nightly and while I was
> doing it I tagged the reviewed-by. Maybe Daniel/Jani can comment if this
> is procedure is ok or not. There is one exception to this and that
> is the patch 'HSW cdclk support' which I had to modify to support the
> recent atomic changes. This patch requires a review. 
> 
> Ville Syrjälä (12):
>   drm/i915: Fix i855 get_display_clock_speed
>   drm/i915: Fix 852GM/GMV cdclk
>   drm/i915: Add cdclk extraction for g33, g965gm and g4x
>   drm/i915: Warn when cdclk for the platforms is not known
>   drm/i915: Cache current cdclk frequency in dev_priv
>   drm/i915: Use cached cdclk value
>   drm/i915: Unify ilk and hsw .get_aux_clock_divider
>   drm/i915: Store max cdclk value in dev_priv
>   drm/i915: Don't enable IPS when pixel rate exceeds 95%
>   drm/i915: HSW cdclk support
>   drm/i915: Add IS_BDW_ULX
>   drm/i915: BDW clock change support
> 
>  drivers/gpu/drm/i915/i915_drv.h      |   5 +-
>  drivers/gpu/drm/i915/i915_reg.h      |  18 +-
>  drivers/gpu/drm/i915/intel_display.c | 546 +++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_dp.c      |  24 +-
>  drivers/gpu/drm/i915/intel_drv.h     |   2 +-
>  drivers/gpu/drm/i915/intel_pm.c      |  19 +-
>  6 files changed, 560 insertions(+), 54 deletions(-)
> 

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

Re: [PATCH v4 00/12] All sort of cdclk stuff

[Damien Lespiau]

On Wed, May 27, 2015 at 02:49:38PM -0700, Joe Konno wrote:
> Do we have an idea when this patch series will be reviewed? Customers
> are awaiting for this to be merged to the drm-intel fd.o repository.

This series is mostly reviewed, all but one patch the HSW CDCLK code.
I'm not sure that we want to enable that on HSW as AFAIK changing CDCLK
hasn't been validated on that platform.

Next step is for Mika to resend the series with jani's comments I
believe?

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

Re: [PATCH v4 00/12] All sort of cdclk stuff

[Daniel Vetter]

On Thu, May 28, 2015 at 04:29:10PM +0100, Damien Lespiau wrote:
> On Wed, May 27, 2015 at 02:49:38PM -0700, Joe Konno wrote:
> > Do we have an idea when this patch series will be reviewed? Customers
> > are awaiting for this to be merged to the drm-intel fd.o repository.
> 
> This series is mostly reviewed, all but one patch the HSW CDCLK code.
> I'm not sure that we want to enable that on HSW as AFAIK changing CDCLK
> hasn't been validated on that platform.
> 
> Next step is for Mika to resend the series with jani's comments I
> believe?

Yeah.

The other thing I realized (and the reason I didn't realize that most here
is reviewed) is that the r-b tags magically appeared when resending and
there seems to never have been a review on the m-l. I don't like when that
kind of backroom review happens, since it excludes everyone else. Besides
that I'll simply not noticed it has happened ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 00/12] All sort of cdclk stuff

[Joe Konno]

Who will commit to reviewing this series to danvet's satisfaction? Jani
and Damien?

On 05/28/2015 09:01 AM, Daniel Vetter wrote:
> On Thu, May 28, 2015 at 04:29:10PM +0100, Damien Lespiau wrote:
>> On Wed, May 27, 2015 at 02:49:38PM -0700, Joe Konno wrote:
>>> Do we have an idea when this patch series will be reviewed? Customers
>>> are awaiting for this to be merged to the drm-intel fd.o repository.
>>
>> This series is mostly reviewed, all but one patch the HSW CDCLK code.
>> I'm not sure that we want to enable that on HSW as AFAIK changing CDCLK
>> hasn't been validated on that platform.
>>
>> Next step is for Mika to resend the series with jani's comments I
>> believe?
> 
> Yeah.
> 
> The other thing I realized (and the reason I didn't realize that most here
> is reviewed) is that the r-b tags magically appeared when resending and
> there seems to never have been a review on the m-l. I don't like when that
> kind of backroom review happens, since it excludes everyone else. Besides
> that I'll simply not noticed it has happened ...
> -Daniel
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 00/12] All sort of cdclk stuff

[Damien Lespiau]

On Thu, May 28, 2015 at 10:11:59AM -0700, Joe Konno wrote:
> Who will commit to reviewing this series to danvet's satisfaction? Jani
> and Damien?

Please re-read my first answer :) There's even the next step to push
this work forward in there (also intel-gfx list is full of old fashioned
farts that live and die by avoiding to top post when possible).

HTH,

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

Re: [PATCH v4 00/12] All sort of cdclk stuff

[Jani Nikula]

On Thu, 28 May 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, May 28, 2015 at 04:29:10PM +0100, Damien Lespiau wrote:
>> On Wed, May 27, 2015 at 02:49:38PM -0700, Joe Konno wrote:
>> > Do we have an idea when this patch series will be reviewed? Customers
>> > are awaiting for this to be merged to the drm-intel fd.o repository.
>> 
>> This series is mostly reviewed, all but one patch the HSW CDCLK code.
>> I'm not sure that we want to enable that on HSW as AFAIK changing CDCLK
>> hasn't been validated on that platform.
>> 
>> Next step is for Mika to resend the series with jani's comments I
>> believe?
>
> Yeah.
>
> The other thing I realized (and the reason I didn't realize that most here
> is reviewed) is that the r-b tags magically appeared when resending and
> there seems to never have been a review on the m-l. I don't like when that
> kind of backroom review happens, since it excludes everyone else. Besides
> that I'll simply not noticed it has happened ...

Well, as I mentioned, Mika should've added his Signed-off-by, not
Reviewed-by, when picking up where Ville left off. I'm not sure if much
further review is necessary if the patches have gone through two
developers. So I don't think it's as bad as you imply.

BR,
Jani.


> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
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 v4 00/12] All sort of cdclk stuff

[Damien Lespiau]

On Thu, May 28, 2015 at 08:17:35PM +0300, Jani Nikula wrote:
> On Thu, 28 May 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Thu, May 28, 2015 at 04:29:10PM +0100, Damien Lespiau wrote:
> >> On Wed, May 27, 2015 at 02:49:38PM -0700, Joe Konno wrote:
> >> > Do we have an idea when this patch series will be reviewed? Customers
> >> > are awaiting for this to be merged to the drm-intel fd.o repository.
> >> 
> >> This series is mostly reviewed, all but one patch the HSW CDCLK code.
> >> I'm not sure that we want to enable that on HSW as AFAIK changing CDCLK
> >> hasn't been validated on that platform.
> >> 
> >> Next step is for Mika to resend the series with jani's comments I
> >> believe?
> >
> > Yeah.
> >
> > The other thing I realized (and the reason I didn't realize that most here
> > is reviewed) is that the r-b tags magically appeared when resending and
> > there seems to never have been a review on the m-l. I don't like when that
> > kind of backroom review happens, since it excludes everyone else. Besides
> > that I'll simply not noticed it has happened ...
> 
> Well, as I mentioned, Mika should've added his Signed-off-by, not
> Reviewed-by, when picking up where Ville left off. I'm not sure if much
> further review is necessary if the patches have gone through two
> developers. So I don't think it's as bad as you imply.

Oh, you mean those were supposed to be sob tags and not r-b tags?

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

Re: [PATCH v4 00/12] All sort of cdclk stuff

[Jani Nikula]

On Thu, 28 May 2015, Damien Lespiau <damien.lespiau@intel.com> wrote:
> On Thu, May 28, 2015 at 08:17:35PM +0300, Jani Nikula wrote:
>> On Thu, 28 May 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
>> > On Thu, May 28, 2015 at 04:29:10PM +0100, Damien Lespiau wrote:
>> >> On Wed, May 27, 2015 at 02:49:38PM -0700, Joe Konno wrote:
>> >> > Do we have an idea when this patch series will be reviewed? Customers
>> >> > are awaiting for this to be merged to the drm-intel fd.o repository.
>> >> 
>> >> This series is mostly reviewed, all but one patch the HSW CDCLK code.
>> >> I'm not sure that we want to enable that on HSW as AFAIK changing CDCLK
>> >> hasn't been validated on that platform.
>> >> 
>> >> Next step is for Mika to resend the series with jani's comments I
>> >> believe?
>> >
>> > Yeah.
>> >
>> > The other thing I realized (and the reason I didn't realize that most here
>> > is reviewed) is that the r-b tags magically appeared when resending and
>> > there seems to never have been a review on the m-l. I don't like when that
>> > kind of backroom review happens, since it excludes everyone else. Besides
>> > that I'll simply not noticed it has happened ...
>> 
>> Well, as I mentioned, Mika should've added his Signed-off-by, not
>> Reviewed-by, when picking up where Ville left off. I'm not sure if much
>> further review is necessary if the patches have gone through two
>> developers. So I don't think it's as bad as you imply.
>
> Oh, you mean those were supposed to be sob tags and not r-b tags?

Yes. *sob*. That's what you're supposed to do when you send patches to
the list, whether they are yours or not. While the Developer's
Certificate of Origin does not explicitly state anything about the
quality of the contribution like the Reviewer's Statement of Oversight
does, I think it's implied you stand by your signed off work and not
send someone else's crap, even if it's certified open source crap.

BR,
Jani.


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