[Fix][microTVM] QEMU RPC issue #8021
Conversation
| @@ -214,7 +214,8 @@ tvm_crt_error_t TVMPlatformTimerStop(double* elapsed_time_seconds) { | |||
| } | |||
|
|
|||
| // Ring buffer used to store data read from the UART on rx interrupt. | |||
| #define RING_BUF_SIZE_BYTES 4 * 1024 | |||
| // Should be larger than TVM_CRT_MAX_PACKET_SIZE_BYTES | |||
There was a problem hiding this comment.
can you comment how much larger, and why?
There was a problem hiding this comment.
the minimum value is 25 bytes and I don't know why.
Also I tested with different baudrates (default is 115200) to see if that solve the issue, but it didn't help.
mehrdadh
force-pushed
the
qemu_large_rpc
branch
3 times, most recently
from
f21013c
to
669f71c
Compare
|
@areusch this is ready to review. PTAL. |
|
|
||
| // Small buffer used to read data from the UART into the ring buffer. | ||
| static uint8_t uart_data[8]; | ||
| // TODO(mehrdadh): Explore why offset is required. |
There was a problem hiding this comment.
can you explain why TVM_CRT_MAX_PACKET_SIZE_BYTES is required, though? I believe it's not needed on most physical hardware
There was a problem hiding this comment.
This could be because of qemu only. Let me confirm that and add comment on the file.
There was a problem hiding this comment.
just wanted to follow-up on this one
There was a problem hiding this comment.
I checked this on physical hardware and as you mentioned this large value for ring buffer size is only required for testing on QEMU. I added a comment about this. Please let me know if you have any other concern.
* add test * fix test * add parameter to test * cleanup * format * address comments * address comments * direct read/write from/to ring buffer * merge fix * add comment
* add test * fix test * add parameter to test * cleanup * format * address comments * address comments * direct read/write from/to ring buffer * merge fix * add comment
This issue happens when we send a large tensor over RPC to the zephyr qemu target specially when it runs on a fast machine. We fixed the RPC large transfer size for microTVM at #7838, however, #7838 doesn't include RPC test with qemu and didn't catch this problem.
In this PR: