This will not work for MST, here a example Previous state: MST master pipe A MST slave pipe B New state: Pipe A being disabled On drm_atomic_helper_check_modeset() both intel_crtc_states will be added to the state with crtc_state->uapi.mode_changed=true. Then on the regular for_each_oldnew_intel_crtc_in_state() loop config of CRTC B will have mst_master_transcoder=INVALID_TRANSCODER that differs from TRANSCODER_A and will keep mode_changed set. Then on the config_late loop it will skip the interation on the needs_modeset() check keeping CRTC B with mst_master_transcoder=INVALID_TRANSCODER. That would be caugh by CI if this tests were merged and the TGL machine with MST is still on: https://patchwork.freedesktop.org/series/72211/ On Fri, 2020-03-13 at 18:48 +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Use the recently introduced encoder .compute_config_late() hook to > do the MST master transcoder assignment. Avoids having to do it > in a funny way before we know the CPU transcoder of each pipe. > > And now we can also properly use hw.active instead of uapi.active > since it too has been calculated earlier for everyone. > > Cc: José Roberto de Souza <jose.souza@xxxxxxxxx> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 98 +++++++++++------ > ---- > 1 file changed, 51 insertions(+), 47 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index 44f3fd251ca1..b9afc1135b9b 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -88,56 +88,10 @@ static int > intel_dp_mst_compute_link_config(struct intel_encoder *encoder, > return 0; > } > > -/* > - * Iterate over all connectors and return the smallest transcoder in > the MST > - * stream > - */ > -static enum transcoder > -intel_dp_mst_master_trans_compute(struct intel_atomic_state *state, > - struct intel_dp *mst_port) > -{ > - struct drm_i915_private *dev_priv = to_i915(state->base.dev); > - struct intel_digital_connector_state *conn_state; > - struct intel_connector *connector; > - enum pipe ret = I915_MAX_PIPES; > - int i; > - > - if (INTEL_GEN(dev_priv) < 12) > - return INVALID_TRANSCODER; > - > - for_each_new_intel_connector_in_state(state, connector, > conn_state, i) { > - struct intel_crtc_state *crtc_state; > - struct intel_crtc *crtc; > - > - if (connector->mst_port != mst_port || !conn_state- > >base.crtc) > - continue; > - > - crtc = to_intel_crtc(conn_state->base.crtc); > - crtc_state = intel_atomic_get_new_crtc_state(state, > crtc); > - if (!crtc_state->uapi.active) > - continue; > - > - /* > - * Using crtc->pipe because crtc_state->cpu_transcoder > is > - * computed, so others CRTCs could have non-computed > - * cpu_transcoder > - */ > - if (crtc->pipe < ret) > - ret = crtc->pipe; > - } > - > - if (ret == I915_MAX_PIPES) > - return INVALID_TRANSCODER; > - > - /* Simple cast works because TGL don't have a eDP transcoder */ > - return (enum transcoder)ret; > -} > - > static int intel_dp_mst_compute_config(struct intel_encoder > *encoder, > struct intel_crtc_state > *pipe_config, > struct drm_connector_state > *conn_state) > { > - struct intel_atomic_state *state = > to_intel_atomic_state(conn_state->state); > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder); > struct intel_dp *intel_dp = &intel_mst->primary->dp; > @@ -201,7 +155,56 @@ static int intel_dp_mst_compute_config(struct > intel_encoder *encoder, > > intel_ddi_compute_min_voltage_level(dev_priv, pipe_config); > > - pipe_config->mst_master_transcoder = > intel_dp_mst_master_trans_compute(state, intel_dp); > + return 0; > +} > + > +/* > + * Iterate over all connectors and return a mask of > + * all CPU transcoders streaming over the same DP link. > + */ > +static unsigned int > +intel_dp_mst_transcoder_mask(struct intel_atomic_state *state, > + struct intel_dp *mst_port) > +{ > + struct drm_i915_private *dev_priv = to_i915(state->base.dev); > + const struct intel_digital_connector_state *conn_state; > + struct intel_connector *connector; > + u8 transcoders = 0; > + int i; > + > + if (INTEL_GEN(dev_priv) < 12) > + return 0; > + > + for_each_new_intel_connector_in_state(state, connector, > conn_state, i) { > + const struct intel_crtc_state *crtc_state; > + struct intel_crtc *crtc; > + > + if (connector->mst_port != mst_port || !conn_state- > >base.crtc) > + continue; > + > + crtc = to_intel_crtc(conn_state->base.crtc); > + crtc_state = intel_atomic_get_new_crtc_state(state, > crtc); > + > + if (!crtc_state->hw.active) > + continue; > + > + transcoders |= BIT(crtc_state->cpu_transcoder); > + } > + > + return transcoders; > +} > + > +static int intel_dp_mst_compute_config_late(struct intel_encoder > *encoder, > + struct intel_crtc_state > *crtc_state, > + struct drm_connector_state > *conn_state) > +{ > + struct intel_atomic_state *state = > to_intel_atomic_state(conn_state->state); > + struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder); > + struct intel_dp *intel_dp = &intel_mst->primary->dp; > + > + /* lowest numbered transcoder will be designated master */ > + crtc_state->mst_master_transcoder = > + ffs(intel_dp_mst_transcoder_mask(state, intel_dp)) - 1; > > return 0; > } > @@ -786,6 +789,7 @@ intel_dp_create_fake_mst_encoder(struct > intel_digital_port *intel_dig_port, enum > intel_encoder->pipe_mask = ~0; > > intel_encoder->compute_config = intel_dp_mst_compute_config; > + intel_encoder->compute_config_late = > intel_dp_mst_compute_config_late; > intel_encoder->disable = intel_mst_disable_dp; > intel_encoder->post_disable = intel_mst_post_disable_dp; > intel_encoder->pre_pll_enable = intel_mst_pre_pll_enable_dp; _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
