All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/i915: add bunit read/write routines
@ 2013-11-04 19:52 Jesse Barnes
  2013-11-04 19:52 ` [PATCH 2/3] drm/i915: move VLV DDR freq fetch into init_clock_gating Jesse Barnes
  2013-11-04 19:52 ` [PATCH 3/3] drm/i915/vlv: modeset_global_* for VLV v5 Jesse Barnes
  0 siblings, 2 replies; 12+ messages in thread
From: Jesse Barnes @ 2013-11-04 19:52 UTC (permalink / raw
  To: intel-gfx

For modifying self-refresh exit latency.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
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

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] drm/i915: move VLV DDR freq fetch into init_clock_gating
  2013-11-04 19:52 [PATCH 1/3] drm/i915: add bunit read/write routines Jesse Barnes
@ 2013-11-04 19:52 ` Jesse Barnes
  2013-11-04 22:49   ` Ville Syrjälä
  2013-11-04 19:52 ` [PATCH 3/3] drm/i915/vlv: modeset_global_* for VLV v5 Jesse Barnes
  1 sibling, 1 reply; 12+ messages in thread
From: Jesse Barnes @ 2013-11-04 19:52 UTC (permalink / raw
  To: intel-gfx

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] drm/i915/vlv: modeset_global_* for VLV v5
  2013-11-04 19:52 [PATCH 1/3] drm/i915: add bunit read/write routines Jesse Barnes
  2013-11-04 19:52 ` [PATCH 2/3] drm/i915: move VLV DDR freq fetch into init_clock_gating Jesse Barnes
@ 2013-11-04 19:52 ` Jesse Barnes
  2013-11-04 20:37   ` Ville Syrjälä
  2013-11-04 20:54   ` [PATCH] drm/i915/vlv: modeset_global_* for VLV v6 Jesse Barnes
  1 sibling, 2 replies; 12+ messages in thread
From: Jesse Barnes @ 2013-11-04 19:52 UTC (permalink / raw
  To: intel-gfx

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 a new callback in modeset_affected_pipes and a
modeset_global_resources function 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)
v5: drop duplicate define (Ville)
    use shifts by 1 for fixed point (Ville)
    drop new callback (Daniel)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_reg.h      |   6 ++
 drivers/gpu/drm/i915/intel_display.c | 179 +++++++++++++++++++++++++++++++++++
 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..5708a6f 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
@@ -425,6 +430,7 @@
 #define  DSI_PLL_N1_DIV_MASK			(3 << 16)
 #define  DSI_PLL_M1_DIV_SHIFT			0
 #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
+#define CCK_DISPLAY_CLOCK_CONTROL		0x6b
 
 /*
  * DPIO - a special bus for various display related registers to hide behind
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 606a594..8cf42a6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3894,6 +3894,175 @@ 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 << 1) / cdclk) - 1;
+
+		mutex_lock(&dev_priv->dpio_lock);
+		/* adjust cdclk divider */
+		val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
+		val &= ~0xf;
+		val |= divider;
+		vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL, 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, CCK_DISPLAY_CLOCK_CONTROL);
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	divider &= 0xf;
+
+	cur_cdclk = (vco << 1) / (divider + 1);
+
+	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 > 240000) {
+		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.crtc_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;
@@ -8844,6 +9013,13 @@ intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes,
 	*modeset_pipes &= 1 << intel_crtc->pipe;
 	*prepare_pipes &= 1 << intel_crtc->pipe;
 
+	/*
+	 * See if the config requires any additional preparation, e.g.
+	 * to adjust global state with pipes off.
+	 */
+	if (IS_VALLEYVIEW(dev))
+		valleyview_modeset_global_pipes(dev, prepare_pipes);
+
 	DRM_DEBUG_KMS("set mode pipe masks: modeset: %x, prepare: %x, disable: %x\n",
 		      *modeset_pipes, *prepare_pipes, *disable_pipes);
 }
@@ -10329,6 +10505,9 @@ 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;
 	}
 
 	/* 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] drm/i915/vlv: modeset_global_* for VLV v5
  2013-11-04 19:52 ` [PATCH 3/3] drm/i915/vlv: modeset_global_* for VLV v5 Jesse Barnes
@ 2013-11-04 20:37   ` Ville Syrjälä
  2013-11-04 20:54   ` [PATCH] drm/i915/vlv: modeset_global_* for VLV v6 Jesse Barnes
  1 sibling, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2013-11-04 20:37 UTC (permalink / raw
  To: Jesse Barnes; +Cc: intel-gfx

On Mon, Nov 04, 2013 at 11:52:46AM -0800, 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 a new callback in modeset_affected_pipes and a
> modeset_global_resources function 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)
> v5: drop duplicate define (Ville)
>     use shifts by 1 for fixed point (Ville)
>     drop new callback (Daniel)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |   6 ++
>  drivers/gpu/drm/i915/intel_display.c | 179 +++++++++++++++++++++++++++++++++++
>  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..5708a6f 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
> @@ -425,6 +430,7 @@
>  #define  DSI_PLL_N1_DIV_MASK			(3 << 16)
>  #define  DSI_PLL_M1_DIV_SHIFT			0
>  #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
> +#define CCK_DISPLAY_CLOCK_CONTROL		0x6b
>  
>  /*
>   * DPIO - a special bus for various display related registers to hide behind
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 606a594..8cf42a6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3894,6 +3894,175 @@ 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 << 1) / cdclk) - 1;
> +
> +		mutex_lock(&dev_priv->dpio_lock);
> +		/* adjust cdclk divider */
> +		val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
> +		val &= ~0xf;
> +		val |= divider;
> +		vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL, 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;

I just went trawling in configdb to figure out what this does. Maybe
we should make this a bit less magic. For example:

 /*
  * ... set a higher self refresh exit latency ...
  */
 if (cdclk == 400)
 	val |= 4500 / 250; /* 4.5 usec */
 else
 	val |= 3000 / 250; /* 3.0 usec */

> +	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, CCK_DISPLAY_CLOCK_CONTROL);
> +	mutex_unlock(&dev_priv->dpio_lock);
> +
> +	divider &= 0xf;
> +
> +	cur_cdclk = (vco << 1) / (divider + 1);
> +
> +	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 > 240000) {
> +		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)
                                                                  ^^^^^

Maybe just write it as:

max_pixclk = max(max_pixclk, intel_crtc->config.adjusted_mode.crtc_clock);

Otherwise it's looking pretty good to me.

> +			max_pixclk = intel_crtc->config.adjusted_mode.crtc_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;
> @@ -8844,6 +9013,13 @@ intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes,
>  	*modeset_pipes &= 1 << intel_crtc->pipe;
>  	*prepare_pipes &= 1 << intel_crtc->pipe;
>  
> +	/*
> +	 * See if the config requires any additional preparation, e.g.
> +	 * to adjust global state with pipes off.
> +	 */
> +	if (IS_VALLEYVIEW(dev))
> +		valleyview_modeset_global_pipes(dev, prepare_pipes);
> +
>  	DRM_DEBUG_KMS("set mode pipe masks: modeset: %x, prepare: %x, disable: %x\n",
>  		      *modeset_pipes, *prepare_pipes, *disable_pipes);
>  }
> @@ -10329,6 +10505,9 @@ 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;
>  	}
>  
>  	/* 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] drm/i915/vlv: modeset_global_* for VLV v6
  2013-11-04 19:52 ` [PATCH 3/3] drm/i915/vlv: modeset_global_* for VLV v5 Jesse Barnes
  2013-11-04 20:37   ` Ville Syrjälä
@ 2013-11-04 20:54   ` Jesse Barnes
  2013-11-04 21:22     ` Ville Syrjälä
  2013-11-04 21:48     ` [PATCH] drm/i915/vlv: modeset_global_* for VLV v7 Jesse Barnes
  1 sibling, 2 replies; 12+ messages in thread
From: Jesse Barnes @ 2013-11-04 20:54 UTC (permalink / raw
  To: intel-gfx

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 a new callback in modeset_affected_pipes and a
modeset_global_resources function 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)
v5: drop duplicate define (Ville)
    use shifts by 1 for fixed point (Ville)
    drop new callback (Daniel)
v6: fixup adjusted_mode.clock -> adjusted_mode.crtc_clock again (Ville)
    document Bunit reg access better (Ville)

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 737d8a3..d644d75 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -360,9 +360,17 @@
 #define VLV_IOSF_DATA				(VLV_DISPLAY_BASE + 0x2104)
 #define VLV_IOSF_ADDR				(VLV_DISPLAY_BASE + 0x2108)
 
+/* See configdb bunit SB addr map */
+#define BUNIT_REG_BISOC				0x11
+
 #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
@@ -425,6 +433,7 @@
 #define  DSI_PLL_N1_DIV_MASK			(3 << 16)
 #define  DSI_PLL_M1_DIV_SHIFT			0
 #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
+#define CCK_DISPLAY_CLOCK_CONTROL		0x6b
 
 /*
  * DPIO - a special bus for various display related registers to hide behind
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 606a594..ae83f01 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3894,6 +3894,175 @@ 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 << 1) / cdclk) - 1;
+
+		mutex_lock(&dev_priv->dpio_lock);
+		/* adjust cdclk divider */
+		val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
+		val &= ~0xf;
+		val |= divider;
+		vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL, 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, BUNIT_REG_BISOC);
+	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 |= 4500 / 250; /* 4.5 usec */
+	else
+		val |= 3000 / 250; /* 3.0 usec */
+	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);
+}
+
+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, CCK_DISPLAY_CLOCK_CONTROL);
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	divider &= 0xf;
+
+	cur_cdclk = (vco << 1) / (divider + 1);
+
+	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 > 240000) {
+		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;
+
+		max_pixclk = max(max_pixclk,
+				 intel_crtc->config.adjusted_mode.crtc_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;
@@ -8844,6 +9013,13 @@ intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes,
 	*modeset_pipes &= 1 << intel_crtc->pipe;
 	*prepare_pipes &= 1 << intel_crtc->pipe;
 
+	/*
+	 * See if the config requires any additional preparation, e.g.
+	 * to adjust global state with pipes off.
+	 */
+	if (IS_VALLEYVIEW(dev))
+		valleyview_modeset_global_pipes(dev, prepare_pipes);
+
 	DRM_DEBUG_KMS("set mode pipe masks: modeset: %x, prepare: %x, disable: %x\n",
 		      *modeset_pipes, *prepare_pipes, *disable_pipes);
 }
@@ -10329,6 +10505,9 @@ 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;
 	}
 
 	/* 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915/vlv: modeset_global_* for VLV v6
  2013-11-04 20:54   ` [PATCH] drm/i915/vlv: modeset_global_* for VLV v6 Jesse Barnes
@ 2013-11-04 21:22     ` Ville Syrjälä
  2013-11-04 21:24       ` Jesse Barnes
  2013-11-04 21:28       ` Ville Syrjälä
  2013-11-04 21:48     ` [PATCH] drm/i915/vlv: modeset_global_* for VLV v7 Jesse Barnes
  1 sibling, 2 replies; 12+ messages in thread
From: Ville Syrjälä @ 2013-11-04 21:22 UTC (permalink / raw
  To: Jesse Barnes; +Cc: intel-gfx

On Mon, Nov 04, 2013 at 12:54:24PM -0800, 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 a new callback in modeset_affected_pipes and a
> modeset_global_resources function 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)
> v5: drop duplicate define (Ville)
>     use shifts by 1 for fixed point (Ville)
>     drop new callback (Daniel)
> v6: fixup adjusted_mode.clock -> adjusted_mode.crtc_clock again (Ville)
>     document Bunit reg access better (Ville)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |   9 ++
>  drivers/gpu/drm/i915/intel_display.c | 179 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_i2c.c     |   4 -
>  3 files changed, 188 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 737d8a3..d644d75 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -360,9 +360,17 @@
>  #define VLV_IOSF_DATA				(VLV_DISPLAY_BASE + 0x2104)
>  #define VLV_IOSF_ADDR				(VLV_DISPLAY_BASE + 0x2108)
>  
> +/* See configdb bunit SB addr map */
> +#define BUNIT_REG_BISOC				0x11
> +
>  #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
> @@ -425,6 +433,7 @@
>  #define  DSI_PLL_N1_DIV_MASK			(3 << 16)
>  #define  DSI_PLL_M1_DIV_SHIFT			0
>  #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
> +#define CCK_DISPLAY_CLOCK_CONTROL		0x6b
>  
>  /*
>   * DPIO - a special bus for various display related registers to hide behind
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 606a594..ae83f01 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3894,6 +3894,175 @@ 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 << 1) / cdclk) - 1;
> +
> +		mutex_lock(&dev_priv->dpio_lock);
> +		/* adjust cdclk divider */
> +		val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
> +		val &= ~0xf;
> +		val |= divider;
> +		vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL, 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, BUNIT_REG_BISOC);
> +	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 |= 4500 / 250; /* 4.5 usec */
> +	else
> +		val |= 3000 / 250; /* 3.0 usec */
> +	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);
> +}
> +
> +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, CCK_DISPLAY_CLOCK_CONTROL);
> +	mutex_unlock(&dev_priv->dpio_lock);
> +
> +	divider &= 0xf;
> +
> +	cur_cdclk = (vco << 1) / (divider + 1);
> +
> +	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 > 240000) {
> +		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;
> +
> +		max_pixclk = max(max_pixclk,
> +				 intel_crtc->config.adjusted_mode.crtc_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);
> +}
> +
>


So now the problem is now that we have to calculate max_pixclk before
intel_crtc->config has been update for modeset_pipes.

If we make intel_mode_max_pixclk() and valleyview_modeset_global_pipes() like so:

static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv,
				unsigned int modeset_pipes,
				struct intel_crtc_config *pipe_config)
{
	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 (modeset_pipes & (1 << intel_crtc_pipe))
			max_pixclk = max(max_pixclk, pipe_config->adjusted_mode.crtc_clock);
		else
			max_pixclk = max(max_pixclk, intel_crtc->config.adjusted_mode.crtc_clock);
	}

	return max_pixclk;
}

static void valleyview_modeset_global_pipes(struct drm_device *dev,
					    unsigned *prepare_pipes,
					    unsigned int modeset_pipes,
					    struct intel_crtc_config *pipe_config)
{
	struct drm_i915_private *dev_priv = dev->dev_private;
	struct intel_crtc *intel_crtc;
	int max_pixclk = intel_mode_max_pixclk(dev_priv, modeset_pipes, pipe_config);
	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);
}

And then we calculate it after the new pipe config has been computed like so:

	...
	if (modeset_pipes) {
		pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
	        if (IS_ERR(pipe_config)) {
                        ret = PTR_ERR(pipe_config);
                        pipe_config = NULL;

		        goto out;
                }
                intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
		                       "[modeset]");
	}

+	if (IS_VALLEYVIEW(dev))
+		valleyview_modeset_global_pipes(dev, &prepare_pipes, modeset_pipes, pipe_config);

	for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
                intel_crtc_disable(&intel_crtc->base);
	...


Then things should just work (tm).

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915/vlv: modeset_global_* for VLV v6
  2013-11-04 21:22     ` Ville Syrjälä
@ 2013-11-04 21:24       ` Jesse Barnes
  2013-11-04 21:28       ` Ville Syrjälä
  1 sibling, 0 replies; 12+ messages in thread
From: Jesse Barnes @ 2013-11-04 21:24 UTC (permalink / raw
  To: Ville Syrjälä; +Cc: intel-gfx

On Mon, 4 Nov 2013 23:22:03 +0200
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> So now the problem is now that we have to calculate max_pixclk before
> intel_crtc->config has been update for modeset_pipes.
> 
> If we make intel_mode_max_pixclk() and valleyview_modeset_global_pipes() like so:
> 
> static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv,
> 				unsigned int modeset_pipes,
> 				struct intel_crtc_config *pipe_config)
> {
> 	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 (modeset_pipes & (1 << intel_crtc_pipe))
> 			max_pixclk = max(max_pixclk, pipe_config->adjusted_mode.crtc_clock);
> 		else
> 			max_pixclk = max(max_pixclk, intel_crtc->config.adjusted_mode.crtc_clock);
> 	}
> 
> 	return max_pixclk;
> }
> 
> static void valleyview_modeset_global_pipes(struct drm_device *dev,
> 					    unsigned *prepare_pipes,
> 					    unsigned int modeset_pipes,
> 					    struct intel_crtc_config *pipe_config)
> {
> 	struct drm_i915_private *dev_priv = dev->dev_private;
> 	struct intel_crtc *intel_crtc;
> 	int max_pixclk = intel_mode_max_pixclk(dev_priv, modeset_pipes, pipe_config);
> 	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);
> }
> 
> And then we calculate it after the new pipe config has been computed like so:
> 
> 	...
> 	if (modeset_pipes) {
> 		pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
> 	        if (IS_ERR(pipe_config)) {
>                         ret = PTR_ERR(pipe_config);
>                         pipe_config = NULL;
> 
> 		        goto out;
>                 }
>                 intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
> 		                       "[modeset]");
> 	}
> 
> +	if (IS_VALLEYVIEW(dev))
> +		valleyview_modeset_global_pipes(dev, &prepare_pipes, modeset_pipes, pipe_config);
> 
> 	for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
>                 intel_crtc_disable(&intel_crtc->base);
> 	...
> 
> 
> Then things should just work (tm).
> 

Yeah, good catch.  It should be safe to push it that late since we
disable the prepare_pipes just after this point.

Maybe once we have atomic mode setting this will all get easier. :)

-- 
Jesse Barnes, Intel Open Source Technology Center

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915/vlv: modeset_global_* for VLV v6
  2013-11-04 21:22     ` Ville Syrjälä
  2013-11-04 21:24       ` Jesse Barnes
@ 2013-11-04 21:28       ` Ville Syrjälä
  1 sibling, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2013-11-04 21:28 UTC (permalink / raw
  To: Jesse Barnes; +Cc: intel-gfx

On Mon, Nov 04, 2013 at 11:22:03PM +0200, Ville Syrjälä wrote:
> On Mon, Nov 04, 2013 at 12:54:24PM -0800, 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 a new callback in modeset_affected_pipes and a
> > modeset_global_resources function 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)
> > v5: drop duplicate define (Ville)
> >     use shifts by 1 for fixed point (Ville)
> >     drop new callback (Daniel)
> > v6: fixup adjusted_mode.clock -> adjusted_mode.crtc_clock again (Ville)
> >     document Bunit reg access better (Ville)
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      |   9 ++
> >  drivers/gpu/drm/i915/intel_display.c | 179 +++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_i2c.c     |   4 -
> >  3 files changed, 188 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 737d8a3..d644d75 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -360,9 +360,17 @@
> >  #define VLV_IOSF_DATA				(VLV_DISPLAY_BASE + 0x2104)
> >  #define VLV_IOSF_ADDR				(VLV_DISPLAY_BASE + 0x2108)
> >  
> > +/* See configdb bunit SB addr map */
> > +#define BUNIT_REG_BISOC				0x11
> > +
> >  #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
> > @@ -425,6 +433,7 @@
> >  #define  DSI_PLL_N1_DIV_MASK			(3 << 16)
> >  #define  DSI_PLL_M1_DIV_SHIFT			0
> >  #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
> > +#define CCK_DISPLAY_CLOCK_CONTROL		0x6b
> >  
> >  /*
> >   * DPIO - a special bus for various display related registers to hide behind
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 606a594..ae83f01 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3894,6 +3894,175 @@ 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 << 1) / cdclk) - 1;
> > +
> > +		mutex_lock(&dev_priv->dpio_lock);
> > +		/* adjust cdclk divider */
> > +		val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
> > +		val &= ~0xf;
> > +		val |= divider;
> > +		vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL, 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, BUNIT_REG_BISOC);
> > +	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 |= 4500 / 250; /* 4.5 usec */
> > +	else
> > +		val |= 3000 / 250; /* 3.0 usec */
> > +	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);
> > +}
> > +
> > +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, CCK_DISPLAY_CLOCK_CONTROL);
> > +	mutex_unlock(&dev_priv->dpio_lock);
> > +
> > +	divider &= 0xf;
> > +
> > +	cur_cdclk = (vco << 1) / (divider + 1);
> > +
> > +	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 > 240000) {
> > +		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;
> > +
> > +		max_pixclk = max(max_pixclk,
> > +				 intel_crtc->config.adjusted_mode.crtc_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);
> > +}
> > +
> >
> 
> 
> So now the problem is now that we have to calculate max_pixclk before
> intel_crtc->config has been update for modeset_pipes.
> 
> If we make intel_mode_max_pixclk() and valleyview_modeset_global_pipes() like so:
> 
> static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv,
> 				unsigned int modeset_pipes,
> 				struct intel_crtc_config *pipe_config)
> {
> 	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;

Hmm. After a second though, this enabled check is still wrong. But we
know that if the pipe is in modeset_pipes it will be enabled, so we
could move the enabled check to the second branch like so:

if (modeset_pipes & (1 << pipe))
	max(pipe_config);
else if (crtc->enabled)
	max(intel_crtc->config);


> 
> 		if (modeset_pipes & (1 << intel_crtc_pipe))
> 			max_pixclk = max(max_pixclk, pipe_config->adjusted_mode.crtc_clock);
> 		else
> 			max_pixclk = max(max_pixclk, intel_crtc->config.adjusted_mode.crtc_clock);
> 	}
> 
> 	return max_pixclk;
> }
> 
> static void valleyview_modeset_global_pipes(struct drm_device *dev,
> 					    unsigned *prepare_pipes,
> 					    unsigned int modeset_pipes,
> 					    struct intel_crtc_config *pipe_config)
> {
> 	struct drm_i915_private *dev_priv = dev->dev_private;
> 	struct intel_crtc *intel_crtc;
> 	int max_pixclk = intel_mode_max_pixclk(dev_priv, modeset_pipes, pipe_config);
> 	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);
> }
> 
> And then we calculate it after the new pipe config has been computed like so:
> 
> 	...
> 	if (modeset_pipes) {
> 		pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
> 	        if (IS_ERR(pipe_config)) {
>                         ret = PTR_ERR(pipe_config);
>                         pipe_config = NULL;
> 
> 		        goto out;
>                 }
>                 intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
> 		                       "[modeset]");
> 	}
> 
> +	if (IS_VALLEYVIEW(dev))
> +		valleyview_modeset_global_pipes(dev, &prepare_pipes, modeset_pipes, pipe_config);
> 
> 	for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
>                 intel_crtc_disable(&intel_crtc->base);
> 	...
> 
> 
> Then things should just work (tm).
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] drm/i915/vlv: modeset_global_* for VLV v7
  2013-11-04 20:54   ` [PATCH] drm/i915/vlv: modeset_global_* for VLV v6 Jesse Barnes
  2013-11-04 21:22     ` Ville Syrjälä
@ 2013-11-04 21:48     ` Jesse Barnes
  2013-11-04 22:27       ` Ville Syrjälä
  1 sibling, 1 reply; 12+ messages in thread
From: Jesse Barnes @ 2013-11-04 21:48 UTC (permalink / raw
  To: intel-gfx

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 a new callback in modeset_affected_pipes and a
modeset_global_resources function 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)
v5: drop duplicate define (Ville)
    use shifts by 1 for fixed point (Ville)
    drop new callback (Daniel)
v6: fixup adjusted_mode.clock -> adjusted_mode.crtc_clock again (Ville)
    document Bunit reg access better (Ville)
v7: pass modeset_pipes and pipe_config to global_pipes so we get the right
    clock data (Ville)

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 737d8a3..d644d75 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -360,9 +360,17 @@
 #define VLV_IOSF_DATA				(VLV_DISPLAY_BASE + 0x2104)
 #define VLV_IOSF_ADDR				(VLV_DISPLAY_BASE + 0x2108)
 
+/* See configdb bunit SB addr map */
+#define BUNIT_REG_BISOC				0x11
+
 #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
@@ -425,6 +433,7 @@
 #define  DSI_PLL_N1_DIV_MASK			(3 << 16)
 #define  DSI_PLL_M1_DIV_SHIFT			0
 #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
+#define CCK_DISPLAY_CLOCK_CONTROL		0x6b
 
 /*
  * DPIO - a special bus for various display related registers to hide behind
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 606a594..7e0af61 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3894,6 +3894,181 @@ 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 << 1) / cdclk) - 1;
+
+		mutex_lock(&dev_priv->dpio_lock);
+		/* adjust cdclk divider */
+		val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
+		val &= ~0xf;
+		val |= divider;
+		vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL, 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, BUNIT_REG_BISOC);
+	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 |= 4500 / 250; /* 4.5 usec */
+	else
+		val |= 3000 / 250; /* 3.0 usec */
+	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);
+}
+
+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, CCK_DISPLAY_CLOCK_CONTROL);
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	divider &= 0xf;
+
+	cur_cdclk = (vco << 1) / (divider + 1);
+
+	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 > 240000) {
+		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,
+				 unsigned modeset_pipes,
+				 struct intel_crtc_config *pipe_config)
+{
+	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 (modeset_pipes & (1 << intel_crtc->pipe))
+			max_pixclk = max(max_pixclk,
+					 pipe_config->adjusted_mode.crtc_clock);
+		else if (intel_crtc->base.enabled)
+			max_pixclk = max(max_pixclk,
+					 intel_crtc->config.adjusted_mode.crtc_clock);
+	}
+
+	return max_pixclk;
+}
+
+static void valleyview_modeset_global_pipes(struct drm_device *dev,
+					    unsigned *prepare_pipes,
+					    unsigned modeset_pipes,
+					    struct intel_crtc_config *pipe_config)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc;
+	int max_pixclk = intel_mode_max_pixclk(dev_priv, modeset_pipes,
+					       pipe_config);
+	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, 0, NULL);
+	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;
@@ -9318,6 +9493,17 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 				       "[modeset]");
 	}
 
+	/*
+	 * See if the config requires any additional preparation, e.g.
+	 * to adjust global state with pipes off.  We need to do this
+	 * here so we can get the modeset_pipe updated config for the new
+	 * mode set on this crtc.  For other crtcs we need to use the
+	 * adjusted_mode bits in the crtc directly.
+	 */
+	if (IS_VALLEYVIEW(dev))
+		valleyview_modeset_global_pipes(dev, &prepare_pipes,
+						modeset_pipes, pipe_config);
+
 	for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
 		intel_crtc_disable(&intel_crtc->base);
 
@@ -10329,6 +10515,9 @@ 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;
 	}
 
 	/* 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915/vlv: modeset_global_* for VLV v7
  2013-11-04 21:48     ` [PATCH] drm/i915/vlv: modeset_global_* for VLV v7 Jesse Barnes
@ 2013-11-04 22:27       ` Ville Syrjälä
  0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2013-11-04 22:27 UTC (permalink / raw
  To: Jesse Barnes; +Cc: intel-gfx

On Mon, Nov 04, 2013 at 01:48:12PM -0800, 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 a new callback in modeset_affected_pipes and a
> modeset_global_resources function 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)
> v5: drop duplicate define (Ville)
>     use shifts by 1 for fixed point (Ville)
>     drop new callback (Daniel)
> v6: fixup adjusted_mode.clock -> adjusted_mode.crtc_clock again (Ville)
>     document Bunit reg access better (Ville)
> v7: pass modeset_pipes and pipe_config to global_pipes so we get the right
>     clock data (Ville)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

The pipe_config we pass manually could be const, but since I've
already bikeshedded this patch to death, I'm going to let that one
slip :)

I'm reasonably sure this version should do the right thing, so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h      |   9 ++
>  drivers/gpu/drm/i915/intel_display.c | 189 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_i2c.c     |   4 -
>  3 files changed, 198 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 737d8a3..d644d75 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -360,9 +360,17 @@
>  #define VLV_IOSF_DATA				(VLV_DISPLAY_BASE + 0x2104)
>  #define VLV_IOSF_ADDR				(VLV_DISPLAY_BASE + 0x2108)
>  
> +/* See configdb bunit SB addr map */
> +#define BUNIT_REG_BISOC				0x11
> +
>  #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
> @@ -425,6 +433,7 @@
>  #define  DSI_PLL_N1_DIV_MASK			(3 << 16)
>  #define  DSI_PLL_M1_DIV_SHIFT			0
>  #define  DSI_PLL_M1_DIV_MASK			(0x1ff << 0)
> +#define CCK_DISPLAY_CLOCK_CONTROL		0x6b
>  
>  /*
>   * DPIO - a special bus for various display related registers to hide behind
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 606a594..7e0af61 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3894,6 +3894,181 @@ 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 << 1) / cdclk) - 1;
> +
> +		mutex_lock(&dev_priv->dpio_lock);
> +		/* adjust cdclk divider */
> +		val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
> +		val &= ~0xf;
> +		val |= divider;
> +		vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL, 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, BUNIT_REG_BISOC);
> +	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 |= 4500 / 250; /* 4.5 usec */
> +	else
> +		val |= 3000 / 250; /* 3.0 usec */
> +	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);
> +}
> +
> +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, CCK_DISPLAY_CLOCK_CONTROL);
> +	mutex_unlock(&dev_priv->dpio_lock);
> +
> +	divider &= 0xf;
> +
> +	cur_cdclk = (vco << 1) / (divider + 1);
> +
> +	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 > 240000) {
> +		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,
> +				 unsigned modeset_pipes,
> +				 struct intel_crtc_config *pipe_config)
> +{
> +	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 (modeset_pipes & (1 << intel_crtc->pipe))
> +			max_pixclk = max(max_pixclk,
> +					 pipe_config->adjusted_mode.crtc_clock);
> +		else if (intel_crtc->base.enabled)
> +			max_pixclk = max(max_pixclk,
> +					 intel_crtc->config.adjusted_mode.crtc_clock);
> +	}
> +
> +	return max_pixclk;
> +}
> +
> +static void valleyview_modeset_global_pipes(struct drm_device *dev,
> +					    unsigned *prepare_pipes,
> +					    unsigned modeset_pipes,
> +					    struct intel_crtc_config *pipe_config)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc;
> +	int max_pixclk = intel_mode_max_pixclk(dev_priv, modeset_pipes,
> +					       pipe_config);
> +	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, 0, NULL);
> +	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;
> @@ -9318,6 +9493,17 @@ static int __intel_set_mode(struct drm_crtc *crtc,
>  				       "[modeset]");
>  	}
>  
> +	/*
> +	 * See if the config requires any additional preparation, e.g.
> +	 * to adjust global state with pipes off.  We need to do this
> +	 * here so we can get the modeset_pipe updated config for the new
> +	 * mode set on this crtc.  For other crtcs we need to use the
> +	 * adjusted_mode bits in the crtc directly.
> +	 */
> +	if (IS_VALLEYVIEW(dev))
> +		valleyview_modeset_global_pipes(dev, &prepare_pipes,
> +						modeset_pipes, pipe_config);
> +
>  	for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
>  		intel_crtc_disable(&intel_crtc->base);
>  
> @@ -10329,6 +10515,9 @@ 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;
>  	}
>  
>  	/* 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] drm/i915: move VLV DDR freq fetch into init_clock_gating
  2013-11-04 19:52 ` [PATCH 2/3] drm/i915: move VLV DDR freq fetch into init_clock_gating Jesse Barnes
@ 2013-11-04 22:49   ` Ville Syrjälä
  2013-11-05  0:01     ` Jesse Barnes
  0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2013-11-04 22:49 UTC (permalink / raw
  To: Jesse Barnes; +Cc: intel-gfx

On Mon, Nov 04, 2013 at 11:52:45AM -0800, Jesse Barnes wrote:
> 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;
> +	}

This doesn't actually match the punit HAS I have. What I see
is 0=800, 1=1066, 2=1333, 3=invalid.

But using the DDR rate to determine the CCK clock isn't the best idea
I think. It seems there's an option of running CCK at 800MHz even for
the DDR 1066 case. So I think we should just use CCK_FUSE_REG to
figure it out, just like gmbus_set_freq() already does.

> +	DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
>  
>  	I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE);
>  
> -- 
> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] drm/i915: move VLV DDR freq fetch into init_clock_gating
  2013-11-04 22:49   ` Ville Syrjälä
@ 2013-11-05  0:01     ` Jesse Barnes
  0 siblings, 0 replies; 12+ messages in thread
From: Jesse Barnes @ 2013-11-05  0:01 UTC (permalink / raw
  To: Ville Syrjälä; +Cc: intel-gfx

On Tue, 5 Nov 2013 00:49:57 +0200
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> On Mon, Nov 04, 2013 at 11:52:45AM -0800, Jesse Barnes wrote:
> > 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;
> > +	}
> 
> This doesn't actually match the punit HAS I have. What I see
> is 0=800, 1=1066, 2=1333, 3=invalid.
> 
> But using the DDR rate to determine the CCK clock isn't the best idea
> I think. It seems there's an option of running CCK at 800MHz even for
> the DDR 1066 case. So I think we should just use CCK_FUSE_REG to
> figure it out, just like gmbus_set_freq() already does.
> 

Hm this is pretty old code, so the docs may have changed.  IIRC there
were 3 places with this info, but the Punit turbo HAS no longer seems
to have it.  My current Punit HAS matches yours though, so I guess we
should fix that with a separate patch (though it makes me a bit nervous
changing this old code w/o a bug report; it will affect which turbo
freqs are used on current machines).

If the CCK and DDR freqs can really mismatch then we should definitely
use the CCK read directly.  I'll export that function from
intel_display.c and  use it in both places.

So I'll send follow ups for the VCO calc (including an i2c change) and
another patch to fixup the DDR rate detection.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-11-05  0:00 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-04 19:52 [PATCH 1/3] drm/i915: add bunit read/write routines Jesse Barnes
2013-11-04 19:52 ` [PATCH 2/3] drm/i915: move VLV DDR freq fetch into init_clock_gating Jesse Barnes
2013-11-04 22:49   ` Ville Syrjälä
2013-11-05  0:01     ` Jesse Barnes
2013-11-04 19:52 ` [PATCH 3/3] drm/i915/vlv: modeset_global_* for VLV v5 Jesse Barnes
2013-11-04 20:37   ` Ville Syrjälä
2013-11-04 20:54   ` [PATCH] drm/i915/vlv: modeset_global_* for VLV v6 Jesse Barnes
2013-11-04 21:22     ` Ville Syrjälä
2013-11-04 21:24       ` Jesse Barnes
2013-11-04 21:28       ` Ville Syrjälä
2013-11-04 21:48     ` [PATCH] drm/i915/vlv: modeset_global_* for VLV v7 Jesse Barnes
2013-11-04 22:27       ` Ville Syrjälä

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.