[PATCH 3/3] drm/amd: Don't always set s3 for dGPUs in all sleep modes

[Limonciello, Mario]

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 <pekopec at redhat.com>
>>>> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
>>>> ---
>>>>   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(&adev->ddev) &&
>>>> +        !amdgpu_device_supports_boco(&adev->ddev);
>>>> +
>>>>   }
>>>>   /**
>>