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 <[email protected]>
> ---
> 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))