Re: [PATCH v2 2/5] drm/i915/adlp+/dp_mst: Align slave transcoder enabling with spec wrt. DDI function

Luca Coelho
2024-10-31T10:49:29Z

On Wed, 2024-10-30 at 21:23 +0200, Imre Deak wrote:
> On ADLP+ 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.
>
> Align the code with the spec based on the above.
>
> v2: Move this patch earlier in the series, addressing the DP2
> config fixes for all ADLP+ platforms later.
>
> Bspec: 55424, 54128, 65448, 68849
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 7c16406883594..bf264bd1881b5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -1224,7 +1224,7 @@ 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)
> + if (DISPLAY_VER(dev_priv) >= 13 && !first_mst_stream)
> intel_ddi_config_transcoder_func(encoder, pipe_config);
>
> intel_dsc_dp_pps_write(&dig_port->base, pipe_config);

You are essentially modifying the first patch in the series here, is it
because you want to keep the ADLP+ change separate? In that case,
wouldn't it be better to do this first so the ADLP+ change could be
more easily backported?

IMHO it's better to first fix stuff for older HW that is not correct
and then add new changes for new HW on top of that.

--
Cheers,
Luca.