On 10/11/2024 11:31 AM, Prike Liang wrote:
> In the normal S3 entry, the TOS cycle counter is not
> reset during BIOS execution the _S3 method, so it doesn't
> determine whether the _S3 method is executed exactly.
> Howerver, the PM core performs the S3 suspend will set the
> PM_SUSPEND_FLAG_FW_RESUME bit if all the devices suspend
> successfully. Therefore, drivers can check the
> pm_suspend_global_flags bit(1) to detect the S3 suspend
> abort event.
>
> Fixes: 4d58c599df75 ("drm/amdgpu: update suspend status
> for aborting from deeper suspend")
> Signed-off-by: Prike Liang <Prike.Liang@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/soc15.c | 26 +++++++++++++++++---------
> 1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
> index 619933f252aa..d07cf8137bd7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> @@ -578,16 +578,18 @@ soc15_asic_reset_method(struct amdgpu_device *adev)
>
> static bool soc15_need_reset_on_resume(struct amdgpu_device *adev)
> {
> - u32 sol_reg;
> -
> - sol_reg = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_81);
> + bool suspend_complete;
>
> /* Will reset for the following suspend abort cases.
> - * 1) Only reset limit on APU side, dGPU hasn't checked yet.
> - * 2) S3 suspend abort and TOS already launched.
> + * 1) Only reset on APU side, dGPU hasn't checked yet.
> + * 2) S3 suspend aborted in the normal S3 suspend or
> + * performing pm core test.
> */
> + suspend_complete = !!(pm_suspend_global_flags &
> + PM_SUSPEND_FLAG_FW_RESUME);
> +
Is this expected to be done after hibernate also? There is a function to
check this - pm_resume_via_firmware()
> if (adev->flags & AMD_IS_APU && adev->in_s3 &&
> - sol_reg) {
> + !suspend_complete) {
> adev->suspend_complete = false;
> return true;
> } else {
> @@ -603,11 +605,17 @@ static int soc15_asic_reset(struct amdgpu_device *adev)
> * successfully. So now, temporarily enable it for the
> * S3 suspend abort case.
> */
> - if (((adev->apu_flags & AMD_APU_IS_RAVEN) ||
> - (adev->apu_flags & AMD_APU_IS_RAVEN2)) &&
> - !soc15_need_reset_on_resume(adev))
> +
> + if ((adev->apu_flags & AMD_APU_IS_PICASSO ||
> + !(adev->apu_flags & AMD_APU_IS_RAVEN)) &&
> + soc15_need_reset_on_resume(adev))
> + goto asic_reset;
> +
> + if ((adev->apu_flags & AMD_APU_IS_RAVEN) ||
> + (adev->apu_flags & AMD_APU_IS_RAVEN2))
> return 0;
If this check happens first, then !(adev->apu_flags & AMD_APU_IS_RAVEN)
and goto/label can be avoided.
Thanks,
Lijo
>
> +asic_reset:
> switch (soc15_asic_reset_method(adev)) {
> case AMD_RESET_METHOD_PCI:
> dev_info(adev->dev, "PCI reset\n");
