diff mbox

[6/7] drm/i915: Disable FDI RX before FDI TX

Message ID 1344918891-6283-7-git-send-email-keithp@keithp.com
State New, archived
Headers show

Commit Message

Keith Packard Aug. 14, 2012, 4:34 a.m. UTC
Doesn't make sense to disable in the other order.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/intel_display.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Lespiau, Damien Aug. 17, 2012, 4:43 p.m. UTC | #1
On Tue, Aug 14, 2012 at 5:34 AM, Keith Packard <keithp@keithp.com> wrote:
> Doesn't make sense to disable in the other order.
>
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

I can't see anything in the docs about an order requirement for those.
Not sure why the other way does not make sense. Somehow disabling TX
before RX makes some sense to me (TX enabled without a ready RX looks
weird?, no data should flow as the pipe is shutdown at that point
anyway). Maybe it just does not matter?

Another detail is that disabling the PLLs seem to have an order in the
disabling sequence, TX, then RX.

I.  Disable CPU FDI Transmitter PLL
II. Disable PCH FDI Receiver PLL
Keith Packard Aug. 17, 2012, 11:10 p.m. UTC | #2
"Lespiau, Damien" <damien.lespiau@intel.com> writes:

> I can't see anything in the docs about an order requirement for those.

Right, the docs don't say anything, which is a bit disconcerting.

> Not sure why the other way does not make sense. Somehow disabling TX
> before RX makes some sense to me (TX enabled without a ready RX looks
> weird?, no data should flow as the pipe is shutdown at that point
> anyway). Maybe it just does not matter?

And here I figured disabling RX before TX made more sense -- otherwise
the receiver wouldn't be seeing anything. In other areas of the driver,
we're careful to disable receivers before senders (disable CRTC before
PLL, etc).

> Another detail is that disabling the PLLs seem to have an order in the
> disabling sequence, TX, then RX.
>
> I.  Disable CPU FDI Transmitter PLL
> II. Disable PCH FDI Receiver PLL

That ordering doesn't matter as the FDI receiver and transmitter are
both disabled by that point, so they aren't talking at all.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b099a17..754f10f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2776,17 +2776,17 @@  static void ironlake_fdi_disable(struct drm_crtc *crtc)
 	u32 reg, temp;
 
 	/* disable CPU FDI tx and PCH FDI rx */
-	reg = FDI_TX_CTL(pipe);
-	temp = I915_READ(reg);
-	I915_WRITE(reg, temp & ~FDI_TX_ENABLE);
-	POSTING_READ(reg);
-
 	reg = FDI_RX_CTL(pipe);
 	temp = I915_READ(reg);
 	temp &= ~(0x7 << 16);
 	temp |= (I915_READ(PIPECONF(pipe)) & PIPE_BPC_MASK) << 11;
 	I915_WRITE(reg, temp & ~FDI_RX_ENABLE);
+	POSTING_READ(reg);
+	udelay(100);
 
+	reg = FDI_TX_CTL(pipe);
+	temp = I915_READ(reg);
+	I915_WRITE(reg, temp & ~FDI_TX_ENABLE);
 	POSTING_READ(reg);
 	udelay(100);