Re: [PATCH V3 3/3] drm/amd/pm: disable cstate feature for gpu reset scenario

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

 





On 10/13/2022 10:56 AM, Quan, Evan wrote:
[AMD Official Use Only - General]



-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
Sent: Thursday, October 13, 2022 12:14 PM
To: Quan, Evan <Evan.Quan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Zhang, Hawking
<Hawking.Zhang@xxxxxxx>
Subject: Re: [PATCH V3 3/3] drm/amd/pm: disable cstate feature for gpu
reset scenario



On 10/13/2022 7:01 AM, Evan Quan wrote:
Suggested by PMFW team and same as what did for gfxoff feature.
This can address some Mode1Reset failures observed on SMU13.0.0.

Signed-off-by: Evan Quan <evan.quan@xxxxxxx>
Reviewed-by: Hawking Zhang <Hawking.Zhang@xxxxxxx>
Change-Id: Ieb4e204c49abd405b1dce559c2ff75bb3887b6f9
--
v1->v2:
    - revise the code comments(Alex)
    - limit this to SMU13.0.0 and 13.0.7
v2->v3:
    - make this happen before display suspending

A better thing would be do
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c         | 8 ++++++++
   drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c  | 7 +++++++
   drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 7 +++++++
   3 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index ab8f970b2849..874bf623f394 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2928,6 +2928,14 @@ static int
amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev)
   	amdgpu_device_set_pg_state(adev, AMD_PG_STATE_UNGATE);
   	amdgpu_device_set_cg_state(adev, AMD_CG_STATE_UNGATE);

+	/*
+	 * Per PMFW team's suggestion, driver needs to handle gfxoff
+	 * and df cstate features disablement for gpu reset(e.g. Mode1Reset)
+	 * scenario. Add the missing df cstate disablement here.
+	 */
+	if (amdgpu_dpm_set_df_cstate(adev, DF_CSTATE_DISALLOW))
+		dev_warn(adev->dev, "Failed to disallow df cstate");
+

If it's only related to display, you could move this right after below line so that
headless systems don't need to take care of this. That will avoid any special
handling needed for Aldebaran/Arcuturus also.
[Quan, Evan] Not quite sure. I know df cstate affects DAL a lot(MALL related features).
But I'm not sure whether there is other clients/IPs which are affected by df cstate.
I want this(cstate disablement) performed before all 'consumers'.


Thanks for the clarification.

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

Thanks,
Lijo

BR
Evan

                 if (adev->ip_blocks[i].version->type !=
AMD_IP_BLOCK_TYPE_DCE)
                          continue;

Thanks,
Lijo

   	for (i = adev->num_ip_blocks - 1; i >= 0; i--) {
   		if (!adev->ip_blocks[i].status.valid)
   			continue;
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
index 445005571f76..7d34f40460eb 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
@@ -2245,6 +2245,13 @@ static int arcturus_set_df_cstate(struct
smu_context *smu,
   	uint32_t smu_version;
   	int ret;

+	/*
+	 * Arcturus does not need the cstate disablement
+	 * prerequisite for gpu reset.
+	 */
+	if (amdgpu_in_reset(adev) || adev->in_suspend)
+		return 0;
+
   	ret = smu_cmn_get_smc_version(smu, NULL, &smu_version);
   	if (ret) {
   		dev_err(smu->adev->dev, "Failed to get smu version!\n");
diff
--git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
index 619aee51b123..93a0f7f6a34e 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
@@ -1640,6 +1640,13 @@ static bool aldebaran_is_baco_supported(struct
smu_context *smu)
   static int aldebaran_set_df_cstate(struct smu_context *smu,
   				   enum pp_df_cstate state)
   {
+	/*
+	 * Aldebaran does not need the cstate disablement
+	 * prerequisite for gpu reset.
+	 */
+	if (amdgpu_in_reset(adev) || adev->in_suspend)
+		return 0;
+
   	return smu_cmn_send_smc_msg_with_param(smu,
SMU_MSG_DFCstateControl, state, NULL);
   }




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

  Powered by Linux