Skip to content

Conversation

@naveen-manohar
Copy link
Contributor

topology2: ace3: Add Deep-buffer & enhanced audio features to ptl-rt721-4ch target

  • Enhanced DMIC0 capture with TDFB & DRC processing
  • Add Bluetooth PCM support (ID 20, device ID 10)
  • Enable deep buffer speaker processing with 10ms DMA buffer
  • Configure TDFB line4_pass and DRC dmic_default parameters

Verified on PTL & WCL
cc: uday.m.bhat@intel.com

Copilot AI review requested due to automatic review settings September 8, 2025 09:46
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 adds deep-buffer and enhanced audio features to the ptl-rt721-4ch target configuration for the ACE3 platform. The changes enable advanced DMIC capture processing, Bluetooth PCM support, and deep buffer speaker processing with optimized DMA buffer settings.

  • Enhanced DMIC0 capture with TDFB (Time Domain Fixed Beamformer) and DRC (Dynamic Range Control) processing
  • Added Bluetooth PCM support with dedicated IDs and naming
  • Enabled deep buffer speaker processing with 10ms DMA buffer configuration

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tools/topology/topology2/production/tplg-targets-ace3.cmake Adds configuration parameters for enhanced DMIC capture, Bluetooth PCM, and deep buffer settings
tools/topology/topology2/cavs-sdw.conf Defines additional deep buffer pipeline constants for secondary audio path

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@naveen-manohar naveen-manohar requested review from kv2019i and removed request for jsarha and ranj063 September 8, 2025 09:47
Add support to extend ptl-rt721-4ch conf with audio processing:
- Enhanced DMIC0 capture with TDFB & DRC processing
- Enable deep buffer speaker processing with 10ms DMA buffer
- Configure TDFB line4_pass and DRC dmic_default parameters

Signed-off-by: Uday M Bhat <uday.m.bhat@intel.com>
Signed-off-by: Naveen Manohar <naveen.m@intel.com>
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Ok with change. PLease update the commit message on the second patch.

Copy link
Contributor

@ujfalusi ujfalusi left a comment

Choose a reason for hiding this comment

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

@naveen-manohar, I'm not sure how this play with the existing DB definition for SDW speaker, something is not quite right if we need to make hoops.

Also to note: how this will work with the function topologies? FYI: @bardliao

DEEP_BUFFER_PCM_ID_2 35
DEEP_BUFFER_PIPELINE_SRC_2 'mixin.16.1'
DEEP_BUFFER_PIPELINE_SINK_2 'mixout.21.1'
DEEP_BUFFER_PCM_NAME_2 'Deepbuffer Amps'
Copy link
Contributor

Choose a reason for hiding this comment

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

The PCM name shall be "Deepbuffer Speaker"

Copy link
Contributor

Choose a reason for hiding this comment

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

and we have now double definition of this DB for Speaker and yes, I realize that the existing naming also uses "Deepbuffer Amps", which is incorrect, it should be really Speaker. We have Amp(lifier) on headset as well..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for Headset its "Deepbuffer Jack Out"
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree the "Deepbuffer Amps" name is bad, but it's already in use, so not sure if we can change without breaking existing UCM files.

@ujfalusi Where is the double definition?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it is, because the Headset playback is named "Jack Out" :
cat /proc/asound/pcm
00-00: Jack Out ( * ) : : playback 1
00-01: Jack In ( * ) : : capture 1
00-02: Speaker ( * ) : : playback 1
00-04: Microphone ( * ) : : capture 1
00-05: HDMI1 ( * ) : : playback 1
00-06: HDMI2 ( * ) : : playback 1
00-07: HDMI3 ( * ) : : playback 1
00-31: Deepbuffer Jack Out ( * ) : : playback 1

Similarly, for "Speaker" we should have "Deepbuffer Speaker" for consistency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok let me check the "Deepbuffer Speaker" change

Further as discussed, there is another issue noticed with just this patch naveen-manohar@cde8889 Deep-buffer should have worked, but it dint add       DEEP_BUFFER_PIPELINE_ID_2  16       DEEP_BUFFER_PCM_ID_2    35 from https://github.com/thesofproject/sof/commit/5359a7f6743e0ea3d878f466c3b063169bcebc7a#diff-fc6601cfd…

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ujfalusi @naveen-manohar how about something like #10226 ?

PREPROCESS_PLUGINS=nhlt,NHLT_BIN=nhlt-sof-ptl-rt721-4ch.bin"
PREPROCESS_PLUGINS=nhlt,NHLT_BIN=nhlt-sof-ptl-rt721-4ch.bin,DMIC0_ENHANCED_CAPTURE=true,\
EFX_DMIC0_TDFB_PARAMS=line4_pass,EFX_DMIC0_DRC_PARAMS=dmic_default,\
DEEPBUFFER_FW_DMA_MS=10,DEEP_BUF_SPK=true"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure how this supposed to work, but the DEEP_BUF_SPK=true will enable the existing definition from tools/topology/topology2/platform/intel/deep-buffer.conf

# Spawn another instance
IncludeByKey.DEEP_BUF_SPK {
"true"	{
		Define {
			DEEP_BUFFER_PIPELINE_ID_2	16
			DEEP_BUFFER_PCM_ID_2		35
			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
		}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Now, I am wondering is it necessary to add those defines to cavs-sdw.conf?

Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Commit message is good now. For @ujfalusi and @lgirdwood , the naming conventions for speaker deep buffer have been use already in I2S config (that share definitions via intel/deep-buffer.conf). I guess the UCM files will be new for SDW machines, so we have a chance to change some (e.g. maybe the PCM name), but needs modifications to UCMs.

Otherwise, I'd like @bardliao to double-check how this plays together with functional topologies. I'm assuming it works as this is just another DSP-driven feature added and does not affect the definitions towards the DAIs. But please check.

DEEP_BUFFER_PCM_ID_2 35
DEEP_BUFFER_PIPELINE_SRC_2 'mixin.16.1'
DEEP_BUFFER_PIPELINE_SINK_2 'mixout.21.1'
DEEP_BUFFER_PCM_NAME_2 'Deepbuffer Amps'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree the "Deepbuffer Amps" name is bad, but it's already in use, so not sure if we can change without breaking existing UCM files.

@ujfalusi Where is the double definition?

@bardliao
Copy link
Collaborator

Commit message is good now. For @ujfalusi and @lgirdwood , the naming conventions for speaker deep buffer have been use already in I2S config (that share definitions via intel/deep-buffer.conf). I guess the UCM files will be new for SDW machines, so we have a chance to change some (e.g. maybe the PCM name), but needs modifications to UCMs.

Otherwise, I'd like @bardliao to double-check how this plays together with functional topologies. I'm assuming it works as this is just another DSP-driven feature added and does not affect the definitions towards the DAIs. But please check.

The change is specific to sof-ptl-rt721-4ch.tplg. I don't think it will impact the function topologies. On the other hands, the change will not take effect if function topology is used. We need to remove .get_function_tplg_files = sof_sdw_get_tplg_files, in soc-acpi-intel-ptl-match.c to avoid using function topology in this config.

@ujfalusi
Copy link
Contributor

Commit message is good now. For @ujfalusi and @lgirdwood , the naming conventions for speaker deep buffer have been use already in I2S config (that share definitions via intel/deep-buffer.conf). I guess the UCM files will be new for SDW machines, so we have a chance to change some (e.g. maybe the PCM name), but needs modifications to UCMs.
Otherwise, I'd like @bardliao to double-check how this plays together with functional topologies. I'm assuming it works as this is just another DSP-driven feature added and does not affect the definitions towards the DAIs. But please check.

The change is specific to sof-ptl-rt721-4ch.tplg. I don't think it will impact the function topologies. On the other hands, the change will not take effect if function topology is used. We need to remove .get_function_tplg_files = sof_sdw_get_tplg_files, in soc-acpi-intel-ptl-match.c to avoid using function topology in this config.

So, only machines which uses rt721 and 4ch PCH-DMIC will have Deepbuffer on Speaker and all other sound cards will not? Is it granted that this configuration will only be available w/ Chrome and we will not have different PCM list on sof-soundwire cards?

What I'm saying is that we should enable the "Deepbuffer Speaker" for all and every soundwire cards, not only trying to target a specific one and shot more in the process.

@lgirdwood lgirdwood merged commit 42c0875 into thesofproject:main Sep 10, 2025
35 of 45 checks passed
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.

5 participants