Re: [PATCH 08/20] ASoC: soc-pcm.c: cleanup soc_get_playback_capture()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/23/2023 1:49 AM, Kuninori Morimoto wrote:

Hi Amadeusz

static int soc_get_playback_capture(...)
{
	...
(A)	if (dai_link->dynamic || dai_link->no_pcm) {
		...
		if (dai_link->dpcm_playback) {
			...
(B)			for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
				...
			}
			...
		}
		if (dai_link->dpcm_capture) {
			...
(B)			for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
				...
			}
		}
		...
	}
}

It checks CPU (B) when no_pcm (A) on original.
But I think "no_pcm - CPU" is dummy DAI -> above check is no meaning.
After the patch, it checks both CPU/Codec.
(snip)
I wonder is your Codec which is connected to no_pcm DAI valid ?

I'm not sure what do you mean, if it is valid? It works fine before this
patch ;)

Ah, sorry, let me explain detail.

	static int soc_get_playback_capture(...)
	{
		...
(A)		if (dai_link->dynamic || dai_link->no_pcm) {
			int stream;

			if (dai_link->dpcm_playback) {
				stream = SNDRV_PCM_STREAM_PLAYBACK;

(B)				for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
(C)					if (snd_soc_dai_stream_valid(cpu_dai, stream)) {
						has_playback = 1;
						break;
					}
				}
			...
	}

Before appling the patch, in DPCM case (A), it checks CPU valid only (B)/(C).
In many case, DPCM is like this

	dynamic			no_pcm
	[CPU/dummy-Codec] - [dummy-CPU/Codec]

I noticed that before the patch, it checks dummy DAI only for no_pcm case.
But after appling the patch, it will check both CPU and Codec
valid (X)/(Y)/(Z).

I think this was changed after patch.
So, my question was what happen for snd_soc_dai_stream_valid() on your Codec.

# I notcied that avs_create_dai_links() is not using dummy CPU on no_pcm case...


	static int soc_get_playback_capture(...)
	{
		...
(X)		for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
(Y)			codec_dai = asoc_rtd_to_codec(rtd, i); /* get paired codec */

(Z)			if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK) &&
(Z)			    snd_soc_dai_stream_valid(cpu_dai,   cpu_playback))
				has_playback = 1;
(Z)			if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE) &&
(Z)			    snd_soc_dai_stream_valid(cpu_dai,   cpu_capture))
				has_capture = 1;
		}
		...
	}

Thank you for your help !!


Hi,
sorry for small delay, I've debugged the issue. Seems like one less critical problem is in ASoC HDA codec, which blocks HDA and HDMI probing altogether in our driver, something like this should fix this:

commit ed93e4fbc045b8da7906484a9c88e6b53864949b (HEAD)
Author: Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx>
Date:   Wed May 24 20:54:44 2023 +0200

    ASoC: codecs: hda: Fix probe codec definition

In order to properly bind snd_soc_dai_driver its snd_soc_pcm_stream need
    to provide channels_min value, otherwise ASoC framework may think that
    stream is improper.

    Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx>

diff --git a/sound/soc/codecs/hda.c b/sound/soc/codecs/hda.c
index d57b043d6bfe..62fd99f530bf 100644
--- a/sound/soc/codecs/hda.c
+++ b/sound/soc/codecs/hda.c
@@ -341,6 +341,8 @@ static const struct snd_soc_dapm_widget hda_dapm_widgets[] = {
 static struct snd_soc_dai_driver card_binder_dai = {
        .id = -1,
        .name = "codec-probing-DAI",
+       .capture.channels_min = 1,
+       .playback.channels_min = 1,
 };

 static int hda_hdev_attach(struct hdac_device *hdev)


With the above fixed, there is issue with how HDMI is being initialized and I consider it a blocker. So it needs to be fixed first before this patchset can be merged. I did simple patch like:

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 64a944016c78..e84c3e46557e 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -2317,6 +2317,7 @@ static int generic_hdmi_build_pcms(struct hda_codec *codec)
                if (spec->pcm_used >= ARRAY_SIZE(spec->pcm_rec))
                        break;
                /* other pstr fields are set in open */
+               pstr->channels_min = 1;
        }

        return 0;

and it works for testing purposes, but as commented in code, those fields are initialized later (in hdmi_pcm_open()), which apparently in case of ASoC binding is too late. Likely those assignments need to be moved earlier.

Another thing that should probably be done is some kind of look through ASoC codec files to make sure that they all define channels_min properly, otherwise there may be more unexpected failures.

Thanks,
Amadeusz



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux