-
Notifications
You must be signed in to change notification settings - Fork 349
tools: topology2: move deepbuffer defaults to separate file #10226
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tools: topology2: move deepbuffer defaults to separate file #10226
Conversation
| DEEP_BUFFER_PIPELINE_SINK 'mixout.1.1' | ||
| DEEP_BUFFER_PCM_NAME 'Deepbuffer Jack Out' | ||
| DEEP_BUFFER_PIPELINE_ID_2 16 | ||
| DEEP_BUFFER_PCM_ID_2 35 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some typos in git commit, will fix:
- this also changes the defaults, need to address that (not just SDW)
- will need to add a note this will not change the name for old I2S topologies (these have been shipping for some time and will be a bigger problem if the PCM name changes)
Follow similar split between defaults and defitinions for deep buffer as has been used for Bluetooth, HDMI and other optional features features. This fixes a regression in commit b6ee409 ("Topology2: Add sof-ptl-rt712-l3-rt1320-l3/-4ch support"). This commit changed the defaults to match DSP topology of cavs-sdw.conf and broke topologies based on cavs-rt5682.conf. cavs-rt5682.conf had correct settings, but these were ignored as deep-buffer.conf is included in wrong order. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Change the default PCM name for deep buffer speaker to "Deepbuffer Speaker". After this change, the naming is more consistent: - Jack Out - Speaker - Deepbuffer Jack Out - Deepbuffer Speaker Keep the existing topologies based on cavs-rt5682.conf unaffected (keep using old "Deepbuffer Amps" PCM name) as these are widely used. Deep buffer has so far been enabled only in one Soundwire based configuration (sof-ptl-rt712-l3-rt1320-l3-4ch.tplg). Change the PCM name as this is not yet referred in any upstream UCM definition yet. Suggest-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com> Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
8191f75 to
4dd67e9
Compare
|
V2:
|
There was a problem hiding this 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 extracts deep buffer default configuration values from the main topology files into a separate configuration file to improve modularity and reduce code duplication.
- Creates a new
deep-buffer-default.conffile containing default deep buffer configuration values - Removes duplicate default definitions from
deep-buffer.conf - Updates multiple topology configuration files to include the new defaults file
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tools/topology/topology2/sof-hda-generic.conf | Adds include for new deep-buffer-default.conf |
| tools/topology/topology2/platform/intel/deep-buffer.conf | Removes local default definitions and some pipeline ID constants |
| tools/topology/topology2/platform/intel/deep-buffer-default.conf | New file containing extracted deep buffer default configurations |
| tools/topology/topology2/cavs-sdw.conf | Adds include for deep-buffer-default.conf and defines missing pipeline constants |
| tools/topology/topology2/cavs-rt5682.conf | Adds include for deep-buffer-default.conf |
| tools/topology/topology2/cavs-nocodec.conf | Adds include for deep-buffer-default.conf |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| DEEP_BUFFER_PIPELINE_SRC_2 'mixin.16.1' | ||
| DEEP_BUFFER_PIPELINE_SINK_2 'mixout.21.1' | ||
| DEEP_BUFFER_PCM_NAME_2 'Deepbuffer Amps' | ||
| SPEAKER_PCM_CORE_ID 0 |
Copilot
AI
Sep 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removal of DEEP_BUFFER_PIPELINE_ID_2, DEEP_BUFFER_PCM_ID_2, DEEP_BUFFER_PIPELINE_SRC_2, DEEP_BUFFER_PIPELINE_SINK_2, and DEEP_BUFFER_PCM_NAME_2 definitions from this conditional block may cause undefined variable errors when DEEP_BUF_SPK is true, since these variables are still referenced in the pipeline configuration below but are no longer defined here.
| SPEAKER_PCM_CORE_ID 0 | |
| SPEAKER_PCM_CORE_ID 0 | |
| DEEP_BUFFER_PIPELINE_ID_2 2 | |
| DEEP_BUFFER_PCM_ID_2 2 | |
| DEEP_BUFFER_PIPELINE_SRC_2 host-copier.$DEEP_BUFFER_PCM_ID_2.playback | |
| DEEP_BUFFER_PIPELINE_SINK_2 gain.$DEEP_BUFFER_PIPELINE_ID_2.1 | |
| DEEP_BUFFER_PCM_NAME_2 "Deep Buffer Playback 2" |
lgirdwood
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
| DEEP_BUFFER_PIPELINE_SRC_2 'mixin.16.1' | ||
| DEEP_BUFFER_PIPELINE_SINK_2 'mixout.21.1' | ||
| DEEP_BUFFER_PCM_NAME_2 'Deepbuffer Amps' | ||
| DEEP_BUFFER_PCM_NAME_2 'Deepbuffer Speaker' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know we are doing more of a mechanical move in this PR, but I think it makes sense to look at our _ID_n and align them with a string name.
|
Tested on google karis (mtl), this fixes the issue. |
naveen-manohar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, this works at my end on RT721..
I have updated https://github.com/thesofproject/sof/pull/10221/commits
Related to #10221 and solving the issue found in #10165
Marking as draft, need to do more testing, but getting this out early as this impacts #10221 review process.