ve2: updating buffer allocation logic to support cma allocation from multiple regions#678
ve2: updating buffer allocation logic to support cma allocation from multiple regions#678bisingha-xilinx wants to merge 4 commits intoamd:mainfrom
Conversation
| dma = drm_gem_dma_create(dev, size); | ||
| if (IS_ERR(dma)) | ||
| return ERR_CAST(dma); | ||
| vaddr = dma_alloc_coherent(dev->dev, size, &dma_addr, GFP_KERNEL); |
There was a problem hiding this comment.
gem_dma bo was used which alloc non-coherent or wc buffer. Why do you need coherent buffer here? Non-coherent is preferred if possible.
|
CI failed due to checkpatch warnings. |
src/driver/amdxdna/amdxdna_gem_of.c
Outdated
|
|
||
| gem_obj->funcs = &amdxdna_gem_dma_funcs; | ||
|
|
||
| //manually init the drm gem obj |
There was a problem hiding this comment.
Hi @houlz0507 thank you for the review. Sure, I'll update it.
| dma_addr_t dma_addr; | ||
| void *vaddr; | ||
|
|
||
| /* Round up to more than 4K to ensure to allocate memory from CMA always */ |
There was a problem hiding this comment.
Can you briefly explain why the CMA allocation can be determined by size?
There was a problem hiding this comment.
Hi @houlz0507
We observed one issue, for smaller buffer sizes, Linux allocates randomly CMA memory from some other region not from the dedicated CMA memory region. And rounding up the smaller buffer sizes to page size, guarantees to get CMA memory from the dedicated CMA memory region.
It creates an issue for some tests. In some usecases kernels do not have access/connectivity to all memory regions but to only CMA memory region. If Linux allocates CMA memory from some other region, then the test fails.
There was a problem hiding this comment.
MM... how do you guarantee >=8K memory will be from CMA region in all circumstance?
There was a problem hiding this comment.
Hi,
This is the lowest size I tested continuously multiple times having 1000/2000 iteration in each test. I didnt observe the issue. If I just round up to 4K (for smaller buffer sizes) then I saw the mentioned issue.
So, we aggreed to round up to 2*page_size for smaller sizes and round up with page_size for bigger sizes.
There was a problem hiding this comment.
What if the CMA region is used up? I think alloc_coherent will try to allocate memory from other places instead of returning an error. I did not see a way to make sure the memory is from CMA regions by using alloc_coherent.
There was a problem hiding this comment.
Hi @houlz0507
if the CMA region is used up, alloc_coherent is not allocating from other places. it throws error. In my test I've observed this behavior. Let me confirm again.
There was a problem hiding this comment.
Please try small buffer e.g. 8k for your experiment.
There was a problem hiding this comment.
Hi @houlz0507 thank you for the review.
Sure.
Signed-off-by: Bikash Singha <bisingha@xcobisingha40x.xlnx.xilinx.com>
Signed-off-by: Bikash Singha <bisingha@xcobisingha40x.xlnx.xilinx.com>
Signed-off-by: Bikash Singha <bisingha@xcobisingha40x.xlnx.xilinx.com>
Signed-off-by: Bikash Singha <bisingha@xcobisingha40x.xlnx.xilinx.com>
bisingha-xilinx
force-pushed
the
xdna-cma-exp
branch
from
af387d8 to
9176687
Compare
|
Hi @bisingha-xilinx , Lets send a separate PR once you implement and verify memory region logic. Closing this PR. |
Sure Vamshi |
4 participants
EDF/Vitis Common CED contains multiple DDR (or LPDDR) memory banks, this pr is to support DMA access to any of the memory banks. This pr has the initial changes to support this feature.
Updated the logic to allocate buffers with dma_alloc_coherent() instead of directly using drm_gem_dma_create(). This update is required to support https://jira.xilinx.com/browse/VITIS-15145