[PATCH 0/3] drm/i915/bxt: Fix PHY init with partial BIOS config

[PATCH 0/3] drm/i915/bxt: Fix PHY init with partial BIOS config

[Imre Deak]

It's possible that BIOS initializes only a single channel on PHY0 and then
enables it or only enables PHY0 but leaves PHY1 disabled. This patchset
fixes handling of these cases.

CC: Matt Roper <matthew.d.roper@intel.com>

Imre Deak (3):
  drm/i915/bxt: Use PHY0 GRC value for HW state verification
  drm/i915/bxt: Wait for PHY1 GRC done if PHY0 was already enabled
  drm/i915/bxt: Force reprogramming a PHY with invalid HW state

 drivers/gpu/drm/i915/intel_ddi.c | 43 +++++++++++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 9 deletions(-)

-- 
2.5.0

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

[PATCH 1/3] drm/i915/bxt: Use PHY0 GRC value for HW state verification

[Imre Deak]

It's possible that BIOS enables PHY1 only to read out the GRC value from
it to be used in PHY0 and then disables PHY1. In this case we can't use
the PHY1 GRC value for state verification, so use instead the one in PHY0
always.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c2348fb..943aa93 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1770,7 +1770,7 @@ static void broxton_phy_init(struct drm_i915_private *dev_priv,
 		DRM_DEBUG_DRIVER("DDI PHY %d already enabled, "
 				 "won't reprogram it\n", phy);
 		/* Still read out the GRC value for state verification */
-		if (phy == DPIO_PHY1)
+		if (phy == DPIO_PHY0)
 			dev_priv->bxt_phy_grc = broxton_get_grc(dev_priv, phy);
 
 		return;
-- 
2.5.0

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

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

[Imre Deak]

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.

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..e68914a 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 PHY1 GRC\n");
+}
+
 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,
-- 
2.5.0

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

[PATCH 3/3] drm/i915/bxt: Force reprogramming a PHY with invalid HW state

[Imre Deak]

It's possible that BIOS enables PHY0, but it programmes only the first
channel on it. Since we program the PHYs only during driver loading this
is an incorrect configuration from the driver's point of view, since we
may use both channels eventually. Detect this scenario and force
reprogramming the PHY in this case.

The actual scenario for me was that the lane optimization for the second
channel in PHY0 was not setup by BIOS and so a state verification
warning was triggered. Everything else was setup properly.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e68914a..32ac456 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1767,6 +1767,9 @@ static void broxton_phy_wait_grc_done(struct drm_i915_private *dev_priv,
 		DRM_ERROR("timeout waiting for PHY1 GRC\n");
 }
 
+static bool broxton_phy_verify_state(struct drm_i915_private *dev_priv,
+				     enum dpio_phy phy);
+
 static void broxton_phy_init(struct drm_i915_private *dev_priv,
 			     enum dpio_phy phy)
 {
@@ -1774,17 +1777,24 @@ static void broxton_phy_init(struct drm_i915_private *dev_priv,
 	u32 ports, val;
 
 	if (broxton_phy_is_enabled(dev_priv, phy)) {
-		DRM_DEBUG_DRIVER("DDI PHY %d already enabled, "
-				 "won't reprogram it\n", phy);
 		/* Still read out the GRC value for state verification */
 		if (phy == DPIO_PHY0)
 			dev_priv->bxt_phy_grc = broxton_get_grc(dev_priv, phy);
 
-		return;
+
+		if (broxton_phy_verify_state(dev_priv, phy)) {
+			DRM_DEBUG_DRIVER("DDI PHY %d already enabled, "
+					 "won't reprogram it\n", phy);
+
+			return;
+		}
+
+		DRM_DEBUG_DRIVER("DDI PHY %d enabled with invalid state, "
+				 "force reprogramming it\n", phy);
+	} else {
+		DRM_DEBUG_DRIVER("DDI PHY %d not enabled, enabling it\n", phy);
 	}
 
-	DRM_DEBUG_DRIVER("DDI PHY %d not enabled, enabling it\n", phy);
-
 	val = I915_READ(BXT_P_CR_GT_DISP_PWRON);
 	val |= GT_DISPLAY_POWER_ON(phy);
 	I915_WRITE(BXT_P_CR_GT_DISP_PWRON, val);
-- 
2.5.0

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

✓ Fi.CI.BAT: success for drm/i915/bxt: Fix PHY init with partial BIOS config

[Patchwork]

== Series Details ==

Series: drm/i915/bxt: Fix PHY init with partial BIOS config
URL   : https://patchwork.freedesktop.org/series/6010/
State : success

== Summary ==

Series 6010v1 drm/i915/bxt: Fix PHY init with partial BIOS config
http://patchwork.freedesktop.org/api/1.0/series/6010/revisions/1/mbox/


bdw-nuci7        total:194  pass:182  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:194  pass:170  dwarn:0   dfail:0   fail:1   skip:23 
bsw-nuc-2        total:193  pass:153  dwarn:0   dfail:0   fail:0   skip:40 
byt-nuc          total:193  pass:155  dwarn:0   dfail:0   fail:0   skip:38 
hsw-brixbox      total:194  pass:170  dwarn:0   dfail:0   fail:0   skip:24 
hsw-gt2          total:194  pass:175  dwarn:0   dfail:0   fail:0   skip:19 
ilk-hp8440p      total:194  pass:135  dwarn:0   dfail:1   fail:0   skip:58 
ivb-t430s        total:194  pass:166  dwarn:0   dfail:0   fail:0   skip:28 
skl-i7k-2        total:194  pass:168  dwarn:0   dfail:0   fail:1   skip:25 
skl-nuci5        total:194  pass:183  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:130  pass:104  dwarn:0   dfail:0   fail:0   skip:25 

Results at /archive/results/CI_IGT_test/Patchwork_1967/

9dabb0053b63bc32ab6ad5d13209d1e43395313f drm-intel-nightly: 2016y-04m-21d-09h-27m-12s UTC integration manifest
e11c1d6 drm/i915/bxt: Force reprogramming a PHY with invalid HW state
4738f4e drm/i915/bxt: Wait for PHY1 GRC done if PHY0 was already enabled
3f40f0f drm/i915/bxt: Use PHY0 GRC value for HW state verification

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

✗ Fi.CI.BAT: failure for drm/i915/bxt: Fix PHY init with partial BIOS config

[Patchwork]

== Series Details ==

Series: drm/i915/bxt: Fix PHY init with partial BIOS config
URL   : https://patchwork.freedesktop.org/series/6010/
State : failure

== Summary ==

Series 6010v1 drm/i915/bxt: Fix PHY init with partial BIOS config
http://patchwork.freedesktop.org/api/1.0/series/6010/revisions/1/mbox/

Test gem_sync:
        Subgroup basic-each:
                pass       -> DMESG-FAIL (ilk-hp8440p)
Test kms_force_connector_basic:
        Subgroup force-edid:
                pass       -> SKIP       (ilk-hp8440p)

bdw-nuci7        total:194  pass:182  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:194  pass:170  dwarn:0   dfail:0   fail:1   skip:23 
bsw-nuc-2        total:193  pass:153  dwarn:0   dfail:0   fail:0   skip:40 
byt-nuc          total:193  pass:155  dwarn:0   dfail:0   fail:0   skip:38 
hsw-brixbox      total:194  pass:170  dwarn:0   dfail:0   fail:0   skip:24 
hsw-gt2          total:194  pass:175  dwarn:0   dfail:0   fail:0   skip:19 
ilk-hp8440p      total:194  pass:135  dwarn:0   dfail:1   fail:0   skip:58 
ivb-t430s        total:194  pass:166  dwarn:0   dfail:0   fail:0   skip:28 
skl-i7k-2        total:194  pass:168  dwarn:0   dfail:0   fail:1   skip:25 
skl-nuci5        total:194  pass:183  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:130  pass:104  dwarn:0   dfail:0   fail:0   skip:25 

Results at /archive/results/CI_IGT_test/Patchwork_1967/

9dabb0053b63bc32ab6ad5d13209d1e43395313f drm-intel-nightly: 2016y-04m-21d-09h-27m-12s UTC integration manifest
e11c1d6 drm/i915/bxt: Force reprogramming a PHY with invalid HW state
4738f4e drm/i915/bxt: Wait for PHY1 GRC done if PHY0 was already enabled
3f40f0f drm/i915/bxt: Use PHY0 GRC value for HW state verification

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

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

[Ville Syrjälä]

On Wed, Apr 20, 2016 at 08:46:05PM +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.
> 
> 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..e68914a 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 PHY1 GRC\n");

You pass the phy as a parameter, so supposedly the debug message should
reflect that parameter.

> +}
> +
>  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,
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/bxt: Force reprogramming a PHY with invalid HW state

[Ville Syrjälä]

On Wed, Apr 20, 2016 at 08:46:06PM +0300, Imre Deak wrote:
> It's possible that BIOS enables PHY0, but it programmes only the first
> channel on it. Since we program the PHYs only during driver loading this
> is an incorrect configuration from the driver's point of view, since we
> may use both channels eventually. Detect this scenario and force
> reprogramming the PHY in this case.
> 
> The actual scenario for me was that the lane optimization for the second
> channel in PHY0 was not setup by BIOS and so a state verification
> warning was triggered. Everything else was setup properly.

One thing that bothers me with this reprogramming scheme is that we
don't fully power down the PHY before reprogramming. If CHV is any
indication that could leave the PHY in some wonky intermediate state
where it doesn't fully power down when unused. That problem was caught
on CHV by assert_chv_phy_status() and/or assert_chv_phy_powergate().

> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e68914a..32ac456 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1767,6 +1767,9 @@ static void broxton_phy_wait_grc_done(struct drm_i915_private *dev_priv,
>  		DRM_ERROR("timeout waiting for PHY1 GRC\n");
>  }
>  
> +static bool broxton_phy_verify_state(struct drm_i915_private *dev_priv,
> +				     enum dpio_phy phy);
> +
>  static void broxton_phy_init(struct drm_i915_private *dev_priv,
>  			     enum dpio_phy phy)
>  {
> @@ -1774,17 +1777,24 @@ static void broxton_phy_init(struct drm_i915_private *dev_priv,
>  	u32 ports, val;
>  
>  	if (broxton_phy_is_enabled(dev_priv, phy)) {
> -		DRM_DEBUG_DRIVER("DDI PHY %d already enabled, "
> -				 "won't reprogram it\n", phy);
>  		/* Still read out the GRC value for state verification */
>  		if (phy == DPIO_PHY0)
>  			dev_priv->bxt_phy_grc = broxton_get_grc(dev_priv, phy);
>  
> -		return;
> +
> +		if (broxton_phy_verify_state(dev_priv, phy)) {
> +			DRM_DEBUG_DRIVER("DDI PHY %d already enabled, "
> +					 "won't reprogram it\n", phy);
> +
> +			return;
> +		}
> +
> +		DRM_DEBUG_DRIVER("DDI PHY %d enabled with invalid state, "
> +				 "force reprogramming it\n", phy);
> +	} else {
> +		DRM_DEBUG_DRIVER("DDI PHY %d not enabled, enabling it\n", phy);
>  	}
>  
> -	DRM_DEBUG_DRIVER("DDI PHY %d not enabled, enabling it\n", phy);
> -
>  	val = I915_READ(BXT_P_CR_GT_DISP_PWRON);
>  	val |= GT_DISPLAY_POWER_ON(phy);
>  	I915_WRITE(BXT_P_CR_GT_DISP_PWRON, val);
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Imre Deak]

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

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

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

[Ville Syrjälä]

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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [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@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
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915/bxt: Fix PHY init with partial BIOS config (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/bxt: Fix PHY init with partial BIOS config (rev2)
URL   : https://patchwork.freedesktop.org/series/6010/
State : failure

== Summary ==

Series 6010v2 drm/i915/bxt: Fix PHY init with partial BIOS config
http://patchwork.freedesktop.org/api/1.0/series/6010/revisions/2/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (ilk-hp8440p) UNSTABLE
Test kms_force_connector_basic:
        Subgroup prune-stale-modes:
                pass       -> SKIP       (ilk-hp8440p)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-c:
                pass       -> FAIL       (bdw-nuci7)

bdw-nuci7        total:193  pass:180  dwarn:0   dfail:0   fail:1   skip:12 
bdw-ultra        total:193  pass:170  dwarn:0   dfail:0   fail:0   skip:23 
bsw-nuc-2        total:192  pass:152  dwarn:0   dfail:0   fail:0   skip:40 
byt-nuc          total:192  pass:154  dwarn:0   dfail:0   fail:0   skip:38 
hsw-brixbox      total:193  pass:169  dwarn:0   dfail:0   fail:0   skip:24 
hsw-gt2          total:193  pass:174  dwarn:0   dfail:0   fail:0   skip:19 
ilk-hp8440p      total:193  pass:135  dwarn:0   dfail:0   fail:0   skip:58 
ivb-t430s        total:193  pass:165  dwarn:0   dfail:0   fail:0   skip:28 
skl-i7k-2        total:193  pass:168  dwarn:0   dfail:0   fail:0   skip:25 
skl-nuci5        total:193  pass:182  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:193  pass:155  dwarn:0   dfail:0   fail:0   skip:38 
snb-x220t        total:193  pass:155  dwarn:0   dfail:0   fail:1   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_1983/

d5b5101bd09a7b7e48b1cd78fe8f8a40b21d4deb drm-intel-nightly: 2016y-04m-21d-16h-37m-06s UTC integration manifest
76092d6 drm/i915/bxt: Wait for PHY1 GRC done if PHY0 was already enabled
0ed1b74 drm/i915/bxt: Use PHY0 GRC value for HW state verification

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

Re: ✗ Fi.CI.BAT: failure for drm/i915/bxt: Fix PHY init with partial BIOS config

[Imre Deak]

On to, 2016-04-21 at 12:05 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915/bxt: Fix PHY init with partial BIOS config
> URL   : https://patchwork.freedesktop.org/series/6010/
> State : failure
> 
> == Summary ==
> 
> Series 6010v1 drm/i915/bxt: Fix PHY init with partial BIOS config
> http://patchwork.freedesktop.org/api/1.0/series/6010/revisions/1/mbox
> /
> 
> Test gem_sync:
>         Subgroup basic-each:
>                 pass       -> DMESG-FAIL (ilk-hp8440p)

BSD ring hang:
https://bugs.freedesktop.org/show_bug.cgi?id=94307

> Test kms_force_connector_basic:
>         Subgroup force-edid:
>                 pass       -> SKIP       (ilk-hp8440p)
> 
> bdw-
> nuci7        total:194  pass:182  dwarn:0   dfail:0   fail:0   skip:1
> 2 
> bdw-
> ultra        total:194  pass:170  dwarn:0   dfail:0   fail:1   skip:2
> 3 
> bsw-nuc-
> 2        total:193  pass:153  dwarn:0   dfail:0   fail:0   skip:40 
> byt-
> nuc          total:193  pass:155  dwarn:0   dfail:0   fail:0   skip:3
> 8 
> hsw-
> brixbox      total:194  pass:170  dwarn:0   dfail:0   fail:0   skip:2
> 4 
> hsw-
> gt2          total:194  pass:175  dwarn:0   dfail:0   fail:0   skip:1
> 9 
> ilk-
> hp8440p      total:194  pass:135  dwarn:0   dfail:1   fail:0   skip:5
> 8 
> ivb-
> t430s        total:194  pass:166  dwarn:0   dfail:0   fail:0   skip:2
> 8 
> skl-i7k-
> 2        total:194  pass:168  dwarn:0   dfail:0   fail:1   skip:25 
> skl-
> nuci5        total:194  pass:183  dwarn:0   dfail:0   fail:0   skip:1
> 1 
> snb-
> dellxps      total:130  pass:104  dwarn:0   dfail:0   fail:0   skip:2
> 5 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1967/
> 
> 9dabb0053b63bc32ab6ad5d13209d1e43395313f drm-intel-nightly: 2016y-
> 04m-21d-09h-27m-12s UTC integration manifest
> e11c1d6 drm/i915/bxt: Force reprogramming a PHY with invalid HW state
> 4738f4e drm/i915/bxt: Wait for PHY1 GRC done if PHY0 was already
> enabled
> 3f40f0f drm/i915/bxt: Use PHY0 GRC value for HW state verification
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: failure for drm/i915/bxt: Fix PHY init with partial BIOS config (rev2)

[Imre Deak]

On pe, 2016-04-22 at 07:27 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915/bxt: Fix PHY init with partial BIOS config (rev2)
> URL   : https://patchwork.freedesktop.org/series/6010/
> State : failure
>

Thanks for the review I pushed the patchset to -dinq. Explanation for
the failure below.

> == Summary ==
> 
> Series 6010v2 drm/i915/bxt: Fix PHY init with partial BIOS config
> http://patchwork.freedesktop.org/api/1.0/series/6010/revisions/2/mbox
> /
> 
> Test kms_flip:
>         Subgroup basic-flip-vs-wf_vblank:
>                 fail       -> PASS       (ilk-hp8440p) UNSTABLE
> Test kms_force_connector_basic:
>         Subgroup prune-stale-modes:
>                 pass       -> SKIP       (ilk-hp8440p)

Unrelated platform and I can't find the log for such SKIP results.

> Test kms_pipe_crc_basic:
>         Subgroup read-crc-pipe-c:
>                 pass       -> FAIL       (bdw-nuci7)

Unrelated platform, I contribute it to the other pipe-crc errors where
the HDMI connector vanishes in the middle of the test run, although
here the failure mode is a bit different with timing out during the CRC
read:
 
IGT-Version: 1.14-gde4f3ab (x86_64) (Linux: 4.6.0-rc4-CI-Patchwork_1983+ x86_64)
read-crc-pipe-C: Testing connector HDMI-A-2 using pipe C
Timed out: CRC reading
Subtest read-crc-pipe-C: FAIL (5.091s)

Subtest read-crc-pipe-C failed.
**** DEBUG ****
(kms_pipe_crc_basic:6663) DEBUG: Test requirement passed: !(pipe >= data->display.n_pipes)
(kms_pipe_crc_basic:6663) INFO: read-crc-pipe-C: Testing connector HDMI-A-2 using pipe C
(kms_pipe_crc_basic:6663) igt-kms-DEBUG: display: HDMI-A-2: set_pipe(C)
(kms_pipe_crc_basic:6663) igt-kms-DEBUG: display: commit {
(kms_pipe_crc_basic:6663) igt-kms-DEBUG: display:     HDMI-A-2: Selecting pipe C
(kms_pipe_crc_basic:6663) igt-kms-DEBUG: display: }
(kms_pipe_crc_basic:6663) DEBUG: Clearing the fb with color (0.00,1.00,0.00)
(kms_pipe_crc_basic:6663) igt-fb-DEBUG: igt_create_fb_with_bo_size(width=1920, height=1080, format=0x34325258, tiling=0x0, size=0)
(kms_pipe_crc_basic:6663) igt-fb-DEBUG: igt_create_fb_with_bo_size(handle=1, pitch=7680)
(kms_pipe_crc_basic:6663) igt-kms-DEBUG: display: C.0: plane_set_fb(67)
(kms_pipe_crc_basic:6663) igt-kms-DEBUG: display: commit {
(kms_pipe_crc_basic:6663) igt-kms-DEBUG: display:     HDMI-A-2: Selecting pipe C
(kms_pipe_crc_basic:6663) igt-kms-DEBUG: display:     HDMI-A-2: SetCrtc pipe C, fb 67, panning (0, 0), mode 1920x1080
(kms_pipe_crc_basic:6663) igt-kms-DEBUG: display: }
(kms_pipe_crc_basic:6663) igt-core-INFO: Timed out: CRC reading

> 
> bdw-
> nuci7        total:193  pass:180  dwarn:0   dfail:0   fail:1   skip:1
> 2 
> bdw-
> ultra        total:193  pass:170  dwarn:0   dfail:0   fail:0   skip:2
> 3 
> bsw-nuc-
> 2        total:192  pass:152  dwarn:0   dfail:0   fail:0   skip:40 
> byt-
> nuc          total:192  pass:154  dwarn:0   dfail:0   fail:0   skip:3
> 8 
> hsw-
> brixbox      total:193  pass:169  dwarn:0   dfail:0   fail:0   skip:2
> 4 
> hsw-
> gt2          total:193  pass:174  dwarn:0   dfail:0   fail:0   skip:1
> 9 
> ilk-
> hp8440p      total:193  pass:135  dwarn:0   dfail:0   fail:0   skip:5
> 8 
> ivb-
> t430s        total:193  pass:165  dwarn:0   dfail:0   fail:0   skip:2
> 8 
> skl-i7k-
> 2        total:193  pass:168  dwarn:0   dfail:0   fail:0   skip:25 
> skl-
> nuci5        total:193  pass:182  dwarn:0   dfail:0   fail:0   skip:1
> 1 
> snb-
> dellxps      total:193  pass:155  dwarn:0   dfail:0   fail:0   skip:3
> 8 
> snb-
> x220t        total:193  pass:155  dwarn:0   dfail:0   fail:1   skip:3
> 7 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1983/
> 
> d5b5101bd09a7b7e48b1cd78fe8f8a40b21d4deb drm-intel-nightly: 2016y-
> 04m-21d-16h-37m-06s UTC integration manifest
> 76092d6 drm/i915/bxt: Wait for PHY1 GRC done if PHY0 was already
> enabled
> 0ed1b74 drm/i915/bxt: Use PHY0 GRC value for HW state verification
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: failure for drm/i915/bxt: Fix PHY init with partial BIOS config (rev2)

[Ville Syrjälä]

On Fri, Apr 22, 2016 at 04:37:29PM +0300, Imre Deak wrote:
> On pe, 2016-04-22 at 07:27 +0000, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: drm/i915/bxt: Fix PHY init with partial BIOS config (rev2)
> > URL   : https://patchwork.freedesktop.org/series/6010/
> > State : failure
> >
> 
> Thanks for the review I pushed the patchset to -dinq. Explanation for
> the failure below.
> 
> > == Summary ==
> > 
> > Series 6010v2 drm/i915/bxt: Fix PHY init with partial BIOS config
> > http://patchwork.freedesktop.org/api/1.0/series/6010/revisions/2/mbox
> > /
> > 
> > Test kms_flip:
> >         Subgroup basic-flip-vs-wf_vblank:
> >                 fail       -> PASS       (ilk-hp8440p) UNSTABLE
> > Test kms_force_connector_basic:
> >         Subgroup prune-stale-modes:
> >                 pass       -> SKIP       (ilk-hp8440p)
> 
> Unrelated platform and I can't find the log for such SKIP results.

It probably skipped before igt_subtest() which meas the only trace in
the log will be 'executing kms_force_connector_basic:' w/o the
corresponding subtest print. I would assume it's another case of the
phantom vga cable making an appearance.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx