[PATCH 1/2] drm/amdgpu/gfx: adjust workload profile handling

[Alex Deucher]

On Fri, Mar 14, 2025 at 10:53 AM Lazar, Lijo <lijo.lazar at amd.com> wrote:
>
>
>
> On 3/14/2025 7:17 PM, Alex Deucher wrote:
> > No need to make the workload profile setup dependent
> > on the results of cancelling the delayed work thread.
> > We have all of the necessary checking in place for the
> > workload profile reference counting, so separate the
> > two.  As it is now, we can theoretically end up with
> > the call from begin_use happening while the worker
> > thread is executing which would result in the profile
> > not getting set for that submission.  It should not
> > affect the reference counting.
> >
> > Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 22 +++++++++++-----------
> >  1 file changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > index 099329d15b9ff..20424f8c4925f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > @@ -2188,18 +2188,18 @@ void amdgpu_gfx_profile_ring_begin_use(struct amdgpu_ring *ring)
> >
> >       atomic_inc(&adev->gfx.total_submission_cnt);
> >
> > -     if (!cancel_delayed_work_sync(&adev->gfx.idle_work)) {
> > -             mutex_lock(&adev->gfx.workload_profile_mutex);
> > -             if (!adev->gfx.workload_profile_active) {
> > -                     r = amdgpu_dpm_switch_power_profile(adev, profile, true);
> > -                     if (r)
> > -                             dev_warn(adev->dev, "(%d) failed to disable %s power profile mode\n", r,
> > -                                      profile == PP_SMC_POWER_PROFILE_FULLSCREEN3D ?
> > -                                      "fullscreen 3D" : "compute");
> > -                     adev->gfx.workload_profile_active = true;
> > -             }
> > -             mutex_unlock(&adev->gfx.workload_profile_mutex);
> > +     cancel_delayed_work_sync(&adev->gfx.idle_work);
> > +
>
> To avoid locking/unlocking mutex for each begin_use, I think here we
> could do like
>
> if (adev->gfx.workload_profile_active)
>         return;

But that is what the mutex is protecting.

Alex

>
> Thanks,
> Lijo
>
> > +     mutex_lock(&adev->gfx.workload_profile_mutex);
> > +     if (!adev->gfx.workload_profile_active) {
> > +             r = amdgpu_dpm_switch_power_profile(adev, profile, true);
> > +             if (r)
> > +                     dev_warn(adev->dev, "(%d) failed to disable %s power profile mode\n", r,
> > +                              profile == PP_SMC_POWER_PROFILE_FULLSCREEN3D ?
> > +                              "fullscreen 3D" : "compute");
> > +             adev->gfx.workload_profile_active = true;
> >       }
> > +     mutex_unlock(&adev->gfx.workload_profile_mutex);
> >  }
> >
> >  void amdgpu_gfx_profile_ring_end_use(struct amdgpu_ring *ring)
>