Hi Imre
Thank you for pointing out my mistake and kindly providing clear guidance.
I’ll fix the issue, run some tests, and update the commit description
accordingly.
Thank you.
BR
An
On Tue, May 5, 2026 at 4:35 PM Imre Deak <[email protected]> wrote:
> On Tue, May 05, 2026 at 11:35:57AM +0800, ChunAn Wu wrote:
> > When Fn+F4 triggers mirror mode, the BIOS/EC reconfigures a TC port
> > mode (e.g. DP-Alt to TBT-Alt via UCSI) without generating HPD. The
> > driver's cached TC mode becomes stale, so intel_dp_aux_xfer() uses
> > the wrong power domain and IO routing, and AUX transactions fail.
> >
> > This occurs when the USB-C-to-HDMI dongle does not support TBT, so
> > the mode switch creates a mismatch. It also occurs with slow monitors
> > whose delayed HPD/DDC/EDID recovery causes the Type-C layer to settle
> > on a stale tbt-alt state before the dongle finishes re-negotiation.
> >
> > Add intel_tc_port_aux_recover() to detect hardware/cached TC mode
> > divergence and reset the PHY. Wrap it with
> > intel_dp_aux_xfer_with_recovery() to retry with corrected settings.
> >
> > Tested on Panther Lake and Lunar Lake with a BenQ HDMI monitor via
> > USB-C-to-HDMI dongle.
> >
> > Signed-off-by: ChunAn Wu <[email protected]>
> > ---
> >
> > [...]
> >
> > +/**
> > + * intel_tc_port_aux_recover - Recover AUX channel after external TC
> mode change
> > + * @dig_port: digital port
> > + *
> > + * When firmware causes a TC port mode change (e.g., dp-alt ->
> disconnect ->
> > + * tbt-alt) during a hotkey-triggered display mode switch, AUX
> transactions
> > + * using the stale power domain and IO flags will fail with timeouts or
> errors.
> > + * This happens because:
> > + *
> > + * 1. The display driver holds the TC link in dp-alt mode
> (link_refcount > 0)
> > + * 2. Firmware (e.g., via HP WMI hotkey BIOS action) reconfigures the
> TC port
> > + * mode externally without notifying the display driver
> > + * 3. tc->mode stays as TC_PORT_DP_ALT while HW transitions to a
> different
> > + * mode, invalidating the AUX power domain and IO flags used by
> > + * intel_dp_aux_xfer()
> > + *
> > + * This function detects the discrepancy between tc->mode and actual HW
> state,
> > + * and re-synchronizes them via a TC PHY reset. After recovery, AUX
> retries
> > + * will use the correct power domain and control flags.
> > + *
> > + * The link_refcount is temporarily cleared to allow
> intel_tc_port_reset_mode()
> > + * to proceed without the PHY-ownership assertion that fires when
> link_refcount
> > + * is non-zero and firmware has already released PHY ownership.
> > + *
> > + * Must be called outside the TC port lock (tc->lock).
> > + *
> > + * Returns: %true if recovery was performed and AUX can be retried,
> > + * %false if recovery was not needed or not possible.
> > + */
> > +bool intel_tc_port_aux_recover(struct intel_digital_port *dig_port)
> > +{
> > + struct intel_tc_port *tc;
> > + struct intel_display *display;
> > + bool recovered = false;
> > +
> > + if (!intel_encoder_is_tc(&dig_port->base))
> > + return false;
> > +
> > + tc = to_tc_port(dig_port);
> > + display = to_intel_display(dig_port);
> > +
> > + mutex_lock(&tc->lock);
> > +
> > + /*
> > + * Recovery is only needed when the link is actively held AND the
> HW
> > + * TC mode has diverged from the driver's cached state.
> > + */
> > + if (!tc->link_refcount || !intel_tc_port_needs_reset(tc))
> > + goto out;
> > +
> > + drm_dbg_kms(display->drm,
> > + "Port %s: AUX recover: external TC mode change
> detected (%s -> HW), reconnecting TC PHY\n",
> > + tc->port_name, tc_port_mode_name(tc->mode));
> > +
> > + /*
> > + * Temporarily clear link_refcount so intel_tc_port_reset_mode()
> can
> > + * run the PHY disconnect/connect cycle without the ownership
> assertion
> > + * that fires when link_refcount > 0 and firmware has already
> released
> > + * PHY ownership externally.
> > + */
> > + tc->link_refcount = 0;
>
> This looks wrong, NAK from my side:
>
> A non-zero link_refcount guards an active output, which must maintain
> the TypeC mode with which the output was enabled. So that TypeC mode
> cannot be reset from under the active output as done in this patch.
>
> The commit message fails to mention how the display driver is supposed
> to notice that the user have pressed F4 and the firmware has
> reconfigured things in the background. Assuming that the FW's
> reconfiguration in the background is actually a valid thing to do, i.e.
> doesn't result in getting the active TypeC port into a broken state, the
> transition to the new TypeC mode should happen in the regular way: via
> an HPD event, if neecessary using the
> drm_connector_funcs::oob_hotplug_event interface, disabling any active
> outputs and _then_ switching the driver's idea of the TypeC mode to
> reflect the HW state.
>
> > + intel_tc_port_reset_mode(tc, 1, false);
> > + tc->link_refcount = 1;
> > +
> > + recovered = tc->mode != TC_PORT_DISCONNECTED;
> > + if (!recovered)
> > + drm_warn(display->drm,
> > + "Port %s: AUX recover: failed to restore TC port
> mode\n",
> > + tc->port_name);
> > +
> > +out:
> > + mutex_unlock(&tc->lock);
> > + return recovered;
> > +}
> > +
> > static void __intel_tc_port_lock(struct intel_tc_port *tc,
> > int required_lanes)
> > {
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.h
> b/drivers/gpu/drm/i915/display/intel_tc.h
> > index 6719aea5bd58..d44998a3081a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.h
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.h
> > @@ -108,6 +108,7 @@ bool intel_tc_port_ref_held(struct
> intel_digital_port *dig_port);
> > bool intel_tc_port_link_needs_reset(struct intel_digital_port
> *dig_port);
> > bool intel_tc_port_link_reset(struct intel_digital_port *dig_port);
> > void intel_tc_port_link_cancel_reset_work(struct intel_digital_port
> *dig_port);
> > +bool intel_tc_port_aux_recover(struct intel_digital_port *dig_port);
> >
> > int intel_tc_port_init(struct intel_digital_port *dig_port, bool
> is_legacy);
> > void intel_tc_port_cleanup(struct intel_digital_port *dig_port);
> > --
> > 2.34.1
> >
>
