Re: [PATCH v2 2/7] drm/i915/dp: Ensure the FEC state stays disabled for UHBR links

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

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

Reviewed-by: Jouni Högander <jouni.hogander@intel.com>

> ---
>  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;