Re: [PATCH v2] drm/bridge: dw-hdmi-qp: Guard clear_audio_infoframe when PHY is down

[Frank Zhang <rmxpzlb@gmail.com>]

On 4/19/26 08:40, Dmitry Baryshkov wrote:
> On Sat, Apr 18, 2026 at 06:19:36PM +0800, Frank Zhang wrote:
>> The following panic was observed during system reboot:
>>
>> Kernel panic - not syncing: Asynchronous SError Interrupt
>> CPU: 7 UID: 1000 PID: 2637 Comm: pipewire ... 6.19.10-300.fc44.aarch64
>> Call trace:
>>   ...
>>   regmap_update_bits_base+0x5c/0x90
>>   dw_hdmi_qp_bridge_clear_infoframe+0xb0/0x120 [dw_hdmi_qp]
>>   drm_bridge_connector_clear_infoframe+0x28/0x48 [drm_display_helper]
>>   ...
>>   dw_hdmi_qp_audio_disable+0x24/0xb8 [dw_hdmi_qp]
>>   drm_bridge_connector_audio_shutdown+0x30/0x60 [drm_display_helper]
>>   drm_connector_hdmi_audio_shutdown+0x24/0x38 [drm_display_helper]
>>   hdmi_codec_shutdown+0x60/0x90 [snd_soc_hdmi_codec]
>>   ...
>>   snd_pcm_release_substream.part.0+0x44/0xd8 [snd_pcm]
>>   snd_pcm_release+0x60/0xe8 [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 disable the PHY.
>>
>> In this case, dw_hdmi_qp_audio_disable() will call
>> drm_atomic_helper_connector_hdmi_clear_audio_infoframe() directly,
>> accessing registers without checking tmds_char_rate.
>>
>> Move drm_atomic_helper_connector_hdmi_clear_audio_infoframe() inside the
>> if (hdmi->tmds_char_rate) of dw_hdmi_qp_audio_disable().
>>
>> Fixes: fd0141d1a8a2 ("drm/bridge: synopsys: Add audio support for dw-hdmi-qp")
>> Signed-off-by: Frank Zhang <rmxpzlb@gmail.com>
>>
>> ---
>> 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/20260416093150.13853-1-rmxpzlb@gmail.com/
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
>> index d649a1cf07f5..7760527484c8 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
>> @@ -526,10 +526,10 @@ static void dw_hdmi_qp_audio_disable(struct drm_bridge *bridge,
>>   {
>>   	struct dw_hdmi_qp *hdmi = dw_hdmi_qp_from_bridge(bridge);
>>   
>> -	drm_atomic_helper_connector_hdmi_clear_audio_infoframe(connector);
>> -
>> -	if (hdmi->tmds_char_rate)
>> +	if (hdmi->tmds_char_rate) {
>> +		drm_atomic_helper_connector_hdmi_clear_audio_infoframe(connector);
>>   		dw_hdmi_qp_audio_disable_regs(hdmi);
>> +	}
> 
> Will audio and audio infoframe remain disabled after consequetive
> atomic_enable() call?
> 
>>   }
>>   
>>   static int dw_hdmi_qp_i2c_read(struct dw_hdmi_qp *hdmi,
>> -- 
>> 2.53.0
>>
> 

Sorry, I missed clearing the audio infoframe when the PHY is down. The 
next atomic_enable() will write the stale audio infoframe. My mistake.

To clear the stale audio infoframe, dw_hdmi_qp_audio_disable() can 
handle it in the else branch directly, but this seems like a layering 
violation for a bridge driver

I think the better approach is to add a 'reset_audio_infoframe' 
interface in drm_hdmi_state_helper.c that does basically the same as 
drm_atomic_helper_connector_hdmi_clear_audio_infoframe(), but only 
clearing the software state without calling clear_infoframe(). It's also 
a bit odd since it would only be used by dw-hdmi-qp.

I'd like to get the maintainers' opinion about adding such an interface.

Thanks,
Frank Zhang