[Intel-gfx] [PATCH 13/13] drm/i915: Move the port sync DP_TP_CTL stuff to the encoder hook

Souza, Jose jose.souza at intel.com
Fri Apr 3 00:59:23 UTC 2020


On Fri, 2020-03-13 at 18:48 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Move the final DP_TP_CTL frobbing of port sync to the master
> encoder's enable hook. Now neatly out of sight from the high level
> modeset code.
> 
> And thus we've eliminated all the special casing of port sync
> in the high level modeset code.

Reviewed-by: José Roberto de Souza <jose.souza at intel.com>

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c     | 37 ++++++++
>  drivers/gpu/drm/i915/display/intel_display.c | 99 ++++------------
> ----
>  2 files changed, 53 insertions(+), 83 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 98475c81f1da..856c56f84833 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3547,6 +3547,41 @@ void intel_ddi_fdi_post_disable(struct
> intel_atomic_state *state,
>  	intel_de_write(dev_priv, FDI_RX_CTL(PIPE_A), val);
>  }
>  
> +static void trans_port_sync_stop_link_train(struct
> intel_atomic_state *state,
> +					    struct intel_encoder
> *encoder,
> +					    const struct
> intel_crtc_state *crtc_state)
> +{
> +	const struct drm_connector_state *conn_state;
> +	struct drm_connector *conn;
> +	int i;
> +
> +	if (!crtc_state->sync_mode_slaves_mask)
> +		return;
> +
> +	for_each_new_connector_in_state(&state->base, conn, conn_state,
> i) {
> +		struct intel_encoder *slave_encoder =
> +			to_intel_encoder(conn_state->best_encoder);
> +		struct intel_crtc *slave_crtc =
> to_intel_crtc(conn_state->crtc);
> +		const struct intel_crtc_state *slave_crtc_state;
> +
> +		if (!slave_crtc)
> +			continue;
> +
> +		slave_crtc_state =
> +			intel_atomic_get_new_crtc_state(state,
> slave_crtc);
> +
> +		if (slave_crtc_state->master_transcoder !=
> +		    crtc_state->cpu_transcoder)
> +			continue;
> +
> +		intel_dp_stop_link_train(enc_to_intel_dp(slave_encoder)
> );
> +	}
> +
> +	usleep_range(200, 400);
> +
> +	intel_dp_stop_link_train(enc_to_intel_dp(encoder));
> +}
> +
>  static void intel_enable_ddi_dp(struct intel_atomic_state *state,
>  				struct intel_encoder *encoder,
>  				const struct intel_crtc_state
> *crtc_state,
> @@ -3567,6 +3602,8 @@ static void intel_enable_ddi_dp(struct
> intel_atomic_state *state,
>  
>  	if (crtc_state->has_audio)
>  		intel_audio_codec_enable(encoder, crtc_state,
> conn_state);
> +
> +	trans_port_sync_stop_link_train(state, encoder, crtc_state);
>  }
>  
>  static i915_reg_t
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 84e59f6ab8e4..cdae7a680e4a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -544,19 +544,25 @@ needs_modeset(const struct intel_crtc_state
> *state)
>  	return drm_atomic_crtc_needs_modeset(&state->uapi);
>  }
>  
> -bool
> -is_trans_port_sync_mode(const struct intel_crtc_state *crtc_state)
> -{
> -	return (crtc_state->master_transcoder != INVALID_TRANSCODER ||
> -		crtc_state->sync_mode_slaves_mask);
> -}
> -
>  static bool
>  is_trans_port_sync_slave(const struct intel_crtc_state *crtc_state)
>  {
>  	return crtc_state->master_transcoder != INVALID_TRANSCODER;
>  }
>  
> +static bool
> +is_trans_port_sync_master(const struct intel_crtc_state *crtc_state)
> +{
> +	return crtc_state->sync_mode_slaves_mask != 0;
> +}
> +
> +bool
> +is_trans_port_sync_mode(const struct intel_crtc_state *crtc_state)
> +{
> +	return is_trans_port_sync_master(crtc_state) ||
> +		is_trans_port_sync_slave(crtc_state);
> +}
> +
>  /*
>   * Platform specific helpers to calculate the port PLL loopback-
> (clock.m),
>   * and post-divider (clock.p) values, pre- (clock.vco) and post-
> divided fast
> @@ -15104,63 +15110,6 @@ static void
> intel_commit_modeset_enables(struct intel_atomic_state *state)
>  	}
>  }
>  
> -static void intel_set_dp_tp_ctl_normal(struct intel_atomic_state
> *state,
> -				       struct intel_crtc *crtc)
> -{
> -	struct drm_connector *uninitialized_var(conn);
> -	struct drm_connector_state *conn_state;
> -	struct intel_dp *intel_dp;
> -	int i;
> -
> -	for_each_new_connector_in_state(&state->base, conn, conn_state,
> i) {
> -		if (conn_state->crtc == &crtc->base)
> -			break;
> -	}
> -	intel_dp = intel_attached_dp(to_intel_connector(conn));
> -	intel_dp_stop_link_train(intel_dp);
> -}
> -
> -static void intel_update_trans_port_sync_crtcs(struct
> intel_atomic_state *state,
> -					       struct intel_crtc *crtc)
> -{
> -	struct drm_i915_private *i915 = to_i915(state->base.dev);
> -	const struct intel_crtc_state *new_slave_crtc_state;
> -	const struct intel_crtc_state *new_crtc_state;
> -	struct intel_crtc *slave_crtc;
> -	int i;
> -
> -	for_each_new_intel_crtc_in_state(state, slave_crtc,
> -					 new_slave_crtc_state, i) {
> -		if (new_slave_crtc_state->master_transcoder !=
> -		    new_crtc_state->cpu_transcoder)
> -			continue;
> -
> -		drm_dbg_kms(&i915->drm,
> -			    "Updating transcoder port sync slave
> [CRTC:%d:%s]\n",
> -			    slave_crtc->base.base.id, slave_crtc-
> >base.name);
> -
> -		intel_enable_crtc(state, slave_crtc);
> -	}
> -
> -	drm_dbg_kms(&i915->drm,
> -		    "Updating transcoder port sync master
> [CRTC:%d:%s]\n",
> -		    crtc->base.base.id, crtc->base.name);
> -
> -	intel_enable_crtc(state, crtc);
> -
> -	for_each_new_intel_crtc_in_state(state, slave_crtc,
> -					 new_slave_crtc_state, i) {
> -		if (new_slave_crtc_state->master_transcoder !=
> -		    new_crtc_state->cpu_transcoder)
> -			continue;
> -
> -		intel_set_dp_tp_ctl_normal(state, slave_crtc);
> -	}
> -
> -	usleep_range(200, 400);
> -	intel_set_dp_tp_ctl_normal(state, crtc);
> -}
> -
>  static void icl_dbuf_slice_pre_update(struct intel_atomic_state
> *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> @@ -15261,33 +15210,17 @@ static void
> skl_commit_modeset_enables(struct intel_atomic_state *state)
>  			continue;
>  
>  		if (intel_dp_mst_is_slave_trans(new_crtc_state) ||
> -		    is_trans_port_sync_slave(new_crtc_state))
> +		    is_trans_port_sync_master(new_crtc_state))
>  			continue;
>  
>  		modeset_pipes &= ~BIT(pipe);
>  
> -		if (is_trans_port_sync_mode(new_crtc_state)) {
> -			const struct intel_crtc_state
> *new_slave_crtc_state;
> -			struct intel_crtc *slave_crtc;
> -			int i;
> -
> -			intel_update_trans_port_sync_crtcs(state,
> crtc);
> -
> -			for_each_new_intel_crtc_in_state(state,
> slave_crtc,
> -							 new_slave_crtc
> _state, i) {
> -
> -				/* TODO: update entries[] of slave */
> -				modeset_pipes &= ~BIT(slave_crtc-
> >pipe);
> -			}
> -		} else {
> -			intel_enable_crtc(state, crtc);
> -		}
> +		intel_enable_crtc(state, crtc);
>  	}
>  
>  	/*
>  	 * Then we enable all remaining pipes that depend on other
> -	 * pipes, right now it is only MST slaves as both port sync
> -	 * slave and master are enabled together
> +	 * pipes: MST slaves and port sync masters.
>  	 */
>  	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state,
> i) {
>  		enum pipe pipe = crtc->pipe;


More information about the Intel-gfx mailing list