Re: [Intel-gfx] [v2] drm/i915: Add correct hw/sw config check for DSI encoder
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
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 = 10;
>
> + /* 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)
>
> mute
[Intel-gfx] [v2] drm/i915: Add correct hw/sw config check for DSI encoder
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
---
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 = 10;
+ /* 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;
+
