[PATCH 1/2] drm/amdgpu: reposition the gpu reset checking for reuse

[PATCH 1/2] drm/amdgpu: reposition the gpu reset checking for reuse

[Tim Huang]

Move the amdgpu_acpi_should_gpu_reset out of
CONFIG_SUSPEND to share it with hibernate case.

Signed-off-by: Tim Huang <tim.huang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h      |  4 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 40 +++++++++++++-----------
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 5c6132502f35..5bddc03332b3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1392,10 +1392,12 @@ int amdgpu_acpi_smart_shift_update(struct drm_device *dev, enum amdgpu_ss ss_sta
 int amdgpu_acpi_pcie_notify_device_ready(struct amdgpu_device *adev);
 
 void amdgpu_acpi_get_backlight_caps(struct amdgpu_dm_backlight_caps *caps);
+bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev);
 void amdgpu_acpi_detect(void);
 #else
 static inline int amdgpu_acpi_init(struct amdgpu_device *adev) { return 0; }
 static inline void amdgpu_acpi_fini(struct amdgpu_device *adev) { }
+static inline bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev) { return false; }
 static inline void amdgpu_acpi_detect(void) { }
 static inline bool amdgpu_acpi_is_power_shift_control_supported(void) { return false; }
 static inline int amdgpu_acpi_power_shift_control(struct amdgpu_device *adev,
@@ -1406,11 +1408,9 @@ static inline int amdgpu_acpi_smart_shift_update(struct drm_device *dev,
 
 #if defined(CONFIG_ACPI) && defined(CONFIG_SUSPEND)
 bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev);
-bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev);
 bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev);
 #else
 static inline bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev) { return false; }
-static inline bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev) { return false; }
 static inline bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev) { return false; }
 #endif
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 25e902077caf..065944bdeee4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -971,6 +971,28 @@ static bool amdgpu_atcs_pci_probe_handle(struct pci_dev *pdev)
 	return true;
 }
 
+
+/**
+ * amdgpu_acpi_should_gpu_reset
+ *
+ * @adev: amdgpu_device_pointer
+ *
+ * returns true if should reset GPU, false if not
+ */
+bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev)
+{
+	if (adev->flags & AMD_IS_APU)
+		return false;
+
+	if (amdgpu_sriov_vf(adev))
+		return false;
+
+#if IS_ENABLED(CONFIG_SUSPEND)
+	return pm_suspend_target_state != PM_SUSPEND_TO_IDLE;
+#endif /* CONFIG_SUSPEND */
+	return true;
+}
+
 /*
  * amdgpu_acpi_detect - detect ACPI ATIF/ATCS methods
  *
@@ -1042,24 +1064,6 @@ bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev)
 		(pm_suspend_target_state == PM_SUSPEND_MEM);
 }
 
-/**
- * amdgpu_acpi_should_gpu_reset
- *
- * @adev: amdgpu_device_pointer
- *
- * returns true if should reset GPU, false if not
- */
-bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev)
-{
-	if (adev->flags & AMD_IS_APU)
-		return false;
-
-	if (amdgpu_sriov_vf(adev))
-		return false;
-
-	return pm_suspend_target_state != PM_SUSPEND_TO_IDLE;
-}
-
 /**
  * amdgpu_acpi_is_s0ix_active
  *
-- 
2.25.1

[PATCH 2/2] drm/amdgpu: skip ASIC reset for APUs when go to S4

[Tim Huang]

For GC IP v11.0.4/11, PSP TMR need to be reserved
for ASIC mode2 reset. But for S4, when psp suspend,
it will destroy the TMR that fails the ASIC reset.

[  96.006101] amdgpu 0000:62:00.0: amdgpu: MODE2 reset
[  100.409717] amdgpu 0000:62:00.0: amdgpu: SMU: I'm not done with your previous command: SMN_C2PMSG_66:0x00000011 SMN_C2PMSG_82:0x00000002
[  100.411593] amdgpu 0000:62:00.0: amdgpu: Mode2 reset failed!
[  100.412470] amdgpu 0000:62:00.0: PM: pci_pm_freeze(): amdgpu_pmops_freeze+0x0/0x50 [amdgpu] returns -62
[  100.414020] amdgpu 0000:62:00.0: PM: dpm_run_callback(): pci_pm_freeze+0x0/0xd0 returns -62
[  100.415311] amdgpu 0000:62:00.0: PM: pci_pm_freeze+0x0/0xd0 returned -62 after 4623202 usecs
[  100.416608] amdgpu 0000:62:00.0: PM: failed to freeze async: error -62

We can skip the reset on APUs, assuming we can resume them
properly. Verified on some GFX11, GFX10 and old GFX9 APUs.

Signed-off-by: Tim Huang <tim.huang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 5f02c530e2cc..64214996278b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2467,7 +2467,10 @@ static int amdgpu_pmops_freeze(struct device *dev)
 	adev->in_s4 = false;
 	if (r)
 		return r;
-	return amdgpu_asic_reset(adev);
+
+	if (amdgpu_acpi_should_gpu_reset(adev))
+		return amdgpu_asic_reset(adev);
+	return 0;
 }
 
 static int amdgpu_pmops_thaw(struct device *dev)
-- 
2.25.1

Re: [PATCH 1/2] drm/amdgpu: reposition the gpu reset checking for reuse

[Alex Deucher]

On Wed, Mar 15, 2023 at 7:05 AM Tim Huang <tim.huang@amd.com> wrote:
>
> Move the amdgpu_acpi_should_gpu_reset out of
> CONFIG_SUSPEND to share it with hibernate case.
>
> Signed-off-by: Tim Huang <tim.huang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h      |  4 +--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 40 +++++++++++++-----------
>  2 files changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 5c6132502f35..5bddc03332b3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1392,10 +1392,12 @@ int amdgpu_acpi_smart_shift_update(struct drm_device *dev, enum amdgpu_ss ss_sta
>  int amdgpu_acpi_pcie_notify_device_ready(struct amdgpu_device *adev);
>
>  void amdgpu_acpi_get_backlight_caps(struct amdgpu_dm_backlight_caps *caps);
> +bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev);
>  void amdgpu_acpi_detect(void);
>  #else
>  static inline int amdgpu_acpi_init(struct amdgpu_device *adev) { return 0; }
>  static inline void amdgpu_acpi_fini(struct amdgpu_device *adev) { }
> +static inline bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev) { return false; }
>  static inline void amdgpu_acpi_detect(void) { }
>  static inline bool amdgpu_acpi_is_power_shift_control_supported(void) { return false; }
>  static inline int amdgpu_acpi_power_shift_control(struct amdgpu_device *adev,
> @@ -1406,11 +1408,9 @@ static inline int amdgpu_acpi_smart_shift_update(struct drm_device *dev,
>
>  #if defined(CONFIG_ACPI) && defined(CONFIG_SUSPEND)
>  bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev);
> -bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev);
>  bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev);
>  #else
>  static inline bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev) { return false; }
> -static inline bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev) { return false; }
>  static inline bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev) { return false; }
>  #endif
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> index 25e902077caf..065944bdeee4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -971,6 +971,28 @@ static bool amdgpu_atcs_pci_probe_handle(struct pci_dev *pdev)
>         return true;
>  }
>
> +
> +/**
> + * amdgpu_acpi_should_gpu_reset
> + *
> + * @adev: amdgpu_device_pointer
> + *
> + * returns true if should reset GPU, false if not
> + */
> +bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev)
> +{
> +       if (adev->flags & AMD_IS_APU)
> +               return false;
> +
> +       if (amdgpu_sriov_vf(adev))
> +               return false;
> +
> +#if IS_ENABLED(CONFIG_SUSPEND)
> +       return pm_suspend_target_state != PM_SUSPEND_TO_IDLE;
> +#endif /* CONFIG_SUSPEND */
> +       return true;

Should probably be:
#if IS_ENABLED(CONFIG_SUSPEND)
    return pm_suspend_target_state != PM_SUSPEND_TO_IDLE;
#else
    return true;
#endif

With that fixed, series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> +}
> +
>  /*
>   * amdgpu_acpi_detect - detect ACPI ATIF/ATCS methods
>   *
> @@ -1042,24 +1064,6 @@ bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev)
>                 (pm_suspend_target_state == PM_SUSPEND_MEM);
>  }
>
> -/**
> - * amdgpu_acpi_should_gpu_reset
> - *
> - * @adev: amdgpu_device_pointer
> - *
> - * returns true if should reset GPU, false if not
> - */
> -bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev)
> -{
> -       if (adev->flags & AMD_IS_APU)
> -               return false;
> -
> -       if (amdgpu_sriov_vf(adev))
> -               return false;
> -
> -       return pm_suspend_target_state != PM_SUSPEND_TO_IDLE;
> -}
> -
>  /**
>   * amdgpu_acpi_is_s0ix_active
>   *
> --
> 2.25.1
>

RE: [PATCH 1/2] drm/amdgpu: reposition the gpu reset checking for reuse

[Huang, Tim]

[Public]

-----Original Message-----
From: Alex Deucher <alexdeucher@gmail.com>
Sent: Wednesday, March 15, 2023 10:36 PM
To: Huang, Tim <Tim.Huang@amd.com>
Cc: amd-gfx@lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher@amd.com>; Zhang, Yifan <Yifan1.Zhang@amd.com>; Limonciello, Mario <Mario.Limonciello@amd.com>
Subject: Re: [PATCH 1/2] drm/amdgpu: reposition the gpu reset checking for reuse

On Wed, Mar 15, 2023 at 7:05 AM Tim Huang <tim.huang@amd.com> wrote:
>
> Move the amdgpu_acpi_should_gpu_reset out of CONFIG_SUSPEND to share
> it with hibernate case.
>
> Signed-off-by: Tim Huang <tim.huang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h      |  4 +--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 40
> +++++++++++++-----------
>  2 files changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 5c6132502f35..5bddc03332b3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1392,10 +1392,12 @@ int amdgpu_acpi_smart_shift_update(struct
> drm_device *dev, enum amdgpu_ss ss_sta  int
> amdgpu_acpi_pcie_notify_device_ready(struct amdgpu_device *adev);
>
>  void amdgpu_acpi_get_backlight_caps(struct amdgpu_dm_backlight_caps
> *caps);
> +bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev);
>  void amdgpu_acpi_detect(void);
>  #else
>  static inline int amdgpu_acpi_init(struct amdgpu_device *adev) {
> return 0; }  static inline void amdgpu_acpi_fini(struct amdgpu_device
> *adev) { }
> +static inline bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device
> +*adev) { return false; }
>  static inline void amdgpu_acpi_detect(void) { }  static inline bool
> amdgpu_acpi_is_power_shift_control_supported(void) { return false; }
> static inline int amdgpu_acpi_power_shift_control(struct amdgpu_device
> *adev, @@ -1406,11 +1408,9 @@ static inline int
> amdgpu_acpi_smart_shift_update(struct drm_device *dev,
>
>  #if defined(CONFIG_ACPI) && defined(CONFIG_SUSPEND)  bool
> amdgpu_acpi_is_s3_active(struct amdgpu_device *adev); -bool
> amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev);  bool
> amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev);  #else  static
> inline bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev) {
> return false; } -static inline bool
> amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev) { return
> false; }  static inline bool amdgpu_acpi_is_s3_active(struct
> amdgpu_device *adev) { return false; }  #endif
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> index 25e902077caf..065944bdeee4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -971,6 +971,28 @@ static bool amdgpu_atcs_pci_probe_handle(struct pci_dev *pdev)
>         return true;
>  }
>
> +
> +/**
> + * amdgpu_acpi_should_gpu_reset
> + *
> + * @adev: amdgpu_device_pointer
> + *
> + * returns true if should reset GPU, false if not  */ bool
> +amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev) {
> +       if (adev->flags & AMD_IS_APU)
> +               return false;
> +
> +       if (amdgpu_sriov_vf(adev))
> +               return false;
> +
> +#if IS_ENABLED(CONFIG_SUSPEND)
> +       return pm_suspend_target_state != PM_SUSPEND_TO_IDLE; #endif
> +/* CONFIG_SUSPEND */
> +       return true;


Should probably be:
#if IS_ENABLED(CONFIG_SUSPEND)
    return pm_suspend_target_state != PM_SUSPEND_TO_IDLE; #else
    return true;
#endif

Yes, will fix it. Thanks Alex.

With that fixed, series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> +}
> +
>  /*
>   * amdgpu_acpi_detect - detect ACPI ATIF/ATCS methods
>   *
> @@ -1042,24 +1064,6 @@ bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev)
>                 (pm_suspend_target_state == PM_SUSPEND_MEM);  }
>
> -/**
> - * amdgpu_acpi_should_gpu_reset
> - *
> - * @adev: amdgpu_device_pointer
> - *
> - * returns true if should reset GPU, false if not
> - */
> -bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev) -{
> -       if (adev->flags & AMD_IS_APU)
> -               return false;
> -
> -       if (amdgpu_sriov_vf(adev))
> -               return false;
> -
> -       return pm_suspend_target_state != PM_SUSPEND_TO_IDLE;
> -}
> -
>  /**
>   * amdgpu_acpi_is_s0ix_active
>   *
> --
> 2.25.1
>