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

2019-01-17 Thread Imre Deak
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 

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 
> > Cc: Jani Nikula 
> > Signed-off-by: Imre Deak 
> > ---
> >  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);
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2019-01-16 Thread Dhinakaran Pandiyan
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 

> 
> 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 
> Cc: Jani Nikula 
> Signed-off-by: Imre Deak 
> ---
>  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);

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx