From a91eab67d8389d979d29fdf6633b2edd0718eb4b Mon Sep 17 00:00:00 2001 From: Mark Todorovich Date: Sat, 31 Oct 2026 16:49:23 -0700 Subject: [PATCH] hexagon : fix DMA transfers with strides > 24 bits (#31442) * hexagon : fix DMA transfers with strides > 24 bits On v75+ the 2D DMA descriptor stores row_size and strides in 24 bits. dma_queue_push sent oversized strides to fallback_2d, which only splits nrows, so strides of 16 MB or more were truncated - wrong addresses. Fix will fall back to one transfer per row in that case. Also added two CONT tests with source strides over 16 MB. * hex-dma: refactor the fallback paths to remove duplication --------- Co-authored-by: Max Krasnyansky --- ggml/src/ggml-hexagon/htp/dma-queue.c | 67 ++++++++------------------- ggml/src/ggml-hexagon/htp/dma-queue.h | 11 +---- tests/test-backend-ops.cpp | 4 ++ 3 files changed, 26 insertions(+), 56 deletions(-) diff --git a/ggml/src/ggml-hexagon/htp/dma-queue.c b/ggml/src/ggml-hexagon/htp/dma-queue.c index ef61d2d24..11d52a919 100644 --- a/ggml/src/ggml-hexagon/htp/dma-queue.c +++ b/ggml/src/ggml-hexagon/htp/dma-queue.c @@ -141,43 +141,9 @@ bool dma_queue_push_fallback_2d(dma_queue * q, dma_data ddata, size_t dst_stride dma_ring_flush(r1); r0->tail = r1->tail; - return dma_ring_push_single_2d(r0, ddata, 0, 0, 0, /*nrows=*/ 0); -} - -bool dma_queue_push_fallback_contig(dma_queue * q, dma_data ddata, size_t total) { - dma_ring * r0 = q->ring0; - dma_ring * r1 = q->ring1; - - if (((r0->push_idx + 1) & r0->idx_mask) == r0->pop_idx) { - return false; - } - - r1->tail = r0->tail; - - size_t rem_bytes = total; - dma_addr_t cur_dst = ddata.dst; - dma_addr_t cur_src = ddata.src; - - while (rem_bytes > 0) { - const uint32_t cur_bytes = MIN(rem_bytes, DMA_SAFE_CHUNK_SIZE); - dma_data cur_data = dma_make_data(cur_dst, cur_src); - if (!dma_ring_push_single_contig(r1, cur_data, cur_bytes)) { - dma_ring_flush(r1); - dma_ring_push_single_contig(r1, cur_data, cur_bytes); - } - cur_dst += cur_bytes; - cur_src += cur_bytes; - rem_bytes -= cur_bytes; - } - - dma_ring_flush(r1); - r0->tail = r1->tail; - return dma_ring_push_single_1d(r0, ddata, /*size=*/ 0); } -#if __HVX_ARCH__ < 75 - bool dma_queue_push_fallback_1d(dma_queue * q, dma_data ddata, size_t dst_stride, size_t src_stride, size_t row_size, size_t nrows) { dma_ring * r0 = q->ring0; dma_ring * r1 = q->ring1; @@ -188,19 +154,28 @@ bool dma_queue_push_fallback_1d(dma_queue * q, dma_data ddata, size_t dst_stride r1->tail = r0->tail; - size_t rem_rows = nrows; - dma_addr_t cur_dst = ddata.dst; - dma_addr_t cur_src = ddata.src; + dma_addr_t row_dst = ddata.dst; + dma_addr_t row_src = ddata.src; - while (rem_rows > 0) { - dma_data cur_data = dma_make_data(cur_dst, cur_src); - if (!dma_ring_push_single_1d(r1, cur_data, row_size)) { - dma_ring_flush(r1); - dma_ring_push_single_1d(r1, cur_data, row_size); + for (size_t i = 0; i < nrows; i++) { + size_t rem_bytes = row_size; + dma_addr_t cur_dst = row_dst; + dma_addr_t cur_src = row_src; + + while (rem_bytes > 0) { + const uint32_t cur_bytes = MIN(rem_bytes, DMA_SAFE_CHUNK_SIZE); + dma_data cur_data = dma_make_data(cur_dst, cur_src); + if (!dma_ring_push_single_contig(r1, cur_data, cur_bytes)) { + dma_ring_flush(r1); + dma_ring_push_single_contig(r1, cur_data, cur_bytes); + } + cur_dst += cur_bytes; + cur_src += cur_bytes; + rem_bytes -= cur_bytes; } - cur_dst += dst_stride; - cur_src += src_stride; - rem_rows -= 1; + + row_dst += dst_stride; + row_src += src_stride; } dma_ring_flush(r1); @@ -208,5 +183,3 @@ bool dma_queue_push_fallback_1d(dma_queue * q, dma_data ddata, size_t dst_stride return dma_ring_push_single_1d(r0, ddata, /*size=*/ 0); } - -#endif diff --git a/ggml/src/ggml-hexagon/htp/dma-queue.h b/ggml/src/ggml-hexagon/htp/dma-queue.h index 05f37305d..678fc61d7 100644 --- a/ggml/src/ggml-hexagon/htp/dma-queue.h +++ b/ggml/src/ggml-hexagon/htp/dma-queue.h @@ -148,10 +148,7 @@ dma_queue_t dma_queue_alias_init(void * ptr, dma_queue_t main_q); void dma_queue_alias_free(dma_queue_t q); bool dma_queue_push_fallback_2d(dma_queue * q, dma_data ddata, size_t dst_stride, size_t src_stride, size_t row_size, size_t nrows); -bool dma_queue_push_fallback_contig(dma_queue * q, dma_data ddata, size_t total); -#if __HVX_ARCH__ < 75 bool dma_queue_push_fallback_1d(dma_queue * q, dma_data ddata, size_t dst_stride, size_t src_stride, size_t row_size, size_t nrows); -#endif // TODO: technically we don't need these and could use Q6_dmstart/wait/etc instead // but those do not seem to always compiler properly. @@ -400,7 +397,7 @@ static inline bool dma_queue_push(dma_queue * q, dma_data ddata, size_t dst_stri if (__builtin_expect(total <= DMA_MAX_SIZE_24B, 1)) { return dma_ring_push_single_contig(q->ring0, ddata, total); } - return dma_queue_push_fallback_contig(q, ddata, total); + return dma_queue_push_fallback_1d(q, ddata, 0, 0, total, 1); } // 2. Hot path: Standard strided 2D (10-20% of calls) @@ -410,14 +407,10 @@ static inline bool dma_queue_push(dma_queue * q, dma_data ddata, size_t dst_stri } // 3. Cold path: Descriptor chunking fallbacks (< 0.1%) -#if __HVX_ARCH__ < 75 - if (row_size <= DMA_MAX_SIZE_16B && (src_stride | dst_stride) <= DMA_MAX_STRIDE_16B) { + if ((row_size | src_stride | dst_stride) <= DMA_MAX_2D_ROW_SIZE) { return dma_queue_push_fallback_2d(q, ddata, dst_stride, src_stride, row_size, nrows); } return dma_queue_push_fallback_1d(q, ddata, dst_stride, src_stride, row_size, nrows); -#else - return dma_queue_push_fallback_2d(q, ddata, dst_stride, src_stride, row_size, nrows); -#endif } static inline void dma_sync_read(dma_queue * dma_q, void * dst, dma_addr_t src, size_t bytes) { diff --git a/tests/test-backend-ops.cpp b/tests/test-backend-ops.cpp index f75ff11b9..e4ebd7c5f 100644 --- a/tests/test-backend-ops.cpp +++ b/tests/test-backend-ops.cpp @@ -10831,6 +10831,10 @@ static std::vector> make_test_cases_eval() { } } + // source strides over 16 MB (24-bit DMA descriptor limit) + test_cases.emplace_back(new test_cont(GGML_TYPE_F32, {512, 8193, 2, 1}, false, {2, 1, 0, 3})); + test_cases.emplace_back(new test_cont(GGML_TYPE_F32, {256, 16385, 2, 1}, false, {0, 2, 1, 3})); + auto add_test_bin_bcast = [&](ggml_type type, std::array ne, std::array nr, bool perm1 = false, bool src_overlap = false) { for (auto op : {ggml_add, ggml_sub, ggml_mul, ggml_div}) { test_cases.emplace_back(new test_bin_bcast(op, type, ne, nr, 1, perm1, src_overlap));