Skip to content

[RFC] topology: move volume/switch control naming to upper level - #2099

Closed
juimonen wants to merge 1 commit into
thesofproject:masterfrom
juimonen:dmic_name_fix
Closed

[RFC] topology: move volume/switch control naming to upper level#2099
juimonen wants to merge 1 commit into
thesofproject:masterfrom
juimonen:dmic_name_fix

Conversation

@juimonen

@juimonenjuimonen commented Nov 15, 2019

Copy link
Copy Markdown

Dmic volume and switch naming was hardcoded into capture eq pipelines,
which introduced problems when creating several instances of those
pipelines. So move the naming defines up 1 level to the topologies
using those pipelines.

Signed-off-by: Jaska Uimonen jaska.uimonen@intel.com

@juimonenjuimonen changed the title topology: move volume/switch control naming to upper level[RFC] topology: move volume/switch control naming to upper levelNov 15, 2019
lgirdwood
lgirdwood requested changes Nov 15, 2019
View reviewed changes
Comment threadtools/topology/sof/pipe-eq-capture.m4 Outdated
ranj063
ranj063 reviewed Nov 15, 2019
View reviewed changes
Comment threadtools/topology/sof/pipe-eq-capture.m4 Outdated
ranj063
ranj063 reviewed Nov 15, 2019
View reviewed changes
Comment threadtools/topology/sof/pipe-eq-capture.m4 Outdated
@zrombel

zrombel commented Nov 28, 2019

Copy link
Copy Markdown

Due to PR#2029 merge and CI bit depth conversion tests changes, this PR require rebase.

@lgirdwood

lgirdwood commented Jan 6, 2020

Copy link
Copy Markdown
Member

@ranj063@juimonen any agreement on this from you guys ?

@juimonen

juimonen commented Jan 7, 2020

Copy link
Copy Markdown
Author

@ranj063 if you have time, can you still clarify what kind of changes you meant? I was not fully getting it...

@lgirdwood

lgirdwood commented Jan 20, 2020

Copy link
Copy Markdown
Member

@juimonen@ranj063 ping, any updates ?

@juimonen

juimonen commented Jan 20, 2020

Copy link
Copy Markdown
Author

@lgirdwood I'll check this tomorrow... @ranj063 if you have time to elaborate what was the idea in your proposal with this as I didn't quite get it...

@juimonen

juimonen commented Jan 23, 2020

Copy link
Copy Markdown
Author

@ranj063 I modified this now quite heavily, so trying to make it so that you can set explicit pga/control names from pipeline files if you wish to do so.... You think this is going to the right direction? It should be more clear now and I got rid of the messy defines etc.

lgirdwood
lgirdwood reviewed Jan 23, 2020
View reviewed changes

@lgirdwoodlgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@juimonen as a quick sanity check, can you compare the ASCII conf files after M4 processing with and without this patch. This should highlight us to any issues quickly.

@juimonen

juimonen commented Jan 23, 2020

Copy link
Copy Markdown
Author

@lgirdwood yes will do. I need to rebase this also, was doing against old master...

@ranj063

ranj063 commented Jan 23, 2020

Copy link
Copy Markdown
Collaborator

@juimonen sorry for the delay. I have lost track of what I said earlier in this PR. Let me look through your changes today

@juimonen

juimonen commented Jan 23, 2020

Copy link
Copy Markdown
Author

@ranj063 hold on for a while still :) I need to resolve some issues still...

@juimonen

juimonen commented Jan 24, 2020

Copy link
Copy Markdown
Author

@ranjani ok, now should be good for review...
@lgirdwood added diff between master topologies and this PR, there's only some empty line changes and the dmic naming related to this PR.
diffi.txt

lgirdwood
lgirdwood approved these changes Jan 24, 2020
View reviewed changes

@lgirdwoodlgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@plbossart@ranj063 any comments, if not pls merge.

@juimonenjuimonen mentioned this pull request Feb 11, 2020
Merged
@juimonen

juimonen commented Feb 25, 2020

Copy link
Copy Markdown
Author

SOFCI TEST

@lgirdwood

lgirdwood commented Feb 26, 2020

Copy link
Copy Markdown
Member

@juimonen CI failing due to conflicts

Jaska Uimonen
topology: enable explicit name setting in pga and mixercontrol
4e5f36e
Currently pga and related mixercontrol have widget naming schemes inside
the macro. This is done to help the person writing topologies as pga's
are automatically named by concatenating pipeline ids with hard coded
names. As pga and mixercontrol define the control name visible in
alsamixer we want to allow explicit naming outside the macro call. For
example dmic control needs to be "Dmic0 Capture Volume" as this is
defined in UCM and used by Pulseaudio. So let the name argument in pga
and mixercontrol macros to be the widget name and don't modify it inside
the macro. There are still N_PGA and N_CONTROLMIXER macros which can be
used to generate the old style control names.
Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>
@lgirdwood

lgirdwood commented Feb 27, 2020

Copy link
Copy Markdown
Member

@juimonen still conflicting ?

@juimonen

juimonen commented Feb 27, 2020

Copy link
Copy Markdown
Author

@lgirdwood yes should read the github conflict messages... now updated

@lgirdwood

lgirdwood commented Feb 27, 2020

Copy link
Copy Markdown
Member

SOFCI TEST

@lgirdwood
lgirdwood deleted the branch thesofproject:masterMarch 28, 2021 13:44
@paulstelian97

paulstelian97 commented Mar 29, 2021

Copy link
Copy Markdown
Collaborator

Please resubmit with "main" as PR base branch.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ranj063ranj063ranj063 left review comments
@lgirdwoodlgirdwoodlgirdwood approved these changes
@lbetlejlbetlejAwaiting requested review from lbetlej
@mmaka1mmaka1Awaiting requested review from mmaka1
@plbossartplbossartAwaiting requested review from plbossart
@xiulipanxiulipanAwaiting requested review from xiulipan
@dbalutadbalutaAwaiting requested review from dbaluta
@jajanuszjajanuszAwaiting requested review from jajanusz
@tlaudatlaudaAwaiting requested review from tlauda

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@juimonen@zrombel@lgirdwood@ranj063@paulstelian97