On Thu, Oct 31, 2024 at 12:44:38PM +0200, Luca Coelho wrote:
> On Wed, 2024-10-30 at 21:23 +0200, Imre Deak wrote:
> > On PTL during modeset enabling configure the DDI function without
> > enabling it for MST slave transcoders before programming the data and
> > link M/N values. The DDI function gets enabled separately later in the
> > transcoder enabling sequence.
> >
> > This fixes a slave transcoder getting stuck during enabling, leading
> > to page flip timeout errors on the corresponding pipe.
> >
> > The spec requires the same programming step for ADLP+ platforms, that
> > will be addressed separately (on those platforms the above transcoder
> > getting stuck issue was not observed).
> >
> > Bspec: 68849
> > Signed-off-by: Imre Deak <[email protected]>
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
> > drivers/gpu/drm/i915/display/intel_ddi.h | 2 ++
> > drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +++
> > 3 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 2bd14e2134be9..069cca4b38b2e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -619,7 +619,7 @@ void intel_ddi_enable_transcoder_func(struct
> > intel_encoder *encoder,
> > * Same as intel_ddi_enable_transcoder_func(), but it does not set the
> > enable
> > * bit.
> > */
> > -static void
> > +void
> > intel_ddi_config_transcoder_func(struct intel_encoder *encoder,
> > const struct intel_crtc_state *crtc_state)
>
> The void fits on the same line, and I think it looks better?
Ok, will change that.
> > {
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.h
> > b/drivers/gpu/drm/i915/display/intel_ddi.h
> > index 6d85422bdefef..1aa2e3a190aee 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.h
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.h
> > @@ -57,6 +57,8 @@ void intel_wait_ddi_buf_idle(struct drm_i915_private
> > *dev_priv,
> > void intel_ddi_init(struct intel_display *display,
> > const struct intel_bios_encoder_data *devdata);
> > bool intel_ddi_get_hw_state(struct intel_encoder *encoder, enum pipe
> > *pipe);
> > +void intel_ddi_config_transcoder_func(struct intel_encoder *encoder,
> > + const struct intel_crtc_state
> > *crtc_state);
> > void intel_ddi_enable_transcoder_func(struct intel_encoder *encoder,
> > const struct intel_crtc_state
> > *crtc_state);
> > void intel_ddi_disable_transcoder_func(const struct intel_crtc_state
> > *crtc_state);
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index 1a2ff3e1cb68f..7c16406883594 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -1224,6 +1224,9 @@ static void intel_mst_pre_enable_dp(struct
> > intel_atomic_state *state,
> > if (DISPLAY_VER(dev_priv) < 12 || !first_mst_stream)
> > intel_ddi_enable_transcoder_clock(encoder, pipe_config);
> >
> > + if (DISPLAY_VER(dev_priv) >= 30 && !first_mst_stream)
> > + intel_ddi_config_transcoder_func(encoder, pipe_config);
> > +
> > intel_dsc_dp_pps_write(&dig_port->base, pipe_config);
> > intel_ddi_set_dp_msa(pipe_config, conn_state);
> > }
>
> I don't see the part where you changed the code to _not_ enable this,
> since you're calling intel_ddi_config_transcoder_func() here now...
>
> Did I miss something?
This only _configures_ the DDI function - which was missing so far - the
enabling and disabling happens separately later, in
intel_mst_enable_dp() / intel_mst_post_disable_dp() - which are done
already as expected.
> --
> Cheers,
> Luca.
