RE: [PATCH 3/3] drm/amd: Don't always set s3 for dGPUs in all sleep modes
[AMD Official Use Only - General]
> -Original Message-
> From: amd-gfx On Behalf Of
> Mario Limonciello
> Sent: Tuesday, February 21, 2023 4:16 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Peter Kopec ; Limonciello, Mario
>
> Subject: [PATCH 3/3] drm/amd: Don't always set s3 for dGPUs in all sleep
> modes
>
> dGPUs that will be using BACO or BOCO shouldn't be put into S3
> when the system is being put into s2idle.
Will this break regular suspend to ram scenario? I mean
- For s2idle scenario, amdgpu_pmops_prepare will be called. And for this
scenario, it is expected dgpu using BACO or BOCO to be not put into S3. Right?
- For regular s2ram scenario, amdgpu_pmops_prepare will also get called. Then
dgpu using BACO or BOCO will be not put into S3 either?
BR
Evan
>
> Cc: Peter Kopec
> Signed-off-by: Mario Limonciello
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> index 25e902077caf..5c69116bc883 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -1038,8 +1038,13 @@ void amdgpu_acpi_detect(void)
> */
> bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev)
> {
> - return !(adev->flags & AMD_IS_APU) ||
> - (pm_suspend_target_state == PM_SUSPEND_MEM);
> + if (pm_suspend_target_state == PM_SUSPEND_MEM)
> + return true;
> + if (adev->flags & AMD_IS_APU)
> + return false;
> + return !amdgpu_device_supports_baco(>ddev) &&
> + !amdgpu_device_supports_boco(>ddev);
> +
> }
>
> /**
> --
> 2.34.1
Re: [PATCH 3/3] drm/amd: Don't always set s3 for dGPUs in all sleep modes
On 2/21/2023 07:34, Lazar, Lijo wrote:
On 2/21/2023 6:57 PM, Mario Limonciello wrote:
On 2/21/23 07:25, Lazar, Lijo wrote:
On 2/21/2023 1:46 AM, Mario Limonciello wrote:
dGPUs that will be using BACO or BOCO shouldn't be put into S3
when the system is being put into s2idle.
Cc: Peter Kopec
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 25e902077caf..5c69116bc883 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -1038,8 +1038,13 @@ void amdgpu_acpi_detect(void)
*/
bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev)
{
- return !(adev->flags & AMD_IS_APU) ||
- (pm_suspend_target_state == PM_SUSPEND_MEM);
+ if (pm_suspend_target_state == PM_SUSPEND_MEM)
+ return true;
+ if (adev->flags & AMD_IS_APU)
+ return false;
What is the expected path of APUs which don't support S2idle?
They should staying powered on and not running any suspend code.
Since they don't support BACO or BOCO I expect the call to enter
autosuspend to be a no-op for them.
This was shown to improve power consumption for such cases (a reporter
actually measured it).
To clarify on this - someone tried s2idle on an APU which doesn't
support it (no FW S0ix support/PMC driver support) and the power
consumption is better for the APU. Is it because the peripherals went
idle now, but in earlier path APU prevented S2idle entry altogether?
I double checked and realize I misspoke - it's not that they don't run
any suspend code, but they handle the s0ix flow even even without
underlying hardware support.
https://gitlab.freedesktop.org/agd5f/linux/-/commit/9cdb69924f545fdc3086bc8b085dad8146057141
So the path for them doesn't change in this series.
Thanks,
Lijo
Thanks,
Lijo
+ return !amdgpu_device_supports_baco(>ddev) &&
+ !amdgpu_device_supports_boco(>ddev);
+
}
/**
Re: [PATCH 3/3] drm/amd: Don't always set s3 for dGPUs in all sleep modes
On 2/21/2023 6:57 PM, Mario Limonciello wrote:
On 2/21/23 07:25, Lazar, Lijo wrote:
On 2/21/2023 1:46 AM, Mario Limonciello wrote:
dGPUs that will be using BACO or BOCO shouldn't be put into S3
when the system is being put into s2idle.
Cc: Peter Kopec
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 25e902077caf..5c69116bc883 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -1038,8 +1038,13 @@ void amdgpu_acpi_detect(void)
*/
bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev)
{
- return !(adev->flags & AMD_IS_APU) ||
- (pm_suspend_target_state == PM_SUSPEND_MEM);
+ if (pm_suspend_target_state == PM_SUSPEND_MEM)
+ return true;
+ if (adev->flags & AMD_IS_APU)
+ return false;
What is the expected path of APUs which don't support S2idle?
They should staying powered on and not running any suspend code.
Since they don't support BACO or BOCO I expect the call to enter
autosuspend to be a no-op for them.
This was shown to improve power consumption for such cases (a reporter
actually measured it).
To clarify on this - someone tried s2idle on an APU which doesn't
support it (no FW S0ix support/PMC driver support) and the power
consumption is better for the APU. Is it because the peripherals went
idle now, but in earlier path APU prevented S2idle entry altogether?
Thanks,
Lijo
Thanks,
Lijo
+ return !amdgpu_device_supports_baco(>ddev) &&
+ !amdgpu_device_supports_boco(>ddev);
+
}
/**
Re: [PATCH 3/3] drm/amd: Don't always set s3 for dGPUs in all sleep modes
On 2/21/23 07:25, Lazar, Lijo wrote:
On 2/21/2023 1:46 AM, Mario Limonciello wrote:
dGPUs that will be using BACO or BOCO shouldn't be put into S3
when the system is being put into s2idle.
Cc: Peter Kopec
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 25e902077caf..5c69116bc883 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -1038,8 +1038,13 @@ void amdgpu_acpi_detect(void)
*/
bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev)
{
- return !(adev->flags & AMD_IS_APU) ||
- (pm_suspend_target_state == PM_SUSPEND_MEM);
+ if (pm_suspend_target_state == PM_SUSPEND_MEM)
+ return true;
+ if (adev->flags & AMD_IS_APU)
+ return false;
What is the expected path of APUs which don't support S2idle?
They should staying powered on and not running any suspend code.
Since they don't support BACO or BOCO I expect the call to enter
autosuspend to be a no-op for them.
This was shown to improve power consumption for such cases (a reporter
actually measured it).
Thanks,
Lijo
+ return !amdgpu_device_supports_baco(>ddev) &&
+ !amdgpu_device_supports_boco(>ddev);
+
}
/**
Re: [PATCH 3/3] drm/amd: Don't always set s3 for dGPUs in all sleep modes
On 2/21/2023 1:46 AM, Mario Limonciello wrote:
dGPUs that will be using BACO or BOCO shouldn't be put into S3
when the system is being put into s2idle.
Cc: Peter Kopec
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 25e902077caf..5c69116bc883 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -1038,8 +1038,13 @@ void amdgpu_acpi_detect(void)
*/
bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev)
{
- return !(adev->flags & AMD_IS_APU) ||
- (pm_suspend_target_state == PM_SUSPEND_MEM);
+ if (pm_suspend_target_state == PM_SUSPEND_MEM)
+ return true;
+ if (adev->flags & AMD_IS_APU)
+ return false;
What is the expected path of APUs which don't support S2idle?
Thanks,
Lijo
+ return !amdgpu_device_supports_baco(>ddev) &&
+ !amdgpu_device_supports_boco(>ddev);
+
}
/**
