[Intel-gfx] [PATCH v2 03/10] drm/i915: Check VBT for CRT port presence on HSW/BDW

[Paulo Zanoni]

2015-12-01 14:07 GMT-02:00  <ville.syrjala at linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> 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.

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

(by the way, I'm trusting you that VLV VBT versions are always >= 155,
I couldn't check this)

With or without any changes:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni