[Public]
> -----Original Message-----
> From: Liang, Prike <Prike.Liang@xxxxxxx>
> Sent: Thursday, November 9, 2023 2:37 AM
> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Liang, Prike
> <Prike.Liang@xxxxxxx>
> Subject: [PATCH 1/2] drm/amdgpu: correct the amdgpu runtime dereference
> usage count
>
> Fix the amdgpu runpm dereference usage count.
>
> Signed-off-by: Prike Liang <Prike.Liang@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index a53f436fa9f1..f6bbbbe5d9f7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -1992,7 +1992,7 @@ static int amdgpu_debugfs_sclk_set(void *data,
> u64 val)
>
> ret = amdgpu_dpm_set_soft_freq_range(adev, PP_SCLK,
> (uint32_t)val, (uint32_t)val);
> if (ret)
> - ret = -EINVAL;
> + goto out;
I think this hunk can be dropped. It doesn't really change anything. Or you could just drop the whole ret check since we just return ret at the end anyway. Not sure if changing the error code is important here or not.
>
> out:
> pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index 0cacd0b9f8be..ff1f42ae6d8e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -346,6 +346,7 @@ int amdgpu_display_crtc_set_config(struct
> drm_mode_set *set,
> if (!active && adev->have_disp_power_ref) {
> pm_runtime_put_autosuspend(dev->dev);
> adev->have_disp_power_ref = false;
> + return ret;
> }
I think it would be cleaner to just drop the runtime_put above and update the comment. We'll just fall through to the end of the function.
Alex
>
> out:
> --
> 2.34.1
