[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e7b9b328-8fa9-dac9-20a9-f9530f3195ac@amd.com>
Date: Thu, 8 Jan 2026 09:51:52 -0800
From: Lizhi Hou <lizhi.hou@....com>
To: "Mario Limonciello (AMD) (kernel.org)" <superm1@...nel.org>,
<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
Applied to drm-misc-next.
On 1/7/26 19:26, Mario Limonciello (AMD) (kernel.org) wrote:
>
>
> On 1/7/2026 4:20 PM, Lizhi Hou wrote:
>>
>> 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.
>>
> Thank for confirming, no other concerns with this patch.
>
> Reviewed-by: Mario Limonciello (AMD) <superm1@...nel.org>
>
>>
>> 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