Re: [PATCH V4 07/19] accel/amdxdna: Add get and set power_mode for AIE4

[yidong Zhang <yidong.zhang@amd.com>]

On 10/12/26 14:29, Lizhi Hou wrote:
>
> On 10/9/26 15:53, David Zhang wrote:
>> Add power mode support for AIE4 devices via DRM_AMDXDNA_GET_INFO and
>> DRM_AMDXDNA_SET_STATE:
>> - Add AIE4_MSG_OP_POWER_OVERRIDE mailbox support
>> - Add DRM_AMDXDNA_GET_POWER_MODE support
>> - Implement aie4_set_power_mode() and wire .set_aie_state
>>
>> Firmware boots in POWER_MODE_DEFAULT after a reload, so re-send the
>> cached user power mode override whenever the hardware starts via
>> aie4_restore_power_mode(). On a fresh probe, pw_mode is
>> POWER_MODE_DEFAULT.
>>
>> Co-developed-by: Soham Donwalkar <soham.donwalkar@amd.com>
>> Signed-off-by: Soham Donwalkar <soham.donwalkar@amd.com>
>> Co-developed-by: Nishad Saraf <nishad.saraf@amd.com>
>> Signed-off-by: Nishad Saraf <nishad.saraf@amd.com>
>> Signed-off-by: David Zhang <yidong.zhang@amd.com>
>> ---
>>   drivers/accel/amdxdna/aie4_message.c  |  17 ++++
>>   drivers/accel/amdxdna/aie4_msg_priv.h |   9 ++
>>   drivers/accel/amdxdna/aie4_pci.c      | 116 ++++++++++++++++++++++++++
>>   drivers/accel/amdxdna/aie4_pci.h      |   6 ++
>>   4 files changed, 148 insertions(+)
>>
>> diff --git a/drivers/accel/amdxdna/aie4_message.c b/drivers/accel/amdxdna/aie4_message.c
>> index c83f81a0e65b..8d1cad50cbab 100644
>> --- a/drivers/accel/amdxdna/aie4_message.c
>> +++ b/drivers/accel/amdxdna/aie4_message.c
>> @@ -158,3 +158,20 @@ int aie4_attach_work_buffer(struct amdxdna_dev_hdl *ndev)
>>         return ret;
>>   }
>> +
>> +int aie4_msg_set_power_mode(struct amdxdna_dev_hdl *ndev, u8 power_mode)
>> +{
>> +    DECLARE_AIE_MSG(aie4_msg_power_override, AIE4_MSG_OP_POWER_OVERRIDE);
>> +    int ret;
>> +
>> +    req.power_mode = power_mode;
>> +
>> +    ret = aie_send_mgmt_msg_wait(&ndev->aie, &msg);
>> +    if (ret)
>> +        XDNA_WARN(ndev->aie.xdna,
>
> Should be XDNA_ERR(). 
I will fix this in next version of the patch. 
Thanks, David
>
> Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
>
>> +              "Failed to set power mode %u, ret %d", (u32)power_mode, ret);
>> +    else
>> +        XDNA_DBG(ndev->aie.xdna, "Power mode set to %u", (u32)power_mode);
>> +
>> +    return ret;
>> +}
>> diff --git a/drivers/accel/amdxdna/aie4_msg_priv.h b/drivers/accel/amdxdna/aie4_msg_priv.h
>> index 81842fa1d6ce..4c06792df1bd 100644
>> --- a/drivers/accel/amdxdna/aie4_msg_priv.h
>> +++ b/drivers/accel/amdxdna/aie4_msg_priv.h
>> @@ -23,6 +23,7 @@ enum aie4_msg_opcode {
>>       AIE4_MSG_OP_DESTROY_HW_CONTEXT               = 0x30004,
>>       AIE4_MSG_OP_AIE_TILE_INFO                    = 0x30006,
>>       AIE4_MSG_OP_AIE_VERSION_INFO                 = 0x30007,
>> +    AIE4_MSG_OP_POWER_OVERRIDE                   = 0x3000B,
>>         AIE4_MSG_OP_ATTACH_WORK_BUFFER               = 0x40001,
>>   };
>> @@ -187,6 +188,14 @@ struct aie4_msg_query_cert_firmware_version_resp {
>>       __u16 host_queue_minor;
>>   } __packed;
>>   +struct aie4_msg_power_override_req {
>> +    __u32 power_mode;
>> +} __packed;
>> +
>> +struct aie4_msg_power_override_resp {
>> +    enum aie4_msg_status status;
>> +} __packed;
>> +
>>   #define AIE4_WORK_BUFFER_MIN_SIZE      SZ_4M
>>     struct aie4_msg_attach_work_buffer_req {
>> diff --git a/drivers/accel/amdxdna/aie4_pci.c b/drivers/accel/amdxdna/aie4_pci.c
>> index 1cbd1665a28f..6eacdc026ec7 100644
>> --- a/drivers/accel/amdxdna/aie4_pci.c
>> +++ b/drivers/accel/amdxdna/aie4_pci.c
>> @@ -4,6 +4,7 @@
>>    */
>>     #include <drm/amdxdna_accel.h>
>> +#include <drm/drm_drv.h>
>>   #include <drm/drm_managed.h>
>>   #include <drm/drm_print.h>
>>   #include <linux/firmware.h>
>> @@ -16,6 +17,7 @@
>>   #include "amdxdna_mailbox.h"
>>   #include "amdxdna_mailbox_helper.h"
>>   #include "amdxdna_pci_drv.h"
>> +#include "amdxdna_pm.h"
>>     #define NO_IOHUB        0
>>   #define PSP_NOTIFY_INTR        0xD007BE11
>> @@ -300,6 +302,15 @@ static int aie4_query(struct amdxdna_dev_hdl *ndev)
>>       return 0;
>>   }
>>   +/* Restore cached power mode override across hardware start. */
>> +int aie4_restore_power_mode(struct amdxdna_dev_hdl *ndev)
>> +{
>> +    if (ndev->pw_mode == POWER_MODE_DEFAULT)
>> +        return 0;
>> +
>> +    return aie4_msg_set_power_mode(ndev, ndev->pw_mode);
>> +}
>> +
>>   static int aie4_pf_hw_start(struct amdxdna_dev_hdl *ndev)
>>   {
>>       int ret;
>> @@ -316,6 +327,10 @@ static int aie4_pf_hw_start(struct amdxdna_dev_hdl *ndev)
>>       if (ret)
>>           goto mbox_fini;
>>   +    ret = aie4_restore_power_mode(ndev);
>> +    if (ret)
>> +        goto mbox_fini;
>> +
>>       return 0;
>>     mbox_fini:
>> @@ -353,8 +368,14 @@ static int aie4_vf_hw_start(struct amdxdna_dev_hdl *ndev)
>>       if (ret)
>>           goto mailbox_fini;
>>   +    ret = aie4_restore_power_mode(ndev);
>> +    if (ret)
>> +        goto partition_fini;
>> +
>>       return 0;
>>   +partition_fini:
>> +    aie4_partition_fini(ndev);
>>   mailbox_fini:
>>       aie4_mailbox_fini(ndev);
>>       return ret;
>> @@ -394,8 +415,14 @@ static int aie4_classic_hw_start(struct amdxdna_dev_hdl *ndev)
>>       if (ret)
>>           goto mailbox_fini;
>>   +    ret = aie4_restore_power_mode(ndev);
>> +    if (ret)
>> +        goto partition_fini;
>> +
>>       return 0;
>>   +partition_fini:
>> +    aie4_partition_fini(ndev);
>>   mailbox_fini:
>>       aie4_mailbox_fini(ndev);
>>   stop_fw:
>> @@ -535,6 +562,7 @@ static int aie4m_pcidev_init(struct amdxdna_dev *xdna)
>>         ndev->priv = xdna->dev_info->dev_priv;
>>       ndev->aie.xdna = xdna;
>> +    ndev->pw_mode = POWER_MODE_DEFAULT;
>>       xdna->dev_handle = ndev;
>>         xa_init_flags(&ndev->cert_comp_xa, XA_FLAGS_ALLOC);
>> @@ -589,6 +617,24 @@ static int aie4m_pcidev_init(struct amdxdna_dev *xdna)
>>       return 0;
>>   }
>>   +static int aie4_get_power_mode(struct amdxdna_client *client,
>> +                   struct amdxdna_drm_get_info *args)
>> +{
>> +    struct amdxdna_drm_get_power_mode mode = {};
>> +    struct amdxdna_dev *xdna = client->xdna;
>> +    struct amdxdna_dev_hdl *ndev;
>> +    u32 buf_sz;
>> +
>> +    ndev = xdna->dev_handle;
>> +    mode.power_mode = ndev->pw_mode;
>> +
>> +    buf_sz = min_t(u32, args->buffer_size, sizeof(mode));
>> +    if (copy_to_user(u64_to_user_ptr(args->buffer), &mode, buf_sz))
>> +        return -EFAULT;
>> +
>> +    return 0;
>> +}
>> +
>>   static int aie4_get_info(struct amdxdna_client *client, struct amdxdna_drm_get_info *args)
>>   {
>>       struct amdxdna_dev *xdna = client->xdna;
>> @@ -605,6 +651,9 @@ static int aie4_get_info(struct amdxdna_client *client, struct amdxdna_drm_get_i
>>       case DRM_AMDXDNA_QUERY_FIRMWARE_VERSION:
>>           ret = amdxdna_get_firmware_version(client, args, &xdna->fw_ver);
>>           break;
>> +    case DRM_AMDXDNA_GET_POWER_MODE:
>> +        ret = aie4_get_power_mode(client, args);
>> +        break;
>>       default:
>>           XDNA_ERR(xdna, "Not supported request parameter %u", args->param);
>>           ret = -EOPNOTSUPP;
>> @@ -615,6 +664,71 @@ static int aie4_get_info(struct amdxdna_client *client, struct amdxdna_drm_get_i
>>       return ret;
>>   }
>>   +static int aie4_set_power_mode(struct amdxdna_client *client,
>> +                   struct amdxdna_drm_set_state *args)
>> +{
>> +    struct amdxdna_drm_set_power_mode power_state = { 0 };
>> +    struct amdxdna_dev *xdna = client->xdna;
>> +    struct amdxdna_dev_hdl *ndev = xdna->dev_handle;
>> +    u8 power_mode;
>> +    int ret;
>> +
>> +    if (args->buffer_size < sizeof(power_state))
>> +        return -EINVAL;
>> +
>> +    if (copy_from_user(&power_state, u64_to_user_ptr(args->buffer),
>> +               sizeof(power_state))) {
>> +        XDNA_ERR(xdna, "Failed to copy power mode request into kernel");
>> +        return -EFAULT;
>> +    }
>> +
>> +    if (XDNA_MBZ_DBG(xdna, power_state.pad, sizeof(power_state.pad)))
>> +        return -EINVAL;
>> +
>> +    power_mode = power_state.power_mode;
>> +    if (power_mode > POWER_MODE_TURBO) {
>> +        XDNA_ERR(xdna, "Invalid power mode %d", power_mode);
>> +        return -EINVAL;
>> +    }
>> +
>> +    ret = aie4_msg_set_power_mode(xdna->dev_handle, power_mode);
>> +    if (ret)
>> +        return ret;
>> +
>> +    ndev->pw_mode = power_mode;
>> +    return 0;
>> +}
>> +
>> +static int aie4_set_state(struct amdxdna_client *client,
>> +              struct amdxdna_drm_set_state *args)
>> +{
>> +    struct amdxdna_dev *xdna = client->xdna;
>> +    int ret, idx;
>> +
>> +    if (!drm_dev_enter(&xdna->ddev, &idx))
>> +        return -ENODEV;
>> +
>> +    ret = amdxdna_pm_resume_get_locked(xdna);
>> +    if (ret)
>> +        goto dev_exit;
>> +
>> +    switch (args->param) {
>> +    case DRM_AMDXDNA_SET_POWER_MODE:
>> +        ret = aie4_set_power_mode(client, args);
>> +        break;
>> +    default:
>> +        XDNA_ERR(xdna, "Not supported request parameter %u", args->param);
>> +        ret = -EOPNOTSUPP;
>> +        break;
>> +    }
>> +
>> +    amdxdna_pm_suspend_put(xdna);
>> +
>> +dev_exit:
>> +    drm_dev_exit(idx);
>> +    return ret;
>> +}
>> +
>>   static int aie4_alloc_work_buffer(struct amdxdna_dev_hdl *ndev)
>>   {
>>       struct amdxdna_dev *xdna = ndev->aie.xdna;
>> @@ -737,6 +851,7 @@ const struct amdxdna_dev_ops aie4_vf_ops = {
>>       .hwctx_fini        = aie4_hwctx_fini,
>>       .cmd_wait        = aie4_cmd_wait,
>>       .get_aie_info        = aie4_get_info,
>> +    .set_aie_state        = aie4_set_state,
>>   };
>>     const struct amdxdna_dev_ops aie4_classic_ops = {
>> @@ -746,4 +861,5 @@ const struct amdxdna_dev_ops aie4_classic_ops = {
>>       .hwctx_fini        = aie4_hwctx_fini,
>>       .cmd_wait        = aie4_cmd_wait,
>>       .get_aie_info        = aie4_get_info,
>> +    .set_aie_state        = aie4_set_state,
>>   };
>> diff --git a/drivers/accel/amdxdna/aie4_pci.h b/drivers/accel/amdxdna/aie4_pci.h
>> index 5ae5e8427a3b..fd2c50dc8080 100644
>> --- a/drivers/accel/amdxdna/aie4_pci.h
>> +++ b/drivers/accel/amdxdna/aie4_pci.h
>> @@ -58,6 +58,8 @@ struct amdxdna_dev_hdl {
>>       dma_addr_t            work_buf_addr;
>>       u32                work_buf_size;
>>   +    u8                pw_mode;
>> +
>>       struct amdxdna_drm_query_firmware_version cert_version;
>>   };
>>   @@ -77,6 +79,7 @@ int aie4_query_npu_firmware_version(struct amdxdna_dev_hdl *ndev,
>>                       struct amdxdna_drm_query_firmware_version *fw_version);
>>   int aie4_query_cert_firmware_version(struct amdxdna_dev_hdl *ndev,
>>                        struct amdxdna_drm_query_firmware_version *cert_version);
>> +int aie4_msg_set_power_mode(struct amdxdna_dev_hdl *ndev, u8 power_mode);
>>   u32 aie4_msg_pasid(struct amdxdna_client *client);
>>     /* aie4_ctx.c */
>> @@ -84,6 +87,9 @@ int aie4_hwctx_init(struct amdxdna_hwctx *hwctx);
>>   void aie4_hwctx_fini(struct amdxdna_hwctx *hwctx);
>>   int aie4_cmd_wait(struct amdxdna_hwctx *hwctx, u64 seq, u32 timeout);
>>   +/* aie4_pci.c */
>> +int aie4_restore_power_mode(struct amdxdna_dev_hdl *ndev);
>> +
>>   /* aie4_sriov.c */
>>   #if IS_ENABLED(CONFIG_PCI_IOV)
>>   int aie4_sriov_configure(struct amdxdna_dev *xdna, int num_vfs);