Skip to content
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.

Sign up for GitHub

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

Jump to bottom

topology2: add a mixin-mixout level to nocodec #6981

Merged
merged 1 commit into from
Mar 13, 2023
Merged

topology2: add a mixin-mixout level to nocodec #6981

merged 1 commit into from
Mar 13, 2023

Conversation

lyakh
Copy link
Collaborator

@lyakh lyakh commented Jan 20, 2023

Add a second level of mixin-mixout mixing and a PCM to the nocodec topology. Note, that it adds more hard-coded pipeline, route and PCM numbers in included topology files, but apparently there's no way ATM to specify those numbers at the caller level

@lyakh lyakh requested a review from ranj063 January 20, 2023 14:50
ranj063
ranj063 reviewed Jan 21, 2023
View reviewed changes
tools/topology/topology2/cavs-nocodec.conf Outdated
@@ -512,3 +519,8 @@ Object.Base {
}

}

IncludeByKey.SSP0_MIXER_2LEVEL {
"0" "platform/intel/nocodec-ssp0-1level.conf"
Copy link
Collaborator

Choose a reason for hiding this comment

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

@lyakh there's pending PR in alsa-utils that allows us to include short conf blocks without vreating a new conf file. alsa-project/alsa-utils#187. It will be good to use that after it is merged

@lyakh
Copy link
Collaborator Author

lyakh commented Jan 23, 2023

SOFCI TEST

@lyakh
Copy link
Collaborator Author

lyakh commented Jan 23, 2023

on-device test got stuck, retrigger

@lyakh
Copy link
Collaborator Author

lyakh commented Jan 23, 2023

SOFCI TEST

@lyakh
Copy link
Collaborator Author

lyakh commented Jan 23, 2023

@plbossart @lgirdwood took me a while but here's how we can make this per-platform. This approach to selecting "ID ranges" and making sure they don't overlap is ugly, but I'm not aware of a better solution with topology2.

@lyakh
Copy link
Collaborator Author

lyakh commented Jan 23, 2023

Opened #6985 to address ID hard-coding

@lgirdwood
Copy link
Member

@ranj063 whats the plan here wrt the ALSA update, i.e. do you want ALSA update merged first and this reworked to use it ?

lgirdwood
lgirdwood reviewed Jan 26, 2023
View reviewed changes
src/include/ipc4/base_fw.h Outdated
@@ -42,7 +42,7 @@
#define IPC4_MAX_LIBS_COUNT 16

/* Max pipeline count supported by ipc4 */
#define IPC4_MAX_PPL_COUNT 16
#define IPC4_MAX_PPL_COUNT 32
Copy link
Member

Choose a reason for hiding this comment

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

@marcinszkudlinski @abonislawski @mwasko fyi - any issue here ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@lgirdwood looks good to me, I believe it should be 32 at a first place. @mmaka1?

@ranj063
Copy link
Collaborator

ranj063 commented Jan 26, 2023

@ranj063 whats the plan here wrt the ALSA update, i.e. do you want ALSA update merged first and this reworked to use it ?

@lgirdwood yes, if this is not urgent i'd like to wait until after the alsautils PR.

@lgirdwood
Copy link
Member

@ranj063 whats the plan here wrt the ALSA update, i.e. do you want ALSA update merged first and this reworked to use it ?

@lgirdwood yes, if this is not urgent i'd like to wait until after the alsautils PR.

ok @lyakh @ranj063 pls ping once ALSA PR is merged.

@lgirdwood
Copy link
Member

@lyakh ping

@lyakh
Copy link
Collaborator Author

lyakh commented Feb 10, 2023

@lyakh ping

@lgirdwood alsa-utils PR from @ranj063 has now been dropped in favour of an alternative approach completely internal to SOF. @ranj063 has developed a PR to convert SOF to that new approach, once it's merged we should be able to proceed with this PR too.

@lyakh lyakh mentioned this pull request Feb 17, 2023
Closed
@lyakh
topology2: add a mixin-mixout level to nocodec
5a6692d 
Add a second level of mixin-mixout mixing and a PCM to the nocodec
topology.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
@lyakh lyakh marked this pull request as ready for review February 28, 2023 06:52
@lyakh lyakh requested a review from ranj063 February 28, 2023 06:53
@lgirdwood
Copy link
Member

@plbossart this now looks like https://learn.microsoft.com/en-us/windows-hardware/drivers/audio/audio-processing-object-architecture but with 3 playback PCMS and not 4 as per Microsoft diagram, good for you ?

ranj063
ranj063 reviewed Mar 6, 2023
View reviewed changes
tools/topology/topology2/development/CMakeLists.txt
@@ -13,7 +13,9 @@ set(TPLGS

# CAVS SSP topology for TGL
"cavs-nocodec\;sof-tgl-nocodec\;NUM_DMICS=4,PDM1_MIC_A_ENABLE=1,PDM1_MIC_B_ENABLE=1,\
PREPROCESS_PLUGINS=nhlt,NHLT_BIN=nhlt-sof-tgl-nocodec.bin,DEEPBUFFER_FW_DMA_MS=100"
PREPROCESS_PLUGINS=nhlt,NHLT_BIN=nhlt-sof-tgl-nocodec.bin,DEEPBUFFER_FW_DMA_MS=100,\
SSP0_MIXER_2LEVEL=1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

@lyakh this is good but why not MTL too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ranj063 because we don't test MTL in PR CI yet. Maybe we can enable MTL separately after this one is merged and has run a daily test

@lgirdwood
Copy link
Member

@plbossart good now ?

@lgirdwood
Copy link
Member

@plbossart ping, good for you ?

plbossart
plbossart approved these changes Mar 10, 2023
View reviewed changes
Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

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

I thought this was merged eons ago... Time to get this in and break more things haha.

@kv2019i kv2019i merged commit 8a48c78 into thesofproject:main Mar 13, 2023
@lyakh lyakh deleted the mixer branch March 13, 2023 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mwasko mwasko mwasko left review comments

@lgirdwood lgirdwood lgirdwood approved these changes

@ranj063 ranj063 ranj063 approved these changes

@plbossart plbossart plbossart approved these changes

@marc-hb marc-hb Awaiting requested review from marc-hb

@aborisovich aborisovich Awaiting requested review from aborisovich

@jsarha jsarha Awaiting requested review from jsarha jsarha is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@lyakh @lgirdwood @ranj063 @plbossart @mwasko @kv2019i