topology2: add SDW echo reference function topologies#10573
topology2: add SDW echo reference function topologies#10573bardliao wants to merge 3 commits intothesofproject:mainfrom
Conversation
Currently, we implement the echo reference pipeline, widgets, routes in the SoundWire config files. It is hard to create a stand alone echo reference topology for the function topology usages. This commit moves the echo reference part from the original config file and include the new echo reference in the original file. The commit also use macros to set the echo reference IDs to fix the issue that both the 2nd SoundWire amp and the echo reference use the same pipeline ID. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
set sdw-amp-echo-ref.conf and sdw-jack-echo-ref.conf are included in the sdw-amp-generic.conf and sdw-jack-generic.conf if needed. But for the function topology usages, we will create the jack/amp function topology and the corresponding echo reference topology separately. Therefore, we need to include the echo reference config files if SDW_ECHO_REF_DAI is set but sdw-jack/amp-generic.conf is not included. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Add SoundWire echo reference function topologies. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors and adds SoundWire (SDW) echo reference function topologies that are loaded when the "Loopback_Virtual" DAI link is created by the kernel. The refactoring extracts echo reference configuration from inline definitions into separate, reusable configuration files.
Changes:
- Added three new topology targets for echo reference configurations: amp-only, jack-only, and combined jack-amp
- Extracted jack echo reference configuration from sdw-jack-generic.conf into a new standalone file sdw-jack-echo-ref.conf
- Extracted speaker/amp echo reference configuration from sdw-amp-generic.conf into a new standalone file sdw-amp-echo-ref.conf
- Added conditional includes in cavs-sdw.conf to include echo reference configurations when SDW_ECHO_REF_DAI is enabled
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/topology/topology2/production/tplg-targets-sdca-generic.cmake | Adds three new topology build targets for echo reference function configurations (amp-ref, jack-ref, jack-amp-ref) |
| tools/topology/topology2/platform/intel/sdw-jack-generic.conf | Removes inline jack echo reference code and replaces with include statement for new separate file |
| tools/topology/topology2/platform/intel/sdw-jack-echo-ref.conf | New file containing extracted and parameterized jack echo reference configuration |
| tools/topology/topology2/platform/intel/sdw-amp-generic.conf | Removes inline amp echo reference code and replaces with include statement for new separate file |
| tools/topology/topology2/platform/intel/sdw-amp-echo-ref.conf | New file containing extracted and parameterized amp echo reference configuration |
| tools/topology/topology2/cavs-sdw.conf | Adds conditional includes to load echo reference configurations when SDW_ECHO_REF_DAI is enabled |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| IncludeByKey.SDW_JACK { | ||
| "false" "platform/intel/sdw-jack-echo-ref.conf" |
There was a problem hiding this comment.
The conditional logic appears incorrect. This condition includes the jack echo reference configuration when SDW_JACK is "false", but it should logically check if SDW_JACK_ECHO_REF is "true" instead. The current check on SDW_JACK being false is too broad and doesn't directly correlate with whether jack echo reference functionality is needed. This should check "SDW_JACK_ECHO_REF" with value "true" to match the pattern used in sdw-jack-generic.conf line 715.
| IncludeByKey.SDW_JACK { | |
| "false" "platform/intel/sdw-jack-echo-ref.conf" | |
| IncludeByKey.SDW_JACK_ECHO_REF { | |
| "true" "platform/intel/sdw-jack-echo-ref.conf" |
| SDW_JACK_OUT_STREAM 'Playback-SimpleJack' | ||
| JACK_RATE 48000 | ||
| SDW_JACK_ECHO_REF_PCM_ID 11 | ||
| SDW_JACK_ECHO_ERF_PIPELINE_ID 110 |
There was a problem hiding this comment.
The variable name contains a spelling error: "SDW_JACK_ECHO_ERF_PIPELINE_ID" should be "SDW_JACK_ECHO_REF_PIPELINE_ID" (ERF -> REF). This inconsistency with the rest of the codebase where all similar variables use "ECHO_REF" (for "Reference") may cause confusion. The same typo appears throughout this file in multiple locations.
| Define { | ||
| SDW_SPK_STREAM 'Playback-SmartAmp' | ||
| SDW_SPK_ECHO_REF_PCM_ID 12 | ||
| SDW_SPK_ECHO_ERF_PIPELINE_ID 120 |
There was a problem hiding this comment.
The variable name contains a spelling error: "SDW_SPK_ECHO_ERF_PIPELINE_ID" should be "SDW_SPK_ECHO_REF_PIPELINE_ID" (ERF -> REF). This is inconsistent with other echo reference variables in the codebase which use "ECHO_REF" (for "Reference"). The same typo appears throughout this file in multiple locations.
| IncludeByKey.NUM_SDW_AMP_LINKS { | ||
| "0" "platform/intel/sdw-amp-echo-ref.conf" |
There was a problem hiding this comment.
The conditional logic appears incorrect. This condition includes the amp echo reference configuration only when NUM_SDW_AMP_LINKS is "0" (no amp links), but amp echo reference should logically be included when SDW_SPK_ECHO_REF is "true" instead. The current logic would include speaker echo reference configuration even when there are no speaker/amp devices, which doesn't make sense. This should check "SDW_SPK_ECHO_REF" with value "true" to match the pattern used in sdw-amp-generic.conf line 1006.
| IncludeByKey.NUM_SDW_AMP_LINKS { | |
| "0" "platform/intel/sdw-amp-echo-ref.conf" | |
| IncludeByKey.SDW_SPK_ECHO_REF { | |
| "true" "platform/intel/sdw-amp-echo-ref.conf" |
Add SoundWire echo reference function topologies. Those topologies will be loaded only when the "Loopback_Virtual" DAI link is created by the kernel.