[Intel-gfx] [PATCH v2 14/23] drm/i915: Keep the TypeC port mode fixed for detect/AUX transfers

Tue Jun 25 13:43:16 UTC 2019

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 at intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak at intel.com>

lgtm.

Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

> ---
>  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