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.

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

Probes ipc4 get config #5249

Open
wants to merge 5 commits into
base: topic/sof-dev
Choose a base branch
from
Open

Probes ipc4 get config #5249

wants to merge 5 commits into from

Conversation

jsarha
Copy link

@jsarha jsarha commented Nov 20, 2024 •
edited
Loading

  • Some trivial fixes
  • proper sof_probes_ipc_ops points_info() implementation for ipc4
  • more informative debufs read output for ipc4
  • all availabe probe points option to points_info() and a separate debugfs file for it

The functionality of the new features depend on thesofproject/sof#9669

@jsarha jsarha marked this pull request as ready for review November 20, 2024 20:43
@jsarha jsarha force-pushed the probes_ipc4_get_config branch 5 times, most recently from 39862fe to 32b8154 Compare November 21, 2024 15:46
lgirdwood
lgirdwood previously approved these changes Nov 21, 2024
View reviewed changes
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.

LGTM, nice addition to make this way easier to use.

@jsarha
Copy link
Author

jsarha commented Nov 21, 2024

The small updates are for fixing sparse warnings from exotic builds. The check fails at first failed file and commit, so there is many iterations. Missing documentation, wrong format string for 32-bit build, etc. etc.

@jsarha jsarha force-pushed the probes_ipc4_get_config branch 2 times, most recently from c5ea419 to c0b2ba6 Compare November 21, 2024 21:25
ujfalusi
ujfalusi requested changes Nov 22, 2024
View reviewed changes
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.

@jsarha, you are flexing again the boundaries of what sof-clients can do ;)
There are couple of places that we need to align with the definition of sof-client, I know it makes things a bit harder, but I want to keep the rules applicable.

sound/soc/sof/sof-client-probes-ipc4.c Outdated Show resolved Hide resolved
sound/soc/sof/sof-client-probes-ipc4.c Outdated Show resolved Hide resolved
sound/soc/sof/sof-client-probes-ipc4.c Outdated Show resolved Hide resolved
sound/soc/sof/sof-client-probes.c Show resolved Hide resolved
sound/soc/sof/sof-client-probes.c Outdated Show resolved Hide resolved
sound/soc/sof/sof-client-probes.c Outdated Show resolved Hide resolved
jsarha
jsarha commented Nov 22, 2024
View reviewed changes
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.

Thanks @ujfalusi for feedback! I'll drop the first commit, implement the find widgets wrapper, and add the new sof_probes_ipc_ops callback.

sound/soc/sof/sof-client-probes-ipc4.c Outdated Show resolved Hide resolved
sound/soc/sof/sof-client-probes.c Outdated Show resolved Hide resolved
@jsarha jsarha force-pushed the probes_ipc4_get_config branch 2 times, most recently from e0c8597 to ccc2da5 Compare November 25, 2024 08:16
lgirdwood
lgirdwood previously approved these changes Nov 28, 2024
View reviewed changes
ujfalusi
ujfalusi requested changes Nov 29, 2024
View reviewed changes
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.

@jsarha, this version looks much cleaner but there are few things that should be checked.

sound/soc/sof/sof-client-probes-ipc4.c Outdated Show resolved Hide resolved
sound/soc/sof/sof-client-probes-ipc4.c Outdated Show resolved Hide resolved
sound/soc/sof/sof-client-probes-ipc4.c Outdated Show resolved Hide resolved
sound/soc/sof/sof-client-probes-ipc4.c Outdated Show resolved Hide resolved
sound/soc/sof/sof-client-probes.h Outdated Show resolved Hide resolved
sound/soc/sof/sof-client-probes.c Show resolved Hide resolved
@jsarha jsarha force-pushed the probes_ipc4_get_config branch from ccc2da5 to 42620ca Compare November 29, 2024 12:50
jsarha
jsarha commented Nov 29, 2024
View reviewed changes
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.

Thanks again. Hope its now good. At least its converging.

sound/soc/sof/sof-client-probes-ipc4.c Outdated Show resolved Hide resolved
sound/soc/sof/sof-client-probes.h Outdated Show resolved Hide resolved
sound/soc/sof/sof-client-probes.c Show resolved Hide resolved
sound/soc/sof/sof-client-probes-ipc4.c Outdated Show resolved Hide resolved
sound/soc/sof/sof-client-probes-ipc3.c Show resolved Hide resolved
@ujfalusi
Copy link
Collaborator

ujfalusi commented Dec 2, 2024

@jsarha, can you check the W=1 build errors?

@jsarha jsarha force-pushed the probes_ipc4_get_config branch from 42620ca to f5bbc39 Compare December 9, 2024 09:42
@jsarha
Copy link
Author

jsarha commented Dec 9, 2024

@jsarha, can you check the W=1 build errors?

Oops, there were other short comings in the previous version too. I'll ping you once the compile CI tests are ready, if they are now OK.

@jsarha jsarha force-pushed the probes_ipc4_get_config branch from c3528ea to 4276e4f Compare December 30, 2024 17:44
@jsarha
Copy link
Author

jsarha commented Dec 30, 2024

Rebased on top of 6.13 based sof-dev.

@jsarha
Copy link
Author

jsarha commented Jan 7, 2025

@lgirdwood & @ujfalusi, a gentle ping!

@lgirdwood
Copy link
Member

@ujfalusi @ranj063 pls review.

ranj063
ranj063 approved these changes Jan 8, 2025
View reviewed changes
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.

just one nit-pick, looks good otherwise

ranj063
ranj063 previously approved these changes Jan 8, 2025
View reviewed changes
ujfalusi
ujfalusi reviewed Jan 14, 2025
View reviewed changes
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.

@jsarha, sorry for the delay, I have only one minor comment.

sound/soc/sof/sof-client-probes-ipc4.c Outdated Show resolved Hide resolved
lgirdwood
lgirdwood previously approved these changes Jan 14, 2025
View reviewed changes
@jsarha jsarha dismissed stale reviews from lgirdwood and ranj063 via cd94f96 January 15, 2025 11:04
@jsarha jsarha force-pushed the probes_ipc4_get_config branch from 4276e4f to cd94f96 Compare January 15, 2025 11:04
jsarha
jsarha commented Jan 15, 2025
View reviewed changes
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.

Fixes recommended in last review round applied.

sound/soc/sof/sof-client-probes.h Outdated Show resolved Hide resolved
sound/soc/sof/sof-client-probes-ipc4.c Outdated Show resolved Hide resolved
@jsarha jsarha force-pushed the probes_ipc4_get_config branch from cd94f96 to e6cd124 Compare January 15, 2025 17:18
@jsarha
Copy link
Author

jsarha commented Jan 15, 2025

@ujfalusi & @ranj063 , hope this is now good.

ranj063
ranj063 previously approved these changes Jan 15, 2025
View reviewed changes
@ranj063 ranj063 requested a review from ujfalusi January 15, 2025 19:37
lgirdwood
lgirdwood reviewed Jan 16, 2025
View reviewed changes
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.

Almost good to go for me, lets use HEX for our IDs though.

sound/soc/sof/sof-client-probes-ipc4.c Show resolved Hide resolved
@jsarha jsarha force-pushed the probes_ipc4_get_config branch from e6cd124 to 4c1ceb7 Compare January 17, 2025 10:01
@jsarha
Copy link
Author

jsarha commented Jan 17, 2025

@ujfalusi , @ranj063 and @lgirdwood , on more time please 😅.

@jsarha jsarha force-pushed the probes_ipc4_get_config branch from 4c1ceb7 to 845c02b Compare January 17, 2025 13:45
Jyri Sarha added 2 commits January 17, 2025 18:11
ASoC: SOF: sof-client-probes-ipc4: Human readable debugfs "probe_points"
6a12067 
The current output of three integers is not very human readable. Use
ipc4 functions to describe in more detail what the struct
sof_probe_point_desc buffer_id is actually referring to in an ipc4 SOF
system.

Before this commit the "probe_points" debugfs file could read as:

Id: 0x01000004  Purpose: 0  Node id: 0x100
Id: 0x00000006  Purpose: 0  Node id: 0x100

And after in the same situation in an ipc4 system it reads:

0x7,0x0,0x100   gain.1.1 input buf idx 0 (connected)
0x1000005,0x0,0x100     host-copier.0.playback output buf idx 0 (connected)

The triplet in the beginning of the line can be used to reinserted the
probe point again by writing it into "probe_points" debugfs file, if
its first removed by writing the fist number in "probe_points_remove".
The last number is ignored when creating a probe point.

Signed-off-by: Jyri Sarha <[email protected]>
ASoC: SOF: sof-client-probes: Add available points_info(), IPC4 only
47ce578 
Add another debugfs file, "probe_points_available", that shows all the
available probe points in the SOF FW at the time of query. The probe
points are there only when an active SOF stream exists in the
system. However, the stream identifiers are persistent in the sense
that the same probe point identifiers always appear with the same
playback or capture command in the same system configuration.

The output, when reading "probe_points_available", may look like this:

0x1000005,0x0,0x100     host-copier.0.playback output buf idx 0 (connected)
0x7,0x0,0x100   gain.1.1 input buf idx 0 (connected)
0x1000007,0x0,0x0       gain.1.1 output buf idx 0
0x3,0x0,0x0     mixin.1.1 input buf idx 0
0x1000003,0x0,0x0       mixin.1.1 output buf idx 0
0x4,0x0,0x0     mixout.2.1 input buf idx 0
0x1000004,0x0,0x0       mixout.2.1 output buf idx 0
0x10007,0x0,0x0 gain.2.1 input buf idx 0
0x1010007,0x0,0x0       gain.2.1 output buf idx 0
0x11,0x0,0x0    smart_amp.2.1 input buf idx 0
0x1000011,0x0,0x0       smart_amp.2.1 output buf idx 0
0x10005,0x0,0x0 dai-copier.SSP.NoCodec-0.playback input buf idx 0

The triplet at the beginning of a line can be copy-pasted as such to
"probe_points" debugfs file for adding a probe point. The rest of the
line tries to give human readable explanation of what this probe point
is.

Signed-off-by: Jyri Sarha <[email protected]>
@jsarha jsarha force-pushed the probes_ipc4_get_config branch from 845c02b to 47ce578 Compare January 17, 2025 16:12
ujfalusi
ujfalusi approved these changes Jan 20, 2025
View reviewed changes
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.

@jsarha, thank you, I run out of the nits to pick.

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

@ujfalusi ujfalusi ujfalusi approved these changes

@lgirdwood lgirdwood lgirdwood approved these changes

@ranj063 ranj063 ranj063 left review comments

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

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

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

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

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jsarha @ujfalusi @lgirdwood @ranj063