> On 6. 6. 2022, at 23:22, Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> wrote:
>
> On 6/6/22 15:46, Martin Povišer wrote:
>> (I am having trouble delivering mail to linux.intel.com, so I reply to the list
>> and CC at least...)
>>
>>> On 6. 6. 2022, at 22:02, Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> wrote:
>>>
>>>
>>>> + * Virtual FE/BE Playback Topology
>>>> + * -------------------------------
>>>> + *
>>>> + * The platform driver has independent frontend and backend DAIs with the
>>>> + * option of routing backends to any of the frontends. The platform
>>>> + * driver configures the routing based on DPCM couplings in ASoC runtime
>>>> + * structures, which in turn is determined from DAPM paths by ASoC. But the
>>>> + * platform driver doesn't supply relevant DAPM paths and leaves that up for
>>>> + * the machine driver to fill in. The filled-in virtual topology can be
>>>> + * anything as long as a particular backend isn't connected to more than one
>>>> + * frontend at any given time. (The limitation is due to the unsupported case
>>>> + * of reparenting of live BEs.)
>>>> + *
>>>> + * The DAPM routing that this machine-level driver makes up has two use-cases
>>>> + * in mind:
>>>> + *
>>>> + * - Using a single PCM for playback such that it conditionally sinks to either
>>>> + * speakers or headphones based on the plug-in state of the headphones jack.
>>>> + * All the while making the switch transparent to userspace. This has the
>>>> + * drawback of requiring a sample stream suited for both speakers and
>>>> + * headphones, which is hard to come by on machines where tailored DSP for
>>>> + * speakers in userspace is desirable or required.
>>>> + *
>>>> + * - Driving the headphones and speakers from distinct PCMs, having userspace
>>>> + * bridge the difference and apply different signal processing to the two.
>>>> + *
>>>> + * In the end the topology supplied by this driver looks like this:
>>>> + *
>>>> + * PCMs (frontends) I2S Port Groups (backends)
>>>> + * ──────────────── ──────────────────────────
>>>> + *
>>>> + * ┌──────────┐ ┌───────────────► ┌─────┐ ┌──────────┐
>>>> + * │ Primary ├───────┤ │ Mux │ ──► │ Speakers │
>>>> + * └──────────┘ │ ┌──────────► └─────┘ └──────────┘
>>>> + * ┌─── │ ───┘ ▲
>>>> + * ┌──────────┐ │ │ │
>>>> + * │Secondary ├──┘ │ ┌────────────┴┐
>>>> + * └──────────┘ ├────►│Plug-in Demux│
>>>> + * │ └────────────┬┘
>>>> + * │ │
>>>> + * │ ▼
>>>> + * │ ┌─────┐ ┌──────────┐
>>>> + * └───────────────► │ Mux │ ──► │Headphones│
>>>> + * └─────┘ └──────────┘
>>>> + */
>>>
>>> In Patch2, the 'clusters' are described as front-ends, with I2S as
>>> back-ends. Here the PCMs are described as front-ends, but there's no
>>> mention of clusters. Either one of the two descriptions is outdated, or
>>> there's something missing to help reconcile the two pieces of information?
>>
>> Both descriptions should be in sync. Maybe I don’t know the proper
>> terminology. In both cases the frontend is meant to be the actual I2S
>> transceiver unit, and backend the I2S port on the SoC’s periphery,
>> which can be routed to any of transceiver units. (Multiple ports can
>> be routed to the same unit, which means the ports will have the same
>> clocks and data line -- that's a configuration we need to support to
>> drive some of the speaker arrays, hence the backend/frontend
>> distinction).
>>
>> Maybe I am using 'PCM' in a confusing way here? What I meant is a
>> subdevice that’s visible from userspace, because I have seen it used
>> that way in ALSA codebase.
>
> Yes, I think most people familiar with DPCM would take the 'PCM
> frontend' as some sort of generic DMA transfer from system memory, while
> the 'back end' is more the actual serial link. In your case, the
> front-end is already very low-level and tied to I2S already. I think
> that's fine, it's just that using different terms for 'cluster' and
> 'PCM' in different patches could lead to confusions.
OK, will take this into account then.
>>>> +static int macaudio_get_runtime_mclk_fs(struct snd_pcm_substream *substream)
>>>> +{
>>>> + struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
>>>> + struct macaudio_snd_data *ma = snd_soc_card_get_drvdata(rtd->card);
>>>> + struct snd_soc_dpcm *dpcm;
>>>> +
>>>> + /*
>>>> + * If this is a FE, look it up in link_props directly.
>>>> + * If this is a BE, look it up in the respective FE.
>>>> + */
>>>> + if (!rtd->dai_link->no_pcm)
>>>> + return ma->link_props[rtd->dai_link->id].mclk_fs;
>>>> +
>>>> + for_each_dpcm_fe(rtd, substream->stream, dpcm) {
>>>> + int fe_id = dpcm->fe->dai_link->id;
>>>> +
>>>> + return ma->link_props[fe_id].mclk_fs;
>>>> + }
>>>
>>> I am not sure what the concept of mclk would mean for a front-end? This
>>> is typically very I2S-specific, i.e. a back-end property, no?
>>
>> Right, that’s a result of the confusion from above. Hope I cleared it up
>> somehow. The frontend already decides the clocks and data serialization,
>> hence mclk/fs is a frontend-prop here.
>
> What confuses me in this code is that it looks possible that the front-
> and back-end could have different mclk values? I think a comment is
> missing that the values are identical, it's just that there's a
> different way to access it depending on the link type?
Well, that’s exactly what I am trying to convey by the comment above
in macaudio_get_runtime_mclk_fs. Maybe I should do something to make
it more obvious.
>>>> +static int macaudio_be_init(struct snd_soc_pcm_runtime *rtd)
>>>> +{
>>>> + struct snd_soc_card *card = rtd->card;
>>>> + struct macaudio_snd_data *ma = snd_soc_card_get_drvdata(card);
>>>> + struct macaudio_link_props *props = &ma->link_props[rtd->dai_link->id];
>>>> + struct snd_soc_dai *dai;
>>>> + int i, ret;
>>>> +
>>>> + ret = macaudio_be_assign_tdm(rtd);
>>>> + if (ret < 0)
>>>> + return ret;
>>>> +
>>>> + if (props->is_headphones) {
>>>> + for_each_rtd_codec_dais(rtd, i, dai)
>>>> + snd_soc_component_set_jack(dai->component, &ma->jack, NULL);
>>>> + }
>>>
>>> this is weird, set_jack() is invoked by the ASoC core. You shouldn't
>>> need to do this?
>>
>> That’s interesting. Where would it be invoked? How does ASoC know which codec
>> it attaches to?
>
> sorry, my comment was partly invalid.
>
> set_jack() is invoked in the machine driver indeed, what I found strange
> is that you may have different codecs handling the jack? What is the
> purpose of that loop?
There’s no need for the loop, there’s a single codec. OK, will remove the loop
to make it less confusing.
>>>> +static int macaudio_jack_event(struct notifier_block *nb, unsigned long event,
>>>> + void *data);
>>>> +
>>>> +static struct notifier_block macaudio_jack_nb = {
>>>> + .notifier_call = macaudio_jack_event,
>>>> +};
>>>
>>> why is this needed? we have already many ways of dealing with the jack
>>> events (dare I say too many ways?).
>>
>> Because I want to update the DAPM paths based on the jack status,
>> specifically I want to set macaudio_plugin_demux. I don’t know how
>> else it could be done.
>
> I don't know either but I have never seen notifier blocks being used. I
> would think there are already ways to do this with DAPM events.
>
>
>>>> +static int macaudio_jack_event(struct notifier_block *nb, unsigned long event,
>>>> + void *data)
>>>> +{
>>>> + struct snd_soc_jack *jack = data;
>>>> + struct macaudio_snd_data *ma = snd_soc_card_get_drvdata(jack->card);
>>>> +
>>>> + ma->jack_plugin_state = !!event;
>>>> +
>>>> + if (!ma->plugin_demux_kcontrol)
>>>> + return 0;
>>>> +
>>>> + snd_soc_dapm_mux_update_power(&ma->card.dapm, ma->plugin_demux_kcontrol,
>>>> + ma->jack_plugin_state,
>>>> + (struct soc_enum *) &macaudio_plugin_demux_enum, NULL);
>>>
>>> the term 'plugin' can be understood in many ways by different audio
>>> folks. 'plugin' is usually the term used for processing libraries (VST,
>>> LADSPA, etc). I think here you meant 'jack control'?
>>
>> So ‘jack control’ would be understood as the jack plugged/unplugged status?
>
> The 'Headphone Jack' or 'Headset Mic Jack' kcontrols typically track the
> status. Other terms are 'jack detection'. "plugin" is not a very common
> term here.
OK
