On Wed, Jun 19, 2019 at 03:58:46PM +0300, Ville Syrjälä wrote:
> 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 :(
Yep, it took a while to wrap my mind around the tracking of the MST
connector->encoder mapping. The only certainty here is that we have a
fixed encoder for all non-MST connectors and the primary encoder for MST
connectors. Things would simplify a lot even by keeping the above hooks
connector specific, but adding separate MST versions of them to deal
with the connector->primary encoder mapping, which would remove the need
for intel_connector_primary_digital_port().
I see now though, that it makes more sense to add these hooks to the
encoder, which also simplifies things a lot, so I chose that instead.
>
> 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.
Yep, this turned out to be simplifying things a lot (by adding a new
intel_connector_primary_encoder() helper instead):
https://github.com/ideak/linux/commit/49dc011132
This way it's enough to assign the hooks to the common DDI encoder,
instead of assigning them to each type of connectors used with a DDI
encoder.
Will post a v2 patchset with all the comments addressed after some
testing, an updated version is at:
https://github.com/ideak/linux/commits/typec-mode-switch
>
> 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?
Nope, it can be simplified.
>
> > +}
> > +
> > /*
> > * 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
