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

Copy from and to stream #100

Merged
merged 7 commits into from
Oct 26, 2022
Merged

Copy from and to stream #100

merged 7 commits into from
Oct 26, 2022

Conversation

Mellich
Copy link
Contributor

@Mellich Mellich commented Oct 6, 2022

Allow to use the copy operation to copy data from stream to buffer and from buffer to stream.
Adds a test to the hls test suite to show this behavior.

@TristanLaan
Copy link
Contributor

Here the same as #94 probably applies, that we will want to add some extra functions to avoid the need to use dummybuffers.

@Mellich
Copy link
Contributor Author

Mellich commented Oct 17, 2022

Here the same as #94 probably applies, that we will want to add some extra functions to avoid the need to use dummybuffers.

Wouldn't it make sense to do the API change in a single PR and merge this before?

@TristanLaan
Copy link
Contributor

Here the same as #94 probably applies, that we will want to add some extra functions to avoid the need to use dummybuffers.

Wouldn't it make sense to do the API change in a single PR and merge this before?

I think it’s better to first merge the API change, so that if someone stumbles upon the dev branch, they’re not going to see the dummybuffers as intended behaviour.

@quetric quetric assigned quetric and TristanLaan and unassigned quetric Oct 21, 2022

std::vector<unsigned int> dest = {0};

CCLO_BFM cclo(options.start_port, rank, size, dest, callreq, callack, data_cclo2krnl, data_krnl2cclo);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is not applicable on hardware. When I run it with the axis3x design for U55c it tries to connect to a non-existing ZMQ server and the test crashes with CCLO @0x0: during copy the following error(s) occured: DMA DECODE ERROR, DMA NOT OKAY ERROR (00000000000000000000010100).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I create a BFM, which is not used in hardware. So I have to change this. But the error sounds like something else is... not okay. Let me check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed the BFM creation for hardware builds. But I get the same error during hardware execution. Maybe we need a recent build to test this. It works in the simulator though...

Copy link
Collaborator

@quetric quetric left a comment

Choose a reason for hiding this comment

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

which hardware are we targeting for running this test? the vadd_put and copy tests seem to require different PL kernels attached to the CCLO. Tests on hardware will either fail one or the other

@Mellich
Copy link
Contributor Author

Mellich commented Oct 26, 2022

I moved the copy stream test to the XRT test suite since it should work with the default loopback kernel. In the HLS suite it would lead to problems in hardware. It now works for me when I execute the XRT test across two U55C.

Copy link
Contributor

@TristanLaan TristanLaan left a comment

Choose a reason for hiding this comment

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

Apart from the wrong naming scheme for the hostctrl kernel this works on hardware and emulator.

test/host/xrt/test.cpp Outdated Show resolved Hide resolved
@TristanLaan TristanLaan merged commit 43912a1 into Xilinx:dev Oct 26, 2022
@Mellich Mellich mentioned this pull request Oct 26, 2022
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.

None yet

3 participants