Re: [PATCH 09/10] ASoC: Intel: sof_sdw: Add support for Cirrus Logic CS35L56

[Richard Fitzgerald]

On 21/02/2023 16:49, Pierre-Louis Bossart wrote:
> > +static int cs35l56_init_multiple(struct snd_soc_pcm_runtime *rtd, int count)
> > +{
> > +	struct snd_soc_card *card = rtd->card;
> > +	struct snd_soc_dai *codec_dai;
> > +	int i, ret;
> > +
> > +	card->components = devm_kasprintf(card->dev, GFP_KERNEL,
> > +					  "%s hs:cs35l56",
>
> the string is wrong here, this is an amplifier so it should be
> amp:cs35l56 or spk:cs36l56 (not sure which of the two we ended-up using).

Will change it.

> > +					  card->components);
> > +	if (!card->components)
> > +		return -ENOMEM;
> > +
> > +	ret = snd_soc_dapm_new_controls(&card->dapm,
> > +					cs35l56_sof_widgets, ARRAY_SIZE(cs35l56_sof_widgets));
> > +	if (ret) {
> > +		dev_err(card->dev, "Widgets add failed: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	ret = snd_soc_dapm_add_routes(&card->dapm, cs35l56_sof_map, count);
> > +	if (ret) {
> > +		dev_err(card->dev, "Map add %d failed: %d\n", count, ret);
> > +		return ret;
> > +	}
> > +
> > +	/* Enable one feedback TX per amp on different slots */
> > +	for_each_rtd_codec_dais(rtd, i, codec_dai) {
> > +		ret = snd_soc_dai_set_tdm_slot(codec_dai, 0x3, 1 << i, 4, 16);
>
> TDM slots? Not getting how this would work with SoundWire?

Strictly speaking Soundwire is TDM (the frame time is divided up into
slots for each sample...).

The problem is if you have N amps on the dailink all feeding back audio
on the same bus. Their DP slots are all programmed to the same positions
in the frame, same as for the playback. So you have 4 amps all trying to
send 6 audio channels in the same positions in the frame and you'll just
get a ton of bus clash interrupts.

So we use the set_tdm_slot() like we do with I2S TDM to set which slots
are active for each amp.

I can't see that there's any obvious "generic" way that the manager code
can automatically figure out how many channels to enable on each amp and
what order to map them, so we do it here. Just as with I2S TDM - you
have many slots and many codecs but the machine driver has to tell it
how to map those.

> > +		if (ret < 0)
> > +			return ret;
> > +	}
> > +
> > +	return 0;
> > +}