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.

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

ASoc: SOF: topology: connect DAI to a single DAI link #5309

Merged

Conversation

kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Jan 24, 2025

Due to partial matching, DAI stream name may match to multiple DAI links. Break out when a match is found as only a single link can be connected to a SOF DAI object.

Fixes: fe88788 ("ASoC: SOF: topology: Use partial match for connecting DAI link and DAI widget")
Link: #5308

@kv2019i kv2019i requested a review from ujfalusi January 24, 2025 13:22
@kv2019i
Copy link
Collaborator Author

kv2019i commented Jan 24, 2025

@ranj063 @bardliao Not 100% about the fix, but given "dai->name = rtd->dai_link->name;" is a single field, it seem unintentional the loop over all DAI links and set the dai name to last matching DAI link.

One alternative would be to break out only if the stream name is an exact match (not partial), but in the end, I didn't see the benefit of continuing to search for matches.

@ranj063
Copy link
Collaborator

ranj063 commented Jan 24, 2025

@ranj063 @bardliao Not 100% about the fix, but given "dai->name = rtd->dai_link->name;" is a single field, it seem unintentional the loop over all DAI links and set the dai name to last matching DAI link.

One alternative would be to break out only if the stream name is an exact match (not partial), but in the end, I didn't see the benefit of continuing to search for matches.

This relies on the fact that the DAI links are created in the ascending order of the port numbers. But I guess it's reasonable to assume that. I think this issue is specific to nocodec anyway.

ranj063
ranj063 previously approved these changes Jan 24, 2025
Copy link
Collaborator

@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.

We need a CI run on this

sound/soc/sof/topology.c Outdated Show resolved Hide resolved
ujfalusi
ujfalusi previously approved these changes Jan 27, 2025
sound/soc/sof/topology.c Outdated Show resolved Hide resolved
@kv2019i kv2019i dismissed stale reviews from ujfalusi and ranj063 via 09c2646 February 3, 2025 16:17
@kv2019i kv2019i force-pushed the 202501-fix-ssp1-nocodec branch from 4992360 to 09c2646 Compare February 3, 2025 16:17
@kv2019i kv2019i requested review from ujfalusi and ranj063 February 3, 2025 16:19
full = rtd;
break;
}
else if (strstr(rtd->dai_link->stream_name, w->sname)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this looks much better @kv2019i but just a minor nit, this else can go in the previous line.

ranj063
ranj063 previously approved these changes Feb 3, 2025
The partial matching of DAI widget to link names, can cause problems if
one of the widget names is a substring of another. E.g. with names
"Foo1" and Foo10", it's not possible to correctly link up "Foo1".

Modify the logic so that if multiple DAI links match the widget stream
name, prioritize a full match if one is found.

Fixes: fe88788 ("ASoC: SOF: topology: Use partial match for connecting DAI link and DAI widget")
Link: thesofproject#5308
Signed-off-by: Kai Vehmanen <[email protected]>
@ranj063 ranj063 merged commit ca38f1f into thesofproject:topic/sof-dev Feb 4, 2025
10 of 14 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.

3 participants