[PATCH V1 1/2] accel/amdxdna: Update message DMA buffer allocation

[PATCH V1 1/2] accel/amdxdna: Update message DMA buffer allocation

[Lizhi Hou]

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.

Signed-off-by: Lizhi Hou <lizhi.hou@amd.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);
-- 
2.34.1

[PATCH V1 2/2] accel/amdxdna: Update firmware version check for latest firmware

[Lizhi Hou]

The latest firmware increases the major version number. Update
aie2_check_protocol() to accept and support the new firmware version.

Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
---
 drivers/accel/amdxdna/aie2_pci.c  | 36 ++++++++-----------------------
 drivers/accel/amdxdna/aie2_pci.h  |  5 ++---
 drivers/accel/amdxdna/npu1_regs.c |  6 +++---
 drivers/accel/amdxdna/npu4_regs.c | 11 +++++-----
 drivers/accel/amdxdna/npu5_regs.c |  2 --
 drivers/accel/amdxdna/npu6_regs.c |  2 --
 6 files changed, 20 insertions(+), 42 deletions(-)

diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c
index 81a8e4137bfd..181fdbc10dae 100644
--- a/drivers/accel/amdxdna/aie2_pci.c
+++ b/drivers/accel/amdxdna/aie2_pci.c
@@ -56,41 +56,23 @@ struct mgmt_mbox_chann_info {
 static int aie2_check_protocol(struct amdxdna_dev_hdl *ndev, u32 fw_major, u32 fw_minor)
 {
 	const struct aie2_fw_feature_tbl *feature;
-	struct amdxdna_dev *xdna = ndev->xdna;
-
-	/*
-	 * The driver supported mailbox behavior is defined by
-	 * ndev->priv->protocol_major and protocol_minor.
-	 *
-	 * When protocol_major and fw_major are different, it means driver
-	 * and firmware are incompatible.
-	 */
-	if (ndev->priv->protocol_major != fw_major) {
-		XDNA_ERR(xdna, "Incompatible firmware protocol major %d minor %d",
-			 fw_major, fw_minor);
-		return -EINVAL;
-	}
+	bool found = false;
 
-	/*
-	 * When protocol_minor is greater then fw_minor, that means driver
-	 * relies on operation the installed firmware does not support.
-	 */
-	if (ndev->priv->protocol_minor > fw_minor) {
-		XDNA_ERR(xdna, "Firmware minor version smaller than supported");
-		return -EINVAL;
-	}
-
-	for (feature = ndev->priv->fw_feature_tbl; feature && feature->min_minor;
-	     feature++) {
+	for (feature = ndev->priv->fw_feature_tbl; feature->major; feature++) {
+		if (feature->major != fw_major)
+			continue;
 		if (fw_minor < feature->min_minor)
 			continue;
 		if (feature->max_minor > 0 && fw_minor > feature->max_minor)
 			continue;
 
-		set_bit(feature->feature, &ndev->feature_mask);
+		ndev->feature_mask |= feature->features;
+
+		/* firmware version matches one of the driver support entry */
+		found = true;
 	}
 
-	return 0;
+	return found ? 0 : -EOPNOTSUPP;
 }
 
 static void aie2_dump_chann_info_debug(struct amdxdna_dev_hdl *ndev)
diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/amdxdna/aie2_pci.h
index e1745f07b268..b20a3661078c 100644
--- a/drivers/accel/amdxdna/aie2_pci.h
+++ b/drivers/accel/amdxdna/aie2_pci.h
@@ -237,7 +237,8 @@ enum aie2_fw_feature {
 };
 
 struct aie2_fw_feature_tbl {
-	enum aie2_fw_feature feature;
+	u64 features;
+	u32 major;
 	u32 max_minor;
 	u32 min_minor;
 };
@@ -246,8 +247,6 @@ struct aie2_fw_feature_tbl {
 
 struct amdxdna_dev_priv {
 	const char			*fw_path;
-	u64				protocol_major;
-	u64				protocol_minor;
 	const struct rt_config		*rt_config;
 	const struct dpm_clk_freq	*dpm_clk_tbl;
 	const struct aie2_fw_feature_tbl *fw_feature_tbl;
diff --git a/drivers/accel/amdxdna/npu1_regs.c b/drivers/accel/amdxdna/npu1_regs.c
index ebc6e2802297..6f36a27b5a02 100644
--- a/drivers/accel/amdxdna/npu1_regs.c
+++ b/drivers/accel/amdxdna/npu1_regs.c
@@ -6,6 +6,7 @@
 #include <drm/amdxdna_accel.h>
 #include <drm/drm_device.h>
 #include <drm/gpu_scheduler.h>
+#include <linux/bits.h>
 #include <linux/sizes.h>
 
 #include "aie2_pci.h"
@@ -65,14 +66,13 @@ const struct dpm_clk_freq npu1_dpm_clk_table[] = {
 };
 
 static const struct aie2_fw_feature_tbl npu1_fw_feature_table[] = {
-	{ .feature = AIE2_NPU_COMMAND, .min_minor = 8 },
+	{ .major = 5, .min_minor = 7 },
+	{ .features = BIT_U64(AIE2_NPU_COMMAND), .min_minor = 8 },
 	{ 0 }
 };
 
 static const struct amdxdna_dev_priv npu1_dev_priv = {
 	.fw_path        = "amdnpu/1502_00/npu.sbin",
-	.protocol_major = 0x5,
-	.protocol_minor = 0x7,
 	.rt_config	= npu1_default_rt_cfg,
 	.dpm_clk_tbl	= npu1_dpm_clk_table,
 	.fw_feature_tbl = npu1_fw_feature_table,
diff --git a/drivers/accel/amdxdna/npu4_regs.c b/drivers/accel/amdxdna/npu4_regs.c
index a62234fd266d..a8d6f76dde5f 100644
--- a/drivers/accel/amdxdna/npu4_regs.c
+++ b/drivers/accel/amdxdna/npu4_regs.c
@@ -6,6 +6,7 @@
 #include <drm/amdxdna_accel.h>
 #include <drm/drm_device.h>
 #include <drm/gpu_scheduler.h>
+#include <linux/bits.h>
 #include <linux/sizes.h>
 
 #include "aie2_pci.h"
@@ -88,16 +89,16 @@ const struct dpm_clk_freq npu4_dpm_clk_table[] = {
 };
 
 const struct aie2_fw_feature_tbl npu4_fw_feature_table[] = {
-	{ .feature = AIE2_NPU_COMMAND, .min_minor = 15 },
-	{ .feature = AIE2_PREEMPT, .min_minor = 12 },
-	{ .feature = AIE2_TEMPORAL_ONLY, .min_minor = 12 },
+	{ .major = 6, .min_minor = 12 },
+	{ .features = BIT_U64(AIE2_NPU_COMMAND), .major = 6, .min_minor = 15 },
+	{ .features = BIT_U64(AIE2_PREEMPT), .major = 6, .min_minor = 12 },
+	{ .features = BIT_U64(AIE2_TEMPORAL_ONLY), .major = 6, .min_minor = 12 },
+	{ .features = GENMASK_ULL(AIE2_TEMPORAL_ONLY, AIE2_NPU_COMMAND), .major = 7 },
 	{ 0 }
 };
 
 static const struct amdxdna_dev_priv npu4_dev_priv = {
 	.fw_path        = "amdnpu/17f0_10/npu.sbin",
-	.protocol_major = 0x6,
-	.protocol_minor = 12,
 	.rt_config	= npu4_default_rt_cfg,
 	.dpm_clk_tbl	= npu4_dpm_clk_table,
 	.fw_feature_tbl = npu4_fw_feature_table,
diff --git a/drivers/accel/amdxdna/npu5_regs.c b/drivers/accel/amdxdna/npu5_regs.c
index 131080652ef0..c0a35cfd886c 100644
--- a/drivers/accel/amdxdna/npu5_regs.c
+++ b/drivers/accel/amdxdna/npu5_regs.c
@@ -64,8 +64,6 @@
 
 static const struct amdxdna_dev_priv npu5_dev_priv = {
 	.fw_path        = "amdnpu/17f0_11/npu.sbin",
-	.protocol_major = 0x6,
-	.protocol_minor = 12,
 	.rt_config	= npu4_default_rt_cfg,
 	.dpm_clk_tbl	= npu4_dpm_clk_table,
 	.fw_feature_tbl = npu4_fw_feature_table,
diff --git a/drivers/accel/amdxdna/npu6_regs.c b/drivers/accel/amdxdna/npu6_regs.c
index 1f71285655b2..1fb07df99186 100644
--- a/drivers/accel/amdxdna/npu6_regs.c
+++ b/drivers/accel/amdxdna/npu6_regs.c
@@ -64,8 +64,6 @@
 
 static const struct amdxdna_dev_priv npu6_dev_priv = {
 	.fw_path        = "amdnpu/17f0_10/npu.sbin",
-	.protocol_major = 0x6,
-	.protocol_minor = 12,
 	.rt_config	= npu4_default_rt_cfg,
 	.dpm_clk_tbl	= npu4_dpm_clk_table,
 	.fw_feature_tbl = npu4_fw_feature_table,
-- 
2.34.1

Re: [PATCH V1 1/2] accel/amdxdna: Update message DMA buffer allocation

[Mario Limonciello]

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@amd.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);

Re: [PATCH V1 2/2] accel/amdxdna: Update firmware version check for latest firmware

[Mario Limonciello]

On 12/18/25 7:43 PM, Lizhi Hou wrote:
> The latest firmware increases the major version number. Update
> aie2_check_protocol() to accept and support the new firmware version.
> 
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>

I know it's painful and tech debt; but I suggest you add new paths to 
handle both versions of firmware at least until the next LTS kernel.

> ---
>   drivers/accel/amdxdna/aie2_pci.c  | 36 ++++++++-----------------------
>   drivers/accel/amdxdna/aie2_pci.h  |  5 ++---
>   drivers/accel/amdxdna/npu1_regs.c |  6 +++---
>   drivers/accel/amdxdna/npu4_regs.c | 11 +++++-----
>   drivers/accel/amdxdna/npu5_regs.c |  2 --
>   drivers/accel/amdxdna/npu6_regs.c |  2 --
>   6 files changed, 20 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c
> index 81a8e4137bfd..181fdbc10dae 100644
> --- a/drivers/accel/amdxdna/aie2_pci.c
> +++ b/drivers/accel/amdxdna/aie2_pci.c
> @@ -56,41 +56,23 @@ struct mgmt_mbox_chann_info {
>   static int aie2_check_protocol(struct amdxdna_dev_hdl *ndev, u32 fw_major, u32 fw_minor)
>   {
>   	const struct aie2_fw_feature_tbl *feature;
> -	struct amdxdna_dev *xdna = ndev->xdna;
> -
> -	/*
> -	 * The driver supported mailbox behavior is defined by
> -	 * ndev->priv->protocol_major and protocol_minor.
> -	 *
> -	 * When protocol_major and fw_major are different, it means driver
> -	 * and firmware are incompatible.
> -	 */
> -	if (ndev->priv->protocol_major != fw_major) {
> -		XDNA_ERR(xdna, "Incompatible firmware protocol major %d minor %d",
> -			 fw_major, fw_minor);
> -		return -EINVAL;
> -	}
> +	bool found = false;
>   
> -	/*
> -	 * When protocol_minor is greater then fw_minor, that means driver
> -	 * relies on operation the installed firmware does not support.
> -	 */
> -	if (ndev->priv->protocol_minor > fw_minor) {
> -		XDNA_ERR(xdna, "Firmware minor version smaller than supported");
> -		return -EINVAL;
> -	}
> -
> -	for (feature = ndev->priv->fw_feature_tbl; feature && feature->min_minor;
> -	     feature++) {
> +	for (feature = ndev->priv->fw_feature_tbl; feature->major; feature++) {
> +		if (feature->major != fw_major)
> +			continue;
>   		if (fw_minor < feature->min_minor)
>   			continue;
>   		if (feature->max_minor > 0 && fw_minor > feature->max_minor)
>   			continue;
>   
> -		set_bit(feature->feature, &ndev->feature_mask);
> +		ndev->feature_mask |= feature->features;
> +
> +		/* firmware version matches one of the driver support entry */
> +		found = true;
>   	}
>   
> -	return 0;
> +	return found ? 0 : -EOPNOTSUPP;
>   }
>   
>   static void aie2_dump_chann_info_debug(struct amdxdna_dev_hdl *ndev)
> diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/amdxdna/aie2_pci.h
> index e1745f07b268..b20a3661078c 100644
> --- a/drivers/accel/amdxdna/aie2_pci.h
> +++ b/drivers/accel/amdxdna/aie2_pci.h
> @@ -237,7 +237,8 @@ enum aie2_fw_feature {
>   };
>   
>   struct aie2_fw_feature_tbl {
> -	enum aie2_fw_feature feature;
> +	u64 features;
> +	u32 major;
>   	u32 max_minor;
>   	u32 min_minor;
>   };
> @@ -246,8 +247,6 @@ struct aie2_fw_feature_tbl {
>   
>   struct amdxdna_dev_priv {
>   	const char			*fw_path;
> -	u64				protocol_major;
> -	u64				protocol_minor;
>   	const struct rt_config		*rt_config;
>   	const struct dpm_clk_freq	*dpm_clk_tbl;
>   	const struct aie2_fw_feature_tbl *fw_feature_tbl;
> diff --git a/drivers/accel/amdxdna/npu1_regs.c b/drivers/accel/amdxdna/npu1_regs.c
> index ebc6e2802297..6f36a27b5a02 100644
> --- a/drivers/accel/amdxdna/npu1_regs.c
> +++ b/drivers/accel/amdxdna/npu1_regs.c
> @@ -6,6 +6,7 @@
>   #include <drm/amdxdna_accel.h>
>   #include <drm/drm_device.h>
>   #include <drm/gpu_scheduler.h>
> +#include <linux/bits.h>
>   #include <linux/sizes.h>
>   
>   #include "aie2_pci.h"
> @@ -65,14 +66,13 @@ const struct dpm_clk_freq npu1_dpm_clk_table[] = {
>   };
>   
>   static const struct aie2_fw_feature_tbl npu1_fw_feature_table[] = {
> -	{ .feature = AIE2_NPU_COMMAND, .min_minor = 8 },
> +	{ .major = 5, .min_minor = 7 },
> +	{ .features = BIT_U64(AIE2_NPU_COMMAND), .min_minor = 8 },
>   	{ 0 }
>   };
>   
>   static const struct amdxdna_dev_priv npu1_dev_priv = {
>   	.fw_path        = "amdnpu/1502_00/npu.sbin",
> -	.protocol_major = 0x5,
> -	.protocol_minor = 0x7,
>   	.rt_config	= npu1_default_rt_cfg,
>   	.dpm_clk_tbl	= npu1_dpm_clk_table,
>   	.fw_feature_tbl = npu1_fw_feature_table,
> diff --git a/drivers/accel/amdxdna/npu4_regs.c b/drivers/accel/amdxdna/npu4_regs.c
> index a62234fd266d..a8d6f76dde5f 100644
> --- a/drivers/accel/amdxdna/npu4_regs.c
> +++ b/drivers/accel/amdxdna/npu4_regs.c
> @@ -6,6 +6,7 @@
>   #include <drm/amdxdna_accel.h>
>   #include <drm/drm_device.h>
>   #include <drm/gpu_scheduler.h>
> +#include <linux/bits.h>
>   #include <linux/sizes.h>
>   
>   #include "aie2_pci.h"
> @@ -88,16 +89,16 @@ const struct dpm_clk_freq npu4_dpm_clk_table[] = {
>   };
>   
>   const struct aie2_fw_feature_tbl npu4_fw_feature_table[] = {
> -	{ .feature = AIE2_NPU_COMMAND, .min_minor = 15 },
> -	{ .feature = AIE2_PREEMPT, .min_minor = 12 },
> -	{ .feature = AIE2_TEMPORAL_ONLY, .min_minor = 12 },
> +	{ .major = 6, .min_minor = 12 },
> +	{ .features = BIT_U64(AIE2_NPU_COMMAND), .major = 6, .min_minor = 15 },
> +	{ .features = BIT_U64(AIE2_PREEMPT), .major = 6, .min_minor = 12 },
> +	{ .features = BIT_U64(AIE2_TEMPORAL_ONLY), .major = 6, .min_minor = 12 },
> +	{ .features = GENMASK_ULL(AIE2_TEMPORAL_ONLY, AIE2_NPU_COMMAND), .major = 7 },
>   	{ 0 }
>   };
>   
>   static const struct amdxdna_dev_priv npu4_dev_priv = {
>   	.fw_path        = "amdnpu/17f0_10/npu.sbin",
> -	.protocol_major = 0x6,
> -	.protocol_minor = 12,
>   	.rt_config	= npu4_default_rt_cfg,
>   	.dpm_clk_tbl	= npu4_dpm_clk_table,
>   	.fw_feature_tbl = npu4_fw_feature_table,
> diff --git a/drivers/accel/amdxdna/npu5_regs.c b/drivers/accel/amdxdna/npu5_regs.c
> index 131080652ef0..c0a35cfd886c 100644
> --- a/drivers/accel/amdxdna/npu5_regs.c
> +++ b/drivers/accel/amdxdna/npu5_regs.c
> @@ -64,8 +64,6 @@
>   
>   static const struct amdxdna_dev_priv npu5_dev_priv = {
>   	.fw_path        = "amdnpu/17f0_11/npu.sbin",
> -	.protocol_major = 0x6,
> -	.protocol_minor = 12,
>   	.rt_config	= npu4_default_rt_cfg,
>   	.dpm_clk_tbl	= npu4_dpm_clk_table,
>   	.fw_feature_tbl = npu4_fw_feature_table,
> diff --git a/drivers/accel/amdxdna/npu6_regs.c b/drivers/accel/amdxdna/npu6_regs.c
> index 1f71285655b2..1fb07df99186 100644
> --- a/drivers/accel/amdxdna/npu6_regs.c
> +++ b/drivers/accel/amdxdna/npu6_regs.c
> @@ -64,8 +64,6 @@
>   
>   static const struct amdxdna_dev_priv npu6_dev_priv = {
>   	.fw_path        = "amdnpu/17f0_10/npu.sbin",
> -	.protocol_major = 0x6,
> -	.protocol_minor = 12,
>   	.rt_config	= npu4_default_rt_cfg,
>   	.dpm_clk_tbl	= npu4_dpm_clk_table,
>   	.fw_feature_tbl = npu4_fw_feature_table,

Re: [PATCH V1 1/2] accel/amdxdna: Update message DMA buffer allocation

[Lizhi Hou]

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.


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@amd.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);
>

Re: [PATCH V1 1/2] accel/amdxdna: Update message DMA buffer allocation

[Mario Limonciello]

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?

> 
> 
> 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@amd.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);
>>

Re: [PATCH V1 2/2] accel/amdxdna: Update firmware version check for latest firmware

[Lizhi Hou]

On 1/7/26 13:20, Mario Limonciello wrote:
> On 12/18/25 7:43 PM, Lizhi Hou wrote:
>> The latest firmware increases the major version number. Update
>> aie2_check_protocol() to accept and support the new firmware version.
>>
>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
>
> I know it's painful and tech debt; but I suggest you add new paths to 
> handle both versions of firmware at least until the next LTS kernel.

With the patch 1, the driver works fine for both current firmware and 
upcoming firmware.

Lizhi

>
>> ---
>>   drivers/accel/amdxdna/aie2_pci.c  | 36 ++++++++-----------------------
>>   drivers/accel/amdxdna/aie2_pci.h  |  5 ++---
>>   drivers/accel/amdxdna/npu1_regs.c |  6 +++---
>>   drivers/accel/amdxdna/npu4_regs.c | 11 +++++-----
>>   drivers/accel/amdxdna/npu5_regs.c |  2 --
>>   drivers/accel/amdxdna/npu6_regs.c |  2 --
>>   6 files changed, 20 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/accel/amdxdna/aie2_pci.c 
>> b/drivers/accel/amdxdna/aie2_pci.c
>> index 81a8e4137bfd..181fdbc10dae 100644
>> --- a/drivers/accel/amdxdna/aie2_pci.c
>> +++ b/drivers/accel/amdxdna/aie2_pci.c
>> @@ -56,41 +56,23 @@ struct mgmt_mbox_chann_info {
>>   static int aie2_check_protocol(struct amdxdna_dev_hdl *ndev, u32 
>> fw_major, u32 fw_minor)
>>   {
>>       const struct aie2_fw_feature_tbl *feature;
>> -    struct amdxdna_dev *xdna = ndev->xdna;
>> -
>> -    /*
>> -     * The driver supported mailbox behavior is defined by
>> -     * ndev->priv->protocol_major and protocol_minor.
>> -     *
>> -     * When protocol_major and fw_major are different, it means driver
>> -     * and firmware are incompatible.
>> -     */
>> -    if (ndev->priv->protocol_major != fw_major) {
>> -        XDNA_ERR(xdna, "Incompatible firmware protocol major %d 
>> minor %d",
>> -             fw_major, fw_minor);
>> -        return -EINVAL;
>> -    }
>> +    bool found = false;
>>   -    /*
>> -     * When protocol_minor is greater then fw_minor, that means driver
>> -     * relies on operation the installed firmware does not support.
>> -     */
>> -    if (ndev->priv->protocol_minor > fw_minor) {
>> -        XDNA_ERR(xdna, "Firmware minor version smaller than 
>> supported");
>> -        return -EINVAL;
>> -    }
>> -
>> -    for (feature = ndev->priv->fw_feature_tbl; feature && 
>> feature->min_minor;
>> -         feature++) {
>> +    for (feature = ndev->priv->fw_feature_tbl; feature->major; 
>> feature++) {
>> +        if (feature->major != fw_major)
>> +            continue;
>>           if (fw_minor < feature->min_minor)
>>               continue;
>>           if (feature->max_minor > 0 && fw_minor > feature->max_minor)
>>               continue;
>>   -        set_bit(feature->feature, &ndev->feature_mask);
>> +        ndev->feature_mask |= feature->features;
>> +
>> +        /* firmware version matches one of the driver support entry */
>> +        found = true;
>>       }
>>   -    return 0;
>> +    return found ? 0 : -EOPNOTSUPP;
>>   }
>>     static void aie2_dump_chann_info_debug(struct amdxdna_dev_hdl *ndev)
>> diff --git a/drivers/accel/amdxdna/aie2_pci.h 
>> b/drivers/accel/amdxdna/aie2_pci.h
>> index e1745f07b268..b20a3661078c 100644
>> --- a/drivers/accel/amdxdna/aie2_pci.h
>> +++ b/drivers/accel/amdxdna/aie2_pci.h
>> @@ -237,7 +237,8 @@ enum aie2_fw_feature {
>>   };
>>     struct aie2_fw_feature_tbl {
>> -    enum aie2_fw_feature feature;
>> +    u64 features;
>> +    u32 major;
>>       u32 max_minor;
>>       u32 min_minor;
>>   };
>> @@ -246,8 +247,6 @@ struct aie2_fw_feature_tbl {
>>     struct amdxdna_dev_priv {
>>       const char            *fw_path;
>> -    u64                protocol_major;
>> -    u64                protocol_minor;
>>       const struct rt_config        *rt_config;
>>       const struct dpm_clk_freq    *dpm_clk_tbl;
>>       const struct aie2_fw_feature_tbl *fw_feature_tbl;
>> diff --git a/drivers/accel/amdxdna/npu1_regs.c 
>> b/drivers/accel/amdxdna/npu1_regs.c
>> index ebc6e2802297..6f36a27b5a02 100644
>> --- a/drivers/accel/amdxdna/npu1_regs.c
>> +++ b/drivers/accel/amdxdna/npu1_regs.c
>> @@ -6,6 +6,7 @@
>>   #include <drm/amdxdna_accel.h>
>>   #include <drm/drm_device.h>
>>   #include <drm/gpu_scheduler.h>
>> +#include <linux/bits.h>
>>   #include <linux/sizes.h>
>>     #include "aie2_pci.h"
>> @@ -65,14 +66,13 @@ const struct dpm_clk_freq npu1_dpm_clk_table[] = {
>>   };
>>     static const struct aie2_fw_feature_tbl npu1_fw_feature_table[] = {
>> -    { .feature = AIE2_NPU_COMMAND, .min_minor = 8 },
>> +    { .major = 5, .min_minor = 7 },
>> +    { .features = BIT_U64(AIE2_NPU_COMMAND), .min_minor = 8 },
>>       { 0 }
>>   };
>>     static const struct amdxdna_dev_priv npu1_dev_priv = {
>>       .fw_path        = "amdnpu/1502_00/npu.sbin",
>> -    .protocol_major = 0x5,
>> -    .protocol_minor = 0x7,
>>       .rt_config    = npu1_default_rt_cfg,
>>       .dpm_clk_tbl    = npu1_dpm_clk_table,
>>       .fw_feature_tbl = npu1_fw_feature_table,
>> diff --git a/drivers/accel/amdxdna/npu4_regs.c 
>> b/drivers/accel/amdxdna/npu4_regs.c
>> index a62234fd266d..a8d6f76dde5f 100644
>> --- a/drivers/accel/amdxdna/npu4_regs.c
>> +++ b/drivers/accel/amdxdna/npu4_regs.c
>> @@ -6,6 +6,7 @@
>>   #include <drm/amdxdna_accel.h>
>>   #include <drm/drm_device.h>
>>   #include <drm/gpu_scheduler.h>
>> +#include <linux/bits.h>
>>   #include <linux/sizes.h>
>>     #include "aie2_pci.h"
>> @@ -88,16 +89,16 @@ const struct dpm_clk_freq npu4_dpm_clk_table[] = {
>>   };
>>     const struct aie2_fw_feature_tbl npu4_fw_feature_table[] = {
>> -    { .feature = AIE2_NPU_COMMAND, .min_minor = 15 },
>> -    { .feature = AIE2_PREEMPT, .min_minor = 12 },
>> -    { .feature = AIE2_TEMPORAL_ONLY, .min_minor = 12 },
>> +    { .major = 6, .min_minor = 12 },
>> +    { .features = BIT_U64(AIE2_NPU_COMMAND), .major = 6, .min_minor 
>> = 15 },
>> +    { .features = BIT_U64(AIE2_PREEMPT), .major = 6, .min_minor = 12 },
>> +    { .features = BIT_U64(AIE2_TEMPORAL_ONLY), .major = 6, 
>> .min_minor = 12 },
>> +    { .features = GENMASK_ULL(AIE2_TEMPORAL_ONLY, AIE2_NPU_COMMAND), 
>> .major = 7 },
>>       { 0 }
>>   };
>>     static const struct amdxdna_dev_priv npu4_dev_priv = {
>>       .fw_path        = "amdnpu/17f0_10/npu.sbin",
>> -    .protocol_major = 0x6,
>> -    .protocol_minor = 12,
>>       .rt_config    = npu4_default_rt_cfg,
>>       .dpm_clk_tbl    = npu4_dpm_clk_table,
>>       .fw_feature_tbl = npu4_fw_feature_table,
>> diff --git a/drivers/accel/amdxdna/npu5_regs.c 
>> b/drivers/accel/amdxdna/npu5_regs.c
>> index 131080652ef0..c0a35cfd886c 100644
>> --- a/drivers/accel/amdxdna/npu5_regs.c
>> +++ b/drivers/accel/amdxdna/npu5_regs.c
>> @@ -64,8 +64,6 @@
>>     static const struct amdxdna_dev_priv npu5_dev_priv = {
>>       .fw_path        = "amdnpu/17f0_11/npu.sbin",
>> -    .protocol_major = 0x6,
>> -    .protocol_minor = 12,
>>       .rt_config    = npu4_default_rt_cfg,
>>       .dpm_clk_tbl    = npu4_dpm_clk_table,
>>       .fw_feature_tbl = npu4_fw_feature_table,
>> diff --git a/drivers/accel/amdxdna/npu6_regs.c 
>> b/drivers/accel/amdxdna/npu6_regs.c
>> index 1f71285655b2..1fb07df99186 100644
>> --- a/drivers/accel/amdxdna/npu6_regs.c
>> +++ b/drivers/accel/amdxdna/npu6_regs.c
>> @@ -64,8 +64,6 @@
>>     static const struct amdxdna_dev_priv npu6_dev_priv = {
>>       .fw_path        = "amdnpu/17f0_10/npu.sbin",
>> -    .protocol_major = 0x6,
>> -    .protocol_minor = 12,
>>       .rt_config    = npu4_default_rt_cfg,
>>       .dpm_clk_tbl    = npu4_dpm_clk_table,
>>       .fw_feature_tbl = npu4_fw_feature_table,
>

Re: [PATCH V1 1/2] accel/amdxdna: Update message DMA buffer allocation

[Lizhi Hou]

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@amd.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);
>>>
>

Re: [PATCH V1 2/2] accel/amdxdna: Update firmware version check for latest firmware

[Mario Limonciello (AMD) (kernel.org)]

On 1/7/2026 4:12 PM, Lizhi Hou wrote:
> 
> On 1/7/26 13:20, Mario Limonciello wrote:
>> On 12/18/25 7:43 PM, Lizhi Hou wrote:
>>> The latest firmware increases the major version number. Update
>>> aie2_check_protocol() to accept and support the new firmware version.
>>>
>>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
>>
>> I know it's painful and tech debt; but I suggest you add new paths to 
>> handle both versions of firmware at least until the next LTS kernel.
> 
> With the patch 1, the driver works fine for both current firmware and 
> upcoming firmware.
> 
> Lizhi
> 

Thank for confirming, no other concerns with this patch.

Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>

>>
>>> ---
>>>   drivers/accel/amdxdna/aie2_pci.c  | 36 ++++++++-----------------------
>>>   drivers/accel/amdxdna/aie2_pci.h  |  5 ++---
>>>   drivers/accel/amdxdna/npu1_regs.c |  6 +++---
>>>   drivers/accel/amdxdna/npu4_regs.c | 11 +++++-----
>>>   drivers/accel/amdxdna/npu5_regs.c |  2 --
>>>   drivers/accel/amdxdna/npu6_regs.c |  2 --
>>>   6 files changed, 20 insertions(+), 42 deletions(-)
>>>
>>> diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/ 
>>> amdxdna/aie2_pci.c
>>> index 81a8e4137bfd..181fdbc10dae 100644
>>> --- a/drivers/accel/amdxdna/aie2_pci.c
>>> +++ b/drivers/accel/amdxdna/aie2_pci.c
>>> @@ -56,41 +56,23 @@ struct mgmt_mbox_chann_info {
>>>   static int aie2_check_protocol(struct amdxdna_dev_hdl *ndev, u32 
>>> fw_major, u32 fw_minor)
>>>   {
>>>       const struct aie2_fw_feature_tbl *feature;
>>> -    struct amdxdna_dev *xdna = ndev->xdna;
>>> -
>>> -    /*
>>> -     * The driver supported mailbox behavior is defined by
>>> -     * ndev->priv->protocol_major and protocol_minor.
>>> -     *
>>> -     * When protocol_major and fw_major are different, it means driver
>>> -     * and firmware are incompatible.
>>> -     */
>>> -    if (ndev->priv->protocol_major != fw_major) {
>>> -        XDNA_ERR(xdna, "Incompatible firmware protocol major %d 
>>> minor %d",
>>> -             fw_major, fw_minor);
>>> -        return -EINVAL;
>>> -    }
>>> +    bool found = false;
>>>   -    /*
>>> -     * When protocol_minor is greater then fw_minor, that means driver
>>> -     * relies on operation the installed firmware does not support.
>>> -     */
>>> -    if (ndev->priv->protocol_minor > fw_minor) {
>>> -        XDNA_ERR(xdna, "Firmware minor version smaller than 
>>> supported");
>>> -        return -EINVAL;
>>> -    }
>>> -
>>> -    for (feature = ndev->priv->fw_feature_tbl; feature && feature- 
>>> >min_minor;
>>> -         feature++) {
>>> +    for (feature = ndev->priv->fw_feature_tbl; feature->major; 
>>> feature++) {
>>> +        if (feature->major != fw_major)
>>> +            continue;
>>>           if (fw_minor < feature->min_minor)
>>>               continue;
>>>           if (feature->max_minor > 0 && fw_minor > feature->max_minor)
>>>               continue;
>>>   -        set_bit(feature->feature, &ndev->feature_mask);
>>> +        ndev->feature_mask |= feature->features;
>>> +
>>> +        /* firmware version matches one of the driver support entry */
>>> +        found = true;
>>>       }
>>>   -    return 0;
>>> +    return found ? 0 : -EOPNOTSUPP;
>>>   }
>>>     static void aie2_dump_chann_info_debug(struct amdxdna_dev_hdl *ndev)
>>> diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/ 
>>> amdxdna/aie2_pci.h
>>> index e1745f07b268..b20a3661078c 100644
>>> --- a/drivers/accel/amdxdna/aie2_pci.h
>>> +++ b/drivers/accel/amdxdna/aie2_pci.h
>>> @@ -237,7 +237,8 @@ enum aie2_fw_feature {
>>>   };
>>>     struct aie2_fw_feature_tbl {
>>> -    enum aie2_fw_feature feature;
>>> +    u64 features;
>>> +    u32 major;
>>>       u32 max_minor;
>>>       u32 min_minor;
>>>   };
>>> @@ -246,8 +247,6 @@ struct aie2_fw_feature_tbl {
>>>     struct amdxdna_dev_priv {
>>>       const char            *fw_path;
>>> -    u64                protocol_major;
>>> -    u64                protocol_minor;
>>>       const struct rt_config        *rt_config;
>>>       const struct dpm_clk_freq    *dpm_clk_tbl;
>>>       const struct aie2_fw_feature_tbl *fw_feature_tbl;
>>> diff --git a/drivers/accel/amdxdna/npu1_regs.c b/drivers/accel/ 
>>> amdxdna/npu1_regs.c
>>> index ebc6e2802297..6f36a27b5a02 100644
>>> --- a/drivers/accel/amdxdna/npu1_regs.c
>>> +++ b/drivers/accel/amdxdna/npu1_regs.c
>>> @@ -6,6 +6,7 @@
>>>   #include <drm/amdxdna_accel.h>
>>>   #include <drm/drm_device.h>
>>>   #include <drm/gpu_scheduler.h>
>>> +#include <linux/bits.h>
>>>   #include <linux/sizes.h>
>>>     #include "aie2_pci.h"
>>> @@ -65,14 +66,13 @@ const struct dpm_clk_freq npu1_dpm_clk_table[] = {
>>>   };
>>>     static const struct aie2_fw_feature_tbl npu1_fw_feature_table[] = {
>>> -    { .feature = AIE2_NPU_COMMAND, .min_minor = 8 },
>>> +    { .major = 5, .min_minor = 7 },
>>> +    { .features = BIT_U64(AIE2_NPU_COMMAND), .min_minor = 8 },
>>>       { 0 }
>>>   };
>>>     static const struct amdxdna_dev_priv npu1_dev_priv = {
>>>       .fw_path        = "amdnpu/1502_00/npu.sbin",
>>> -    .protocol_major = 0x5,
>>> -    .protocol_minor = 0x7,
>>>       .rt_config    = npu1_default_rt_cfg,
>>>       .dpm_clk_tbl    = npu1_dpm_clk_table,
>>>       .fw_feature_tbl = npu1_fw_feature_table,
>>> diff --git a/drivers/accel/amdxdna/npu4_regs.c b/drivers/accel/ 
>>> amdxdna/npu4_regs.c
>>> index a62234fd266d..a8d6f76dde5f 100644
>>> --- a/drivers/accel/amdxdna/npu4_regs.c
>>> +++ b/drivers/accel/amdxdna/npu4_regs.c
>>> @@ -6,6 +6,7 @@
>>>   #include <drm/amdxdna_accel.h>
>>>   #include <drm/drm_device.h>
>>>   #include <drm/gpu_scheduler.h>
>>> +#include <linux/bits.h>
>>>   #include <linux/sizes.h>
>>>     #include "aie2_pci.h"
>>> @@ -88,16 +89,16 @@ const struct dpm_clk_freq npu4_dpm_clk_table[] = {
>>>   };
>>>     const struct aie2_fw_feature_tbl npu4_fw_feature_table[] = {
>>> -    { .feature = AIE2_NPU_COMMAND, .min_minor = 15 },
>>> -    { .feature = AIE2_PREEMPT, .min_minor = 12 },
>>> -    { .feature = AIE2_TEMPORAL_ONLY, .min_minor = 12 },
>>> +    { .major = 6, .min_minor = 12 },
>>> +    { .features = BIT_U64(AIE2_NPU_COMMAND), .major = 6, .min_minor 
>>> = 15 },
>>> +    { .features = BIT_U64(AIE2_PREEMPT), .major = 6, .min_minor = 12 },
>>> +    { .features = BIT_U64(AIE2_TEMPORAL_ONLY), .major = 
>>> 6, .min_minor = 12 },
>>> +    { .features = GENMASK_ULL(AIE2_TEMPORAL_ONLY, 
>>> AIE2_NPU_COMMAND), .major = 7 },
>>>       { 0 }
>>>   };
>>>     static const struct amdxdna_dev_priv npu4_dev_priv = {
>>>       .fw_path        = "amdnpu/17f0_10/npu.sbin",
>>> -    .protocol_major = 0x6,
>>> -    .protocol_minor = 12,
>>>       .rt_config    = npu4_default_rt_cfg,
>>>       .dpm_clk_tbl    = npu4_dpm_clk_table,
>>>       .fw_feature_tbl = npu4_fw_feature_table,
>>> diff --git a/drivers/accel/amdxdna/npu5_regs.c b/drivers/accel/ 
>>> amdxdna/npu5_regs.c
>>> index 131080652ef0..c0a35cfd886c 100644
>>> --- a/drivers/accel/amdxdna/npu5_regs.c
>>> +++ b/drivers/accel/amdxdna/npu5_regs.c
>>> @@ -64,8 +64,6 @@
>>>     static const struct amdxdna_dev_priv npu5_dev_priv = {
>>>       .fw_path        = "amdnpu/17f0_11/npu.sbin",
>>> -    .protocol_major = 0x6,
>>> -    .protocol_minor = 12,
>>>       .rt_config    = npu4_default_rt_cfg,
>>>       .dpm_clk_tbl    = npu4_dpm_clk_table,
>>>       .fw_feature_tbl = npu4_fw_feature_table,
>>> diff --git a/drivers/accel/amdxdna/npu6_regs.c b/drivers/accel/ 
>>> amdxdna/npu6_regs.c
>>> index 1f71285655b2..1fb07df99186 100644
>>> --- a/drivers/accel/amdxdna/npu6_regs.c
>>> +++ b/drivers/accel/amdxdna/npu6_regs.c
>>> @@ -64,8 +64,6 @@
>>>     static const struct amdxdna_dev_priv npu6_dev_priv = {
>>>       .fw_path        = "amdnpu/17f0_10/npu.sbin",
>>> -    .protocol_major = 0x6,
>>> -    .protocol_minor = 12,
>>>       .rt_config    = npu4_default_rt_cfg,
>>>       .dpm_clk_tbl    = npu4_dpm_clk_table,
>>>       .fw_feature_tbl = npu4_fw_feature_table,
>>

Re: [PATCH V1 1/2] accel/amdxdna: Update message DMA buffer allocation

[Mario Limonciello (AMD) (kernel.org)]

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@kernel.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@amd.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);
>>>>
>>

Re: [PATCH V1 1/2] accel/amdxdna: Update message DMA buffer allocation

[Lizhi Hou]

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@kernel.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@amd.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);
>>>>>
>>>
>

Re: [PATCH V1 2/2] accel/amdxdna: Update firmware version check for latest firmware

[Lizhi Hou]

Applied to drm-misc-next.

On 1/7/26 19:26, Mario Limonciello (AMD) (kernel.org) wrote:
>
>
> On 1/7/2026 4:12 PM, Lizhi Hou wrote:
>>
>> On 1/7/26 13:20, Mario Limonciello wrote:
>>> On 12/18/25 7:43 PM, Lizhi Hou wrote:
>>>> The latest firmware increases the major version number. Update
>>>> aie2_check_protocol() to accept and support the new firmware version.
>>>>
>>>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
>>>
>>> I know it's painful and tech debt; but I suggest you add new paths 
>>> to handle both versions of firmware at least until the next LTS kernel.
>>
>> With the patch 1, the driver works fine for both current firmware and 
>> upcoming firmware.
>>
>> Lizhi
>>
>
> Thank for confirming, no other concerns with this patch.
>
> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
>
>>>
>>>> ---
>>>>   drivers/accel/amdxdna/aie2_pci.c  | 36 
>>>> ++++++++-----------------------
>>>>   drivers/accel/amdxdna/aie2_pci.h  |  5 ++---
>>>>   drivers/accel/amdxdna/npu1_regs.c |  6 +++---
>>>>   drivers/accel/amdxdna/npu4_regs.c | 11 +++++-----
>>>>   drivers/accel/amdxdna/npu5_regs.c |  2 --
>>>>   drivers/accel/amdxdna/npu6_regs.c |  2 --
>>>>   6 files changed, 20 insertions(+), 42 deletions(-)
>>>>
>>>> diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/ 
>>>> amdxdna/aie2_pci.c
>>>> index 81a8e4137bfd..181fdbc10dae 100644
>>>> --- a/drivers/accel/amdxdna/aie2_pci.c
>>>> +++ b/drivers/accel/amdxdna/aie2_pci.c
>>>> @@ -56,41 +56,23 @@ struct mgmt_mbox_chann_info {
>>>>   static int aie2_check_protocol(struct amdxdna_dev_hdl *ndev, u32 
>>>> fw_major, u32 fw_minor)
>>>>   {
>>>>       const struct aie2_fw_feature_tbl *feature;
>>>> -    struct amdxdna_dev *xdna = ndev->xdna;
>>>> -
>>>> -    /*
>>>> -     * The driver supported mailbox behavior is defined by
>>>> -     * ndev->priv->protocol_major and protocol_minor.
>>>> -     *
>>>> -     * When protocol_major and fw_major are different, it means 
>>>> driver
>>>> -     * and firmware are incompatible.
>>>> -     */
>>>> -    if (ndev->priv->protocol_major != fw_major) {
>>>> -        XDNA_ERR(xdna, "Incompatible firmware protocol major %d 
>>>> minor %d",
>>>> -             fw_major, fw_minor);
>>>> -        return -EINVAL;
>>>> -    }
>>>> +    bool found = false;
>>>>   -    /*
>>>> -     * When protocol_minor is greater then fw_minor, that means 
>>>> driver
>>>> -     * relies on operation the installed firmware does not support.
>>>> -     */
>>>> -    if (ndev->priv->protocol_minor > fw_minor) {
>>>> -        XDNA_ERR(xdna, "Firmware minor version smaller than 
>>>> supported");
>>>> -        return -EINVAL;
>>>> -    }
>>>> -
>>>> -    for (feature = ndev->priv->fw_feature_tbl; feature && feature- 
>>>> >min_minor;
>>>> -         feature++) {
>>>> +    for (feature = ndev->priv->fw_feature_tbl; feature->major; 
>>>> feature++) {
>>>> +        if (feature->major != fw_major)
>>>> +            continue;
>>>>           if (fw_minor < feature->min_minor)
>>>>               continue;
>>>>           if (feature->max_minor > 0 && fw_minor > feature->max_minor)
>>>>               continue;
>>>>   -        set_bit(feature->feature, &ndev->feature_mask);
>>>> +        ndev->feature_mask |= feature->features;
>>>> +
>>>> +        /* firmware version matches one of the driver support 
>>>> entry */
>>>> +        found = true;
>>>>       }
>>>>   -    return 0;
>>>> +    return found ? 0 : -EOPNOTSUPP;
>>>>   }
>>>>     static void aie2_dump_chann_info_debug(struct amdxdna_dev_hdl 
>>>> *ndev)
>>>> diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/ 
>>>> amdxdna/aie2_pci.h
>>>> index e1745f07b268..b20a3661078c 100644
>>>> --- a/drivers/accel/amdxdna/aie2_pci.h
>>>> +++ b/drivers/accel/amdxdna/aie2_pci.h
>>>> @@ -237,7 +237,8 @@ enum aie2_fw_feature {
>>>>   };
>>>>     struct aie2_fw_feature_tbl {
>>>> -    enum aie2_fw_feature feature;
>>>> +    u64 features;
>>>> +    u32 major;
>>>>       u32 max_minor;
>>>>       u32 min_minor;
>>>>   };
>>>> @@ -246,8 +247,6 @@ struct aie2_fw_feature_tbl {
>>>>     struct amdxdna_dev_priv {
>>>>       const char            *fw_path;
>>>> -    u64                protocol_major;
>>>> -    u64                protocol_minor;
>>>>       const struct rt_config        *rt_config;
>>>>       const struct dpm_clk_freq    *dpm_clk_tbl;
>>>>       const struct aie2_fw_feature_tbl *fw_feature_tbl;
>>>> diff --git a/drivers/accel/amdxdna/npu1_regs.c b/drivers/accel/ 
>>>> amdxdna/npu1_regs.c
>>>> index ebc6e2802297..6f36a27b5a02 100644
>>>> --- a/drivers/accel/amdxdna/npu1_regs.c
>>>> +++ b/drivers/accel/amdxdna/npu1_regs.c
>>>> @@ -6,6 +6,7 @@
>>>>   #include <drm/amdxdna_accel.h>
>>>>   #include <drm/drm_device.h>
>>>>   #include <drm/gpu_scheduler.h>
>>>> +#include <linux/bits.h>
>>>>   #include <linux/sizes.h>
>>>>     #include "aie2_pci.h"
>>>> @@ -65,14 +66,13 @@ const struct dpm_clk_freq npu1_dpm_clk_table[] = {
>>>>   };
>>>>     static const struct aie2_fw_feature_tbl npu1_fw_feature_table[] 
>>>> = {
>>>> -    { .feature = AIE2_NPU_COMMAND, .min_minor = 8 },
>>>> +    { .major = 5, .min_minor = 7 },
>>>> +    { .features = BIT_U64(AIE2_NPU_COMMAND), .min_minor = 8 },
>>>>       { 0 }
>>>>   };
>>>>     static const struct amdxdna_dev_priv npu1_dev_priv = {
>>>>       .fw_path        = "amdnpu/1502_00/npu.sbin",
>>>> -    .protocol_major = 0x5,
>>>> -    .protocol_minor = 0x7,
>>>>       .rt_config    = npu1_default_rt_cfg,
>>>>       .dpm_clk_tbl    = npu1_dpm_clk_table,
>>>>       .fw_feature_tbl = npu1_fw_feature_table,
>>>> diff --git a/drivers/accel/amdxdna/npu4_regs.c b/drivers/accel/ 
>>>> amdxdna/npu4_regs.c
>>>> index a62234fd266d..a8d6f76dde5f 100644
>>>> --- a/drivers/accel/amdxdna/npu4_regs.c
>>>> +++ b/drivers/accel/amdxdna/npu4_regs.c
>>>> @@ -6,6 +6,7 @@
>>>>   #include <drm/amdxdna_accel.h>
>>>>   #include <drm/drm_device.h>
>>>>   #include <drm/gpu_scheduler.h>
>>>> +#include <linux/bits.h>
>>>>   #include <linux/sizes.h>
>>>>     #include "aie2_pci.h"
>>>> @@ -88,16 +89,16 @@ const struct dpm_clk_freq npu4_dpm_clk_table[] = {
>>>>   };
>>>>     const struct aie2_fw_feature_tbl npu4_fw_feature_table[] = {
>>>> -    { .feature = AIE2_NPU_COMMAND, .min_minor = 15 },
>>>> -    { .feature = AIE2_PREEMPT, .min_minor = 12 },
>>>> -    { .feature = AIE2_TEMPORAL_ONLY, .min_minor = 12 },
>>>> +    { .major = 6, .min_minor = 12 },
>>>> +    { .features = BIT_U64(AIE2_NPU_COMMAND), .major = 6, 
>>>> .min_minor = 15 },
>>>> +    { .features = BIT_U64(AIE2_PREEMPT), .major = 6, .min_minor = 
>>>> 12 },
>>>> +    { .features = BIT_U64(AIE2_TEMPORAL_ONLY), .major = 6, 
>>>> .min_minor = 12 },
>>>> +    { .features = GENMASK_ULL(AIE2_TEMPORAL_ONLY, 
>>>> AIE2_NPU_COMMAND), .major = 7 },
>>>>       { 0 }
>>>>   };
>>>>     static const struct amdxdna_dev_priv npu4_dev_priv = {
>>>>       .fw_path        = "amdnpu/17f0_10/npu.sbin",
>>>> -    .protocol_major = 0x6,
>>>> -    .protocol_minor = 12,
>>>>       .rt_config    = npu4_default_rt_cfg,
>>>>       .dpm_clk_tbl    = npu4_dpm_clk_table,
>>>>       .fw_feature_tbl = npu4_fw_feature_table,
>>>> diff --git a/drivers/accel/amdxdna/npu5_regs.c b/drivers/accel/ 
>>>> amdxdna/npu5_regs.c
>>>> index 131080652ef0..c0a35cfd886c 100644
>>>> --- a/drivers/accel/amdxdna/npu5_regs.c
>>>> +++ b/drivers/accel/amdxdna/npu5_regs.c
>>>> @@ -64,8 +64,6 @@
>>>>     static const struct amdxdna_dev_priv npu5_dev_priv = {
>>>>       .fw_path        = "amdnpu/17f0_11/npu.sbin",
>>>> -    .protocol_major = 0x6,
>>>> -    .protocol_minor = 12,
>>>>       .rt_config    = npu4_default_rt_cfg,
>>>>       .dpm_clk_tbl    = npu4_dpm_clk_table,
>>>>       .fw_feature_tbl = npu4_fw_feature_table,
>>>> diff --git a/drivers/accel/amdxdna/npu6_regs.c b/drivers/accel/ 
>>>> amdxdna/npu6_regs.c
>>>> index 1f71285655b2..1fb07df99186 100644
>>>> --- a/drivers/accel/amdxdna/npu6_regs.c
>>>> +++ b/drivers/accel/amdxdna/npu6_regs.c
>>>> @@ -64,8 +64,6 @@
>>>>     static const struct amdxdna_dev_priv npu6_dev_priv = {
>>>>       .fw_path        = "amdnpu/17f0_10/npu.sbin",
>>>> -    .protocol_major = 0x6,
>>>> -    .protocol_minor = 12,
>>>>       .rt_config    = npu4_default_rt_cfg,
>>>>       .dpm_clk_tbl    = npu4_dpm_clk_table,
>>>>       .fw_feature_tbl = npu4_fw_feature_table,
>>>
>