[PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3

[Limonciello, Mario]

[Public]

Right -from an API perspective both amdgpu_acpi_is_s0ix_active and amdgpu_acpi_is_s3_active are only in suspend ops.

But so coming back to the 4th patch (and the associated bug), what is supposed to happen with a dGPU on an Intel system that does s2i?
For AMD APU w/ dGPU in the system doing s2i I would expect that power rails have been cut off for the dGPU so putting it into S3 and doing a reset makes sense, but I don't know about on an Intel system if that is logical.
It seems like Intel expects more that the card is going to be in runtime pm and putting it into S3 and doing reset might not be the right move.

From: Lazar, Lijo <Lijo.Lazar at amd.com>
Sent: Wednesday, January 26, 2022 09:11
To: Limonciello, Mario <Mario.Limonciello at amd.com>; amd-gfx at lists.freedesktop.org
Cc: Liang, Prike <Prike.Liang at amd.com>
Subject: Re: [PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3

Talking from generic API perspective - S3 is considered active for dGPU only if it's going to non-S0 state. If called from anywhere else than suspend op, this should return false.

Thanks,
Lijo
________________________________
From: Limonciello, Mario <Mario.Limonciello at amd.com<mailto:Mario.Limonciello at amd.com>>
Sent: Wednesday, January 26, 2022 8:37:28 PM
To: Lazar, Lijo <Lijo.Lazar at amd.com<mailto:Lijo.Lazar at amd.com>>; amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> <amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>>
Cc: Liang, Prike <Prike.Liang at amd.com<mailto:Prike.Liang at amd.com>>
Subject: RE: [PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3


[Public]



That was intentional - shouldn't dGPU always be going through S3 path currently?



From: Lazar, Lijo <Lijo.Lazar at amd.com<mailto:Lijo.Lazar at amd.com>>
Sent: Wednesday, January 26, 2022 09:06
To: Limonciello, Mario <Mario.Limonciello at amd.com<mailto:Mario.Limonciello at amd.com>>; amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>
Cc: Liang, Prike <Prike.Liang at amd.com<mailto:Prike.Liang at amd.com>>; Limonciello, Mario <Mario.Limonciello at amd.com<mailto:Mario.Limonciello at amd.com>>
Subject: Re: [PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3



[Public]



Returns true for dGPU always. Better to keep the whole check under something like this.



if (pm_suspend_target_state != PM_SUSPEND_ON)



Thanks,
Lijo

________________________________

From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org<mailto:amd-gfx-bounces at lists.freedesktop.org>> on behalf of Mario Limonciello <mario.limonciello at amd.com<mailto:mario.limonciello at amd.com>>
Sent: Wednesday, January 26, 2022 9:39:42 AM
To: amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> <amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>>
Cc: Liang, Prike <Prike.Liang at amd.com<mailto:Prike.Liang at amd.com>>; Limonciello, Mario <Mario.Limonciello at amd.com<mailto:Mario.Limonciello at amd.com>>
Subject: [PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3



This will be used to help make decisions on what to do in
misconfigured systems.

Signed-off-by: Mario Limonciello <mario.limonciello at amd.com<mailto:mario.limonciello at amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h      |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 17 +++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 3bc76759c143..f184c88d3d4f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1409,11 +1409,13 @@ 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_is_s3_active(struct amdgpu_device *adev);
 bool amdgpu_acpi_is_s0ix_active(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_is_s3_active(struct amdgpu_device *adev) { return false };
 static inline bool amdgpu_acpi_is_s0ix_active(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; }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 2531da6cbec3..df673062bc03 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -1031,6 +1031,23 @@ void amdgpu_acpi_detect(void)
         }
 }

+/**
+ * amdgpu_acpi_is_s3_active
+ *
+ * @adev: amdgpu_device_pointer
+ *
+ * returns true if supported, false if not.
+ */
+bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev)
+{
+#if IS_ENABLED(CONFIG_SUSPEND)
+       return !(adev->flags & AMD_IS_APU) ||
+               pm_suspend_target_state == PM_SUSPEND_MEM;
+#else
+       return false;
+#endif
+}
+
 /**
  * amdgpu_acpi_is_s0ix_active
  *
--
2.25.1
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20220126/3fe628c7/attachment-0001.htm>