Skip to content

Comments

topology2: add SDW echo reference function topologies#10573

Open
bardliao wants to merge 3 commits intothesofproject:mainfrom
bardliao:topology2-echo-ref
Open

topology2: add SDW echo reference function topologies#10573
bardliao wants to merge 3 commits intothesofproject:mainfrom
bardliao:topology2-echo-ref

Conversation

@bardliao
Copy link
Collaborator

Add SoundWire echo reference function topologies. Those topologies will be loaded only when the "Loopback_Virtual" DAI link is created by the kernel.

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>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

Comment on lines +173 to +174
IncludeByKey.SDW_JACK {
"false" "platform/intel/sdw-jack-echo-ref.conf"
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
IncludeByKey.SDW_JACK {
"false" "platform/intel/sdw-jack-echo-ref.conf"
IncludeByKey.SDW_JACK_ECHO_REF {
"true" "platform/intel/sdw-jack-echo-ref.conf"

Copilot uses AI. Check for mistakes.
SDW_JACK_OUT_STREAM 'Playback-SimpleJack'
JACK_RATE 48000
SDW_JACK_ECHO_REF_PCM_ID 11
SDW_JACK_ECHO_ERF_PIPELINE_ID 110
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Define {
SDW_SPK_STREAM 'Playback-SmartAmp'
SDW_SPK_ECHO_REF_PCM_ID 12
SDW_SPK_ECHO_ERF_PIPELINE_ID 120
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

@bardliao this looks valid.

Comment on lines +170 to +171
IncludeByKey.NUM_SDW_AMP_LINKS {
"0" "platform/intel/sdw-amp-echo-ref.conf"
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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"

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants