Re: [PATCH] drm/i915/dp_mst: Fix check for FEC support for an uncompressed stream

["Hogander, Jouni" <jouni.hogander@intel.com>]

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 <imre.deak@intel.com>
> ---
>  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;
>