[PATCH v2 1/5] drm/i915/ptl/dp_mst: Fix slave transcoder enabling wrt. DDI function

[Luca Coelho]

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 <imre.deak at intel.com>
> ---
>  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?


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

--
Cheers,
Luca.