multiple SSPs/BEs support for SOF--Kernel Part

[keyonjie]

We need SOFT part merged together, that is stream_name rename for nocodec topology to make nocodec topology works.

[plbossart]

I don't really understand what this patchset fixes compared to the previous version, this is still somewhat messy in terms of link declaration, setup and handling of multiple outputs.

[plbossart]

8 ch?

[keyonjie]

it original from Liam, And it is the maximum of cpu_dai caps, it can still be reduced by the FE/PCM caps:
hw->channels_min = max(chan_min, cpu_stream->channels_min);
hw->channels_max = min(chan_max, cpu_stream->channels_max);

so I think leaving them as 16 has no harm.

[lgirdwood]

8 chan is correct, my original value is wrong.

[keyonjie]

OK, let me change them to 8 in v3.

[plbossart]

Can you look at Rakesh's patches, I think we could update this with Analog, Alt-Analog and Digital?

[keyonjie]

OK, I will go and get that for HDMI enabling, I believe we may need to refine them during enabling each BEs, as I talked previously, it's not one shot fix, it is only framework implementation here.

[lgirdwood]

keyon, can you check the HDA/HDMI naming should be consistent per platform (but it may change between different platforms).

[keyonjie]

confirmed they are same with cAVS ones defined in skl-pcm.c

[plbossart]

I meant use something this instead, see alsa-devel [PATCH v3 11/13] ASoC: hdac_hda: add asoc extension for legacy HDA codec drivers

+static struct snd_soc_dai_driver hdac_hda_dais[] = {
+{

• .id = HDAC_ANALOG_DAI_ID,
• .name = "Analog Codec DAI",
• .ops = &hdac_hda_dai_ops,
• .playback = {

•   .stream_name	= "Analog Codec Playback",
  

•   .channels_min	= 1,
  

•   .channels_max	= 16,
  

•   .rates		= SNDRV_PCM_RATE_8000_192000,
  

•   .formats	= STUB_FORMATS,
  

•   .sig_bits	= 24,
  

• },
• .capture = {

•   .stream_name    = "Analog Codec Capture",
  

•   .channels_min   = 1,
  

•   .channels_max   = 16,
  

•   .rates = SNDRV_PCM_RATE_8000_192000,
  

•   .formats = STUB_FORMATS,
  

•   .sig_bits = 24,
  

• },
  +},
  +{
• .id = HDAC_DIGITAL_DAI_ID,
• .name = "Digital Codec DAI",
• .ops = &hdac_hda_dai_ops,
• .playback = {

•   .stream_name    = "Digital Codec Playback",
  

•   .channels_min   = 1,
  

•   .channels_max   = 16,
  

•   .rates          = SNDRV_PCM_RATE_8000_192000,
  

•   .formats        = STUB_FORMATS,
  

•   .sig_bits = 24,
  

• },
• .capture = {

•   .stream_name    = "Digital Codec Capture",
  

•   .channels_min   = 1,
  

•   .channels_max   = 16,
  

•   .rates = SNDRV_PCM_RATE_8000_192000,
  

•   .formats = STUB_FORMATS,
  

•   .sig_bits = 24,
  

• },
  +},
  +{
• .id = HDAC_ALT_ANALOG_DAI_ID,
• .name = "Alt Analog Codec DAI",
• .ops = &hdac_hda_dai_ops,
• .playback = {

•   .stream_name	= "Alt Analog Codec Playback",
  

•   .channels_min	= 1,
  

•   .channels_max	= 16,
  

•   .rates		= SNDRV_PCM_RATE_8000_192000,
  

•   .formats	= STUB_FORMATS,
  

•   .sig_bits	= 24,
  

• },
• .capture = {

•   .stream_name    = "Alt Analog Codec Capture",
  

•   .channels_min   = 1,
  

•   .channels_max   = 16,
  

•   .rates = SNDRV_PCM_RATE_8000_192000,
  

•   .formats = STUB_FORMATS,
  

•   .sig_bits = 24,
  

• },
  +}

[keyonjie]

so we need 3 for HDA legacy codecs? And should this be same for all platforms?

[plbossart]

I still don't get how we handle multiple nocodec outputs with this solution?

[keyonjie]

multiple nocodec cpu_dais are created during probing, each dai driver defined in platform driver(e.g. apl.c) will be used and corresponding nocodec dai_link will be created in sof_nocodec_setup().

[plbossart]

Is there a way to use common tables when APL/CNL/ICL are identical?

[keyonjie]

it is possible, but separating them is more flexible at the moment in my opinion, if they are totally same after all our features are implemented and they do have no difference, we can merge them then.

[lgirdwood]

SSP rates on all the platforms are the same atm, so safe to merge.

[keyonjie]

Hi Liam, sorry can't get your point here, so do all of us aligned on using common tables for APL/CNL/ICL?

[keyonjie]

how about putting it in hda-pcm.c?

[plbossart]

Pretty sure SKL/KBL don't have 6 SSPs

[keyonjie]

let's shrink them then.

[keyonjie]

by the way, do you know how much we have for that? I get information from cAVS 1.5 HAS that I2SPC is 3, but the document claims it support all BXT/GLK/SPT platforms.

[keyonjie]

per my understanding, for SKL+, cAVS driver use the common table defined in skl-pcm.c, that is the same table we defined in apl.c/cnl.c/skl.c, so here for SKL/KBL, let's keep it, which is same with cAVS one?

[plbossart]

I don't understand why we set formats that are not valid except for SSP, and how we deal with DMIC or HDAudio with this work.
Specifically with DMIC we know it's capture only so shouldn't we look at the information in ops->drv[i] to avoid changing things that make limited sense?

[keyonjie]

here it is for nocodec only, but maybe you are right, if somebody defines to use nocodec DMIC in tplg file, then...

again, this BE.dai_fmt for nocodec actually don't take any effect, maybe I should remove this assignment here to make you feel more relax. :)

[plbossart]

is this still needed to have a nocodec-dai?

[keyonjie]

good point, let me fix it, thanks.

[keyonjie]

update to v2 with addressing some comments.

[lgirdwood]

This file can be completely removed now and this function should be in sof/utils.c
I would also change this function to sof_platform_add_all_bes()

[keyonjie]

how about the probe()/remove(), the platform driver, and the card sound card register invoking?

[lgirdwood]

They are fine, they should register the card and get around any deferred probe binding.

[lgirdwood]

We are no longer "sof-nocodec" as this code can be used for platforms that support add-in audio cards/devices. We should probably pass this in.

[plbossart]

Liam's comment was not addressed?

[keyonjie]

I had a call with Liam about this, and aligned to keep this no-codec driver and keep it simple. The reason for it is that we can't leverage deffer probe without using platform probing.

[lgirdwood]

rename nocodec_ -> sof_platform_

[plbossart]

same, Liam's comment isn't addressed.

[lgirdwood]

What happens if this fails ? Need to handle errors

[keyonjie]

let me add handle of this.

[lgirdwood]

I'll merge this into my upstream patch.

[lgirdwood]

Just minor things to fix for me.

[lgirdwood]

It's probably worth commenting here that some products who use this DAI array only physically have a subset of the DAIs, but no harm is done here by adding the whole set.

[keyonjie]

done.

[lgirdwood]

CHT has 6 SSPs, best to add them and include a comment.

[keyonjie]

are you sure CHT has 6 SSPs? I see from your previous patch series that it is set and commented as 3/

[keyonjie]

and 3 is same with sst-mfld-platform-pcm.c also.

/* BE CPU Dais */
{
.name = "ssp0-port",
.ops = &sst_be_dai_ops,
.playback = {
.stream_name = "ssp0 Tx",
.channels_min = SST_STEREO,
.channels_max = SST_STEREO,
.rates = SNDRV_PCM_RATE_48000,
.formats = SNDRV_PCM_FMTBIT_S16_LE,
},
.capture = {
.stream_name = "ssp0 Rx",
.channels_min = SST_STEREO,
.channels_max = SST_STEREO,
.rates = SNDRV_PCM_RATE_48000,
.formats = SNDRV_PCM_FMTBIT_S16_LE,
},
},
{
.name = "ssp1-port",
.ops = &sst_be_dai_ops,
.playback = {
.stream_name = "ssp1 Tx",
.channels_min = SST_STEREO,
.channels_max = SST_STEREO,
.rates = SNDRV_PCM_RATE_8000|SNDRV_PCM_RATE_16000|SNDRV_PCM_RATE_48000,
.formats = SNDRV_PCM_FMTBIT_S16_LE,
},
.capture = {
.stream_name = "ssp1 Rx",
.channels_min = SST_STEREO,
.channels_max = SST_STEREO,
.rates = SNDRV_PCM_RATE_8000|SNDRV_PCM_RATE_16000|SNDRV_PCM_RATE_48000,
.formats = SNDRV_PCM_FMTBIT_S16_LE,
},
},
{
.name = "ssp2-port",
.ops = &sst_be_dai_ops,
.playback = {
.stream_name = "ssp2 Tx",
.channels_min = SST_STEREO,
.channels_max = SST_STEREO,
.rates = SNDRV_PCM_RATE_48000,
.formats = SNDRV_PCM_FMTBIT_S16_LE,
},
.capture = {
.stream_name = "ssp2 Rx",
.channels_min = SST_STEREO,
.channels_max = SST_STEREO,
.rates = SNDRV_PCM_RATE_48000,
.formats = SNDRV_PCM_FMTBIT_S16_LE,
},
},
};

[lgirdwood]

yes

[lgirdwood]

They are fine, they should register the card and get around any deferred probe binding.

[xiulipan]

You may need to refine the the code here to make dai link can be found for the codec machine drivers.

linux/sound/soc/soc-core.c

Line 1001 in cb05347

struct snd_soc_dai_link *snd_soc_find_dai_link(struct snd_soc_card *card,

This will cause some machine driver dai could not found. It may fix
thesofproject/soft#9

[keyonjie]

xiuli, the issue you mentioned should be fixed in tplg file, we should set .id value for DAI in test codec tplg.

[keyonjie]

Liam/Pierre, just updated, please help review.

[plbossart]

Almost looks good, just a couple of minors comments to look into. Thanks Keyon.

[plbossart]

why is this needed? Can we add later when it's actually doing something?

[keyonjie]

are you meaning this hda-dai.c file? this is suggested by Liam to add, and it addressing your comment about common dai driver table for SKL/APL/CNL also.

[plbossart]

I believe the max number of channels should be 16 for HDAudio (same for all digital parts below)

[keyonjie]

please feel free to change it, as I am not able to test them yet, so I can't give any comment about it.

[plbossart]

do we support FLOAT on BDW? (same comments for all machines btw, I don't think we can deal with FLOAT on a DAI.

[keyonjie]

This is also initiated by Liam, Liam, do you have comment on it?

[plbossart]

Liam's comment was not addressed?

[plbossart]

same, Liam's comment isn't addressed.

[keyonjie]

Hi Pierre/Liam, new version updated, I think it address all comments from you, please help review.

[plbossart]

There is one isolated 16->8 ch issue that needs to be fixed in a follow-up patch. let's merge this.

[plbossart]

8

[keyonjie]

oops, let me send incremental fix soon.