Re: [PATCH v2 3/3] drm/i915/icl: Work around broken VBTs for port F detection

Imre Deak
2019-01-17T16:09:27Z

Hi,

On Wed, Jan 16, 2019 at 05:33:19PM -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2018-12-20 at 17:52 +0200, Imre Deak wrote:
> > VBT may include incorrect information about the presence of port F.
> > Work
> > around this on SKUs where we know the port is not present.
>
> If we cannot trust the data in VBT, can we not test for the absence of
> port-F by enabling the powerwell and checking for a timeout? Or at
> least mark up a non-existent port after the first timeout so that we
> don't keep probing it.

Yes, thought about it too, but I'm not sure if it's a good idea.
Enabling power wells has unexpected (at least to me) side effects on
ICL, so I'd rather avoid using that as a detection method. OTOH the PCI
ID list with fused-off ports must be well-defined and known, there just
seems to be a difficulty to get at that list internally.

So instead I'd suggest pushing for getting that list added to BSpec. I
opened already a change request in BSpec for that, but more poking would
be good.

>
> This patch is an improvement over checking the VBT for all ports, so
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

Thanks for the review.

>
> >
> > v2:
> > - Fix IS_ICL_WITH_PORT_F, so it's useable from any context.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108915
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 2 ++
> > drivers/gpu/drm/i915/intel_display.c | 4 +++-
> > 2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 815db160b966..e45cda9d9555 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2317,6 +2317,8 @@ intel_info(const struct drm_i915_private
> > *dev_priv)
> > (dev_priv)->info.gt == 3)
> > #define IS_CNL_WITH_PORT_F(dev_priv) (IS_CANNONLAKE(dev_priv) && \
> > (INTEL_DEVID(dev_priv) &
> > 0x0004) == 0x0004)
> > +#define IS_ICL_WITH_PORT_F(dev_priv) (IS_ICELAKE(dev_priv) && \
> > + INTEL_DEVID(dev_priv) !=
> > 0x8A51)
> >
> > #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)-
> > >is_alpha_support)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 2b81da068010..bd7fdaf7e093 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14276,8 +14276,10 @@ static void intel_setup_outputs(struct
> > drm_i915_private *dev_priv)
> > /*
> > * On some ICL SKUs port F is not present. No strap
> > bits for
> > * this, so rely on VBT.
> > + * Work around broken VBTs on SKUs known to have no
> > port F.
> > */
> > - if (intel_bios_is_port_present(dev_priv, PORT_F))
> > + if (IS_ICL_WITH_PORT_F(dev_priv) &&
> > + intel_bios_is_port_present(dev_priv, PORT_F))
> > intel_ddi_init(dev_priv, PORT_F);
> >
> > icl_dsi_init(dev_priv);
>