-
Notifications
You must be signed in to change notification settings - Fork 349
Audio: Copier: Add missing DMIC DAI index mapping to create_dai() #6668
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
Conversation
|
The built topology works with a hack in firmware src/lib/dai.c dai_get() to shift the index >> 5. The index from kernel is made with macro
Edit: This problem is solved in the first commit. |
b1ddf8c to
a40b3bc
Compare
Cool, @juimonen found a solution that copier uses for SSP dai_index. Adding similar handling for DMIC fixes the dai_index issue. It's in the second commit of this PR now. |
tools/topology/topology2/cavs/platform/intel/dmic-generic-add-dmic1.conf
Outdated
Show resolved
Hide resolved
|
@singalsu my PR is merged. So this one is good for rebase |
a40b3bc to
632f002
Compare
Thanks! Done now. I kept the DMIC configuration as 4ch so now there's no change to copier.host.13.1, only add as 4ch copier.host.15.1 and copier.DMIC.16.1. |
src/audio/copier/copier.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good find here, I guess this is also needed for mtl-02. @marcinszkudlinski fyi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fix is necessary. Maybe @singalsu can submit a separate PR for this fix to speed up merging this patch.
src/audio/copier/copier.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fix is necessary. Maybe @singalsu can submit a separate PR for this fix to speed up merging this patch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ppl15 is used by deep buffer in the latest code. We need to refresh the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can split DMIC0 and DMIC1 into 2 conf files, dmic0-generic.conf for dmic0 and dmic16k-generic.conf for dmic16k.
DMIC1 may be used by other purpose, such as WoV. If we split DMIC0/1 into different conf files. It will be easy to add such support. We just make some judgement in the top file, such as cavs-nocodec.conf to pick up the right configuration, like:
# If there is DMICs, DMIC0 is enabled by default
IncludeByKey.NUM_DMICS {
"[1-4]" "platform/intel/dmic-generic.conf"
}
# enable dmic16k if DMIC16 is defined
IncludeByKey.DMIC16k {
"1" "platform/intel/dmic16k.conf"
}
# enable dummy_wov if DUMMY_WOV is defined
# DUMMY_WOV and DMIC16 are mutually exclusive
IncludeByKey.DUMMY_WOV {
"1" "platform/intel/dmic-wov.conf"
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. I hope I can work in Q1 next year with tplg2. I would add topology and pipelines those enable testing of all playback and capture processing.
The dai_index needs to be mapped similarly as in kernel. The SSP type was handled correctly in the function but not DMIC. Signed-off-by: Jaska Uimonen <jaska.uimonen@linux.intel.com> Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
632f002 to
d82d526
Compare
|
@libinyang I removed everything but the essential FW patch. The 16k DMIC DAI cannot work without it. Please review this. |
libinyang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@wszypelt This has failures as well, can you check? |
|
@kv2019i I repeated the tests, 20 minutes and you should get the correct results |
This is a fix for a regression introduced by thesofproject#6668. For an unknown reason (probably a bug) Linux and Windows hosts use different range of node_id for DMIC. The fix workarounds the problem adding support for both ranges. Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>
This reverts commit 8d166a4. This restores working DMIC functionality on Windows platform broken by thesofproject#6668. ATTENTION: This will break DMIC1 (DMIC with DAI index 1) functionality on Linux platform, DMIC0 should still work fine! Non working DMIC1 should be fixed on Linux driver side, not this firmware. Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>
This reverts commit 8d166a4. This restores working DMIC functionality on Windows platform broken by thesofproject#6668. ATTENTION: This will break DMIC1 (DMIC with DAI index 1) functionality on Linux platform, DMIC0 should still work fine! Non working DMIC1 should be fixed on Linux driver side, not this firmware. Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>
This reverts commit 8d166a4. This restores working DMIC functionality on Windows platform broken by #6668. ATTENTION: This will break DMIC1 (DMIC with DAI index 1) functionality on Linux platform, DMIC0 should still work fine! Non working DMIC1 should be fixed on Linux driver side, not this firmware. Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>
This reverts commit 8d166a4. This restores working DMIC functionality on Windows platform broken by #6668. ATTENTION: This will break DMIC1 (DMIC with DAI index 1) functionality on Linux platform, DMIC0 should still work fine! Non working DMIC1 should be fixed on Linux driver side, not this firmware. Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>

The PR contains two commits
Edit - I removed the second topology2 commit. Now this is only a FW patch.