[Intel-gfx] [PATCH v2] drm/i915/ehl: Allow combo PHY A to drive a third external display

[Ville Syrjälä]

On Mon, Jun 17, 2019 at 04:48:10PM -0700, Matt Roper wrote:
> EHL has a mux on combo PHY A that allows it to be driven either by an
> internal display (DDI-A or DSI DPHY) or by an external display (DDI-D).
> This is a motherboard design decision that can not be changed on the
> fly.  Unfortunately there are no strap registers that allow us to detect
> the board configuration directly, so let's use the VBT to try to figure
> it out and program the mux accordingly.
> 
> v2:
>  - Confirmed that VBT's dvo port refers to the DDI and not the PHY.
>    Thus we can check more explicitly for (ddi_d && !(ddi_a || dsi)).  If
>    a bad VBT contradicts itself, let internal display win.  (Ville)
> 
> Cc: Clint Taylor <Clinton.A.Taylor at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c     |  3 ++
>  .../gpu/drm/i915/display/intel_combo_phy.c    | 36 +++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h               |  1 +
>  3 files changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index c4710889cb32..0c9808132d67 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -1668,6 +1668,9 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
>  		if (!child->device_type)
>  			continue;
>  
> +		DRM_DEBUG_KMS("Found VBT child device with type 0x%x\n",
> +			      child->device_type);
> +

Was this hunk intentional?

>  		/*
>  		 * Copy as much as we know (sizeof) and is available
>  		 * (child_dev_size) of the child device. Accessing the data must
> diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c b/drivers/gpu/drm/i915/display/intel_combo_phy.c
> index 841708da5a56..c18079a09a2e 100644
> --- a/drivers/gpu/drm/i915/display/intel_combo_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_combo_phy.c
> @@ -260,6 +260,32 @@ void intel_combo_phy_power_up_lanes(struct drm_i915_private *dev_priv,
>  	I915_WRITE(ICL_PORT_CL_DW10(port), val);
>  }
>  
> +static u32 ehl_combo_phy_a_mux(struct drm_i915_private *i915, u32 val)
> +{
> +	bool ddi_a_present = i915->vbt.ddi_port_info[PORT_A].child != NULL;
> +	bool ddi_d_present = i915->vbt.ddi_port_info[PORT_D].child != NULL;
> +	bool dsi_present = intel_bios_is_dsi_present(i915, NULL);
> +
> +	/*
> +	 * VBT's 'dvo port' field for child devices references the DDI, not
> +	 * the PHY.  So if combo PHY A is wired up to drive an external
> +	 * display, we should see a child device present on PORT_D and
> +	 * nothing on PORT_A and no DSI.
> +	 */
> +	if (ddi_d_present && !ddi_a_present && !dsi_present)
> +		return val | ICL_PHY_MISC_MUX_DDID;
> +
> +	/*
> +	 * If we encounter a VBT that claims to have an external display on
> +	 * DDI-D _and_ an internal display on DDI-A/DSI leave an error message
> +	 * in the log and let the internal display win.
> +	 */
> +	if (ddi_d_present)
> +		DRM_ERROR("VBT claims to have both internal and external displays on PHY A.  Configuring for internal.\n");
> +
> +	return val & ~ICL_PHY_MISC_MUX_DDID;
> +}
> +
>  static void icl_combo_phys_init(struct drm_i915_private *dev_priv)
>  {
>  	enum port port;
> @@ -273,7 +299,17 @@ static void icl_combo_phys_init(struct drm_i915_private *dev_priv)
>  			continue;
>  		}
>  
> +		/*
> +		 * 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_ICELAKE(dev_priv) && port == PORT_A)

Maybe IS_EHL instead?

Patch is
Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

> +			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);
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1ae36b9efc85..68afafafd312 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -11138,6 +11138,7 @@ enum skl_power_gate {
>  #define _ICL_PHY_MISC_B		0x64C04
>  #define ICL_PHY_MISC(port)	_MMIO_PORT(port, _ICL_PHY_MISC_A, \
>  						 _ICL_PHY_MISC_B)
> +#define  ICL_PHY_MISC_MUX_DDID			(1 << 28)
>  #define  ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN	(1 << 23)
>  
>  /* Icelake Display Stream Compression Registers */
> -- 
> 2.17.2

-- 
Ville Syrjälä
Intel