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

XDP: Timer synchronization and microcontroller trace changes #8682

Merged
merged 5 commits into from
Jan 9, 2025

Conversation

parthash0804
Copy link
Collaborator

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

Signed-off-by: parthash0804 <ParthAshwin.Jain@amd.com>
Signed-off-by: parthash0804 <ParthAshwin.Jain@amd.com>
Signed-off-by: parthash0804 <ParthAshwin.Jain@amd.com>
Signed-off-by: parthash0804 <ParthAshwin.Jain@amd.com>
Copy link
Collaborator

@pgschuey pgschuey left a 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.

@IshitaGhosh
Copy link
Collaborator

Except for concern raised by Paul for new files, the PR looks good. If we absolutely need the new files, please add copyright message.

Copy link
Collaborator

@IshitaGhosh IshitaGhosh left a 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>
@parthash0804
Copy link
Collaborator Author

@pgschuey I have made the changes as per your suggestions we had offline.
cc: @IshitaGhosh

Copy link
Collaborator

@pgschuey pgschuey left a comment

Choose a reason for hiding this comment

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

Changes look good.

@jvillarre jvillarre merged commit 5d84ad2 into Xilinx:master Jan 9, 2025
23 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.

4 participants