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

["Lazar, Lijo" <lijo.lazar@amd.com>]

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@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;

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)