Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

back-porting DMIC fixes to Tgl 013 #4612

Merged
merged 13 commits into from
Sep 2, 2021

Conversation

keyonjie
Copy link
Contributor

@keyonjie keyonjie commented Aug 9, 2021

c33e8f7 Drivers: Intel: DMIC: Move driver code to subdirectory dmic
1f71b50 drivers: intel: dmic: fix double free dmic_remove()
f47fe82 Drivers: Intel: DMIC: Code lines cleanup
1cfee18 Drivers: Intel: DMIC: Run dmic_work() in the same core as other driver
b4f7438 Drivers: Intel: DMIC: Re-arrange DMIC DAI component data
3551feb Drivers: Intel: DMIC: Spinlocks notes add
3fabb11 Drivers: Intel: DMIC: Fix FW panic caused by too eager dmic_remove()
2a87538 Drivers: Intel: DMIC: Change active_fifos_mask to uint32_t
8eed33d Drivers: Intel: DMIC: Remove in_active parameter from dmic_stop()
ccfc9e6 Driver: Intel: DMIC: Remove state check from active FIFO mask update
f445426 Drivers: Intel: DMIC: Track fifos activity with mask bits
4d38fdb drivers: dmic: fix multi-fifo logic in interrupt handler
f3ca63c dmic: fix pause/release error leading to invalid dmic_active_fifos

kv2019i and others added 13 commits August 9, 2021 16:57
The 'in_active' parameter of dmic_stop() is used to control whether
the fifo should be released or not. In case of COMP_TRIGGER_PAUSE,
this should be false and the 'dmic_active_fifos' count should not
be modified.

Without this fix:

  PAUSED -> RELEASE -> dmic_start() -> no change on dmic_active_fifos

  ACTIVE -> PAUSE   -> dmic_stop()  -> dmic_active_fifos--

If a test case repeatedly pauses and releases, 'dmic_active_fifos'
will go out-of-sync.

Fix the issue that dmic_stop for PAUSE does not release the fifo
reference.

Fixes: 2273174 ("dmic: don't decrement active FIFO count below 0")
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
When the dmic driver is instantiated multiple times (e.g. for fifo-A and
fifo-B), the interrupt gets registered also twice.

While supported usage of interrupt interface, there is no guarantee that
the interrupt context data is for the expected dai instance. It is thus
not safe to modify the dai state directly or call dai_stop().

Modify the interrupt handler not to make any assumptions on which dai
instance is passed as 'data' matches a specific fifo instance.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
The counter dmic_active_fifos has been a source of many issues. The
use of bits to track active FIFOs is simpler and avoids the risk
of counting to negative or over the actual FIFOs count in various
control scenarios. The incorrect counter value has caused resources
allocate and free and start/stop sequences fails.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
The mask does not over/undercount events so no need to protect
from multiple start() or stop() calls.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Cleanup to trigger() since the parameter is not needed to protect from
errors with bit mask active FIFOs tracking.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
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>
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>
This patch collects the global DMIC driver parameters into single
struct. The pointer global in every DMIC DAI instance points into
the same data. All access from functions happens via the pointer
that simplifies the next step to add NHLT binary data configuration
for the driver.

The previous dmic_prm[] is no more allocated dynamically since the
single data structure is fixed size and not large. It simplifies the
driver code that handles the multiple ownership.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Previously the gain ramp updates were executed in core 0 only even
if rest of driver was executed in other core. It caused issues with
driver state variables in cached memory.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch only removes some unnecessary line feeds for better
readability.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Disable dmic interrupts first followed freeing by the dmic task
and finally, free the dmic pdata.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
In preparation for more functionality the module is moved
to own directory similarly as other DAI drivers.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
@mwasko mwasko merged commit c04ce77 into thesofproject:tgl-013-drop-stable Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants