Re: [PATCH 2/6] drm/i915: Extract intel_ddi_post_disable_hdmi_or_sst()

From: "Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>
To: Manasi Navare <navaremanasi@chromium.org>
Cc: intel-gfx@lists.freedesktop.org, jani.saarinen@intel.com,
	ville.syrjala@linux.intel.com, vidya.srinivas@intel.com
Subject: Re: [PATCH 2/6] drm/i915: Extract intel_ddi_post_disable_hdmi_or_sst()
Date: Wed, 13 Mar 2024 10:42:45 +0200	[thread overview]
Message-ID: <ZfFnBbBoRuPis5qT@intel.com> (raw)
In-Reply-To: <CAE72mNmBWeCNig3p0y3-dAcDjvy_XELCxynYbsqP3yEMGzYkoA@mail.gmail.com>

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
> >