Re: [Intel-gfx] [PATCH] drm/i915: CPT/PPT pch dp transcoder workaround

2012-11-01 Thread Jesse Barnes
On Thu,  1 Nov 2012 09:15:30 +0100
Daniel Vetter daniel.vet...@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 jbar...@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 jbar...@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 jbar...@virtuousgeek.org
 Cc: Paulo Zanoni przan...@gmail.com
 Cc: Arthur Ranyan arthur.j.run...@intel.com
 Signed-off-by: Daniel Vetter daniel.vet...@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  (131)
 +#define  TRANS_CHICKEN2_TIMING_OVERRIDE  (131)
 +
  
  #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  (131)
  #define  TRANS_DP_PORT_SEL_B (029)
  #define  TRANS_DP_PORT_SEL_C (129)
 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 

Re: [Intel-gfx] [PATCH] drm/i915: CPT/PPT pch dp transcoder workaround

2012-11-01 Thread Daniel Vetter
On Thu, Nov 01, 2012 at 07:37:36AM -0700, Jesse Barnes wrote:
  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.

See above: Paulo Zanoni pointed out that this workaround is also required
on the LPT PCH.

  +   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).

Paulo's vga patch bomb will split this up, so we can use HAS_PCH_CPT
instead of !IBX. But since I've written this patch against dinq without
paulo's patches, hence HAS_CPT would be wrong. So:

What colour would please you most, Sir?

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: CPT/PPT pch dp transcoder workaround

2012-11-01 Thread Jesse Barnes
On Thu, 1 Nov 2012 16:33:45 +0100
Daniel Vetter dan...@ffwll.ch wrote:

 On Thu, Nov 01, 2012 at 07:37:36AM -0700, Jesse Barnes wrote:
   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.
 
 See above: Paulo Zanoni pointed out that this workaround is also required
 on the LPT PCH.
 
   + 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).
 
 Paulo's vga patch bomb will split this up, so we can use HAS_PCH_CPT
 instead of !IBX. But since I've written this patch against dinq without
 paulo's patches, hence HAS_CPT would be wrong. So:
 
 What colour would please you most, Sir?

Just fix the changelog summary then!

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: CPT/PPT pch dp transcoder workaround

2012-11-01 Thread Paulo Zanoni
Hi

2012/11/1 Daniel Vetter daniel.vet...@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 jbar...@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 jbar...@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 jbar...@virtuousgeek.org
 Cc: Paulo Zanoni przan...@gmail.com
 Cc: Arthur Ranyan arthur.j.run...@intel.com
 Signed-off-by: Daniel Vetter daniel.vet...@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.zan...@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_CHICKEN20xf0064
  #define _TRANSB_CHICKEN20xf1064
  #define TRANS_CHICKEN2(pipe) _PIPE(pipe, _TRANSA_CHICKEN2, _TRANSB_CHICKEN2)
 -#define   TRANS_AUTOTRAIN_GEN_STALL_DIS(131)
 +#define  TRANS_CHICKEN2_TIMING_OVERRIDE(131)
 +

  #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(131)
  #define  TRANS_DP_PORT_SEL_B   (029)
  #define  TRANS_DP_PORT_SEL_C   (129)
 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;
 +