Re: [PATCH 20/23] drm/i915: Keep the TypeC port mode fixed when the port is active

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jun 04, 2019 at 05:58:23PM +0300, Imre Deak wrote:
> The TypeC port mode needs to stay fixed whenever the port is active. Do
> that by introducing a tc_link_refcount to account for active ports,
> avoiding changing the port mode if a reference is held.
> 
> During the modeset commit phase we also have to reset the port mode and
> update the active PLL reflecting the new port mode. We can do this only
> once the port and its old PLL has been already disabled. Add the new
> connector_update_prepare/complete hooks that are called around the whole
> enabling sequence. The TypeC specific hooks of these will reset the port
> mode, update the active PLL if the port will be active and ensure that
> the port mode will stay fixed for the duration of the whole enabling
> sequence by holding a tc_link_refcount.
> 
> During the port enabling, the pre_pll_enable/post_pll_disable hooks will
> take/release a tc_link_refcount to ensure the port mode stays fixed
> while the port is active.
> 
> Changing the port mode should also be avoided during connector detection
> and AUX transfers if the port is active, we'll do that by checking the
> port's tc_link_refcount.
> 
> When resetting the port mode we also have to take into account the
> maximum lanes provided by the FIA. It's guaranteed to be 4 in TBT-alt
> and legacy modes, but there may be less lanes available in DP-alt mode,
> in which case we have to fall back to TBT-alt mode.
> 
> While at it also update icl_tc_phy_connect()'s code comment, reflecting
> the current way of switching the port mode.
> 
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c      | 46 +++++++++++--
>  drivers/gpu/drm/i915/intel_ddi.h      |  7 ++
>  drivers/gpu/drm/i915/intel_display.c  | 90 +++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_dp.c       |  7 ++
>  drivers/gpu/drm/i915/intel_dp_mst.c   |  6 ++
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 28 +++++++-
>  drivers/gpu/drm/i915/intel_dpll_mgr.h |  3 +
>  drivers/gpu/drm/i915/intel_drv.h      |  9 +++
>  drivers/gpu/drm/i915/intel_hdmi.c     |  7 ++
>  drivers/gpu/drm/i915/intel_tc.c       | 93 ++++++++++++++++++++-------
>  drivers/gpu/drm/i915/intel_tc.h       |  3 +
>  11 files changed, 269 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index ad2f7bb2f50b..138950941246 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3610,6 +3610,38 @@ static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
>  	I915_WRITE(PORT_TX_DFLEXDPMLE1, val);
>  }
>  
> +void
> +intel_ddi_connector_update_prepare(struct intel_atomic_state *state,
> +				   struct intel_connector *connector)
> +{
> +	struct drm_connector_state *conn_state =
> +		drm_atomic_get_new_connector_state(&state->base,
> +						   &connector->base);
> +	struct intel_crtc *crtc = conn_state->crtc ?
> +		to_intel_crtc(conn_state->crtc) : NULL;
> +	struct intel_crtc_state *crtc_state =
> +		crtc ? intel_atomic_get_new_crtc_state(state, crtc) : NULL;
> +	struct intel_digital_port *primary_port =
> +		intel_connector_primary_digital_port(connector);
> +	int required_lanes = crtc_state ? crtc_state->lane_count : 1;
> +
> +	WARN_ON(crtc && crtc->active);
> +
> +	intel_tc_port_get_link(primary_port, required_lanes);
> +	if (crtc_state && crtc_state->base.active)
> +		intel_update_active_dpll(state, crtc, &primary_port->base);
> +}

Having this as a connector hook feels a bit strange. I guess moving it
to encoder level would feel more in line with the rest of the hooks, and
would avoid having that intel_connector_primary_digital_port() thing
because then the mst code could just pass in the right thing itself.
Hmm. Or maybe that wouldn't actually work because of
intel_connector->encoder being wonky on MST :(

Although if we just put the hooks on the main encoder and not on the
MST ones it could maybe work. Would still need
intel_connector_primary_digital_port() to dig out the encoder though.

Not sure.

> +
> +void
> +intel_ddi_connector_update_complete(struct intel_atomic_state *state,
> +				    struct intel_connector *connector)
> +{
> +	struct intel_digital_port *primary_port =
> +		intel_connector_primary_digital_port(connector);
> +
> +	intel_tc_port_put_link(primary_port);
> +}
> +
>  static void
>  intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
>  			 const struct intel_crtc_state *crtc_state,
> @@ -3617,10 +3649,13 @@ intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> +	bool is_tc_port = intel_port_is_tc(dev_priv, encoder->port);
>  	enum port port = encoder->port;
>  
> -	if (intel_crtc_has_dp_encoder(crtc_state) ||
> -	    intel_port_is_tc(dev_priv, encoder->port))
> +	if (is_tc_port)
> +		intel_tc_port_get_link(dig_port, crtc_state->lane_count);
> +
> +	if (intel_crtc_has_dp_encoder(crtc_state) || is_tc_port)
>  		intel_display_power_get(dev_priv,
>  					intel_ddi_main_link_aux_domain(dig_port));
>  
> @@ -3645,11 +3680,14 @@ intel_ddi_post_pll_disable(struct intel_encoder *encoder,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> +	bool is_tc_port = intel_port_is_tc(dev_priv, encoder->port);
>  
> -	if (intel_crtc_has_dp_encoder(crtc_state) ||
> -	    intel_port_is_tc(dev_priv, encoder->port))
> +	if (intel_crtc_has_dp_encoder(crtc_state) || is_tc_port)
>  		intel_display_power_put_unchecked(dev_priv,
>  						  intel_ddi_main_link_aux_domain(dig_port));
> +
> +	if (is_tc_port)
> +		intel_tc_port_put_link(dig_port);
>  }
>  
>  void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.h b/drivers/gpu/drm/i915/intel_ddi.h
> index 9cf69175942e..1559d1fbf7bd 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.h
> +++ b/drivers/gpu/drm/i915/intel_ddi.h
> @@ -12,6 +12,7 @@
>  
>  struct drm_connector_state;
>  struct drm_i915_private;
> +struct intel_atomic_state;
>  struct intel_connector;
>  struct intel_crtc;
>  struct intel_crtc_state;
> @@ -35,6 +36,12 @@ void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp);
>  bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
>  void intel_ddi_get_config(struct intel_encoder *encoder,
>  			  struct intel_crtc_state *pipe_config);
> +void
> +intel_ddi_connector_update_prepare(struct intel_atomic_state *state,
> +				   struct intel_connector *connector);
> +void
> +intel_ddi_connector_update_complete(struct intel_atomic_state *state,
> +				    struct intel_connector *connector);
>  void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
>  				    bool state);
>  void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f86b5b848cbc..2c65587d1622 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -520,6 +520,20 @@ needs_modeset(const struct drm_crtc_state *state)
>  	return drm_atomic_crtc_needs_modeset(state);
>  }
>  
> +static bool
> +intel_connector_needs_modeset(struct intel_atomic_state *state,
> +			      const struct drm_connector_state *old_conn_state,
> +			      const struct drm_connector_state *new_conn_state)
> +{
> +	if (new_conn_state->crtc != old_conn_state->crtc ||
> +	    (new_conn_state->crtc &&
> +	     needs_modeset(drm_atomic_get_new_crtc_state(&state->base,
> +							 new_conn_state->crtc))))
> +		return true;
> +
> +	return false;

Pointless if statement. Or is there more coming here?

> +}
> +
>  /*
>   * Platform specific helpers to calculate the port PLL loopback- (clock.m),
>   * and post-divider (clock.p) values, pre- (clock.vco) and post-divided fast
> @@ -6032,6 +6046,52 @@ static void intel_crtc_disable_planes(struct intel_atomic_state *state,
>  	intel_frontbuffer_flip(dev_priv, fb_bits);
>  }
>  
> +static void intel_connectors_update_prepare(struct intel_atomic_state *state)
> +{
> +	struct drm_connector_state *old_conn_state;
> +	struct drm_connector_state *new_conn_state;
> +	struct drm_connector *conn;
> +	int i;
> +
> +	for_each_oldnew_connector_in_state(&state->base, conn,
> +					   old_conn_state, new_conn_state, i) {
> +		struct intel_connector *connector = to_intel_connector(conn);
> +
> +		if (!connector->update_prepare)
> +			continue;
> +
> +		if (!intel_connector_needs_modeset(state,
> +						   old_conn_state,
> +						   new_conn_state))
> +			continue;
> +
> +		connector->update_prepare(state, connector);
> +	}
> +}
> +
> +static void intel_connectors_update_complete(struct intel_atomic_state *state)
> +{
> +	struct drm_connector_state *old_conn_state;
> +	struct drm_connector_state *new_conn_state;
> +	struct drm_connector *conn;
> +	int i;
> +
> +	for_each_oldnew_connector_in_state(&state->base, conn,
> +					   old_conn_state, new_conn_state, i) {
> +		struct intel_connector *connector = to_intel_connector(conn);
> +
> +		if (!connector->update_complete)
> +			continue;
> +
> +		if (!intel_connector_needs_modeset(state,
> +						   old_conn_state,
> +						   new_conn_state))
> +			continue;
> +
> +		connector->update_complete(state, connector);
> +	}
> +}
> +
>  static void intel_encoders_pre_pll_enable(struct drm_crtc *crtc,
>  					  struct intel_crtc_state *crtc_state,
>  					  struct drm_atomic_state *old_state)
> @@ -6556,6 +6616,28 @@ static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state)
>  	I915_WRITE(BCLRPAT(crtc->pipe), 0);
>  }
>  
> +/*
> + * intel_connector_primary_digital_port - get the primary port for a connector
> + * @connector: connector for which to return the port
> + *
> + * Returns the primary digital port for a DP MST, the single digital port for
> + * DP SST and HDMI and NULL for all other connector types.
> + */
> +struct intel_digital_port *
> +intel_connector_primary_digital_port(struct intel_connector *connector)
> +{
> +	struct intel_encoder *encoder;
> +
> +	if (connector->mst_port)
> +		return dp_to_dig_port(connector->mst_port);
> +
> +	encoder = intel_attached_encoder(&connector->base);
> +	if (WARN_ON(!encoder))
> +		return NULL;
> +
> +	return enc_to_dig_port(&encoder->base);
> +}
> +
>  bool intel_port_is_combophy(struct drm_i915_private *dev_priv, enum port port)
>  {
>  	if (port == PORT_NONE)
> @@ -13805,14 +13887,20 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>  		}
>  	}
>  
> +	if (intel_state->modeset)
> +		intel_connectors_update_prepare(intel_state);
> +
>  	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
>  	dev_priv->display.update_crtcs(state);
>  
> -	if (intel_state->modeset)
> +	if (intel_state->modeset) {
> +		intel_connectors_update_complete(intel_state);
> +
>  		intel_set_cdclk_post_plane_update(dev_priv,
>  						  &intel_state->cdclk.actual,
>  						  &dev_priv->cdclk.actual,
>  						  intel_state->cdclk.pipe);
> +	}
>  
>  	/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
>  	 * already, but still need the state for the delayed optimization. To
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b984410f41a4..2f63476e3cf2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -7195,6 +7195,13 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  	else
>  		intel_connector->get_hw_state = intel_connector_get_hw_state;
>  
> +	if (intel_port_is_tc(dev_priv, intel_encoder->port)) {
> +		intel_connector->update_prepare =
> +			intel_ddi_connector_update_prepare;
> +		intel_connector->update_complete =
> +			intel_ddi_connector_update_complete;
> +	}
> +
>  	/* init MST on ports that can support it */
>  	if (HAS_DP_MST(dev_priv) && !intel_dp_is_edp(intel_dp) &&
>  	    (port == PORT_B || port == PORT_C ||
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 0caf645fbbb8..9d5b048b9c96 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -505,6 +505,12 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
>  	if (!intel_connector)
>  		return NULL;
>  
> +	if (intel_port_is_tc(dev_priv, intel_dig_port->base.port)) {
> +		intel_connector->update_prepare =
> +			intel_ddi_connector_update_prepare;
> +		intel_connector->update_complete =
> +			intel_ddi_connector_update_complete;
> +	}
>  	intel_connector->get_hw_state = intel_dp_mst_get_hw_state;
>  	intel_connector->mst_port = intel_dp;
>  	intel_connector->port = port;
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index ce08a2eee55f..ce397b69b1d6 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -1939,7 +1939,9 @@ struct intel_dpll_mgr {
>  			  struct intel_encoder *encoder);
>  	void (*put_dplls)(struct intel_atomic_state *state,
>  			  struct intel_crtc *crtc);
> -
> +	void (*update_active_dpll)(struct intel_atomic_state *state,
> +				   struct intel_crtc *crtc,
> +				   struct intel_encoder *encoder);
>  	void (*dump_hw_state)(struct drm_i915_private *dev_priv,
>  			      const struct intel_dpll_hw_state *hw_state);
>  };
> @@ -3401,6 +3403,7 @@ static const struct intel_dpll_mgr icl_pll_mgr = {
>  	.dpll_info = icl_plls,
>  	.get_dplls = icl_get_dplls,
>  	.put_dplls = icl_put_dplls,
> +	.update_active_dpll = icl_update_active_dpll,
>  	.dump_hw_state = icl_dump_hw_state,
>  };
>  
> @@ -3525,6 +3528,29 @@ void intel_release_shared_dplls(struct intel_atomic_state *state,
>  	dpll_mgr->put_dplls(state, crtc);
>  }
>  
> +/**
> + * intel_update_active_dpll - update the active DPLL for a CRTC/encoder
> + * @state: atomic state
> + * @crtc: the CRTC for which to update the active DPLL
> + * @encoder: encoder determining the type of port DPLL
> + *
> + * Update the active DPLL for the given @crtc/@encoder in @crtc's atomic state,
> + * from the port DPLLs reserved previously by intel_reserve_shared_dplls(). The
> + * DPLL selected will be based on the current mode of the encoder's port.
> + */
> +void intel_update_active_dpll(struct intel_atomic_state *state,
> +			      struct intel_crtc *crtc,
> +			      struct intel_encoder *encoder)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	const struct intel_dpll_mgr *dpll_mgr = dev_priv->dpll_mgr;
> +
> +	if (WARN_ON(!dpll_mgr))
> +		return;
> +
> +	dpll_mgr->update_active_dpll(state, crtc, encoder);
> +}
> +
>  /**
>   * intel_shared_dpll_dump_hw_state - write hw_state to dmesg
>   * @dev_priv: i915 drm device
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> index 3bea81bde343..5817faa129d5 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> @@ -346,6 +346,9 @@ void intel_release_shared_dplls(struct intel_atomic_state *state,
>  				struct intel_crtc *crtc);
>  void icl_set_active_port_dpll(struct intel_crtc_state *crtc_state,
>  			      enum icl_port_dpll_id port_dpll_id);
> +void intel_update_active_dpll(struct intel_atomic_state *state,
> +			      struct intel_crtc *crtc,
> +			      struct intel_encoder *encoder);
>  void intel_prepare_shared_dpll(const struct intel_crtc_state *crtc_state);
>  void intel_enable_shared_dpll(const struct intel_crtc_state *crtc_state);
>  void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c61955c41976..b96656f1b8d4 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -375,6 +375,11 @@ struct intel_connector {
>  	 * and active (i.e. dpms ON state). */
>  	bool (*get_hw_state)(struct intel_connector *);
>  
> +	void (*update_prepare)(struct intel_atomic_state *state,
> +			       struct intel_connector *connector);
> +	void (*update_complete)(struct intel_atomic_state *state,
> +				struct intel_connector *connector);
> +
>  	/* Panel info for eDP and LVDS */
>  	struct intel_panel panel;
>  
> @@ -1234,6 +1239,8 @@ struct intel_digital_port {
>  	enum aux_ch aux_ch;
>  	enum intel_display_power_domain ddi_io_power_domain;
>  	struct mutex tc_lock;
> +	intel_wakeref_t tc_lock_wakeref;
> +	int tc_link_refcount;
>  	bool tc_legacy_port:1;
>  	enum tc_port_mode tc_mode;
>  
> @@ -1485,6 +1492,8 @@ void intel_pps_unlock_regs_wa(struct drm_i915_private *dev_priv);
>  void intel_encoder_destroy(struct drm_encoder *encoder);
>  struct drm_display_mode *
>  intel_encoder_current_mode(struct intel_encoder *encoder);
> +struct intel_digital_port *
> +intel_connector_primary_digital_port(struct intel_connector *connector);
>  bool intel_port_is_combophy(struct drm_i915_private *dev_priv, enum port port);
>  bool intel_port_is_tc(struct drm_i915_private *dev_priv, enum port port);
>  enum tc_port intel_port_to_tc(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 097bfa504ece..89f09e27b741 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -3092,6 +3092,13 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  	else
>  		intel_connector->get_hw_state = intel_connector_get_hw_state;
>  
> +	if (intel_port_is_tc(dev_priv, intel_encoder->port)) {
> +		intel_connector->update_prepare =
> +			intel_ddi_connector_update_prepare;
> +		intel_connector->update_complete =
> +			intel_ddi_connector_update_complete;
> +	}
> +
>  	intel_hdmi_add_properties(intel_hdmi, connector);
>  
>  	intel_connector_attach_encoder(intel_connector, intel_encoder);
> diff --git a/drivers/gpu/drm/i915/intel_tc.c b/drivers/gpu/drm/i915/intel_tc.c
> index 4b2f525bc2a6..e79f6ceb26f3 100644
> --- a/drivers/gpu/drm/i915/intel_tc.c
> +++ b/drivers/gpu/drm/i915/intel_tc.c
> @@ -188,21 +188,13 @@ static bool icl_tc_phy_is_in_safe_mode(struct intel_digital_port *dig_port)
>   * display, USB, etc. As a result, handshaking through FIA is required around
>   * connect and disconnect to cleanly transfer ownership with the controller and
>   * set the type-C power state.
> - *
> - * We could opt to only do the connect flow when we actually try to use the AUX
> - * channels or do a modeset, then immediately run the disconnect flow after
> - * usage, but there are some implications on this for a dynamic environment:
> - * things may go away or change behind our backs. So for now our driver is
> - * always trying to acquire ownership of the controller as soon as it gets an
> - * interrupt (or polls state and sees a port is connected) and only gives it
> - * back when it sees a disconnect. Implementation of a more fine-grained model
> - * will require a lot of coordination with user space and thorough testing for
> - * the extra possible cases.
>   */
> -static void icl_tc_phy_connect(struct intel_digital_port *dig_port)
> +static void icl_tc_phy_connect(struct intel_digital_port *dig_port,
> +			       int required_lanes)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
>  	enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
> +	int max_lanes;
>  
>  	if (!icl_tc_phy_status_complete(dig_port)) {
>  		DRM_DEBUG_KMS("Port %s: PHY not ready\n",
> @@ -214,8 +206,9 @@ static void icl_tc_phy_connect(struct intel_digital_port *dig_port)
>  	    !WARN_ON(dig_port->tc_legacy_port))
>  		goto out_set_tbt_alt_mode;
>  
> +	max_lanes = intel_tc_port_fia_max_lane_count(dig_port);
>  	if (dig_port->tc_legacy_port) {
> -		WARN_ON(intel_tc_port_fia_max_lane_count(dig_port) != 4);
> +		WARN_ON(max_lanes != 4);
>  		dig_port->tc_mode = TC_PORT_LEGACY;
>  
>  		return;
> @@ -231,6 +224,13 @@ static void icl_tc_phy_connect(struct intel_digital_port *dig_port)
>  		goto out_set_safe_mode;
>  	}
>  
> +	if (max_lanes < required_lanes) {
> +		DRM_DEBUG_KMS("Port %s: PHY max lanes %d < required lanes %d\n",
> +			      tc_port_name(dev_priv, tc_port),
> +			      max_lanes, required_lanes);
> +		goto out_set_safe_mode;
> +	}
> +
>  	dig_port->tc_mode = TC_PORT_DP_ALT;
>  
>  	return;
> @@ -317,7 +317,8 @@ intel_tc_port_get_target_mode(struct intel_digital_port *dig_port)
>  					  TC_PORT_TBT_ALT;
>  }
>  
> -static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port)
> +static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port,
> +				     int required_lanes)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
>  	enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
> @@ -326,7 +327,7 @@ static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port)
>  	intel_display_power_flush_work(dev_priv);
>  
>  	icl_tc_phy_disconnect(dig_port);
> -	icl_tc_phy_connect(dig_port);
> +	icl_tc_phy_connect(dig_port, required_lanes);
>  
>  	DRM_DEBUG_KMS("Port %s: TC port mode reset (%s -> %s)\n",
>  		      tc_port_name(dev_priv, tc_port),
> @@ -334,6 +335,14 @@ static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port)
>  		      tc_port_mode_name(dig_port->tc_mode));
>  }
>  
> +static void
> +intel_tc_port_link_init_refcount(struct intel_digital_port *dig_port,
> +				 int refcount)
> +{
> +	WARN_ON(dig_port->tc_link_refcount);
> +	dig_port->tc_link_refcount = refcount;
> +}
> +
>  void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> @@ -354,11 +363,13 @@ void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
>  			DRM_DEBUG_DRIVER("Port %s: PHY disconnected with %d active link(s)\n",
>  					 tc_port_name(dev_priv, tc_port),
>  					 active_links);
> +		intel_tc_port_link_init_refcount(dig_port, active_links);
> +
>  		goto out;
>  	}
>  
>  	if (dig_port->tc_legacy_port)
> -		icl_tc_phy_connect(dig_port);
> +		icl_tc_phy_connect(dig_port, 1);
>  
>  out:
>  	DRM_DEBUG_DRIVER("Port %s: sanitize mode (%s)\n",
> @@ -388,27 +399,60 @@ bool intel_tc_port_connected(struct intel_digital_port *dig_port)
>  {
>  	bool is_connected;
>  
> -	mutex_lock(&dig_port->tc_lock);
> -
> -	if (intel_tc_port_needs_reset(dig_port))
> -		intel_tc_port_reset_mode(dig_port);
> -
> +	intel_tc_port_lock(dig_port);
>  	is_connected = tc_port_live_status_mask(dig_port) &
>  		       BIT(dig_port->tc_mode);
> -
> -	mutex_unlock(&dig_port->tc_lock);
> +	intel_tc_port_unlock(dig_port);
>  
>  	return is_connected;
>  }
>  
> -void intel_tc_port_lock(struct intel_digital_port *dig_port)
> +static void __intel_tc_port_lock(struct intel_digital_port *dig_port,
> +				 int required_lanes)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> +	intel_wakeref_t wakeref;
> +
> +	wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_DISPLAY_CORE);
> +
>  	mutex_lock(&dig_port->tc_lock);
> -	/* TODO: reset the TypeC port mode if needed */
> +
> +	if (!dig_port->tc_link_refcount &&
> +	    intel_tc_port_needs_reset(dig_port))
> +		intel_tc_port_reset_mode(dig_port, required_lanes);
> +
> +	WARN_ON(dig_port->tc_lock_wakeref);
> +	dig_port->tc_lock_wakeref = wakeref;
> +}
> +
> +void intel_tc_port_lock(struct intel_digital_port *dig_port)
> +{
> +	__intel_tc_port_lock(dig_port, 1);
>  }
>  
>  void intel_tc_port_unlock(struct intel_digital_port *dig_port)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> +	intel_wakeref_t wakeref = fetch_and_zero(&dig_port->tc_lock_wakeref);
> +
> +	mutex_unlock(&dig_port->tc_lock);
> +
> +	intel_display_power_put_async(dev_priv, POWER_DOMAIN_DISPLAY_CORE,
> +				      wakeref);
> +}
> +
> +void intel_tc_port_get_link(struct intel_digital_port *dig_port,
> +			    int required_lanes)
> +{
> +	__intel_tc_port_lock(dig_port, required_lanes);
> +	dig_port->tc_link_refcount++;
> +	intel_tc_port_unlock(dig_port);
> +}
> +
> +void intel_tc_port_put_link(struct intel_digital_port *dig_port)
> +{
> +	mutex_lock(&dig_port->tc_lock);
> +	dig_port->tc_link_refcount--;
>  	mutex_unlock(&dig_port->tc_lock);
>  }
>  
> @@ -417,4 +461,5 @@ intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy)
>  {
>  	mutex_init(&dig_port->tc_lock);
>  	dig_port->tc_legacy_port = is_legacy;
> +	dig_port->tc_link_refcount = 0;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_tc.h b/drivers/gpu/drm/i915/intel_tc.h
> index 91c6e7459cc9..c1870acf6516 100644
> --- a/drivers/gpu/drm/i915/intel_tc.h
> +++ b/drivers/gpu/drm/i915/intel_tc.h
> @@ -14,6 +14,9 @@ int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port);
>  void intel_tc_port_sanitize(struct intel_digital_port *dig_port);
>  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_get_link(struct intel_digital_port *dig_port,
> +			    int required_lanes);
> +void intel_tc_port_put_link(struct intel_digital_port *dig_port);
>  
>  void intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy);
>  
> -- 
> 2.17.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux