On 2/11/2025 5:01 AM, Alex Deucher wrote:
> Simplify the driver logic. We support multiple instances
> now so reflect that in the driver state.
>
> Fixes: ff69bba05f08 ("drm/amd/pm: add inst to dpm_set_powergating_by_smu")
> Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
> Cc: Boyuan Zhang <boyuan.zhang@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 9 ++++++---
> drivers/gpu/drm/amd/include/amd_shared.h | 2 ++
> drivers/gpu/drm/amd/pm/amdgpu_dpm.c | 16 ++++++++++------
> drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h | 2 +-
> 4 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 32926458d50ce..7325c21d60fc8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -4184,7 +4184,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> {
> struct drm_device *ddev = adev_to_drm(adev);
> struct pci_dev *pdev = adev->pdev;
> - int r, i;
> + int r, i, j;
> bool px = false;
> u32 max_MBps;
> int tmp;
> @@ -4338,8 +4338,11 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> adev->rmmio_size = pci_resource_len(adev->pdev, 2);
> }
>
> - for (i = 0; i < AMD_IP_BLOCK_TYPE_NUM; i++)
> - atomic_set(&adev->pm.pwr_state[i], POWER_STATE_UNKNOWN);
> + for (i = 0; i < AMD_IP_BLOCK_TYPE_NUM; i++) {
> + for (j = 0; j < AMD_IP_BLOCK_MAX_INST; j++) {
> + atomic_set(&adev->pm.pwr_state[i][j], POWER_STATE_UNKNOWN);
> + }
> + }
>
> adev->rmmio = ioremap(adev->rmmio_base, adev->rmmio_size);
> if (!adev->rmmio)
> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h b/drivers/gpu/drm/amd/include/amd_shared.h
> index 6dccee403a3d1..5dee173485c9f 100644
> --- a/drivers/gpu/drm/amd/include/amd_shared.h
> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> @@ -112,6 +112,8 @@ enum amd_ip_block_type {
> AMD_IP_BLOCK_TYPE_NUM,
> };
>
> +#define AMD_IP_BLOCK_MAX_INST 8
> +
> enum amd_clockgating_state {
> AMD_CG_STATE_GATE = 0,
> AMD_CG_STATE_UNGATE,
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> index 7a22aef6e59c3..d2f1b27d22d4f 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> @@ -78,12 +78,16 @@ int amdgpu_dpm_set_powergating_by_smu(struct amdgpu_device *adev,
> int ret = 0;
> const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
> enum ip_power_state pwr_state = gate ? POWER_STATE_OFF : POWER_STATE_ON;
> - bool is_vcn = block_type == AMD_IP_BLOCK_TYPE_VCN;
>
> - if (atomic_read(&adev->pm.pwr_state[block_type]) == pwr_state &&
> - (!is_vcn || adev->vcn.num_vcn_inst == 1)) {
> - dev_dbg(adev->dev, "IP block%d already in the target %s state!",
> - block_type, gate ? "gate" : "ungate");
> + if (inst >= AMD_IP_BLOCK_MAX_INST) {
> + dev_err(adev->dev, "IP block %d inst %d invalid!",
> + block_type, inst);
> + return -EINVAL;
> + }
> +
> + if (atomic_read(&adev->pm.pwr_state[block_type][inst]) == pwr_state) {
> + dev_dbg(adev->dev, "IP block %d inst %d already in the target %s state!",
> + block_type, inst, gate ? "gate" : "ungate");
> return 0;
> }
>
> @@ -112,7 +116,7 @@ int amdgpu_dpm_set_powergating_by_smu(struct amdgpu_device *adev,
> }
>
> if (!ret)
> - atomic_set(&adev->pm.pwr_state[block_type], pwr_state);
> + atomic_set(&adev->pm.pwr_state[block_type][inst], pwr_state);
>
> mutex_unlock(&adev->pm.mutex);
>
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> index 1f5ac7e0230d2..cbb110f0cb898 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> @@ -370,7 +370,7 @@ struct amdgpu_pm {
> struct i2c_adapter *fru_eeprom_i2c_bus;
> struct list_head pm_attr_list;
>
> - atomic_t pwr_state[AMD_IP_BLOCK_TYPE_NUM];
> + atomic_t pwr_state[AMD_IP_BLOCK_TYPE_NUM][AMD_IP_BLOCK_MAX_INST];
If it's getting refactored, it's better to keep this state in IP block
than in pm.
Thanks,
Lijo
>
> /*
> * 0 = disabled (default), otherwise enable corresponding debug mode
