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

EDGEML-8777: Support firmware log buffer #328

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

EDGEML-8777: Support firmware log buffer #328

wants to merge 1 commit into from
+483 −1

Conversation

VinitAmd
Copy link

@VinitAmd VinitAmd commented Dec 16, 2024 •
edited
Loading

[Why]
Log buffer support is required to enhance debugging support on NPU stack

[How]

  1. allocate log buffer and share with firmware via 'start event trace' message Config start_event_trace request param, send via mgmt channel. Handle the send buffer resp.
  2. receive notifications about log buffer fullness via interrupt from firmware. Handle channel interrupt process read FW lg buffer and update log metadata head/tail.

Signed-off-by: Vinit vinitkumar.shukla@amd.com

@AMDGithubSCIMAdmin
Copy link

Can one of the admins verify this patch?

mamin506
mamin506 reviewed Dec 16, 2024
View reviewed changes
Copy link
Contributor

@mamin506 mamin506 left a comment

Choose a reason for hiding this comment

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

"__packed" is missing for the request and response structs.

src/driver/amdxdna/aie2_msg_priv.h Outdated Show resolved Hide resolved
src/driver/amdxdna/aie2_msg_priv.h Outdated Show resolved Hide resolved
src/driver/amdxdna/aie2_msg_priv.h Outdated Show resolved Hide resolved
@mamin506 mamin506 requested a review from vengutta18 December 16, 2024 19:51
@maxzhen maxzhen added the draft label Dec 19, 2024
@VinitAmd VinitAmd force-pushed the fw_log_buff_x branch 3 times, most recently from 5ac87bf to add9c42 Compare December 23, 2024 09:35
@VinitAmd VinitAmd force-pushed the fw_log_buff_x branch 3 times, most recently from a64f459 to bf466db Compare January 7, 2025 08:34
@VinitAmd VinitAmd marked this pull request as ready for review January 7, 2025 08:38
@vengutta18 vengutta18 removed the draft label Jan 7, 2025
@mamin506
Copy link
Contributor

mamin506 commented Jan 9, 2025

ok to test

@VinitAmd
Copy link
Author

VinitAmd commented Jan 9, 2025

=== 3 tests failed on Linux_npu_tests run.
ipu_suspend : aie2_state_write:NPU suspend \xf8,\x94\x84\xff\xff\xff\xff\xb8%n\x84\xff\xff\xff\xff\x08\x1c̅\xff\xff\xff\xff failed
ipu_resume : aie2_state_write: NPU resume \xff\xf8,\x94\x84\xff\xff\xff\xff(\xbcw\x8eW\xa8\xff\xff\x08\x1c̅\xff\xff\xff\xff failed
ipu_check_header_hash
Already ticket is raised for FW team FWDEV-103771 .

@mamin506
Copy link
Contributor

@VinitAmd , please fix the pipeline failure. The driver is not able to load. Please try your driver on a NPU1 machine. Please contact @vengutta18 for the machine if you don't have one.

mamin506
mamin506 requested changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@mamin506 mamin506 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I'm not expect any changes for amdxdna_mailbox* files for this feature.
Please cleanup the amdxdna_mailbox* files.

After this is fix, I will review other code.
Please follow https://github.com/amd/xdna-driver?tab=readme-ov-file#checkpatch to setup your workspace.
The coding style needs to pass checkpatch.pl test.

src/driver/amdxdna/amdxdna_mailbox_helper.h Outdated Show resolved Hide resolved
src/driver/amdxdna/amdxdna_mailbox.h Outdated Show resolved Hide resolved
src/driver/amdxdna/amdxdna_mailbox.c Outdated Show resolved Hide resolved
src/driver/amdxdna/amdxdna_mailbox.c Outdated Show resolved Hide resolved
@VinitAmd
Copy link
Author

I'm not expect any changes for amdxdna_mailbox* files for this feature. Please cleanup the amdxdna_mailbox* files.

After this is fix, I will review other code. Please follow https://github.com/amd/xdna-driver?tab=readme-ov-file#checkpatch to setup your workspace. The coding style needs to pass checkpatch.pl test.

Thanks, I was looking for this info

@VinitAmd VinitAmd force-pushed the fw_log_buff_x branch 5 times, most recently from 7d9850d to 2d0d0b4 Compare January 13, 2025 11:02
@VinitAmd
Copy link
Author

I'm not expect any changes for amdxdna_mailbox* files for this feature. Please cleanup the amdxdna_mailbox* files.
After this is fix, I will review other code. Please follow https://github.com/amd/xdna-driver?tab=readme-ov-file#checkpatch to setup your workspace. The coding style needs to pass checkpatch.pl test.

Thanks, I was looking for this info

Code formatting done with checkpatch.pl

@VinitAmd VinitAmd force-pushed the fw_log_buff_x branch 2 times, most recently from e77d462 to 8d6b7a1 Compare January 15, 2025 17:45
@mamin506
Copy link
Contributor

retest this please

@amd-akshatah amd-akshatah reopened this Jan 27, 2025
@amd-akshatah
Copy link

retest this please

vengutta18
vengutta18 requested changes Jan 28, 2025
View reviewed changes
src/driver/amdxdna/aie2_event_trace.c Outdated Show resolved Hide resolved
src/driver/amdxdna/aie2_pci.h Outdated Show resolved Hide resolved
src/driver/amdxdna/aie2_event_trace.c Outdated Show resolved Hide resolved
src/driver/amdxdna/aie2_event_trace.c Outdated Show resolved Hide resolved
src/driver/amdxdna/aie2_message.c Outdated Show resolved Hide resolved
src/driver/amdxdna/aie2_message.c Outdated Show resolved Hide resolved
src/driver/amdxdna/aie2_pci.c Outdated Show resolved Hide resolved
@VinitAmd VinitAmd force-pushed the fw_log_buff_x branch 2 times, most recently from be8f46f to fa2a7b8 Compare January 29, 2025 06:00
vengutta18
vengutta18 reviewed Jan 29, 2025
View reviewed changes
src/driver/amdxdna/aie2_event_trace.c Outdated Show resolved Hide resolved
src/driver/amdxdna/aie2_event_trace.c Outdated Show resolved Hide resolved
src/driver/amdxdna/aie2_pci.c Outdated Show resolved Hide resolved
@VinitAmd VinitAmd force-pushed the fw_log_buff_x branch 4 times, most recently from 697afd2 to 26e8b0e Compare February 3, 2025 06:20
mamin506
mamin506 requested changes Feb 3, 2025
View reviewed changes
src/driver/amdxdna/aie2_event_trace.c Outdated Show resolved Hide resolved
src/driver/amdxdna/aie2_event_trace.c Outdated Show resolved Hide resolved
@VinitAmd VinitAmd force-pushed the fw_log_buff_x branch 2 times, most recently from e3d6e4f to 0163508 Compare February 5, 2025 09:33
@VinitAmd VinitAmd force-pushed the fw_log_buff_x branch 4 times, most recently from c2ed2f3 to c103db3 Compare February 12, 2025 19:25
EDGEML-8777 - [Linux NPU Driver]: Support firmware log buffer
397b267 
[Why]
  Log buffer support is required to enhance debugging support on NPU stack

[How]
  1. Allocate log buffer and share with firmware via 'start_event_trace' message
     Config start_event_trace request param, send via mgmt channel.
     Handle the send buffer resp.
  2. Receive interrupt about log buffer half fullness from firmware.
     Handle interrupt, process further buffer data and update buffer head_offset for FW.
  3. Add stop_event_trace_send api and it's handle to stop logging when aie2 shutdown.
  4. Add event_trace debugfs for dynamic control of logging.

Signed-off-by: vinit shukla <vinitkumar.shukla@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxzhen maxzhen maxzhen left review comments

@vengutta18 vengutta18 vengutta18 requested changes

@mamin506 mamin506 mamin506 requested changes

Requested changes must be addressed to merge this pull request.

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

Successfully merging this pull request may close these issues.
7 participants
@VinitAmd @AMDGithubSCIMAdmin @mamin506 @amd-akshatah @sreedharamd @maxzhen @vengutta18