On Thu, Jun 20, 2019 at 05:05:51PM +0300, Imre Deak wrote: > We must keep the TypeC port mode fixed for the duration of the connector > detection and each AUX transfers. Add a new TypeC lock holding it around > these two sequences. For consistency also hold the lock during the port > mode sanitization. > > Whenever resetting the port mode (only during the detection for now) the > port's AUX power domain must be disabled already. Flush the async power > domain disabling work to ensure this. > > A follow-up patch will make the port mode changing more robust by > postponing the change for active ports. > > v2: > - Fix checkpatch issue: missing annotation for tc_lock. > > Cc: José Roberto de Souza <[email protected]> > Cc: Rodrigo Vivi <[email protected]> > Cc: Paulo Zanoni <[email protected]> > Cc: Ville Syrjälä <[email protected]> > Signed-off-by: Imre Deak <[email protected]>
lgtm. Reviewed-by: Ville Syrjälä <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 7 ++++++ > drivers/gpu/drm/i915/display/intel_tc.c | 30 ++++++++++++++++++++++++- > drivers/gpu/drm/i915/display/intel_tc.h | 2 ++ > drivers/gpu/drm/i915/intel_drv.h | 1 + > 4 files changed, 39 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index 0c6afec78f93..8f7188d71d08 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -1192,6 +1192,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, > struct drm_i915_private *i915 = > to_i915(intel_dig_port->base.base.dev); > struct intel_uncore *uncore = &i915->uncore; > + bool is_tc_port = intel_port_is_tc(i915, intel_dig_port->base.port); > i915_reg_t ch_ctl, ch_data[5]; > u32 aux_clock_divider; > enum intel_display_power_domain aux_domain = > @@ -1207,6 +1208,9 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, > for (i = 0; i < ARRAY_SIZE(ch_data); i++) > ch_data[i] = intel_dp->aux_ch_data_reg(intel_dp, i); > > + if (is_tc_port) > + intel_tc_port_lock(intel_dig_port); > + > aux_wakeref = intel_display_power_get(i915, aux_domain); > pps_wakeref = pps_lock(intel_dp); > > @@ -1359,6 +1363,9 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, > pps_unlock(intel_dp, pps_wakeref); > intel_display_power_put_async(i915, aux_domain, aux_wakeref); > > + if (is_tc_port) > + intel_tc_port_unlock(intel_dig_port); > + > return ret; > } > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c > b/drivers/gpu/drm/i915/display/intel_tc.c > index 81b0d5676108..019bc53af28b 100644 > --- a/drivers/gpu/drm/i915/display/intel_tc.c > +++ b/drivers/gpu/drm/i915/display/intel_tc.c > @@ -296,8 +296,11 @@ intel_tc_port_get_target_mode(struct intel_digital_port > *dig_port) > > static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port) > { > + struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev); > enum tc_port_mode old_tc_mode = dig_port->tc_mode; > > + intel_display_power_flush_work(dev_priv); > + > icl_tc_phy_disconnect(dig_port); > icl_tc_phy_connect(dig_port); > > @@ -312,6 +315,8 @@ void intel_tc_port_sanitize(struct intel_digital_port > *dig_port) > struct intel_encoder *encoder = &dig_port->base; > int active_links = 0; > > + mutex_lock(&dig_port->tc_lock); > + > dig_port->tc_mode = intel_tc_port_get_current_mode(dig_port); > if (dig_port->dp.is_mst) > active_links = intel_dp_mst_encoder_active_links(dig_port); > @@ -332,6 +337,8 @@ void intel_tc_port_sanitize(struct intel_digital_port > *dig_port) > DRM_DEBUG_KMS("Port %s: sanitize mode (%s)\n", > dig_port->tc_port_name, > tc_port_mode_name(dig_port->tc_mode)); > + > + mutex_unlock(&dig_port->tc_lock); > } > > static bool intel_tc_port_needs_reset(struct intel_digital_port *dig_port) > @@ -351,10 +358,30 @@ static bool intel_tc_port_needs_reset(struct > intel_digital_port *dig_port) > */ > 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); > > - return tc_port_live_status_mask(dig_port) & BIT(dig_port->tc_mode); > + is_connected = tc_port_live_status_mask(dig_port) & > + BIT(dig_port->tc_mode); > + > + mutex_unlock(&dig_port->tc_lock); > + > + return is_connected; > +} > + > +void intel_tc_port_lock(struct intel_digital_port *dig_port) > +{ > + mutex_lock(&dig_port->tc_lock); > + /* TODO: reset the TypeC port mode if needed */ > +} > + > +void intel_tc_port_unlock(struct intel_digital_port *dig_port) > +{ > + mutex_unlock(&dig_port->tc_lock); > } > > void intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy) > @@ -369,5 +396,6 @@ void intel_tc_port_init(struct intel_digital_port > *dig_port, bool is_legacy) > snprintf(dig_port->tc_port_name, sizeof(dig_port->tc_port_name), > "%c/TC#%d", port_name(port), tc_port + 1); > > + mutex_init(&dig_port->tc_lock); > dig_port->tc_legacy_port = is_legacy; > } > diff --git a/drivers/gpu/drm/i915/display/intel_tc.h > b/drivers/gpu/drm/i915/display/intel_tc.h > index 5a7876a74522..b5af2fe60b22 100644 > --- a/drivers/gpu/drm/i915/display/intel_tc.h > +++ b/drivers/gpu/drm/i915/display/intel_tc.h > @@ -17,6 +17,8 @@ u32 intel_tc_port_get_lane_mask(struct intel_digital_port > *dig_port); > 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_init(struct intel_digital_port *dig_port, bool is_legacy); > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 19f6a360acde..d9e7d011ed4a 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1224,6 +1224,7 @@ struct intel_digital_port { > /* Used for DP and ICL+ TypeC/DP and TypeC/HDMI ports. */ > enum aux_ch aux_ch; > enum intel_display_power_domain ddi_io_power_domain; > + struct mutex tc_lock; /* protects the TypeC port mode */ > bool tc_legacy_port:1; > char tc_port_name[8]; > enum tc_port_mode tc_mode; > -- > 2.17.1 -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
