Re: [PATCH] drm/amdgpu: Update message IDs to PMFW to correctly gather GFXOFF residency logs

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

On 7/8/26 17:57, Liang, Richard qi wrote:
> AMD General
> 
> @Limonciello, Mario for code review.
> 
> Thanks,
> Richard
> 
> -----Original Message-----
> From: Soliman, Fares <Fares.Soliman@amd.com>
> Sent: Monday, July 6, 2026 4:47 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Soliman, Fares <Fares.Soliman@amd.com>; Liang, Richard qi <Richardqi.Liang@amd.com>; Soliman, Fares <Fares.Soliman@amd.com>
> Subject: [PATCH] drm/amdgpu: Update message IDs to PMFW to correctly gather GFXOFF residency logs
> 
> Updates PPSMC_MSGs and set/get functions for gathering GFXOFF logs on Van Gogh. Logs are now gathered live rather than starting then stopping logging and reading an average value afterwards. This is in accordance to changes made in PMFW.
> 
> Signed-off-by: Fares Soliman <Fares.Soliman@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c   |  4 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |  1 -
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h       |  1 -
>   drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h |  2 +-
>   .../pm/swsmu/inc/pmfw_if/smu_v11_5_ppsmc.h    |  6 ++--
>   drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h  |  4 ++-  .../gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c  | 30 ++++++++++++-------
>   7 files changed, 29 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index 5c4d4ff001ea..46514a1faf1f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -1320,8 +1320,8 @@ static ssize_t amdgpu_debugfs_gpr_read(struct file *f, char __user *buf,
>    * @size: Number of bytes to read
>    * @pos:  Offset to seek to
>    *
> - * Read the last residency value logged. It doesn't auto update, one needs to
> - * stop logging before getting the current value.
> + * Read a live GFXOFF residency sample from firmware. One needs to
> + start logging
> + * before getting the current value.
>    */
>   static ssize_t amdgpu_debugfs_gfxoff_residency_read(struct file *f, char __user *buf,
>                                                      size_t size, loff_t *pos)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 62b5ad4d10b0..1bde0645c6f8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3725,7 +3725,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>          amdgpu_coredump_init(adev);
> 
>          adev->gfx.gfx_off_req_count = 1;
> -       adev->gfx.gfx_off_residency = 0;
>          adev->gfx.gfx_off_entrycount = 0;
>          adev->pm.ac_power = power_supply_is_system_supplied() > 0;
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index aefd4f03b443..4c1f8504ee1d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -493,7 +493,6 @@ struct amdgpu_gfx {
>          struct mutex                    gfx_off_mutex;      /* mutex to change gfxoff state */
>          uint32_t                        gfx_off_req_count;  /* default 1, enable gfx off: dec 1, disable gfx off: add 1 */
>          struct delayed_work             gfx_off_delay_work; /* async work to set gfx block off */
> -       uint32_t                        gfx_off_residency;  /* last logged residency */
>          uint64_t                        gfx_off_entrycount; /* count of times GPU has get into GFXOFF state */
> 
>          /* pipe reservation */
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> index f8fd93999617..d94e3dcf7f9a 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> @@ -1333,7 +1333,7 @@ struct pptable_funcs {
>          u32 (*set_gfx_off_residency)(struct smu_context *smu, bool start);
> 
>          /**
> -        * @get_gfx_off_residency: Average GFXOFF residency % during the logging interval
> +        * @get_gfx_off_residency: Live GFXOFF residency percentage
>           */
>          u32 (*get_gfx_off_residency)(struct smu_context *smu, uint32_t *residency);
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v11_5_ppsmc.h b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v11_5_ppsmc.h
> index 7471e2df2828..4206514765cd 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v11_5_ppsmc.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v11_5_ppsmc.h
> @@ -110,8 +110,10 @@
>   #define PPSMC_MSG_GetSlowPPTLimit                      0x4C
>   #define PPSMC_MSG_GetGfxOffStatus                     0x50
>   #define PPSMC_MSG_GetGfxOffEntryCount                 0x51
> -#define PPSMC_MSG_LogGfxOffResidency                  0x52
> -#define PPSMC_Message_Count                            0x53
> +#define PPSMC_MSG_GfxOffResidencyLogReadSample        0x52
> +#define PPSMC_MSG_StopGfxOffResidencyLogging            0x53
> +#define PPSMC_MSG_StartGfxOffResidencyLogging           0x56
> +#define PPSMC_Message_Count                            0x57
> 

So the definition of 0x52 changed rather than additive?  That will be a 
bit painful in the code.  You'll need to have a PMFW version lookup and 
decide how to treat it based on the version.

>   //Argument for PPSMC_MSG_GfxDeviceDriverReset  enum { diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
> index 636ff90923d9..acf03838d49d 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
> @@ -252,7 +252,9 @@
>          __SMU_DUMMY_MAP(DriverMode2Reset), \
>          __SMU_DUMMY_MAP(GetGfxOffStatus),                \
>          __SMU_DUMMY_MAP(GetGfxOffEntryCount),            \
> -       __SMU_DUMMY_MAP(LogGfxOffResidency),                    \
> +       __SMU_DUMMY_MAP(StartGfxOffResidencyLogging),           \
> +       __SMU_DUMMY_MAP(GfxOffResidencyLogReadSample),          \
> +       __SMU_DUMMY_MAP(StopGfxOffResidencyLogging),            \
>          __SMU_DUMMY_MAP(SetNumBadMemoryPagesRetired),           \
>          __SMU_DUMMY_MAP(SetBadMemoryPagesRetiredFlagsPerChannel), \
>          __SMU_DUMMY_MAP(AllowGpo),      \
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
> index 717ba46c8933..4212abfdbe53 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
> @@ -142,7 +142,9 @@ static struct cmn2asic_msg_mapping vangogh_message_map[SMU_MSG_MAX_COUNT] = {
>          MSG_MAP(GetSlowPPTLimit,                    PPSMC_MSG_GetSlowPPTLimit,                                          0),
>          MSG_MAP(GetGfxOffStatus,                    PPSMC_MSG_GetGfxOffStatus,                                          0),
>          MSG_MAP(GetGfxOffEntryCount,                PPSMC_MSG_GetGfxOffEntryCount,                                      0),
> -       MSG_MAP(LogGfxOffResidency,                 PPSMC_MSG_LogGfxOffResidency,                                       0),
> +       MSG_MAP(StartGfxOffResidencyLogging,  PPSMC_MSG_StartGfxOffResidencyLogging,    0),
> +       MSG_MAP(GfxOffResidencyLogReadSample, PPSMC_MSG_GfxOffResidencyLogReadSample,   0),
> +       MSG_MAP(StopGfxOffResidencyLogging,   PPSMC_MSG_StopGfxOffResidencyLogging,             0),
>   };
> 
>   static struct cmn2asic_mapping vangogh_feature_mask_map[SMU_FEATURE_COUNT] = { @@ -2450,19 +2452,20 @@ static int vangogh_set_power_limit(struct smu_context *smu,  static u32 vangogh_set_gfxoff_residency(struct smu_context *smu, bool start)  {
>          int ret = 0;
> -       u32 residency;
>          struct amdgpu_device *adev = smu->adev;
> 
>          if (!(adev->pm.pp_feature & PP_GFXOFF_MASK))
>                  return 0;
> 
> -       ret = smu_cmn_send_smc_msg_with_param(smu, SMU_MSG_LogGfxOffResidency,
> -                                             start, &residency);
> -       if (ret)
> -               return ret;
> -
> -       if (!start)
> -               adev->gfx.gfx_off_residency = residency;
> +       if (start) {
> +               ret = smu_cmn_send_smc_msg(smu, SMU_MSG_StartGfxOffResidencyLogging, NULL);

This needs a FW version guard.  Old FW won't have this command, newer will.

> +               if (ret)
> +                       return ret;
> +       } else {
> +               ret = smu_cmn_send_smc_msg(smu, SMU_MSG_StopGfxOffResidencyLogging, NULL);
> +               if (ret)
> +                       return ret;
> +       }
> 
>          return ret;
>   }
> @@ -2479,11 +2482,16 @@ static u32 vangogh_set_gfxoff_residency(struct smu_context *smu, bool start)
>    */
>   static u32 vangogh_get_gfxoff_residency(struct smu_context *smu, uint32_t *residency)  {
> +       int ret = 0;
>          struct amdgpu_device *adev = smu->adev;
> 
> -       *residency = adev->gfx.gfx_off_residency;
> +       if (!(adev->pm.pp_feature & PP_GFXOFF_MASK))
> +               return 0;
> 
> -       return 0;
> +       ret = smu_cmn_send_smc_msg(smu, SMU_MSG_GfxOffResidencyLogReadSample,
> +                                   residency);

Same here.

> +
> +       return ret;
>   }
> 
>   /**
> --
> 2.43.0
>