[PATCH] drm/amd/pm: avoid duplicate powergate/ungate setting
Lazar, Lijo
lijo.lazar at amd.com
Mon Nov 8 11:15:58 UTC 2021
On 11/8/2021 10:17 AM, Evan Quan wrote:
> Just bail out if the target IP block is already in the desired
> powergate/ungate state. This can avoid some duplicate settings
> which sometime may cause unexpected issues.
>
> Change-Id: I66346c69f121df0f5ee20182451313ae4fda2d04
> Signed-off-by: Evan Quan <evan.quan at amd.com>
> Tested-by: Borislav Petkov <bp at suse.de>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 7 +++++++
> drivers/gpu/drm/amd/include/amd_shared.h | 3 ++-
> drivers/gpu/drm/amd/pm/amdgpu_dpm.c | 13 ++++++++++++-
> drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c | 3 +++
> drivers/gpu/drm/amd/pm/powerplay/kv_dpm.c | 3 +++
> drivers/gpu/drm/amd/pm/powerplay/si_dpm.c | 3 +++
> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 3 +++
> 7 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index b85b67a88a3d..19fa21ad8a44 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -767,9 +767,16 @@ enum amd_hw_ip_block_type {
> #define HW_ID_MAX 300
> #define IP_VERSION(mj, mn, rv) (((mj) << 16) | ((mn) << 8) | (rv))
>
> +enum amd_ip_powergate_state {
> + POWERGATE_STATE_INITIAL,
> + POWERGATE_STATE_GATE,
> + POWERGATE_STATE_UNGATE,
> +};
> +
To reflect the current state, they could be named like
POWERGATE_STATE_UNKNOWN/ON/OFF.
> struct amd_powerplay {
> void *pp_handle;
> const struct amd_pm_funcs *pp_funcs;
> + atomic_t pg_state[AMD_IP_BLOCK_TYPE_NUM];
Maybe add another field in amdgpu_ip_block_status? Downside is it won't
reflect the PG state achieved through paths other than PMFW control and
ipblock needs to be queried through amdgpu_device_ip_get_ip_block()
Thanks,
Lijo
> };
>
> /* polaris10 kickers */
> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h b/drivers/gpu/drm/amd/include/amd_shared.h
> index f1a46d16f7ea..4b9e68a79f06 100644
> --- a/drivers/gpu/drm/amd/include/amd_shared.h
> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> @@ -98,7 +98,8 @@ enum amd_ip_block_type {
> AMD_IP_BLOCK_TYPE_ACP,
> AMD_IP_BLOCK_TYPE_VCN,
> AMD_IP_BLOCK_TYPE_MES,
> - AMD_IP_BLOCK_TYPE_JPEG
> + AMD_IP_BLOCK_TYPE_JPEG,
> + AMD_IP_BLOCK_TYPE_NUM,
> };
>
> enum amd_clockgating_state {
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> index 03581d5b1836..a6b337b6f4a9 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> @@ -927,6 +927,14 @@ int amdgpu_dpm_set_powergating_by_smu(struct amdgpu_device *adev, uint32_t block
> {
> int ret = 0;
> const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
> + enum amd_ip_powergate_state pg_state =
> + gate ? POWERGATE_STATE_GATE : POWERGATE_STATE_UNGATE;
> +
> + if (atomic_read(&adev->powerplay.pg_state[block_type]) == pg_state) {
> + dev_dbg(adev->dev, "IP block%d already in the target %s state!",
> + block_type, gate ? "gate" : "ungate");
> + return 0;
> + }
>
> switch (block_type) {
> case AMD_IP_BLOCK_TYPE_UVD:
> @@ -976,9 +984,12 @@ int amdgpu_dpm_set_powergating_by_smu(struct amdgpu_device *adev, uint32_t block
> }
> break;
> default:
> - break;
> + return -EINVAL;
> }
>
> + if (!ret)
> + atomic_set(&adev->powerplay.pg_state[block_type], pg_state);
> +
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> index bba7479f6265..8d8a7cf615eb 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> @@ -38,6 +38,7 @@ static const struct amd_pm_funcs pp_dpm_funcs;
> static int amd_powerplay_create(struct amdgpu_device *adev)
> {
> struct pp_hwmgr *hwmgr;
> + int i;
>
> if (adev == NULL)
> return -EINVAL;
> @@ -57,6 +58,8 @@ static int amd_powerplay_create(struct amdgpu_device *adev)
> hwmgr->display_config = &adev->pm.pm_display_cfg;
> adev->powerplay.pp_handle = hwmgr;
> adev->powerplay.pp_funcs = &pp_dpm_funcs;
> + for (i = 0; i < AMD_IP_BLOCK_TYPE_NUM; i++)
> + atomic_set(&adev->powerplay.pg_state[i], POWERGATE_STATE_INITIAL);
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/kv_dpm.c b/drivers/gpu/drm/amd/pm/powerplay/kv_dpm.c
> index bcae42cef374..f84f56552fd0 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/kv_dpm.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/kv_dpm.c
> @@ -2965,9 +2965,12 @@ static int kv_dpm_get_temp(void *handle)
> static int kv_dpm_early_init(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> + int i;
>
> adev->powerplay.pp_funcs = &kv_dpm_funcs;
> adev->powerplay.pp_handle = adev;
> + for (i = 0; i < AMD_IP_BLOCK_TYPE_NUM; i++)
> + atomic_set(&adev->powerplay.pg_state[i], POWERGATE_STATE_INITIAL);
> kv_dpm_set_irq_funcs(adev);
>
> return 0;
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c b/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
> index 81f82aa05ec2..f1eb22c9bb59 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
> @@ -7916,9 +7916,12 @@ static int si_dpm_early_init(void *handle)
> {
>
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> + int i;
>
> adev->powerplay.pp_funcs = &si_dpm_funcs;
> adev->powerplay.pp_handle = adev;
> + for (i = 0; i < AMD_IP_BLOCK_TYPE_NUM; i++)
> + atomic_set(&adev->powerplay.pg_state[i], POWERGATE_STATE_INITIAL);
> si_dpm_set_irq_funcs(adev);
> return 0;
> }
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index 01168b8955bf..fdc25bae8237 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -627,6 +627,7 @@ static int smu_early_init(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> struct smu_context *smu = &adev->smu;
> + int i;
>
> smu->adev = adev;
> smu->pm_enabled = !!amdgpu_dpm;
> @@ -639,6 +640,8 @@ static int smu_early_init(void *handle)
>
> adev->powerplay.pp_handle = smu;
> adev->powerplay.pp_funcs = &swsmu_pm_funcs;
> + for (i = 0; i < AMD_IP_BLOCK_TYPE_NUM; i++)
> + atomic_set(&adev->powerplay.pg_state[i], POWERGATE_STATE_INITIAL);
>
> return smu_set_funcs(adev);
> }
>
More information about the amd-gfx
mailing list