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

Submitted by Imre Deak on Dec. 20, 2018, 3:52 p.m.

Details

Message ID 20181220155211.31456-1-imre.deak@intel.com
State New
Headers show
Series "Series without cover letter" ( rev: 2 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Imre Deak Dec. 20, 2018, 3:52 p.m.
VBT may include incorrect information about the presence of port F. Work
around this on SKUs where we know the port is not present.

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(-)

Patch hide | download patch | download mbox

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);

Comments

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.  

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

> 
> 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);
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);
>