[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e96f3ed6-fed5-649a-d953-eed3dac7d82a@amd.com>
Date: Wed, 7 Jan 2026 14:20:10 -0800
From: Lizhi Hou <lizhi.hou@....com>
To: Mario Limonciello <mario.limonciello@....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 1/7/26 14:07, Mario Limonciello wrote:
> On 1/7/26 4:05 PM, Lizhi Hou wrote:
>>
>> On 1/7/26 13:19, Mario Limonciello wrote:
>>> 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?
>>
>> The old firmware runs fine this these changes. The new firmware adds
>> more alignment and size requirements which the old one does not need.
>
> Ah OK - so patch 2 won't reject current firmware right?
Correct. It will not reject current firmware.
Lizhi
>
>>
>>
>> Lizhi
>>
>>>
>>> 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