[Intel-gfx] [PATCH v2 2/3] drm/i915/bxt: Wait for PHY1 GRC done if PHY0 was already enabled

[Imre Deak]

On to, 2016-04-21 at 19:43 +0300, Ville Syrjälä wrote:
> On Thu, Apr 21, 2016 at 07:19:21PM +0300, Imre Deak wrote:
> > If we skipped PHY0 initialization because it was already enabled by
> > BIOS, we still have to wait for the PHY1 GRC calibration as that is
> > done as part of the PHY0 init.
> > 
> > v2:
> > - Use the actual PHY index in the debug message in
> >   broxton_phy_wait_grc_done() (Ville)
> > 
> > CC: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 21 ++++++++++++++++++---
> >  1 file changed, 18 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 943aa93..c836f21 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1760,6 +1760,13 @@ static u32 broxton_get_grc(struct
> > drm_i915_private *dev_priv, enum dpio_phy phy)
> >  	return (val & GRC_CODE_MASK) >> GRC_CODE_SHIFT;
> >  }
> >  
> > +static void broxton_phy_wait_grc_done(struct drm_i915_private
> > *dev_priv,
> > +				      enum dpio_phy phy)
> > +{
> > +	if (wait_for(I915_READ(BXT_PORT_REF_DW3(phy)) & GRC_DONE,
> > 10))
> > +		DRM_ERROR("timeout waiting for PHY%d GRC\n", phy);
> > +}
> > +
> >  static void broxton_phy_init(struct drm_i915_private *dev_priv,
> >  			     enum dpio_phy phy)
> >  {
> > @@ -1863,9 +1870,7 @@ static void broxton_phy_init(struct
> > drm_i915_private *dev_priv,
> >  		 * the corresponding calibrated value from PHY1,
> > and disable
> >  		 * the automatic calibration on PHY0.
> >  		 */
> > -		if
> > (wait_for(I915_READ(BXT_PORT_REF_DW3(DPIO_PHY1)) & GRC_DONE,
> > -			     10))
> > -			DRM_ERROR("timeout waiting for PHY1
> > GRC\n");
> > +		broxton_phy_wait_grc_done(dev_priv, DPIO_PHY1);
> >  
> >  		val = dev_priv->bxt_phy_grc =
> > broxton_get_grc(dev_priv,
> >  							      DPIO
> > _PHY1);
> > @@ -1878,6 +1883,10 @@ static void broxton_phy_init(struct
> > drm_i915_private *dev_priv,
> >  		val |= GRC_DIS | GRC_RDY_OVRD;
> >  		I915_WRITE(BXT_PORT_REF_DW8(DPIO_PHY0), val);
> >  	}
> > +	/*
> > +	 * During PHY1 init delay waiting for GRC calibration to
> > finish, since
> > +	 * it can happen in parallel with the subsequent PHY0
> > init.
> > +	 */
> >  
> >  	val = I915_READ(BXT_PHY_CTL_FAMILY(phy));
> >  	val |= COMMON_RESET_DIS;
> > @@ -1889,6 +1898,12 @@ void broxton_ddi_phy_init(struct
> > drm_i915_private *dev_priv)
> >  	/* Enable PHY1 first since it provides Rcomp for PHY0 */
> >  	broxton_phy_init(dev_priv, DPIO_PHY1);
> >  	broxton_phy_init(dev_priv, DPIO_PHY0);
> > +
> > +	/*
> > +	 * If BIOS enabled only PHY0 and not PHY1, we skipped
> > waiting for the
> > +	 * PHY1 GRC calibration to finish, so wait for it here.
> > +	 */
> > +	broxton_phy_wait_grc_done(dev_priv, DPIO_PHY1);
> 
> Hmm. Should we just do that always in the PHY1 init? A tad slower
> perhaps but maybe less confusing?

Yea, I was thinking about it too. I will measure how much this actually
takes and will simplify things.

> Anyway series looks fine to me
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> >  }
> >  
> >  static void broxton_phy_uninit(struct drm_i915_private *dev_priv,
> > -- 
> > 2.5.0
>