Skip to content

ASoC: intel: sof_sdw: init all aggregated codecs #1962

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

Jump to bottom
Merged
kv2019i merged 1 commit into from
Apr 3, 2020
Merged

ASoC: intel: sof_sdw: init all aggregated codecs #1962

kv2019i merged 1 commit into from
Apr 3, 2020

Conversation

@bardliao
Copy link
Collaborator

@bardliao bardliao commented Mar 31, 2020 •
edited
Loading

Init codecs which belong to the same group id on all links.

Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com

Fixes #1953

RanderWang
RanderWang previously approved these changes Mar 31, 2020
View reviewed changes
Copy link

@RanderWang RanderWang left a comment

Choose a reason for hiding this comment

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

Thanks!

plbossart
plbossart previously approved these changes Mar 31, 2020
View reviewed changes
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.

The code looks fine @bardliao but I don't know how things worked before? Is this required now that we have the NO_AGGREGATION removed?

@bardliao
Copy link
Collaborator Author

bardliao commented Mar 31, 2020

The code looks fine @bardliao but I don't know how things worked before? Is this required now that we have the NO_AGGREGATION removed?

Yes, NO_AGGREGATION flag will hide the issue.

@RanderWang
Copy link

RanderWang commented Apr 1, 2020 •
edited
Loading

The code looks fine @bardliao but I don't know how things worked before? Is this required now that we have the NO_AGGREGATION removed?

yes, without aggregation we only need to init codec on current link for current BE DAI.

ranj063
ranj063 reviewed Apr 1, 2020
View reviewed changes
@bardliao bardliao dismissed stale reviews from plbossart and RanderWang via dff3515 April 1, 2020 05:33
ranj063
ranj063 previously approved these changes Apr 1, 2020
View reviewed changes
lyakh
lyakh reviewed Apr 1, 2020
View reviewed changes
sound/soc/intel/boards/sof_sdw.c

part_id = SDW_PART_ID(link->adr_d[i].adr);
codec_index = find_codec_info_part(part_id);
do {
Copy link
Collaborator

@lyakh lyakh Apr 1, 2020

Choose a reason for hiding this comment

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

Took me a while to figure out why we need to loop here. It looks like now with aggregation the loop over links in sof_card_dai_links_create() only calls create_sdw_dailink() for the first link in each group. That's why you now have to walk over all links, looking for others from the same group. Maybe you could add a comment about that for the next reader :-)

lyakh
lyakh previously approved these changes Apr 1, 2020
View reviewed changes
kv2019i
kv2019i requested changes Apr 1, 2020
View reviewed changes
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.

@bardliao Functionality looks good, but I think some commentary is needed to keep the code maintainable.

@plbossart
Copy link
Member

plbossart commented Apr 1, 2020

@bardliao please add comments to explain that the loop is done once and group_id > 0 means aggregation. You and I know this, but others may not.

@bardliao bardliao dismissed stale reviews from lyakh and ranj063 via 0a5dd8c April 2, 2020 01:10
@bardliao bardliao requested review from kv2019i and lyakh April 2, 2020 01:11
kv2019i
kv2019i requested changes Apr 2, 2020
View reviewed changes
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.

Still some iteration on code comments needed.

@bardliao
ASoC: intel: sof_sdw: init all aggregated codecs
9f6eeb4 
Init codecs which belong to the same group id on all links.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
@kv2019i
Copy link
Collaborator

kv2019i commented Apr 3, 2020

Jenkins build stalled, CI errors are known or unrelated (GLK sof-logger error), merging.

@kv2019i kv2019i merged commit 052e45d into thesofproject:topic/sof-dev Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@plbossart plbossart plbossart approved these changes

@lyakh lyakh lyakh approved these changes

@ranj063 ranj063 ranj063 approved these changes

@kv2019i kv2019i kv2019i approved these changes

@dbaluta dbaluta Awaiting requested review from dbaluta dbaluta is a code owner

@lgirdwood lgirdwood Awaiting requested review from lgirdwood lgirdwood is a code owner

@RanderWang RanderWang Awaiting requested review from RanderWang

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG][CML]Aplay0,2 rt1308 right speaker have sound output when muted speaker in alsamixer.

6 participants

@bardliao @RanderWang @plbossart @kv2019i @lyakh @ranj063