Skip to content

dmic: change spin_lock to spin_try_lock in dmic_work() - #2078

Merged
lgirdwood merged 3 commits into
thesofproject:masterfrom
bkokoszx:topic/fix-release-dmic-timeout
Nov 11, 2019
Merged

lgirdwood merged 3 commits into
thesofproject:masterfrom
bkokoszx:topic/fix-release-dmic-timeout

Conversation

@bkokoszx

@bkokoszx bkokoszx commented Nov 8, 2019 •

Copy link
Copy Markdown
Collaborator

PR fixes 2 bugs with dmic:

  1. There should be dmic task cancellation in dmic_stop() function
  2. Sometimes deadlock appears:
    In dmic_stop() function we invoked spin_lock(dai->lock),
    then we had irq from low latency scheduler and run dmic_work().
    At the beginning of dmic_work() there was spin_lock(dai->lock)
    invocation which led to deadlock.

This PR fixes #2044.

After trigger stop we should cancel dmic task
from low latency scheduler.

Signed-off-by: Bartosz Kokoszko <bartoszx.kokoszko@linux.intel.com>
Adds return to spin_try_lock() in order to retrieve status
from arch_try_lock() function.

Signed-off-by: Bartosz Kokoszko <bartoszx.kokoszko@linux.intel.com>
In order to avoid dead locks in dmic_work I've replaced
spin_lock() with spin_try_lock(). In case when
spin_try_lock() fails we reschedule dmic work.

Signed-off-by: Bartosz Kokoszko <bartoszx.kokoszko@linux.intel.com>
do { \
spin_lock_dbg(); \
arch_try_lock(lock); \
ret = arch_try_lock(lock); \

@paulstelian97 paulstelian97 Nov 8, 2019 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't a better approach be to use a block expression? (GNU extension). As in,

#define spin_try_lock(lock) ({ \
              spin_lock_dbg();  \
              arch_try_lock(lock); \
})

(with the proper alignment)? That would also be of lower impact to the remainder of the code.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency I think that we can change it for every spin_lock*() macro in another PR.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, we could make this an incremental update in another PR.

@jajanusz jajanusz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks legit, if it really fixes reported issues we can merge it.
Good job

@jajanusz

jajanusz commented Nov 8, 2019

Copy link
Copy Markdown
Contributor

@bkokoszx please fix checkpatch warning

do { \
spin_lock_dbg(); \
arch_try_lock(lock); \
ret = arch_try_lock(lock); \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, we could make this an incremental update in another PR.

@lgirdwood

Copy link
Copy Markdown
Member

CI look unrelated.

@lgirdwood
lgirdwood merged commit 6a0cf67 into thesofproject:master Nov 11, 2019
@bkokoszx
bkokoszx deleted the topic/fix-release-dmic-timeout branch March 10, 2020 11:57
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.

[BUG]DMIC capture pause-release cause ipc timed out 0x60070000 size 12

4 participants