diff mbox

[v2] drm/i915: Add correct hw/sw config check for DSI encoder

Message ID 1405428329-5575-1-git-send-email-shobhit.kumar@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kumar, Shobhit July 15, 2014, 12:45 p.m. UTC
Check in vlv_crtc_clock_get if DPLL is enabled before calling dpio read.
It will not be enabled for DSI and avoid dpio read WARN dumps.

Absence of ->get_config was causing other WARN dumps as well. Update
dpll_hw_state as well correctly

v2: Address review comments by Daniel
    - Check if DPLL is enabled rather than checking pipe output type
    - set adjusted_mode->flags to 0 in compute_config rather than using
      pipe_config->quirks
    - Add helper function in intel_dsi_pll.c and use that in intel_dsi.c
    - updated dpll_hw_state correctly
    - Updated commit message and title

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  4 ++++
 drivers/gpu/drm/i915/intel_dsi.c     | 21 +++++++++++++++-
 drivers/gpu/drm/i915/intel_dsi.h     |  1 +
 drivers/gpu/drm/i915/intel_dsi_pll.c | 46 ++++++++++++++++++++++++++++++++++++
 4 files changed, 71 insertions(+), 1 deletion(-)

Comments

Imre Deak July 29, 2014, 12:22 p.m. UTC | #1
On Tue, 2014-07-15 at 18:15 +0530, Shobhit Kumar wrote:
> Check in vlv_crtc_clock_get if DPLL is enabled before calling dpio read.
> It will not be enabled for DSI and avoid dpio read WARN dumps.
> 
> Absence of ->get_config was causing other WARN dumps as well. Update
> dpll_hw_state as well correctly
> 
> v2: Address review comments by Daniel
>     - Check if DPLL is enabled rather than checking pipe output type
>     - set adjusted_mode->flags to 0 in compute_config rather than using
>       pipe_config->quirks
>     - Add helper function in intel_dsi_pll.c and use that in intel_dsi.c
>     - updated dpll_hw_state correctly
>     - Updated commit message and title
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>

Ok, reviewing now the latest version after Daniel pointed me to it.

> ---
>  drivers/gpu/drm/i915/intel_display.c |  4 ++++
>  drivers/gpu/drm/i915/intel_dsi.c     | 21 +++++++++++++++-
>  drivers/gpu/drm/i915/intel_dsi.h     |  1 +
>  drivers/gpu/drm/i915/intel_dsi_pll.c | 46 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 71 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c89b4ac..d9c34e4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6132,6 +6132,10 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
>  	u32 mdiv;
>  	int refclk = 100000;
>  
> +	/* In case of MIPI DPLL will not even be used */
> +	if (!(pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE))
> +		return;
> +
>  	mutex_lock(&dev_priv->dpio_lock);
>  	mdiv = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW3(pipe));
>  	mutex_unlock(&dev_priv->dpio_lock);
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index bfcefbf..43be71bf 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -92,6 +92,9 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
>  	if (fixed_mode)
>  		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
>  
> +	/* DSI uses short packets for sync events, so clear mode flags for DSI */
> +	adjusted_mode->flags = 0;
> +
>  	if (intel_dsi->dev.dev_ops->mode_fixup)
>  		return intel_dsi->dev.dev_ops->mode_fixup(&intel_dsi->dev,
>  							  mode, adjusted_mode);
> @@ -177,6 +180,10 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>  	tmp |= DPLL_REFA_CLK_ENABLE_VLV;
>  	I915_WRITE(DPLL(pipe), tmp);
>  
> +	/* update the hw state for DPLL */
> +	intel_crtc->config.dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
> +						DPLL_REFA_CLK_ENABLE_VLV;
> +
>  	tmp = I915_READ(DSPCLK_GATE_D);
>  	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
>  	I915_WRITE(DSPCLK_GATE_D, tmp);
> @@ -351,9 +358,21 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>  static void intel_dsi_get_config(struct intel_encoder *encoder,
>  				 struct intel_crtc_config *pipe_config)
>  {
> +	u32 pclk;
>  	DRM_DEBUG_KMS("\n");
>  
> -	/* XXX: read flags, set to adjusted_mode */
> +	/*
> +	 * DPLL_MD is not used in case of DSI, reading will get some default value
> +	 * set dpll_md = 0
> +	 */
> +	pipe_config->dpll_hw_state.dpll_md = 0;
> +
> +	pclk = vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
> +	if (!pclk)
> +		return;
> +
> +	pipe_config->adjusted_mode.crtc_clock = pclk;
> +	pipe_config->port_clock = pclk;
>  }
>  
>  static enum drm_mode_status
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 31db33d..fd51867 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -132,6 +132,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
>  
>  extern void vlv_enable_dsi_pll(struct intel_encoder *encoder);
>  extern void vlv_disable_dsi_pll(struct intel_encoder *encoder);
> +extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
>  
>  extern struct intel_dsi_dev_ops vbt_generic_dsi_display_ops;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index ba79ec1..8085afe 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -50,6 +50,8 @@ static const u32 lfsr_converts[] = {
>  	71, 35							/* 91 - 92 */
>  };
>  
> +static const int num_lfsr_converts = sizeof(lfsr_converts) / sizeof(lfsr_converts[0]);
> +

This could be just inlined using ARRAY_SIZE.

>  #ifdef DSI_CLK_FROM_RR
>  
>  static u32 dsi_rr_formula(const struct drm_display_mode *mode,
> @@ -298,3 +300,47 @@ void vlv_disable_dsi_pll(struct intel_encoder *encoder)
>  
>  	mutex_unlock(&dev_priv->dpio_lock);
>  }
> +
> +u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
> +{
> +	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	u32 dsi_clock, pclk;
> +	u32 pll_ctl, pll_div;
> +	u32 m = 0, p = 0;
> +	int refclk = 25000;
> +	int i;
> +
> +	DRM_DEBUG_KMS("\n");
> +
> +	mutex_lock(&dev_priv->dpio_lock);
> +	pll_ctl = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);
> +	pll_div = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_DIVIDER);
> +	mutex_unlock(&dev_priv->dpio_lock);
> +
> +	pll_ctl &= ~(DSI_PLL_CLK_GATE_DSI0_DSIPLL | DSI_PLL_VCO_EN);
> +	pll_ctl = pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2);
> +
> +	while (pll_ctl) {
> +		pll_ctl = pll_ctl >> 1;
> +		p++;
> +	}
> +	p--;
> +
> +	for (i = 0; i < num_lfsr_converts; i++) {
> +		if (lfsr_converts[i] == pll_div)
> +			break;
> +	}
> +
> +	if (i == num_lfsr_converts) {
> +		DRM_ERROR("wrong m_seed programmed\n");
> +		return 0;
> +	}
> +
> +	m = i + 62;
> +
> +	dsi_clock = (m * refclk) / p;
> +	pclk = DIV_ROUND_CLOSEST(dsi_clock * intel_dsi->lane_count, pipe_bpp);
> +
> +	return pclk;
> +}

Please see my comments for intel_dsi_get_config() in my v1 review where
they apply in vlv_get_dsi_pclk().

--Imre
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c89b4ac..d9c34e4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6132,6 +6132,10 @@  static void vlv_crtc_clock_get(struct intel_crtc *crtc,
 	u32 mdiv;
 	int refclk = 100000;
 
+	/* In case of MIPI DPLL will not even be used */
+	if (!(pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE))
+		return;
+
 	mutex_lock(&dev_priv->dpio_lock);
 	mdiv = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW3(pipe));
 	mutex_unlock(&dev_priv->dpio_lock);
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index bfcefbf..43be71bf 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -92,6 +92,9 @@  static bool intel_dsi_compute_config(struct intel_encoder *encoder,
 	if (fixed_mode)
 		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
 
+	/* DSI uses short packets for sync events, so clear mode flags for DSI */
+	adjusted_mode->flags = 0;
+
 	if (intel_dsi->dev.dev_ops->mode_fixup)
 		return intel_dsi->dev.dev_ops->mode_fixup(&intel_dsi->dev,
 							  mode, adjusted_mode);
@@ -177,6 +180,10 @@  static void intel_dsi_pre_enable(struct intel_encoder *encoder)
 	tmp |= DPLL_REFA_CLK_ENABLE_VLV;
 	I915_WRITE(DPLL(pipe), tmp);
 
+	/* update the hw state for DPLL */
+	intel_crtc->config.dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
+						DPLL_REFA_CLK_ENABLE_VLV;
+
 	tmp = I915_READ(DSPCLK_GATE_D);
 	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
 	I915_WRITE(DSPCLK_GATE_D, tmp);
@@ -351,9 +358,21 @@  static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
 static void intel_dsi_get_config(struct intel_encoder *encoder,
 				 struct intel_crtc_config *pipe_config)
 {
+	u32 pclk;
 	DRM_DEBUG_KMS("\n");
 
-	/* XXX: read flags, set to adjusted_mode */
+	/*
+	 * DPLL_MD is not used in case of DSI, reading will get some default value
+	 * set dpll_md = 0
+	 */
+	pipe_config->dpll_hw_state.dpll_md = 0;
+
+	pclk = vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
+	if (!pclk)
+		return;
+
+	pipe_config->adjusted_mode.crtc_clock = pclk;
+	pipe_config->port_clock = pclk;
 }
 
 static enum drm_mode_status
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 31db33d..fd51867 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -132,6 +132,7 @@  static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
 
 extern void vlv_enable_dsi_pll(struct intel_encoder *encoder);
 extern void vlv_disable_dsi_pll(struct intel_encoder *encoder);
+extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
 
 extern struct intel_dsi_dev_ops vbt_generic_dsi_display_ops;
 
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index ba79ec1..8085afe 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -50,6 +50,8 @@  static const u32 lfsr_converts[] = {
 	71, 35							/* 91 - 92 */
 };
 
+static const int num_lfsr_converts = sizeof(lfsr_converts) / sizeof(lfsr_converts[0]);
+
 #ifdef DSI_CLK_FROM_RR
 
 static u32 dsi_rr_formula(const struct drm_display_mode *mode,
@@ -298,3 +300,47 @@  void vlv_disable_dsi_pll(struct intel_encoder *encoder)
 
 	mutex_unlock(&dev_priv->dpio_lock);
 }
+
+u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
+{
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	u32 dsi_clock, pclk;
+	u32 pll_ctl, pll_div;
+	u32 m = 0, p = 0;
+	int refclk = 25000;
+	int i;
+
+	DRM_DEBUG_KMS("\n");
+
+	mutex_lock(&dev_priv->dpio_lock);
+	pll_ctl = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);
+	pll_div = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_DIVIDER);
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	pll_ctl &= ~(DSI_PLL_CLK_GATE_DSI0_DSIPLL | DSI_PLL_VCO_EN);
+	pll_ctl = pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2);
+
+	while (pll_ctl) {
+		pll_ctl = pll_ctl >> 1;
+		p++;
+	}
+	p--;
+
+	for (i = 0; i < num_lfsr_converts; i++) {
+		if (lfsr_converts[i] == pll_div)
+			break;
+	}
+
+	if (i == num_lfsr_converts) {
+		DRM_ERROR("wrong m_seed programmed\n");
+		return 0;
+	}
+
+	m = i + 62;
+
+	dsi_clock = (m * refclk) / p;
+	pclk = DIV_ROUND_CLOSEST(dsi_clock * intel_dsi->lane_count, pipe_bpp);
+
+	return pclk;
+}