On Tue, Dec 01, 2015 at 05:28:19PM -0200, Paulo Zanoni wrote:
> 2015-12-01 14:07 GMT-02:00 <ville.syrjala@xxxxxxxxxxxxxxx>:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > Unfortunatey there appear to quite a few HSW/BDW machines (eg.
> > NUCs, Brix Pro) in the wild with LPT/WPT-H but non-working FDI.
> > FDI training fails every single time on these machines. Dunno,
> > maybe they just didn't bother wiring it up or something?
>
> I know you clarified this on IRC, but just a little nitpick: when
> reading this, it was not very clear to me whether these machines had
> an actual physical CRT connector or not. If you read this while
> assuming the machine does have a CRT connector, the patch would sound
> like "we fail to properly train FDI, so let's give up and not register
> the CRT connector at all", which is not our case.
I'll try revise a bit.
>
> >
> > Unfortunately all the fuse bits and whatnot are telling us that
> > the CRT connector is present. And so what we get from this is tons
> > of false positives from the CI systems due to VGA connector forcing.
> >
> > I've not found any way to detect this purely from hardware, so we
> > have to resort to looking at the VBT int_crt_support bit. We used
> > to check this bit on all platforms, but that broke all the old
> > machines, so the check was then restricted to VLV only in
> > commit 84b4e042c470 ("drm/i915: only apply crt_present check on VLV")
> >
> > Considering HSW and VLV VBT probably got defined around the same time,
> > it should be reasonably safe to assume that the bits is sane for
> > HSW/BDW as well. At least I have one copy of some VBT spec here that
> > says it's meant for both VLV and HSW, and it knows about the bit
> > (lists it being valid from version 155 onwards). Also I have two
> > desktop machines with actual CRT ports and both have
> > int_crt_support==1 in their VBTs.
> >
> > v2: Move the platform checks into the VBT parsing code
> > Also check that the VBT version is at least 155
>
> You could also add that HAS_DDI platforms are already believing the
> VBT for versions >= 155: see intel_ddi_init(), so we have a precedent.
Can do.
>
> (by the way, I'm trusting you that VLV VBT versions are always >= 155,
> I couldn't check this)
It is at least on the VLV I have on my desk. But even if it wasn't, I
don't think things should break. We live with shadow VGA connectors on
many platforms, and I don't think VLV would be too upset by one. IIRC
it was mainly a speed optimization to get rid of it.
>
> With or without any changes:
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>
> >
> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_bios.c | 5 ++++-
> > drivers/gpu/drm/i915/intel_display.c | 2 +-
> > 2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> > index ce82f9c7df24..070470fe9a91 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -356,7 +356,10 @@ parse_general_features(struct drm_i915_private *dev_priv,
> > general = find_section(bdb, BDB_GENERAL_FEATURES);
> > if (general) {
> > dev_priv->vbt.int_tv_support = general->int_tv_support;
> > - dev_priv->vbt.int_crt_support = general->int_crt_support;
> > + /* int_crt_support can't be trusted on earlier platforms */
> > + if (bdb->version >= 155 &&
> > + (HAS_DDI(dev_priv) || IS_VALLEYVIEW(dev_priv)))
> > + dev_priv->vbt.int_crt_support = general->int_crt_support;
> > dev_priv->vbt.lvds_use_ssc = general->enable_ssc;
> > dev_priv->vbt.lvds_ssc_freq =
> > intel_bios_ssc_frequency(dev, general->ssc_freq);
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index e80387dd6582..68822395914b 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14263,7 +14263,7 @@ static bool intel_crt_present(struct drm_device *dev)
> > if (HAS_DDI(dev) && I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES)
> > return false;
> >
> > - if (IS_VALLEYVIEW(dev) && !dev_priv->vbt.int_crt_support)
> > + if (!dev_priv->vbt.int_crt_support)
> > return false;
> >
> > return true;
> > --
> > 2.4.10
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
