[v6,7/8] drm/i915: BDW clock change support

Submitted by Mika Kahola on June 3, 2015, 12:45 p.m.

Details

Message ID 1433335514-4156-8-git-send-email-mika.kahola@intel.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Mika Kahola June 3, 2015, 12:45 p.m.
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

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

v2: Grab rps.hw_lock around sandybridge_pcode_write()
v3: Rebase due to power well vs. .global_resources() reordering
v4: Rebased to the latest
v5: Rebased to the latest
v6: Patch order shuffle so that Broadwell CD clock change is
    applied before the patch for Haswell CD clock change
v7: Fix for patch style problems

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

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

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7213224..0f72c0e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6705,6 +6705,7 @@  enum skl_disp_power_wells {
 #define	  GEN6_PCODE_READ_RC6VIDS		0x5
 #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
 #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
+#define   BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ	0x18
 #define   GEN9_PCODE_READ_MEM_LATENCY		0x6
 #define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
 #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
@@ -7170,6 +7171,7 @@  enum skl_disp_power_wells {
 #define  LCPLL_CLK_FREQ_337_5_BDW	(2<<26)
 #define  LCPLL_CLK_FREQ_675_BDW		(3<<26)
 #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
+#define  LCPLL_ROOT_CD_CLOCK_DISABLE	(1<<24)
 #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
 #define  LCPLL_POWER_DOWN_ALLOW		(1<<22)
 #define  LCPLL_CD_SOURCE_FCLK		(1<<21)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c3f01aa..b1e2069 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5751,7 +5751,22 @@  static void intel_update_max_cdclk(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (IS_VALLEYVIEW(dev)) {
+	if (IS_BROADWELL(dev))  {
+		/*
+		 * FIXME with extra cooling we can allow
+		 * 540 MHz for ULX and 675 Mhz for ULT.
+		 * How can we know if extra cooling is
+		 * available? PCI ID, VTB, something else?
+		 */
+		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
+			dev_priv->max_cdclk_freq = 450000;
+		else if (IS_BDW_ULX(dev))
+			dev_priv->max_cdclk_freq = 450000;
+		else if (IS_BDW_ULT(dev))
+			dev_priv->max_cdclk_freq = 540000;
+		else
+			dev_priv->max_cdclk_freq = 675000;
+	} else if (IS_VALLEYVIEW(dev)) {
 		dev_priv->max_cdclk_freq = 400000;
 	} else {
 		/* otherwise assume cdclk is fixed */
@@ -6621,13 +6636,11 @@  static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv,
 		return true;
 
 	/*
-	 * FIXME if we compare against max we should then
-	 * increase the cdclk frequency when the current
-	 * value is too low. The other option is to compare
-	 * against the cdclk frequency we're going have post
-	 * modeset (ie. one we computed using other constraints).
-	 * Need to measure whether using a lower cdclk w/o IPS
-	 * is better or worse than a higher cdclk w/ IPS.
+	 * We compare against max which means we must take
+	 * the increased cdclk requirement into account when
+	 * calculating the new cdclk.
+	 *
+	 * Should measure whether using a lower cdclk w/o IPS
 	 */
 	return ilk_pipe_pixel_rate(pipe_config) <=
 		dev_priv->max_cdclk_freq * 95 / 100;
@@ -9608,6 +9621,182 @@  static void broxton_modeset_global_resources(struct drm_atomic_state *old_state)
 		broxton_set_cdclk(dev, req_cdclk);
 }
 
+/* compute the max rate for new configuration */
+static int ilk_max_pixel_rate(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = dev_priv->dev;
+	struct intel_crtc *intel_crtc;
+	struct drm_crtc *crtc;
+	int max_pixel_rate = 0;
+	int pixel_rate;
+
+	for_each_crtc(dev, crtc) {
+		if (!crtc->state->enable)
+			continue;
+
+		intel_crtc = to_intel_crtc(crtc);
+		pixel_rate = ilk_pipe_pixel_rate(intel_crtc->config);
+
+		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
+		if (IS_BROADWELL(dev) && intel_crtc->config->ips_enabled)
+			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
+
+		max_pixel_rate = max(max_pixel_rate, pixel_rate);
+	}
+
+	return max_pixel_rate;
+}
+
+static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t val, data;
+	int ret;
+
+	if (WARN((I915_READ(LCPLL_CTL) &
+		  (LCPLL_PLL_DISABLE | LCPLL_PLL_LOCK |
+		   LCPLL_CD_CLOCK_DISABLE | LCPLL_ROOT_CD_CLOCK_DISABLE |
+		   LCPLL_CD2X_CLOCK_DISABLE | LCPLL_POWER_DOWN_ALLOW |
+		   LCPLL_CD_SOURCE_FCLK)) != LCPLL_PLL_LOCK,
+		 "trying to change cdclk frequency with cdclk not enabled\n"))
+		return;
+
+	mutex_lock(&dev_priv->rps.hw_lock);
+	ret = sandybridge_pcode_write(dev_priv,
+				      BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
+	mutex_unlock(&dev_priv->rps.hw_lock);
+	if (ret) {
+		DRM_ERROR("failed to inform pcode about cdclk change\n");
+		return;
+	}
+
+	val = I915_READ(LCPLL_CTL);
+	val |= LCPLL_CD_SOURCE_FCLK;
+	I915_WRITE(LCPLL_CTL, val);
+
+	if (wait_for_atomic_us(I915_READ(LCPLL_CTL) &
+			       LCPLL_CD_SOURCE_FCLK_DONE, 1))
+		DRM_ERROR("Switching to FCLK failed\n");
+
+	val = I915_READ(LCPLL_CTL);
+	val &= ~LCPLL_CLK_FREQ_MASK;
+
+	switch (cdclk) {
+	case 450000:
+		val |= LCPLL_CLK_FREQ_450;
+		data = 0;
+		break;
+	case 540000:
+		val |= LCPLL_CLK_FREQ_54O_BDW;
+		data = 1;
+		break;
+	case 337500:
+		val |= LCPLL_CLK_FREQ_337_5_BDW;
+		data = 2;
+		break;
+	case 675000:
+		val |= LCPLL_CLK_FREQ_675_BDW;
+		data = 3;
+		break;
+	default:
+		WARN(1, "invalid cdclk frequency\n");
+		return;
+	}
+
+	I915_WRITE(LCPLL_CTL, val);
+
+	val = I915_READ(LCPLL_CTL);
+	val &= ~LCPLL_CD_SOURCE_FCLK;
+	I915_WRITE(LCPLL_CTL, val);
+
+	if (wait_for_atomic_us((I915_READ(LCPLL_CTL) &
+				LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
+		DRM_ERROR("Switching back to LCPLL failed\n");
+
+	mutex_lock(&dev_priv->rps.hw_lock);
+	sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ, data);
+	mutex_unlock(&dev_priv->rps.hw_lock);
+
+	intel_update_cdclk(dev);
+
+	WARN(cdclk != dev_priv->cdclk_freq,
+	     "cdclk requested %d kHz but got %d kHz\n",
+	     cdclk, dev_priv->cdclk_freq);
+}
+
+static int broadwell_calc_cdclk(struct drm_i915_private *dev_priv,
+			      int max_pixel_rate)
+{
+	int cdclk;
+
+	/*
+	 * FIXME should also account for plane ratio
+	 * once 64bpp pixel formats are supported.
+	 */
+	if (max_pixel_rate > 540000)
+		cdclk = 675000;
+	else if (max_pixel_rate > 450000)
+		cdclk = 540000;
+	else if (max_pixel_rate > 337500)
+		cdclk = 450000;
+	else
+		cdclk = 337500;
+
+	/*
+	 * FIXME move the cdclk caclulation to
+	 * compute_config() so we can fail gracegully.
+	 */
+	if (cdclk > dev_priv->max_cdclk_freq) {
+		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
+			  cdclk, dev_priv->max_cdclk_freq);
+		cdclk = dev_priv->max_cdclk_freq;
+	}
+
+	return cdclk;
+}
+
+static int broadwell_modeset_global_pipes(struct drm_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->dev);
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+	int max_pixclk = ilk_max_pixel_rate(dev_priv);
+	int cdclk, i;
+
+	cdclk = broadwell_calc_cdclk(dev_priv, max_pixclk);
+
+	if (cdclk == dev_priv->cdclk_freq)
+		return 0;
+
+	/* add all active pipes to the state */
+	for_each_crtc(state->dev, crtc) {
+		if (!crtc->state->enable)
+			continue;
+
+		crtc_state = drm_atomic_get_crtc_state(state, crtc);
+		if (IS_ERR(crtc_state))
+			return PTR_ERR(crtc_state);
+	}
+
+	/* disable/enable all currently active pipes while we change cdclk */
+	for_each_crtc_in_state(state, crtc, crtc_state, i)
+		if (crtc_state->enable)
+			crtc_state->mode_changed = true;
+
+	return 0;
+}
+
+static void broadwell_modeset_global_resources(struct drm_atomic_state *state)
+{
+	struct drm_device *dev = state->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
+	int req_cdclk = broadwell_calc_cdclk(dev_priv, max_pixel_rate);
+
+	if (req_cdclk != dev_priv->cdclk_freq)
+		broadwell_set_cdclk(dev, req_cdclk);
+}
+
 static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
 				      struct intel_crtc_state *crtc_state)
 {
@@ -12788,8 +12977,12 @@  static int __intel_set_mode_checks(struct drm_atomic_state *state)
 	 * mode set on this crtc.  For other crtcs we need to use the
 	 * adjusted_mode bits in the crtc directly.
 	 */
-	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
-		ret = valleyview_modeset_global_pipes(state);
+	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_BROADWELL(dev)) {
+		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev))
+			ret = valleyview_modeset_global_pipes(state);
+		else
+			ret = broadwell_modeset_global_pipes(state);
+
 		if (ret)
 			return ret;
 	}
@@ -14677,6 +14870,9 @@  static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
 	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
 		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
+		if (IS_BROADWELL(dev))
+			dev_priv->display.modeset_global_resources =
+				broadwell_modeset_global_resources;
 	} else if (IS_VALLEYVIEW(dev)) {
 		dev_priv->display.modeset_global_resources =
 			valleyview_modeset_global_resources;

Comments

On Wed, 03 Jun 2015, Mika Kahola <mika.kahola@intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Add support for changing cdclk frequency during runtime on BDW. The
> procedure is quite a bit different on BDW from the one on HSW, so
> add a separate function for it.
>
> Also with IPS enabled the actual pixel rate mustn't exceed 95% of cdclk,
> so take that into account when computing the max pixel rate.
>
> v2: Grab rps.hw_lock around sandybridge_pcode_write()
> v3: Rebase due to power well vs. .global_resources() reordering
> v4: Rebased to the latest
> v5: Rebased to the latest
> v6: Patch order shuffle so that Broadwell CD clock change is
>     applied before the patch for Haswell CD clock change
> v7: Fix for patch style problems
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>

This patch hard hangs my BDW NUC at boot when both DP and HDMI are
connected. Either DP or HDMI alone are good, same with hotplugging the
other afterwards. Booting to grub with both connected, and unplugging
HDMI before loading the kernel also reproduces the issue.

It looks like the problem boils down to the BIOS setting up a smaller
resolution on the DP display when both are connected, and this patch
fails to cope with that on i915 load.

BR,
Jani.



>
> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |   2 +
>  drivers/gpu/drm/i915/intel_display.c | 216 +++++++++++++++++++++++++++++++++--
>  2 files changed, 208 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7213224..0f72c0e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6705,6 +6705,7 @@ enum skl_disp_power_wells {
>  #define	  GEN6_PCODE_READ_RC6VIDS		0x5
>  #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
>  #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
> +#define   BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ	0x18
>  #define   GEN9_PCODE_READ_MEM_LATENCY		0x6
>  #define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
>  #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
> @@ -7170,6 +7171,7 @@ enum skl_disp_power_wells {
>  #define  LCPLL_CLK_FREQ_337_5_BDW	(2<<26)
>  #define  LCPLL_CLK_FREQ_675_BDW		(3<<26)
>  #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
> +#define  LCPLL_ROOT_CD_CLOCK_DISABLE	(1<<24)
>  #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
>  #define  LCPLL_POWER_DOWN_ALLOW		(1<<22)
>  #define  LCPLL_CD_SOURCE_FCLK		(1<<21)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c3f01aa..b1e2069 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5751,7 +5751,22 @@ static void intel_update_max_cdclk(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	if (IS_VALLEYVIEW(dev)) {
> +	if (IS_BROADWELL(dev))  {
> +		/*
> +		 * FIXME with extra cooling we can allow
> +		 * 540 MHz for ULX and 675 Mhz for ULT.
> +		 * How can we know if extra cooling is
> +		 * available? PCI ID, VTB, something else?
> +		 */
> +		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
> +			dev_priv->max_cdclk_freq = 450000;
> +		else if (IS_BDW_ULX(dev))
> +			dev_priv->max_cdclk_freq = 450000;
> +		else if (IS_BDW_ULT(dev))
> +			dev_priv->max_cdclk_freq = 540000;
> +		else
> +			dev_priv->max_cdclk_freq = 675000;
> +	} else if (IS_VALLEYVIEW(dev)) {
>  		dev_priv->max_cdclk_freq = 400000;
>  	} else {
>  		/* otherwise assume cdclk is fixed */
> @@ -6621,13 +6636,11 @@ static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv,
>  		return true;
>  
>  	/*
> -	 * FIXME if we compare against max we should then
> -	 * increase the cdclk frequency when the current
> -	 * value is too low. The other option is to compare
> -	 * against the cdclk frequency we're going have post
> -	 * modeset (ie. one we computed using other constraints).
> -	 * Need to measure whether using a lower cdclk w/o IPS
> -	 * is better or worse than a higher cdclk w/ IPS.
> +	 * We compare against max which means we must take
> +	 * the increased cdclk requirement into account when
> +	 * calculating the new cdclk.
> +	 *
> +	 * Should measure whether using a lower cdclk w/o IPS
>  	 */
>  	return ilk_pipe_pixel_rate(pipe_config) <=
>  		dev_priv->max_cdclk_freq * 95 / 100;
> @@ -9608,6 +9621,182 @@ static void broxton_modeset_global_resources(struct drm_atomic_state *old_state)
>  		broxton_set_cdclk(dev, req_cdclk);
>  }
>  
> +/* compute the max rate for new configuration */
> +static int ilk_max_pixel_rate(struct drm_i915_private *dev_priv)
> +{
> +	struct drm_device *dev = dev_priv->dev;
> +	struct intel_crtc *intel_crtc;
> +	struct drm_crtc *crtc;
> +	int max_pixel_rate = 0;
> +	int pixel_rate;
> +
> +	for_each_crtc(dev, crtc) {
> +		if (!crtc->state->enable)
> +			continue;
> +
> +		intel_crtc = to_intel_crtc(crtc);
> +		pixel_rate = ilk_pipe_pixel_rate(intel_crtc->config);
> +
> +		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> +		if (IS_BROADWELL(dev) && intel_crtc->config->ips_enabled)
> +			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> +
> +		max_pixel_rate = max(max_pixel_rate, pixel_rate);
> +	}
> +
> +	return max_pixel_rate;
> +}
> +
> +static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	uint32_t val, data;
> +	int ret;
> +
> +	if (WARN((I915_READ(LCPLL_CTL) &
> +		  (LCPLL_PLL_DISABLE | LCPLL_PLL_LOCK |
> +		   LCPLL_CD_CLOCK_DISABLE | LCPLL_ROOT_CD_CLOCK_DISABLE |
> +		   LCPLL_CD2X_CLOCK_DISABLE | LCPLL_POWER_DOWN_ALLOW |
> +		   LCPLL_CD_SOURCE_FCLK)) != LCPLL_PLL_LOCK,
> +		 "trying to change cdclk frequency with cdclk not enabled\n"))
> +		return;
> +
> +	mutex_lock(&dev_priv->rps.hw_lock);
> +	ret = sandybridge_pcode_write(dev_priv,
> +				      BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
> +	mutex_unlock(&dev_priv->rps.hw_lock);
> +	if (ret) {
> +		DRM_ERROR("failed to inform pcode about cdclk change\n");
> +		return;
> +	}
> +
> +	val = I915_READ(LCPLL_CTL);
> +	val |= LCPLL_CD_SOURCE_FCLK;
> +	I915_WRITE(LCPLL_CTL, val);
> +
> +	if (wait_for_atomic_us(I915_READ(LCPLL_CTL) &
> +			       LCPLL_CD_SOURCE_FCLK_DONE, 1))
> +		DRM_ERROR("Switching to FCLK failed\n");
> +
> +	val = I915_READ(LCPLL_CTL);
> +	val &= ~LCPLL_CLK_FREQ_MASK;
> +
> +	switch (cdclk) {
> +	case 450000:
> +		val |= LCPLL_CLK_FREQ_450;
> +		data = 0;
> +		break;
> +	case 540000:
> +		val |= LCPLL_CLK_FREQ_54O_BDW;
> +		data = 1;
> +		break;
> +	case 337500:
> +		val |= LCPLL_CLK_FREQ_337_5_BDW;
> +		data = 2;
> +		break;
> +	case 675000:
> +		val |= LCPLL_CLK_FREQ_675_BDW;
> +		data = 3;
> +		break;
> +	default:
> +		WARN(1, "invalid cdclk frequency\n");
> +		return;
> +	}
> +
> +	I915_WRITE(LCPLL_CTL, val);
> +
> +	val = I915_READ(LCPLL_CTL);
> +	val &= ~LCPLL_CD_SOURCE_FCLK;
> +	I915_WRITE(LCPLL_CTL, val);
> +
> +	if (wait_for_atomic_us((I915_READ(LCPLL_CTL) &
> +				LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
> +		DRM_ERROR("Switching back to LCPLL failed\n");
> +
> +	mutex_lock(&dev_priv->rps.hw_lock);
> +	sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ, data);
> +	mutex_unlock(&dev_priv->rps.hw_lock);
> +
> +	intel_update_cdclk(dev);
> +
> +	WARN(cdclk != dev_priv->cdclk_freq,
> +	     "cdclk requested %d kHz but got %d kHz\n",
> +	     cdclk, dev_priv->cdclk_freq);
> +}
> +
> +static int broadwell_calc_cdclk(struct drm_i915_private *dev_priv,
> +			      int max_pixel_rate)
> +{
> +	int cdclk;
> +
> +	/*
> +	 * FIXME should also account for plane ratio
> +	 * once 64bpp pixel formats are supported.
> +	 */
> +	if (max_pixel_rate > 540000)
> +		cdclk = 675000;
> +	else if (max_pixel_rate > 450000)
> +		cdclk = 540000;
> +	else if (max_pixel_rate > 337500)
> +		cdclk = 450000;
> +	else
> +		cdclk = 337500;
> +
> +	/*
> +	 * FIXME move the cdclk caclulation to
> +	 * compute_config() so we can fail gracegully.
> +	 */
> +	if (cdclk > dev_priv->max_cdclk_freq) {
> +		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> +			  cdclk, dev_priv->max_cdclk_freq);
> +		cdclk = dev_priv->max_cdclk_freq;
> +	}
> +
> +	return cdclk;
> +}
> +
> +static int broadwell_modeset_global_pipes(struct drm_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> +	struct drm_crtc *crtc;
> +	struct drm_crtc_state *crtc_state;
> +	int max_pixclk = ilk_max_pixel_rate(dev_priv);
> +	int cdclk, i;
> +
> +	cdclk = broadwell_calc_cdclk(dev_priv, max_pixclk);
> +
> +	if (cdclk == dev_priv->cdclk_freq)
> +		return 0;
> +
> +	/* add all active pipes to the state */
> +	for_each_crtc(state->dev, crtc) {
> +		if (!crtc->state->enable)
> +			continue;
> +
> +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> +		if (IS_ERR(crtc_state))
> +			return PTR_ERR(crtc_state);
> +	}
> +
> +	/* disable/enable all currently active pipes while we change cdclk */
> +	for_each_crtc_in_state(state, crtc, crtc_state, i)
> +		if (crtc_state->enable)
> +			crtc_state->mode_changed = true;
> +
> +	return 0;
> +}
> +
> +static void broadwell_modeset_global_resources(struct drm_atomic_state *state)
> +{
> +	struct drm_device *dev = state->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
> +	int req_cdclk = broadwell_calc_cdclk(dev_priv, max_pixel_rate);
> +
> +	if (req_cdclk != dev_priv->cdclk_freq)
> +		broadwell_set_cdclk(dev, req_cdclk);
> +}
> +
>  static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
>  				      struct intel_crtc_state *crtc_state)
>  {
> @@ -12788,8 +12977,12 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
>  	 * mode set on this crtc.  For other crtcs we need to use the
>  	 * adjusted_mode bits in the crtc directly.
>  	 */
> -	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
> -		ret = valleyview_modeset_global_pipes(state);
> +	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_BROADWELL(dev)) {
> +		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev))
> +			ret = valleyview_modeset_global_pipes(state);
> +		else
> +			ret = broadwell_modeset_global_pipes(state);
> +
>  		if (ret)
>  			return ret;
>  	}
> @@ -14677,6 +14870,9 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
>  	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
> +		if (IS_BROADWELL(dev))
> +			dev_priv->display.modeset_global_resources =
> +				broadwell_modeset_global_resources;
>  	} else if (IS_VALLEYVIEW(dev)) {
>  		dev_priv->display.modeset_global_resources =
>  			valleyview_modeset_global_resources;
> -- 
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 16 Jun 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Wed, 03 Jun 2015, Mika Kahola <mika.kahola@intel.com> wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Add support for changing cdclk frequency during runtime on BDW. The
>> procedure is quite a bit different on BDW from the one on HSW, so
>> add a separate function for it.
>>
>> Also with IPS enabled the actual pixel rate mustn't exceed 95% of cdclk,
>> so take that into account when computing the max pixel rate.
>>
>> v2: Grab rps.hw_lock around sandybridge_pcode_write()
>> v3: Rebase due to power well vs. .global_resources() reordering
>> v4: Rebased to the latest
>> v5: Rebased to the latest
>> v6: Patch order shuffle so that Broadwell CD clock change is
>>     applied before the patch for Haswell CD clock change
>> v7: Fix for patch style problems
>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
>
> This patch hard hangs my BDW NUC at boot when both DP and HDMI are
> connected. Either DP or HDMI alone are good, same with hotplugging the
> other afterwards. Booting to grub with both connected, and unplugging
> HDMI before loading the kernel also reproduces the issue.
>
> It looks like the problem boils down to the BIOS setting up a smaller
> resolution on the DP display when both are connected, and this patch
> fails to cope with that on i915 load.

By "this patch" I obviously refer to

commit b432e5cfd5e92127ad2dd83bfc3083f1dbce43fb
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
Date:   Wed Jun 3 15:45:13 2015 +0300

    drm/i915: BDW clock change support

and everything works for the commit before that.

BR,
Jani.





>
> BR,
> Jani.
>
>
>
>>
>> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h      |   2 +
>>  drivers/gpu/drm/i915/intel_display.c | 216 +++++++++++++++++++++++++++++++++--
>>  2 files changed, 208 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 7213224..0f72c0e 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6705,6 +6705,7 @@ enum skl_disp_power_wells {
>>  #define	  GEN6_PCODE_READ_RC6VIDS		0x5
>>  #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
>>  #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
>> +#define   BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ	0x18
>>  #define   GEN9_PCODE_READ_MEM_LATENCY		0x6
>>  #define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
>>  #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
>> @@ -7170,6 +7171,7 @@ enum skl_disp_power_wells {
>>  #define  LCPLL_CLK_FREQ_337_5_BDW	(2<<26)
>>  #define  LCPLL_CLK_FREQ_675_BDW		(3<<26)
>>  #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
>> +#define  LCPLL_ROOT_CD_CLOCK_DISABLE	(1<<24)
>>  #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
>>  #define  LCPLL_POWER_DOWN_ALLOW		(1<<22)
>>  #define  LCPLL_CD_SOURCE_FCLK		(1<<21)
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index c3f01aa..b1e2069 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -5751,7 +5751,22 @@ static void intel_update_max_cdclk(struct drm_device *dev)
>>  {
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>  
>> -	if (IS_VALLEYVIEW(dev)) {
>> +	if (IS_BROADWELL(dev))  {
>> +		/*
>> +		 * FIXME with extra cooling we can allow
>> +		 * 540 MHz for ULX and 675 Mhz for ULT.
>> +		 * How can we know if extra cooling is
>> +		 * available? PCI ID, VTB, something else?
>> +		 */
>> +		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
>> +			dev_priv->max_cdclk_freq = 450000;
>> +		else if (IS_BDW_ULX(dev))
>> +			dev_priv->max_cdclk_freq = 450000;
>> +		else if (IS_BDW_ULT(dev))
>> +			dev_priv->max_cdclk_freq = 540000;
>> +		else
>> +			dev_priv->max_cdclk_freq = 675000;
>> +	} else if (IS_VALLEYVIEW(dev)) {
>>  		dev_priv->max_cdclk_freq = 400000;
>>  	} else {
>>  		/* otherwise assume cdclk is fixed */
>> @@ -6621,13 +6636,11 @@ static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv,
>>  		return true;
>>  
>>  	/*
>> -	 * FIXME if we compare against max we should then
>> -	 * increase the cdclk frequency when the current
>> -	 * value is too low. The other option is to compare
>> -	 * against the cdclk frequency we're going have post
>> -	 * modeset (ie. one we computed using other constraints).
>> -	 * Need to measure whether using a lower cdclk w/o IPS
>> -	 * is better or worse than a higher cdclk w/ IPS.
>> +	 * We compare against max which means we must take
>> +	 * the increased cdclk requirement into account when
>> +	 * calculating the new cdclk.
>> +	 *
>> +	 * Should measure whether using a lower cdclk w/o IPS
>>  	 */
>>  	return ilk_pipe_pixel_rate(pipe_config) <=
>>  		dev_priv->max_cdclk_freq * 95 / 100;
>> @@ -9608,6 +9621,182 @@ static void broxton_modeset_global_resources(struct drm_atomic_state *old_state)
>>  		broxton_set_cdclk(dev, req_cdclk);
>>  }
>>  
>> +/* compute the max rate for new configuration */
>> +static int ilk_max_pixel_rate(struct drm_i915_private *dev_priv)
>> +{
>> +	struct drm_device *dev = dev_priv->dev;
>> +	struct intel_crtc *intel_crtc;
>> +	struct drm_crtc *crtc;
>> +	int max_pixel_rate = 0;
>> +	int pixel_rate;
>> +
>> +	for_each_crtc(dev, crtc) {
>> +		if (!crtc->state->enable)
>> +			continue;
>> +
>> +		intel_crtc = to_intel_crtc(crtc);
>> +		pixel_rate = ilk_pipe_pixel_rate(intel_crtc->config);
>> +
>> +		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
>> +		if (IS_BROADWELL(dev) && intel_crtc->config->ips_enabled)
>> +			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
>> +
>> +		max_pixel_rate = max(max_pixel_rate, pixel_rate);
>> +	}
>> +
>> +	return max_pixel_rate;
>> +}
>> +
>> +static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
>> +{
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	uint32_t val, data;
>> +	int ret;
>> +
>> +	if (WARN((I915_READ(LCPLL_CTL) &
>> +		  (LCPLL_PLL_DISABLE | LCPLL_PLL_LOCK |
>> +		   LCPLL_CD_CLOCK_DISABLE | LCPLL_ROOT_CD_CLOCK_DISABLE |
>> +		   LCPLL_CD2X_CLOCK_DISABLE | LCPLL_POWER_DOWN_ALLOW |
>> +		   LCPLL_CD_SOURCE_FCLK)) != LCPLL_PLL_LOCK,
>> +		 "trying to change cdclk frequency with cdclk not enabled\n"))
>> +		return;
>> +
>> +	mutex_lock(&dev_priv->rps.hw_lock);
>> +	ret = sandybridge_pcode_write(dev_priv,
>> +				      BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
>> +	mutex_unlock(&dev_priv->rps.hw_lock);
>> +	if (ret) {
>> +		DRM_ERROR("failed to inform pcode about cdclk change\n");
>> +		return;
>> +	}
>> +
>> +	val = I915_READ(LCPLL_CTL);
>> +	val |= LCPLL_CD_SOURCE_FCLK;
>> +	I915_WRITE(LCPLL_CTL, val);
>> +
>> +	if (wait_for_atomic_us(I915_READ(LCPLL_CTL) &
>> +			       LCPLL_CD_SOURCE_FCLK_DONE, 1))
>> +		DRM_ERROR("Switching to FCLK failed\n");
>> +
>> +	val = I915_READ(LCPLL_CTL);
>> +	val &= ~LCPLL_CLK_FREQ_MASK;
>> +
>> +	switch (cdclk) {
>> +	case 450000:
>> +		val |= LCPLL_CLK_FREQ_450;
>> +		data = 0;
>> +		break;
>> +	case 540000:
>> +		val |= LCPLL_CLK_FREQ_54O_BDW;
>> +		data = 1;
>> +		break;
>> +	case 337500:
>> +		val |= LCPLL_CLK_FREQ_337_5_BDW;
>> +		data = 2;
>> +		break;
>> +	case 675000:
>> +		val |= LCPLL_CLK_FREQ_675_BDW;
>> +		data = 3;
>> +		break;
>> +	default:
>> +		WARN(1, "invalid cdclk frequency\n");
>> +		return;
>> +	}
>> +
>> +	I915_WRITE(LCPLL_CTL, val);
>> +
>> +	val = I915_READ(LCPLL_CTL);
>> +	val &= ~LCPLL_CD_SOURCE_FCLK;
>> +	I915_WRITE(LCPLL_CTL, val);
>> +
>> +	if (wait_for_atomic_us((I915_READ(LCPLL_CTL) &
>> +				LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
>> +		DRM_ERROR("Switching back to LCPLL failed\n");
>> +
>> +	mutex_lock(&dev_priv->rps.hw_lock);
>> +	sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ, data);
>> +	mutex_unlock(&dev_priv->rps.hw_lock);
>> +
>> +	intel_update_cdclk(dev);
>> +
>> +	WARN(cdclk != dev_priv->cdclk_freq,
>> +	     "cdclk requested %d kHz but got %d kHz\n",
>> +	     cdclk, dev_priv->cdclk_freq);
>> +}
>> +
>> +static int broadwell_calc_cdclk(struct drm_i915_private *dev_priv,
>> +			      int max_pixel_rate)
>> +{
>> +	int cdclk;
>> +
>> +	/*
>> +	 * FIXME should also account for plane ratio
>> +	 * once 64bpp pixel formats are supported.
>> +	 */
>> +	if (max_pixel_rate > 540000)
>> +		cdclk = 675000;
>> +	else if (max_pixel_rate > 450000)
>> +		cdclk = 540000;
>> +	else if (max_pixel_rate > 337500)
>> +		cdclk = 450000;
>> +	else
>> +		cdclk = 337500;
>> +
>> +	/*
>> +	 * FIXME move the cdclk caclulation to
>> +	 * compute_config() so we can fail gracegully.
>> +	 */
>> +	if (cdclk > dev_priv->max_cdclk_freq) {
>> +		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
>> +			  cdclk, dev_priv->max_cdclk_freq);
>> +		cdclk = dev_priv->max_cdclk_freq;
>> +	}
>> +
>> +	return cdclk;
>> +}
>> +
>> +static int broadwell_modeset_global_pipes(struct drm_atomic_state *state)
>> +{
>> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
>> +	struct drm_crtc *crtc;
>> +	struct drm_crtc_state *crtc_state;
>> +	int max_pixclk = ilk_max_pixel_rate(dev_priv);
>> +	int cdclk, i;
>> +
>> +	cdclk = broadwell_calc_cdclk(dev_priv, max_pixclk);
>> +
>> +	if (cdclk == dev_priv->cdclk_freq)
>> +		return 0;
>> +
>> +	/* add all active pipes to the state */
>> +	for_each_crtc(state->dev, crtc) {
>> +		if (!crtc->state->enable)
>> +			continue;
>> +
>> +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
>> +		if (IS_ERR(crtc_state))
>> +			return PTR_ERR(crtc_state);
>> +	}
>> +
>> +	/* disable/enable all currently active pipes while we change cdclk */
>> +	for_each_crtc_in_state(state, crtc, crtc_state, i)
>> +		if (crtc_state->enable)
>> +			crtc_state->mode_changed = true;
>> +
>> +	return 0;
>> +}
>> +
>> +static void broadwell_modeset_global_resources(struct drm_atomic_state *state)
>> +{
>> +	struct drm_device *dev = state->dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
>> +	int req_cdclk = broadwell_calc_cdclk(dev_priv, max_pixel_rate);
>> +
>> +	if (req_cdclk != dev_priv->cdclk_freq)
>> +		broadwell_set_cdclk(dev, req_cdclk);
>> +}
>> +
>>  static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
>>  				      struct intel_crtc_state *crtc_state)
>>  {
>> @@ -12788,8 +12977,12 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
>>  	 * mode set on this crtc.  For other crtcs we need to use the
>>  	 * adjusted_mode bits in the crtc directly.
>>  	 */
>> -	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
>> -		ret = valleyview_modeset_global_pipes(state);
>> +	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_BROADWELL(dev)) {
>> +		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev))
>> +			ret = valleyview_modeset_global_pipes(state);
>> +		else
>> +			ret = broadwell_modeset_global_pipes(state);
>> +
>>  		if (ret)
>>  			return ret;
>>  	}
>> @@ -14677,6 +14870,9 @@ static void intel_init_display(struct drm_device *dev)
>>  		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
>>  	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>>  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
>> +		if (IS_BROADWELL(dev))
>> +			dev_priv->display.modeset_global_resources =
>> +				broadwell_modeset_global_resources;
>>  	} else if (IS_VALLEYVIEW(dev)) {
>>  		dev_priv->display.modeset_global_resources =
>>  			valleyview_modeset_global_resources;
>> -- 
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Jani Nikula, Intel Open Source Technology Center
On Tue, 16 Jun 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Tue, 16 Jun 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
>> On Wed, 03 Jun 2015, Mika Kahola <mika.kahola@intel.com> wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> Add support for changing cdclk frequency during runtime on BDW. The
>>> procedure is quite a bit different on BDW from the one on HSW, so
>>> add a separate function for it.
>>>
>>> Also with IPS enabled the actual pixel rate mustn't exceed 95% of cdclk,
>>> so take that into account when computing the max pixel rate.
>>>
>>> v2: Grab rps.hw_lock around sandybridge_pcode_write()
>>> v3: Rebase due to power well vs. .global_resources() reordering
>>> v4: Rebased to the latest
>>> v5: Rebased to the latest
>>> v6: Patch order shuffle so that Broadwell CD clock change is
>>>     applied before the patch for Haswell CD clock change
>>> v7: Fix for patch style problems
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
>>
>> This patch hard hangs my BDW NUC at boot when both DP and HDMI are
>> connected. Either DP or HDMI alone are good, same with hotplugging the
>> other afterwards. Booting to grub with both connected, and unplugging
>> HDMI before loading the kernel also reproduces the issue.
>>
>> It looks like the problem boils down to the BIOS setting up a smaller
>> resolution on the DP display when both are connected, and this patch
>> fails to cope with that on i915 load.
>
> By "this patch" I obviously refer to
>
> commit b432e5cfd5e92127ad2dd83bfc3083f1dbce43fb
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date:   Wed Jun 3 15:45:13 2015 +0300
>
>     drm/i915: BDW clock change support
>
> and everything works for the commit before that.

Anyone working on this, or should we revert?

BR,
Jani.


>
> BR,
> Jani.
>
>
>
>
>
>>
>> BR,
>> Jani.
>>
>>
>>
>>>
>>> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/i915_reg.h      |   2 +
>>>  drivers/gpu/drm/i915/intel_display.c | 216 +++++++++++++++++++++++++++++++++--
>>>  2 files changed, 208 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index 7213224..0f72c0e 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -6705,6 +6705,7 @@ enum skl_disp_power_wells {
>>>  #define	  GEN6_PCODE_READ_RC6VIDS		0x5
>>>  #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
>>>  #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
>>> +#define   BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ	0x18
>>>  #define   GEN9_PCODE_READ_MEM_LATENCY		0x6
>>>  #define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
>>>  #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
>>> @@ -7170,6 +7171,7 @@ enum skl_disp_power_wells {
>>>  #define  LCPLL_CLK_FREQ_337_5_BDW	(2<<26)
>>>  #define  LCPLL_CLK_FREQ_675_BDW		(3<<26)
>>>  #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
>>> +#define  LCPLL_ROOT_CD_CLOCK_DISABLE	(1<<24)
>>>  #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
>>>  #define  LCPLL_POWER_DOWN_ALLOW		(1<<22)
>>>  #define  LCPLL_CD_SOURCE_FCLK		(1<<21)
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index c3f01aa..b1e2069 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -5751,7 +5751,22 @@ static void intel_update_max_cdclk(struct drm_device *dev)
>>>  {
>>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>>  
>>> -	if (IS_VALLEYVIEW(dev)) {
>>> +	if (IS_BROADWELL(dev))  {
>>> +		/*
>>> +		 * FIXME with extra cooling we can allow
>>> +		 * 540 MHz for ULX and 675 Mhz for ULT.
>>> +		 * How can we know if extra cooling is
>>> +		 * available? PCI ID, VTB, something else?
>>> +		 */
>>> +		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
>>> +			dev_priv->max_cdclk_freq = 450000;
>>> +		else if (IS_BDW_ULX(dev))
>>> +			dev_priv->max_cdclk_freq = 450000;
>>> +		else if (IS_BDW_ULT(dev))
>>> +			dev_priv->max_cdclk_freq = 540000;
>>> +		else
>>> +			dev_priv->max_cdclk_freq = 675000;
>>> +	} else if (IS_VALLEYVIEW(dev)) {
>>>  		dev_priv->max_cdclk_freq = 400000;
>>>  	} else {
>>>  		/* otherwise assume cdclk is fixed */
>>> @@ -6621,13 +6636,11 @@ static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv,
>>>  		return true;
>>>  
>>>  	/*
>>> -	 * FIXME if we compare against max we should then
>>> -	 * increase the cdclk frequency when the current
>>> -	 * value is too low. The other option is to compare
>>> -	 * against the cdclk frequency we're going have post
>>> -	 * modeset (ie. one we computed using other constraints).
>>> -	 * Need to measure whether using a lower cdclk w/o IPS
>>> -	 * is better or worse than a higher cdclk w/ IPS.
>>> +	 * We compare against max which means we must take
>>> +	 * the increased cdclk requirement into account when
>>> +	 * calculating the new cdclk.
>>> +	 *
>>> +	 * Should measure whether using a lower cdclk w/o IPS
>>>  	 */
>>>  	return ilk_pipe_pixel_rate(pipe_config) <=
>>>  		dev_priv->max_cdclk_freq * 95 / 100;
>>> @@ -9608,6 +9621,182 @@ static void broxton_modeset_global_resources(struct drm_atomic_state *old_state)
>>>  		broxton_set_cdclk(dev, req_cdclk);
>>>  }
>>>  
>>> +/* compute the max rate for new configuration */
>>> +static int ilk_max_pixel_rate(struct drm_i915_private *dev_priv)
>>> +{
>>> +	struct drm_device *dev = dev_priv->dev;
>>> +	struct intel_crtc *intel_crtc;
>>> +	struct drm_crtc *crtc;
>>> +	int max_pixel_rate = 0;
>>> +	int pixel_rate;
>>> +
>>> +	for_each_crtc(dev, crtc) {
>>> +		if (!crtc->state->enable)
>>> +			continue;
>>> +
>>> +		intel_crtc = to_intel_crtc(crtc);
>>> +		pixel_rate = ilk_pipe_pixel_rate(intel_crtc->config);
>>> +
>>> +		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
>>> +		if (IS_BROADWELL(dev) && intel_crtc->config->ips_enabled)
>>> +			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
>>> +
>>> +		max_pixel_rate = max(max_pixel_rate, pixel_rate);
>>> +	}
>>> +
>>> +	return max_pixel_rate;
>>> +}
>>> +
>>> +static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
>>> +{
>>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>>> +	uint32_t val, data;
>>> +	int ret;
>>> +
>>> +	if (WARN((I915_READ(LCPLL_CTL) &
>>> +		  (LCPLL_PLL_DISABLE | LCPLL_PLL_LOCK |
>>> +		   LCPLL_CD_CLOCK_DISABLE | LCPLL_ROOT_CD_CLOCK_DISABLE |
>>> +		   LCPLL_CD2X_CLOCK_DISABLE | LCPLL_POWER_DOWN_ALLOW |
>>> +		   LCPLL_CD_SOURCE_FCLK)) != LCPLL_PLL_LOCK,
>>> +		 "trying to change cdclk frequency with cdclk not enabled\n"))
>>> +		return;
>>> +
>>> +	mutex_lock(&dev_priv->rps.hw_lock);
>>> +	ret = sandybridge_pcode_write(dev_priv,
>>> +				      BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
>>> +	mutex_unlock(&dev_priv->rps.hw_lock);
>>> +	if (ret) {
>>> +		DRM_ERROR("failed to inform pcode about cdclk change\n");
>>> +		return;
>>> +	}
>>> +
>>> +	val = I915_READ(LCPLL_CTL);
>>> +	val |= LCPLL_CD_SOURCE_FCLK;
>>> +	I915_WRITE(LCPLL_CTL, val);
>>> +
>>> +	if (wait_for_atomic_us(I915_READ(LCPLL_CTL) &
>>> +			       LCPLL_CD_SOURCE_FCLK_DONE, 1))
>>> +		DRM_ERROR("Switching to FCLK failed\n");
>>> +
>>> +	val = I915_READ(LCPLL_CTL);
>>> +	val &= ~LCPLL_CLK_FREQ_MASK;
>>> +
>>> +	switch (cdclk) {
>>> +	case 450000:
>>> +		val |= LCPLL_CLK_FREQ_450;
>>> +		data = 0;
>>> +		break;
>>> +	case 540000:
>>> +		val |= LCPLL_CLK_FREQ_54O_BDW;
>>> +		data = 1;
>>> +		break;
>>> +	case 337500:
>>> +		val |= LCPLL_CLK_FREQ_337_5_BDW;
>>> +		data = 2;
>>> +		break;
>>> +	case 675000:
>>> +		val |= LCPLL_CLK_FREQ_675_BDW;
>>> +		data = 3;
>>> +		break;
>>> +	default:
>>> +		WARN(1, "invalid cdclk frequency\n");
>>> +		return;
>>> +	}
>>> +
>>> +	I915_WRITE(LCPLL_CTL, val);
>>> +
>>> +	val = I915_READ(LCPLL_CTL);
>>> +	val &= ~LCPLL_CD_SOURCE_FCLK;
>>> +	I915_WRITE(LCPLL_CTL, val);
>>> +
>>> +	if (wait_for_atomic_us((I915_READ(LCPLL_CTL) &
>>> +				LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
>>> +		DRM_ERROR("Switching back to LCPLL failed\n");
>>> +
>>> +	mutex_lock(&dev_priv->rps.hw_lock);
>>> +	sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ, data);
>>> +	mutex_unlock(&dev_priv->rps.hw_lock);
>>> +
>>> +	intel_update_cdclk(dev);
>>> +
>>> +	WARN(cdclk != dev_priv->cdclk_freq,
>>> +	     "cdclk requested %d kHz but got %d kHz\n",
>>> +	     cdclk, dev_priv->cdclk_freq);
>>> +}
>>> +
>>> +static int broadwell_calc_cdclk(struct drm_i915_private *dev_priv,
>>> +			      int max_pixel_rate)
>>> +{
>>> +	int cdclk;
>>> +
>>> +	/*
>>> +	 * FIXME should also account for plane ratio
>>> +	 * once 64bpp pixel formats are supported.
>>> +	 */
>>> +	if (max_pixel_rate > 540000)
>>> +		cdclk = 675000;
>>> +	else if (max_pixel_rate > 450000)
>>> +		cdclk = 540000;
>>> +	else if (max_pixel_rate > 337500)
>>> +		cdclk = 450000;
>>> +	else
>>> +		cdclk = 337500;
>>> +
>>> +	/*
>>> +	 * FIXME move the cdclk caclulation to
>>> +	 * compute_config() so we can fail gracegully.
>>> +	 */
>>> +	if (cdclk > dev_priv->max_cdclk_freq) {
>>> +		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
>>> +			  cdclk, dev_priv->max_cdclk_freq);
>>> +		cdclk = dev_priv->max_cdclk_freq;
>>> +	}
>>> +
>>> +	return cdclk;
>>> +}
>>> +
>>> +static int broadwell_modeset_global_pipes(struct drm_atomic_state *state)
>>> +{
>>> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
>>> +	struct drm_crtc *crtc;
>>> +	struct drm_crtc_state *crtc_state;
>>> +	int max_pixclk = ilk_max_pixel_rate(dev_priv);
>>> +	int cdclk, i;
>>> +
>>> +	cdclk = broadwell_calc_cdclk(dev_priv, max_pixclk);
>>> +
>>> +	if (cdclk == dev_priv->cdclk_freq)
>>> +		return 0;
>>> +
>>> +	/* add all active pipes to the state */
>>> +	for_each_crtc(state->dev, crtc) {
>>> +		if (!crtc->state->enable)
>>> +			continue;
>>> +
>>> +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
>>> +		if (IS_ERR(crtc_state))
>>> +			return PTR_ERR(crtc_state);
>>> +	}
>>> +
>>> +	/* disable/enable all currently active pipes while we change cdclk */
>>> +	for_each_crtc_in_state(state, crtc, crtc_state, i)
>>> +		if (crtc_state->enable)
>>> +			crtc_state->mode_changed = true;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static void broadwell_modeset_global_resources(struct drm_atomic_state *state)
>>> +{
>>> +	struct drm_device *dev = state->dev;
>>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>>> +	int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
>>> +	int req_cdclk = broadwell_calc_cdclk(dev_priv, max_pixel_rate);
>>> +
>>> +	if (req_cdclk != dev_priv->cdclk_freq)
>>> +		broadwell_set_cdclk(dev, req_cdclk);
>>> +}
>>> +
>>>  static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
>>>  				      struct intel_crtc_state *crtc_state)
>>>  {
>>> @@ -12788,8 +12977,12 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
>>>  	 * mode set on this crtc.  For other crtcs we need to use the
>>>  	 * adjusted_mode bits in the crtc directly.
>>>  	 */
>>> -	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
>>> -		ret = valleyview_modeset_global_pipes(state);
>>> +	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_BROADWELL(dev)) {
>>> +		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev))
>>> +			ret = valleyview_modeset_global_pipes(state);
>>> +		else
>>> +			ret = broadwell_modeset_global_pipes(state);
>>> +
>>>  		if (ret)
>>>  			return ret;
>>>  	}
>>> @@ -14677,6 +14870,9 @@ static void intel_init_display(struct drm_device *dev)
>>>  		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
>>>  	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>>>  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
>>> +		if (IS_BROADWELL(dev))
>>> +			dev_priv->display.modeset_global_resources =
>>> +				broadwell_modeset_global_resources;
>>>  	} else if (IS_VALLEYVIEW(dev)) {
>>>  		dev_priv->display.modeset_global_resources =
>>>  			valleyview_modeset_global_resources;
>>> -- 
>>> 1.9.1
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> -- 
>> Jani Nikula, Intel Open Source Technology Center
>
> -- 
> Jani Nikula, Intel Open Source Technology Center
On Mon, 2015-06-29 at 14:24 +0300, Jani Nikula wrote:
> On Tue, 16 Jun 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > On Tue, 16 Jun 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> >> On Wed, 03 Jun 2015, Mika Kahola <mika.kahola@intel.com> wrote:
> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>
> >>> Add support for changing cdclk frequency during runtime on BDW. The
> >>> procedure is quite a bit different on BDW from the one on HSW, so
> >>> add a separate function for it.
> >>>
> >>> Also with IPS enabled the actual pixel rate mustn't exceed 95% of cdclk,
> >>> so take that into account when computing the max pixel rate.
> >>>
> >>> v2: Grab rps.hw_lock around sandybridge_pcode_write()
> >>> v3: Rebase due to power well vs. .global_resources() reordering
> >>> v4: Rebased to the latest
> >>> v5: Rebased to the latest
> >>> v6: Patch order shuffle so that Broadwell CD clock change is
> >>>     applied before the patch for Haswell CD clock change
> >>> v7: Fix for patch style problems
> >>>
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> >>
> >> This patch hard hangs my BDW NUC at boot when both DP and HDMI are
> >> connected. Either DP or HDMI alone are good, same with hotplugging the
> >> other afterwards. Booting to grub with both connected, and unplugging
> >> HDMI before loading the kernel also reproduces the issue.
> >>
> >> It looks like the problem boils down to the BIOS setting up a smaller
> >> resolution on the DP display when both are connected, and this patch
> >> fails to cope with that on i915 load.
> >
> > By "this patch" I obviously refer to
> >
> > commit b432e5cfd5e92127ad2dd83bfc3083f1dbce43fb
> > Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Date:   Wed Jun 3 15:45:13 2015 +0300
> >
> >     drm/i915: BDW clock change support
> >
> > and everything works for the commit before that.
> 
> Anyone working on this, or should we revert?
> 
> BR,
> Jani.
> 
My understanding is that Ville has an idea or how to fix this.

Cheers,
Mika

> 
> >
> > BR,
> > Jani.
> >
> >
> >
> >
> >
> >>
> >> BR,
> >> Jani.
> >>
> >>
> >>
> >>>
> >>> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> ---
> >>>  drivers/gpu/drm/i915/i915_reg.h      |   2 +
> >>>  drivers/gpu/drm/i915/intel_display.c | 216 +++++++++++++++++++++++++++++++++--
> >>>  2 files changed, 208 insertions(+), 10 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >>> index 7213224..0f72c0e 100644
> >>> --- a/drivers/gpu/drm/i915/i915_reg.h
> >>> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >>> @@ -6705,6 +6705,7 @@ enum skl_disp_power_wells {
> >>>  #define	  GEN6_PCODE_READ_RC6VIDS		0x5
> >>>  #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
> >>>  #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
> >>> +#define   BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ	0x18
> >>>  #define   GEN9_PCODE_READ_MEM_LATENCY		0x6
> >>>  #define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
> >>>  #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
> >>> @@ -7170,6 +7171,7 @@ enum skl_disp_power_wells {
> >>>  #define  LCPLL_CLK_FREQ_337_5_BDW	(2<<26)
> >>>  #define  LCPLL_CLK_FREQ_675_BDW		(3<<26)
> >>>  #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
> >>> +#define  LCPLL_ROOT_CD_CLOCK_DISABLE	(1<<24)
> >>>  #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
> >>>  #define  LCPLL_POWER_DOWN_ALLOW		(1<<22)
> >>>  #define  LCPLL_CD_SOURCE_FCLK		(1<<21)
> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>> index c3f01aa..b1e2069 100644
> >>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>> @@ -5751,7 +5751,22 @@ static void intel_update_max_cdclk(struct drm_device *dev)
> >>>  {
> >>>  	struct drm_i915_private *dev_priv = dev->dev_private;
> >>>  
> >>> -	if (IS_VALLEYVIEW(dev)) {
> >>> +	if (IS_BROADWELL(dev))  {
> >>> +		/*
> >>> +		 * FIXME with extra cooling we can allow
> >>> +		 * 540 MHz for ULX and 675 Mhz for ULT.
> >>> +		 * How can we know if extra cooling is
> >>> +		 * available? PCI ID, VTB, something else?
> >>> +		 */
> >>> +		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
> >>> +			dev_priv->max_cdclk_freq = 450000;
> >>> +		else if (IS_BDW_ULX(dev))
> >>> +			dev_priv->max_cdclk_freq = 450000;
> >>> +		else if (IS_BDW_ULT(dev))
> >>> +			dev_priv->max_cdclk_freq = 540000;
> >>> +		else
> >>> +			dev_priv->max_cdclk_freq = 675000;
> >>> +	} else if (IS_VALLEYVIEW(dev)) {
> >>>  		dev_priv->max_cdclk_freq = 400000;
> >>>  	} else {
> >>>  		/* otherwise assume cdclk is fixed */
> >>> @@ -6621,13 +6636,11 @@ static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv,
> >>>  		return true;
> >>>  
> >>>  	/*
> >>> -	 * FIXME if we compare against max we should then
> >>> -	 * increase the cdclk frequency when the current
> >>> -	 * value is too low. The other option is to compare
> >>> -	 * against the cdclk frequency we're going have post
> >>> -	 * modeset (ie. one we computed using other constraints).
> >>> -	 * Need to measure whether using a lower cdclk w/o IPS
> >>> -	 * is better or worse than a higher cdclk w/ IPS.
> >>> +	 * We compare against max which means we must take
> >>> +	 * the increased cdclk requirement into account when
> >>> +	 * calculating the new cdclk.
> >>> +	 *
> >>> +	 * Should measure whether using a lower cdclk w/o IPS
> >>>  	 */
> >>>  	return ilk_pipe_pixel_rate(pipe_config) <=
> >>>  		dev_priv->max_cdclk_freq * 95 / 100;
> >>> @@ -9608,6 +9621,182 @@ static void broxton_modeset_global_resources(struct drm_atomic_state *old_state)
> >>>  		broxton_set_cdclk(dev, req_cdclk);
> >>>  }
> >>>  
> >>> +/* compute the max rate for new configuration */
> >>> +static int ilk_max_pixel_rate(struct drm_i915_private *dev_priv)
> >>> +{
> >>> +	struct drm_device *dev = dev_priv->dev;
> >>> +	struct intel_crtc *intel_crtc;
> >>> +	struct drm_crtc *crtc;
> >>> +	int max_pixel_rate = 0;
> >>> +	int pixel_rate;
> >>> +
> >>> +	for_each_crtc(dev, crtc) {
> >>> +		if (!crtc->state->enable)
> >>> +			continue;
> >>> +
> >>> +		intel_crtc = to_intel_crtc(crtc);
> >>> +		pixel_rate = ilk_pipe_pixel_rate(intel_crtc->config);
> >>> +
> >>> +		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> >>> +		if (IS_BROADWELL(dev) && intel_crtc->config->ips_enabled)
> >>> +			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> >>> +
> >>> +		max_pixel_rate = max(max_pixel_rate, pixel_rate);
> >>> +	}
> >>> +
> >>> +	return max_pixel_rate;
> >>> +}
> >>> +
> >>> +static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
> >>> +{
> >>> +	struct drm_i915_private *dev_priv = dev->dev_private;
> >>> +	uint32_t val, data;
> >>> +	int ret;
> >>> +
> >>> +	if (WARN((I915_READ(LCPLL_CTL) &
> >>> +		  (LCPLL_PLL_DISABLE | LCPLL_PLL_LOCK |
> >>> +		   LCPLL_CD_CLOCK_DISABLE | LCPLL_ROOT_CD_CLOCK_DISABLE |
> >>> +		   LCPLL_CD2X_CLOCK_DISABLE | LCPLL_POWER_DOWN_ALLOW |
> >>> +		   LCPLL_CD_SOURCE_FCLK)) != LCPLL_PLL_LOCK,
> >>> +		 "trying to change cdclk frequency with cdclk not enabled\n"))
> >>> +		return;
> >>> +
> >>> +	mutex_lock(&dev_priv->rps.hw_lock);
> >>> +	ret = sandybridge_pcode_write(dev_priv,
> >>> +				      BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
> >>> +	mutex_unlock(&dev_priv->rps.hw_lock);
> >>> +	if (ret) {
> >>> +		DRM_ERROR("failed to inform pcode about cdclk change\n");
> >>> +		return;
> >>> +	}
> >>> +
> >>> +	val = I915_READ(LCPLL_CTL);
> >>> +	val |= LCPLL_CD_SOURCE_FCLK;
> >>> +	I915_WRITE(LCPLL_CTL, val);
> >>> +
> >>> +	if (wait_for_atomic_us(I915_READ(LCPLL_CTL) &
> >>> +			       LCPLL_CD_SOURCE_FCLK_DONE, 1))
> >>> +		DRM_ERROR("Switching to FCLK failed\n");
> >>> +
> >>> +	val = I915_READ(LCPLL_CTL);
> >>> +	val &= ~LCPLL_CLK_FREQ_MASK;
> >>> +
> >>> +	switch (cdclk) {
> >>> +	case 450000:
> >>> +		val |= LCPLL_CLK_FREQ_450;
> >>> +		data = 0;
> >>> +		break;
> >>> +	case 540000:
> >>> +		val |= LCPLL_CLK_FREQ_54O_BDW;
> >>> +		data = 1;
> >>> +		break;
> >>> +	case 337500:
> >>> +		val |= LCPLL_CLK_FREQ_337_5_BDW;
> >>> +		data = 2;
> >>> +		break;
> >>> +	case 675000:
> >>> +		val |= LCPLL_CLK_FREQ_675_BDW;
> >>> +		data = 3;
> >>> +		break;
> >>> +	default:
> >>> +		WARN(1, "invalid cdclk frequency\n");
> >>> +		return;
> >>> +	}
> >>> +
> >>> +	I915_WRITE(LCPLL_CTL, val);
> >>> +
> >>> +	val = I915_READ(LCPLL_CTL);
> >>> +	val &= ~LCPLL_CD_SOURCE_FCLK;
> >>> +	I915_WRITE(LCPLL_CTL, val);
> >>> +
> >>> +	if (wait_for_atomic_us((I915_READ(LCPLL_CTL) &
> >>> +				LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
> >>> +		DRM_ERROR("Switching back to LCPLL failed\n");
> >>> +
> >>> +	mutex_lock(&dev_priv->rps.hw_lock);
> >>> +	sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ, data);
> >>> +	mutex_unlock(&dev_priv->rps.hw_lock);
> >>> +
> >>> +	intel_update_cdclk(dev);
> >>> +
> >>> +	WARN(cdclk != dev_priv->cdclk_freq,
> >>> +	     "cdclk requested %d kHz but got %d kHz\n",
> >>> +	     cdclk, dev_priv->cdclk_freq);
> >>> +}
> >>> +
> >>> +static int broadwell_calc_cdclk(struct drm_i915_private *dev_priv,
> >>> +			      int max_pixel_rate)
> >>> +{
> >>> +	int cdclk;
> >>> +
> >>> +	/*
> >>> +	 * FIXME should also account for plane ratio
> >>> +	 * once 64bpp pixel formats are supported.
> >>> +	 */
> >>> +	if (max_pixel_rate > 540000)
> >>> +		cdclk = 675000;
> >>> +	else if (max_pixel_rate > 450000)
> >>> +		cdclk = 540000;
> >>> +	else if (max_pixel_rate > 337500)
> >>> +		cdclk = 450000;
> >>> +	else
> >>> +		cdclk = 337500;
> >>> +
> >>> +	/*
> >>> +	 * FIXME move the cdclk caclulation to
> >>> +	 * compute_config() so we can fail gracegully.
> >>> +	 */
> >>> +	if (cdclk > dev_priv->max_cdclk_freq) {
> >>> +		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> >>> +			  cdclk, dev_priv->max_cdclk_freq);
> >>> +		cdclk = dev_priv->max_cdclk_freq;
> >>> +	}
> >>> +
> >>> +	return cdclk;
> >>> +}
> >>> +
> >>> +static int broadwell_modeset_global_pipes(struct drm_atomic_state *state)
> >>> +{
> >>> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> >>> +	struct drm_crtc *crtc;
> >>> +	struct drm_crtc_state *crtc_state;
> >>> +	int max_pixclk = ilk_max_pixel_rate(dev_priv);
> >>> +	int cdclk, i;
> >>> +
> >>> +	cdclk = broadwell_calc_cdclk(dev_priv, max_pixclk);
> >>> +
> >>> +	if (cdclk == dev_priv->cdclk_freq)
> >>> +		return 0;
> >>> +
> >>> +	/* add all active pipes to the state */
> >>> +	for_each_crtc(state->dev, crtc) {
> >>> +		if (!crtc->state->enable)
> >>> +			continue;
> >>> +
> >>> +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> >>> +		if (IS_ERR(crtc_state))
> >>> +			return PTR_ERR(crtc_state);
> >>> +	}
> >>> +
> >>> +	/* disable/enable all currently active pipes while we change cdclk */
> >>> +	for_each_crtc_in_state(state, crtc, crtc_state, i)
> >>> +		if (crtc_state->enable)
> >>> +			crtc_state->mode_changed = true;
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +
> >>> +static void broadwell_modeset_global_resources(struct drm_atomic_state *state)
> >>> +{
> >>> +	struct drm_device *dev = state->dev;
> >>> +	struct drm_i915_private *dev_priv = dev->dev_private;
> >>> +	int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
> >>> +	int req_cdclk = broadwell_calc_cdclk(dev_priv, max_pixel_rate);
> >>> +
> >>> +	if (req_cdclk != dev_priv->cdclk_freq)
> >>> +		broadwell_set_cdclk(dev, req_cdclk);
> >>> +}
> >>> +
> >>>  static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
> >>>  				      struct intel_crtc_state *crtc_state)
> >>>  {
> >>> @@ -12788,8 +12977,12 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
> >>>  	 * mode set on this crtc.  For other crtcs we need to use the
> >>>  	 * adjusted_mode bits in the crtc directly.
> >>>  	 */
> >>> -	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
> >>> -		ret = valleyview_modeset_global_pipes(state);
> >>> +	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_BROADWELL(dev)) {
> >>> +		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev))
> >>> +			ret = valleyview_modeset_global_pipes(state);
> >>> +		else
> >>> +			ret = broadwell_modeset_global_pipes(state);
> >>> +
> >>>  		if (ret)
> >>>  			return ret;
> >>>  	}
> >>> @@ -14677,6 +14870,9 @@ static void intel_init_display(struct drm_device *dev)
> >>>  		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
> >>>  	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> >>>  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
> >>> +		if (IS_BROADWELL(dev))
> >>> +			dev_priv->display.modeset_global_resources =
> >>> +				broadwell_modeset_global_resources;
> >>>  	} else if (IS_VALLEYVIEW(dev)) {
> >>>  		dev_priv->display.modeset_global_resources =
> >>>  			valleyview_modeset_global_resources;
> >>> -- 
> >>> 1.9.1
> >>>
> >>> _______________________________________________
> >>> Intel-gfx mailing list
> >>> Intel-gfx@lists.freedesktop.org
> >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >>
> >> -- 
> >> Jani Nikula, Intel Open Source Technology Center
> >
> > -- 
> > Jani Nikula, Intel Open Source Technology Center
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
On Mon, 29 Jun 2015, Mika Kahola <mika.kahola@intel.com> wrote:
> On Mon, 2015-06-29 at 14:24 +0300, Jani Nikula wrote:
>> On Tue, 16 Jun 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
>> > On Tue, 16 Jun 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
>> >> On Wed, 03 Jun 2015, Mika Kahola <mika.kahola@intel.com> wrote:
>> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >>>
>> >>> Add support for changing cdclk frequency during runtime on BDW. The
>> >>> procedure is quite a bit different on BDW from the one on HSW, so
>> >>> add a separate function for it.
>> >>>
>> >>> Also with IPS enabled the actual pixel rate mustn't exceed 95% of cdclk,
>> >>> so take that into account when computing the max pixel rate.
>> >>>
>> >>> v2: Grab rps.hw_lock around sandybridge_pcode_write()
>> >>> v3: Rebase due to power well vs. .global_resources() reordering
>> >>> v4: Rebased to the latest
>> >>> v5: Rebased to the latest
>> >>> v6: Patch order shuffle so that Broadwell CD clock change is
>> >>>     applied before the patch for Haswell CD clock change
>> >>> v7: Fix for patch style problems
>> >>>
>> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >>> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
>> >>
>> >> This patch hard hangs my BDW NUC at boot when both DP and HDMI are
>> >> connected. Either DP or HDMI alone are good, same with hotplugging the
>> >> other afterwards. Booting to grub with both connected, and unplugging
>> >> HDMI before loading the kernel also reproduces the issue.
>> >>
>> >> It looks like the problem boils down to the BIOS setting up a smaller
>> >> resolution on the DP display when both are connected, and this patch
>> >> fails to cope with that on i915 load.
>> >
>> > By "this patch" I obviously refer to
>> >
>> > commit b432e5cfd5e92127ad2dd83bfc3083f1dbce43fb
>> > Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > Date:   Wed Jun 3 15:45:13 2015 +0300
>> >
>> >     drm/i915: BDW clock change support
>> >
>> > and everything works for the commit before that.
>> 
>> Anyone working on this, or should we revert?
>> 
>> BR,
>> Jani.
>> 
> My understanding is that Ville has an idea or how to fix this.

Ville, when do you think we can convert the idea into a patch? Should we
revert in the mean time?

BR,
Jani.



>
> Cheers,
> Mika
>
>> 
>> >
>> > BR,
>> > Jani.
>> >
>> >
>> >
>> >
>> >
>> >>
>> >> BR,
>> >> Jani.
>> >>
>> >>
>> >>
>> >>>
>> >>> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >>> ---
>> >>>  drivers/gpu/drm/i915/i915_reg.h      |   2 +
>> >>>  drivers/gpu/drm/i915/intel_display.c | 216 +++++++++++++++++++++++++++++++++--
>> >>>  2 files changed, 208 insertions(+), 10 deletions(-)
>> >>>
>> >>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> >>> index 7213224..0f72c0e 100644
>> >>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> >>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> >>> @@ -6705,6 +6705,7 @@ enum skl_disp_power_wells {
>> >>>  #define	  GEN6_PCODE_READ_RC6VIDS		0x5
>> >>>  #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
>> >>>  #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
>> >>> +#define   BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ	0x18
>> >>>  #define   GEN9_PCODE_READ_MEM_LATENCY		0x6
>> >>>  #define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
>> >>>  #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
>> >>> @@ -7170,6 +7171,7 @@ enum skl_disp_power_wells {
>> >>>  #define  LCPLL_CLK_FREQ_337_5_BDW	(2<<26)
>> >>>  #define  LCPLL_CLK_FREQ_675_BDW		(3<<26)
>> >>>  #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
>> >>> +#define  LCPLL_ROOT_CD_CLOCK_DISABLE	(1<<24)
>> >>>  #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
>> >>>  #define  LCPLL_POWER_DOWN_ALLOW		(1<<22)
>> >>>  #define  LCPLL_CD_SOURCE_FCLK		(1<<21)
>> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> >>> index c3f01aa..b1e2069 100644
>> >>> --- a/drivers/gpu/drm/i915/intel_display.c
>> >>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> >>> @@ -5751,7 +5751,22 @@ static void intel_update_max_cdclk(struct drm_device *dev)
>> >>>  {
>> >>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>> >>>  
>> >>> -	if (IS_VALLEYVIEW(dev)) {
>> >>> +	if (IS_BROADWELL(dev))  {
>> >>> +		/*
>> >>> +		 * FIXME with extra cooling we can allow
>> >>> +		 * 540 MHz for ULX and 675 Mhz for ULT.
>> >>> +		 * How can we know if extra cooling is
>> >>> +		 * available? PCI ID, VTB, something else?
>> >>> +		 */
>> >>> +		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
>> >>> +			dev_priv->max_cdclk_freq = 450000;
>> >>> +		else if (IS_BDW_ULX(dev))
>> >>> +			dev_priv->max_cdclk_freq = 450000;
>> >>> +		else if (IS_BDW_ULT(dev))
>> >>> +			dev_priv->max_cdclk_freq = 540000;
>> >>> +		else
>> >>> +			dev_priv->max_cdclk_freq = 675000;
>> >>> +	} else if (IS_VALLEYVIEW(dev)) {
>> >>>  		dev_priv->max_cdclk_freq = 400000;
>> >>>  	} else {
>> >>>  		/* otherwise assume cdclk is fixed */
>> >>> @@ -6621,13 +6636,11 @@ static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv,
>> >>>  		return true;
>> >>>  
>> >>>  	/*
>> >>> -	 * FIXME if we compare against max we should then
>> >>> -	 * increase the cdclk frequency when the current
>> >>> -	 * value is too low. The other option is to compare
>> >>> -	 * against the cdclk frequency we're going have post
>> >>> -	 * modeset (ie. one we computed using other constraints).
>> >>> -	 * Need to measure whether using a lower cdclk w/o IPS
>> >>> -	 * is better or worse than a higher cdclk w/ IPS.
>> >>> +	 * We compare against max which means we must take
>> >>> +	 * the increased cdclk requirement into account when
>> >>> +	 * calculating the new cdclk.
>> >>> +	 *
>> >>> +	 * Should measure whether using a lower cdclk w/o IPS
>> >>>  	 */
>> >>>  	return ilk_pipe_pixel_rate(pipe_config) <=
>> >>>  		dev_priv->max_cdclk_freq * 95 / 100;
>> >>> @@ -9608,6 +9621,182 @@ static void broxton_modeset_global_resources(struct drm_atomic_state *old_state)
>> >>>  		broxton_set_cdclk(dev, req_cdclk);
>> >>>  }
>> >>>  
>> >>> +/* compute the max rate for new configuration */
>> >>> +static int ilk_max_pixel_rate(struct drm_i915_private *dev_priv)
>> >>> +{
>> >>> +	struct drm_device *dev = dev_priv->dev;
>> >>> +	struct intel_crtc *intel_crtc;
>> >>> +	struct drm_crtc *crtc;
>> >>> +	int max_pixel_rate = 0;
>> >>> +	int pixel_rate;
>> >>> +
>> >>> +	for_each_crtc(dev, crtc) {
>> >>> +		if (!crtc->state->enable)
>> >>> +			continue;
>> >>> +
>> >>> +		intel_crtc = to_intel_crtc(crtc);
>> >>> +		pixel_rate = ilk_pipe_pixel_rate(intel_crtc->config);
>> >>> +
>> >>> +		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
>> >>> +		if (IS_BROADWELL(dev) && intel_crtc->config->ips_enabled)
>> >>> +			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
>> >>> +
>> >>> +		max_pixel_rate = max(max_pixel_rate, pixel_rate);
>> >>> +	}
>> >>> +
>> >>> +	return max_pixel_rate;
>> >>> +}
>> >>> +
>> >>> +static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
>> >>> +{
>> >>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> >>> +	uint32_t val, data;
>> >>> +	int ret;
>> >>> +
>> >>> +	if (WARN((I915_READ(LCPLL_CTL) &
>> >>> +		  (LCPLL_PLL_DISABLE | LCPLL_PLL_LOCK |
>> >>> +		   LCPLL_CD_CLOCK_DISABLE | LCPLL_ROOT_CD_CLOCK_DISABLE |
>> >>> +		   LCPLL_CD2X_CLOCK_DISABLE | LCPLL_POWER_DOWN_ALLOW |
>> >>> +		   LCPLL_CD_SOURCE_FCLK)) != LCPLL_PLL_LOCK,
>> >>> +		 "trying to change cdclk frequency with cdclk not enabled\n"))
>> >>> +		return;
>> >>> +
>> >>> +	mutex_lock(&dev_priv->rps.hw_lock);
>> >>> +	ret = sandybridge_pcode_write(dev_priv,
>> >>> +				      BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
>> >>> +	mutex_unlock(&dev_priv->rps.hw_lock);
>> >>> +	if (ret) {
>> >>> +		DRM_ERROR("failed to inform pcode about cdclk change\n");
>> >>> +		return;
>> >>> +	}
>> >>> +
>> >>> +	val = I915_READ(LCPLL_CTL);
>> >>> +	val |= LCPLL_CD_SOURCE_FCLK;
>> >>> +	I915_WRITE(LCPLL_CTL, val);
>> >>> +
>> >>> +	if (wait_for_atomic_us(I915_READ(LCPLL_CTL) &
>> >>> +			       LCPLL_CD_SOURCE_FCLK_DONE, 1))
>> >>> +		DRM_ERROR("Switching to FCLK failed\n");
>> >>> +
>> >>> +	val = I915_READ(LCPLL_CTL);
>> >>> +	val &= ~LCPLL_CLK_FREQ_MASK;
>> >>> +
>> >>> +	switch (cdclk) {
>> >>> +	case 450000:
>> >>> +		val |= LCPLL_CLK_FREQ_450;
>> >>> +		data = 0;
>> >>> +		break;
>> >>> +	case 540000:
>> >>> +		val |= LCPLL_CLK_FREQ_54O_BDW;
>> >>> +		data = 1;
>> >>> +		break;
>> >>> +	case 337500:
>> >>> +		val |= LCPLL_CLK_FREQ_337_5_BDW;
>> >>> +		data = 2;
>> >>> +		break;
>> >>> +	case 675000:
>> >>> +		val |= LCPLL_CLK_FREQ_675_BDW;
>> >>> +		data = 3;
>> >>> +		break;
>> >>> +	default:
>> >>> +		WARN(1, "invalid cdclk frequency\n");
>> >>> +		return;
>> >>> +	}
>> >>> +
>> >>> +	I915_WRITE(LCPLL_CTL, val);
>> >>> +
>> >>> +	val = I915_READ(LCPLL_CTL);
>> >>> +	val &= ~LCPLL_CD_SOURCE_FCLK;
>> >>> +	I915_WRITE(LCPLL_CTL, val);
>> >>> +
>> >>> +	if (wait_for_atomic_us((I915_READ(LCPLL_CTL) &
>> >>> +				LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
>> >>> +		DRM_ERROR("Switching back to LCPLL failed\n");
>> >>> +
>> >>> +	mutex_lock(&dev_priv->rps.hw_lock);
>> >>> +	sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ, data);
>> >>> +	mutex_unlock(&dev_priv->rps.hw_lock);
>> >>> +
>> >>> +	intel_update_cdclk(dev);
>> >>> +
>> >>> +	WARN(cdclk != dev_priv->cdclk_freq,
>> >>> +	     "cdclk requested %d kHz but got %d kHz\n",
>> >>> +	     cdclk, dev_priv->cdclk_freq);
>> >>> +}
>> >>> +
>> >>> +static int broadwell_calc_cdclk(struct drm_i915_private *dev_priv,
>> >>> +			      int max_pixel_rate)
>> >>> +{
>> >>> +	int cdclk;
>> >>> +
>> >>> +	/*
>> >>> +	 * FIXME should also account for plane ratio
>> >>> +	 * once 64bpp pixel formats are supported.
>> >>> +	 */
>> >>> +	if (max_pixel_rate > 540000)
>> >>> +		cdclk = 675000;
>> >>> +	else if (max_pixel_rate > 450000)
>> >>> +		cdclk = 540000;
>> >>> +	else if (max_pixel_rate > 337500)
>> >>> +		cdclk = 450000;
>> >>> +	else
>> >>> +		cdclk = 337500;
>> >>> +
>> >>> +	/*
>> >>> +	 * FIXME move the cdclk caclulation to
>> >>> +	 * compute_config() so we can fail gracegully.
>> >>> +	 */
>> >>> +	if (cdclk > dev_priv->max_cdclk_freq) {
>> >>> +		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
>> >>> +			  cdclk, dev_priv->max_cdclk_freq);
>> >>> +		cdclk = dev_priv->max_cdclk_freq;
>> >>> +	}
>> >>> +
>> >>> +	return cdclk;
>> >>> +}
>> >>> +
>> >>> +static int broadwell_modeset_global_pipes(struct drm_atomic_state *state)
>> >>> +{
>> >>> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
>> >>> +	struct drm_crtc *crtc;
>> >>> +	struct drm_crtc_state *crtc_state;
>> >>> +	int max_pixclk = ilk_max_pixel_rate(dev_priv);
>> >>> +	int cdclk, i;
>> >>> +
>> >>> +	cdclk = broadwell_calc_cdclk(dev_priv, max_pixclk);
>> >>> +
>> >>> +	if (cdclk == dev_priv->cdclk_freq)
>> >>> +		return 0;
>> >>> +
>> >>> +	/* add all active pipes to the state */
>> >>> +	for_each_crtc(state->dev, crtc) {
>> >>> +		if (!crtc->state->enable)
>> >>> +			continue;
>> >>> +
>> >>> +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
>> >>> +		if (IS_ERR(crtc_state))
>> >>> +			return PTR_ERR(crtc_state);
>> >>> +	}
>> >>> +
>> >>> +	/* disable/enable all currently active pipes while we change cdclk */
>> >>> +	for_each_crtc_in_state(state, crtc, crtc_state, i)
>> >>> +		if (crtc_state->enable)
>> >>> +			crtc_state->mode_changed = true;
>> >>> +
>> >>> +	return 0;
>> >>> +}
>> >>> +
>> >>> +static void broadwell_modeset_global_resources(struct drm_atomic_state *state)
>> >>> +{
>> >>> +	struct drm_device *dev = state->dev;
>> >>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> >>> +	int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
>> >>> +	int req_cdclk = broadwell_calc_cdclk(dev_priv, max_pixel_rate);
>> >>> +
>> >>> +	if (req_cdclk != dev_priv->cdclk_freq)
>> >>> +		broadwell_set_cdclk(dev, req_cdclk);
>> >>> +}
>> >>> +
>> >>>  static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
>> >>>  				      struct intel_crtc_state *crtc_state)
>> >>>  {
>> >>> @@ -12788,8 +12977,12 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
>> >>>  	 * mode set on this crtc.  For other crtcs we need to use the
>> >>>  	 * adjusted_mode bits in the crtc directly.
>> >>>  	 */
>> >>> -	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
>> >>> -		ret = valleyview_modeset_global_pipes(state);
>> >>> +	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_BROADWELL(dev)) {
>> >>> +		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev))
>> >>> +			ret = valleyview_modeset_global_pipes(state);
>> >>> +		else
>> >>> +			ret = broadwell_modeset_global_pipes(state);
>> >>> +
>> >>>  		if (ret)
>> >>>  			return ret;
>> >>>  	}
>> >>> @@ -14677,6 +14870,9 @@ static void intel_init_display(struct drm_device *dev)
>> >>>  		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
>> >>>  	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>> >>>  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
>> >>> +		if (IS_BROADWELL(dev))
>> >>> +			dev_priv->display.modeset_global_resources =
>> >>> +				broadwell_modeset_global_resources;
>> >>>  	} else if (IS_VALLEYVIEW(dev)) {
>> >>>  		dev_priv->display.modeset_global_resources =
>> >>>  			valleyview_modeset_global_resources;
>> >>> -- 
>> >>> 1.9.1
>> >>>
>> >>> _______________________________________________
>> >>> Intel-gfx mailing list
>> >>> Intel-gfx@lists.freedesktop.org
>> >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >>
>> >> -- 
>> >> Jani Nikula, Intel Open Source Technology Center
>> >
>> > -- 
>> > Jani Nikula, Intel Open Source Technology Center
>> 
>> -- 
>> Jani Nikula, Intel Open Source Technology Center
>
>
On Mon, Jun 29, 2015 at 02:42:25PM +0300, Jani Nikula wrote:
> On Mon, 29 Jun 2015, Mika Kahola <mika.kahola@intel.com> wrote:
> > On Mon, 2015-06-29 at 14:24 +0300, Jani Nikula wrote:
> >> On Tue, 16 Jun 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> >> > On Tue, 16 Jun 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> >> >> On Wed, 03 Jun 2015, Mika Kahola <mika.kahola@intel.com> wrote:
> >> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> >>>
> >> >>> Add support for changing cdclk frequency during runtime on BDW. The
> >> >>> procedure is quite a bit different on BDW from the one on HSW, so
> >> >>> add a separate function for it.
> >> >>>
> >> >>> Also with IPS enabled the actual pixel rate mustn't exceed 95% of cdclk,
> >> >>> so take that into account when computing the max pixel rate.
> >> >>>
> >> >>> v2: Grab rps.hw_lock around sandybridge_pcode_write()
> >> >>> v3: Rebase due to power well vs. .global_resources() reordering
> >> >>> v4: Rebased to the latest
> >> >>> v5: Rebased to the latest
> >> >>> v6: Patch order shuffle so that Broadwell CD clock change is
> >> >>>     applied before the patch for Haswell CD clock change
> >> >>> v7: Fix for patch style problems
> >> >>>
> >> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> >>> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> >> >>
> >> >> This patch hard hangs my BDW NUC at boot when both DP and HDMI are
> >> >> connected. Either DP or HDMI alone are good, same with hotplugging the
> >> >> other afterwards. Booting to grub with both connected, and unplugging
> >> >> HDMI before loading the kernel also reproduces the issue.
> >> >>
> >> >> It looks like the problem boils down to the BIOS setting up a smaller
> >> >> resolution on the DP display when both are connected, and this patch
> >> >> fails to cope with that on i915 load.
> >> >
> >> > By "this patch" I obviously refer to
> >> >
> >> > commit b432e5cfd5e92127ad2dd83bfc3083f1dbce43fb
> >> > Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> > Date:   Wed Jun 3 15:45:13 2015 +0300
> >> >
> >> >     drm/i915: BDW clock change support
> >> >
> >> > and everything works for the commit before that.
> >> 
> >> Anyone working on this, or should we revert?
> >> 
> >> BR,
> >> Jani.
> >> 
> > My understanding is that Ville has an idea or how to fix this.
> 
> Ville, when do you think we can convert the idea into a patch? Should we
> revert in the mean time?

I was thinking it may have been atomic making a mess of things and
enabling planes while the pipe was off. But I may be totally off here
since the current atomic code doesn't agree with my brain at all.

So no, I have no patch to fix it. Someone needs to first check where
exactly the hang happens.

> 
> BR,
> Jani.
> 
> 
> 
> >
> > Cheers,
> > Mika
> >
> >> 
> >> >
> >> > BR,
> >> > Jani.
> >> >
> >> >
> >> >
> >> >
> >> >
> >> >>
> >> >> BR,
> >> >> Jani.
> >> >>
> >> >>
> >> >>
> >> >>>
> >> >>> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> >>> ---
> >> >>>  drivers/gpu/drm/i915/i915_reg.h      |   2 +
> >> >>>  drivers/gpu/drm/i915/intel_display.c | 216 +++++++++++++++++++++++++++++++++--
> >> >>>  2 files changed, 208 insertions(+), 10 deletions(-)
> >> >>>
> >> >>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >> >>> index 7213224..0f72c0e 100644
> >> >>> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> >>> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> >>> @@ -6705,6 +6705,7 @@ enum skl_disp_power_wells {
> >> >>>  #define	  GEN6_PCODE_READ_RC6VIDS		0x5
> >> >>>  #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
> >> >>>  #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
> >> >>> +#define   BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ	0x18
> >> >>>  #define   GEN9_PCODE_READ_MEM_LATENCY		0x6
> >> >>>  #define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
> >> >>>  #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
> >> >>> @@ -7170,6 +7171,7 @@ enum skl_disp_power_wells {
> >> >>>  #define  LCPLL_CLK_FREQ_337_5_BDW	(2<<26)
> >> >>>  #define  LCPLL_CLK_FREQ_675_BDW		(3<<26)
> >> >>>  #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
> >> >>> +#define  LCPLL_ROOT_CD_CLOCK_DISABLE	(1<<24)
> >> >>>  #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
> >> >>>  #define  LCPLL_POWER_DOWN_ALLOW		(1<<22)
> >> >>>  #define  LCPLL_CD_SOURCE_FCLK		(1<<21)
> >> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> >>> index c3f01aa..b1e2069 100644
> >> >>> --- a/drivers/gpu/drm/i915/intel_display.c
> >> >>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> >>> @@ -5751,7 +5751,22 @@ static void intel_update_max_cdclk(struct drm_device *dev)
> >> >>>  {
> >> >>>  	struct drm_i915_private *dev_priv = dev->dev_private;
> >> >>>  
> >> >>> -	if (IS_VALLEYVIEW(dev)) {
> >> >>> +	if (IS_BROADWELL(dev))  {
> >> >>> +		/*
> >> >>> +		 * FIXME with extra cooling we can allow
> >> >>> +		 * 540 MHz for ULX and 675 Mhz for ULT.
> >> >>> +		 * How can we know if extra cooling is
> >> >>> +		 * available? PCI ID, VTB, something else?
> >> >>> +		 */
> >> >>> +		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
> >> >>> +			dev_priv->max_cdclk_freq = 450000;
> >> >>> +		else if (IS_BDW_ULX(dev))
> >> >>> +			dev_priv->max_cdclk_freq = 450000;
> >> >>> +		else if (IS_BDW_ULT(dev))
> >> >>> +			dev_priv->max_cdclk_freq = 540000;
> >> >>> +		else
> >> >>> +			dev_priv->max_cdclk_freq = 675000;
> >> >>> +	} else if (IS_VALLEYVIEW(dev)) {
> >> >>>  		dev_priv->max_cdclk_freq = 400000;
> >> >>>  	} else {
> >> >>>  		/* otherwise assume cdclk is fixed */
> >> >>> @@ -6621,13 +6636,11 @@ static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv,
> >> >>>  		return true;
> >> >>>  
> >> >>>  	/*
> >> >>> -	 * FIXME if we compare against max we should then
> >> >>> -	 * increase the cdclk frequency when the current
> >> >>> -	 * value is too low. The other option is to compare
> >> >>> -	 * against the cdclk frequency we're going have post
> >> >>> -	 * modeset (ie. one we computed using other constraints).
> >> >>> -	 * Need to measure whether using a lower cdclk w/o IPS
> >> >>> -	 * is better or worse than a higher cdclk w/ IPS.
> >> >>> +	 * We compare against max which means we must take
> >> >>> +	 * the increased cdclk requirement into account when
> >> >>> +	 * calculating the new cdclk.
> >> >>> +	 *
> >> >>> +	 * Should measure whether using a lower cdclk w/o IPS
> >> >>>  	 */
> >> >>>  	return ilk_pipe_pixel_rate(pipe_config) <=
> >> >>>  		dev_priv->max_cdclk_freq * 95 / 100;
> >> >>> @@ -9608,6 +9621,182 @@ static void broxton_modeset_global_resources(struct drm_atomic_state *old_state)
> >> >>>  		broxton_set_cdclk(dev, req_cdclk);
> >> >>>  }
> >> >>>  
> >> >>> +/* compute the max rate for new configuration */
> >> >>> +static int ilk_max_pixel_rate(struct drm_i915_private *dev_priv)
> >> >>> +{
> >> >>> +	struct drm_device *dev = dev_priv->dev;
> >> >>> +	struct intel_crtc *intel_crtc;
> >> >>> +	struct drm_crtc *crtc;
> >> >>> +	int max_pixel_rate = 0;
> >> >>> +	int pixel_rate;
> >> >>> +
> >> >>> +	for_each_crtc(dev, crtc) {
> >> >>> +		if (!crtc->state->enable)
> >> >>> +			continue;
> >> >>> +
> >> >>> +		intel_crtc = to_intel_crtc(crtc);
> >> >>> +		pixel_rate = ilk_pipe_pixel_rate(intel_crtc->config);
> >> >>> +
> >> >>> +		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> >> >>> +		if (IS_BROADWELL(dev) && intel_crtc->config->ips_enabled)
> >> >>> +			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> >> >>> +
> >> >>> +		max_pixel_rate = max(max_pixel_rate, pixel_rate);
> >> >>> +	}
> >> >>> +
> >> >>> +	return max_pixel_rate;
> >> >>> +}
> >> >>> +
> >> >>> +static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
> >> >>> +{
> >> >>> +	struct drm_i915_private *dev_priv = dev->dev_private;
> >> >>> +	uint32_t val, data;
> >> >>> +	int ret;
> >> >>> +
> >> >>> +	if (WARN((I915_READ(LCPLL_CTL) &
> >> >>> +		  (LCPLL_PLL_DISABLE | LCPLL_PLL_LOCK |
> >> >>> +		   LCPLL_CD_CLOCK_DISABLE | LCPLL_ROOT_CD_CLOCK_DISABLE |
> >> >>> +		   LCPLL_CD2X_CLOCK_DISABLE | LCPLL_POWER_DOWN_ALLOW |
> >> >>> +		   LCPLL_CD_SOURCE_FCLK)) != LCPLL_PLL_LOCK,
> >> >>> +		 "trying to change cdclk frequency with cdclk not enabled\n"))
> >> >>> +		return;
> >> >>> +
> >> >>> +	mutex_lock(&dev_priv->rps.hw_lock);
> >> >>> +	ret = sandybridge_pcode_write(dev_priv,
> >> >>> +				      BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
> >> >>> +	mutex_unlock(&dev_priv->rps.hw_lock);
> >> >>> +	if (ret) {
> >> >>> +		DRM_ERROR("failed to inform pcode about cdclk change\n");
> >> >>> +		return;
> >> >>> +	}
> >> >>> +
> >> >>> +	val = I915_READ(LCPLL_CTL);
> >> >>> +	val |= LCPLL_CD_SOURCE_FCLK;
> >> >>> +	I915_WRITE(LCPLL_CTL, val);
> >> >>> +
> >> >>> +	if (wait_for_atomic_us(I915_READ(LCPLL_CTL) &
> >> >>> +			       LCPLL_CD_SOURCE_FCLK_DONE, 1))
> >> >>> +		DRM_ERROR("Switching to FCLK failed\n");
> >> >>> +
> >> >>> +	val = I915_READ(LCPLL_CTL);
> >> >>> +	val &= ~LCPLL_CLK_FREQ_MASK;
> >> >>> +
> >> >>> +	switch (cdclk) {
> >> >>> +	case 450000:
> >> >>> +		val |= LCPLL_CLK_FREQ_450;
> >> >>> +		data = 0;
> >> >>> +		break;
> >> >>> +	case 540000:
> >> >>> +		val |= LCPLL_CLK_FREQ_54O_BDW;
> >> >>> +		data = 1;
> >> >>> +		break;
> >> >>> +	case 337500:
> >> >>> +		val |= LCPLL_CLK_FREQ_337_5_BDW;
> >> >>> +		data = 2;
> >> >>> +		break;
> >> >>> +	case 675000:
> >> >>> +		val |= LCPLL_CLK_FREQ_675_BDW;
> >> >>> +		data = 3;
> >> >>> +		break;
> >> >>> +	default:
> >> >>> +		WARN(1, "invalid cdclk frequency\n");
> >> >>> +		return;
> >> >>> +	}
> >> >>> +
> >> >>> +	I915_WRITE(LCPLL_CTL, val);
> >> >>> +
> >> >>> +	val = I915_READ(LCPLL_CTL);
> >> >>> +	val &= ~LCPLL_CD_SOURCE_FCLK;
> >> >>> +	I915_WRITE(LCPLL_CTL, val);
> >> >>> +
> >> >>> +	if (wait_for_atomic_us((I915_READ(LCPLL_CTL) &
> >> >>> +				LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
> >> >>> +		DRM_ERROR("Switching back to LCPLL failed\n");
> >> >>> +
> >> >>> +	mutex_lock(&dev_priv->rps.hw_lock);
> >> >>> +	sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ, data);
> >> >>> +	mutex_unlock(&dev_priv->rps.hw_lock);
> >> >>> +
> >> >>> +	intel_update_cdclk(dev);
> >> >>> +
> >> >>> +	WARN(cdclk != dev_priv->cdclk_freq,
> >> >>> +	     "cdclk requested %d kHz but got %d kHz\n",
> >> >>> +	     cdclk, dev_priv->cdclk_freq);
> >> >>> +}
> >> >>> +
> >> >>> +static int broadwell_calc_cdclk(struct drm_i915_private *dev_priv,
> >> >>> +			      int max_pixel_rate)
> >> >>> +{
> >> >>> +	int cdclk;
> >> >>> +
> >> >>> +	/*
> >> >>> +	 * FIXME should also account for plane ratio
> >> >>> +	 * once 64bpp pixel formats are supported.
> >> >>> +	 */
> >> >>> +	if (max_pixel_rate > 540000)
> >> >>> +		cdclk = 675000;
> >> >>> +	else if (max_pixel_rate > 450000)
> >> >>> +		cdclk = 540000;
> >> >>> +	else if (max_pixel_rate > 337500)
> >> >>> +		cdclk = 450000;
> >> >>> +	else
> >> >>> +		cdclk = 337500;
> >> >>> +
> >> >>> +	/*
> >> >>> +	 * FIXME move the cdclk caclulation to
> >> >>> +	 * compute_config() so we can fail gracegully.
> >> >>> +	 */
> >> >>> +	if (cdclk > dev_priv->max_cdclk_freq) {
> >> >>> +		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> >> >>> +			  cdclk, dev_priv->max_cdclk_freq);
> >> >>> +		cdclk = dev_priv->max_cdclk_freq;
> >> >>> +	}
> >> >>> +
> >> >>> +	return cdclk;
> >> >>> +}
> >> >>> +
> >> >>> +static int broadwell_modeset_global_pipes(struct drm_atomic_state *state)
> >> >>> +{
> >> >>> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> >> >>> +	struct drm_crtc *crtc;
> >> >>> +	struct drm_crtc_state *crtc_state;
> >> >>> +	int max_pixclk = ilk_max_pixel_rate(dev_priv);
> >> >>> +	int cdclk, i;
> >> >>> +
> >> >>> +	cdclk = broadwell_calc_cdclk(dev_priv, max_pixclk);
> >> >>> +
> >> >>> +	if (cdclk == dev_priv->cdclk_freq)
> >> >>> +		return 0;
> >> >>> +
> >> >>> +	/* add all active pipes to the state */
> >> >>> +	for_each_crtc(state->dev, crtc) {
> >> >>> +		if (!crtc->state->enable)
> >> >>> +			continue;
> >> >>> +
> >> >>> +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> >> >>> +		if (IS_ERR(crtc_state))
> >> >>> +			return PTR_ERR(crtc_state);
> >> >>> +	}
> >> >>> +
> >> >>> +	/* disable/enable all currently active pipes while we change cdclk */
> >> >>> +	for_each_crtc_in_state(state, crtc, crtc_state, i)
> >> >>> +		if (crtc_state->enable)
> >> >>> +			crtc_state->mode_changed = true;
> >> >>> +
> >> >>> +	return 0;
> >> >>> +}
> >> >>> +
> >> >>> +static void broadwell_modeset_global_resources(struct drm_atomic_state *state)
> >> >>> +{
> >> >>> +	struct drm_device *dev = state->dev;
> >> >>> +	struct drm_i915_private *dev_priv = dev->dev_private;
> >> >>> +	int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
> >> >>> +	int req_cdclk = broadwell_calc_cdclk(dev_priv, max_pixel_rate);
> >> >>> +
> >> >>> +	if (req_cdclk != dev_priv->cdclk_freq)
> >> >>> +		broadwell_set_cdclk(dev, req_cdclk);
> >> >>> +}
> >> >>> +
> >> >>>  static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
> >> >>>  				      struct intel_crtc_state *crtc_state)
> >> >>>  {
> >> >>> @@ -12788,8 +12977,12 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
> >> >>>  	 * mode set on this crtc.  For other crtcs we need to use the
> >> >>>  	 * adjusted_mode bits in the crtc directly.
> >> >>>  	 */
> >> >>> -	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
> >> >>> -		ret = valleyview_modeset_global_pipes(state);
> >> >>> +	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_BROADWELL(dev)) {
> >> >>> +		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev))
> >> >>> +			ret = valleyview_modeset_global_pipes(state);
> >> >>> +		else
> >> >>> +			ret = broadwell_modeset_global_pipes(state);
> >> >>> +
> >> >>>  		if (ret)
> >> >>>  			return ret;
> >> >>>  	}
> >> >>> @@ -14677,6 +14870,9 @@ static void intel_init_display(struct drm_device *dev)
> >> >>>  		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
> >> >>>  	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> >> >>>  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
> >> >>> +		if (IS_BROADWELL(dev))
> >> >>> +			dev_priv->display.modeset_global_resources =
> >> >>> +				broadwell_modeset_global_resources;
> >> >>>  	} else if (IS_VALLEYVIEW(dev)) {
> >> >>>  		dev_priv->display.modeset_global_resources =
> >> >>>  			valleyview_modeset_global_resources;
> >> >>> -- 
> >> >>> 1.9.1
> >> >>>
> >> >>> _______________________________________________
> >> >>> Intel-gfx mailing list
> >> >>> Intel-gfx@lists.freedesktop.org
> >> >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> >>
> >> >> -- 
> >> >> Jani Nikula, Intel Open Source Technology Center
> >> >
> >> > -- 
> >> > Jani Nikula, Intel Open Source Technology Center
> >> 
> >> -- 
> >> Jani Nikula, Intel Open Source Technology Center
> >
> >
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
On Mon, Jun 29, 2015 at 02:46:41PM +0300, Ville Syrjälä wrote:
> On Mon, Jun 29, 2015 at 02:42:25PM +0300, Jani Nikula wrote:
> > On Mon, 29 Jun 2015, Mika Kahola <mika.kahola@intel.com> wrote:
> > > On Mon, 2015-06-29 at 14:24 +0300, Jani Nikula wrote:
> > >> On Tue, 16 Jun 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > >> > On Tue, 16 Jun 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > >> >> On Wed, 03 Jun 2015, Mika Kahola <mika.kahola@intel.com> wrote:
> > >> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >> >>>
> > >> >>> Add support for changing cdclk frequency during runtime on BDW. The
> > >> >>> procedure is quite a bit different on BDW from the one on HSW, so
> > >> >>> add a separate function for it.
> > >> >>>
> > >> >>> Also with IPS enabled the actual pixel rate mustn't exceed 95% of cdclk,
> > >> >>> so take that into account when computing the max pixel rate.
> > >> >>>
> > >> >>> v2: Grab rps.hw_lock around sandybridge_pcode_write()
> > >> >>> v3: Rebase due to power well vs. .global_resources() reordering
> > >> >>> v4: Rebased to the latest
> > >> >>> v5: Rebased to the latest
> > >> >>> v6: Patch order shuffle so that Broadwell CD clock change is
> > >> >>>     applied before the patch for Haswell CD clock change
> > >> >>> v7: Fix for patch style problems
> > >> >>>
> > >> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >> >>> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > >> >>
> > >> >> This patch hard hangs my BDW NUC at boot when both DP and HDMI are
> > >> >> connected. Either DP or HDMI alone are good, same with hotplugging the
> > >> >> other afterwards. Booting to grub with both connected, and unplugging
> > >> >> HDMI before loading the kernel also reproduces the issue.
> > >> >>
> > >> >> It looks like the problem boils down to the BIOS setting up a smaller
> > >> >> resolution on the DP display when both are connected, and this patch
> > >> >> fails to cope with that on i915 load.
> > >> >
> > >> > By "this patch" I obviously refer to
> > >> >
> > >> > commit b432e5cfd5e92127ad2dd83bfc3083f1dbce43fb
> > >> > Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >> > Date:   Wed Jun 3 15:45:13 2015 +0300
> > >> >
> > >> >     drm/i915: BDW clock change support
> > >> >
> > >> > and everything works for the commit before that.
> > >> 
> > >> Anyone working on this, or should we revert?
> > >> 
> > >> BR,
> > >> Jani.
> > >> 
> > > My understanding is that Ville has an idea or how to fix this.
> > 
> > Ville, when do you think we can convert the idea into a patch? Should we
> > revert in the mean time?
> 
> I was thinking it may have been atomic making a mess of things and
> enabling planes while the pipe was off. But I may be totally off here
> since the current atomic code doesn't agree with my brain at all.
> 
> So no, I have no patch to fix it. Someone needs to first check where
> exactly the hang happens.

We do have evidence that at least the code in 4.2 does touch planes when
the pipe is off. Linus has backtraces at least.
-Daniel
On Tue, Jun 16, 2015 at 04:07:40PM +0300, Jani Nikula wrote:
> On Tue, 16 Jun 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > On Wed, 03 Jun 2015, Mika Kahola <mika.kahola@intel.com> wrote:
> >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> Add support for changing cdclk frequency during runtime on BDW. The
> >> procedure is quite a bit different on BDW from the one on HSW, so
> >> add a separate function for it.
> >>
> >> Also with IPS enabled the actual pixel rate mustn't exceed 95% of cdclk,
> >> so take that into account when computing the max pixel rate.
> >>
> >> v2: Grab rps.hw_lock around sandybridge_pcode_write()
> >> v3: Rebase due to power well vs. .global_resources() reordering
> >> v4: Rebased to the latest
> >> v5: Rebased to the latest
> >> v6: Patch order shuffle so that Broadwell CD clock change is
> >>     applied before the patch for Haswell CD clock change
> >> v7: Fix for patch style problems
> >>
> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> >
> > This patch hard hangs my BDW NUC at boot when both DP and HDMI are
> > connected. Either DP or HDMI alone are good, same with hotplugging the
> > other afterwards. Booting to grub with both connected, and unplugging
> > HDMI before loading the kernel also reproduces the issue.
> >
> > It looks like the problem boils down to the BIOS setting up a smaller
> > resolution on the DP display when both are connected, and this patch
> > fails to cope with that on i915 load.
> 
> By "this patch" I obviously refer to
> 
> commit b432e5cfd5e92127ad2dd83bfc3083f1dbce43fb
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date:   Wed Jun 3 15:45:13 2015 +0300
> 
>     drm/i915: BDW clock change support
> 
> and everything works for the commit before that.

Another regression for Jairo to track. Jairo please add the bisect result
to the bugzilla and mark it as bisected too.

Thanks, Daniel

> 
> BR,
> Jani.
> 
> 
> 
> 
> 
> >
> > BR,
> > Jani.
> >
> >
> >
> >>
> >> Author:    Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/i915_reg.h      |   2 +
> >>  drivers/gpu/drm/i915/intel_display.c | 216 +++++++++++++++++++++++++++++++++--
> >>  2 files changed, 208 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >> index 7213224..0f72c0e 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -6705,6 +6705,7 @@ enum skl_disp_power_wells {
> >>  #define	  GEN6_PCODE_READ_RC6VIDS		0x5
> >>  #define     GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
> >>  #define     GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
> >> +#define   BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ	0x18
> >>  #define   GEN9_PCODE_READ_MEM_LATENCY		0x6
> >>  #define     GEN9_MEM_LATENCY_LEVEL_MASK		0xFF
> >>  #define     GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT	8
> >> @@ -7170,6 +7171,7 @@ enum skl_disp_power_wells {
> >>  #define  LCPLL_CLK_FREQ_337_5_BDW	(2<<26)
> >>  #define  LCPLL_CLK_FREQ_675_BDW		(3<<26)
> >>  #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
> >> +#define  LCPLL_ROOT_CD_CLOCK_DISABLE	(1<<24)
> >>  #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
> >>  #define  LCPLL_POWER_DOWN_ALLOW		(1<<22)
> >>  #define  LCPLL_CD_SOURCE_FCLK		(1<<21)
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index c3f01aa..b1e2069 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -5751,7 +5751,22 @@ static void intel_update_max_cdclk(struct drm_device *dev)
> >>  {
> >>  	struct drm_i915_private *dev_priv = dev->dev_private;
> >>  
> >> -	if (IS_VALLEYVIEW(dev)) {
> >> +	if (IS_BROADWELL(dev))  {
> >> +		/*
> >> +		 * FIXME with extra cooling we can allow
> >> +		 * 540 MHz for ULX and 675 Mhz for ULT.
> >> +		 * How can we know if extra cooling is
> >> +		 * available? PCI ID, VTB, something else?
> >> +		 */
> >> +		if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT)
> >> +			dev_priv->max_cdclk_freq = 450000;
> >> +		else if (IS_BDW_ULX(dev))
> >> +			dev_priv->max_cdclk_freq = 450000;
> >> +		else if (IS_BDW_ULT(dev))
> >> +			dev_priv->max_cdclk_freq = 540000;
> >> +		else
> >> +			dev_priv->max_cdclk_freq = 675000;
> >> +	} else if (IS_VALLEYVIEW(dev)) {
> >>  		dev_priv->max_cdclk_freq = 400000;
> >>  	} else {
> >>  		/* otherwise assume cdclk is fixed */
> >> @@ -6621,13 +6636,11 @@ static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv,
> >>  		return true;
> >>  
> >>  	/*
> >> -	 * FIXME if we compare against max we should then
> >> -	 * increase the cdclk frequency when the current
> >> -	 * value is too low. The other option is to compare
> >> -	 * against the cdclk frequency we're going have post
> >> -	 * modeset (ie. one we computed using other constraints).
> >> -	 * Need to measure whether using a lower cdclk w/o IPS
> >> -	 * is better or worse than a higher cdclk w/ IPS.
> >> +	 * We compare against max which means we must take
> >> +	 * the increased cdclk requirement into account when
> >> +	 * calculating the new cdclk.
> >> +	 *
> >> +	 * Should measure whether using a lower cdclk w/o IPS
> >>  	 */
> >>  	return ilk_pipe_pixel_rate(pipe_config) <=
> >>  		dev_priv->max_cdclk_freq * 95 / 100;
> >> @@ -9608,6 +9621,182 @@ static void broxton_modeset_global_resources(struct drm_atomic_state *old_state)
> >>  		broxton_set_cdclk(dev, req_cdclk);
> >>  }
> >>  
> >> +/* compute the max rate for new configuration */
> >> +static int ilk_max_pixel_rate(struct drm_i915_private *dev_priv)
> >> +{
> >> +	struct drm_device *dev = dev_priv->dev;
> >> +	struct intel_crtc *intel_crtc;
> >> +	struct drm_crtc *crtc;
> >> +	int max_pixel_rate = 0;
> >> +	int pixel_rate;
> >> +
> >> +	for_each_crtc(dev, crtc) {
> >> +		if (!crtc->state->enable)
> >> +			continue;
> >> +
> >> +		intel_crtc = to_intel_crtc(crtc);
> >> +		pixel_rate = ilk_pipe_pixel_rate(intel_crtc->config);
> >> +
> >> +		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
> >> +		if (IS_BROADWELL(dev) && intel_crtc->config->ips_enabled)
> >> +			pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
> >> +
> >> +		max_pixel_rate = max(max_pixel_rate, pixel_rate);
> >> +	}
> >> +
> >> +	return max_pixel_rate;
> >> +}
> >> +
> >> +static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
> >> +{
> >> +	struct drm_i915_private *dev_priv = dev->dev_private;
> >> +	uint32_t val, data;
> >> +	int ret;
> >> +
> >> +	if (WARN((I915_READ(LCPLL_CTL) &
> >> +		  (LCPLL_PLL_DISABLE | LCPLL_PLL_LOCK |
> >> +		   LCPLL_CD_CLOCK_DISABLE | LCPLL_ROOT_CD_CLOCK_DISABLE |
> >> +		   LCPLL_CD2X_CLOCK_DISABLE | LCPLL_POWER_DOWN_ALLOW |
> >> +		   LCPLL_CD_SOURCE_FCLK)) != LCPLL_PLL_LOCK,
> >> +		 "trying to change cdclk frequency with cdclk not enabled\n"))
> >> +		return;
> >> +
> >> +	mutex_lock(&dev_priv->rps.hw_lock);
> >> +	ret = sandybridge_pcode_write(dev_priv,
> >> +				      BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
> >> +	mutex_unlock(&dev_priv->rps.hw_lock);
> >> +	if (ret) {
> >> +		DRM_ERROR("failed to inform pcode about cdclk change\n");
> >> +		return;
> >> +	}
> >> +
> >> +	val = I915_READ(LCPLL_CTL);
> >> +	val |= LCPLL_CD_SOURCE_FCLK;
> >> +	I915_WRITE(LCPLL_CTL, val);
> >> +
> >> +	if (wait_for_atomic_us(I915_READ(LCPLL_CTL) &
> >> +			       LCPLL_CD_SOURCE_FCLK_DONE, 1))
> >> +		DRM_ERROR("Switching to FCLK failed\n");
> >> +
> >> +	val = I915_READ(LCPLL_CTL);
> >> +	val &= ~LCPLL_CLK_FREQ_MASK;
> >> +
> >> +	switch (cdclk) {
> >> +	case 450000:
> >> +		val |= LCPLL_CLK_FREQ_450;
> >> +		data = 0;
> >> +		break;
> >> +	case 540000:
> >> +		val |= LCPLL_CLK_FREQ_54O_BDW;
> >> +		data = 1;
> >> +		break;
> >> +	case 337500:
> >> +		val |= LCPLL_CLK_FREQ_337_5_BDW;
> >> +		data = 2;
> >> +		break;
> >> +	case 675000:
> >> +		val |= LCPLL_CLK_FREQ_675_BDW;
> >> +		data = 3;
> >> +		break;
> >> +	default:
> >> +		WARN(1, "invalid cdclk frequency\n");
> >> +		return;
> >> +	}
> >> +
> >> +	I915_WRITE(LCPLL_CTL, val);
> >> +
> >> +	val = I915_READ(LCPLL_CTL);
> >> +	val &= ~LCPLL_CD_SOURCE_FCLK;
> >> +	I915_WRITE(LCPLL_CTL, val);
> >> +
> >> +	if (wait_for_atomic_us((I915_READ(LCPLL_CTL) &
> >> +				LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
> >> +		DRM_ERROR("Switching back to LCPLL failed\n");
> >> +
> >> +	mutex_lock(&dev_priv->rps.hw_lock);
> >> +	sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ, data);
> >> +	mutex_unlock(&dev_priv->rps.hw_lock);
> >> +
> >> +	intel_update_cdclk(dev);
> >> +
> >> +	WARN(cdclk != dev_priv->cdclk_freq,
> >> +	     "cdclk requested %d kHz but got %d kHz\n",
> >> +	     cdclk, dev_priv->cdclk_freq);
> >> +}
> >> +
> >> +static int broadwell_calc_cdclk(struct drm_i915_private *dev_priv,
> >> +			      int max_pixel_rate)
> >> +{
> >> +	int cdclk;
> >> +
> >> +	/*
> >> +	 * FIXME should also account for plane ratio
> >> +	 * once 64bpp pixel formats are supported.
> >> +	 */
> >> +	if (max_pixel_rate > 540000)
> >> +		cdclk = 675000;
> >> +	else if (max_pixel_rate > 450000)
> >> +		cdclk = 540000;
> >> +	else if (max_pixel_rate > 337500)
> >> +		cdclk = 450000;
> >> +	else
> >> +		cdclk = 337500;
> >> +
> >> +	/*
> >> +	 * FIXME move the cdclk caclulation to
> >> +	 * compute_config() so we can fail gracegully.
> >> +	 */
> >> +	if (cdclk > dev_priv->max_cdclk_freq) {
> >> +		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> >> +			  cdclk, dev_priv->max_cdclk_freq);
> >> +		cdclk = dev_priv->max_cdclk_freq;
> >> +	}
> >> +
> >> +	return cdclk;
> >> +}
> >> +
> >> +static int broadwell_modeset_global_pipes(struct drm_atomic_state *state)
> >> +{
> >> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> >> +	struct drm_crtc *crtc;
> >> +	struct drm_crtc_state *crtc_state;
> >> +	int max_pixclk = ilk_max_pixel_rate(dev_priv);
> >> +	int cdclk, i;
> >> +
> >> +	cdclk = broadwell_calc_cdclk(dev_priv, max_pixclk);
> >> +
> >> +	if (cdclk == dev_priv->cdclk_freq)
> >> +		return 0;
> >> +
> >> +	/* add all active pipes to the state */
> >> +	for_each_crtc(state->dev, crtc) {
> >> +		if (!crtc->state->enable)
> >> +			continue;
> >> +
> >> +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> >> +		if (IS_ERR(crtc_state))
> >> +			return PTR_ERR(crtc_state);
> >> +	}
> >> +
> >> +	/* disable/enable all currently active pipes while we change cdclk */
> >> +	for_each_crtc_in_state(state, crtc, crtc_state, i)
> >> +		if (crtc_state->enable)
> >> +			crtc_state->mode_changed = true;
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static void broadwell_modeset_global_resources(struct drm_atomic_state *state)
> >> +{
> >> +	struct drm_device *dev = state->dev;
> >> +	struct drm_i915_private *dev_priv = dev->dev_private;
> >> +	int max_pixel_rate = ilk_max_pixel_rate(dev_priv);
> >> +	int req_cdclk = broadwell_calc_cdclk(dev_priv, max_pixel_rate);
> >> +
> >> +	if (req_cdclk != dev_priv->cdclk_freq)
> >> +		broadwell_set_cdclk(dev, req_cdclk);
> >> +}
> >> +
> >>  static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
> >>  				      struct intel_crtc_state *crtc_state)
> >>  {
> >> @@ -12788,8 +12977,12 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
> >>  	 * mode set on this crtc.  For other crtcs we need to use the
> >>  	 * adjusted_mode bits in the crtc directly.
> >>  	 */
> >> -	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
> >> -		ret = valleyview_modeset_global_pipes(state);
> >> +	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev) || IS_BROADWELL(dev)) {
> >> +		if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev))
> >> +			ret = valleyview_modeset_global_pipes(state);
> >> +		else
> >> +			ret = broadwell_modeset_global_pipes(state);
> >> +
> >>  		if (ret)
> >>  			return ret;
> >>  	}
> >> @@ -14677,6 +14870,9 @@ static void intel_init_display(struct drm_device *dev)
> >>  		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
> >>  	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> >>  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
> >> +		if (IS_BROADWELL(dev))
> >> +			dev_priv->display.modeset_global_resources =
> >> +				broadwell_modeset_global_resources;
> >>  	} else if (IS_VALLEYVIEW(dev)) {
> >>  		dev_priv->display.modeset_global_resources =
> >>  			valleyview_modeset_global_resources;
> >> -- 
> >> 1.9.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > -- 
> > Jani Nikula, Intel Open Source Technology Center
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx