Skip to content

Conversation

@jsarha
Copy link

@jsarha jsarha commented Apr 7, 2025

Add SOF_TKN_COMP_SCHED_DOMAIN and connect it to struct snd_sof_widget comp_domain member, with new get_token_comp_domain() function.

The logic is such that if the topology attribute is not present in the widget node the corresponding IPC4 extension value is taken from the module's manifest like before. But if the attribute is found its value overrides what is there in the manifest.

I do not yet know how to test this feature end to end, so that is why this is only Draft now. However, with a corresponding topology change the attribute is correctly passed to domain extension bit in widget setup message

@jsarha jsarha force-pushed the topology2_sched_domain_support branch from e7f2745 to 6bbbbb5 Compare April 7, 2025 21:28
@jsarha jsarha force-pushed the topology2_sched_domain_support branch from 6bbbbb5 to 22e80a0 Compare April 9, 2025 17:25
ranj063
ranj063 previously approved these changes Apr 9, 2025
@jsarha jsarha marked this pull request as ready for review April 22, 2025 20:36
@jsarha
Copy link
Author

jsarha commented Apr 29, 2025

@ujfalusi could you approve this to get this merged?

@jsarha
Copy link
Author

jsarha commented May 7, 2025

@lyakh , @ujfalusi , I still think no warning should be needed if we somehow encounter unrecognized topology value. Something like that should not really happen as long as the current topology attribute definition is not tampered: https://github.com/thesofproject/sof/blob/d6786df73a3e6185d06b90c2c02a3c54c63e7907/tools/topology/topology2/include/components/widget-common.conf#L124

But would it get this PR moving if I device a mechanism to provide a warning in such a case by introducing one more enum value for invalid string to be stored in swidget->comp_domain and report a warning in sof_ipc4_widget_setup_msg() about that value? That solution is far from ideal, as the warning would come only if that widget is set up, and always when its set up, but it could not tell what the wrong string was.

Of course the ultimate solution would be changing the prototype for struct sof_topology_token -> get_token() and add sof_dev there, but that is a big change touching all over the place, and I really don't thing it is in the scope of this PR.

lyakh
lyakh previously approved these changes May 8, 2025
@jsarha jsarha dismissed stale reviews from lyakh and ranj063 via 6759ebb May 9, 2025 12:20
@jsarha jsarha force-pushed the topology2_sched_domain_support branch 2 times, most recently from 6759ebb to e52a93a Compare May 9, 2025 13:32
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

Just some simplifications.

@jsarha jsarha force-pushed the topology2_sched_domain_support branch from e52a93a to b077804 Compare May 12, 2025 08:59
Add SOF_TKN_COMP_SCHED_DOMAIN and connect it to struct snd_sof_widget
comp_domain member, with new get_token_comp_domain() function.

The logic is such that if the topology attribute is not present in the
widget node the corresponding IPC4 extension value is taken from the
module's manifest like before. But if the attribute is found and
recognized its value overrides what is there in the manifest.

Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
@jsarha jsarha force-pushed the topology2_sched_domain_support branch from b077804 to 33b3efb Compare May 12, 2025 09:19
Copy link
Author

@jsarha jsarha left a comment

Choose a reason for hiding this comment

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

@ujfalusi & @lgirdwood , New version pushded. Hope its finally good.

@ranj063
Copy link
Collaborator

ranj063 commented May 13, 2025

SOFCI TEST

@lgirdwood
Copy link
Member

@ranj063 good to merge ?

@ranj063 ranj063 merged commit dee6209 into thesofproject:topic/sof-dev May 17, 2025
9 of 16 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.

5 participants