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

Adding support for Hexagon User DMA Engine #10217

Merged
merged 10 commits into from
Feb 11, 2022

Conversation

adstraw
Copy link
Contributor

@adstraw adstraw commented Feb 10, 2022

Adding support for the Hexagon User DMA engine consisting of three header files which describe the DMA engine in terms of its descriptors, instructions and registers. Also adding a flow for synchronous (blocking) 1D (vs. 2D) DMA which mimics memcpy semantics as a starting place for Hexagon User DMA engine support. This existing test covers this PR:

def test_add_vtcm(tvm_tracker_host, tvm_tracker_port, android_serial_number):

Many thanks to the folks at Qualcomm for sending over code which was leveraged for this PR.

@adstraw adstraw force-pushed the hexagon-user-dma-1d-sync branch from db46dcf to 5e12171 Compare February 10, 2022 21:10
#define DESC_DESCTYPE_1D 0
#define DESC_DESCTYPE_2D 1

// TODO(Straw): Definition? Not in the spec.
Copy link
Contributor

Choose a reason for hiding this comment

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

Trying to understand what this TODO means, could you elaborate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. The fields marked with this TODO were in header files sent by Qualcomm but the fields are not described in the spec. I left myself a TODO to investigate. Also added a "thank you" to Qualcomm in the description for sending code that was leveraged here.

Copy link
Contributor

@tmoreau89 tmoreau89 left a comment

Choose a reason for hiding this comment

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

LGTM. For posterity, when you say existing tests cover this PR; which tests are we talking about?

@adstraw
Copy link
Contributor Author

adstraw commented Feb 10, 2022

LGTM. For posterity, when you say existing tests cover this PR; which tests are we talking about?

I added a link to the test in the description.

@tmoreau89
Copy link
Contributor

CC @masahi and @kparzysz-quic

Copy link
Contributor

@kparzysz-quic kparzysz-quic left a comment

Choose a reason for hiding this comment

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

Looks good overall.

src/runtime/hexagon/hexagon/hexagon_user_dma_descriptors.h Outdated Show resolved Hide resolved
src/runtime/hexagon/hexagon/hexagon_user_dma.cc Outdated Show resolved Hide resolved
Copy link
Contributor

@kparzysz-quic kparzysz-quic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@jroesch jroesch merged commit 6dece18 into apache:main Feb 11, 2022
ylc pushed a commit to ylc/tvm that referenced this pull request Feb 16, 2022
* initial hexagon user dma impl

* Hexagon User DMA descriptor, instruction and register headers

* Synchronous 1D DMA working

* HexagonBuffer unit tests passing with memcpy

* cleanup

* comments and orgnanize code

* format and lint

* init function + other code review feedback

* add ifdef hexagon around inline asm
@adstraw adstraw deleted the hexagon-user-dma-1d-sync branch February 16, 2022 19:35
pfk-beta pushed a commit to pfk-beta/tvm that referenced this pull request Apr 11, 2022
* initial hexagon user dma impl

* Hexagon User DMA descriptor, instruction and register headers

* Synchronous 1D DMA working

* HexagonBuffer unit tests passing with memcpy

* cleanup

* comments and orgnanize code

* format and lint

* init function + other code review feedback

* add ifdef hexagon around inline asm
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