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.

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

Vitis 6327 Add PS kernel xclbins into APU Package #7594

Merged
merged 17 commits into from
Jun 29, 2023

Conversation

dbenusov
Copy link
Contributor

@dbenusov dbenusov commented Jun 16, 2023

Problem solved by the commit

https://jira.xilinx.com/browse/VITIS-6327

The platform team requested that XRT create the xclbins that contain the PS kernels. They will generate the xclbins that contain the PL portions. This PR addresses adding the PS kernel xclbins into the APU package.

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered

The validation PS kernels were added into the build for edge platforms. The installation directory for all generated ps kernels is /lib/firmware/xilinx/ps_kernels. This is the same directory in which the apu files are stored.

An additional json file is included to list out which PL xclbins are required to operate this PS kernel.

How problem was solved, alternative solutions (if any) and why they were rejected

Created CMake files that generate the Xclbins using a passed in reference to a VITIS directory. Added logic to install the generated xclbins into /lib/firmware/xilinx/ps_kernels within pkgapu.sh. xbutil will need to be updated in a 2nd PR.

Risks (if any) associated the changes in the commit

Only one APU package can be installed at a time. If the time comes where multiple APU packages can be installed we must revist how the PS kernels are packaged.

Additionally, I am concerned about sourcing an internal directory within the yocto build to get access to xclbinutil. It would be better to allow this to be set some other way so that non-internal customers can build the apu package.

What has been tested and how, request additional testing if necessary

Ubuntu 20.04

// Install the package
dbenusov@xsjdbenusov50:/proj/rdi/staff/dbenusov/XRT/build$ sudo apt-get install ./versal/apu_packages/xrt-apu-vck5000_202310.2.15.0_all.deb
Reading package lists... Done
Building dependency tree
Reading state information... Done
Note, selecting 'xrt-apu-vck5000' instead of './versal/apu_packages/xrt-apu-vck5000_202310.2.15.0_all.deb'
The following NEW packages will be installed:
  xrt-apu-vck5000
0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded.
Need to get 0 B/90.3 MB of archives.
After this operation, 0 B of additional disk space will be used.
Get:1 /proj/xsjhdstaff6/dbenusov/XRT/build/versal/apu_packages/xrt-apu-vck5000_202310.2.15.0_all.deb xrt-apu-vck5000 all 2.15.0 [90.3 MB]
dpkg: warning: failed to open configuration file '/home/dbenusov/.dpkg.cfg' for reading: Permission denied
Selecting previously unselected package xrt-apu-vck5000.
(Reading database ... 258103 files and directories currently installed.)
Preparing to unpack .../xrt-apu-vck5000_202310.2.15.0_all.deb ...
Unpacking xrt-apu-vck5000 (2.15.0) ...
dpkg: warning: failed to open configuration file '/home/dbenusov/.dpkg.cfg' for reading: Permission denied
Setting up xrt-apu-vck5000 (2.15.0) ...

// Validate the contents are installed
dbenusov@xsjdbenusov50:/proj/rdi/staff/dbenusov/XRT/build$ ls /lib/firmware/xilinx/ps_kernels/
ps_aie.xclbin  ps_bandwidth.xclbin  ps_validate.xclbin  test_dependencies.json

Documentation impact (if any)

None.

…. pkgapu not able to create xclbins due to missing symbols in ps kernel libraries

Signed-off-by: Daniel Benusovich <[email protected]>
@gbuildx
Copy link
Collaborator

gbuildx commented Jun 16, 2023

Build Passed!

@dbenusov dbenusov marked this pull request as ready for review June 16, 2023 15:59
build/build_edge.sh Outdated Show resolved Hide resolved
src/runtime_src/tools/scripts/pkgapu.sh Show resolved Hide resolved
@dayeh-xilinx
Copy link

retest this please.

@gbuildx
Copy link
Collaborator

gbuildx commented Jun 16, 2023

Build failed :(

…build script to use new file. Update apu package script to use new file

Signed-off-by: Daniel Benusovich <[email protected]>
@gbuildx
Copy link
Collaborator

gbuildx commented Jun 19, 2023

Build Passed!

@gbuildx
Copy link
Collaborator

gbuildx commented Jun 23, 2023

Build Passed!

Copy link
Collaborator

@rozumx rozumx left a comment

Choose a reason for hiding this comment

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

Overall this this looks good.

Regarding the validation tests, it would be great if future PRs has more comments in the cpp source code to assist the next developer in debugging issues.

@sabarinaxil
Copy link

retest this please

@gbuildx
Copy link
Collaborator

gbuildx commented Jun 26, 2023

Build failed :(

@manikandan-xilinx
Copy link
Collaborator

retest this please.

@gbuildx
Copy link
Collaborator

gbuildx commented Jun 27, 2023

Build failed :(

@manikandan-xilinx
Copy link
Collaborator

retest this please.

@gbuildx
Copy link
Collaborator

gbuildx commented Jun 27, 2023

Build Passed!

@gbuildx
Copy link
Collaborator

gbuildx commented Jun 27, 2023

Build Passed!

Copy link
Member

@sonals sonals left a comment

Choose a reason for hiding this comment

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

We should move away from sk_types.h. However, that should be taken up as a separate effort.

@stsoe stsoe merged commit d880395 into Xilinx:master Jun 29, 2023
2 checks passed
Sign up for free to 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.