Actually, there was one big piece I overlooked. The whole DPCM BE operation is *always* tied with FE's. That is, the nonatomic flag is completely ignored for BE, but just follows what FE sets up.
And that's the very confusing point when reviewing the code. You cannot know whether it's written for non-atomic context or not. This means that it's also error-prone; the code that assumes the operation in a certain mode might mismatch with the bound FE.
So, ideally, both FE and BE should set the proper nonatomic flags, and have a consistency check with WARN_ON() at the run time.
Sorry Takashi, I am not following. Are you asking me to add a .nonatomic flag in all the exiting BEs along with a WARN_ON?
I can do this, but that's a sure way to trigger massive amounts of user-reported "regression in kernel 5.1x". Is this really what you want?
That's why I wrote "ideally". We all know that the world is no perfect... So hardening in that way would be possible, but it has to be done carefully if we really go for it, and I'm not asking you to do that now.
Also I don't understand how this would help with the specific problem raised in this patch: can we yes/no do something 'heavy' in a *DAI* callback? What is the definition of 'heavy'?
My previous comment wasn't specifically about your patch itself but rather arguing a generic problem. We have no notion or matching mechanism of the atomicity of DPCM BE.
I think the only problem is actually on the SoundWire dailinks.
For SSP/DMIC we don't do anything for BE dailinks, there's no IPC or waits, only some settings/masks. I don't see any need to set the .nonatomic field in those cases.
But for SoundWire, we do use the 'stream' functions from the BE ops callbacks - sdw_prepare_stream, sdw_trigger_stream - which will do a bank switch operation. That's certainly not an atomic operation, there's a clear wait_for_completion().
That seems like a miss indeed, I'll add a patch to set the .nonatomic field for these links.
But for this patch proper, does anyone have an objection? I am still not clear on what is permissible at the DAI level.
And last, I am not sure it's always the case that a BE follows the FE configuration. We've had cases of BE->BE loopbacks where the host doesn't see or configured the data.
Hm, how the trigger and other PCM callbacks for BE get called in that mode?
IIRC everything was handled with DAPM, changing pin states would enable data transfers. Not 100% sure and that's not really relevant anyways, you did have a point that the SoundWire BEs are not correctly configured.