Hi Jani, Imre, Ville,
    The patch is really not appropriate and also this issue should be
fixed elsewhere.
Therefore, I will drop this patch.
Appreciate your patience and sorry for the noise.

BR
    An

On Thu, May 7, 2026 at 9:58 AM An Wu <[email protected]> wrote:
>
> Hi Ville
> I have filed a bug 
> (https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/7918)
> Thank you for your comment.
>
> BR
>     An
>
> On Thu, May 7, 2026 at 2:55 AM Ville Syrjälä <[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.
>>
>> This sounds like a completely insane thing for the firmware to do.
>> It's not supposed to screw with the hardware while the driver is in
>> control.
>>
>> Please file a bug in gitlab and also attach full acpidumps there so
>> that we can check if that's where the insanity lives. This is isn't
>> some Fujitsu-Siemens laptop is it? Those have historically known to
>> do stupid stuff like this...
>>
>> >
>> > Signed-off-by: ChunAn Wu <[email protected]>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_dp_aux.c | 41 +++++++++--
>> >  drivers/gpu/drm/i915/display/intel_tc.c     | 75 +++++++++++++++++++++
>> >  drivers/gpu/drm/i915/display/intel_tc.h     |  1 +
>> >  3 files changed, 113 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c 
>> > b/drivers/gpu/drm/i915/display/intel_dp_aux.c
>> > index b20ec3e589fa..d496b2231656 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
>> > @@ -481,6 +481,39 @@ static u32 intel_dp_aux_xfer_flags(const struct 
>> > drm_dp_aux_msg *msg)
>> >       return 0;
>> >  }
>> >
>> > +/**
>> > + * intel_dp_aux_xfer_with_recovery - AUX transfer with TC port recovery
>> > + * @intel_dp: the DP port
>> > + * @send: buffer of bytes to send
>> > + * @send_bytes: number of bytes to send
>> > + * @recv: buffer to store received reply
>> > + * @recv_size: maximum number of bytes to receive
>> > + * @aux_send_ctl_flags: extra flags for the AUX send control register
>> > + *
>> > + * Wrapper around intel_dp_aux_xfer() that attempts to recover from an
>> > + * external TC port mode change (e.g., dp-alt -> tbt-alt via hotkey BIOS
>> > + * action) when the initial AUX transfer fails. On failure, recovery is
>> > + * attempted once via intel_tc_port_aux_recover() before retrying.
>> > + *
>> > + * Returns: number of received bytes on success, negative error code on 
>> > failure.
>> > + */
>> > +static int
>> > +intel_dp_aux_xfer_with_recovery(struct intel_dp *intel_dp,
>> > +                             const u8 *send, int send_bytes,
>> > +                             u8 *recv, int recv_size,
>> > +                             u32 aux_send_ctl_flags)
>> > +{
>> > +     struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>> > +     int ret;
>> > +
>> > +     ret = intel_dp_aux_xfer(intel_dp, send, send_bytes, recv, recv_size,
>> > +                             aux_send_ctl_flags);
>> > +     if (ret < 0 && intel_tc_port_aux_recover(dig_port))
>> > +             ret = intel_dp_aux_xfer(intel_dp, send, send_bytes, recv,
>> > +                                     recv_size, aux_send_ctl_flags);
>> > +     return ret;
>> > +}
>> > +
>> >  static ssize_t
>> >  intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>> >  {
>> > @@ -508,8 +541,8 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct 
>> > drm_dp_aux_msg *msg)
>> >               if (msg->buffer)
>> >                       memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
>> >
>> > -             ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
>> > -                                     rxbuf, rxsize, flags);
>> > +             ret = intel_dp_aux_xfer_with_recovery(intel_dp, txbuf, 
>> > txsize,
>> > +                                                   rxbuf, rxsize, flags);
>> >               if (ret > 0) {
>> >                       msg->reply = rxbuf[0] >> 4;
>> >
>> > @@ -531,8 +564,8 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct 
>> > drm_dp_aux_msg *msg)
>> >               if (drm_WARN_ON(display->drm, rxsize > 20))
>> >                       return -E2BIG;
>> >
>> > -             ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
>> > -                                     rxbuf, rxsize, flags);
>> > +             ret = intel_dp_aux_xfer_with_recovery(intel_dp, txbuf, 
>> > txsize,
>> > +                                                   rxbuf, rxsize, flags);
>> >               if (ret > 0) {
>> >                       msg->reply = rxbuf[0] >> 4;
>> >                       /*
>> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c 
>> > b/drivers/gpu/drm/i915/display/intel_tc.c
>> > index a21dd4e3fe4c..1b23161e4eea 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_tc.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_tc.c
>> > @@ -1861,6 +1861,81 @@ void intel_tc_port_link_cancel_reset_work(struct 
>> > intel_digital_port *dig_port)
>> >       cancel_delayed_work(&tc->link_reset_work);
>> >  }
>> >
>> > +/**
>> > + * 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;
>> > +     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
>>
>> --
>> Ville Syrjälä
>> Intel

Reply via email to