Re: [PATCH V3] drm/amd/pm: avoid duplicate powergate/ungate setting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11/15/2021 8:21 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 sometimes may cause unexpected issues.

Link: https://lore.kernel.org/all/YV81vidWQLWvATMM@xxxxxxx/

Change-Id: I66346c69f121df0f5ee20182451313ae4fda2d04
Signed-off-by: Evan Quan <evan.quan@xxxxxxx>
Tested-by: Borislav Petkov <bp@xxxxxxx>
--
v1->v2:
   - typo fix and add link for the issue referred in commit
     message(Paul/Boris)
   - update the data type to uint32_t(Paul)
   - better Macro naming(Lijo)
v2->v3:
   - stick to original logics on handling unmentioned IP blocks(Lijo)
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  3 +++
  drivers/gpu/drm/amd/include/amd_shared.h   |  3 ++-
  drivers/gpu/drm/amd/pm/amdgpu_dpm.c        | 10 ++++++++++
  drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h    |  8 ++++++++
  4 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 0bd90ec9e43e..fca592394fa1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3508,6 +3508,9 @@ 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);
+
  	adev->rmmio = ioremap(adev->rmmio_base, adev->rmmio_size);
  	if (adev->rmmio == NULL) {
  		return -ENOMEM;
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..08362d506534 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
@@ -927,6 +927,13 @@ 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 ip_power_state pwr_state = gate ? POWER_STATE_OFF : POWER_STATE_ON;
+
+	if (atomic_read(&adev->pm.pwr_state[block_type]) == pwr_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:
@@ -979,6 +986,9 @@ int amdgpu_dpm_set_powergating_by_smu(struct amdgpu_device *adev, uint32_t block
  		break;
  	}
+ if (!ret)
+		atomic_set(&adev->pm.pwr_state[block_type], pwr_state);
+

Once the default case is skipped, this will set the state for unhandled blocks. Should be fine if such blocks are not expected. Otherwise, just return 0 such that they hold unknown power states.

Reviewed-by: Lijo Lazar <lijo.lazar@xxxxxxx>

Thanks,
Lijo
  	return ret;
  }
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
index 98f1b3d8c1d5..16e3f72d31b9 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
@@ -417,6 +417,12 @@ struct amdgpu_dpm {
  	enum amd_dpm_forced_level forced_level;
  };
+enum ip_power_state {
+	POWER_STATE_UNKNOWN,
+	POWER_STATE_ON,
+	POWER_STATE_OFF,
+};
+
  struct amdgpu_pm {
  	struct mutex		mutex;
  	u32                     current_sclk;
@@ -452,6 +458,8 @@ struct amdgpu_pm {
  	struct i2c_adapter smu_i2c;
  	struct mutex		smu_i2c_mutex;
  	struct list_head	pm_attr_list;
+
+	atomic_t		pwr_state[AMD_IP_BLOCK_TYPE_NUM];
  };
#define R600_SSTU_DFLT 0




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux