-
Notifications
You must be signed in to change notification settings - Fork 52
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
Implement xrt-smi query to report preemption counters #368
base: main
Are you sure you want to change the base?
Conversation
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.
Some minor change is needed. Not reviewed SHIM changes.
pci_dev_impl.ioctl(DRM_IOCTL_AMDXDNA_GET_INFO, &query_telemetry); | ||
|
||
if (telemetry.major != NPU_TELEMETRY_VERSION_MAJOR || telemetry.minor != NPU_TELEMETRY_VERSION_MINOR) { | ||
memset(&telemetry, 0, sizeof(telemetry)); |
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.
why do you need to clear the buffer?
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.
This was done to align with the Windows driver implementation for the Phoenix devices. Since we don't have preemption support in Phoenix, the expectation is that querying the preemption counter should return zeros instead of an exception.
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.
You want to clear the buffer, send it down to driver and return to xrt-smi with whatever is returned from driver. On PHX, since there is no preemption support, driver should have already put all zeros in the buffer, no?
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.
XDNA driver do put all zeros in the buffer.
- buff = dma_alloc_noncoherent()
- memset(buff, 0, aligned_buffer_size)
- query telemetry data from firmware
- copy_to_user(args->buffer, buff, args->buffer_size)
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.
As @mamin506 mentioned, the driver does initialize the buffer to all 0s. However, the firmware memsets it to all 0xFF values by default.
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 driver change looks good.
Implement xrt-smi query to report preemption counters. Signed-off-by: Nishad Saraf <[email protected]>
Implement xrt-smi query to report preemption counters.
PS: Error "Subcommand not found" is due to the recent changes in xrt, unrelated to this change.