Skip to content

Audio: Copier: Add missing DMIC DAI index mapping to create_dai() - #6668

Merged
lgirdwood merged 1 commit into
thesofproject:mainfrom
singalsu:add_cavs_tgl_dmic16k
Dec 19, 2022
Merged

Audio: Copier: Add missing DMIC DAI index mapping to create_dai()#6668
lgirdwood merged 1 commit into
thesofproject:mainfrom
singalsu:add_cavs_tgl_dmic16k

Conversation

@singalsu

@singalsusingalsu commented Nov 25, 2022

Copy link
Copy Markdown
Collaborator

The PR contains two commits

  • Audio: Copier: Add missing DMIC DAI index mapping to create_dai()
  • Tools: Topology2: Add DMIC16k capture to cavs-nocodec topologies

Edit - I removed the second topology2 commit. Now this is only a FW patch.

@singalsu

singalsu commented Nov 25, 2022

Copy link
Copy Markdown
CollaboratorAuthor

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

#define SOF_IPC4_NODE_INDEX_INTEL_DMIC(x) (((x) & 0x7) << 5)
It needs to be addressed separately.

Edit: This problem is solved in the first commit.

@singalsu

Copy link
Copy Markdown
CollaboratorAuthor

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

#define SOF_IPC4_NODE_INDEX_INTEL_DMIC(x) (((x) & 0x7) << 5) It needs to be addressed separately.

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.

Comment threadtools/topology/topology2/cavs/cavs-nocodec.conf Outdated
Comment threadtools/topology/topology2/cavs/cavs-nocodec.conf Outdated
@singalsu

Copy link
Copy Markdown
CollaboratorAuthor

The modified topology:
cavs-tgl-nocodec

@singalsu
singalsu marked this pull request as ready for review November 25, 2022 11:48
Comment threadtools/topology/topology2/cavs/cavs-nocodec.conf Outdated
Comment threadtools/topology/topology2/cavs/platform/intel/dmic-generic-add-dmic1.conf Outdated
@ranj063

Copy link
Copy Markdown
Collaborator

@singalsu can we please wait till #6592 is merged before you add the DMIC16K additions?

@ranj063

Copy link
Copy Markdown
Collaborator

@singalsu my PR is merged. So this one is good for rebase

@singalsu
singalsuforce-pushed the add_cavs_tgl_dmic16k branch from a40b3bc to 632f002CompareDecember 2, 2022 09:58
@singalsu

Copy link
Copy Markdown
CollaboratorAuthor

@singalsu my PR is merged. So this one is good for rebase

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.

Comment threadsrc/audio/copier/copier.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.

Good find here, I guess this is also needed for mtl-02. @marcinszkudlinski fyi

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.

This fix is necessary. Maybe @singalsu can submit a separate PR for this fix to speed up merging this patch.

Comment threadsrc/audio/copier/copier.c Outdated

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.

This fix is necessary. Maybe @singalsu can submit a separate PR for this fix to speed up merging this patch.

@libinyanglibinyangDec 13, 2022

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.

The ppl15 is used by deep buffer in the latest code. We need to refresh the code.

@libinyanglibinyangDec 13, 2022

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.

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"
}

Copy link
Copy Markdown
CollaboratorAuthor

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>
@singalsusingalsu changed the title Tools: Topology2: Add DMIC16k capture to cavs-nocodec topologiesAudio: Copier: Add missing DMIC DAI index mapping to create_dai()Dec 16, 2022
@singalsu

Copy link
Copy Markdown
CollaboratorAuthor

@libinyang I removed everything but the essential FW patch. The 16k DMIC DAI cannot work without it. Please review this.

@libinyanglibinyang 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.

LGTM

@kv2019i

Copy link
Copy Markdown
Collaborator

@wszypelt This has failures as well, can you check?

@wszypelt

Copy link
Copy Markdown

@kv2019i I repeated the tests, 20 minutes and you should get the correct results

@lgirdwood
lgirdwood merged commit 8d166a4 into thesofproject:mainDec 19, 2022
serhiy-katsyuba-intel added a commit to serhiy-katsyuba-intel/sof that referenced this pull request Dec 21, 2022
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>
serhiy-katsyuba-intel added a commit to serhiy-katsyuba-intel/sof that referenced this pull request Dec 21, 2022
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>
serhiy-katsyuba-intel added a commit to serhiy-katsyuba-intel/sof that referenced this pull request Dec 22, 2022
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>
abonislawski pushed a commit that referenced this pull request Dec 22, 2022
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>
lgirdwood pushed a commit that referenced this pull request Jan 4, 2023
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>
@singalsu
singalsu deleted the add_cavs_tgl_dmic16k branch February 2, 2023 15:26
Sign up for freeto 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.

6 participants

@singalsu@ranj063@kv2019i@wszypelt@libinyang@lgirdwood