Re: [PATCH v2 3/7] drm/i915/dp: Export helper to determine if FEC on non-UHBR links is required

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

On Wed, 2025-10-15 at 19:19 +0300, Imre Deak wrote:
> Export the helper function to determine if FEC is required on a non-
> UHBR
> (8b10b) SST or MST link. A follow up change will take this into use
> for
> MST as well.
> 
> While at it determine the output type from the CRTC state, which
> allows
> dropping the intel_dp argument. Also make the function return the
> required FEC state, instead of setting this in the CRTC state, which
> allows only querying this requirement, without changing the state.
> 
> Also rename the function to intel_dp_needs_8b10b_fec(), to clarify
> that
> the function determines if FEC is required on an 8b10b link (on
> 128b132b
> links FEC is always enabled by the HW implicitly, so the function
> will
> return false for that case).
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c     | 21 +++++++++++++------
> --
>  drivers/gpu/drm/i915/display/intel_dp.h     |  2 ++
>  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 +-
>  3 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index b523c4e661412..3ffb015004c54 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2365,24 +2365,29 @@ static int
> intel_edp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
>  	return 0;
>  }
>  
> -static void intel_dp_fec_compute_config(struct intel_dp *intel_dp,
> -					struct intel_crtc_state
> *crtc_state)
> +/*
> + * Return whether FEC must be enabled for 8b10b SST or MST links. On
> 128b132b
> + * links FEC is always enabled implicitly by the HW, so this
> function returns
> + * false for that case.
> + */
> +bool intel_dp_needs_8b10b_fec(const struct intel_crtc_state
> *crtc_state,
> +			      bool dsc_enabled_on_crtc)
>  {
>  	if (intel_dp_is_uhbr(crtc_state))
> -		return;
> +		return false;
>  
>  	if (crtc_state->fec_enable)
> -		return;
> +		return true;

Not really changed in this patch: Do you know in what kind of case
"crtc_state->fec_enable == true" before intel_dp_needs_8b10b_fec is
called?

BR,

Jouni Högander
>  
>  	/*
>  	 * Though eDP v1.5 supports FEC with DSC, unlike DP, it is
> optional.
>  	 * Since, FEC is a bandwidth overhead, continue to not
> enable it for
>  	 * eDP. Until, there is a good reason to do so.
>  	 */
> -	if (intel_dp_is_edp(intel_dp))
> -		return;
> +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
> +		return false;
>  
> -	crtc_state->fec_enable = true;
> +	return dsc_enabled_on_crtc;
>  }
>  
>  int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> @@ -2404,7 +2409,7 @@ int intel_dp_dsc_compute_config(struct intel_dp
> *intel_dp,
>  	 * 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);
> +	pipe_config->fec_enable =
> intel_dp_needs_8b10b_fec(pipe_config, true);
>  
>  	if (!intel_dp_dsc_supports_format(connector, pipe_config-
> >output_format))
>  		return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
> b/drivers/gpu/drm/i915/display/intel_dp.h
> index b379443e0211e..55059bd5c7efb 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -73,6 +73,8 @@ void intel_dp_encoder_flush_work(struct drm_encoder
> *encoder);
>  int intel_dp_compute_config(struct intel_encoder *encoder,
>  			    struct intel_crtc_state *pipe_config,
>  			    struct drm_connector_state *conn_state);
> +bool intel_dp_needs_8b10b_fec(const struct intel_crtc_state
> *crtc_state,
> +			      bool dsc_enabled_on_crtc);
>  int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>  				struct intel_crtc_state
> *pipe_config,
>  				struct drm_connector_state
> *conn_state,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 27e952a67c343..d0590b5ffffd7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -296,7 +296,7 @@ int intel_dp_mtp_tu_compute_config(struct
> intel_dp *intel_dp,
>  	/*
>  	 * 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().
> +	 * intel_dp_needs_8b10b_fec().
>  	 */
>  	if (dsc) {
>  		if (!intel_dp_supports_fec(intel_dp, connector,
> crtc_state))