-
Notifications
You must be signed in to change notification settings - Fork 321
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
debug: debug_stream: Access debug slot directly using cavstool.py #9498
debug: debug_stream: Access debug slot directly using cavstool.py #9498
Conversation
e3ebc10
to
cafdef9
Compare
My latest cleanups were still un committed, thus the last update. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's sync with the version of cavstool.py that will get merged. Current version fails to
AttributeError: module 'cavstool' has no attribute 'fw_is_alive_dsp'. Did you mean: 'fw_is_alive'?
cafdef9
to
5c1957e
Compare
Latest push added some robustness improvements to the python script and config overlay for building debug_stream_thread_info. |
5c1957e
to
4bfe83a
Compare
The important part of this PR - the debug_stream.py direct access - is now split in two:
Then there is the debug_stream_overlay.conf added that is identical to the previous version and on simple typo fix commit. |
@jsarha does this PR depend on the Zephyr PRs ? if so , lets mark as DNM until Zephyr parts merged. |
Access debug slot directly using cavstool.py, by specifying the debug slot number where the debug_stream data is transferred. Adds one command line parameter for selecting the debug slot directly and adds an alternative mainloop for polling the slot through cavstool.py direct access. The commit also adds quite a few data consistency checks and error handling improvements as with the direct memory access its much more likely to get inconsistent data when the DSP is booting up, and code needs to be robust enough not to crash in such a situation. Also the logging messages about those checks failing has been lowered so that they are not too noisy. Signed-off-by: Jyri Sarha <[email protected]>
4bfe83a
to
e040178
Compare
Fixed couple of simple codestyle issues found by checkpatch. |
The script actually does not stop from working even if the cavstool.py changes are not available, that is, if the debugfs file is there. But maybe the error messages if trying to use -c flag are too confusing. I'll add the tag. |
@jsarha Both Zephyr PRs look very similar - best to close one PR if its been deprecated by a new PR. |
@lgirdwood , the first PR [1] is the base of the second PR [2]. Actually the second [2] has only one commit on top of [1]. I was expecting the first PR to get merged soon as it already had two approvals (and I did not want it to get back to square one). Once the first is merged, I'll rebase the second, and it becomes only one commit PR. [1] zephyrproject-rtos/zephyr#78753 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main Zephyr cavstool.py change is merged, so I thnk this can be merged now as the interface is settled. The other Zephyr change seems nice-to-have and not a strict dependency to this one, so I don't think this PR needs to block on that either.
Add overlay to build debug_stream protocol over a debug window slot, make room for the slot, and send thread info data through it. Signed-off-by: Jyri Sarha <[email protected]>
A simple typo fix to a logging message. Signed-off-by: Jyri Sarha <[email protected]>
The new cavstool.debug_slot_offset_by_type() opens an opportunity to improve debug_stream.py usability a bit. If the debugfs file is not found, then try to find the correct debug slot the correct slot using cavstool direct access and debug_slot_offset_by_type(). Signed-off-by: Jyri Sarha <[email protected]>
e040178
to
49e74b9
Compare
I dropped the commit that depends on zephyrproject-rtos/zephyr#79061 so this should be good for merging now. I'll make another PR that depends on the above PR. The new PR will be rebased on top of this, so it will contain the same commit as this. |
zephyrproject-rtos/zephyr#79061 was merged last night, so picking "tools: debug_stream.py: Use cavstool.debug_slot_offset_by_type()" back on board of this PR again. |
Do we need a west update now ? |
@lgirdwood wrote:
I don't think that's needed. This is a debug tool that is not compiled so there's no hard dependency. So if CI is good (not yet the case), I'd just proceed and merge this as it is now. |
Problem with Windows builds in github flows, but now passing. Proceeding to merge. |
Access debug slot directly using cavstool.py, by specifying the debug slot number where the debug_stream data is transferred.
Adds one command line parameter for selecting the debug slot directly, adds an alternative mainloop for polling the slot through cavstool.py direct access, and relaxes some often occurring warning print. The warning prints happen when the FW boots and the cavstool code wakes up before the init code has initialized the slot.
NOTE:
This code depends from this PR: zephyrproject-rtos/zephyr#78753 . However, since the import line is in the function that is only called if the --direct-access-slot flag is present, the old code will still work without cavstool.py, but that would in turn need this PR: thesofproject/linux#5154