diff mbox series

[3/5] drm/i915/ehl: Don't program PHY_MISC on EHL PHY C

Message ID 20190621020132.1164-4-matthew.d.roper@intel.com (mailing list archive)
State New, archived
Headers show
Series EHL port programming | expand

Commit Message

Matt Roper June 21, 2019, 2:01 a.m. UTC 
Although EHL added a third combo PHY, no PHY_MISC register was added for
PHY C.  The bspec indicates that there's no need to program the "DE to
IO Comp Pwr Down" setting for this PHY that we usually need to set in
PHY_MISC.

Bspec: 33148
Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 .../gpu/drm/i915/display/intel_combo_phy.c    | 53 +++++++++++++------
 1 file changed, 36 insertions(+), 17 deletions(-)

Comments

Souza, Jose June 21, 2019, 8:34 p.m. UTC | #1 
On Thu, 2019-06-20 at 19:01 -0700, Matt Roper wrote:
> Although EHL added a third combo PHY, no PHY_MISC register was added
> for
> PHY C.  The bspec indicates that there's no need to program the "DE
> to
> IO Comp Pwr Down" setting for this PHY that we usually need to set in
> PHY_MISC.
> 
> Bspec: 33148
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  .../gpu/drm/i915/display/intel_combo_phy.c    | 53 +++++++++++++--
> ----
>  1 file changed, 36 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c
> b/drivers/gpu/drm/i915/display/intel_combo_phy.c
> index 075bab2500eb..da590f1a998b 100644
> --- a/drivers/gpu/drm/i915/display/intel_combo_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_combo_phy.c
> @@ -183,9 +183,13 @@ static void cnl_combo_phys_uninit(struct
> drm_i915_private *dev_priv)
>  static bool icl_combo_phy_enabled(struct drm_i915_private *dev_priv,
>  				  enum port port)
>  {
> -	return !(I915_READ(ICL_PHY_MISC(port)) &
> -		 ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN) &&
> -		(I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT);
> +	/* The PHY C added by EHL has no PHY_MISC register */
> +	if (port == PORT_C)
> +		return I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT;

Please add IS_ELKHARTLAKE() to the if, this is a particularity of EHL,
future platforms will reuse this function and they have PHY_MISC.

> +	else
> +		return !(I915_READ(ICL_PHY_MISC(port)) &
> +			 ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN) &&
> +			(I915_READ(ICL_PORT_COMP_DW0(port)) &
> COMP_INIT);
>  }
>  
>  static bool icl_combo_phy_verify_state(struct drm_i915_private
> *dev_priv,
> @@ -300,18 +304,26 @@ static void icl_combo_phys_init(struct
> drm_i915_private *dev_priv)
>  		}
>  
>  		/*
> -		 * EHL's combo PHY A can be hooked up to either an
> external
> -		 * display (via DDI-D) or an internal display (via DDI-
> A or
> -		 * the DSI DPHY).  This is a motherboard design
> decision that
> -		 * can't be changed on the fly, so initialize the PHY's
> mux
> -		 * based on whether our VBT indicates the presence of
> any
> -		 * "internal" child devices.
> +		 * Although EHL adds a combo PHY C, there's no PHY_MISC
> +		 * register for it and no need to program the
> +		 * DE_IO_COMP_PWR_DOWN setting on PHY C.
>  		 */
> -		val = I915_READ(ICL_PHY_MISC(port));
> -		if (IS_ELKHARTLAKE(dev_priv) && port == PORT_A)
> -			val = ehl_combo_phy_a_mux(dev_priv, val);
> -		val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> -		I915_WRITE(ICL_PHY_MISC(port), val);
> +		if (port != PORT_C) {

Something as above here too.

Maybe something with less changes like:

if (IS_ELKHARTLAKE(dev_priv) && port == PORT_C)
	goto skip_phy_misc;

> +			/*
> +			 * EHL's combo PHY A can be hooked up to either
> an
> +			 * external display (via DDI-D) or an internal
> display
> +			 * (via DDI-A or the DSI DPHY).  This is a
> motherboard
> +			 * design decision that can't be changed on the
> fly, so
> +			 * initialize the PHY's mux based on whether
> our VBT
> +			 * indicates the presence of any "internal"
> child
> +			 * devices.
> +			 */
> +			val = I915_READ(ICL_PHY_MISC(port));
> +			if (IS_ELKHARTLAKE(dev_priv) && port == PORT_A)
> +				val = ehl_combo_phy_a_mux(dev_priv,
> val);
> +			val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> +			I915_WRITE(ICL_PHY_MISC(port), val);
> +		}
>  
>  		cnl_set_procmon_ref_values(dev_priv, port);
>  
> @@ -343,9 +355,16 @@ static void icl_combo_phys_uninit(struct
> drm_i915_private *dev_priv)
>  			DRM_WARN("Port %c combo PHY HW state changed
> unexpectedly\n",
>  				 port_name(port));
>  
> -		val = I915_READ(ICL_PHY_MISC(port));
> -		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> -		I915_WRITE(ICL_PHY_MISC(port), val);
> +		/*
> +		 * Although EHL adds a combo PHY C, there's no PHY_MISC
> +		 * register for it and no need to program the
> +		 * DE_IO_COMP_PWR_DOWN setting on PHY C.
> +		 */
> +		if (port != PORT_C) {
> +			val = I915_READ(ICL_PHY_MISC(port));
> +			val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> +			I915_WRITE(ICL_PHY_MISC(port), val);
> +		}

Same as above.

>  
>  		val = I915_READ(ICL_PORT_COMP_DW0(port));
>  		val &= ~COMP_INIT;
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c b/drivers/gpu/drm/i915/display/intel_combo_phy.c
index 075bab2500eb..da590f1a998b 100644
--- a/drivers/gpu/drm/i915/display/intel_combo_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_combo_phy.c
@@ -183,9 +183,13 @@  static void cnl_combo_phys_uninit(struct drm_i915_private *dev_priv)
 static bool icl_combo_phy_enabled(struct drm_i915_private *dev_priv,
 				  enum port port)
 {
-	return !(I915_READ(ICL_PHY_MISC(port)) &
-		 ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN) &&
-		(I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT);
+	/* The PHY C added by EHL has no PHY_MISC register */
+	if (port == PORT_C)
+		return I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT;
+	else
+		return !(I915_READ(ICL_PHY_MISC(port)) &
+			 ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN) &&
+			(I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT);
 }
 
 static bool icl_combo_phy_verify_state(struct drm_i915_private *dev_priv,
@@ -300,18 +304,26 @@  static void icl_combo_phys_init(struct drm_i915_private *dev_priv)
 		}
 
 		/*
-		 * EHL's combo PHY A can be hooked up to either an external
-		 * display (via DDI-D) or an internal display (via DDI-A or
-		 * the DSI DPHY).  This is a motherboard design decision that
-		 * can't be changed on the fly, so initialize the PHY's mux
-		 * based on whether our VBT indicates the presence of any
-		 * "internal" child devices.
+		 * Although EHL adds a combo PHY C, there's no PHY_MISC
+		 * register for it and no need to program the
+		 * DE_IO_COMP_PWR_DOWN setting on PHY C.
 		 */
-		val = I915_READ(ICL_PHY_MISC(port));
-		if (IS_ELKHARTLAKE(dev_priv) && port == PORT_A)
-			val = ehl_combo_phy_a_mux(dev_priv, val);
-		val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
-		I915_WRITE(ICL_PHY_MISC(port), val);
+		if (port != PORT_C) {
+			/*
+			 * EHL's combo PHY A can be hooked up to either an
+			 * external display (via DDI-D) or an internal display
+			 * (via DDI-A or the DSI DPHY).  This is a motherboard
+			 * design decision that can't be changed on the fly, so
+			 * initialize the PHY's mux based on whether our VBT
+			 * indicates the presence of any "internal" child
+			 * devices.
+			 */
+			val = I915_READ(ICL_PHY_MISC(port));
+			if (IS_ELKHARTLAKE(dev_priv) && port == PORT_A)
+				val = ehl_combo_phy_a_mux(dev_priv, val);
+			val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
+			I915_WRITE(ICL_PHY_MISC(port), val);
+		}
 
 		cnl_set_procmon_ref_values(dev_priv, port);
 
@@ -343,9 +355,16 @@  static void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
 			DRM_WARN("Port %c combo PHY HW state changed unexpectedly\n",
 				 port_name(port));
 
-		val = I915_READ(ICL_PHY_MISC(port));
-		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
-		I915_WRITE(ICL_PHY_MISC(port), val);
+		/*
+		 * Although EHL adds a combo PHY C, there's no PHY_MISC
+		 * register for it and no need to program the
+		 * DE_IO_COMP_PWR_DOWN setting on PHY C.
+		 */
+		if (port != PORT_C) {
+			val = I915_READ(ICL_PHY_MISC(port));
+			val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
+			I915_WRITE(ICL_PHY_MISC(port), val);
+		}
 
 		val = I915_READ(ICL_PORT_COMP_DW0(port));
 		val &= ~COMP_INIT;