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

2020-04-02 Thread Souza, Jose
On Fri, 2020-03-13 at 18:48 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> 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 

> 
> Signed-off-by: Ville Syrjälä 
> ---
>  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(>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(>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(>base, conn, conn_state,
> i) {
> - if (conn_state->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-gfx] [PATCH 13/13] drm/i915: Move the port sync DP_TP_CTL stuff to the encoder hook

2020-03-13 Thread Ville Syrjala
From: Ville Syrjälä 

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.

Signed-off-by: Ville Syrjälä 
---
 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(>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(>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(>base, conn, conn_state, i) {
-   if (conn_state->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;
-
-