Repository navigation
Drivers: Intel: DMIC: Fix FW panic with multiple DAI capture, pausing, stopping and resuming - #4313
Conversation
This patch does not change any functionality. The mask is no more signed value and cache_to_uncache() needs a multiple of 32 bit type. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
Some notes inline, but actual patch content looks good!
There was a problem hiding this comment.
This is really problematic. There's no actual reference counting in cavs_pm_runtime_dis_dmic_clk_gating() and the "dai->index" passed to pm_runtime_put_sync() is just printed out.
The problem is that if we switch to pm_runtime_put_sync() that actual does refcounting, this code will break. FYI @lgirdwood @lyakh @marc-hb
Oh well, this goes beyond this PR's scope.
There was a problem hiding this comment.
Lets get the ref counting in the PM code. This PM code should also be able to transfer to Zephyr with minimal refactoring.
There was a problem hiding this comment.
it looks like we may need some locking around the mask manipulations .
There was a problem hiding this comment.
I added 3rd commit about spinlocks. I was testing spinlock add in dmic_remove() but it froze. It turned out that dai_put() had already applied spinlock. I also found out that the registers write function was already covered by spinlock in set_config() so the patch adds only comments about spinlocks applied.
This patch fixes an FW panic that could be triggered by multiple DMIC DAI capture when pausing, stopping, and resuming. A sequence like this crashed in last operation start capture on dmic0, dmic1 pause 0 stop 1 resume 0 It was caused by dmic_remove() for dmic1 that issued pm_runtime_put_sync() for DMIC clock and power, and freed the dmic_prm[] that resume for 0 needed. commit 282fe22 ("dmic: fix pause/release error leading to invalid dmic_active_fifos") solved the panic but left the HW start/stop sequence details incorrect. commit 73e6a6f ("Driver: Intel: DMIC: Remove state check from active FIFO mask update") re-introduced the FW panic issue in multiple DAI capture. This should be a proper quick fix for pause handling. A successive patch will clean up the DMIC component data and do this in a nicer looking way. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch adds comments to code parts about spinlocks applied elsewhere to avoid confusion. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
7002534 to
96e144e
Compare
|
I just updated the PR: Added pause_mask clear to dmic_stop() in case it was called to stop without pause. A typo was fixed. Also added comments about spinlocks applied. Thanks for help with these @ranj063! |
|
looks good. Thanks @singalsu |
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
Lets fix the PM reference counting incrementally.
No description provided.