Enable DT topology-driven automatic CMA region allocation#1050
Enable DT topology-driven automatic CMA region allocation#1050bisingha-xilinx wants to merge 5 commits intoamd:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements automatic CMA (Contiguous Memory Allocator) region selection for AIE (AI Engine) hardware contexts based on device tree topology. The driver now automatically maps AIE column ranges to dedicated CMA memory regions, eliminating the need for manual memory index management by users. Users only need to specify start_col when creating hardware contexts, and the driver handles the appropriate CMA region selection based on the parsed device tree topology.
Changes:
- Added device tree parsing to extract AIE memory topology mapping (column ranges to CMA regions)
- Implemented automatic memory index selection based on allocated start column from XRS
- Modified BO allocation to inject the auto-selected memory index into buffer flags
- Updated HSA queue allocation/deallocation to use the correct CMA device based on memory index
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/shim_ve2/xdna_hwctx.h | Added m_mem_index member and query_mem_index() method declaration |
| src/shim_ve2/xdna_hwctx.cpp | Implemented query_mem_index() to retrieve auto-selected memory index from driver and inject it into BO flags |
| src/include/uapi/drm_local/amdxdna_accel.h | Added DRM_AMDXDNA_HWCTX_MEM_INDEX parameter for querying hardware context memory index |
| src/driver/amdxdna/ve2_of.h | Added memory topology structures and function declarations |
| src/driver/amdxdna/ve2_of.c | Implemented device tree parsing and automatic memory index selection logic |
| src/driver/amdxdna/ve2_hwctx.c | Reordered initialization to request XRS before HSA queue creation and use correct CMA device |
| src/driver/amdxdna/ve2_debug.c | Added ioctl handler to return memory index to userspace |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e32e09d to
a5d0846
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/shim_ve2/xdna_hwctx.cpp
Outdated
| // This ensures BOs are allocated from the correct CMA region | ||
| xcl_bo_flags xflags{flags}; | ||
| if (m_mem_index < MAX_MEM_REGIONS) | ||
| xflags.bank = m_mem_index & 0xFF; // Lower 8 bits |
There was a problem hiding this comment.
if (xfflags.use > 0) // this is internal
{ xflags.bank = m_mem_index & 0xFF;
}
else
{
if (xflags.bank & m_mem_index)
// bank passed by user is valid
else
throw;
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/driver/amdxdna/ve2_of.c
Outdated
| int num_phandles; | ||
| int ret; | ||
| u32 i; | ||
| u32 k; |
There was a problem hiding this comment.
The variable name 'k' is ambiguous. Consider renaming it to 'phandle_idx' or 'mem_region_idx' to clarify that it indexes memory-region phandles.
| ret = ve2_xrs_request(xdna, hwctx); | ||
| if (ret) { | ||
| XDNA_ERR(xdna, "Failed to create host queue, ret=%d", ret); | ||
| goto free_priv; | ||
| XDNA_ERR(xdna, "XRS resource request failed, ret=%d", ret); | ||
| goto cleanup_priv; | ||
| } | ||
|
|
||
| ret = ve2_xrs_request(xdna, hwctx); | ||
| /* Auto-select mem_index based on ACTUAL allocated start_col from XRS */ | ||
| ve2_auto_select_mem_index(xdna, hwctx); |
There was a problem hiding this comment.
The automatic mem_index selection logic introduced by ve2_auto_select_mem_index() lacks test coverage. Consider adding tests to verify correct mem_index assignment for various start_col values and topology configurations.
| if (!topo_np) { | ||
| XDNA_DBG(xdna, "No aie_mem_topology node found, using default CMA"); | ||
| xdna_hdl->mem_topology.num_regions = 0; | ||
| return -ENOENT; |
There was a problem hiding this comment.
Returning -ENOENT when topology node is not found, but the function continues execution as if this is a success case. Consider returning 0 or a distinct code to indicate optional feature unavailability vs. actual errors.
| return -ENOENT; | |
| return 0; |
ed85ca6 to
76f55db
Compare
Signed-off-by: Bikash Singha <bisingha@xcobisingha50x.amd.com>
Signed-off-by: Bikash Singha <bisingha@xcobisingha50x.amd.com>
Signed-off-by: Bikash Singha <bisingha@xcobisingha50x.amd.com>
Signed-off-by: Bikash Singha <bisingha@xcobisingha50x.amd.com>
Signed-off-by: Bikash Singha <bisingha@xcobisingha50x.amd.com>
76f55db to
b7ba13c
Compare
Implement automatic CMA memory region selection based on device tree AIE topology and start_col. Users now only need to specify start_col when creating a hardware context - the driver automatically selects the appropriate CMA region based on DT topology mapping. This eliminates manual mem_index management and enables seamless multi-partition AIE workloads across dedicated memory regions.
Tested:
Expected information in the DTB (eg.):
`
`