| Message ID | 20230314153409.1805280-1-daniel.baluta@oss.nxp.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ASoC: soc-compress: Inherit atomicity from DAI link for Compress FE | expand |
On 3/14/23 11:37, Daniel Baluta wrote: > On Tue, Mar 14, 2023 at 6:14 PM Pierre-Louis Bossart > <pierre-louis.bossart@linux.intel.com> wrote: >> >> >> >> On 3/14/23 10:34, Daniel Baluta wrote: >>> From: Daniel Baluta <daniel.baluta@nxp.com> >>> >>> 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.
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index e7aa6f360cab..d649b0cf4744 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -622,6 +622,9 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num) return ret; } + /* inherit atomicity from DAI link */ + be_pcm->nonatomic = rtd->dai_link->nonatomic; + rtd->pcm = be_pcm; rtd->fe_compr = 1; if (rtd->dai_link->dpcm_playback)