public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: ChunAn Wu <an.wu@canonical.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Tvrtko Ursulin <tursulin@ursulin.net>,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] drm/i915/tc: Recover AUX channel after external TC port mode change
Date: Tue, 05 May 2026 11:02:13 +0300	[thread overview]
Message-ID: <68d0176c7022293678a49e7641ad9308ed0438a4@intel.com> (raw)
In-Reply-To: <20260505033558.157604-1-an.wu@canonical.com>

On Tue, 05 May 2026, ChunAn Wu <an.wu@canonical.com> 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.

What are the changes between v1 and v2?

>
> Signed-off-by: ChunAn Wu <an.wu@canonical.com>
> ---
>  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);

-- 
Jani Nikula, Intel

  reply	other threads:[~2026-05-05  8:02 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-05  3:35 [PATCH v2] drm/i915/tc: Recover AUX channel after external TC port mode change ChunAn Wu
2026-05-05  8:02 ` Jani Nikula [this message]
2026-05-05  8:34 ` Imre Deak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=68d0176c7022293678a49e7641ad9308ed0438a4@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=airlied@gmail.com \
    --cc=an.wu@canonical.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rodrigo.vivi@intel.com \
    --cc=simona@ffwll.ch \
    --cc=tursulin@ursulin.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox