Repository navigation
Conversation
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
|
Though this is a nice to have addition, this will need to go through approvals for this information to be a part of the platforms report. |
aktondak
left a comment
There was a problem hiding this comment.
Please go through my last comment and let us know if it suffices
|
It does not. The gap is the subprocess, not the parsing.
A correction to my own framing.
I took (1): no new query, no I can take it through whatever approval the platform report needs, or switch to (2). Your call. |
xrt-smi examine resolves the AIE architecture from the PCIe device and revision pair and reports it as aie_architecture_version (Xilinx#9861). The platform ptree behind xrt::device::get_info<platform>() carries only the vbnv name and total_columns, so a library consumer cannot reach the same fact without shelling out to xrt-smi and parsing its output. Add the field to the Ryzen static region through the same helper, under the same key, and print it in the platform report. Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
The alveo branch of this same function adds "N/A" from both of its catches so the key is always present; the ryzen branch omitted it instead. Widen the catch to match XBUtilities, which runs the identical pcie_id sequence: the amdxdna shim throws xrt_core::generic_error, which does not derive from xrt_core::query::exception and so escaped the narrower catch. Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
e6bbe1c to
8cd7f36
Compare
Problem
xrt-smi examinereportsaie_architecture_version(aie2,aie2p,aie4,aie2ps), resolvedfrom
query::pcie_idthroughsmi_hardware_config(#9861). The same fact is not reachable from thelibrary:
xrt::device::get_info<xrt::info::device::platform>()puts onlynameandtotal_columnsin the Ryzen static region, so a program linked against XRT -- including pyxrt consumers -- has to
run
xrt-smiand parse its output to learn which AIE architecture it is on.The
namefield is not a substitute. It is the driver's vbnv string, resolved at runtime from afirmware revision query, so it carries the SKU name and falls back to a different naming scheme when
that query fails. On this host
{0x17f0, 0x10}reportsNPU Gorgon Point 1, while the same pair isstxB0inhardware_map.Fix
Add
aie_architecture_versionto the Ryzen branch ofadd_static_region_info, resolved through thesame
smi_hardware_confighelper and stored under the same key as the examine report, withN/Awhen the identity cannot be resolved -- so the key is always present, as it already is in the alveo
branch of the same function.
ReportRyzenPlatformprints it.No new query and no new API surface: there is no added
xrt::info::deviceenumerator and nosignature change. It is additive to the JSON that
xrt::device::get_info<xrt::info::device::platform>()already returns, which is a change to that output's shape, not to the API itself.
Test
Built Release with
-DXRT_NPU=1 -DXRT_ENABLE_WERROR=1;xrt-smilinks clean.The NPU on this host is
{device 0x17f0, revision 0x10}(/sys/class/accel/accel0/device/), vbnvNPU Gorgon Point 1. Calling the resolver this patch uses, in the library built from this branch:so the Ryzen platform report gains
aie_architecture_version: aie2pnext to the existingnameandtotal_columns.Scope
I could not run the built
xrt-smiagainst the device on this host: the tree is 2.26.0 while theinstalled NPU driver plugin is 2.21.75, and the core-to-plugin interface is not versioned across that
gap. The rendering path is covered by construction rather than by a device run.