On Mon, Jun 01, 2026 at 07:59:43PM +0530, Arun R Murthy wrote:
> The FEC_ENABLE bit is per port basis and is enabled/disabled on ddi
> pre_enable and post_disable. This fec is shared across the mst streams
> and can be enabled per stream basis as well.
> So have a refcount to track the usage of FEC and then enable/disable
> accordingly.
>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/16073
> Signed-off-by: Arun R Murthy <[email protected]>
> Tested-by: Stephen Fuhry <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 66 +++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_ddi.h | 1 +
> .../drm/i915/display/intel_display_types.h | 12 ++++
> .../drm/i915/display/intel_modeset_setup.c | 6 ++
> 4 files changed, 85 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 86520848892e..e12a3d6d6a67 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2096,6 +2096,47 @@ void intel_ddi_disable_clock(struct intel_encoder
> *encoder)
> encoder->disable_clock(encoder);
> }
>
> +/**
> + * intel_ddi_seed_fec_refcounts - Seed per-port FEC refcounts from active
> CRTCs
> + * @display: display device
> + *
> + * intel_digital_port::fec_active_streams is the per-port refcount that gates
> + * programming of the shared DP_TP_CTL_FEC_ENABLE bit. After initial HW state
> + * readout (driver load, resume, GPU reset takeover), the persistent
> + * crtc_state->fec_enable values reflect what HW currently has; we need to
> + * align the refcount with that so the first paired disable doesn't underflow
> + * and the next enable doesn't incorrectly skip programming the HW bit.
> + *
> + * Must be called once after intel_modeset_readout_hw_state(), before any new
> + * modeset commit can run.
> + */
> +void intel_ddi_seed_fec_refcounts(struct intel_display *display)
> +{
> + struct intel_crtc *crtc;
> +
> + for_each_intel_crtc(display->drm, crtc) {
> + const struct intel_crtc_state *crtc_state =
> + to_intel_crtc_state(crtc->base.state);
> + struct intel_encoder *encoder;
> +
> + if (!crtc_state->hw.active || !crtc_state->fec_enable)
> + continue;
> +
> + for_each_intel_encoder(display->drm, encoder) {
> + struct intel_digital_port *dig_port;
> +
> + if (encoder->base.crtc != &crtc->base)
> + continue;
> + if (!intel_encoder_is_dig_port(encoder))
> + continue;
> +
> + dig_port = enc_to_dig_port(encoder);
> + dig_port->fec_active_streams++;
> + break;
> + }
> + }
> +}
> +
> void intel_ddi_sanitize_encoder_pll_mapping(struct intel_encoder *encoder)
> {
> struct intel_display *display = to_intel_display(encoder);
> @@ -2413,12 +2454,22 @@ static void intel_ddi_enable_fec(struct intel_encoder
> *encoder,
> const struct intel_crtc_state *crtc_state)
> {
> struct intel_display *display = to_intel_display(encoder);
> + struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> int i;
> int ret;
>
> if (!crtc_state->fec_enable)
> return;
>
> + /*
> + * FEC is link-wide: DP_TP_CTL_FEC_ENABLE is per-port while
> + * crtc_state->fec_enable is per-stream. For DP MST, several streams
> + * on this port share the bit. Only program HW on the first stream
> + * needing FEC; subsequent streams just bump the refcount.
> + */
> + if (dig_port->fec_active_streams++ > 0)
> + return;
This doesn't make sense to me. FEC is enabled for the MST link and if
it's enabled then fec_enabled is set in the crtc_state for all the
streams in the MST topology. intel_ddi_enable_fec() will be called only
for the first MST stream being enabled and intel_ddi_disable_fec() will
be called only for the last MST stream being disabled. So I don't see
why the above refcounting would be needed.
> +
> intel_de_rmw(display, dp_tp_ctl_reg(encoder, crtc_state),
> 0, DP_TP_CTL_FEC_ENABLE);
>
> @@ -2454,10 +2505,25 @@ static void intel_ddi_disable_fec(struct
> intel_encoder *encoder,
> const struct intel_crtc_state *crtc_state)
> {
> struct intel_display *display = to_intel_display(encoder);
> + struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
>
> if (!crtc_state->fec_enable)
> return;
>
> + /*
> + * FEC is a link-wide property and DP_TP_CTL_FEC_ENABLE is a per-port
> + * register, but crtc_state->fec_enable is per-stream. For DP MST,
> + * multiple streams on the same port share this bit. Refcount the
> + * active FEC users on the port and only clear the HW bit when the
> + * last user goes away, otherwise tearing down one MST stream would
> + * disable FEC for sibling streams still using it.
> + */
> + if (drm_WARN_ON(display->drm, dig_port->fec_active_streams <= 0))
> + return;
> +
> + if (--dig_port->fec_active_streams > 0)
> + return;
> +
> intel_de_rmw(display, dp_tp_ctl_reg(encoder, crtc_state),
> DP_TP_CTL_FEC_ENABLE, 0);
> intel_de_posting_read(display, dp_tp_ctl_reg(encoder, crtc_state));
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.h
> b/drivers/gpu/drm/i915/display/intel_ddi.h
> index 580ecb09b8b6..3678c28a0dc9 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.h
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.h
> @@ -78,6 +78,7 @@ int intel_ddi_toggle_hdcp_bits(struct intel_encoder
> *intel_encoder,
> enum transcoder cpu_transcoder,
> bool enable, u32 hdcp_mask);
> void intel_ddi_sanitize_encoder_pll_mapping(struct intel_encoder *encoder);
> +void intel_ddi_seed_fec_refcounts(struct intel_display *display);
> int intel_ddi_level(struct intel_encoder *encoder,
> const struct intel_crtc_state *crtc_state,
> int lane);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index f44be5c689ae..84bd0d993197 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1987,6 +1987,18 @@ struct intel_digital_port {
> struct ref_tracker *ddi_io_wakeref;
> struct ref_tracker *aux_wakeref;
>
> + /*
> + * Number of active streams on this port currently using FEC.
> + *
> + * DP_TP_CTL_FEC_ENABLE is a per-port (link-wide) HW bit, but
> + * crtc_state->fec_enable is per-stream. For DP MST several streams
> + * share the same port and therefore the same FEC enable bit. Track
> + * how many active streams want FEC so that the HW bit is only
> + * programmed on the first enable and only cleared on the last
> + * disable. Modified under the modeset locks.
> + */
> + int fec_active_streams;
> +
> struct intel_tc_port *tc;
>
> struct {
> diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> index e88082c8caac..14f038b8ef81 100644
> --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> @@ -950,6 +950,12 @@ void intel_modeset_setup_hw_state(struct intel_display
> *display,
>
> intel_modeset_readout_hw_state(display);
>
> + /*
> + * Seed per-port FEC refcounts from the just-populated active
> + * crtc_states before anything can issue an enable/disable.
> + */
> + intel_ddi_seed_fec_refcounts(display);
> +
> /* HW state is read out, now we need to sanitize this mess. */
> get_encoder_power_domains(display);
>
> --
> 2.25.1
>
