On Thu, 1 Nov 2012 09:15:30 +0100
Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> We need to set the timing override chicken bit after fdi link training
> has completed and before we enable the dp transcoder. We also have to
> clear that bit again after disabling the pch dp transcoder.
>
> See "Graphics BSpec: vol4g North Display Engine Registers [IVB],
> Display Mode Set Sequence" and "Graphics BSpec: vol4h South Display
> Engine Registers [CPT, PPT], South Display Engine Transcoder and FDI
> Control, Transcoder Debug and DFT, TRANS_CHICKEN_2" bit 31:
>
> "Workaround : Enable the override prior to enabling the transcoder.
> Disable the override after disabling the transcoder."
>
> While at it, use the _PIPE macro for the other TRANS_DP register.
>
> v2: Keep the w/a as-is, but kill the original (but wrongly placed)
> workaround introduced in
>
> commit 3bcf603f6d5d18bd9d076dc280de71f48add4101
> Author: Jesse Barnes <jbarnes at virtuousgeek.org>
> Date: Wed Jul 27 11:51:40 2011 -0700
>
> drm/i915: apply timing generator bug workaround on CPT and PPT
>
> and
>
> commit d4270e57efe9e2536798c59e1ed2fd0a1e5cdfcf
> Author: Jesse Barnes <jbarnes at virtuousgeek.org>
> Date: Tue Oct 11 10:43:02 2011 -0700
>
> drm/i915: export a CPT mode set verification function
>
> Note that this old code has unconditionally set the w/a, which might
> explain why fdi link training sometimes silently fails, and especially
> why the auto-train did not seem to work properly.
>
> v3: Paulo Zanoni pointed out that this workaround is also required on
> the LPT PCH. And Arthur Ranyan confirmed that this workaround is
> requierd for all ports on the pch, not just DP: The important part
> is that the bit is set whenever the pch transcoder is enabled, and
> that it is _not_ set while the fdi link is trained. It is also
> important that the pch transcoder is fully disabled, i.e. we have to
> wait for bit 30 to clear before clearing the w/a bit.
>
> Hence move to workaround into enable/disable_transcoder, where the pch
> transcoder gets enabled/disabled.
>
> v4: Whitespace changes dropped.
>
> v5: Don't run the w/a on IBX, we only need it on CPT/PPT and LPT.
>
> Cc: Jesse Barnes <jbarnes at virtuousgeek.org>
> Cc: Paulo Zanoni <przanoni at gmail.com>
> Cc: Arthur Ranyan <arthur.j.runyan at intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 5 +++--
> drivers/gpu/drm/i915/intel_display.c | 32 +++++++++++++++++++++++---------
> drivers/gpu/drm/i915/intel_pm.c | 4 ----
> 3 files changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2dd880f..f1fe3a0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3806,7 +3806,8 @@
> #define _TRANSA_CHICKEN2 0xf0064
> #define _TRANSB_CHICKEN2 0xf1064
> #define TRANS_CHICKEN2(pipe) _PIPE(pipe, _TRANSA_CHICKEN2, _TRANSB_CHICKEN2)
> -#define TRANS_AUTOTRAIN_GEN_STALL_DIS (1<<31)
> +#define TRANS_CHICKEN2_TIMING_OVERRIDE (1<<31)
> +
>
> #define SOUTH_CHICKEN1 0xc2000
> #define FDIA_PHASE_SYNC_SHIFT_OVR 19
> @@ -4064,7 +4065,7 @@
> #define TRANS_DP_CTL_A 0xe0300
> #define TRANS_DP_CTL_B 0xe1300
> #define TRANS_DP_CTL_C 0xe2300
> -#define TRANS_DP_CTL(pipe) (TRANS_DP_CTL_A + (pipe) * 0x01000)
> +#define TRANS_DP_CTL(pipe) _PIPE(pipe, TRANS_DP_CTL_A, TRANS_DP_CTL_B)
> #define TRANS_DP_OUTPUT_ENABLE (1<<31)
> #define TRANS_DP_PORT_SEL_B (0<<29)
> #define TRANS_DP_PORT_SEL_C (1<<29)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 129059b..675079a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1673,9 +1673,9 @@ static void intel_disable_pch_pll(struct intel_crtc *intel_crtc)
> static void intel_enable_transcoder(struct drm_i915_private *dev_priv,
> enum pipe pipe)
> {
> - int reg;
> - u32 val, pipeconf_val;
> + struct drm_device *dev = dev_priv->dev;
> struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> + uint32_t reg, val, pipeconf_val;
>
> /* PCH only available on ILK+ */
> BUG_ON(dev_priv->info->gen < 5);
> @@ -1693,6 +1693,16 @@ static void intel_enable_transcoder(struct drm_i915_private *dev_priv,
> DRM_ERROR("Attempting to enable transcoder on Haswell with pipe > 0\n");
> return;
> }
> +
> + if (!HAS_PCH_IBX(dev)) {
> + /* Workaround: Set the timing override bit before enabling the
> + * pch transcoder. */
> + reg = TRANS_CHICKEN2(pipe);
> + val = I915_READ(reg);
> + val |= TRANS_CHICKEN2_TIMING_OVERRIDE;
> + I915_WRITE(reg, val);
> + }
I'd like this better if it were HAS_PCH_CPT; we use that as a synonym
for PPT elsehwere, and it shouldn't apply to LPT right? I see LPT has
the bit, but I don't know if it's needed (the changelong and summary
are misleading if so).
> +
> reg = TRANSCONF(pipe);
> val = I915_READ(reg);
> pipeconf_val = I915_READ(PIPECONF(pipe));
> @@ -1724,8 +1734,8 @@ static void intel_enable_transcoder(struct drm_i915_private *dev_priv,
> static void intel_disable_transcoder(struct drm_i915_private *dev_priv,
> enum pipe pipe)
> {
> - int reg;
> - u32 val;
> + struct drm_device *dev = dev_priv->dev;
> + uint32_t reg, val;
>
> /* FDI relies on the transcoder */
> assert_fdi_tx_disabled(dev_priv, pipe);
> @@ -1741,6 +1751,14 @@ static void intel_disable_transcoder(struct drm_i915_private *dev_priv,
> /* wait for PCH transcoder off, transcoder state */
> if (wait_for((I915_READ(reg) & TRANS_STATE_ENABLE) == 0, 50))
> DRM_ERROR("failed to disable transcoder %d\n", pipe);
> +
> + if (!HAS_PCH_IBX(dev)) {
> + /* Workaround: Clear the timing override chicken bit again. */
> + reg = TRANS_CHICKEN2(pipe);
> + val = I915_READ(reg);
> + val &= ~TRANS_CHICKEN2_TIMING_OVERRIDE;
> + I915_WRITE(reg, val);
> + }
> }
>
> /**
> @@ -3217,16 +3235,12 @@ prepare: /* separate function? */
> void intel_cpt_verify_modeset(struct drm_device *dev, int pipe)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - int dslreg = PIPEDSL(pipe), tc2reg = TRANS_CHICKEN2(pipe);
> + int dslreg = PIPEDSL(pipe);
> u32 temp;
>
> temp = I915_READ(dslreg);
> udelay(500);
> if (wait_for(I915_READ(dslreg) != temp, 5)) {
> - /* Without this, mode sets may fail silently on FDI */
> - I915_WRITE(tc2reg, TRANS_AUTOTRAIN_GEN_STALL_DIS);
> - udelay(250);
> - I915_WRITE(tc2reg, 0);
> if (wait_for(I915_READ(dslreg) != temp, 5))
> DRM_ERROR("mode set failed: pipe %d stuck\n", pipe);
> }
Note this was an older version of the workaround. "wrongly placed"
indeed.
--
Jesse Barnes, Intel Open Source Technology Center
