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

[sashiko-bot@kernel.org]

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