Skip to content

topology2: add src support - #5913

Merged
lgirdwood merged 5 commits into
thesofproject:mainfrom
RanderWang:src-ipc4
Jun 29, 2022
Merged

topology2: add src support#5913
lgirdwood merged 5 commits into
thesofproject:mainfrom
RanderWang:src-ipc4

Conversation

@RanderWang

@RanderWangRanderWang commented Jun 13, 2022

Copy link
Copy Markdown
Collaborator

This PR add ipc src support in topology2.

@RanderWang
RanderWang requested a review from ranj063June 13, 2022 07:55
lgirdwood
lgirdwood reviewed Jun 13, 2022
View reviewed changes
Comment threadtools/topology/topology2/include/common/audio_format.conf Outdated
Comment threadtools/topology/topology2/include/components/src-cavs.conf Outdated
Comment threadtools/topology/topology2/include/pipelines/cavs/src-gain-playback.conf Outdated
Comment threadtools/topology/topology2/include/pipelines/cavs/src-gain-playback.conf Outdated
@RanderWang

RanderWang commented Jun 14, 2022

Copy link
Copy Markdown
CollaboratorAuthor

updated, thanks

@RanderWangRanderWang mentioned this pull request Jun 14, 2022
Merged
lgirdwood
lgirdwood reviewed Jun 14, 2022
View reviewed changes
Comment threadtools/topology/topology2/include/components/cavs/src.conf Outdated
Comment threadtools/topology/topology2/include/pipelines/cavs/src-gain-playback.conf Outdated
singalsu
singalsu reviewed Jun 14, 2022
View reviewed changes
Comment threadtools/topology/topology2/include/common/audio_format.conf Outdated
singalsu
singalsu reviewed Jun 14, 2022
View reviewed changes
Comment threadtools/topology/topology2/include/pipelines/cavs/src-gain-playback.conf Outdated
Comment threadtools/topology/topology2/include/components/gain.conf Outdated
plbossart
plbossart reviewed Jun 14, 2022
View reviewed changes
Comment threadtools/topology/topology2/include/components/cavs/src.conf Outdated
Comment threadtools/topology/topology2/include/pipelines/cavs/src-gain-playback.conf Outdated
Comment threadtools/topology/topology2/include/pipelines/cavs/src-gain-playback.conf Outdated
Comment threadtools/topology/topology2/include/pipelines/cavs/src-gain-playback.conf Outdated
@RanderWangRanderWang changed the title topology2: add src support[RFC] topology2: add src supportJun 15, 2022
@RanderWang

RanderWang commented Jun 15, 2022

Copy link
Copy Markdown
CollaboratorAuthor

change it to RFC since we need to support
(1) all the formats. The debugging and validation effort will be significantly increased
(2) add mixin & mixout

@RanderWang

RanderWang commented Jun 15, 2022•
edited
Loading

Copy link
Copy Markdown
CollaboratorAuthor

@plbossart@lgirdwood@ranj063@bardliao I have an idea bout SRC support in Linux kernel.
SOF can support many sample rates but it is not optimized to build it in topology. The reason is that 48khz audio file will be used in 4x% chance, 4x% 44.1khz and n% for other sample rates. If SRC is built in topology it will introduce more cpu usage for normal 48khz audio. It is not a good deal.

My solution is dynamically add SRC in driver according to sample rate so that we have optimized performance. Another way is to build it in topology but delete it in driver for no conversion rate.

@lgirdwood

lgirdwood commented Jun 15, 2022

Copy link
Copy Markdown
Member

@plbossart@lgirdwood@ranj063@bardliao I have an idea bout SRC support in Linux kernel. SOF can support many sample rates but it is not optimized to build it in topology. The reason is that 48khz audio file will be used in 4x% chance, 4x% 44.1khz and n% for other sample rates. If SRC is built in topology it will introduce more cpu usage for normal 48khz audio. It is not a good deal.

My solution is dynamically add SRC in driver according to sample rate so that we have optimized performance. Another way is to build it in topology but delete it in driver for no conversion rate.

@RanderWang just a thought, rather than expressing all the supported in/out rates for the SRC in the topology, we could use a wildcard rate (like SNDRV_PCM_RATE_CONTINUOUS using 0) which means we can define a single SRC object with wildcard in/out rates where the FW could reject any invalid combination.

@plbossart

plbossart commented Jun 15, 2022

Copy link
Copy Markdown
Member

@plbossart@lgirdwood@ranj063@bardliao I have an idea bout SRC support in Linux kernel. SOF can support many sample rates but it is not optimized to build it in topology. The reason is that 48khz audio file will be used in 4x% chance, 4x% 44.1khz and n% for other sample rates. If SRC is built in topology it will introduce more cpu usage for normal 48khz audio. It is not a good deal.

My solution is dynamically add SRC in driver according to sample rate so that we have optimized performance. Another way is to build it in topology but delete it in driver for no conversion rate.

That's what others have suggested in the 'AVS' driver, you could bind modules only if required, e.g. if you don't need an instance of SRC you can bypass the SRC.

The problem is that it's of course a lot more complicated to decide whether to bind or not. This would not work for e.g. multiple steps of conversions, because at the moment the DPCM driver cannot be informed of possible rate changes.

Also the introduction of the SRC is a nice feature, but who needs it? In practice, sound servers (CRAS, PulseAudio, etc) already resample to a desired rate, so it's not like we can expect tons of different configurations. Specifically the 44.1kHz case is not required, we've lived since 2013 without SRC in the firmware and no one ever complained about the host-based conversion.

And last, SRC takes resources that could be used for other things that have more added value, such as branded processing, speaker amp. SRC is an item on a program manager checklist, its value as part of the firmware is very very questionable with very very limited returned IMHO. I vote that we don't spend too much time on the optimization and just focus on getting SRC to work. It remains to be seen whether this will actually be used in a product.

@RanderWang
topology2: add support for sampling rate
a2e0194
The original calculation will generate 44 sample size for
44.1khz, actually it at least needs 45 sample size.
This patch uses ceil value for such rate, also inlcude
11.025kh, 22.05kh, 88.2khz, 176.4khz.
Signed-off-by: Rander Wang <rander.wang@intel.com>
@RanderWang
topology2: add cavs src support
690c4d0
Cavs src is different with sof src. It has different uuid
and param which are composed of base module definition and
output rate.
Signed-off-by: Rander Wang <rander.wang@intel.com>
@RanderWang

RanderWang commented Jun 16, 2022

Copy link
Copy Markdown
CollaboratorAuthor

updated.
(1) support all the rate from 8k to 192k
(2) add mixer support

Validated on ADL

@RanderWangRanderWang changed the title [RFC] topology2: add src supporttopology2: add src supportJun 16, 2022
@marc-hb
marc-hb removed their request for review June 16, 2022 17:17
lgirdwood
lgirdwood reviewed Jun 16, 2022
View reviewed changes
Comment threadtools/topology/topology2/include/pipelines/cavs/src-gain-mixin-playback.conf Outdated
fredoh9
fredoh9 reviewed Jun 16, 2022
View reviewed changes
Comment threadtools/topology/topology2/cavs/cavs-sdw-src-gain-mixin.conf Outdated
Comment threadtools/topology/topology2/include/pipelines/cavs/src-gain-mixin-playback.conf Outdated
@RanderWang

RanderWang commented Jun 17, 2022

Copy link
Copy Markdown
CollaboratorAuthor

updated, thanks!

singalsu
singalsu approved these changes Jun 17, 2022
View reviewed changes

@singalsusingalsu left a comment

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.

Looks OK to me for discussed part: frames count calculation.

@fredoh9fredoh9 mentioned this pull request Jun 21, 2022
Closed
@RanderWang
RanderWangforce-pushed the src-ipc4 branch 3 times, most recently from a314de7 to a8afc94CompareJune 24, 2022 05:41
@RanderWang
topology2: add pipeline for src
2af0d94
This pipeline supports conversion from 8k ~ 192k to 48k.
Signed-off-by: Rander Wang <rander.wang@intel.com>
@RanderWang
topology2: add src support on sdw platform
a1d1292
Signed-off-by: Rander Wang <rander.wang@intel.com>
@RanderWang
topology2: add S24LE support
c659e92
S24LE is the major format used by SOF IPC4 FW and windows platform
and has better perfomance than S32.
Signed-off-by: Rander Wang <rander.wang@intel.com>
@lgirdwood
lgirdwood merged commit 773a05a into thesofproject:mainJun 29, 2022
@XiaoyunWu6666

XiaoyunWu6666 commented Jul 1, 2022

Copy link
Copy Markdown
Contributor

Hi @RanderWang , this PR seem to cause some error in cavs-mixin-mixout-hda.tplg and cause multiple IPC4 HDA platforms failed to load the tplg correctly.
Do you have any idea? Thanks

@bardliao

bardliao commented Jul 1, 2022

Copy link
Copy Markdown
Collaborator

Hi @RanderWang , this PR seem to cause some error in cavs-mixin-mixout-hda.tplg and cause multiple IPC4 HDA platforms failed to load the tplg correctly. Do you have any idea? Thanks

@XiaoyunWu6666 Please try #5971

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@plbossartplbossartplbossart left review comments
@lgirdwoodlgirdwoodlgirdwood approved these changes
@singalsusingalsusingalsu approved these changes
@ranj063ranj063Awaiting requested review from ranj063ranj063 is a code owner
@mmaka1mmaka1Awaiting requested review from mmaka1
@lbetlejlbetlejAwaiting requested review from lbetlej
@dbalutadbalutaAwaiting requested review from dbaluta
@aborisovichaborisovichAwaiting requested review from aborisovich
+1 more reviewer
@fredoh9fredoh9fredoh9 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@RanderWang@lgirdwood@plbossart@XiaoyunWu6666@bardliao@singalsu@fredoh9