[PATCH v3] drm/amd/display: Route connector DDC over AUX for native DP sinks

[PATCH v3] drm/amd/display: Route connector DDC over AUX for native DP sinks

[Mario Limonciello]

On DisplayPort and eDP connectors DDC is tunneled over the AUX channel,
but the connector's "ddc" adapter ("AMDGPU DM i2c hw bus N") always
drives the physical i2c pins, which don't reach a native DP sink.  The
AUX backed adapter ("AMDGPU DM aux hw bus N") does, so userspace such as
fwupd, which uses the "ddc" sysfs link for DDC/CI, has to work around
this.

Keep a single i2c bus per connector and pick the transport per transfer
based on the link's transaction mode (link->aux_mode), as the EDID read
helpers already do: for native DP/eDP/MST sinks forward the transfer to
the AUX adapter, otherwise (DVI/HDMI, including DP++ with a passive
adapter) use the hw i2c engine as before.

Reported-by: simonit.orig@gmail.com
Closes: https://github.com/fwupd/fwupd/pull/11308
Signed-off-by: Mario Limonciello <superm1@kernel.org>
---
Changes in v3:
- Select the transport with link->aux_mode instead of
  link->local_sink->sink_signal, matching the EDID read helpers in
  amdgpu_dm_helpers.c.  This drops the unlocked local_sink dereference
  that could race with link detection/hotplug (use-after-free), and it
  also covers MST links (local_sink is NULL there) and links with
  force_to_use_aux set.
- Check ddc_service->link for NULL before dereferencing it, so
  amdgpu_dm_i2c_xfer() still returns -EIO when there is no ddc_pin
  (fixes dm_test_i2c_xfer_no_ddc_pin()).
- Reword the commit message to describe the aux_mode selection.
- Link to v2: https://patch.msgid.link/20261103-superm1-amdgpu-ddc-v2-1-7bde6d004382@kernel.org

Changes in v2:
- Check which i2c bus to use on the fly instead of changing static symlink to
  avoid regressing passive adapters.
- Link to v1: https://patch.msgid.link/20261102-superm1-amdgpu-ddc-v1-1-9c5cb4a4f62d@amd.com
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c  | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
index b498fc5fbb140..f80de9e7f850c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
@@ -3373,6 +3373,22 @@ STATIC_IFN_KUNIT int amdgpu_dm_i2c_xfer(struct i2c_adapter *i2c_adap,
 	int i;
 	int result = -EIO;
 
+	if (!i2c->oem && ddc_service->link) {
+		struct dc_link *link = ddc_service->link;
+		struct amdgpu_dm_connector *aconnector = link->priv;
+
+		/*
+		 * DDC to a native DisplayPort sink (SST, MST or eDP) is
+		 * tunneled over the AUX channel; the physical i2c pins only
+		 * reach DVI/HDMI sinks (e.g. DP++ with a passive adapter).
+		 * Use the same aux_mode selection as the EDID read helpers so
+		 * the connector's single "ddc" bus works for either sink.
+		 */
+		if (aconnector && link->aux_mode &&
+		    aconnector->dm_dp_aux.aux.ddc.algo)
+			return i2c_transfer(&aconnector->dm_dp_aux.aux.ddc, msgs, num);
+	}
+
 	if (!ddc_service->ddc_pin)
 		return result;
 

---
base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb
change-id: 20261102-superm1-amdgpu-ddc-63f0eb10c022

Best regards,
--  
Mario Limonciello <superm1@kernel.org>

Re: [PATCH v3] drm/amd/display: Route connector DDC over AUX for native DP sinks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] drm/amd/display: stale link->aux_mode in amdgpu_dm_i2c_xfer()
  breaks DP++
--

commit 98cc22cc3e67bb7ce06a4e571268aa8a05d6952f
Author: Mario Limonciello <superm1@kernel.org>

drm/amd/display: Route connector DDC over AUX for native DP sinks

This commit routes I2C transfers on a connector's DDC adapter to the AUX
DDC adapter when link->aux_mode is set so that userspace DDC/CI transfers
reach native DisplayPort sinks.

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> index b498fc5fbb140..f80de9e7f850c 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> @@ -3373,6 +3373,22 @@ STATIC_IFN_KUNIT int amdgpu_dm_i2c_xfer(struct i2c_adapter *i2c_adap,
>  	int i;
>  	int result = -EIO;
>  
> +	if (!i2c->oem && ddc_service->link) {
> +		struct dc_link *link = ddc_service->link;
> +		struct amdgpu_dm_connector *aconnector = link->priv;
> +
> +		/*
> +		 * DDC to a native DisplayPort sink (SST, MST or eDP) is
> +		 * tunneled over the AUX channel; the physical i2c pins only
> +		 * reach DVI/HDMI sinks (e.g. DP++ with a passive adapter).
> +		 * Use the same aux_mode selection as the EDID read helpers so
> +		 * the connector's single "ddc" bus works for either sink.
> +		 */
> +		if (aconnector && link->aux_mode &&
> +		    aconnector->dm_dp_aux.aux.ddc.algo)
> +			return i2c_transfer(&aconnector->dm_dp_aux.aux.ddc, msgs, num);
> +	}

[Severity: High]
Can checking link->aux_mode here misroute passive DP++ dongle detection
over AUX when a passive DP-to-HDMI adapter is plugged into a port that
previously had a native DP sink connected?

In detect_link_and_local_sink(), link->aux_mode is not cleared on
disconnect and is only updated after detect_dp() finishes:

drivers/gpu/drm/amd/display/dc/link/link_detection.c:detect_link_and_local_sink() {
	...
			if (!detect_dp(link, &sink_caps, reason)) {
	...
		set_ddc_transaction_type(link->ddc,
					 sink_caps.transaction_type);

		link->aux_mode =
			link_is_in_aux_transaction_mode(link->ddc);
	...
}

While dm_helpers_read_local_edid() runs after link->aux_mode is updated,
detect_dp() runs before it and calls dp_passive_dongle_detection() ->
query_dp_dual_mode_adaptor() -> i2c_read() -> dm_helpers_submit_i2c(),
which goes through i2c_transfer(&aconnector->i2c->base, ...) and
amdgpu_dm_i2c_xfer().  If a native DP monitor was previously connected,
link->aux_mode is still true during detect_dp(), so the read of
DP_HDMI_DONGLE_ADDRESS (0x40) is redirected to the AUX adapter instead of
the physical I2C pins.  Because a passive adapter has no AUX responder,
the read fails and query_dp_dual_mode_adaptor() falls back to
DISPLAY_DONGLE_DP_DVI_DONGLE (SIGNAL_TYPE_DVI_SINGLE_LINK), capping the
pixel clock to 165 MHz and disabling HDMI audio and infoframes.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-superm1-amdgpu-ddc-v3-1-9436b04d1e31@kernel.org?part=1