On Wed, 2013-06-05 at 13:34 +0200, Daniel Vetter wrote:
> Lots of bangin my head against the wall^UExperiments have shown that
> we really need to enable the lvds port before we enable plls. Strangely
> that seems to include the fdi rx pll on the pch.
>
> Anyway, encode this new evidence with a few nice WARNs.
>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_display.c | 23 ++++++++++-------------
> drivers/gpu/drm/i915/intel_drv.h | 16 ++++++++++++++++
> drivers/gpu/drm/i915/intel_lvds.c | 17 ++++++++++++-----
> 3 files changed, 38 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5d84fea..7b34a92 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -874,8 +874,8 @@ static const char *state_string(bool enabled)
> }
>
> /* Only for pre-ILK configs */
> -static void assert_pll(struct drm_i915_private *dev_priv,
> - enum pipe pipe, bool state)
> +void assert_pll(struct drm_i915_private *dev_priv,
> + enum pipe pipe, bool state)
> {
> int reg;
> u32 val;
> @@ -888,10 +888,8 @@ static void assert_pll(struct drm_i915_private *dev_priv,
> "PLL state assertion failure (expected %s, current %s)\n",
> state_string(state), state_string(cur_state));
> }
> -#define assert_pll_enabled(d, p) assert_pll(d, p, true)
> -#define assert_pll_disabled(d, p) assert_pll(d, p, false)
>
> -static struct intel_shared_dpll *
> +struct intel_shared_dpll *
> intel_crtc_to_shared_dpll(struct intel_crtc *crtc)
> {
> struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> @@ -903,9 +901,9 @@ intel_crtc_to_shared_dpll(struct intel_crtc *crtc)
> }
>
> /* For ILK+ */
> -static void assert_shared_dpll(struct drm_i915_private *dev_priv,
> - struct intel_shared_dpll *pll,
> - bool state)
> +void assert_shared_dpll(struct drm_i915_private *dev_priv,
> + struct intel_shared_dpll *pll,
> + bool state)
> {
> bool cur_state;
> struct intel_dpll_hw_state hw_state;
> @@ -924,8 +922,6 @@ static void assert_shared_dpll(struct drm_i915_private *dev_priv,
> "%s assertion failure (expected %s, current %s)\n",
> pll->name, state_string(state), state_string(cur_state));
> }
> -#define assert_shared_dpll_enabled(d, p) assert_shared_dpll(d, p, true)
> -#define assert_shared_dpll_disabled(d, p) assert_shared_dpll(d, p, false)
>
> static void assert_fdi_tx(struct drm_i915_private *dev_priv,
> enum pipe pipe, bool state)
> @@ -989,15 +985,16 @@ static void assert_fdi_tx_pll_enabled(struct drm_i915_private *dev_priv,
> WARN(!(val & FDI_TX_PLL_ENABLE), "FDI TX PLL assertion failure, should be active but is disabled\n");
> }
>
> -static void assert_fdi_rx_pll_enabled(struct drm_i915_private *dev_priv,
> - enum pipe pipe)
> +void assert_fdi_rx_pll(struct drm_i915_private *dev_priv,
> + enum pipe pipe, bool state)
> {
> int reg;
> u32 val;
>
> reg = FDI_RX_CTL(pipe);
> val = I915_READ(reg);
> - WARN(!(val & FDI_RX_PLL_ENABLE), "FDI RX PLL assertion failure, should be active but is disabled\n");
> + WARN(!!(val & FDI_RX_PLL_ENABLE) != state,
> + "FDI RX PLL assertion failure, should be active but is disabled\n");
The message should be updated too.
> }
>
> static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 6f28375..ea8aa5e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -732,6 +732,22 @@ extern int intel_overlay_attrs(struct drm_device *dev, void *data,
> extern void intel_fb_output_poll_changed(struct drm_device *dev);
> extern void intel_fb_restore_mode(struct drm_device *dev);
>
> +struct intel_shared_dpll *
> +intel_crtc_to_shared_dpll(struct intel_crtc *crtc);
> +
> +void assert_shared_dpll(struct drm_i915_private *dev_priv,
> + struct intel_shared_dpll *pll,
> + bool state);
> +#define assert_shared_dpll_enabled(d, p) assert_shared_dpll(d, p, true)
> +#define assert_shared_dpll_disabled(d, p) assert_shared_dpll(d, p, false)
> +void assert_pll(struct drm_i915_private *dev_priv,
> + enum pipe pipe, bool state);
> +#define assert_pll_enabled(d, p) assert_pll(d, p, true)
> +#define assert_pll_disabled(d, p) assert_pll(d, p, false)
> +void assert_fdi_rx_pll(struct drm_i915_private *dev_priv,
> + enum pipe pipe, bool state);
> +#define assert_fdi_rx_pll_enabled(d, p) assert_fdi_rx_pll(d, p, true)
> +#define assert_fdi_rx_pll_disabled(d, p) assert_fdi_rx_pll(d, p, false)
> extern void assert_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
> bool state);
> #define assert_pipe_enabled(d, p) assert_pipe(d, p, true)
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 159aa9f..36f8901 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -120,12 +120,20 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder)
> struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
> struct drm_device *dev = encoder->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> + struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> struct drm_display_mode *fixed_mode =
> lvds_encoder->attached_connector->base.panel.fixed_mode;
> - int pipe = intel_crtc->pipe;
> + int pipe = crtc->pipe;
> u32 temp;
>
> + if (HAS_PCH_SPLIT(dev)) {
> + assert_fdi_rx_pll_disabled(dev_priv, pipe);
> + assert_shared_dpll_disabled(dev_priv,
> + intel_crtc_to_shared_dpll(crtc));
I think if we pick a shared PLL that is currently used by another port
this will trigger. Should the PLL selection be limited to non-shared
PLLs for LVDS?
> + } else {
> + assert_pll_disabled(dev_priv, pipe);
> + }
> +
> temp = I915_READ(lvds_encoder->reg);
> temp |= LVDS_PORT_EN | LVDS_A0A2_CLKA_POWER_UP;
>
> @@ -142,7 +150,7 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder)
>
> /* set the corresponsding LVDS_BORDER bit */
> temp &= ~LVDS_BORDER_ENABLE;
> - temp |= intel_crtc->config.gmch_pfit.lvds_border_bits;
> + temp |= crtc->config.gmch_pfit.lvds_border_bits;
> /* Set the B0-B3 data pairs corresponding to whether we're going to
> * set the DPLLs for dual-channel mode or not.
> */
> @@ -162,8 +170,7 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder)
> if (INTEL_INFO(dev)->gen == 4) {
> /* Bspec wording suggests that LVDS port dithering only exists
> * for 18bpp panels. */
> - if (intel_crtc->config.dither &&
> - intel_crtc->config.pipe_bpp == 18)
> + if (crtc->config.dither && crtc->config.pipe_bpp == 18)
> temp |= LVDS_ENABLE_DITHER;
> else
> temp &= ~LVDS_ENABLE_DITHER;
