Re: [PATCH] ASoC: soc-compress: Inherit atomicity from DAI link for Compress FE

[Pierre-Louis Bossart]

On 3/14/23 11:37, Daniel Baluta wrote:
> On Tue, Mar 14, 2023 at 6:14 PM Pierre-Louis Bossart
> <pierre-louis.bossart@xxxxxxxxxxxxxxx> wrote:
>>
>>
>>
>> On 3/14/23 10:34, Daniel Baluta wrote:
>>> From: Daniel Baluta <daniel.baluta@xxxxxxx>
>>>
>>> After commit bbf7d3b1c4f40 ("ASoC: soc-pcm: align BE 'atomicity' with
>>> that of the FE") BE and FE atomicity must match.
>>>
>>> In the case of Compress PCM there is a mismatch in atomicity between FE
>>> and BE and we get errors like this:
>>>
>>> [   36.434566]  sai1-wm8960-hifi: dpcm_be_connect: FE is atomic but BE
>>> is nonatomic, invalid configuration
>>
>> Not clear on the 'FE is atomic' in the case of a compressed stream,
>> which has to be handled with some sort of IPC, i.e. be nonatomic.
>>
> 
> 'FE is atomic' in this message is printed because this is the default value
> of nonatomic field when PCM struct associated for a Compress PCM
> struct is allocated.
> 
> No one changes 'nonatomic' field for Compress FE until my current patch.
> 
>> Also not sure why the FE is not set as nonatomic by the SOF parts?
>> If it's needed for PCM, why wouldn't it be needed for compressed data?
> 
> FE is not touched for SOF parts. Only BE is set to nonatomic by SOF.

Where do you see the BE being changed by SOF?

> 
> See: sound/soc/topology.c
> 
> »       /* Set nonatomic property for FE dai links as their trigger
> action involves IPC's */
> »       if (!link->no_pcm) {
> »       »       link->nonatomic = true;
> »       »       return 0;
> »       }

that's a FE property, not BE.

> FE for PCM is modified by sound/soc/soc-pcm.c
> 
> int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
> »       pcm->nonatomic = rtd->dai_link->nonatomic;
> 
> So, I guess people assumed that is enough to use RTD dai link to set
> pcm->noatomic field
> and didn't look at it in SOF.

Ah yes, now I see your point now. You still had a logical inversion
above but you're correct here.

> When FE for Compress PCM is created, we don't use soc_new_pcm but instead
> we use snd_pcm_new_internal which doesn't inherit the nonatomic field
> of the rtd->dai_link
> as Normal PCM does inside soc_pcm_new.
> 
> So, my patch makes sure we inherit the nonatomic field from
> rtd->dai_link also for Compress PCM
> similar with what already happens for Normal PCM.
> 
> tl;dr: when creating a Normal PCM pcm->nonatomic is inherited from RTD
> DAI link. when creating a
> Compress PCM pcm->nonatomic field is not set. This patch makes sure
> that for Compres PCM
> we also inherit nonatomic from RTD DAI link.

That makes sense. It's quite likely that the compress PCM should be
nonatomic by default, not sure how it can work otherwise.