| Message ID | 20210716084821.260594-3-chris.chiu@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix this missing NIC after resume | expand |
Backports of more then simple complexity generally deserve some explanation. In this case it looks like you've dropped a couple of functions. On 7/16/21 2:48 AM, chris.chiu@canonical.com wrote: > From: Imre Deak <imre.deak@intel.com> > > BugLink: https://bugs.launchpad.net/bugs/1931072 > > Disconnect TypeC PHYs during system suspend and shutdown, even with the > corresponding TypeC sink still plugged to its connector, since leaving > the PHY connected causes havoc at least during system resume in the > presence of an Nvidia card. > > Note that this will only make a difference in the TypeC DP alternate > mode, since in Thunderbolt alternate mode the PHY is never owned by the > display engine and there is no notion of PHY ownership in legacy mode > (the display engine being the only possible owner in that mode and the > TypeC subsystem not having anything to do with the port in that case). > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3500 > Reported-and-tested-by: Chris Chiu <chris.chiu@canonical.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> > Reviewed-by: Uma Shankar <uma.shankar@intel.com> > Link: https://patchwork.freedesktop.org/patch/msgid/20210610174223.605904-1-imre.deak@intel.com > (backported from commit 151ec347b06a2fb6ecd2922475dca71a7af827a5 > linux-next) > Signed-off-by: Chris Chiu <chris.chiu@canonical.com> > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 34 ++++++++++++++++++++++-- > drivers/gpu/drm/i915/display/intel_tc.c | 34 +++++++++++++++++++----- > drivers/gpu/drm/i915/display/intel_tc.h | 2 ++ > 3 files changed, 61 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > index cf665636408b..b272da304795 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -5338,6 +5338,36 @@ static enum hpd_pin cnl_hpd_pin(struct drm_i915_private *dev_priv, > return HPD_PORT_A + port - PORT_A; > } > > +static void intel_ddi_encoder_suspend(struct intel_encoder *encoder) > +{ > + struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > + enum phy phy = intel_port_to_phy(i915, encoder->port); > + > + intel_dp_encoder_suspend(encoder); > + > + if (!intel_phy_is_tc(i915, phy)) > + return; > + > + intel_tc_port_disconnect_phy(dig_port); > +} > + > +static void intel_ddi_encoder_shutdown(struct intel_encoder *encoder) > +{ > + struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > + enum phy phy = intel_port_to_phy(i915, encoder->port); > + > + intel_dp_encoder_shutdown(encoder); > + > + if (!intel_phy_is_tc(i915, phy)) > + return; > + > + intel_tc_port_disconnect_phy(dig_port); > +} > + > #define port_tc_name(port) ((port) - PORT_TC1 + '1') > #define tc_port_name(tc_port) ((tc_port) - TC_PORT_1 + '1') > > @@ -5432,8 +5462,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port) > encoder->get_config = intel_ddi_get_config; > encoder->sync_state = intel_ddi_sync_state; > encoder->initial_fastset_check = intel_ddi_initial_fastset_check; > - encoder->suspend = intel_dp_encoder_suspend; > - encoder->shutdown = intel_dp_encoder_shutdown; > + encoder->suspend = intel_ddi_encoder_suspend; > + encoder->shutdown = intel_ddi_encoder_shutdown; > encoder->get_power_domains = intel_ddi_get_power_domains; > > encoder->type = INTEL_OUTPUT_DDI; > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c > index 8b6f16f9d0d1..2f4d7336d76e 100644 > --- a/drivers/gpu/drm/i915/display/intel_tc.c > +++ b/drivers/gpu/drm/i915/display/intel_tc.c > @@ -449,7 +449,7 @@ intel_tc_port_get_target_mode(struct intel_digital_port *dig_port) > } > > static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port, > - int required_lanes) > + int required_lanes, bool force_disconnect) > { > struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); > enum tc_port_mode old_tc_mode = dig_port->tc_mode; > @@ -465,7 +465,8 @@ static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port, > } > > icl_tc_phy_disconnect(dig_port); > - icl_tc_phy_connect(dig_port, required_lanes); > + if (!force_disconnect) > + icl_tc_phy_connect(dig_port, required_lanes); > > drm_dbg_kms(&i915->drm, "Port %s: TC port mode reset (%s -> %s)\n", > dig_port->tc_port_name, > @@ -555,7 +556,7 @@ bool intel_tc_port_connected(struct intel_encoder *encoder) > } > > static void __intel_tc_port_lock(struct intel_digital_port *dig_port, > - int required_lanes) > + int required_lanes, bool force_disconnect) > { > struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); > intel_wakeref_t wakeref; > @@ -569,8 +570,9 @@ static void __intel_tc_port_lock(struct intel_digital_port *dig_port, > > tc_cold_wref = tc_cold_block(dig_port); > > - if (intel_tc_port_needs_reset(dig_port)) > - intel_tc_port_reset_mode(dig_port, required_lanes); > + if (force_disconnect || intel_tc_port_needs_reset(dig_port)) > + intel_tc_port_reset_mode(dig_port, required_lanes, > + force_disconnect); > > tc_cold_unblock(dig_port, tc_cold_wref); > } > @@ -581,7 +583,7 @@ static void __intel_tc_port_lock(struct intel_digital_port *dig_port, > > void intel_tc_port_lock(struct intel_digital_port *dig_port) > { > - __intel_tc_port_lock(dig_port, 1); > + __intel_tc_port_lock(dig_port, 1, false); > } > > void intel_tc_port_unlock(struct intel_digital_port *dig_port) > @@ -595,6 +597,24 @@ void intel_tc_port_unlock(struct intel_digital_port *dig_port) > wakeref); > } > > +/** > + * intel_tc_port_disconnect_phy: disconnect TypeC PHY from display port > + * @dig_port: digital port > + * > + * Disconnect the given digital port from its TypeC PHY (handing back the > + * control of the PHY to the TypeC subsystem). The only purpose of this > + * function is to force the disconnect even with a TypeC display output still > + * plugged to the TypeC connector, which is required by the TypeC firmwares > + * during system suspend and shutdown. Otherwise - during the unplug event > + * handling - the PHY ownership is released automatically by > + * intel_tc_port_reset_mode(), when calling this function is not required. > + */ > +void intel_tc_port_disconnect_phy(struct intel_digital_port *dig_port) > +{ > + __intel_tc_port_lock(dig_port, 1, true); > + intel_tc_port_unlock(dig_port); > +} > + > bool intel_tc_port_ref_held(struct intel_digital_port *dig_port) > { > return mutex_is_locked(&dig_port->tc_lock) || > @@ -604,7 +624,7 @@ bool intel_tc_port_ref_held(struct intel_digital_port *dig_port) > void intel_tc_port_get_link(struct intel_digital_port *dig_port, > int required_lanes) > { > - __intel_tc_port_lock(dig_port, required_lanes); > + __intel_tc_port_lock(dig_port, required_lanes, false); > dig_port->tc_link_refcount++; > intel_tc_port_unlock(dig_port); > } > diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h > index b619e4736f85..57d157fc822f 100644 > --- a/drivers/gpu/drm/i915/display/intel_tc.h > +++ b/drivers/gpu/drm/i915/display/intel_tc.h > @@ -13,6 +13,8 @@ struct intel_digital_port; > struct intel_encoder; > > bool intel_tc_port_connected(struct intel_encoder *encoder); > +void intel_tc_port_disconnect_phy(struct intel_digital_port *dig_port); > + > u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port); > u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port); > int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port); >
On Mon, Jul 19, 2021 at 10:29 PM Tim Gardner <tim.gardner@canonical.com> wrote: > Backports of more then simple complexity generally deserve some > explanation. In this case it looks like you've dropped a couple of > functions. > > In the v2 version, I did miss the code of intel_ddi_encoder_suspend and shutdown, so I added them back in v3. In the v3 version, the dropped function which causes unclean cherry-pick is skl_hpd_pin() which is handled by the following commit which does not affect the TGL models. commit c8455098c67914c59d07f01819469e2e6f76f358 Author: Lyude Paul <lyude@redhat.com> Date: Tue Feb 9 14:16:28 2021 -0500 drm/i915/gen9_bc: Introduce HPD pin mappings for TGP PCH + CML combos and intel_ddi_is_tc() from the following commit which could cause lots of conflicts but we don't really need it for TGL model. commit 36ecb0ec105412aa7e7c89991a7cff90bf90b2f1 Author: Ville Syrjälä <ville.syrjala@linux.intel.com> Date: Fri Feb 5 23:46:26 2021 +0200 drm/i915: Extract icl+ .{enable,disable}_clock() vfuncs Should I explain in the cover letter? Which part would be appropriate? [Fix]? I'll propose a v4 with explanations. Thanks On 7/16/21 2:48 AM, chris.chiu@canonical.com wrote: > > From: Imre Deak <imre.deak@intel.com> > > > > BugLink: https://bugs.launchpad.net/bugs/1931072 > > > > Disconnect TypeC PHYs during system suspend and shutdown, even with the > > corresponding TypeC sink still plugged to its connector, since leaving > > the PHY connected causes havoc at least during system resume in the > > presence of an Nvidia card. > > > > Note that this will only make a difference in the TypeC DP alternate > > mode, since in Thunderbolt alternate mode the PHY is never owned by the > > display engine and there is no notion of PHY ownership in legacy mode > > (the display engine being the only possible owner in that mode and the > > TypeC subsystem not having anything to do with the port in that case). > > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3500 > > Reported-and-tested-by: Chris Chiu <chris.chiu@canonical.com> > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > Reviewed-by: Uma Shankar <uma.shankar@intel.com> > > Link: > https://patchwork.freedesktop.org/patch/msgid/20210610174223.605904-1-imre.deak@intel.com > > (backported from commit 151ec347b06a2fb6ecd2922475dca71a7af827a5 > > linux-next) > > Signed-off-by: Chris Chiu <chris.chiu@canonical.com> > > --- > > drivers/gpu/drm/i915/display/intel_ddi.c | 34 ++++++++++++++++++++++-- > > drivers/gpu/drm/i915/display/intel_tc.c | 34 +++++++++++++++++++----- > > drivers/gpu/drm/i915/display/intel_tc.h | 2 ++ > > 3 files changed, 61 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > b/drivers/gpu/drm/i915/display/intel_ddi.c > > index cf665636408b..b272da304795 100644 > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > @@ -5338,6 +5338,36 @@ static enum hpd_pin cnl_hpd_pin(struct > drm_i915_private *dev_priv, > > return HPD_PORT_A + port - PORT_A; > > } > > > > +static void intel_ddi_encoder_suspend(struct intel_encoder *encoder) > > +{ > > + struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > > + enum phy phy = intel_port_to_phy(i915, encoder->port); > > + > > + intel_dp_encoder_suspend(encoder); > > + > > + if (!intel_phy_is_tc(i915, phy)) > > + return; > > + > > + intel_tc_port_disconnect_phy(dig_port); > > +} > > + > > +static void intel_ddi_encoder_shutdown(struct intel_encoder *encoder) > > +{ > > + struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > > + enum phy phy = intel_port_to_phy(i915, encoder->port); > > + > > + intel_dp_encoder_shutdown(encoder); > > + > > + if (!intel_phy_is_tc(i915, phy)) > > + return; > > + > > + intel_tc_port_disconnect_phy(dig_port); > > +} > > + > > #define port_tc_name(port) ((port) - PORT_TC1 + '1') > > #define tc_port_name(tc_port) ((tc_port) - TC_PORT_1 + '1') > > > > @@ -5432,8 +5462,8 @@ void intel_ddi_init(struct drm_i915_private > *dev_priv, enum port port) > > encoder->get_config = intel_ddi_get_config; > > encoder->sync_state = intel_ddi_sync_state; > > encoder->initial_fastset_check = intel_ddi_initial_fastset_check; > > - encoder->suspend = intel_dp_encoder_suspend; > > - encoder->shutdown = intel_dp_encoder_shutdown; > > + encoder->suspend = intel_ddi_encoder_suspend; > > + encoder->shutdown = intel_ddi_encoder_shutdown; > > encoder->get_power_domains = intel_ddi_get_power_domains; > > > > encoder->type = INTEL_OUTPUT_DDI; > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c > b/drivers/gpu/drm/i915/display/intel_tc.c > > index 8b6f16f9d0d1..2f4d7336d76e 100644 > > --- a/drivers/gpu/drm/i915/display/intel_tc.c > > +++ b/drivers/gpu/drm/i915/display/intel_tc.c > > @@ -449,7 +449,7 @@ intel_tc_port_get_target_mode(struct > intel_digital_port *dig_port) > > } > > > > static void intel_tc_port_reset_mode(struct intel_digital_port > *dig_port, > > - int required_lanes) > > + int required_lanes, bool > force_disconnect) > > { > > struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); > > enum tc_port_mode old_tc_mode = dig_port->tc_mode; > > @@ -465,7 +465,8 @@ static void intel_tc_port_reset_mode(struct > intel_digital_port *dig_port, > > } > > > > icl_tc_phy_disconnect(dig_port); > > - icl_tc_phy_connect(dig_port, required_lanes); > > + if (!force_disconnect) > > + icl_tc_phy_connect(dig_port, required_lanes); > > > > drm_dbg_kms(&i915->drm, "Port %s: TC port mode reset (%s -> %s)\n", > > dig_port->tc_port_name, > > @@ -555,7 +556,7 @@ bool intel_tc_port_connected(struct intel_encoder > *encoder) > > } > > > > static void __intel_tc_port_lock(struct intel_digital_port *dig_port, > > - int required_lanes) > > + int required_lanes, bool force_disconnect) > > { > > struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); > > intel_wakeref_t wakeref; > > @@ -569,8 +570,9 @@ static void __intel_tc_port_lock(struct > intel_digital_port *dig_port, > > > > tc_cold_wref = tc_cold_block(dig_port); > > > > - if (intel_tc_port_needs_reset(dig_port)) > > - intel_tc_port_reset_mode(dig_port, required_lanes); > > + if (force_disconnect || > intel_tc_port_needs_reset(dig_port)) > > + intel_tc_port_reset_mode(dig_port, required_lanes, > > + force_disconnect); > > > > tc_cold_unblock(dig_port, tc_cold_wref); > > } > > @@ -581,7 +583,7 @@ static void __intel_tc_port_lock(struct > intel_digital_port *dig_port, > > > > void intel_tc_port_lock(struct intel_digital_port *dig_port) > > { > > - __intel_tc_port_lock(dig_port, 1); > > + __intel_tc_port_lock(dig_port, 1, false); > > } > > > > void intel_tc_port_unlock(struct intel_digital_port *dig_port) > > @@ -595,6 +597,24 @@ void intel_tc_port_unlock(struct intel_digital_port > *dig_port) > > wakeref); > > } > > > > +/** > > + * intel_tc_port_disconnect_phy: disconnect TypeC PHY from display port > > + * @dig_port: digital port > > + * > > + * Disconnect the given digital port from its TypeC PHY (handing back > the > > + * control of the PHY to the TypeC subsystem). The only purpose of this > > + * function is to force the disconnect even with a TypeC display output > still > > + * plugged to the TypeC connector, which is required by the TypeC > firmwares > > + * during system suspend and shutdown. Otherwise - during the unplug > event > > + * handling - the PHY ownership is released automatically by > > + * intel_tc_port_reset_mode(), when calling this function is not > required. > > + */ > > +void intel_tc_port_disconnect_phy(struct intel_digital_port *dig_port) > > +{ > > + __intel_tc_port_lock(dig_port, 1, true); > > + intel_tc_port_unlock(dig_port); > > +} > > + > > bool intel_tc_port_ref_held(struct intel_digital_port *dig_port) > > { > > return mutex_is_locked(&dig_port->tc_lock) || > > @@ -604,7 +624,7 @@ bool intel_tc_port_ref_held(struct > intel_digital_port *dig_port) > > void intel_tc_port_get_link(struct intel_digital_port *dig_port, > > int required_lanes) > > { > > - __intel_tc_port_lock(dig_port, required_lanes); > > + __intel_tc_port_lock(dig_port, required_lanes, false); > > dig_port->tc_link_refcount++; > > intel_tc_port_unlock(dig_port); > > } > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.h > b/drivers/gpu/drm/i915/display/intel_tc.h > > index b619e4736f85..57d157fc822f 100644 > > --- a/drivers/gpu/drm/i915/display/intel_tc.h > > +++ b/drivers/gpu/drm/i915/display/intel_tc.h > > @@ -13,6 +13,8 @@ struct intel_digital_port; > > struct intel_encoder; > > > > bool intel_tc_port_connected(struct intel_encoder *encoder); > > +void intel_tc_port_disconnect_phy(struct intel_digital_port *dig_port); > > + > > u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port); > > u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port > *dig_port); > > int intel_tc_port_fia_max_lane_count(struct intel_digital_port > *dig_port); > > > > -- > ----------- > Tim Gardner > Canonical, Inc >
On 7/20/21 3:32 AM, Chris Chiu wrote: > > > On Mon, Jul 19, 2021 at 10:29 PM Tim Gardner <tim.gardner@canonical.com > <mailto:tim.gardner@canonical.com>> wrote: > > Backports of more then simple complexity generally deserve some > explanation. In this case it looks like you've dropped a couple of > functions. > > > In the v2 version, I did miss the code of intel_ddi_encoder_suspend and > shutdown, so I added them back in v3. In the v3 version, the dropped > function which causes unclean cherry-pick is skl_hpd_pin() which is > handled by the following commit which does not affect the TGL models. > > commit c8455098c67914c59d07f01819469e2e6f76f358 > Author: Lyude Paul <lyude@redhat.com <mailto:lyude@redhat.com>> > Date: Tue Feb 9 14:16:28 2021 -0500 > > drm/i915/gen9_bc: Introduce HPD pin mappings for TGP PCH + CML combos > > and intel_ddi_is_tc() from the following commit which could cause lots > of conflicts but we don't really need it for TGL model. > commit 36ecb0ec105412aa7e7c89991a7cff90bf90b2f1 > Author: Ville Syrjälä <ville.syrjala@linux.intel.com > <mailto:ville.syrjala@linux.intel.com>> > Date: Fri Feb 5 23:46:26 2021 +0200 > > drm/i915: Extract icl+ .{enable,disable}_clock() vfuncs > Should I explain in the cover letter? Which part would be appropriate? > [Fix]? I'll propose a v4 with explanations. Thanks > Typically backport changes are explained right after the '(backported from commit ...)' in the form: [rtg - minor context adjustments] or [chris - dropped 2 functions to avoid backporting intrusive scaffolding patches] rtg ----------- Tim Gardner Canonical, Inc
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index cf665636408b..b272da304795 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -5338,6 +5338,36 @@ static enum hpd_pin cnl_hpd_pin(struct drm_i915_private *dev_priv, return HPD_PORT_A + port - PORT_A; } +static void intel_ddi_encoder_suspend(struct intel_encoder *encoder) +{ + struct intel_dp *intel_dp = enc_to_intel_dp(encoder); + struct drm_i915_private *i915 = dp_to_i915(intel_dp); + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); + enum phy phy = intel_port_to_phy(i915, encoder->port); + + intel_dp_encoder_suspend(encoder); + + if (!intel_phy_is_tc(i915, phy)) + return; + + intel_tc_port_disconnect_phy(dig_port); +} + +static void intel_ddi_encoder_shutdown(struct intel_encoder *encoder) +{ + struct intel_dp *intel_dp = enc_to_intel_dp(encoder); + struct drm_i915_private *i915 = dp_to_i915(intel_dp); + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); + enum phy phy = intel_port_to_phy(i915, encoder->port); + + intel_dp_encoder_shutdown(encoder); + + if (!intel_phy_is_tc(i915, phy)) + return; + + intel_tc_port_disconnect_phy(dig_port); +} + #define port_tc_name(port) ((port) - PORT_TC1 + '1') #define tc_port_name(tc_port) ((tc_port) - TC_PORT_1 + '1') @@ -5432,8 +5462,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port) encoder->get_config = intel_ddi_get_config; encoder->sync_state = intel_ddi_sync_state; encoder->initial_fastset_check = intel_ddi_initial_fastset_check; - encoder->suspend = intel_dp_encoder_suspend; - encoder->shutdown = intel_dp_encoder_shutdown; + encoder->suspend = intel_ddi_encoder_suspend; + encoder->shutdown = intel_ddi_encoder_shutdown; encoder->get_power_domains = intel_ddi_get_power_domains; encoder->type = INTEL_OUTPUT_DDI; diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c index 8b6f16f9d0d1..2f4d7336d76e 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.c +++ b/drivers/gpu/drm/i915/display/intel_tc.c @@ -449,7 +449,7 @@ intel_tc_port_get_target_mode(struct intel_digital_port *dig_port) } static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port, - int required_lanes) + int required_lanes, bool force_disconnect) { struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); enum tc_port_mode old_tc_mode = dig_port->tc_mode; @@ -465,7 +465,8 @@ static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port, } icl_tc_phy_disconnect(dig_port); - icl_tc_phy_connect(dig_port, required_lanes); + if (!force_disconnect) + icl_tc_phy_connect(dig_port, required_lanes); drm_dbg_kms(&i915->drm, "Port %s: TC port mode reset (%s -> %s)\n", dig_port->tc_port_name, @@ -555,7 +556,7 @@ bool intel_tc_port_connected(struct intel_encoder *encoder) } static void __intel_tc_port_lock(struct intel_digital_port *dig_port, - int required_lanes) + int required_lanes, bool force_disconnect) { struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); intel_wakeref_t wakeref; @@ -569,8 +570,9 @@ static void __intel_tc_port_lock(struct intel_digital_port *dig_port, tc_cold_wref = tc_cold_block(dig_port); - if (intel_tc_port_needs_reset(dig_port)) - intel_tc_port_reset_mode(dig_port, required_lanes); + if (force_disconnect || intel_tc_port_needs_reset(dig_port)) + intel_tc_port_reset_mode(dig_port, required_lanes, + force_disconnect); tc_cold_unblock(dig_port, tc_cold_wref); } @@ -581,7 +583,7 @@ static void __intel_tc_port_lock(struct intel_digital_port *dig_port, void intel_tc_port_lock(struct intel_digital_port *dig_port) { - __intel_tc_port_lock(dig_port, 1); + __intel_tc_port_lock(dig_port, 1, false); } void intel_tc_port_unlock(struct intel_digital_port *dig_port) @@ -595,6 +597,24 @@ void intel_tc_port_unlock(struct intel_digital_port *dig_port) wakeref); } +/** + * intel_tc_port_disconnect_phy: disconnect TypeC PHY from display port + * @dig_port: digital port + * + * Disconnect the given digital port from its TypeC PHY (handing back the + * control of the PHY to the TypeC subsystem). The only purpose of this + * function is to force the disconnect even with a TypeC display output still + * plugged to the TypeC connector, which is required by the TypeC firmwares + * during system suspend and shutdown. Otherwise - during the unplug event + * handling - the PHY ownership is released automatically by + * intel_tc_port_reset_mode(), when calling this function is not required. + */ +void intel_tc_port_disconnect_phy(struct intel_digital_port *dig_port) +{ + __intel_tc_port_lock(dig_port, 1, true); + intel_tc_port_unlock(dig_port); +} + bool intel_tc_port_ref_held(struct intel_digital_port *dig_port) { return mutex_is_locked(&dig_port->tc_lock) || @@ -604,7 +624,7 @@ bool intel_tc_port_ref_held(struct intel_digital_port *dig_port) void intel_tc_port_get_link(struct intel_digital_port *dig_port, int required_lanes) { - __intel_tc_port_lock(dig_port, required_lanes); + __intel_tc_port_lock(dig_port, required_lanes, false); dig_port->tc_link_refcount++; intel_tc_port_unlock(dig_port); } diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h index b619e4736f85..57d157fc822f 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.h +++ b/drivers/gpu/drm/i915/display/intel_tc.h @@ -13,6 +13,8 @@ struct intel_digital_port; struct intel_encoder; bool intel_tc_port_connected(struct intel_encoder *encoder); +void intel_tc_port_disconnect_phy(struct intel_digital_port *dig_port); + u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port); u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port); int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port);