On Wed, Oct 29, 2025 at 04:06:07PM +0200, Hogander, Jouni wrote: > 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?
It is going to be enabled as required by all the CRTCs on the same link, when any CRTC on this link uses DSC compression. It is also supported by the link as explained by the commit message (the CRTC using DSC ensured this support, all the way to the sink DPRX). It's not necessarily supported all the way until the sink DPRX on _this_ CRTC, hence no need to check the support for _that_ calling intel_dp_supports_fec(). > Also there seems to be very similar check in mst_stream_compute_config. > Do we need to change that as well? Right. intel_crtc_state::fec_enable isn't yet set at that point, since the state computation starts with a zeroed CRTC state. So, that check should be removed (as a follow-up). > BR, > > Jouni Högander > > > !intel_dp_supports_fec(intel_dp, connector, crtc_state)) > > return -EINVAL; > > >
