On Tue, 2025-10-28 at 13:35 +0200, Imre Deak wrote: > The reason for enabling FEC for an uncompressed stream on an MST link > is > that the DSC compression is enabled for another stream on the same > link. > For such an uncompressed stream FEC doesn't need to be supported on > the > whole path until the (DP-SST) sink DPRX. For instance if a branch > device > - like a monitor with an MST branch device within it - is plugged to > a > DFP connector of an MST docking station and the monitor's branch > device does not support FEC, the docking station's branch device will > still enable the link to the monitor correctly, disabling the FEC on > that link as expected. Since it's been verified already that FEC is > supported for the compressed stream above, the corresponding check > for > the uncompressed stream can be dropped: the check for the compressed > stream implies already that FEC is supported on the link between the > source DPTX and immediate downstream branch device. If FEC is not > supported on the whole path until the sink DPRX, FEC will be disabled > by > a downstream branch device on the path as described above for the MST > dock + MST monitor configuration example. > > This fixes a problem in the above MST dock + MST monitor example, > where > the dock supports FEC, but the monitor doesn't support it and FEC > gets > enabled on the link due to DSC getting enabled for another monitor's > stream on the same link. > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14254 > Signed-off-by: Imre Deak <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 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 a845b2612a3fa..21a60b8c880ee 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -299,7 +299,14 @@ int intel_dp_mtp_tu_compute_config(struct > intel_dp *intel_dp, > * intel_dp_needs_8b10b_fec(). > */ > crtc_state->fec_enable = > intel_dp_needs_8b10b_fec(crtc_state, dsc); > - if (crtc_state->fec_enable && > + /* > + * If FEC gets enabled only because of another compressed > stream, FEC > + * may not be supported for this uncompressed stream on the > whole link > + * path until the sink DPRX. In this case a downstream > branch device > + * will disable FEC for the uncompressed stream as expected > and so the > + * FEC support doesn't need to be checked for this > uncompressed stream. > + */ > + if (crtc_state->fec_enable && dsc &&
Why crtc_state->fec_enable is set if it's not going to enabled and not even supported in the crtc this crtc_state is for? Also there seems to be very similar check in mst_stream_compute_config. Do we need to change that as well? BR, Jouni Högander > !intel_dp_supports_fec(intel_dp, connector, crtc_state)) > return -EINVAL; >
