On 3/15/2025 12:15 AM, 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.
> 
> v2: bail early if the the profile is already active (Lijo)
> 
> Signed-off-by: Alex Deucher <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 29 +++++++++++++++----------
>  1 file changed, 18 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..4beb0609e7034 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -2188,18 +2188,25 @@ 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);
> +
> +     /* We can safely return early here because we've cancelled the
> +      * the delayed work so there is no one else to set it to false
> +      * and we don't care if someone else sets it to true.
> +      */

The same may be extended to vcn also. Anyway, series is

        Reviewed-by: Lijo Lazar <[email protected]>

Thanks,
Lijo

> +     if (adev->gfx.workload_profile_active)
> +             return;
> +
> +     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)

Reply via email to