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

[Richard Fitzgerald]

On 21/02/2023 18:03, Pierre-Louis Bossart wrote:
> > > > +                      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.
>
> IIRC Bard did the same thing recently, and the order of the feedback
> channels is really defined by the order in which the peripheral devices
> are added in the dailink. See
> https://github.com/thesofproject/linux/pull/4108

I don't really see how it is determining how to split the channels.
I see there is a discussion about that but I didn't see any explanation
that I could understand.

Also that only seems to have been done for IPCv4. What if my system
uses IPCv3 ?

Anyway, I'll drop the machine driver patches for now until the IPC code
is stable and consistent behavior for v3 and v4.

> There's also another open related to the number of channels, we need to
> patch what the CPU DAI can handle, see
> https://github.com/thesofproject/linux/pull/4136 or
> https://github.com/thesofproject/linux/pull/4134