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 <jose.souza@xxxxxxxxx>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
lgtm.
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> 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
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
