Skip to content
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.

Sign up for GitHub

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

topology2: cavs-sdw: Add Deep buffer PCM for sdw-amps #8879

Closed
wants to merge 1 commit into from
Closed

topology2: cavs-sdw: Add Deep buffer PCM for sdw-amps #8879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions tools/topology/topology2/cavs-sdw.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ Define {
DEEP_BUFFER_PIPELINE_SRC 'mixin.15.1'
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
Copy link
Member

Choose a reason for hiding this comment

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

@naveen-manohar can you remind me where this 35 comes from? I don't have access to my usual table of devices and this one does not ring a bell at all...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Considered the next available PCM-ID similar to https://github.com/thesofproject/sof/blob/main/tools/topology/topology2/cavs-rt5682.conf#L80

arkham ~ # aplay -l
**** List of PLAYBACK Hardware Devices ****
card 0: sofsoundwire [sof-soundwire], device 0: Jack Out () []
Subdevices: 1/1
Subdevice #0: subdevice #0
card 0: sofsoundwire [sof-soundwire], device 2: Speaker () []
Subdevices: 1/1
Subdevice #0: subdevice #0
card 0: sofsoundwire [sof-soundwire], device 31: Deepbuffer Jack Out () []
Subdevices: 1/1
Subdevice #0: subdevice #0
card 0: sofsoundwire [sof-soundwire], device 35: Deepbuffer Amps () []
Subdevices: 1/1
Subdevice #0: subdevice #0
arkham ~ # aplay -Dhw:0,35 -c 2 -r 48000 -d 10 -f S16_LE /root/pubg.wav

Copy link
Member

Choose a reason for hiding this comment

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

give me a week to look into this, I don't recall us ever agreeing on all this.

Copy link
Member

Choose a reason for hiding this comment

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

After checking my internal cheat sheet, I can confirm that the device 35 shows as reserved.

PCM28  RESERVED
PCM29  RESERVED
PCM30  playback	LowLatency
PCM31  playback	DeepBuffer (for low-power)
PCM32  playback	Media (e.g. movies)
PCM33  playback	Voice (Communication with a human)
PCM34  playback	Ultrasonics
PCM35  RESERVED
PCM36  RESERVED
PCM37  RESERVED
PCM38  RESERVED
PCM39  RESERVED

I don't mind reserving a Deep Buffer for amplifier endpoints, but note that we cannot keep adding in the PCM numbering both the content type and the endpoint. Are we e.g. going to have a LowLatency path for headphone AND speaker?

The "right" approach would be to keep the content-based stream generated by apps, and dynamically route to the current endpoint. What are the odds that we need a Deep buffer stream on the headphone and a Deep buffer stream on the amplifiers running concurrently?

That's what we had on the Merrifield and SKL drivers...The FE and BE were conditionally connected depending on a control switch.

@ranj063 @cujomalainey @ujfalusi @kv2019i FYI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

++ @yongzhi1 fyi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@plbossart jfyi, Chrome PnP team was asking deepbuf for Amp only, during their tests, they don't connect headset jack any more.
@sathya-nujella @cujomalainey can you help with below Query,

What are the odds that we need a Deep buffer stream on the headphone and a Deep buffer stream on the amplifiers running concurrently?

Copy link
Member

Choose a reason for hiding this comment

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

then remove the deep buffer for headsets and move it to amps...

I am still not clear on the benefits to be honest. Amps will by construction require 3+ digit milliwatts. In all my previous power optimizations, we optimized for the headset...

We can't continue adding buffers just in case they might be useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

completely agree with you, thank you @plbossart
@sathya-nujella we can close based on your comments.

Copy link
Contributor

Choose a reason for hiding this comment

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

completely agree with you, thank you @plbossart @sathya-nujella we can close based on your comments.

If we have to choose one, then adding to speaker amp is needed as default BKM used by PnP team is that endpoint. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

the other question is: why add this for LNL only? if this is really useful why not add this capability in all Chrome topologies?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As the current topology for sdw, had Deepbuffer option only for "device 31: Deepbuffer Jack Out"
thought of adding to Spk-amps too..
Dint intend to add for all Chrome devices, as only from MTL, we started to validate Multiple Stream.

Copy link
Member

Choose a reason for hiding this comment

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

Is this a real need though? I mean, every deep-buffer stream will eat-up memory that will be reserved, so every time we add such a stream it means less memory for 3rd party libraries. Multiple streams for all endpoints has a cost in the end...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the explanation about memory impact with this new stream.
In this scenario, we should remove existing "device 31: Deepbuffer Jack Out", until requested by customer ?

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
Member

Choose a reason for hiding this comment

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

@ranj063 @naveen-manohar should this be "Speaker" instead of "Amps" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lgirdwood @ranj063: I noticed a uniformity throughout for 'Deepbuffer Amps'

development/mtl-sdw-hdmi.conf:83: DEEP_BUFFER_PCM_NAME_2 'Deepbuffer Amps'
development/cavs-sdw-hdmi.conf:83: DEEP_BUFFER_PCM_NAME_2 'Deepbuffer Amps'
development/cavs-sdw.conf:83: DEEP_BUFFER_PCM_NAME_2 'Deepbuffer Amps'

Please confirm if I need to modify to 'Deepbuffer Speaker' ?

Copy link
Member

Choose a reason for hiding this comment

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

Good point - this may change UCM too, but the users today are all in "development" folder so are not used in production. @naveen-manohar I think we should change all to "Speaker" for clarity. @plbossart @ranj063 @singalsu anyone disagree ?

SDW_JACK_OUT_STREAM 'SDW0-Playback'
SDW_JACK_IN_STREAM 'SDW0-Capture'
SDW_JACK_OUT_BE_ID 0
Expand Down
2 changes: 1 addition & 1 deletion tools/topology/topology2/production/tplg-targets-ace2.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ SDW_JACK_OUT_STREAM=Playback-SimpleJack,SDW_JACK_IN_STREAM=Capture-SimpleJack"

"cavs-sdw\;sof-lnl-rt722-l0\;PLATFORM=lnl,SDW_DMIC=1,NUM_HDMIS=0,NUM_SDW_AMP_LINKS=1,\
SDW_AMP_FEEDBACK=false,SDW_SPK_STREAM=Playback-SmartAmp,SDW_DMIC_STREAM=Capture-SmartMic,\
SDW_JACK_OUT_STREAM=Playback-SimpleJack,SDW_JACK_IN_STREAM=Capture-SimpleJack"
SDW_JACK_OUT_STREAM=Playback-SimpleJack,SDW_JACK_IN_STREAM=Capture-SimpleJack,DEEP_BUF_SPK=true"

)
Loading