[PATCH 1/4] drm/amdgpu: correct clock/power gating sequence on UVD/VCE suspend/resume

[PATCH 1/4] drm/amdgpu: correct clock/power gating sequence on UVD/VCE suspend/resume

[Evan Quan]

The clocks should be gated before power. And reverse sequence should be
used on ungating.

Change-Id: Iab09f1f616560ff1083b75e95bfc6433d05d7f98
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c       |  8 +++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c       |  8 +++----
 .../powerplay/hwmgr/smu7_clockpowergating.c   | 24 +++++++++----------
 .../drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c   | 24 +++++++++----------
 4 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index 445480b50f48..859840ac5f0b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -1238,10 +1238,10 @@ static void amdgpu_uvd_idle_work_handler(struct work_struct *work)
 		} else {
 			amdgpu_asic_set_uvd_clocks(adev, 0, 0);
 			/* shutdown the UVD block */
-			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
-							       AMD_PG_STATE_GATE);
 			amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
 							       AMD_CG_STATE_GATE);
+			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
+							       AMD_PG_STATE_GATE);
 		}
 	} else {
 		schedule_delayed_work(&adev->uvd.idle_work, UVD_IDLE_TIMEOUT);
@@ -1262,10 +1262,10 @@ void amdgpu_uvd_ring_begin_use(struct amdgpu_ring *ring)
 			amdgpu_dpm_enable_uvd(adev, true);
 		} else {
 			amdgpu_asic_set_uvd_clocks(adev, 53300, 40000);
-			amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
-							       AMD_CG_STATE_UNGATE);
 			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
 							       AMD_PG_STATE_UNGATE);
+			amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
+							       AMD_CG_STATE_UNGATE);
 		}
 	}
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
index 7ad83da613ed..21b4fc48d33f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
@@ -344,10 +344,10 @@ static void amdgpu_vce_idle_work_handler(struct work_struct *work)
 			amdgpu_dpm_enable_vce(adev, false);
 		} else {
 			amdgpu_asic_set_vce_clocks(adev, 0, 0);
-			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCE,
-							       AMD_PG_STATE_GATE);
 			amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_VCE,
 							       AMD_CG_STATE_GATE);
+			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCE,
+							       AMD_PG_STATE_GATE);
 		}
 	} else {
 		schedule_delayed_work(&adev->vce.idle_work, VCE_IDLE_TIMEOUT);
@@ -376,10 +376,10 @@ void amdgpu_vce_ring_begin_use(struct amdgpu_ring *ring)
 			amdgpu_dpm_enable_vce(adev, true);
 		} else {
 			amdgpu_asic_set_vce_clocks(adev, 53300, 40000);
-			amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_VCE,
-							       AMD_CG_STATE_UNGATE);
 			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCE,
 							       AMD_PG_STATE_UNGATE);
+			amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_VCE,
+							       AMD_CG_STATE_UNGATE);
 
 		}
 	}
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_clockpowergating.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_clockpowergating.c
index f2bda3bcbbde..e1f85f777eac 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_clockpowergating.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_clockpowergating.c
@@ -118,22 +118,22 @@ void smu7_powergate_uvd(struct pp_hwmgr *hwmgr, bool bgate)
 	data->uvd_power_gated = bgate;
 
 	if (bgate) {
-		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
-						AMD_IP_BLOCK_TYPE_UVD,
-						AMD_PG_STATE_GATE);
 		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
 				AMD_IP_BLOCK_TYPE_UVD,
 				AMD_CG_STATE_GATE);
+		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
+						AMD_IP_BLOCK_TYPE_UVD,
+						AMD_PG_STATE_GATE);
 		smu7_update_uvd_dpm(hwmgr, true);
 		smu7_powerdown_uvd(hwmgr);
 	} else {
 		smu7_powerup_uvd(hwmgr);
-		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
-				AMD_IP_BLOCK_TYPE_UVD,
-				AMD_CG_STATE_UNGATE);
 		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
 						AMD_IP_BLOCK_TYPE_UVD,
 						AMD_PG_STATE_UNGATE);
+		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
+				AMD_IP_BLOCK_TYPE_UVD,
+				AMD_CG_STATE_UNGATE);
 		smu7_update_uvd_dpm(hwmgr, false);
 	}
 
@@ -146,22 +146,22 @@ void smu7_powergate_vce(struct pp_hwmgr *hwmgr, bool bgate)
 	data->vce_power_gated = bgate;
 
 	if (bgate) {
-		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
-						AMD_IP_BLOCK_TYPE_VCE,
-						AMD_PG_STATE_GATE);
 		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
 				AMD_IP_BLOCK_TYPE_VCE,
 				AMD_CG_STATE_GATE);
+		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
+						AMD_IP_BLOCK_TYPE_VCE,
+						AMD_PG_STATE_GATE);
 		smu7_update_vce_dpm(hwmgr, true);
 		smu7_powerdown_vce(hwmgr);
 	} else {
 		smu7_powerup_vce(hwmgr);
-		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
-				AMD_IP_BLOCK_TYPE_VCE,
-				AMD_CG_STATE_UNGATE);
 		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
 						AMD_IP_BLOCK_TYPE_VCE,
 						AMD_PG_STATE_UNGATE);
+		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
+				AMD_IP_BLOCK_TYPE_VCE,
+				AMD_CG_STATE_UNGATE);
 		smu7_update_vce_dpm(hwmgr, false);
 	}
 }
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
index b94a77e4e714..a6147db548ca 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
@@ -1957,22 +1957,22 @@ static void smu8_dpm_powergate_uvd(struct pp_hwmgr *hwmgr, bool bgate)
 	data->uvd_power_gated = bgate;
 
 	if (bgate) {
-		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
-						AMD_IP_BLOCK_TYPE_UVD,
-						AMD_PG_STATE_GATE);
 		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
 						AMD_IP_BLOCK_TYPE_UVD,
 						AMD_CG_STATE_GATE);
+		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
+						AMD_IP_BLOCK_TYPE_UVD,
+						AMD_PG_STATE_GATE);
 		smu8_dpm_update_uvd_dpm(hwmgr, true);
 		smu8_dpm_powerdown_uvd(hwmgr);
 	} else {
 		smu8_dpm_powerup_uvd(hwmgr);
-		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
-						AMD_IP_BLOCK_TYPE_UVD,
-						AMD_CG_STATE_UNGATE);
 		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
 						AMD_IP_BLOCK_TYPE_UVD,
 						AMD_PG_STATE_UNGATE);
+		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
+						AMD_IP_BLOCK_TYPE_UVD,
+						AMD_CG_STATE_UNGATE);
 		smu8_dpm_update_uvd_dpm(hwmgr, false);
 	}
 
@@ -1983,24 +1983,24 @@ static void smu8_dpm_powergate_vce(struct pp_hwmgr *hwmgr, bool bgate)
 	struct smu8_hwmgr *data = hwmgr->backend;
 
 	if (bgate) {
-		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
-					AMD_IP_BLOCK_TYPE_VCE,
-					AMD_PG_STATE_GATE);
 		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
 					AMD_IP_BLOCK_TYPE_VCE,
 					AMD_CG_STATE_GATE);
+		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
+					AMD_IP_BLOCK_TYPE_VCE,
+					AMD_PG_STATE_GATE);
 		smu8_enable_disable_vce_dpm(hwmgr, false);
 		smu8_dpm_powerdown_vce(hwmgr);
 		data->vce_power_gated = true;
 	} else {
 		smu8_dpm_powerup_vce(hwmgr);
 		data->vce_power_gated = false;
-		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
-					AMD_IP_BLOCK_TYPE_VCE,
-					AMD_CG_STATE_UNGATE);
 		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
 					AMD_IP_BLOCK_TYPE_VCE,
 					AMD_PG_STATE_UNGATE);
+		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
+					AMD_IP_BLOCK_TYPE_VCE,
+					AMD_CG_STATE_UNGATE);
 		smu8_dpm_update_vce_dpm(hwmgr);
 		smu8_enable_disable_vce_dpm(hwmgr, true);
 	}
-- 
2.29.0

[PATCH 2/4] drm/amdgpu: add missing cleanups for Polaris12 UVD/VCE on suspend

[Evan Quan]

Perform proper cleanups on UVD/VCE suspend: powergate enablement,
clockgating enablement and dpm disablement. This can fix some hangs
observed on suspending when UVD/VCE still using(e.g. issue
"pm-suspend" when video is still playing).

Change-Id: I36f39d9731e0a9638b52d5d92558b0ee9c23a9ed
Signed-off-by: Evan Quan <evan.quan@amd.com>
Signed-off-by: xinhui pan <xinhui.pan@amd.com>
--
v1->v2:
  - move the changes to ->hw_fini() (James Zhu)
v2->v3:
  - correct the sequence for clock/power gating (Lijo Lazar)
---
 drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 23 +++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
index 4eebf973a065..b45d0914b20c 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
@@ -543,6 +543,30 @@ static int uvd_v6_0_hw_fini(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
+	/*
+	 * Proper cleanups before halting the HW engine:
+	 *   - cancel the delayed idle work
+	 *   - enable powergating
+	 *   - enable clockgating
+	 *   - disable dpm
+	 *
+	 * TODO: to align with the VCN implementation, move the
+	 * jobs for clockgating/powergating/dpm setting to
+	 * ->set_powergating_state().
+	 */
+	cancel_delayed_work_sync(&adev->uvd.idle_work);
+
+	if (adev->pm.dpm_enabled) {
+		amdgpu_dpm_enable_uvd(adev, false);
+	} else {
+		amdgpu_asic_set_uvd_clocks(adev, 0, 0);
+		/* shutdown the UVD block */
+		amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
+						       AMD_CG_STATE_GATE);
+		amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
+						       AMD_PG_STATE_GATE);
+	}
+
 	if (RREG32(mmUVD_STATUS) != 0)
 		uvd_v6_0_stop(adev);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
index 6d9108fa22e0..67af20a38ca1 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
@@ -490,6 +490,29 @@ static int vce_v3_0_hw_fini(void *handle)
 	int r;
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
+	/*
+	 * Proper cleanups before halting the HW engine:
+	 *   - cancel the delayed idle work
+	 *   - enable powergating
+	 *   - enable clockgating
+	 *   - disable dpm
+	 *
+	 * TODO: to align with the VCN implementation, move the
+	 * jobs for clockgating/powergating/dpm setting to
+	 * ->set_powergating_state().
+	 */
+	cancel_delayed_work_sync(&adev->vce.idle_work);
+
+	if (adev->pm.dpm_enabled) {
+		amdgpu_dpm_enable_vce(adev, false);
+	} else {
+		amdgpu_asic_set_vce_clocks(adev, 0, 0);
+		amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_VCE,
+						       AMD_CG_STATE_GATE);
+		amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCE,
+						       AMD_PG_STATE_GATE);
+	}
+
 	r = vce_v3_0_wait_for_idle(handle);
 	if (r)
 		return r;
-- 
2.29.0

[PATCH 3/4] drm/amdgpu: add missing cleanups for more ASICs on UVD/VCE suspend

[Evan Quan]

This is a supplement for commit below:
"drm/amdgpu: add missing cleanups for Polaris12 UVD/VCE on suspend".

Change-Id: I7ff5692fd0c3e880ec8e55a7329469a67e5a1363
Signed-off-by: Evan Quan <evan.quan@amd.com>
--
v1->v2:
  - correct the sequence for clock/power gating (Lijo Lazar)
---
 drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c | 25 ++++++++++++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/vce_v2_0.c | 25 +++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/vce_v4_0.c | 23 +++++++++++++++++++++++
 6 files changed, 144 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c b/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
index 3c725de9df95..64ab28d1163b 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
@@ -698,6 +698,30 @@ static int uvd_v3_1_hw_fini(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
+	/*
+	 * Proper cleanups before halting the HW engine:
+	 *   - cancel the delayed idle work
+	 *   - enable powergating
+	 *   - enable clockgating
+	 *   - disable dpm
+	 *
+	 * TODO: to align with the VCN implementation, move the
+	 * jobs for clockgating/powergating/dpm setting to
+	 * ->set_powergating_state().
+	 */
+	cancel_delayed_work_sync(&adev->uvd.idle_work);
+
+	if (adev->pm.dpm_enabled) {
+		amdgpu_dpm_enable_uvd(adev, false);
+	} else {
+		amdgpu_asic_set_uvd_clocks(adev, 0, 0);
+		/* shutdown the UVD block */
+		amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
+						       AMD_CG_STATE_GATE);
+		amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
+						       AMD_PG_STATE_GATE);
+	}
+
 	if (RREG32(mmUVD_STATUS) != 0)
 		uvd_v3_1_stop(adev);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
index a70d2a0de316..4a3f80d3589a 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
@@ -212,6 +212,30 @@ static int uvd_v4_2_hw_fini(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
+	/*
+	 * Proper cleanups before halting the HW engine:
+	 *   - cancel the delayed idle work
+	 *   - enable powergating
+	 *   - enable clockgating
+	 *   - disable dpm
+	 *
+	 * TODO: to align with the VCN implementation, move the
+	 * jobs for clockgating/powergating/dpm setting to
+	 * ->set_powergating_state().
+	 */
+	cancel_delayed_work_sync(&adev->uvd.idle_work);
+
+	if (adev->pm.dpm_enabled) {
+		amdgpu_dpm_enable_uvd(adev, false);
+	} else {
+		amdgpu_asic_set_uvd_clocks(adev, 0, 0);
+		/* shutdown the UVD block */
+		amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
+						       AMD_CG_STATE_GATE);
+		amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
+						       AMD_PG_STATE_GATE);
+	}
+
 	if (RREG32(mmUVD_STATUS) != 0)
 		uvd_v4_2_stop(adev);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
index f3b0a927101b..715a5687b2b0 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
@@ -210,6 +210,30 @@ static int uvd_v5_0_hw_fini(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
+	/*
+	 * Proper cleanups before halting the HW engine:
+	 *   - cancel the delayed idle work
+	 *   - enable powergating
+	 *   - enable clockgating
+	 *   - disable dpm
+	 *
+	 * TODO: to align with the VCN implementation, move the
+	 * jobs for clockgating/powergating/dpm setting to
+	 * ->set_powergating_state().
+	 */
+	cancel_delayed_work_sync(&adev->uvd.idle_work);
+
+	if (adev->pm.dpm_enabled) {
+		amdgpu_dpm_enable_uvd(adev, false);
+	} else {
+		amdgpu_asic_set_uvd_clocks(adev, 0, 0);
+		/* shutdown the UVD block */
+		amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
+						       AMD_CG_STATE_GATE);
+		amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
+						       AMD_PG_STATE_GATE);
+	}
+
 	if (RREG32(mmUVD_STATUS) != 0)
 		uvd_v5_0_stop(adev);
 
@@ -224,7 +248,6 @@ static int uvd_v5_0_suspend(void *handle)
 	r = uvd_v5_0_hw_fini(adev);
 	if (r)
 		return r;
-	uvd_v5_0_set_clockgating_state(adev, AMD_CG_STATE_GATE);
 
 	return amdgpu_uvd_suspend(adev);
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
index 7cd67cb2ac5f..0b5225a32234 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
@@ -605,6 +605,30 @@ static int uvd_v7_0_hw_fini(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
+	/*
+	 * Proper cleanups before halting the HW engine:
+	 *   - cancel the delayed idle work
+	 *   - enable powergating
+	 *   - enable clockgating
+	 *   - disable dpm
+	 *
+	 * TODO: to align with the VCN implementation, move the
+	 * jobs for clockgating/powergating/dpm setting to
+	 * ->set_powergating_state().
+	 */
+	cancel_delayed_work_sync(&adev->uvd.idle_work);
+
+	if (adev->pm.dpm_enabled) {
+		amdgpu_dpm_enable_uvd(adev, false);
+	} else {
+		amdgpu_asic_set_uvd_clocks(adev, 0, 0);
+		/* shutdown the UVD block */
+		amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
+						       AMD_CG_STATE_GATE);
+		amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
+						       AMD_PG_STATE_GATE);
+	}
+
 	if (!amdgpu_sriov_vf(adev))
 		uvd_v7_0_stop(adev);
 	else {
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
index 0e2945baf0f1..ac5d6a2bd1af 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
@@ -478,6 +478,31 @@ static int vce_v2_0_hw_init(void *handle)
 
 static int vce_v2_0_hw_fini(void *handle)
 {
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+	/*
+	 * Proper cleanups before halting the HW engine:
+	 *   - cancel the delayed idle work
+	 *   - enable powergating
+	 *   - enable clockgating
+	 *   - disable dpm
+	 *
+	 * TODO: to align with the VCN implementation, move the
+	 * jobs for clockgating/powergating/dpm setting to
+	 * ->set_powergating_state().
+	 */
+	cancel_delayed_work_sync(&adev->vce.idle_work);
+
+	if (adev->pm.dpm_enabled) {
+		amdgpu_dpm_enable_vce(adev, false);
+	} else {
+		amdgpu_asic_set_vce_clocks(adev, 0, 0);
+		amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_VCE,
+						       AMD_CG_STATE_GATE);
+		amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCE,
+						       AMD_PG_STATE_GATE);
+	}
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
index 37fa163393fd..feda5524f18c 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
@@ -541,6 +541,29 @@ static int vce_v4_0_hw_fini(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
+	/*
+	 * Proper cleanups before halting the HW engine:
+	 *   - cancel the delayed idle work
+	 *   - enable powergating
+	 *   - enable clockgating
+	 *   - disable dpm
+	 *
+	 * TODO: to align with the VCN implementation, move the
+	 * jobs for clockgating/powergating/dpm setting to
+	 * ->set_powergating_state().
+	 */
+	cancel_delayed_work_sync(&adev->vce.idle_work);
+
+	if (adev->pm.dpm_enabled) {
+		amdgpu_dpm_enable_vce(adev, false);
+	} else {
+		amdgpu_asic_set_vce_clocks(adev, 0, 0);
+		amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_VCE,
+						       AMD_CG_STATE_GATE);
+		amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCE,
+						       AMD_PG_STATE_GATE);
+	}
+
 	if (!amdgpu_sriov_vf(adev)) {
 		/* vce_v4_0_wait_for_idle(handle); */
 		vce_v4_0_stop(adev);
-- 
2.29.0

[PATCH 4/4] drm/amdgpu: drop redundant cancel_delayed_work_sync call

[Evan Quan]

As those _sw_fini() APIs follow just after _suspend() APIs.
And the cancel_delayed_work_sync was already called in latter.

Change-Id: I7f092e39242a1ffbc3c29e1fcd7bf31b769b0ef5
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c | 2 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c  | 1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c  | 1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c  | 2 --
 4 files changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
index 8996cb4ed57a..9342aa23ebd2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
@@ -47,8 +47,6 @@ int amdgpu_jpeg_sw_fini(struct amdgpu_device *adev)
 {
 	int i;
 
-	cancel_delayed_work_sync(&adev->jpeg.idle_work);
-
 	for (i = 0; i < adev->jpeg.num_jpeg_inst; ++i) {
 		if (adev->jpeg.harvest_config & (1 << i))
 			continue;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index 859840ac5f0b..3dcd0c0dbd09 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -325,7 +325,6 @@ int amdgpu_uvd_sw_fini(struct amdgpu_device *adev)
 {
 	int i, j;
 
-	cancel_delayed_work_sync(&adev->uvd.idle_work);
 	drm_sched_entity_destroy(&adev->uvd.entity);
 
 	for (j = 0; j < adev->uvd.num_uvd_inst; ++j) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
index 21b4fc48d33f..310a456090cd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
@@ -217,7 +217,6 @@ int amdgpu_vce_sw_fini(struct amdgpu_device *adev)
 	if (adev->vce.vcpu_bo == NULL)
 		return 0;
 
-	cancel_delayed_work_sync(&adev->vce.idle_work);
 	drm_sched_entity_destroy(&adev->vce.entity);
 
 	amdgpu_bo_free_kernel(&adev->vce.vcpu_bo, &adev->vce.gpu_addr,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 84b025405578..1f21e8fceab3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -257,8 +257,6 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
 {
 	int i, j;
 
-	cancel_delayed_work_sync(&adev->vcn.idle_work);
-
 	for (j = 0; j < adev->vcn.num_vcn_inst; ++j) {
 		if (adev->vcn.harvest_config & (1 << j))
 			continue;
-- 
2.29.0

RE: [PATCH 1/4] drm/amdgpu: correct clock/power gating sequence on UVD/VCE suspend/resume

[Quan, Evan]

[AMD Official Use Only]

Have to drop this patch as the following errors were observed with it.
[   87.420822] [drm:uvd_v6_0_start [amdgpu]] *ERROR* UVD not responding, trying to reset the VCPU!!!
[   88.443029] [drm:uvd_v6_0_start [amdgpu]] *ERROR* UVD not responding, trying to reset the VCPU!!!
[   89.465386] [drm:uvd_v6_0_start [amdgpu]] *ERROR* UVD not responding, trying to reset the VCPU!!!
[   90.487629] [drm:uvd_v6_0_start [amdgpu]] *ERROR* UVD not responding, trying to reset the VCPU!!!
[   91.510380] [drm:uvd_v6_0_start [amdgpu]] *ERROR* UVD not responding, trying to reset the VCPU!!!
[   92.533782] [drm:uvd_v6_0_start [amdgpu]] *ERROR* UVD not responding, trying to reset the VCPU!!!
[   93.557400] [drm:uvd_v6_0_start [amdgpu]] *ERROR* UVD not responding, trying to reset the VCPU!!!
[   94.580708] [drm:uvd_v6_0_start [amdgpu]] *ERROR* UVD not responding, trying to reset the VCPU!!!
[   95.603832] [drm:uvd_v6_0_start [amdgpu]] *ERROR* UVD not responding, trying to reset the VCPU!!!
[   96.627727] [drm:uvd_v6_0_start [amdgpu]] *ERROR* UVD not responding, trying to reset the VCPU!!!
[   96.657453] [drm:uvd_v6_0_start [amdgpu]] *ERROR* UVD not responding, giving up!!!
[   96.665892] [drm:amdgpu_device_ip_set_powergating_state [amdgpu]] *ERROR* set_powergating_state of IP block <uvd_v6_0> failed -1
[   97.697422] amdgpu 0000:02:00.0: [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR* IB test failed on uvd (-110).
[   98.721432] amdgpu 0000:02:00.0: [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR* IB test failed on uvd_enc0 (-110).
[   99.745407] amdgpu 0000:02:00.0: [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR* IB test failed on uvd_enc1 (-110).
[   99.857784] [drm:amdgpu_device_delayed_init_work_handler [amdgpu]] *ERROR* ib ring test failed (-110).

BR
Evan
> -----Original Message-----
> From: Quan, Evan <Evan.Quan@amd.com>
> Sent: Monday, August 23, 2021 12:18 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Chen, Guchun
> <Guchun.Chen@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Zhu, James
> <James.Zhu@amd.com>; Liu, Leo <Leo.Liu@amd.com>; Quan, Evan
> <Evan.Quan@amd.com>
> Subject: [PATCH 1/4] drm/amdgpu: correct clock/power gating sequence on
> UVD/VCE suspend/resume
> 
> The clocks should be gated before power. And reverse sequence should be
> used on ungating.
> 
> Change-Id: Iab09f1f616560ff1083b75e95bfc6433d05d7f98
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c       |  8 +++----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c       |  8 +++----
>  .../powerplay/hwmgr/smu7_clockpowergating.c   | 24 +++++++++----------
>  .../drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c   | 24 +++++++++-------
> ---
>  4 files changed, 32 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> index 445480b50f48..859840ac5f0b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> @@ -1238,10 +1238,10 @@ static void
> amdgpu_uvd_idle_work_handler(struct work_struct *work)
>  		} else {
>  			amdgpu_asic_set_uvd_clocks(adev, 0, 0);
>  			/* shutdown the UVD block */
> -			amdgpu_device_ip_set_powergating_state(adev,
> AMD_IP_BLOCK_TYPE_UVD,
> -
> AMD_PG_STATE_GATE);
>  			amdgpu_device_ip_set_clockgating_state(adev,
> AMD_IP_BLOCK_TYPE_UVD,
> 
> AMD_CG_STATE_GATE);
> +			amdgpu_device_ip_set_powergating_state(adev,
> AMD_IP_BLOCK_TYPE_UVD,
> +
> AMD_PG_STATE_GATE);
>  		}
>  	} else {
>  		schedule_delayed_work(&adev->uvd.idle_work,
> UVD_IDLE_TIMEOUT); @@ -1262,10 +1262,10 @@ void
> amdgpu_uvd_ring_begin_use(struct amdgpu_ring *ring)
>  			amdgpu_dpm_enable_uvd(adev, true);
>  		} else {
>  			amdgpu_asic_set_uvd_clocks(adev, 53300, 40000);
> -			amdgpu_device_ip_set_clockgating_state(adev,
> AMD_IP_BLOCK_TYPE_UVD,
> -
> AMD_CG_STATE_UNGATE);
>  			amdgpu_device_ip_set_powergating_state(adev,
> AMD_IP_BLOCK_TYPE_UVD,
> 
> AMD_PG_STATE_UNGATE);
> +			amdgpu_device_ip_set_clockgating_state(adev,
> AMD_IP_BLOCK_TYPE_UVD,
> +
> AMD_CG_STATE_UNGATE);
>  		}
>  	}
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> index 7ad83da613ed..21b4fc48d33f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> @@ -344,10 +344,10 @@ static void amdgpu_vce_idle_work_handler(struct
> work_struct *work)
>  			amdgpu_dpm_enable_vce(adev, false);
>  		} else {
>  			amdgpu_asic_set_vce_clocks(adev, 0, 0);
> -			amdgpu_device_ip_set_powergating_state(adev,
> AMD_IP_BLOCK_TYPE_VCE,
> -
> AMD_PG_STATE_GATE);
>  			amdgpu_device_ip_set_clockgating_state(adev,
> AMD_IP_BLOCK_TYPE_VCE,
> 
> AMD_CG_STATE_GATE);
> +			amdgpu_device_ip_set_powergating_state(adev,
> AMD_IP_BLOCK_TYPE_VCE,
> +
> AMD_PG_STATE_GATE);
>  		}
>  	} else {
>  		schedule_delayed_work(&adev->vce.idle_work,
> VCE_IDLE_TIMEOUT); @@ -376,10 +376,10 @@ void
> amdgpu_vce_ring_begin_use(struct amdgpu_ring *ring)
>  			amdgpu_dpm_enable_vce(adev, true);
>  		} else {
>  			amdgpu_asic_set_vce_clocks(adev, 53300, 40000);
> -			amdgpu_device_ip_set_clockgating_state(adev,
> AMD_IP_BLOCK_TYPE_VCE,
> -
> AMD_CG_STATE_UNGATE);
>  			amdgpu_device_ip_set_powergating_state(adev,
> AMD_IP_BLOCK_TYPE_VCE,
> 
> AMD_PG_STATE_UNGATE);
> +			amdgpu_device_ip_set_clockgating_state(adev,
> AMD_IP_BLOCK_TYPE_VCE,
> +
> AMD_CG_STATE_UNGATE);
> 
>  		}
>  	}
> diff --git
> a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_clockpowergating.c
> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_clockpowergating.c
> index f2bda3bcbbde..e1f85f777eac 100644
> ---
> a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_clockpowergating.c
> +++
> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_clockpowergating.c
> @@ -118,22 +118,22 @@ void smu7_powergate_uvd(struct pp_hwmgr
> *hwmgr, bool bgate)
>  	data->uvd_power_gated = bgate;
> 
>  	if (bgate) {
> -		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
> -						AMD_IP_BLOCK_TYPE_UVD,
> -						AMD_PG_STATE_GATE);
>  		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
>  				AMD_IP_BLOCK_TYPE_UVD,
>  				AMD_CG_STATE_GATE);
> +		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
> +						AMD_IP_BLOCK_TYPE_UVD,
> +						AMD_PG_STATE_GATE);
>  		smu7_update_uvd_dpm(hwmgr, true);
>  		smu7_powerdown_uvd(hwmgr);
>  	} else {
>  		smu7_powerup_uvd(hwmgr);
> -		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
> -				AMD_IP_BLOCK_TYPE_UVD,
> -				AMD_CG_STATE_UNGATE);
>  		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
>  						AMD_IP_BLOCK_TYPE_UVD,
>  						AMD_PG_STATE_UNGATE);
> +		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
> +				AMD_IP_BLOCK_TYPE_UVD,
> +				AMD_CG_STATE_UNGATE);
>  		smu7_update_uvd_dpm(hwmgr, false);
>  	}
> 
> @@ -146,22 +146,22 @@ void smu7_powergate_vce(struct pp_hwmgr
> *hwmgr, bool bgate)
>  	data->vce_power_gated = bgate;
> 
>  	if (bgate) {
> -		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
> -						AMD_IP_BLOCK_TYPE_VCE,
> -						AMD_PG_STATE_GATE);
>  		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
>  				AMD_IP_BLOCK_TYPE_VCE,
>  				AMD_CG_STATE_GATE);
> +		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
> +						AMD_IP_BLOCK_TYPE_VCE,
> +						AMD_PG_STATE_GATE);
>  		smu7_update_vce_dpm(hwmgr, true);
>  		smu7_powerdown_vce(hwmgr);
>  	} else {
>  		smu7_powerup_vce(hwmgr);
> -		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
> -				AMD_IP_BLOCK_TYPE_VCE,
> -				AMD_CG_STATE_UNGATE);
>  		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
>  						AMD_IP_BLOCK_TYPE_VCE,
>  						AMD_PG_STATE_UNGATE);
> +		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
> +				AMD_IP_BLOCK_TYPE_VCE,
> +				AMD_CG_STATE_UNGATE);
>  		smu7_update_vce_dpm(hwmgr, false);
>  	}
>  }
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
> index b94a77e4e714..a6147db548ca 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
> @@ -1957,22 +1957,22 @@ static void smu8_dpm_powergate_uvd(struct
> pp_hwmgr *hwmgr, bool bgate)
>  	data->uvd_power_gated = bgate;
> 
>  	if (bgate) {
> -		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
> -						AMD_IP_BLOCK_TYPE_UVD,
> -						AMD_PG_STATE_GATE);
>  		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
>  						AMD_IP_BLOCK_TYPE_UVD,
>  						AMD_CG_STATE_GATE);
> +		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
> +						AMD_IP_BLOCK_TYPE_UVD,
> +						AMD_PG_STATE_GATE);
>  		smu8_dpm_update_uvd_dpm(hwmgr, true);
>  		smu8_dpm_powerdown_uvd(hwmgr);
>  	} else {
>  		smu8_dpm_powerup_uvd(hwmgr);
> -		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
> -						AMD_IP_BLOCK_TYPE_UVD,
> -						AMD_CG_STATE_UNGATE);
>  		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
>  						AMD_IP_BLOCK_TYPE_UVD,
>  						AMD_PG_STATE_UNGATE);
> +		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
> +						AMD_IP_BLOCK_TYPE_UVD,
> +						AMD_CG_STATE_UNGATE);
>  		smu8_dpm_update_uvd_dpm(hwmgr, false);
>  	}
> 
> @@ -1983,24 +1983,24 @@ static void smu8_dpm_powergate_vce(struct
> pp_hwmgr *hwmgr, bool bgate)
>  	struct smu8_hwmgr *data = hwmgr->backend;
> 
>  	if (bgate) {
> -		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
> -					AMD_IP_BLOCK_TYPE_VCE,
> -					AMD_PG_STATE_GATE);
>  		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
>  					AMD_IP_BLOCK_TYPE_VCE,
>  					AMD_CG_STATE_GATE);
> +		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
> +					AMD_IP_BLOCK_TYPE_VCE,
> +					AMD_PG_STATE_GATE);
>  		smu8_enable_disable_vce_dpm(hwmgr, false);
>  		smu8_dpm_powerdown_vce(hwmgr);
>  		data->vce_power_gated = true;
>  	} else {
>  		smu8_dpm_powerup_vce(hwmgr);
>  		data->vce_power_gated = false;
> -		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
> -					AMD_IP_BLOCK_TYPE_VCE,
> -					AMD_CG_STATE_UNGATE);
>  		amdgpu_device_ip_set_powergating_state(hwmgr->adev,
>  					AMD_IP_BLOCK_TYPE_VCE,
>  					AMD_PG_STATE_UNGATE);
> +		amdgpu_device_ip_set_clockgating_state(hwmgr->adev,
> +					AMD_IP_BLOCK_TYPE_VCE,
> +					AMD_CG_STATE_UNGATE);
>  		smu8_dpm_update_vce_dpm(hwmgr);
>  		smu8_enable_disable_vce_dpm(hwmgr, true);
>  	}
> --
> 2.29.0