On Tue, 16 Jun 2026, Arun R Murthy <[email protected]> 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.
The FEC enable/disable should only happen on the first/last DP MST stream through the primary_encoder pre_enable()/post_disable() hooks. There's a lot of stuff that's only relevant for the primary_encoder, and we don't have to jump through hoops for them. We have the active stream counting already in place. We shouldn't need separate tracking for FEC. BR, Jani. > > 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 | 58 > ++++++++++++++++++++++ > drivers/gpu/drm/i915/display/intel_ddi.h | 1 + > drivers/gpu/drm/i915/display/intel_display_types.h | 3 ++ > drivers/gpu/drm/i915/display/intel_modeset_setup.c | 6 +++ > 4 files changed, 68 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > b/drivers/gpu/drm/i915/display/intel_ddi.c > index > 25314ec65ae77b91bf4d732c229f236d070e18cc..477a11a63fe8f8d6731905be21de34dcbaa895b5 > 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, 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; > + > intel_de_rmw(display, dp_tp_ctl_reg(encoder, crtc_state), > 0, DP_TP_CTL_FEC_ENABLE); > > @@ -2454,10 +2505,17 @@ 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; > > + 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 > 580ecb09b8b606e07445c7e26142a2fcfa69a2d2..3678c28a0dc952d4962428893c519fb7d41e4422 > 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 > aa4772a1c208e4cb4bb6f51dc0dcc2349e422dd0..276d4cc21d6ecdd8c17777608e59223d9f49c554 > 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1989,6 +1989,9 @@ 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 */ > + 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 > e8730b5baf2a4bd2e5edfc5fc8fd2622a57d2a4e..4b6abcb1dd928ab2bc9b17f0521db78ebb6ef586 > 100644 > --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c > +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c > @@ -952,6 +952,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); -- Jani Nikula, Intel
