Tools: Topology2: Intel: Add enhanced speaker and DMIC processing to SDW PCs #9652

singalsu · 2024-11-13T18:02:49Z

No description provided.

tools/topology/topology2/cavs-sdw.conf

lgirdwood

LGTM - are there before and after audio signal response measurements ?

singalsu · 2024-11-14T17:28:33Z

Here's pictures of topologies. The added components are marked:

singalsu · 2024-11-14T17:31:49Z

LGTM - are there before and after audio signal response measurements ?

I have the upper type of device that is without microphone, so I can't demonstrate the capture enhance. But I can make some recording demo of playback with DRC on/off. If I'm able to measure the speaker response I can also demo the EQ on/off. The current 100 Hz high-pass filter is not audible though it helps to reduce speaker distortion when playing loud.

tools/topology/topology2/include/pipelines/cavs/mixout-gain-efx-alh-dai-copier-playback.conf

...gy/topology2/include/pipelines/cavs/mixout-gain-eqiir-eqfir-drc-alh-dai-copier-playback.conf

singalsu · 2024-11-19T17:55:15Z

Note: There's CI SDW capture fail with this PR, finding out what causes it.

singalsu · 2024-11-20T10:28:17Z

Note: There's CI SDW capture fail with this PR, finding out what causes it.

It should be fixed now (module-copier stream name set). Huge thanks to @ujfalusi for testing it and finding the issue!

ujfalusi · 2024-11-20T11:48:56Z

The original PR caused NULL pointer dereference in kernel when using capture on :0,4 because the module copier in the patch have the same stream_name as the ALH copier:

[ 1751.414965] sof_ipc4_prepare_copier_module: looking for Capture-SmartMic (alh-copier.Capture-SmartMic.0)
...
[ 1751.415039] sof_ipc4_prepare_copier_module: checking Capture-SmartMic (module-copier.41.0)
[ 1751.415045] sof_ipc4_prepare_copier_module: alh_copier is NULL!
[ 1751.415050] sof_ipc4_prepare_copier_module: checking  (eqiir.41.0)
[ 1751.415060] sof_ipc4_prepare_copier_module: checking Capture-SmartMic (alh-copier.Capture-SmartMic.0)
[ 1751.415066] sof_ipc4_prepare_copier_module: using Capture-SmartMic (alh-copier.Capture-SmartMic.0)

Other, non DAI copier widgets could have the same stream name (sname) as the ALH copier and in that case the copier->data is NULL, no alh_data is attached, which could lead to NULL pointer dereference. We could check for this NULL pointer in sof_ipc4_prepare_copier_module() and avoid the crash, but a similar loop in sof_ipc4_widget_setup_comp_dai() will miscalculate the ALH device count, causing broken audio. The correct fix is to harden the matching logic by making sure that the 1. widget is a DAI widget - so dai = w->private is valid 2. the dai (and thus the copier) is ALH copier Fixes: 0e357b5 ("ASoC: SOF: ipc4-topology: add SoundWire/ALH aggregation support") Reported-by: Seppo Ingalsuo <[email protected]> Link: thesofproject/sof#9652 Signed-off-by: Peter Ujfalusi <[email protected]>

ujfalusi · 2024-11-20T11:53:05Z

The kernel side fix: thesofproject/linux#5247

ujfalusi · 2024-11-20T12:00:48Z

Note: There's CI SDW capture fail with this PR, finding out what causes it.

It should be fixed now (module-copier stream name set). Huge thanks to @ujfalusi for testing it and finding the issue!

You mean that the module-copier name is no longer set, right?

Other, non DAI copier widgets could have the same stream name (sname) as the ALH copier and in that case the copier->data is NULL, no alh_data is attached, which could lead to NULL pointer dereference. We could check for this NULL pointer in sof_ipc4_prepare_copier_module() and avoid the crash, but a similar loop in sof_ipc4_widget_setup_comp_dai() will miscalculate the ALH device count, causing broken audio. The correct fix is to harden the matching logic by making sure that the 1. widget is a DAI widget - so dai = w->private is valid 2. the dai (and thus the copier) is ALH copier Fixes: 0e357b5 ("ASoC: SOF: ipc4-topology: add SoundWire/ALH aggregation support") Reported-by: Seppo Ingalsuo <[email protected]> Link: thesofproject/sof#9652 Signed-off-by: Peter Ujfalusi <[email protected]>

singalsu · 2024-11-20T16:28:08Z

You mean that the module-copier name is no longer set, right?

Yes that's correct, I left out the stream_name set for module-copier. I want this to work with released kernels.

singalsu · 2024-11-20T17:03:45Z

LGTM - are there before and after audio signal response measurements ?

I made a video of processing off, generic processing settings for speaker, and (quickly done) tuned settings but with size limit of 10 MB I'm not able to attach it even with lowest 360p export quality. Attached are the tuned IIR and FIR EQ responses and before and after frequency responses those I applied for the used notebook PC.

The used DRC compression curve is as shown here:

Other, non DAI copier widgets could have the same stream name (sname) as the ALH copier and in that case the copier->data is NULL, no alh_data is attached, which could lead to NULL pointer dereference. We could check for this NULL pointer in sof_ipc4_prepare_copier_module() and avoid the crash, but a similar loop in sof_ipc4_widget_setup_comp_dai() will miscalculate the ALH device count, causing broken audio. The correct fix is to harden the matching logic by making sure that the 1. widget is a DAI widget - so dai = w->private is valid 2. the dai (and thus the copier) is ALH copier Fixes: 0e357b5 ("ASoC: SOF: ipc4-topology: add SoundWire/ALH aggregation support") Reported-by: Seppo Ingalsuo <[email protected]> Link: thesofproject/sof#9652 Signed-off-by: Peter Ujfalusi <[email protected]>

lgirdwood

@bardliao kernel ready now ?
@ranj063 pls review.

kv2019i

Looks good! Some minor/non-blocking comments inline.

kv2019i · 2024-11-21T19:58:54Z

tools/topology/topology2/platform/intel/sdw-amp-generic.conf

@@ -10,6 +10,7 @@ Define {
 	AMP_FEEDBACK_CH 2


@singalsu Style note, avoid using passive verbs like this "DAI pipelines are aligned". Just explain what the patch does like "align DAI with other topologies" (more examples at https://docs.zephyrproject.org/latest/contribute/guidelines.html#commit-message-guidelines ). E.g here I first thought you were describing how things are before your changes. But that's not the case, but it's actually thi spatch that does the aligning.

Yes, that makes sense, the text really could be interpreted as not what I meant. I'll fix.

bardliao · 2024-11-22T09:29:41Z

@bardliao kernel ready now ? @ranj063 pls review.

@lgirdwood I checked with @singalsu and there is no new required from kernel.

The control names "Amplifier Volume" and "Main Amplifier Volume" are not describing well the fact that they control speaker endpoint volume before and after SOF mixer. This patch changes the control names to: - "Pre Mixer Speaker Playback Volume" - "Post Mixer Speaker Playback Volume" These new names are similar as in other SOF topologies. There is no UCM rules dependency with the names yet, so good to do it now. Signed-off-by: Seppo Ingalsuo <[email protected]>

…oute To use the pipeline class as SubTreeCopy base class the route needs to be an array. Without it the route from base is not present in the modified class. Suggested-by: Ranjani Sridharan <[email protected]> Signed-off-by: Seppo Ingalsuo <[email protected]>

This patch adds to ALH pipeline between gain and ALH processing with IIR, FIR, and DRC. The change is intended for topologies for PCs running Linux OS. Topologies with enable enhanced playback are currently: - sof-mtl-rt713-l0-rt1316-l12.tplg - sof-lnl-rt722-l0.tplg The change adds in MTL platform 4.2 MCPS load to speaker playback in pass-through mode. With highpass IIR and default speaker DRC the load increase is 45 MCPS. Other topologies than above have no MCPS increase. Signed-off-by: Seppo Ingalsuo <[email protected]>

This patch adds to all topologies with SDW DMIC the module copier to in ALH copier pipeline after the IIR equalizer. The module copier allows to duplicate the stream for several post-processing pipelines as analogy with mixin/mixout in playback pipelines. When SDW_DMIC_ENHANCED_CAPTURE is set "true" in topology build the multi-microphone beamformer (TDFB) and dynamic range control (DRC) components are added to host pipeline. The capture pre-processing is enabled for PC topology sof-lnl-rt722-l0.tplg. The beamformer is set for a narrow opening stereo image suitable for notebook user voice enhance. The DRC is set for a default DMIC level boost. Signed-off-by: Seppo Ingalsuo <[email protected]>

This patch adds speaker post-processing and microphone pre-processing to next topologies: - sof-mtl-rt711-l0-rt1316-l23-rt714-l1.tplg - sof-lnl-rt711-l0-rt1316-l23-rt714-l1.tplg The SOF CI system currently tests these two SDW platforms. This patch adds test coverage for processing with IIR, FIR, DRC, and TDFB components. Signed-off-by: Seppo Ingalsuo <[email protected]>

Other, non DAI copier widgets could have the same stream name (sname) as the ALH copier and in that case the copier->data is NULL, no alh_data is attached, which could lead to NULL pointer dereference. We could check for this NULL pointer in sof_ipc4_prepare_copier_module() and avoid the crash, but a similar loop in sof_ipc4_widget_setup_comp_dai() will miscalculate the ALH device count, causing broken audio. The correct fix is to harden the matching logic by making sure that the 1. widget is a DAI widget - so dai = w->private is valid 2. the dai (and thus the copier) is ALH copier Fixes: 0e357b5 ("ASoC: SOF: ipc4-topology: add SoundWire/ALH aggregation support") Reported-by: Seppo Ingalsuo <[email protected]> Link: thesofproject/sof#9652 Signed-off-by: Peter Ujfalusi <[email protected]> Reviewed-by: Liam Girdwood <[email protected]> Reviewed-by: Ranjani Sridharan <[email protected]> Reviewed-by: Bard Liao <[email protected]>

singalsu commented Nov 14, 2024

View reviewed changes

tools/topology/topology2/cavs-sdw.conf Show resolved Hide resolved

singalsu force-pushed the tplg_sdw_add_enhanced_speaker_mic branch from 04b0c72 to 86e992c Compare November 14, 2024 16:04

singalsu changed the title ~~[wip]Tools: Topology2: Add enhanced speaker processing to SDW PCs~~ Tools: Topology2: Intel: Add enhanced speaker and DMIC processing to SDW PCs Nov 14, 2024

lgirdwood reviewed Nov 14, 2024

View reviewed changes

ranj063 reviewed Nov 14, 2024

View reviewed changes

tools/topology/topology2/include/pipelines/cavs/mixout-gain-efx-alh-dai-copier-playback.conf Outdated Show resolved Hide resolved

singalsu force-pushed the tplg_sdw_add_enhanced_speaker_mic branch from 86e992c to 3f8259b Compare November 19, 2024 14:43

singalsu marked this pull request as ready for review November 19, 2024 15:30

singalsu requested a review from jsarha as a code owner November 19, 2024 15:30

lgirdwood approved these changes Nov 19, 2024

View reviewed changes

ranj063 reviewed Nov 19, 2024

View reviewed changes

...gy/topology2/include/pipelines/cavs/mixout-gain-eqiir-eqfir-drc-alh-dai-copier-playback.conf Outdated Show resolved Hide resolved

singalsu force-pushed the tplg_sdw_add_enhanced_speaker_mic branch from 3f8259b to a837f57 Compare November 20, 2024 10:33

ujfalusi mentioned this pull request Nov 20, 2024

ASoC: SOF: ipc4-topology: Harden loops for looking up ALH copiers thesofproject/linux#5247

Merged

lgirdwood approved these changes Nov 21, 2024

View reviewed changes

singalsu force-pushed the tplg_sdw_add_enhanced_speaker_mic branch from a837f57 to abf578d Compare November 21, 2024 17:20

singalsu requested a review from ranj063 November 21, 2024 17:22

ranj063 approved these changes Nov 21, 2024

View reviewed changes

kv2019i approved these changes Nov 21, 2024

View reviewed changes

singalsu added 5 commits November 22, 2024 11:44

singalsu force-pushed the tplg_sdw_add_enhanced_speaker_mic branch from abf578d to 591c2cb Compare November 22, 2024 09:45

lgirdwood approved these changes Nov 22, 2024

View reviewed changes

lgirdwood merged commit 9844fa7 into thesofproject:main Nov 22, 2024
43 of 47 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tools: Topology2: Intel: Add enhanced speaker and DMIC processing to SDW PCs #9652

Tools: Topology2: Intel: Add enhanced speaker and DMIC processing to SDW PCs #9652

singalsu commented Nov 13, 2024 •

edited

Loading

lgirdwood left a comment

singalsu commented Nov 14, 2024

singalsu commented Nov 14, 2024 •

edited

Loading

singalsu commented Nov 19, 2024

singalsu commented Nov 20, 2024

ujfalusi commented Nov 20, 2024

ujfalusi commented Nov 20, 2024

ujfalusi commented Nov 20, 2024

singalsu commented Nov 20, 2024 •

edited

Loading

singalsu commented Nov 20, 2024 •

edited

Loading

lgirdwood left a comment

kv2019i left a comment

kv2019i Nov 21, 2024

singalsu Nov 22, 2024

bardliao commented Nov 22, 2024

Tools: Topology2: Intel: Add enhanced speaker and DMIC processing to SDW PCs #9652

Tools: Topology2: Intel: Add enhanced speaker and DMIC processing to SDW PCs #9652

Conversation

singalsu commented Nov 13, 2024 • edited Loading

lgirdwood left a comment

Choose a reason for hiding this comment

singalsu commented Nov 14, 2024

singalsu commented Nov 14, 2024 • edited Loading

singalsu commented Nov 19, 2024

singalsu commented Nov 20, 2024

ujfalusi commented Nov 20, 2024

ujfalusi commented Nov 20, 2024

ujfalusi commented Nov 20, 2024

singalsu commented Nov 20, 2024 • edited Loading

singalsu commented Nov 20, 2024 • edited Loading

lgirdwood left a comment

Choose a reason for hiding this comment

kv2019i left a comment

Choose a reason for hiding this comment

kv2019i Nov 21, 2024

Choose a reason for hiding this comment

singalsu Nov 22, 2024

Choose a reason for hiding this comment

bardliao commented Nov 22, 2024

singalsu commented Nov 13, 2024 •

edited

Loading

singalsu commented Nov 14, 2024 •

edited

Loading

singalsu commented Nov 20, 2024 •

edited

Loading

singalsu commented Nov 20, 2024 •

edited

Loading