ipc4: Unify component ID logging format across various IPC and pipeline logs #8970
Conversation
Unify the logging format for component IDs by converting them to hexadecimal in ipc-helper.c. This change ensures consistency with other log entries and improves the ease of identifying components when cross-referencing with other debugging outputs that use hex notation for component IDs. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
Standardize component ID logging by displaying them in hexadecimal format across all pipeline logs. This change aligns the component ID representation with other parts of the firmware logs where hexadecimal is used, facilitating quicker cross-referencing and searchability for specific components during debugging. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
Switch component ID logging to hexadecimal in ipc4/helper.c to align with the rest of the system's logging conventions. This enhances consistency and aids in debugging by matching the component ID format used in other diagnostic tools. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
Update the logging statements in pipeline-graph.c to display component IDs in hexadecimal format. This change ensures consistency with other logging within the system and improves the clarity of the logs when tracking component IDs during debugging sessions. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
tmleman
requested review from
bardliao,
marcinszkudlinski,
pblaszko,
lgirdwood,
plbossart,
mmaka1,
lbetlej,
dbaluta and
kv2019i
as code owners
| @@ -205,7 +205,7 @@ int ipc_pipeline_complete(struct ipc *ipc, uint32_t comp_id) | |||
| /* check whether pipeline exists */ | |||
| ipc_pipe = ipc_get_pipeline_by_id(ipc, comp_id); | |||
| if (!ipc_pipe) { | |||
| tr_err(&ipc_tr, "ipc: ipc_pipeline_complete looking for pipe component id %d failed", | |||
| tr_err(&ipc_tr, "ipc: ipc_pipeline_complete looking for pipe component id 0x%x failed", | |||
There was a problem hiding this comment.
@lyakh I think the problem has been that sof-logger/IPC3 doesn't handle this. We've had some usage (e.g. in pipeline-trace.h) but so far only in IPC4 only code.
That's also the reason why this cleanup has not been done yet in generic code, as you end up in some complicated compromises to make (as the many of these ids are bitmasks in IPC4 while there are unique integers in IPC3, so there's no single best represention of the numbers).
There was a problem hiding this comment.
I think the problem has been that sof-logger/IPC3 doesn't handle this.
Doesn't handle what? sof-logger supports any format string as long as the arguments are between 0 and 4 and all 32bits wides. It merely invokes fprintf() in print_entry_params()
There was a problem hiding this comment.
Emphasis "I think". If "%#x" works, by all means let's go with that, but someone needs to ack it works with sof-logger.
There was a problem hiding this comment.
%x is already in use in many places:
git grep -E 'tr_.*%.{0,3}x'
git grep -E 'comp_.*%.{0,3}x'
If only one of these statements is a frequent one, then verifying it works it's just the matter of looking at stable-v2.2 test results in any recent sof-test or linux Pull Request:
https://github.com/thesofproject/sof-test/pulls
https://github.com/thesofproject/linux/pulls
There was a problem hiding this comment.
If "%#x" works
BTW %#x is less obvious and readable than 0x%x for near-zero advantage?
https://pubs.opengroup.org/onlinepubs/9699919799/functions/printf.html
There was a problem hiding this comment.
Thanks for doing this @tmleman ! As noted in inline comments, this is somewhat long overdue for generic code (and even in some parts of IPC4 specific code) as the numbering schemes are different in IPC3/4.
But let's have a review round for this and especially IPC3 users please raise concerns if you have any. We can do conditional formatting like we have in pipeline-trace.h (__PIPE_FMT), but this is always a bit clunky, so better if we can just have simple formatting in code.
@fredoh9 @marc-hb @singalsu This may impact the performance analysis infra Intel SOF team has been doing (there's some analysis that parses the component and pipeline ids from logs). If you can cross-check and estimate impact?
| @@ -205,7 +205,7 @@ int ipc_pipeline_complete(struct ipc *ipc, uint32_t comp_id) | |||
| /* check whether pipeline exists */ | |||
| ipc_pipe = ipc_get_pipeline_by_id(ipc, comp_id); | |||
| if (!ipc_pipe) { | |||
| tr_err(&ipc_tr, "ipc: ipc_pipeline_complete looking for pipe component id %d failed", | |||
| tr_err(&ipc_tr, "ipc: ipc_pipeline_complete looking for pipe component id 0x%x failed", | |||
There was a problem hiding this comment.
@lyakh I think the problem has been that sof-logger/IPC3 doesn't handle this. We've had some usage (e.g. in pipeline-trace.h) but so far only in IPC4 only code.
That's also the reason why this cleanup has not been done yet in generic code, as you end up in some complicated compromises to make (as the many of these ids are bitmasks in IPC4 while there are unique integers in IPC3, so there's no single best represention of the numbers).
This pull request includes a series of commits that standardize the logging format for component IDs across different parts of the audio firmware. The changes convert component ID logs from decimal to hexadecimal format, aligning with the common logging practices within the firmware and other debugging tools. This consistency simplifies the process of cross-referencing component IDs during development and debugging.
The commits cover updates to ipc-helper logs, ipc4 helper logs, and pipeline graph logs, ensuring that component IDs are uniformly displayed in hexadecimal format. These changes not only improve readability but also enhance the efficiency of searching and correlating logs when investigating issues.
List of changes:
Each commit is focused on a specific area of the codebase, making the changes easy to review and verify.