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

[Imre Deak]

On Thu, Oct 31, 2024 at 12:49:22PM +0200, Luca Coelho wrote:
> 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 at 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?

Yes, the first patch clearly fixes an issue on PTL. On other ADLP+
platforms this (and the follow-up DP2 config changes) is just what bspec
says to do, the lack of which never having caused a problem. Based on
all this I wanted to keep the PTL fix separate and first, making it
easier to revert any of the other changes if they caused a problem
(turning out that the spec was incorrect).

> In that case, wouldn't it be better to do this first so the ADLP+
> change could be more easily backported?

If that would be required - I'd only backport if there was an evidence
that it fixes things - I'd either backport it along with the first patch
as a dependency, or just send this one patch separately rebased on the
stable trees (which would be the first two patches combined).

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

Agreed for actual fixes, but in this case the only clear fix is the
first one, the rest being just alignment with the spec (and as such
better kept them easy to revert).

> --
> Cheers,
> Luca.