[PATCH] drm/amd/display: fall back to software I2C on hardware engine failure

NepNep7601 posted 1 patch 1 month ago
drivers/gpu/drm/amd/display/dc/dce/dce_i2c.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
[PATCH] drm/amd/display: fall back to software I2C on hardware engine failure
Posted by NepNep7601 1 month ago
dce_i2c_submit_command() returns the hardware I2C engine's result
directly, so a failed transfer is reported to the caller even though a
bit-banging software engine is available. That engine is only reachable
when the hardware engine cannot be acquired, never when its transfer
fails.

On DCE6 the hardware engine acknowledges a slave address but fails to
complete a 128 byte EDID block read. On an Oland-based GPU (Radeon
R7 430, rebranded R7 240, 1002:6611) driving a monitor through a
passive DP to HDMI to DVI chain, this leaves
dm_helpers_read_local_edid() with EDID_NO_RESPONSE:

  [    6.684221] [drm:dm_helpers_read_local_edid [amdgpu]] *ERROR* EDID err: 2, on connector: DP-1
  [    6.684726] amdgpu 0000:01:00.0: [drm] *ERROR* No EDID read.

The connector is then left with no modes and falls back to 640x480
instead of the display's native 1600x900. The DDC bus itself is fine:
i2cdetect sees the EDID EEPROM acknowledge at 0x50, while a real block
read with "i2ctransfer -y 1 w1@0x50 0x00 r128" fails with EIO. Short
transfers work, long ones do not.

dce_i2c_submit_command_hw() clears i2c_hw_buffer_in_use, releases the
engine and closes the DDC line on every exit path, so the software
engine can safely re-acquire it. Retry there rather than failing
outright. Behaviour is unchanged whenever the hardware engine
succeeds, and the fallback only runs where the transfer had already
failed.

Assisted-by: Claude:claude-opus-5
Signed-off-by: NepNep7601 <neptune@imm0nv1nhtv.is-a.dev>
---
Tested on 7.1.8 on an Oland (Radeon R7 430, rebranded R7 240, 1002:6611)
with a passive DP to HDMI to DVI chain: with this patch the EDID reads
256 bytes and the display comes up at its native 1600x900 instead of
640x480. Without it, dm_helpers_read_local_edid() returns
EDID_NO_RESPONSE.

On amd-staging-drm-next this is compile-tested only.
 drivers/gpu/drm/amd/display/dc/dce/dce_i2c.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_i2c.c b/drivers/gpu/drm/amd/display/dc/dce/dce_i2c.c
index f5261e8d7678..238c17e6f51d 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_i2c.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_i2c.c
@@ -72,9 +72,16 @@ bool dce_i2c_submit_command(
 
 	dce_i2c_hw = acquire_i2c_hw_engine(pool, ddc);
 
-	if (dce_i2c_hw)
-		return dce_i2c_submit_command_hw(pool, ddc, cmd, dce_i2c_hw);
+	if (dce_i2c_hw && dce_i2c_submit_command_hw(pool, ddc, cmd, dce_i2c_hw))
+		return true;
 
+	/*
+	 * The hardware I2C engine on DCE6 can fail to complete longer
+	 * transfers, such as a 128 byte EDID block read, even when the slave
+	 * acknowledges its address. dce_i2c_submit_command_hw() releases the
+	 * engine and closes the DDC line on every exit path, so retry the
+	 * transfer on the bit-banging software engine instead of giving up.
+	 */
 	dce_i2c_sw.ctx = ddc->ctx;
 	if (dce_i2c_engine_acquire_sw(&dce_i2c_sw, ddc)) {
 		return dce_i2c_submit_command_sw(pool, ddc, cmd, &dce_i2c_sw);
-- 
2.47.3
[PATCH v2 1/2] drm/amd/display: skip receiver power control without AUX
Posted by NepNep7601 1 month ago
Passive DP to TMDS dongles do not provide a DP receiver and use native
GPIO I2C rather than AUX. The normal link PHY and stream blanking paths
nevertheless try to write DP_SET_POWER, causing the DP helpers to retry
a transaction that cannot succeed 32 times before giving up.

Gate the receiver-power calls in dp_enable_link_phy(),
dp_disable_link_phy() and link_blank_dp_stream() on the post-detection
aux_mode state. Keep dpcd_write_rx_power_ctrl() unchanged because early
DP detection calls it before aux_mode is initialized, and active dongles
may require the receiver power-up before DPCD reads.

On an Oland GPU with a passive DP to HDMI to DVI chain, this reduced
boot-time "DP AUX transfer fail" messages from 32 to 0. The EDID
remained 256 bytes and the display continued to use its native
1600x900 mode.

Assisted-by: Claude:claude-opus-5
Assisted-by: Codex:gpt-5
Signed-off-by: NepNep7601 <neptune@imm0nv1nhtv.is-a.dev>
---

Notes (amdgpu-followups-v2):
    v2:
    - Move the aux_mode check out of dpcd_write_rx_power_ctrl() so early
      detection can still power active dongles before reading DPCD.
    - Gate receiver-power writes at the normal PHY and stream-blanking call
      sites.
    - Retest the revised placement on the affected Oland system.
    
    v1: https://lore.kernel.org/r/20260826193602.6441-1-neptune@imm0nv1nhtv.is-a.dev

 drivers/gpu/drm/amd/display/dc/link/link_dpms.c        |  6 ++++--
 .../drm/amd/display/dc/link/protocols/link_dp_phy.c    | 10 ++++++----
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
index 48b086d15ab0..81d63e1aab53 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
@@ -146,8 +146,10 @@ void link_blank_dp_stream(struct dc_link *link, bool hw_init)
 				}
 		}
 
-		if (((!dc->is_switch_in_progress_dest) && ((!link->wa_flags.dp_keep_receiver_powered) || hw_init)) &&
-			(link->type != dc_connection_none))
+		if (link->aux_mode &&
+		    !dc->is_switch_in_progress_dest &&
+		    (!link->wa_flags.dp_keep_receiver_powered || hw_init) &&
+		    link->type != dc_connection_none)
 			dpcd_write_rx_power_ctrl(link, false);
 	}
 }
diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_phy.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_phy.c
index 49521ac4b0e8..24f09bdcab51 100644
--- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_phy.c
+++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_phy.c
@@ -65,7 +65,8 @@ void dp_enable_link_phy(
 	link->cur_link_settings = *link_settings;
 	link->dc->hwss.enable_dp_link_output(link, link_res, signal,
 			clock_source, link_settings);
-	dpcd_write_rx_power_ctrl(link, true);
+	if (link->aux_mode)
+		dpcd_write_rx_power_ctrl(link, true);
 }
 
 void dp_disable_link_phy(struct dc_link *link,
@@ -74,9 +75,10 @@ void dp_disable_link_phy(struct dc_link *link,
 {
 	struct dc  *dc = link->ctx->dc;
 
-	if (!link->wa_flags.dp_keep_receiver_powered &&
-			!link->skip_implict_edp_power_control &&
-			link->type != dc_connection_none)
+	if (link->aux_mode &&
+	    !link->wa_flags.dp_keep_receiver_powered &&
+	    !link->skip_implict_edp_power_control &&
+	    link->type != dc_connection_none)
 		dpcd_write_rx_power_ctrl(link, false);
 
 	dc->hwss.disable_link_output(link, link_res, signal);
-- 
2.47.3
[PATCH v2 2/2] drm/amd/display: close DDC on I2C engine setup failure
Posted by NepNep7601 1 month ago
acquire_i2c_hw_engine() opens the DDC pins before setting up the
hardware engine. If setup_engine() fails, the error path releases the
engine but leaves the DDC pins open and the engine's DDC pointer set.

Subsequent attempts to open the pins then return
GPIO_RESULT_ALREADY_OPENED, preventing further I2C transfers, EDID
reads and hotplug detection on that port until reboot.

Mirror the normal teardown path by closing the DDC pins and clearing
the pointer after releasing the engine.

Assisted-by: Codex:gpt-5
Signed-off-by: NepNep7601 <neptune@imm0nv1nhtv.is-a.dev>
---
 drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c b/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c
index 05892ab4529f..e7a05494abab 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c
@@ -485,6 +485,8 @@ struct dce_i2c_hw *acquire_i2c_hw_engine(
 
 	if (!setup_engine(dce_i2c_hw)) {
 		release_engine(dce_i2c_hw);
+		dal_ddc_close(dce_i2c_hw->ddc);
+		dce_i2c_hw->ddc = NULL;
 		return NULL;
 	}
 
-- 
2.47.3
Re: [PATCH v2 2/2] drm/amd/display: close DDC on I2C engine setup failure
Posted by NepNep7601 2 weeks, 2 days ago
Just a kind reminder to check this patch, thank you!

Best regards,
NepNep7601

On 27/08/2026 03:44, NepNep7601 wrote:
> acquire_i2c_hw_engine() opens the DDC pins before setting up the
> hardware engine. If setup_engine() fails, the error path releases the
> engine but leaves the DDC pins open and the engine's DDC pointer set.
> 
> Subsequent attempts to open the pins then return
> GPIO_RESULT_ALREADY_OPENED, preventing further I2C transfers, EDID
> reads and hotplug detection on that port until reboot.
> 
> Mirror the normal teardown path by closing the DDC pins and clearing
> the pointer after releasing the engine.
> 
> Assisted-by: Codex:gpt-5
> Signed-off-by: NepNep7601 <neptune@imm0nv1nhtv.is-a.dev>
> ---
>   drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c b/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c
> index 05892ab4529f..e7a05494abab 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c
> @@ -485,6 +485,8 @@ struct dce_i2c_hw *acquire_i2c_hw_engine(
>   
>   	if (!setup_engine(dce_i2c_hw)) {
>   		release_engine(dce_i2c_hw);
> +		dal_ddc_close(dce_i2c_hw->ddc);
> +		dce_i2c_hw->ddc = NULL;
>   		return NULL;
>   	}
[PATCH 1/2] drm/amd/display: skip receiver power control without AUX
Posted by NepNep7601 1 month ago
Passive DP to TMDS dongles do not provide a DP receiver and use native
GPIO I2C rather than AUX. dpcd_write_rx_power_ctrl() nevertheless tries
to write DP_SET_POWER, causing the DP helpers to retry a transaction
that cannot succeed 32 times before giving up.

Skip receiver power control when the link is not using AUX mode.

On an Oland GPU with a passive DP to HDMI to DVI chain, this reduced
boot-time "DP AUX transfer fail" messages from 32 to 0. The EDID
remained 256 bytes and the display continued to use its native
1600x900 mode.

Assisted-by: Claude:claude-opus-5
Signed-off-by: NepNep7601 <neptune@imm0nv1nhtv.is-a.dev>
---
 .../gpu/drm/amd/display/dc/link/protocols/link_dp_phy.c  | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_phy.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_phy.c
index 49521ac4b0e8..7991531f6ef4 100644
--- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_phy.c
+++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_phy.c
@@ -50,6 +50,15 @@ void dpcd_write_rx_power_ctrl(struct dc_link *link, bool on)
 	if (link->sync_lt_in_progress)
 		return;
 
+	/*
+	 * A passive DP to TMDS dongle presents no DP receiver, so there is
+	 * nothing to power up or down. The write can only fail, and the DP
+	 * helpers retry it 32 times before giving up, which adds tens of
+	 * milliseconds to link bring up.
+	 */
+	if (!link->aux_mode)
+		return;
+
 	core_link_write_dpcd(link, DP_SET_POWER, &state,
 						 sizeof(state));
 
-- 
2.47.3
[PATCH 2/2] drm/amd/display: close DDC on I2C engine setup failure
Posted by NepNep7601 1 month ago
acquire_i2c_hw_engine() opens the DDC pins before setting up the
hardware engine. If setup_engine() fails, the error path releases the
engine but leaves the DDC pins open and the engine's DDC pointer set.

Subsequent attempts to open the pins then return
GPIO_RESULT_ALREADY_OPENED, preventing further I2C transfers, EDID
reads and hotplug detection on that port until reboot.

Mirror the normal teardown path by closing the DDC pins and clearing
the pointer after releasing the engine.

Assisted-by: Codex:gpt-5
Signed-off-by: NepNep7601 <neptune@imm0nv1nhtv.is-a.dev>
---
 drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c b/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c
index 05892ab4529f..e7a05494abab 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c
@@ -485,6 +485,8 @@ struct dce_i2c_hw *acquire_i2c_hw_engine(
 
 	if (!setup_engine(dce_i2c_hw)) {
 		release_engine(dce_i2c_hw);
+		dal_ddc_close(dce_i2c_hw->ddc);
+		dce_i2c_hw->ddc = NULL;
 		return NULL;
 	}
 
-- 
2.47.3