[PATCH] drm/i915: always set FDI composite sync bit

[PATCH] drm/i915: always set FDI composite sync bit

[Jesse Barnes]

It's needed for 3 pipe support as well as just regular functionality
(e.g. DisplayPort).

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_reg.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |    2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 138eae1..20e2299 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3180,6 +3180,7 @@
 #define  FDI_LINK_TRAIN_NONE_IVB            (3<<8)
 
 /* both Tx and Rx */
+#define  FDI_COMPOSITE_SYNC		(1<<11)
 #define  FDI_LINK_TRAIN_AUTO		(1<<10)
 #define  FDI_SCRAMBLING_ENABLE          (0<<7)
 #define  FDI_SCRAMBLING_DISABLE         (1<<7)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f0e5f9f..cd891fc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2593,6 +2593,7 @@ static void ivb_manual_fdi_link_train(struct drm_crtc *crtc)
 	temp |= FDI_LINK_TRAIN_PATTERN_1_IVB;
 	temp &= ~FDI_LINK_TRAIN_VOL_EMP_MASK;
 	temp |= FDI_LINK_TRAIN_400MV_0DB_SNB_B;
+	temp |= FDI_COMPOSITE_SYNC;
 	I915_WRITE(reg, temp | FDI_TX_ENABLE);
 
 	reg = FDI_RX_CTL(pipe);
@@ -2600,6 +2601,7 @@ static void ivb_manual_fdi_link_train(struct drm_crtc *crtc)
 	temp &= ~FDI_LINK_TRAIN_AUTO;
 	temp &= ~FDI_LINK_TRAIN_PATTERN_MASK_CPT;
 	temp |= FDI_LINK_TRAIN_PATTERN_1_CPT;
+	temp |= FDI_COMPOSITE_SYNC;
 	I915_WRITE(reg, temp | FDI_RX_ENABLE);
 
 	POSTING_READ(reg);
-- 
1.7.4.1

Re: [PATCH] drm/i915: always set FDI composite sync bit

[Adam Jackson]

[-- Attachment #1.1: Type: text/plain, Size: 268 bytes --]

On Mon, 2011-10-10 at 14:28 -0700, Jesse Barnes wrote:
> It's needed for 3 pipe support as well as just regular functionality
> (e.g. DisplayPort).
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Tested-by: Adam Jackson <ajax@redhat.com>

- ajax

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: always set FDI composite sync bit

[Eugeni Dodonov]

[-- Attachment #1.1: Type: text/plain, Size: 335 bytes --]

On Mon, Oct 10, 2011 at 18:28, Jesse Barnes <jbarnes@virtuousgeek.org>wrote:

> It's needed for 3 pipe support as well as just regular functionality
> (e.g. DisplayPort).
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>

Tested-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

-- 
Eugeni Dodonov
<http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 746 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: always set FDI composite sync bit

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 298 bytes --]

On Mon, 10 Oct 2011 14:28:52 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> It's needed for 3 pipe support as well as just regular functionality
> (e.g. DisplayPort).

Any explanation on how you get sync without this? As in, why did this
ever work?

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 827 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: always set FDI composite sync bit

[Adam Jackson]

On 10/10/11 7:22 PM, Keith Packard wrote:
> On Mon, 10 Oct 2011 14:28:52 -0700, Jesse Barnes<jbarnes@virtuousgeek.org>  wrote:
>
>> It's needed for 3 pipe support as well as just regular functionality
>> (e.g. DisplayPort).
>
> Any explanation on how you get sync without this? As in, why did this
> ever work?

To a first approximation, it didn't.  I was never able to get anything 
reliably lit up besides the boot output (and only that one; hotplug 
after was a disaster).

I don't have register dumps handy to compare BIOS setup with this 
change, but I could if needed.

It still seems entirely magical and probably wrong in some situations. 
And I'm thrilled to see that PPT is functionally different from CPT 
(seriously, stop doing that) instead of just moving bit definitions 
around (seriously, stop doing that).  But this change is massively 
better than before.

- ajax

Re: [PATCH] drm/i915: always set FDI composite sync bit

[Jesse Barnes]

[-- Attachment #1.1: Type: text/plain, Size: 1575 bytes --]

On Tue, 11 Oct 2011 10:10:24 -0400
Adam Jackson <ajax@redhat.com> wrote:

> On 10/10/11 7:22 PM, Keith Packard wrote:
> > On Mon, 10 Oct 2011 14:28:52 -0700, Jesse Barnes<jbarnes@virtuousgeek.org>  wrote:
> >
> >> It's needed for 3 pipe support as well as just regular functionality
> >> (e.g. DisplayPort).
> >
> > Any explanation on how you get sync without this? As in, why did this
> > ever work?
> 
> To a first approximation, it didn't.  I was never able to get anything 
> reliably lit up besides the boot output (and only that one; hotplug 
> after was a disaster).
> 
> I don't have register dumps handy to compare BIOS setup with this 
> change, but I could if needed.
> 
> It still seems entirely magical and probably wrong in some situations. 
> And I'm thrilled to see that PPT is functionally different from CPT 
> (seriously, stop doing that) instead of just moving bit definitions 
> around (seriously, stop doing that).  But this change is massively 
> better than before.

Without composite sync, FDI needs frame/line sync (the fsync/lsync
mentioned in the docs), which means extra wires between the CPU and
PCH.  Some boards don't have these and so we need to use composite.
For the same reason, if FDI B and C are sharing lanes, we need to use
composite or we won't have enough lines to go around.

I've asked the hardware guys whether using composite on a board that
supports fsync/lsync is ok generally; if so just defaulting to that
everywhere should be fine.

-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: always set FDI composite sync bit

[Adam Jackson]

On 10/11/11 12:16 PM, Jesse Barnes wrote:
> On Tue, 11 Oct 2011 10:10:24 -0400 Adam Jackson<ajax@redhat.com>  wrote:
>> It still seems entirely magical and probably wrong in some situations.
>> And I'm thrilled to see that PPT is functionally different from CPT
>> (seriously, stop doing that) instead of just moving bit definitions
>> around (seriously, stop doing that).  But this change is massively
>> better than before.
>
> Without composite sync, FDI needs frame/line sync (the fsync/lsync
> mentioned in the docs), which means extra wires between the CPU and
> PCH.  Some boards don't have these and so we need to use composite.
> For the same reason, if FDI B and C are sharing lanes, we need to use
> composite or we won't have enough lines to go around.
>
> I've asked the hardware guys whether using composite on a board that
> supports fsync/lsync is ok generally; if so just defaulting to that
> everywhere should be fine.

Makes sense.  If true this seems worth doing for CPT too.  The IBX docs 
don't make any mention of composite sync.

- ajax

Re: [PATCH] drm/i915: always set FDI composite sync bit

[Jesse Barnes]

[-- Attachment #1.1: Type: text/plain, Size: 1418 bytes --]

On Tue, 11 Oct 2011 12:42:22 -0400
Adam Jackson <ajax@redhat.com> wrote:

> On 10/11/11 12:16 PM, Jesse Barnes wrote:
> > On Tue, 11 Oct 2011 10:10:24 -0400 Adam Jackson<ajax@redhat.com>  wrote:
> >> It still seems entirely magical and probably wrong in some situations.
> >> And I'm thrilled to see that PPT is functionally different from CPT
> >> (seriously, stop doing that) instead of just moving bit definitions
> >> around (seriously, stop doing that).  But this change is massively
> >> better than before.
> >
> > Without composite sync, FDI needs frame/line sync (the fsync/lsync
> > mentioned in the docs), which means extra wires between the CPU and
> > PCH.  Some boards don't have these and so we need to use composite.
> > For the same reason, if FDI B and C are sharing lanes, we need to use
> > composite or we won't have enough lines to go around.
> >
> > I've asked the hardware guys whether using composite on a board that
> > supports fsync/lsync is ok generally; if so just defaulting to that
> > everywhere should be fine.
> 
> Makes sense.  If true this seems worth doing for CPT too.  The IBX docs 
> don't make any mention of composite sync.

Supposedly SNB doesn't support this on the CPU side, so I've only put
it in the IVB FDI training code.  ILK + IBX don't support it at all
according to the hw folks.

-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: always set FDI composite sync bit

[Jesse Barnes]

[-- Attachment #1.1: Type: text/plain, Size: 1281 bytes --]

On Tue, 11 Oct 2011 10:10:24 -0400
Adam Jackson <ajax@redhat.com> wrote:

> On 10/10/11 7:22 PM, Keith Packard wrote:
> > On Mon, 10 Oct 2011 14:28:52 -0700, Jesse Barnes<jbarnes@virtuousgeek.org>  wrote:
> >
> >> It's needed for 3 pipe support as well as just regular functionality
> >> (e.g. DisplayPort).
> >
> > Any explanation on how you get sync without this? As in, why did this
> > ever work?
> 
> To a first approximation, it didn't.  I was never able to get anything 
> reliably lit up besides the boot output (and only that one; hotplug 
> after was a disaster).
> 
> I don't have register dumps handy to compare BIOS setup with this 
> change, but I could if needed.
> 
> It still seems entirely magical and probably wrong in some situations. 
> And I'm thrilled to see that PPT is functionally different from CPT 
> (seriously, stop doing that) instead of just moving bit definitions 
> around (seriously, stop doing that).  But this change is massively 
> better than before.

Ok just got confirmation that always setting the composite bit is safe
on IVB per the specs we provide to board designers (i.e. they're
required to make composite sync work, and fsync/lsync is optional).

-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: always set FDI composite sync bit

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 375 bytes --]

On Tue, 11 Oct 2011 09:40:18 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> Ok just got confirmation that always setting the composite bit is safe
> on IVB per the specs we provide to board designers (i.e. they're
> required to make composite sync work, and fsync/lsync is optional).

Thanks. That sounds conclusive enough :)

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 827 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx