[Public]
> -----Original Message-----
> From: Chen, Guchun <Guchun.Chen@xxxxxxx>
> Sent: Thursday, January 6, 2022 9:22 PM
> To: Quan, Evan <Evan.Quan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Quan, Evan
> <Evan.Quan@xxxxxxx>
> Subject: RE: [PATCH 2/7] drm/amd/pm: drop unneeded vcn/jpeg_gate_lock
>
> [Public]
>
> err0_out:
> - mutex_unlock(&power_gate->jpeg_gate_lock);
> - mutex_unlock(&power_gate->vcn_gate_lock);
> -
> return ret;
>
> Is it better to refactor the code to drop error path of "err0_out"?
[Quan, Evan] Sure, I can do that.
BR
Evan
>
> Regards,
> Guchun
>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Evan
> Quan
> Sent: Thursday, January 6, 2022 1:57 PM
> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Quan, Evan
> <Evan.Quan@xxxxxxx>
> Subject: [PATCH 2/7] drm/amd/pm: drop unneeded vcn/jpeg_gate_lock
>
> As those related APIs are already protected by adev->pm.mutex.
>
> Signed-off-by: Evan Quan <evan.quan@xxxxxxx>
> Change-Id: I762fab96bb1c034c153b029f939ec6e498460007
> ---
> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 56 +++----------------
> drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 2 -
> 2 files changed, 8 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index ae91ef2078bf..ecbc768dfe2f 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -158,8 +158,8 @@ static u32 smu_get_sclk(void *handle, bool low)
> return clk_freq * 100;
> }
>
> -static int smu_dpm_set_vcn_enable_locked(struct smu_context *smu,
> - bool enable)
> +static int smu_dpm_set_vcn_enable(struct smu_context *smu,
> + bool enable)
> {
> struct smu_power_context *smu_power = &smu->smu_power;
> struct smu_power_gate *power_gate = &smu_power->power_gate;
> @@ -178,24 +178,8 @@ static int smu_dpm_set_vcn_enable_locked(struct
> smu_context *smu,
> return ret;
> }
>
> -static int smu_dpm_set_vcn_enable(struct smu_context *smu,
> - bool enable)
> -{
> - struct smu_power_context *smu_power = &smu->smu_power;
> - struct smu_power_gate *power_gate = &smu_power->power_gate;
> - int ret = 0;
> -
> - mutex_lock(&power_gate->vcn_gate_lock);
> -
> - ret = smu_dpm_set_vcn_enable_locked(smu, enable);
> -
> - mutex_unlock(&power_gate->vcn_gate_lock);
> -
> - return ret;
> -}
> -
> -static int smu_dpm_set_jpeg_enable_locked(struct smu_context *smu,
> - bool enable)
> +static int smu_dpm_set_jpeg_enable(struct smu_context *smu,
> + bool enable)
> {
> struct smu_power_context *smu_power = &smu->smu_power;
> struct smu_power_gate *power_gate = &smu_power->power_gate;
> @@ -214,22 +198,6 @@ static int smu_dpm_set_jpeg_enable_locked(struct
> smu_context *smu,
> return ret;
> }
>
> -static int smu_dpm_set_jpeg_enable(struct smu_context *smu,
> - bool enable)
> -{
> - struct smu_power_context *smu_power = &smu->smu_power;
> - struct smu_power_gate *power_gate = &smu_power->power_gate;
> - int ret = 0;
> -
> - mutex_lock(&power_gate->jpeg_gate_lock);
> -
> - ret = smu_dpm_set_jpeg_enable_locked(smu, enable);
> -
> - mutex_unlock(&power_gate->jpeg_gate_lock);
> -
> - return ret;
> -}
> -
> /**
> * smu_dpm_set_power_gate - power gate/ungate the specific IP block
> *
> @@ -619,17 +587,14 @@ static int smu_set_default_dpm_table(struct
> smu_context *smu)
> if (!smu->ppt_funcs->set_default_dpm_table)
> return 0;
>
> - mutex_lock(&power_gate->vcn_gate_lock);
> - mutex_lock(&power_gate->jpeg_gate_lock);
> -
> vcn_gate = atomic_read(&power_gate->vcn_gated);
> jpeg_gate = atomic_read(&power_gate->jpeg_gated);
>
> - ret = smu_dpm_set_vcn_enable_locked(smu, true);
> + ret = smu_dpm_set_vcn_enable(smu, true);
> if (ret)
> goto err0_out;
>
> - ret = smu_dpm_set_jpeg_enable_locked(smu, true);
> + ret = smu_dpm_set_jpeg_enable(smu, true);
> if (ret)
> goto err1_out;
>
> @@ -638,13 +603,10 @@ static int smu_set_default_dpm_table(struct
> smu_context *smu)
> dev_err(smu->adev->dev,
> "Failed to setup default dpm clock tables!\n");
>
> - smu_dpm_set_jpeg_enable_locked(smu, !jpeg_gate);
> + smu_dpm_set_jpeg_enable(smu, !jpeg_gate);
> err1_out:
> - smu_dpm_set_vcn_enable_locked(smu, !vcn_gate);
> + smu_dpm_set_vcn_enable(smu, !vcn_gate);
> err0_out:
> - mutex_unlock(&power_gate->jpeg_gate_lock);
> - mutex_unlock(&power_gate->vcn_gate_lock);
> -
> return ret;
> }
>
> @@ -1006,8 +968,6 @@ static int smu_sw_init(void *handle)
>
> atomic_set(&smu->smu_power.power_gate.vcn_gated, 1);
> atomic_set(&smu->smu_power.power_gate.jpeg_gated, 1);
> - mutex_init(&smu->smu_power.power_gate.vcn_gate_lock);
> - mutex_init(&smu->smu_power.power_gate.jpeg_gate_lock);
>
> smu->workload_mask = 1 << smu-
> >workload_prority[PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT];
> smu-
> >workload_prority[PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT] = 0; 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 00760f3c6da5..c3efe4fea5e0 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> @@ -376,8 +376,6 @@ struct smu_power_gate {
> bool vce_gated;
> atomic_t vcn_gated;
> atomic_t jpeg_gated;
> - struct mutex vcn_gate_lock;
> - struct mutex jpeg_gate_lock;
> };
>
> struct smu_power_context {
> --
> 2.29.0
