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

[Mario Limonciello <mario.limonciello@amd.com>]

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,