[PATCH] ASoC: soc-compress: Inherit atomicity from DAI link for Compress FE
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 [ 36.444278] PCM Deep Buffer: ASoC: can't connect SAI1.OUT
In order to fix this we must inherit the atomicity from DAI link associated with current PCM Compress FE.
Fixes: bbf7d3b1c4f4 ("ASoC: soc-pcm: align BE 'atomicity' with that of the FE") Signed-off-by: Daniel Baluta daniel.baluta@nxp.com --- sound/soc/soc-compress.c | 3 +++ 1 file changed, 3 insertions(+)
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)
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.
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?
[ 36.444278] PCM Deep Buffer: ASoC: can't connect SAI1.OUT
In order to fix this we must inherit the atomicity from DAI link associated with current PCM Compress FE.
Fixes: bbf7d3b1c4f4 ("ASoC: soc-pcm: align BE 'atomicity' with that of the FE") Signed-off-by: Daniel Baluta daniel.baluta@nxp.com
sound/soc/soc-compress.c | 3 +++ 1 file changed, 3 insertions(+)
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)
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.
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; » }
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.
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.
thanks, Daniel.
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.
On Tue, Mar 14, 2023 at 6:52 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
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.
You are right.
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.
To sum up:
- we need to merge current patch because Compress PCM needs to inherit the atomicity from FE DAI
Because SOF FE DAI links are made to be nonatomic:
sound/soc/sof/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; » }
and with my patch:
sound/soc/soc-compress.c
+ /* inherit atomicity from DAI link */ + be_pcm->nonatomic = rtd->dai_link->nonatomic; + rtd->pcm = be_pcm;
... then Compres PCM will be nonatomic.
Side note: I think be_pcm from the patch above should be called fe_pcm instead. But that's a story for another patch.
thanks, Daniel.
participants (3)
-
Daniel Baluta -
Daniel Baluta -
Pierre-Louis Bossart