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 <daniel.vetter@xxxxxxxx>
> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> > Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
> > Signed-off-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> > ---
> > 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);
> > + 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(&state->base, conn, conn_state, i) {
> > + if (conn_state->crtc == &crtc->base)
> > + break;
> > + }
> > + intel_dp = enc_to_intel_dp(&intel_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,
> > + struct intel_crtc_state *old_crtc_state,
> > + struct intel_crtc_state *new_crtc_state)
> > +{
> > + struct intel_plane_state *new_plane_state =
> > + intel_atomic_get_new_plane_state(state,
> > + to_intel_plane(crtc->base.primary));
> > +
> > + if (new_crtc_state->update_pipe && !new_crtc_state->enable_fbc)
> > + intel_fbc_disable(crtc);
> > + else if (new_plane_state)
> > + intel_fbc_enable(crtc, new_crtc_state, new_plane_state);
> > +
> > + intel_begin_crtc_commit(state, crtc);
> > + skl_update_planes_on_crtc(state, crtc);
> > + intel_finish_crtc_commit(state, crtc);
> > +}
> > +
> > +static void intel_update_trans_port_sync_crtcs(struct intel_crtc *crtc,
> > + struct intel_atomic_state *state,
> > + struct intel_crtc_state *old_crtc_state,
> > + struct intel_crtc_state *new_crtc_state)
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> > + struct intel_crtc *slave_crtc = intel_get_slave_crtc(dev_priv,
> > + new_crtc_state);
> > + struct intel_crtc_state *new_slave_crtc_state =
> > + intel_atomic_get_new_crtc_state(state, slave_crtc);
> > + struct intel_crtc_state *old_slave_crtc_state =
> > + intel_atomic_get_old_crtc_state(state, slave_crtc);
> > +
> > + WARN_ON(!slave_crtc || !new_slave_crtc_state ||
> > + !old_slave_crtc_state);
> > +
> > + DRM_DEBUG_KMS("Updating Transcoder Port Sync Master CRTC = %d %s and Slave CRTC %d %s\n",
> > + crtc->base.base.id, crtc->base.name, slave_crtc->base.base.id,
> > + slave_crtc->base.name);
> > +
> > + /* Enable seq for slave with with DP_TP_CTL left Idle until the
> > + * master is ready
> > + */
> > + intel_crtc_enable_trans_port_sync(slave_crtc,
> > + state,
> > + new_slave_crtc_state);
> > +
> > + /* Enable seq for master with with DP_TP_CTL left Idle */
> > + intel_crtc_enable_trans_port_sync(crtc,
> > + state,
> > + new_crtc_state);
> > +
> > + /* Set Slave's DP_TP_CTL to Normal */
> > + intel_set_dp_tp_ctl_normal(slave_crtc,
> > + state);
> > +
> > + /* Set Master's DP_TP_CTL To Normal */
> > + usleep_range(200, 400);
> > + intel_set_dp_tp_ctl_normal(crtc,
> > + state);
> > +
> > + /* Now do the post crtc enable for all master and slaves */
> > + intel_post_crtc_enable_updates(slave_crtc,
> > + state,
> > + new_slave_crtc_state,
> > + old_slave_crtc_state);
> > + intel_post_crtc_enable_updates(crtc,
> > + state,
> > + new_crtc_state,
> > + old_crtc_state);
> > +}
> > +
> > static void skl_commit_modeset_enables(struct intel_atomic_state *state)
> > {
> > struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> > @@ -14040,6 +14162,7 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
> > for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> > bool vbl_wait = false;
> > unsigned int cmask = drm_crtc_mask(&crtc->base);
> > + bool modeset = needs_modeset(new_crtc_state);
> >
> > pipe = crtc->pipe;
> >
> > @@ -14062,12 +14185,24 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
> > */
> > if (!skl_ddb_entry_equal(&new_crtc_state->wm.skl.ddb,
> > &old_crtc_state->wm.skl.ddb) &&
> > - !new_crtc_state->base.active_changed &&
> > + !modeset &&
> > state->wm_results.dirty_pipes != updated)
> > vbl_wait = true;
> >
> > - intel_update_crtc(crtc, state, old_crtc_state,
> > - new_crtc_state);
> > + if (modeset && is_trans_port_sync_mode(dev_priv,
> > + new_crtc_state)) {
> > + if (is_trans_port_sync_master(dev_priv,
> > + new_crtc_state))
> > + intel_update_trans_port_sync_crtcs(crtc,
> > + state,
> > + old_crtc_state,
> > + new_crtc_state);
> > + else
> > + continue;
> > + } else {
> > + intel_update_crtc(crtc, state, old_crtc_state,
> > + new_crtc_state);
> > + }
> >
> > if (vbl_wait)
> > intel_wait_for_vblank(dev_priv, pipe);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> > index 1623face436b..efa4d62514ce 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > @@ -27,6 +27,7 @@
> >
> > #include <drm/drm_util.h>
> > #include <drm/i915_drm.h>
> > +#include "intel_dp_link_training.h"
> >
> > enum link_m_n_set;
> > struct dpll;
> > @@ -54,6 +55,7 @@ struct intel_plane;
> > struct intel_plane_state;
> > struct intel_remapped_info;
> > struct intel_rotation_info;
> > +struct intel_crtc_state;
> >
> > enum i915_gpio {
> > GPIOA,
> > --
> > 2.19.1
>
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
