<div dir="ltr"><div dir="ltr"> </div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jul 19, 2021 at 10:29 PM Tim Gardner <<a href="mailto:tim.gardner@canonical.com">tim.gardner@canonical.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Backports of more then simple complexity generally deserve some explanation. In this case it looks like you've dropped a couple of functions. </blockquote><div> </div><div>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.</div><div> </div><div>commit c8455098c67914c59d07f01819469e2e6f76f358 Author: Lyude Paul <<a href="mailto:lyude@redhat.com">lyude@redhat.com</a>> Date: Tue Feb 9 14:16:28 2021 -0500 drm/i915/gen9_bc: Introduce HPD pin mappings for TGP PCH + CML combos </div><div> </div><div>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.</div><div>commit 36ecb0ec105412aa7e7c89991a7cff90bf90b2f1</div>Author: Ville Syrjälä <<a href="mailto:ville.syrjala@linux.intel.com">ville.syrjala@linux.intel.com</a>> Date: Fri Feb 5 23:46:26 2021 +0200 drm/i915: Extract icl+ .{enable,disable}_clock() vfuncs </div><div class="gmail_quote">Should I explain in the cover letter? Which part would be appropriate? [Fix]? I'll propose a v4 with explanations. Thanks</div><div class="gmail_quote"> </div><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 7/16/21 2:48 AM, <a href="mailto:chris.chiu@canonical.com" target="_blank">chris.chiu@canonical.com</a> wrote: > From: Imre Deak <<a href="mailto:imre.deak@intel.com" target="_blank">imre.deak@intel.com</a>> > > BugLink: <a href="https://bugs.launchpad.net/bugs/1931072" rel="noreferrer" target="_blank">https://bugs.launchpad.net/bugs/1931072</a> > > 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: <a href="https://gitlab.freedesktop.org/drm/intel/-/issues/3500" rel="noreferrer" target="_blank">https://gitlab.freedesktop.org/drm/intel/-/issues/3500</a> > Reported-and-tested-by: Chris Chiu <<a href="mailto:chris.chiu@canonical.com" target="_blank">chris.chiu@canonical.com</a>> > Signed-off-by: Imre Deak <<a href="mailto:imre.deak@intel.com" target="_blank">imre.deak@intel.com</a>> > Reviewed-by: Uma Shankar <<a href="mailto:uma.shankar@intel.com" target="_blank">uma.shankar@intel.com</a>> > Link: <a href="https://patchwork.freedesktop.org/patch/msgid/20210610174223.605904-1-imre.deak@intel.com" rel="noreferrer" target="_blank">https://patchwork.freedesktop.org/patch/msgid/20210610174223.605904-1-imre.deak@intel.com</a> > (backported from commit 151ec347b06a2fb6ecd2922475dca71a7af827a5 > linux-next) > Signed-off-by: Chris Chiu <<a href="mailto:chris.chiu@canonical.com" target="_blank">chris.chiu@canonical.com</a>> > --- > 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-><a href="http://base.base.dev" rel="noreferrer" target="_blank">base.base.dev</a>); > 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-><a href="http://base.base.dev" rel="noreferrer" target="_blank">base.base.dev</a>); > 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 </blockquote></div></div>