diff mbox

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

Message ID 1461255561-1644-1-git-send-email-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Imre Deak April 21, 2016, 4:19 p.m. UTC
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@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

Comments

Ville Syrjälä April 21, 2016, 4:43 p.m. UTC | #1
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@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@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?

Anyway series looks fine to me
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  }
>  
>  static void broxton_phy_uninit(struct drm_i915_private *dev_priv,
> -- 
> 2.5.0
Imre Deak April 21, 2016, 4:51 p.m. UTC | #2
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@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@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@linux.intel.com>
> 
> >  }
> >  
> >  static void broxton_phy_uninit(struct drm_i915_private *dev_priv,
> > -- 
> > 2.5.0
>
diff mbox

Patch

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);
 }
 
 static void broxton_phy_uninit(struct drm_i915_private *dev_priv,