On Tue, Jun 02, 2026 at 11:33:09AM -0400, Stephen Fuhry wrote:
> I can share some observations from the debug logs captured on the affected
> system (drm.debug=0x10e, Alder Lake-P [8086:46a6], ThinkPad dock, 1080p on
> pipe B + 4K DSC on pipe C) that may help clarify the root cause.
>
> Imre's assertion about the enable/disable gating appears to be correct from
> the logs. The active_links sequence across every modeset cycle in the
> captured trace is always clean:
>
> disable: 2 -> 1 -> 0 (intel_mst_disable_dp, intel_encoders_post_disable)
> enable: 0 -> 1 -> 2 (intel_mst_pre_enable_dp, intel_mst_enable_dp)
>
> No interleaving, no double-calls. intel_ddi_enable_fec and
> intel_ddi_disable_fec are each called exactly once per link per modeset
> cycle, as expected.
>
> The mismatch and cascade are instead driven by repeated failures in the
> fastset check. In the cable-unplug log, 8 "fastset requirement not met,
> forcing full modeset" messages fire for both pipes between t=110.803s and
> t=110.899s -- before a single active_links change occurs. After replug,
> there are dozens more at ~20ms intervals (t=125.5s to t=126.3s) as the
> driver re-probes the dock, each one attempting and failing to commit a
> modeset.
>
> The direction of the first mismatch after dock connect is notable. The
> very first fastset failure shows:
>
> [CRTC:186:pipe C] fastset requirement not met in fec_enable
> (expected no, found yes)
> [CRTC:186:pipe C] fastset requirement not met in
> hw.pipe_mode.crtc_hdisplay
> (expected 1920, found 3840)
> [CRTC:186:pipe C] fastset requirement not met in dsc.config.pic_width
> (expected 0, found 3840)
>
> The SW computed state has pipe C at 1920x1080 with no DSC and no FEC, while
> HW has it at 3840x2160 with DSC and FEC active (from the previous
> session/firmware state). The modeset that follows clears FEC. The next
> compute correctly selects DSC (and therefore FEC), triggering the opposite
> mismatch ("expected yes, found no"). This back-and-forth drives the cascade.
>
> The question of why the first SW compute produces fec_enable=no for pipe C
> when DSC is needed seems to be where the actual root cause lies -- whether
> that's a mode-negotiation timing issue (compositor requesting 1080p before
> 4K+DSC is negotiated) or something in the compute ordering in
> intel_dp_mst_compute_config.
>
> Full log (drm.debug=0x10e, cable unplug/replug) is attached to issue #16303
> if useful.
I found drm-debug-dmesg-fec-patch.txt on the ticket. This seems to be a
6.12 kernel, with the change in this patch applied. There is a strange
delay at boot-up between 13.894595 and 148.167 when nothing seems to
happen.
I see indeed multiple disconnect/reconnect events for the dock starting
like:
[ 159.428600] usb 3-6: USB disconnect, device number 10
[ 159.428605] usb 3-6.3: USB disconnect, device number 11
[ 159.428607] usb 3-6.3.2: USB disconnect, device number 12
[ 159.431529] i915 0000:00:02.0: [drm:gen11_hpd_irq_handler [i915]] hotplug
event received, stat 0x00040000, dig 0x00008a88, pins 0x00000800, long
0x00000800
[ 159.440423] usb 3-6.3.3: USB disconnect, device number 13
[ 159.440430] usb 3-6.3.3.1: USB disconnect, device number 14
[ 159.442548] usb 3-6.3.3.2: USB disconnect, device number 15
[ 159.452345] usb 2-3: USB disconnect, device number 6
[ 159.452352] r8152-cfgselector 2-3.1: USB disconnect, device number 7
[ 159.521344] usb 3-6.3.3.4: USB disconnect, device number 16
[ 159.544816] usb 2-3.3: USB disconnect, device number 8
[ 165.839439] i915 0000:00:02.0: [drm:gen11_hpd_irq_handler [i915]] hotplug
event received, stat 0x00040000, dig 0x00008a88, pins 0x00000800, long
0x00000800
repeated about 5 times start at 159.428. At first look this seems like a
non-display related issue leading to the disconnection of the whole dock
with all the USB device on it.
To get a better understanding of the root cause could you please provide
a log with the latest drm-tip kernel without any patch applied as I
requested on the ticket?
Thanks.
>
> Stephen Fuhry
>
> On Tue, Jun 2, 2026 at 9:39 AM Imre Deak <[email protected]> wrote:
>
> > On Tue, Jun 02, 2026 at 04:35:42PM +0300, Murthy, Arun R wrote:
> > >
> > > > -----Original Message-----
> > > > From: Deak, Imre <[email protected]>
> > > > Sent: Tuesday, June 2, 2026 4:17 PM
> > > > To: Murthy, Arun R <[email protected]>
> > > > Cc: [email protected]; [email protected];
> > Stephen
> > > > Fuhry <[email protected]>
> > > > Subject: Re: [PATCH] drm/i915/display: Refcount for fec enable/disable
> > > >
> > > > 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.
> > >
> > > The logs mentioned in the above listed gitlab issue shows mismatch in
> > > fec enable/disable in the MST scenario. Hence added this refcount
> > > logic to overcome the mismatch.
> >
> > The root cause for the mismatch should be better understood then. I
> > still think that it's something else than the lack of refcounting.
> >
> > > Thanks and Regards,
> > > Arun R Murthy
> > > --------------------
> > > > > +
> > > > > 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
> > > > >
> >
