* [PATCH v5 1/4] drm/amd: Warn users about potential s0ix problems
@ 2022-01-26 4:09 Mario Limonciello
2022-01-26 4:09 ` [PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3 Mario Limonciello
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Mario Limonciello @ 2022-01-26 4:09 UTC (permalink / raw)
To: amd-gfx; +Cc: Bjoren Dasse, prike.liang, Mario Limonciello
On some OEM setups users can configure the BIOS for S3 or S2idle.
When configured to S3 users can still choose 's2idle' in the kernel by
using `/sys/power/mem_sleep`. Before commit 6dc8265f9803 ("drm/amdgpu:
always reset the asic in suspend (v2)"), the GPU would crash. Now when
configured this way, the system should resume but will use more power.
As such, adjust the `amdpu_acpi_is_s0ix function` to warn users about
potential power consumption issues during their first attempt at
suspending.
Reported-by: Bjoren Dasse <bjoern.daase@gmail.com>
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1824
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 4811b0faafd9..2531da6cbec3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -1040,11 +1040,20 @@ void amdgpu_acpi_detect(void)
*/
bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev)
{
-#if IS_ENABLED(CONFIG_AMD_PMC) && IS_ENABLED(CONFIG_SUSPEND)
- if (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0) {
- if (adev->flags & AMD_IS_APU)
- return pm_suspend_target_state == PM_SUSPEND_TO_IDLE;
- }
-#endif
+#if IS_ENABLED(CONFIG_SUSPEND)
+ if (!(adev->flags & AMD_IS_APU) ||
+ pm_suspend_target_state != PM_SUSPEND_TO_IDLE)
+ return false;
+#else
return false;
+#endif
+ if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0))
+ dev_warn_once(adev->dev,
+ "Power consumption will be higher as BIOS has not been configured for suspend-to-idle.\n"
+ "To use suspend-to-idle change the sleep mode in BIOS setup.\n");
+#if !IS_ENABLED(CONFIG_AMD_PMC)
+ dev_warn_once(adev->dev,
+ "Power consumption will be higher as the kernel has not been compiled with CONFIG_AMD_PMC.\n");
+#endif
+ return true;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3
2022-01-26 4:09 [PATCH v5 1/4] drm/amd: Warn users about potential s0ix problems Mario Limonciello
@ 2022-01-26 4:09 ` Mario Limonciello
2022-01-26 15:06 ` Lazar, Lijo
2022-01-26 4:09 ` [PATCH v5 3/4] drm/amd: Only run s3 or s0ix if system is configured properly Mario Limonciello
2022-01-26 4:09 ` [PATCH v5 4/4] drm/amd: don't reset dGPUs that don't go through system S3 Mario Limonciello
2 siblings, 1 reply; 12+ messages in thread
From: Mario Limonciello @ 2022-01-26 4:09 UTC (permalink / raw)
To: amd-gfx; +Cc: prike.liang, Mario Limonciello
This will be used to help make decisions on what to do in
misconfigured systems.
Signed-off-by: Mario Limonciello <mario.limonciello@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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v5 3/4] drm/amd: Only run s3 or s0ix if system is configured properly
2022-01-26 4:09 [PATCH v5 1/4] drm/amd: Warn users about potential s0ix problems Mario Limonciello
2022-01-26 4:09 ` [PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3 Mario Limonciello
@ 2022-01-26 4:09 ` Mario Limonciello
2022-01-26 4:09 ` [PATCH v5 4/4] drm/amd: don't reset dGPUs that don't go through system S3 Mario Limonciello
2 siblings, 0 replies; 12+ messages in thread
From: Mario Limonciello @ 2022-01-26 4:09 UTC (permalink / raw)
To: amd-gfx; +Cc: prike.liang, Mario Limonciello
This will cause misconfigured systems to not run the GPU suspend
routines.
* In APUs that are properly configured system will go into s2idle.
* In APUs that are intended to be S3 but user selects
s2idle the GPU will stay fully powered for the suspend.
* In APUs that are intended to be s2idle and system misconfigured
the GPU will stay fully powered for the suspend.
* In systems that are intended to be s2idle, but AMD dGPU is also
present, the dGPU will go through S3
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index b0a620c26ae2..123ec5a07dd5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2309,17 +2309,19 @@ static int amdgpu_pmops_suspend(struct device *dev)
{
struct drm_device *drm_dev = dev_get_drvdata(dev);
struct amdgpu_device *adev = drm_to_adev(drm_dev);
- int r;
+ int r = 0;
if (amdgpu_acpi_is_s0ix_active(adev))
adev->in_s0ix = true;
- else
+ else if (amdgpu_acpi_is_s3_active(adev))
adev->in_s3 = true;
- r = amdgpu_device_suspend(drm_dev, true);
- if (r)
- return r;
- if (!adev->in_s0ix)
- r = amdgpu_asic_reset(adev);
+ if (adev->in_s0ix || adev->in_s3) {
+ r = amdgpu_device_suspend(drm_dev, true);
+ if (r)
+ return r;
+ if (!adev->in_s0ix)
+ r = amdgpu_asic_reset(adev);
+ }
return r;
}
@@ -2327,17 +2329,17 @@ static int amdgpu_pmops_resume(struct device *dev)
{
struct drm_device *drm_dev = dev_get_drvdata(dev);
struct amdgpu_device *adev = drm_to_adev(drm_dev);
- int r;
+ int r = 0;
/* Avoids registers access if device is physically gone */
if (!pci_device_is_present(adev->pdev))
adev->no_hw_access = true;
- r = amdgpu_device_resume(drm_dev, true);
- if (amdgpu_acpi_is_s0ix_active(adev))
+ if (adev->in_s0ix || adev->in_s3) {
+ r = amdgpu_device_resume(drm_dev, true);
adev->in_s0ix = false;
- else
adev->in_s3 = false;
+ }
return r;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v5 4/4] drm/amd: don't reset dGPUs that don't go through system S3
2022-01-26 4:09 [PATCH v5 1/4] drm/amd: Warn users about potential s0ix problems Mario Limonciello
2022-01-26 4:09 ` [PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3 Mario Limonciello
2022-01-26 4:09 ` [PATCH v5 3/4] drm/amd: Only run s3 or s0ix if system is configured properly Mario Limonciello
@ 2022-01-26 4:09 ` Mario Limonciello
2022-01-26 14:57 ` Limonciello, Mario
2 siblings, 1 reply; 12+ messages in thread
From: Mario Limonciello @ 2022-01-26 4:09 UTC (permalink / raw)
To: amd-gfx; +Cc: prike.liang, Mario Limonciello
dGPUs connected to Intel systems configured for suspend to idle
will not necessarily have the power rails cut at suspend and
resetting the GPU may lead to problematic behaviors.
Fixes: 6dc8265f9803 ("drm/amdgpu: always reset the asic in suspend (v2)")
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1879
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 123ec5a07dd5..66290f986544 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2319,7 +2319,7 @@ static int amdgpu_pmops_suspend(struct device *dev)
r = amdgpu_device_suspend(drm_dev, true);
if (r)
return r;
- if (!adev->in_s0ix)
+ if (!adev->in_s0ix && pm_suspend_via_firmware())
r = amdgpu_asic_reset(adev);
}
return r;
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* RE: [PATCH v5 4/4] drm/amd: don't reset dGPUs that don't go through system S3
2022-01-26 4:09 ` [PATCH v5 4/4] drm/amd: don't reset dGPUs that don't go through system S3 Mario Limonciello
@ 2022-01-26 14:57 ` Limonciello, Mario
0 siblings, 0 replies; 12+ messages in thread
From: Limonciello, Mario @ 2022-01-26 14:57 UTC (permalink / raw)
To: amd-gfx; +Cc: Liang, Prike
[AMD Official Use Only]
> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> Sent: Tuesday, January 25, 2022 22:10
> To: amd-gfx@lists.freedesktop.org
> Cc: Liang, Prike <Prike.Liang@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>
> Subject: [PATCH v5 4/4] drm/amd: don't reset dGPUs that don't go through
> system S3
>
> dGPUs connected to Intel systems configured for suspend to idle
> will not necessarily have the power rails cut at suspend and
> resetting the GPU may lead to problematic behaviors.
>
> Fixes: 6dc8265f9803 ("drm/amdgpu: always reset the asic in suspend (v2)")
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1879
Testing has failed with this patch on the original issue mentioned above, so if
the rest of this series is OK, this one at least should be dropped for now.
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 123ec5a07dd5..66290f986544 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2319,7 +2319,7 @@ static int amdgpu_pmops_suspend(struct device
> *dev)
> r = amdgpu_device_suspend(drm_dev, true);
> if (r)
> return r;
> - if (!adev->in_s0ix)
> + if (!adev->in_s0ix && pm_suspend_via_firmware())
> r = amdgpu_asic_reset(adev);
> }
> return r;
> --
> 2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3
2022-01-26 4:09 ` [PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3 Mario Limonciello
@ 2022-01-26 15:06 ` Lazar, Lijo
2022-01-26 15:07 ` Limonciello, Mario
0 siblings, 1 reply; 12+ messages in thread
From: Lazar, Lijo @ 2022-01-26 15:06 UTC (permalink / raw)
To: Limonciello, Mario, amd-gfx; +Cc: Liang, Prike, Limonciello, Mario
[-- Attachment #1: Type: text/plain, Size: 2772 bytes --]
[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@lists.freedesktop.org> on behalf of Mario Limonciello <mario.limonciello@amd.com>
Sent: Wednesday, January 26, 2022 9:39:42 AM
To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Liang, Prike <Prike.Liang@amd.com>; Limonciello, Mario <Mario.Limonciello@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@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
[-- Attachment #2: Type: text/html, Size: 4300 bytes --]
^ permalink raw reply related [flat|nested] 12+ messages in thread
* RE: [PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3
2022-01-26 15:06 ` Lazar, Lijo
@ 2022-01-26 15:07 ` Limonciello, Mario
2022-01-26 15:11 ` Lazar, Lijo
0 siblings, 1 reply; 12+ messages in thread
From: Limonciello, Mario @ 2022-01-26 15:07 UTC (permalink / raw)
To: Lazar, Lijo, amd-gfx; +Cc: Liang, Prike
[-- Attachment #1: Type: text/plain, Size: 3465 bytes --]
[Public]
That was intentional - shouldn't dGPU always be going through S3 path currently?
From: Lazar, Lijo <Lijo.Lazar@amd.com>
Sent: Wednesday, January 26, 2022 09:06
To: Limonciello, Mario <Mario.Limonciello@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Liang, Prike <Prike.Liang@amd.com>; Limonciello, Mario <Mario.Limonciello@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@lists.freedesktop.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Mario Limonciello <mario.limonciello@amd.com<mailto:mario.limonciello@amd.com>>
Sent: Wednesday, January 26, 2022 9:39:42 AM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Liang, Prike <Prike.Liang@amd.com<mailto:Prike.Liang@amd.com>>; Limonciello, Mario <Mario.Limonciello@amd.com<mailto:Mario.Limonciello@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@amd.com<mailto:mario.limonciello@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
[-- Attachment #2: Type: text/html, Size: 7788 bytes --]
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3
2022-01-26 15:07 ` Limonciello, Mario
@ 2022-01-26 15:11 ` Lazar, Lijo
2022-01-26 15:17 ` Limonciello, Mario
0 siblings, 1 reply; 12+ messages in thread
From: Lazar, Lijo @ 2022-01-26 15:11 UTC (permalink / raw)
To: Limonciello, Mario, amd-gfx; +Cc: Liang, Prike
[-- Attachment #1: Type: text/plain, Size: 4063 bytes --]
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@amd.com>
Sent: Wednesday, January 26, 2022 8:37:28 PM
To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Liang, Prike <Prike.Liang@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@amd.com>
Sent: Wednesday, January 26, 2022 09:06
To: Limonciello, Mario <Mario.Limonciello@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Liang, Prike <Prike.Liang@amd.com>; Limonciello, Mario <Mario.Limonciello@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@lists.freedesktop.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Mario Limonciello <mario.limonciello@amd.com<mailto:mario.limonciello@amd.com>>
Sent: Wednesday, January 26, 2022 9:39:42 AM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Liang, Prike <Prike.Liang@amd.com<mailto:Prike.Liang@amd.com>>; Limonciello, Mario <Mario.Limonciello@amd.com<mailto:Mario.Limonciello@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@amd.com<mailto:mario.limonciello@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
[-- Attachment #2: Type: text/html, Size: 7622 bytes --]
^ permalink raw reply related [flat|nested] 12+ messages in thread
* RE: [PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3
2022-01-26 15:11 ` Lazar, Lijo
@ 2022-01-26 15:17 ` Limonciello, Mario
2022-01-26 15:32 ` Lazar, Lijo
0 siblings, 1 reply; 12+ messages in thread
From: Limonciello, Mario @ 2022-01-26 15:17 UTC (permalink / raw)
To: Lazar, Lijo, amd-gfx; +Cc: Liang, Prike
[-- Attachment #1: Type: text/plain, Size: 5324 bytes --]
[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@amd.com>
Sent: Wednesday, January 26, 2022 09:11
To: Limonciello, Mario <Mario.Limonciello@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Liang, Prike <Prike.Liang@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@amd.com<mailto:Mario.Limonciello@amd.com>>
Sent: Wednesday, January 26, 2022 8:37:28 PM
To: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Liang, Prike <Prike.Liang@amd.com<mailto:Prike.Liang@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@amd.com<mailto:Lijo.Lazar@amd.com>>
Sent: Wednesday, January 26, 2022 09:06
To: Limonciello, Mario <Mario.Limonciello@amd.com<mailto:Mario.Limonciello@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Liang, Prike <Prike.Liang@amd.com<mailto:Prike.Liang@amd.com>>; Limonciello, Mario <Mario.Limonciello@amd.com<mailto:Mario.Limonciello@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@lists.freedesktop.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Mario Limonciello <mario.limonciello@amd.com<mailto:mario.limonciello@amd.com>>
Sent: Wednesday, January 26, 2022 9:39:42 AM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Liang, Prike <Prike.Liang@amd.com<mailto:Prike.Liang@amd.com>>; Limonciello, Mario <Mario.Limonciello@amd.com<mailto:Mario.Limonciello@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@amd.com<mailto:mario.limonciello@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
[-- Attachment #2: Type: text/html, Size: 11363 bytes --]
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3
2022-01-26 15:17 ` Limonciello, Mario
@ 2022-01-26 15:32 ` Lazar, Lijo
2022-01-26 15:40 ` Alex Deucher
0 siblings, 1 reply; 12+ messages in thread
From: Lazar, Lijo @ 2022-01-26 15:32 UTC (permalink / raw)
To: Limonciello, Mario, amd-gfx; +Cc: Liang, Prike
[-- Attachment #1: Type: text/plain, Size: 5911 bytes --]
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 <Mario.Limonciello@amd.com>
Sent: Wednesday, January 26, 2022 8:47:05 PM
To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Liang, Prike <Prike.Liang@amd.com>
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 <Lijo.Lazar@amd.com>
Sent: Wednesday, January 26, 2022 09:11
To: Limonciello, Mario <Mario.Limonciello@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Liang, Prike <Prike.Liang@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@amd.com<mailto:Mario.Limonciello@amd.com>>
Sent: Wednesday, January 26, 2022 8:37:28 PM
To: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Liang, Prike <Prike.Liang@amd.com<mailto:Prike.Liang@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@amd.com<mailto:Lijo.Lazar@amd.com>>
Sent: Wednesday, January 26, 2022 09:06
To: Limonciello, Mario <Mario.Limonciello@amd.com<mailto:Mario.Limonciello@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Liang, Prike <Prike.Liang@amd.com<mailto:Prike.Liang@amd.com>>; Limonciello, Mario <Mario.Limonciello@amd.com<mailto:Mario.Limonciello@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@lists.freedesktop.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Mario Limonciello <mario.limonciello@amd.com<mailto:mario.limonciello@amd.com>>
Sent: Wednesday, January 26, 2022 9:39:42 AM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Liang, Prike <Prike.Liang@amd.com<mailto:Prike.Liang@amd.com>>; Limonciello, Mario <Mario.Limonciello@amd.com<mailto:Mario.Limonciello@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@amd.com<mailto:mario.limonciello@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
[-- Attachment #2: Type: text/html, Size: 11118 bytes --]
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3
2022-01-26 15:32 ` Lazar, Lijo
@ 2022-01-26 15:40 ` Alex Deucher
2022-01-26 16:31 ` Limonciello, Mario
0 siblings, 1 reply; 12+ messages in thread
From: Alex Deucher @ 2022-01-26 15:40 UTC (permalink / raw)
To: Lazar, Lijo; +Cc: Liang, Prike, Limonciello, Mario, amd-gfx
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 <Lijo.Lazar@amd.com> 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 <Mario.Limonciello@amd.com>
> Sent: Wednesday, January 26, 2022 8:47:05 PM
> To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
> Cc: Liang, Prike <Prike.Liang@amd.com>
> 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 <Lijo.Lazar@amd.com>
> Sent: Wednesday, January 26, 2022 09:11
> To: Limonciello, Mario <Mario.Limonciello@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Liang, Prike <Prike.Liang@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@amd.com>
> Sent: Wednesday, January 26, 2022 8:37:28 PM
> To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
> Cc: Liang, Prike <Prike.Liang@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@amd.com>
> Sent: Wednesday, January 26, 2022 09:06
> To: Limonciello, Mario <Mario.Limonciello@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Liang, Prike <Prike.Liang@amd.com>; Limonciello, Mario <Mario.Limonciello@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@lists.freedesktop.org> on behalf of Mario Limonciello <mario.limonciello@amd.com>
> Sent: Wednesday, January 26, 2022 9:39:42 AM
> To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
> Cc: Liang, Prike <Prike.Liang@amd.com>; Limonciello, Mario <Mario.Limonciello@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@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
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3
2022-01-26 15:40 ` Alex Deucher
@ 2022-01-26 16:31 ` Limonciello, Mario
0 siblings, 0 replies; 12+ messages in thread
From: Limonciello, Mario @ 2022-01-26 16:31 UTC (permalink / raw)
To: Alex Deucher, Lazar, Lijo; +Cc: Liang, Prike, amd-gfx
[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 <Lijo.Lazar@amd.com> 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 <Mario.Limonciello@amd.com>
> > Sent: Wednesday, January 26, 2022 8:47:05 PM
> > To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org <amd-
> gfx@lists.freedesktop.org>
> > Cc: Liang, Prike <Prike.Liang@amd.com>
> > 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 <Lijo.Lazar@amd.com>
> > Sent: Wednesday, January 26, 2022 09:11
> > To: Limonciello, Mario <Mario.Limonciello@amd.com>; amd-
> gfx@lists.freedesktop.org
> > Cc: Liang, Prike <Prike.Liang@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@amd.com>
> > Sent: Wednesday, January 26, 2022 8:37:28 PM
> > To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org <amd-
> gfx@lists.freedesktop.org>
> > Cc: Liang, Prike <Prike.Liang@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@amd.com>
> > Sent: Wednesday, January 26, 2022 09:06
> > To: Limonciello, Mario <Mario.Limonciello@amd.com>; amd-
> gfx@lists.freedesktop.org
> > Cc: Liang, Prike <Prike.Liang@amd.com>; Limonciello, Mario
> <Mario.Limonciello@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@lists.freedesktop.org> on behalf of Mario
> Limonciello <mario.limonciello@amd.com>
> > Sent: Wednesday, January 26, 2022 9:39:42 AM
> > To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
> > Cc: Liang, Prike <Prike.Liang@amd.com>; Limonciello, Mario
> <Mario.Limonciello@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@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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-01-26 16:31 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-26 4:09 [PATCH v5 1/4] drm/amd: Warn users about potential s0ix problems Mario Limonciello
2022-01-26 4:09 ` [PATCH v5 2/4] drm/amd: add support to check whether the system is set to s3 Mario Limonciello
2022-01-26 15:06 ` Lazar, Lijo
2022-01-26 15:07 ` Limonciello, Mario
2022-01-26 15:11 ` Lazar, Lijo
2022-01-26 15:17 ` Limonciello, Mario
2022-01-26 15:32 ` Lazar, Lijo
2022-01-26 15:40 ` Alex Deucher
2022-01-26 16:31 ` Limonciello, Mario
2022-01-26 4:09 ` [PATCH v5 3/4] drm/amd: Only run s3 or s0ix if system is configured properly Mario Limonciello
2022-01-26 4:09 ` [PATCH v5 4/4] drm/amd: don't reset dGPUs that don't go through system S3 Mario Limonciello
2022-01-26 14:57 ` Limonciello, Mario
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.