[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <812bcf91-d903-410f-91c8-8a71be3f3f02@amd.com>
Date: Wed, 7 Jan 2026 15:19:44 -0600
From: Mario Limonciello <mario.limonciello@....com>
To: Lizhi Hou <lizhi.hou@....com>, ogabbay@...nel.org,
quic_jhugo@...cinc.com, dri-devel@...ts.freedesktop.org,
maciej.falkowski@...ux.intel.com
Cc: linux-kernel@...r.kernel.org, max.zhen@....com, sonal.santan@....com
Subject: Re: [PATCH V1 1/2] accel/amdxdna: Update message DMA buffer
allocation
On 12/18/25 7:43 PM, Lizhi Hou wrote:
> The latest firmware requires the message DMA buffer to
> - have a minimum size of 8K
> - use a power-of-two size
> - be aligned to the buffer size
> - not cross 64M boundary
>
> Update the buffer allocation logic to meet these requirements and support
> the latest firmware.
We can't guarantee that kernel and firmware are moving at the same time.
What happens if you run old firmware with these changes?
If the old firmware can't run with these changes then it would be better
to instead add a fallback system.
IE:
1) kernel tries to load new firmware name and use new behavior
2) if firmware is missing, kernel tries to load old firmware name and
use old behavior.
3) if firmware is missing in old name then fail probe
>
> Signed-off-by: Lizhi Hou <lizhi.hou@....com>
> ---
> drivers/accel/amdxdna/aie2_error.c | 10 ++++-----
> drivers/accel/amdxdna/aie2_message.c | 33 ++++++++++++++++++++--------
> drivers/accel/amdxdna/aie2_pci.h | 5 +++++
> 3 files changed, 33 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/accel/amdxdna/aie2_error.c b/drivers/accel/amdxdna/aie2_error.c
> index d452008ec4f4..5e82df2b7cf6 100644
> --- a/drivers/accel/amdxdna/aie2_error.c
> +++ b/drivers/accel/amdxdna/aie2_error.c
> @@ -338,8 +338,7 @@ void aie2_error_async_events_free(struct amdxdna_dev_hdl *ndev)
> destroy_workqueue(events->wq);
> mutex_lock(&xdna->dev_lock);
>
> - dma_free_noncoherent(xdna->ddev.dev, events->size, events->buf,
> - events->addr, DMA_FROM_DEVICE);
> + aie2_free_msg_buffer(ndev, events->size, events->buf, events->addr);
> kfree(events);
> }
>
> @@ -355,8 +354,8 @@ int aie2_error_async_events_alloc(struct amdxdna_dev_hdl *ndev)
> if (!events)
> return -ENOMEM;
>
> - events->buf = dma_alloc_noncoherent(xdna->ddev.dev, total_size, &events->addr,
> - DMA_FROM_DEVICE, GFP_KERNEL);
> + events->buf = aie2_alloc_msg_buffer(ndev, &total_size, &events->addr);
> +
> if (!events->buf) {
> ret = -ENOMEM;
> goto free_events;
> @@ -396,8 +395,7 @@ int aie2_error_async_events_alloc(struct amdxdna_dev_hdl *ndev)
> free_wq:
> destroy_workqueue(events->wq);
> free_buf:
> - dma_free_noncoherent(xdna->ddev.dev, events->size, events->buf,
> - events->addr, DMA_FROM_DEVICE);
> + aie2_free_msg_buffer(ndev, events->size, events->buf, events->addr);
> free_events:
> kfree(events);
> return ret;
> diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c
> index 051f4ceaabae..99215328505e 100644
> --- a/drivers/accel/amdxdna/aie2_message.c
> +++ b/drivers/accel/amdxdna/aie2_message.c
> @@ -55,6 +55,22 @@ static int aie2_send_mgmt_msg_wait(struct amdxdna_dev_hdl *ndev,
> return ret;
> }
>
> +void *aie2_alloc_msg_buffer(struct amdxdna_dev_hdl *ndev, u32 *size,
> + dma_addr_t *dma_addr)
> +{
> + struct amdxdna_dev *xdna = ndev->xdna;
> + int order;
> +
> + *size = max(*size, SZ_8K);
> + order = get_order(*size);
> + if (order > MAX_PAGE_ORDER)
> + return NULL;
> + *size = PAGE_SIZE << order;
> +
> + return dma_alloc_noncoherent(xdna->ddev.dev, *size, dma_addr,
> + DMA_FROM_DEVICE, GFP_KERNEL);
> +}
> +
> int aie2_suspend_fw(struct amdxdna_dev_hdl *ndev)
> {
> DECLARE_AIE2_MSG(suspend, MSG_OP_SUSPEND);
> @@ -346,14 +362,13 @@ int aie2_query_status(struct amdxdna_dev_hdl *ndev, char __user *buf,
> {
> DECLARE_AIE2_MSG(aie_column_info, MSG_OP_QUERY_COL_STATUS);
> struct amdxdna_dev *xdna = ndev->xdna;
> + u32 buf_sz = size, aie_bitmap = 0;
> struct amdxdna_client *client;
> dma_addr_t dma_addr;
> - u32 aie_bitmap = 0;
> u8 *buff_addr;
> int ret;
>
> - buff_addr = dma_alloc_noncoherent(xdna->ddev.dev, size, &dma_addr,
> - DMA_FROM_DEVICE, GFP_KERNEL);
> + buff_addr = aie2_alloc_msg_buffer(ndev, &buf_sz, &dma_addr);
> if (!buff_addr)
> return -ENOMEM;
>
> @@ -363,7 +378,7 @@ int aie2_query_status(struct amdxdna_dev_hdl *ndev, char __user *buf,
>
> *cols_filled = 0;
> req.dump_buff_addr = dma_addr;
> - req.dump_buff_size = size;
> + req.dump_buff_size = buf_sz;
> req.num_cols = hweight32(aie_bitmap);
> req.aie_bitmap = aie_bitmap;
>
> @@ -391,7 +406,7 @@ int aie2_query_status(struct amdxdna_dev_hdl *ndev, char __user *buf,
> *cols_filled = aie_bitmap;
>
> fail:
> - dma_free_noncoherent(xdna->ddev.dev, size, buff_addr, dma_addr, DMA_FROM_DEVICE);
> + aie2_free_msg_buffer(ndev, buf_sz, buff_addr, dma_addr);
> return ret;
> }
>
> @@ -402,19 +417,19 @@ int aie2_query_telemetry(struct amdxdna_dev_hdl *ndev,
> DECLARE_AIE2_MSG(get_telemetry, MSG_OP_GET_TELEMETRY);
> struct amdxdna_dev *xdna = ndev->xdna;
> dma_addr_t dma_addr;
> + u32 buf_sz = size;
> u8 *addr;
> int ret;
>
> if (header->type >= MAX_TELEMETRY_TYPE)
> return -EINVAL;
>
> - addr = dma_alloc_noncoherent(xdna->ddev.dev, size, &dma_addr,
> - DMA_FROM_DEVICE, GFP_KERNEL);
> + addr = aie2_alloc_msg_buffer(ndev, &buf_sz, &dma_addr);
> if (!addr)
> return -ENOMEM;
>
> req.buf_addr = dma_addr;
> - req.buf_size = size;
> + req.buf_size = buf_sz;
> req.type = header->type;
>
> drm_clflush_virt_range(addr, size); /* device can access */
> @@ -440,7 +455,7 @@ int aie2_query_telemetry(struct amdxdna_dev_hdl *ndev,
> header->minor = resp.minor;
>
> free_buf:
> - dma_free_noncoherent(xdna->ddev.dev, size, addr, dma_addr, DMA_FROM_DEVICE);
> + aie2_free_msg_buffer(ndev, buf_sz, addr, dma_addr);
> return ret;
> }
>
> diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/amdxdna/aie2_pci.h
> index a929fa98a121..e1745f07b268 100644
> --- a/drivers/accel/amdxdna/aie2_pci.h
> +++ b/drivers/accel/amdxdna/aie2_pci.h
> @@ -336,6 +336,11 @@ int aie2_sync_bo(struct amdxdna_hwctx *hwctx, struct amdxdna_sched_job *job,
> int (*notify_cb)(void *, void __iomem *, size_t));
> int aie2_config_debug_bo(struct amdxdna_hwctx *hwctx, struct amdxdna_sched_job *job,
> int (*notify_cb)(void *, void __iomem *, size_t));
> +void *aie2_alloc_msg_buffer(struct amdxdna_dev_hdl *ndev, u32 *size,
> + dma_addr_t *dma_addr);
> +#define aie2_free_msg_buffer(ndev, size, buff_addr, dma_addr) \
> + dma_free_noncoherent((ndev)->xdna->ddev.dev, size, buff_addr, \
> + dma_addr, DMA_FROM_DEVICE)
>
> /* aie2_hwctx.c */
> int aie2_hwctx_init(struct amdxdna_hwctx *hwctx);
Powered by blists - more mailing lists