Fix the spelling mistake in the title of the patch,
"unlcok" --> "unlock".
Use present tense in the title and in commit text:
"Moving" --> "Move".
Shorten your title to fit within 80-char limit.
A Git hook checks for this and complains about it
when doing a git-push. I suggest:
drm/amdgpu: move the mutex lock/unlock out
And then you start your first sentence of your commit
message with the title as a capitalized sentence:
Move the mutext lock/unlock outside of the if(),
as the mutex is always taken: either in the if()
branch or in the else branch.
The commit message text should be 55-65 line width
aligned as Git indents it by 8 spaces when viewed
individually or in a log. (Modern Emacs recognizes
that you're writing a Git commit message and sets
the wrap at 55 chars.)
After all those are fixed, you can add,
Reviewed-by: Luben Tuikov <luben.tuikov@xxxxxxx>
On 2020-07-06 12:10 p.m., Alex Jivin wrote:
> Moving mutex unlock and lock outside of the "if" statement as it can be shown that
> the mutex will be taken and released, regardless of the value checked in the if statement.
>
> Signed-off-by: Alex Jivin <alex.jivin@xxxxxxx>
> Suggested-By: Luben Tukov <luben.tuikov@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 838d6d51904c..d2401379bd33 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -3559,16 +3559,14 @@ void amdgpu_dpm_enable_uvd(struct amdgpu_device *adev, bool enable)
> int ret = 0;
>
> if (adev->family == AMDGPU_FAMILY_SI) {
> + mutex_lock(&adev->pm.mutex);
> if (enable) {
> - mutex_lock(&adev->pm.mutex);
> adev->pm.dpm.uvd_active = true;
> adev->pm.dpm.state = POWER_STATE_TYPE_INTERNAL_UVD;
> - mutex_unlock(&adev->pm.mutex);
> } else {
> - mutex_lock(&adev->pm.mutex);
> adev->pm.dpm.uvd_active = false;
> - mutex_unlock(&adev->pm.mutex);
> }
> + mutex_unlock(&adev->pm.mutex);
>
> amdgpu_pm_compute_clocks(adev);
> } else {
> @@ -3596,17 +3594,15 @@ void amdgpu_dpm_enable_vce(struct amdgpu_device *adev, bool enable)
> int ret = 0;
>
> if (adev->family == AMDGPU_FAMILY_SI) {
> + mutex_lock(&adev->pm.mutex);
> if (enable) {
> - mutex_lock(&adev->pm.mutex);
> adev->pm.dpm.vce_active = true;
> /* XXX select vce level based on ring/task */
> adev->pm.dpm.vce_level = AMD_VCE_LEVEL_AC_ALL;
> - mutex_unlock(&adev->pm.mutex);
> } else {
> - mutex_lock(&adev->pm.mutex);
> adev->pm.dpm.vce_active = false;
> - mutex_unlock(&adev->pm.mutex);
> }
> + mutex_unlock(&adev->pm.mutex);
>
> amdgpu_pm_compute_clocks(adev);
> } else {
>
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
