[PATCH] drm/i915: Consider SPLL as another shared pll, v2.

[PATCH] drm/i915: Consider SPLL as another shared pll, v2.

[Maarten Lankhorst]

When diagnosing a unrelated bug for someone on irc, it would seem the hardware can
be brought up by the BIOS with the embedded displayport using the SPLL for spread spectrum.

Right now this is not handled well in i915, and it calculates the crtc needs to
be reprogrammed on the first modeset without SSC, but  the SPLL itself was kept
active. Fix this by exposing SPLL as a shared pll that will not be returned
by intel_get_shared_dpll; you have to know it exists to use it.

Changes since v1:
- Create a separate dpll_hw_state.spll for spll, and use
  separate pll functions for spll.

Tested-by: Emil Renner Berthing <kernel@esmil.dk> #v1
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
Emil, can you retest?

 drivers/gpu/drm/i915/i915_drv.h      |  3 ++
 drivers/gpu/drm/i915/intel_crt.c     | 31 ++-------------
 drivers/gpu/drm/i915/intel_ddi.c     | 75 +++++++++++++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_display.c | 15 ++++++--
 4 files changed, 83 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 85c84c624ed1..b6b62387cbe1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -351,6 +351,8 @@ enum intel_dpll_id {
 	/* hsw/bdw */
 	DPLL_ID_WRPLL1 = 0,
 	DPLL_ID_WRPLL2 = 1,
+	DPLL_ID_SPLL = 2,
+
 	/* skl */
 	DPLL_ID_SKL_DPLL1 = 0,
 	DPLL_ID_SKL_DPLL2 = 1,
@@ -367,6 +369,7 @@ struct intel_dpll_hw_state {
 
 	/* hsw, bdw */
 	uint32_t wrpll;
+	uint32_t spll;
 
 	/* skl */
 	/*
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index b84aaa0bb48a..6a2c76e367a5 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -138,18 +138,6 @@ static void hsw_crt_get_config(struct intel_encoder *encoder,
 	pipe_config->base.adjusted_mode.flags |= intel_crt_get_flags(encoder);
 }
 
-static void hsw_crt_pre_enable(struct intel_encoder *encoder)
-{
-	struct drm_device *dev = encoder->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	WARN(I915_READ(SPLL_CTL) & SPLL_PLL_ENABLE, "SPLL already enabled\n");
-	I915_WRITE(SPLL_CTL,
-		   SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz | SPLL_PLL_SSC);
-	POSTING_READ(SPLL_CTL);
-	udelay(20);
-}
-
 /* Note: The caller is required to filter out dpms modes not supported by the
  * platform. */
 static void intel_crt_set_dpms(struct intel_encoder *encoder, int mode)
@@ -216,19 +204,6 @@ static void pch_post_disable_crt(struct intel_encoder *encoder)
 	intel_disable_crt(encoder);
 }
 
-static void hsw_crt_post_disable(struct intel_encoder *encoder)
-{
-	struct drm_device *dev = encoder->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	uint32_t val;
-
-	DRM_DEBUG_KMS("Disabling SPLL\n");
-	val = I915_READ(SPLL_CTL);
-	WARN_ON(!(val & SPLL_PLL_ENABLE));
-	I915_WRITE(SPLL_CTL, val & ~SPLL_PLL_ENABLE);
-	POSTING_READ(SPLL_CTL);
-}
-
 static void intel_enable_crt(struct intel_encoder *encoder)
 {
 	struct intel_crt *crt = intel_encoder_to_crt(encoder);
@@ -280,6 +255,10 @@ static bool intel_crt_compute_config(struct intel_encoder *encoder,
 	if (HAS_DDI(dev)) {
 		pipe_config->ddi_pll_sel = PORT_CLK_SEL_SPLL;
 		pipe_config->port_clock = 135000 * 2;
+
+		pipe_config->dpll_hw_state.wrpll = 0;
+		pipe_config->dpll_hw_state.spll =
+			SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz | SPLL_PLL_SSC;
 	}
 
 	return true;
@@ -860,8 +839,6 @@ void intel_crt_init(struct drm_device *dev)
 	if (HAS_DDI(dev)) {
 		crt->base.get_config = hsw_crt_get_config;
 		crt->base.get_hw_state = intel_ddi_get_hw_state;
-		crt->base.pre_enable = hsw_crt_pre_enable;
-		crt->base.post_disable = hsw_crt_post_disable;
 	} else {
 		crt->base.get_config = intel_crt_get_config;
 		crt->base.get_hw_state = intel_crt_get_hw_state;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index abb4a265a6df..90fad7ad12ac 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1286,6 +1286,18 @@ hsw_ddi_pll_select(struct intel_crtc *intel_crtc,
 		}
 
 		crtc_state->ddi_pll_sel = PORT_CLK_SEL_WRPLL(pll->id);
+	} else if (crtc_state->ddi_pll_sel == PORT_CLK_SEL_SPLL) {
+		struct drm_atomic_state *state = crtc_state->base.state;
+		struct intel_shared_dpll_config *spll =
+			&intel_atomic_get_shared_dpll_state(state)[DPLL_ID_SPLL];
+
+		if (spll->crtc_mask &&
+		    WARN_ON(spll->hw_state.spll != crtc_state->dpll_hw_state.spll))
+			return false;
+
+		crtc_state->shared_dpll = DPLL_ID_SPLL;
+		spll->hw_state.spll = crtc_state->dpll_hw_state.spll;
+		spll->crtc_mask |= 1 << intel_crtc->pipe;
 	}
 
 	return true;
@@ -2446,7 +2458,7 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder)
 	}
 }
 
-static void hsw_ddi_pll_enable(struct drm_i915_private *dev_priv,
+static void hsw_ddi_wrpll_enable(struct drm_i915_private *dev_priv,
 			       struct intel_shared_dpll *pll)
 {
 	I915_WRITE(WRPLL_CTL(pll->id), pll->config.hw_state.wrpll);
@@ -2454,9 +2466,17 @@ static void hsw_ddi_pll_enable(struct drm_i915_private *dev_priv,
 	udelay(20);
 }
 
-static void hsw_ddi_pll_disable(struct drm_i915_private *dev_priv,
+static void hsw_ddi_spll_enable(struct drm_i915_private *dev_priv,
 				struct intel_shared_dpll *pll)
 {
+	I915_WRITE(SPLL_CTL, pll->config.hw_state.spll);
+	POSTING_READ(SPLL_CTL);
+	udelay(20);
+}
+
+static void hsw_ddi_wrpll_disable(struct drm_i915_private *dev_priv,
+				  struct intel_shared_dpll *pll)
+{
 	uint32_t val;
 
 	val = I915_READ(WRPLL_CTL(pll->id));
@@ -2464,9 +2484,19 @@ static void hsw_ddi_pll_disable(struct drm_i915_private *dev_priv,
 	POSTING_READ(WRPLL_CTL(pll->id));
 }
 
-static bool hsw_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
-				     struct intel_shared_dpll *pll,
-				     struct intel_dpll_hw_state *hw_state)
+static void hsw_ddi_spll_disable(struct drm_i915_private *dev_priv,
+				 struct intel_shared_dpll *pll)
+{
+	uint32_t val;
+
+	val = I915_READ(SPLL_CTL);
+	I915_WRITE(SPLL_CTL, val & ~SPLL_PLL_ENABLE);
+	POSTING_READ(SPLL_CTL);
+}
+
+static bool hsw_ddi_wrpll_get_hw_state(struct drm_i915_private *dev_priv,
+				       struct intel_shared_dpll *pll,
+				       struct intel_dpll_hw_state *hw_state)
 {
 	uint32_t val;
 
@@ -2479,25 +2509,50 @@ static bool hsw_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
 	return val & WRPLL_PLL_ENABLE;
 }
 
+static bool hsw_ddi_spll_get_hw_state(struct drm_i915_private *dev_priv,
+				      struct intel_shared_dpll *pll,
+				      struct intel_dpll_hw_state *hw_state)
+{
+	uint32_t val;
+
+	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
+		return false;
+
+	val = I915_READ(SPLL_CTL);
+	hw_state->spll = val;
+
+	return val & SPLL_PLL_ENABLE;
+}
+
+
 static const char * const hsw_ddi_pll_names[] = {
 	"WRPLL 1",
 	"WRPLL 2",
+	"SPLL"
 };
 
 static void hsw_shared_dplls_init(struct drm_i915_private *dev_priv)
 {
 	int i;
 
-	dev_priv->num_shared_dpll = 2;
+	dev_priv->num_shared_dpll = 3;
 
-	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
+	for (i = 0; i < 2; i++) {
 		dev_priv->shared_dplls[i].id = i;
 		dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
-		dev_priv->shared_dplls[i].disable = hsw_ddi_pll_disable;
-		dev_priv->shared_dplls[i].enable = hsw_ddi_pll_enable;
+		dev_priv->shared_dplls[i].disable = hsw_ddi_wrpll_disable;
+		dev_priv->shared_dplls[i].enable = hsw_ddi_wrpll_enable;
 		dev_priv->shared_dplls[i].get_hw_state =
-			hsw_ddi_pll_get_hw_state;
+			hsw_ddi_wrpll_get_hw_state;
 	}
+
+	/* SPLL is special, but needs to be initialized anyway.. */
+	dev_priv->shared_dplls[i].id = i;
+	dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
+	dev_priv->shared_dplls[i].disable = hsw_ddi_spll_disable;
+	dev_priv->shared_dplls[i].enable = hsw_ddi_spll_enable;
+	dev_priv->shared_dplls[i].get_hw_state = hsw_ddi_spll_get_hw_state;
+
 }
 
 static const char * const skl_ddi_pll_names[] = {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3ca38fc493de..b383cbe3964c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4210,6 +4210,7 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
 	struct intel_shared_dpll *pll;
 	struct intel_shared_dpll_config *shared_dpll;
 	enum intel_dpll_id i;
+	int max = dev_priv->num_shared_dpll;
 
 	shared_dpll = intel_atomic_get_shared_dpll_state(crtc_state->base.state);
 
@@ -4244,9 +4245,11 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
 		WARN_ON(shared_dpll[i].crtc_mask);
 
 		goto found;
-	}
+	} else if (INTEL_INFO(dev_priv)->gen < 9 && HAS_DDI(dev_priv))
+		/* Do not consider SPLL */
+		max = 2;
 
-	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
+	for (i = 0; i < max; i++) {
 		pll = &dev_priv->shared_dplls[i];
 
 		/* Only want to check enabled timings first */
@@ -9767,6 +9770,8 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
 	case PORT_CLK_SEL_WRPLL2:
 		pipe_config->shared_dpll = DPLL_ID_WRPLL2;
 		break;
+	case PORT_CLK_SEL_SPLL:
+		pipe_config->shared_dpll = DPLL_ID_SPLL;
 	}
 }
 
@@ -12070,9 +12075,10 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 			      pipe_config->dpll_hw_state.cfgcr1,
 			      pipe_config->dpll_hw_state.cfgcr2);
 	} else if (HAS_DDI(dev)) {
-		DRM_DEBUG_KMS("ddi_pll_sel: %u; dpll_hw_state: wrpll: 0x%x\n",
+		DRM_DEBUG_KMS("ddi_pll_sel: %u; dpll_hw_state: wrpll: 0x%x spll: 0x%x\n",
 			      pipe_config->ddi_pll_sel,
-			      pipe_config->dpll_hw_state.wrpll);
+			      pipe_config->dpll_hw_state.wrpll,
+			      pipe_config->dpll_hw_state.spll);
 	} else {
 		DRM_DEBUG_KMS("dpll_hw_state: dpll: 0x%x, dpll_md: 0x%x, "
 			      "fp0: 0x%x, fp1: 0x%x\n",
@@ -12607,6 +12613,7 @@ intel_pipe_config_compare(struct drm_device *dev,
 	PIPE_CONF_CHECK_X(dpll_hw_state.fp0);
 	PIPE_CONF_CHECK_X(dpll_hw_state.fp1);
 	PIPE_CONF_CHECK_X(dpll_hw_state.wrpll);
+	PIPE_CONF_CHECK_X(dpll_hw_state.spll);
 	PIPE_CONF_CHECK_X(dpll_hw_state.ctrl1);
 	PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr1);
 	PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr2);
-- 
2.1.0

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

Re: [PATCH] drm/i915: Consider SPLL as another shared pll, v2.

[Jani Nikula]

On Mon, 16 Nov 2015, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
> When diagnosing a unrelated bug for someone on irc, it would seem the hardware can
> be brought up by the BIOS with the embedded displayport using the SPLL for spread spectrum.
>
> Right now this is not handled well in i915, and it calculates the crtc needs to
> be reprogrammed on the first modeset without SSC, but  the SPLL itself was kept
> active. Fix this by exposing SPLL as a shared pll that will not be returned
> by intel_get_shared_dpll; you have to know it exists to use it.
>
> Changes since v1:
> - Create a separate dpll_hw_state.spll for spll, and use
>   separate pll functions for spll.
>
> Tested-by: Emil Renner Berthing <kernel@esmil.dk> #v1
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> Emil, can you retest?

Gabriel, you too please!

BR,
Jani.

>
>  drivers/gpu/drm/i915/i915_drv.h      |  3 ++
>  drivers/gpu/drm/i915/intel_crt.c     | 31 ++-------------
>  drivers/gpu/drm/i915/intel_ddi.c     | 75 +++++++++++++++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_display.c | 15 ++++++--
>  4 files changed, 83 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 85c84c624ed1..b6b62387cbe1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -351,6 +351,8 @@ enum intel_dpll_id {
>  	/* hsw/bdw */
>  	DPLL_ID_WRPLL1 = 0,
>  	DPLL_ID_WRPLL2 = 1,
> +	DPLL_ID_SPLL = 2,
> +
>  	/* skl */
>  	DPLL_ID_SKL_DPLL1 = 0,
>  	DPLL_ID_SKL_DPLL2 = 1,
> @@ -367,6 +369,7 @@ struct intel_dpll_hw_state {
>  
>  	/* hsw, bdw */
>  	uint32_t wrpll;
> +	uint32_t spll;
>  
>  	/* skl */
>  	/*
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index b84aaa0bb48a..6a2c76e367a5 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -138,18 +138,6 @@ static void hsw_crt_get_config(struct intel_encoder *encoder,
>  	pipe_config->base.adjusted_mode.flags |= intel_crt_get_flags(encoder);
>  }
>  
> -static void hsw_crt_pre_enable(struct intel_encoder *encoder)
> -{
> -	struct drm_device *dev = encoder->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	WARN(I915_READ(SPLL_CTL) & SPLL_PLL_ENABLE, "SPLL already enabled\n");
> -	I915_WRITE(SPLL_CTL,
> -		   SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz | SPLL_PLL_SSC);
> -	POSTING_READ(SPLL_CTL);
> -	udelay(20);
> -}
> -
>  /* Note: The caller is required to filter out dpms modes not supported by the
>   * platform. */
>  static void intel_crt_set_dpms(struct intel_encoder *encoder, int mode)
> @@ -216,19 +204,6 @@ static void pch_post_disable_crt(struct intel_encoder *encoder)
>  	intel_disable_crt(encoder);
>  }
>  
> -static void hsw_crt_post_disable(struct intel_encoder *encoder)
> -{
> -	struct drm_device *dev = encoder->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	uint32_t val;
> -
> -	DRM_DEBUG_KMS("Disabling SPLL\n");
> -	val = I915_READ(SPLL_CTL);
> -	WARN_ON(!(val & SPLL_PLL_ENABLE));
> -	I915_WRITE(SPLL_CTL, val & ~SPLL_PLL_ENABLE);
> -	POSTING_READ(SPLL_CTL);
> -}
> -
>  static void intel_enable_crt(struct intel_encoder *encoder)
>  {
>  	struct intel_crt *crt = intel_encoder_to_crt(encoder);
> @@ -280,6 +255,10 @@ static bool intel_crt_compute_config(struct intel_encoder *encoder,
>  	if (HAS_DDI(dev)) {
>  		pipe_config->ddi_pll_sel = PORT_CLK_SEL_SPLL;
>  		pipe_config->port_clock = 135000 * 2;
> +
> +		pipe_config->dpll_hw_state.wrpll = 0;
> +		pipe_config->dpll_hw_state.spll =
> +			SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz | SPLL_PLL_SSC;
>  	}
>  
>  	return true;
> @@ -860,8 +839,6 @@ void intel_crt_init(struct drm_device *dev)
>  	if (HAS_DDI(dev)) {
>  		crt->base.get_config = hsw_crt_get_config;
>  		crt->base.get_hw_state = intel_ddi_get_hw_state;
> -		crt->base.pre_enable = hsw_crt_pre_enable;
> -		crt->base.post_disable = hsw_crt_post_disable;
>  	} else {
>  		crt->base.get_config = intel_crt_get_config;
>  		crt->base.get_hw_state = intel_crt_get_hw_state;
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index abb4a265a6df..90fad7ad12ac 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1286,6 +1286,18 @@ hsw_ddi_pll_select(struct intel_crtc *intel_crtc,
>  		}
>  
>  		crtc_state->ddi_pll_sel = PORT_CLK_SEL_WRPLL(pll->id);
> +	} else if (crtc_state->ddi_pll_sel == PORT_CLK_SEL_SPLL) {
> +		struct drm_atomic_state *state = crtc_state->base.state;
> +		struct intel_shared_dpll_config *spll =
> +			&intel_atomic_get_shared_dpll_state(state)[DPLL_ID_SPLL];
> +
> +		if (spll->crtc_mask &&
> +		    WARN_ON(spll->hw_state.spll != crtc_state->dpll_hw_state.spll))
> +			return false;
> +
> +		crtc_state->shared_dpll = DPLL_ID_SPLL;
> +		spll->hw_state.spll = crtc_state->dpll_hw_state.spll;
> +		spll->crtc_mask |= 1 << intel_crtc->pipe;
>  	}
>  
>  	return true;
> @@ -2446,7 +2458,7 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder)
>  	}
>  }
>  
> -static void hsw_ddi_pll_enable(struct drm_i915_private *dev_priv,
> +static void hsw_ddi_wrpll_enable(struct drm_i915_private *dev_priv,
>  			       struct intel_shared_dpll *pll)
>  {
>  	I915_WRITE(WRPLL_CTL(pll->id), pll->config.hw_state.wrpll);
> @@ -2454,9 +2466,17 @@ static void hsw_ddi_pll_enable(struct drm_i915_private *dev_priv,
>  	udelay(20);
>  }
>  
> -static void hsw_ddi_pll_disable(struct drm_i915_private *dev_priv,
> +static void hsw_ddi_spll_enable(struct drm_i915_private *dev_priv,
>  				struct intel_shared_dpll *pll)
>  {
> +	I915_WRITE(SPLL_CTL, pll->config.hw_state.spll);
> +	POSTING_READ(SPLL_CTL);
> +	udelay(20);
> +}
> +
> +static void hsw_ddi_wrpll_disable(struct drm_i915_private *dev_priv,
> +				  struct intel_shared_dpll *pll)
> +{
>  	uint32_t val;
>  
>  	val = I915_READ(WRPLL_CTL(pll->id));
> @@ -2464,9 +2484,19 @@ static void hsw_ddi_pll_disable(struct drm_i915_private *dev_priv,
>  	POSTING_READ(WRPLL_CTL(pll->id));
>  }
>  
> -static bool hsw_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
> -				     struct intel_shared_dpll *pll,
> -				     struct intel_dpll_hw_state *hw_state)
> +static void hsw_ddi_spll_disable(struct drm_i915_private *dev_priv,
> +				 struct intel_shared_dpll *pll)
> +{
> +	uint32_t val;
> +
> +	val = I915_READ(SPLL_CTL);
> +	I915_WRITE(SPLL_CTL, val & ~SPLL_PLL_ENABLE);
> +	POSTING_READ(SPLL_CTL);
> +}
> +
> +static bool hsw_ddi_wrpll_get_hw_state(struct drm_i915_private *dev_priv,
> +				       struct intel_shared_dpll *pll,
> +				       struct intel_dpll_hw_state *hw_state)
>  {
>  	uint32_t val;
>  
> @@ -2479,25 +2509,50 @@ static bool hsw_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  	return val & WRPLL_PLL_ENABLE;
>  }
>  
> +static bool hsw_ddi_spll_get_hw_state(struct drm_i915_private *dev_priv,
> +				      struct intel_shared_dpll *pll,
> +				      struct intel_dpll_hw_state *hw_state)
> +{
> +	uint32_t val;
> +
> +	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
> +		return false;
> +
> +	val = I915_READ(SPLL_CTL);
> +	hw_state->spll = val;
> +
> +	return val & SPLL_PLL_ENABLE;
> +}
> +
> +
>  static const char * const hsw_ddi_pll_names[] = {
>  	"WRPLL 1",
>  	"WRPLL 2",
> +	"SPLL"
>  };
>  
>  static void hsw_shared_dplls_init(struct drm_i915_private *dev_priv)
>  {
>  	int i;
>  
> -	dev_priv->num_shared_dpll = 2;
> +	dev_priv->num_shared_dpll = 3;
>  
> -	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
> +	for (i = 0; i < 2; i++) {
>  		dev_priv->shared_dplls[i].id = i;
>  		dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
> -		dev_priv->shared_dplls[i].disable = hsw_ddi_pll_disable;
> -		dev_priv->shared_dplls[i].enable = hsw_ddi_pll_enable;
> +		dev_priv->shared_dplls[i].disable = hsw_ddi_wrpll_disable;
> +		dev_priv->shared_dplls[i].enable = hsw_ddi_wrpll_enable;
>  		dev_priv->shared_dplls[i].get_hw_state =
> -			hsw_ddi_pll_get_hw_state;
> +			hsw_ddi_wrpll_get_hw_state;
>  	}
> +
> +	/* SPLL is special, but needs to be initialized anyway.. */
> +	dev_priv->shared_dplls[i].id = i;
> +	dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
> +	dev_priv->shared_dplls[i].disable = hsw_ddi_spll_disable;
> +	dev_priv->shared_dplls[i].enable = hsw_ddi_spll_enable;
> +	dev_priv->shared_dplls[i].get_hw_state = hsw_ddi_spll_get_hw_state;
> +
>  }
>  
>  static const char * const skl_ddi_pll_names[] = {
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3ca38fc493de..b383cbe3964c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4210,6 +4210,7 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
>  	struct intel_shared_dpll *pll;
>  	struct intel_shared_dpll_config *shared_dpll;
>  	enum intel_dpll_id i;
> +	int max = dev_priv->num_shared_dpll;
>  
>  	shared_dpll = intel_atomic_get_shared_dpll_state(crtc_state->base.state);
>  
> @@ -4244,9 +4245,11 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
>  		WARN_ON(shared_dpll[i].crtc_mask);
>  
>  		goto found;
> -	}
> +	} else if (INTEL_INFO(dev_priv)->gen < 9 && HAS_DDI(dev_priv))
> +		/* Do not consider SPLL */
> +		max = 2;
>  
> -	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
> +	for (i = 0; i < max; i++) {
>  		pll = &dev_priv->shared_dplls[i];
>  
>  		/* Only want to check enabled timings first */
> @@ -9767,6 +9770,8 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
>  	case PORT_CLK_SEL_WRPLL2:
>  		pipe_config->shared_dpll = DPLL_ID_WRPLL2;
>  		break;
> +	case PORT_CLK_SEL_SPLL:
> +		pipe_config->shared_dpll = DPLL_ID_SPLL;
>  	}
>  }
>  
> @@ -12070,9 +12075,10 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  			      pipe_config->dpll_hw_state.cfgcr1,
>  			      pipe_config->dpll_hw_state.cfgcr2);
>  	} else if (HAS_DDI(dev)) {
> -		DRM_DEBUG_KMS("ddi_pll_sel: %u; dpll_hw_state: wrpll: 0x%x\n",
> +		DRM_DEBUG_KMS("ddi_pll_sel: %u; dpll_hw_state: wrpll: 0x%x spll: 0x%x\n",
>  			      pipe_config->ddi_pll_sel,
> -			      pipe_config->dpll_hw_state.wrpll);
> +			      pipe_config->dpll_hw_state.wrpll,
> +			      pipe_config->dpll_hw_state.spll);
>  	} else {
>  		DRM_DEBUG_KMS("dpll_hw_state: dpll: 0x%x, dpll_md: 0x%x, "
>  			      "fp0: 0x%x, fp1: 0x%x\n",
> @@ -12607,6 +12613,7 @@ intel_pipe_config_compare(struct drm_device *dev,
>  	PIPE_CONF_CHECK_X(dpll_hw_state.fp0);
>  	PIPE_CONF_CHECK_X(dpll_hw_state.fp1);
>  	PIPE_CONF_CHECK_X(dpll_hw_state.wrpll);
> +	PIPE_CONF_CHECK_X(dpll_hw_state.spll);
>  	PIPE_CONF_CHECK_X(dpll_hw_state.ctrl1);
>  	PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr1);
>  	PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr2);

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

Re: [PATCH] drm/i915: Consider SPLL as another shared pll, v2.

[Gabriel Feceoru]

On 16.11.2015 17:53, Jani Nikula wrote:
> On Mon, 16 Nov 2015, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
>> When diagnosing a unrelated bug for someone on irc, it would seem the hardware can
>> be brought up by the BIOS with the embedded displayport using the SPLL for spread spectrum.
>>
>> Right now this is not handled well in i915, and it calculates the crtc needs to
>> be reprogrammed on the first modeset without SSC, but  the SPLL itself was kept
>> active. Fix this by exposing SPLL as a shared pll that will not be returned
>> by intel_get_shared_dpll; you have to know it exists to use it.
>>
>> Changes since v1:
>> - Create a separate dpll_hw_state.spll for spll, and use
>>    separate pll functions for spll.
>>
>> Tested-by: Emil Renner Berthing <kernel@esmil.dk> #v1
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>> Emil, can you retest?
>
> Gabriel, you too please!

I retested it and it's ok.

Regards,
Gabriel.

>
> BR,
> Jani.
>
>>
>>   drivers/gpu/drm/i915/i915_drv.h      |  3 ++
>>   drivers/gpu/drm/i915/intel_crt.c     | 31 ++-------------
>>   drivers/gpu/drm/i915/intel_ddi.c     | 75 +++++++++++++++++++++++++++++++-----
>>   drivers/gpu/drm/i915/intel_display.c | 15 ++++++--
>>   4 files changed, 83 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 85c84c624ed1..b6b62387cbe1 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -351,6 +351,8 @@ enum intel_dpll_id {
>>   	/* hsw/bdw */
>>   	DPLL_ID_WRPLL1 = 0,
>>   	DPLL_ID_WRPLL2 = 1,
>> +	DPLL_ID_SPLL = 2,
>> +
>>   	/* skl */
>>   	DPLL_ID_SKL_DPLL1 = 0,
>>   	DPLL_ID_SKL_DPLL2 = 1,
>> @@ -367,6 +369,7 @@ struct intel_dpll_hw_state {
>>
>>   	/* hsw, bdw */
>>   	uint32_t wrpll;
>> +	uint32_t spll;
>>
>>   	/* skl */
>>   	/*
>> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
>> index b84aaa0bb48a..6a2c76e367a5 100644
>> --- a/drivers/gpu/drm/i915/intel_crt.c
>> +++ b/drivers/gpu/drm/i915/intel_crt.c
>> @@ -138,18 +138,6 @@ static void hsw_crt_get_config(struct intel_encoder *encoder,
>>   	pipe_config->base.adjusted_mode.flags |= intel_crt_get_flags(encoder);
>>   }
>>
>> -static void hsw_crt_pre_enable(struct intel_encoder *encoder)
>> -{
>> -	struct drm_device *dev = encoder->base.dev;
>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>> -
>> -	WARN(I915_READ(SPLL_CTL) & SPLL_PLL_ENABLE, "SPLL already enabled\n");
>> -	I915_WRITE(SPLL_CTL,
>> -		   SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz | SPLL_PLL_SSC);
>> -	POSTING_READ(SPLL_CTL);
>> -	udelay(20);
>> -}
>> -
>>   /* Note: The caller is required to filter out dpms modes not supported by the
>>    * platform. */
>>   static void intel_crt_set_dpms(struct intel_encoder *encoder, int mode)
>> @@ -216,19 +204,6 @@ static void pch_post_disable_crt(struct intel_encoder *encoder)
>>   	intel_disable_crt(encoder);
>>   }
>>
>> -static void hsw_crt_post_disable(struct intel_encoder *encoder)
>> -{
>> -	struct drm_device *dev = encoder->base.dev;
>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>> -	uint32_t val;
>> -
>> -	DRM_DEBUG_KMS("Disabling SPLL\n");
>> -	val = I915_READ(SPLL_CTL);
>> -	WARN_ON(!(val & SPLL_PLL_ENABLE));
>> -	I915_WRITE(SPLL_CTL, val & ~SPLL_PLL_ENABLE);
>> -	POSTING_READ(SPLL_CTL);
>> -}
>> -
>>   static void intel_enable_crt(struct intel_encoder *encoder)
>>   {
>>   	struct intel_crt *crt = intel_encoder_to_crt(encoder);
>> @@ -280,6 +255,10 @@ static bool intel_crt_compute_config(struct intel_encoder *encoder,
>>   	if (HAS_DDI(dev)) {
>>   		pipe_config->ddi_pll_sel = PORT_CLK_SEL_SPLL;
>>   		pipe_config->port_clock = 135000 * 2;
>> +
>> +		pipe_config->dpll_hw_state.wrpll = 0;
>> +		pipe_config->dpll_hw_state.spll =
>> +			SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz | SPLL_PLL_SSC;
>>   	}
>>
>>   	return true;
>> @@ -860,8 +839,6 @@ void intel_crt_init(struct drm_device *dev)
>>   	if (HAS_DDI(dev)) {
>>   		crt->base.get_config = hsw_crt_get_config;
>>   		crt->base.get_hw_state = intel_ddi_get_hw_state;
>> -		crt->base.pre_enable = hsw_crt_pre_enable;
>> -		crt->base.post_disable = hsw_crt_post_disable;
>>   	} else {
>>   		crt->base.get_config = intel_crt_get_config;
>>   		crt->base.get_hw_state = intel_crt_get_hw_state;
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index abb4a265a6df..90fad7ad12ac 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -1286,6 +1286,18 @@ hsw_ddi_pll_select(struct intel_crtc *intel_crtc,
>>   		}
>>
>>   		crtc_state->ddi_pll_sel = PORT_CLK_SEL_WRPLL(pll->id);
>> +	} else if (crtc_state->ddi_pll_sel == PORT_CLK_SEL_SPLL) {
>> +		struct drm_atomic_state *state = crtc_state->base.state;
>> +		struct intel_shared_dpll_config *spll =
>> +			&intel_atomic_get_shared_dpll_state(state)[DPLL_ID_SPLL];
>> +
>> +		if (spll->crtc_mask &&
>> +		    WARN_ON(spll->hw_state.spll != crtc_state->dpll_hw_state.spll))
>> +			return false;
>> +
>> +		crtc_state->shared_dpll = DPLL_ID_SPLL;
>> +		spll->hw_state.spll = crtc_state->dpll_hw_state.spll;
>> +		spll->crtc_mask |= 1 << intel_crtc->pipe;
>>   	}
>>
>>   	return true;
>> @@ -2446,7 +2458,7 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder)
>>   	}
>>   }
>>
>> -static void hsw_ddi_pll_enable(struct drm_i915_private *dev_priv,
>> +static void hsw_ddi_wrpll_enable(struct drm_i915_private *dev_priv,
>>   			       struct intel_shared_dpll *pll)
>>   {
>>   	I915_WRITE(WRPLL_CTL(pll->id), pll->config.hw_state.wrpll);
>> @@ -2454,9 +2466,17 @@ static void hsw_ddi_pll_enable(struct drm_i915_private *dev_priv,
>>   	udelay(20);
>>   }
>>
>> -static void hsw_ddi_pll_disable(struct drm_i915_private *dev_priv,
>> +static void hsw_ddi_spll_enable(struct drm_i915_private *dev_priv,
>>   				struct intel_shared_dpll *pll)
>>   {
>> +	I915_WRITE(SPLL_CTL, pll->config.hw_state.spll);
>> +	POSTING_READ(SPLL_CTL);
>> +	udelay(20);
>> +}
>> +
>> +static void hsw_ddi_wrpll_disable(struct drm_i915_private *dev_priv,
>> +				  struct intel_shared_dpll *pll)
>> +{
>>   	uint32_t val;
>>
>>   	val = I915_READ(WRPLL_CTL(pll->id));
>> @@ -2464,9 +2484,19 @@ static void hsw_ddi_pll_disable(struct drm_i915_private *dev_priv,
>>   	POSTING_READ(WRPLL_CTL(pll->id));
>>   }
>>
>> -static bool hsw_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
>> -				     struct intel_shared_dpll *pll,
>> -				     struct intel_dpll_hw_state *hw_state)
>> +static void hsw_ddi_spll_disable(struct drm_i915_private *dev_priv,
>> +				 struct intel_shared_dpll *pll)
>> +{
>> +	uint32_t val;
>> +
>> +	val = I915_READ(SPLL_CTL);
>> +	I915_WRITE(SPLL_CTL, val & ~SPLL_PLL_ENABLE);
>> +	POSTING_READ(SPLL_CTL);
>> +}
>> +
>> +static bool hsw_ddi_wrpll_get_hw_state(struct drm_i915_private *dev_priv,
>> +				       struct intel_shared_dpll *pll,
>> +				       struct intel_dpll_hw_state *hw_state)
>>   {
>>   	uint32_t val;
>>
>> @@ -2479,25 +2509,50 @@ static bool hsw_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
>>   	return val & WRPLL_PLL_ENABLE;
>>   }
>>
>> +static bool hsw_ddi_spll_get_hw_state(struct drm_i915_private *dev_priv,
>> +				      struct intel_shared_dpll *pll,
>> +				      struct intel_dpll_hw_state *hw_state)
>> +{
>> +	uint32_t val;
>> +
>> +	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PLLS))
>> +		return false;
>> +
>> +	val = I915_READ(SPLL_CTL);
>> +	hw_state->spll = val;
>> +
>> +	return val & SPLL_PLL_ENABLE;
>> +}
>> +
>> +
>>   static const char * const hsw_ddi_pll_names[] = {
>>   	"WRPLL 1",
>>   	"WRPLL 2",
>> +	"SPLL"
>>   };
>>
>>   static void hsw_shared_dplls_init(struct drm_i915_private *dev_priv)
>>   {
>>   	int i;
>>
>> -	dev_priv->num_shared_dpll = 2;
>> +	dev_priv->num_shared_dpll = 3;
>>
>> -	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
>> +	for (i = 0; i < 2; i++) {
>>   		dev_priv->shared_dplls[i].id = i;
>>   		dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
>> -		dev_priv->shared_dplls[i].disable = hsw_ddi_pll_disable;
>> -		dev_priv->shared_dplls[i].enable = hsw_ddi_pll_enable;
>> +		dev_priv->shared_dplls[i].disable = hsw_ddi_wrpll_disable;
>> +		dev_priv->shared_dplls[i].enable = hsw_ddi_wrpll_enable;
>>   		dev_priv->shared_dplls[i].get_hw_state =
>> -			hsw_ddi_pll_get_hw_state;
>> +			hsw_ddi_wrpll_get_hw_state;
>>   	}
>> +
>> +	/* SPLL is special, but needs to be initialized anyway.. */
>> +	dev_priv->shared_dplls[i].id = i;
>> +	dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
>> +	dev_priv->shared_dplls[i].disable = hsw_ddi_spll_disable;
>> +	dev_priv->shared_dplls[i].enable = hsw_ddi_spll_enable;
>> +	dev_priv->shared_dplls[i].get_hw_state = hsw_ddi_spll_get_hw_state;
>> +
>>   }
>>
>>   static const char * const skl_ddi_pll_names[] = {
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 3ca38fc493de..b383cbe3964c 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -4210,6 +4210,7 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
>>   	struct intel_shared_dpll *pll;
>>   	struct intel_shared_dpll_config *shared_dpll;
>>   	enum intel_dpll_id i;
>> +	int max = dev_priv->num_shared_dpll;
>>
>>   	shared_dpll = intel_atomic_get_shared_dpll_state(crtc_state->base.state);
>>
>> @@ -4244,9 +4245,11 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
>>   		WARN_ON(shared_dpll[i].crtc_mask);
>>
>>   		goto found;
>> -	}
>> +	} else if (INTEL_INFO(dev_priv)->gen < 9 && HAS_DDI(dev_priv))
>> +		/* Do not consider SPLL */
>> +		max = 2;
>>
>> -	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
>> +	for (i = 0; i < max; i++) {
>>   		pll = &dev_priv->shared_dplls[i];
>>
>>   		/* Only want to check enabled timings first */
>> @@ -9767,6 +9770,8 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
>>   	case PORT_CLK_SEL_WRPLL2:
>>   		pipe_config->shared_dpll = DPLL_ID_WRPLL2;
>>   		break;
>> +	case PORT_CLK_SEL_SPLL:
>> +		pipe_config->shared_dpll = DPLL_ID_SPLL;
>>   	}
>>   }
>>
>> @@ -12070,9 +12075,10 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>>   			      pipe_config->dpll_hw_state.cfgcr1,
>>   			      pipe_config->dpll_hw_state.cfgcr2);
>>   	} else if (HAS_DDI(dev)) {
>> -		DRM_DEBUG_KMS("ddi_pll_sel: %u; dpll_hw_state: wrpll: 0x%x\n",
>> +		DRM_DEBUG_KMS("ddi_pll_sel: %u; dpll_hw_state: wrpll: 0x%x spll: 0x%x\n",
>>   			      pipe_config->ddi_pll_sel,
>> -			      pipe_config->dpll_hw_state.wrpll);
>> +			      pipe_config->dpll_hw_state.wrpll,
>> +			      pipe_config->dpll_hw_state.spll);
>>   	} else {
>>   		DRM_DEBUG_KMS("dpll_hw_state: dpll: 0x%x, dpll_md: 0x%x, "
>>   			      "fp0: 0x%x, fp1: 0x%x\n",
>> @@ -12607,6 +12613,7 @@ intel_pipe_config_compare(struct drm_device *dev,
>>   	PIPE_CONF_CHECK_X(dpll_hw_state.fp0);
>>   	PIPE_CONF_CHECK_X(dpll_hw_state.fp1);
>>   	PIPE_CONF_CHECK_X(dpll_hw_state.wrpll);
>> +	PIPE_CONF_CHECK_X(dpll_hw_state.spll);
>>   	PIPE_CONF_CHECK_X(dpll_hw_state.ctrl1);
>>   	PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr1);
>>   	PIPE_CONF_CHECK_X(dpll_hw_state.cfgcr2);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Consider SPLL as another shared pll, v2.

[Emil Renner Berthing]

On 18 November 2015 at 13:31, Gabriel Feceoru <gabriel.feceoru@intel.com> wrote:
>
>
> On 16.11.2015 17:53, Jani Nikula wrote:
>>
>> On Mon, 16 Nov 2015, Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> wrote:
>>>
>>> When diagnosing a unrelated bug for someone on irc, it would seem the
>>> hardware can
>>> be brought up by the BIOS with the embedded displayport using the SPLL
>>> for spread spectrum.
>>>
>>> Right now this is not handled well in i915, and it calculates the crtc
>>> needs to
>>> be reprogrammed on the first modeset without SSC, but  the SPLL itself
>>> was kept
>>> active. Fix this by exposing SPLL as a shared pll that will not be
>>> returned
>>> by intel_get_shared_dpll; you have to know it exists to use it.
>>>
>>> Changes since v1:
>>> - Create a separate dpll_hw_state.spll for spll, and use
>>>    separate pll functions for spll.
>>>
>>> Tested-by: Emil Renner Berthing <kernel@esmil.dk> #v1
>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> ---
>>> Emil, can you retest?
>>
>>
>> Gabriel, you too please!
>
>
> I retested it and it's ok.

It works fine for me too. I haven't run the tests yet, but I've run
the patch applied to v4.3 the last couple of days.

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

Re: [PATCH] drm/i915: Consider SPLL as another shared pll, v2.

[Jani Nikula]

On Wed, 18 Nov 2015, Emil Renner Berthing <kernel@esmil.dk> wrote:
> On 18 November 2015 at 13:31, Gabriel Feceoru <gabriel.feceoru@intel.com> wrote:
>>
>>
>> On 16.11.2015 17:53, Jani Nikula wrote:
>>>
>>> On Mon, 16 Nov 2015, Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> wrote:
>>>>
>>>> When diagnosing a unrelated bug for someone on irc, it would seem the
>>>> hardware can
>>>> be brought up by the BIOS with the embedded displayport using the SPLL
>>>> for spread spectrum.
>>>>
>>>> Right now this is not handled well in i915, and it calculates the crtc
>>>> needs to
>>>> be reprogrammed on the first modeset without SSC, but  the SPLL itself
>>>> was kept
>>>> active. Fix this by exposing SPLL as a shared pll that will not be
>>>> returned
>>>> by intel_get_shared_dpll; you have to know it exists to use it.
>>>>
>>>> Changes since v1:
>>>> - Create a separate dpll_hw_state.spll for spll, and use
>>>>    separate pll functions for spll.
>>>>
>>>> Tested-by: Emil Renner Berthing <kernel@esmil.dk> #v1
>>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> ---
>>>> Emil, can you retest?
>>>
>>>
>>> Gabriel, you too please!
>>
>>
>> I retested it and it's ok.
>
> It works fine for me too. I haven't run the tests yet, but I've run
> the patch applied to v4.3 the last couple of days.

Pushed to drm-intel-fixes, thanks for the patch, review and testing.

BR,
Jani.




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