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.syrj...@linux.intel.com>
> Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Signed-off-by: Imre Deak <imre.d...@intel.com>
> ---
>  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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to