[PATCH 3/3] drm/amdgpu/pm: store power state per instance

[Lazar, Lijo]

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 at amd.com>
> Cc: Boyuan Zhang <boyuan.zhang at amd.com>
> ---
>  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