[PATCH 1/3] drm/amdgpu: ungate power gating when system suspend

[PATCH 1/3] drm/amdgpu: ungate power gating when system suspend

[Perry Yuan]

[Why] During suspend, if GFX DPM is enabled and GFXOFF feature is
enabled the system may get hung. So, it is suggested to disable
GFXOFF feature during suspend and enable it after resume.

[How] Update the code to disable GFXOFF feature during suspend and enable
it after resume.

[  311.396526] amdgpu 0000:03:00.0: amdgpu: SMU: I'm not done with your previous command: SMN_C2PMSG_66:0x0000001E SMN_C2PMSG_82:0x00000000
[  311.396530] amdgpu 0000:03:00.0: amdgpu: Fail to disable dpm features!
[  311.396531] [drm:amdgpu_device_ip_suspend_phase2 [amdgpu]] *ERROR* suspend of IP block <smu> failed -62

Signed-off-by: Perry Yuan <perry.yuan@amd.com>
Signed-off-by: Kun Liu <kun.liu2@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index d9ccacd06fba..6399bc71c56d 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -3498,6 +3498,8 @@ static void gfx_v10_0_ring_invalidate_tlbs(struct amdgpu_ring *ring,
 static void gfx_v10_0_update_spm_vmid_internal(struct amdgpu_device *adev,
 					       unsigned int vmid);
 
+static int gfx_v10_0_set_powergating_state(void *handle,
+					  enum amd_powergating_state state);
 static void gfx10_kiq_set_resources(struct amdgpu_ring *kiq_ring, uint64_t queue_mask)
 {
 	amdgpu_ring_write(kiq_ring, PACKET3(PACKET3_SET_RESOURCES, 6));
@@ -7172,6 +7174,13 @@ static int gfx_v10_0_hw_fini(void *handle)
 	amdgpu_irq_put(adev, &adev->gfx.priv_reg_irq, 0);
 	amdgpu_irq_put(adev, &adev->gfx.priv_inst_irq, 0);
 
+	/* WA added for Vangogh asic fixing the SMU suspend failure
+	 * It needs to set power gating again during gfxoff control
+	 * otherwise the gfxoff disallowing will be failed to set.
+	 */
+	if (amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(10, 3, 1))
+		gfx_v10_0_set_powergating_state(handle, AMD_PG_STATE_UNGATE);
+
 	if (!adev->no_hw_access) {
 		if (amdgpu_async_gfx_ring) {
 			if (amdgpu_gfx_disable_kgq(adev, 0))
-- 
2.34.1

[PATCH 2/3] drm/amdgpu: avoid sending csib command when system resumes from S3

[Perry Yuan]

Previously the CSIB command pocket was sent to GFX block while amdgpu
driver loading or S3 resuming time all the time.
As the CP protocol required, the CSIB is not needed to send again while
GC is not powered down while resuming from aborted S3 suspend sequence.

PREAMBLE_CNTL packet coming in the ring after PG event where the RLC
already sent its copy of CSIB, send another CSIB pocket will cause
Gfx IB testing timeout when system resume from S3.

Add flag `csib_initialized` to make sure normal S3 suspend/resume
will initialize csib normally, when system abort to S3 suspend and
resume immediately because of some failed suspend callback, GPU is not
power down at that time, so csib command is not needed to send again.

Error dmesg log:
amdgpu 0000:04:00.0: [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR* IB test failed on gfx_0.0.0 (-110).
[drm:amdgpu_device_delayed_init_work_handler [amdgpu]] *ERROR* ib ring test failed (-110).
PM: resume of devices complete after 2373.995 msecs
PM: Finishing wakeup.

Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  5 +++++
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c  | 29 ++++++++++++++++++-------
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 44df1a5bce7f..e5d85ea26a5e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1114,6 +1114,7 @@ struct amdgpu_device {
 	bool                            debug_vm;
 	bool                            debug_largebar;
 	bool                            debug_disable_soft_recovery;
+	bool				csib_initialized;
 };
 
 static inline uint32_t amdgpu_ip_version(const struct amdgpu_device *adev,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 420196a17e22..a47c9f840754 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2468,6 +2468,11 @@ static int amdgpu_pmops_suspend_noirq(struct device *dev)
 	if (amdgpu_acpi_should_gpu_reset(adev))
 		return amdgpu_asic_reset(adev);
 
+	/* update flag to make sure csib will be sent when system
+	 * resume from normal S3
+	 */
+	adev->csib_initialized = false;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 6399bc71c56d..ab2e3e592dfc 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -3481,6 +3481,7 @@ static uint64_t gfx_v10_0_get_gpu_clock_counter(struct amdgpu_device *adev);
 static void gfx_v10_0_select_se_sh(struct amdgpu_device *adev, u32 se_num,
 				   u32 sh_num, u32 instance, int xcc_id);
 static u32 gfx_v10_0_get_wgp_active_bitmap_per_sh(struct amdgpu_device *adev);
+static int gfx_v10_0_wait_for_idle(void *handle);
 
 static int gfx_v10_0_rlc_backdoor_autoload_buffer_init(struct amdgpu_device *adev);
 static void gfx_v10_0_rlc_backdoor_autoload_buffer_fini(struct amdgpu_device *adev);
@@ -5958,7 +5959,7 @@ static int gfx_v10_0_cp_gfx_load_microcode(struct amdgpu_device *adev)
 	return 0;
 }
 
-static int gfx_v10_0_cp_gfx_start(struct amdgpu_device *adev)
+static int gfx_v10_csib_submit(struct amdgpu_device *adev)
 {
 	struct amdgpu_ring *ring;
 	const struct cs_section_def *sect = NULL;
@@ -5966,13 +5967,6 @@ static int gfx_v10_0_cp_gfx_start(struct amdgpu_device *adev)
 	int r, i;
 	int ctx_reg_offset;
 
-	/* init the CP */
-	WREG32_SOC15(GC, 0, mmCP_MAX_CONTEXT,
-		     adev->gfx.config.max_hw_contexts - 1);
-	WREG32_SOC15(GC, 0, mmCP_DEVICE_ID, 1);
-
-	gfx_v10_0_cp_gfx_enable(adev, true);
-
 	ring = &adev->gfx.gfx_ring[0];
 	r = amdgpu_ring_alloc(ring, gfx_v10_0_get_csb_size(adev) + 4);
 	if (r) {
@@ -6035,6 +6029,25 @@ static int gfx_v10_0_cp_gfx_start(struct amdgpu_device *adev)
 
 		amdgpu_ring_commit(ring);
 	}
+
+	gfx_v10_0_wait_for_idle(adev);
+	adev->csib_initialized = true;
+
+	return 0;
+};
+
+static int gfx_v10_0_cp_gfx_start(struct amdgpu_device *adev)
+{
+	/* init the CP */
+	WREG32_SOC15(GC, 0, mmCP_MAX_CONTEXT,
+		     adev->gfx.config.max_hw_contexts - 1);
+	WREG32_SOC15(GC, 0, mmCP_DEVICE_ID, 1);
+
+	gfx_v10_0_cp_gfx_enable(adev, true);
+
+	if (!adev->csib_initialized)
+		gfx_v10_csib_submit(adev);
+
 	return 0;
 }
 
-- 
2.34.1

[PATCH 3/3] drm/amdgpu: optimize RLC powerdown notification on Vangogh

[Perry Yuan]

The smu needs to get the rlc power down message to sync the rlc state
with smu, the rlc state updating message need to be sent at while smu
begin suspend sequence , otherwise SMU will crash while RLC state is not
notified by driver, and rlc state probally changed after that
notification, so it needs to notify rlc state to smu at the end of the
suspend sequence in amdgpu_device_suspend() that can make sure the rlc
state  is correctly set to SMU.

[  101.000590] amdgpu 0000:03:00.0: amdgpu: SMU: I'm not done with your previous command: SMN_C2PMSG_66:0x0000001E SMN_C2PMSG_82:0x00000000
[  101.000598] amdgpu 0000:03:00.0: amdgpu: Failed to disable gfxoff!
[  110.838026] amdgpu 0000:03:00.0: amdgpu: SMU: I'm not done with your previous command: SMN_C2PMSG_66:0x0000001E SMN_C2PMSG_82:0x00000000
[  110.838035] amdgpu 0000:03:00.0: amdgpu: Failed to disable smu features.
[  110.838039] amdgpu 0000:03:00.0: amdgpu: Fail to disable dpm features!
[  110.838040] [drm:amdgpu_device_ip_suspend_phase2 [amdgpu]] *ERROR* suspend of IP block <smu> failed -62
[  110.884394] PM: suspend of devices aborted after 21213.620 msecs
[  110.884402] PM: start suspend of devices aborted after 21213.882 msecs
[  110.884405] PM: Some devices failed to suspend, or early wake event detected

Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c     |  4 ++++
 drivers/gpu/drm/amd/include/kgd_pp_interface.h |  1 +
 drivers/gpu/drm/amd/pm/amdgpu_dpm.c            | 18 ++++++++++++++++++
 drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h        |  2 ++
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c      | 10 ++++++++++
 drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h  |  5 +++++
 .../gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c   |  5 ++---
 drivers/gpu/drm/amd/pm/swsmu/smu_internal.h    |  1 +
 8 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index cc047fe0b7ee..be08ffc69231 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4428,6 +4428,10 @@ int amdgpu_device_suspend(struct drm_device *dev, bool fbcon)
 	if (amdgpu_sriov_vf(adev))
 		amdgpu_virt_release_full_gpu(adev, false);
 
+	r = amdgpu_dpm_notify_rlc_state(adev, false);
+	if (r)
+		return r;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index 3201808c2dd8..4eacfdfcfd4b 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -444,6 +444,7 @@ struct amd_pm_funcs {
 				   struct dpm_clocks *clock_table);
 	int (*get_smu_prv_buf_details)(void *handle, void **addr, size_t *size);
 	void (*pm_compute_clocks)(void *handle);
+	int (*notify_rlc_state)(void *handle, bool en);
 };
 
 struct metrics_table_header {
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
index acf3527fff2d..ed7237bb64c8 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
@@ -181,6 +181,24 @@ int amdgpu_dpm_set_mp1_state(struct amdgpu_device *adev,
 	return ret;
 }
 
+int amdgpu_dpm_notify_rlc_state(struct amdgpu_device *adev, bool en)
+{
+	int ret = 0;
+	const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
+
+	if (pp_funcs && pp_funcs->notify_rlc_state) {
+		mutex_lock(&adev->pm.mutex);
+
+		ret = pp_funcs->notify_rlc_state(
+				adev->powerplay.pp_handle,
+				en);
+
+		mutex_unlock(&adev->pm.mutex);
+	}
+
+	return ret;
+}
+
 bool amdgpu_dpm_is_baco_supported(struct amdgpu_device *adev)
 {
 	const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
index feccd2a7120d..482ea30147ab 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
@@ -415,6 +415,8 @@ int amdgpu_dpm_mode1_reset(struct amdgpu_device *adev);
 int amdgpu_dpm_set_mp1_state(struct amdgpu_device *adev,
 			     enum pp_mp1_state mp1_state);
 
+int amdgpu_dpm_notify_rlc_state(struct amdgpu_device *adev, bool en);
+
 int amdgpu_dpm_set_gfx_power_up_by_imu(struct amdgpu_device *adev);
 
 int amdgpu_dpm_baco_exit(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index a0b8d5d78beb..a8fb914f746b 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -1710,6 +1710,16 @@ static int smu_disable_dpms(struct smu_context *smu)
 		}
 	}
 
+	/* Notify SMU RLC is going to be off, stop RLC and SMU interaction.
+	 * otherwise SMU will hang while interacting with RLC if RLC is halted
+	 * this is a WA for Vangogh asic which fix the SMU hang issue.
+	 */
+	ret = smu_notify_rlc_state(smu, false);
+	if (ret) {
+		dev_err(adev->dev, "Fail to notify rlc status!\n");
+		return ret;
+	}
+
 	if (amdgpu_ip_version(adev, GC_HWIP, 0) >= IP_VERSION(9, 4, 2) &&
 	    !amdgpu_sriov_vf(adev) && adev->gfx.rlc.funcs->stop)
 		adev->gfx.rlc.funcs->stop(adev);
diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
index 839553a86aa2..7778a98f192c 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
@@ -1359,6 +1359,11 @@ struct pptable_funcs {
 	 *                       management.
 	 */
 	int (*dpm_set_umsch_mm_enable)(struct smu_context *smu, bool enable);
+
+	/**
+	 * @notify_rlc_state: Notify RLC power state to SMU.
+	 */
+	int (*notify_rlc_state)(struct smu_context *smu, bool en);
 };
 
 typedef enum {
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
index 3efc6aed28f1..b42771471416 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
@@ -2202,8 +2202,7 @@ static int vangogh_get_dpm_clock_table(struct smu_context *smu, struct dpm_clock
 	return 0;
 }
 
-
-static int vangogh_system_features_control(struct smu_context *smu, bool en)
+static int vangogh_notify_rlc_state(struct smu_context *smu, bool en)
 {
 	struct amdgpu_device *adev = smu->adev;
 	int ret = 0;
@@ -2532,7 +2531,7 @@ static const struct pptable_funcs vangogh_ppt_funcs = {
 	.print_clk_levels = vangogh_common_print_clk_levels,
 	.set_default_dpm_table = vangogh_set_default_dpm_tables,
 	.set_fine_grain_gfx_freq_parameters = vangogh_set_fine_grain_gfx_freq_parameters,
-	.system_features_control = vangogh_system_features_control,
+	.notify_rlc_state = vangogh_notify_rlc_state,
 	.feature_is_enabled = smu_cmn_feature_is_enabled,
 	.set_power_profile_mode = vangogh_set_power_profile_mode,
 	.get_power_profile_mode = vangogh_get_power_profile_mode,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
index 80b3c3efc006..64766ac69c53 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
@@ -97,6 +97,7 @@
 #define smu_get_default_config_table_settings(smu, config_table)	smu_ppt_funcs(get_default_config_table_settings, -EOPNOTSUPP, smu, config_table)
 #define smu_set_config_table(smu, config_table)				smu_ppt_funcs(set_config_table, -EOPNOTSUPP, smu, config_table)
 #define smu_init_pptable_microcode(smu)					smu_ppt_funcs(init_pptable_microcode, 0, smu)
+#define smu_notify_rlc_state(smu, en)					smu_ppt_funcs(notify_rlc_state, 0, smu, en)
 
 #endif
 #endif
-- 
2.34.1

RE: [PATCH 2/3] drm/amdgpu: avoid sending csib command when system resumes from S3

[Wang, Yang(Kevin)]

[AMD Official Use Only - General]

-----Original Message-----
From: Yuan, Perry <Perry.Yuan@amd.com>
Sent: Tuesday, October 24, 2023 10:33 AM
To: Zhang, Yifan <Yifan1.Zhang@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com>; Limonciello, Mario <Mario.Limonciello@amd.com>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Wang, Yang(Kevin) <KevinYang.Wang@amd.com>; amd-gfx@lists.freedesktop.org
Subject: [PATCH 2/3] drm/amdgpu: avoid sending csib command when system resumes from S3

Previously the CSIB command pocket was sent to GFX block while amdgpu driver loading or S3 resuming time all the time.
As the CP protocol required, the CSIB is not needed to send again while GC is not powered down while resuming from aborted S3 suspend sequence.

PREAMBLE_CNTL packet coming in the ring after PG event where the RLC already sent its copy of CSIB, send another CSIB pocket will cause Gfx IB testing timeout when system resume from S3.

Add flag `csib_initialized` to make sure normal S3 suspend/resume will initialize csib normally, when system abort to S3 suspend and resume immediately because of some failed suspend callback, GPU is not power down at that time, so csib command is not needed to send again.

Error dmesg log:
amdgpu 0000:04:00.0: [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR* IB test failed on gfx_0.0.0 (-110).
[drm:amdgpu_device_delayed_init_work_handler [amdgpu]] *ERROR* ib ring test failed (-110).
PM: resume of devices complete after 2373.995 msecs
PM: Finishing wakeup.

Signed-off-by: Perry Yuan <perry.yuan@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  5 +++++  drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c  | 29 ++++++++++++++++++-------
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 44df1a5bce7f..e5d85ea26a5e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1114,6 +1114,7 @@ struct amdgpu_device {
        bool                            debug_vm;
        bool                            debug_largebar;
        bool                            debug_disable_soft_recovery;
+       bool                            csib_initialized;
[Kevin]:
you'd better use space to instead of "tab" , to align with other field.

 };

 static inline uint32_t amdgpu_ip_version(const struct amdgpu_device *adev, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 420196a17e22..a47c9f840754 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2468,6 +2468,11 @@ static int amdgpu_pmops_suspend_noirq(struct device *dev)
        if (amdgpu_acpi_should_gpu_reset(adev))
                return amdgpu_asic_reset(adev);

+       /* update flag to make sure csib will be sent when system
+        * resume from normal S3
+        */
+       adev->csib_initialized = false;
+
        return 0;
 }

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 6399bc71c56d..ab2e3e592dfc 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -3481,6 +3481,7 @@ static uint64_t gfx_v10_0_get_gpu_clock_counter(struct amdgpu_device *adev);  static void gfx_v10_0_select_se_sh(struct amdgpu_device *adev, u32 se_num,
                                   u32 sh_num, u32 instance, int xcc_id);  static u32 gfx_v10_0_get_wgp_active_bitmap_per_sh(struct amdgpu_device *adev);
+static int gfx_v10_0_wait_for_idle(void *handle);

 static int gfx_v10_0_rlc_backdoor_autoload_buffer_init(struct amdgpu_device *adev);  static void gfx_v10_0_rlc_backdoor_autoload_buffer_fini(struct amdgpu_device *adev); @@ -5958,7 +5959,7 @@ static int gfx_v10_0_cp_gfx_load_microcode(struct amdgpu_device *adev)
        return 0;
 }

-static int gfx_v10_0_cp_gfx_start(struct amdgpu_device *adev)
+static int gfx_v10_csib_submit(struct amdgpu_device *adev)
 {
        struct amdgpu_ring *ring;
        const struct cs_section_def *sect = NULL; @@ -5966,13 +5967,6 @@ static int gfx_v10_0_cp_gfx_start(struct amdgpu_device *adev)
        int r, i;
        int ctx_reg_offset;

-       /* init the CP */
-       WREG32_SOC15(GC, 0, mmCP_MAX_CONTEXT,
-                    adev->gfx.config.max_hw_contexts - 1);
-       WREG32_SOC15(GC, 0, mmCP_DEVICE_ID, 1);
-
-       gfx_v10_0_cp_gfx_enable(adev, true);
-
        ring = &adev->gfx.gfx_ring[0];
        r = amdgpu_ring_alloc(ring, gfx_v10_0_get_csb_size(adev) + 4);
        if (r) {
@@ -6035,6 +6029,25 @@ static int gfx_v10_0_cp_gfx_start(struct amdgpu_device *adev)

                amdgpu_ring_commit(ring);
        }
+
+       gfx_v10_0_wait_for_idle(adev);
[kevin]:
Do you forgot to check return value here?  If you want to ignore the return result, you'd better put some comments here.
Thanks.

Best Regards,
Kevin

+       adev->csib_initialized = true;
+
+       return 0;
+};
+
+static int gfx_v10_0_cp_gfx_start(struct amdgpu_device *adev) {
+       /* init the CP */
+       WREG32_SOC15(GC, 0, mmCP_MAX_CONTEXT,
+                    adev->gfx.config.max_hw_contexts - 1);
+       WREG32_SOC15(GC, 0, mmCP_DEVICE_ID, 1);
+
+       gfx_v10_0_cp_gfx_enable(adev, true);
+
+       if (!adev->csib_initialized)
+               gfx_v10_csib_submit(adev);
+
        return 0;
 }

--
2.34.1

RE: [PATCH 1/3] drm/amdgpu: ungate power gating when system suspend

[Feng, Kenneth]

[AMD Official Use Only - General]

Reviewed-by: Kenneth Feng <kenneth.feng@amd.com>


-----Original Message-----
From: Yuan, Perry <Perry.Yuan@amd.com>
Sent: Tuesday, October 24, 2023 10:33 AM
To: Zhang, Yifan <Yifan1.Zhang@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com>; Limonciello, Mario <Mario.Limonciello@amd.com>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Wang, Yang(Kevin) <KevinYang.Wang@amd.com>; amd-gfx@lists.freedesktop.org
Subject: [PATCH 1/3] drm/amdgpu: ungate power gating when system suspend

[Why] During suspend, if GFX DPM is enabled and GFXOFF feature is enabled the system may get hung. So, it is suggested to disable GFXOFF feature during suspend and enable it after resume.

[How] Update the code to disable GFXOFF feature during suspend and enable it after resume.

[  311.396526] amdgpu 0000:03:00.0: amdgpu: SMU: I'm not done with your previous command: SMN_C2PMSG_66:0x0000001E SMN_C2PMSG_82:0x00000000 [  311.396530] amdgpu 0000:03:00.0: amdgpu: Fail to disable dpm features!
[  311.396531] [drm:amdgpu_device_ip_suspend_phase2 [amdgpu]] *ERROR* suspend of IP block <smu> failed -62

Signed-off-by: Perry Yuan <perry.yuan@amd.com>
Signed-off-by: Kun Liu <kun.liu2@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index d9ccacd06fba..6399bc71c56d 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -3498,6 +3498,8 @@ static void gfx_v10_0_ring_invalidate_tlbs(struct amdgpu_ring *ring,  static void gfx_v10_0_update_spm_vmid_internal(struct amdgpu_device *adev,
                                               unsigned int vmid);

+static int gfx_v10_0_set_powergating_state(void *handle,
+                                         enum amd_powergating_state state);
 static void gfx10_kiq_set_resources(struct amdgpu_ring *kiq_ring, uint64_t queue_mask)  {
        amdgpu_ring_write(kiq_ring, PACKET3(PACKET3_SET_RESOURCES, 6)); @@ -7172,6 +7174,13 @@ static int gfx_v10_0_hw_fini(void *handle)
        amdgpu_irq_put(adev, &adev->gfx.priv_reg_irq, 0);
        amdgpu_irq_put(adev, &adev->gfx.priv_inst_irq, 0);

+       /* WA added for Vangogh asic fixing the SMU suspend failure
+        * It needs to set power gating again during gfxoff control
+        * otherwise the gfxoff disallowing will be failed to set.
+        */
+       if (amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(10, 3, 1))
+               gfx_v10_0_set_powergating_state(handle, AMD_PG_STATE_UNGATE);
+
        if (!adev->no_hw_access) {
                if (amdgpu_async_gfx_ring) {
                        if (amdgpu_gfx_disable_kgq(adev, 0))
--
2.34.1

RE: [PATCH 1/3] drm/amdgpu: ungate power gating when system suspend

[Wang, Yang(Kevin)]

[AMD Official Use Only - General]

Acked-by: Yang Wang <kevinyang.wang@amd.com>

Best Regards,
Kevin

-----Original Message-----
From: Feng, Kenneth <Kenneth.Feng@amd.com>
Sent: Tuesday, October 24, 2023 2:33 PM
To: Yuan, Perry <Perry.Yuan@amd.com>; Zhang, Yifan <Yifan1.Zhang@amd.com>; Limonciello, Mario <Mario.Limonciello@amd.com>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Wang, Yang(Kevin) <KevinYang.Wang@amd.com>; amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH 1/3] drm/amdgpu: ungate power gating when system suspend

[AMD Official Use Only - General]

Reviewed-by: Kenneth Feng <kenneth.feng@amd.com>


-----Original Message-----
From: Yuan, Perry <Perry.Yuan@amd.com>
Sent: Tuesday, October 24, 2023 10:33 AM
To: Zhang, Yifan <Yifan1.Zhang@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com>; Limonciello, Mario <Mario.Limonciello@amd.com>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Wang, Yang(Kevin) <KevinYang.Wang@amd.com>; amd-gfx@lists.freedesktop.org
Subject: [PATCH 1/3] drm/amdgpu: ungate power gating when system suspend

[Why] During suspend, if GFX DPM is enabled and GFXOFF feature is enabled the system may get hung. So, it is suggested to disable GFXOFF feature during suspend and enable it after resume.

[How] Update the code to disable GFXOFF feature during suspend and enable it after resume.

[  311.396526] amdgpu 0000:03:00.0: amdgpu: SMU: I'm not done with your previous command: SMN_C2PMSG_66:0x0000001E SMN_C2PMSG_82:0x00000000 [  311.396530] amdgpu 0000:03:00.0: amdgpu: Fail to disable dpm features!
[  311.396531] [drm:amdgpu_device_ip_suspend_phase2 [amdgpu]] *ERROR* suspend of IP block <smu> failed -62

Signed-off-by: Perry Yuan <perry.yuan@amd.com>
Signed-off-by: Kun Liu <kun.liu2@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index d9ccacd06fba..6399bc71c56d 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -3498,6 +3498,8 @@ static void gfx_v10_0_ring_invalidate_tlbs(struct amdgpu_ring *ring,  static void gfx_v10_0_update_spm_vmid_internal(struct amdgpu_device *adev,
                                               unsigned int vmid);

+static int gfx_v10_0_set_powergating_state(void *handle,
+                                         enum amd_powergating_state state);
 static void gfx10_kiq_set_resources(struct amdgpu_ring *kiq_ring, uint64_t queue_mask)  {
        amdgpu_ring_write(kiq_ring, PACKET3(PACKET3_SET_RESOURCES, 6)); @@ -7172,6 +7174,13 @@ static int gfx_v10_0_hw_fini(void *handle)
        amdgpu_irq_put(adev, &adev->gfx.priv_reg_irq, 0);
        amdgpu_irq_put(adev, &adev->gfx.priv_inst_irq, 0);

+       /* WA added for Vangogh asic fixing the SMU suspend failure
+        * It needs to set power gating again during gfxoff control
+        * otherwise the gfxoff disallowing will be failed to set.
+        */
+       if (amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(10, 3, 1))
+               gfx_v10_0_set_powergating_state(handle, AMD_PG_STATE_UNGATE);
+
        if (!adev->no_hw_access) {
                if (amdgpu_async_gfx_ring) {
                        if (amdgpu_gfx_disable_kgq(adev, 0))
--
2.34.1

RE: [PATCH 2/3] drm/amdgpu: avoid sending csib command when system resumes from S3

[Yuan, Perry]

[AMD Official Use Only - General]

Hi Kevin,


> -----Original Message-----
> From: Wang, Yang(Kevin) <KevinYang.Wang@amd.com>
> Sent: Tuesday, October 24, 2023 1:24 PM
> To: Yuan, Perry <Perry.Yuan@amd.com>; Zhang, Yifan
> <Yifan1.Zhang@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com>;
> Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-
> gfx@lists.freedesktop.org
> Subject: RE: [PATCH 2/3] drm/amdgpu: avoid sending csib command when
> system resumes from S3
>
> [AMD Official Use Only - General]
>
> -----Original Message-----
> From: Yuan, Perry <Perry.Yuan@amd.com>
> Sent: Tuesday, October 24, 2023 10:33 AM
> To: Zhang, Yifan <Yifan1.Zhang@amd.com>; Feng, Kenneth
> <Kenneth.Feng@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Wang, Yang(Kevin)
> <KevinYang.Wang@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: [PATCH 2/3] drm/amdgpu: avoid sending csib command when
> system resumes from S3
>
> Previously the CSIB command pocket was sent to GFX block while amdgpu
> driver loading or S3 resuming time all the time.
> As the CP protocol required, the CSIB is not needed to send again while GC is
> not powered down while resuming from aborted S3 suspend sequence.
>
> PREAMBLE_CNTL packet coming in the ring after PG event where the RLC
> already sent its copy of CSIB, send another CSIB pocket will cause Gfx IB
> testing timeout when system resume from S3.
>
> Add flag `csib_initialized` to make sure normal S3 suspend/resume will
> initialize csib normally, when system abort to S3 suspend and resume
> immediately because of some failed suspend callback, GPU is not power
> down at that time, so csib command is not needed to send again.
>
> Error dmesg log:
> amdgpu 0000:04:00.0: [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR* IB
> test failed on gfx_0.0.0 (-110).
> [drm:amdgpu_device_delayed_init_work_handler [amdgpu]] *ERROR* ib
> ring test failed (-110).
> PM: resume of devices complete after 2373.995 msecs
> PM: Finishing wakeup.
>
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  5 +++++
> drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c  | 29 ++++++++++++++++++----
> ---
>  3 files changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 44df1a5bce7f..e5d85ea26a5e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1114,6 +1114,7 @@ struct amdgpu_device {
>         bool                            debug_vm;
>         bool                            debug_largebar;
>         bool                            debug_disable_soft_recovery;
> +       bool                            csib_initialized;
> [Kevin]:
> you'd better use space to instead of "tab" , to align with other field.

Cool, I didn`t notice that, changed in v2.
Thanks !

>
>  };
>
>  static inline uint32_t amdgpu_ip_version(const struct amdgpu_device *adev,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 420196a17e22..a47c9f840754 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2468,6 +2468,11 @@ static int amdgpu_pmops_suspend_noirq(struct
> device *dev)
>         if (amdgpu_acpi_should_gpu_reset(adev))
>                 return amdgpu_asic_reset(adev);
>
> +       /* update flag to make sure csib will be sent when system
> +        * resume from normal S3
> +        */
> +       adev->csib_initialized = false;
> +
>         return 0;
>  }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index 6399bc71c56d..ab2e3e592dfc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -3481,6 +3481,7 @@ static uint64_t
> gfx_v10_0_get_gpu_clock_counter(struct amdgpu_device *adev);  static
> void gfx_v10_0_select_se_sh(struct amdgpu_device *adev, u32 se_num,
>                                    u32 sh_num, u32 instance, int xcc_id);  static u32
> gfx_v10_0_get_wgp_active_bitmap_per_sh(struct amdgpu_device *adev);
> +static int gfx_v10_0_wait_for_idle(void *handle);
>
>  static int gfx_v10_0_rlc_backdoor_autoload_buffer_init(struct
> amdgpu_device *adev);  static void
> gfx_v10_0_rlc_backdoor_autoload_buffer_fini(struct amdgpu_device
> *adev); @@ -5958,7 +5959,7 @@ static int
> gfx_v10_0_cp_gfx_load_microcode(struct amdgpu_device *adev)
>         return 0;
>  }
>
> -static int gfx_v10_0_cp_gfx_start(struct amdgpu_device *adev)
> +static int gfx_v10_csib_submit(struct amdgpu_device *adev)
>  {
>         struct amdgpu_ring *ring;
>         const struct cs_section_def *sect = NULL; @@ -5966,13 +5967,6 @@
> static int gfx_v10_0_cp_gfx_start(struct amdgpu_device *adev)
>         int r, i;
>         int ctx_reg_offset;
>
> -       /* init the CP */
> -       WREG32_SOC15(GC, 0, mmCP_MAX_CONTEXT,
> -                    adev->gfx.config.max_hw_contexts - 1);
> -       WREG32_SOC15(GC, 0, mmCP_DEVICE_ID, 1);
> -
> -       gfx_v10_0_cp_gfx_enable(adev, true);
> -
>         ring = &adev->gfx.gfx_ring[0];
>         r = amdgpu_ring_alloc(ring, gfx_v10_0_get_csb_size(adev) + 4);
>         if (r) {
> @@ -6035,6 +6029,25 @@ static int gfx_v10_0_cp_gfx_start(struct
> amdgpu_device *adev)
>
>                 amdgpu_ring_commit(ring);
>         }
> +
> +       gfx_v10_0_wait_for_idle(adev);
> [kevin]:
> Do you forgot to check return value here?  If you want to ignore the return
> result, you'd better put some comments here.
> Thanks.
>
> Best Regards,
> Kevin

It is better to add check, changed in v2.
Thanks.

>
> +       adev->csib_initialized = true;
> +
> +       return 0;
> +};
> +
> +static int gfx_v10_0_cp_gfx_start(struct amdgpu_device *adev) {
> +       /* init the CP */
> +       WREG32_SOC15(GC, 0, mmCP_MAX_CONTEXT,
> +                    adev->gfx.config.max_hw_contexts - 1);
> +       WREG32_SOC15(GC, 0, mmCP_DEVICE_ID, 1);
> +
> +       gfx_v10_0_cp_gfx_enable(adev, true);
> +
> +       if (!adev->csib_initialized)
> +               gfx_v10_csib_submit(adev);
> +
>         return 0;
>  }
>
> --
> 2.34.1
>

RE: [PATCH 1/3] drm/amdgpu: ungate power gating when system suspend

[Yuan, Perry]

[AMD Official Use Only - General]

Hi Kevin, Kenneth,

Thanks for the review and ack, will pick it up in v2.

Regards.
Perry

> -----Original Message-----
> From: Wang, Yang(Kevin) <KevinYang.Wang@amd.com>
> Sent: Tuesday, October 24, 2023 4:15 PM
> To: Feng, Kenneth <Kenneth.Feng@amd.com>; Yuan, Perry
> <Perry.Yuan@amd.com>; Zhang, Yifan <Yifan1.Zhang@amd.com>;
> Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-
> gfx@lists.freedesktop.org
> Subject: RE: [PATCH 1/3] drm/amdgpu: ungate power gating when system
> suspend
>
> [AMD Official Use Only - General]
>
> Acked-by: Yang Wang <kevinyang.wang@amd.com>
>
> Best Regards,
> Kevin
>
> -----Original Message-----
> From: Feng, Kenneth <Kenneth.Feng@amd.com>
> Sent: Tuesday, October 24, 2023 2:33 PM
> To: Yuan, Perry <Perry.Yuan@amd.com>; Zhang, Yifan
> <Yifan1.Zhang@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Wang, Yang(Kevin)
> <KevinYang.Wang@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: RE: [PATCH 1/3] drm/amdgpu: ungate power gating when system
> suspend
>
> [AMD Official Use Only - General]
>
> Reviewed-by: Kenneth Feng <kenneth.feng@amd.com>
>
>
> -----Original Message-----
> From: Yuan, Perry <Perry.Yuan@amd.com>
> Sent: Tuesday, October 24, 2023 10:33 AM
> To: Zhang, Yifan <Yifan1.Zhang@amd.com>; Feng, Kenneth
> <Kenneth.Feng@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Wang, Yang(Kevin)
> <KevinYang.Wang@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: [PATCH 1/3] drm/amdgpu: ungate power gating when system
> suspend
>
> [Why] During suspend, if GFX DPM is enabled and GFXOFF feature is enabled
> the system may get hung. So, it is suggested to disable GFXOFF feature
> during suspend and enable it after resume.
>
> [How] Update the code to disable GFXOFF feature during suspend and
> enable it after resume.
>
> [  311.396526] amdgpu 0000:03:00.0: amdgpu: SMU: I'm not done with your
> previous command: SMN_C2PMSG_66:0x0000001E
> SMN_C2PMSG_82:0x00000000 [  311.396530] amdgpu 0000:03:00.0: amdgpu:
> Fail to disable dpm features!
> [  311.396531] [drm:amdgpu_device_ip_suspend_phase2 [amdgpu]]
> *ERROR* suspend of IP block <smu> failed -62
>
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> Signed-off-by: Kun Liu <kun.liu2@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index d9ccacd06fba..6399bc71c56d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -3498,6 +3498,8 @@ static void gfx_v10_0_ring_invalidate_tlbs(struct
> amdgpu_ring *ring,  static void
> gfx_v10_0_update_spm_vmid_internal(struct amdgpu_device *adev,
>                                                unsigned int vmid);
>
> +static int gfx_v10_0_set_powergating_state(void *handle,
> +                                         enum amd_powergating_state state);
>  static void gfx10_kiq_set_resources(struct amdgpu_ring *kiq_ring, uint64_t
> queue_mask)  {
>         amdgpu_ring_write(kiq_ring, PACKET3(PACKET3_SET_RESOURCES, 6));
> @@ -7172,6 +7174,13 @@ static int gfx_v10_0_hw_fini(void *handle)
>         amdgpu_irq_put(adev, &adev->gfx.priv_reg_irq, 0);
>         amdgpu_irq_put(adev, &adev->gfx.priv_inst_irq, 0);
>
> +       /* WA added for Vangogh asic fixing the SMU suspend failure
> +        * It needs to set power gating again during gfxoff control
> +        * otherwise the gfxoff disallowing will be failed to set.
> +        */
> +       if (amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(10, 3, 1))
> +               gfx_v10_0_set_powergating_state(handle,
> AMD_PG_STATE_UNGATE);
> +
>         if (!adev->no_hw_access) {
>                 if (amdgpu_async_gfx_ring) {
>                         if (amdgpu_gfx_disable_kgq(adev, 0))
> --
> 2.34.1
>
>