Skip to contentSkip to conversation

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

PATCH Superseded Frank Zhang 3 participants 3 messages May 12 – Sep 7

Subsystems (5)

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

PATCH v4 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.

Add a tmds_char_rate guard in dw_hdmi_qp_bridge_clear_audio_infoframe(). Decouple write_audio_infoframe from clear_audio_infoframe to avoid the redundant check in the write path.
Add PKTSCHED_AMD_TX_EN to the clear mask to keep the enable/disable balance.

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

---
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]/

---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)
drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c +9 -6
index d649a1cf07f5..1c18f8650fcd 100644
@@ -886,11 +886,11 @@ static int dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge)
 {
 	struct dw_hdmi_qp *hdmi = bridge->driver_private;
 
-	dw_hdmi_qp_mod(hdmi, 0,
-		       PKTSCHED_ACR_TX_EN |
-		       PKTSCHED_AUDS_TX_EN |
-		       PKTSCHED_AUDI_TX_EN,
-		       PKTSCHED_PKT_EN);
+	if (hdmi->tmds_char_rate)
+		dw_hdmi_qp_mod(hdmi, 0,
+			       PKTSCHED_ACR_TX_EN | PKTSCHED_AMD_TX_EN |
+			       PKTSCHED_AUDS_TX_EN | PKTSCHED_AUDI_TX_EN,
+			       PKTSCHED_PKT_EN);
 
 	return 0;
 }
@@ -989,7 +989,10 @@ 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_mod(hdmi, 0,
+		       PKTSCHED_ACR_TX_EN | PKTSCHED_AMD_TX_EN |
+		       PKTSCHED_AUDS_TX_EN | PKTSCHED_AUDI_TX_EN,
+		       PKTSCHED_PKT_EN);
 
 	/*
 	 * AUDI_CONTENTS0: { RSV, HB2, HB1, RSV }
Signature · 1 line
2.54.0
#1 lore raw

Hi Frank,

Quoted 77 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.

Add a tmds_char_rate guard in dw_hdmi_qp_bridge_clear_audio_infoframe(). Decouple write_audio_infoframe from clear_audio_infoframe to avoid the redundant check in the write path.
Add PKTSCHED_AMD_TX_EN to the clear mask to keep the enable/disable balance.

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

---
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]/

---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)
drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c +5 -5
index d649a1cf07f5..1c18f8650fcd 100644
@@ -886,11 +886,11 @@ static int dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge)
 {
 	struct dw_hdmi_qp *hdmi = bridge->driver_private;
 
-	dw_hdmi_qp_mod(hdmi, 0,
-		       PKTSCHED_ACR_TX_EN |
-		       PKTSCHED_AUDS_TX_EN |
-		       PKTSCHED_AUDI_TX_EN,
-		       PKTSCHED_PKT_EN);
+	if (hdmi->tmds_char_rate)
+		dw_hdmi_qp_mod(hdmi, 0,
+			       PKTSCHED_ACR_TX_EN | PKTSCHED_AMD_TX_EN |
+			       PKTSCHED_AUDS_TX_EN | PKTSCHED_AUDI_TX_EN,
+			       PKTSCHED_PKT_EN);
 
 	return 0;
 }

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

Quoted 8 lines
drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c +4 -1
@@ -989,7 +989,10 @@ 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_mod(hdmi, 0,
+		       PKTSCHED_ACR_TX_EN | PKTSCHED_AMD_TX_EN |
+		       PKTSCHED_AUDS_TX_EN | PKTSCHED_AUDI_TX_EN,
+		       PKTSCHED_PKT_EN);

Is "avoid the redundant check in the write path" the only reason of open-coding dw_hdmi_qp_bridge_clear_audio_infoframe()?

Performance wise, I don't think there are any real gains here, so we'd be better off reusing the existing code where possible.

Regards,
Cristian

#2 lore raw
In reply to Frank Zhang

Hi Frank, Cristian,

Independent hit of the same crash, in case it helps get this one moving again.

Board: Rockchip RK3588, Orange Pi 5 Plus. Kernel: 7.2.0-rc7 with the Collabora dw-hdmi-qp HDMI 2.1/FRL work on top (so not plain mainline, but the audio shutdown path is the same code). Userspace: PipeWire/WirePlumber.

At that boot the HDMI link never came up (the sink did not answer SCDC), so every snd_soc_dai_prepare() failed with -ENODEV, and about five seconds later WirePlumber closing the PCM took the same path your patch guards:

snd_pcm_release -> hdmi_codec_shutdown

    -> drm_connector_hdmi_audio_shutdown
    -> dw_hdmi_qp_audio_disable
    -> drm_atomic_helper_connector_hdmi_clear_audio_infoframe
    -> dw_hdmi_qp_bridge_clear_audio_infoframe
    -> regmap_update_bits (read) -> SError, panic

Serial console excerpt (7.2.0-rc7, tmds_char_rate was 0 at that point):

[   16.597316] hdmi-audio-codec hdmi-audio-codec.7.auto: ASoC error (-19): at snd_soc_dai_prepare() on i2s-hifi   (x4)
[   16.601847] SError Interrupt on CPU5, code 0x00000000be000011 -- SError
[   16.601852] CPU: 5 UID: 112 PID: 2821 Comm: wireplumber Tainted: G   M       OE       7.2.0-rc7-rk3588-igor-claude-hdr2+ #10 PREEMPT(lazy)
[   16.601862] pc : regmap_mmio_read32le+0x38/0xf0
[   16.601867] lr : regmap_mmio_read+0x50/0x98
[...]
[   16.601903] Call trace:
[   16.601904]  show_stack+0x24/0x50 (C)
[   16.601908]  dump_stack_lvl+0xe0/0x140
[   16.601912]  dump_stack+0x1c/0x38
[   16.601915]  vpanic+0x4e8/0x5b0
[   16.601919]  panic+0x6c/0x78
[   16.601921]  nmi_panic+0x8c/0x98
[   16.601924]  arm64_serror_panic+0x7c/0x98
[   16.601927]  arm64_is_fatal_ras_serror+0xa8/0xb0
[   16.601930]  do_serror+0x3c/0x78
[   16.601932]  el1h_64_error_handler+0x40/0x80
[   16.601937]  el1h_64_error+0x84/0x88
[   16.601938]  regmap_mmio_read32le+0x38/0xf0 (P)
[   16.601941]  regmap_mmio_read+0x50/0x98
[   16.601943]  _regmap_bus_reg_read+0x68/0xd0
[   16.601947]  _regmap_read+0x80/0x2a8
[   16.601950]  _regmap_update_bits+0x13c/0x1e0
[   16.601953]  regmap_update_bits_base+0x70/0xd0
[   16.601956]  dw_hdmi_qp_bridge_clear_audio_infoframe+0x3c/0x78 [dw_hdmi_qp]
[   16.601964]  drm_bridge_connector_clear_audio_infoframe+0x2c/0x68 [drm_display_helper]
[   16.601993]  clear_infoframe+0x5c/0x128 [drm_display_helper]
[   16.602016]  drm_atomic_helper_connector_hdmi_clear_audio_infoframe+0x70/0x110 [drm_display_helper]
[   16.602039]  dw_hdmi_qp_audio_disable+0x28/0x108 [dw_hdmi_qp]
[   16.602042]  drm_bridge_connector_audio_shutdown+0x38/0x80 [drm_display_helper]
[   16.602065]  drm_connector_hdmi_audio_shutdown+0x28/0x50 [drm_display_helper]
[   16.602086]  hdmi_codec_shutdown+0x68/0xb0 [snd_soc_hdmi_codec]
[   16.602091]  snd_soc_dai_shutdown+0x60/0xe0 [snd_soc_core]
[   16.602123]  soc_pcm_clean.isra.0+0x5c/0x1f8 [snd_soc_core]
[   16.602151]  soc_pcm_close+0x40/0x80 [snd_soc_core]
[   16.602177]  snd_pcm_release_substream.part.0+0x48/0xf0 [snd_pcm]
[   16.602193]  snd_pcm_release+0x68/0x110 [snd_pcm]
[   16.602205]  __fput+0xe4/0x340
[   16.602209]  fput_close_sync+0x4c/0x138
[   16.602211]  __arm64_sys_close+0x44/0xa0
[   16.602215]  invoke_syscall+0xa8/0x138
[   16.602217]  el0_svc_common.constprop.0+0x114/0x140
[   16.602219]  do_el0_svc+0x28/0x58
[   16.602222]  el0_svc+0x48/0x310
[   16.602225]  el0t_64_sync_handler+0xc0/0x108

So the asymmetry is exactly what you describe: audio_enable/prepare check hdmi->tmds_char_rate, audio_disable does not, and the read-modify-write in clear_audio_infoframe hits the block with the PHY down.

Today I could reproduce it on demand, with the in-tree module of the same
kernel: disable one HDMI output in the compositor (KWin, so atomic_disable() runs: tmds_char_rate = 0, PHY off), then

aplay -D plughw:hdmi1,0 /usr/share/sounds/alsa/Front_Center.wav

open succeeds, prepare logs the usual "ASoC error (-19)", and the close that follows dies in the same place, this time as a synchronous abort:

  Internal error: synchronous external abort: 0000000096000010 [#1] SMP
  pc : regmap_mmio_read32le+0x30/0xf0
  ...
  dw_hdmi_qp_bridge_clear_audio_infoframe+0x3c/0x78 [dw_hdmi_qp]
  drm_bridge_connector_clear_audio_infoframe+0x2c/0x68 [drm_display_helper]
  drm_atomic_helper_connector_hdmi_clear_audio_infoframe+0x70/0x110 [drm_display_helper]
  dw_hdmi_qp_audio_disable+0x28/0x108 [dw_hdmi_qp]
  hdmi_codec_shutdown+0x68/0xb0 [snd_soc_hdmi_codec]
  snd_soc_dai_shutdown+0x60/0xe0 [snd_soc_core]
  soc_pcm_close+0x40/0x80 [snd_soc_core]
  snd_pcm_release+0x68/0x110 [snd_pcm]
  __arm64_sys_close+0x44/0xa0
  note: aplay[52181] exited with irqs disabled
  note: aplay[52181] exited with preempt_count 1

(x19 = 0xffff800081ca0aa8, i.e. PKTSCHED_PKT_EN.) One more consequence of the non-fatal variant that may be worth a line in the changelog: the task dies while snd_pcm_release() is still holding the PCM's open_mutex, so every later open() of that PCM blocks in D state until reboot - the card is gone for the session. The reboot after that hung too (soft lockup in rcu_exp_gp_kthread, a CPU never answering the NMI - I don't have a clean causal chain for that second hang) and ended in a hard reset.

I read Cristian's comment on v4 - if you post a v5 that reuses dw_hdmi_qp_bridge_clear_audio_infoframe() from the write path instead of open-coding it, I am happy to run it on this board and reply with a Tested-by. I have a serial console attached, so the negative case (PCM close after the link failed to train) is something I can exercise here.

Thanks,
Igor