Skip to content

Audio: DAI: ASRC: Fix DSP panic due to timestamping setup request in asrc_prepare() - #4095

Merged
lgirdwood merged 2 commits into
thesofproject:mainfrom
singalsu:dai_asrc_fix_panic_due_to_timestamping_setup
Apr 28, 2021
Merged

lgirdwood merged 2 commits into
thesofproject:mainfrom
singalsu:dai_asrc_fix_panic_due_to_timestamping_setup

Conversation

@singalsu

Copy link
Copy Markdown
Collaborator

The start sequence has changed to ASRC prepare before DAI, so there was as a null pointer access issue. The first patch adds check for null and the second patch moves setup to trigger start.

@singalsu

Copy link
Copy Markdown
Collaborator Author

This addresses issue #4068.

Comment thread src/audio/dai.c Outdated

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.

Can we add a comment in the dai API header doxygen comments that state when this API can be called.

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.

@singalsu I would move the dd->chan check just at the top of the function and leave this line untouched.

E. g

if (!dd->chan) {
      comp_err()...
     return -EINVAL;
}

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.

Fwiw, we seem to be lacking some general documentation in the DAI logic but AFAICT this fix should do the trick.

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.

Yep, my add seemed to be only one there.

I'll test updated here and convert to non-draft if OK.

This patch adds error handling for case where DAI data pointer
to DMA channel is not set. The situation can happen if ASRC tries
to set up timestamping before DAI has been prepared.

More specifically access to dd->chan->index with NULL in chan
caused a DSP panic.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
The asrc_dai_configure_timestamp() caused a DSP panic when called from
asrc_prepare(). The reason is that DAI prepare now happens after ASRC
prepare. Call from trigger start is after pipeline prepare so it
avoids this problem.

Fixes: thesofproject#4068

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
@singalsu
singalsu force-pushed the dai_asrc_fix_panic_due_to_timestamping_setup branch from 8af3963 to 53eafd4 Compare April 28, 2021 12:08
@singalsu
singalsu marked this pull request as ready for review April 28, 2021 12:12
@lgirdwood
lgirdwood merged commit 7522a7b into thesofproject:main Apr 28, 2021
@singalsu
singalsu deleted the dai_asrc_fix_panic_due_to_timestamping_setup branch June 15, 2021 08:20
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