Re: [Intel-gfx] [PATCH] drm/i915: Disconnect PHYs left connected by BIOS on disabled ports

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

On Thu, 2022-02-17 at 17:22 +0200, Imre Deak wrote:
> BIOS may leave a TypeC PHY in a connected state even though the
> corresponding port is disabled. This will prevent any hotplug events
> from being signalled (after the monitor deasserts and then reasserts its
> HPD) until the PHY is disconnected and so the driver will not detect a
> connected sink. Rebooting with the PHY in the connected state also
> results in a system hang.
> 
> Fix the above by disconnecting TypeC PHYs on disabled ports.
> 
> Before commit 64851a32c463e5 the PHY connected state was read out even
> for disabled ports and later the PHY got disconnected as a side effect
> of a tc_port_lock/unlock() sequence (during connector probing), hence
> recovering the port's hotplug functionality.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5014
> Fixes: 64851a32c463 ("drm/i915/tc: Add a mode for the TypeC PHY's disconnected state")
> Cc: <stable@vger.kernel.org> # v5.16+
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_tc.c | 28 ++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> index feead08ddf8ff..fc037c027ea5a 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.c
> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> @@ -693,6 +693,8 @@ void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
>  {
>  	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
>  	struct intel_encoder *encoder = &dig_port->base;
> +	intel_wakeref_t tc_cold_wref;
> +	enum intel_display_power_domain domain;
>  	int active_links = 0;
>  
>  	mutex_lock(&dig_port->tc_lock);
> @@ -704,12 +706,11 @@ void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
>  
>  	drm_WARN_ON(&i915->drm, dig_port->tc_mode != TC_PORT_DISCONNECTED);
>  	drm_WARN_ON(&i915->drm, dig_port->tc_lock_wakeref);
> +
> +	tc_cold_wref = tc_cold_block(dig_port, &domain);
> +
> +	dig_port->tc_mode = intel_tc_port_get_current_mode(dig_port);
>  	if (active_links) {
> -		enum intel_display_power_domain domain;
> -		intel_wakeref_t tc_cold_wref = tc_cold_block(dig_port, &domain);
> -
> -		dig_port->tc_mode = intel_tc_port_get_current_mode(dig_port);
> -
>  		if (!icl_tc_phy_is_connected(dig_port))
>  			drm_dbg_kms(&i915->drm,
>  				    "Port %s: PHY disconnected with %d active link(s)\n",
> @@ -718,10 +719,23 @@ void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
>  
>  		dig_port->tc_lock_wakeref = tc_cold_block(dig_port,
>  							  &dig_port->tc_lock_power_domain);
> -
> -		tc_cold_unblock(dig_port, domain, tc_cold_wref);
> +	} else {
> +		/*
> +		 * TBT-alt is the default mode in any case the PHY ownership is not
> +		 * held (regardless of the sink's connected live state), so
> +		 * we'll just switch to disconnected mode from it here without
> +		 * a note.
> +		 */
> +		if (dig_port->tc_mode != TC_PORT_TBT_ALT)
> +			drm_dbg_kms(&i915->drm,
> +				    "Port %s: PHY left in %s mode on disabled port, disconnecting it\n",
> +				    dig_port->tc_port_name,
> +				    tc_port_mode_name(dig_port->tc_mode));
> +		icl_tc_phy_disconnect(dig_port);
>  	}
>  
> +	tc_cold_unblock(dig_port, domain, tc_cold_wref);
> +
>  	drm_dbg_kms(&i915->drm, "Port %s: sanitize mode (%s)\n",
>  		    dig_port->tc_port_name,
>  		    tc_port_mode_name(dig_port->tc_mode));