All of lore.kernel.org
 help / color / mirror / Atom feed
From: Imre Deak <imre.deak@intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org,
	Kai-Heng Feng <kai.heng.feng@canonical.com>
Subject: Re: [Intel-gfx] [PATCH v4 14/14] drm/i915/tc: Reset TypeC PHYs left enabled in DP-alt mode after the sink disconnects
Date: Fri, 12 May 2023 16:50:58 +0300	[thread overview]
Message-ID: <ZF5EQjI7CqVkEVh3@ideak-desk.fi.intel.com> (raw)
In-Reply-To: <ZF1ANptc8cI4N3K0@intel.com>

On Thu, May 11, 2023 at 10:21:26PM +0300, Ville Syrjälä wrote:
> On Wed, May 10, 2023 at 01:31:31PM +0300, Imre Deak wrote:
> > If the output on a DP-alt link with its sink disconnected is kept
> > enabled for too long (about 20 sec), then some IOM/TCSS firmware timeout
> > will cause havoc on the PCI bus, at least for other GFX devices on it
> > which will stop powering up. Since user space is not guaranteed to do a
> > disabling modeset in time, switch such disconnected but active links to
> > TBT mode - which is without such shortcomings - with a 2 second delay.
> > 
> > If the above condition is detected already during the driver load/system
> > resume sanitization step disable the output instead, as at that point no
> > user space or kernel client depends on a consistent output state yet and
> > because subsequent atomic modeset on such connectors - without the
> > actual sink capabilities available - can fail.
> > 
> > An active/disconnected port as above will also block the HPD status of
> > other active/disconnected ports to get updated (stuck in the connected
> > state), until the former port is disabled, its PHY is disconnected and
> > a ~10 ms delay has elapsed. This means the link state for all TypeC
> > ports/CRTCs must be rechecked after a CRTC is disabled due to the above
> > reason. For this disconnect the PHY synchronously after the CRTC/port is
> > disabled and recheck all CRTCs for the above condition whenever such a
> > port is disabled.
> > 
> > To account for a race condition during driver loading where the sink is
> > disconnected after the above sanitization step and before the HPD
> > interrupts get enabled, do an explicit check/link reset if needed from
> > the encoder's late_register hook, which is called after the HPD
> > interrupts are enabled already.
> > 
> > v2:
> > - Handle an active/disconnected port blocking the HPD state update of
> >   another active/disconnected port.
> > - Cancel the delayed work resetting the link also from the encoder
> >   enable/suspend/shutdown hooks.
> > - Rebase on the earlier intel_modeset_lock_ctx_retry() addition,
> >   fixing here the missed atomic state reset in case of a retry.
> > - Fix handling of an error return from intel_atomic_get_crtc_state().
> > - Recheck if the port needs to be reset after all the atomic state
> >   is locked and async commits are waited on.
> > 
> > v3:
> > - Add intel_crtc_needs_link_reset(), instead of open-coding it,
> >   keep intel_crtc_has_encoders(). (Ville)
> > - Fix state dumping and use a bitmask to track disabled CRTCs in
> >   intel_sanitize_all_crtcs(). (Ville)
> > - Set internal in intel_atomic_state right after allocating it.
> >   (Ville)
> > - Recheck all CRTCs (not yet force-disabled) after a CRTC is
> >   force-disabled for any reason (not only due to a link state)
> >   in intel_sanitize_all_crtcs().
> > - Reduce delay after CRTC disabling to 20ms, and use the simpler
> >   msleep().
> > - Clarify code comment about HPD behaviour in
> >   intel_sanitize_all_crtcs().
> > - Move all the TC link reset logic to intel_tc.c .
> > - Cancel the link reset work synchronously during system suspend,
> >   driver unload and shutdown.
> > 
> > v4:
> > - Rebased on previous patch, which allows calling the TC port
> >   suspend/cleanup handlers without modeset locks held; remove the
> >   display driver suspended assert from the link reset work
> >   accordingly.
> > 
> > Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5860
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c      |  32 +++-
> >  drivers/gpu/drm/i915/display/intel_dp.c       |   6 +-
> >  drivers/gpu/drm/i915/display/intel_dp.h       |   3 +
> >  .../drm/i915/display/intel_modeset_setup.c    |  85 ++++++++--
> >  drivers/gpu/drm/i915/display/intel_tc.c       | 156 +++++++++++++++++-
> >  drivers/gpu/drm/i915/display/intel_tc.h       |   5 +-
> >  6 files changed, 262 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 7d09bd2412352..b443a79bc9803 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3313,6 +3313,8 @@ static void intel_disable_ddi(struct intel_atomic_state *state,
> >  			      const struct intel_crtc_state *old_crtc_state,
> >  			      const struct drm_connector_state *old_conn_state)
> >  {
> > +	intel_tc_port_link_cancel_reset_work(enc_to_dig_port(encoder));
> > +
> >  	intel_hdcp_disable(to_intel_connector(old_conn_state->connector));
> >  
> >  	if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_HDMI))
> > @@ -3381,6 +3383,8 @@ intel_ddi_pre_pll_enable(struct intel_atomic_state *state,
> >  	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> >  	bool is_tc_port = intel_phy_is_tc(dev_priv, phy);
> >  
> > +	intel_tc_port_link_cancel_reset_work(dig_port);
> 
> Can the reset work still be schduled at this time?

IIUC a hotplug->intel_tc_port_link_reset() could race with the above
encoder disable->intel_tc_port_link_cancel_reset_work() call in an async
commit (which doesn't hold modeset locks).

But it's just an optimization to avoid the overhead of checking the link
status from the work, not actually guaranteeing that it won't be scheduled.
So I can remove it from here.

> > +
> >  	if (is_tc_port) {
> >  		struct intel_crtc *master_crtc =
> >  			to_intel_crtc(crtc_state->uapi.crtc);
> > @@ -4232,9 +4236,19 @@ static void intel_ddi_encoder_reset(struct drm_encoder *encoder)
> >  		intel_tc_port_init_mode(dig_port);
> >  }
> >  
> > +static int intel_ddi_encoder_late_register(struct drm_encoder *_encoder)
> > +{
> > +	struct intel_encoder *encoder = to_intel_encoder(_encoder);
> > +
> > +	intel_tc_port_link_reset(enc_to_dig_port(encoder));
> > +
> > +	return 0;
> > +}
> > +
> >  static const struct drm_encoder_funcs intel_ddi_funcs = {
> >  	.reset = intel_ddi_encoder_reset,
> >  	.destroy = intel_ddi_encoder_destroy,
> > +	.late_register = intel_ddi_encoder_late_register,
> >  };
> >  
> >  static struct intel_connector *
> > @@ -4404,14 +4418,16 @@ intel_ddi_hotplug(struct intel_encoder *encoder,
> >  
> >  	state = intel_encoder_hotplug(encoder, connector);
> >  
> > -	intel_modeset_lock_ctx_retry(&ctx, NULL, 0, ret) {
> > -		if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA)
> > -			ret = intel_hdmi_reset_link(encoder, &ctx);
> > -		else
> > -			ret = intel_dp_retrain_link(encoder, &ctx);
> > -	}
> > +	if (!intel_tc_port_link_reset(dig_port)) {
> > +		intel_modeset_lock_ctx_retry(&ctx, NULL, 0, ret) {
> > +			if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA)
> > +				ret = intel_hdmi_reset_link(encoder, &ctx);
> > +			else
> > +				ret = intel_dp_retrain_link(encoder, &ctx);
> > +		}
> >  
> > -	drm_WARN_ON(encoder->base.dev, ret);
> > +		drm_WARN_ON(encoder->base.dev, ret);
> > +	}
> >  
> >  	/*
> >  	 * Unpowered type-c dongles can take some time to boot and be
> > @@ -4625,7 +4641,7 @@ static void intel_ddi_tc_encoder_suspend_complete(struct intel_encoder *encoder)
> >  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> >  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> >  
> > -	intel_tc_port_flush_work(dig_port);
> > +	intel_tc_port_suspend(dig_port);
> >  }
> >  
> >  static void intel_ddi_encoder_shutdown(struct intel_encoder *encoder)
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 99ceaa7d90b62..9a13ec09755fc 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -4231,9 +4231,9 @@ static bool intel_dp_has_connector(struct intel_dp *intel_dp,
> >  	return false;
> >  }
> >  
> > -static int intel_dp_get_active_pipes(struct intel_dp *intel_dp,
> > -				     struct drm_modeset_acquire_ctx *ctx,
> > -				     u8 *pipe_mask)
> > +int intel_dp_get_active_pipes(struct intel_dp *intel_dp,
> > +			      struct drm_modeset_acquire_ctx *ctx,
> > +			      u8 *pipe_mask)
> >  {
> >  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> >  	struct drm_connector_list_iter conn_iter;
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> > index ef39e4f7a329e..5f86157a10d2d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> > @@ -42,6 +42,9 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp,
> >  			      int link_rate, int lane_count);
> >  int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> >  					    int link_rate, u8 lane_count);
> > +int intel_dp_get_active_pipes(struct intel_dp *intel_dp,
> > +			      struct drm_modeset_acquire_ctx *ctx,
> > +			      u8 *pipe_mask);
> >  int intel_dp_retrain_link(struct intel_encoder *encoder,
> >  			  struct drm_modeset_acquire_ctx *ctx);
> >  void intel_dp_set_power(struct intel_dp *intel_dp, u8 mode);
> > diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > index 75b4dea1e442b..85f6401b8c709 100644
> > --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > @@ -26,6 +26,7 @@
> >  #include "intel_fifo_underrun.h"
> >  #include "intel_modeset_setup.h"
> >  #include "intel_pch_display.h"
> > +#include "intel_tc.h"
> >  #include "intel_vblank.h"
> >  #include "intel_wm.h"
> >  #include "skl_watermark.h"
> > @@ -379,6 +380,21 @@ static bool intel_crtc_has_encoders(struct intel_crtc *crtc)
> >  	return false;
> >  }
> >  
> > +static bool intel_crtc_needs_link_reset(struct intel_crtc *crtc)
> > +{
> > +	struct drm_device *dev = crtc->base.dev;
> > +	struct intel_encoder *encoder;
> > +
> > +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> > +		struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> > +
> > +		if (dig_port && intel_tc_port_link_needs_reset(dig_port))
> > +			return true;
> > +	}
> > +
> > +	return false;
> > +}
> > +
> >  static struct intel_connector *intel_encoder_find_connector(struct intel_encoder *encoder)
> >  {
> >  	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > @@ -421,11 +437,12 @@ static void intel_sanitize_fifo_underrun_reporting(const struct intel_crtc_state
> >  					   !HAS_GMCH(i915));
> >  }
> >  
> > -static void intel_sanitize_crtc(struct intel_crtc *crtc,
> > +static bool intel_sanitize_crtc(struct intel_crtc *crtc,
> >  				struct drm_modeset_acquire_ctx *ctx)
> >  {
> >  	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> >  	struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state);
> > +	bool needs_link_reset;
> >  
> >  	if (crtc_state->hw.active) {
> >  		struct intel_plane *plane;
> > @@ -445,13 +462,65 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
> >  		intel_color_commit_arm(crtc_state);
> >  	}
> >  
> > +	if (!crtc_state->hw.active ||
> > +	    intel_crtc_is_bigjoiner_slave(crtc_state))
> > +		return false;
> > +
> > +	needs_link_reset = intel_crtc_needs_link_reset(crtc);
> > +
> >  	/*
> >  	 * Adjust the state of the output pipe according to whether we have
> >  	 * active connectors/encoders.
> >  	 */
> > -	if (crtc_state->hw.active && !intel_crtc_has_encoders(crtc) &&
> > -	    !intel_crtc_is_bigjoiner_slave(crtc_state))
> > -		intel_crtc_disable_noatomic(crtc, ctx);
> > +	if (!needs_link_reset && intel_crtc_has_encoders(crtc))
> > +		return false;
> > +
> > +	intel_crtc_disable_noatomic(crtc, ctx);
> > +
> > +	/*
> > +	 * The HPD state on other active/disconnected TC ports may be stuck in
> > +	 * the connected state until this port is disabled and a ~10ms delay has
> > +	 * passed, wait here for that so that sanitizing other CRTCs will see the
> > +	 * up-to-date HPD state.
> > +	 */
> > +	if (needs_link_reset)
> > +		msleep(20);
> > +
> > +	return true;
> > +}
> > +
> > +static void intel_sanitize_all_crtcs(struct drm_i915_private *i915,
> > +				     struct drm_modeset_acquire_ctx *ctx)
> > +{
> > +	struct intel_crtc *crtc;
> > +	u8 crtcs_forced_off = 0;
> 
> Bit of a pipe vs. crtc confusion here. Maybe we should use
> u32+drm_crtc_mask() here for real since we're not really interested
> in individual pipes.

Ok.

> > +
> > +	/*
> > +	 * An active and disconnected TypeC port prevents the HPD live state
> > +	 * to get updated on other active/disconnected TypeC ports, so after
> > +	 * a port gets disabled the CRTCs using other TypeC ports must be
> > +	 * rechecked wrt. their link status.
> > +	 */
> > +	for (;;) {
> > +		u8 old_mask = crtcs_forced_off;
> > +
> > +		for_each_intel_crtc(&i915->drm, crtc) {
> > +			if (BIT(crtc->pipe) & crtcs_forced_off)
> > +				continue;
> > +
> > +			if (intel_sanitize_crtc(crtc, ctx))
> > +				crtcs_forced_off |= BIT(crtc->pipe);
> > +		}
> > +		if (crtcs_forced_off == old_mask)
> > +			break;
> > +	}
> > +
> > +	for_each_intel_crtc(&i915->drm, crtc) {
> > +		struct intel_crtc_state *crtc_state =
> > +			to_intel_crtc_state(crtc->base.state);
> > +
> > +		intel_crtc_state_dump(crtc_state, NULL, "setup_hw_state");
> > +	}
> >  }
> >  
> >  static bool has_bogus_dpll_config(const struct intel_crtc_state *crtc_state)
> > @@ -871,13 +940,7 @@ void intel_modeset_setup_hw_state(struct drm_i915_private *i915,
> >  	 */
> >  	intel_modeset_update_connector_atomic_state(i915);
> >  
> > -	for_each_intel_crtc(&i915->drm, crtc) {
> > -		struct intel_crtc_state *crtc_state =
> > -			to_intel_crtc_state(crtc->base.state);
> > -
> > -		intel_sanitize_crtc(crtc, ctx);
> > -		intel_crtc_state_dump(crtc_state, NULL, "setup_hw_state");
> > -	}
> > +	intel_sanitize_all_crtcs(i915, ctx);
> >  
> >  	intel_dpll_sanitize_state(i915);
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> > index 4fca711a58bce..4d45571fba956 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> > @@ -5,15 +5,19 @@
> >  
> >  #include "i915_drv.h"
> >  #include "i915_reg.h"
> > +#include "intel_atomic.h"
> >  #include "intel_cx0_phy_regs.h"
> >  #include "intel_ddi.h"
> >  #include "intel_de.h"
> >  #include "intel_display.h"
> > +#include "intel_display_driver.h"
> >  #include "intel_display_power_map.h"
> >  #include "intel_display_types.h"
> >  #include "intel_dkl_phy_regs.h"
> > +#include "intel_dp.h"
> >  #include "intel_dp_mst.h"
> >  #include "intel_mg_phy_regs.h"
> > +#include "intel_modeset_lock.h"
> >  #include "intel_tc.h"
> >  
> >  #define DP_PIN_ASSIGNMENT_C	0x3
> > @@ -51,6 +55,7 @@ struct intel_tc_port {
> >  	enum intel_display_power_domain lock_power_domain;
> >  #endif
> >  	struct delayed_work disconnect_phy_work;
> > +	struct delayed_work link_reset_work;
> >  	int link_refcount;
> >  	bool legacy_port:1;
> >  	char port_name[8];
> > @@ -1572,6 +1577,135 @@ bool intel_tc_port_connected(struct intel_encoder *encoder)
> >  	return is_connected;
> >  }
> >  
> > +static bool __intel_tc_port_link_needs_reset(struct intel_tc_port *tc)
> > +{
> > +	bool ret;
> > +
> > +	mutex_lock(&tc->lock);
> > +
> > +	ret = tc->link_refcount &&
> > +	      tc->mode == TC_PORT_DP_ALT &&
> > +	      intel_tc_port_needs_reset(tc);
> > +
> > +	mutex_unlock(&tc->lock);
> > +
> > +	return ret;
> > +}
> > +
> > +bool intel_tc_port_link_needs_reset(struct intel_digital_port *dig_port)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> > +	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
> > +
> > +	if (!intel_phy_is_tc(i915, phy))
> > +		return false;
> > +
> > +	return __intel_tc_port_link_needs_reset(to_tc_port(dig_port));
> > +}
> > +
> > +static int reset_link(struct intel_tc_port *tc)
> > +{
> > +	struct drm_i915_private *i915 = tc_to_i915(tc);
> > +	struct intel_digital_port *dig_port = tc->dig_port;
> > +	struct intel_dp *intel_dp = enc_to_intel_dp(&dig_port->base);
> > +	struct drm_modeset_acquire_ctx ctx;
> > +	struct drm_atomic_state *_state;
> > +	struct intel_atomic_state *state;
> > +	int ret = 0;
> > +
> > +	_state = drm_atomic_state_alloc(&i915->drm);
> > +	if (!_state)
> > +		return -ENOMEM;
> > +
> > +	state = to_intel_atomic_state(_state);
> > +	state->internal = true;
> > +
> > +	intel_modeset_lock_ctx_retry(&ctx, state, 0, ret) {
> > +		struct intel_crtc *crtc;
> > +		u8 pipe_mask;
> > +
> > +		ret = drm_modeset_lock(&i915->drm.mode_config.connection_mutex, &ctx);
> > +		if (ret)
> > +			continue;
> > +
> > +		ret = intel_dp_get_active_pipes(intel_dp, &ctx, &pipe_mask);
> > +		if (ret)
> > +			continue;
> > +
> > +		if (!pipe_mask)
> > +			continue;
> > +
> > +		for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask) {
> > +			struct intel_crtc_state *crtc_state;
> > +
> > +			crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
> > +			if (IS_ERR(crtc_state)) {
> > +				ret = PTR_ERR(crtc_state);
> > +				break;
> > +			}
> > +
> > +			crtc_state->uapi.connectors_changed = true;
> > +		}
> > +
> > +		if (ret)
> > +			continue;
> > +
> > +		if (!__intel_tc_port_link_needs_reset(tc))
> > +			continue;
> > +
> > +		ret = drm_atomic_commit(&state->base);
> 
> I suppose one way to avoid the dangers of the continue vs. not
> in these things would be to extract the entire body of the loop to
> a separate function.

Yes, having a function for it is clearer, will resend this with the
above changes.

> Anyways, the patch looks decent enough to me so
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks.

> 
> > +	}
> > +
> > +	drm_atomic_state_put(&state->base);
> > +
> > +	return ret;
> > +}
> > +
> > +static void intel_tc_port_link_reset_work(struct work_struct *work)
> > +{
> > +	struct intel_tc_port *tc =
> > +		container_of(work, struct intel_tc_port, link_reset_work.work);
> > +	struct drm_i915_private *i915 = tc_to_i915(tc);
> > +	int ret;
> > +
> > +	if (!__intel_tc_port_link_needs_reset(tc))
> > +		return;
> > +
> > +	mutex_lock(&i915->drm.mode_config.mutex);
> > +
> > +	drm_dbg_kms(&i915->drm,
> > +		    "Port %s: TypeC DP-alt sink disconnected, resetting link\n",
> > +		    tc->port_name);
> > +	ret = reset_link(tc);
> > +	drm_WARN_ON(&i915->drm, ret);
> > +
> > +	mutex_unlock(&i915->drm.mode_config.mutex);
> > +}
> > +
> > +bool intel_tc_port_link_reset(struct intel_digital_port *dig_port)
> > +{
> > +	if (!intel_tc_port_link_needs_reset(dig_port))
> > +		return false;
> > +
> > +	queue_delayed_work(system_unbound_wq,
> > +			   &to_tc_port(dig_port)->link_reset_work,
> > +			   msecs_to_jiffies(2000));
> > +
> > +	return true;
> > +}
> > +
> > +void intel_tc_port_link_cancel_reset_work(struct intel_digital_port *dig_port)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> > +	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
> > +	struct intel_tc_port *tc = to_tc_port(dig_port);
> > +
> > +	if (!intel_phy_is_tc(i915, phy))
> > +		return;
> > +
> > +	cancel_delayed_work(&tc->link_reset_work);
> > +}
> > +
> >  static void __intel_tc_port_lock(struct intel_tc_port *tc,
> >  				 int required_lanes)
> >  {
> > @@ -1619,11 +1753,19 @@ static void intel_tc_port_disconnect_phy_work(struct work_struct *work)
> >   *
> >   * Flush the delayed work disconnecting an idle PHY.
> >   */
> > -void intel_tc_port_flush_work(struct intel_digital_port *dig_port)
> > +static void intel_tc_port_flush_work(struct intel_digital_port *dig_port)
> >  {
> >  	flush_delayed_work(&to_tc_port(dig_port)->disconnect_phy_work);
> >  }
> >  
> > +void intel_tc_port_suspend(struct intel_digital_port *dig_port)
> > +{
> > +	struct intel_tc_port *tc = to_tc_port(dig_port);
> > +
> > +	cancel_delayed_work_sync(&tc->link_reset_work);
> > +	intel_tc_port_flush_work(dig_port);
> > +}
> > +
> >  void intel_tc_port_unlock(struct intel_digital_port *dig_port)
> >  {
> >  	struct intel_tc_port *tc = to_tc_port(dig_port);
> > @@ -1660,6 +1802,14 @@ void intel_tc_port_put_link(struct intel_digital_port *dig_port)
> >  	intel_tc_port_lock(dig_port);
> >  	__intel_tc_port_put_link(tc);
> >  	intel_tc_port_unlock(dig_port);
> > +
> > +	/*
> > +	 * The firmware will not update the HPD status of other TypeC ports
> > +	 * that are active in DP-alt mode with their sink disconnected, until
> > +	 * this port is disabled and its PHY gets disconnected. Make sure this
> > +	 * happens in a timely manner by disconnecting the PHY synchronously.
> > +	 */
> > +	intel_tc_port_flush_work(dig_port);
> >  }
> >  
> >  int intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy)
> > @@ -1692,7 +1842,9 @@ int intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy)
> >  		 "%c/TC#%d", port_name(port), tc_port + 1);
> >  
> >  	mutex_init(&tc->lock);
> > +	/* TODO: Combine the two works */
> >  	INIT_DELAYED_WORK(&tc->disconnect_phy_work, intel_tc_port_disconnect_phy_work);
> > +	INIT_DELAYED_WORK(&tc->link_reset_work, intel_tc_port_link_reset_work);
> >  	tc->legacy_port = is_legacy;
> >  	tc->mode = TC_PORT_DISCONNECTED;
> >  	tc->link_refcount = 0;
> > @@ -1706,7 +1858,7 @@ int intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy)
> >  
> >  void intel_tc_port_cleanup(struct intel_digital_port *dig_port)
> >  {
> > -	intel_tc_port_flush_work(dig_port);
> > +	intel_tc_port_suspend(dig_port);
> >  
> >  	kfree(dig_port->tc);
> >  	dig_port->tc = NULL;
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
> > index dd0810f9ea95e..3b16491925fa9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.h
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> > @@ -30,11 +30,14 @@ void intel_tc_port_sanitize_mode(struct intel_digital_port *dig_port,
> >  				 const struct intel_crtc_state *crtc_state);
> >  void intel_tc_port_lock(struct intel_digital_port *dig_port);
> >  void intel_tc_port_unlock(struct intel_digital_port *dig_port);
> > -void intel_tc_port_flush_work(struct intel_digital_port *dig_port);
> > +void intel_tc_port_suspend(struct intel_digital_port *dig_port);
> >  void intel_tc_port_get_link(struct intel_digital_port *dig_port,
> >  			    int required_lanes);
> >  void intel_tc_port_put_link(struct intel_digital_port *dig_port);
> >  bool intel_tc_port_ref_held(struct intel_digital_port *dig_port);
> > +bool intel_tc_port_link_needs_reset(struct intel_digital_port *dig_port);
> > +bool intel_tc_port_link_reset(struct intel_digital_port *dig_port);
> > +void intel_tc_port_link_cancel_reset_work(struct intel_digital_port *dig_port);
> >  
> >  int intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy);
> >  void intel_tc_port_cleanup(struct intel_digital_port *dig_port);
> > -- 
> > 2.37.2
> 
> -- 
> Ville Syrjälä
> Intel

  reply	other threads:[~2023-05-12 13:50 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-10 10:31 [Intel-gfx] [PATCH v4 00/14] drm/i915/tc: Add a workaround for an IOM/TCSS firmware hang issue Imre Deak
2023-05-10 10:31 ` [Intel-gfx] [PATCH v4 01/14] drm/i915: Fix PIPEDMC disabling for a bigjoiner configuration Imre Deak
2023-05-30 13:49   ` [v4,01/14] " Rudi Heitbaum
2023-05-30 13:49     ` [Intel-gfx] [v4, 01/14] " Rudi Heitbaum
2023-05-30 13:49     ` [v4,01/14] " Rudi Heitbaum
2023-05-31  8:47     ` Imre Deak
2023-05-31  8:47       ` Imre Deak
2023-05-31  8:47       ` [Intel-gfx] [v4, 01/14] " Imre Deak
2023-05-31 11:31       ` [v4,01/14] " Rudi Heitbaum
2023-05-31 11:31         ` [Intel-gfx] [v4, 01/14] " Rudi Heitbaum
2023-05-31 11:31         ` Rudi Heitbaum
2023-05-10 10:31 ` [Intel-gfx] [PATCH v4 02/14] drm/i915: Add helpers to reference/unreference a DPLL for a CRTC Imre Deak
2023-05-10 10:31 ` [Intel-gfx] [PATCH v4 03/14] drm/i915: Make the CRTC state consistent during sanitize-disabling Imre Deak
2023-05-10 10:31 ` [Intel-gfx] [PATCH v4 04/14] drm/i915: Update connector atomic state before crtc sanitize-disabling Imre Deak
2023-05-10 10:31 ` [Intel-gfx] [PATCH v4 05/14] drm/i915: Separate intel_crtc_disable_noatomic_begin/complete() Imre Deak
2023-05-10 10:31 ` [Intel-gfx] [PATCH v4 06/14] drm/i915: Factor out set_encoder_for_connector() Imre Deak
2023-05-10 10:31 ` [Intel-gfx] [PATCH v4 07/14] drm/i915: Add support for disabling any CRTCs during HW readout/sanitization Imre Deak
2023-05-10 10:31 ` [Intel-gfx] [PATCH v4 08/14] drm/i915/dp: Add link training debug and error printing helpers Imre Deak
2023-05-10 10:31 ` [Intel-gfx] [PATCH v4 09/14] drm/i915/dp: Convert link training error to debug message on disconnected sink Imre Deak
2023-05-10 10:31 ` [Intel-gfx] [PATCH v4 10/14] drm/i915/dp: Prevent link training fallback on disconnected port Imre Deak
2023-05-10 10:31 ` [Intel-gfx] [PATCH v4 11/14] drm/i915/dp: Factor out intel_dp_get_active_pipes() Imre Deak
2023-05-10 10:31 ` [Intel-gfx] [PATCH v4 12/14] drm/i915: Factor out a helper for handling atomic modeset locks/state Imre Deak
2023-05-10 10:31 ` [Intel-gfx] [PATCH v4 13/14] drm/i915/tc: Call TypeC port flush_work/cleanup without modeset locks held Imre Deak
2023-05-10 14:03   ` Ville Syrjälä
2023-05-10 14:10     ` Imre Deak
2023-05-11 17:40       ` Ville Syrjälä
2023-05-12 13:38         ` Imre Deak
2023-05-12 19:55   ` [Intel-gfx] [PATCH v5 " Imre Deak
2023-05-10 10:31 ` [Intel-gfx] [PATCH v4 14/14] drm/i915/tc: Reset TypeC PHYs left enabled in DP-alt mode after the sink disconnects Imre Deak
2023-05-11 19:21   ` Ville Syrjälä
2023-05-12 13:50     ` Imre Deak [this message]
2023-05-12 19:55   ` [Intel-gfx] [PATCH v5 " Imre Deak
2023-05-10 11:13 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/tc: Add a workaround for an IOM/TCSS firmware hang issue (rev11) Patchwork
2023-05-10 11:13 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-05-10 11:18 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-05-10 13:52 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2023-05-12 21:17 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/tc: Add a workaround for an IOM/TCSS firmware hang issue (rev13) Patchwork
2023-05-12 21:17 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-05-12 21:31 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-05-13  1:08 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2023-05-16 14:06   ` Imre Deak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZF5EQjI7CqVkEVh3@ideak-desk.fi.intel.com \
    --to=imre.deak@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=kai.heng.feng@canonical.com \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.