On Sun, Sep 22, 2019 at 10:08:02AM -0700, Manasi Navare wrote: > In case of tiled displays when the two tiles are sent across two CRTCs > over two separate DP SST connectors, we need a mechanism to synchronize > the two CRTCs and their corresponding transcoders. > So use the master-slave mode where there is one master corresponding > to last horizontal and vertical tile that needs to be genlocked with > all other slave tiles. > This patch identifies saves the master transcoder in all the slave > CRTC states. This is needed to select the master CRTC/transcoder > while configuring transcoder port sync for the corresponding slaves. > > v4: > * Rebase > v3: > * Use master_tramscoder instead of master_crtc for valid > HW state readouts (Ville) > v2: > * Move this to intel_mode_set_pipe_config(Jani N, Ville) > * Use slave_bitmask to save associated slaves in master crtc state (Ville) > > Cc: Daniel Vetter <daniel.vet...@ffwll.ch> > Cc: Ville Syrjälä <ville.syrj...@linux.intel.com> > Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com> > Cc: Matt Roper <matthew.d.ro...@intel.com> > Signed-off-by: Manasi Navare <manasi.d.nav...@intel.com> > Reviewed-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 123 ++++++++++++++++++ > drivers/gpu/drm/i915/display/intel_display.h | 3 + > .../drm/i915/display/intel_display_types.h | 6 + > 3 files changed, 132 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index c05ba6af6226..4ff375d5852d 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -521,6 +521,24 @@ needs_modeset(const struct intel_crtc_state *state) > return drm_atomic_crtc_needs_modeset(&state->base); > } > > +bool > +is_trans_port_sync_mode(struct drm_i915_private *dev_priv,
Redundant function parameter. Can be derived if needed. > + const struct intel_crtc_state *state) 'crtc_state' > +{ > + return (INTEL_GEN(dev_priv) >= 11 && I don't think we need a gen check at all. The state should not have master/slaves set if the feature is not supported. > + (state->master_transcoder != INVALID_TRANSCODER || > + state->sync_mode_slaves_mask)); > +} > + > +static bool > +is_trans_port_sync_master(struct drm_i915_private *dev_priv, > + const struct intel_crtc_state *state) > +{ > + return (INTEL_GEN(dev_priv) >= 11 && > + (state->master_transcoder == INVALID_TRANSCODER && > + state->sync_mode_slaves_mask)); > +} > + > /* > * Platform specific helpers to calculate the port PLL loopback- (clock.m), > * and post-divider (clock.p) values, pre- (clock.vco) and post-divided fast > @@ -11773,6 +11791,91 @@ static bool c8_planes_changed(const struct > intel_crtc_state *new_crtc_state) > return !old_crtc_state->c8_planes != !new_crtc_state->c8_planes; > } > > +static int icl_add_sync_mode_crtcs(struct drm_crtc *crtc, intel_ types all over please. Also don't need all three funciton arguments. Either just crtc_state or state+crtc will do. > + struct intel_crtc_state *crtc_state, > + struct drm_atomic_state *state) > +{ > + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); > + struct drm_connector *master_connector, *connector; > + struct drm_connector_state *connector_state; > + struct drm_connector_list_iter conn_iter; > + struct drm_crtc *master_crtc = NULL; > + struct drm_crtc_state *master_crtc_state; > + struct intel_crtc_state *master_pipe_config; > + int i, tile_group_id; > + > + if (INTEL_GEN(dev_priv) < 11) > + return 0; > + > + /* > + * In case of tiled displays there could be one or more slaves but > there is > + * only one master. Lets make the CRTC used by the connector > corresponding > + * to the last horizonal and last vertical tile a master/genlock CRTC. > + * All the other CRTCs corresponding to other tiles of the same Tile > group > + * are the slave CRTCs and hold a pointer to their genlock CRTC. > + */ > + for_each_new_connector_in_state(state, connector, connector_state, i) { > + if (connector_state->crtc != crtc) > + continue; > + if (!connector->has_tile) > + continue; > + if (crtc_state->base.mode.hdisplay != connector->tile_h_size || > + crtc_state->base.mode.vdisplay != connector->tile_v_size) > + return 0; > + if (connector->tile_h_loc == connector->num_h_tile - 1 && > + connector->tile_v_loc == connector->num_v_tile - 1) > + continue; > + crtc_state->sync_mode_slaves_mask = 0; > + tile_group_id = connector->tile_group->id; > + drm_connector_list_iter_begin(&dev_priv->drm, &conn_iter); > + drm_for_each_connector_iter(master_connector, &conn_iter) { > + struct drm_connector_state *master_conn_state = NULL; > + > + if (!master_connector->has_tile) > + continue; > + if (master_connector->tile_h_loc != > master_connector->num_h_tile - 1 || > + master_connector->tile_v_loc != > master_connector->num_v_tile - 1) > + continue; > + if (master_connector->tile_group->id != tile_group_id) > + continue; > + > + master_conn_state = > drm_atomic_get_connector_state(state, > + > master_connector); > + if (IS_ERR(master_conn_state)) { > + drm_connector_list_iter_end(&conn_iter); > + return PTR_ERR(master_conn_state); > + } > + if (master_conn_state->crtc) { > + master_crtc = master_conn_state->crtc; > + break; > + } > + } > + drm_connector_list_iter_end(&conn_iter); > + > + if (!master_crtc) { > + DRM_DEBUG_KMS("Could not find Master CRTC for Slave > CRTC %d\n", > + connector_state->crtc->base.id); > + return -EINVAL; > + } > + > + master_crtc_state = drm_atomic_get_crtc_state(state, > + master_crtc); > + if (IS_ERR(master_crtc_state)) > + return PTR_ERR(master_crtc_state); > + > + master_pipe_config = to_intel_crtc_state(master_crtc_state); > + crtc_state->master_transcoder = > master_pipe_config->cpu_transcoder; > + master_pipe_config->sync_mode_slaves_mask |= > + BIT(crtc_state->cpu_transcoder); > + DRM_DEBUG_KMS("Master Transcoder = %s added for Slave CRTC = > %d, slave transcoder bitmask = %d\n", > + transcoder_name(crtc_state->master_transcoder), > + crtc_state->base.crtc->base.id, > + master_pipe_config->sync_mode_slaves_mask); > + } > + > + return 0; > +} > + > static int intel_crtc_atomic_check(struct drm_crtc *_crtc, > struct drm_crtc_state *_crtc_state) > { > @@ -12276,6 +12379,12 @@ clear_intel_crtc_state(struct intel_crtc_state > *crtc_state) > if (IS_G4X(dev_priv) || > IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > saved_state->wm = crtc_state->wm; > + /* Save the slave bitmask which gets filled for master crtc state during > + * slave atomic check call. > + */ Wrong comment format. > + if (is_trans_port_sync_master(dev_priv, crtc_state)) > + saved_state->sync_mode_slaves_mask = > + crtc_state->sync_mode_slaves_mask; > > /* Keep base drm_crtc_state intact, only clear our extended struct */ > BUILD_BUG_ON(offsetof(struct intel_crtc_state, base)); > @@ -12369,6 +12478,15 @@ intel_modeset_pipe_config(struct intel_crtc_state > *pipe_config) > drm_mode_set_crtcinfo(&pipe_config->base.adjusted_mode, > CRTC_STEREO_DOUBLE); > > + /* Set the crtc_state defaults for trans_port_sync */ > + pipe_config->master_transcoder = INVALID_TRANSCODER; > + ret = icl_add_sync_mode_crtcs(crtc, pipe_config, state); > + if (ret) { > + DRM_DEBUG_KMS("Cannot assign Sync Mode CRTCs: %d\n", > + ret); > + return ret; > + } > + > /* Pass our mode to the connectors and the CRTC to give them a chance to > * adjust it according to limitations or connector properties, and also > * a chance to reject the mode entirely. > @@ -12882,6 +13000,11 @@ intel_pipe_config_compare(const struct > intel_crtc_state *current_config, > PIPE_CONF_CHECK_INFOFRAME(hdmi); > PIPE_CONF_CHECK_INFOFRAME(drm); > > + if (INTEL_GEN(dev_priv) >= 11) { Just do the check uncoditionally. > + PIPE_CONF_CHECK_I(sync_mode_slaves_mask); > + PIPE_CONF_CHECK_I(master_transcoder); > + } > + > #undef PIPE_CONF_CHECK_X > #undef PIPE_CONF_CHECK_I > #undef PIPE_CONF_CHECK_BOOL > diff --git a/drivers/gpu/drm/i915/display/intel_display.h > b/drivers/gpu/drm/i915/display/intel_display.h > index b1ae0e59c715..1623face436b 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.h > +++ b/drivers/gpu/drm/i915/display/intel_display.h > @@ -93,6 +93,7 @@ enum pipe { > #define pipe_name(p) ((p) + 'A') > > enum transcoder { > + INVALID_TRANSCODER = -1, > /* > * The following transcoders have a 1:1 transcoder -> pipe mapping, > * keep their values fixed: the code assumes that TRANSCODER_A=0, the > @@ -453,6 +454,8 @@ enum drm_mode_status > intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv, > const struct drm_display_mode *mode); > enum phy intel_port_to_phy(struct drm_i915_private *i915, enum port port); > +bool is_trans_port_sync_mode(struct drm_i915_private *i915, > + const struct intel_crtc_state *state); > > void intel_plane_destroy(struct drm_plane *plane); > void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe); > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > b/drivers/gpu/drm/i915/display/intel_display_types.h > index d5cc4b810d9e..17ff34ca298b 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -991,6 +991,12 @@ struct intel_crtc_state { > > /* Forward Error correction State */ > bool fec_enable; > + > + /* Pointer to master transcoder in case of tiled displays */ > + enum transcoder master_transcoder; > + > + /* Bitmask to indicate slaves attached */ > + u8 sync_mode_slaves_mask; > }; > > struct intel_crtc { > -- > 2.19.1 -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx