| Message ID | 20240308131146.32714-3-stanislav.lisovskiy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Bigjoiner refactoring | expand |
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@intel.com> 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@linux.intel.com> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > Credits-to: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > 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 > @@ -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 >
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@intel.com> 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@linux.intel.com> > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > > Credits-to: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > 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 > >
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)) { @@ -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: