Re: [Intel-gfx] [PATCH v3 4/6] drm/i915/display/icl: Enable master-slaves in trans port sync
On Mon, Sep 30, 2019 at 06:28:16PM +0300, Ville Syrjälä wrote:
> On Sun, Sep 22, 2019 at 10:08:05AM -0700, Manasi Navare wrote:
> > As per the display enable sequence, we need to follow the enable sequence
> > for slaves first with DP_TP_CTL set to Idle and configure the transcoder
> > port sync register to select the corersponding master, then follow the
> > enable sequence for master leaving DP_TP_CTL to idle.
> > At this point the transcoder port sync mode is configured and enabled
> > and the Vblanks of both ports are synchronized so then set DP_TP_CTL
> > for the slave and master to Normal and do post crtc enable updates.
> >
> > v6:
> > * Modeset implies active_changed, remove one condition (Maarten)
> > v5:
> > * Fix checkpatch warning (Manasi)
> > v4:
> > * Reuse skl_commit_modeset_enables() hook (Maarten)
> > * Obtain slave crtc and states from master (Maarten)
> > v3:
> > * Rebase on drm-tip (Manasi)
> > v2:
> > * Create a icl_update_crtcs hook (Maarten, Danvet)
> > * This sequence only for CRTCs in trans port sync mode (Maarten)
> >
> > Cc: Daniel Vetter
> > Cc: Ville Syrjälä
> > Cc: Maarten Lankhorst
> > Cc: Matt Roper
> > Signed-off-by: Manasi Navare
> > Reviewed-by: Maarten Lankhorst
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 3 +-
> > drivers/gpu/drm/i915/display/intel_display.c | 141 ++-
> > drivers/gpu/drm/i915/display/intel_display.h | 2 +
> > 3 files changed, 142 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 3e6394139964..62e9f5602b6b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3347,7 +3347,8 @@ static void hsw_ddi_pre_enable_dp(struct
> > intel_encoder *encoder,
> > true);
> > intel_dp_sink_set_fec_ready(intel_dp, crtc_state);
> > intel_dp_start_link_train(intel_dp);
> > - if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
> > + if ((port != PORT_A || INTEL_GEN(dev_priv) >= 9) &&
> > + !is_trans_port_sync_mode(dev_priv, crtc_state))
> > intel_dp_stop_link_train(intel_dp);
> >
> > intel_ddi_enable_fec(encoder, crtc_state);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 711987eb4e9e..10425a789b5e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -13928,6 +13928,30 @@ static void intel_update_crtc(struct intel_crtc
> > *crtc,
> > intel_finish_crtc_commit(state, crtc);
> > }
> >
> > +static struct intel_crtc *intel_get_slave_crtc(struct drm_i915_private
> > *dev_priv,
>
> 'dev_priv' can be derived from the crtc state.
Ok yes will just pass the crtc_state
>
> > + struct intel_crtc_state
> > *new_crtc_state)
>
> const
>
> > +{
>
> cpu_transcoder = ffs(slaves) - 1;
Cool, thanks for pointing out this ffs function
> return get_crtc((enum pipe)cpu_transcoder);
>
> or something.
>
> Maybe toss in a WARN_ON(!is_power_of_2(slaves)).
Yes and that shd take care of ffs(slaves) == 0 as well
Manasi
>
>
>
> > + if (new_crtc_state->sync_mode_slaves_mask &
> > + BIT(TRANSCODER_A))
> > + return intel_get_crtc_for_pipe(dev_priv,
> > + PIPE_A);
> > + else if (new_crtc_state->sync_mode_slaves_mask &
> > +BIT(TRANSCODER_B))
> > + return intel_get_crtc_for_pipe(dev_priv,
> > + PIPE_B);
> > + else if (new_crtc_state->sync_mode_slaves_mask &
> > +BIT(TRANSCODER_C))
> > + return intel_get_crtc_for_pipe(dev_priv,
> > + PIPE_C);
> > + else if (new_crtc_state->sync_mode_slaves_mask &
> > +BIT(TRANSCODER_D))
> > + return intel_get_crtc_for_pipe(dev_priv,
> > + PIPE_D);
> > + /* should never happen */
> > + WARN_ON(1);
> > + return NULL;
> > +}
> > +
> > static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
> > struct intel_crtc_state
> > *old_crtc_state,
> > struct intel_crtc_state
> > *new_crtc_state,
> > @@ -14006,6 +14030,104 @@ static void intel_commit_modeset_enables(struct
> > intel_atomic_state *state)
> > }
> > }
> >
> > +static void intel_crtc_enable_trans_port_sync(struct intel_crtc *crtc,
> > + struct intel_atomic_state *state,
> > + struct intel_crtc_state
> > *new_crtc_state)
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> > +
> > + update_scanline_offset(new_crtc_state);
> > +
Re: [Intel-gfx] [PATCH v3 4/6] drm/i915/display/icl: Enable master-slaves in trans port sync
On Sun, Sep 22, 2019 at 10:08:05AM -0700, Manasi Navare wrote:
> As per the display enable sequence, we need to follow the enable sequence
> for slaves first with DP_TP_CTL set to Idle and configure the transcoder
> port sync register to select the corersponding master, then follow the
> enable sequence for master leaving DP_TP_CTL to idle.
> At this point the transcoder port sync mode is configured and enabled
> and the Vblanks of both ports are synchronized so then set DP_TP_CTL
> for the slave and master to Normal and do post crtc enable updates.
>
> v6:
> * Modeset implies active_changed, remove one condition (Maarten)
> v5:
> * Fix checkpatch warning (Manasi)
> v4:
> * Reuse skl_commit_modeset_enables() hook (Maarten)
> * Obtain slave crtc and states from master (Maarten)
> v3:
> * Rebase on drm-tip (Manasi)
> v2:
> * Create a icl_update_crtcs hook (Maarten, Danvet)
> * This sequence only for CRTCs in trans port sync mode (Maarten)
>
> Cc: Daniel Vetter
> Cc: Ville Syrjälä
> Cc: Maarten Lankhorst
> Cc: Matt Roper
> Signed-off-by: Manasi Navare
> Reviewed-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 3 +-
> drivers/gpu/drm/i915/display/intel_display.c | 141 ++-
> drivers/gpu/drm/i915/display/intel_display.h | 2 +
> 3 files changed, 142 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 3e6394139964..62e9f5602b6b 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3347,7 +3347,8 @@ static void hsw_ddi_pre_enable_dp(struct intel_encoder
> *encoder,
> true);
> intel_dp_sink_set_fec_ready(intel_dp, crtc_state);
> intel_dp_start_link_train(intel_dp);
> - if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
> + if ((port != PORT_A || INTEL_GEN(dev_priv) >= 9) &&
> + !is_trans_port_sync_mode(dev_priv, crtc_state))
> intel_dp_stop_link_train(intel_dp);
>
> intel_ddi_enable_fec(encoder, crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 711987eb4e9e..10425a789b5e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -13928,6 +13928,30 @@ static void intel_update_crtc(struct intel_crtc
> *crtc,
> intel_finish_crtc_commit(state, crtc);
> }
>
> +static struct intel_crtc *intel_get_slave_crtc(struct drm_i915_private
> *dev_priv,
'dev_priv' can be derived from the crtc state.
> +struct intel_crtc_state
> *new_crtc_state)
const
> +{
cpu_transcoder = ffs(slaves) - 1;
return get_crtc((enum pipe)cpu_transcoder);
or something.
Maybe toss in a WARN_ON(!is_power_of_2(slaves)).
> + if (new_crtc_state->sync_mode_slaves_mask &
> + BIT(TRANSCODER_A))
> + return intel_get_crtc_for_pipe(dev_priv,
> +PIPE_A);
> + else if (new_crtc_state->sync_mode_slaves_mask &
> + BIT(TRANSCODER_B))
> + return intel_get_crtc_for_pipe(dev_priv,
> +PIPE_B);
> + else if (new_crtc_state->sync_mode_slaves_mask &
> + BIT(TRANSCODER_C))
> + return intel_get_crtc_for_pipe(dev_priv,
> +PIPE_C);
> + else if (new_crtc_state->sync_mode_slaves_mask &
> + BIT(TRANSCODER_D))
> + return intel_get_crtc_for_pipe(dev_priv,
> +PIPE_D);
> + /* should never happen */
> + WARN_ON(1);
> + return NULL;
> +}
> +
> static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
> struct intel_crtc_state
> *old_crtc_state,
> struct intel_crtc_state
> *new_crtc_state,
> @@ -14006,6 +14030,104 @@ static void intel_commit_modeset_enables(struct
> intel_atomic_state *state)
> }
> }
>
> +static void intel_crtc_enable_trans_port_sync(struct intel_crtc *crtc,
> + struct intel_atomic_state *state,
> + struct intel_crtc_state
> *new_crtc_state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +
> + update_scanline_offset(new_crtc_state);
> + dev_priv->display.crtc_enable(new_crtc_state, state);
> + intel_crtc_enable_pipe_crc(crtc);
> +}
> +
> +static void intel_set_dp_tp_ctl_normal(struct intel_crtc *crtc,
> +struct intel_atomic_state *state)
> +{
> + struct drm_connector_state *conn_state;
> + struct drm_connector *conn;
> + struct intel_dp *intel_dp;
> + int i;
> +
> +
[Intel-gfx] [PATCH v3 4/6] drm/i915/display/icl: Enable master-slaves in trans port sync
As per the display enable sequence, we need to follow the enable sequence
for slaves first with DP_TP_CTL set to Idle and configure the transcoder
port sync register to select the corersponding master, then follow the
enable sequence for master leaving DP_TP_CTL to idle.
At this point the transcoder port sync mode is configured and enabled
and the Vblanks of both ports are synchronized so then set DP_TP_CTL
for the slave and master to Normal and do post crtc enable updates.
v6:
* Modeset implies active_changed, remove one condition (Maarten)
v5:
* Fix checkpatch warning (Manasi)
v4:
* Reuse skl_commit_modeset_enables() hook (Maarten)
* Obtain slave crtc and states from master (Maarten)
v3:
* Rebase on drm-tip (Manasi)
v2:
* Create a icl_update_crtcs hook (Maarten, Danvet)
* This sequence only for CRTCs in trans port sync mode (Maarten)
Cc: Daniel Vetter
Cc: Ville Syrjälä
Cc: Maarten Lankhorst
Cc: Matt Roper
Signed-off-by: Manasi Navare
Reviewed-by: Maarten Lankhorst
---
drivers/gpu/drm/i915/display/intel_ddi.c | 3 +-
drivers/gpu/drm/i915/display/intel_display.c | 141 ++-
drivers/gpu/drm/i915/display/intel_display.h | 2 +
3 files changed, 142 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
b/drivers/gpu/drm/i915/display/intel_ddi.c
index 3e6394139964..62e9f5602b6b 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3347,7 +3347,8 @@ static void hsw_ddi_pre_enable_dp(struct intel_encoder
*encoder,
true);
intel_dp_sink_set_fec_ready(intel_dp, crtc_state);
intel_dp_start_link_train(intel_dp);
- if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
+ if ((port != PORT_A || INTEL_GEN(dev_priv) >= 9) &&
+ !is_trans_port_sync_mode(dev_priv, crtc_state))
intel_dp_stop_link_train(intel_dp);
intel_ddi_enable_fec(encoder, crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 711987eb4e9e..10425a789b5e 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -13928,6 +13928,30 @@ static void intel_update_crtc(struct intel_crtc *crtc,
intel_finish_crtc_commit(state, crtc);
}
+static struct intel_crtc *intel_get_slave_crtc(struct drm_i915_private
*dev_priv,
+ struct intel_crtc_state
*new_crtc_state)
+{
+ if (new_crtc_state->sync_mode_slaves_mask &
+ BIT(TRANSCODER_A))
+ return intel_get_crtc_for_pipe(dev_priv,
+ PIPE_A);
+ else if (new_crtc_state->sync_mode_slaves_mask &
+BIT(TRANSCODER_B))
+ return intel_get_crtc_for_pipe(dev_priv,
+ PIPE_B);
+ else if (new_crtc_state->sync_mode_slaves_mask &
+BIT(TRANSCODER_C))
+ return intel_get_crtc_for_pipe(dev_priv,
+ PIPE_C);
+ else if (new_crtc_state->sync_mode_slaves_mask &
+BIT(TRANSCODER_D))
+ return intel_get_crtc_for_pipe(dev_priv,
+ PIPE_D);
+ /* should never happen */
+ WARN_ON(1);
+ return NULL;
+}
+
static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
struct intel_crtc_state
*old_crtc_state,
struct intel_crtc_state
*new_crtc_state,
@@ -14006,6 +14030,104 @@ static void intel_commit_modeset_enables(struct
intel_atomic_state *state)
}
}
+static void intel_crtc_enable_trans_port_sync(struct intel_crtc *crtc,
+ struct intel_atomic_state *state,
+ struct intel_crtc_state
*new_crtc_state)
+{
+ struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+
+ update_scanline_offset(new_crtc_state);
+ dev_priv->display.crtc_enable(new_crtc_state, state);
+ intel_crtc_enable_pipe_crc(crtc);
+}
+
+static void intel_set_dp_tp_ctl_normal(struct intel_crtc *crtc,
+ struct intel_atomic_state *state)
+{
+ struct drm_connector_state *conn_state;
+ struct drm_connector *conn;
+ 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 = enc_to_intel_dp(_attached_encoder(conn)->base);
+ intel_dp_stop_link_train(intel_dp);
+}
+
+static void intel_post_crtc_enable_updates(struct intel_crtc *crtc,
+ struct intel_atomic_state *state,
+
