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

2022-01-25 Thread Mario Limonciello
This will be used to help make decisions on what to do in
misconfigured systems.

Signed-off-by: Mario Limonciello 
---
 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



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

2022-01-26 Thread Lazar, Lijo
[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  on behalf of Mario 
Limonciello 
Sent: Wednesday, January 26, 2022 9:39:42 AM
To: amd-gfx@lists.freedesktop.org 
Cc: Liang, Prike ; Limonciello, Mario 

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 
---
 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



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

2022-01-26 Thread Limonciello, Mario
[Public]

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

From: Lazar, Lijo 
Sent: Wednesday, January 26, 2022 09:06
To: Limonciello, Mario ; 
amd-gfx@lists.freedesktop.org
Cc: Liang, Prike ; Limonciello, Mario 

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 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of Mario Limonciello 
mailto:mario.limoncie...@amd.com>>
Sent: Wednesday, January 26, 2022 9:39:42 AM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Liang, Prike mailto:prike.li...@amd.com>>; 
Limonciello, Mario mailto:mario.limoncie...@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 
mailto:mario.limoncie...@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


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

2022-01-26 Thread Lazar, Lijo
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 
Sent: Wednesday, January 26, 2022 8:37:28 PM
To: Lazar, Lijo ; amd-gfx@lists.freedesktop.org 

Cc: Liang, Prike 
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 
Sent: Wednesday, January 26, 2022 09:06
To: Limonciello, Mario ; 
amd-gfx@lists.freedesktop.org
Cc: Liang, Prike ; Limonciello, Mario 

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 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of Mario Limonciello 
mailto:mario.limoncie...@amd.com>>
Sent: Wednesday, January 26, 2022 9:39:42 AM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Liang, Prike mailto:prike.li...@amd.com>>; 
Limonciello, Mario mailto:mario.limoncie...@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 
mailto:mario.limoncie...@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


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

2022-01-26 Thread 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 
Sent: Wednesday, January 26, 2022 09:11
To: Limonciello, Mario ; 
amd-gfx@lists.freedesktop.org
Cc: Liang, Prike 
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 
mailto:mario.limoncie...@amd.com>>
Sent: Wednesday, January 26, 2022 8:37:28 PM
To: Lazar, Lijo mailto:lijo.la...@amd.com>>; 
amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Liang, Prike mailto:prike.li...@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 mailto:lijo.la...@amd.com>>
Sent: Wednesday, January 26, 2022 09:06
To: Limonciello, Mario 
mailto:mario.limoncie...@amd.com>>; 
amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Liang, Prike mailto:prike.li...@amd.com>>; 
Limonciello, Mario mailto:mario.limoncie...@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 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of Mario Limonciello 
mailto:mario.limoncie...@amd.com>>
Sent: Wednesday, January 26, 2022 9:39:42 AM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Liang, Prike mailto:prike.li...@amd.com>>; 
Limonciello, Mario mailto:mario.limoncie...@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 
mailto:mario.limoncie...@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


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

2022-01-26 Thread Lazar, Lijo
I remember Alex adding a patch for smart suspend such that it skips the suspend 
call if runtime pm suspended.

In summary, the resume doesn't work with/without reset?

Thanks,
Lijo

From: Limonciello, Mario 
Sent: Wednesday, January 26, 2022 8:47:05 PM
To: Lazar, Lijo ; amd-gfx@lists.freedesktop.org 

Cc: Liang, Prike 
Subject: RE: [PATCH v5 2/4] drm/amd: add support to check whether the system is 
set to s3


[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 
Sent: Wednesday, January 26, 2022 09:11
To: Limonciello, Mario ; 
amd-gfx@lists.freedesktop.org
Cc: Liang, Prike 
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 
mailto:mario.limoncie...@amd.com>>
Sent: Wednesday, January 26, 2022 8:37:28 PM
To: Lazar, Lijo mailto:lijo.la...@amd.com>>; 
amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Liang, Prike mailto:prike.li...@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 mailto:lijo.la...@amd.com>>
Sent: Wednesday, January 26, 2022 09:06
To: Limonciello, Mario 
mailto:mario.limoncie...@amd.com>>; 
amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Liang, Prike mailto:prike.li...@amd.com>>; 
Limonciello, Mario mailto:mario.limoncie...@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 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of Mario Limonciello 
mailto:mario.limoncie...@amd.com>>
Sent: Wednesday, January 26, 2022 9:39:42 AM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Liang, Prike mailto:prike.li...@amd.com>>; 
Limonciello, Mario mailto:mario.limoncie...@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 
mailto:mario.limoncie...@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
+ *
+ * @

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

2022-01-26 Thread Alex Deucher
I don't think smart suspend works as expected.  I asked Raphael about
it several times, but he never got around to following up with me.  I
think that is probably the preferred way to go, but the tricky part is
that the dGPUs have integrated bridges and audio and usb and all of
that probably needs proper smart suspend support for this to work
properly.  Alternatively, the OS needs to properly use the ACPI _PR3
methods to power down all of the devices on suspend if the system
doesn't automatically take down the power rails when the system enters
suspend.  I'm not sure Linux does this today.

Alex

On Wed, Jan 26, 2022 at 10:32 AM Lazar, Lijo  wrote:
>
> I remember Alex adding a patch for smart suspend such that it skips the 
> suspend call if runtime pm suspended.
>
> In summary, the resume doesn't work with/without reset?
>
> Thanks,
> Lijo
> 
> From: Limonciello, Mario 
> Sent: Wednesday, January 26, 2022 8:47:05 PM
> To: Lazar, Lijo ; amd-gfx@lists.freedesktop.org 
> 
> Cc: Liang, Prike 
> Subject: RE: [PATCH v5 2/4] drm/amd: add support to check whether the system 
> is set to s3
>
>
> [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 
> Sent: Wednesday, January 26, 2022 09:11
> To: Limonciello, Mario ; 
> amd-gfx@lists.freedesktop.org
> Cc: Liang, Prike 
> 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 
> Sent: Wednesday, January 26, 2022 8:37:28 PM
> To: Lazar, Lijo ; amd-gfx@lists.freedesktop.org 
> 
> Cc: Liang, Prike 
> 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 
> Sent: Wednesday, January 26, 2022 09:06
> To: Limonciello, Mario ; 
> amd-gfx@lists.freedesktop.org
> Cc: Liang, Prike ; Limonciello, Mario 
> 
> 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  on behalf of Mario 
> Limonciello 
> Sent: Wednesday, January 26, 2022 9:39:42 AM
> To: amd-gfx@lists.freedesktop.org 
> Cc: Liang, Prike ; Limonciello, Mario 
> 
> 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 
> ---
>  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) { }
>

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

2022-01-26 Thread Limonciello, Mario
[AMD Official Use Only]

They key here is that smart suspend seems to have a dependency on
pm_suspend_via_firmware().

So if you have an APU doing S2I or Intel SOC doing S2I it will always return 0.
Can we drop that dependency of pm_suspend_via_firmware for it perhaps?

> I don't think smart suspend works as expected.  I asked Raphael about
> it several times, but he never got around to following up with me.  I
> think that is probably the preferred way to go, but the tricky part is
> that the dGPUs have integrated bridges and audio and usb and all of
> that probably needs proper smart suspend support for this to work
> properly.  Alternatively, the OS needs to properly use the ACPI _PR3
> methods to power down all of the devices on suspend if the system
> doesn't automatically take down the power rails when the system enters
> suspend.  I'm not sure Linux does this today.
> 
> Alex
> 
> On Wed, Jan 26, 2022 at 10:32 AM Lazar, Lijo  wrote:
> >
> > I remember Alex adding a patch for smart suspend such that it skips the
> suspend call if runtime pm suspended.
> >
> > In summary, the resume doesn't work with/without reset?
> >
> > Thanks,
> > Lijo
> > 
> > From: Limonciello, Mario 
> > Sent: Wednesday, January 26, 2022 8:47:05 PM
> > To: Lazar, Lijo ; amd-gfx@lists.freedesktop.org  g...@lists.freedesktop.org>
> > Cc: Liang, Prike 
> > Subject: RE: [PATCH v5 2/4] drm/amd: add support to check whether the
> system is set to s3
> >
> >
> > [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 
> > Sent: Wednesday, January 26, 2022 09:11
> > To: Limonciello, Mario ; amd-
> g...@lists.freedesktop.org
> > Cc: Liang, Prike 
> > 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 
> > Sent: Wednesday, January 26, 2022 8:37:28 PM
> > To: Lazar, Lijo ; amd-gfx@lists.freedesktop.org  g...@lists.freedesktop.org>
> > Cc: Liang, Prike 
> > 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 
> > Sent: Wednesday, January 26, 2022 09:06
> > To: Limonciello, Mario ; amd-
> g...@lists.freedesktop.org
> > Cc: Liang, Prike ; Limonciello, Mario
> 
> > 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  on behalf of Mario
> Limonciello 
> > Sent: Wednesday, January 26, 2022 9:39:42 AM
> > To: amd-gfx@lists.freedesktop.org 
> > Cc: Liang, Prike ; Limonciello, Mario
> 
> > 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 
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu.h  |  2 ++
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 17 +