* [PATCH v6 1/4] drm/amd: avoid suspend on dGPUs w/ s2idle support when runtime PM enabled
@ 2022-01-26 22:58 Mario Limonciello
2022-01-26 22:58 ` [PATCH v6 2/4] drm/amd: Warn users about potential s0ix problems Mario Limonciello
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Mario Limonciello @ 2022-01-26 22:58 UTC (permalink / raw)
To: amd-gfx; +Cc: Prike.Liang, Mario Limonciello
dGPUs connected to Intel systems configured for suspend to idle
will not have the power rails cut at suspend and resetting the GPU
may lead to problematic behaviors.
Fixes: e25443d2765f4 ("drm/amdgpu: add a dev_pm_ops prepare callback (v2)")
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1879
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v5->v6:
* Move patch to the start of the series so it can go in even if the rest need
work
* Change approach that if dGPU is in BOCO just skip suspend
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index b0a620c26ae2..922accdd4246 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2294,8 +2294,7 @@ static int amdgpu_pmops_prepare(struct device *dev)
* DPM_FLAG_SMART_SUSPEND works properly
*/
if (amdgpu_device_supports_boco(drm_dev))
- return pm_runtime_suspended(dev) &&
- pm_suspend_via_firmware();
+ return pm_runtime_suspended(dev);
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v6 2/4] drm/amd: Warn users about potential s0ix problems 2022-01-26 22:58 [PATCH v6 1/4] drm/amd: avoid suspend on dGPUs w/ s2idle support when runtime PM enabled Mario Limonciello @ 2022-01-26 22:58 ` Mario Limonciello 2022-01-26 22:58 ` [PATCH v6 3/4] drm/amd: add support to check whether the system is set to s3 Mario Limonciello 2022-01-26 22:58 ` [PATCH v6 4/4] drm/amd: Only run s3 or s0ix if system is configured properly Mario Limonciello 2 siblings, 0 replies; 6+ messages in thread From: Mario Limonciello @ 2022-01-26 22:58 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> --- v5->v6: * Make CONFIG_SUSPEND wrap around entire function to make more readable * Return false if system not properly configured (relevant with later patch) drivers/gpu/drm/amd/amdgpu/amdgpu.h | 8 ++++++-- drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 24 +++++++++++++++++++----- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 3bc76759c143..b1db703753f2 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1409,12 +1409,10 @@ 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_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_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; } static inline int amdgpu_acpi_power_shift_control(struct amdgpu_device *adev, @@ -1423,6 +1421,12 @@ static inline int amdgpu_acpi_smart_shift_update(struct drm_device *dev, enum amdgpu_ss ss_state) { return 0; } #endif +#if defined(CONFIG_ACPI) && defined(CONFIG_SUSPEND) +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; } +#endif + int amdgpu_cs_find_mapping(struct amdgpu_cs_parser *parser, uint64_t addr, struct amdgpu_bo **bo, struct amdgpu_bo_va_mapping **mapping); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c index 4811b0faafd9..b19d40751802 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c @@ -1031,6 +1031,7 @@ void amdgpu_acpi_detect(void) } } +#if IS_ENABLED(CONFIG_SUSPEND) /** * amdgpu_acpi_is_s0ix_active * @@ -1040,11 +1041,24 @@ 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; + if (!(adev->flags & AMD_IS_APU) || + (pm_suspend_target_state != PM_SUSPEND_TO_IDLE)) + return false; + + 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"); + return false; } -#endif + +#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"); return false; +#else + return true; +#endif /* CONFIG_AMD_PMC */ } + +#endif /* CONFIG_SUSPEND */ -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v6 3/4] drm/amd: add support to check whether the system is set to s3 2022-01-26 22:58 [PATCH v6 1/4] drm/amd: avoid suspend on dGPUs w/ s2idle support when runtime PM enabled Mario Limonciello 2022-01-26 22:58 ` [PATCH v6 2/4] drm/amd: Warn users about potential s0ix problems Mario Limonciello @ 2022-01-26 22:58 ` Mario Limonciello 2022-01-26 22:58 ` [PATCH v6 4/4] drm/amd: Only run s3 or s0ix if system is configured properly Mario Limonciello 2 siblings, 0 replies; 6+ messages in thread From: Mario Limonciello @ 2022-01-26 22:58 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> --- v5->v6: * Move in CONFIG_SUSPEND block drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 13 +++++++++++++ 2 files changed, 15 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index b1db703753f2..30dc18c2d1fc 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1422,9 +1422,11 @@ static inline int amdgpu_acpi_smart_shift_update(struct drm_device *dev, #endif #if defined(CONFIG_ACPI) && defined(CONFIG_SUSPEND) +bool amdgpu_acpi_is_s3_active(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_is_s3_active(struct amdgpu_device *adev) { return false }; #endif int amdgpu_cs_find_mapping(struct amdgpu_cs_parser *parser, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c index b19d40751802..0e12315fa0cb 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c @@ -1032,6 +1032,19 @@ void amdgpu_acpi_detect(void) } #if IS_ENABLED(CONFIG_SUSPEND) +/** + * 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) +{ + return !(adev->flags & AMD_IS_APU) || + (pm_suspend_target_state == PM_SUSPEND_MEM); +} + /** * amdgpu_acpi_is_s0ix_active * -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v6 4/4] drm/amd: Only run s3 or s0ix if system is configured properly 2022-01-26 22:58 [PATCH v6 1/4] drm/amd: avoid suspend on dGPUs w/ s2idle support when runtime PM enabled Mario Limonciello 2022-01-26 22:58 ` [PATCH v6 2/4] drm/amd: Warn users about potential s0ix problems Mario Limonciello 2022-01-26 22:58 ` [PATCH v6 3/4] drm/amd: add support to check whether the system is set to s3 Mario Limonciello @ 2022-01-26 22:58 ` Mario Limonciello 2022-01-27 14:50 ` Deucher, Alexander 2 siblings, 1 reply; 6+ messages in thread From: Mario Limonciello @ 2022-01-26 22:58 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> --- v5->v6: * Move code into prepare vfunc and use DPM_FLAG_SMART_SUSPEND to skip suspend code in incorrectly configured systems. drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 922accdd4246..3e581f35f19d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -2289,6 +2289,7 @@ static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work) static int amdgpu_pmops_prepare(struct device *dev) { struct drm_device *drm_dev = dev_get_drvdata(dev); + struct amdgpu_device *adev = drm_to_adev(drm_dev); /* Return a positive number here so * DPM_FLAG_SMART_SUSPEND works properly @@ -2296,6 +2297,13 @@ static int amdgpu_pmops_prepare(struct device *dev) if (amdgpu_device_supports_boco(drm_dev)) return pm_runtime_suspended(dev); + /* if we will not support s3 or s2i for the device + * then skip suspend + */ + if (!amdgpu_acpi_is_s0ix_active(adev) && + !amdgpu_acpi_is_s3_active(adev)) + return 1; + return 0; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v6 4/4] drm/amd: Only run s3 or s0ix if system is configured properly 2022-01-26 22:58 ` [PATCH v6 4/4] drm/amd: Only run s3 or s0ix if system is configured properly Mario Limonciello @ 2022-01-27 14:50 ` Deucher, Alexander 2022-01-27 19:31 ` Limonciello, Mario 0 siblings, 1 reply; 6+ messages in thread From: Deucher, Alexander @ 2022-01-27 14:50 UTC (permalink / raw) To: Limonciello, Mario, amd-gfx; +Cc: Liang, Prike [-- Attachment #1: Type: text/plain, Size: 2354 bytes --] [Public] Series is: Reviewed-by: Alex Deucher <alexander.deucher@amd.com> ________________________________ From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Mario Limonciello <mario.limonciello@amd.com> Sent: Wednesday, January 26, 2022 5:58 PM 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 v6 4/4] drm/amd: Only run s3 or s0ix if system is configured properly 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> --- v5->v6: * Move code into prepare vfunc and use DPM_FLAG_SMART_SUSPEND to skip suspend code in incorrectly configured systems. drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 922accdd4246..3e581f35f19d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -2289,6 +2289,7 @@ static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work) static int amdgpu_pmops_prepare(struct device *dev) { struct drm_device *drm_dev = dev_get_drvdata(dev); + struct amdgpu_device *adev = drm_to_adev(drm_dev); /* Return a positive number here so * DPM_FLAG_SMART_SUSPEND works properly @@ -2296,6 +2297,13 @@ static int amdgpu_pmops_prepare(struct device *dev) if (amdgpu_device_supports_boco(drm_dev)) return pm_runtime_suspended(dev); + /* if we will not support s3 or s2i for the device + * then skip suspend + */ + if (!amdgpu_acpi_is_s0ix_active(adev) && + !amdgpu_acpi_is_s3_active(adev)) + return 1; + return 0; } -- 2.25.1 [-- Attachment #2: Type: text/html, Size: 4180 bytes --] ^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH v6 4/4] drm/amd: Only run s3 or s0ix if system is configured properly 2022-01-27 14:50 ` Deucher, Alexander @ 2022-01-27 19:31 ` Limonciello, Mario 0 siblings, 0 replies; 6+ messages in thread From: Limonciello, Mario @ 2022-01-27 19:31 UTC (permalink / raw) To: Deucher, Alexander, amd-gfx; +Cc: Liang, Prike [-- Attachment #1: Type: text/plain, Size: 3201 bytes --] [Public] Thanks for the review. I'm only going to merge 2-4 initially though. The reporter on 1 has some questionable results, and I have a follow up patch for them to try. If that combined with 1 looks good I'll bring that patch for review. From: Deucher, Alexander <Alexander.Deucher@amd.com> Sent: Thursday, January 27, 2022 08:50 To: Limonciello, Mario <Mario.Limonciello@amd.com>; amd-gfx@lists.freedesktop.org Cc: Liang, Prike <Prike.Liang@amd.com> Subject: Re: [PATCH v6 4/4] drm/amd: Only run s3 or s0ix if system is configured properly [Public] Series is: Reviewed-by: Alex Deucher <alexander.deucher@amd.com<mailto:alexander.deucher@amd.com>> ________________________________ 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 5:58 PM 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 v6 4/4] drm/amd: Only run s3 or s0ix if system is configured properly 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<mailto:mario.limonciello@amd.com>> --- v5->v6: * Move code into prepare vfunc and use DPM_FLAG_SMART_SUSPEND to skip suspend code in incorrectly configured systems. drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 922accdd4246..3e581f35f19d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -2289,6 +2289,7 @@ static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work) static int amdgpu_pmops_prepare(struct device *dev) { struct drm_device *drm_dev = dev_get_drvdata(dev); + struct amdgpu_device *adev = drm_to_adev(drm_dev); /* Return a positive number here so * DPM_FLAG_SMART_SUSPEND works properly @@ -2296,6 +2297,13 @@ static int amdgpu_pmops_prepare(struct device *dev) if (amdgpu_device_supports_boco(drm_dev)) return pm_runtime_suspended(dev); + /* if we will not support s3 or s2i for the device + * then skip suspend + */ + if (!amdgpu_acpi_is_s0ix_active(adev) && + !amdgpu_acpi_is_s3_active(adev)) + return 1; + return 0; } -- 2.25.1 [-- Attachment #2: Type: text/html, Size: 7642 bytes --] ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-27 19:31 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-26 22:58 [PATCH v6 1/4] drm/amd: avoid suspend on dGPUs w/ s2idle support when runtime PM enabled Mario Limonciello 2022-01-26 22:58 ` [PATCH v6 2/4] drm/amd: Warn users about potential s0ix problems Mario Limonciello 2022-01-26 22:58 ` [PATCH v6 3/4] drm/amd: add support to check whether the system is set to s3 Mario Limonciello 2022-01-26 22:58 ` [PATCH v6 4/4] drm/amd: Only run s3 or s0ix if system is configured properly Mario Limonciello 2022-01-27 14:50 ` Deucher, Alexander 2022-01-27 19:31 ` 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.