[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <dc741e29-19bd-4de1-86bb-6bbf5799a9a4@kernel.org>
Date: Wed, 7 Jan 2026 21:26:10 -0600
From: "Mario Limonciello (AMD) (kernel.org)" <superm1@...nel.org>
To: Lizhi Hou <lizhi.hou@....com>, ogabbay@...nel.org,
quic_jhugo@...cinc.com, dri-devel@...ts.freedesktop.org,
maciej.falkowski@...ux.intel.com
Cc: linux-kernel@...r.kernel.org, max.zhen@....com, sonal.santan@....com
Subject: Re: [PATCH V1 2/2] accel/amdxdna: Update firmware version check for
latest firmware
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@....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@...nel.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,
>>
Powered by blists - more mailing lists