Intel-GFX Archive on lore.kernel.org
 help / color / Atom feed
From: Paulo Zanoni <przanoni@gmail.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	Arthur Ranyan <arthur.j.runyan@intel.com>
Subject: Re: [PATCH] drm/i915: CPT/PPT pch dp transcoder workaround
Date: Thu, 1 Nov 2012 14:12:58 -0200
Message-ID: <CA+gsUGS0Ut-Gge_LZ346hXmLY_4vt3CDfB9rT-8tPJD9=fu8ug@mail.gmail.com> (raw)
In-Reply-To: <1351757730-11463-1-git-send-email-daniel.vetter@ffwll.ch>

Hi

2012/11/1 Daniel Vetter <daniel.vetter@ffwll.ch>:
> 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@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@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

s/requierd/required/


> 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@virtuousgeek.org>
> Cc: Paulo Zanoni <przanoni@gmail.com>
> Cc: Arthur Ranyan <arthur.j.runyan@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

For me, it doesn't matter if you do !HAS_PCH_IBX or HAS_PCH_CPT, I
will have to fork the functions for LPT anyway, so I will fix that,
independently of what you do here.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> ---
>  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);
> +       }
> +
>         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);
>         }
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a04b336..3469fbd 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3786,7 +3786,6 @@ static void ibx_init_clock_gating(struct drm_device *dev)
>  static void cpt_init_clock_gating(struct drm_device *dev)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
> -       int pipe;
>
>         /*
>          * On Ibex Peak and Cougar Point, we need to disable clock
> @@ -3796,9 +3795,6 @@ static void cpt_init_clock_gating(struct drm_device *dev)
>         I915_WRITE(SOUTH_DSPCLK_GATE_D, PCH_DPLSUNIT_CLOCK_GATE_DISABLE);
>         I915_WRITE(SOUTH_CHICKEN2, I915_READ(SOUTH_CHICKEN2) |
>                    DPLS_EDP_PPS_FIX_DIS);
> -       /* Without this, mode sets may fail silently on FDI */
> -       for_each_pipe(pipe)
> -               I915_WRITE(TRANS_CHICKEN2(pipe), TRANS_AUTOTRAIN_GEN_STALL_DIS);
>  }
>
>  void intel_init_clock_gating(struct drm_device *dev)
> --
> 1.7.11.4
>



-- 
Paulo Zanoni

  parent reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-31 21:52 [PATCH 0/5] ivb fdi b/c fixes Daniel Vetter
2012-10-31 21:52 ` [PATCH 1/5] drm/i915: check whether the pch is the soulmate of the cpu Daniel Vetter
2012-11-01 15:58   ` Paulo Zanoni
2012-10-31 21:52 ` [PATCH 2/5] drm/i915: drop unnecessary check from fdi_link_train code Daniel Vetter
2012-10-31 21:52 ` [PATCH 3/5] drm/i915: CPT/PPT pch transcoder workaround Daniel Vetter
2012-11-01  8:15   ` [PATCH] drm/i915: CPT/PPT pch dp " Daniel Vetter
2012-11-01 14:37     ` Jesse Barnes
2012-11-01 15:33       ` Daniel Vetter
2012-11-01 15:35         ` Jesse Barnes
2012-11-01 16:12     ` Paulo Zanoni [this message]
2012-10-31 21:52 ` [PATCH 4/5] drm/i915: implement WADP0ClockGatingDisable Daniel Vetter
2012-11-01 16:18   ` Paulo Zanoni
2012-10-31 21:52 ` [PATCH 5/5] drm/i915: kill pch_init_clock_gating indirection Daniel Vetter
2012-11-01 16:35   ` Paulo Zanoni
2012-11-01 22:03     ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CA+gsUGS0Ut-Gge_LZ346hXmLY_4vt3CDfB9rT-8tPJD9=fu8ug@mail.gmail.com' \
    --to=przanoni@gmail.com \
    --cc=arthur.j.runyan@intel.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Intel-GFX Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/intel-gfx/0 intel-gfx/git/0.git
	git clone --mirror https://lore.kernel.org/intel-gfx/1 intel-gfx/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 intel-gfx intel-gfx/ https://lore.kernel.org/intel-gfx \
		intel-gfx@lists.freedesktop.org
	public-inbox-index intel-gfx

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.freedesktop.lists.intel-gfx


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git