linux icon indicating copy to clipboard operation
linux copied to clipboard

ADL / RPL : Enable hostless playback and capture

Open sathyap-chrome opened this issue 3 years ago • 14 comments

When we use Smart AMP needing IV feedback in DSP ( Like MAX98373) the IV feedback starts only when we open associated PCM on the host side. This indirectly leads to high power consumption even for playback scenario.

Also for upcoming new features like ULTRASOUND, a playback and capture pipe without assocaited PCM is required. Hence started this issue for overall understanding dependancies and get this working.

cc @kv2019i @ranj063 @sathya-nujella @RDharageswari @jairaj-arava @Vamshigopal

sathyap-chrome avatar Jul 08 '22 11:07 sathyap-chrome

referring to the TGL issue here: https://github.com/thesofproject/sof/issues/3663

sathya-nujella avatar Jul 08 '22 15:07 sathya-nujella

@sathya-nujella can you clarify if the host capture for echo reference is required or not? If the echo reference was only used internally in the firmware by an AEC module, that would simplify the graph and make this echo reference pipeline useless.

plbossart avatar Jul 20 '22 19:07 plbossart

@sathya-nujella can you clarify if the host capture for echo reference is required or not? If the echo reference was only used internally in the firmware by an AEC module, that would simplify the graph and make this echo reference pipeline useless.

We have couple of scenarios:

  1. DUT has "smart amp" which needs DSM to run in DSP: This is the current problem scenario. Here, without "echo ref" being open by CRAS, DSM is not able to get IV data from amp. (as hostess capture not working).

  2. DUT has "smart amp", but DSM happening within amp: We had some models in ADL with this, in these models - Google has enabled "AEC" in SOF. When AEC enabled in that model, echo reference is not given to user space.

  3. We have some models, where AEC not enabled in SOF: For these models, echo ref is given to user space.

IMHO: It might take a while to assume AEC will be enabled by default in all OEM devices using SOF. So, it would be helpful to keep an option to allow to send "echo ref" to user space.

@cujomalainey - FYI

sathya-nujella avatar Jul 21 '22 02:07 sathya-nujella

@plbossart it depends on the scenario/use case whether the host needs the data. There are few cases missing here.

  • E.g. Given a device that has AEC in DSP on DMIC path
    • With Speaker playback and DMIC capture no host capture of feedback needed of the speaker protection path
    • With Speaker playback and HS capture, host capture is needed of the speaker protection path

cujomalainey avatar Jul 21 '22 17:07 cujomalainey

  • With Speaker playback and HS capture, host capture is needed @cujomalainey in this case is the idea that AEC will run on the host?

ranj063 avatar Jul 21 '22 17:07 ranj063

I think these two are inverted @cujomalainey, did you mean

With Speaker playback and DMIC capture, host capture of feedback needed (to deal with AEC) With Speaker playback and HS capture, no host capture is needed (since there's no need for AEC)

plbossart avatar Jul 21 '22 17:07 plbossart

@plbossart no I did not make an error, the AEC can still run on second scenario but it runs in cras vs sof

cujomalainey avatar Jul 21 '22 19:07 cujomalainey

oh wow. different instances of AEC depending on the endpoint.

@cujomalainey can this be simplified instead of asking for a really complicated infrastructure change? I don't see how we can deal with 'we need this sometimes but others not really'. Every time we fragment a capability in multiple subcases, we add a risk factor and the implementation complexity increases exponentially. Do few things and do them well (tm).

plbossart avatar Jul 21 '22 20:07 plbossart

@plbossart we actually have 3 AECs in the stack (one is slated for removal once the other two reach the whole fleet)

I agree, but we have a number of issues to resolve this.

  1. AEC needs to be in front of any algorithms
  2. AEC is expensive both computationally and memory wise
  3. AEC needs to be instantiated on all capture paths
  4. AEC needs to be able to use all playback paths as a reference if we choose to use it

point 1. forces AEC on DSP for any NC based systems point 2. prevents us from instantiating it more than once on NC systems point 4. would make a spaghetti mess of the pipes in SOF if we tried to do it for HDMI, hence we fall back to CRAS AEC in those situations and disable any algorithms if we have to.

In an ideal world we could just provide the summed up playback paths to SOF directly from CRAS for the AEC so we could avoid 4 (doesn't work for smart amp systems) and containerize any algorithms so 2 is not an issue on the DSP, but unfortunately we are not there yet.

cujomalainey avatar Jul 21 '22 21:07 cujomalainey

point 2. prevents us from instantiating it more than once on NC systems

I don't know what NC means, but having to duplicate AEC on multiple paths is a rather sub-optimal implementation. If that's the issue then it's what needs to be fixed first. We should be able to route capture data to a single AEC module. That's how most phones work :-)

plbossart avatar Jul 21 '22 22:07 plbossart

NC -> noise cancelling

The problem with a single AEC is it mixes the capture path which is not what we want, we want the end points to be independent

cujomalainey avatar Jul 22 '22 03:07 cujomalainey

we want the end points to be independent

we're quickly going to run out of resources (memory, MCPS) and engineering time to deal with all this....

plbossart avatar Jul 22 '22 15:07 plbossart

@plbossart @cujomalainey Can I ask for a clarification here going back to the original ask for fixing the power consumption issue? Are we talking about having AEC in the DSP even on the IV feedbackpath for the smartamp implementation as well? And is that AEC going to be done in the DSP or do we want that done in the userspace?

ranj063 avatar Jul 22 '22 16:07 ranj063

@plbossart agreed, hence why we ask for more RAM

@ranj063 ideally the option for AP or DSP for AEC. It would be good if we can dynamically switch it.

cujomalainey avatar Jul 26 '22 22:07 cujomalainey