-
Notifications
You must be signed in to change notification settings - Fork 480
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
XDP: Timer synchronization and microcontroller trace changes #8682
Conversation
Signed-off-by: parthash0804 <ParthAshwin.Jain@amd.com>
Signed-off-by: parthash0804 <ParthAshwin.Jain@amd.com>
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.
PR looks good. Why were new files created in the trace util subdirectory instead of adding the new functions to the existing aie_trace_config.* files? The idea of the util directory was to include functions common across implementations, so marking a file as common is redundant.
Except for concern raised by Paul for new files, the PR looks good. If we absolutely need the new files, please add copyright message. |
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.
Except for concern raised by Paul for new files, the PR looks good. If we absolutely need the new files, please add copyright message.
Signed-off-by: parthash0804 <ParthAshwin.Jain@amd.com>
@pgschuey I have made the changes as per your suggestions we had offline. |
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.
Changes look good.
Problem solved by the commit
Timer were not in sync for ve2 device and microcontroller trace was not supported.
How problem was solved, alternative solutions (if any) and why they were rejected
Generated user event in cdo which will trigger the broadcast network and will be used to start the trace modules. Moved timer synchronization from cdo to XDP. @jyothees99 implemented the trace module start broadcast network.
Risks (if any) associated the changes in the commit
Aie_partition_info query from XRT which we are using is returning incorrect information, so until that is fixed trace won't be generated. We have informed @chvamshi-xilinx and @saifuddin-xilinx about it.
Micro controller trace has not been tested because the metric set changes needs to be submitted by @pgschuey . It will be submitted once this PR is merged.
What has been tested and how, request additional testing if necessary
Tested a multi layer design on QEMU+A72 flow.
Documentation impact (if any)
NA