[PATCH 0/4] ASoC: Intel: Mark BE DAIs as nonatomic for hsw and

[PATCH 0/4] ASoC: Intel: Mark BE DAIs as nonatomic for hsw and

[Cezary Rojewski]

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(+)

-- 
2.25.1

[PATCH 1/4] ASoC: Intel: hsw_rt5640: Mark BE DAI as nonatomic

[Cezary Rojewski]

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,
-- 
2.25.1

[PATCH 2/4] ASoC: Intel: bdw_rt286: Mark BE DAI as nonatomic

[Cezary Rojewski]

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,
-- 
2.25.1

[PATCH 3/4] ASoC: Intel: bdw_rt5650: Mark BE DAI as nonatomic

[Cezary Rojewski]

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,
-- 
2.25.1

[PATCH 4/4] ASoC: Intel: bdw_rt5677: Mark BE DAI as nonatomic

[Cezary Rojewski]

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,
-- 
2.25.1

Re: [PATCH 0/4] ASoC: Intel: Mark BE DAIs as nonatomic for hsw and

[Pierre-Louis Bossart]

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(+)
> 

Re: [PATCH 0/4] ASoC: Intel: Mark BE DAIs as nonatomic for hsw and

[Cezary Rojewski]

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

Re: [PATCH 0/4] ASoC: Intel: Mark BE DAIs as nonatomic for hsw and

[Pierre-Louis Bossart]

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.

See https://github.com/thesofproject/linux/pull/3723

Re: [PATCH 0/4] ASoC: Intel: Mark BE DAIs as nonatomic for hsw and

[Cezary Rojewski]

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.
> 
> See https://github.com/thesofproject/linux/pull/3723

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

Re: [PATCH 0/4] ASoC: Intel: Mark BE DAIs as nonatomic for hsw and

[Pierre-Louis Bossart]

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.
>>
>> See https://github.com/thesofproject/linux/pull/3723
> 
> 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...

Re: [PATCH 0/4] ASoC: Intel: Mark BE DAIs as nonatomic for hsw and

[Cezary Rojewski]

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.

Re: [PATCH 0/4] ASoC: Intel: Mark BE DAIs as nonatomic for hsw and

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 651 bytes --]

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.

> See https://github.com/thesofproject/linux/pull/3723

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/4] ASoC: Intel: Mark BE DAIs as nonatomic for hsw and

[Cezary Rojewski]

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.
> 
>> See https://github.com/thesofproject/linux/pull/3723
> 
> 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.

Re: [PATCH 0/4] ASoC: Intel: Mark BE DAIs as nonatomic for hsw and

[Mark Brown]

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