Skip to content

debug: debug_stream: Access debug slot directly using cavstool.py - #9498

Merged
kv2019i merged 4 commits into
thesofproject:mainfrom
jsarha:debug_stream_thread_info_cavstool_access
Oct 7, 2024
Merged

debug: debug_stream: Access debug slot directly using cavstool.py#9498
kv2019i merged 4 commits into
thesofproject:mainfrom
jsarha:debug_stream_thread_info_cavstool_access

Conversation

@jsarha

Copy link
Copy Markdown
Contributor

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

@jsarha
jsarhaforce-pushed the debug_stream_thread_info_cavstool_access branch from e3ebc10 to cafdef9CompareSeptember 20, 2024 09:02
@jsarha

Copy link
Copy Markdown
ContributorAuthor

My latest cleanups were still un committed, thus the last update.

@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.

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'?

@jsarha
jsarhaforce-pushed the debug_stream_thread_info_cavstool_access branch from cafdef9 to 5c1957eCompareSeptember 24, 2024 12:49
@jsarha

Copy link
Copy Markdown
ContributorAuthor

Latest push added some robustness improvements to the python script and config overlay for building debug_stream_thread_info.

@jsarha
jsarhaforce-pushed the debug_stream_thread_info_cavstool_access branch from 5c1957e to 4bfe83aCompareSeptember 26, 2024 14:15
@jsarha

Copy link
Copy Markdown
ContributorAuthor

The important part of this PR - the debug_stream.py direct access - is now split in two:

  1. The basic functionality that depends on: Make cavstool code more shareable zephyrproject-rtos/zephyr#78753

    • Some more cleaning up usability improvements on top of previous version
  2. A usability improvement to always just work if at all possible, depends on: Cavstool debug slot offset by type zephyrproject-rtos/zephyr#79061

Then there is the debug_stream_overlay.conf added that is identical to the previous version and on simple typo fix commit.

@lgirdwood

Copy link
Copy Markdown
Member

@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 <jyri.sarha@linux.intel.com>
@jsarha
jsarhaforce-pushed the debug_stream_thread_info_cavstool_access branch from 4bfe83a to e040178CompareSeptember 27, 2024 07:17
@jsarha

Copy link
Copy Markdown
ContributorAuthor

Fixed couple of simple codestyle issues found by checkpatch.

@jsarha

Copy link
Copy Markdown
ContributorAuthor

@jsarha does this PR depend on the Zephyr PRs ? if so , lets mark as DNM until Zephyr parts merged.

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.

@jsarhajsarha changed the title debug: debug_stream: Access debug slot directly using cavstool.py[DMN] debug: debug_stream: Access debug slot directly using cavstool.pySep 27, 2024
@jsarhajsarha changed the title [DMN] debug: debug_stream: Access debug slot directly using cavstool.py[DNM] debug: debug_stream: Access debug slot directly using cavstool.pySep 27, 2024
@lgirdwood

Copy link
Copy Markdown
Member

@jsarha does this PR depend on the Zephyr PRs ? if so , lets mark as DNM until Zephyr parts merged.

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.

@jsarha

Copy link
Copy Markdown
ContributorAuthor

@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
[2] zephyrproject-rtos/zephyr#79061

@jsarhajsarha added the DNM Do Not Merge tag label Oct 2, 2024

@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.

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.

Jyri Sarha added 3 commits October 3, 2024 19:46
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 <jyri.sarha@linux.intel.com>
A simple typo fix to a logging message.
Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
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 <jyri.sarha@linux.intel.com>
@jsarha
jsarhaforce-pushed the debug_stream_thread_info_cavstool_access branch from e040178 to 49e74b9CompareOctober 3, 2024 17:08
@jsarhajsarha changed the title [DNM] debug: debug_stream: Access debug slot directly using cavstool.pydebug: debug_stream: Access debug slot directly using cavstool.pyOct 3, 2024
@jsarhajsarha removed the DNM Do Not Merge tag label Oct 3, 2024
@jsarha

Copy link
Copy Markdown
ContributorAuthor

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.

@jsarha

jsarha commented Oct 4, 2024

Copy link
Copy Markdown
ContributorAuthor

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.

@lgirdwood

Copy link
Copy Markdown
Member

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 ?

@kv2019i

Copy link
Copy Markdown
Collaborator

@lgirdwood wrote:

Do we need a west update now ?

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.

@kv2019i

Copy link
Copy Markdown
Collaborator

Problem with Windows builds in github flows, but now passing. Proceeding to merge.

@kv2019i
kv2019i merged commit af55b45 into thesofproject:mainOct 7, 2024
Sign up for freeto 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.

3 participants

@jsarha@lgirdwood@kv2019i