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

["Souza, Jose" <jose.souza@intel.com>]

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