Skip to content

Conversation

@jsarha
Copy link
Contributor

@jsarha jsarha commented May 24, 2023

This tries to fix the first tap in #6576 for cavs-nocodec topology.

…lumes

Rename SSP0, SSP1, and SSP2 playback volumes according to PCM names
they are connected to. Define the PCM names as variables so the name
can be changed in a single place.

This is how the change will look like in sof-tgl-nocodec.tplg:

'gain.1.1 Playback Volume 1' > 'gain.1.1 Pre Mixer Port0 Playback Volume'
'gain.2.1 Main Playback Volume 2' > 'gain.2.1 Post Mixer Port0 Playback Volume'
'gain.3.1 Playback Volume 3' > 'gain.3.1 Pre Mixer Port1 Playback Volume'
'gain.5.1 Playback Volume 5' > 'gain.5.1 Pre Mixer Port2 Playback Volume'
'gain.6.1 Main Playback Volume 6' > 'gain.6.1 Post Mixer Port2 Playback Volume'

Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
@jsarha jsarha requested a review from ranj063 as a code owner May 24, 2023 19:26
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jsarha you're hitting the 44 char limit with this one right? Maybe it will be fixed when you removed the widget name from the name?

Copy link
Contributor Author

@jsarha jsarha May 25, 2023

Choose a reason for hiding this comment

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

Yes. I decided to ignore those cases that that that would be Ok without the widget name prefix. However, I am open for other suggestions for naming standard that would fit into smaller space, that would still be informative enough, and would fulfill this: https://docs.kernel.org/sound/designs/control-names.html

Copy link
Member

Choose a reason for hiding this comment

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

is there a typo between "mix" and "mux" ? I don't recall where we used a mux and what a 'post mux volume' might be

Copy link
Collaborator

@ranj063 ranj063 left a comment

Choose a reason for hiding this comment

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

looks good to me

@kv2019i kv2019i requested a review from plbossart May 25, 2023 10:25
@kv2019i
Copy link
Collaborator

kv2019i commented May 25, 2023

@jsarha Btw, please always ensure there are enough reviewers in the PR. Add people who've edited the files recently, ask collegues, and what not.

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.

Welcome improvement! And thanks for the good examples in git commti messages. These helped a lot when reviewing.

@jsarha
Copy link
Contributor Author

jsarha commented May 25, 2023

Welcome improvement! And thanks for the good examples in git commti messages. These helped a lot when reviewing.

Coming up with the examples will be harder in the sdw and rt5682 topologies (@plbossart , they are in scope of #6576, aren't they?) as I do not have access to the HW in question?

Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

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

I don't understand the use of the 'mux', there's no muxing capability in our topologies @jsarha

Copy link
Member

Choose a reason for hiding this comment

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

is there a typo between "mix" and "mux" ? I don't recall where we used a mux and what a 'post mux volume' might be

Copy link
Member

Choose a reason for hiding this comment

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

again where is there a MUX on DMIC capture?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand the use of the 'mux', there's no muxing capability in our topologies @jsarha

@plbossart In the capture streams - at least in this topology - there is a single source at the DAI side that is split to multiple PCMs. To my understanding that is multiplexing (= muxing).

Copy link
Member

Choose a reason for hiding this comment

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

that's demux...

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, will fix.

Jyri Sarha added 4 commits May 25, 2023 19:09
Rename SSP0 capture related volumes. Define a variable for PCM name
and refer to it in the mixer names.

Using sof-tgl-nocodec.tplg the changes are:

'gain.8.1 Host Capture Volume' > 'gain.8.1 Pre Demux Port0 Capture Volume'
'gain.7.1 Main Capture Volume 1' > 'gain.7.1 Post Demux Port0 Capture Volume'
'gain.17.1 Main Capture Volume 2' > 'gain.17.1 Post Demux ssp-capture Capture Vo'

Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Rename cavs-nocodec.conf DMIC0 volumes. Define variables for the
associated PCM names and refer to them in the mixer names. The changes
shown in sof-tgl-nocodec.tplg are:

'gain.18.1 Capture Raw Volume 1' > 'gain.18.1 Post Demux DMIC0 Raw Capture Volu'
'gain.19.1 Main Capture Volume 3' > 'gain.19.1 Pre Demux DMIC0 Raw Capture Volum'
'gain.20.1 Capture Raw Volume 2' > 'gain.20.1 Post Demux DMIC0 Raw 2 Capture Vo'

Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Rename SSP0 Aux Playback stream volume before mixing. Use the
associated PCM name in the mixer name. This change affects nocodec
topologies and appears on sof-tgl-nocodec.tplg as a following change:

'gain.21.1 Playback Volume 8' -> 'gain.21.1 Pre Mixer Port0 Aux Playback Volu'

Unfortunately the new mixer name does fit into the space reserved for
it, but it should get fixed once we get rid of the "gain 21.1"-prefix.

Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Rename generic DMIC mixer name. Define the associated PCM name in a
common place and refer to that name on the mixer name.

On cavs-tgl-nocodec.tplg is seen a following mixer name change:

'gain.13.1 DMIC0 Capture Volume 1' > 'gain.13.1 DMIC Capture Volume'

Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
@jsarha jsarha force-pushed the rename_cavs_nocodec_mixers branch from e33cf5a to b5b0f22 Compare May 25, 2023 16:18
Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

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

thanks @jsarha

@kv2019i
Copy link
Collaborator

kv2019i commented May 26, 2023

Known fails only, proceeding with merge.

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.

4 participants