AMD General

> For that reason, I’d like to keep the original placement: restore at the end 
> of
> amdgpu_pmops_runtime_resume(), and only re-assert UMD stable pstate
> when current level is a profile forced level.
> This keeps smu_late_init() for normal policy replay, while ensuring perfmon
> state is restored at a point where it is not overwritten afterward.
>

I agree with your point, please isolate all runpm-related logic away from SWSMU.
And also split this patch into two separate parts: one for KFD, the other for 
PM.
For the PM parts, please add my RB:

Reviewed-by: Yang Wang <[email protected]>

Best Regards,
Kevin

> -----Original Message-----
> From: amd-gfx <[email protected]> On Behalf Of Li,
> Candice
> Sent: Friday, July 24, 2026 11:08 AM
> To: Li, Candice <[email protected]>; Lazar, Lijo <[email protected]>;
> Zhang, Hawking <[email protected]>; [email protected]
> Subject: RE: [PATCH] drm/amdgpu: restore perf state after runtime resume
>
> AMD General
>
> Hi Lijo,
>
> I want to revise my earlier agreement after rechecking the full runtime-resume
> flow.
>
> I agree profile/forced DPM is a user policy and D3 can lose HW programming,
> but for this bug the key is restore timing.
> If we do this in smu_late_init(), later resume stages can still touch CG/PG/GC
> and clear PERFMON_CLOCK_STATE again, so the restore is not persistent.
>
> For that reason, I’d like to keep the original placement: restore at the end 
> of
> amdgpu_pmops_runtime_resume(), and only re-assert UMD stable pstate
> when current level is a profile forced level.
> This keeps smu_late_init() for normal policy replay, while ensuring perfmon
> state is restored at a point where it is not overwritten afterward.
>
>
> Thanks,
> Candice
>
> -----Original Message-----
> From: amd-gfx <[email protected]> On Behalf Of Li,
> Candice
> Sent: Thursday, July 23, 2026 2:16 PM
> To: Lazar, Lijo <[email protected]>; Zhang, Hawking
> <[email protected]>; [email protected]
> Subject: RE: [PATCH] drm/amdgpu: restore perf state after runtime resume
>
> AMD General
>
> -----Original Message-----
> From: Lazar, Lijo <[email protected]>
> Sent: Thursday, July 23, 2026 11:46 AM
> To: Li, Candice <[email protected]>; Zhang, Hawking
> <[email protected]>; [email protected]
> Subject: Re: [PATCH] drm/amdgpu: restore perf state after runtime resume
>
>
>
> On 23-Jul-26 9:00 AM, Li, Candice wrote:
> > AMD General
> >
> > -----Original Message-----
> > From: Lazar, Lijo <[email protected]>
> > Sent: Wednesday, July 22, 2026 8:11 PM
> > To: Zhang, Hawking <[email protected]>; Li, Candice
> > <[email protected]>; [email protected]
> > Subject: Re: [PATCH] drm/amdgpu: restore perf state after runtime
> > resume
> >
> >
> >
> > On 22-Jul-26 4:43 PM, Zhang, Hawking wrote:
> >> AMD General
> >>
> >> Reviewed-by: Hawking Zhang <[email protected]>
> >>
> >> Regards,
> >> Hawking
> >> -----Original Message-----
> >> From: amd-gfx <[email protected]> On Behalf Of
> >> Candice Li
> >> Sent: Wednesday, July 22, 2026 6:36 PM
> >> To: [email protected]
> >> Cc: Li, Candice <[email protected]>
> >> Subject: [PATCH] drm/amdgpu: restore perf state after runtime resume
> >>
> >> Runtime suspend runs GFX hw_fini and clears perfmon clock gating while
> KFD keeps running, so block perf counters read zero after resume even
> though profiling remains enabled in software.
> >>
> >> Re-apply UMD stable pstate for profile DPM modes and re-enable KFD
> perfcount after a successful runtime resume.
> >>
> >> Signed-off-by: Candice Li <[email protected]>
> >> ---
> >>    drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 46
> ++++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h |  1 +
> >>    drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    | 17 ++++++++
> >>    drivers/gpu/drm/amd/amdkfd/kfd_chardev.c   |  8 ++++
> >>    drivers/gpu/drm/amd/amdkfd/kfd_priv.h      |  2 +
> >>    drivers/gpu/drm/amd/amdkfd/kfd_process.c   |  1 +
> >>    6 files changed, 75 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> >> index 121282dd30c161..38974beb98ef88 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> >> @@ -38,6 +38,7 @@
> >>    #include "amdgpu_reset.h"
> >>    #if IS_ENABLED(CONFIG_HSA_AMD)
> >>    #include "kfd_priv.h"
> >> +#include "kfd_device_queue_manager.h"
> >>    #endif
> >>
> >>    /* Total memory size in system memory and all GPU VRAM. Used to @@
> -280,6 +281,51 @@ int amdgpu_amdkfd_resume(struct amdgpu_device
> *adev, bool resume_proc)
> >>           return r;
> >>    }
> >>
> >> +void amdgpu_amdkfd_restore_profiler_after_runpm(struct
> amdgpu_device
> >> +*adev) { #if IS_ENABLED(CONFIG_HSA_AMD)
> >> +       struct kfd_dev *kfd;
> >> +       struct kfd_process *p;
> >> +       struct kfd_process_device *pdd;
> >> +       struct device_queue_manager *dqm;
> >> +       int i;
> >> +
> >> +       if (!adev->kfd.dev || !adev->kfd.init_complete)
> >> +               return;
> >> +
> >> +       kfd = adev->kfd.dev;
> >> +
> >> +       mutex_lock(&kfd->profiler_lock);
> >> +       p = kfd->profiler_process;
> >> +       if (!p || !kfd->profiler_perfcount_enabled) {
> >> +               mutex_unlock(&kfd->profiler_lock);
> >> +               return;
> >> +       }
> >> +       mutex_unlock(&kfd->profiler_lock);
> >> +
> >> +       mutex_lock(&p->mutex);
> >> +
> >> +       mutex_lock(&kfd->profiler_lock);
> >> +       if (kfd->profiler_process != p || 
> >> !kfd->profiler_perfcount_enabled) {
> >> +               mutex_unlock(&kfd->profiler_lock);
> >> +               mutex_unlock(&p->mutex);
> >> +               return;
> >> +       }
> >> +       mutex_unlock(&kfd->profiler_lock);
> >> +
> >> +       for (i = 0; i < p->n_pdds; i++) {
> >> +               pdd = p->pdds[i];
> >> +               if (!pdd || !pdd->dev || pdd->dev->kfd != kfd)
> >> +                       continue;
> >> +
> >> +               dqm = pdd->dev->dqm;
> >> +               if (dqm && dqm->ops.set_perfcount)
> >> +                       dqm->ops.set_perfcount(dqm, 1);
> >> +       }
> >> +       mutex_unlock(&p->mutex);
> >> +#endif
> >> +}
> >> +
> >>    void amdgpu_amdkfd_suspend_process(struct amdgpu_device *adev)  {
> >>           if (adev->kfd.dev)
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> >> index 338412a750edf0..1be9d5ef97d6d0 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> >> @@ -164,6 +164,7 @@ void amdgpu_amdkfd_teardown_processes(struct
> >> amdgpu_device *adev);
> >>
> >>    void amdgpu_amdkfd_suspend(struct amdgpu_device *adev, bool
> >> suspend_proc);  int amdgpu_amdkfd_resume(struct amdgpu_device
> *adev,
> >> bool resume_proc);
> >> +void amdgpu_amdkfd_restore_profiler_after_runpm(struct
> amdgpu_device
> >> +*adev);
> >>    void amdgpu_amdkfd_suspend_process(struct amdgpu_device *adev);
> >> int amdgpu_amdkfd_resume_process(struct amdgpu_device *adev);  void
> >> amdgpu_amdkfd_interrupt(struct amdgpu_device *adev, diff --git
> >> a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> index 5362705143bc75..f7a1064c340692 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> @@ -2920,6 +2920,21 @@ static int
> amdgpu_pmops_runtime_suspend(struct device *dev)
> >>           return 0;
> >>    }
> >>
> >> +static void amdgpu_restore_perf_state_after_runpm(struct
> >> +amdgpu_device
> >> +*adev) {
> >> +       enum amd_dpm_forced_level level;
> >> +       uint32_t profile_mode_mask =
> AMD_DPM_FORCED_LEVEL_PROFILE_STANDARD |
> >> +               AMD_DPM_FORCED_LEVEL_PROFILE_MIN_SCLK |
> >> +               AMD_DPM_FORCED_LEVEL_PROFILE_MIN_MCLK |
> >> +               AMD_DPM_FORCED_LEVEL_PROFILE_PEAK;
> >> +
> >> +       level = amdgpu_dpm_get_performance_level(adev);
> >> +       if (level & profile_mode_mask)
> >> +               amdgpu_asic_update_umd_stable_pstate(adev, true);
> >
> > This is supposed to be called before entering the force dpm level mode.
> > It's better to move this logic inside swsmu late_init before
> > reapplying the level.
> >
> > Thanks,
> > Lijo
> >
> > Thanks, Lijo.
> >
> > I think this logic should remain in runtime resume. The
> amdgpu_asic_update_umd_stable_pstate(adev, true) call here is a restore/re-
> apply step for an already-selected profile forced level, not a force-level 
> entry.
> > Enter/exit behavior is already covered by the regular DPM force path
> (amdgpu_dpm_force_performance_level() / SMU force handling) when level
> transitions occur.
> >
> > During runpm suspend/resume, the software forced level can remain profile
> mode, but HW-side stable pstate (and KFD profiler state) may be cleared by
> hw_fini/hw_init.
> > That is why this restore is done after amdgpu_device_resume().
> >
> > Moving this into swsmu late_init would apply the logic to broader flows (not
> only runpm), so I’d prefer to keep it in runpm resume.
> >
>
>
> Profile setting is a user level setting and it's not the default SOC behavior.
> During runtime PM, chip is in ~D3 state and some settings are lost. For proper
> restoration, all the steps that are followed for a regular user profile 
> setting
> needs to be done (you don't need to go by the 'force' name). That is done in
> the logic here also - it's checking if the level is one of the forced dpm 
> levels. To
> properly reapply, it needs to follow the same process.
>
> This is followed in other user setting scenarios also. smu late init has a
> specific call to restore user setttings.
>
> Thanks,
> Lijo
>
> Thanks, Lijo.
> Good point - profile mode is user-level state and after runpm we should
> replay it through the same user-setting restore path, not only re-enable UMD
> stable pstate.
> I’ll move this logic into the swsmu late init restore flow (alongside user 
> profile
> restoration) so profile reapply follows the same sequence as normal user
> profile programming.
> Then runtime-resume side will keep only kfd profiler restore hook.
>
> Thanks,
> Candice
>
> >
> > Thanks,
> > Candice
> >
> >> +
> >> +       amdgpu_amdkfd_restore_profiler_after_runpm(adev);
> >> +}
> >> +
> >>    static int amdgpu_pmops_runtime_resume(struct device *dev)  {
> >>           struct pci_dev *pdev = to_pci_dev(dev); @@ -2964,6 +2979,8
> >> @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
> >>
> >>           if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
> >>                   drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
> >> +
> >> +       amdgpu_restore_perf_state_after_runpm(adev);
> >>           adev->in_runpm = false;
> >>           return 0;
> >>    }
> >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> >> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> >> index 2c1a936459ac87..2a140c40b53f9c 100644
> >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> >> @@ -3347,6 +3347,7 @@ static inline uint32_t profile_lock_device(struct
> kfd_process *p,
> >>           if (op == 1) {
> >>                   if (!kfd->profiler_process) {
> >>                           kfd->profiler_process = p;
> >> +                       kfd->profiler_perfcount_enabled = false;
> >>                           status = 0;
> >>                           mutex_unlock(&kfd->profiler_lock);
> >>                           if (ptl->hw_supported) { @@ -3364,6 +3365,7
> >> @@ static inline uint32_t profile_lock_device(struct kfd_process *p,
> >>                   }
> >>           } else if (op == 0 && kfd->profiler_process == p) {
> >>                   kfd->profiler_process = NULL;
> >> +               kfd->profiler_perfcount_enabled = false;
> >>                   status = 0;
> >>                   mutex_unlock(&kfd->profiler_lock);
> >>
> >> @@ -3404,6 +3406,12 @@ static inline int kfd_profiler_pmc(struct
> kfd_process *p,
> >>           mutex_unlock(&p->mutex);
> >>
> >>           dqm->ops.set_perfcount(dqm, args->perfcount_enable);
> >> +
> >> +       mutex_lock(&pdd->dev->kfd->profiler_lock);
> >> +       if (pdd->dev->kfd->profiler_process == p)
> >> +               pdd->dev->kfd->profiler_perfcount_enabled = !!args-
> >perfcount_enable;
> >> +       mutex_unlock(&pdd->dev->kfd->profiler_lock);
> >> +
> >>           return status;
> >>    }
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> >> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> >> index 88191a4c165796..81055ada4f80b8 100644
> >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> >> @@ -389,6 +389,8 @@ struct kfd_dev {
> >>           struct mutex profiler_lock;
> >>           /* Process currently holding the lock */
> >>           struct kfd_process *profiler_process;
> >> +       /* Whether perfcount is enabled for profiler_process */
> >> +       bool profiler_perfcount_enabled;
> >>    };
> >>
> >>    enum kfd_mempool {
> >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> >> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> >> index eb508fe3ded780..8b7020fc0577a2 100644
> >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> >> @@ -1180,6 +1180,7 @@ static void kfd_process_profiler_release(struct
> kfd_process *p, struct kfd_proce
> >>           if (pdd->dev->kfd->profiler_process == p) {
> >>                   pdd->qpd.dqm->ops.set_perfcount(pdd->qpd.dqm, 0);
> >>                   pdd->dev->kfd->profiler_process = NULL;
> >> +               pdd->dev->kfd->profiler_perfcount_enabled = false;
> >>           }
> >>           mutex_unlock(&pdd->dev->kfd->profiler_lock);
> >>    }
> >> --
> >> 2.25.1
> >>
> >

Reply via email to