On Thu, Mar 28, 2019 at 11:32:18AM +0200, Jani Nikula wrote: > On Thu, 21 Mar 2019, Manasi Navare <manasi.d.navare@xxxxxxxxx> 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 CRTC pointer in all the slave > > CRTC states. This pointer is needed to select the master CRTC/transcoder > > while configuring transcoder port sync for the corresponding slaves. > > > > 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> > > --- > > drivers/gpu/drm/i915/intel_display.c | 84 ++++++++++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_drv.h | 3 + > > 2 files changed, 87 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 8ff7aa8cb3cf..9980a4ed8c9c 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -11281,6 +11281,86 @@ static int icl_check_nv12_planes(struct intel_crtc_state *crtc_state) > > return 0; > > } > > > > +static int icl_add_genlock_crtcs(struct drm_device *dev, > > + struct drm_atomic_state *state) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(dev); > > + struct drm_connector *genlock_connector, *connector; > > + struct drm_connector_state *connector_state; > > + struct drm_connector_list_iter conn_iter; > > + struct drm_crtc *genlock_crtc = NULL; > > + struct drm_crtc_state *genlock_crtc_state; > > + struct intel_crtc_state *slave_crtc_state; > > + 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) > > + continue; > > + if (!connector->has_tile) > > + continue; > > + if (connector->tile_h_loc == connector->num_h_tile - 1 && > > + connector->tile_v_loc == connector->num_v_tile - 1) > > + continue; > > + slave_crtc_state = to_intel_crtc_state( > > + drm_atomic_get_new_crtc_state(state, > > + connector_state->crtc)); > > + slave_crtc_state->genlock_crtc = NULL; > > + tile_group_id = connector->tile_group->id; > > + drm_connector_list_iter_begin(dev, &conn_iter); > > + drm_for_each_connector_iter(genlock_connector, &conn_iter) { > > + struct drm_connector_state *genlock_conn_state = NULL; > > + > > + if (!genlock_connector->has_tile) > > + continue; > > + if (genlock_connector->tile_h_loc != genlock_connector->num_h_tile - 1 || > > + genlock_connector->tile_v_loc != genlock_connector->num_v_tile - 1) > > + continue; > > + if (genlock_connector->tile_group->id != tile_group_id) > > + continue; > > + > > + genlock_conn_state = drm_atomic_get_connector_state(state, > > + genlock_connector); > > + if (IS_ERR(genlock_conn_state)) { > > + drm_connector_list_iter_end(&conn_iter); > > + return PTR_ERR(genlock_conn_state); > > + } > > + if (genlock_conn_state->crtc) { > > + genlock_crtc = genlock_conn_state->crtc; > > + break; > > + } > > + } > > + drm_connector_list_iter_end(&conn_iter); > > The above loop would benefit from being abstracted to a separate > function. "find genlock master based on tile info" > > I wonder if it would work to have each relevant encoder ->compute_config > hook look for its genlock master instead of adding another top level > loop. So are you suggesting adding this directly inside say compute_config() hook for DP encoder so inside intel_dp_compute_config(), or would it be better to add just the inner find_genlock_master() function in the existing for_each_connector() in intel_modeset_pipe_config() before calling into the compute_config encoder hook? Regards Manasi > > BR, > Jani. > > > > + > > + if (!genlock_crtc) { > > + DRM_DEBUG_KMS("Could not add Genlock CRTC for Slave CRTC %d\n", > > + connector_state->crtc->base.id); > > + return -EINVAL; > > + } > > + > > + genlock_crtc_state = drm_atomic_get_crtc_state(state, > > + genlock_crtc); > > + if (IS_ERR(genlock_crtc_state)) > > + return PTR_ERR(genlock_crtc_state); > > + > > + slave_crtc_state->genlock_crtc = to_intel_crtc(genlock_crtc); > > + DRM_DEBUG_KMS("Genlock CRTC = %d added for Slave CRTC = %d\n", > > + genlock_crtc->base.id, > > + slave_crtc_state->base.crtc->base.id); > > + } > > + > > + return 0; > > +} > > + > > static int intel_crtc_atomic_check(struct drm_crtc *crtc, > > struct drm_crtc_state *crtc_state) > > { > > @@ -13098,6 +13178,10 @@ static int intel_atomic_check(struct drm_device *dev, > > if (ret) > > return ret; > > > > + ret = icl_add_genlock_crtcs(dev, state); > > + if (ret) > > + return ret; > > + > > for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, crtc_state, i) { > > struct intel_crtc_state *pipe_config = > > to_intel_crtc_state(crtc_state); > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 4d7ae579fc92..b41728946b73 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1074,6 +1074,9 @@ struct intel_crtc_state { > > > > /* Forward Error correction State */ > > bool fec_enable; > > + > > + /* Pointer to master/genlock crtc in case of tiled displays */ > > + struct intel_crtc *genlock_crtc; > > }; > > > > struct intel_crtc { > > -- > Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx