[PATCH 0/4] ASoC: Intel: Mark BE DAIs as nonatomic for hsw and
Address the warning: "Codec: dpcm_be_connect: FE is nonatomic but BE is not, forcing BE as nonatomic" by marking BE DAI as nonatomic. Aligns with what is already done for FE DAIs.
This patchset iterates the change over all HSW and BDW related machine board drivers.
Cezary Rojewski (4): ASoC: Intel: hsw_rt5640: Mark BE DAI as nonatomic ASoC: Intel: bdw_rt286: Mark BE DAI as nonatomic ASoC: Intel: bdw_rt5650: Mark BE DAI as nonatomic ASoC: Intel: bdw_rt5677: Mark BE DAI as nonatomic
sound/soc/intel/boards/bdw-rt5650.c | 1 + sound/soc/intel/boards/bdw-rt5677.c | 1 + sound/soc/intel/boards/bdw_rt286.c | 1 + sound/soc/intel/boards/hsw_rt5640.c | 1 + 4 files changed, 4 insertions(+)
Address the warning: "Codec: dpcm_be_connect: FE is nonatomic but BE is not, forcing BE as nonatomic" by marking BE DAI as nonatomic. Aligns with what is already done for FE DAIs.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/boards/hsw_rt5640.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/soc/intel/boards/hsw_rt5640.c b/sound/soc/intel/boards/hsw_rt5640.c index ad747363d112..050c53ebd6ba 100644 --- a/sound/soc/intel/boards/hsw_rt5640.c +++ b/sound/soc/intel/boards/hsw_rt5640.c @@ -121,6 +121,7 @@ static struct snd_soc_dai_link card_dai_links[] = { /* SSP0 - Codec */ .name = "Codec", .id = 0, + .nonatomic = 1, .no_pcm = 1, .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBC_CFC, .ignore_pmdown_time = 1,
Address the warning: "Codec: dpcm_be_connect: FE is nonatomic but BE is not, forcing BE as nonatomic" by marking BE DAI as nonatomic. Aligns with what is already done for FE DAIs.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/boards/bdw_rt286.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/soc/intel/boards/bdw_rt286.c b/sound/soc/intel/boards/bdw_rt286.c index 47eaddb00936..6b76df0e7c9b 100644 --- a/sound/soc/intel/boards/bdw_rt286.c +++ b/sound/soc/intel/boards/bdw_rt286.c @@ -162,6 +162,7 @@ static struct snd_soc_dai_link card_dai_links[] = { /* SSP0 - Codec */ .name = "Codec", .id = 0, + .nonatomic = 1, .no_pcm = 1, .init = codec_link_init, .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBC_CFC,
Address the warning: "Codec: dpcm_be_connect: FE is nonatomic but BE is not, forcing BE as nonatomic" by marking BE DAI as nonatomic. Aligns with what is already done for FE ones.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/boards/bdw-rt5650.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/soc/intel/boards/bdw-rt5650.c b/sound/soc/intel/boards/bdw-rt5650.c index aae857fdcdb8..67c3f49b924c 100644 --- a/sound/soc/intel/boards/bdw-rt5650.c +++ b/sound/soc/intel/boards/bdw-rt5650.c @@ -249,6 +249,7 @@ static struct snd_soc_dai_link bdw_rt5650_dais[] = { /* SSP0 - Codec */ .name = "Codec", .id = 0, + .nonatomic = 1, .no_pcm = 1, .dai_fmt = SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBC_CFC,
Address the warning: "Codec: dpcm_be_connect: FE is nonatomic but BE is not, forcing BE as nonatomic" by marking BE DAI as nonatomic. Aligns with what is already done for FE ones.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/boards/bdw-rt5677.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c index d0ecbba2febe..31488702768e 100644 --- a/sound/soc/intel/boards/bdw-rt5677.c +++ b/sound/soc/intel/boards/bdw-rt5677.c @@ -349,6 +349,7 @@ static struct snd_soc_dai_link bdw_rt5677_dais[] = { /* SSP0 - Codec */ .name = "Codec", .id = 0, + .nonatomic = 1, .no_pcm = 1, .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBC_CFC,
On 6/24/22 08:43, Cezary Rojewski wrote:
Address the warning: "Codec: dpcm_be_connect: FE is nonatomic but BE is not, forcing BE as nonatomic" by marking BE DAI as nonatomic. Aligns with what is already done for FE DAIs.
This patchset iterates the change over all HSW and BDW related machine board drivers.
I don't think this is necessary, I was planning to demote this warning to a simple dev_dbg or possibly remove this message entirely.
The BE DAIs can perfectly be declared as non-atomic in all Intel machine drivers, except for SoundWire where there's a known delay during the .trigger.
Cezary Rojewski (4): ASoC: Intel: hsw_rt5640: Mark BE DAI as nonatomic ASoC: Intel: bdw_rt286: Mark BE DAI as nonatomic ASoC: Intel: bdw_rt5650: Mark BE DAI as nonatomic ASoC: Intel: bdw_rt5677: Mark BE DAI as nonatomic
sound/soc/intel/boards/bdw-rt5650.c | 1 + sound/soc/intel/boards/bdw-rt5677.c | 1 + sound/soc/intel/boards/bdw_rt286.c | 1 + sound/soc/intel/boards/hsw_rt5640.c | 1 + 4 files changed, 4 insertions(+)
On 2022-06-24 3:52 PM, Pierre-Louis Bossart wrote:
On 6/24/22 08:43, Cezary Rojewski wrote:
Address the warning: "Codec: dpcm_be_connect: FE is nonatomic but BE is not, forcing BE as nonatomic" by marking BE DAI as nonatomic. Aligns with what is already done for FE DAIs.
This patchset iterates the change over all HSW and BDW related machine board drivers.
I don't think this is necessary, I was planning to demote this warning to a simple dev_dbg or possibly remove this message entirely.
The BE DAIs can perfectly be declared as non-atomic in all Intel machine drivers, except for SoundWire where there's a known delay during the .trigger.
Hmm.. that's a good feedback. Isn't ASoC's FE<->BE treated as a single PCM substream in sound/core/pcm_native.c though? If so, does it even make sense for card's BE DAI to be atomic, if it's FE counterpart is nonatomic already? Especially if it is specifying platform and cpu_dai that matches Intel's components which we know communicate using IPCs.
Warning is one thing, but will you be also getting rid of the if-statement in soc-pcm.c that actually forces nonatomic=1 on BE when FE is already declared as such? If the if-statement stays, I believe the declaring BE DAIs 'correctly' in the way to go.
Regards, Czarek
On 6/25/22 03:29, Cezary Rojewski wrote:
On 2022-06-24 3:52 PM, Pierre-Louis Bossart wrote:
On 6/24/22 08:43, Cezary Rojewski wrote:
Address the warning: "Codec: dpcm_be_connect: FE is nonatomic but BE is not, forcing BE as nonatomic" by marking BE DAI as nonatomic. Aligns with what is already done for FE DAIs.
This patchset iterates the change over all HSW and BDW related machine board drivers.
I don't think this is necessary, I was planning to demote this warning to a simple dev_dbg or possibly remove this message entirely.
The BE DAIs can perfectly be declared as non-atomic in all Intel machine drivers, except for SoundWire where there's a known delay during the .trigger.
Hmm.. that's a good feedback. Isn't ASoC's FE<->BE treated as a single PCM substream in sound/core/pcm_native.c though? If so, does it even make sense for card's BE DAI to be atomic, if it's FE counterpart is nonatomic already? Especially if it is specifying platform and cpu_dai that matches Intel's components which we know communicate using IPCs.
I guess it depends on the cpu_dai implementation. Not all implementations implement a delay in the .trigger callback and/or rely on IPCs.
Warning is one thing, but will you be also getting rid of the if-statement in soc-pcm.c that actually forces nonatomic=1 on BE when FE is already declared as such? If the if-statement stays, I believe the declaring BE DAIs 'correctly' in the way to go.
I meant just removing the dev_warn() only.
On 2022-06-27 4:45 PM, Pierre-Louis Bossart wrote:
On 6/25/22 03:29, Cezary Rojewski wrote:
Hmm.. that's a good feedback. Isn't ASoC's FE<->BE treated as a single PCM substream in sound/core/pcm_native.c though? If so, does it even make sense for card's BE DAI to be atomic, if it's FE counterpart is nonatomic already? Especially if it is specifying platform and cpu_dai that matches Intel's components which we know communicate using IPCs.
I guess it depends on the cpu_dai implementation. Not all implementations implement a delay in the .trigger callback and/or rely on IPCs.
Warning is one thing, but will you be also getting rid of the if-statement in soc-pcm.c that actually forces nonatomic=1 on BE when FE is already declared as such? If the if-statement stays, I believe the declaring BE DAIs 'correctly' in the way to go.
I meant just removing the dev_warn() only.
So the framework is still fixing the flag for the driver. Ideally we would like to have all the drivers assign correct values to ->nonatomic flag themselves.
Now when I think about it, the message seems useful - at least as dev_dbg(). It _guides_ driver developer to the desired approach: setting the ->nonatomic flag for BE to '1' if the corresponding FE is already configured as such.
I've provided similar answer in the linked thread.
Regards, Czarek
On 6/27/22 10:41, Cezary Rojewski wrote:
On 2022-06-27 4:45 PM, Pierre-Louis Bossart wrote:
On 6/25/22 03:29, Cezary Rojewski wrote:
Hmm.. that's a good feedback. Isn't ASoC's FE<->BE treated as a single PCM substream in sound/core/pcm_native.c though? If so, does it even make sense for card's BE DAI to be atomic, if it's FE counterpart is nonatomic already? Especially if it is specifying platform and cpu_dai that matches Intel's components which we know communicate using IPCs.
I guess it depends on the cpu_dai implementation. Not all implementations implement a delay in the .trigger callback and/or rely on IPCs.
Warning is one thing, but will you be also getting rid of the if-statement in soc-pcm.c that actually forces nonatomic=1 on BE when FE is already declared as such? If the if-statement stays, I believe the declaring BE DAIs 'correctly' in the way to go.
I meant just removing the dev_warn() only.
So the framework is still fixing the flag for the driver. Ideally we would like to have all the drivers assign correct values to ->nonatomic flag themselves.
Now when I think about it, the message seems useful - at least as dev_dbg(). It _guides_ driver developer to the desired approach: setting the ->nonatomic flag for BE to '1' if the corresponding FE is already configured as such.
that would result in unnecessary changes to all machine drivers to get rid of the message...
On 2022-06-27 5:59 PM, Pierre-Louis Bossart wrote:
On 6/27/22 10:41, Cezary Rojewski wrote:
So the framework is still fixing the flag for the driver. Ideally we would like to have all the drivers assign correct values to ->nonatomic flag themselves.
Now when I think about it, the message seems useful - at least as dev_dbg(). It _guides_ driver developer to the desired approach: setting the ->nonatomic flag for BE to '1' if the corresponding FE is already configured as such.
that would result in unnecessary changes to all machine drivers to get rid of the message...
I would have rather used the word _optional_ here. dev_dbg() is a diplomatic solution.
As I'm responsible for catpt and avs drivers, this very series fixes the issue for the former. AVS driver have had the flag set appropriately from the get go, so no fixes needed.
On Mon, Jun 27, 2022 at 09:45:30AM -0500, Pierre-Louis Bossart wrote:
On 6/25/22 03:29, Cezary Rojewski wrote:
Warning is one thing, but will you be also getting rid of the if-statement in soc-pcm.c that actually forces nonatomic=1 on BE when FE is already declared as such? If the if-statement stays, I believe the declaring BE DAIs 'correctly' in the way to go.
I meant just removing the dev_warn() only.
Is something going to be upstreamed here? I don't really mind which solution is adopted here but right now Cezary's patches are the ones that were posted upstream.
On 2022-07-08 5:44 PM, Mark Brown wrote:
On Mon, Jun 27, 2022 at 09:45:30AM -0500, Pierre-Louis Bossart wrote:
On 6/25/22 03:29, Cezary Rojewski wrote:
Warning is one thing, but will you be also getting rid of the if-statement in soc-pcm.c that actually forces nonatomic=1 on BE when FE is already declared as such? If the if-statement stays, I believe the declaring BE DAIs 'correctly' in the way to go.
I meant just removing the dev_warn() only.
Is something going to be upstreamed here? I don't really mind which solution is adopted here but right now Cezary's patches are the ones that were posted upstream.
My 2 cents:
Both series can co-exist. We should still encourage people to configure their DAIs properly so the if-statements like the one forcing ->nonatomic=1 in time are gone.
On Fri, 24 Jun 2022 15:43:13 +0200, Cezary Rojewski wrote:
Address the warning: "Codec: dpcm_be_connect: FE is nonatomic but BE is not, forcing BE as nonatomic" by marking BE DAI as nonatomic. Aligns with what is already done for FE DAIs.
This patchset iterates the change over all HSW and BDW related machine board drivers.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/4] ASoC: Intel: hsw_rt5640: Mark BE DAI as nonatomic commit: 58ef0d3d5716000c153acc5401109fd90afbdf09 [2/4] ASoC: Intel: bdw_rt286: Mark BE DAI as nonatomic commit: 6d7e011808504e0aabbbf3b66d4c14982394abae [3/4] ASoC: Intel: bdw_rt5650: Mark BE DAI as nonatomic commit: 5c4ef9529b12865c2402784a7506c880178effda [4/4] ASoC: Intel: bdw_rt5677: Mark BE DAI as nonatomic commit: bdd15ec4888a375848030cbf7d9fc16c7f430f48
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (3)
-
Cezary Rojewski -
Mark Brown -
Pierre-Louis Bossart