Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions src/driver/amdxdna/amdxdna_cma_buf.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#include "amdxdna_cma_buf.h"

struct amdxdna_cmabuf_priv {
struct drm_device *dev;
struct device *dev;
dma_addr_t dma_addr;
void *cpu_addr;
size_t size;
Expand Down Expand Up @@ -76,7 +76,7 @@ static void amdxdna_cmabuf_release(struct dma_buf *dbuf)
if (!cmabuf)
return;

dma_free_coherent(cmabuf->dev->dev, cmabuf->size,
dma_free_coherent(cmabuf->dev, cmabuf->size,
cmabuf->cpu_addr, cmabuf->dma_addr);
kfree(cmabuf);
dbuf->priv = NULL;
Expand All @@ -98,7 +98,7 @@ static int amdxdna_cmabuf_mmap(struct dma_buf *dbuf, struct vm_area_struct *vma)

vm_flags_set(vma, VM_IO | VM_DONTEXPAND | VM_DONTDUMP);

ret = dma_mmap_coherent(cmabuf->dev->dev, vma,
ret = dma_mmap_coherent(cmabuf->dev, vma,
cmabuf->cpu_addr,
cmabuf->dma_addr,
cmabuf->size);
Expand All @@ -125,7 +125,7 @@ static const struct dma_buf_ops amdxdna_cmabuf_dmabuf_ops = {
.vmap = amdxdna_cmabuf_vmap,
};

struct dma_buf *amdxdna_get_cma_buf(struct drm_device *dev,
struct dma_buf *amdxdna_get_cma_buf(struct device *dev,
size_t size)
{
struct amdxdna_cmabuf_priv *cmabuf;
Expand All @@ -141,7 +141,7 @@ struct dma_buf *amdxdna_get_cma_buf(struct drm_device *dev,

size = PAGE_ALIGN(size);

cpu_addr = dma_alloc_coherent(dev->dev, size, &dma_addr, GFP_KERNEL);
cpu_addr = dma_alloc_coherent(dev, size, &dma_addr, GFP_KERNEL);
if (!cpu_addr) {
ret = -ENOMEM;
goto free_cmabuf;
Expand All @@ -166,7 +166,7 @@ struct dma_buf *amdxdna_get_cma_buf(struct drm_device *dev,
return dbuf;

free_dma:
dma_free_coherent(dev->dev, size, cpu_addr, dma_addr);
dma_free_coherent(dev, size, cpu_addr, dma_addr);
free_cmabuf:
kfree(cmabuf);
return ERR_PTR(ret);
Expand All @@ -180,3 +180,13 @@ bool amdxdna_use_cma(void)
return false;
#endif
}

int get_cma_mem_index(u64 flags)
{
/*
* Extract lower 8 bits for memory index (0-255 range).
* Valid indexes: 0-15 (validated at call site against MAX_MEM_REGIONS)
* Invalid indexes (>=16): handled as fallback to default CMA
*/
return flags & 0xFF;
}
3 changes: 2 additions & 1 deletion src/driver/amdxdna/amdxdna_cma_buf.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <drm/drm_device.h>

bool amdxdna_use_cma(void);
struct dma_buf *amdxdna_get_cma_buf(struct drm_device *dev, size_t size);
int get_cma_mem_index(u64 flags);
struct dma_buf *amdxdna_get_cma_buf(struct device *dev, size_t size);

#endif /* _AMDXDNA_CMA_BUF_H */
15 changes: 13 additions & 2 deletions src/driver/amdxdna/amdxdna_drm.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@
#include "amdxdna_dpt.h"
#include "amdxdna_gem.h"

#define MAX_MEM_REGIONS 16

#define XDNA_INFO(xdna, fmt, args...) dev_info((xdna)->ddev.dev, fmt, ##args)
#define XDNA_WARN(xdna, fmt, args...) dev_warn((xdna)->ddev.dev, "%s: "fmt, __func__, ##args)
#define XDNA_WARN(xdna, fmt, args...) \
dev_warn((xdna)->ddev.dev, "%s: " fmt, __func__, ##args)
#define XDNA_ERR(xdna, fmt, args...) \
dev_err_ratelimited((xdna)->ddev.dev, "%s: "fmt, __func__, ##args)
dev_err_ratelimited((xdna)->ddev.dev, "%s: " fmt, __func__, ##args)
#define XDNA_DBG(xdna, fmt, args...) dev_dbg((xdna)->ddev.dev, fmt, ##args)

#define XDNA_INFO_ONCE(xdna, fmt, args...) dev_info_once((xdna)->ddev.dev, fmt, ##args)
Expand Down Expand Up @@ -116,6 +119,11 @@ struct amdxdna_fw_ver {
u32 build;
};

struct amdxdna_cma_mem_region {
struct device *dev;
bool initialized;
};

struct amdxdna_dev {
struct drm_device ddev;
struct amdxdna_dev_hdl *dev_handle;
Expand All @@ -132,6 +140,9 @@ struct amdxdna_dev {
#endif
struct rw_semaphore notifier_lock; /* for mmu notifier */
struct workqueue_struct *notifier_wq;

u32 num_cma_regions;
struct amdxdna_cma_mem_region cma_mem_regions[MAX_MEM_REGIONS];
};

struct amdxdna_stats {
Expand Down
46 changes: 41 additions & 5 deletions src/driver/amdxdna/amdxdna_gem.c
Original file line number Diff line number Diff line change
Expand Up @@ -762,23 +762,62 @@ amdxdna_gem_create_carvedout_object(struct drm_device *dev, struct amdxdna_drm_c
return to_xdna_obj(gobj);
}

static bool is_valid_cma_region(struct amdxdna_dev *xdna, int mem_index)
{
if (mem_index < 0 || mem_index >= MAX_MEM_REGIONS)
return false;

return xdna->cma_mem_regions[mem_index].initialized;
}

static struct amdxdna_gem_obj *
amdxdna_gem_create_cma_object(struct drm_device *dev, struct amdxdna_drm_create_bo *args)
{
struct amdxdna_dev *xdna = to_xdna_dev(dev);
size_t size = PAGE_ALIGN(args->size);
struct drm_gem_object *gobj;
struct dma_buf *dma_buf;
int mem_index;
int i;

if (args->type == AMDXDNA_BO_DEV_HEAP) {
XDNA_ERR(xdna, "Heap BO is not supported on CMA platform");
return NULL;
return ERR_PTR(-EINVAL);
}

mem_index = get_cma_mem_index(args->flags);

/* Try indexed allocation with mem_index */
if (is_valid_cma_region(xdna, mem_index)) {
dma_buf = amdxdna_get_cma_buf(xdna->cma_mem_regions[mem_index].dev,
size);
if (!IS_ERR(dma_buf))
goto import_buf;

XDNA_DBG(xdna, "CMA region %d failed, trying others\n",
mem_index);
} else {
XDNA_DBG(xdna, "Invalid mem_index %d, trying other regions\n",
mem_index);
}

dma_buf = amdxdna_get_cma_buf(dev, size);
/* Try any available initialized region */
for (i = 0; i < MAX_MEM_REGIONS; i++) {
if (i == mem_index || !is_valid_cma_region(xdna, i))
continue;

dma_buf = amdxdna_get_cma_buf(xdna->cma_mem_regions[i].dev,
size);
if (!IS_ERR(dma_buf))
goto import_buf;
}

/* Final fallback to system default CMA */
dma_buf = amdxdna_get_cma_buf(dev->dev, size);
if (IS_ERR(dma_buf))
return ERR_CAST(dma_buf);

import_buf:
gobj = dev->driver->gem_prime_import(dev, dma_buf);
if (IS_ERR(gobj)) {
dma_buf_put(dma_buf);
Expand Down Expand Up @@ -998,9 +1037,6 @@ int amdxdna_drm_create_bo_ioctl(struct drm_device *dev, void *data, struct drm_f
struct amdxdna_gem_obj *abo;
int ret;

if (args->flags)
return -EINVAL;

XDNA_DBG(xdna, "BO arg type %d va_tbl 0x%llx size 0x%llx flags 0x%llx",
args->type, args->vaddr, args->size, args->flags);
switch (args->type) {
Expand Down
98 changes: 97 additions & 1 deletion src/driver/amdxdna/ve2_of.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
#include <linux/device.h>
#include <linux/firmware.h>
#include <linux/xlnx-ai-engine.h>
#include <linux/firmware.h>
#include <linux/of_reserved_mem.h>
#include <linux/of_address.h>

#include "ve2_of.h"
#include "ve2_mgmt.h"
Expand Down Expand Up @@ -84,6 +85,94 @@ static int ve2_load_fw(struct amdxdna_dev_hdl *xdna_hdl)
return ret;
}

static void ve2_cma_device_release(struct device *dev)
{
/*
* All DMA and reserved memory resources are released by
* of_reserved_mem_device_release() before this function is called.
* This release function only needs to free the device structure itself.
*/
kfree(dev);
}

static void ve2_cma_mem_region_remove(struct amdxdna_dev *xdna)
{
int i;

for (i = 0; i < MAX_MEM_REGIONS; i++) {
struct amdxdna_cma_mem_region *region;

region = &xdna->cma_mem_regions[i];
if (region->initialized) {
of_reserved_mem_device_release(region->dev);
put_device(region->dev);
region->dev = NULL;
region->initialized = false;
}
}
}

static int
ve2_cma_mem_region_init(struct amdxdna_dev *xdna,
struct platform_device *pdev)
{
int num_regions;
int ret = 0;
int i;

xdna->num_cma_regions = 0;

num_regions = of_count_phandle_with_args(pdev->dev.of_node,
"memory-region", NULL);
if (num_regions <= 0)
return -EINVAL;

for (i = 0; i < num_regions && i < MAX_MEM_REGIONS; i++) {
struct device *child_dev;

child_dev = kzalloc(sizeof(*child_dev), GFP_KERNEL);
if (!child_dev) {
XDNA_ERR(xdna,
"Failed to alloc child_dev for cma region %d\n",
i);
ve2_cma_mem_region_remove(xdna);
return -ENOMEM;
}

device_initialize(child_dev);
child_dev->parent = &pdev->dev;
child_dev->of_node = pdev->dev.of_node;
child_dev->coherent_dma_mask = DMA_BIT_MASK(64);
child_dev->release = ve2_cma_device_release;

ret = dev_set_name(child_dev, "amdxdna-mem%d", i);
if (ret) {
XDNA_DBG(xdna,
"Failed to set name for cma region %d\n", i);
put_device(child_dev);
continue;
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

On dev_set_name() failure, the device is freed but ve2_cma_mem_region_remove() is not called to clean up any previously initialized regions before returning. This leaves partially initialized state. Consider calling ve2_cma_mem_region_remove(xdna) before continuing or ensure proper cleanup on any failure path.

Suggested change
continue;
ve2_cma_mem_region_remove(xdna);
return ret;

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The continue statement implements intentional fallback behavior. If one CMA region
fails to initialize, the driver tries remaining regions. The put_device() call
properly cleans up the failed region. Only if all regions fail the function
returns -EINVAL (line 170).

}

ret = of_reserved_mem_device_init_by_idx(child_dev,
pdev->dev.of_node, i);
if (ret) {
XDNA_DBG(xdna,
"Failed to init reserved cma region %d\n", i);
put_device(child_dev);
continue;
Comment on lines +156 to +162
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

Similar to the dev_set_name() failure case, when of_reserved_mem_device_init_by_idx() fails, the code continues without cleaning up previously initialized regions. If all regions fail to initialize after some succeed, those successful regions remain allocated until the final check at line 170. Consider more explicit cleanup or ensure the final check properly handles partial initialization.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Partial initialization is the intended behavior. Successfully initialized regions are
tracked in xdna->cma_mem_regions[] and used for buffer allocation with automatic
fallback. Only if zero regions initialized the function returns -EINVAL. This
design provides resilience - the system works even if some CMA regions are unavailable.
All regions are properly cleaned up during device removal via ve2_cma_mem_region_remove().

}

xdna->cma_mem_regions[i].dev = child_dev;
xdna->cma_mem_regions[i].initialized = true;
xdna->num_cma_regions++;
}

if (xdna->num_cma_regions == 0)
return -EINVAL;

return 0;
}

static int ve2_init(struct amdxdna_dev *xdna)
{
struct platform_device *pdev = to_platform_device(xdna->ddev.dev);
Expand Down Expand Up @@ -133,6 +222,12 @@ static int ve2_init(struct amdxdna_dev *xdna)
xdna->dev_handle->fw_slots[col] = fw_slots;
}

ret = ve2_cma_mem_region_init(xdna, pdev);
if (ret < 0) {
/* CMA region initialization is optional - system will fall back to default CMA */
XDNA_DBG(xdna, "Failed to initialize the cma memories\n");
}

return 0;
done:
ve2_free_firmware_slots(xdna_hdl, VE2_MAX_COL);
Expand All @@ -143,6 +238,7 @@ static int ve2_init(struct amdxdna_dev *xdna)
static void ve2_fini(struct amdxdna_dev *xdna)
{
ve2_free_firmware_slots(xdna->dev_handle, VE2_MAX_COL);
ve2_cma_mem_region_remove(xdna);
}

const struct amdxdna_dev_ops ve2_ops = {
Expand Down
4 changes: 3 additions & 1 deletion src/include/uapi/drm_local/amdxdna_accel.h
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,9 @@ struct amdxdna_drm_va_tbl {

/**
* struct amdxdna_drm_create_bo - Create a buffer object.
* @flags: Buffer flags. MBZ.
* @flags: Buffer flags.
* Bits [7:0] - CMA memory region index for allocation.
* Bits [63:8] - Reserved for other flags.
* @vaddr: Pointer of va address table.
* @size: Size in bytes.
* @type: Buffer type.
Expand Down
1 change: 1 addition & 0 deletions src/shim_ve2/xdna_bo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ xdna_bo::
alloc_bo()
{
amdxdna_drm_create_bo cbo = {
.flags = m_flags,
.size = m_aligned_size,
.type = m_type,
};
Expand Down