Re: [PATCH] drm/bridge: dw-hdmi-i2s: support the S/PDIF audio input

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/bridge: dw-hdmi-i2s: duplicate Capture DAPM widgets when spdif is enabled
--

commit 566412f94af4997a1eb5d5114c6f602dc558da39
Author: Christian Stefan Kövér-Draxl <ckoever@outlook.de>

drm/bridge: dw-hdmi-i2s: support the S/PDIF audio input

This commit adds support for the S/PDIF audio input on the DesignWare HDMI TX
controller by allowing platforms to set spdif_input in dw_hdmi_plat_data,
registering the hdmi-codec S/PDIF DAI when enabled, and configuring the audio
sampler for S/PDIF in hw_params.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
> index 2c903c9fe8052..304ef881e06c8 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
[ ... ]
> @@ -197,6 +244,7 @@ static int snd_dw_hdmi_probe(struct platform_device *pdev)
>  	memset(&pdata, 0, sizeof(pdata));
>  	pdata.ops		= &dw_hdmi_i2s_ops;
>  	pdata.i2s		= 1;
> +	pdata.spdif		= audio->spdif;
>  	pdata.max_i2s_channels	= 8;
>  	pdata.data		= audio;

[Severity: Medium]
When plat_data->spdif_input is true, dw_hdmi_probe() sets audio.spdif = true,
and snd_dw_hdmi_probe() in drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
registers hdmi-audio-codec with both pdata.i2s and pdata.spdif set while
leaving pdata.no_i2s_capture and pdata.no_spdif_capture set to 0.

In sound/soc/codecs/hdmi-codec.c:hdmi_codec_probe(), both hdmi_i2s_dai and
hdmi_spdif_dai retain their default capture stream definition
(.capture.stream_name = "Capture") unless no_i2s_capture and no_spdif_capture
are set:

	if (hcd->i2s) {
		daidrv[i] = hdmi_i2s_dai;
		...
		if (hcd->no_i2s_capture)
			memset(&daidrv[i].capture, 0,
			       sizeof(daidrv[i].capture));
		i++;
	}

	if (hcd->spdif) {
		daidrv[i] = hdmi_spdif_dai;
		...
		if (hcd->no_spdif_playback)
			memset(&daidrv[i].playback, 0,
			       sizeof(daidrv[i].playback));
		if (hcd->no_spdif_capture)
			memset(&daidrv[i].capture, 0,
			       sizeof(daidrv[i].capture));
	}

When the sound card probes the component via
soc_probe_component()->snd_soc_dapm_new_dai_widgets(), does having two DAIs
with the same capture stream name "Capture" on the same component create two
duplicate "Capture" snd_soc_dapm_dai_out widgets in the same DAPM context,
triggering a debugfs file collision error ("debugfs: File 'Capture' in
directory 'dapm' already present!") in
snd_soc_dapm_new_widgets()->dapm_debugfs_add_widget() and causing duplicate
"RX" -> "Capture" DAPM routes to resolve to the first DAI's widget?

Since the DesignWare HDMI TX controller is transmit-only, should
snd_dw_hdmi_probe() also set pdata.no_i2s_capture = 1 and
pdata.no_spdif_capture = 1, matching other HDMI TX drivers that enable both
I2S and S/PDIF such as drm_connector_hdmi_audio_init() and
tda998x_audio_codec_init()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261025192436.209908-1-christian.koever-draxl@student.uibk.ac.at?part=1