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.so...@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: <sta...@vger.kernel.org> # v5.16+
> Cc: José Roberto de Souza <jose.so...@intel.com>
> Signed-off-by: Imre Deak <imre.d...@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));

Reply via email to