sof
sof copied to clipboard
Topology2: add ssp multi stream capture
This PR added multi stream capture support on SSP.
@bardliao @RanderWang which platform were you able to test this on ?
I tested it on ADL & MTL devices.
@bardliao @RanderWang are those topologies used in CI? Or can you share what they look like with the png image?
@bardliao @RanderWang are those topologies used in CI? Or can you share what they look like with the png image?
No, they are not used in CI.
@bardliao @RanderWang which platform were you able to test this on ?
I tested it on TGL
@bardliao not sure if you saw my comments last week but we'd want to have one 'master' gain and 2 'local' gains for the two PCMs. Once we've done the gains we can replace the master gain by EQIIR for DMICs, which is badly needed as confirmed by @singalsu
@bardliao not sure if you saw my comments last week but we'd want to have one 'master' gain and 2 'local' gains for the two PCMs. Once we've done the gains we can replace the master gain by EQIIR for DMICs, which is badly needed as confirmed by @singalsu
@plbossart I updated my PR. New topology looks like
Is this what you expect?
@bardliao yes this is what I had in mind, I was wondering if we could actually use 3 copiers in this way,
One remark that I have for topologies is that we may want to be clearer on indices used. It's not clear to me is if the copier.SSP2.1 refers to pipeline2 or SSP2. We should have something that help bind copiers to interfaces, otherwise it'll be a support nightmare. @bardliao @RanderWang @ranj063 @singalsu @lgirdwood thoughts?
@bardliao yes this is what I had in mind, I was wondering if we could actually use 3 copiers in this way,
@plbossart Currently, only DAI and host type copiers are supported by kernel. I am adding support for the copier "module" type.
@bardliao yes this is what I had in mind, I was wondering if we could actually use 3 copiers in this way,
One remark that I have for topologies is that we may want to be clearer on indices used. It's not clear to me is if the copier.SSP2.1 refers to pipeline2 or SSP2. We should have something that help bind copiers to interfaces, otherwise it'll be a support nightmare. @bardliao @RanderWang @ranj063 @singalsu @lgirdwood thoughts?
@plbossart the name should be "component_name.pipeline_id.component_id" . so copier.SSP.2.1 means this copier.SSP1 is in pipeline 2 and copier.host.1.1 = copier.host1 in pipeline 1
@bardliao yes this is what I had in mind, I was wondering if we could actually use 3 copiers in this way,
@plbossart Currently, only DAI and host type copiers are supported by kernel. I am adding support for the copier "module" type.
ok, sounds good.
@bardliao yes this is what I had in mind, I was wondering if we could actually use 3 copiers in this way, One remark that I have for topologies is that we may want to be clearer on indices used. It's not clear to me is if the copier.SSP2.1 refers to pipeline2 or SSP2. We should have something that help bind copiers to interfaces, otherwise it'll be a support nightmare. @bardliao @RanderWang @ranj063 @singalsu @lgirdwood thoughts?
@plbossart the name should be "component_name.pipeline_id.component_id" . so copier.SSP.2.1 means this copier.SSP1 is in pipeline 2 and copier.host.1.1 = copier.host1 in pipeline 1
My point exactly, we have NOTHING that tells us which SSP this copier deals with. The .1 suffix is meaningless for a user trying to understand what the topology does.
@bardliao not sure if you saw my comments last week but we'd want to have one 'master' gain and 2 'local' gains for the two PCMs. Once we've done the gains we can replace the master gain by EQIIR for DMICs, which is badly needed as confirmed by @singalsu
@plbossart I updated my PR. New topology looks like
Is this what you expect?
@bardliao why do we need copier.module.8.2 and gain.8.1 at all?
@kv2019i @RanderWang @ranj063 pls review
@bardliao do you finalize the design ?
@kv2019i @RanderWang @ranj063 pls review
@bardliao do you finalize the design ?
@lgirdwood @RanderWang That is a good question. I hope this is the final design. But the IPC4 tester will report bind error with this topology. Removing the gain module in the host pipelines can fix the error. But the IPC4 tester will crash when the second capture PCM is started after fixing the bind error issue. I am not sure if I need to modify the topology or we need to fix the IPC4 tester issue.
@kv2019i @RanderWang @ranj063 pls review
@bardliao do you finalize the design ?
@lgirdwood @RanderWang That is a good question. I hope this is the final design. But the IPC4 tester will report bind error with this topology. Removing the gain module in the host pipelines can fix the error. But the IPC4 tester will crash when the second capture PCM is started after fixing the bind error issue. I am not sure if I need to modify the topology or we need to fix the IPC4 tester issue.
ok, does the topology work on main branch and not on tester ?
ok, does the topology work on main branch and not on tester ?
Yes
ok, does the topology work on main branch and not on tester ?
Yes
ok, this is fine. It can be merged.
I am not comfortable with differences in behavior between different implementations of the IPC4 protocol and sequences.
We already have a number of issues related to trigger, now this is with bind. This probably highlights a conceptual miss somewhere.
Can one of the admins verify this patch?
ok, does the topology work on main branch and not on tester ?
Yes
ok, this is fine. It can be merged.
@lgirdwood if we merge this with the tester failing, we would lose our reference point. I have a feeling we're missing something trivial in tplg. Let me take a look
@bardliao this is the fix you need and this is how the tplg will look like:
0001-gain-capture-Add-copier-module.txt
@plbossart @RanderWang what do you think?
@bardliao this is the fix you need and this is how the tplg will look like: 0001-gain-capture-Add-copier-module.txt
This fixed the binding error, but the IPC4 tester will still crash when the second capture PCM is started.
This fixed the binding error, but the IPC4 tester will still crash when the second capture PCM is started.
@RanderWang pls let the tester folks know about this tester bug and CC @ranj063 . @ranj063 the topology looks valid and is permitted in the ABI, it could be a restriction in the tester though.
This fixed the binding error, but the IPC4 tester will still crash when the second capture PCM is started.
@RanderWang pls let the tester folks know about this tester bug and CC @ranj063 . @ranj063 the topology looks valid and is permitted in the ABI, it could be a restriction in the tester though.
@bardliao pls let the tester folks know about this tester bug and CC
@bardliao @RanderWang @ranj063 any blocker now (except for the conflict) ?
@bardliao @RanderWang @ranj063 any blocker now (except for the conflict) ?
@lgirdwood we fixed one issue with the copier yesterday to make this tplg work. We'd have to run more test cases before we merge this. I'll let @bardliao start a test run and report back when things look decent
@bardliao @RanderWang @ranj063 any blocker now (except for the conflict) ?
@lgirdwood we fixed one issue with the copier yesterday to make this tplg work. We'd have to run more test cases before we merge this. I'll let @bardliao start a test run and report back when things look decent
and oh I forgot the kernel PR #3957 is a pre-requisite for this
This fixed the binding error, but the IPC4 tester will still crash when the second capture PCM is started.
@RanderWang pls let the tester folks know about this tester bug and CC @ranj063 . @ranj063 the topology looks valid and is permitted in the ABI, it could be a restriction in the tester though.
@bardliao pls let the tester folks know about this tester bug and CC
Yes, I sent email to them and CC you and Ranjani on Oct. 26th.
@bardliao @RanderWang @ranj063 any blocker now (except for the conflict) ?
@lgirdwood @ranj063 The conflict will be fixed if https://github.com/thesofproject/sof/pull/6499 is merged.
I tested with https://github.com/thesofproject/linux/pull/3888 + https://github.com/thesofproject/linux/pull/3957 and didn't find any issue.
@bardliao @RanderWang @ranj063 any blocker now (except for the conflict) ?
@lgirdwood @ranj063 The conflict will be fixed if #6499 is merged.
I tested with thesofproject/linux#3888 + thesofproject/linux#3957 and didn't find any issue.
All dependencies now merged. Lets get it in daily test for a few days.