[PATCH 1/4] drm/i915: add bunit read/write routines

[PATCH 1/4] drm/i915: add bunit read/write routines

[Jesse Barnes]

For modifying self-refresh exit latency.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.h       |  2 ++
 drivers/gpu/drm/i915/i915_reg.h       |  1 +
 drivers/gpu/drm/i915/intel_sideband.c | 16 ++++++++++++++++
 3 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cc40cbf..5edf9bb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2403,6 +2403,8 @@ u32 vlv_cck_read(struct drm_i915_private *dev_priv, u32 reg);
 void vlv_cck_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
 u32 vlv_ccu_read(struct drm_i915_private *dev_priv, u32 reg);
 void vlv_ccu_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
+u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg);
+void vlv_bunit_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
 u32 vlv_gps_core_read(struct drm_i915_private *dev_priv, u32 reg);
 void vlv_gps_core_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
 u32 vlv_dpio_read(struct drm_i915_private *dev_priv, enum pipe pipe, int reg);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index de58947..737d8a3 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -349,6 +349,7 @@
 #define   IOSF_BYTE_ENABLES_SHIFT		4
 #define   IOSF_BAR_SHIFT			1
 #define   IOSF_SB_BUSY				(1<<0)
+#define   IOSF_PORT_BUNIT			0x3
 #define   IOSF_PORT_PUNIT			0x4
 #define   IOSF_PORT_NC				0x11
 #define   IOSF_PORT_DPIO			0x12
diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
index 9944d81..d43e457 100644
--- a/drivers/gpu/drm/i915/intel_sideband.c
+++ b/drivers/gpu/drm/i915/intel_sideband.c
@@ -90,6 +90,22 @@ void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
 	mutex_unlock(&dev_priv->dpio_lock);
 }
 
+u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg)
+{
+	u32 val = 0;
+
+	vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT,
+			PUNIT_OPCODE_REG_READ, reg, &val);
+
+	return val;
+}
+
+void vlv_bunit_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
+{
+	vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT,
+			PUNIT_OPCODE_REG_WRITE, reg, &val);
+}
+
 u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
 {
 	u32 val = 0;
-- 
1.8.3.1

[PATCH 2/4] drm/i915: move VLV DDR freq fetch into init_clock_gating

[Jesse Barnes]

We don't want it delayed with the RPS work.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_pm.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a0c907f..2e7072e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4064,19 +4064,6 @@ static void valleyview_enable_rps(struct drm_device *dev)
 	I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
 
 	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
-	switch ((val >> 6) & 3) {
-	case 0:
-	case 1:
-		dev_priv->mem_freq = 800;
-		break;
-	case 2:
-		dev_priv->mem_freq = 1066;
-		break;
-	case 3:
-		dev_priv->mem_freq = 1333;
-		break;
-	}
-	DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
 
 	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & 0x10 ? "yes" : "no");
 	DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
@@ -5325,6 +5312,24 @@ static void ivybridge_init_clock_gating(struct drm_device *dev)
 static void valleyview_init_clock_gating(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val;
+
+	mutex_lock(&dev_priv->rps.hw_lock);
+	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
+	mutex_unlock(&dev_priv->rps.hw_lock);
+	switch ((val >> 6) & 3) {
+	case 0:
+	case 1:
+		dev_priv->mem_freq = 800;
+		break;
+	case 2:
+		dev_priv->mem_freq = 1066;
+		break;
+	case 3:
+		dev_priv->mem_freq = 1333;
+		break;
+	}
+	DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
 
 	I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE);
 
-- 
1.8.3.1

[PATCH 3/4] drm/i915: add modeset_global_pipes callback for global config adjustments

[Jesse Barnes]

In some cases we may need to turn off more pipes than just the ones
affected by a connector or encoder change, probably due to modifying
some global resource that requires all pipes to shut down.  Add a hook
for this purpose to allow platform code to adjust the prepare_pipes
mask, which allows otherwise active pipes to get shut down for the
duration of the mode set, then restored at the end.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.h      | 12 ++++++++++++
 drivers/gpu/drm/i915/intel_display.c |  7 +++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5edf9bb..7530cd4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -380,6 +380,18 @@ struct drm_i915_display_funcs {
 				 struct drm_crtc *crtc,
 				 uint32_t sprite_width, int pixel_size,
 				 bool enable, bool scaled);
+	/**
+	 * modeset_global_pipes - figure out which pipes need to be disabled
+	 * @dev: drm device
+	 * @prepare_pipes: bitmask of pipes to disable
+	 *
+	 * In some cases, adjusting a global resource may require shutting
+	 * down seemingly unrelated pipes on a mode set.  This function
+	 * should determine that and adjust the prepare_pipes bitmask
+	 * as needed.
+	 */
+	void (*modeset_global_pipes)(struct drm_device *dev,
+				     unsigned *prepare_pipes);
 	void (*modeset_global_resources)(struct drm_device *dev);
 	/* Returns the active state of the crtc, and if the crtc is active,
 	 * fills out the pipe-config with the hw state. */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 606a594..faa7548 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9298,6 +9298,13 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 	intel_modeset_affected_pipes(crtc, &modeset_pipes,
 				     &prepare_pipes, &disable_pipes);
 
+	/*
+	 * See if the config requires any additional preparation, e.g.
+	 * to adjust global state with pipes off.
+	 */
+	if (dev_priv->display.modeset_global_pipes)
+		dev_priv->display.modeset_global_pipes(dev, &prepare_pipes);
+
 	*saved_hwmode = crtc->hwmode;
 	*saved_mode = crtc->mode;
 
-- 
1.8.3.1

Re: [PATCH 3/4] drm/i915: add modeset_global_pipes callback for global config adjustments

[Daniel Vetter]

On Fri, Nov 01, 2013 at 08:41:26AM -0700, Jesse Barnes wrote:
> In some cases we may need to turn off more pipes than just the ones
> affected by a connector or encoder change, probably due to modifying
> some global resource that requires all pipes to shut down.  Add a hook
> for this purpose to allow platform code to adjust the prepare_pipes
> mask, which allows otherwise active pipes to get shut down for the
> duration of the mode set, then restored at the end.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      | 12 ++++++++++++
>  drivers/gpu/drm/i915/intel_display.c |  7 +++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5edf9bb..7530cd4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -380,6 +380,18 @@ struct drm_i915_display_funcs {
>  				 struct drm_crtc *crtc,
>  				 uint32_t sprite_width, int pixel_size,
>  				 bool enable, bool scaled);
> +	/**
> +	 * modeset_global_pipes - figure out which pipes need to be disabled
> +	 * @dev: drm device
> +	 * @prepare_pipes: bitmask of pipes to disable
> +	 *
> +	 * In some cases, adjusting a global resource may require shutting
> +	 * down seemingly unrelated pipes on a mode set.  This function
> +	 * should determine that and adjust the prepare_pipes bitmask
> +	 * as needed.
> +	 */
> +	void (*modeset_global_pipes)(struct drm_device *dev,
> +				     unsigned *prepare_pipes);
>  	void (*modeset_global_resources)(struct drm_device *dev);
>  	/* Returns the active state of the crtc, and if the crtc is active,
>  	 * fills out the pipe-config with the hw state. */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 606a594..faa7548 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9298,6 +9298,13 @@ static int __intel_set_mode(struct drm_crtc *crtc,
>  	intel_modeset_affected_pipes(crtc, &modeset_pipes,
>  				     &prepare_pipes, &disable_pipes);
>  
> +	/*
> +	 * See if the config requires any additional preparation, e.g.
> +	 * to adjust global state with pipes off.
> +	 */
> +	if (dev_priv->display.modeset_global_pipes)
> +		dev_priv->display.modeset_global_pipes(dev, &prepare_pipes);

tbh I'd have just added a small callback here - as long as all the global
modeset config stuff is still in flux I wouldn't bother with fancy-looking
callbacks which we need to change later on anyway. And avoiding the vfunc
helps with following the code a bit ...
-Daniel

> +
>  	*saved_hwmode = crtc->hwmode;
>  	*saved_mode = crtc->mode;
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> 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 4/4] drm/i915/vlv: modeset_global_* for VLV

[Jesse Barnes]

On VLV/BYT, we can adjust the CDclk frequency up or down based on the
max pixel clock we need to drive.  Lowering it can save power, while
raising it is necessary to support high resolution.

Add proper modeset_global_pipes and modeset_global_resources support to
perform this adjustment as necessary.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_reg.h      |   2 +
 drivers/gpu/drm/i915/intel_display.c | 151 +++++++++++++++++++++++++++++++++++
 2 files changed, 153 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 737d8a3..9de4adb 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1453,6 +1453,8 @@
 #define   CZCLK_FREQ_MASK	0xf
 #define GMBUSFREQ_VLV		(VLV_DISPLAY_BASE + 0x6510)
 
+#define CZCLK_CDCLK_FREQ_RATIO	(VLV_DISPLAY_BASE + 0x6508)
+
 /*
  * Palette regs
  */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index faa7548..96f0ac6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3894,6 +3894,152 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
 	I915_WRITE(BCLRPAT(crtc->pipe), 0);
 }
 
+static int valleyview_get_vco(struct drm_i915_private *dev_priv)
+{
+	int vco;
+
+	switch (dev_priv->mem_freq) {
+	default:
+	case 800:
+		vco = 800;
+		break;
+	case 1066:
+		vco = 1600;
+		break;
+	case 1333:
+		vco = 2000;
+		break;
+	}
+
+	return vco;
+}
+
+/* Adjust CDclk dividers to allow high res or save power if possible */
+static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val, divider, vco;
+
+	vco = valleyview_get_vco(dev_priv);
+
+	divider = (vco * 10) / cdclk;
+
+	divider = ((divider * 2) / 10) - 1;
+
+	/* Maybe use punit for anything other than 400MHz cdclk? */
+
+	/* adjust czclk ratio */
+	mutex_lock(&dev_priv->dpio_lock);
+	val = vlv_cck_read(dev_priv, 0x6b);
+	val &= ~0xf;
+	val |= divider;
+	vlv_cck_write(dev_priv, 0x6b, val);
+
+	/* adjust self-refresh exit latency value */
+	val = vlv_bunit_read(dev_priv, 0x11);
+	val &= ~0x7f;
+	/*
+	 * 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)
+		val |= 0x12;
+	else
+		val |= 0xc;
+	vlv_bunit_write(dev_priv, 0x11, val);
+	mutex_unlock(&dev_priv->dpio_lock);
+}
+
+static 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, 0x6b);
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	divider &= 0xf;
+	divider = ((divider + 1) * 10) / 2;
+
+	cur_cdclk = (vco * 10) / divider;
+
+	return cur_cdclk;
+}
+
+static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
+				 int max_pixclk)
+{
+	int cur_cdclk;
+
+	cur_cdclk = valleyview_cur_cdclk(dev_priv);
+
+	/*
+	 * Really only a few cases to deal with, as only 4 CDclks are supported:
+	 *   200MHz
+	 *   267MHz
+	 *   320MHz
+	 *   400MHz
+	 * 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 <= 288000 && max_pixclk > 240300) {
+		return 320;
+	} else
+		return 266;
+	/* Looks like lower CDclk freqs don't work on some configs */
+}
+
+static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = dev_priv->dev;
+	struct intel_crtc *intel_crtc;
+	int max_pixclk = 0;
+
+	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
+			    base.head) {
+		if (!intel_crtc->base.enabled)
+			continue;
+
+		if (max_pixclk < intel_crtc->config.adjusted_mode.clock)
+			max_pixclk = intel_crtc->config.adjusted_mode.clock;
+	}
+
+	return max_pixclk;
+}
+
+static void valleyview_modeset_global_pipes(struct drm_device *dev,
+					    unsigned *prepare_pipes)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc;
+	int max_pixclk = intel_mode_max_pixclk(dev_priv);
+	int cur_cdclk = valleyview_cur_cdclk(dev_priv);
+
+	if (valleyview_calc_cdclk(dev_priv, max_pixclk) == cur_cdclk)
+		return;
+
+	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
+			    base.head)
+		if (intel_crtc->base.enabled)
+			*prepare_pipes |= (1 << intel_crtc->pipe);
+}
+
+static void valleyview_modeset_global_resources(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int max_pixclk = intel_mode_max_pixclk(dev_priv);
+	int cur_cdclk = valleyview_cur_cdclk(dev_priv);
+	int req_cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
+
+	if (req_cdclk != cur_cdclk)
+		valleyview_set_cdclk(dev, req_cdclk);
+}
+
 static void valleyview_crtc_enable(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -10336,6 +10482,11 @@ static void intel_init_display(struct drm_device *dev)
 		}
 	} else if (IS_G4X(dev)) {
 		dev_priv->display.write_eld = g4x_write_eld;
+	} else if (IS_VALLEYVIEW(dev)) {
+		dev_priv->display.modeset_global_resources =
+			valleyview_modeset_global_resources;
+		dev_priv->display.modeset_global_pipes =
+			valleyview_modeset_global_pipes;
 	}
 
 	/* Default just returns -ENODEV to indicate unsupported */
-- 
1.8.3.1

[PATCH] drm/i915/vlv: modeset_global_* for VLV v2

[Jesse Barnes]

On VLV/BYT, we can adjust the CDclk frequency up or down based on the
max pixel clock we need to drive.  Lowering it can save power, while
raising it is necessary to support high resolution.

Add proper modeset_global_pipes and modeset_global_resources support to
perform this adjustment as necessary.

v2: use punit interface for 320 and 266 MHz CDclk adjustments (Ville)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_reg.h      |   7 ++
 drivers/gpu/drm/i915/intel_display.c | 175 +++++++++++++++++++++++++++++++++++
 2 files changed, 182 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 737d8a3..8a34dcc 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -363,6 +363,11 @@
 #define PUNIT_OPCODE_REG_READ			6
 #define PUNIT_OPCODE_REG_WRITE			7
 
+#define PUNIT_REG_DSPFREQ			0x36
+#define   DSPFREQSTAT_SHIFT			30
+#define   DSPFREQSTAT_MASK			(0x3 << DSPFREQSTAT_SHIFT)
+#define   DSPFREQGUAR_SHIFT			14
+#define   DSPFREQGUAR_MASK			(0x3 << DSPFREQGUAR_SHIFT)
 #define PUNIT_REG_PWRGT_CTRL			0x60
 #define PUNIT_REG_PWRGT_STATUS			0x61
 #define	  PUNIT_CLK_GATE			1
@@ -1453,6 +1458,8 @@
 #define   CZCLK_FREQ_MASK	0xf
 #define GMBUSFREQ_VLV		(VLV_DISPLAY_BASE + 0x6510)
 
+#define CZCLK_CDCLK_FREQ_RATIO	(VLV_DISPLAY_BASE + 0x6508)
+
 /*
  * Palette regs
  */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index faa7548..85b1dbf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3894,6 +3894,176 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
 	I915_WRITE(BCLRPAT(crtc->pipe), 0);
 }
 
+static int valleyview_get_vco(struct drm_i915_private *dev_priv)
+{
+	int vco;
+
+	switch (dev_priv->mem_freq) {
+	default:
+	case 800:
+		vco = 800;
+		break;
+	case 1066:
+		vco = 1600;
+		break;
+	case 1333:
+		vco = 2000;
+		break;
+	}
+
+	return vco;
+}
+
+/* Adjust CDclk dividers to allow high res or save power if possible */
+static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val;
+
+	if (cdclk == 400) {
+		u32 divider, vco;
+
+		vco = valleyview_get_vco(dev_priv);
+		divider = (vco * 10) / cdclk;
+		divider = ((divider * 2) / 10) - 1;
+
+		mutex_lock(&dev_priv->dpio_lock);
+		/* adjust cdclk divider */
+		val = vlv_cck_read(dev_priv, 0x6b);
+		val &= ~0xf;
+		val |= divider;
+		vlv_cck_write(dev_priv, 0x6b, val);
+		mutex_unlock(&dev_priv->dpio_lock);
+	} else {
+		u32 cmd;
+
+		if (cdclk == 320)
+			cmd = 2;
+		else if (cdclk == 266)
+			cmd = 1;
+		else
+			cmd = 0;
+
+		mutex_lock(&dev_priv->rps.hw_lock);
+		val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
+		val &= ~DSPFREQGUAR_MASK;
+		val |= (cmd << DSPFREQGUAR_SHIFT);
+		vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
+		if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) &
+			      DSPFREQSTAT_MASK) == (cmd << DSPFREQSTAT_SHIFT),
+			     50)) {
+			DRM_ERROR("timed out waiting for CDclk change\n");
+		}
+		mutex_unlock(&dev_priv->rps.hw_lock);
+	}
+
+	mutex_lock(&dev_priv->dpio_lock);
+	/* adjust self-refresh exit latency value */
+	val = vlv_bunit_read(dev_priv, 0x11);
+	val &= ~0x7f;
+
+	/*
+	 * 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)
+		val |= 0x12;
+	else
+		val |= 0xc;
+	vlv_bunit_write(dev_priv, 0x11, val);
+	mutex_unlock(&dev_priv->dpio_lock);
+}
+
+static 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, 0x6b);
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	divider &= 0xf;
+	divider = ((divider + 1) * 10) / 2;
+
+	cur_cdclk = (vco * 10) / divider;
+
+	return cur_cdclk;
+}
+
+static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
+				 int max_pixclk)
+{
+	int cur_cdclk;
+
+	cur_cdclk = valleyview_cur_cdclk(dev_priv);
+
+	/*
+	 * Really only a few cases to deal with, as only 4 CDclks are supported:
+	 *   200MHz
+	 *   267MHz
+	 *   320MHz
+	 *   400MHz
+	 * 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 <= 288000 && max_pixclk > 240300) {
+		return 320;
+	} else
+		return 266;
+	/* Looks like the 200MHz CDclk freq doesn't work on some configs */
+}
+
+static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = dev_priv->dev;
+	struct intel_crtc *intel_crtc;
+	int max_pixclk = 0;
+
+	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
+			    base.head) {
+		if (!intel_crtc->base.enabled)
+			continue;
+
+		if (max_pixclk < intel_crtc->config.adjusted_mode.clock)
+			max_pixclk = intel_crtc->config.adjusted_mode.clock;
+	}
+
+	return max_pixclk;
+}
+
+static void valleyview_modeset_global_pipes(struct drm_device *dev,
+					    unsigned *prepare_pipes)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc;
+	int max_pixclk = intel_mode_max_pixclk(dev_priv);
+	int cur_cdclk = valleyview_cur_cdclk(dev_priv);
+
+	if (valleyview_calc_cdclk(dev_priv, max_pixclk) == cur_cdclk)
+		return;
+
+	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
+			    base.head)
+		if (intel_crtc->base.enabled)
+			*prepare_pipes |= (1 << intel_crtc->pipe);
+}
+
+static void valleyview_modeset_global_resources(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int max_pixclk = intel_mode_max_pixclk(dev_priv);
+	int cur_cdclk = valleyview_cur_cdclk(dev_priv);
+	int req_cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
+
+	if (req_cdclk != cur_cdclk)
+		valleyview_set_cdclk(dev, req_cdclk);
+}
+
 static void valleyview_crtc_enable(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -10336,6 +10506,11 @@ static void intel_init_display(struct drm_device *dev)
 		}
 	} else if (IS_G4X(dev)) {
 		dev_priv->display.write_eld = g4x_write_eld;
+	} else if (IS_VALLEYVIEW(dev)) {
+		dev_priv->display.modeset_global_resources =
+			valleyview_modeset_global_resources;
+		dev_priv->display.modeset_global_pipes =
+			valleyview_modeset_global_pipes;
 	}
 
 	/* Default just returns -ENODEV to indicate unsupported */
-- 
1.8.3.1

[PATCH] drm/i915/vlv: modeset_global_* for VLV v3

[Jesse Barnes]

On VLV/BYT, we can adjust the CDclk frequency up or down based on the
max pixel clock we need to drive.  Lowering it can save power, while
raising it is necessary to support high resolution.

Add proper modeset_global_pipes and modeset_global_resources support to
perform this adjustment as necessary.

v2: use punit interface for 320 and 266 MHz CDclk adjustments (Ville)
v3: reset GMBUS dividers too, since we changed CDclk (Ville)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_reg.h      |   7 ++
 drivers/gpu/drm/i915/intel_display.c | 178 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_i2c.c     |   4 -
 3 files changed, 185 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 737d8a3..8a34dcc 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -363,6 +363,11 @@
 #define PUNIT_OPCODE_REG_READ			6
 #define PUNIT_OPCODE_REG_WRITE			7
 
+#define PUNIT_REG_DSPFREQ			0x36
+#define   DSPFREQSTAT_SHIFT			30
+#define   DSPFREQSTAT_MASK			(0x3 << DSPFREQSTAT_SHIFT)
+#define   DSPFREQGUAR_SHIFT			14
+#define   DSPFREQGUAR_MASK			(0x3 << DSPFREQGUAR_SHIFT)
 #define PUNIT_REG_PWRGT_CTRL			0x60
 #define PUNIT_REG_PWRGT_STATUS			0x61
 #define	  PUNIT_CLK_GATE			1
@@ -1453,6 +1458,8 @@
 #define   CZCLK_FREQ_MASK	0xf
 #define GMBUSFREQ_VLV		(VLV_DISPLAY_BASE + 0x6510)
 
+#define CZCLK_CDCLK_FREQ_RATIO	(VLV_DISPLAY_BASE + 0x6508)
+
 /*
  * Palette regs
  */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index faa7548..ee97789 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3894,6 +3894,179 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
 	I915_WRITE(BCLRPAT(crtc->pipe), 0);
 }
 
+static int valleyview_get_vco(struct drm_i915_private *dev_priv)
+{
+	int vco;
+
+	switch (dev_priv->mem_freq) {
+	default:
+	case 800:
+		vco = 800;
+		break;
+	case 1066:
+		vco = 1600;
+		break;
+	case 1333:
+		vco = 2000;
+		break;
+	}
+
+	return vco;
+}
+
+/* Adjust CDclk dividers to allow high res or save power if possible */
+static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val;
+
+	if (cdclk == 400) {
+		u32 divider, vco;
+
+		vco = valleyview_get_vco(dev_priv);
+		divider = (vco * 10) / cdclk;
+		divider = ((divider * 2) / 10) - 1;
+
+		mutex_lock(&dev_priv->dpio_lock);
+		/* adjust cdclk divider */
+		val = vlv_cck_read(dev_priv, 0x6b);
+		val &= ~0xf;
+		val |= divider;
+		vlv_cck_write(dev_priv, 0x6b, val);
+		mutex_unlock(&dev_priv->dpio_lock);
+	} else {
+		u32 cmd;
+
+		if (cdclk == 320)
+			cmd = 2;
+		else if (cdclk == 266)
+			cmd = 1;
+		else
+			cmd = 0;
+
+		mutex_lock(&dev_priv->rps.hw_lock);
+		val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
+		val &= ~DSPFREQGUAR_MASK;
+		val |= (cmd << DSPFREQGUAR_SHIFT);
+		vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
+		if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) &
+			      DSPFREQSTAT_MASK) == (cmd << DSPFREQSTAT_SHIFT),
+			     50)) {
+			DRM_ERROR("timed out waiting for CDclk change\n");
+		}
+		mutex_unlock(&dev_priv->rps.hw_lock);
+	}
+
+	mutex_lock(&dev_priv->dpio_lock);
+	/* adjust self-refresh exit latency value */
+	val = vlv_bunit_read(dev_priv, 0x11);
+	val &= ~0x7f;
+
+	/*
+	 * 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)
+		val |= 0x12;
+	else
+		val |= 0xc;
+	vlv_bunit_write(dev_priv, 0x11, val);
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	/* Since we changed the CDclk, we need to update the GMBUSFREQ too */
+	intel_i2c_reset(dev);
+}
+
+static 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, 0x6b);
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	divider &= 0xf;
+	divider = ((divider + 1) * 10) / 2;
+
+	cur_cdclk = (vco * 10) / divider;
+
+	return cur_cdclk;
+}
+
+static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
+				 int max_pixclk)
+{
+	int cur_cdclk;
+
+	cur_cdclk = valleyview_cur_cdclk(dev_priv);
+
+	/*
+	 * Really only a few cases to deal with, as only 4 CDclks are supported:
+	 *   200MHz
+	 *   267MHz
+	 *   320MHz
+	 *   400MHz
+	 * 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 <= 288000 && max_pixclk > 240300) {
+		return 320;
+	} else
+		return 266;
+	/* Looks like the 200MHz CDclk freq doesn't work on some configs */
+}
+
+static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = dev_priv->dev;
+	struct intel_crtc *intel_crtc;
+	int max_pixclk = 0;
+
+	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
+			    base.head) {
+		if (!intel_crtc->base.enabled)
+			continue;
+
+		if (max_pixclk < intel_crtc->config.adjusted_mode.clock)
+			max_pixclk = intel_crtc->config.adjusted_mode.clock;
+	}
+
+	return max_pixclk;
+}
+
+static void valleyview_modeset_global_pipes(struct drm_device *dev,
+					    unsigned *prepare_pipes)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc;
+	int max_pixclk = intel_mode_max_pixclk(dev_priv);
+	int cur_cdclk = valleyview_cur_cdclk(dev_priv);
+
+	if (valleyview_calc_cdclk(dev_priv, max_pixclk) == cur_cdclk)
+		return;
+
+	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
+			    base.head)
+		if (intel_crtc->base.enabled)
+			*prepare_pipes |= (1 << intel_crtc->pipe);
+}
+
+static void valleyview_modeset_global_resources(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int max_pixclk = intel_mode_max_pixclk(dev_priv);
+	int cur_cdclk = valleyview_cur_cdclk(dev_priv);
+	int req_cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
+
+	if (req_cdclk != cur_cdclk)
+		valleyview_set_cdclk(dev, req_cdclk);
+}
+
 static void valleyview_crtc_enable(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -10336,6 +10509,11 @@ static void intel_init_display(struct drm_device *dev)
 		}
 	} else if (IS_G4X(dev)) {
 		dev_priv->display.write_eld = g4x_write_eld;
+	} else if (IS_VALLEYVIEW(dev)) {
+		dev_priv->display.modeset_global_resources =
+			valleyview_modeset_global_resources;
+		dev_priv->display.modeset_global_pipes =
+			valleyview_modeset_global_pipes;
 	}
 
 	/* Default just returns -ENODEV to indicate unsupported */
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index 2ca17b1..1263409 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -87,10 +87,6 @@ static void gmbus_set_freq(struct drm_i915_private *dev_priv)
 
 	BUG_ON(!IS_VALLEYVIEW(dev_priv->dev));
 
-	/* Skip setting the gmbus freq if BIOS has already programmed it */
-	if (I915_READ(GMBUSFREQ_VLV) != 0xA0)
-		return;
-
 	/* Obtain SKU information */
 	mutex_lock(&dev_priv->dpio_lock);
 	hpll_freq =
-- 
1.8.3.1

[PATCH] drm/i915: add modeset_global_pipes callback for global config adjustments

[Jesse Barnes]

In some cases we may need to turn off more pipes than just the ones
affected by a connector or encoder change, probably due to modifying
some global resource that requires all pipes to shut down.  Add a hook
for this purpose to allow platform code to adjust the prepare_pipes
mask, which allows otherwise active pipes to get shut down for the
duration of the mode set, then restored at the end.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.h      | 12 ++++++++++++
 drivers/gpu/drm/i915/intel_display.c |  7 +++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5edf9bb..7530cd4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -380,6 +380,18 @@ struct drm_i915_display_funcs {
 				 struct drm_crtc *crtc,
 				 uint32_t sprite_width, int pixel_size,
 				 bool enable, bool scaled);
+	/**
+	 * modeset_global_pipes - figure out which pipes need to be disabled
+	 * @dev: drm device
+	 * @prepare_pipes: bitmask of pipes to disable
+	 *
+	 * In some cases, adjusting a global resource may require shutting
+	 * down seemingly unrelated pipes on a mode set.  This function
+	 * should determine that and adjust the prepare_pipes bitmask
+	 * as needed.
+	 */
+	void (*modeset_global_pipes)(struct drm_device *dev,
+				     unsigned *prepare_pipes);
 	void (*modeset_global_resources)(struct drm_device *dev);
 	/* Returns the active state of the crtc, and if the crtc is active,
 	 * fills out the pipe-config with the hw state. */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 606a594..faa7548 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9298,6 +9298,13 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 	intel_modeset_affected_pipes(crtc, &modeset_pipes,
 				     &prepare_pipes, &disable_pipes);
 
+	/*
+	 * See if the config requires any additional preparation, e.g.
+	 * to adjust global state with pipes off.
+	 */
+	if (dev_priv->display.modeset_global_pipes)
+		dev_priv->display.modeset_global_pipes(dev, &prepare_pipes);
+
 	*saved_hwmode = crtc->hwmode;
 	*saved_mode = crtc->mode;
 
-- 
1.8.3.1

Re: [PATCH] drm/i915: add modeset_global_pipes callback for global config adjustments

[Jesse Barnes]

On Fri,  1 Nov 2013 12:24:06 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> In some cases we may need to turn off more pipes than just the ones
> affected by a connector or encoder change, probably due to modifying
> some global resource that requires all pipes to shut down.  Add a hook
> for this purpose to allow platform code to adjust the prepare_pipes
> mask, which allows otherwise active pipes to get shut down for the
> duration of the mode set, then restored at the end.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Oops just went back in time with the wrong commit... will resend yet
again.
-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH] drm/i915/vlv: modeset_global_* for VLV v4

[Jesse Barnes]

On VLV/BYT, we can adjust the CDclk frequency up or down based on the
max pixel clock we need to drive.  Lowering it can save power, while
raising it is necessary to support high resolution.

Add proper modeset_global_pipes and modeset_global_resources support to
perform this adjustment as necessary.

v2: use punit interface for 320 and 266 MHz CDclk adjustments (Ville)
v3: reset GMBUS dividers too, since we changed CDclk (Ville)
v4: jump to highest voltage when going to 400MHz CDclk (Jesse)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_reg.h      |   7 ++
 drivers/gpu/drm/i915/intel_display.c | 176 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_i2c.c     |   4 -
 3 files changed, 183 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 737d8a3..8a34dcc 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -363,6 +363,11 @@
 #define PUNIT_OPCODE_REG_READ			6
 #define PUNIT_OPCODE_REG_WRITE			7
 
+#define PUNIT_REG_DSPFREQ			0x36
+#define   DSPFREQSTAT_SHIFT			30
+#define   DSPFREQSTAT_MASK			(0x3 << DSPFREQSTAT_SHIFT)
+#define   DSPFREQGUAR_SHIFT			14
+#define   DSPFREQGUAR_MASK			(0x3 << DSPFREQGUAR_SHIFT)
 #define PUNIT_REG_PWRGT_CTRL			0x60
 #define PUNIT_REG_PWRGT_STATUS			0x61
 #define	  PUNIT_CLK_GATE			1
@@ -1453,6 +1458,8 @@
 #define   CZCLK_FREQ_MASK	0xf
 #define GMBUSFREQ_VLV		(VLV_DISPLAY_BASE + 0x6510)
 
+#define CZCLK_CDCLK_FREQ_RATIO	(VLV_DISPLAY_BASE + 0x6508)
+
 /*
  * Palette regs
  */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index faa7548..2ff2a29 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3894,6 +3894,177 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
 	I915_WRITE(BCLRPAT(crtc->pipe), 0);
 }
 
+static int valleyview_get_vco(struct drm_i915_private *dev_priv)
+{
+	int vco;
+
+	switch (dev_priv->mem_freq) {
+	default:
+	case 800:
+		vco = 800;
+		break;
+	case 1066:
+		vco = 1600;
+		break;
+	case 1333:
+		vco = 2000;
+		break;
+	}
+
+	return vco;
+}
+
+/* Adjust CDclk dividers to allow high res or save power if possible */
+static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val, cmd;
+
+	if (cdclk >= 320) /* jump to highest voltage for 400MHz too */
+		cmd = 2;
+	else if (cdclk == 266)
+		cmd = 1;
+	else
+		cmd = 0;
+
+	mutex_lock(&dev_priv->rps.hw_lock);
+	val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
+	val &= ~DSPFREQGUAR_MASK;
+	val |= (cmd << DSPFREQGUAR_SHIFT);
+	vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
+	if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) &
+		      DSPFREQSTAT_MASK) == (cmd << DSPFREQSTAT_SHIFT),
+		     50)) {
+		DRM_ERROR("timed out waiting for CDclk change\n");
+	}
+	mutex_unlock(&dev_priv->rps.hw_lock);
+
+	if (cdclk == 400) {
+		u32 divider, vco;
+
+		vco = valleyview_get_vco(dev_priv);
+		divider = (vco * 10) / cdclk;
+		divider = ((divider * 2) / 10) - 1;
+
+		mutex_lock(&dev_priv->dpio_lock);
+		/* adjust cdclk divider */
+		val = vlv_cck_read(dev_priv, 0x6b);
+		val &= ~0xf;
+		val |= divider;
+		vlv_cck_write(dev_priv, 0x6b, val);
+		mutex_unlock(&dev_priv->dpio_lock);
+	}
+
+	mutex_lock(&dev_priv->dpio_lock);
+	/* adjust self-refresh exit latency value */
+	val = vlv_bunit_read(dev_priv, 0x11);
+	val &= ~0x7f;
+
+	/*
+	 * 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)
+		val |= 0x12;
+	else
+		val |= 0xc;
+	vlv_bunit_write(dev_priv, 0x11, val);
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	/* Since we changed the CDclk, we need to update the GMBUSFREQ too */
+	intel_i2c_reset(dev);
+}
+
+static 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, 0x6b);
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	divider &= 0xf;
+	divider = ((divider + 1) * 10) / 2;
+
+	cur_cdclk = (vco * 10) / divider;
+
+	return cur_cdclk;
+}
+
+static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
+				 int max_pixclk)
+{
+	int cur_cdclk;
+
+	cur_cdclk = valleyview_cur_cdclk(dev_priv);
+
+	/*
+	 * Really only a few cases to deal with, as only 4 CDclks are supported:
+	 *   200MHz
+	 *   267MHz
+	 *   320MHz
+	 *   400MHz
+	 * 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 <= 288000 && max_pixclk > 240300) {
+		return 320;
+	} else
+		return 266;
+	/* Looks like the 200MHz CDclk freq doesn't work on some configs */
+}
+
+static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = dev_priv->dev;
+	struct intel_crtc *intel_crtc;
+	int max_pixclk = 0;
+
+	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
+			    base.head) {
+		if (!intel_crtc->base.enabled)
+			continue;
+
+		if (max_pixclk < intel_crtc->config.adjusted_mode.clock)
+			max_pixclk = intel_crtc->config.adjusted_mode.clock;
+	}
+
+	return max_pixclk;
+}
+
+static void valleyview_modeset_global_pipes(struct drm_device *dev,
+					    unsigned *prepare_pipes)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc;
+	int max_pixclk = intel_mode_max_pixclk(dev_priv);
+	int cur_cdclk = valleyview_cur_cdclk(dev_priv);
+
+	if (valleyview_calc_cdclk(dev_priv, max_pixclk) == cur_cdclk)
+		return;
+
+	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
+			    base.head)
+		if (intel_crtc->base.enabled)
+			*prepare_pipes |= (1 << intel_crtc->pipe);
+}
+
+static void valleyview_modeset_global_resources(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int max_pixclk = intel_mode_max_pixclk(dev_priv);
+	int cur_cdclk = valleyview_cur_cdclk(dev_priv);
+	int req_cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
+
+	if (req_cdclk != cur_cdclk)
+		valleyview_set_cdclk(dev, req_cdclk);
+}
+
 static void valleyview_crtc_enable(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -10336,6 +10507,11 @@ static void intel_init_display(struct drm_device *dev)
 		}
 	} else if (IS_G4X(dev)) {
 		dev_priv->display.write_eld = g4x_write_eld;
+	} else if (IS_VALLEYVIEW(dev)) {
+		dev_priv->display.modeset_global_resources =
+			valleyview_modeset_global_resources;
+		dev_priv->display.modeset_global_pipes =
+			valleyview_modeset_global_pipes;
 	}
 
 	/* Default just returns -ENODEV to indicate unsupported */
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index 2ca17b1..1263409 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -87,10 +87,6 @@ static void gmbus_set_freq(struct drm_i915_private *dev_priv)
 
 	BUG_ON(!IS_VALLEYVIEW(dev_priv->dev));
 
-	/* Skip setting the gmbus freq if BIOS has already programmed it */
-	if (I915_READ(GMBUSFREQ_VLV) != 0xA0)
-		return;
-
 	/* Obtain SKU information */
 	mutex_lock(&dev_priv->dpio_lock);
 	hpll_freq =
-- 
1.8.3.1

Re: [PATCH] drm/i915/vlv: modeset_global_* for VLV v4

[Ville Syrjälä]

On Fri, Nov 01, 2013 at 12:28:16PM -0700, Jesse Barnes wrote:
> On VLV/BYT, we can adjust the CDclk frequency up or down based on the
> max pixel clock we need to drive.  Lowering it can save power, while
> raising it is necessary to support high resolution.
> 
> Add proper modeset_global_pipes and modeset_global_resources support to
> perform this adjustment as necessary.
> 
> v2: use punit interface for 320 and 266 MHz CDclk adjustments (Ville)
> v3: reset GMBUS dividers too, since we changed CDclk (Ville)
> v4: jump to highest voltage when going to 400MHz CDclk (Jesse)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |   7 ++
>  drivers/gpu/drm/i915/intel_display.c | 176 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_i2c.c     |   4 -
>  3 files changed, 183 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 737d8a3..8a34dcc 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -363,6 +363,11 @@
>  #define PUNIT_OPCODE_REG_READ			6
>  #define PUNIT_OPCODE_REG_WRITE			7
>  
> +#define PUNIT_REG_DSPFREQ			0x36
> +#define   DSPFREQSTAT_SHIFT			30
> +#define   DSPFREQSTAT_MASK			(0x3 << DSPFREQSTAT_SHIFT)
> +#define   DSPFREQGUAR_SHIFT			14
> +#define   DSPFREQGUAR_MASK			(0x3 << DSPFREQGUAR_SHIFT)
>  #define PUNIT_REG_PWRGT_CTRL			0x60
>  #define PUNIT_REG_PWRGT_STATUS			0x61
>  #define	  PUNIT_CLK_GATE			1
> @@ -1453,6 +1458,8 @@
>  #define   CZCLK_FREQ_MASK	0xf
>  #define GMBUSFREQ_VLV		(VLV_DISPLAY_BASE + 0x6510)
>  
> +#define CZCLK_CDCLK_FREQ_RATIO	(VLV_DISPLAY_BASE + 0x6508)
> +

It's already there a few lines above, w/ fancy names for the bits and 
everything.

>  /*
>   * Palette regs
>   */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index faa7548..2ff2a29 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3894,6 +3894,177 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
>  	I915_WRITE(BCLRPAT(crtc->pipe), 0);
>  }
>  
> +static int valleyview_get_vco(struct drm_i915_private *dev_priv)
> +{
> +	int vco;
> +
> +	switch (dev_priv->mem_freq) {
> +	default:
> +	case 800:
> +		vco = 800;
> +		break;
> +	case 1066:
> +		vco = 1600;
> +		break;
> +	case 1333:
> +		vco = 2000;
> +		break;
> +	}
> +
> +	return vco;
> +}

We have two ways to get this information now. The other is in intel_i2c.c.
Maybe we should unify a bit.

> +
> +/* Adjust CDclk dividers to allow high res or save power if possible */
> +static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	u32 val, cmd;
> +
> +	if (cdclk >= 320) /* jump to highest voltage for 400MHz too */
> +		cmd = 2;
> +	else if (cdclk == 266)
> +		cmd = 1;
> +	else
> +		cmd = 0;
> +
> +	mutex_lock(&dev_priv->rps.hw_lock);
> +	val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> +	val &= ~DSPFREQGUAR_MASK;
> +	val |= (cmd << DSPFREQGUAR_SHIFT);
> +	vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
> +	if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) &
> +		      DSPFREQSTAT_MASK) == (cmd << DSPFREQSTAT_SHIFT),
> +		     50)) {
> +		DRM_ERROR("timed out waiting for CDclk change\n");
> +	}
> +	mutex_unlock(&dev_priv->rps.hw_lock);
> +
> +	if (cdclk == 400) {
> +		u32 divider, vco;
> +
> +		vco = valleyview_get_vco(dev_priv);
> +		divider = (vco * 10) / cdclk;
> +		divider = ((divider * 2) / 10) - 1;

Why the *10/10?

Just this should do:
divider = (vco << 1) / cdclk - 1

> +
> +		mutex_lock(&dev_priv->dpio_lock);
> +		/* adjust cdclk divider */
> +		val = vlv_cck_read(dev_priv, 0x6b);
> +		val &= ~0xf;
> +		val |= divider;
> +		vlv_cck_write(dev_priv, 0x6b, val);
> +		mutex_unlock(&dev_priv->dpio_lock);
> +	}
> +
> +	mutex_lock(&dev_priv->dpio_lock);
> +	/* adjust self-refresh exit latency value */
> +	val = vlv_bunit_read(dev_priv, 0x11);
> +	val &= ~0x7f;
> +
> +	/*
> +	 * 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)
> +		val |= 0x12;
> +	else
> +		val |= 0xc;
> +	vlv_bunit_write(dev_priv, 0x11, val);
> +	mutex_unlock(&dev_priv->dpio_lock);
> +
> +	/* Since we changed the CDclk, we need to update the GMBUSFREQ too */
> +	intel_i2c_reset(dev);
> +}
> +
> +static 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, 0x6b);
> +	mutex_unlock(&dev_priv->dpio_lock);
> +
> +	divider &= 0xf;
> +	divider = ((divider + 1) * 10) / 2;
> +
> +	cur_cdclk = (vco * 10) / divider;

Again *10/10 seems useless.
Just 'cur_cdclk = (vco << 1) / (divider + 1)'

But again we have a bit of code duplication w/ intel_i2c.c.

> +
> +	return cur_cdclk;
> +}
> +
> +static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
> +				 int max_pixclk)
> +{
> +	int cur_cdclk;
> +
> +	cur_cdclk = valleyview_cur_cdclk(dev_priv);
> +
> +	/*
> +	 * Really only a few cases to deal with, as only 4 CDclks are supported:
> +	 *   200MHz
> +	 *   267MHz
> +	 *   320MHz
> +	 *   400MHz
> +	 * 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 <= 288000 && max_pixclk > 240300) {

Assuming the 267 mhz is in fact 266.666... that would be just 240000.

Also the <= check is useless. Just > is enough.

> +		return 320;
> +	} else
> +		return 266;
> +	/* Looks like the 200MHz CDclk freq doesn't work on some configs */

Too bad. But I guess we just need to avoid it for now. Maybe we can get
it working later.

> +}
> +
> +static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv)
> +{
> +	struct drm_device *dev = dev_priv->dev;
> +	struct intel_crtc *intel_crtc;
> +	int max_pixclk = 0;
> +
> +	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
> +			    base.head) {
> +		if (!intel_crtc->base.enabled)
> +			continue;
> +
> +		if (max_pixclk < intel_crtc->config.adjusted_mode.clock)
> +			max_pixclk = intel_crtc->config.adjusted_mode.clock;

Should be .crtc_clock actually.

> +	}
> +
> +	return max_pixclk;
> +}
> +
> +static void valleyview_modeset_global_pipes(struct drm_device *dev,
> +					    unsigned *prepare_pipes)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc;
> +	int max_pixclk = intel_mode_max_pixclk(dev_priv);
> +	int cur_cdclk = valleyview_cur_cdclk(dev_priv);
> +
> +	if (valleyview_calc_cdclk(dev_priv, max_pixclk) == cur_cdclk)
> +		return;
> +
> +	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
> +			    base.head)
> +		if (intel_crtc->base.enabled)

intel_crtc->active maybe? Although I guess they should be the same when
this gets called.

> +			*prepare_pipes |= (1 << intel_crtc->pipe);
> +}
> +
> +static void valleyview_modeset_global_resources(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int max_pixclk = intel_mode_max_pixclk(dev_priv);
> +	int cur_cdclk = valleyview_cur_cdclk(dev_priv);
> +	int req_cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
> +
> +	if (req_cdclk != cur_cdclk)
> +		valleyview_set_cdclk(dev, req_cdclk);
> +}
> +
>  static void valleyview_crtc_enable(struct drm_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->dev;
> @@ -10336,6 +10507,11 @@ static void intel_init_display(struct drm_device *dev)
>  		}
>  	} else if (IS_G4X(dev)) {
>  		dev_priv->display.write_eld = g4x_write_eld;
> +	} else if (IS_VALLEYVIEW(dev)) {
> +		dev_priv->display.modeset_global_resources =
> +			valleyview_modeset_global_resources;
> +		dev_priv->display.modeset_global_pipes =
> +			valleyview_modeset_global_pipes;
>  	}
>  
>  	/* Default just returns -ENODEV to indicate unsupported */
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index 2ca17b1..1263409 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -87,10 +87,6 @@ static void gmbus_set_freq(struct drm_i915_private *dev_priv)
>  
>  	BUG_ON(!IS_VALLEYVIEW(dev_priv->dev));
>  
> -	/* Skip setting the gmbus freq if BIOS has already programmed it */
> -	if (I915_READ(GMBUSFREQ_VLV) != 0xA0)
> -		return;
> -
>  	/* Obtain SKU information */
>  	mutex_lock(&dev_priv->dpio_lock);
>  	hpll_freq =
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915/vlv: modeset_global_* for VLV v4

[Daniel Vetter]

On Fri, Nov 1, 2013 at 8:52 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>> +static void valleyview_modeset_global_pipes(struct drm_device *dev,
>> +                                         unsigned *prepare_pipes)
>> +{
>> +     struct drm_i915_private *dev_priv = dev->dev_private;
>> +     struct intel_crtc *intel_crtc;
>> +     int max_pixclk = intel_mode_max_pixclk(dev_priv);
>> +     int cur_cdclk = valleyview_cur_cdclk(dev_priv);
>> +
>> +     if (valleyview_calc_cdclk(dev_priv, max_pixclk) == cur_cdclk)
>> +             return;
>> +
>> +     list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
>> +                         base.head)
>> +             if (intel_crtc->base.enabled)
>
> intel_crtc->active maybe? Although I guess they should be the same when
> this gets called.

A WARN_ON(intel_crtc->active) might be useful here to double-check
that the separet logic to frob prepare_pipes actually did it's job
correctly. It's always good to be paranoid when the logic needs to be
splattered over disparate pieces of code.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915/vlv: modeset_global_* for VLV v4

[Jesse Barnes]

On Fri, 1 Nov 2013 21:52:44 +0200
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> On Fri, Nov 01, 2013 at 12:28:16PM -0700, Jesse Barnes wrote:
> > On VLV/BYT, we can adjust the CDclk frequency up or down based on the
> > max pixel clock we need to drive.  Lowering it can save power, while
> > raising it is necessary to support high resolution.
> > 
> > Add proper modeset_global_pipes and modeset_global_resources support to
> > perform this adjustment as necessary.
> > 
> > v2: use punit interface for 320 and 266 MHz CDclk adjustments (Ville)
> > v3: reset GMBUS dividers too, since we changed CDclk (Ville)
> > v4: jump to highest voltage when going to 400MHz CDclk (Jesse)
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      |   7 ++
> >  drivers/gpu/drm/i915/intel_display.c | 176 +++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_i2c.c     |   4 -
> >  3 files changed, 183 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 737d8a3..8a34dcc 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -363,6 +363,11 @@
> >  #define PUNIT_OPCODE_REG_READ			6
> >  #define PUNIT_OPCODE_REG_WRITE			7
> >  
> > +#define PUNIT_REG_DSPFREQ			0x36
> > +#define   DSPFREQSTAT_SHIFT			30
> > +#define   DSPFREQSTAT_MASK			(0x3 << DSPFREQSTAT_SHIFT)
> > +#define   DSPFREQGUAR_SHIFT			14
> > +#define   DSPFREQGUAR_MASK			(0x3 << DSPFREQGUAR_SHIFT)
> >  #define PUNIT_REG_PWRGT_CTRL			0x60
> >  #define PUNIT_REG_PWRGT_STATUS			0x61
> >  #define	  PUNIT_CLK_GATE			1
> > @@ -1453,6 +1458,8 @@
> >  #define   CZCLK_FREQ_MASK	0xf
> >  #define GMBUSFREQ_VLV		(VLV_DISPLAY_BASE + 0x6510)
> >  
> > +#define CZCLK_CDCLK_FREQ_RATIO	(VLV_DISPLAY_BASE + 0x6508)
> > +
> 
> It's already there a few lines above, w/ fancy names for the bits and 
> everything.

Oh missed that obviously, will drop and use the existing bits.  Should
have known since gmbus uses this...

> 
> >  /*
> >   * Palette regs
> >   */
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index faa7548..2ff2a29 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3894,6 +3894,177 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
> >  	I915_WRITE(BCLRPAT(crtc->pipe), 0);
> >  }
> >  
> > +static int valleyview_get_vco(struct drm_i915_private *dev_priv)
> > +{
> > +	int vco;
> > +
> > +	switch (dev_priv->mem_freq) {
> > +	default:
> > +	case 800:
> > +		vco = 800;
> > +		break;
> > +	case 1066:
> > +		vco = 1600;
> > +		break;
> > +	case 1333:
> > +		vco = 2000;
> > +		break;
> > +	}
> > +
> > +	return vco;
> > +}
> 
> We have two ways to get this information now. The other is in intel_i2c.c.
> Maybe we should unify a bit.

I'll take a look when I fix up the register define duplication.

> 
> > +
> > +/* Adjust CDclk dividers to allow high res or save power if possible */
> > +static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
> > +{
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	u32 val, cmd;
> > +
> > +	if (cdclk >= 320) /* jump to highest voltage for 400MHz too */
> > +		cmd = 2;
> > +	else if (cdclk == 266)
> > +		cmd = 1;
> > +	else
> > +		cmd = 0;
> > +
> > +	mutex_lock(&dev_priv->rps.hw_lock);
> > +	val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> > +	val &= ~DSPFREQGUAR_MASK;
> > +	val |= (cmd << DSPFREQGUAR_SHIFT);
> > +	vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
> > +	if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) &
> > +		      DSPFREQSTAT_MASK) == (cmd << DSPFREQSTAT_SHIFT),
> > +		     50)) {
> > +		DRM_ERROR("timed out waiting for CDclk change\n");
> > +	}
> > +	mutex_unlock(&dev_priv->rps.hw_lock);
> > +
> > +	if (cdclk == 400) {
> > +		u32 divider, vco;
> > +
> > +		vco = valleyview_get_vco(dev_priv);
> > +		divider = (vco * 10) / cdclk;
> > +		divider = ((divider * 2) / 10) - 1;
> 
> Why the *10/10?
> 
> Just this should do:
> divider = (vco << 1) / cdclk - 1

Yeah *2 would be sufficient here.  I just liked the 10 since I used it
elsewhere for fixed point bits.

> 
> > +
> > +		mutex_lock(&dev_priv->dpio_lock);
> > +		/* adjust cdclk divider */
> > +		val = vlv_cck_read(dev_priv, 0x6b);
> > +		val &= ~0xf;
> > +		val |= divider;
> > +		vlv_cck_write(dev_priv, 0x6b, val);
> > +		mutex_unlock(&dev_priv->dpio_lock);
> > +	}
> > +
> > +	mutex_lock(&dev_priv->dpio_lock);
> > +	/* adjust self-refresh exit latency value */
> > +	val = vlv_bunit_read(dev_priv, 0x11);
> > +	val &= ~0x7f;
> > +
> > +	/*
> > +	 * 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)
> > +		val |= 0x12;
> > +	else
> > +		val |= 0xc;
> > +	vlv_bunit_write(dev_priv, 0x11, val);
> > +	mutex_unlock(&dev_priv->dpio_lock);
> > +
> > +	/* Since we changed the CDclk, we need to update the GMBUSFREQ too */
> > +	intel_i2c_reset(dev);
> > +}
> > +
> > +static 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, 0x6b);
> > +	mutex_unlock(&dev_priv->dpio_lock);
> > +
> > +	divider &= 0xf;
> > +	divider = ((divider + 1) * 10) / 2;
> > +
> > +	cur_cdclk = (vco * 10) / divider;
> 
> Again *10/10 seems useless.
> Just 'cur_cdclk = (vco << 1) / (divider + 1)'
> 
> But again we have a bit of code duplication w/ intel_i2c.c.

Yeah and the reg contents should match the CCK read, so these could be
unified.

> 
> > +
> > +	return cur_cdclk;
> > +}
> > +
> > +static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
> > +				 int max_pixclk)
> > +{
> > +	int cur_cdclk;
> > +
> > +	cur_cdclk = valleyview_cur_cdclk(dev_priv);
> > +
> > +	/*
> > +	 * Really only a few cases to deal with, as only 4 CDclks are supported:
> > +	 *   200MHz
> > +	 *   267MHz
> > +	 *   320MHz
> > +	 *   400MHz
> > +	 * 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 <= 288000 && max_pixclk > 240300) {
> 
> Assuming the 267 mhz is in fact 266.666... that would be just 240000.
> 
> Also the <= check is useless. Just > is enough.

I just liked documenting the range.  But I can drop that.

> 
> > +		return 320;
> > +	} else
> > +		return 266;
> > +	/* Looks like the 200MHz CDclk freq doesn't work on some configs */
> 
> Too bad. But I guess we just need to avoid it for now. Maybe we can get
> it working later.

Yeah if we can do some more testing and figure out the limitations we
might be able to enable it.  Should save a bit more power, especially
if we can drop the voltage even further for the 200MHz clock.

> 
> > +}
> > +
> > +static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv)
> > +{
> > +	struct drm_device *dev = dev_priv->dev;
> > +	struct intel_crtc *intel_crtc;
> > +	int max_pixclk = 0;
> > +
> > +	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
> > +			    base.head) {
> > +		if (!intel_crtc->base.enabled)
> > +			continue;
> > +
> > +		if (max_pixclk < intel_crtc->config.adjusted_mode.clock)
> > +			max_pixclk = intel_crtc->config.adjusted_mode.clock;
> 
> Should be .crtc_clock actually.

will fix.

> 
> > +	}
> > +
> > +	return max_pixclk;
> > +}
> > +
> > +static void valleyview_modeset_global_pipes(struct drm_device *dev,
> > +					    unsigned *prepare_pipes)
> > +{
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct intel_crtc *intel_crtc;
> > +	int max_pixclk = intel_mode_max_pixclk(dev_priv);
> > +	int cur_cdclk = valleyview_cur_cdclk(dev_priv);
> > +
> > +	if (valleyview_calc_cdclk(dev_priv, max_pixclk) == cur_cdclk)
> > +		return;
> > +
> > +	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
> > +			    base.head)
> > +		if (intel_crtc->base.enabled)
> 
> intel_crtc->active maybe? Although I guess they should be the same when
> this gets called.

Either works, I was looking at the other global_resources and mode
setting code for this.

Thanks for the review, will re-post.

-- 
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 1/4] drm/i915: add bunit read/write routines

[Ville Syrjälä]

On Fri, Nov 01, 2013 at 08:41:24AM -0700, Jesse Barnes wrote:
> For modifying self-refresh exit latency.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

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

> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  2 ++
>  drivers/gpu/drm/i915/i915_reg.h       |  1 +
>  drivers/gpu/drm/i915/intel_sideband.c | 16 ++++++++++++++++
>  3 files changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cc40cbf..5edf9bb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2403,6 +2403,8 @@ u32 vlv_cck_read(struct drm_i915_private *dev_priv, u32 reg);
>  void vlv_cck_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
>  u32 vlv_ccu_read(struct drm_i915_private *dev_priv, u32 reg);
>  void vlv_ccu_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
> +u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg);
> +void vlv_bunit_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
>  u32 vlv_gps_core_read(struct drm_i915_private *dev_priv, u32 reg);
>  void vlv_gps_core_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
>  u32 vlv_dpio_read(struct drm_i915_private *dev_priv, enum pipe pipe, int reg);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index de58947..737d8a3 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -349,6 +349,7 @@
>  #define   IOSF_BYTE_ENABLES_SHIFT		4
>  #define   IOSF_BAR_SHIFT			1
>  #define   IOSF_SB_BUSY				(1<<0)
> +#define   IOSF_PORT_BUNIT			0x3
>  #define   IOSF_PORT_PUNIT			0x4
>  #define   IOSF_PORT_NC				0x11
>  #define   IOSF_PORT_DPIO			0x12
> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
> index 9944d81..d43e457 100644
> --- a/drivers/gpu/drm/i915/intel_sideband.c
> +++ b/drivers/gpu/drm/i915/intel_sideband.c
> @@ -90,6 +90,22 @@ void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
>  	mutex_unlock(&dev_priv->dpio_lock);
>  }
>  
> +u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg)
> +{
> +	u32 val = 0;
> +
> +	vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT,
> +			PUNIT_OPCODE_REG_READ, reg, &val);
> +
> +	return val;
> +}
> +
> +void vlv_bunit_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
> +{
> +	vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT,
> +			PUNIT_OPCODE_REG_WRITE, reg, &val);
> +}
> +
>  u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
>  {
>  	u32 val = 0;
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC