Skip to content

trace: Runtime filter message handler - #3329

Merged
lgirdwood merged 2 commits into
thesofproject:masterfrom
ktrzcinx:logger-trace-handler
Sep 18, 2020
Merged

trace: Runtime filter message handler#3329
lgirdwood merged 2 commits into
thesofproject:masterfrom
ktrzcinx:logger-trace-handler

Conversation

@ktrzcinx

@ktrzcinxktrzcinx commented Aug 20, 2020•
edited
Loading

Copy link
Copy Markdown
Member

It's a FW part extracted from #2965. This part have ABI changes.

Kernel part: thesofproject/linux#2377

@ktrzcinxktrzcinx added the ABI ABI change involved label Aug 20, 2020
lgirdwood
lgirdwood requested changes Aug 20, 2020
View reviewed changes

@lgirdwoodlgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Almost good to go.

Comment threadsrc/ipc/handler.c Outdated
Comment threadsrc/trace/trace.c Outdated
@lgirdwoodlgirdwood added this to the ABI-3.17 milestone Aug 20, 2020
@lgirdwood

lgirdwood commented Aug 20, 2020

Copy link
Copy Markdown
Member

@ktrzcinx can you link kernel PR here too.

@ktrzcinx
ktrzcinxforce-pushed the logger-trace-handler branch from 80ae6de to 56c28bbCompareAugust 21, 2020 07:47
@lgirdwood

lgirdwood commented Aug 21, 2020

Copy link
Copy Markdown
Member

@ktrzcinx do we have a pending kernel PR or logger PR ?

@ktrzcinx

ktrzcinx commented Aug 21, 2020•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

@ktrzcinx do we have a pending kernel PR or logger PR ?

I'm currently updating changes to kernel code before making kernel PR, logger PR is #2965 (I need there to apply your suggestions, after sending kernel PR)

@ktrzcinxktrzcinx mentioned this pull request Aug 21, 2020
Merged
@ktrzcinx
ktrzcinxforce-pushed the logger-trace-handler branch from 56c28bb to cd9f110CompareAugust 27, 2020 07:30
lgirdwood
lgirdwood approved these changes Aug 27, 2020
View reviewed changes

@lgirdwoodlgirdwood left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Kernel now merged, just waiting on CI.... EDIT. logger merged not kernel.

@lgirdwood

lgirdwood commented Sep 1, 2020

Copy link
Copy Markdown
Member

@ktrzcinx can you ping me when kernel part is good and we can merge this one.

@ktrzcinx

ktrzcinx commented Sep 1, 2020

Copy link
Copy Markdown
MemberAuthor

@lgirdwood, of course, I will do that

kv2019i
kv2019i requested changes Sep 14, 2020
View reviewed changes

@kv2019ikv2019i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

With IPC ABI hat on, some minor concerns raised in kernel side review, let's keep open until those are solved: -> thesofproject/linux#2377 (comment)

@lgirdwoodlgirdwood modified the milestones: ABI-3.17, ABI-3.18Sep 15, 2020
@plbossart

plbossart commented Sep 16, 2020

Copy link
Copy Markdown
Member

@ktrzcinx can you update the firmware part to reflect the changes made on the kernel side. I think it's quite good now, and it'd be a nice addition to firmware 1.6 and kernel 5.10.

@ktrzcinx

ktrzcinx commented Sep 17, 2020

Copy link
Copy Markdown
MemberAuthor

@plbossart Aligned with kernel code.

kv2019i
kv2019i approved these changes Sep 17, 2020
View reviewed changes

@kv2019ikv2019i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ABI is now ok for kernel side as well. We are continuing with some codestyle issues for the kernel PR, but ABI part is good.

@lgirdwood As this missed the 1.6 release branch, should this bump ABI to 3.18? If yes, @ktrzcinx could you update the PR with annotations following the recently merged thesofproject/sof-docs#283 .. ?

lgirdwood
lgirdwood reviewed Sep 17, 2020
View reviewed changes
Comment threadsrc/include/user/abi_dbg.h Outdated

@lgirdwoodlgirdwoodSep 17, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ktrzcinx can you confirm this is a debug ABI update only. If so, we can put in in ABI3.17.

@ktrzcinxktrzcinxSep 17, 2020

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

This change bumps both ABI numbers

@ktrzcinx

ktrzcinx commented Sep 17, 2020

Copy link
Copy Markdown
MemberAuthor

@lgirdwood As this missed the 1.6 release branch, should this bump ABI to 3.18? If yes, @ktrzcinx could you update the PR with annotations following the recently merged thesofproject/sof-docs#283 .. ?

ABI bumped, added ABI version information for new structures .

@lgirdwood

lgirdwood commented Sep 17, 2020

Copy link
Copy Markdown
Member

@ktrzcinx thank you, btw I've cleaned up the ABI MINOR PRs and I think we are good for this is 3.17 (which we can also close for v1.6). Sorry to ask you to change the ABI back to 3.17 but I think we can get in in v1.6. @kv2019i do you agree ?

@ktrzcinx
ipc: trace: Add trace filter message handling
097cf65
Message received from host carry information about new log level for
component with pass selected filter values. Filter values are passed
as a set of key-value values, what make it easy to extend in future.
Last element in set must have SOF_IPC_TRACE_FILTER_ELEM_FIN bit set
in key field, to mark end of filter values.
New log level is also obligatory, must be within LOG_LEVEL_CRITICAL
to LOG_LEVEL_VERBOSE range.
It allows to changing trace lelvel for each components in firmware,
on particular pipeline or with particular uuid key with single ipc
command.
Updating trace level is not fully implemented yet.
Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>
@ktrzcinx

ktrzcinx commented Sep 17, 2020•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Updated comments to ABI 3.17

@ktrzcinx
trace: Update trace level after ipc message
e4b5921
Update process must be splited into two parts - updating global
components, which have trace contect saved in dedicated section
and updating ipc components, where trace contect is part of
particular instance and data.
Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>
@lgirdwood

lgirdwood commented Sep 18, 2020

Copy link
Copy Markdown
Member

@kv2019i ping any comment here to keep thi sin ABI 3.17 for v1.6 ?

@kv2019i

kv2019i commented Sep 18, 2020

Copy link
Copy Markdown
Collaborator

@lgirdwood wrote:

@kv2019i ping any comment here to keep thi sin ABI 3.17 for v1.6 ?
Aa, you missed my thumbs-up reaction to #3329 (comment) . But yes, go ahead with 3.17 and v1.6!

@lgirdwoodlgirdwood modified the milestones: ABI-3.18, ABI-3.17Sep 18, 2020
@lgirdwood
lgirdwood merged commit 0e1176d into thesofproject:masterSep 18, 2020
@marc-hbmarc-hb mentioned this pull request Dec 9, 2021
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lgirdwoodlgirdwoodlgirdwood approved these changes
@kv2019ikv2019ikv2019i approved these changes
@akloniexakloniexAwaiting requested review from akloniex
@bardliaobardliaoAwaiting requested review from bardliao
@xiulipanxiulipanAwaiting requested review from xiulipan

Assignees

No one assigned

Labels

ABIABI change involved

Projects

None yet

Milestone

ABI-3.17

Development

Successfully merging this pull request may close these issues.

4 participants

@ktrzcinx@lgirdwood@plbossart@kv2019i