[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'.
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);
> > }
> >
