On Tue, Mar 12, 2024 at 09:36:22PM -0700, Manasi Navare wrote: > Thanks Stan for the cleanup around post disable non MST case, one comment below > > On Fri, Mar 8, 2024 at 5:11 AM Stanislav Lisovskiy > <stanislav.lisovskiy@xxxxxxxxx> wrote: > > > > Extract the "not-MST" stuff from intel_ddi_post_disable() so that > > the whole thing isn't so cluttered. > > > > The bigjoiner slave handling was outside of the !MST check, > > but it really should have been inside it as its the counterpart > > to the master handling inside the check. So we pull that > > in as well. There is no functional change here as we don't > > currently support bigjoiner+MST anyway. > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx> > > Credits-to: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/display/intel_ddi.c | 37 +++++++++++++++--------- > > 1 file changed, 23 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > > index bbce74f011d40..5628a4ab608d4 100644 > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > @@ -3095,28 +3095,26 @@ static void intel_ddi_post_disable_hdmi(struct intel_atomic_state *state, > > intel_dp_dual_mode_set_tmds_output(intel_hdmi, false); > > } > > > > -static void intel_ddi_post_disable(struct intel_atomic_state *state, > > - struct intel_encoder *encoder, > > - const struct intel_crtc_state *old_crtc_state, > > - const struct drm_connector_state *old_conn_state) > > +static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state, > > + struct intel_encoder *encoder, > > + const struct intel_crtc_state *old_master_crtc_state, > > + const struct drm_connector_state *old_conn_state) > > { > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > struct intel_crtc *slave_crtc; > > > > - if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST)) { > > - intel_crtc_vblank_off(old_crtc_state); > > + intel_crtc_vblank_off(old_crtc_state); > > > > - intel_disable_transcoder(old_crtc_state); > > + intel_disable_transcoder(old_crtc_state); > > > > - intel_ddi_disable_transcoder_func(old_crtc_state); > > + intel_ddi_disable_transcoder_func(old_crtc_state); > > > > - intel_dsc_disable(old_crtc_state); > > + intel_dsc_disable(old_crtc_state); > > > > - if (DISPLAY_VER(dev_priv) >= 9) > > - skl_scaler_disable(old_crtc_state); > > - else > > - ilk_pfit_disable(old_crtc_state); > > - } > > + if (DISPLAY_VER(dev_priv) >= 9) > > + skl_scaler_disable(old_crtc_state); > > + else > > + ilk_pfit_disable(old_crtc_state); > > > > for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, slave_crtc, > > intel_crtc_bigjoiner_slave_pipes(old_crtc_state)) { > > This bigjoiner slave handling for MST path will be added later to the > intel_ddi_post_post_disable() > when we enable bigjoiner for MST? > > Manasi Hi Manasi, yes, currently I'm evaluating what would be the best way to do that. Stan > > > @@ -3128,6 +3126,17 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state, > > intel_dsc_disable(old_slave_crtc_state); > > skl_scaler_disable(old_slave_crtc_state); > > } > > +} > > + > > +static void intel_ddi_post_disable(struct intel_atomic_state *state, > > + struct intel_encoder *encoder, > > + const struct intel_crtc_state *old_crtc_state, > > + const struct drm_connector_state *old_conn_state) > > +{ > > + > > + if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST)) > > + intel_ddi_post_disable_hdmi_or_sst(state, encoder, > > + old_crtc_state, old_conn_state); > > > > /* > > * When called from DP MST code: > > -- > > 2.37.3 > >