Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: rip out early dp port write for gm45/ilk
@ 2012-09-12 21:24 Daniel Vetter
  2012-09-13 10:10 ` Chris Wilson
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Vetter @ 2012-09-12 21:24 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

It's bogus.

If I've followed the history of this piece of code correctly, i.e. the
initial register write with the following vblank wait, this goes all
the way back to the original enabling of DP support in

commit a4fc5ed69817c73e32571ad7837bb707f9890009
Author: Keith Packard <keithp@keithp.com>
Date:   Tue Apr 7 16:16:42 2009 -0700

    drm/i915: Add Display Port support

Unfortunately it seems to be nothing more than glorified duct-tape and
sometimes actively harmful. Adam Jackson noticed this for CPT
platforms with

commit e85194641bec56179dcf5e1704ce5c6bf30340c6
Author: Adam Jackson <ajax@redhat.com>
Date:   Thu Jul 21 17:48:38 2011 -0400

    drm/i915/dp: Don't turn CPT DP ports on too early

Unfortunately this kept the code around for ilk and gm45.

The specific failure case I'm seeing here is that after a dpms off/on
cycle we have the bits from the last link training (hopefully
successful link training) set in intel_dp->DP. This is requiered so
that complete_link_train can enable the port with the right tuning
values.

Unfortunately writing these again to the disabled port at dpms on time
kills the port somehow until it's disabled - dp link training fails in
an endless loop without this patch on my mobile ilk and gm45.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_dp.c |   13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 376ae28..3a4ebd4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1778,25 +1778,12 @@ static void
 intel_dp_start_link_train(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp->base.base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp->base.base.crtc);
 	int i;
 	uint8_t voltage;
 	bool clock_recovery = false;
 	int voltage_tries, loop_tries;
 	uint32_t DP = intel_dp->DP;
 
-	/*
-	 * On CPT we have to enable the port in training pattern 1, which
-	 * will happen below in intel_dp_set_link_train.  Otherwise, enable
-	 * the port and wait for it to become active.
-	 */
-	if (!HAS_PCH_CPT(dev)) {
-		I915_WRITE(intel_dp->output_reg, intel_dp->DP);
-		POSTING_READ(intel_dp->output_reg);
-		intel_wait_for_vblank(dev, intel_crtc->pipe);
-	}
-
 	/* Write the link configuration data */
 	intel_dp_aux_native_write(intel_dp, DP_LINK_BW_SET,
 				  intel_dp->link_configuration,
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/i915: rip out early dp port write for gm45/ilk
  2012-09-12 21:24 [PATCH] drm/i915: rip out early dp port write for gm45/ilk Daniel Vetter
@ 2012-09-13 10:10 ` Chris Wilson
  2012-09-13 11:39   ` Daniel Vetter
  0 siblings, 1 reply; 3+ messages in thread
From: Chris Wilson @ 2012-09-13 10:10 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

On Wed, 12 Sep 2012 23:24:09 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> It's bogus.
> 
> If I've followed the history of this piece of code correctly, i.e. the
> initial register write with the following vblank wait, this goes all
> the way back to the original enabling of DP support in
> 
> commit a4fc5ed69817c73e32571ad7837bb707f9890009
> Author: Keith Packard <keithp@keithp.com>
> Date:   Tue Apr 7 16:16:42 2009 -0700
> 
>     drm/i915: Add Display Port support
> 
> Unfortunately it seems to be nothing more than glorified duct-tape and
> sometimes actively harmful. Adam Jackson noticed this for CPT
> platforms with
> 
> commit e85194641bec56179dcf5e1704ce5c6bf30340c6
> Author: Adam Jackson <ajax@redhat.com>
> Date:   Thu Jul 21 17:48:38 2011 -0400
> 
>     drm/i915/dp: Don't turn CPT DP ports on too early
> 
> Unfortunately this kept the code around for ilk and gm45.
> 
> The specific failure case I'm seeing here is that after a dpms off/on
> cycle we have the bits from the last link training (hopefully
> successful link training) set in intel_dp->DP. This is requiered so
> that complete_link_train can enable the port with the right tuning
> values.
> 
> Unfortunately writing these again to the disabled port at dpms on time
> kills the port somehow until it's disabled - dp link training fails in
> an endless loop without this patch on my mobile ilk and gm45.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Works for me, only time will time if I no longer get the occasional
failure in link training, but it definitely fixes the issue with dpms
off.

Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
Probably https://bugs.freedesktop.org/show_bug.cgi?id=51493
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/i915: rip out early dp port write for gm45/ilk
  2012-09-13 10:10 ` Chris Wilson
@ 2012-09-13 11:39   ` Daniel Vetter
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Vetter @ 2012-09-13 11:39 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Daniel Vetter, Intel Graphics Development

On Thu, Sep 13, 2012 at 11:10:14AM +0100, Chris Wilson wrote:
> On Wed, 12 Sep 2012 23:24:09 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > It's bogus.
> > 
> > If I've followed the history of this piece of code correctly, i.e. the
> > initial register write with the following vblank wait, this goes all
> > the way back to the original enabling of DP support in
> > 
> > commit a4fc5ed69817c73e32571ad7837bb707f9890009
> > Author: Keith Packard <keithp@keithp.com>
> > Date:   Tue Apr 7 16:16:42 2009 -0700
> > 
> >     drm/i915: Add Display Port support
> > 
> > Unfortunately it seems to be nothing more than glorified duct-tape and
> > sometimes actively harmful. Adam Jackson noticed this for CPT
> > platforms with
> > 
> > commit e85194641bec56179dcf5e1704ce5c6bf30340c6
> > Author: Adam Jackson <ajax@redhat.com>
> > Date:   Thu Jul 21 17:48:38 2011 -0400
> > 
> >     drm/i915/dp: Don't turn CPT DP ports on too early
> > 
> > Unfortunately this kept the code around for ilk and gm45.
> > 
> > The specific failure case I'm seeing here is that after a dpms off/on
> > cycle we have the bits from the last link training (hopefully
> > successful link training) set in intel_dp->DP. This is requiered so
> > that complete_link_train can enable the port with the right tuning
> > values.
> > 
> > Unfortunately writing these again to the disabled port at dpms on time
> > kills the port somehow until it's disabled - dp link training fails in
> > an endless loop without this patch on my mobile ilk and gm45.
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Works for me, only time will time if I no longer get the occasional
> failure in link training, but it definitely fixes the issue with dpms
> off.
> 
> Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Probably https://bugs.freedesktop.org/show_bug.cgi?id=51493

Queued for -next, thanks for testing - imo this patch is too risky for
-fixes at this point in the release cycle.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-09-13 11:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-12 21:24 [PATCH] drm/i915: rip out early dp port write for gm45/ilk Daniel Vetter
2012-09-13 10:10 ` Chris Wilson
2012-09-13 11:39   ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox