On Wed, 2025-10-15 at 19:19 +0300, Imre Deak wrote: > Atm, in the DP SST case the FEC state is computed before > intel_crtc_state::port_clock is initialized, hence intel_dp_is_uhbr() > will always return false and the FEC state will be always computed > assuming a non-UHBR link. > > This happens to work, since the FEC state is recomputed later in > intel_dp_mtp_tu_compute_config(), where port_clock will be set > already, > so intel_crtc_state::fec_enable will be reset as expected for UHBR. > This > also depends on link rates being tried in an increasing order (i.e. > from > non-UHBR -> UHBR link rates) in dsc_compute_link_config(), thus > intel_crtc_state::fec_enable being set for the non-UHBR rates and > getting reset for the first UHBR rate as expected. > > A follow-up change will reuse intel_dp_fec_compute_config() for the > DP > MST state computation, prepare for that here, making sure that the > function determines the correct intel_crtc_state::fec_enable=false > state > for UHBR link rates based on the above. > > The DP SST and MST state computation should be further unified to > avoid > computing/setting the intel_crtc_state::fec_enable state multiple > times, > but that's left for a follow-up change. For now add only code > comments > about this. > > Signed-off-by: Imre Deak <[email protected]>
Reviewed-by: Jouni Högander <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 10 +++++++--- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 5 +++++ > 2 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index 1d3ca1970f25f..b523c4e661412 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -2368,6 +2368,9 @@ static int > intel_edp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp, > static void intel_dp_fec_compute_config(struct intel_dp *intel_dp, > struct intel_crtc_state > *crtc_state) > { > + if (intel_dp_is_uhbr(crtc_state)) > + return; > + > if (crtc_state->fec_enable) > return; > > @@ -2379,9 +2382,6 @@ static void intel_dp_fec_compute_config(struct > intel_dp *intel_dp, > if (intel_dp_is_edp(intel_dp)) > return; > > - if (intel_dp_is_uhbr(crtc_state)) > - return; > - > crtc_state->fec_enable = true; > } > > @@ -2400,6 +2400,10 @@ int intel_dp_dsc_compute_config(struct > intel_dp *intel_dp, > bool is_mst = intel_crtc_has_type(pipe_config, > INTEL_OUTPUT_DP_MST); > int ret; > > + /* > + * FIXME: set the FEC enabled state once pipe_config- > >port_clock is > + * already known, so the UHBR/non-UHBR mode can be > determined. > + */ > intel_dp_fec_compute_config(intel_dp, pipe_config); > > if (!intel_dp_dsc_supports_format(connector, pipe_config- > >output_format)) > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index f2266b2653046..27e952a67c343 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -293,6 +293,11 @@ int intel_dp_mtp_tu_compute_config(struct > intel_dp *intel_dp, > mst_stream_update_slots(crtc_state, mst_state); > } > > + /* > + * NOTE: The following must reset crtc_state->fec_enable for > UHBR/DSC > + * after it was set by intel_dp_dsc_compute_config() -> > + * intel_dp_fec_compute_config(). > + */ > if (dsc) { > if (!intel_dp_supports_fec(intel_dp, connector, > crtc_state)) > return -EINVAL;
