On Thu, 2024-10-31 at 13:40 +0200, Imre Deak wrote: > 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@xxxxxxxxx> > > > --- > > > 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). Fair enough. Sorry for not responding before, for some reason it seems that I didn't get this email in my inbox, only in the list. Reviewed-by: Luca Coelho <luciano.coelho@xxxxxxxxx> -- Cheers, Luca.
