drm/bridge: dw-hdmi-qp: Guard clear_audio_infoframe when PHY is down

PATCH In review Frank Zhang 4 participants 6 messages Sep 8 – Sep 9

Subsystems (5)

Stable branch DRM drivers DRM misc DRM drivers for bridge chips +1 more subsystems

lore

Share & tools

lore

Frank Zhang

author

2026-09-08 07:02 UTC #0 lore raw

PATCH v5 1/1 drm/bridge: dw-hdmi-qp: Guard clear_audio_infoframe when PHY is down

The following panic was observed during system reboot:

Kernel panic - not syncing: Asynchronous SError Interrupt
CPU: 6 UID: 1000 PID: 2348 Comm: pipewire ... 7.0.5+ #4 PREEMPT(full)
Call trace:
 ...
 regmap_update_bits_base+0x70/0xa8
 dw_hdmi_qp_bridge_clear_audio_infoframe+0x3c/0x58 [dw_hdmi_qp]
 drm_bridge_connector_clear_audio_infoframe+0x2c/0x48 [drm_display_helper]
 ...
 dw_hdmi_qp_audio_disable+0x28/0xa8 [dw_hdmi_qp]
 drm_bridge_connector_audio_shutdown+0x38/0x68 [drm_display_helper]
 drm_connector_hdmi_audio_shutdown+0x28/0x40 [drm_display_helper]
 hdmi_codec_shutdown+0x60/0x90 [snd_soc_hdmi_codec]
 ...
 snd_pcm_release_substream+0xcc/0x120 [snd_pcm]
 snd_pcm_release+0x4c/0xc0 [snd_pcm]
 ...

The root cause is pipewire tries to close the HDMI audio device after atomic_disable(), which sets tmds_char_rate to 0 and disables the PHY.

In this case, dw_hdmi_qp_audio_disable() will call dw_hdmi_qp_bridge_clear_audio_infoframe(), accessing register without checking tmds_char_rate.

Guard the register access in drm_display_helper callback function with tmds_char_rate check.

Fixes fd0141d1a8a2 ("drm/bridge: synopsys: Add audio support for dw-hdmi-qp")

Cc [email protected]

Signed-off-by Frank Zhang <[email protected]>

---
Changes in v2:
- Move drm_atomic_helper_connector_hdmi_clear_audio_infoframe() inside the if (hdmi->tmds_char_rate) of dw_hdmi_qp_audio_disable().
- Link to v1: https://lore.kernel.org/all/[email protected]/

Changes in v3:
- Add a tmds_char_rate guard in clear_audio_infoframe path.
- Decouple write_audio_infoframe from clear_audio_infoframe.
- Balance the PKTSCHED_AMD_TX_EN bit enable/disable.
- Link to v2: https://lore.kernel.org/all/[email protected]/

Changes in v4:
- Update panic stack on 7.0.5
- Link to v3: https://lore.kernel.org/all/[email protected]/

Changes in v5:
- Reuse the origin dw_hdmi_qp_bridge_clear_audio_infoframe and add tmds_char_rate guard in new callback function.
- Link to v4: https://lore.kernel.org/all/[email protected]/

---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c +12 -2

index 1c214a8e6dc2..8729783d4eb6 100644
@@ -880,7 +880,7 @@ static int dw_hdmi_qp_bridge_clear_spd_infoframe(struct drm_bridge *bridge)
 	return 0;
 }
 
-static int dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge)
+static int __dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge)
 {
 	struct dw_hdmi_qp *hdmi = bridge->driver_private;

@@ -893,6 +893,16 @@ static int dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge)
 	return 0;
 }
 
+static int dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge)
+{
+	struct dw_hdmi_qp *hdmi = bridge->driver_private;
+
+	if (hdmi->tmds_char_rate > 0)
+		return __dw_hdmi_qp_bridge_clear_audio_infoframe(bridge);
+
+	return 0;
+}
+
 static void dw_hdmi_qp_write_pkt(struct dw_hdmi_qp *hdmi, const u8 *buffer,
 				 size_t start, size_t len, unsigned int reg)
 {

@@ -987,7 +997,7 @@ static int dw_hdmi_qp_bridge_write_audio_infoframe(struct drm_bridge *bridge,
 {
 	struct dw_hdmi_qp *hdmi = bridge->driver_private;
 
-	dw_hdmi_qp_bridge_clear_audio_infoframe(bridge);
+	__dw_hdmi_qp_bridge_clear_audio_infoframe(bridge);
 
 	/*
 	 * AUDI_CONTENTS0: { RSV, HB2, HB1, RSV }

Signature · 1 line

2.55.0

[email protected]

AI review

2026-09-08 07:15 UTC #1 lore raw

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The lockless if (hdmi->tmds_char_rate > 0) check introduces a Time-Of-Check to Time-Of-Use (TOCTOU) data race with the DRM atomic commit thread, failing to fully prevent the SError panic.

Pre-existing issues:
- [High] tmds_char_rate is assigned before the PHY is actually initialized in dw_hdmi_qp_bridge_atomic_enable, allowing concurrent audio operations to crash the kernel by accessing unpowered hardware. --

commit 9d8465ca2470e46cc48135ada91995d64bb0dec1
Author: Frank Zhang <[email protected]>

drm/bridge: dw-hdmi-qp: Guard clear_audio_infoframe when PHY is down

This patch attempts to prevent an Asynchronous SError Interrupt kernel panic by checking hdmi->tmds_char_rate before accessing PHY registers during ALSA audio shutdown. It introduces a lockless check to decouple the audio infoframe operations from powered-down hardware states.

Link https://lore.kernel.org/all/[email protected]/ open here

Quoted 4 lines

drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c +0 -0
index cfd1b18c06e62..f0e9324249d66 100644

[ ... ]

Quoted 4 lines

drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c +0 -0

@@ -893,6 +893,16 @@ static int dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge)
 	return 0;
 }

drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c static int dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge) · line 905

Quoted 10 lines

drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c +10 -0

+static int dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge)
+{
+	struct dw_hdmi_qp *hdmi = bridge->driver_private;
+
+	if (hdmi->tmds_char_rate > 0)
+		return __dw_hdmi_qp_bridge_clear_audio_infoframe(bridge);
+
+	return 0;
+}
+

[Severity: High]
Does this lockless check introduce a Time-Of-Check to Time-Of-Use race?

If a userspace process closes the ALSA audio stream concurrently with a DRM atomic disable, could the DRM commit thread clear the tmds_char_rate state variable and disable the PHY hardware right after this check passes, but before __dw_hdmi_qp_bridge_clear_audio_infoframe() executes?

For example:

ALSA thread in dw_hdmi_qp_bridge_clear_audio_infoframe():

    if (hdmi->tmds_char_rate > 0) // passes

DRM thread in dw_hdmi_qp_bridge_atomic_disable():

    hdmi->tmds_char_rate = 0;
    hdmi->phy.ops->disable(hdmi, hdmi->phy.data);

ALSA thread continues:

    return __dw_hdmi_qp_bridge_clear_audio_infoframe(bridge);

This would result in writing to the disabled PHY and causing the same SError panic this patch intends to fix. Would it be safer to use a mutex to serialize audio operations with PHY state changes?

[Severity: High]
This is a pre-existing issue, but looking at how tmds_char_rate is used as a proxy for hardware readiness, could concurrent audio operations crash the kernel during a DRM atomic enable?

In dw_hdmi_qp_bridge_atomic_enable(), tmds_char_rate is assigned a non-zero value before the hardware is actually powered on:

dw_hdmi_qp_bridge_atomic_enable():

    ...
    hdmi->tmds_char_rate = conn_state->hdmi.tmds_char_rate;
    hdmi->phy.ops->init(hdmi, hdmi->phy.data);

If an ALSA thread concurrently executes dw_hdmi_qp_audio_prepare() or dw_hdmi_qp_bridge_write_audio_infoframe(), couldn't it observe tmds_char_rate > 0 and proceed to access PHY registers before they are powered on?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Sebastian Reichel

2026-09-08 12:11 UTC #2 lore raw

In reply to Frank Zhang

Hi,

Quoted 88 lines · Frank Zhang

The following panic was observed during system reboot:
Kernel panic - not syncing: Asynchronous SError Interrupt
CPU: 6 UID: 1000 PID: 2348 Comm: pipewire ... 7.0.5+ #4 PREEMPT(full)
Call trace:
 ...
 regmap_update_bits_base+0x70/0xa8
 dw_hdmi_qp_bridge_clear_audio_infoframe+0x3c/0x58 [dw_hdmi_qp]
 drm_bridge_connector_clear_audio_infoframe+0x2c/0x48 [drm_display_helper]
 ...
 dw_hdmi_qp_audio_disable+0x28/0xa8 [dw_hdmi_qp]
 drm_bridge_connector_audio_shutdown+0x38/0x68 [drm_display_helper]
 drm_connector_hdmi_audio_shutdown+0x28/0x40 [drm_display_helper]
 hdmi_codec_shutdown+0x60/0x90 [snd_soc_hdmi_codec]
 ...
 snd_pcm_release_substream+0xcc/0x120 [snd_pcm]
 snd_pcm_release+0x4c/0xc0 [snd_pcm]
 ...
The root cause is pipewire tries to close the HDMI audio device after atomic_disable(), which sets tmds_char_rate to 0 and disables the PHY.
In this case, dw_hdmi_qp_audio_disable() will call dw_hdmi_qp_bridge_clear_audio_infoframe(), accessing register without checking tmds_char_rate.
Guard the register access in drm_display_helper callback function with tmds_char_rate check.
Fixes fd0141d1a8a2 ("drm/bridge: synopsys: Add audio support for dw-hdmi-qp")
Cc [email protected]
Signed-off-by Frank Zhang <[email protected]>
---
Changes in v2:
- Move drm_atomic_helper_connector_hdmi_clear_audio_infoframe() inside the if (hdmi->tmds_char_rate) of dw_hdmi_qp_audio_disable().
- Link to v1: https://lore.kernel.org/all/[email protected]/
Changes in v3:
- Add a tmds_char_rate guard in clear_audio_infoframe path.
- Decouple write_audio_infoframe from clear_audio_infoframe.
- Balance the PKTSCHED_AMD_TX_EN bit enable/disable.
- Link to v2: https://lore.kernel.org/all/[email protected]/
Changes in v4:
- Update panic stack on 7.0.5
- Link to v3: https://lore.kernel.org/all/[email protected]/
Changes in v5:
- Reuse the origin dw_hdmi_qp_bridge_clear_audio_infoframe and add tmds_char_rate guard in new callback function.
- Link to v4: https://lore.kernel.org/all/[email protected]/
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c +11 -1
index 1c214a8e6dc2..8729783d4eb6 100644
@@ -880,7 +880,7 @@ static int dw_hdmi_qp_bridge_clear_spd_infoframe(struct drm_bridge *bridge)
 	return 0;
 }
 
-static int dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge)
+static int __dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge)
 {
 	struct dw_hdmi_qp *hdmi = bridge->driver_private;
 
@@ -893,6 +893,16 @@ static int dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge)
 	return 0;
 }
 
+static int dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge)
+{
+	struct dw_hdmi_qp *hdmi = bridge->driver_private;
+
+	if (hdmi->tmds_char_rate > 0)
+		return __dw_hdmi_qp_bridge_clear_audio_infoframe(bridge);
+
+	return 0;
+}
+
 static void dw_hdmi_qp_write_pkt(struct dw_hdmi_qp *hdmi, const u8 *buffer,
 				 size_t start, size_t len, unsigned int reg)
 {

drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c static int dw_hdmi_qp_bridge_write_audio_infoframe(struct drm_bridge *bridge, · line 1000

Quoted 5 lines

drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c +1 -1

@@ -987,7 +997,7 @@ static int dw_hdmi_qp_bridge_write_audio_infoframe(struct drm_bridge *bridge,
 {
 	struct dw_hdmi_qp *hdmi = bridge->driver_private;
 
-	dw_hdmi_qp_bridge_clear_audio_infoframe(bridge);
+	__dw_hdmi_qp_bridge_clear_audio_infoframe(bridge);

Why? Having the extra tmds_char_rate check being done here is not a problem except for some extra instructions. Considering this is definetly not a hot path, introducing the extra function is not worth the trouble. Just patch the original function to add the check resulting in a nice and simple 2 lines fix and you are done.

Quoted 6 lines

drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c +0 -0
 	/*
 	 * AUDI_CONTENTS0: { RSV, HB2, HB1, RSV }
Signature · 1 line
2.55.0

Greetings,

-- Sebastian

Igor Paunovic

2026-09-08 13:36 UTC #3 lore raw

Hi Frank, Sebastian,

Just so the tag is not lost when the split goes away: what I tested and what my Tested-by is about is the tmds_char_rate check on the path that runs from hdmi_codec_shutdown(), not the introduction of the second function. With ftrace I confirmed the check is what stops the register access, so folding it back into the original function as Sebastian suggests does not change anything I measured, and the tag applies to that form as well.

If you would still rather have it re-tested on the simplified version, send v6 and I will run the same reproducer and confirm.

Igor

Frank Zhang

author

2026-09-09 01:39 UTC #4 lore raw

In reply to Sebastian Reichel

Quoted 94 lines · Sebastian Reichel

Hi,
Quoted 91 lines · Frank Zhang
The following panic was observed during system reboot:
Kernel panic - not syncing: Asynchronous SError Interrupt
CPU: 6 UID: 1000 PID: 2348 Comm: pipewire ... 7.0.5+ #4 PREEMPT(full)
Call trace:
  ...
  regmap_update_bits_base+0x70/0xa8
  dw_hdmi_qp_bridge_clear_audio_infoframe+0x3c/0x58 [dw_hdmi_qp]
  drm_bridge_connector_clear_audio_infoframe+0x2c/0x48 [drm_display_helper]
  ...
  dw_hdmi_qp_audio_disable+0x28/0xa8 [dw_hdmi_qp]
  drm_bridge_connector_audio_shutdown+0x38/0x68 [drm_display_helper]
  drm_connector_hdmi_audio_shutdown+0x28/0x40 [drm_display_helper]
  hdmi_codec_shutdown+0x60/0x90 [snd_soc_hdmi_codec]
  ...
  snd_pcm_release_substream+0xcc/0x120 [snd_pcm]
  snd_pcm_release+0x4c/0xc0 [snd_pcm]
  ...
The root cause is pipewire tries to close the HDMI audio device after atomic_disable(), which sets tmds_char_rate to 0 and disables the PHY.
In this case, dw_hdmi_qp_audio_disable() will call dw_hdmi_qp_bridge_clear_audio_infoframe(), accessing register without checking tmds_char_rate.
Guard the register access in drm_display_helper callback function with tmds_char_rate check.
Fixes fd0141d1a8a2 ("drm/bridge: synopsys: Add audio support for dw-hdmi-qp")
Cc [email protected]
Signed-off-by Frank Zhang <[email protected]>
---
Changes in v2:
- Move drm_atomic_helper_connector_hdmi_clear_audio_infoframe() inside the if (hdmi->tmds_char_rate) of dw_hdmi_qp_audio_disable().
- Link to v1: https://lore.kernel.org/all/[email protected]/
Changes in v3:
- Add a tmds_char_rate guard in clear_audio_infoframe path.
- Decouple write_audio_infoframe from clear_audio_infoframe.
- Balance the PKTSCHED_AMD_TX_EN bit enable/disable.
- Link to v2: https://lore.kernel.org/all/[email protected]/
Changes in v4:
- Update panic stack on 7.0.5
- Link to v3: https://lore.kernel.org/all/[email protected]/
Changes in v5:
- Reuse the origin dw_hdmi_qp_bridge_clear_audio_infoframe and add tmds_char_rate guard in new callback function.
- Link to v4: https://lore.kernel.org/all/[email protected]/
---
  drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 14 ++++++++++++--
  1 file changed, 12 insertions(+), 2 deletions(-)
drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c +11 -1
index 1c214a8e6dc2..8729783d4eb6 100644
@@ -880,7 +880,7 @@ static int dw_hdmi_qp_bridge_clear_spd_infoframe(struct drm_bridge *bridge)
  	return 0;
  }
 
-static int dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge)
+static int __dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge)
  {
  	struct dw_hdmi_qp *hdmi = bridge->driver_private;
 
@@ -893,6 +893,16 @@ static int dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge)
  	return 0;
  }
 
+static int dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge)
+{
+	struct dw_hdmi_qp *hdmi = bridge->driver_private;
+
+	if (hdmi->tmds_char_rate > 0)
+		return __dw_hdmi_qp_bridge_clear_audio_infoframe(bridge);
+
+	return 0;
+}
+
  static void dw_hdmi_qp_write_pkt(struct dw_hdmi_qp *hdmi, const u8 *buffer,
  				 size_t start, size_t len, unsigned int reg)
  {
@@ -987,7 +997,7 @@ static int dw_hdmi_qp_bridge_write_audio_infoframe(struct drm_bridge *bridge,
  {
  	struct dw_hdmi_qp *hdmi = bridge->driver_private;
 

Quoted 8 lines

drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c static int dw_hdmi_qp_bridge_write_audio_infoframe(struct drm_bridge *bridge, · line 1000
Quoted 2 lines
drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c +1 -1
-	dw_hdmi_qp_bridge_clear_audio_infoframe(bridge);
+	__dw_hdmi_qp_bridge_clear_audio_infoframe(bridge);
Why? Having the extra tmds_char_rate check being done here is not a problem except for some extra instructions. Considering this is definetly not a hot path, introducing the extra function is not worth the trouble. Just patch the original function to add the check resulting in a nice and simple 2 lines fix and you are done.

Add tmds_char_rate check in the original function dw_hdmi_qp_bridge_clear_audio_infoframe discussed in v1, Detlev Casanova recommend no need to check for tmds_char_rate in every dw_hdmi_qp_bridge_clear_audio_infoframe() call.>

Quoted 10 lines

Quoted 6 lines
drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c +0 -0
  	/*
  	 * AUDI_CONTENTS0: { RSV, HB2, HB1, RSV }
Signature · 1 line
2.55.0
Greetings,
-- Sebastian

Igor Paunovic

Tested-by on v5 1/1

2026-09-08 12:37 UTC #5 lore raw

In reply to Frank Zhang

Hi Frank,

Thank you for sending v5 so quickly.

I tested it on an Orange Pi 5 Plus (RK3588) against the exact case I reported in [1]. v5 applied cleanly to my 7.2.0-rc7 based tree with no context fixup, which v4 still needed here.

Reproducer, unchanged from [1]: the compositor turns the HDMI sink off (so tmds_char_rate is 0 and the PHY is down), then plughw:hdmi1,0 is opened and closed. Before, that took the machine down on close, in dw_hdmi_qp_bridge_clear_audio_infoframe() -> regmap_update_bits_base() -> _regmap_read() -> regmap_mmio_read32le(), either as a synchronous external abort in the closing task or as an asynchronous SError panic. The task died with interrupts disabled, so the codec lock stayed held and every later open of that PCM hung in D state until reboot.

With v5 applied, three consecutive open/close cycles in that state all behave identically: prepare fails with -ENODEV as before (3 x "ASoC error (-19) at snd_soc_dai_prepare()"), no external abort, no SError, the shutdown path completes, and the PCM can be opened again afterwards.

I also confirmed with ftrace that the guard is what stops it, rather than the path simply not being reached. Tracing dw_hdmi_qp_bridge_clear_audio_infoframe with function_graph:

- sink off (tmds_char_rate == 0): the function is entered and returns as a leaf, 2.9 us, with no calls inside it at all.

- sink on, as a positive control: the same function shows regmap_update_bits_base() -> _regmap_update_bits() -> _regmap_read() / _regmap_write() nested inside - the same frames the crash walked through - so the tracer does see the body, and "empty" in the first case really is the tmds_char_rate check taking effect.

Tested-by Igor Paunovic <[email protected]> # Orange Pi 5 Plus (RK3588)

Two notes, so this is not read as more than it is.

First, what I exercised is the sequential case: the display is already off before the audio device is closed. I did not try to hit the narrow window the automated review raised in this thread, where the atomic disable lands between the tmds_char_rate check and the register access, so my test says nothing about that race either way.

Second, for whoever picks this up: Detlev Casanova's patch [2] covers the other half of the same problem on this hardware - the enable/prepare side returning -EOPNOTSUPP when there is no link - and has three Tested-by tags. The two are complementary here. His cuts the case where the PCM is opened while the sink is already off; yours covers the case where the PCM is already open and the output goes away underneath it. Taking only one of them still leaves a way to reach the crash. With both applied together on this board the path is quiet and the -19 messages are gone as well.

[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/

Igor