Skip to content

Conversation

@singalsu
Copy link
Collaborator

@singalsu singalsu commented Nov 25, 2022

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
Copy link
Collaborator Author

singalsu commented Nov 25, 2022

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 singalsu force-pushed the add_cavs_tgl_dmic16k branch from b1ddf8c to a40b3bc Compare November 25, 2022 11:30
@singalsu
Copy link
Collaborator Author

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.

@singalsu
Copy link
Collaborator Author

The modified topology:
cavs-tgl-nocodec

@singalsu singalsu marked this pull request as ready for review November 25, 2022 11:48
@singalsu singalsu requested a review from libinyang November 25, 2022 11:48
@ranj063
Copy link
Collaborator

ranj063 commented Nov 25, 2022

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

@ranj063
Copy link
Collaborator

ranj063 commented Dec 1, 2022

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

@singalsu singalsu force-pushed the add_cavs_tgl_dmic16k branch from a40b3bc to 632f002 Compare December 2, 2022 09:58
@singalsu
Copy link
Collaborator Author

singalsu commented Dec 2, 2022

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

@singalsu singalsu requested a review from lgirdwood December 2, 2022 13:27
Copy link
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
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.

Copy link
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.

Copy link
Contributor

@libinyang libinyang Dec 13, 2022

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.

Copy link
Contributor

@libinyang libinyang Dec 13, 2022

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
Collaborator Author

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>
@singalsu singalsu force-pushed the add_cavs_tgl_dmic16k branch from 632f002 to d82d526 Compare December 16, 2022 08:55
@singalsu singalsu changed the title Tools: Topology2: Add DMIC16k capture to cavs-nocodec topologies Audio: Copier: Add missing DMIC DAI index mapping to create_dai() Dec 16, 2022
@singalsu
Copy link
Collaborator Author

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

@singalsu singalsu requested a review from libinyang December 16, 2022 08:58
Copy link
Contributor

@libinyang libinyang left a comment

Choose a reason for hiding this comment

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

LGTM

@kv2019i
Copy link
Collaborator

kv2019i commented Dec 16, 2022

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

@wszypelt
Copy link

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

@lgirdwood lgirdwood merged commit 8d166a4 into thesofproject:main Dec 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 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.

6 participants