[PATCH 00/11] drm/i915: VLV display clock/phy stuff

[PATCH 00/11] drm/i915: VLV display clock/phy stuff

[ville.syrjala]

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

I was tring to see if the 200 MHz cdclk option would work on VLV, and also
whether the Vnn voltage drops in response to cdclk. Sadly neither seems be to
true on my VLV. Might be some other unit is keeping Vnn elevated. I need to
trawl the docs more to see if I can find some fuses somewhere so I could
see what are the min/max correesponding to the the min/max cdclk.

I have no explanation why the 200MHz cdclk doesn't work since the voltage
doesn't change, the CCK clock ratio looks correct, and the pixel clock doens't
seem to be a factor either (both 150Hz and 25MHz modes fail).

Given that my goal of using the 200MHz cdclk failed this series consists
mainly of cleanups and removal of duplicated code. There are some real
changes too however:
- use 200MHz cdclk when all pipes are off
- fix 320MHz vs. 333MHz cdclk confusion
- wait for cdclk change to happen when going for 400MHz
- fix Jesse's cmnlane side reset workaround

And finally I attempted to have the driver gate off all display clocks
when powering down the disp2d well. Sadly that didn't go so well either,
so the last patch is here just for future reference. Maybe someone can
figure out what we're actually supposed to do there, if anything.

Ville Syrjälä (11):
  drm/i915: Change vlv cdclk to use kHz units
  drm/i915: Give names to the CCK_DISPLAY_CLOCK_CONTROL bits
  drm/i915: Move vlv cdclk code to .get_display_clock_speed()
  drm/i915: Handle 320 vs. 333 MHz cdclk on vlv
  drm/i915: Use 200MHz cdclk on vlv when all pipes are off
  drm/i915: Wait for cdclk change to occure when going for 400MHz
  drm/i915: Warn if there's a cdclk change in progess
  drm/i915: Kill duplicated cdclk readout code from i2c
  drm/i915: Pull the cmnlane tricks into its own power well ops
  drm/i915: Move VLV cmnlane workaround to intel_power_domains_init_hw()
  drm/i915: Turn off clocks when disp2d is powered down

 drivers/gpu/drm/i915/i915_reg.h      |  10 ++
 drivers/gpu/drm/i915/intel_display.c | 129 ++++++++++++-----------
 drivers/gpu/drm/i915/intel_drv.h     |   5 +-
 drivers/gpu/drm/i915/intel_i2c.c     |  54 ----------
 drivers/gpu/drm/i915/intel_pm.c      | 198 ++++++++++++++++++++++++++---------
 5 files changed, 225 insertions(+), 171 deletions(-)

-- 
1.8.5.5

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

[PATCH 01/11] drm/i915: Change vlv cdclk to use kHz units

[ville.syrjala]

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

Use kHz units in vlv cdclk code since that's more customary.

Also replace the precomputed 90% values with *9/10 computation
for extra clarity.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++-------------
 drivers/gpu/drm/i915/intel_i2c.c     |  2 +-
 drivers/gpu/drm/i915/intel_pm.c      |  2 +-
 3 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5762726..5f66dc8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4429,6 +4429,7 @@ static void modeset_update_crtc_power_domains(struct drm_device *dev)
 	intel_display_set_init_power(dev_priv, false);
 }
 
+/* returns HPLL frequency in kHz */
 int valleyview_get_vco(struct drm_i915_private *dev_priv)
 {
 	int hpll_freq, vco_freq[] = { 800, 1600, 2000, 2400 };
@@ -4439,7 +4440,7 @@ int valleyview_get_vco(struct drm_i915_private *dev_priv)
 		CCK_FUSE_HPLL_FREQ_MASK;
 	mutex_unlock(&dev_priv->dpio_lock);
 
-	return vco_freq[hpll_freq];
+	return vco_freq[hpll_freq] * 1000;
 }
 
 /* Adjust CDclk dividers to allow high res or save power if possible */
@@ -4451,9 +4452,9 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
 	WARN_ON(valleyview_cur_cdclk(dev_priv) != dev_priv->vlv_cdclk_freq);
 	dev_priv->vlv_cdclk_freq = cdclk;
 
-	if (cdclk >= 320) /* jump to highest voltage for 400MHz too */
+	if (cdclk >= 320000) /* jump to highest voltage for 400MHz too */
 		cmd = 2;
-	else if (cdclk == 266)
+	else if (cdclk == 266667)
 		cmd = 1;
 	else
 		cmd = 0;
@@ -4470,11 +4471,11 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
 	}
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
-	if (cdclk == 400) {
+	if (cdclk == 400000) {
 		u32 divider, vco;
 
 		vco = valleyview_get_vco(dev_priv);
-		divider = ((vco << 1) / cdclk) - 1;
+		divider = DIV_ROUND_CLOSEST(vco << 1, cdclk) - 1;
 
 		mutex_lock(&dev_priv->dpio_lock);
 		/* adjust cdclk divider */
@@ -4494,7 +4495,7 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
 	 * For high bandwidth configs, we set a higher latency in the bunit
 	 * so that the core display fetch happens in time to avoid underruns.
 	 */
-	if (cdclk == 400)
+	if (cdclk == 400000)
 		val |= 4500 / 250; /* 4.5 usec */
 	else
 		val |= 3000 / 250; /* 3.0 usec */
@@ -4518,7 +4519,7 @@ int valleyview_cur_cdclk(struct drm_i915_private *dev_priv)
 
 	divider &= 0xf;
 
-	cur_cdclk = (vco << 1) / (divider + 1);
+	cur_cdclk = DIV_ROUND_CLOSEST(vco << 1, divider + 1);
 
 	return cur_cdclk;
 }
@@ -4535,12 +4536,12 @@ static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
 	 * So we check to see whether we're above 90% of the lower bin and
 	 * adjust if needed.
 	 */
-	if (max_pixclk > 288000) {
-		return 400;
-	} else if (max_pixclk > 240000) {
-		return 320;
-	} else
-		return 266;
+	if (max_pixclk > 320000*9/10)
+		return 400000;
+	else if (max_pixclk > 266667*9/10)
+		return 320000;
+	else
+		return 266667;
 	/* Looks like the 200MHz CDclk freq doesn't work on some configs */
 }
 
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index d33b61d..9ce4f09 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -86,7 +86,7 @@ static void gmbus_set_freq(struct drm_i915_private *dev_priv)
 
 	BUG_ON(!IS_VALLEYVIEW(dev_priv->dev));
 
-	vco = valleyview_get_vco(dev_priv);
+	vco = valleyview_get_vco(dev_priv) / 1000;
 
 	/* Get the CDCLK divide ratio */
 	cdclk_div = get_disp_clk_div(dev_priv, CDCLK);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5024bc8..a9ddf74 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5536,7 +5536,7 @@ static void valleyview_init_clock_gating(struct drm_device *dev)
 	DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
 
 	dev_priv->vlv_cdclk_freq = valleyview_cur_cdclk(dev_priv);
-	DRM_DEBUG_DRIVER("Current CD clock rate: %d MHz",
+	DRM_DEBUG_DRIVER("Current CD clock rate: %d kHz",
 			 dev_priv->vlv_cdclk_freq);
 
 	I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE);
-- 
1.8.5.5

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

Re: [PATCH 01/11] drm/i915: Change vlv cdclk to use kHz units

[Jesse Barnes]

On Fri, 13 Jun 2014 13:37:47 +0300
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Use kHz units in vlv cdclk code since that's more customary.
> 
> Also replace the precomputed 90% values with *9/10 computation
> for extra clarity.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++-------------
>  drivers/gpu/drm/i915/intel_i2c.c     |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c      |  2 +-
>  3 files changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5762726..5f66dc8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4429,6 +4429,7 @@ static void modeset_update_crtc_power_domains(struct drm_device *dev)
>  	intel_display_set_init_power(dev_priv, false);
>  }
>  
> +/* returns HPLL frequency in kHz */
>  int valleyview_get_vco(struct drm_i915_private *dev_priv)
>  {
>  	int hpll_freq, vco_freq[] = { 800, 1600, 2000, 2400 };
> @@ -4439,7 +4440,7 @@ int valleyview_get_vco(struct drm_i915_private *dev_priv)
>  		CCK_FUSE_HPLL_FREQ_MASK;
>  	mutex_unlock(&dev_priv->dpio_lock);
>  
> -	return vco_freq[hpll_freq];
> +	return vco_freq[hpll_freq] * 1000;
>  }
>  
>  /* Adjust CDclk dividers to allow high res or save power if possible */
> @@ -4451,9 +4452,9 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
>  	WARN_ON(valleyview_cur_cdclk(dev_priv) != dev_priv->vlv_cdclk_freq);
>  	dev_priv->vlv_cdclk_freq = cdclk;
>  
> -	if (cdclk >= 320) /* jump to highest voltage for 400MHz too */
> +	if (cdclk >= 320000) /* jump to highest voltage for 400MHz too */
>  		cmd = 2;
> -	else if (cdclk == 266)
> +	else if (cdclk == 266667)
>  		cmd = 1;
>  	else
>  		cmd = 0;
> @@ -4470,11 +4471,11 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
>  	}
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
> -	if (cdclk == 400) {
> +	if (cdclk == 400000) {
>  		u32 divider, vco;
>  
>  		vco = valleyview_get_vco(dev_priv);
> -		divider = ((vco << 1) / cdclk) - 1;
> +		divider = DIV_ROUND_CLOSEST(vco << 1, cdclk) - 1;
>  
>  		mutex_lock(&dev_priv->dpio_lock);
>  		/* adjust cdclk divider */
> @@ -4494,7 +4495,7 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
>  	 * For high bandwidth configs, we set a higher latency in the bunit
>  	 * so that the core display fetch happens in time to avoid underruns.
>  	 */
> -	if (cdclk == 400)
> +	if (cdclk == 400000)
>  		val |= 4500 / 250; /* 4.5 usec */
>  	else
>  		val |= 3000 / 250; /* 3.0 usec */
> @@ -4518,7 +4519,7 @@ int valleyview_cur_cdclk(struct drm_i915_private *dev_priv)
>  
>  	divider &= 0xf;
>  
> -	cur_cdclk = (vco << 1) / (divider + 1);
> +	cur_cdclk = DIV_ROUND_CLOSEST(vco << 1, divider + 1);
>  
>  	return cur_cdclk;
>  }
> @@ -4535,12 +4536,12 @@ static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
>  	 * So we check to see whether we're above 90% of the lower bin and
>  	 * adjust if needed.
>  	 */
> -	if (max_pixclk > 288000) {
> -		return 400;
> -	} else if (max_pixclk > 240000) {
> -		return 320;
> -	} else
> -		return 266;
> +	if (max_pixclk > 320000*9/10)
> +		return 400000;
> +	else if (max_pixclk > 266667*9/10)
> +		return 320000;
> +	else
> +		return 266667;
>  	/* Looks like the 200MHz CDclk freq doesn't work on some configs */
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index d33b61d..9ce4f09 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -86,7 +86,7 @@ static void gmbus_set_freq(struct drm_i915_private *dev_priv)
>  
>  	BUG_ON(!IS_VALLEYVIEW(dev_priv->dev));
>  
> -	vco = valleyview_get_vco(dev_priv);
> +	vco = valleyview_get_vco(dev_priv) / 1000;
>  
>  	/* Get the CDCLK divide ratio */
>  	cdclk_div = get_disp_clk_div(dev_priv, CDCLK);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5024bc8..a9ddf74 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5536,7 +5536,7 @@ static void valleyview_init_clock_gating(struct drm_device *dev)
>  	DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
>  
>  	dev_priv->vlv_cdclk_freq = valleyview_cur_cdclk(dev_priv);
> -	DRM_DEBUG_DRIVER("Current CD clock rate: %d MHz",
> +	DRM_DEBUG_DRIVER("Current CD clock rate: %d kHz",
>  			 dev_priv->vlv_cdclk_freq);
>  
>  	I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE);

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

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

[PATCH 02/11] drm/i915: Give names to the CCK_DISPLAY_CLOCK_CONTROL bits

[ville.syrjala]

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

Avoid using magic values for CCK frequency bits. Also the mask we were
using for the requested frequency was one bit too short. Fix it up.

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0f4a0ed..2aa9a3c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -584,6 +584,11 @@ enum punit_power_well {
 #define  DSI_PLL_M1_DIV_SHIFT			0
 #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
 #define CCK_DISPLAY_CLOCK_CONTROL		0x6b
+#define  DISPLAY_TRUNK_FORCE_ON			(1 << 17)
+#define  DISPLAY_TRUNK_FORCE_OFF		(1 << 16)
+#define  DISPLAY_FREQUENCY_STATUS		(0x1f << 8)
+#define  DISPLAY_FREQUENCY_STATUS_SHIFT		8
+#define  DISPLAY_FREQUENCY_VALUES		(0x1f << 0)
 
 /**
  * DOC: DPIO
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5f66dc8..36562b5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4480,7 +4480,7 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
 		mutex_lock(&dev_priv->dpio_lock);
 		/* adjust cdclk divider */
 		val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
-		val &= ~0xf;
+		val &= ~DISPLAY_FREQUENCY_VALUES;
 		val |= divider;
 		vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL, val);
 		mutex_unlock(&dev_priv->dpio_lock);
@@ -4517,7 +4517,7 @@ int valleyview_cur_cdclk(struct drm_i915_private *dev_priv)
 	divider = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
 	mutex_unlock(&dev_priv->dpio_lock);
 
-	divider &= 0xf;
+	divider &= DISPLAY_FREQUENCY_VALUES;
 
 	cur_cdclk = DIV_ROUND_CLOSEST(vco << 1, divider + 1);
 
-- 
1.8.5.5

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

Re: [PATCH 02/11] drm/i915: Give names to the CCK_DISPLAY_CLOCK_CONTROL bits

[Jesse Barnes]

On Fri, 13 Jun 2014 13:37:48 +0300
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Avoid using magic values for CCK frequency bits. Also the mask we were
> using for the requested frequency was one bit too short. Fix it up.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 5 +++++
>  drivers/gpu/drm/i915/intel_display.c | 4 ++--
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0f4a0ed..2aa9a3c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -584,6 +584,11 @@ enum punit_power_well {
>  #define  DSI_PLL_M1_DIV_SHIFT			0
>  #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
>  #define CCK_DISPLAY_CLOCK_CONTROL		0x6b
> +#define  DISPLAY_TRUNK_FORCE_ON			(1 << 17)
> +#define  DISPLAY_TRUNK_FORCE_OFF		(1 << 16)
> +#define  DISPLAY_FREQUENCY_STATUS		(0x1f << 8)
> +#define  DISPLAY_FREQUENCY_STATUS_SHIFT		8
> +#define  DISPLAY_FREQUENCY_VALUES		(0x1f << 0)
>  
>  /**
>   * DOC: DPIO
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5f66dc8..36562b5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4480,7 +4480,7 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
>  		mutex_lock(&dev_priv->dpio_lock);
>  		/* adjust cdclk divider */
>  		val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
> -		val &= ~0xf;
> +		val &= ~DISPLAY_FREQUENCY_VALUES;
>  		val |= divider;
>  		vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL, val);
>  		mutex_unlock(&dev_priv->dpio_lock);
> @@ -4517,7 +4517,7 @@ int valleyview_cur_cdclk(struct drm_i915_private *dev_priv)
>  	divider = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
>  	mutex_unlock(&dev_priv->dpio_lock);
>  
> -	divider &= 0xf;
> +	divider &= DISPLAY_FREQUENCY_VALUES;
>  
>  	cur_cdclk = DIV_ROUND_CLOSEST(vco << 1, divider + 1);
>  

You snuck in a fix for the mask here, but it looks correct.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org

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

[PATCH 03/11] drm/i915: Move vlv cdclk code to .get_display_clock_speed()

[ville.syrjala]

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

We have a standard hook for reading out the current cdclk. Move the VLV
code from valleyview_cur_cdclk() to .get_display_clock_speed().

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 36562b5..61d7ea2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4449,7 +4449,7 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 val, cmd;
 
-	WARN_ON(valleyview_cur_cdclk(dev_priv) != dev_priv->vlv_cdclk_freq);
+	WARN_ON(dev_priv->display.get_display_clock_speed(dev) != dev_priv->vlv_cdclk_freq);
 	dev_priv->vlv_cdclk_freq = cdclk;
 
 	if (cdclk >= 320000) /* jump to highest voltage for 400MHz too */
@@ -4506,24 +4506,6 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
 	intel_i2c_reset(dev);
 }
 
-int valleyview_cur_cdclk(struct drm_i915_private *dev_priv)
-{
-	int cur_cdclk, vco;
-	int divider;
-
-	vco = valleyview_get_vco(dev_priv);
-
-	mutex_lock(&dev_priv->dpio_lock);
-	divider = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
-	mutex_unlock(&dev_priv->dpio_lock);
-
-	divider &= DISPLAY_FREQUENCY_VALUES;
-
-	cur_cdclk = DIV_ROUND_CLOSEST(vco << 1, divider + 1);
-
-	return cur_cdclk;
-}
-
 static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
 				 int max_pixclk)
 {
@@ -5228,7 +5210,18 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 
 static int valleyview_get_display_clock_speed(struct drm_device *dev)
 {
-	return 400000; /* FIXME */
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int vco = valleyview_get_vco(dev_priv);
+	u32 val;
+	int divider;
+
+	mutex_lock(&dev_priv->dpio_lock);
+	val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	divider = val & DISPLAY_FREQUENCY_VALUES;
+
+	return DIV_ROUND_CLOSEST(vco << 1, divider + 1);
 }
 
 static int i945_get_display_clock_speed(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 78d4124..65ce0bb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -717,7 +717,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 const char *intel_output_name(int output);
 bool intel_has_pending_fb_unpin(struct drm_device *dev);
 int intel_pch_rawclk(struct drm_device *dev);
-int valleyview_cur_cdclk(struct drm_i915_private *dev_priv);
 void intel_mark_busy(struct drm_device *dev);
 void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
 			struct intel_engine_cs *ring);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a9ddf74..67f019c1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5535,7 +5535,7 @@ static void valleyview_init_clock_gating(struct drm_device *dev)
 	}
 	DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
 
-	dev_priv->vlv_cdclk_freq = valleyview_cur_cdclk(dev_priv);
+	dev_priv->vlv_cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
 	DRM_DEBUG_DRIVER("Current CD clock rate: %d kHz",
 			 dev_priv->vlv_cdclk_freq);
 
-- 
1.8.5.5

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

Re: [PATCH 03/11] drm/i915: Move vlv cdclk code to .get_display_clock_speed()

[Jesse Barnes]

On Fri, 13 Jun 2014 13:37:49 +0300
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We have a standard hook for reading out the current cdclk. Move the VLV
> code from valleyview_cur_cdclk() to .get_display_clock_speed().
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 33 +++++++++++++--------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  1 -
>  drivers/gpu/drm/i915/intel_pm.c      |  2 +-
>  3 files changed, 14 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 36562b5..61d7ea2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4449,7 +4449,7 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 val, cmd;
>  
> -	WARN_ON(valleyview_cur_cdclk(dev_priv) != dev_priv->vlv_cdclk_freq);
> +	WARN_ON(dev_priv->display.get_display_clock_speed(dev) != dev_priv->vlv_cdclk_freq);
>  	dev_priv->vlv_cdclk_freq = cdclk;
>  
>  	if (cdclk >= 320000) /* jump to highest voltage for 400MHz too */
> @@ -4506,24 +4506,6 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
>  	intel_i2c_reset(dev);
>  }
>  
> -int valleyview_cur_cdclk(struct drm_i915_private *dev_priv)
> -{
> -	int cur_cdclk, vco;
> -	int divider;
> -
> -	vco = valleyview_get_vco(dev_priv);
> -
> -	mutex_lock(&dev_priv->dpio_lock);
> -	divider = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
> -	mutex_unlock(&dev_priv->dpio_lock);
> -
> -	divider &= DISPLAY_FREQUENCY_VALUES;
> -
> -	cur_cdclk = DIV_ROUND_CLOSEST(vco << 1, divider + 1);
> -
> -	return cur_cdclk;
> -}
> -
>  static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
>  				 int max_pixclk)
>  {
> @@ -5228,7 +5210,18 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  
>  static int valleyview_get_display_clock_speed(struct drm_device *dev)
>  {
> -	return 400000; /* FIXME */
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int vco = valleyview_get_vco(dev_priv);
> +	u32 val;
> +	int divider;
> +
> +	mutex_lock(&dev_priv->dpio_lock);
> +	val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
> +	mutex_unlock(&dev_priv->dpio_lock);
> +
> +	divider = val & DISPLAY_FREQUENCY_VALUES;
> +
> +	return DIV_ROUND_CLOSEST(vco << 1, divider + 1);
>  }
>  
>  static int i945_get_display_clock_speed(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 78d4124..65ce0bb 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -717,7 +717,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  const char *intel_output_name(int output);
>  bool intel_has_pending_fb_unpin(struct drm_device *dev);
>  int intel_pch_rawclk(struct drm_device *dev);
> -int valleyview_cur_cdclk(struct drm_i915_private *dev_priv);
>  void intel_mark_busy(struct drm_device *dev);
>  void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
>  			struct intel_engine_cs *ring);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a9ddf74..67f019c1 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5535,7 +5535,7 @@ static void valleyview_init_clock_gating(struct drm_device *dev)
>  	}
>  	DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
>  
> -	dev_priv->vlv_cdclk_freq = valleyview_cur_cdclk(dev_priv);
> +	dev_priv->vlv_cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
>  	DRM_DEBUG_DRIVER("Current CD clock rate: %d kHz",
>  			 dev_priv->vlv_cdclk_freq);
>  

Looks like we only really use that on < gen4 but so it seems harmless.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

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

Re: [PATCH 03/11] drm/i915: Move vlv cdclk code to .get_display_clock_speed()

[Daniel Vetter]

On Wed, Jun 25, 2014 at 11:47:50AM -0700, Jesse Barnes wrote:
> On Fri, 13 Jun 2014 13:37:49 +0300
> ville.syrjala@linux.intel.com wrote:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We have a standard hook for reading out the current cdclk. Move the VLV
> > code from valleyview_cur_cdclk() to .get_display_clock_speed().
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 33 +++++++++++++--------------------
> >  drivers/gpu/drm/i915/intel_drv.h     |  1 -
> >  drivers/gpu/drm/i915/intel_pm.c      |  2 +-
> >  3 files changed, 14 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 36562b5..61d7ea2 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4449,7 +4449,7 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	u32 val, cmd;
> >  
> > -	WARN_ON(valleyview_cur_cdclk(dev_priv) != dev_priv->vlv_cdclk_freq);
> > +	WARN_ON(dev_priv->display.get_display_clock_speed(dev) != dev_priv->vlv_cdclk_freq);
> >  	dev_priv->vlv_cdclk_freq = cdclk;
> >  
> >  	if (cdclk >= 320000) /* jump to highest voltage for 400MHz too */
> > @@ -4506,24 +4506,6 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
> >  	intel_i2c_reset(dev);
> >  }
> >  
> > -int valleyview_cur_cdclk(struct drm_i915_private *dev_priv)
> > -{
> > -	int cur_cdclk, vco;
> > -	int divider;
> > -
> > -	vco = valleyview_get_vco(dev_priv);
> > -
> > -	mutex_lock(&dev_priv->dpio_lock);
> > -	divider = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
> > -	mutex_unlock(&dev_priv->dpio_lock);
> > -
> > -	divider &= DISPLAY_FREQUENCY_VALUES;
> > -
> > -	cur_cdclk = DIV_ROUND_CLOSEST(vco << 1, divider + 1);
> > -
> > -	return cur_cdclk;
> > -}
> > -
> >  static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
> >  				 int max_pixclk)
> >  {
> > @@ -5228,7 +5210,18 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> >  
> >  static int valleyview_get_display_clock_speed(struct drm_device *dev)
> >  {
> > -	return 400000; /* FIXME */
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	int vco = valleyview_get_vco(dev_priv);
> > +	u32 val;
> > +	int divider;
> > +
> > +	mutex_lock(&dev_priv->dpio_lock);
> > +	val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
> > +	mutex_unlock(&dev_priv->dpio_lock);
> > +
> > +	divider = val & DISPLAY_FREQUENCY_VALUES;
> > +
> > +	return DIV_ROUND_CLOSEST(vco << 1, divider + 1);
> >  }
> >  
> >  static int i945_get_display_clock_speed(struct drm_device *dev)
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 78d4124..65ce0bb 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -717,7 +717,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >  const char *intel_output_name(int output);
> >  bool intel_has_pending_fb_unpin(struct drm_device *dev);
> >  int intel_pch_rawclk(struct drm_device *dev);
> > -int valleyview_cur_cdclk(struct drm_i915_private *dev_priv);
> >  void intel_mark_busy(struct drm_device *dev);
> >  void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
> >  			struct intel_engine_cs *ring);
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index a9ddf74..67f019c1 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5535,7 +5535,7 @@ static void valleyview_init_clock_gating(struct drm_device *dev)
> >  	}
> >  	DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
> >  
> > -	dev_priv->vlv_cdclk_freq = valleyview_cur_cdclk(dev_priv);
> > +	dev_priv->vlv_cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
> >  	DRM_DEBUG_DRIVER("Current CD clock rate: %d kHz",
> >  			 dev_priv->vlv_cdclk_freq);
> >  
> 
> Looks like we only really use that on < gen4 but so it seems harmless.

We have the same (or at least very similar) "can we dot this dotclock with
the given display clock, if not boost" kind of logic so imo fits into the
gen3 heritage of the vlv display block ...
-Daniel

> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> -- 
> Jesse Barnes, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 04/11] drm/i915: Handle 320 vs. 333 MHz cdclk on vlv

[ville.syrjala]

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

Depending on the HPLL frequency one of the supported cdclk frquencies is
either 320MHz or 333MHz. Figure out which one it is to accurately pick
the minimal required cdclk. This would also avoid a warning from the
cdclk code where it compares the actual cdclk read out from the hardware
with a value that was calculated using valleyview_calc_cdclk().

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 61d7ea2..1f3985f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4509,19 +4509,22 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
 static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
 				 int max_pixclk)
 {
+	int vco = valleyview_get_vco(dev_priv);
+	int freq_320 = (vco <<  1) % 320000 != 0 ? 333333 : 320000;
+
 	/*
 	 * Really only a few cases to deal with, as only 4 CDclks are supported:
 	 *   200MHz
 	 *   267MHz
-	 *   320MHz
+	 *   320/333MHz (depends on HPLL freq)
 	 *   400MHz
 	 * So we check to see whether we're above 90% of the lower bin and
 	 * adjust if needed.
 	 */
-	if (max_pixclk > 320000*9/10)
+	if (max_pixclk > freq_320*9/10)
 		return 400000;
 	else if (max_pixclk > 266667*9/10)
-		return 320000;
+		return freq_320;
 	else
 		return 266667;
 	/* Looks like the 200MHz CDclk freq doesn't work on some configs */
-- 
1.8.5.5

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

Re: [PATCH 04/11] drm/i915: Handle 320 vs. 333 MHz cdclk on vlv

[Jesse Barnes]

On Fri, 13 Jun 2014 13:37:50 +0300
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Depending on the HPLL frequency one of the supported cdclk frquencies is
> either 320MHz or 333MHz. Figure out which one it is to accurately pick
> the minimal required cdclk. This would also avoid a warning from the
> cdclk code where it compares the actual cdclk read out from the hardware
> with a value that was calculated using valleyview_calc_cdclk().
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 61d7ea2..1f3985f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4509,19 +4509,22 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
>  static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
>  				 int max_pixclk)
>  {
> +	int vco = valleyview_get_vco(dev_priv);
> +	int freq_320 = (vco <<  1) % 320000 != 0 ? 333333 : 320000;
> +
>  	/*
>  	 * Really only a few cases to deal with, as only 4 CDclks are supported:
>  	 *   200MHz
>  	 *   267MHz
> -	 *   320MHz
> +	 *   320/333MHz (depends on HPLL freq)
>  	 *   400MHz
>  	 * So we check to see whether we're above 90% of the lower bin and
>  	 * adjust if needed.
>  	 */
> -	if (max_pixclk > 320000*9/10)
> +	if (max_pixclk > freq_320*9/10)
>  		return 400000;
>  	else if (max_pixclk > 266667*9/10)
> -		return 320000;
> +		return freq_320;
>  	else
>  		return 266667;
>  	/* Looks like the 200MHz CDclk freq doesn't work on some configs */

Looks good.  This doc searching has been fun.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

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

[PATCH 05/11] drm/i915: Use 200MHz cdclk on vlv when all pipes are off

[ville.syrjala]

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

Drop the cdclk frequency to 200MHz on vlv when all pipes are off. In
theory we should be able to use 200MHz also when the pixel clock is at
most 90% of 200MHz. However in practice all we seem to get is a solid
color picture or an otherwise corrupted display.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1f3985f..3a9b017 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4520,14 +4520,19 @@ static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
 	 *   400MHz
 	 * So we check to see whether we're above 90% of the lower bin and
 	 * adjust if needed.
+	 *
+	 * We seem to get an unstable or solid color picture at 200MHz.
+	 * Not sure what's wrong. For now use 200MHz only when all pipes
+	 * are off.
 	 */
 	if (max_pixclk > freq_320*9/10)
 		return 400000;
 	else if (max_pixclk > 266667*9/10)
 		return freq_320;
-	else
+	else if (max_pixclk > 0)
 		return 266667;
-	/* Looks like the 200MHz CDclk freq doesn't work on some configs */
+	else
+		return 200000;
 }
 
 /* compute the max pixel clock for new configuration */
-- 
1.8.5.5

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

Re: [PATCH 05/11] drm/i915: Use 200MHz cdclk on vlv when all pipes are off

[Jesse Barnes]

On Fri, 13 Jun 2014 13:37:51 +0300
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Drop the cdclk frequency to 200MHz on vlv when all pipes are off. In
> theory we should be able to use 200MHz also when the pixel clock is at
> most 90% of 200MHz. However in practice all we seem to get is a solid
> color picture or an otherwise corrupted display.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1f3985f..3a9b017 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4520,14 +4520,19 @@ static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
>  	 *   400MHz
>  	 * So we check to see whether we're above 90% of the lower bin and
>  	 * adjust if needed.
> +	 *
> +	 * We seem to get an unstable or solid color picture at 200MHz.
> +	 * Not sure what's wrong. For now use 200MHz only when all pipes
> +	 * are off.
>  	 */
>  	if (max_pixclk > freq_320*9/10)
>  		return 400000;
>  	else if (max_pixclk > 266667*9/10)
>  		return freq_320;
> -	else
> +	else if (max_pixclk > 0)
>  		return 266667;
> -	/* Looks like the 200MHz CDclk freq doesn't work on some configs */
> +	else
> +		return 200000;
>  }
>  
>  /* compute the max pixel clock for new configuration */

I guess this is safe, but optional (won't we be shutting off the clocks
anyway?).

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

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

Re: [PATCH 05/11] drm/i915: Use 200MHz cdclk on vlv when all pipes are off

[Ville Syrjälä]

On Wed, Jun 25, 2014 at 11:54:06AM -0700, Jesse Barnes wrote:
> On Fri, 13 Jun 2014 13:37:51 +0300
> ville.syrjala@linux.intel.com wrote:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Drop the cdclk frequency to 200MHz on vlv when all pipes are off. In
> > theory we should be able to use 200MHz also when the pixel clock is at
> > most 90% of 200MHz. However in practice all we seem to get is a solid
> > color picture or an otherwise corrupted display.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 1f3985f..3a9b017 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4520,14 +4520,19 @@ static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
> >  	 *   400MHz
> >  	 * So we check to see whether we're above 90% of the lower bin and
> >  	 * adjust if needed.
> > +	 *
> > +	 * We seem to get an unstable or solid color picture at 200MHz.
> > +	 * Not sure what's wrong. For now use 200MHz only when all pipes
> > +	 * are off.
> >  	 */
> >  	if (max_pixclk > freq_320*9/10)
> >  		return 400000;
> >  	else if (max_pixclk > 266667*9/10)
> >  		return freq_320;
> > -	else
> > +	else if (max_pixclk > 0)
> >  		return 266667;
> > -	/* Looks like the 200MHz CDclk freq doesn't work on some configs */
> > +	else
> > +		return 200000;
> >  }
> >  
> >  /* compute the max pixel clock for new configuration */
> 
> I guess this is safe, but optional (won't we be shutting off the clocks
> anyway?).

Ideally yes. But currently I'm not sure if that happens.

> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> -- 
> Jesse Barnes, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC

[PATCH 06/11] drm/i915: Wait for cdclk change to occure when going for 400MHz

[ville.syrjala]

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

VLV Punit doesn't support the 400MHz cdclk option, so we bypass the
Punit and poke at CCK directly. However we forgot to wait for the
frequeency change to complete. Poll the CCK clock status to make sure
the clock has changed before we fire up any pipes.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3a9b017..29dddec 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4483,6 +4483,11 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
 		val &= ~DISPLAY_FREQUENCY_VALUES;
 		val |= divider;
 		vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL, val);
+
+		if (wait_for((vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL) &
+			      DISPLAY_FREQUENCY_STATUS) == (divider << DISPLAY_FREQUENCY_STATUS_SHIFT),
+			     50))
+			DRM_ERROR("timed out waiting for CDclk change\n");
 		mutex_unlock(&dev_priv->dpio_lock);
 	}
 
-- 
1.8.5.5

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

Re: [PATCH 06/11] drm/i915: Wait for cdclk change to occure when going for 400MHz

[Jesse Barnes]

On Fri, 13 Jun 2014 13:37:52 +0300
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> VLV Punit doesn't support the 400MHz cdclk option, so we bypass the
> Punit and poke at CCK directly. However we forgot to wait for the
> frequeency change to complete. Poll the CCK clock status to make sure
> the clock has changed before we fire up any pipes.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3a9b017..29dddec 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4483,6 +4483,11 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
>  		val &= ~DISPLAY_FREQUENCY_VALUES;
>  		val |= divider;
>  		vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL, val);
> +
> +		if (wait_for((vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL) &
> +			      DISPLAY_FREQUENCY_STATUS) == (divider << DISPLAY_FREQUENCY_STATUS_SHIFT),
> +			     50))
> +			DRM_ERROR("timed out waiting for CDclk change\n");
>  		mutex_unlock(&dev_priv->dpio_lock);
>  	}
>  

Seems reasonable, assuming this actually works in testing.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

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

[PATCH 07/11] drm/i915: Warn if there's a cdclk change in progess

[ville.syrjala]

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

If someone is interested in the current cdclk frquency it should
be stable and not in process of changing frquency. Warn if the current
and requested cdclk don't match in .get_display_clock_spee() on vlv.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 29dddec..601e97e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5234,6 +5234,10 @@ static int valleyview_get_display_clock_speed(struct drm_device *dev)
 
 	divider = val & DISPLAY_FREQUENCY_VALUES;
 
+	WARN((val & DISPLAY_FREQUENCY_STATUS) !=
+	     (divider << DISPLAY_FREQUENCY_STATUS_SHIFT),
+	     "cdclk change in progress\n");
+
 	return DIV_ROUND_CLOSEST(vco << 1, divider + 1);
 }
 
-- 
1.8.5.5

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

Re: [PATCH 07/11] drm/i915: Warn if there's a cdclk change in progess

[Jesse Barnes]

On Fri, 13 Jun 2014 13:37:53 +0300
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> If someone is interested in the current cdclk frquency it should
> be stable and not in process of changing frquency. Warn if the current
> and requested cdclk don't match in .get_display_clock_spee() on vlv.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 29dddec..601e97e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5234,6 +5234,10 @@ static int valleyview_get_display_clock_speed(struct drm_device *dev)
>  
>  	divider = val & DISPLAY_FREQUENCY_VALUES;
>  
> +	WARN((val & DISPLAY_FREQUENCY_STATUS) !=
> +	     (divider << DISPLAY_FREQUENCY_STATUS_SHIFT),
> +	     "cdclk change in progress\n");
> +
>  	return DIV_ROUND_CLOSEST(vco << 1, divider + 1);
>  }
>  

Hm, there's not much we can do in this case, so rather than warn maybe
we should try a wait instead, and only warn if it times out?  Even then
there's not much we can do aside from poking the PUnit folks.

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

Re: [PATCH 07/11] drm/i915: Warn if there's a cdclk change in progess

[Ville Syrjälä]

On Wed, Jun 25, 2014 at 11:55:58AM -0700, Jesse Barnes wrote:
> On Fri, 13 Jun 2014 13:37:53 +0300
> ville.syrjala@linux.intel.com wrote:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > If someone is interested in the current cdclk frquency it should
> > be stable and not in process of changing frquency. Warn if the current
> > and requested cdclk don't match in .get_display_clock_spee() on vlv.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 29dddec..601e97e 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5234,6 +5234,10 @@ static int valleyview_get_display_clock_speed(struct drm_device *dev)
> >  
> >  	divider = val & DISPLAY_FREQUENCY_VALUES;
> >  
> > +	WARN((val & DISPLAY_FREQUENCY_STATUS) !=
> > +	     (divider << DISPLAY_FREQUENCY_STATUS_SHIFT),
> > +	     "cdclk change in progress\n");
> > +
> >  	return DIV_ROUND_CLOSEST(vco << 1, divider + 1);
> >  }
> >  
> 
> Hm, there's not much we can do in this case, so rather than warn maybe
> we should try a wait instead, and only warn if it times out?  Even then
> there's not much we can do aside from poking the PUnit folks.

This shouldn't happen unless we somehow messed up and triggered a cdclk
change and didn't wait for it to complete, which would be a driver bug.
So I think a simple WARN seems sufficient.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 07/11] drm/i915: Warn if there's a cdclk change in progess

[Daniel Vetter]

On Wed, Jun 25, 2014 at 10:34:48PM +0300, Ville Syrjälä wrote:
> On Wed, Jun 25, 2014 at 11:55:58AM -0700, Jesse Barnes wrote:
> > On Fri, 13 Jun 2014 13:37:53 +0300
> > ville.syrjala@linux.intel.com wrote:
> > 
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > If someone is interested in the current cdclk frquency it should
> > > be stable and not in process of changing frquency. Warn if the current
> > > and requested cdclk don't match in .get_display_clock_spee() on vlv.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 29dddec..601e97e 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -5234,6 +5234,10 @@ static int valleyview_get_display_clock_speed(struct drm_device *dev)
> > >  
> > >  	divider = val & DISPLAY_FREQUENCY_VALUES;
> > >  
> > > +	WARN((val & DISPLAY_FREQUENCY_STATUS) !=
> > > +	     (divider << DISPLAY_FREQUENCY_STATUS_SHIFT),
> > > +	     "cdclk change in progress\n");
> > > +
> > >  	return DIV_ROUND_CLOSEST(vco << 1, divider + 1);
> > >  }
> > >  
> > 
> > Hm, there's not much we can do in this case, so rather than warn maybe
> > we should try a wait instead, and only warn if it times out?  Even then
> > there's not much we can do aside from poking the PUnit folks.
> 
> This shouldn't happen unless we somehow messed up and triggered a cdclk
> change and didn't wait for it to complete, which would be a driver bug.
> So I think a simple WARN seems sufficient.

I concur with Ville here so merged the patch. If we hit this we can figure
out where exactly we've been wrong and whether there's a legitimate case
where we need to wait for cdclk to settle.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 08/11] drm/i915: Kill duplicated cdclk readout code from i2c

[ville.syrjala]

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

We have a slightly different way of readoing out the cdclk in
gmbus_set_freq(). Kill that and just call .get_display_clock_speed().

Also need to remove the GMBUSFREQ update from intel_i2c_reset() since
that gets called way too early. Let's do it in intel_modeset_init_hw()
instead, and also pull the initial vlv_cdclk_freq update there from
init_clock gating.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 25 ++++++++++++++---
 drivers/gpu/drm/i915/intel_drv.h     |  1 -
 drivers/gpu/drm/i915/intel_i2c.c     | 54 ------------------------------------
 drivers/gpu/drm/i915/intel_pm.c      |  4 ---
 4 files changed, 21 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 601e97e..33cc213 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4430,7 +4430,7 @@ static void modeset_update_crtc_power_domains(struct drm_device *dev)
 }
 
 /* returns HPLL frequency in kHz */
-int valleyview_get_vco(struct drm_i915_private *dev_priv)
+static int valleyview_get_vco(struct drm_i915_private *dev_priv)
 {
 	int hpll_freq, vco_freq[] = { 800, 1600, 2000, 2400 };
 
@@ -4443,6 +4443,22 @@ 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)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	dev_priv->vlv_cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
+	DRM_DEBUG_DRIVER("Current CD clock rate: %d kHz",
+			 dev_priv->vlv_cdclk_freq);
+
+	/*
+	 * 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, dev_priv->vlv_cdclk_freq);
+}
+
 /* Adjust CDclk dividers to allow high res or save power if possible */
 static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
 {
@@ -4450,7 +4466,6 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
 	u32 val, cmd;
 
 	WARN_ON(dev_priv->display.get_display_clock_speed(dev) != dev_priv->vlv_cdclk_freq);
-	dev_priv->vlv_cdclk_freq = cdclk;
 
 	if (cdclk >= 320000) /* jump to highest voltage for 400MHz too */
 		cmd = 2;
@@ -4507,8 +4522,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);
 
-	/* Since we changed the CDclk, we need to update the GMBUSFREQ too */
-	intel_i2c_reset(dev);
+	vlv_update_cdclk(dev);
 }
 
 static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
@@ -11974,6 +11988,9 @@ void intel_modeset_init_hw(struct drm_device *dev)
 {
 	intel_prepare_ddi(dev);
 
+	if (IS_VALLEYVIEW(dev))
+		vlv_update_cdclk(dev);
+
 	intel_init_clock_gating(dev);
 
 	intel_reset_dpio(dev);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 65ce0bb..5740be0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -800,7 +800,6 @@ void hsw_disable_ips(struct intel_crtc *crtc);
 void intel_display_set_init_power(struct drm_i915_private *dev, bool enable);
 enum intel_display_power_domain
 intel_display_port_power_domain(struct intel_encoder *intel_encoder);
-int valleyview_get_vco(struct drm_i915_private *dev_priv);
 void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 				 struct intel_crtc_config *pipe_config);
 int intel_format_to_fourcc(int format);
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index 9ce4f09..b31088a 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -34,11 +34,6 @@
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
 
-enum disp_clk {
-	CDCLK,
-	CZCLK
-};
-
 struct gmbus_port {
 	const char *name;
 	int reg;
@@ -63,60 +58,11 @@ to_intel_gmbus(struct i2c_adapter *i2c)
 	return container_of(i2c, struct intel_gmbus, adapter);
 }
 
-static int get_disp_clk_div(struct drm_i915_private *dev_priv,
-			    enum disp_clk clk)
-{
-	u32 reg_val;
-	int clk_ratio;
-
-	reg_val = I915_READ(CZCLK_CDCLK_FREQ_RATIO);
-
-	if (clk == CDCLK)
-		clk_ratio =
-			((reg_val & CDCLK_FREQ_MASK) >> CDCLK_FREQ_SHIFT) + 1;
-	else
-		clk_ratio = (reg_val & CZCLK_FREQ_MASK) + 1;
-
-	return clk_ratio;
-}
-
-static void gmbus_set_freq(struct drm_i915_private *dev_priv)
-{
-	int vco, gmbus_freq = 0, cdclk_div;
-
-	BUG_ON(!IS_VALLEYVIEW(dev_priv->dev));
-
-	vco = valleyview_get_vco(dev_priv) / 1000;
-
-	/* Get the CDCLK divide ratio */
-	cdclk_div = get_disp_clk_div(dev_priv, CDCLK);
-
-	/*
-	 * 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.
-	 */
-	if (cdclk_div)
-		gmbus_freq = (vco << 1) / cdclk_div;
-
-	if (WARN_ON(gmbus_freq == 0))
-		return;
-
-	I915_WRITE(GMBUSFREQ_VLV, gmbus_freq);
-}
-
 void
 intel_i2c_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	/*
-	 * In BIOS-less system, program the correct gmbus frequency
-	 * before reading edid.
-	 */
-	if (IS_VALLEYVIEW(dev))
-		gmbus_set_freq(dev_priv);
-
 	I915_WRITE(dev_priv->gpio_mmio_base + GMBUS0, 0);
 	I915_WRITE(dev_priv->gpio_mmio_base + GMBUS4, 0);
 }
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 67f019c1..9d7b082 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5535,10 +5535,6 @@ static void valleyview_init_clock_gating(struct drm_device *dev)
 	}
 	DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
 
-	dev_priv->vlv_cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
-	DRM_DEBUG_DRIVER("Current CD clock rate: %d kHz",
-			 dev_priv->vlv_cdclk_freq);
-
 	I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE);
 
 	/* WaDisableEarlyCull:vlv */
-- 
1.8.5.5

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

Re: [PATCH 08/11] drm/i915: Kill duplicated cdclk readout code from i2c

[Jesse Barnes]

On Fri, 13 Jun 2014 13:37:54 +0300
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We have a slightly different way of readoing out the cdclk in
> gmbus_set_freq(). Kill that and just call .get_display_clock_speed().
> 
> Also need to remove the GMBUSFREQ update from intel_i2c_reset() since
> that gets called way too early. Let's do it in intel_modeset_init_hw()
> instead, and also pull the initial vlv_cdclk_freq update there from
> init_clock gating.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 25 ++++++++++++++---
>  drivers/gpu/drm/i915/intel_drv.h     |  1 -
>  drivers/gpu/drm/i915/intel_i2c.c     | 54 ------------------------------------
>  drivers/gpu/drm/i915/intel_pm.c      |  4 ---
>  4 files changed, 21 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 601e97e..33cc213 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4430,7 +4430,7 @@ static void modeset_update_crtc_power_domains(struct drm_device *dev)
>  }
>  
>  /* returns HPLL frequency in kHz */
> -int valleyview_get_vco(struct drm_i915_private *dev_priv)
> +static int valleyview_get_vco(struct drm_i915_private *dev_priv)
>  {
>  	int hpll_freq, vco_freq[] = { 800, 1600, 2000, 2400 };
>  
> @@ -4443,6 +4443,22 @@ 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)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	dev_priv->vlv_cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
> +	DRM_DEBUG_DRIVER("Current CD clock rate: %d kHz",
> +			 dev_priv->vlv_cdclk_freq);
> +
> +	/*
> +	 * 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, dev_priv->vlv_cdclk_freq);
> +}
> +
>  /* Adjust CDclk dividers to allow high res or save power if possible */
>  static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
>  {
> @@ -4450,7 +4466,6 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
>  	u32 val, cmd;
>  
>  	WARN_ON(dev_priv->display.get_display_clock_speed(dev) != dev_priv->vlv_cdclk_freq);
> -	dev_priv->vlv_cdclk_freq = cdclk;
>  
>  	if (cdclk >= 320000) /* jump to highest voltage for 400MHz too */
>  		cmd = 2;
> @@ -4507,8 +4522,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);
>  
> -	/* Since we changed the CDclk, we need to update the GMBUSFREQ too */
> -	intel_i2c_reset(dev);
> +	vlv_update_cdclk(dev);
>  }
>  
>  static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
> @@ -11974,6 +11988,9 @@ void intel_modeset_init_hw(struct drm_device *dev)
>  {
>  	intel_prepare_ddi(dev);
>  
> +	if (IS_VALLEYVIEW(dev))
> +		vlv_update_cdclk(dev);
> +
>  	intel_init_clock_gating(dev);
>  
>  	intel_reset_dpio(dev);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 65ce0bb..5740be0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -800,7 +800,6 @@ void hsw_disable_ips(struct intel_crtc *crtc);
>  void intel_display_set_init_power(struct drm_i915_private *dev, bool enable);
>  enum intel_display_power_domain
>  intel_display_port_power_domain(struct intel_encoder *intel_encoder);
> -int valleyview_get_vco(struct drm_i915_private *dev_priv);
>  void intel_mode_from_pipe_config(struct drm_display_mode *mode,
>  				 struct intel_crtc_config *pipe_config);
>  int intel_format_to_fourcc(int format);
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index 9ce4f09..b31088a 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -34,11 +34,6 @@
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
>  
> -enum disp_clk {
> -	CDCLK,
> -	CZCLK
> -};
> -
>  struct gmbus_port {
>  	const char *name;
>  	int reg;
> @@ -63,60 +58,11 @@ to_intel_gmbus(struct i2c_adapter *i2c)
>  	return container_of(i2c, struct intel_gmbus, adapter);
>  }
>  
> -static int get_disp_clk_div(struct drm_i915_private *dev_priv,
> -			    enum disp_clk clk)
> -{
> -	u32 reg_val;
> -	int clk_ratio;
> -
> -	reg_val = I915_READ(CZCLK_CDCLK_FREQ_RATIO);
> -
> -	if (clk == CDCLK)
> -		clk_ratio =
> -			((reg_val & CDCLK_FREQ_MASK) >> CDCLK_FREQ_SHIFT) + 1;
> -	else
> -		clk_ratio = (reg_val & CZCLK_FREQ_MASK) + 1;
> -
> -	return clk_ratio;
> -}
> -
> -static void gmbus_set_freq(struct drm_i915_private *dev_priv)
> -{
> -	int vco, gmbus_freq = 0, cdclk_div;
> -
> -	BUG_ON(!IS_VALLEYVIEW(dev_priv->dev));
> -
> -	vco = valleyview_get_vco(dev_priv) / 1000;
> -
> -	/* Get the CDCLK divide ratio */
> -	cdclk_div = get_disp_clk_div(dev_priv, CDCLK);
> -
> -	/*
> -	 * 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.
> -	 */
> -	if (cdclk_div)
> -		gmbus_freq = (vco << 1) / cdclk_div;
> -
> -	if (WARN_ON(gmbus_freq == 0))
> -		return;
> -
> -	I915_WRITE(GMBUSFREQ_VLV, gmbus_freq);
> -}
> -
>  void
>  intel_i2c_reset(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	/*
> -	 * In BIOS-less system, program the correct gmbus frequency
> -	 * before reading edid.
> -	 */
> -	if (IS_VALLEYVIEW(dev))
> -		gmbus_set_freq(dev_priv);
> -
>  	I915_WRITE(dev_priv->gpio_mmio_base + GMBUS0, 0);
>  	I915_WRITE(dev_priv->gpio_mmio_base + GMBUS4, 0);
>  }
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 67f019c1..9d7b082 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5535,10 +5535,6 @@ static void valleyview_init_clock_gating(struct drm_device *dev)
>  	}
>  	DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
>  
> -	dev_priv->vlv_cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
> -	DRM_DEBUG_DRIVER("Current CD clock rate: %d kHz",
> -			 dev_priv->vlv_cdclk_freq);
> -
>  	I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE);
>  
>  	/* WaDisableEarlyCull:vlv */

Nice diffstat.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

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

[PATCH 09/11] drm/i915: Pull the cmnlane tricks into its own power well ops

[ville.syrjala]

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

Remove the clutter in __vlv_set_power_well() by moving the cmnlane
handling into custom enable/disable hooks for the cmnlane.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9d7b082..e9a8565 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5965,31 +5965,9 @@ static bool i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv,
 void __vlv_set_power_well(struct drm_i915_private *dev_priv,
 			  enum punit_power_well power_well_id, bool enable)
 {
-	struct drm_device *dev = dev_priv->dev;
 	u32 mask;
 	u32 state;
 	u32 ctrl;
-	enum pipe pipe;
-
-	if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC) {
-		if (enable) {
-			/*
-			 * Enable the CRI clock source so we can get at the
-			 * display and the reference clock for VGA
-			 * hotplug / manual detection.
-			 */
-			I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
-				   DPLL_REFA_CLK_ENABLE_VLV |
-				   DPLL_INTEGRATED_CRI_CLK_VLV);
-			udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
-		} else {
-			for_each_pipe(pipe)
-				assert_pll_disabled(dev_priv, pipe);
-			/* Assert common reset */
-			I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) &
-				   ~DPIO_CMNRST);
-		}
-	}
 
 	mask = PUNIT_PWRGT_MASK(power_well_id);
 	state = enable ? PUNIT_PWRGT_PWR_ON(power_well_id) :
@@ -6017,20 +5995,6 @@ void __vlv_set_power_well(struct drm_i915_private *dev_priv,
 
 out:
 	mutex_unlock(&dev_priv->rps.hw_lock);
-
-	/*
-	 * From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx -
-	 *  6.	De-assert cmn_reset/side_reset. Same as VLV X0.
-	 *   a.	GUnit 0x2110 bit[0] set to 1 (def 0)
-	 *   b.	The other bits such as sfr settings / modesel may all
-	 *	be set to 0.
-	 *
-	 * This should only be done on init and resume from S3 with
-	 * both PLLs disabled, or we risk losing DPIO and PLL
-	 * synchronization.
-	 */
-	if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC && enable)
-		I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_CMNRST);
 }
 
 static void vlv_set_power_well(struct drm_i915_private *dev_priv,
@@ -6130,6 +6094,53 @@ static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
 	vlv_set_power_well(dev_priv, power_well, false);
 }
 
+static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
+					   struct i915_power_well *power_well)
+{
+	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DPIO_CMN_BC);
+
+	/*
+	 * Enable the CRI clock source so we can get at the
+	 * display and the reference clock for VGA
+	 * hotplug / manual detection.
+	 */
+	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
+		   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
+	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
+
+	vlv_set_power_well(dev_priv, power_well, true);
+
+	/*
+	 * From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx -
+	 *  6.	De-assert cmn_reset/side_reset. Same as VLV X0.
+	 *   a.	GUnit 0x2110 bit[0] set to 1 (def 0)
+	 *   b.	The other bits such as sfr settings / modesel may all
+	 *	be set to 0.
+	 *
+	 * This should only be done on init and resume from S3 with
+	 * both PLLs disabled, or we risk losing DPIO and PLL
+	 * synchronization.
+	 */
+	I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_CMNRST);
+}
+
+static void vlv_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv,
+					    struct i915_power_well *power_well)
+{
+	struct drm_device *dev = dev_priv->dev;
+	enum pipe pipe;
+
+	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DPIO_CMN_BC);
+
+	for_each_pipe(pipe)
+		assert_pll_disabled(dev_priv, pipe);
+
+	/* Assert common reset */
+	I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) & ~DPIO_CMNRST);
+
+	vlv_set_power_well(dev_priv, power_well, false);
+}
+
 static void check_power_well_state(struct drm_i915_private *dev_priv,
 				   struct i915_power_well *power_well)
 {
@@ -6353,6 +6364,13 @@ static const struct i915_power_well_ops vlv_display_power_well_ops = {
 	.is_enabled = vlv_power_well_enabled,
 };
 
+static const struct i915_power_well_ops vlv_dpio_cmn_power_well_ops = {
+	.sync_hw = vlv_power_well_sync_hw,
+	.enable = vlv_dpio_cmn_power_well_enable,
+	.disable = vlv_dpio_cmn_power_well_disable,
+	.is_enabled = vlv_power_well_enabled,
+};
+
 static const struct i915_power_well_ops vlv_dpio_power_well_ops = {
 	.sync_hw = vlv_power_well_sync_hw,
 	.enable = vlv_power_well_enable,
@@ -6413,7 +6431,7 @@ static struct i915_power_well vlv_power_wells[] = {
 		.name = "dpio-common",
 		.domains = VLV_DPIO_CMN_BC_POWER_DOMAINS,
 		.data = PUNIT_POWER_WELL_DPIO_CMN_BC,
-		.ops = &vlv_dpio_power_well_ops,
+		.ops = &vlv_dpio_cmn_power_well_ops,
 	},
 };
 
-- 
1.8.5.5

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

Re: [PATCH 09/11] drm/i915: Pull the cmnlane tricks into its own power well ops

[Jesse Barnes]

On Fri, 13 Jun 2014 13:37:55 +0300
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Remove the clutter in __vlv_set_power_well() by moving the cmnlane
> handling into custom enable/disable hooks for the cmnlane.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 92 ++++++++++++++++++++++++-----------------
>  1 file changed, 55 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 9d7b082..e9a8565 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5965,31 +5965,9 @@ static bool i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv,
>  void __vlv_set_power_well(struct drm_i915_private *dev_priv,
>  			  enum punit_power_well power_well_id, bool enable)
>  {
> -	struct drm_device *dev = dev_priv->dev;
>  	u32 mask;
>  	u32 state;
>  	u32 ctrl;
> -	enum pipe pipe;
> -
> -	if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC) {
> -		if (enable) {
> -			/*
> -			 * Enable the CRI clock source so we can get at the
> -			 * display and the reference clock for VGA
> -			 * hotplug / manual detection.
> -			 */
> -			I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> -				   DPLL_REFA_CLK_ENABLE_VLV |
> -				   DPLL_INTEGRATED_CRI_CLK_VLV);
> -			udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
> -		} else {
> -			for_each_pipe(pipe)
> -				assert_pll_disabled(dev_priv, pipe);
> -			/* Assert common reset */
> -			I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) &
> -				   ~DPIO_CMNRST);
> -		}
> -	}
>  
>  	mask = PUNIT_PWRGT_MASK(power_well_id);
>  	state = enable ? PUNIT_PWRGT_PWR_ON(power_well_id) :
> @@ -6017,20 +5995,6 @@ void __vlv_set_power_well(struct drm_i915_private *dev_priv,
>  
>  out:
>  	mutex_unlock(&dev_priv->rps.hw_lock);
> -
> -	/*
> -	 * From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx -
> -	 *  6.	De-assert cmn_reset/side_reset. Same as VLV X0.
> -	 *   a.	GUnit 0x2110 bit[0] set to 1 (def 0)
> -	 *   b.	The other bits such as sfr settings / modesel may all
> -	 *	be set to 0.
> -	 *
> -	 * This should only be done on init and resume from S3 with
> -	 * both PLLs disabled, or we risk losing DPIO and PLL
> -	 * synchronization.
> -	 */
> -	if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC && enable)
> -		I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_CMNRST);
>  }
>  
>  static void vlv_set_power_well(struct drm_i915_private *dev_priv,
> @@ -6130,6 +6094,53 @@ static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
>  	vlv_set_power_well(dev_priv, power_well, false);
>  }
>  
> +static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
> +					   struct i915_power_well *power_well)
> +{
> +	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DPIO_CMN_BC);
> +
> +	/*
> +	 * Enable the CRI clock source so we can get at the
> +	 * display and the reference clock for VGA
> +	 * hotplug / manual detection.
> +	 */
> +	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> +		   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
> +	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
> +
> +	vlv_set_power_well(dev_priv, power_well, true);
> +
> +	/*
> +	 * From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx -
> +	 *  6.	De-assert cmn_reset/side_reset. Same as VLV X0.
> +	 *   a.	GUnit 0x2110 bit[0] set to 1 (def 0)
> +	 *   b.	The other bits such as sfr settings / modesel may all
> +	 *	be set to 0.
> +	 *
> +	 * This should only be done on init and resume from S3 with
> +	 * both PLLs disabled, or we risk losing DPIO and PLL
> +	 * synchronization.
> +	 */
> +	I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_CMNRST);
> +}
> +
> +static void vlv_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv,
> +					    struct i915_power_well *power_well)
> +{
> +	struct drm_device *dev = dev_priv->dev;
> +	enum pipe pipe;
> +
> +	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DPIO_CMN_BC);
> +
> +	for_each_pipe(pipe)
> +		assert_pll_disabled(dev_priv, pipe);
> +
> +	/* Assert common reset */
> +	I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) & ~DPIO_CMNRST);
> +
> +	vlv_set_power_well(dev_priv, power_well, false);
> +}
> +
>  static void check_power_well_state(struct drm_i915_private *dev_priv,
>  				   struct i915_power_well *power_well)
>  {
> @@ -6353,6 +6364,13 @@ static const struct i915_power_well_ops vlv_display_power_well_ops = {
>  	.is_enabled = vlv_power_well_enabled,
>  };
>  
> +static const struct i915_power_well_ops vlv_dpio_cmn_power_well_ops = {
> +	.sync_hw = vlv_power_well_sync_hw,
> +	.enable = vlv_dpio_cmn_power_well_enable,
> +	.disable = vlv_dpio_cmn_power_well_disable,
> +	.is_enabled = vlv_power_well_enabled,
> +};
> +
>  static const struct i915_power_well_ops vlv_dpio_power_well_ops = {
>  	.sync_hw = vlv_power_well_sync_hw,
>  	.enable = vlv_power_well_enable,
> @@ -6413,7 +6431,7 @@ static struct i915_power_well vlv_power_wells[] = {
>  		.name = "dpio-common",
>  		.domains = VLV_DPIO_CMN_BC_POWER_DOMAINS,
>  		.data = PUNIT_POWER_WELL_DPIO_CMN_BC,
> -		.ops = &vlv_dpio_power_well_ops,
> +		.ops = &vlv_dpio_cmn_power_well_ops,
>  	},
>  };
>  

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

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

[PATCH 10/11] drm/i915: Move VLV cmnlane workaround to intel_power_domains_init_hw()

[ville.syrjala]

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

Now that the CMNRESET deassert is part of the cmnlane power well,
intel_reset_dpio() is called too late to make any difference. We've
deasserted CMNRESET by that time, and so the off+on toggle w/a will
never kick in.

Move the workaround to intel_power_domains_init_hw() where it gets
called before we enable the init power domain.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 23 -------------
 drivers/gpu/drm/i915/intel_drv.h     |  3 +-
 drivers/gpu/drm/i915/intel_pm.c      | 67 ++++++++++++++++++++++++++++++------
 3 files changed, 58 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 33cc213..bcd32322 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1508,9 +1508,6 @@ static void intel_reset_dpio(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (!IS_VALLEYVIEW(dev))
-		return;
-
 	if (IS_CHERRYVIEW(dev)) {
 		enum dpio_phy phy;
 		u32 val;
@@ -1532,26 +1529,6 @@ static void intel_reset_dpio(struct drm_device *dev)
 			I915_WRITE(DISPLAY_PHY_CONTROL,
 				PHY_COM_LANE_RESET_DEASSERT(phy, val));
 		}
-
-	} else {
-		/*
-		 * If DPIO has already been reset, e.g. by BIOS, just skip all
-		 * this.
-		 */
-		if (I915_READ(DPIO_CTL) & DPIO_CMNRST)
-			return;
-
-		/*
-		 * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
-		 * Need to assert and de-assert PHY SB reset by gating the
-		 * common lane power, then un-gating it.
-		 * Simply ungating isn't enough to reset the PHY enough to get
-		 * ports and lanes running.
-		 */
-		__vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
-				     false);
-		__vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
-				     true);
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5740be0..e565906 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -970,8 +970,7 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
 void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
 void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
 void ilk_wm_get_hw_state(struct drm_device *dev);
-void __vlv_set_power_well(struct drm_i915_private *dev_priv,
-			  enum punit_power_well power_well_id, bool enable);
+
 
 /* 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 e9a8565..d8e20d3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5962,9 +5962,10 @@ static bool i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv,
 	return true;
 }
 
-void __vlv_set_power_well(struct drm_i915_private *dev_priv,
-			  enum punit_power_well power_well_id, bool enable)
+static void vlv_set_power_well(struct drm_i915_private *dev_priv,
+			       struct i915_power_well *power_well, bool enable)
 {
+	enum punit_power_well power_well_id = power_well->data;
 	u32 mask;
 	u32 state;
 	u32 ctrl;
@@ -5997,14 +5998,6 @@ out:
 	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
-static void vlv_set_power_well(struct drm_i915_private *dev_priv,
-			       struct i915_power_well *power_well, bool enable)
-{
-	enum punit_power_well power_well_id = power_well->data;
-
-	__vlv_set_power_well(dev_priv, power_well_id, enable);
-}
-
 static void vlv_power_well_sync_hw(struct drm_i915_private *dev_priv,
 				   struct i915_power_well *power_well)
 {
@@ -6435,6 +6428,21 @@ static struct i915_power_well vlv_power_wells[] = {
 	},
 };
 
+static struct i915_power_well *lookup_power_well(struct drm_i915_private *dev_priv,
+						 enum punit_power_well power_well_id)
+{
+	struct i915_power_domains *power_domains = &dev_priv->power_domains;
+	struct i915_power_well *power_well;
+	int i;
+
+	for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) {
+		if (power_well->data == power_well_id)
+			return power_well;
+	}
+
+	return NULL;
+}
+
 #define set_power_wells(power_domains, __power_wells) ({		\
 	(power_domains)->power_wells = (__power_wells);			\
 	(power_domains)->power_well_count = ARRAY_SIZE(__power_wells);	\
@@ -6482,11 +6490,50 @@ static void intel_power_domains_resume(struct drm_i915_private *dev_priv)
 	mutex_unlock(&power_domains->lock);
 }
 
+static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv)
+{
+	struct i915_power_well *cmn =
+		lookup_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC);
+	struct i915_power_well *disp2d =
+		lookup_power_well(dev_priv, PUNIT_POWER_WELL_DISP2D);
+
+	/* nothing to do if common lane is already off */
+	if (!cmn->ops->is_enabled(dev_priv, cmn))
+		return;
+
+	/* If the display might be already active skip this */
+	if (disp2d->ops->is_enabled(dev_priv, disp2d) &&
+	    I915_READ(DPIO_CTL) & DPIO_CMNRST)
+		return;
+
+	DRM_DEBUG_KMS("toggling display PHY side reset\n");
+
+	/* cmnlane needs DPLL registers */
+	disp2d->ops->enable(dev_priv, disp2d);
+
+	/*
+	 * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
+	 * Need to assert and de-assert PHY SB reset by gating the
+	 * common lane power, then un-gating it.
+	 * Simply ungating isn't enough to reset the PHY enough to get
+	 * ports and lanes running.
+	 */
+	cmn->ops->disable(dev_priv, cmn);
+}
+
 void intel_power_domains_init_hw(struct drm_i915_private *dev_priv)
 {
+	struct drm_device *dev = dev_priv->dev;
 	struct i915_power_domains *power_domains = &dev_priv->power_domains;
 
 	power_domains->initializing = true;
+
+	if (IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)) {
+		mutex_lock(&power_domains->lock);
+		vlv_cmnlane_wa(dev_priv);
+		mutex_unlock(&power_domains->lock);
+	}
+
 	/* For now, we need the power well to be always enabled. */
 	intel_display_set_init_power(dev_priv, true);
 	intel_power_domains_resume(dev_priv);
-- 
1.8.5.5

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

Re: [PATCH 10/11] drm/i915: Move VLV cmnlane workaround to intel_power_domains_init_hw()

[Jesse Barnes]

On Fri, 13 Jun 2014 13:37:56 +0300
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Now that the CMNRESET deassert is part of the cmnlane power well,
> intel_reset_dpio() is called too late to make any difference. We've
> deasserted CMNRESET by that time, and so the off+on toggle w/a will
> never kick in.
> 
> Move the workaround to intel_power_domains_init_hw() where it gets
> called before we enable the init power domain.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 23 -------------
>  drivers/gpu/drm/i915/intel_drv.h     |  3 +-
>  drivers/gpu/drm/i915/intel_pm.c      | 67 ++++++++++++++++++++++++++++++------
>  3 files changed, 58 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 33cc213..bcd32322 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1508,9 +1508,6 @@ static void intel_reset_dpio(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	if (!IS_VALLEYVIEW(dev))
> -		return;
> -
>  	if (IS_CHERRYVIEW(dev)) {
>  		enum dpio_phy phy;
>  		u32 val;
> @@ -1532,26 +1529,6 @@ static void intel_reset_dpio(struct drm_device *dev)
>  			I915_WRITE(DISPLAY_PHY_CONTROL,
>  				PHY_COM_LANE_RESET_DEASSERT(phy, val));
>  		}
> -
> -	} else {
> -		/*
> -		 * If DPIO has already been reset, e.g. by BIOS, just skip all
> -		 * this.
> -		 */
> -		if (I915_READ(DPIO_CTL) & DPIO_CMNRST)
> -			return;
> -
> -		/*
> -		 * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
> -		 * Need to assert and de-assert PHY SB reset by gating the
> -		 * common lane power, then un-gating it.
> -		 * Simply ungating isn't enough to reset the PHY enough to get
> -		 * ports and lanes running.
> -		 */
> -		__vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
> -				     false);
> -		__vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
> -				     true);
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 5740be0..e565906 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -970,8 +970,7 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
>  void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
>  void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
>  void ilk_wm_get_hw_state(struct drm_device *dev);
> -void __vlv_set_power_well(struct drm_i915_private *dev_priv,
> -			  enum punit_power_well power_well_id, bool enable);
> +
>  
>  /* 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 e9a8565..d8e20d3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5962,9 +5962,10 @@ static bool i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv,
>  	return true;
>  }
>  
> -void __vlv_set_power_well(struct drm_i915_private *dev_priv,
> -			  enum punit_power_well power_well_id, bool enable)
> +static void vlv_set_power_well(struct drm_i915_private *dev_priv,
> +			       struct i915_power_well *power_well, bool enable)
>  {
> +	enum punit_power_well power_well_id = power_well->data;
>  	u32 mask;
>  	u32 state;
>  	u32 ctrl;
> @@ -5997,14 +5998,6 @@ out:
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
> -static void vlv_set_power_well(struct drm_i915_private *dev_priv,
> -			       struct i915_power_well *power_well, bool enable)
> -{
> -	enum punit_power_well power_well_id = power_well->data;
> -
> -	__vlv_set_power_well(dev_priv, power_well_id, enable);
> -}
> -
>  static void vlv_power_well_sync_hw(struct drm_i915_private *dev_priv,
>  				   struct i915_power_well *power_well)
>  {
> @@ -6435,6 +6428,21 @@ static struct i915_power_well vlv_power_wells[] = {
>  	},
>  };
>  
> +static struct i915_power_well *lookup_power_well(struct drm_i915_private *dev_priv,
> +						 enum punit_power_well power_well_id)
> +{
> +	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> +	struct i915_power_well *power_well;
> +	int i;
> +
> +	for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) {
> +		if (power_well->data == power_well_id)
> +			return power_well;
> +	}
> +
> +	return NULL;
> +}
> +
>  #define set_power_wells(power_domains, __power_wells) ({		\
>  	(power_domains)->power_wells = (__power_wells);			\
>  	(power_domains)->power_well_count = ARRAY_SIZE(__power_wells);	\
> @@ -6482,11 +6490,50 @@ static void intel_power_domains_resume(struct drm_i915_private *dev_priv)
>  	mutex_unlock(&power_domains->lock);
>  }
>  
> +static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv)
> +{
> +	struct i915_power_well *cmn =
> +		lookup_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC);
> +	struct i915_power_well *disp2d =
> +		lookup_power_well(dev_priv, PUNIT_POWER_WELL_DISP2D);
> +
> +	/* nothing to do if common lane is already off */
> +	if (!cmn->ops->is_enabled(dev_priv, cmn))
> +		return;
> +
> +	/* If the display might be already active skip this */
> +	if (disp2d->ops->is_enabled(dev_priv, disp2d) &&
> +	    I915_READ(DPIO_CTL) & DPIO_CMNRST)
> +		return;
> +
> +	DRM_DEBUG_KMS("toggling display PHY side reset\n");
> +
> +	/* cmnlane needs DPLL registers */
> +	disp2d->ops->enable(dev_priv, disp2d);
> +
> +	/*
> +	 * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
> +	 * Need to assert and de-assert PHY SB reset by gating the
> +	 * common lane power, then un-gating it.
> +	 * Simply ungating isn't enough to reset the PHY enough to get
> +	 * ports and lanes running.
> +	 */
> +	cmn->ops->disable(dev_priv, cmn);
> +}
> +
>  void intel_power_domains_init_hw(struct drm_i915_private *dev_priv)
>  {
> +	struct drm_device *dev = dev_priv->dev;
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>  
>  	power_domains->initializing = true;
> +
> +	if (IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)) {
> +		mutex_lock(&power_domains->lock);
> +		vlv_cmnlane_wa(dev_priv);
> +		mutex_unlock(&power_domains->lock);
> +	}
> +
>  	/* For now, we need the power well to be always enabled. */
>  	intel_display_set_init_power(dev_priv, true);
>  	intel_power_domains_resume(dev_priv);

I kind of preferred the low level function that just took a well ID
directly since the idea of looking something we already know seems
silly (and it should probably be a separate patch anyway). Also I'm not
sure I would describe this as a W/A; the phy needs to get reset
somehow...

But with or without those changes, we need this.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

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

Re: [PATCH 10/11] drm/i915: Move VLV cmnlane workaround to intel_power_domains_init_hw()

[Ville Syrjälä]

On Wed, Jun 25, 2014 at 12:03:01PM -0700, Jesse Barnes wrote:
> On Fri, 13 Jun 2014 13:37:56 +0300
> ville.syrjala@linux.intel.com wrote:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Now that the CMNRESET deassert is part of the cmnlane power well,
> > intel_reset_dpio() is called too late to make any difference. We've
> > deasserted CMNRESET by that time, and so the off+on toggle w/a will
> > never kick in.
> > 
> > Move the workaround to intel_power_domains_init_hw() where it gets
> > called before we enable the init power domain.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 23 -------------
> >  drivers/gpu/drm/i915/intel_drv.h     |  3 +-
> >  drivers/gpu/drm/i915/intel_pm.c      | 67 ++++++++++++++++++++++++++++++------
> >  3 files changed, 58 insertions(+), 35 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 33cc213..bcd32322 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -1508,9 +1508,6 @@ static void intel_reset_dpio(struct drm_device *dev)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  
> > -	if (!IS_VALLEYVIEW(dev))
> > -		return;
> > -
> >  	if (IS_CHERRYVIEW(dev)) {
> >  		enum dpio_phy phy;
> >  		u32 val;
> > @@ -1532,26 +1529,6 @@ static void intel_reset_dpio(struct drm_device *dev)
> >  			I915_WRITE(DISPLAY_PHY_CONTROL,
> >  				PHY_COM_LANE_RESET_DEASSERT(phy, val));
> >  		}
> > -
> > -	} else {
> > -		/*
> > -		 * If DPIO has already been reset, e.g. by BIOS, just skip all
> > -		 * this.
> > -		 */
> > -		if (I915_READ(DPIO_CTL) & DPIO_CMNRST)
> > -			return;
> > -
> > -		/*
> > -		 * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
> > -		 * Need to assert and de-assert PHY SB reset by gating the
> > -		 * common lane power, then un-gating it.
> > -		 * Simply ungating isn't enough to reset the PHY enough to get
> > -		 * ports and lanes running.
> > -		 */
> > -		__vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
> > -				     false);
> > -		__vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
> > -				     true);
> >  	}
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 5740be0..e565906 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -970,8 +970,7 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
> >  void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
> >  void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
> >  void ilk_wm_get_hw_state(struct drm_device *dev);
> > -void __vlv_set_power_well(struct drm_i915_private *dev_priv,
> > -			  enum punit_power_well power_well_id, bool enable);
> > +
> >  
> >  /* 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 e9a8565..d8e20d3 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5962,9 +5962,10 @@ static bool i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv,
> >  	return true;
> >  }
> >  
> > -void __vlv_set_power_well(struct drm_i915_private *dev_priv,
> > -			  enum punit_power_well power_well_id, bool enable)
> > +static void vlv_set_power_well(struct drm_i915_private *dev_priv,
> > +			       struct i915_power_well *power_well, bool enable)
> >  {
> > +	enum punit_power_well power_well_id = power_well->data;
> >  	u32 mask;
> >  	u32 state;
> >  	u32 ctrl;
> > @@ -5997,14 +5998,6 @@ out:
> >  	mutex_unlock(&dev_priv->rps.hw_lock);
> >  }
> >  
> > -static void vlv_set_power_well(struct drm_i915_private *dev_priv,
> > -			       struct i915_power_well *power_well, bool enable)
> > -{
> > -	enum punit_power_well power_well_id = power_well->data;
> > -
> > -	__vlv_set_power_well(dev_priv, power_well_id, enable);
> > -}
> > -
> >  static void vlv_power_well_sync_hw(struct drm_i915_private *dev_priv,
> >  				   struct i915_power_well *power_well)
> >  {
> > @@ -6435,6 +6428,21 @@ static struct i915_power_well vlv_power_wells[] = {
> >  	},
> >  };
> >  
> > +static struct i915_power_well *lookup_power_well(struct drm_i915_private *dev_priv,
> > +						 enum punit_power_well power_well_id)
> > +{
> > +	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > +	struct i915_power_well *power_well;
> > +	int i;
> > +
> > +	for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) {
> > +		if (power_well->data == power_well_id)
> > +			return power_well;
> > +	}
> > +
> > +	return NULL;
> > +}
> > +
> >  #define set_power_wells(power_domains, __power_wells) ({		\
> >  	(power_domains)->power_wells = (__power_wells);			\
> >  	(power_domains)->power_well_count = ARRAY_SIZE(__power_wells);	\
> > @@ -6482,11 +6490,50 @@ static void intel_power_domains_resume(struct drm_i915_private *dev_priv)
> >  	mutex_unlock(&power_domains->lock);
> >  }
> >  
> > +static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv)
> > +{
> > +	struct i915_power_well *cmn =
> > +		lookup_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC);
> > +	struct i915_power_well *disp2d =
> > +		lookup_power_well(dev_priv, PUNIT_POWER_WELL_DISP2D);
> > +
> > +	/* nothing to do if common lane is already off */
> > +	if (!cmn->ops->is_enabled(dev_priv, cmn))
> > +		return;
> > +
> > +	/* If the display might be already active skip this */
> > +	if (disp2d->ops->is_enabled(dev_priv, disp2d) &&
> > +	    I915_READ(DPIO_CTL) & DPIO_CMNRST)
> > +		return;
> > +
> > +	DRM_DEBUG_KMS("toggling display PHY side reset\n");
> > +
> > +	/* cmnlane needs DPLL registers */
> > +	disp2d->ops->enable(dev_priv, disp2d);
> > +
> > +	/*
> > +	 * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
> > +	 * Need to assert and de-assert PHY SB reset by gating the
> > +	 * common lane power, then un-gating it.
> > +	 * Simply ungating isn't enough to reset the PHY enough to get
> > +	 * ports and lanes running.
> > +	 */
> > +	cmn->ops->disable(dev_priv, cmn);
> > +}
> > +
> >  void intel_power_domains_init_hw(struct drm_i915_private *dev_priv)
> >  {
> > +	struct drm_device *dev = dev_priv->dev;
> >  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> >  
> >  	power_domains->initializing = true;
> > +
> > +	if (IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)) {
> > +		mutex_lock(&power_domains->lock);
> > +		vlv_cmnlane_wa(dev_priv);
> > +		mutex_unlock(&power_domains->lock);
> > +	}
> > +
> >  	/* For now, we need the power well to be always enabled. */
> >  	intel_display_set_init_power(dev_priv, true);
> >  	intel_power_domains_resume(dev_priv);
> 
> I kind of preferred the low level function that just took a well ID
> directly since the idea of looking something we already know seems
> silly (and it should probably be a separate patch anyway).

Yeah the lookup part is a bit silly. But it avoids leaking the
implementation details of the relevant power well ops into vlv_cmnlane_wa().
The disp2d and cmnlane enable/disable hooks do other things besides just
frobbing the punit register, and we should always follow the proper
sequence.

> Also I'm not
> sure I would describe this as a W/A; the phy needs to get reset
> somehow...

I'd call it a hardware or firmware bug if it manages to power things up
in the wrong order and we have to step in to fix it. So calling it a w/a
seems reasonable to me. I can rename though if desired.

> 
> But with or without those changes, we need this.
> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> -- 
> Jesse Barnes, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC

[WIP][PATCH 11/11] drm/i915: Turn off clocks when disp2d is powered down

[ville.syrjala]

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

Set some bits in CCK/CCU to turn off display clocks when disp2d is power
gated. Not sure this really helps with anything. Docs aren't all that clear.

XXX: Doesn't actually work. CCK_DISPLAY_REF_CLOCK_CONTROL and CCU_ICLK_5
writes don't have any effect on the registers for some reason. When clock
gating disp2d via Punit CCK_DISPLAY_REF_CLOCK_CONTROL trunk off force bit
gets set but again direct write has no effect.
---
 drivers/gpu/drm/i915/i915_reg.h |  5 +++++
 drivers/gpu/drm/i915/intel_pm.c | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2aa9a3c..be88b13 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -584,12 +584,17 @@ enum punit_power_well {
 #define  DSI_PLL_M1_DIV_SHIFT			0
 #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
 #define CCK_DISPLAY_CLOCK_CONTROL		0x6b
+#define CCK_DISPLAY_REF_CLOCK_CONTROL		0x6c
 #define  DISPLAY_TRUNK_FORCE_ON			(1 << 17)
 #define  DISPLAY_TRUNK_FORCE_OFF		(1 << 16)
 #define  DISPLAY_FREQUENCY_STATUS		(0x1f << 8)
 #define  DISPLAY_FREQUENCY_STATUS_SHIFT		8
 #define  DISPLAY_FREQUENCY_VALUES		(0x1f << 0)
 
+#define CCU_ICLK_5				0x114
+#define  DISPSS_CLKREQ				(1 << 1)
+#define  DISPBEND_CLKREQ			(1 << 0)
+
 /**
  * DOC: DPIO
  *
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d8e20d3..96614c3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6057,6 +6057,20 @@ static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
 {
 	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DISP2D);
 
+	mutex_lock(&dev_priv->dpio_lock);
+	/*
+	 * (re)enable ref clocks at CCU
+	 * FIXME maybe move to cmnlane?
+	 */
+	vlv_ccu_write(dev_priv, CCU_ICLK_5,
+		      vlv_ccu_read(dev_priv, CCU_ICLK_5) |
+		      DISPBEND_CLKREQ | DISPSS_CLKREQ);
+	/*
+	 * Punit clears CCK trunk force off bits
+	 * automagically while ungating disp2d
+	 */
+	mutex_unlock(&dev_priv->dpio_lock);
+
 	vlv_set_power_well(dev_priv, power_well, true);
 
 	spin_lock_irq(&dev_priv->irq_lock);
@@ -6085,6 +6099,27 @@ static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
 	spin_unlock_irq(&dev_priv->irq_lock);
 
 	vlv_set_power_well(dev_priv, power_well, false);
+
+	mutex_lock(&dev_priv->dpio_lock);
+	/*
+	 * Punit doesn't set the CCK trunk force off bits when power gating
+	 * disp2d. It does set them when clock gating disp2d, but we ask it
+	 * to power gate instead of clock gate.
+	 */
+	vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL,
+		      vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL) |
+		      DISPLAY_TRUNK_FORCE_OFF);
+	vlv_cck_write(dev_priv, CCK_DISPLAY_REF_CLOCK_CONTROL,
+		      vlv_cck_read(dev_priv, CCK_DISPLAY_REF_CLOCK_CONTROL) |
+		      DISPLAY_TRUNK_FORCE_OFF);
+	/*
+	 * disable ref clocks at CCU
+	 * FIXME maybe move to cmnlane?
+	 */
+	vlv_ccu_write(dev_priv, CCU_ICLK_5,
+		      vlv_ccu_read(dev_priv, CCU_ICLK_5) &
+		      ~(DISPBEND_CLKREQ | DISPSS_CLKREQ));
+	mutex_unlock(&dev_priv->dpio_lock);
 }
 
 static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
-- 
1.8.5.5

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

Re: [WIP][PATCH 11/11] drm/i915: Turn off clocks when disp2d is powered down

[Jesse Barnes]

On Fri, 13 Jun 2014 13:37:57 +0300
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Set some bits in CCK/CCU to turn off display clocks when disp2d is power
> gated. Not sure this really helps with anything. Docs aren't all that clear.
> 
> XXX: Doesn't actually work. CCK_DISPLAY_REF_CLOCK_CONTROL and CCU_ICLK_5
> writes don't have any effect on the registers for some reason. When clock
> gating disp2d via Punit CCK_DISPLAY_REF_CLOCK_CONTROL trunk off force bit
> gets set but again direct write has no effect.
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  5 +++++
>  drivers/gpu/drm/i915/intel_pm.c | 35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2aa9a3c..be88b13 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -584,12 +584,17 @@ enum punit_power_well {
>  #define  DSI_PLL_M1_DIV_SHIFT			0
>  #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
>  #define CCK_DISPLAY_CLOCK_CONTROL		0x6b
> +#define CCK_DISPLAY_REF_CLOCK_CONTROL		0x6c
>  #define  DISPLAY_TRUNK_FORCE_ON			(1 << 17)
>  #define  DISPLAY_TRUNK_FORCE_OFF		(1 << 16)
>  #define  DISPLAY_FREQUENCY_STATUS		(0x1f << 8)
>  #define  DISPLAY_FREQUENCY_STATUS_SHIFT		8
>  #define  DISPLAY_FREQUENCY_VALUES		(0x1f << 0)
>  
> +#define CCU_ICLK_5				0x114
> +#define  DISPSS_CLKREQ				(1 << 1)
> +#define  DISPBEND_CLKREQ			(1 << 0)
> +
>  /**
>   * DOC: DPIO
>   *
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d8e20d3..96614c3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6057,6 +6057,20 @@ static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
>  {
>  	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DISP2D);
>  
> +	mutex_lock(&dev_priv->dpio_lock);
> +	/*
> +	 * (re)enable ref clocks at CCU
> +	 * FIXME maybe move to cmnlane?
> +	 */
> +	vlv_ccu_write(dev_priv, CCU_ICLK_5,
> +		      vlv_ccu_read(dev_priv, CCU_ICLK_5) |
> +		      DISPBEND_CLKREQ | DISPSS_CLKREQ);
> +	/*
> +	 * Punit clears CCK trunk force off bits
> +	 * automagically while ungating disp2d
> +	 */
> +	mutex_unlock(&dev_priv->dpio_lock);
> +
>  	vlv_set_power_well(dev_priv, power_well, true);
>  
>  	spin_lock_irq(&dev_priv->irq_lock);
> @@ -6085,6 +6099,27 @@ static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
>  	spin_unlock_irq(&dev_priv->irq_lock);
>  
>  	vlv_set_power_well(dev_priv, power_well, false);
> +
> +	mutex_lock(&dev_priv->dpio_lock);
> +	/*
> +	 * Punit doesn't set the CCK trunk force off bits when power gating
> +	 * disp2d. It does set them when clock gating disp2d, but we ask it
> +	 * to power gate instead of clock gate.
> +	 */
> +	vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL,
> +		      vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL) |
> +		      DISPLAY_TRUNK_FORCE_OFF);
> +	vlv_cck_write(dev_priv, CCK_DISPLAY_REF_CLOCK_CONTROL,
> +		      vlv_cck_read(dev_priv, CCK_DISPLAY_REF_CLOCK_CONTROL) |
> +		      DISPLAY_TRUNK_FORCE_OFF);
> +	/*
> +	 * disable ref clocks at CCU
> +	 * FIXME maybe move to cmnlane?
> +	 */
> +	vlv_ccu_write(dev_priv, CCU_ICLK_5,
> +		      vlv_ccu_read(dev_priv, CCU_ICLK_5) &
> +		      ~(DISPBEND_CLKREQ | DISPSS_CLKREQ));
> +	mutex_unlock(&dev_priv->dpio_lock);
>  }
>  
>  static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,

I guess you could ping the hw folks about this, maybe starting with
Cesar, to see if we can drop the power any further by doing this or
poking some other reg...

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

Re: [WIP][PATCH 11/11] drm/i915: Turn off clocks when disp2d is powered down

[Daniel Vetter]

On Wed, Jun 25, 2014 at 12:03:59PM -0700, Jesse Barnes wrote:
> On Fri, 13 Jun 2014 13:37:57 +0300
> ville.syrjala@linux.intel.com wrote:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Set some bits in CCK/CCU to turn off display clocks when disp2d is power
> > gated. Not sure this really helps with anything. Docs aren't all that clear.
> > 
> > XXX: Doesn't actually work. CCK_DISPLAY_REF_CLOCK_CONTROL and CCU_ICLK_5
> > writes don't have any effect on the registers for some reason. When clock
> > gating disp2d via Punit CCK_DISPLAY_REF_CLOCK_CONTROL trunk off force bit
> > gets set but again direct write has no effect.
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h |  5 +++++
> >  drivers/gpu/drm/i915/intel_pm.c | 35 +++++++++++++++++++++++++++++++++++
> >  2 files changed, 40 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 2aa9a3c..be88b13 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -584,12 +584,17 @@ enum punit_power_well {
> >  #define  DSI_PLL_M1_DIV_SHIFT			0
> >  #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
> >  #define CCK_DISPLAY_CLOCK_CONTROL		0x6b
> > +#define CCK_DISPLAY_REF_CLOCK_CONTROL		0x6c
> >  #define  DISPLAY_TRUNK_FORCE_ON			(1 << 17)
> >  #define  DISPLAY_TRUNK_FORCE_OFF		(1 << 16)
> >  #define  DISPLAY_FREQUENCY_STATUS		(0x1f << 8)
> >  #define  DISPLAY_FREQUENCY_STATUS_SHIFT		8
> >  #define  DISPLAY_FREQUENCY_VALUES		(0x1f << 0)
> >  
> > +#define CCU_ICLK_5				0x114
> > +#define  DISPSS_CLKREQ				(1 << 1)
> > +#define  DISPBEND_CLKREQ			(1 << 0)
> > +
> >  /**
> >   * DOC: DPIO
> >   *
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index d8e20d3..96614c3 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -6057,6 +6057,20 @@ static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
> >  {
> >  	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DISP2D);
> >  
> > +	mutex_lock(&dev_priv->dpio_lock);
> > +	/*
> > +	 * (re)enable ref clocks at CCU
> > +	 * FIXME maybe move to cmnlane?
> > +	 */
> > +	vlv_ccu_write(dev_priv, CCU_ICLK_5,
> > +		      vlv_ccu_read(dev_priv, CCU_ICLK_5) |
> > +		      DISPBEND_CLKREQ | DISPSS_CLKREQ);
> > +	/*
> > +	 * Punit clears CCK trunk force off bits
> > +	 * automagically while ungating disp2d
> > +	 */
> > +	mutex_unlock(&dev_priv->dpio_lock);
> > +
> >  	vlv_set_power_well(dev_priv, power_well, true);
> >  
> >  	spin_lock_irq(&dev_priv->irq_lock);
> > @@ -6085,6 +6099,27 @@ static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
> >  	spin_unlock_irq(&dev_priv->irq_lock);
> >  
> >  	vlv_set_power_well(dev_priv, power_well, false);
> > +
> > +	mutex_lock(&dev_priv->dpio_lock);
> > +	/*
> > +	 * Punit doesn't set the CCK trunk force off bits when power gating
> > +	 * disp2d. It does set them when clock gating disp2d, but we ask it
> > +	 * to power gate instead of clock gate.
> > +	 */
> > +	vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL,
> > +		      vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL) |
> > +		      DISPLAY_TRUNK_FORCE_OFF);
> > +	vlv_cck_write(dev_priv, CCK_DISPLAY_REF_CLOCK_CONTROL,
> > +		      vlv_cck_read(dev_priv, CCK_DISPLAY_REF_CLOCK_CONTROL) |
> > +		      DISPLAY_TRUNK_FORCE_OFF);
> > +	/*
> > +	 * disable ref clocks at CCU
> > +	 * FIXME maybe move to cmnlane?
> > +	 */
> > +	vlv_ccu_write(dev_priv, CCU_ICLK_5,
> > +		      vlv_ccu_read(dev_priv, CCU_ICLK_5) &
> > +		      ~(DISPBEND_CLKREQ | DISPSS_CLKREQ));
> > +	mutex_unlock(&dev_priv->dpio_lock);
> >  }
> >  
> >  static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
> 
> I guess you could ping the hw folks about this, maybe starting with
> Cesar, to see if we can drop the power any further by doing this or
> poking some other reg...

Pulled the entire series except this one here int dinq. Thanks for
patches&review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch